Azure / autorest.csharp

Extension for AutoRest (https://github.com/Azure/autorest) that generates C# code
MIT License
142 stars 166 forks source link

Serialization/Deserialization with different media types #3932

Open pshao25 opened 11 months ago

pshao25 commented 11 months ago

Given a content type/accept, we need some logic like https://github.com/Azure/autorest.csharp/issues/3056#issuecomment-1429403557 to category the content type/accept into one of Binary, Form, Json, Multipart, Text, Xml. I think this logic should be at somewhere in TCGC.

In this issue, we just discuss Binary, Json, Text when there is only one content type/accept, with body type model, bytes, primitive type.

Media type: Json, body type: model

Media type: Json, body type: primitive type (string, int, double, etc)

Actual: RequestContentHelper.FromObject(body); Expected: payload is expected, but there is boxing operation, we will optimize at https://github.com/Azure/autorest.csharp/issues/3915

Actual: response.Content.ToObjectFromJson<string>()

Media type: Json, body type: bytes

Actual: RequestContentHelper.FromObject(body); Expected: same as above with boxing operation

Actual: response.Content Expected: response.Content.ToObjectFromJson<BinaryData>()

Media type: Binary, body type: model

Media type: Binary, body type: primitive type (string, int, double, etc)

Actual: RequestContentHelper.FromObject(body); Expected: BinaryData.FromString(body) (only applicable to string)

Actual: response.Content.ToObjectFromJson<string>() Expected: response.Content

Media type: Binary, body type: bytes

Customer issue: https://github.com/Azure/autorest.csharp/issues/3793 Actual: RequestContentHelper.FromObject(body); Expected: body

Customer issue: https://github.com/Azure/autorest.csharp/issues/3963 Actual: response.Content

Media type: Text, body type: model

Media type: Text, body type: primitive type (string, int, double, etc)

Actual: RequestContentHelper.FromObject(body) Expected: RequestContent.Create(body)

Customer issue: https://github.com/Azure/autorest.csharp/issues/3816 Actual: response.Content.ToObjectFromJson<string>() Expected: response.Content.ToString() (only applicable to string)

Media type: Text, body type: bytes

Actual: RequestContentHelper.FromObject(body) Expected: ??

Actual: response.Content Expected: ??

Multiple content types

Case 1: content types come to same media type

op oneBinaryBodyTwoContentTypes(@body body: bytes, @header contentType: "application/octet-stream" | "image/jpeg"): void;

Both of the content type "application/octet-stream" and "image/jpeg" go to the media type Byte. So the conversion of body is the same as single content type. (but a new parameter content type will be added to method)

Case 2: content types come to different media types

op oneBinaryBodyTwoContentTypes(@body body: bytes, @header contentType: "application/octet-stream" | "application/json"): void;

Solution 1:

public virtual async Task<Response> OneBinaryBodyTwoContentTypesAsync(BinaryData body, ContentType contentType, CancellationToken cancellationToken = default)
{
   Argument.AssertNotNull(body, nameof(body));

  RequestContext context = FromCancellationToken(cancellationToken);
  Response response = null;
  if (contentType == ContentType.ApplicationJson)
  {
      using RequestContent content = RequestContentHelper.FromObject(body);
      response = await OneBinaryBodyTwoContentTypesAsync(content, contentType, context).ConfigureAwait(false);
  }
  else if (contentType == ContentType.ApplicationOctetStream)
  {
      using RequestContent content = body;
      response = await OneBinaryBodyTwoContentTypesAsync(content, contentType, context).ConfigureAwait(false);
  }
  return response;
}

Solution 2:

public virtual async Task<Response> OneBinaryBodyTwoContentTypesAsync(BinaryData body, ContentType contentType, CancellationToken cancellationToken = default)
{
    Argument.AssertNotNull(body, nameof(body));

    RequestContext context = FromCancellationToken(cancellationToken);
    using RequestContent content = body.ToRequestContent(contentType);
    Response response = await OneBinaryBodyTwoContentTypesAsync(content, contentType, context).ConfigureAwait(false);
    return response;
}
public static RequestContent ToRequestContent(this BinaryData binaryData, ContentType? contentType = null)
{
    if (contentType == null || contentType == ContentType.ApplicationJson)
    {
        return RequestContentHelper.FromObject(binaryData);
    }
    else if (contentType == ContentType.ApplicationOctetStream)
    {
        return binaryData;
    }

    throw new Exception();
}

Solution 3:

public virtual async Task<Response> OneBinaryBodyTwoContentTypesAsync(BinaryData body, ContentType contentType, CancellationToken cancellationToken = default)
{
    Argument.AssertNotNull(body, nameof(body));

    RequestContext context = FromCancellationToken(cancellationToken);
    RequestContent content = null;
    if (contentType == ContentType.ApplicationJson)
    {
        content = RequestContentHelper.FromObject(body);
    }
    else if (contentType == ContentType.ApplicationOctetStream)
    {
        content = body;
    }
    using (content)
    {
        return await OneBinaryBodyTwoContentTypesAsync(content, contentType, context).ConfigureAwait(false);
    }
}

Solution 4:

public virtual async Task<Response> OneBinaryBodyTwoContentTypesAsync(BinaryData body, ContentType contentType, CancellationToken cancellationToken = default)
{
    Argument.AssertNotNull(body, nameof(body));

    RequestContext context = FromCancellationToken(cancellationToken);
    RequestContent content = null;
    try
    {
        if (contentType == ContentType.ApplicationJson)
        {
            content = RequestContentHelper.FromObject(body);
        }
        else if (contentType == ContentType.ApplicationOctetStream)
        {
            content = body;
        }

        return await OneBinaryBodyTwoContentTypesAsync(content, contentType, context).ConfigureAwait(false);
    }
    finally
    {
        content.Dispose();
    }
}
JoshLove-msft commented 11 months ago

How will this work once azure-sdk-for-net starts consuming the new version of BinaryData with MediaType property?

pshao25 commented 10 months ago

How will this work once azure-sdk-for-net starts consuming the new version of BinaryData with MediaType property?

@JoshLove-msft Thank you for the information! Could you help me understand how this new property is expected to be used? What is the expected value for this property?

chunyu3 commented 10 months ago

how to model bytes: https://github.com/microsoft/typespec/issues/2703