search

Azure / autorest.csharp

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

Add encoding context to properties of BinaryData #3390

Open m-nash opened 1 year ago

m-nash commented 1 year ago

Fixes https://github.com/Azure/autorest.csharp/issues/3390

The original use for BinaryData was from swagger when a customer would mark an object as any or any-object. Because these were effectively json placeholders the usage that we expected and documented was that customers should insert ut8 encoded json into the BinaryData when setting and expect this format when they are reading.

We added this documentation to all BinaryData properties to document this expected usage.

With typespec we have decided to also use this type for bytes. The issue is that for bytes we expect the customer to give us raw binary and by default we should encode that with a base64 string in json.

Although customers can call BinaryData binaryData = BinaryData.FromString("\""+[My Base64 string]+"\""); this is neither intuitive nor ideal for them to have to do.

We need to add an encoding context into the InputModelProperty and JsonPropertySerialization so that within the switch we do on type we can do another switch on encoding.

We would change the property xml summary based on this encoding as well as change the serialization / deserialization based on this encoding.

There are 4 use cases we need to handle here which I will detail out later.

For context today all rows follow the same expectation as any.

spec type model type creation type encoding (wire type)
any/unknown BinaryData utf8 json string utf8 json string
bytes BinaryData byte[] base64/base64url
int8[] sbyte[] sbyte[] sbyte[]
uint8[] BinaryData byte[] ?? potentially base64/base64url as well. Biggest question is what do we do with default encoding. Obviously if they use @encode base64 we will do the same as bytes, but if they don't have any encoding should we do the same as byte[]?
m-nash commented 1 year ago

This PR https://github.com/Azure/autorest.csharp/pull/3397 fixed the bytes with default base64 encoding. We still need to handle / add test cases for bytes with a different encoding like base64url as well as int8[] and uint8[]

chunyu3 commented 1 year ago

We need to set 'content-type' property of BinaryData. File issue https://github.com/Azure/autorest.csharp/issues/3731 to track.