-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Update format to have J and W and implement json always on xml models #37815
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,32 +12,28 @@ namespace Azure.Core.Serialization | |
| public class ModelSerializerOptions | ||
| { | ||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="ModelSerializerOptions" /> class. Takes in a string that determines Format of serialized model. "D" = data format which means IgnoreReadOnly and IgnoreAdditionalProperties are false, "W" = wire format which means both properties are true. Default is "D". | ||
| /// Initializes a new instance of the <see cref="ModelSerializerOptions" /> class using a default for of <see cref="ModelSerializerFormat.Json"/>. | ||
| /// </summary> | ||
| /// <param name="format"></param> | ||
| public ModelSerializerOptions(string format = "D") | ||
| public ModelSerializerOptions() | ||
| : this(ModelSerializerFormat.JsonValue) { } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="ModelSerializerOptions" /> class. | ||
| /// </summary> | ||
| /// <param name="format"> The format to serialize to and deserialize from. </param> | ||
| public ModelSerializerOptions(ModelSerializerFormat format) | ||
| { | ||
| //throw ArgumentException if not "D" or "W" | ||
| Format = ValidateFormat(format); | ||
| Format = format; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It still would be possible to pass a random value for format here - should we leave the validation in place?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. random value is by design so people who implement our interface can come up with their own formats. Each model will throw if it receives a format it doesn't understand. |
||
| } | ||
|
|
||
| /// <summary> | ||
| /// ModelSerializerFormat that determines Format of serialized model. "D" = data format which means IgnoreReadOnly and IgnoreAdditionalProperties are false, "W" = wire format which means both properties are true. Default is "D". | ||
| /// Gets the <see cref="ModelSerializerFormat"/> that determines Format of serialized model. | ||
| /// </summary> | ||
| public ModelSerializerFormat Format; | ||
| public ModelSerializerFormat Format { get; } | ||
|
|
||
| /// <summary> | ||
| /// Dictionary that holds all the serializers for the different model types. | ||
| /// </summary> | ||
| public Dictionary<Type, ObjectSerializer> Serializers { get; } = new Dictionary<Type, ObjectSerializer>(); | ||
|
|
||
| private string ValidateFormat(string x) | ||
| { | ||
| if (x != ModelSerializerFormat.Data && x != ModelSerializerFormat.Wire) | ||
| { | ||
| throw new ArgumentException($"Format must be either '{ModelSerializerFormat.Data}' or '{ModelSerializerFormat.Wire}'."); | ||
| } | ||
| return x; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the doc string here make sense to most users of this API? Reading it without additional context, I'm not exactly sure what it means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are address doc comments in a different PR, but yes these are not final