Skip to content

Implement IJsonModel of DataFactoryElement<> for AOT compat #50447

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.

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

Merged
merged 20 commits into from
Jun 11, 2025

Conversation

live1206
Copy link
Member

@live1206 live1206 commented Jun 5, 2025

Resolves #50403

@Copilot Copilot AI review requested due to automatic review settings June 5, 2025 08:14
@live1206 live1206 requested a review from a team as a code owner June 5, 2025 08:14
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the IJsonModel interface for DataFactoryElement to improve AOT (ahead-of-time) compatibility and adjust serialization behavior accordingly. Key changes include:

  • Enhancing Utf8JsonWriterExtensions with a generic WriteObjectValue overload and modifying its switch cases to handle IJsonModel.
  • Updating test logic in DataFactoryElementTests to use the new IPersistableModel-based serialization approach and modifying TestModel to implement IJsonModel.
  • Refactoring serialization logic in DataFactoryKeyVaultSecret, DataFactoryElementJsonConverter, and DataFactoryElement.Serialization while introducing a new DataFactoryContext and adding a PackageReference for System.ClientModel.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sdk/core/Azure.Core/src/Shared/Utf8JsonWriterExtensions.cs Updated method signature to include generic overload and added handling for IJsonModel.
sdk/core/Azure.Core.Expressions.DataFactory/tests/DataFactoryElementTests.cs Adjusted serialization calls in tests and updated TestModel to implement IJsonModel.
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryKeyVaultSecret.Serialization.cs Modified serialization logic to use IJsonModel for secret properties.
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryElementJsonConverter.cs Removed original Serialize methods and replaced Write override with a NotImplementedException.
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryElement.cs Converted to a partial class with additional warning suppressions.
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryElement.Serialization.cs Implemented IJsonModel and IPersistableModel for DataFactoryElement.
sdk/core/Azure.Core.Expressions.DataFactory/src/DataFactoryContext.cs Added a new context type for AOT-compatible model reading/writing.
sdk/core/Azure.Core.Expressions.DataFactory/src/Azure.Core.Expressions.DataFactory.csproj Added PackageReference to System.ClientModel.

@live1206 live1206 changed the title Implement IJsonModel of DataFactoryElement for AOT compat Implement IJsonModel of DataFactoryElement<> for AOT compat Jun 5, 2025
Copy link

github-actions bot commented Jun 5, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

Azure.Core.Expressions.DataFactory

jsquire
jsquire previously requested changes Jun 5, 2025
@jsquire jsquire dismissed their stale review June 9, 2025 16:10

Core shared class has been reverted.

@live1206 live1206 merged commit c5478db into Azure:main Jun 11, 2025
17 checks passed
@live1206 live1206 deleted the dfe-jsonmodel branch June 11, 2025 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Azure.Core] Adding IJsonModel to DataFactoryElement for AOT compat
4 participants