Skip to content

Commit bf63c60

Browse files
annelo-msftannelo-msft
authored
Return CLR null for JSON elements with JsonValueKind.Null (#36552)
* Return JsonElements with JsonValueKind.Null as CLR null * Add tests and return null from indexer as well. * Add cases validating exceptions are thrown
1 parent fd2f9cb commit bf63c60

File tree

5 files changed

+69
-17
lines changed

5 files changed

+69
-17
lines changed

‎sdk/core/Azure.Core/src/DynamicData/DynamicData.Operators.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,9 +370,6 @@ public static explicit operator Guid(DynamicData value)
370370
/// This operator calls through to <see cref="DynamicData.Equals(object?)"/> when DynamicData is on the left-hand
371371
/// side of the operation. <see cref="DynamicData.Equals(object?)"/> has value semantics when the DynamicData represents
372372
/// a JSON primitive, i.e. string, bool, number, or null, and reference semantics otherwise, i.e. for objects and arrays.
373-
///
374-
/// Please note that if DynamicData is on the right-hand side of a <c>!=</c> operation, this operator will not be invoked.
375-
/// 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>.
376373
/// </remarks>
377374
/// <param name="left">The <see cref="DynamicData"/> to compare.</param>
378375
/// <param name="right">The <see cref="object"/> to compare.</param>

‎sdk/core/Azure.Core/src/DynamicData/DynamicData.cs‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ internal void WriteTo(Stream stream)
9393

9494
if (_element.TryGetProperty(name, out MutableJsonElement element))
9595
{
96+
if (element.ValueKind == JsonValueKind.Null)
97+
{
98+
return null;
99+
}
100+
96101
return new DynamicData(element, _options);
97102
}
98103

@@ -102,6 +107,11 @@ internal void WriteTo(Stream stream)
102107
{
103108
if (_element.TryGetProperty(ConvertToCamelCase(name), out element))
104109
{
110+
if (element.ValueKind == JsonValueKind.Null)
111+
{
112+
return null;
113+
}
114+
105115
return new DynamicData(element, _options);
106116
}
107117
}
@@ -119,13 +129,25 @@ internal void WriteTo(Stream stream)
119129
case string propertyName:
120130
if (_element.TryGetProperty(propertyName, out MutableJsonElement element))
121131
{
132+
if (element.ValueKind == JsonValueKind.Null)
133+
{
134+
return null;
135+
}
136+
122137
return new DynamicData(element, _options);
123138
}
124139

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

127142
case int arrayIndex:
128-
return new DynamicData(_element.GetIndexElement(arrayIndex), _options);
143+
MutableJsonElement arrayElement = _element.GetIndexElement(arrayIndex);
144+
145+
if (arrayElement.ValueKind == JsonValueKind.Null)
146+
{
147+
return null;
148+
}
149+
150+
return new DynamicData(arrayElement, _options);
129151
}
130152

131153
throw new InvalidOperationException($"Tried to access indexer with an unsupported index type: {index}");

‎sdk/core/Azure.Core/tests/DynamicData/DynamicJsonTests.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
using System.Text.Json;
88
using Azure.Core.Dynamic;
99
using Azure.Core.GeoJson;
10+
using Azure.Core.Serialization;
11+
using Microsoft.CSharp.RuntimeBinder;
1012
using NUnit.Framework;
1113

1214
namespace Azure.Core.Tests
@@ -155,6 +157,30 @@ public void CanSetNestedArrayValues()
155157
Assert.AreEqual(6, (int)jsonData.Foo[2]);
156158
}
157159

160+
[Test]
161+
public void CannotGetOrSetValuesOnAbsentArrays()
162+
{
163+
dynamic value = BinaryData.FromString("""{"foo": [1, 2]}""").ToDynamicFromJson(DynamicCaseMapping.PascalToCamel);
164+
165+
Assert.Throws<InvalidOperationException>(() => { int i = value[0]; });
166+
Assert.Throws<InvalidOperationException>(() => { value[0] = 1; });
167+
168+
Assert.Throws<InvalidOperationException>(() => { int i = value.Foo[0][0]; });
169+
Assert.Throws<InvalidOperationException>(() => { value.Foo[0][0] = 2; });
170+
}
171+
172+
[Test]
173+
public void CannotGetOrSetValuesOnAbsentProperties()
174+
{
175+
dynamic value = BinaryData.FromString("""{"foo": 1}""").ToDynamicFromJson(DynamicCaseMapping.PascalToCamel);
176+
177+
Assert.Throws<InvalidOperationException>(() => { int i = value.Foo.Bar.Baz; });
178+
Assert.Throws<InvalidOperationException>(() => { value.Foo.Bar.Baz = "hi"; });
179+
180+
Assert.Throws<RuntimeBinderException>(() => { int i = value.A.B.C; });
181+
Assert.Throws<RuntimeBinderException>(() => { value.A.B.C = 1; });
182+
}
183+
158184
[Test]
159185
public void CanSetArrayValuesToDifferentTypes()
160186
{
@@ -196,6 +222,8 @@ public void CanGetNullPropertyValue()
196222

197223
Assert.IsNull((CustomType)jsonData.Foo);
198224
Assert.IsNull((int?)jsonData.Foo);
225+
Assert.IsNull(jsonData.Foo);
226+
Assert.IsNull(jsonData.foo);
199227
}
200228

201229
[Test]
@@ -205,6 +233,7 @@ public void CanGetNullArrayValue()
205233

206234
Assert.IsNull((CustomType)jsonData[0]);
207235
Assert.IsNull((int?)jsonData[0]);
236+
Assert.IsNull(jsonData[0]);
208237
}
209238

210239
[Test]
@@ -216,6 +245,8 @@ public void CanSetPropertyValueToNull()
216245

217246
Assert.IsNull((CustomType)jsonData.Foo);
218247
Assert.IsNull((int?)jsonData.Foo);
248+
Assert.IsNull(jsonData.Foo);
249+
Assert.IsNull(jsonData.foo);
219250
}
220251

221252
[Test]
@@ -227,6 +258,7 @@ public void CanSetArrayValueToNull()
227258

228259
Assert.IsNull((CustomType)jsonData[0]);
229260
Assert.IsNull((int?)jsonData[0]);
261+
Assert.IsNull(jsonData[0]);
230262
}
231263

232264
[Test]
@@ -445,9 +477,11 @@ public void CanCheckOptionalProperty()
445477

446478
// Property is present
447479
Assert.IsFalse(json.Foo == null);
480+
Assert.AreNotEqual(null, json.Foo);
448481

449482
// Property is absent
450483
Assert.IsTrue(json.Bar == null);
484+
Assert.AreEqual(null, json.Bar);
451485
}
452486

453487
[Test]
@@ -471,11 +505,15 @@ public void CanCheckOptionalPropertyWithChanges()
471505

472506
// Properties are present
473507
Assert.IsFalse(json.Foo == null);
508+
Assert.AreNotEqual(null, json.Foo);
474509
Assert.IsFalse(json.Bar.B == null);
510+
Assert.AreNotEqual(null, json.Bar.B);
475511
Assert.IsFalse(json.Baz == null);
512+
Assert.AreNotEqual(null, json.Baz);
476513

477514
// Properties are absent
478515
Assert.IsTrue(json.Bar.A == null);
516+
Assert.AreEqual(null, json.Bar.A);
479517
}
480518

481519
[Test]
@@ -489,6 +527,7 @@ public void CanSetOptionalProperty()
489527

490528
// Property is absent
491529
Assert.IsTrue(json.OptionalValue == null);
530+
Assert.AreEqual(null, json.OptionalValue);
492531

493532
json.OptionalValue = 5;
494533

@@ -837,10 +876,13 @@ public void CanDifferentiateBetweenNullAndAbsent()
837876
// GetMember binding mirrors Azure SDK models, so we allow a null check for an optional
838877
// property through the C#-style dynamic interface.
839878
Assert.IsTrue(json.foo == null);
879+
Assert.AreEqual(null, json.foo);
840880
Assert.IsTrue(json.bar == null);
881+
Assert.AreEqual(null, json.bar);
841882

842883
// Indexer lookup mimics JsonNode behavior and so throws if a property is absent.
843884
Assert.IsTrue(json["foo"] == null);
885+
Assert.AreEqual(null, json["foo"]);
844886
Assert.Throws<KeyNotFoundException>(() => _ = json["bar"]);
845887
Assert.Throws<KeyNotFoundException>(() => { if (json["bar"] == null) {; } });
846888
}

‎sdk/core/Azure.Core/tests/public/JsonDataArrayTests.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,7 @@ public void CanSetArrayIndex()
197197
Assert.AreEqual(5, (int)data[0]);
198198
Assert.AreEqual("valid", (string)data[1]);
199199
Assert.IsTrue(data[2] == null);
200+
Assert.AreEqual(null, data[2]);
200201
}
201202

202203
[Test]

‎sdk/core/Azure.Core/tests/public/JsonDataPublicTests.cs‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -338,30 +338,20 @@ public void EqualsPrimitiveValues(string a, string b, bool expected)
338338
public void EqualsNull()
339339
{
340340
dynamic value = JsonDataTestHelpers.CreateFromJson("""{ "foo": null }""");
341-
Assert.IsTrue(value.foo.Equals(null));
341+
Assert.AreEqual(null, value.foo);
342342
Assert.IsTrue(value.foo == null);
343343

344344
string nullString = null;
345345
Assert.IsTrue(value.foo == nullString);
346346
Assert.IsTrue(nullString == value.foo);
347347

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

354352
bool? nullBool = null;
355353
Assert.IsTrue(value.foo == nullBool);
356-
Assert.IsTrue(nullBool == (bool?)value.foo);
357-
358-
// We cannot overload the equality operator with two nullable values, so
359-
// the following is the consequence.
360-
Assert.IsFalse(null == value.foo);
361-
362-
// However, this does give us a backdoor to differentiate between an
363-
// absent property and a property whose JSON value is null, if we wanted to
364-
// use it that way, although it's not really very nice.
354+
Assert.IsTrue(nullBool == value.foo);
365355
}
366356

367357
[Test]

0 commit comments

Comments
 (0)