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

Return CLR null for JSON elements with JsonValueKind.Null #36552

Merged
merged 3 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,6 @@ public static explicit operator Guid(DynamicData value)
/// This operator calls through to <see cref="DynamicData.Equals(object?)"/> when DynamicData is on the left-hand
/// side of the operation. <see cref="DynamicData.Equals(object?)"/> has value semantics when the DynamicData represents
/// a JSON primitive, i.e. string, bool, number, or null, and reference semantics otherwise, i.e. for objects and arrays.
///
/// Please note that if DynamicData is on the right-hand side of a <c>!=</c> operation, this operator will not be invoked.
/// Because of this the result of a <c>!=</c> comparison with <c>null</c> on the left and a DynamicData instance on the right will return <c>true</c>.
/// </remarks>
/// <param name="left">The <see cref="DynamicData"/> to compare.</param>
/// <param name="right">The <see cref="object"/> to compare.</param>
Expand Down
24 changes: 23 additions & 1 deletion sdk/core/Azure.Core/src/DynamicData/DynamicData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ internal void WriteTo(Stream stream)

if (_element.TryGetProperty(name, out MutableJsonElement element))
{
if (element.ValueKind == JsonValueKind.Null)
{
return null;
}

return new DynamicData(element, _options);
}

Expand All @@ -102,6 +107,11 @@ internal void WriteTo(Stream stream)
{
if (_element.TryGetProperty(ConvertToCamelCase(name), out element))
{
if (element.ValueKind == JsonValueKind.Null)
{
return null;
}

return new DynamicData(element, _options);
}
}
Expand All @@ -119,13 +129,25 @@ internal void WriteTo(Stream stream)
case string propertyName:
if (_element.TryGetProperty(propertyName, out MutableJsonElement element))
{
if (element.ValueKind == JsonValueKind.Null)
{
return null;
}

return new DynamicData(element, _options);
}

throw new KeyNotFoundException($"Could not find JSON member with name '{propertyName}'.");

case int arrayIndex:
return new DynamicData(_element.GetIndexElement(arrayIndex), _options);
MutableJsonElement arrayElement = _element.GetIndexElement(arrayIndex);

if (arrayElement.ValueKind == JsonValueKind.Null)
{
return null;
}

return new DynamicData(arrayElement, _options);
}

throw new InvalidOperationException($"Tried to access indexer with an unsupported index type: {index}");
Expand Down
16 changes: 16 additions & 0 deletions sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ public void CanGetNullPropertyValue()

Assert.IsNull((CustomType)jsonData.Foo);
Assert.IsNull((int?)jsonData.Foo);
Assert.IsNull(jsonData.Foo);
Assert.IsNull(jsonData.foo);
}

[Test]
Expand All @@ -205,6 +207,7 @@ public void CanGetNullArrayValue()

Assert.IsNull((CustomType)jsonData[0]);
Assert.IsNull((int?)jsonData[0]);
Assert.IsNull(jsonData[0]);
}

[Test]
Expand All @@ -216,6 +219,8 @@ public void CanSetPropertyValueToNull()

Assert.IsNull((CustomType)jsonData.Foo);
Assert.IsNull((int?)jsonData.Foo);
Assert.IsNull(jsonData.Foo);
Assert.IsNull(jsonData.foo);
}

[Test]
Expand All @@ -227,6 +232,7 @@ public void CanSetArrayValueToNull()

Assert.IsNull((CustomType)jsonData[0]);
Assert.IsNull((int?)jsonData[0]);
Assert.IsNull(jsonData[0]);
}

[Test]
Expand Down Expand Up @@ -445,9 +451,11 @@ public void CanCheckOptionalProperty()

// Property is present
Assert.IsFalse(json.Foo == null);
Assert.AreNotEqual(null, json.Foo);

// Property is absent
Assert.IsTrue(json.Bar == null);
Assert.AreEqual(null, json.Bar);
}

[Test]
Expand All @@ -471,11 +479,15 @@ public void CanCheckOptionalPropertyWithChanges()

// Properties are present
Assert.IsFalse(json.Foo == null);
Assert.AreNotEqual(null, json.Foo);
Assert.IsFalse(json.Bar.B == null);
Assert.AreNotEqual(null, json.Bar.B);
Assert.IsFalse(json.Baz == null);
Assert.AreNotEqual(null, json.Baz);

// Properties are absent
Assert.IsTrue(json.Bar.A == null);
Assert.AreEqual(null, json.Bar.A);
}

[Test]
Expand All @@ -489,6 +501,7 @@ public void CanSetOptionalProperty()

// Property is absent
Assert.IsTrue(json.OptionalValue == null);
Assert.AreEqual(null, json.OptionalValue);

json.OptionalValue = 5;

Expand Down Expand Up @@ -837,10 +850,13 @@ public void CanDifferentiateBetweenNullAndAbsent()
// GetMember binding mirrors Azure SDK models, so we allow a null check for an optional
// property through the C#-style dynamic interface.
Assert.IsTrue(json.foo == null);
Assert.AreEqual(null, json.foo);
Assert.IsTrue(json.bar == null);
Assert.AreEqual(null, json.bar);

// Indexer lookup mimics JsonNode behavior and so throws if a property is absent.
Assert.IsTrue(json["foo"] == null);
Assert.AreEqual(null, json["foo"]);
Assert.Throws<KeyNotFoundException>(() => _ = json["bar"]);
Assert.Throws<KeyNotFoundException>(() => { if (json["bar"] == null) {; } });
}
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/tests/public/JsonDataArrayTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ public void CanSetArrayIndex()
Assert.AreEqual(5, (int)data[0]);
Assert.AreEqual("valid", (string)data[1]);
Assert.IsTrue(data[2] == null);
Assert.AreEqual(null, data[2]);
}

[Test]
Expand Down
16 changes: 3 additions & 13 deletions sdk/core/Azure.Core/tests/public/JsonDataPublicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -338,30 +338,20 @@ public void EqualsPrimitiveValues(string a, string b, bool expected)
public void EqualsNull()
{
dynamic value = JsonDataTestHelpers.CreateFromJson("""{ "foo": null }""");
Assert.IsTrue(value.foo.Equals(null));
Assert.AreEqual(null, value.foo);
Assert.IsTrue(value.foo == null);

string nullString = null;
Assert.IsTrue(value.foo == nullString);
Assert.IsTrue(nullString == value.foo);

// Because the DLR resolves `==` for nullable value types to take the non-nullable
// value on the right-hand side, we'll still require a cast for nullable primitives.
int? nullInt = null;
Assert.IsTrue(value.foo == nullInt);
Assert.IsTrue(nullInt == (int?)value.foo);
Assert.IsTrue(nullInt == value.foo);

bool? nullBool = null;
Assert.IsTrue(value.foo == nullBool);
Assert.IsTrue(nullBool == (bool?)value.foo);

// We cannot overload the equality operator with two nullable values, so
// the following is the consequence.
Assert.IsFalse(null == value.foo);
annelo-msft marked this conversation as resolved.
Show resolved Hide resolved

// However, this does give us a backdoor to differentiate between an
// absent property and a property whose JSON value is null, if we wanted to
// use it that way, although it's not really very nice.
Assert.IsTrue(nullBool == value.foo);
}

[Test]
Expand Down