Add model serializer example by m-nash · Pull Request #36418 · Azure/azure-sdk-for-net

m-nash · 2023-05-22T19:25:51Z

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.

sdk/core/Azure.Core/api/Azure.Core.net461.cs

annelo-msft · 2023-05-22T20:05:53Z

sdk/core/Azure.Core/samples/Serialization.md

 string json = JsonSerializer.Serialize(dog);
+
+//modelSerializer example
+Stream stream = ModelSerializer.Serialize(dog);


It would be helpful to document how these are different and when a caller would chose one approach vs another.

There will be a few overloads we end up with here it one by one or the other. For demo purposes we are only showing one to minimize throw away code

annelo-msft · 2023-05-22T20:08:03Z

sdk/core/Azure.Core/api/Azure.Core.net461.cs

+    public static partial class ModelSerializer
+    {
+        public static T Deserialize<T>(System.IO.Stream stream, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }
+        public static T Deserialize<T>(string json, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }


Would this eventually support the full set of types JsonSerializer supports? ReadOnlySpan<byte> could be useful for high-performance scenarios.

Would this also be the way we would convert DynamicData to model types and vice versa?

Yeah the overloads will definitely get filled out if we decide to go this way, only implementing some basic scenarios for now to minimize throw away.

annelo-msft · 2023-05-22T20:08:51Z

sdk/core/Azure.Core/api/Azure.Core.net461.cs

+    {
+        public static T Deserialize<T>(System.IO.Stream stream, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }
+        public static T Deserialize<T>(string json, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }
+        public static System.IO.Stream Serialize<T>(T model, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }


Would SerializableOptions be a superset of what's available in the STJ JsonSerializer?

Would it change to ModelSerializerOptions? That could be the answer for cleanly partitioning model serialization options from more general content serialization options.

Not exactly because we don't want people to be able to pass in their own property mapping settings since that defeats the purpose of not needing to know how to convert a model to azure wire format.

Yeah, conceptually, it feels like in a way ModelSerializerOptions is a subtype of SerializerOptions where a lot of the options have been fixed by the model. So maybe it's not a subtype so much as an "instance of", where a few options are left un-fixed for the user to specify if needed.

I do think renaming it to ModelSerializerOptions to map it directly to ModelSerializer would be a nice way to communicate this to users. That leaves space for a SerializerOptions with more of the options available for callers to designate in more general non-model cases.

annelo-msft · 2023-05-22T20:13:47Z

sdk/core/Azure.Core/api/Azure.Core.net461.cs

+    {
+        public static T Deserialize<T>(System.IO.Stream stream, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }
+        public static T Deserialize<T>(string json, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }
+        public static System.IO.Stream Serialize<T>(T model, Azure.SerializableOptions? options = null) where T : Azure.IJsonSerializable, new() { throw null; }


It might be nice to have the same default return type as the STJ serializer - they use overloads to populate stream, e.g. https://learn.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializer.serialize?view=net-7.0#system-text-json-jsonserializer-serialize(system-io-stream-system-object-system-type-system-text-json-serialization-jsonserializercontext)

azure-sdk · 2023-05-22T20:35:52Z

API change check

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

Azure.Core

* Update serializer (#35735) * wip * wip * wip * wip * wip * Update serializer (#35741) * [Azure.Core] add List property tests and update ReadOnly Test (#35904) * [Azure.Core] Update SerializableOptions.cs (#35947) * [Azure.Core] add test helper for Models (#35968) * [Azure.Core] Update IJsonSerializable with non Try methods (#36164) * Add scaffolding for readme (#36342) * add scaffolding for readme * fix type on class name * one more typo :-( * make tests sync * Add use cases for explicit casts (#36391) * add use cases for explicit casts * remove extra empty line * add custom expected value for net462 * [Azure.Core] Update Serializer (#36376) * Add use cases for stj (#36399) * add use cases for stj * address feedback * add some clarification to readme * [Azure.Core] Add public static deserializer (#36402) * wip * wip * wip * Update StaticDeserializerTests.cs * Update StaticDeserializerTests.cs * Update StaticDeserializerTests.cs * Add model serializer example (#36418) * 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 * [Azure.Core] Add NewtonSoft Json example (#36453) * Update SerializableOptions.cs * wip * wip * update * wip * wip * add generic modeljsonconverter (#36854) * [Azure.Core] Add Newton soft BYOM case (#36794) * wip * Update ModelSerializer.cs * wip * Update ReadOnlyPropertyTests.cs * wip * Update SerializableOptions.cs * Update SerializableOptions.cs * wip * wip * Update SerializationSamples.cs * regen api after merge * Demo of what it would look like for public surface (#36857) * demo of what it would look like for public surface * wip * update tests and more renames * name tweaks * update api * add nullable * adjust public api surface and move test cases to public project to avoid internalsvisibleto * integrate feature/updateSerializer * rename IModel to IModelSerializable since it now has the serialization methods on it * share deserialization code between converter and modelserializer * [Azure.Core] Add XmlSerializer (#37155) * wip * wip * Update Serialization.md * Update ModelXmlTests.cs * wip * wip * update combined interface example --------- Co-authored-by: m-nash <prognash@gmail.com> * add examples for combined interface (#37245) * regen api after merge * Update to v1 api surface (#37276) * update to v1 api surface * update api after merge * wip (#37351) * wip (#37544) * [Azure.Core] Update options to use Format string (#37467) * wip * wip * Update UsingJsonSerializerTests.cs * wip * Update Serialization.md * wip * wip * wip * wip * wip * wip * wip * wip * Update ModelSerializerOptions.cs * wip * Update ModelSerializerOptions.cs * Update ModelSerializerOptions.cs * Update format to have J and W and implement json always on xml models (#37815) * update format to have J and W and implement json always on xml models * update api * Perf checks (#37349) * wip * update benchmark to use a real model from compute * update api * add multi buffer option and benchmarks for larger object * minor fixes on perf * perf updates and split the xml / json interfaces so we can take Utf8JsonWriter * add delayed RequestContent to improve implicit cast perf * fixes after merge * add utf8jsonreader deserialize * update for the rest of the test models with jsonreader * update api * fix typo * upload bimodal repro * return values from benchmarks * updates after merge * update modelserializeroptions to a struct * wip * update to split request content and sequence writer * update api * remove static azure default options since new is zero alloc * update api * add non generic overloads in modelserializer * Revert "update api" This reverts commit 34c0f6b. * Revert "remove static azure default options since new is zero alloc" This reverts commit 5f7bcfa. * wip * updates after merge * update api * add memory diag by default for all benchmarks * use fixed settings to run benchmarks faster * add package needed to use ETW profiler in benchmark * change dictionary to resolver factory * update options back to class * update to make interface generic * remove two way implicit * add XElement api on public interface * update api * swap to struct for options * remove unused shared source * Hide get sequence until we deal with use-after-free (#37893) * hide getsequence until we deal with use-after-free * update api * remove broken inheritdocs * Testing public test project failure in ci (#37894) * testing public test project failure in ci * mark the project as test project * add public test to split list * set support project to false to run tests in ci * add failing case after merge * remove failing test * Address api review comments and add more xml docs (#37916) * address api review feedback * switch options to class and add a fozen static reference * update api * rename WriteTo -> CopyTo * update snippets * update request content create to take an imodelserializable (#37939) * Mnash request conent i model serializable (#37941) * update request content create to take an imodelserializable * make sequenceWriter internal * update reflection case for modelserializer (#37945) * Address archboard feedback (#37949) * move convert to binary data onto modelserializer * validate error scenarios for modelserializer * rename default wire options * add overload to converter ctor * update for format overloads and docs * update format docs * add tests for more validation around multiple types in a single json serializer instance * Remove xml interface (#37967) * remove IModelXmlSerializable * update api * Address feedback from #35946 (#37971) * remove IModelXmlSerializable * update api * address initial feedback * remove reference to utf8jsonreader helpers * update debug assert * Add some more testing around SequenceWriter (#37973) * add some tests * more tests * wip * add benchmarks for RequestContent * update request content to use sequence writer access class * add write lock to serialization * remove empty file * harden flaky test * add singletons for known format types (#38007) * Move SequenceWriter to private class (#38013) * add additional feedback on sequenceWriterAccess * move inner private class to another file * delete Readme and associated files (#38064) * Add tests for model serialization (#38042) * re-organize common test code and add initial tests for ModelWriter * missed files * update perf proj to use common testdata * group model serializer tests * update api * wip * separate read lock * make cast succeed on null (#38105) * Make ModelSerializer compatible with AOT (#38010) * add new attribute to deterministically find the default subclass * update test since message changed * address comments * add aot attributes up the call chain * remove un-needed attribute * add unreferenced code attribute to ModelJsonConverter * remove attribute from jsonconverter overload * conditionally add the unreferencedcode attribute to modeljsonconverter class * address comments * rename unknown subclass attribute * Update sdk/core/Azure.Core/src/Serialization/IModelJsonSerializable.cs Co-authored-by: Anne Thompson <annelo@microsoft.com> * Update sdk/core/Azure.Core/src/Serialization/IModelSerializable.cs Co-authored-by: Anne Thompson <annelo@microsoft.com> * Update sdk/core/Azure.Core/src/Serialization/IModelSerializable.cs Co-authored-by: Anne Thompson <annelo@microsoft.com> * address exception feedback (#38120) * update helper so its non boxing and add struct test cases (#38125) * Address some final feedback (#38130) * hide model writer * rename abstract hierarchy deserializer * update wire format docs * Minor cleanup (#38137) * address feedback * add checks around stream with long length * change from notsupported to format exception (#38147) * address feedback (#38150) * update changelog --------- Co-authored-by: m-nash <64171366+m-nash@users.noreply.github.com> Co-authored-by: m-nash <prognash@gmail.com> Co-authored-by: Anne Thompson <annelo@microsoft.com>

m-nash added 5 commits May 19, 2023 14:41

add use cases for stj

f258de7

address feedback

9af2788

add some clarification to readme

f83d43b

add model serializer example

b33e562

merge feature/updateSerializer

aac711b

m-nash requested review from JoshLove-msft, KrzysztofCwalina, annelo-msft, christothes and tg-msft as code owners May 22, 2023 19:25

github-actions bot added the Azure.Core label May 22, 2023

nisha-bhatia approved these changes May 22, 2023

View reviewed changes

annelo-msft reviewed May 22, 2023

View reviewed changes

sdk/core/Azure.Core/api/Azure.Core.net461.cs Outdated Show resolved Hide resolved

annelo-msft reviewed May 22, 2023

View reviewed changes

move serialization classes into azure.core.serialization

87caa93

update after namespace move

5f6a213

m-nash merged commit 03d2c71 into feature/updateSerializer May 22, 2023

m-nash deleted the mnash-modelSerializerExample branch May 22, 2023 21:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add model serializer example#36418

Add model serializer example#36418
m-nash merged 7 commits intofeature/updateSerializerfrom
mnash-modelSerializerExample

m-nash commented May 22, 2023

Uh oh!

Uh oh!

annelo-msft May 22, 2023 •

edited

Loading

Uh oh!

m-nash May 22, 2023

Uh oh!

annelo-msft May 22, 2023

Uh oh!

m-nash May 22, 2023

Uh oh!

annelo-msft May 22, 2023

Uh oh!

annelo-msft May 22, 2023

Uh oh!

m-nash May 22, 2023

Uh oh!

annelo-msft May 22, 2023

Uh oh!

annelo-msft May 22, 2023

Uh oh!

azure-sdk commented May 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

m-nash commented May 22, 2023

Contributing to the Azure SDK

Uh oh!

Uh oh!

annelo-msft May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azure-sdk commented May 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

annelo-msft May 22, 2023 •

edited

Loading