Skip to content

Need to update calls to JsonSerializer when dealing with an IJsonModel #5330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
m-nash opened this issue May 27, 2025 · 1 comment · May be fixed by #5335
Open

Need to update calls to JsonSerializer when dealing with an IJsonModel #5330

m-nash opened this issue May 27, 2025 · 1 comment · May be fixed by #5335
Assignees
@live1206
Labels
v3 Version 3 of AutoRest C# generator.

Comments

@m-nash
Copy link
Member

m-nash commented May 27, 2025 •
edited
Loading

Related to Azure/azure-sdk-for-net#50279 and Azure/azure-sdk-for-net#50129

Today whenever a type is cross library boundary we fall back to JsonSerializer and expect the other library to have implemented a JsonConverter to support this. Before MRW this was our only way to support public serialization / deserialization.

With MRW we can do things slightly differently.

All calls to JsonSerializer.Serializer where the target is an IJsonModel should be updated to

((IJsonModel<T>)Property).Write(writer, options);

For calls to JsonSerializer.Deserializer where the target is an IJsonModel we will need to add a new helper in ModelSerializationExtension.

//ModelSerializationExtension

private readonly static JsonSerializerOptions s_options = new()
{
  Converters =
  {
    new JsonModelConverter(ModelSerializationExtensions.WireOptions, AzureResourceManagerCosmosDBContext.Default)
  }
};

[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "By passing in the JsonSerializerOptions with a reference to AzureResourceManagerCosmosDBContext.Default we are certain there is no AOT compat issue.")]
[UnconditionalSuppressMessage("Trimming", "IL3050", Justification = "By passing in the JsonSerializerOptions with a reference to AzureResourceManagerCosmosDBContext.Default we are certain there is no AOT compat issue.")]
public static T JsonDeserialize<T>(JsonProperty property, ModelReaderWriterOptions options)
{
  return JsonSerializer.Deserialize<T>(property.Value.GetRawText(), s_options);
}

With this new internal helper in place we can change the calls to

JsonDeserialize<T>(property, options);

This will ensure AOT compatibility for cross library boundary serialization and deserialization.
@m-nash m-nash added the v3 Version 3 of AutoRest C# generator. label May 27, 2025
@m-nash
Copy link
Member Author

m-nash commented May 28, 2025

check aot compatibility

..\..\..\eng\scripts\compatibility\Check-AOT-Compatibility.ps1 -ServiceDirectory appconfiguration -PackageName Azure.Data.AppConfiguration -ExpectedWarningsFilePath None

@live1206 live1206 self-assigned this May 28, 2025
@live1206 live1206 linked a pull request May 28, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@live1206 live1206

Labels
v3 Version 3 of AutoRest C# generator.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace JsonSerializer with IJsonModel live1206/autorest.csharp
2 participants
@live1206 @m-nash