Support TypeSpec @header decorator in response

annelo-msft commented 2 years ago

TBD whether we want to use the same approach as in legacy HLC generator. See: https://github.com/microsoft/cadl/blob/main/docs/tutorial.md#headers

chunyu3 commented 2 years ago

https://github.com/microsoft/cadl/blob/0a528079a54ab74de2614cbdd59eb4a907d58e05/docs/tutorial.md#headers

ArcturusZhang commented 2 years ago

A design example in HLC:

The input header will be generated as signature parameter (it is optional if the header is optional, it is required if the header is required). We already have a rule how the parameters are sorted which also applies to this. The output header will be represented by ResponseWithHeader where T is a model created from all the output headers (as its properties)

ArcturusZhang commented 2 years ago

Current design in protocol method (for input headers, or headers as parameters): The following swagger:

    "/Operation/": {
      "put": {
        "operationId": "Operation",
        "parameters": [
          {
            "name": "body",
            "in": "body",
            "schema": {
              "type": "string"
            }
          },
          {
            "name": "first",
            "in": "header",
            "type": "string"
          },
          {
            "name": "second",
            "in": "header",
            "type": "string",
            "required": true
          }
        ],
        "responses": {
          "200": {
            "description": "Received correct format"
          }
        }
      }
    },

Will generate:

        public virtual async Task<Response> OperationAsync(string second, RequestContent content, string first = null, RequestContext context = null)
        {
            Argument.AssertNotNull(second, nameof(second));

            using var scope = ClientDiagnostics.CreateScope("AccessibilityClient.Operation");
            scope.Start();
            try
            {
                using HttpMessage message = CreateOperationRequest(second, content, first, context);
                return await _pipeline.ProcessMessageAsync(message, context).ConfigureAwait(false);
            }
            catch (Exception e)
            {
                scope.Failed(e);
                throw;
            }
        }

Header parameters are generated as method parameters.
Required headers are required parameters. It goes before the body parameter
Optional headers are optional parameters. It goes after the body parameter

Since we have a requirement of the parity with swagger v1.0 DPG, we need to keep this design for input headers.

Protocol methods do not support output headers (headers in responses) currently. HLC also does not support this. Only the RestOperations class supports that, but those are internal. Public methods in HLC will not generate anything for output headers

ArcturusZhang commented 2 years ago

Design proposal (only for the output headers in response. Headers in requests are already supported):

For protocol methods: we do not need to do anything on the API. RawResponse already contains headers. But we could refine the samples/documents on the protocol methods to add the information about headers in response.
For convenience methods: Do something similar with what HLC is doing in its RestOperations For instance, for cadl models:
```
@route("/pets")
namespace Pets {
op read(@path petId: int32, @header ifMatch?: string): {
@header eTag: string;
@body pet: Pet;
};
}
```
We should generate:
```
public class Pet {}
public class PetsReadHeaders
{
public string ETag {get;} // since it shows only in response, it does not need setter
}
public class PetsClient
{
public ResponseWithHeaders<Pet, PetsGetHeaders> Get(string petId, string? ifMatch = null)
{
}
}
```
Description: gather all the header properties in one operation, creating a model consisting of all the response headers (naming pattern could be {ClientName}{opName}Headers, and use the return type ResponseWithHeaders<TBody, THeader> or ResponseWithHeaders<THeader> if the operation does not return a body. Requirement for emitter: for each operation, the emitter needs to emit the body type, and a header type encapsulating all headers. Or the emitter could emit the body type, and all the headers, the generator could try to wrap them into a "headers" type.

lirenhe commented 2 years ago

We need to update the dotnet guide on how to handle headers in the response

ArcturusZhang commented 2 years ago

Well according to some investigation I did for HLC, HLC now just ignores the headers. The corresponding rest operation is returning a ResponseWithHeaders<TBody, THeaders>: https://github.com/Azure/autorest.csharp/blob/6d12091707a2ee7b0a2449a01ab4d069c7d1d938/test/TestProjects/ExtensionClientName/Generated/AutoRestParameterFlatteningRestClient.cs#L87 But in the corresponding client method, HLC client is just returning that while the result type of the method is Response<TBody> https://github.com/Azure/autorest.csharp/blob/6d12091707a2ee7b0a2449a01ab4d069c7d1d938/test/TestProjects/ExtensionClientName/Generated/AutoRestParameterFlatteningClient.cs#L51 Because the type ResponseWithHeaders<TBody, THeaders> inherits from Response<TBody>, during the cast, the headers are just missing and ignored. HLC never has a process "copying the headers into the header properties into the response body and assign them with actual values" We really need the dotnet guide update for the headers in response.

chunyu3 commented 1 year ago

Customer can get the header via response.rawResponse, so it is not a block. And DPG may not support this. (need confirm) Move it to backlog now.

Azure / autorest.csharp

Support TypeSpec @header decorator in response #2493