Make MutableJsonDocument internal shared source #38159
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using Azure.Core.Json; | ||
|
|
||
| namespace Azure.Core.Tests.PatchModels | ||
| { | ||
| /// <summary> | ||
| /// This model illustrates a nested child model in a parent model. | ||
| /// </summary> | ||
| public partial class ChildPatchModel | ||
| { | ||
| #pragma warning disable AZC0020 // Avoid using banned types in libraries | ||
| private readonly MutableJsonElement _element; | ||
|
Member
There was a problem hiding this comment. If a model is output and roundtrip only, I think storing all the data in MJE is fine. But what will we do for models that are also input models? Should these have two modes: one where data is stored in regular fields (as we do today) and one where the data is stored in MJE?
Contributor
Author
There was a problem hiding this comment. I'll see if I can add some benchmarks to see how favorable that approach is.
Contributor
Author
There was a problem hiding this comment. Per annelo-msft#9, MJD serialization is comparable to standard model serialization, except when we modify the change list, since it's essentially a pass-through to JsonDocument when there are no changes. I'll spend some time looking at whether we can optimize the case where we access the change list. |
||
|
|
||
| // Note: A child patch model doesn't have a public constructor. | ||
|
Member
There was a problem hiding this comment. We have public constructor currently. Is there any reason we change this?
Contributor
Author
There was a problem hiding this comment. My thinking is that, for Patch models, we need a way to connect the MutableJsonElement in the child model to the MutableJsonElement in the parent model, so that they point to the same root MutableJsonDocument. Without this, they don't access the same changelist and writing the Patch JSON would be a lot more complicated. We could make it so that nested models have a public constructor but can't be set on the parent (shown here), but if you can't set the child model, then I don't know why you would have a public constructor on the child model - callers could create an instance of it but then not do anything with it, which seems like it could be frustrating to users. There may be times when a nested model is also an input model on a service method - when that happens the model will need to have a public constructor, and we will need to make the parent model not settable so we can connect the MutableJsonElements. It would be ideal if the generator could detect that case and generate the model with the public constructor when that happens. |
||
| // | ||
| // When a nested model is also an input to a service method, it will | ||
| // need to have a public constructor. When this happens, the parent | ||
| // model it is also part of will not allow it to be set, to ensure the | ||
| // MutableJsonElements point to the same root MutableJsonDocument when | ||
| // they are part of the same input, so the Patch JSON will be correct. | ||
|
|
||
| /// <summary> Serialization constructor. </summary> | ||
| /// <param name="element"></param> | ||
| internal ChildPatchModel(MutableJsonElement element) | ||
| { | ||
| _element = element; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Optional string property corresponding to JSON """{"a": "aaa"}""". | ||
| /// </summary> | ||
| public string A | ||
| { | ||
| get | ||
| { | ||
| if (_element.TryGetProperty("a", out MutableJsonElement value)) | ||
| { | ||
| return value.GetString(); | ||
| } | ||
| return null; | ||
| } | ||
| set => _element.SetProperty("a", value); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Optional string property corresponding to JSON """{"b": "bbb"}""". | ||
| /// </summary> | ||
| public string B | ||
| { | ||
| get | ||
| { | ||
| if (_element.TryGetProperty("b", out MutableJsonElement value)) | ||
| { | ||
| return value.GetString(); | ||
| } | ||
| return null; | ||
| } | ||
| set => _element.SetProperty("b", value); | ||
| } | ||
| #pragma warning restore AZC0020 // Avoid using banned types in libraries | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Text.Json; | ||
| using Azure.Core.Json; | ||
| using Azure.Core.Serialization; | ||
|
|
||
| namespace Azure.Core.Tests.PatchModels | ||
| { | ||
| public partial class ParentPatchModel : IModelJsonSerializable<ParentPatchModel>, IUtf8JsonSerializable | ||
| { | ||
| ParentPatchModel IModelJsonSerializable<ParentPatchModel>.Deserialize(ref Utf8JsonReader reader, ModelSerializerOptions options) | ||
| { | ||
| PatchModelHelper.ValidateFormat(this, options.Format); | ||
|
|
||
| return Deserialize(ref reader, options); | ||
| } | ||
|
|
||
| private static ParentPatchModel Deserialize(ref Utf8JsonReader reader, ModelSerializerOptions options) | ||
| { | ||
| MutableJsonDocument mdoc = MutableJsonDocument.Parse(ref reader); | ||
| return new ParentPatchModel(mdoc.RootElement); | ||
| } | ||
|
|
||
| ParentPatchModel IModelSerializable<ParentPatchModel>.Deserialize(BinaryData data, ModelSerializerOptions options) | ||
| { | ||
| PatchModelHelper.ValidateFormat(this, options.Format); | ||
|
|
||
| return Deserialize(data, options); | ||
| } | ||
|
|
||
| private static ParentPatchModel Deserialize(BinaryData data, ModelSerializerOptions options) | ||
| { | ||
| MutableJsonDocument mdoc = MutableJsonDocument.Parse(data); | ||
| return new ParentPatchModel(mdoc.RootElement); | ||
| } | ||
|
|
||
| void IModelJsonSerializable<ParentPatchModel>.Serialize(Utf8JsonWriter writer, ModelSerializerOptions options) | ||
| { | ||
| PatchModelHelper.ValidateFormat(this, options.Format); | ||
|
|
||
| switch (options.Format.ToString()) | ||
| { | ||
| case "J": | ||
| case "W": | ||
| _element.WriteTo(writer, "J"); | ||
| break; | ||
| case "P": | ||
| _element.WriteTo(writer, "P"); | ||
| break; | ||
| default: | ||
| // Exception was thrown by ValidateFormat. | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| BinaryData IModelSerializable<ParentPatchModel>.Serialize(ModelSerializerOptions options) | ||
| { | ||
| PatchModelHelper.ValidateFormat(this, options.Format); | ||
|
|
||
| return ModelSerializer.SerializeCore(this, options); | ||
| } | ||
|
|
||
| public static implicit operator RequestContent(ParentPatchModel model) | ||
| => RequestContent.Create(model, ModelSerializerOptions.DefaultWireOptions); | ||
|
|
||
| public static explicit operator ParentPatchModel(Response response) | ||
| { | ||
| Argument.AssertNotNull(response, nameof(response)); | ||
|
|
||
| return Deserialize(response.Content, ModelSerializerOptions.DefaultWireOptions); | ||
| } | ||
|
|
||
| void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => ((IModelJsonSerializable<SimplePatchModel>)this).Serialize(writer, ModelSerializerOptions.DefaultWireOptions); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using Azure.Core.Json; | ||
|
|
||
| namespace Azure.Core.Tests.PatchModels | ||
| { | ||
| /// <summary> | ||
| /// This model illustrates a patch model with properties that are nested models. | ||
| /// </summary> | ||
| public partial class ParentPatchModel | ||
| { | ||
| #pragma warning disable AZC0020 // Avoid using banned types in libraries | ||
| private readonly MutableJsonElement _element; | ||
|
|
||
| /// <summary> | ||
| /// Public constructor. | ||
| /// </summary> | ||
| public ParentPatchModel() | ||
| { | ||
| _element = MutableJsonDocument.Parse(MutableJsonDocument.EmptyJson).RootElement; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Serialization constructor. | ||
| /// </summary> | ||
| /// <param name="element"></param> | ||
| internal ParentPatchModel(MutableJsonElement element) | ||
| { | ||
| _element = element; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Optional string property corresponding to JSON """{"id": "abc"}""". | ||
| /// </summary> | ||
| public string Id | ||
|
annelo-msft marked this conversation as resolved.
|
||
| { | ||
| get | ||
| { | ||
| if (_element.TryGetProperty("id", out MutableJsonElement value)) | ||
| { | ||
| return value.GetString(); | ||
| } | ||
| return null; | ||
| } | ||
| set => _element.SetProperty("id", value); | ||
| } | ||
|
|
||
| private ChildPatchModel _child; | ||
| /// <summary> | ||
| /// Optional ChildPatchModel property corresponding to JSON """{"child": {"a":"aa", "b": "bb"}}""". | ||
| /// </summary> | ||
| public ChildPatchModel Child | ||
| { | ||
| get | ||
| { | ||
| if (_child == null) | ||
| { | ||
| if (!_element.TryGetProperty("child", out MutableJsonElement element)) | ||
| { | ||
| _element.SetProperty("child", new { }); | ||
| element = _element.GetProperty("child"); | ||
| } | ||
|
|
||
| _child = new ChildPatchModel(element); | ||
| } | ||
|
|
||
| return _child; | ||
| } | ||
|
|
||
| // Note: a child patch model isn't settable on the parent. | ||
| // This is because its _element property needs to have the | ||
| // same root MutableJsonDocument as the parent. | ||
| // | ||
| // It's unclear how we would plug it in after the fact if we wanted | ||
| // to make an instance of the child model something that could be | ||
| // used in multiple places. | ||
| } | ||
| #pragma warning restore AZC0020 // Avoid using banned types in libraries | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.