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

SystemTextJsonSerializer: Adds Default STJ Serializer #4332

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
bf78e7c
Code changes to add default STJ serializer.
kundadebdatta Feb 27, 2024
306b929
Code changes to add tests for STJ serializer.
kundadebdatta Feb 28, 2024
49844eb
Code changes to rename files.
kundadebdatta Mar 11, 2024
d49dff9
Code changes to clean up files.
kundadebdatta Mar 11, 2024
2d17302
Code changes to add more tests.
kundadebdatta Mar 11, 2024
c78093a
Merge branch 'master' into users/kundadebdatta/4330_add_default_stj_s…
kundadebdatta Mar 11, 2024
004d08d
Code changes to update respective contracts.
kundadebdatta Mar 11, 2024
3a728b1
Code changes to add more documentation.
kundadebdatta Mar 20, 2024
089095c
Code changes to add LINQ tests. Addressing few review comments.
kundadebdatta Apr 11, 2024
8b208de
Code changes to fix build failures.
kundadebdatta Apr 11, 2024
ca3b6a6
Code changes to update contract files.
kundadebdatta Apr 12, 2024
73178e0
Code changes to add more LINQ specific test cases.
kundadebdatta Apr 15, 2024
683ca13
Code changes to address review comments.
kundadebdatta Apr 15, 2024
61171cd
Merge branch 'master' into users/kundadebdatta/4330_add_default_stj_s…
kundadebdatta Apr 15, 2024
5df9c07
Merge branch 'master' into users/kundadebdatta/4330_add_default_stj_s…
kundadebdatta May 9, 2024
cdbf98c
Code changes to port some of the optimizations from Maya.
kundadebdatta May 9, 2024
4938ac3
Merge branch 'master' into users/kundadebdatta/4330_add_default_stj_s…
kundadebdatta May 10, 2024
37009ca
Code changes to update the remarks since the changes from maya will c…
kundadebdatta May 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
// ------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// ------------------------------------------------------------

namespace Microsoft.Azure.Cosmos
{
using System;
using System.IO;
using System.Reflection;
using System.Text.Json;
using System.Text.Json.Serialization;
using global::Azure.Core.Serialization;

/// <summary>
/// This class provides a way to configure Linq Serialization Properties
/// </summary>
public class CosmosSystemTextJsonSerializer : CosmosLinqSerializer
kundadebdatta marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly JsonObjectSerializer systemTextJsonSerializer;

/// <summary>
/// Create an instance of <see cref="CosmosSystemTextJsonSerializer"/>
/// with the default values for the Cosmos SDK
/// </summary>
/// <param name="jsonSerializerOptions">options.</param>
public CosmosSystemTextJsonSerializer(JsonSerializerOptions jsonSerializerOptions)
{
this.systemTextJsonSerializer = new JsonObjectSerializer(jsonSerializerOptions);
}

/// <inheritdoc/>
public override T FromStream<T>(Stream stream)
{
if (stream == null)
throw new ArgumentNullException(nameof(stream));

using (stream)
{
if (stream.CanSeek && stream.Length == 0)
{
return default;
}

if (typeof(Stream).IsAssignableFrom(typeof(T)))
{
return (T)(object)stream;
}

return (T)this.systemTextJsonSerializer.Deserialize(stream, typeof(T), default);
kundadebdatta marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// <inheritdoc/>
public override Stream ToStream<T>(T input)
{
MemoryStream streamPayload = new MemoryStream();
this.systemTextJsonSerializer.Serialize(streamPayload, input, input.GetType(), default);
streamPayload.Position = 0;
return streamPayload;
}

/// <inheritdoc/>
public override string SerializeMemberName(MemberInfo memberInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

I've updated the STJ sample code and made some improvements to this function in this PR #4420. Could you update this function to look like this - https://github.com/Azure/azure-cosmos-dotnet-v3/blob/master/Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson/CosmosSystemTextJsonSerializer.cs ? Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @Maya-Painter. We are trying to gather some feedback from the .NET team on this. I will take your changes as well, and update the PR.

Choose a reason for hiding this comment

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

The JsonTypeInfo/JsonPropertyInfo types contain authoritative information on the JSON contract used by a particular type/property. See https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/custom-contracts for more details.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you @eiriktsarpalis !

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your suggestion @eiriktsarpalis! Looks like, these (JsonSerializerOptions.TryGetTypeInfo, custom contracts) are applicable for the latest versions of the System.Text.JSON package, and it's not available in 4.6.0.

In order to use the System.Text.Json (STJ) serializer, we currently depend on the Azure.Core nuget package (specifically for this class JsonObjectSerializer). The current version of Azure.Core library, referred in our v3 .NET SDK is 1.19.0, which is quite old. This internally uses System.Text.Json version 4.6.0.

I am planning to keep this suggestion as follow-up task 4479 to upgrade the Azure.Core library to the latest. Even the latest version of Azure.Core 1.39.0 library uses the STJ version 4.7.2.

image

Copy link
Member

Choose a reason for hiding this comment

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

How about only make this type public once is fully addressed?

Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that Azure.Core 'uses' (gets by with) an old version does not of itself mean that this lib can't take a dependency on a higher version. I'd ask, what version would simplify and/or significantly improve perf here? Then validate the answer by lookign at package download counts at https://www.nuget.org/packages/system.text.json.
For instance 6.0.1 has a net6.0 variant and seems to be in popular use, and is only 18 months newer - it's not like STJ is known to be a pain to upgrade.
But of course, I'm just being a devil's advocate; there's lots of silent votes for the status quo!
Hopefully there will be a security reason along to trigger a forced upgrade in due course ;)
The other benefit of taking a higher dep is that perf in general has been on an upward trajectory, and the perceived experience of the SDK would hence also improve for people that don't explicitly pin latest in their apps

{
JsonExtensionDataAttribute jsonExtensionDataAttribute =
memberInfo.GetCustomAttribute<JsonExtensionDataAttribute>(true);
if (jsonExtensionDataAttribute != null)
{
return null;
}

JsonPropertyNameAttribute jsonPropertyNameAttribute = memberInfo.GetCustomAttribute<JsonPropertyNameAttribute>(true);

string memberName = !string.IsNullOrEmpty(jsonPropertyNameAttribute?.Name)
? jsonPropertyNameAttribute.Name
: memberInfo.Name;

// Users must add handling for any additional attributes here

return memberName;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
namespace Microsoft.Azure.Cosmos.Tests.Json
{
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Microsoft.Azure.Cosmos.Tests.Poco.STJ;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public sealed class CosmosSystemTextJsonSerializerTest
{
CosmosSystemTextJsonSerializer stjSerializer;

[TestInitialize]
public void SetUp()
{
this.stjSerializer = new(
new System.Text.Json.JsonSerializerOptions());
}

[TestMethod]
public void TestPoco()
{
// Arrange.
Cars car = Cars.GetRandomCar();

// Act.
Stream serializedStream = this.stjSerializer.ToStream<Cars>(car);
Cars deserializedValue = this.stjSerializer.FromStream<Cars>(serializedStream);

// Assert.
Assert.AreEqual(car.Name, deserializedValue.Name);
Assert.AreEqual(car.ColorCode, deserializedValue.ColorCode);
Assert.AreEqual(car.CustomFeatures.Count, deserializedValue.CustomFeatures.Count);
}

[TestMethod]
public void TestList()
{
// Arrange.
List<int> list = new List<int> { 1, 2, 3 };

// Act.
Stream serializedStream = this.stjSerializer.ToStream<List<int>>(list);
List<int> deserializedList = this.stjSerializer.FromStream<List<int>>(serializedStream);

// Assert.
Assert.AreEqual(list[0], deserializedList[0]);
Assert.AreEqual(list[1], deserializedList[1]);
Assert.AreEqual(list[2], deserializedList[2]);
}

[TestMethod]
[DataRow(true)]
[DataRow(false)]
public void TestBool(bool booleanValue)
{
// Arrange and Act.
Stream serializedStream = this.stjSerializer.ToStream<bool>(booleanValue);
bool deserializedValue= this.stjSerializer.FromStream<bool>(serializedStream);

// Assert.
if (booleanValue)
{
Assert.IsTrue(deserializedValue);
}
else
{
Assert.IsFalse(deserializedValue);
}
}

[TestMethod]
public void TestNumber()
{
{
short value = 32;
Stream serializedStream = this.stjSerializer.ToStream<short>(value);
short int16Value = this.stjSerializer.FromStream<short>(serializedStream);
Assert.AreEqual(expected: value, actual: int16Value);
}

{
int value = 32;
Stream serializedStream = this.stjSerializer.ToStream<int>(value);
int int32Value = this.stjSerializer.FromStream<int>(serializedStream);
Assert.AreEqual(expected: value, actual: int32Value);
}

{
long value = 32;
Stream serializedStream = this.stjSerializer.ToStream<long>(value);
long int64Value = this.stjSerializer.FromStream<long>(serializedStream);
Assert.AreEqual(expected: value, actual: int64Value);
}

{
uint value = 32;
Stream serializedStream = this.stjSerializer.ToStream<uint>(value);
uint uintValue = this.stjSerializer.FromStream<uint>(serializedStream);
Assert.AreEqual(expected: value, actual: uintValue);
}

{
float value = 32.1337f;
Stream serializedStream = this.stjSerializer.ToStream<float>(value);
float floatValue = this.stjSerializer.FromStream<float>(serializedStream);
Assert.AreEqual(expected: value, actual: floatValue);
}

{
double value = 32.1337;
Stream serializedStream = this.stjSerializer.ToStream<double>(value);
double doubleValue = this.stjSerializer.FromStream<double>(serializedStream);
Assert.AreEqual(expected: value, actual: doubleValue);
}
}

[TestMethod]
public void TestString()
{
// Arrange.
string value = "asdf";

// Act.
Stream result = this.stjSerializer.ToStream<string>(value);
string cosmosString = this.stjSerializer.FromStream<string>(result);

// Assert.
Assert.AreEqual(expected: value, actual: cosmosString.ToString());
}

[TestMethod]
public void TestBinary()
{
// Arrange.
byte[] value = new byte[] { 1, 2, 3 };

// Act.
Stream serializedStream = this.stjSerializer.ToStream<byte[]>(value);
byte[] array = this.stjSerializer.FromStream<byte[]>(serializedStream);

// Assert.
Assert.IsTrue(array.SequenceEqual(value.ToArray()));
}

[TestMethod]
public void TestGuid()
{
// Arrange.
Guid value = Guid.NewGuid();

// Act.
Stream serializedStream = this.stjSerializer.ToStream<Guid>(value);
Guid guidValue = this.stjSerializer.FromStream<Guid>(serializedStream);

// Assert.
Assert.AreEqual(expected: value, actual: guidValue);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
namespace Microsoft.Azure.Cosmos.Tests.Poco.STJ
{
using System;
using System.Collections.Generic;
using System.Text.Json.Serialization;

public sealed class Cars
{
private static readonly Random random = new();
private static readonly string[] names = new string[]
{
"Toyota 4Runner",
"Toyota Camry",
"Toyota Prius",
"Toyota Rav4",
"Toyota Avalon",
"Honda Civic",
"Honda Accord",
"Honda CRV",
"Honda MRV",
"BMW C18",
kundadebdatta marked this conversation as resolved.
Show resolved Hide resolved
"BMW 328i",
"BMW 530i",
};

private static readonly string[] features = new string[]
{
"all wheel drive",
"abs",
"apple car play",
"power window",
"heated seating",
"steering mounted control",
};

[JsonConstructor]
kundadebdatta marked this conversation as resolved.
Show resolved Hide resolved
public Cars(
string name,
int colorCode,
double vin,
List<string> customFeatures)
{
this.Name = name;
this.ColorCode = colorCode;
this.Vin = vin;
this.CustomFeatures = customFeatures;
}

[JsonPropertyName("name")]
public string Name { get; }

[JsonPropertyName("colorCode")]
public int ColorCode { get; }

[JsonPropertyName("vin")]
public double Vin { get; }

[JsonPropertyName("customFeatures")]
public List<string> CustomFeatures { get; }

public override bool Equals(object obj)
{
if (!(obj is Cars car))
{
return false;
}

return this.Equals(car);
}

public bool Equals(Cars other)
{
return this.Name == other.Name && this.ColorCode == other.ColorCode;
}

public override int GetHashCode()
{
return 0;
}

public static Cars GetRandomCar()
{
string name = names[random.Next(0, names.Length)];
int colorCode = random.Next(0, 100);
double vin = random.NextDouble() * 10000000;
List<string> customFeatures = new();

int numOfFeatures = random.Next(0, 3);
for (int i = 0; i < numOfFeatures; i++)
{
customFeatures.Add(GetRandomFeature());
}

return new Cars(name, colorCode, vin, customFeatures);
}

public static string GetRandomFeature()
{
return features[random.Next(0, features.Length)];
}
}
}