From a5d274fe5766d77f4b40f23193c939b66b7102c9 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 08:12:17 -0700 Subject: [PATCH 01/14] Initial reworking --- .../Azure.Core/src/DynamicData/DynamicData.cs | 157 ++++- .../src/DynamicData/MutableJsonChange.cs | 87 ++- .../MutableJsonDocument.ChangeTracker.cs | 2 +- .../DynamicData/MutableJsonElement.WriteTo.cs | 113 +++- .../src/DynamicData/MutableJsonElement.cs | 597 ++++++++++++++---- .../MutableJsonDocumentChangeTrackerTests.cs | 2 +- .../DynamicData/MutableJsonDocumentTests.cs | 427 +------------ .../MutableJsonDocumentWriteToTests.cs | 115 ++-- .../DynamicData/MutableJsonElementTests.cs | 76 +-- 9 files changed, 872 insertions(+), 704 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs index 03e2ced1e97be..fe6258261a00e 100644 --- a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs +++ b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs @@ -147,12 +147,12 @@ private IEnumerable GetEnumerable() if (_options.PropertyNameFormat == JsonPropertyNames.UseExact || _element.TryGetProperty(name, out MutableJsonElement _)) { - _element = _element.SetProperty(name, value); + SetPropertyInternal(name, value); return null; } // The dynamic content has a specified property name format. - _element = _element.SetProperty(FormatPropertyName(name), value); + SetPropertyInternal(FormatPropertyName(name), value); // Binding machinery expects the call site signature to return an object return null; @@ -166,12 +166,25 @@ private IEnumerable GetEnumerable() _ => false }; - private object ConvertType(object value) + internal static JsonElement SerializeToJsonElement(object value, JsonSerializerOptions? options = default) { - byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, _serializerOptions); - return JsonDocument.Parse(bytes); + byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, options); + + // Most JsonDocument.Parse calls return a that is backed by one or more ArrayPool + // arrays. Those arrays are not returned until the instance is disposed. + // This is a workaround that allows us to dispose the JsonDocument so that we + // don't leak ArrayPool arrays. +#if NET6_0_OR_GREATER + Utf8JsonReader reader = new(bytes); + return JsonElement.ParseValue(ref reader); +#else + using JsonDocument doc = JsonDocument.Parse(bytes); + return doc.RootElement.Clone(); +#endif } + private JsonElement ConvertType(object value) => SerializeToJsonElement(value, _serializerOptions); + private object? SetViaIndexer(object index, object value) { AllowList.AssertAllowedValue(value); @@ -179,17 +192,147 @@ private object ConvertType(object value) switch (index) { case string propertyName: - _element = _element.SetProperty(propertyName, value); + SetPropertyInternal(propertyName, value); return null; case int arrayIndex: MutableJsonElement element = _element.GetIndexElement(arrayIndex); - element.Set(value); + SetInternal(ref element, value); return new DynamicData(element, _options); } throw new InvalidOperationException($"Tried to access indexer with an unsupported index type: {index}"); } + private void SetPropertyInternal(string name, object value) + { + switch (value) + { + case bool b: + _element = _element.SetProperty(name, b); + break; + case string s: + _element = _element.SetProperty(name, s); + break; + case byte b: + _element = _element.SetProperty(name, b); + break; + case sbyte sb: + _element = _element.SetProperty(name, sb); + break; + case short sh: + _element = _element.SetProperty(name, sh); + break; + case ushort us: + _element = _element.SetProperty(name, us); + break; + case int i: + _element = _element.SetProperty(name, i); + break; + case uint u: + _element = _element.SetProperty(name, u); + break; + case long l: + _element = _element.SetProperty(name, l); + break; + case ulong ul: + _element = _element.SetProperty(name, ul); + break; + case float f: + _element = _element.SetProperty(name, f); + break; + case double d: + _element = _element.SetProperty(name, d); + break; + case decimal d: + _element = _element.SetProperty(name, d); + break; + case DateTime d: + _element = _element.SetProperty(name, d); + break; + case DateTimeOffset d: + _element = _element.SetProperty(name, d); + break; + case Guid g: + _element = _element.SetProperty(name, g); + break; + case null: + _element = _element.SetPropertyNull(name); + break; + case JsonElement e: + _element = _element.SetProperty(name, e); + break; + default: + JsonElement element = ConvertType(value); + _element = _element.SetProperty(name, element); + break; + } + } + + private void SetInternal(ref MutableJsonElement element, object value) + { + switch (value) + { + case bool b: + element.Set(b); + break; + case string s: + element.Set(s); + break; + case byte b: + element.Set(b); + break; + case sbyte sb: + element.Set(sb); + break; + case short sh: + element.Set(sh); + break; + case ushort us: + element.Set(us); + break; + case int i: + element.Set(i); + break; + case uint u: + element.Set(u); + break; + case long l: + element.Set(l); + break; + case ulong ul: + element.Set(ul); + break; + case float f: + element.Set(f); + break; + case double d: + element.Set(d); + break; + case decimal d: + element.Set(d); + break; + case DateTime d: + element.Set(d); + break; + case DateTimeOffset d: + element.Set(d); + break; + case Guid g: + element.Set(g); + break; + case null: + element.SetNull(); + break; + case JsonElement e: + element.Set(e); + break; + default: + JsonElement jsonElement = ConvertType(value); + element.Set(jsonElement); + break; + } + } + private T? ConvertTo() { JsonElement element = _element.GetJsonElement(); diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs index 4fc4cebafa204..3dfd09d52dbf1 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs @@ -2,39 +2,23 @@ // Licensed under the MIT License. using System; -using System.Buffers; using System.Text.Json; namespace Azure.Core.Json { internal struct MutableJsonChange { - private JsonElement? _serializedValue; - private readonly JsonSerializerOptions _serializerOptions; - public MutableJsonChange(string path, int index, object? value, - JsonSerializerOptions options, MutableJsonChangeKind changeKind, string? addedPropertyName) { Path = path; Index = index; Value = value; - _serializerOptions = options; ChangeKind = changeKind; AddedPropertyName = addedPropertyName; - - if (value is JsonElement element) - { - _serializedValue = element; - } - - if (value is JsonDocument doc) - { - _serializedValue = doc.RootElement; - } } public string Path { get; } @@ -47,18 +31,68 @@ public MutableJsonChange(string path, public MutableJsonChangeKind ChangeKind { get; } - public JsonValueKind ValueKind => GetSerializedValue().ValueKind; + public readonly JsonValueKind ValueKind => Value switch + { + bool b => b ? JsonValueKind.True : JsonValueKind.False, + string => JsonValueKind.String, + DateTime => JsonValueKind.String, + DateTimeOffset => JsonValueKind.String, + Guid => JsonValueKind.String, + byte => JsonValueKind.Number, + sbyte => JsonValueKind.Number, + short => JsonValueKind.Number, + ushort => JsonValueKind.Number, + int => JsonValueKind.Number, + uint => JsonValueKind.Number, + long => JsonValueKind.Number, + ulong => JsonValueKind.Number, + float => JsonValueKind.Number, + double => JsonValueKind.Number, + decimal => JsonValueKind.Number, + null => JsonValueKind.Null, + JsonElement e => e.ValueKind, + _ => throw new InvalidCastException() // TODO: fix exception + }; + + private void EnsureArray() + { + if (Value is JsonElement e && e.ValueKind == JsonValueKind.Array) + { + return; + } + + // TODO: improve exception + throw new InvalidOperationException($"Expected an 'Array' type for item."); + } - internal JsonElement GetSerializedValue() + internal int GetArrayLength() { - if (_serializedValue != null) + EnsureArray(); + + if (Value is JsonElement e) { - return _serializedValue.Value; + return e.GetArrayLength(); } - byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(Value, _serializerOptions); - _serializedValue = JsonDocument.Parse(bytes).RootElement; - return _serializedValue.Value; + //TODO + throw new InvalidOperationException(); + } + + internal static JsonElement ConvertToJsonElement(MutableJsonChange change, JsonSerializerOptions options) + { + byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(change.Value, options); + + // Most JsonDocument.Parse calls return a that is backed by one or more ArrayPool + // arrays. Those arrays are not returned until the instance is disposed. + // This is a workaround that allows us to dispose the JsonDocument so that we + // don't leak ArrayPool arrays. +#if NET6_0_OR_GREATER + Utf8JsonReader reader = new(bytes); + return JsonElement.ParseValue(ref reader); +#else + using JsonDocument doc = JsonDocument.Parse(bytes); + return doc.RootElement.Clone(); +#endif } internal bool IsDescendant(string path) @@ -110,7 +144,12 @@ internal bool IsGreaterThan(ReadOnlySpan otherPath) internal string AsString() { - return GetSerializedValue().ToString() ?? "null"; + if (Value is null) + { + return "null"; + } + + return Value.ToString()!; } public override string ToString() diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs index 7cf52ca9708f1..ef2354cad7727 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs @@ -100,7 +100,7 @@ internal int AddChange(string path, object? value, MutableJsonChangeKind changeK int index = _changes.Count; - _changes.Add(new MutableJsonChange(path, index, value, _options, changeKind, addedPropertyName)); + _changes.Add(new MutableJsonChange(path, index, value, changeKind, addedPropertyName)); return index; } diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs index cb5bbd43363f5..d9a436a760284 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs @@ -9,11 +9,6 @@ namespace Azure.Core.Json { internal partial struct MutableJsonElement { - internal void WriteTo(Utf8JsonWriter writer) - { - WriteElement(_path, _highWaterMark, _element, writer); - } - internal void WriteTo(Utf8JsonWriter writer, string format) { switch (format) @@ -29,38 +24,25 @@ internal void WriteTo(Utf8JsonWriter writer, string format) break; } } + internal void WriteTo(Utf8JsonWriter writer) + { + WriteElement(_path, _highWaterMark, _element, writer); + } private void WriteElement(string path, int highWaterMark, JsonElement element, Utf8JsonWriter writer) { if (Changes.TryGetChange(path, highWaterMark, out MutableJsonChange change)) { - switch (change.Value) + if (change.Value is JsonElement changeElement) { - case int i: - writer.WriteNumberValue(i); - return; - case long l: - writer.WriteNumberValue(l); - return; - case double d: - writer.WriteNumberValue(d); - return; - case float f: - writer.WriteNumberValue(f); - return; - case bool b: - writer.WriteBooleanValue(b); - return; - case null: - writer.WriteNullValue(); - return; - default: - break; - - // Note: string is not included to let JsonElement handle escaping. + element = changeElement; + } + else + { + WritePrimitiveChange(change, writer); + return; } - element = change.GetSerializedValue(); highWaterMark = change.Index; } @@ -105,7 +87,14 @@ private void WriteObject(string path, int highWaterMark, JsonElement element, Ut string propertyPath = MutableJsonDocument.ChangeTracker.PushProperty(path, propertyName); writer.WritePropertyName(propertyName); - WriteElement(propertyPath, highWaterMark, property.GetSerializedValue(), writer); + if (property.Value is JsonElement changeElement) + { + WriteElement(propertyPath, highWaterMark, changeElement, writer); + } + else + { + WritePrimitiveChange(property, writer); + } } writer.WriteEndObject(); @@ -124,5 +113,69 @@ private void WriteArray(string path, int highWaterMark, JsonElement element, Utf writer.WriteEndArray(); } + private void WritePrimitiveChange(MutableJsonChange change, Utf8JsonWriter writer) + { + switch (change.Value) + { + case bool b: + writer.WriteBooleanValue(b); + return; + case string s: + writer.WriteStringValue(s); + return; + case byte b: + writer.WriteNumberValue(b); + return; + case sbyte sb: + writer.WriteNumberValue(sb); + return; + case short sh: + writer.WriteNumberValue(sh); + return; + case ushort us: + writer.WriteNumberValue(us); + return; + case int i: + writer.WriteNumberValue(i); + return; + case uint u: + writer.WriteNumberValue(u); + return; + case long l: + writer.WriteNumberValue(l); + return; + case ulong ul: + writer.WriteNumberValue(ul); + return; + case float f: + writer.WriteNumberValue(f); + return; + case double d: + writer.WriteNumberValue(d); + return; + case decimal d: + writer.WriteNumberValue(d); + return; + case DateTime d: + // TODO - test + writer.WriteStringValue(d); + return; + case DateTimeOffset d: + // TODO - test + writer.WriteStringValue(d); + return; + case Guid g: + // TODO - test + writer.WriteStringValue(g); + return; + case null: + writer.WriteNullValue(); + return; + default: + // Change can't have the type it has + // TODO + throw new InvalidOperationException(); + } + } } } diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index f02f682faf3fc..179b970e4a4ba 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -9,6 +9,7 @@ using System.Text.Json; using System.Text.Json.Serialization; +#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member namespace Azure.Core.Json { /// @@ -117,7 +118,7 @@ public bool TryGetProperty(ReadOnlySpan name, out MutableJsonElement value return false; } - value = new MutableJsonElement(_root, change.GetSerializedValue(), GetString(path, 0, pathLength), change.Index); + value = new MutableJsonElement(_root, MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions), GetString(path, 0, pathLength), change.Index); return true; } @@ -157,7 +158,7 @@ public int GetArrayLength() if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - return change.GetSerializedValue().GetArrayLength(); + return change.GetArrayLength(); } return _element.GetArrayLength(); @@ -172,7 +173,7 @@ internal MutableJsonElement GetIndexElement(int index) string path = MutableJsonDocument.ChangeTracker.PushIndex(_path, index); if (Changes.TryGetChange(path, _highWaterMark, out MutableJsonChange change)) { - return new MutableJsonElement(_root, change.GetSerializedValue(), path, change.Index); + return new MutableJsonElement(_root, MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions), path, change.Index); } return new MutableJsonElement(_root, _element[index], path, _highWaterMark); @@ -192,6 +193,12 @@ public bool TryGetDouble(out double value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case double d: @@ -203,7 +210,16 @@ public bool TryGetDouble(out double value) value = default; return false; default: - return change.GetSerializedValue().TryGetDouble(out value); + try + { + value = (double)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a double"); + } } } @@ -245,6 +261,12 @@ public bool TryGetInt32(out int value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case int i: @@ -256,7 +278,16 @@ public bool TryGetInt32(out int value) value = default; return false; default: - return change.GetSerializedValue().TryGetInt32(out value); + try + { + value = (int)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not an int"); + } } } @@ -293,6 +324,12 @@ public bool TryGetInt64(out long value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case long l: @@ -304,7 +341,16 @@ public bool TryGetInt64(out long value) value = default; return false; default: - return change.GetSerializedValue().TryGetInt64(out value); + try + { + value = (long)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a long"); + } } } @@ -341,6 +387,12 @@ public bool TryGetSingle(out float value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case float f: @@ -352,7 +404,16 @@ public bool TryGetSingle(out float value) value = default; return false; default: - return change.GetSerializedValue().TryGetSingle(out value); + try + { + value = (float)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a float"); + } } } @@ -395,11 +456,6 @@ public float GetSingle() case null: return null; default: - JsonElement el = change.GetSerializedValue(); - if (el.ValueKind == JsonValueKind.String) - { - return el.GetString(); - } throw new InvalidOperationException($"Element at '{_path}' is not a string."); } } @@ -435,6 +491,12 @@ public bool TryGetByte(out byte value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case byte b: @@ -446,7 +508,16 @@ public bool TryGetByte(out byte value) value = default; return false; default: - return change.GetSerializedValue().TryGetByte(out value); + try + { + value = (byte)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a byte"); + } } } @@ -469,18 +540,29 @@ public bool TryGetDateTime(out DateTime value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.String) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case DateTime d: value = d; return true; + case DateTimeOffset: + case string: + MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions).TryGetDateTime(out value); + return true; case JsonElement element: return element.TryGetDateTime(out value); case null: value = default; return false; default: - return change.GetSerializedValue().TryGetDateTime(out value); + // TODO + throw new InvalidOperationException("Changed element is not a DateTime"); } } @@ -503,19 +585,29 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.String) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case DateTimeOffset o: value = o; return true; - ; + case DateTime: + case string: + MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions).TryGetDateTimeOffset(out value); + return true; case JsonElement element: return element.TryGetDateTimeOffset(out value); case null: value = default; return false; default: - return change.GetSerializedValue().TryGetDateTimeOffset(out value); + // TODO + throw new InvalidOperationException("Changed element is not a DateTime"); } } @@ -538,6 +630,12 @@ public bool TryGetDecimal(out decimal value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case decimal d: @@ -549,7 +647,16 @@ public bool TryGetDecimal(out decimal value) value = default; return false; default: - return change.GetSerializedValue().TryGetDecimal(out value); + try + { + value = (decimal)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a decimal"); + } } } @@ -572,18 +679,28 @@ public bool TryGetGuid(out Guid value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.String) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case Guid g: value = g; return true; + case string: + MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions).TryGetGuid(out value); + return true; case JsonElement element: return element.TryGetGuid(out value); case null: value = default; return false; default: - return change.GetSerializedValue().TryGetGuid(out value); + // TODO + throw new InvalidOperationException("Changed element is not a Guid."); } } @@ -606,6 +723,12 @@ public bool TryGetInt16(out short value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case short s: @@ -617,7 +740,16 @@ public bool TryGetInt16(out short value) value = default; return false; default: - return change.GetSerializedValue().TryGetInt16(out value); + try + { + value = (short)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a short"); + } } } @@ -640,6 +772,12 @@ public bool TryGetSByte(out sbyte value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case sbyte b: @@ -651,7 +789,16 @@ public bool TryGetSByte(out sbyte value) value = default; return false; default: - return change.GetSerializedValue().TryGetSByte(out value); + try + { + value = (sbyte)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not an sbyte"); + } } } @@ -674,6 +821,12 @@ public bool TryGetUInt16(out ushort value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case ushort u: @@ -685,7 +838,16 @@ public bool TryGetUInt16(out ushort value) value = default; return false; default: - return change.GetSerializedValue().TryGetUInt16(out value); + try + { + value = (ushort)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a ushort."); + } } } @@ -708,6 +870,12 @@ public bool TryGetUInt32(out uint value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case uint d: @@ -719,7 +887,16 @@ public bool TryGetUInt32(out uint value) value = default; return false; default: - return change.GetSerializedValue().TryGetUInt32(out value); + try + { + value = (uint)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a unint"); + } } } @@ -742,6 +919,12 @@ public bool TryGetUInt64(out ulong value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + if (change.ValueKind != JsonValueKind.Number) + { + // TODO + throw new InvalidOperationException(); + } + switch (change.Value) { case ulong u: @@ -753,7 +936,16 @@ public bool TryGetUInt64(out ulong value) value = default; return false; default: - return change.GetSerializedValue().TryGetUInt64(out value); + try + { + value = (ulong)change.Value; + return true; + } + catch (InvalidCastException) + { + // TODO + throw new InvalidOperationException("Changed element is not a ulong"); + } } } @@ -773,7 +965,7 @@ public ulong GetUInt64() /// /// Gets an enumerator to enumerate the values in the JSON array represented by this MutableJsonElement. /// - public ArrayEnumerator EnumerateArray() + internal ArrayEnumerator EnumerateArray() { EnsureValid(); @@ -785,7 +977,7 @@ public ArrayEnumerator EnumerateArray() /// /// Gets an enumerator to enumerate the properties in the JSON object represented by this JsonElement. /// - public ObjectEnumerator EnumerateObject() + internal ObjectEnumerator EnumerateObject() { EnsureValid(); @@ -794,34 +986,6 @@ public ObjectEnumerator EnumerateObject() return new ObjectEnumerator(this); } - /// - /// Set the value of the property with the specified name to the passed-in value. If the property is not already present, it will be created. - /// - /// - /// The value to assign to the element. - public MutableJsonElement SetProperty(string name, object value) - { - if (TryGetProperty(name, out MutableJsonElement element)) - { - element.Set(value); - return this; - } - -#if !NET6_0_OR_GREATER - // Earlier versions of JsonSerializer.Serialize include "RootElement" - // as a property when called on JsonDocument. - if (value is JsonDocument doc) - { - value = doc.RootElement; - } -#endif - - // It is a new property. - string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); - Changes.AddChange(path, GetSerializedValue(value), MutableJsonChangeKind.PropertyAddition, name); - return this; - } - /// /// Remove the property with the specified name from the current MutableJsonElement. /// @@ -853,6 +1017,19 @@ public void Set(double value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, double value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -863,6 +1040,18 @@ public void Set(int value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, int value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } /// /// Sets the value of this element to the passed-in value. @@ -875,6 +1064,19 @@ public void Set(long value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, long value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -885,6 +1087,18 @@ public void Set(float value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, float value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } /// /// Sets the value of this element to the passed-in value. @@ -897,6 +1111,39 @@ public void Set(string value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, string value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + + public void SetNull() + { + EnsureValid(); + + Changes.AddChange(_path, null); + } + + public MutableJsonElement SetPropertyNull(string name) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.SetNull(); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, null, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -908,6 +1155,19 @@ public void Set(bool value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, bool value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -919,6 +1179,19 @@ public void Set(byte value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, byte value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -930,6 +1203,19 @@ public void Set(sbyte value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, sbyte value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -940,6 +1226,18 @@ public void Set(short value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, short value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } /// /// Sets the value of this element to the passed-in value. @@ -952,6 +1250,19 @@ public void Set(ushort value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, ushort value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -963,6 +1274,19 @@ public void Set(uint value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, uint value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -974,6 +1298,19 @@ public void Set(ulong value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, ulong value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -985,6 +1322,19 @@ public void Set(decimal value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, decimal value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -995,6 +1345,18 @@ public void Set(Guid value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, Guid value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } /// /// Sets the value of this element to the passed-in value. @@ -1007,6 +1369,19 @@ public void Set(DateTime value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, DateTime value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// @@ -1018,98 +1393,41 @@ public void Set(DateTimeOffset value) Changes.AddChange(_path, value); } + public MutableJsonElement SetProperty(string name, DateTimeOffset value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; + } + /// /// Sets the value of this element to the passed-in value. /// /// The value to assign to the element. - public void Set(object value) + public void Set(JsonElement value) { EnsureValid(); - switch (value) - { - case bool b: - Set(b); - break; - case string s: - Set(s); - break; - case byte b: - Set(b); - break; - case sbyte sb: - Set(sb); - break; - case short sh: - Set(sh); - break; - case ushort us: - Set(us); - break; - case int i: - Set(i); - break; - case uint u: - Set(u); - break; - case long l: - Set(l); - break; - case ulong ul: - Set(ul); - break; - case float f: - Set(f); - break; - case double d: - Set(d); - break; - case decimal d: - Set(d); - break; - case DateTime d: - Set(d); - break; - case DateTimeOffset d: - Set(d); - break; - case Guid g: - Set(g); - break; - case null: - Changes.AddChange(_path, null); - break; - default: - Changes.AddChange(_path, GetSerializedValue(value)); - break; - } - } - - private object GetSerializedValue(object value) - { - if (value is JsonDocument doc) - { - return doc.RootElement; - } - - if (value is JsonElement element) - { - return element; - } - - if (value is MutableJsonDocument mjd) - { - mjd.RootElement.EnsureValid(); - } - - if (value is MutableJsonElement mje) - { - mje.EnsureValid(); - } - - // If it's not a special type, we'll serialize it on assignment. - byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, _root.SerializerOptions); - return JsonDocument.Parse(bytes).RootElement; + Changes.AddChange(_path, value); + } + + public MutableJsonElement SetProperty(string name, JsonElement value) + { + if (TryGetProperty(name, out MutableJsonElement element)) + { + element.Set(value); + return this; + } + + string path = MutableJsonDocument.ChangeTracker.PushProperty(_path, name); + Changes.AddChange(path, value, MutableJsonChangeKind.PropertyAddition, name); + return this; } /// @@ -1137,7 +1455,7 @@ internal JsonElement GetJsonElement() if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - return change.GetSerializedValue(); + return MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions); } // Account for changes to descendants of this element as well @@ -1218,4 +1536,5 @@ public override void Write(Utf8JsonWriter writer, MutableJsonElement value, Json } } } +#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member } diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs index 0ccf4bf9b15a7..37bfbe4e82022 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs @@ -69,7 +69,7 @@ public void CanGetSortedChanges() #region Helpers private MutableJsonChange CreateChange(string name) { - return new MutableJsonChange(name, -1, null, null, MutableJsonChangeKind.PropertyUpdate, null); + return new MutableJsonChange(name, -1, null, MutableJsonChangeKind.PropertyUpdate, null); } #endregion } diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs index 5f0e565487363..a3f3adcd4e3ac 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs @@ -5,6 +5,7 @@ using System.IO; using System.Text.Json; using Azure.Core.Json; +using Azure.Core.Serialization; using NUnit.Framework; namespace Azure.Core.Tests @@ -165,61 +166,6 @@ public void JsonWithDelimiterIsInvalidJson() Assert.IsTrue(threwJsonException); } - [Test] - public void CanSetPropertyToMutableJsonDocument() - { - string json = """ - { - "Foo" : "Hello" - } - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - MutableJsonDocument childMDoc = MutableJsonDocument.Parse("""{ "Baz": "hi" }"""); - - mdoc.RootElement.GetProperty("Foo").Set(childMDoc); - - Assert.AreEqual("hi", mdoc.RootElement.GetProperty("Foo").GetProperty("Baz").GetString()); - - string expected = """ - { - "Foo" : { - "Baz" : "hi" - } - } - """; - - ValidateWriteTo(expected, mdoc); - } - - [Test] - public void CanSetPropertyToJsonDocument() - { - string json = """ - { - "Foo" : "Hello" - } - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - JsonDocument doc = JsonDocument.Parse("""{ "Baz": "hi" }"""); - - mdoc.RootElement.GetProperty("Foo").Set(doc); - - Assert.AreEqual("hi", mdoc.RootElement.GetProperty("Foo").GetProperty("Baz").GetString()); - - string expected = """ - { - "Foo" : { - "Baz" : "hi" - } - } - """; - - ValidateWriteTo(expected, mdoc); - } - [Test] public void CanAddPropertyToRootObject() { @@ -303,63 +249,6 @@ public void CanAddPropertyToObject() ValidateWriteTo(expected, mdoc); } - [Test] - public void CanAddMutableJsonDocumentProperty() - { - string json = """ - { - "Foo" : "Hello" - } - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - MutableJsonDocument anotherMDoc = MutableJsonDocument.Parse("""{ "Baz": "hi" }"""); - - mdoc.RootElement.SetProperty("A", anotherMDoc); - - Assert.AreEqual("hi", mdoc.RootElement.GetProperty("A").GetProperty("Baz").GetString()); - - string expected = """ - { - "Foo" : "Hello", - "A" : { - "Baz" : "hi" - } - } - """; - - ValidateWriteTo(expected, mdoc); - } - - [Test] - public void CanAddJsonDocumentProperty() - { - string json = """ - { - "Foo" : "Hello" - } - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - JsonDocument doc = JsonDocument.Parse("""{ "Baz": "hi" }"""); - - mdoc.RootElement.SetProperty("A", doc); - - Assert.AreEqual("hi", mdoc.RootElement.GetProperty("A").GetProperty("Baz").GetString()); - - string expected = """ - { - "Foo" : "Hello", - "A" : { - "Baz" : "hi" - } - } - """; - - ValidateWriteTo(expected, mdoc); - } - [Test] public void CanRemovePropertyFromRootObject() { @@ -451,9 +340,10 @@ public void CanReplaceObjectWithAnonymousType() } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - mdoc.RootElement.GetProperty("Baz").Set(new { B = 5.5 }); + JsonElement element = DynamicData.SerializeToJsonElement(new { B = 5.5 }); + mdoc.RootElement.GetProperty("Baz").Set(element); // Assert: @@ -611,67 +501,6 @@ public void CanChangeArrayElementType() ValidateWriteTo(expected, mdoc); } - [Test] - public void ChangeToDocumentAppearsInElementReference() - { - // This tests reference semantics. - - string json = """[ { "Foo" : 4 } ]"""; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - // a is a reference to the 0th element; a's path is "0" - MutableJsonElement a = mdoc.RootElement.GetIndexElement(0); - - // resets json to equivalent of "[ 5 ]" - mdoc.RootElement.GetIndexElement(0).Set(5); - - Assert.AreEqual(5, mdoc.RootElement.GetIndexElement(0).GetInt32()); - - // a's path is "0" so a.GetInt32() should return 5. - Assert.AreEqual(5, a.GetInt32()); - - // The following should throw because json[0] is now 5 and not an object. - Assert.Throws(() => a.GetProperty("Foo").Set(6)); - - Assert.AreEqual(5, mdoc.RootElement.GetIndexElement(0).GetInt32()); - - // Setting json[0] back to 'a' makes it 5 again. - mdoc.RootElement.GetIndexElement(0).Set(a); - - Assert.AreEqual(5, mdoc.RootElement.GetIndexElement(0).GetInt32()); - - // Type round-trips correctly. - BinaryData buffer = GetWriteToBuffer(mdoc); - JsonDocument doc = JsonDocument.Parse(buffer); - - Assert.AreEqual(5, doc.RootElement[0].GetInt32()); - - string expected = """[ 5 ]"""; - ValidateWriteTo(expected, mdoc); - } - - [Test] - public void ChangeToChildDocumentDoesNotAppearInParentDocument() - { - // This tests value semantics. - - MutableJsonDocument mdoc = MutableJsonDocument.Parse("{}"); - MutableJsonDocument child = MutableJsonDocument.Parse("{}"); - - mdoc.RootElement.SetProperty("a", child); - Assert.AreEqual("""{"a":{}}""", mdoc.RootElement.ToString()); - Assert.AreEqual("""{}""", child.RootElement.ToString()); - - child.RootElement.SetProperty("b", 2); - - Assert.AreEqual("""{"a":{}}""", mdoc.RootElement.ToString()); - Assert.AreEqual("""{"b":2}""", child.RootElement.ToString()); - - ValidateWriteTo("""{"a":{}}""", mdoc); - ValidateWriteTo("""{"b":2}""", child); - } - [Test] public void ChangeToAddedElementReferenceAppearsInDocument() { @@ -700,244 +529,6 @@ public void ChangeToAddedElementReferenceAppearsInDocument() ValidateWriteTo(expected, mdoc); } - [Test] - public void ChangeToElementReferenceAppearsInDocument() - { - // This tests reference semantics. - - string json = """[ { "Foo" : 4 } ]"""; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - // a is a reference to the 0th element; a's path is "0" - MutableJsonElement a = mdoc.RootElement.GetIndexElement(0); - - a.GetProperty("Foo").Set(new - { - Bar = 5 - }); - - Assert.AreEqual(5, a.GetProperty("Foo").GetProperty("Bar").GetInt32()); - Assert.AreEqual(5, mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetProperty("Bar").GetInt32()); - - string expected = """ - [ { - "Foo" : { - "Bar" : 5 - } - } ] - """; - - ValidateWriteTo(expected, mdoc); - } - - [Test] - public void CanInvalidateElement() - { - string json = """ - [{ - "Foo" : { - "A": 6 - } - }] - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - // a's path points to "0.Foo.A" - var a = mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetProperty("A"); - - // resets json to equivalent of "[ 5 ]" - mdoc.RootElement.GetIndexElement(0).Set(5); - - Assert.AreEqual(5, mdoc.RootElement.GetIndexElement(0).GetInt32()); - - // a's path points to "0.Foo.A" so a.GetInt32() should throw since this - // in an invalid path. - Assert.Throws(() => a.GetInt32()); - - // Setting json[0] to a should throw as well, as the element doesn't point - // to a valid path in the mutated JSON tree. - Assert.Throws(() => mdoc.RootElement.GetIndexElement(0).Set(a)); - - // 3. Type round-trips correctly. - BinaryData buffer = GetWriteToBuffer(mdoc); - JsonDocument doc = JsonDocument.Parse(buffer); - - Assert.AreEqual(5, doc.RootElement[0].GetInt32()); - - string expected = """[ 5 ]"""; - ValidateWriteTo(expected, mdoc); - } - - [Test] - public void CanAccessPropertyInChangedStructure() - { - string json = """ - [ { - "foo" : { - "a": 6 - } - } ] - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - // a's path points to "0.foo.a" - MutableJsonElement a = mdoc.RootElement.GetIndexElement(0).GetProperty("foo").GetProperty("a"); - - // resets json to equivalent of "[ 5 ]" - mdoc.RootElement.GetIndexElement(0).Set(5); - - Assert.AreEqual(5, mdoc.RootElement.GetIndexElement(0).GetInt32()); - - // a's path points to "0.foo.a" so a.GetInt32() should throws since this now an invalid path. - Assert.Throws(() => a.GetInt32()); - - // Since a is invalid now, we can't set it on mdoc. - Assert.Throws(() => mdoc.RootElement.GetIndexElement(0).Set(a)); - - // Reset json[0] to an object - mdoc.RootElement.GetIndexElement(0).Set(new - { - foo = new - { - a = 7 - } - }); - - // We should be able to get the value of 0.foo.a without being tripped up by earlier changes. - int aValue = mdoc.RootElement.GetIndexElement(0).GetProperty("foo").GetProperty("a").GetInt32(); - Assert.AreEqual(7, aValue); - - // 3. Type round-trips correctly. - BinaryData buffer = GetWriteToBuffer(mdoc); - JsonDocument doc = JsonDocument.Parse(buffer); - - string expected = """ - [ { - "foo" : { - "a": 7 - } - } ] - """; - - ValidateWriteTo(expected, mdoc); - } - - [Test] - public void CanAccessChangesInDifferentBranches() - { - string json = """ - [ { - "Foo" : { - "A": 6 - } - }, - { - "Bar" : "hi" - }] - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - // resets json to equivalent of "[ 5, ... ]" - mdoc.RootElement.GetIndexElement(0).Set(5); - - Assert.AreEqual(5, mdoc.RootElement.GetIndexElement(0).GetInt32()); - Assert.AreEqual("hi", mdoc.RootElement.GetIndexElement(1).GetProperty("Bar").GetString()); - - // Make a structural change to json[0] but not json[1] - mdoc.RootElement.GetIndexElement(0).Set(new - { - Foo = new - { - A = 7 - } - }); - - // We should be able to get the value of A without being tripped up by earlier changes. - // We should also be able to get the value of json[1] without it having been invalidated. - Assert.AreEqual(7, mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetProperty("A").GetInt32()); - Assert.AreEqual("hi", mdoc.RootElement.GetIndexElement(1).GetProperty("Bar").GetString()); - - // Now change json[1] - mdoc.RootElement.GetIndexElement(1).GetProperty("Bar").Set("new"); - Assert.AreEqual("new", mdoc.RootElement.GetIndexElement(1).GetProperty("Bar").GetString()); - - // 3. Type round-trips correctly. - string expected = """ - [ { - "Foo" : { - "A": 7 - } - }, - { - "Bar" : "new" - }] - """; - - ValidateWriteTo(expected, mdoc); - } - - [Test] - public void PriorChangeToReplacedPropertyIsIgnored() - { - string json = """ - { - "ArrayProperty": [ - { - "Foo" : { - "A": 6 - } - } - ], - "Bar" : "hi" - } - """; - - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - mdoc.RootElement.GetProperty("ArrayProperty").GetIndexElement(0).GetProperty("Foo").GetProperty("A").Set(8); - - Assert.AreEqual(8, mdoc.RootElement.GetProperty("ArrayProperty").GetIndexElement(0).GetProperty("Foo").GetProperty("A").GetInt32()); - - // resets json to equivalent of "[ 5 ]" - mdoc.RootElement.GetProperty("ArrayProperty").GetIndexElement(0).Set(5); - - Assert.AreEqual(5, mdoc.RootElement.GetProperty("ArrayProperty").GetIndexElement(0).GetInt32()); - - // Reset json[0] to an object - mdoc.RootElement.GetProperty("ArrayProperty").GetIndexElement(0).Set(new - { - Foo = new - { - A = 7 - } - }); - - // We should be able to get the value of A without being tripped up - // by earlier changes. - int aValue = mdoc.RootElement.GetProperty("ArrayProperty").GetIndexElement(0).GetProperty("Foo").GetProperty("A").GetInt32(); - Assert.AreEqual(7, aValue); - - // 3. Type round-trips correctly. - string expected = """ - { - "ArrayProperty": [ - { - "Foo" : { - "A": 7 - } - } - ], - "Bar" : "hi" - } - """; - - ValidateWriteTo(expected, mdoc); - } - [Test] public void CanSetProperty_StringToNumber() { @@ -991,14 +582,15 @@ public void CanSetProperty_StringToObject() { string json = """{ "Foo" : "hi" }"""; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); Assert.AreEqual("hi", mdoc.RootElement.GetProperty("Foo").GetString()); - mdoc.RootElement.GetProperty("Foo").Set(new + JsonElement element = DynamicData.SerializeToJsonElement(new { Bar = 6 }); + mdoc.RootElement.GetProperty("Foo").Set(element); Assert.AreEqual(6, mdoc.RootElement.GetProperty("Foo").GetProperty("Bar").GetInt32()); @@ -1017,11 +609,12 @@ public void CanSetProperty_StringToArray() { string json = """[ { "Foo" : "hi" } ]"""; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); Assert.AreEqual("hi", mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetString()); - mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").Set(new int[] { 1, 2, 3 }); + JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 1, 2, 3 }); + mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").Set(element); Assert.AreEqual(1, mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetIndexElement(0).GetInt32()); Assert.AreEqual(2, mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetIndexElement(1).GetInt32()); diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs index 9febd58087a96..4ac1674479e4e 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Text.Json; using Azure.Core.Json; +using Azure.Core.Serialization; using NUnit.Framework; namespace Azure.Core.Tests @@ -47,10 +48,21 @@ public void CanWriteDateTimeAppConfigValue() """u8; BinaryData data = new(json.ToArray()); - MutableJsonDocument mdoc = MutableJsonDocument.Parse(data); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(data); mdoc.RootElement.GetProperty("foo").Set("hi"); MutableJsonDocumentTests.ValidateWriteTo(data, mdoc); + + ReadOnlySpan json2 = """ + { + "foo": "hi", + "last_modified":"2023-03-23T16:35:35+00:00" + } + """u8; + BinaryData data2 = new(json2.ToArray()); + mdoc.RootElement.GetProperty("last_modified").Set("2023-03-23T16:35:35+00:00"); + + MutableJsonDocumentTests.ValidateWriteTo(data2, mdoc); } [Test] @@ -321,7 +333,7 @@ public void CanWriteObjectWithChangesAndAdditions() } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("Bar").SetProperty("c", "new"); mdoc.RootElement.GetProperty("Bar").GetProperty("a").Set(1.2); @@ -339,7 +351,8 @@ public void CanWriteObjectWithChangesAndAdditions() MutableJsonDocumentTests.ValidateWriteTo(expected, mdoc); - mdoc.RootElement.SetProperty("Baz", new int[] { 1, 2, 3 }); + JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 1, 2, 3 }); + mdoc.RootElement.SetProperty("Baz", element); expected = """ { @@ -396,8 +409,7 @@ public void CanWriteChangesInterleavedAcrossBranches() """; MutableJsonDocumentTests.ValidateWriteTo(expected, mdoc); - - var value = new + JsonElement element = DynamicData.SerializeToJsonElement(new { Foo = new { @@ -408,9 +420,9 @@ public void CanWriteChangesInterleavedAcrossBranches() b = 4, c = "new" } - }; + }); - mdoc.RootElement.Set(value); + mdoc.RootElement.Set(element); expected = """ { @@ -437,7 +449,7 @@ public void CanWriteArrayWithChangedElements() } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("Bar").GetIndexElement(0).Set(2); mdoc.RootElement.GetProperty("Bar").GetIndexElement(1).Set(4); @@ -452,7 +464,8 @@ public void CanWriteArrayWithChangedElements() MutableJsonDocumentTests.ValidateWriteTo(expected, mdoc); - mdoc.RootElement.GetProperty("Bar").Set(new int[] { 0, 1, 2, 3 }); + JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 0, 1, 2, 3 }); + mdoc.RootElement.GetProperty("Bar").Set(element); mdoc.RootElement.GetProperty("Bar").GetIndexElement(3).Set(4); expected = """ @@ -535,8 +548,9 @@ public void WriteToBehaviorMatchesJsonDocument(dynamic json) // Mutate a value string name = mdoc.RootElement.EnumerateObject().First().Name; - var value = mdoc.RootElement.EnumerateObject().First().Value; - mdoc.RootElement.GetProperty(name).Set(value); + MutableJsonElement value = mdoc.RootElement.EnumerateObject().First().Value; + JsonElement element = DynamicData.SerializeToJsonElement(value); + mdoc.RootElement.GetProperty(name).Set(element); // Validate after changes. MutableJsonDocumentTests.ValidateWriteTo(json, mdoc); @@ -566,28 +580,28 @@ public void CanWriteByte() Assert.AreEqual($"{44}", mdoc.RootElement.GetProperty("bar").ToString()); } - [TestCaseSource(nameof(NumberValues))] - public void CanWriteNumber(string serializedX, T x, T y, T z) - { - string json = $"{{\"foo\" : {serializedX}}}"; + //[TestCaseSource(nameof(NumberValues))] + //public void CanWriteNumber(string serializedX, T x, T y, T z) + //{ + // string json = $"{{\"foo\" : {serializedX}}}"; - // Get from parsed JSON - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + // // Get from parsed JSON + // using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - // Get from parsed JSON - Assert.AreEqual($"{x}", mdoc.RootElement.GetProperty("foo").ToString()); - MutableJsonDocumentTests.ValidateWriteTo(json, mdoc); + // // Get from parsed JSON + // Assert.AreEqual($"{x}", mdoc.RootElement.GetProperty("foo").ToString()); + // MutableJsonDocumentTests.ValidateWriteTo(json, mdoc); - // Get from assigned existing value - mdoc.RootElement.GetProperty("foo").Set(y); - Assert.AreEqual($"{y}", mdoc.RootElement.GetProperty("foo").ToString()); - MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\" : {y}}}", mdoc); + // // Get from assigned existing value + // mdoc.RootElement.GetProperty("foo").Set(y); + // Assert.AreEqual($"{y}", mdoc.RootElement.GetProperty("foo").ToString()); + // MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\" : {y}}}", mdoc); - // Get from added value - mdoc.RootElement.SetProperty("bar", z); - Assert.AreEqual($"{z}", mdoc.RootElement.GetProperty("bar").ToString()); - MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\":{y},\"bar\":{z}}}", mdoc); - } + // // Get from added value + // mdoc.RootElement.SetProperty("bar", z); + // Assert.AreEqual($"{z}", mdoc.RootElement.GetProperty("bar").ToString()); + // MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\":{y},\"bar\":{z}}}", mdoc); + //} [Test] public void CanWriteGuid() @@ -862,11 +876,12 @@ public void CanWritePatchInterleaveParentAndChildChanges() } } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("a").GetProperty("aa").Set(3); mdoc.RootElement.GetProperty("b").GetProperty("ba").Set("3"); - mdoc.RootElement.GetProperty("b").Set(new { ba = "3", bb = "4" }); + JsonElement element = DynamicData.SerializeToJsonElement(new { ba = "3", bb = "4" }); + mdoc.RootElement.GetProperty("b").Set(element); mdoc.RootElement.GetProperty("a").GetProperty("ab").Set(4); mdoc.RootElement.GetProperty("b").GetProperty("ba").Set("5"); mdoc.RootElement.GetProperty("a").GetProperty("aa").Set(5); @@ -1022,10 +1037,11 @@ public void CanWritePatchForArraysAtDifferentLevels() } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(2); mdoc.RootElement.GetProperty("b").GetProperty("bb").GetProperty("bbb").GetIndexElement(1).Set(true); - mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(new { cd = "cd" }); + JsonElement element = DynamicData.SerializeToJsonElement(new { cd = "cd" }); + mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(element); ValidatePatch(""" { @@ -1063,11 +1079,12 @@ public void CanWritePatchInterleaveArrayChanges() } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(2); mdoc.RootElement.GetProperty("b").GetProperty("bb").GetProperty("bbb").GetIndexElement(1).Set(true); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(3); - mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(new { cd = "cd" }); + JsonElement element = DynamicData.SerializeToJsonElement(new { cd = "cd" }); + mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(element); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(1).Set(4); ValidatePatch(""" @@ -1109,8 +1126,10 @@ public void CanWritePatchInterleaveParentAndChildArrayChanges() MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(2); mdoc.RootElement.GetProperty("b").GetProperty("bb").GetProperty("bbb").GetIndexElement(1).Set(true); - mdoc.RootElement.GetProperty("a").GetProperty("aa").Set(new int[] { 2, 3 }); - mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(new { cd = "cd" }); + JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 2, 3 }); + mdoc.RootElement.GetProperty("a").GetProperty("aa").Set(element); + element = DynamicData.SerializeToJsonElement(new { cd = "cd" }); + mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(element); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(1).Set(4); ValidatePatch(""" @@ -1144,9 +1163,9 @@ public void CanWritePatchReplaceObject() } } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - mdoc.RootElement.GetProperty("a").Set(new { aa = 3, ab = 4 }); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + JsonElement element = DynamicData.SerializeToJsonElement(new { aa = 3, ab = 4 }); + mdoc.RootElement.GetProperty("a").Set(element); ValidatePatch(""" { @@ -1173,9 +1192,9 @@ public void CanWritePatchReplaceObject_Deletions() } } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - mdoc.RootElement.GetProperty("a").Set(new { ac = 3 }); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + JsonElement element = DynamicData.SerializeToJsonElement(new { ac = 3 }); + mdoc.RootElement.GetProperty("a").Set(element); ValidatePatch(""" { @@ -1257,9 +1276,10 @@ public void CanWritePatchAddObject() } } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - mdoc.RootElement.SetProperty("c", new { ca = true, cb = false }); + JsonElement element = DynamicData.SerializeToJsonElement(new { ca = true, cb = false }); + mdoc.RootElement.SetProperty("c", element); ValidatePatch(""" { @@ -1473,12 +1493,13 @@ public void CanWritePatchRfc7396SecondExample() "content": "This will be unchanged" } """; - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("title").Set("Hello!"); mdoc.RootElement.SetProperty("phoneNumber", "+01-123-456-7890"); mdoc.RootElement.GetProperty("author").RemoveProperty("familyName"); - mdoc.RootElement.SetProperty("tags", new string[] { "example" }); + JsonElement element = DynamicData.SerializeToJsonElement(new string[] { "example" }); + mdoc.RootElement.SetProperty("tags", element); ValidatePatch(""" { diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs index 92b9224fa7326..6c9d157ab0380 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs @@ -381,44 +381,44 @@ public void CanGetByte() Assert.Throws(() => mdoc.RootElement.GetProperty("foo").GetByte()); } - [TestCaseSource(nameof(NumberValues))] - public void CanGetNumber(string serializedX, T x, T y, T z, U invalid, Func tryGet, Func get) - { - string json = $"{{\"foo\" : {serializedX}}}"; - - // Get from parsed JSON - MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); - Assert.AreEqual(x, tryGet(mdoc.RootElement.GetProperty("foo")).Value); - Assert.AreEqual(x, get(mdoc.RootElement.GetProperty("foo"))); - - // Get from assigned existing value - mdoc.RootElement.GetProperty("foo").Set(y); - - Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); - Assert.AreEqual(y, tryGet(mdoc.RootElement.GetProperty("foo")).Value); - Assert.AreEqual(y, get(mdoc.RootElement.GetProperty("foo"))); - - // Get from added value - mdoc.RootElement.SetProperty("bar", z); - Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("bar")).TryGet); - Assert.AreEqual(z, tryGet(mdoc.RootElement.GetProperty("bar")).Value); - Assert.AreEqual(z, get(mdoc.RootElement.GetProperty("bar"))); - - // Doesn't work if number change is outside range - if (invalid is bool testRange && testRange) - { - mdoc.RootElement.GetProperty("foo").Set(invalid); - Assert.IsFalse(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); - Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); - } - - // Doesn't work for non-number change - mdoc.RootElement.GetProperty("foo").Set("string"); - Assert.Throws(() => tryGet(mdoc.RootElement.GetProperty("foo"))); - Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); - } + //[TestCaseSource(nameof(NumberValues))] + //public void CanGetNumber(string serializedX, T x, T y, T z, U invalid, Func tryGet, Func get) + //{ + // string json = $"{{\"foo\" : {serializedX}}}"; + + // // Get from parsed JSON + // MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + + // Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); + // Assert.AreEqual(x, tryGet(mdoc.RootElement.GetProperty("foo")).Value); + // Assert.AreEqual(x, get(mdoc.RootElement.GetProperty("foo"))); + + // // Get from assigned existing value + // mdoc.RootElement.GetProperty("foo").Set(y); + + // Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); + // Assert.AreEqual(y, tryGet(mdoc.RootElement.GetProperty("foo")).Value); + // Assert.AreEqual(y, get(mdoc.RootElement.GetProperty("foo"))); + + // // Get from added value + // mdoc.RootElement.SetProperty("bar", z); + // Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("bar")).TryGet); + // Assert.AreEqual(z, tryGet(mdoc.RootElement.GetProperty("bar")).Value); + // Assert.AreEqual(z, get(mdoc.RootElement.GetProperty("bar"))); + + // // Doesn't work if number change is outside range + // if (invalid is bool testRange && testRange) + // { + // mdoc.RootElement.GetProperty("foo").Set(testRange); + // Assert.IsFalse(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); + // Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); + // } + + // // Doesn't work for non-number change + // mdoc.RootElement.GetProperty("foo").Set("string"); + // Assert.Throws(() => tryGet(mdoc.RootElement.GetProperty("foo"))); + // Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); + //} [Test] public void CanGetGuid() From e76c8dde71adfcfa0e99994ffdc8cbc5bb648775 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 08:26:19 -0700 Subject: [PATCH 02/14] Move static method to MJE --- .../Azure.Core/src/DynamicData/DynamicData.cs | 20 ++---------- .../src/DynamicData/MutableJsonChange.cs | 17 ---------- .../src/DynamicData/MutableJsonElement.cs | 31 ++++++++++++++----- .../MutableJsonDocumentChangeTrackerTests.cs | 3 -- .../DynamicData/MutableJsonDocumentTests.cs | 6 ++-- .../MutableJsonDocumentWriteToTests.cs | 26 ++++++++-------- 6 files changed, 41 insertions(+), 62 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs index fe6258261a00e..1a978bcf7a0f2 100644 --- a/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs +++ b/sdk/core/Azure.Core/src/DynamicData/DynamicData.cs @@ -166,24 +166,8 @@ private IEnumerable GetEnumerable() _ => false }; - internal static JsonElement SerializeToJsonElement(object value, JsonSerializerOptions? options = default) - { - byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, options); - - // Most JsonDocument.Parse calls return a that is backed by one or more ArrayPool - // arrays. Those arrays are not returned until the instance is disposed. - // This is a workaround that allows us to dispose the JsonDocument so that we - // don't leak ArrayPool arrays. -#if NET6_0_OR_GREATER - Utf8JsonReader reader = new(bytes); - return JsonElement.ParseValue(ref reader); -#else - using JsonDocument doc = JsonDocument.Parse(bytes); - return doc.RootElement.Clone(); -#endif - } - - private JsonElement ConvertType(object value) => SerializeToJsonElement(value, _serializerOptions); + private JsonElement ConvertType(object value) => + MutableJsonElement.SerializeToJsonElement(value, _serializerOptions); private object? SetViaIndexer(object index, object value) { diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs index 3dfd09d52dbf1..d9ae56f45dd1d 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs @@ -78,23 +78,6 @@ internal int GetArrayLength() throw new InvalidOperationException(); } - internal static JsonElement ConvertToJsonElement(MutableJsonChange change, JsonSerializerOptions options) - { - byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(change.Value, options); - - // Most JsonDocument.Parse calls return a that is backed by one or more ArrayPool - // arrays. Those arrays are not returned until the instance is disposed. - // This is a workaround that allows us to dispose the JsonDocument so that we - // don't leak ArrayPool arrays. -#if NET6_0_OR_GREATER - Utf8JsonReader reader = new(bytes); - return JsonElement.ParseValue(ref reader); -#else - using JsonDocument doc = JsonDocument.Parse(bytes); - return doc.RootElement.Clone(); -#endif - } - internal bool IsDescendant(string path) { return IsDescendant(path.AsSpan()); diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index 179b970e4a4ba..e82294fb52ce0 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -9,7 +9,6 @@ using System.Text.Json; using System.Text.Json.Serialization; -#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member namespace Azure.Core.Json { /// @@ -118,7 +117,7 @@ public bool TryGetProperty(ReadOnlySpan name, out MutableJsonElement value return false; } - value = new MutableJsonElement(_root, MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions), GetString(path, 0, pathLength), change.Index); + value = new MutableJsonElement(_root, SerializeToJsonElement(change.Value, _root.SerializerOptions), GetString(path, 0, pathLength), change.Index); return true; } @@ -173,7 +172,7 @@ internal MutableJsonElement GetIndexElement(int index) string path = MutableJsonDocument.ChangeTracker.PushIndex(_path, index); if (Changes.TryGetChange(path, _highWaterMark, out MutableJsonChange change)) { - return new MutableJsonElement(_root, MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions), path, change.Index); + return new MutableJsonElement(_root, SerializeToJsonElement(change.Value, _root.SerializerOptions), path, change.Index); } return new MutableJsonElement(_root, _element[index], path, _highWaterMark); @@ -553,7 +552,7 @@ public bool TryGetDateTime(out DateTime value) return true; case DateTimeOffset: case string: - MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions).TryGetDateTime(out value); + SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTime(out value); return true; case JsonElement element: return element.TryGetDateTime(out value); @@ -598,7 +597,7 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value) return true; case DateTime: case string: - MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions).TryGetDateTimeOffset(out value); + SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTimeOffset(out value); return true; case JsonElement element: return element.TryGetDateTimeOffset(out value); @@ -691,7 +690,7 @@ public bool TryGetGuid(out Guid value) value = g; return true; case string: - MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions).TryGetGuid(out value); + SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetGuid(out value); return true; case JsonElement element: return element.TryGetGuid(out value); @@ -1449,13 +1448,30 @@ public override string ToString() return _element.ToString() ?? "null"; } + internal static JsonElement SerializeToJsonElement(object? value, JsonSerializerOptions? options = default) + { + byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, options); + + // Most JsonDocument.Parse calls return a that is backed by one or more ArrayPool + // arrays. Those arrays are not returned until the instance is disposed. + // This is a workaround that allows us to dispose the JsonDocument so that we + // don't leak ArrayPool arrays. +#if NET6_0_OR_GREATER + Utf8JsonReader reader = new(bytes); + return JsonElement.ParseValue(ref reader); +#else + using JsonDocument doc = JsonDocument.Parse(bytes); + return doc.RootElement.Clone(); +#endif + } + internal JsonElement GetJsonElement() { EnsureValid(); if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - return MutableJsonChange.ConvertToJsonElement(change, _root.SerializerOptions); + return SerializeToJsonElement(change.Value, _root.SerializerOptions); } // Account for changes to descendants of this element as well @@ -1536,5 +1552,4 @@ public override void Write(Utf8JsonWriter writer, MutableJsonElement value, Json } } } -#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member } diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs index 37bfbe4e82022..d2ffb301721af 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs @@ -3,9 +3,6 @@ using System; using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Text.Json; using Azure.Core.Json; using NUnit.Framework; diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs index a3f3adcd4e3ac..bc1496c34bc5d 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentTests.cs @@ -342,7 +342,7 @@ public void CanReplaceObjectWithAnonymousType() using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - JsonElement element = DynamicData.SerializeToJsonElement(new { B = 5.5 }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { B = 5.5 }); mdoc.RootElement.GetProperty("Baz").Set(element); // Assert: @@ -586,7 +586,7 @@ public void CanSetProperty_StringToObject() Assert.AreEqual("hi", mdoc.RootElement.GetProperty("Foo").GetString()); - JsonElement element = DynamicData.SerializeToJsonElement(new + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { Bar = 6 }); @@ -613,7 +613,7 @@ public void CanSetProperty_StringToArray() Assert.AreEqual("hi", mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetString()); - JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 1, 2, 3 }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new int[] { 1, 2, 3 }); mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").Set(element); Assert.AreEqual(1, mdoc.RootElement.GetIndexElement(0).GetProperty("Foo").GetIndexElement(0).GetInt32()); diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs index 4ac1674479e4e..4e501ed2c1f55 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs @@ -351,7 +351,7 @@ public void CanWriteObjectWithChangesAndAdditions() MutableJsonDocumentTests.ValidateWriteTo(expected, mdoc); - JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 1, 2, 3 }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new int[] { 1, 2, 3 }); mdoc.RootElement.SetProperty("Baz", element); expected = """ @@ -409,7 +409,7 @@ public void CanWriteChangesInterleavedAcrossBranches() """; MutableJsonDocumentTests.ValidateWriteTo(expected, mdoc); - JsonElement element = DynamicData.SerializeToJsonElement(new + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { Foo = new { @@ -464,7 +464,7 @@ public void CanWriteArrayWithChangedElements() MutableJsonDocumentTests.ValidateWriteTo(expected, mdoc); - JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 0, 1, 2, 3 }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new int[] { 0, 1, 2, 3 }); mdoc.RootElement.GetProperty("Bar").Set(element); mdoc.RootElement.GetProperty("Bar").GetIndexElement(3).Set(4); @@ -549,7 +549,7 @@ public void WriteToBehaviorMatchesJsonDocument(dynamic json) // Mutate a value string name = mdoc.RootElement.EnumerateObject().First().Name; MutableJsonElement value = mdoc.RootElement.EnumerateObject().First().Value; - JsonElement element = DynamicData.SerializeToJsonElement(value); + JsonElement element = MutableJsonElement.SerializeToJsonElement(value); mdoc.RootElement.GetProperty(name).Set(element); // Validate after changes. @@ -880,7 +880,7 @@ public void CanWritePatchInterleaveParentAndChildChanges() mdoc.RootElement.GetProperty("a").GetProperty("aa").Set(3); mdoc.RootElement.GetProperty("b").GetProperty("ba").Set("3"); - JsonElement element = DynamicData.SerializeToJsonElement(new { ba = "3", bb = "4" }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { ba = "3", bb = "4" }); mdoc.RootElement.GetProperty("b").Set(element); mdoc.RootElement.GetProperty("a").GetProperty("ab").Set(4); mdoc.RootElement.GetProperty("b").GetProperty("ba").Set("5"); @@ -1040,7 +1040,7 @@ public void CanWritePatchForArraysAtDifferentLevels() using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(2); mdoc.RootElement.GetProperty("b").GetProperty("bb").GetProperty("bbb").GetIndexElement(1).Set(true); - JsonElement element = DynamicData.SerializeToJsonElement(new { cd = "cd" }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { cd = "cd" }); mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(element); ValidatePatch(""" @@ -1083,7 +1083,7 @@ public void CanWritePatchInterleaveArrayChanges() mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(2); mdoc.RootElement.GetProperty("b").GetProperty("bb").GetProperty("bbb").GetIndexElement(1).Set(true); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(3); - JsonElement element = DynamicData.SerializeToJsonElement(new { cd = "cd" }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { cd = "cd" }); mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(element); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(1).Set(4); @@ -1126,9 +1126,9 @@ public void CanWritePatchInterleaveParentAndChildArrayChanges() MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(0).Set(2); mdoc.RootElement.GetProperty("b").GetProperty("bb").GetProperty("bbb").GetIndexElement(1).Set(true); - JsonElement element = DynamicData.SerializeToJsonElement(new int[] { 2, 3 }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new int[] { 2, 3 }); mdoc.RootElement.GetProperty("a").GetProperty("aa").Set(element); - element = DynamicData.SerializeToJsonElement(new { cd = "cd" }); + element = MutableJsonElement.SerializeToJsonElement(new { cd = "cd" }); mdoc.RootElement.GetProperty("c").GetIndexElement(1).Set(element); mdoc.RootElement.GetProperty("a").GetProperty("aa").GetIndexElement(1).Set(4); @@ -1164,7 +1164,7 @@ public void CanWritePatchReplaceObject() } """; using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - JsonElement element = DynamicData.SerializeToJsonElement(new { aa = 3, ab = 4 }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { aa = 3, ab = 4 }); mdoc.RootElement.GetProperty("a").Set(element); ValidatePatch(""" @@ -1193,7 +1193,7 @@ public void CanWritePatchReplaceObject_Deletions() } """; using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - JsonElement element = DynamicData.SerializeToJsonElement(new { ac = 3 }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { ac = 3 }); mdoc.RootElement.GetProperty("a").Set(element); ValidatePatch(""" @@ -1278,7 +1278,7 @@ public void CanWritePatchAddObject() """; using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - JsonElement element = DynamicData.SerializeToJsonElement(new { ca = true, cb = false }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new { ca = true, cb = false }); mdoc.RootElement.SetProperty("c", element); ValidatePatch(""" @@ -1498,7 +1498,7 @@ public void CanWritePatchRfc7396SecondExample() mdoc.RootElement.GetProperty("title").Set("Hello!"); mdoc.RootElement.SetProperty("phoneNumber", "+01-123-456-7890"); mdoc.RootElement.GetProperty("author").RemoveProperty("familyName"); - JsonElement element = DynamicData.SerializeToJsonElement(new string[] { "example" }); + JsonElement element = MutableJsonElement.SerializeToJsonElement(new string[] { "example" }); mdoc.RootElement.SetProperty("tags", element); ValidatePatch(""" From a21c3fa22f96e5e153739fa0282195f33bc83eb6 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 08:36:39 -0700 Subject: [PATCH 03/14] Some exceptions --- .../src/DynamicData/MutableJsonChange.cs | 29 ++++-- .../src/DynamicData/MutableJsonElement.cs | 88 ++++--------------- 2 files changed, 37 insertions(+), 80 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs index d9ae56f45dd1d..640a8b99f1daa 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs @@ -3,6 +3,7 @@ using System; using System.Text.Json; +using System.Xml.Linq; namespace Azure.Core.Json { @@ -54,18 +55,31 @@ public MutableJsonChange(string path, _ => throw new InvalidCastException() // TODO: fix exception }; - private void EnsureArray() + internal readonly void EnsureString() { - if (Value is JsonElement e && e.ValueKind == JsonValueKind.Array) + if (ValueKind != JsonValueKind.String) { - return; + throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); } + } - // TODO: improve exception - throw new InvalidOperationException($"Expected an 'Array' type for item."); + internal readonly void EnsureNumber() + { + if (ValueKind != JsonValueKind.Number) + { + throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); + } + } + + internal readonly void EnsureArray() + { + if (ValueKind != JsonValueKind.Array) + { + throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); + } } - internal int GetArrayLength() + internal readonly int GetArrayLength() { EnsureArray(); @@ -74,8 +88,7 @@ internal int GetArrayLength() return e.GetArrayLength(); } - //TODO - throw new InvalidOperationException(); + throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); } internal bool IsDescendant(string path) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index e82294fb52ce0..818058736b512 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -192,11 +192,7 @@ public bool TryGetDouble(out double value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -260,11 +256,7 @@ public bool TryGetInt32(out int value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -323,11 +315,7 @@ public bool TryGetInt64(out long value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -386,11 +374,7 @@ public bool TryGetSingle(out float value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -490,11 +474,7 @@ public bool TryGetByte(out byte value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -539,11 +519,7 @@ public bool TryGetDateTime(out DateTime value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.String) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureString(); switch (change.Value) { @@ -584,11 +560,7 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.String) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureString(); switch (change.Value) { @@ -629,11 +601,7 @@ public bool TryGetDecimal(out decimal value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -678,11 +646,7 @@ public bool TryGetGuid(out Guid value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.String) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureString(); switch (change.Value) { @@ -722,11 +686,7 @@ public bool TryGetInt16(out short value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -771,11 +731,7 @@ public bool TryGetSByte(out sbyte value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -820,11 +776,7 @@ public bool TryGetUInt16(out ushort value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -869,11 +821,7 @@ public bool TryGetUInt32(out uint value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -918,11 +866,7 @@ public bool TryGetUInt64(out ulong value) if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - if (change.ValueKind != JsonValueKind.Number) - { - // TODO - throw new InvalidOperationException(); - } + change.EnsureNumber(); switch (change.Value) { @@ -964,7 +908,7 @@ public ulong GetUInt64() /// /// Gets an enumerator to enumerate the values in the JSON array represented by this MutableJsonElement. /// - internal ArrayEnumerator EnumerateArray() + public ArrayEnumerator EnumerateArray() { EnsureValid(); @@ -976,7 +920,7 @@ internal ArrayEnumerator EnumerateArray() /// /// Gets an enumerator to enumerate the properties in the JSON object represented by this JsonElement. /// - internal ObjectEnumerator EnumerateObject() + public ObjectEnumerator EnumerateObject() { EnsureValid(); From 618372010c0f355224d679ead1aa927183b34a83 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 08:49:06 -0700 Subject: [PATCH 04/14] more exceptions --- .../src/DynamicData/MutableJsonChange.cs | 13 +- .../src/DynamicData/MutableJsonElement.cs | 141 ++++-------------- 2 files changed, 31 insertions(+), 123 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs index 640a8b99f1daa..b51cdf5eede95 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs @@ -3,7 +3,6 @@ using System; using System.Text.Json; -using System.Xml.Linq; namespace Azure.Core.Json { @@ -34,6 +33,7 @@ public MutableJsonChange(string path, public readonly JsonValueKind ValueKind => Value switch { + null => JsonValueKind.Null, bool b => b ? JsonValueKind.True : JsonValueKind.False, string => JsonValueKind.String, DateTime => JsonValueKind.String, @@ -50,16 +50,15 @@ public MutableJsonChange(string path, float => JsonValueKind.Number, double => JsonValueKind.Number, decimal => JsonValueKind.Number, - null => JsonValueKind.Null, JsonElement e => e.ValueKind, - _ => throw new InvalidCastException() // TODO: fix exception + _ => throw new InvalidOperationException($"Unrecognized change type '{Value.GetType()}'.") }; internal readonly void EnsureString() { if (ValueKind != JsonValueKind.String) { - throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); + throw new InvalidOperationException($"Expected a 'String' kind but was '{ValueKind}'."); } } @@ -67,7 +66,7 @@ internal readonly void EnsureNumber() { if (ValueKind != JsonValueKind.Number) { - throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); + throw new InvalidOperationException($"Expected a 'Number' kind but was '{ValueKind}'."); } } @@ -75,7 +74,7 @@ internal readonly void EnsureArray() { if (ValueKind != JsonValueKind.Array) { - throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); + throw new InvalidOperationException($"Expected an 'Array' kind but was '{ValueKind}'."); } } @@ -88,7 +87,7 @@ internal readonly int GetArrayLength() return e.GetArrayLength(); } - throw new InvalidOperationException($"Expected an 'Array' type but was '{ValueKind}'."); + throw new InvalidOperationException($"Expected an 'Array' kind but was '{ValueKind}'."); } internal bool IsDescendant(string path) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index 818058736b512..cbc669103faf1 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -205,16 +205,8 @@ public bool TryGetDouble(out double value) value = default; return false; default: - try - { - value = (double)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a double"); - } + value = checked((double)change.Value); + return true; } } @@ -269,16 +261,8 @@ public bool TryGetInt32(out int value) value = default; return false; default: - try - { - value = (int)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not an int"); - } + value = checked((int)change.Value); + return true; } } @@ -328,16 +312,8 @@ public bool TryGetInt64(out long value) value = default; return false; default: - try - { - value = (long)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a long"); - } + value = checked((long)change.Value); + return true; } } @@ -387,16 +363,8 @@ public bool TryGetSingle(out float value) value = default; return false; default: - try - { - value = (float)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a float"); - } + value = checked((float)change.Value); + return true; } } @@ -487,16 +455,8 @@ public bool TryGetByte(out byte value) value = default; return false; default: - try - { - value = (byte)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a byte"); - } + value = checked((byte)change.Value); + return true; } } @@ -536,8 +496,7 @@ public bool TryGetDateTime(out DateTime value) value = default; return false; default: - // TODO - throw new InvalidOperationException("Changed element is not a DateTime"); + throw new InvalidOperationException($"Element {change.Value} cannot be converted to DateTime."); } } @@ -577,8 +536,7 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value) value = default; return false; default: - // TODO - throw new InvalidOperationException("Changed element is not a DateTime"); + throw new InvalidOperationException($"Element {change.Value} cannot be converted to DateTimeOffset."); } } @@ -614,16 +572,8 @@ public bool TryGetDecimal(out decimal value) value = default; return false; default: - try - { - value = (decimal)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a decimal"); - } + value = checked((decimal)change.Value); + return true; } } @@ -662,8 +612,7 @@ public bool TryGetGuid(out Guid value) value = default; return false; default: - // TODO - throw new InvalidOperationException("Changed element is not a Guid."); + throw new InvalidOperationException($"Element {change.Value} cannot be converted to Guid."); } } @@ -699,16 +648,8 @@ public bool TryGetInt16(out short value) value = default; return false; default: - try - { - value = (short)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a short"); - } + value = checked((short)change.Value); + return true; } } @@ -744,16 +685,8 @@ public bool TryGetSByte(out sbyte value) value = default; return false; default: - try - { - value = (sbyte)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not an sbyte"); - } + value = checked((sbyte)change.Value); + return true; } } @@ -789,16 +722,8 @@ public bool TryGetUInt16(out ushort value) value = default; return false; default: - try - { - value = (ushort)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a ushort."); - } + value = checked((ushort)change.Value); + return true; } } @@ -834,16 +759,8 @@ public bool TryGetUInt32(out uint value) value = default; return false; default: - try - { - value = (uint)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a unint"); - } + value = checked((uint)change.Value); + return true; } } @@ -879,16 +796,8 @@ public bool TryGetUInt64(out ulong value) value = default; return false; default: - try - { - value = (ulong)change.Value; - return true; - } - catch (InvalidCastException) - { - // TODO - throw new InvalidOperationException("Changed element is not a ulong"); - } + value = checked((ulong)change.Value); + return true; } } From a54d2be670189bab7ac26322ad54e95a78a2966e Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 08:53:55 -0700 Subject: [PATCH 05/14] nit --- sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index cbc669103faf1..6442abcb27e70 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -398,6 +398,8 @@ public float GetSingle() if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { + change.EnsureString(); + switch (change.Value) { case string s: From 1871d4262983896843a60aa47015b9335bd5866e Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 09:38:47 -0700 Subject: [PATCH 06/14] add back number tests --- .../MutableJsonDocumentWriteToTests.cs | 83 +++++++----- .../DynamicData/MutableJsonElementTests.cs | 123 +++++++++++------- 2 files changed, 127 insertions(+), 79 deletions(-) diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs index 4e501ed2c1f55..2d5ec08b96ef1 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs @@ -7,7 +7,6 @@ using System.Linq; using System.Text.Json; using Azure.Core.Json; -using Azure.Core.Serialization; using NUnit.Framework; namespace Azure.Core.Tests @@ -580,28 +579,30 @@ public void CanWriteByte() Assert.AreEqual($"{44}", mdoc.RootElement.GetProperty("bar").ToString()); } - //[TestCaseSource(nameof(NumberValues))] - //public void CanWriteNumber(string serializedX, T x, T y, T z) - //{ - // string json = $"{{\"foo\" : {serializedX}}}"; + [TestCaseSource(nameof(NumberValues))] + public void CanWriteNumber(string serializedX, T x, T y, T z, + Action set, + Func setProperty) + { + string json = $"{{\"foo\" : {serializedX}}}"; - // // Get from parsed JSON - // using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + // Get from parsed JSON + using MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - // // Get from parsed JSON - // Assert.AreEqual($"{x}", mdoc.RootElement.GetProperty("foo").ToString()); - // MutableJsonDocumentTests.ValidateWriteTo(json, mdoc); + // Get from parsed JSON + Assert.AreEqual($"{x}", mdoc.RootElement.GetProperty("foo").ToString()); + MutableJsonDocumentTests.ValidateWriteTo(json, mdoc); - // // Get from assigned existing value - // mdoc.RootElement.GetProperty("foo").Set(y); - // Assert.AreEqual($"{y}", mdoc.RootElement.GetProperty("foo").ToString()); - // MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\" : {y}}}", mdoc); + // Get from assigned existing value + set(mdoc, "foo", y); + Assert.AreEqual($"{y}", mdoc.RootElement.GetProperty("foo").ToString()); + MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\" : {y}}}", mdoc); - // // Get from added value - // mdoc.RootElement.SetProperty("bar", z); - // Assert.AreEqual($"{z}", mdoc.RootElement.GetProperty("bar").ToString()); - // MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\":{y},\"bar\":{z}}}", mdoc); - //} + // Get from added value + setProperty(mdoc, "bar", z); + Assert.AreEqual($"{z}", mdoc.RootElement.GetProperty("bar").ToString()); + MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\":{y},\"bar\":{z}}}", mdoc); + } [Test] public void CanWriteGuid() @@ -1679,19 +1680,41 @@ public static IEnumerable NumberValues() { // Valid ranges: // https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/integral-numeric-types - yield return new object[] { "42", (byte)42, (byte)43, (byte)44 }; - yield return new object[] { "42", (sbyte)42, (sbyte)43, (sbyte)44 }; - yield return new object[] { "42", (short)42, (short)43, (short)44 }; - yield return new object[] { "42", (ushort)42, (ushort)43, (ushort)44 }; - yield return new object[] { "42", 42, 43, 44 }; - yield return new object[] { "42", 42u, 43u, 44u }; - yield return new object[] { "42", 42L, 43L, 44L }; - yield return new object[] { "42", 42ul, 43ul, 44ul }; + yield return new object[] { "42", (byte)42, (byte)43, (byte)44, + (MutableJsonDocument mdoc, string name, byte value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, byte value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] { "42", (sbyte)42, (sbyte)43, (sbyte)44, + (MutableJsonDocument mdoc, string name, sbyte value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, sbyte value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] {"42", (short)42, (short)43, (short)44, + (MutableJsonDocument mdoc, string name, short value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, short value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] {"42", (ushort)42, (ushort)43, (ushort)44, + (MutableJsonDocument mdoc, string name, ushort value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, ushort value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] { "42", 42, 43, 44, + (MutableJsonDocument mdoc, string name, int value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, int value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] { "42", 42u, 43u, 44u, + (MutableJsonDocument mdoc, string name, uint value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, uint value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] { "42", 42L, 43L, 44L, + (MutableJsonDocument mdoc, string name, long value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, long value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] { "42", 42ul, 43ul, 44ul, + (MutableJsonDocument mdoc, string name, ulong value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, ulong value) => mdoc.RootElement.SetProperty(name, value) }; #if NETCOREAPP - yield return new object[] { "42.1", 42.1f, 43.1f, 44.1f }; - yield return new object[] { "42.1", 42.1d, 43.1d, 44.1d }; + yield return new object[] { "42.1", 42.1f, 43.1f, 44.1f, + (MutableJsonDocument mdoc, string name, float value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, float value) => mdoc.RootElement.SetProperty(name, value) }; + yield return new object[] { "42.1", 42.1d, 43.1d, 44.1d, + (MutableJsonDocument mdoc, string name, double value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, double value) => mdoc.RootElement.SetProperty(name, value) }; #endif - yield return new object[] { "42.1", 42.1m, 43.1m, 44.1m }; + yield return new object[] { "42.1", 42.1m, 43.1m, 44.1m, + (MutableJsonDocument mdoc, string name, decimal value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, decimal value) => mdoc.RootElement.SetProperty(name, value) }; } #endregion } diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs index 6c9d157ab0380..d285cc357c25a 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs @@ -381,44 +381,47 @@ public void CanGetByte() Assert.Throws(() => mdoc.RootElement.GetProperty("foo").GetByte()); } - //[TestCaseSource(nameof(NumberValues))] - //public void CanGetNumber(string serializedX, T x, T y, T z, U invalid, Func tryGet, Func get) - //{ - // string json = $"{{\"foo\" : {serializedX}}}"; - - // // Get from parsed JSON - // MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); - - // Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); - // Assert.AreEqual(x, tryGet(mdoc.RootElement.GetProperty("foo")).Value); - // Assert.AreEqual(x, get(mdoc.RootElement.GetProperty("foo"))); - - // // Get from assigned existing value - // mdoc.RootElement.GetProperty("foo").Set(y); - - // Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); - // Assert.AreEqual(y, tryGet(mdoc.RootElement.GetProperty("foo")).Value); - // Assert.AreEqual(y, get(mdoc.RootElement.GetProperty("foo"))); - - // // Get from added value - // mdoc.RootElement.SetProperty("bar", z); - // Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("bar")).TryGet); - // Assert.AreEqual(z, tryGet(mdoc.RootElement.GetProperty("bar")).Value); - // Assert.AreEqual(z, get(mdoc.RootElement.GetProperty("bar"))); - - // // Doesn't work if number change is outside range - // if (invalid is bool testRange && testRange) - // { - // mdoc.RootElement.GetProperty("foo").Set(testRange); - // Assert.IsFalse(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); - // Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); - // } - - // // Doesn't work for non-number change - // mdoc.RootElement.GetProperty("foo").Set("string"); - // Assert.Throws(() => tryGet(mdoc.RootElement.GetProperty("foo"))); - // Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); - //} + [TestCaseSource(nameof(NumberValues))] + public void CanGetNumber(string serializedX, T x, T y, T z, U invalid, + Func tryGet, + Func get, + Action set, + Func setProperty) + { + string json = $"{{\"foo\" : {serializedX}}}"; + + // Get from parsed JSON + MutableJsonDocument mdoc = MutableJsonDocument.Parse(json); + + Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); + Assert.AreEqual(x, tryGet(mdoc.RootElement.GetProperty("foo")).Value); + Assert.AreEqual(x, get(mdoc.RootElement.GetProperty("foo"))); + + // Get from assigned existing value + set(mdoc, "foo", y); + Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); + Assert.AreEqual(y, tryGet(mdoc.RootElement.GetProperty("foo")).Value); + Assert.AreEqual(y, get(mdoc.RootElement.GetProperty("foo"))); + + // Get from added value + setProperty(mdoc, "bar", z); + Assert.IsTrue(tryGet(mdoc.RootElement.GetProperty("bar")).TryGet); + Assert.AreEqual(z, tryGet(mdoc.RootElement.GetProperty("bar")).Value); + Assert.AreEqual(z, get(mdoc.RootElement.GetProperty("bar"))); + + // Doesn't work if number change is outside range + if (invalid is bool testRange && testRange) + { + mdoc.RootElement.GetProperty("foo").Set(testRange); + Assert.IsFalse(tryGet(mdoc.RootElement.GetProperty("foo")).TryGet); + Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); + } + + // Doesn't work for non-number change + mdoc.RootElement.GetProperty("foo").Set("string"); + Assert.Throws(() => tryGet(mdoc.RootElement.GetProperty("foo"))); + Assert.Throws(() => get(mdoc.RootElement.GetProperty("foo"))); + } [Test] public void CanGetGuid() @@ -572,37 +575,59 @@ public static IEnumerable NumberValues() // https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/integral-numeric-types yield return new object[] { "42", (byte)42, (byte)43, (byte)44, 256, (MutableJsonElement e) => (e.TryGetByte(out byte b), b), - (MutableJsonElement e) => e.GetByte() }; + (MutableJsonElement e) => e.GetByte(), + (MutableJsonDocument mdoc, string name, byte value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, byte value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42", (sbyte)42, (sbyte)43, (sbyte)44, 128, (MutableJsonElement e) => (e.TryGetSByte(out sbyte b), b), - (MutableJsonElement e) => e.GetSByte() }; + (MutableJsonElement e) => e.GetSByte(), + (MutableJsonDocument mdoc, string name, sbyte value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, sbyte value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42", (short)42, (short)43, (short)44, 32768, (MutableJsonElement e) => (e.TryGetInt16(out short i), i), - (MutableJsonElement e) => e.GetInt16() }; + (MutableJsonElement e) => e.GetInt16(), + (MutableJsonDocument mdoc, string name, short value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, short value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42", (ushort)42, (ushort)43, (ushort)44, 65536, (MutableJsonElement e) => (e.TryGetUInt16(out ushort i), i), - (MutableJsonElement e) => e.GetUInt16() }; + (MutableJsonElement e) => e.GetUInt16(), + (MutableJsonDocument mdoc, string name, ushort value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, ushort value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42", 42, 43, 44, 2147483648, (MutableJsonElement e) => (e.TryGetInt32(out int i), i), - (MutableJsonElement e) => e.GetInt32() }; + (MutableJsonElement e) => e.GetInt32(), + (MutableJsonDocument mdoc, string name, int value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, int value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42", 42u, 43u, 44u, 4294967296, (MutableJsonElement e) => (e.TryGetUInt32(out uint i), i), - (MutableJsonElement e) => e.GetUInt32() }; + (MutableJsonElement e) => e.GetUInt32(), + (MutableJsonDocument mdoc, string name, uint value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, uint value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42", 42L, 43L, 44L, 9223372036854775808, (MutableJsonElement e) => (e.TryGetInt64(out long i), i), - (MutableJsonElement e) => e.GetInt64() }; + (MutableJsonElement e) => e.GetInt64(), + (MutableJsonDocument mdoc, string name, long value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, long value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42", 42ul, 43ul, 44ul, -1, (MutableJsonElement e) => (e.TryGetUInt64(out ulong i), i), - (MutableJsonElement e) => e.GetUInt64() }; + (MutableJsonElement e) => e.GetUInt64(), + (MutableJsonDocument mdoc, string name, ulong value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, ulong value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42.1", 42.1f, 43.1f, 44.1f, false, /*don't do range check*/ (MutableJsonElement e) => (e.TryGetSingle(out float d), d), - (MutableJsonElement e) => e.GetSingle() }; + (MutableJsonElement e) => e.GetSingle(), + (MutableJsonDocument mdoc, string name, float value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, float value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42.1", 42.1d, 43.1d, 44.1d, false, /*don't do range check*/ (MutableJsonElement e) => (e.TryGetDouble(out double d), d), - (MutableJsonElement e) => e.GetDouble() }; + (MutableJsonElement e) => e.GetDouble(), + (MutableJsonDocument mdoc, string name, double value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, double value) => mdoc.RootElement.SetProperty(name, value) }; yield return new object[] { "42.1", 42.1m, 43.1m, 44.1m, false, /*don't do range check*/ (MutableJsonElement e) => (e.TryGetDecimal(out decimal d), d), - (MutableJsonElement e) => e.GetDecimal() }; + (MutableJsonElement e) => e.GetDecimal(), + (MutableJsonDocument mdoc, string name, decimal value) => mdoc.RootElement.GetProperty(name).Set(value), + (MutableJsonDocument mdoc, string name, decimal value) => mdoc.RootElement.SetProperty(name, value) }; } #endregion From 629602c5fe4f8c7f50141ee800b0f4360cac1e01 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 09:42:01 -0700 Subject: [PATCH 07/14] Update WriteTo --- .../src/DynamicData/MutableJsonElement.WriteTo.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs index d9a436a760284..eb0453552cde5 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.WriteTo.cs @@ -120,9 +120,6 @@ private void WritePrimitiveChange(MutableJsonChange change, Utf8JsonWriter write case bool b: writer.WriteBooleanValue(b); return; - case string s: - writer.WriteStringValue(s); - return; case byte b: writer.WriteNumberValue(b); return; @@ -156,25 +153,23 @@ private void WritePrimitiveChange(MutableJsonChange change, Utf8JsonWriter write case decimal d: writer.WriteNumberValue(d); return; + case string s: + writer.WriteStringValue(s); + return; case DateTime d: - // TODO - test writer.WriteStringValue(d); return; case DateTimeOffset d: - // TODO - test writer.WriteStringValue(d); return; case Guid g: - // TODO - test writer.WriteStringValue(g); return; case null: writer.WriteNullValue(); return; default: - // Change can't have the type it has - // TODO - throw new InvalidOperationException(); + throw new InvalidOperationException($"Unrecognized change type '{change.Value.GetType()}'."); } } } From 31d034fa5cc0500301e1e5dd303088a6b07ae17e Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 09:57:24 -0700 Subject: [PATCH 08/14] update --- sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs index 8fc4828ae2e36..569c33fbc332e 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs @@ -19,6 +19,8 @@ internal sealed partial class MutableJsonDocument : IDisposable private readonly ReadOnlyMemory _original; private readonly JsonDocument _originalDocument; + private static JsonSerializerOptions DefaultSerializerOptions = new JsonSerializerOptions(); + private readonly JsonSerializerOptions _serializerOptions; internal JsonSerializerOptions SerializerOptions { get => _serializerOptions; } @@ -203,7 +205,7 @@ private MutableJsonDocument(JsonDocument document, ReadOnlyMemory utf8Json { _originalDocument = document; _original = utf8Json; - _serializerOptions = serializerOptions ?? new JsonSerializerOptions(); + _serializerOptions = serializerOptions ?? DefaultSerializerOptions; } private class MutableJsonDocumentConverter : JsonConverter From df1c79c6d2031063d9fb289218511b9ea047c629 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 10:33:57 -0700 Subject: [PATCH 09/14] Updates --- .../DynamicData/MutableJsonDocument.ChangeTracker.cs | 7 ------- .../src/DynamicData/MutableJsonDocument.cs | 8 +------- .../Azure.Core/src/DynamicData/MutableJsonElement.cs | 12 ++++++------ .../MutableJsonDocumentChangeTrackerTests.cs | 2 +- .../tests/DynamicData/MutableJsonElementTests.cs | 6 ++---- 5 files changed, 10 insertions(+), 25 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs index ef2354cad7727..0e971ad32dbb9 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.ChangeTracker.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Text.Json; namespace Azure.Core.Json { @@ -12,13 +11,7 @@ internal partial class MutableJsonDocument { internal class ChangeTracker { - public ChangeTracker(JsonSerializerOptions options) - { - _options = options; - } - private List? _changes; - private JsonSerializerOptions _options; internal const char Delimiter = (char)1; diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs index 569c33fbc332e..2cdc0639041d8 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs @@ -19,17 +19,12 @@ internal sealed partial class MutableJsonDocument : IDisposable private readonly ReadOnlyMemory _original; private readonly JsonDocument _originalDocument; - private static JsonSerializerOptions DefaultSerializerOptions = new JsonSerializerOptions(); - - private readonly JsonSerializerOptions _serializerOptions; - internal JsonSerializerOptions SerializerOptions { get => _serializerOptions; } - private ChangeTracker? _changeTracker; internal ChangeTracker Changes { get { - _changeTracker ??= new ChangeTracker(SerializerOptions); + _changeTracker ??= new(); return _changeTracker; } } @@ -205,7 +200,6 @@ private MutableJsonDocument(JsonDocument document, ReadOnlyMemory utf8Json { _originalDocument = document; _original = utf8Json; - _serializerOptions = serializerOptions ?? DefaultSerializerOptions; } private class MutableJsonDocumentConverter : JsonConverter diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index 6442abcb27e70..8db5b408d8bc5 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -117,7 +117,7 @@ public bool TryGetProperty(ReadOnlySpan name, out MutableJsonElement value return false; } - value = new MutableJsonElement(_root, SerializeToJsonElement(change.Value, _root.SerializerOptions), GetString(path, 0, pathLength), change.Index); + value = new MutableJsonElement(_root, SerializeToJsonElement(change.Value), GetString(path, 0, pathLength), change.Index); return true; } @@ -172,7 +172,7 @@ internal MutableJsonElement GetIndexElement(int index) string path = MutableJsonDocument.ChangeTracker.PushIndex(_path, index); if (Changes.TryGetChange(path, _highWaterMark, out MutableJsonChange change)) { - return new MutableJsonElement(_root, SerializeToJsonElement(change.Value, _root.SerializerOptions), path, change.Index); + return new MutableJsonElement(_root, SerializeToJsonElement(change.Value), path, change.Index); } return new MutableJsonElement(_root, _element[index], path, _highWaterMark); @@ -490,7 +490,7 @@ public bool TryGetDateTime(out DateTime value) return true; case DateTimeOffset: case string: - SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTime(out value); + SerializeToJsonElement(change.Value).TryGetDateTime(out value); return true; case JsonElement element: return element.TryGetDateTime(out value); @@ -530,7 +530,7 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value) return true; case DateTime: case string: - SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTimeOffset(out value); + SerializeToJsonElement(change.Value).TryGetDateTimeOffset(out value); return true; case JsonElement element: return element.TryGetDateTimeOffset(out value); @@ -606,7 +606,7 @@ public bool TryGetGuid(out Guid value) value = g; return true; case string: - SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetGuid(out value); + SerializeToJsonElement(change.Value).TryGetGuid(out value); return true; case JsonElement element: return element.TryGetGuid(out value); @@ -1326,7 +1326,7 @@ internal JsonElement GetJsonElement() if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - return SerializeToJsonElement(change.Value, _root.SerializerOptions); + return SerializeToJsonElement(change.Value); } // Account for changes to descendants of this element as well diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs index d2ffb301721af..814c30c4c7c7c 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentChangeTrackerTests.cs @@ -37,7 +37,7 @@ public void CanCompareChangesByPath() [Test] public void CanGetSortedChanges() { - MutableJsonDocument.ChangeTracker changeTracker = new(null); + MutableJsonDocument.ChangeTracker changeTracker = new(); char delimiter = MutableJsonDocument.ChangeTracker.Delimiter; changeTracker.AddChange("a", 1); diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs index d285cc357c25a..4f91487e4ede4 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonElementTests.cs @@ -559,14 +559,12 @@ internal static void ValidateToString(string json, MutableJsonElement element) internal static string FormatDateTime(DateTime d) { - byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(d); - return JsonDocument.Parse(bytes).RootElement.GetString(); + return MutableJsonElement.SerializeToJsonElement(d).GetString(); } internal static string FormatDateTimeOffset(DateTimeOffset d) { - byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(d); - return JsonDocument.Parse(bytes).RootElement.GetString(); + return MutableJsonElement.SerializeToJsonElement(d).GetString(); } public static IEnumerable NumberValues() From e3d95e62e4e35bc282934096d46584d9e69f106b Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 10:39:49 -0700 Subject: [PATCH 10/14] Tests --- sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs | 8 ++++---- .../tests/DynamicData/MutableJsonDocumentWriteToTests.cs | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index 8db5b408d8bc5..44d0b2166f837 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -1307,10 +1307,10 @@ internal static JsonElement SerializeToJsonElement(object? value, JsonSerializer { byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, options); - // Most JsonDocument.Parse calls return a that is backed by one or more ArrayPool - // arrays. Those arrays are not returned until the instance is disposed. - // This is a workaround that allows us to dispose the JsonDocument so that we - // don't leak ArrayPool arrays. + // Most JsonDocument.Parse calls return an array that is backed by one or more + // ArrayPool arrays. Those arrays are not returned until the instance is disposed. + // This workaround allows us to dispose the JsonDocument so that we don't leak + // ArrayPool arrays. #if NET6_0_OR_GREATER Utf8JsonReader reader = new(bytes); return JsonElement.ParseValue(ref reader); diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs index 2d5ec08b96ef1..58220ad35a0c1 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs @@ -660,7 +660,7 @@ public void CanWriteDateTime() [Test] public void CanWriteDateTimeOffset() { - DateTimeOffset dateTime = DateTimeOffset.Now; + DateTimeOffset dateTime = DateTimeOffset.Parse("2023-08-17T10:36:42.5482841+07:00"); string dateTimeString = MutableJsonElementTests.FormatDateTimeOffset(dateTime); string json = $"{{\"foo\" : \"{dateTimeString}\"}}"; @@ -671,14 +671,14 @@ public void CanWriteDateTimeOffset() MutableJsonDocumentTests.ValidateWriteTo(json, mdoc); // Get from assigned existing value - DateTimeOffset fooValue = DateTimeOffset.Now.AddDays(1); + DateTimeOffset fooValue = dateTime.AddDays(1); string fooString = MutableJsonElementTests.FormatDateTimeOffset(fooValue); mdoc.RootElement.GetProperty("foo").Set(fooValue); Assert.AreEqual(fooString, mdoc.RootElement.GetProperty("foo").ToString()); MutableJsonDocumentTests.ValidateWriteTo($"{{\"foo\" : \"{fooString}\"}}", mdoc); // Get from added value - DateTimeOffset barValue = DateTimeOffset.Now.AddDays(2); + DateTimeOffset barValue = dateTime.AddDays(2); string barString = MutableJsonElementTests.FormatDateTimeOffset(barValue); mdoc.RootElement.SetProperty("bar", barValue); Assert.AreEqual(barString, mdoc.RootElement.GetProperty("bar").ToString()); From c3768f1e2b071e6143d4d30f10b23bae41df81f3 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 13:44:53 -0700 Subject: [PATCH 11/14] Updates --- sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs | 7 +++++-- .../tests/DynamicData/MutableJsonDocumentWriteToTests.cs | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index 44d0b2166f837..0212d85d81d25 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -1306,7 +1306,11 @@ public override string ToString() internal static JsonElement SerializeToJsonElement(object? value, JsonSerializerOptions? options = default) { byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, options); + return ParseFromBytes(bytes); + } + private static JsonElement ParseFromBytes(byte[] bytes) + { // Most JsonDocument.Parse calls return an array that is backed by one or more // ArrayPool arrays. Those arrays are not returned until the instance is disposed. // This workaround allows us to dispose the JsonDocument so that we don't leak @@ -1332,8 +1336,7 @@ internal JsonElement GetJsonElement() // Account for changes to descendants of this element as well if (Changes.DescendantChanged(_path, _highWaterMark)) { - JsonDocument document = JsonDocument.Parse(GetRawBytes()); - return document.RootElement; + return ParseFromBytes(GetRawBytes()); } return _element; diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs index 58220ad35a0c1..163774de184db 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs @@ -62,6 +62,7 @@ public void CanWriteDateTimeAppConfigValue() mdoc.RootElement.GetProperty("last_modified").Set("2023-03-23T16:35:35+00:00"); MutableJsonDocumentTests.ValidateWriteTo(data2, mdoc); + MutableJsonDocumentTests.ValidateWriteTo(data2.ToString(), mdoc); } [Test] @@ -658,6 +659,7 @@ public void CanWriteDateTime() } [Test] + [Ignore("Investigating possible issue in Utf8JsonWriter.")] public void CanWriteDateTimeOffset() { DateTimeOffset dateTime = DateTimeOffset.Parse("2023-08-17T10:36:42.5482841+07:00"); From 725051f2186ffae0140e14ac95b830c644227f8f Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 17 Aug 2023 14:05:28 -0700 Subject: [PATCH 12/14] Skip test while investigating writer behavior --- .../tests/DynamicData/MutableJsonDocumentWriteToTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs index 163774de184db..edff85842b5ae 100644 --- a/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs +++ b/sdk/core/Azure.Core/tests/DynamicData/MutableJsonDocumentWriteToTests.cs @@ -631,6 +631,7 @@ public void CanWriteGuid() } [Test] + [Ignore("Investigating possible issue in Utf8JsonWriter.")] public void CanWriteDateTime() { DateTime dateTime = DateTime.Parse("2023-05-07T21:04:45.1657010-07:00"); From 23cf570a88f97f84e4613793e2b76cbcd1f8964d Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 22 Aug 2023 11:07:11 -0700 Subject: [PATCH 13/14] Respect serializer options for changes where needed --- .../src/DynamicData/MutableJsonDocument.cs | 6 ++++++ .../Azure.Core/src/DynamicData/MutableJsonElement.cs | 12 ++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs index 2cdc0639041d8..d4c508161185b 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs @@ -16,9 +16,14 @@ namespace Azure.Core.Json [JsonConverter(typeof(MutableJsonDocumentConverter))] internal sealed partial class MutableJsonDocument : IDisposable { + private static readonly JsonSerializerOptions DefaultSerializerOptions = new JsonSerializerOptions(); + private readonly ReadOnlyMemory _original; private readonly JsonDocument _originalDocument; + private readonly JsonSerializerOptions _serializerOptions; + internal JsonSerializerOptions SerializerOptions => _serializerOptions; + private ChangeTracker? _changeTracker; internal ChangeTracker Changes { @@ -200,6 +205,7 @@ private MutableJsonDocument(JsonDocument document, ReadOnlyMemory utf8Json { _originalDocument = document; _original = utf8Json; + _serializerOptions = serializerOptions ?? DefaultSerializerOptions; } private class MutableJsonDocumentConverter : JsonConverter diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index 0212d85d81d25..4bfa169ad093c 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -117,7 +117,7 @@ public bool TryGetProperty(ReadOnlySpan name, out MutableJsonElement value return false; } - value = new MutableJsonElement(_root, SerializeToJsonElement(change.Value), GetString(path, 0, pathLength), change.Index); + value = new MutableJsonElement(_root, SerializeToJsonElement(change.Value, _root.SerializerOptions), GetString(path, 0, pathLength), change.Index); return true; } @@ -172,7 +172,7 @@ internal MutableJsonElement GetIndexElement(int index) string path = MutableJsonDocument.ChangeTracker.PushIndex(_path, index); if (Changes.TryGetChange(path, _highWaterMark, out MutableJsonChange change)) { - return new MutableJsonElement(_root, SerializeToJsonElement(change.Value), path, change.Index); + return new MutableJsonElement(_root, SerializeToJsonElement(change.Value, _root.SerializerOptions), path, change.Index); } return new MutableJsonElement(_root, _element[index], path, _highWaterMark); @@ -490,7 +490,7 @@ public bool TryGetDateTime(out DateTime value) return true; case DateTimeOffset: case string: - SerializeToJsonElement(change.Value).TryGetDateTime(out value); + SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTime(out value); return true; case JsonElement element: return element.TryGetDateTime(out value); @@ -530,7 +530,7 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value) return true; case DateTime: case string: - SerializeToJsonElement(change.Value).TryGetDateTimeOffset(out value); + SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTimeOffset(out value); return true; case JsonElement element: return element.TryGetDateTimeOffset(out value); @@ -606,7 +606,7 @@ public bool TryGetGuid(out Guid value) value = g; return true; case string: - SerializeToJsonElement(change.Value).TryGetGuid(out value); + SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetGuid(out value); return true; case JsonElement element: return element.TryGetGuid(out value); @@ -1330,7 +1330,7 @@ internal JsonElement GetJsonElement() if (Changes.TryGetChange(_path, _highWaterMark, out MutableJsonChange change)) { - return SerializeToJsonElement(change.Value); + return SerializeToJsonElement(change.Value, _root.SerializerOptions); } // Account for changes to descendants of this element as well From b033268447b9dede8060a472be8e640d727a0de6 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 22 Aug 2023 14:24:15 -0700 Subject: [PATCH 14/14] pr fb --- .../Azure.Core/src/DynamicData/MutableJsonElement.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs index 4bfa169ad093c..6359fcff9568a 100644 --- a/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs +++ b/sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs @@ -490,8 +490,7 @@ public bool TryGetDateTime(out DateTime value) return true; case DateTimeOffset: case string: - SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTime(out value); - return true; + return SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTime(out value); case JsonElement element: return element.TryGetDateTime(out value); case null: @@ -530,8 +529,7 @@ public bool TryGetDateTimeOffset(out DateTimeOffset value) return true; case DateTime: case string: - SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTimeOffset(out value); - return true; + return SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetDateTimeOffset(out value); case JsonElement element: return element.TryGetDateTimeOffset(out value); case null: @@ -606,8 +604,7 @@ public bool TryGetGuid(out Guid value) value = g; return true; case string: - SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetGuid(out value); - return true; + return SerializeToJsonElement(change.Value, _root.SerializerOptions).TryGetGuid(out value); case JsonElement element: return element.TryGetGuid(out value); case null: