From e56042d9587f923287f5e6c01a299b3c0a05ec18 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 26 May 2023 14:43:34 -0700 Subject: [PATCH 1/3] Return JsonElements with JsonValueKind.Null as CLR null --- .../src/DynamicData/DynamicData.Operators.cs | 3 --- .../Azure.Core/src/DynamicData/DynamicData.cs | 19 ++++++++++++++++++- .../tests/public/JsonDataArrayTests.cs | 1 + .../tests/public/JsonDataPublicTests.cs | 16 +++------------- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/DynamicData.Operators.cs b/sdk/core/Azure.Core/src/DynamicData/DynamicData.Operators.cs index a63531dd13b88..26f1bec218705 100644 --- a/sdk/core/Azure.Core/src/DynamicData/DynamicData.Operators.cs +++ b/sdk/core/Azure.Core/src/DynamicData/DynamicData.Operators.cs @@ -370,9 +370,6 @@ public static explicit operator Guid(DynamicData value) /// This operator calls through to when DynamicData is on the left-hand /// side of the operation. 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 != operation, this operator will not be invoked. - /// Because of this the result of a != comparison with null on the left and a DynamicData instance on the right will return true. /// /// The to compare. /// The to compare. diff --git a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs index 4f98a147f20bd..19788a757fc63 100644 --- a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs +++ b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs @@ -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); } @@ -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); } } @@ -125,7 +135,14 @@ internal void WriteTo(Stream stream) 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}"); diff --git a/sdk/core/Azure.Core/tests/public/JsonDataArrayTests.cs b/sdk/core/Azure.Core/tests/public/JsonDataArrayTests.cs index ab658eb2ff80d..2bc7a0f30c19f 100644 --- a/sdk/core/Azure.Core/tests/public/JsonDataArrayTests.cs +++ b/sdk/core/Azure.Core/tests/public/JsonDataArrayTests.cs @@ -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] diff --git a/sdk/core/Azure.Core/tests/public/JsonDataPublicTests.cs b/sdk/core/Azure.Core/tests/public/JsonDataPublicTests.cs index 8c6249c524886..04f1a6f382b34 100644 --- a/sdk/core/Azure.Core/tests/public/JsonDataPublicTests.cs +++ b/sdk/core/Azure.Core/tests/public/JsonDataPublicTests.cs @@ -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); - - // 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] From e5cad71c9492223c0e9078dcbc231bc61503536b Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 26 May 2023 14:56:30 -0700 Subject: [PATCH 2/3] Add tests and return null from indexer as well. --- .../Azure.Core/src/DynamicData/DynamicData.cs | 5 +++++ .../tests/DynamicData/DynamicJsonTests.cs | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs index 19788a757fc63..5643d5d0f8199 100644 --- a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs +++ b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs @@ -129,6 +129,11 @@ 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); } diff --git a/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs b/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs index a5c9fdadc1f69..9dafee22da4fd 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs @@ -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] @@ -205,6 +207,7 @@ public void CanGetNullArrayValue() Assert.IsNull((CustomType)jsonData[0]); Assert.IsNull((int?)jsonData[0]); + Assert.IsNull(jsonData[0]); } [Test] @@ -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] @@ -227,6 +232,7 @@ public void CanSetArrayValueToNull() Assert.IsNull((CustomType)jsonData[0]); Assert.IsNull((int?)jsonData[0]); + Assert.IsNull(jsonData[0]); } [Test] @@ -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] @@ -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] @@ -489,6 +501,7 @@ public void CanSetOptionalProperty() // Property is absent Assert.IsTrue(json.OptionalValue == null); + Assert.AreEqual(null, json.OptionalValue); json.OptionalValue = 5; @@ -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(() => _ = json["bar"]); Assert.Throws(() => { if (json["bar"] == null) {; } }); } From 26943975f1f3889fc4b8a5a5852255b9f12b83e9 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 26 May 2023 16:11:13 -0700 Subject: [PATCH 3/3] Add cases validating exceptions are thrown --- .../tests/DynamicData/DynamicJsonTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs b/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs index 9dafee22da4fd..6ae03e46c5d46 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs @@ -7,6 +7,8 @@ using System.Text.Json; using Azure.Core.Dynamic; using Azure.Core.GeoJson; +using Azure.Core.Serialization; +using Microsoft.CSharp.RuntimeBinder; using NUnit.Framework; namespace Azure.Core.Tests @@ -155,6 +157,30 @@ public void CanSetNestedArrayValues() Assert.AreEqual(6, (int)jsonData.Foo[2]); } + [Test] + public void CannotGetOrSetValuesOnAbsentArrays() + { + dynamic value = BinaryData.FromString("""{"foo": [1, 2]}""").ToDynamicFromJson(DynamicCaseMapping.PascalToCamel); + + Assert.Throws(() => { int i = value[0]; }); + Assert.Throws(() => { value[0] = 1; }); + + Assert.Throws(() => { int i = value.Foo[0][0]; }); + Assert.Throws(() => { value.Foo[0][0] = 2; }); + } + + [Test] + public void CannotGetOrSetValuesOnAbsentProperties() + { + dynamic value = BinaryData.FromString("""{"foo": 1}""").ToDynamicFromJson(DynamicCaseMapping.PascalToCamel); + + Assert.Throws(() => { int i = value.Foo.Bar.Baz; }); + Assert.Throws(() => { value.Foo.Bar.Baz = "hi"; }); + + Assert.Throws(() => { int i = value.A.B.C; }); + Assert.Throws(() => { value.A.B.C = 1; }); + } + [Test] public void CanSetArrayValuesToDifferentTypes() {