Skip to content
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

[Azure.Core] Add Public Model Serialization #35946

Merged
merged 60 commits into from
Aug 11, 2023
Merged

Conversation

nisha-bhatia
Copy link
Member

@nisha-bhatia nisha-bhatia commented May 2, 2023

Resolves #16977
Previous draft PR: #35742
The goal of this PR is to provide a public interface that exposes the currently internal serialization code so customers can access it without needing to use reflection or write their own translation. This interface would be applied to all models in all of our libraries today. The implementation simply uses the existing internal serialization logic since that code already knows all of the property name changes / structure changes that exist. We can at a future date change the underlying implementation as long as we maintain functional compatibility.

The SerializationOptions class introduces a couple concepts that are common across the other languages serialization implementations which are:

IncludeReadonlyProperties: This flag tells the serializer to include the readonly properties when making the payload. This is always off during client calls since the service would error out if we tried to change a readonly property, however if a customer wants to save the state of the model this can be included by setting this property to true.
HandleAdditionalProperties: This flag tells the serializer to keep any properties it didn't recognize during deserialization as an internal dictionary. If the same model instance is then serialized with the same property set to true these values will be echoed and not lost. This is useful for dealing with a large enterprise where various micro-services could be on different versions of the azure sdk library.
We should consider what defaults we want for the above properties. I would imagine most customers would want true for both in most cases and we can simply have false for both in our client invocations.

Currently the interface only takes in Stream as the input and output object we should consider other convenience overloads such as string.

We should also consider explicit / implict cast operators Model -> RequestContent and Response -> Model.

Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

@nisha-bhatia nisha-bhatia self-assigned this May 2, 2023
@nisha-bhatia nisha-bhatia requested review from m-nash and tg-msft May 2, 2023 21:04
@nisha-bhatia
Copy link
Member Author

Re-creating this PR as we had to rename the branch

@nisha-bhatia
Copy link
Member Author

#35742

@azure-sdk
Copy link
Collaborator

API change check

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

Azure.Core

nisha-bhatia and others added 9 commits May 8, 2023 14:10
* add scaffolding for readme

* fix type on class name

* one more typo :-(

* make tests sync
* add use cases for explicit casts

* remove extra empty line

* add custom expected value for net462
* add use cases for stj

* address feedback

* add some clarification to readme
* wip

* wip

* wip

* Update StaticDeserializerTests.cs

* Update StaticDeserializerTests.cs

* Update StaticDeserializerTests.cs
* add use cases for stj

* address feedback

* add some clarification to readme

* add model serializer example

* move serialization classes into azure.core.serialization

* update after namespace move
nisha-bhatia and others added 4 commits May 30, 2023 11:17
* Update SerializableOptions.cs

* wip

* wip

* update

* wip

* wip
* wip

* Update ModelSerializer.cs

* wip

* Update ReadOnlyPropertyTests.cs

* wip

* Update SerializableOptions.cs

* Update SerializableOptions.cs

* wip

* wip

* Update SerializationSamples.cs
* hide model writer

* rename abstract hierarchy deserializer

* update wire format docs
* address feedback

* add checks around stream with long length
@m-nash m-nash mentioned this pull request Aug 10, 2023
@m-nash m-nash mentioned this pull request Aug 11, 2023
Copy link
Member

@annelo-msft annelo-msft left a comment

Choose a reason for hiding this comment

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

Would you please add an entry to the Azure.Core CHANGELOG before merging?

@m-nash m-nash enabled auto-merge (squash) August 11, 2023 15:41
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.

Make model types deserializable