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.

Sign up for GitHub

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

WIP on new format changes #39817

Merged
merged 7 commits into from
Nov 9, 2023
Merged

WIP on new format changes #39817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3c782fe
wip on new format changes
m-nash Nov 7, 2023
b348341
more tweaks to the API
m-nash Nov 8, 2023
8a55322
impl tweaks
m-nash Nov 8, 2023
3675efe
address feedback
m-nash Nov 8, 2023
bddb09a
update api
m-nash Nov 8, 2023
8aa0fc7
fix tests
m-nash Nov 8, 2023
e5259b5
address feedback
m-nash Nov 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions sdk/core/Azure.Core/tests/common/Internal/ModelSerializerHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the MIT License.

using System;
using System.Net.ClientModel;
using System.Net.ClientModel.Core;
using System.Runtime.CompilerServices;

Expand All @@ -11,17 +10,17 @@ namespace Azure.Core.Tests.Common
internal static class ModelSerializerHelper
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ValidateFormat<T>(IModel<T> model, ModelReaderWriterFormat format)
public static void ValidateFormat<T>(IModel<T> model, string format)
{
bool implementsJson = model is IJsonModel<T>;
bool isValid = (format == ModelReaderWriterFormat.Json && implementsJson) || format == ModelReaderWriterFormat.Wire;
bool isValid = (format == "J" && implementsJson) || format == "W";
Copy link
Member

@annelo-msft annelo-msft Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Curious: would it be valid to do

Suggested change
bool isValid = (format == "J" && implementsJson) || format == "W";
bool isValid = (format == ModelReaderWriterOptions.Json.Format && implementsJson) || format == ModelReaderWriterOptions.Json.Format;

Asking for two reasons:

  • To see if I'm understanding how this works correctly
  • Because if we can, I think that'd be preferable to avoid "magic numbers" type errors

Copy link
Member Author

Choose a reason for hiding this comment

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

We would need to duplicate the constants in all libraries since all libraries will have models which will need to compare against these values.

This is one of the "downsides" to removing the ModelReaderWriterFormat, but it removes ambiguity around the conceptual formats wire and data and the concrete data interchange formats json, xml, etc.

if (!isValid)
{
throw new FormatException($"The model {model.GetType().Name} does not support '{format}' format.");
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void ValidateFormat(IModel<object> model, ModelReaderWriterFormat format) => ValidateFormat<object>(model, format);
public static void ValidateFormat(IModel<object> model, string format) => ValidateFormat<object>(model, format);
}
}
24 changes: 12 additions & 12 deletions sdk/core/Azure.Core/tests/common/ModelReaderWriter/Models/ChildModelXml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ public ChildModelXml(string value, string readonlyProperty)
public string ChildReadOnlyProperty { get; }

void IXmlSerializable.Write(XmlWriter writer, string nameHint) =>
Serialize(writer, ModelReaderWriterOptions.DefaultWireOptions, nameHint);
Serialize(writer, ModelReaderWriterOptions.Wire, nameHint);

private void Serialize(XmlWriter writer, ModelReaderWriterOptions options, string nameHint)
{
writer.WriteStartElement(nameHint ?? "ChildTag");
writer.WriteStartElement("ChildValue");
writer.WriteValue(ChildValue);
writer.WriteEndElement();
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
writer.WriteStartElement("ChildReadOnlyProperty");
writer.WriteValue(ChildReadOnlyProperty);
Expand All @@ -57,7 +57,7 @@ private void Serialize(XmlWriter writer, ModelReaderWriterOptions options, strin

internal static ChildModelXml DeserializeChildModelXml(XElement element, ModelReaderWriterOptions options = default)
{
options ??= ModelReaderWriterOptions.DefaultWireOptions;
options ??= ModelReaderWriterOptions.Wire;

string value = default;
string readonlyProperty = default;
Expand All @@ -74,7 +74,7 @@ internal static ChildModelXml DeserializeChildModelXml(XElement element, ModelRe

internal static ChildModelXml DeserializeChildModelXml(JsonElement element, ModelReaderWriterOptions options = default)
{
options ??= ModelReaderWriterOptions.DefaultWireOptions;
options ??= ModelReaderWriterOptions.Wire;

string childValue = default;
string childReadOnlyProperty = default;
Expand All @@ -96,11 +96,11 @@ internal static ChildModelXml DeserializeChildModelXml(JsonElement element, Mode
return new ChildModelXml(childValue, childReadOnlyProperty);
}

ChildModelXml IModel<ChildModelXml>.Read(BinaryData data, ModelReaderWriterOptions options)
ChildModelXml IModel<ChildModelXml>.Create(BinaryData data, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
using var doc = JsonDocument.Parse(data);
return DeserializeChildModelXml(doc.RootElement, options);
Expand All @@ -115,13 +115,13 @@ BinaryData IModel<ChildModelXml>.Write(ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
return ModelReaderWriter.Write(this, options);
}
else
{
options ??= ModelReaderWriterOptions.DefaultWireOptions;
options ??= ModelReaderWriterOptions.Wire;
using MemoryStream stream = new MemoryStream();
using XmlWriter writer = XmlWriter.Create(stream);
Serialize(writer, options, null);
Expand All @@ -142,7 +142,7 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)
writer.WriteStartObject();
writer.WritePropertyName("childValue"u8);
writer.WriteStringValue(ChildValue);
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
writer.WritePropertyName("childReadOnlyProperty"u8);
writer.WriteStringValue(ChildReadOnlyProperty);
Expand All @@ -157,7 +157,7 @@ void IJsonModel<ChildModelXml>.Write(Utf8JsonWriter writer, ModelReaderWriterOpt
Serialize(writer, options);
}

ChildModelXml IJsonModel<ChildModelXml>.Read(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
ChildModelXml IJsonModel<ChildModelXml>.Create(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

Expand All @@ -166,8 +166,8 @@ ChildModelXml IJsonModel<ChildModelXml>.Read(ref Utf8JsonReader reader, ModelRea
}

void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) =>
Serialize(writer, ModelReaderWriterOptions.DefaultWireOptions);
Serialize(writer, ModelReaderWriterOptions.Wire);

ModelReaderWriterFormat IModel<ChildModelXml>.GetWireFormat(ModelReaderWriterOptions options) => ModelReaderWriterFormat.Xml;
string IModel<ChildModelXml>.GetWireFormat(ModelReaderWriterOptions options) => "X";
}
}
14 changes: 7 additions & 7 deletions sdk/core/Azure.Core/tests/common/ModelReaderWriter/Models/DiscriminatorSet/BaseModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected internal void SerializeRawData(Utf8JsonWriter writer)
}
}

void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => ((IJsonModel<BaseModel>)this).Write(writer, ModelReaderWriterOptions.DefaultWireOptions);
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => ((IJsonModel<BaseModel>)this).Write(writer, ModelReaderWriterOptions.Wire);

void IJsonModel<BaseModel>.Write(Utf8JsonWriter writer, ModelReaderWriterOptions options)
{
Expand All @@ -56,7 +56,7 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)
writer.WritePropertyName("name"u8);
writer.WriteStringValue(Name);
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
SerializeRawData(writer);
}
Expand All @@ -68,7 +68,7 @@ internal static BaseModel DeserializeBaseModel(BinaryData data, ModelReaderWrite

internal static BaseModel DeserializeBaseModel(JsonElement element, ModelReaderWriterOptions options = default)
{
options ??= ModelReaderWriterOptions.DefaultWireOptions;
options ??= ModelReaderWriterOptions.Wire;

if (element.ValueKind == JsonValueKind.Null)
{
Expand Down Expand Up @@ -101,7 +101,7 @@ internal static BaseModel DeserializeBaseModel(JsonElement element, ModelReaderW
name = property.Value.GetString();
continue;
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
//this means it's an unknown property we got
rawData.Add(property.Name, BinaryData.FromString(property.Value.GetRawText()));
Expand All @@ -110,14 +110,14 @@ internal static BaseModel DeserializeBaseModel(JsonElement element, ModelReaderW
return new UnknownBaseModel(kind, name, rawData);
}

BaseModel IModel<BaseModel>.Read(BinaryData data, ModelReaderWriterOptions options)
BaseModel IModel<BaseModel>.Create(BinaryData data, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

return DeserializeBaseModel(JsonDocument.Parse(data.ToString()).RootElement, options);
}

BaseModel IJsonModel<BaseModel>.Read(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
BaseModel IJsonModel<BaseModel>.Create(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

Expand All @@ -132,6 +132,6 @@ BinaryData IModel<BaseModel>.Write(ModelReaderWriterOptions options)
return ModelReaderWriter.Write(this, options);
}

ModelReaderWriterFormat IModel<BaseModel>.GetWireFormat(ModelReaderWriterOptions options) => ModelReaderWriterFormat.Json;
string IModel<BaseModel>.GetWireFormat(ModelReaderWriterOptions options) => "J";
}
}
16 changes: 8 additions & 8 deletions sdk/core/Azure.Core/tests/common/ModelReaderWriter/Models/DiscriminatorSet/ModelX.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ internal ModelX(string kind, string name, int xProperty, int? nullProperty, ILis
public int? NullProperty = null;
public IDictionary<string, string> KeyValuePairs { get; }

void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => ((IJsonModel<ModelX>)this).Write(writer, ModelReaderWriterOptions.DefaultWireOptions);
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => ((IJsonModel<ModelX>)this).Write(writer, ModelReaderWriterOptions.Wire);

void IJsonModel<ModelX>.Write(Utf8JsonWriter writer, ModelReaderWriterOptions options)
{
Expand Down Expand Up @@ -86,12 +86,12 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)
writer.WriteEndObject();
}

if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
writer.WritePropertyName("xProperty"u8);
writer.WriteNumberValue(XProperty);
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
SerializeRawData(writer);
}
Expand All @@ -100,7 +100,7 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)

internal static ModelX DeserializeModelX(JsonElement element, ModelReaderWriterOptions options = default)
{
options ??= ModelReaderWriterOptions.DefaultWireOptions;
options ??= ModelReaderWriterOptions.Wire;

if (element.ValueKind == JsonValueKind.Null)
{
Expand Down Expand Up @@ -155,7 +155,7 @@ internal static ModelX DeserializeModelX(JsonElement element, ModelReaderWriterO
xProperty = property.Value.GetInt32();
continue;
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
//this means it's an unknown property we got
rawData.Add(property.Name, BinaryData.FromString(property.Value.GetRawText()));
Expand All @@ -164,7 +164,7 @@ internal static ModelX DeserializeModelX(JsonElement element, ModelReaderWriterO
return new ModelX(kind, name, xProperty, Optional.ToNullable(nullProperty), Optional.ToList(fields), Optional.ToDictionary(keyValuePairs), rawData);
}

ModelX IModel<ModelX>.Read(BinaryData data, ModelReaderWriterOptions options)
ModelX IModel<ModelX>.Create(BinaryData data, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

Expand All @@ -177,7 +177,7 @@ public void Serialize(Utf8JsonWriter writer)
((IUtf8JsonSerializable)this).Write(writer);
}

ModelX IJsonModel<ModelX>.Read(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
ModelX IJsonModel<ModelX>.Create(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

Expand All @@ -192,6 +192,6 @@ BinaryData IModel<ModelX>.Write(ModelReaderWriterOptions options)
return ModelReaderWriter.Write(this, options);
}

ModelReaderWriterFormat IModel<ModelX>.GetWireFormat(ModelReaderWriterOptions options) => ModelReaderWriterFormat.Json;
string IModel<ModelX>.GetWireFormat(ModelReaderWriterOptions options) => "J";
}
}
16 changes: 8 additions & 8 deletions sdk/core/Azure.Core/tests/common/ModelReaderWriter/Models/DiscriminatorSet/ModelY.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ internal ModelY(string kind, string name, string yProperty, Dictionary<string, B

public string YProperty { get; private set; }

void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => ((IJsonModel<ModelY>)this).Write(writer, ModelReaderWriterOptions.DefaultWireOptions);
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => ((IJsonModel<ModelY>)this).Write(writer, ModelReaderWriterOptions.Wire);

void IJsonModel<ModelY>.Write(Utf8JsonWriter writer, ModelReaderWriterOptions options) => Serialize(writer, options);

Expand All @@ -42,12 +42,12 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)
writer.WritePropertyName("name"u8);
writer.WriteStringValue(Name);
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
writer.WritePropertyName("yProperty"u8);
writer.WriteStringValue(YProperty);
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
SerializeRawData(writer);
}
Expand All @@ -56,7 +56,7 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)

internal static ModelY DeserializeModelY(JsonElement element, ModelReaderWriterOptions options = default)
{
options ??= ModelReaderWriterOptions.DefaultWireOptions;
options ??= ModelReaderWriterOptions.Wire;

if (element.ValueKind == JsonValueKind.Null)
{
Expand All @@ -83,7 +83,7 @@ internal static ModelY DeserializeModelY(JsonElement element, ModelReaderWriterO
yProperty = property.Value.GetString();
continue;
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
//this means it's an unknown property we got
rawData.Add(property.Name, BinaryData.FromString(property.Value.GetRawText()));
Expand All @@ -92,12 +92,12 @@ internal static ModelY DeserializeModelY(JsonElement element, ModelReaderWriterO
return new ModelY(kind, name, yProperty, rawData);
}

ModelY IModel<ModelY>.Read(BinaryData data, ModelReaderWriterOptions options)
ModelY IModel<ModelY>.Create(BinaryData data, ModelReaderWriterOptions options)
{
return DeserializeModelY(JsonDocument.Parse(data.ToString()).RootElement, options);
}

ModelY IJsonModel<ModelY>.Read(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
ModelY IJsonModel<ModelY>.Create(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
{
using var doc = JsonDocument.ParseValue(ref reader);
return DeserializeModelY(doc.RootElement, options);
Expand All @@ -110,6 +110,6 @@ BinaryData IModel<ModelY>.Write(ModelReaderWriterOptions options)
return ModelReaderWriter.Write(this, options);
}

ModelReaderWriterFormat IModel<ModelY>.GetWireFormat(ModelReaderWriterOptions options) => ModelReaderWriterFormat.Json;
string IModel<ModelY>.GetWireFormat(ModelReaderWriterOptions options) => "J";
}
}
8 changes: 4 additions & 4 deletions ...ore/Azure.Core/tests/common/ModelReaderWriter/Models/DiscriminatorSet/UnknownBaseModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal UnknownBaseModel(string kind, string name, Dictionary<string, BinaryDat
Name = name;
}

void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => Serialize(writer, ModelReaderWriterOptions.DefaultWireOptions);
void IUtf8JsonSerializable.Write(Utf8JsonWriter writer) => Serialize(writer, ModelReaderWriterOptions.Wire);

void IJsonModel<BaseModel>.Write(Utf8JsonWriter writer, ModelReaderWriterOptions options)
{
Expand All @@ -44,7 +44,7 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)
writer.WritePropertyName("name"u8);
writer.WriteStringValue(Name);
}
if (options.Format == ModelReaderWriterFormat.Json)
if (options.Format == "J")
{
SerializeRawData(writer);
}
Expand All @@ -53,14 +53,14 @@ private void Serialize(Utf8JsonWriter writer, ModelReaderWriterOptions options)

internal static BaseModel DeserializeUnknownBaseModel(JsonElement element, ModelReaderWriterOptions options = default) => DeserializeBaseModel(element, options);

BaseModel IModel<BaseModel>.Read(BinaryData data, ModelReaderWriterOptions options)
BaseModel IModel<BaseModel>.Create(BinaryData data, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

return DeserializeUnknownBaseModel(JsonDocument.Parse(data.ToString()).RootElement, options);
}

BaseModel IJsonModel<BaseModel>.Read(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
BaseModel IJsonModel<BaseModel>.Create(ref Utf8JsonReader reader, ModelReaderWriterOptions options)
{
ModelSerializerHelper.ValidateFormat(this, options.Format);

Expand Down
Loading