JSON merge patch may use BinaryData incorrectly

[alzimmermsft]

When using BinaryData with JSON merge patch it may be using it incorrectly. The following code is used:

String variableName = convenientParameterName + "InBinaryData";
javaBlock.line(String.format("JsonMergePatchHelper.get%1$sAccessor().prepareModelForJsonMergePatch(%2$s, true);", convenientParameterTypeName, convenientParameterName));
javaBlock.line(String.format("BinaryData %1$s = BinaryData.fromBytes(%2$s.toBytes());", variableName, expression)); // BinaryData.fromObject() will not fire serialization, use toBytes() to fire serialization
javaBlock.line(String.format("JsonMergePatchHelper.get%1$sAccessor().prepareModelForJsonMergePatch(%2$s, false);", convenientParameterTypeName, convenientParameterName));
return variableName;

https://github.com/Azure/autorest.java/blob/main/javagen/src/main/java/com/azure/autorest/template/ConvenienceMethodTemplateBase.java#L788

We should be using the BinaryData.fromObject directly rather than converting it from Object to byte[] then landing on BinaryData. This may also result in BinaryData being used incorrectly later on as it loses type information and may result it being considered binary in JSON (base64 encoded string) rather than a JSON object.

Edit

After further investigation I've realized it is done this way as BinaryData.fromObject doesn't serialize the object until the BinaryData is queried. And serialization needs to be forced as the patch serialization status is only within this block of code. This should still on use BinaryData.fromObject but the created BinaryData should have something like getLength() called which will force serialization.

[haolingdong-msft]

Yes, this part of code is used to fire serialization before calling the service. We can switch to getLength(), may I know the shortage of currrent way?

[alzimmermsft]

After looking again, I see this case is when the BinaryData is the full payload, which works as this will just be sent over the wire as-is. So, it doesn't run into the issue I mentioned with how the BinaryData is created determining serialization. Even though it works in this case, we should look into using getLength() to force serialization in case this is used as part of a larger request body where it would run into issues with serialization.