From b02f735cc800e3cc02cf57fc8813b31488970298 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 28 Jun 2023 18:41:12 +0100 Subject: [PATCH 1/4] Avoid JsonElement torn read in a few JsonArray/JsonDocument and unify code patterns. --- .../System/Text/Json/Nodes/JsonArray.IList.cs | 15 +- .../src/System/Text/Json/Nodes/JsonArray.cs | 119 ++++++------- .../Text/Json/Nodes/JsonObject.IDictionary.cs | 167 ++++++------------ .../src/System/Text/Json/Nodes/JsonObject.cs | 72 ++++---- ...Trimmable.cs => JsonValueOfT.Overrides.cs} | 0 5 files changed, 149 insertions(+), 224 deletions(-) rename src/libraries/System.Text.Json/src/System/Text/Json/Nodes/{JsonValueTrimmable.cs => JsonValueOfT.Overrides.cs} (100%) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.IList.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.IList.cs index 1143d89c0cd86..70bf77ff76c69 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.IList.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.IList.cs @@ -31,12 +31,21 @@ public void Add(JsonNode? item) /// public void Clear() { - for (int i = 0; i < List.Count; i++) + List? list = _list; + + if (list is null) { - DetachParent(List[i]); + _jsonElement = null; } + else + { + for (int i = 0; i < list.Count; i++) + { + DetachParent(list[i]); + } - List.Clear(); + list.Clear(); + } } /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs index 3fd5af169f129..d42bf251fdcd8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs @@ -50,13 +50,15 @@ public JsonArray(params JsonNode?[] items) : base() internal override JsonNode InternalDeepClone() { - if (_jsonElement.HasValue) + GetUnderlyingRepresentation(out List? list, out JsonElement? jsonElement); + + if (list is null) { - return new JsonArray(_jsonElement.Value.Clone(), Options); + return jsonElement.HasValue + ? new JsonArray(jsonElement.Value.Clone(), Options) + : new JsonArray(Options); } - List list = List; - var jsonArray = new JsonArray(Options) { _list = new List(list.Count) @@ -64,15 +66,7 @@ internal override JsonNode InternalDeepClone() for (int i = 0; i < list.Count; i++) { - JsonNode? item = list[i]; - if (item is null) - { - jsonArray.Add(null); - } - else - { - jsonArray.Add(item.DeepClone()); - } + jsonArray.Add(list[i]?.InternalDeepClone()); } return jsonArray; @@ -85,7 +79,7 @@ internal override bool DeepEquals(JsonNode? node) case null or JsonObject: return false; case JsonValue value: - // JsonValueTrimmable/NonTrimmable can hold the array type so calling this method to continue the deep comparision. + // JsonValueTrimmable/NonTrimmable can hold the array type so calling this method to continue the deep comparison. return value.DeepEquals(this); case JsonArray array: List currentList = List; @@ -196,15 +190,10 @@ public void Add(T? value) } } - internal List List - { - get - { - CreateNodes(); - Debug.Assert(_list != null); - return _list; - } - } + /// + /// Gets or creates the underlying list containing the element nodes of the array. + /// + internal List List => _list is { } list ? list : InitializeList(); internal JsonNode? GetItem(int index) { @@ -237,72 +226,74 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio ThrowHelper.ThrowArgumentNullException(nameof(writer)); } - if (_jsonElement.HasValue) + GetUnderlyingRepresentation(out List? list, out JsonElement? jsonElement); + + if (list is null && jsonElement.HasValue) { - _jsonElement.Value.WriteTo(writer); + jsonElement.Value.WriteTo(writer); } else { - CreateNodes(); - Debug.Assert(_list != null); - + list ??= List; options ??= s_defaultOptions; writer.WriteStartArray(); - for (int i = 0; i < _list.Count; i++) + for (int i = 0; i < list.Count; i++) { - JsonNodeConverter.Instance.Write(writer, _list[i]!, options); + JsonNodeConverter.Instance.Write(writer, list[i], options); } writer.WriteEndArray(); } } - private void CreateNodes() + private List InitializeList() { - if (_list is null) - { - CreateNodesCore(); - } + GetUnderlyingRepresentation(out List? list, out JsonElement? jsonElement); - void CreateNodesCore() + if (list is null) { - // Even though _list initialization can be subject to races, - // ensure that contending threads use a coherent view of jsonElement. + if (jsonElement.HasValue) + { + JsonElement jElement = jsonElement.Value; + Debug.Assert(jElement.ValueKind == JsonValueKind.Array); - // Because JsonElement cannot be read atomically there might be torn reads, - // however the order of read/write operations guarantees that that's only - // possible if the value of _list is non-null. - JsonElement? jsonElement = _jsonElement; - Interlocked.MemoryBarrier(); - List? list = _list; + list = new List(jElement.GetArrayLength()); - if (list is null) + foreach (JsonElement element in jElement.EnumerateArray()) + { + JsonNode? node = JsonNodeConverter.Create(element, Options); + node?.AssignParent(this); + list.Add(node); + } + } + else { list = new(); + } - if (jsonElement.HasValue) - { - JsonElement jElement = jsonElement.Value; - Debug.Assert(jElement.ValueKind == JsonValueKind.Array); - - list = new List(jElement.GetArrayLength()); + // Ensure _jsonElement is written to after _list + _list = list; + Interlocked.MemoryBarrier(); + _jsonElement = null; + } - foreach (JsonElement element in jElement.EnumerateArray()) - { - JsonNode? node = JsonNodeConverter.Create(element, Options); - node?.AssignParent(this); - list.Add(node); - } - } + return list; + } - // Ensure _jsonElement is written to after _list - _list = list; - Interlocked.MemoryBarrier(); - _jsonElement = null; - } - } + /// + /// Provides a coherent view of the underlying representation of the current node. + /// The jsonElement value should be consumed if and only if the list value is null. + /// + private void GetUnderlyingRepresentation(out List? list, out JsonElement? jsonElement) + { + // Because JsonElement cannot be read atomically there might be torn reads, + // however the order of read/write operations guarantees that that's only + // possible if the value of _list is non-null. + jsonElement = _jsonElement; + Interlocked.MemoryBarrier(); + list = _list; } [ExcludeFromCodeCoverage] // Justification = "Design-time" diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs index e298b3ad6bd1f..76e6189a12666 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.IDictionary.cs @@ -3,7 +3,6 @@ using System.Collections; using System.Collections.Generic; -using System.Diagnostics; using System.Text.Json.Serialization.Converters; using System.Threading; @@ -26,9 +25,7 @@ public partial class JsonObject : IDictionary /// public void Add(string propertyName, JsonNode? value) { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - _dictionary.Add(propertyName, value); + Dictionary.Add(propertyName, value); value?.AssignParent(this); } @@ -44,34 +41,27 @@ public void Add(string propertyName, JsonNode? value) /// /// The property name of is . /// - public void Add(KeyValuePair property) - { - Add(property.Key, property.Value); - } + public void Add(KeyValuePair property) => Add(property.Key, property.Value); /// /// Removes all elements from the . /// public void Clear() { - if (_jsonElement != null) - { - Debug.Assert(_dictionary == null); - _jsonElement = null; - return; - } + JsonPropertyDictionary? dictionary = _dictionary; - if (_dictionary == null) + if (dictionary is null) { + _jsonElement = null; return; } - foreach (JsonNode? node in _dictionary.GetValueCollection()) + foreach (JsonNode? node in dictionary.GetValueCollection()) { DetachParent(node); } - _dictionary.Clear(); + dictionary.Clear(); } /// @@ -84,25 +74,12 @@ public void Clear() /// /// is . /// - public bool ContainsKey(string propertyName) - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.ContainsKey(propertyName); - } + public bool ContainsKey(string propertyName) => Dictionary.ContainsKey(propertyName); /// /// Gets the number of elements contained in . /// - public int Count - { - get - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.Count; - } - } + public int Count => Dictionary.Count; /// /// Removes the element with the specified property name from the . @@ -121,10 +98,7 @@ public bool Remove(string propertyName) ThrowHelper.ThrowArgumentNullException(nameof(propertyName)); } - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - - bool success = _dictionary.TryRemoveProperty(propertyName, out JsonNode? removedNode); + bool success = Dictionary.TryRemoveProperty(propertyName, out JsonNode? removedNode); if (success) { DetachParent(removedNode); @@ -140,12 +114,7 @@ public bool Remove(string propertyName) /// /// if the contains an element with the property name; otherwise, . /// - bool ICollection>.Contains(KeyValuePair item) - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.Contains(item); - } + bool ICollection>.Contains(KeyValuePair item) => Dictionary.Contains(item); /// /// Copies the elements of the to an array of type KeyValuePair starting at the specified array index. @@ -164,12 +133,7 @@ public bool Remove(string propertyName) /// The number of elements in the source ICollection is greater than the available space from /// to the end of the destination . /// - void ICollection>.CopyTo(KeyValuePair[] array, int index) - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - _dictionary.CopyTo(array, index); - } + void ICollection>.CopyTo(KeyValuePair[] array, int index) => Dictionary.CopyTo(array, index); /// /// Returns an enumerator that iterates through the . @@ -177,12 +141,7 @@ public bool Remove(string propertyName) /// /// An enumerator that iterates through the . /// - public IEnumerator> GetEnumerator() - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.GetEnumerator(); - } + public IEnumerator> GetEnumerator() => Dictionary.GetEnumerator(); /// /// Removes a key and value from the . @@ -198,28 +157,12 @@ public bool Remove(string propertyName) /// /// Gets a collection containing the property names in the . /// - ICollection IDictionary.Keys - { - get - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.Keys; - } - } + ICollection IDictionary.Keys => Dictionary.Keys; /// /// Gets a collection containing the property values in the . /// - ICollection IDictionary.Values - { - get - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.Values; - } - } + ICollection IDictionary.Values => Dictionary.Values; /// /// Gets the value associated with the specified property name. @@ -235,12 +178,7 @@ public bool Remove(string propertyName) /// /// is . /// - bool IDictionary.TryGetValue(string propertyName, out JsonNode? jsonNode) - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.TryGetValue(propertyName, out jsonNode); - } + bool IDictionary.TryGetValue(string propertyName, out JsonNode? jsonNode) => Dictionary.TryGetValue(propertyName, out jsonNode); /// /// Returns . @@ -253,56 +191,51 @@ public bool Remove(string propertyName) /// /// An enumerator that iterates through the . /// - IEnumerator IEnumerable.GetEnumerator() - { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - return _dictionary.GetEnumerator(); - } + IEnumerator IEnumerable.GetEnumerator() => Dictionary.GetEnumerator(); - private void InitializeIfRequired() + private JsonPropertyDictionary InitializeDictionary() { - if (_dictionary is null) - { - InitializeCore(); - } + GetUnderlyingRepresentation(out JsonPropertyDictionary? dictionary, out JsonElement? jsonElement); - void InitializeCore() + if (dictionary is null) { - // Even though _dictionary initialization can be subject to races, - // ensure that contending threads use a coherent view of jsonElement. - - // Because JsonElement cannot be read atomically there might be torn reads, - // however the order of read/write operations guarantees that that's only - // possible if the value of _dictionary is non-null. - JsonElement? jsonElement = _jsonElement; - Interlocked.MemoryBarrier(); - JsonPropertyDictionary? dictionary = _dictionary; - - if (dictionary is null) + bool caseInsensitive = Options.HasValue ? Options.Value.PropertyNameCaseInsensitive : false; + dictionary = new JsonPropertyDictionary(caseInsensitive); + if (jsonElement.HasValue) { - bool caseInsensitive = Options.HasValue ? Options.Value.PropertyNameCaseInsensitive : false; - dictionary = new JsonPropertyDictionary(caseInsensitive); - if (jsonElement.HasValue) + foreach (JsonProperty jElementProperty in jsonElement.Value.EnumerateObject()) { - foreach (JsonProperty jElementProperty in jsonElement.Value.EnumerateObject()) + JsonNode? node = JsonNodeConverter.Create(jElementProperty.Value, Options); + if (node != null) { - JsonNode? node = JsonNodeConverter.Create(jElementProperty.Value, Options); - if (node != null) - { - node.Parent = this; - } - - dictionary.Add(new KeyValuePair(jElementProperty.Name, node)); + node.Parent = this; } - } - // Ensure _jsonElement is written to after _dictionary - _dictionary = dictionary; - Interlocked.MemoryBarrier(); - _jsonElement = null; + dictionary.Add(new KeyValuePair(jElementProperty.Name, node)); + } } + + // Ensure _jsonElement is written to after _dictionary + _dictionary = dictionary; + Interlocked.MemoryBarrier(); + _jsonElement = null; } + + return dictionary; + } + + /// + /// Provides a coherent view of the underlying representation of the current node. + /// The jsonElement value should be consumed if and only if dictionary value is null. + /// + private void GetUnderlyingRepresentation(out JsonPropertyDictionary? dictionary, out JsonElement? jsonElement) + { + // Because JsonElement cannot be read atomically there might be torn reads, + // however the order of read/write operations guarantees that that's only + // possible if the value of _dictionary is non-null. + jsonElement = _jsonElement; + Interlocked.MemoryBarrier(); + dictionary = _dictionary; } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs index 09420fbf6895c..8dce0f54a41d6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs @@ -70,43 +70,39 @@ internal JsonObject(JsonElement element, JsonNodeOptions? options = null) : this _jsonElement = element; } + /// + /// Gets or creates the underlying dictionary containing the properties of the object. + /// + internal JsonPropertyDictionary Dictionary => _dictionary is { } dictionary ? dictionary : InitializeDictionary(); + internal override JsonNode InternalDeepClone() { - if (_jsonElement.HasValue) + GetUnderlyingRepresentation(out JsonPropertyDictionary? dictionary, out JsonElement? jsonElement); + + if (dictionary is null) { - return new JsonObject(_jsonElement!.Value.Clone(), Options); + return jsonElement.HasValue + ? new JsonObject(jsonElement.Value.Clone(), Options) + : new JsonObject(Options); } - if (_dictionary is not null) + bool caseInsensitive = Options.HasValue ? Options.Value.PropertyNameCaseInsensitive : false; + var jObject = new JsonObject(Options) { - bool caseInsensitive = Options.HasValue ? Options.Value.PropertyNameCaseInsensitive : false; - var jObject = new JsonObject(Options) - { - _dictionary = new JsonPropertyDictionary(caseInsensitive, _dictionary.Count) - }; + _dictionary = new JsonPropertyDictionary(caseInsensitive, dictionary.Count) + }; - foreach (KeyValuePair item in _dictionary) - { - if (item.Value is not null) - { - jObject.Add(item.Key, item.Value.DeepClone()); - } - else - { - jObject.Add(item.Key, null); - } - } - return jObject; + foreach (KeyValuePair item in dictionary) + { + jObject.Add(item.Key, item.Value?.InternalDeepClone()); } - return new JsonObject(Options); + return jObject; } internal string GetPropertyName(JsonNode? node) { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - KeyValuePair? item = _dictionary.FindValue(node); + KeyValuePair? item = Dictionary.FindValue(node); return item.HasValue ? item.Value.Key : string.Empty; } @@ -129,10 +125,12 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio ThrowHelper.ThrowArgumentNullException(nameof(writer)); } - if (_jsonElement.HasValue) + GetUnderlyingRepresentation(out JsonPropertyDictionary? dictionary, out JsonElement? jsonElement); + + if (dictionary is null && jsonElement.HasValue) { // Write the element without converting to nodes. - _jsonElement.Value.WriteTo(writer); + jsonElement.Value.WriteTo(writer); } else { @@ -160,19 +158,17 @@ internal override bool DeepEquals(JsonNode? node) // JsonValueTrimmable/NonTrimmable can hold the object type so calling this method to continue the deep comparision. return value.DeepEquals(this); case JsonObject jsonObject: - InitializeIfRequired(); - jsonObject.InitializeIfRequired(); - Debug.Assert(_dictionary is not null); - Debug.Assert(jsonObject._dictionary is not null); + JsonPropertyDictionary currentDict = Dictionary; + JsonPropertyDictionary otherDict = jsonObject.Dictionary; - if (_dictionary.Count != jsonObject._dictionary.Count) + if (currentDict.Count != otherDict.Count) { return false; } - foreach (KeyValuePair item in this) + foreach (KeyValuePair item in currentDict) { - JsonNode? jsonNode = jsonObject._dictionary[item.Key]; + JsonNode? jsonNode = otherDict[item.Key]; if (!DeepEquals(item.Value, jsonNode)) { @@ -202,9 +198,7 @@ internal override void GetPath(List path, JsonNode? child) { if (child != null) { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - string propertyName = _dictionary.FindValue(child)!.Value.Key; + string propertyName = Dictionary.FindValue(child)!.Value.Key; if (propertyName.AsSpan().ContainsSpecialCharacters()) { path.Add($"['{propertyName}']"); @@ -220,15 +214,13 @@ internal override void GetPath(List path, JsonNode? child) internal void SetItem(string propertyName, JsonNode? value) { - InitializeIfRequired(); - Debug.Assert(_dictionary != null); - JsonNode? existing = _dictionary.SetValue(propertyName, value, () => value?.AssignParent(this)); + JsonNode? existing = Dictionary.SetValue(propertyName, value, () => value?.AssignParent(this)); DetachParent(existing); } private void DetachParent(JsonNode? item) { - InitializeIfRequired(); + InitializeDictionary(); Debug.Assert(_dictionary != null); if (item != null) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueTrimmable.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.Overrides.cs similarity index 100% rename from src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueTrimmable.cs rename to src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.Overrides.cs From d9ac996838e8daa8643e5d9df98f794ce6d2ba06 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 29 Jun 2023 15:33:57 +0100 Subject: [PATCH 2/4] Simplify and clean up JsonValue implementation -- fix DeepEquality bugs. --- .../src/System.Text.Json.csproj | 4 +- .../System/Text/Json/Document/JsonDocument.cs | 2 +- .../Text/Json/JsonPropertyDictionary.cs | 8 +- .../src/System/Text/Json/Nodes/JsonArray.cs | 56 ++++--- .../src/System/Text/Json/Nodes/JsonNode.To.cs | 31 ++-- .../src/System/Text/Json/Nodes/JsonNode.cs | 23 +-- .../src/System/Text/Json/Nodes/JsonObject.cs | 44 ++--- .../Json/Nodes/JsonValue.CreateOverloads.cs | 70 ++++---- .../src/System/Text/Json/Nodes/JsonValue.cs | 21 ++- .../Text/Json/Nodes/JsonValueNotTrimmable.cs | 49 ------ .../Text/Json/Nodes/JsonValueOfT.Overrides.cs | 150 ------------------ .../System/Text/Json/Nodes/JsonValueOfT.cs | 79 ++++++--- .../Text/Json/Nodes/JsonValueOfTCustomized.cs | 45 ++++++ .../Text/Json/Nodes/JsonValueOfTPrimitive.cs | 55 +++++++ .../Converters/Node/JsonNodeConverter.cs | 19 +-- .../Node/JsonNodeConverterFactory.cs | 4 +- .../Converters/Node/JsonValueConverter.cs | 2 +- .../Converters/Object/ObjectConverter.cs | 2 +- .../JsonSerializer.Read.HandleMetadata.cs | 5 +- .../JsonSerializerOptions.Caching.cs | 1 - .../System.Text.Json.Tests/JsonNode/Common.cs | 27 ++++ .../JsonNode/JsonArrayTests.cs | 23 ++- .../JsonNode/JsonObjectTests.cs | 30 ++-- .../JsonNode/JsonValueTests.cs | 40 ++--- 24 files changed, 358 insertions(+), 432 deletions(-) delete mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueNotTrimmable.cs delete mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.Overrides.cs create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTCustomized.cs create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index d0d1e3ba9841c..e1cbf82c656e5 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -83,9 +83,9 @@ The System.Text.Json library is built-in as part of the shared framework in .NET - + - + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs index f929db3192987..93f7143a55844 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonDocument.cs @@ -200,7 +200,7 @@ internal int GetEndIndex(int index, bool includeEndElement) internal ReadOnlyMemory GetRootRawValue() { - return GetRawValue(0, includeQuotes : true); + return GetRawValue(0, includeQuotes: true); } internal ReadOnlyMemory GetRawValue(int index, bool includeQuotes) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs index 4ad855e3bbbc5..7642074b506ab 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs @@ -148,13 +148,7 @@ public void CopyTo(KeyValuePair[] array, int index) } } - public IEnumerator> GetEnumerator() - { - foreach (KeyValuePair item in _propertyList) - { - yield return item; - } - } + public List>.Enumerator GetEnumerator() => _propertyList.GetEnumerator(); public IList Keys => GetKeyCollection(); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs index d42bf251fdcd8..8dc0924ad36e6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonArray.cs @@ -48,7 +48,9 @@ public JsonArray(params JsonNode?[] items) : base() InitializeFromArray(items); } - internal override JsonNode InternalDeepClone() + internal override JsonValueKind GetValueKindCore() => JsonValueKind.Array; + + internal override JsonNode DeepCloneCore() { GetUnderlyingRepresentation(out List? list, out JsonElement? jsonElement); @@ -66,21 +68,21 @@ internal override JsonNode InternalDeepClone() for (int i = 0; i < list.Count; i++) { - jsonArray.Add(list[i]?.InternalDeepClone()); + jsonArray.Add(list[i]?.DeepCloneCore()); } return jsonArray; } - internal override bool DeepEquals(JsonNode? node) + internal override bool DeepEqualsCore(JsonNode? node) { switch (node) { case null or JsonObject: return false; case JsonValue value: - // JsonValueTrimmable/NonTrimmable can hold the array type so calling this method to continue the deep comparison. - return value.DeepEquals(this); + // JsonValue instances have special comparison semantics, dispatch to their implementation. + return value.DeepEqualsCore(this); case JsonArray array: List currentList = List; List otherList = array.List; @@ -147,17 +149,12 @@ private void InitializeFromArray(JsonNode?[] items) /// public static JsonArray? Create(JsonElement element, JsonNodeOptions? options = null) { - if (element.ValueKind == JsonValueKind.Null) + return element.ValueKind switch { - return null; - } - - if (element.ValueKind == JsonValueKind.Array) - { - return new JsonArray(element, options); - } - - throw new InvalidOperationException(SR.Format(SR.NodeElementWrongType, nameof(JsonValueKind.Array))); + JsonValueKind.Null => null, + JsonValueKind.Array => new JsonArray(element, options), + _ => throw new InvalidOperationException(SR.Format(SR.NodeElementWrongType, nameof(JsonValueKind.Array))), + }; } internal JsonArray(JsonElement element, JsonNodeOptions? options = null) : base(options) @@ -177,17 +174,14 @@ internal JsonArray(JsonElement element, JsonNodeOptions? options = null) : base( [RequiresDynamicCode(JsonValue.CreateDynamicCodeMessage)] public void Add(T? value) { - if (value == null) + JsonNode? nodeToAdd = value switch { - Add(null); - } - else - { - JsonNode jNode = value as JsonNode ?? new JsonValueNotTrimmable(value); + null => null, + JsonNode node => node, + _ => JsonValue.Create(value, Options) + }; - // Call the IList.Add() implementation. - Add(jNode); - } + Add(nodeToAdd); } /// @@ -234,14 +228,18 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio } else { - list ??= List; - options ??= s_defaultOptions; - writer.WriteStartArray(); - for (int i = 0; i < list.Count; i++) + foreach (JsonNode? element in List) { - JsonNodeConverter.Instance.Write(writer, list[i], options); + if (element is null) + { + writer.WriteNullValue(); + } + else + { + element.WriteTo(writer, options); + } } writer.WriteEndArray(); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.To.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.To.cs index 69288510c5992..838b05423912f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.To.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.To.cs @@ -5,9 +5,6 @@ namespace System.Text.Json.Nodes { public abstract partial class JsonNode { - // trimming-safe default JsonSerializerOptions instance used by JsonNode methods. - private protected static readonly JsonSerializerOptions s_defaultOptions = new(); - /// /// Converts the current instance to string in JSON format. /// @@ -15,12 +12,7 @@ public abstract partial class JsonNode /// JSON representation of current instance. public string ToJsonString(JsonSerializerOptions? options = null) { - using var output = new PooledByteBufferWriter(JsonSerializerOptions.BufferSizeDefault); - using (var writer = new Utf8JsonWriter(output, options == null ? default(JsonWriterOptions) : options.GetWriterOptions())) - { - WriteTo(writer, options); - } - + using PooledByteBufferWriter output = WriteToPooledBuffer(options, options?.GetWriterOptions() ?? default); return JsonHelpers.Utf8GetString(output.WrittenMemory.Span); } @@ -45,12 +37,7 @@ public override string ToString() } } - using var output = new PooledByteBufferWriter(JsonSerializerOptions.BufferSizeDefault); - using (var writer = new Utf8JsonWriter(output, new JsonWriterOptions { Indented = true })) - { - WriteTo(writer); - } - + using PooledByteBufferWriter output = WriteToPooledBuffer(writerOptions: new JsonWriterOptions { Indented = true }); return JsonHelpers.Utf8GetString(output.WrittenMemory.Span); } @@ -63,5 +50,19 @@ public override string ToString() /// /// Options to control the serialization behavior. public abstract void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? options = null); + + /// + /// Creates a pooled buffer writer instance and serializes all contents to it. + /// + internal PooledByteBufferWriter WriteToPooledBuffer( + JsonSerializerOptions? options = null, + JsonWriterOptions writerOptions = default, + int bufferSize = JsonSerializerOptions.BufferSizeDefault) + { + var bufferWriter = new PooledByteBufferWriter(bufferSize); + using var writer = new Utf8JsonWriter(bufferWriter, writerOptions); + WriteTo(writer, options); + return bufferWriter; + } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs index 9150676d5122a..a920469e94151 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonNode.cs @@ -242,25 +242,16 @@ public JsonNode? this[string propertyName] /// /// Creates a new instance of the . All children nodes are recursively cloned. /// - public JsonNode DeepClone() - { - return InternalDeepClone(); - } + public JsonNode DeepClone() => DeepCloneCore(); - internal abstract JsonNode InternalDeepClone(); + internal abstract JsonNode DeepCloneCore(); /// /// Returns of current instance. /// - public JsonValueKind GetValueKind() - { - return this switch - { - JsonObject => JsonValueKind.Object, - JsonArray => JsonValueKind.Array, - _ => AsValue().GetInternalValueKind(), - }; - } + public JsonValueKind GetValueKind() => GetValueKindCore(); + + internal abstract JsonValueKind GetValueKindCore(); /// /// Returns property name of the current node from the parent object. @@ -311,10 +302,10 @@ public static bool DeepEquals(JsonNode? node1, JsonNode? node2) return node2 is null; } - return node1.DeepEquals(node2); + return node1.DeepEqualsCore(node2); } - internal abstract bool DeepEquals(JsonNode? node); + internal abstract bool DeepEqualsCore(JsonNode? node); /// /// Replaces this node with a new value. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs index 8dce0f54a41d6..6272b62a26558 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Text.Json.Serialization.Converters; namespace System.Text.Json.Nodes { @@ -51,17 +50,12 @@ public JsonObject(IEnumerable> properties, JsonN /// A . public static JsonObject? Create(JsonElement element, JsonNodeOptions? options = null) { - if (element.ValueKind == JsonValueKind.Null) + return element.ValueKind switch { - return null; - } - - if (element.ValueKind == JsonValueKind.Object) - { - return new JsonObject(element, options); - } - - throw new InvalidOperationException(SR.Format(SR.NodeElementWrongType, nameof(JsonValueKind.Object))); + JsonValueKind.Null => null, + JsonValueKind.Object => new JsonObject(element, options), + _ => throw new InvalidOperationException(SR.Format(SR.NodeElementWrongType, nameof(JsonValueKind.Object))) + }; } internal JsonObject(JsonElement element, JsonNodeOptions? options = null) : this(options) @@ -75,7 +69,7 @@ internal JsonObject(JsonElement element, JsonNodeOptions? options = null) : this /// internal JsonPropertyDictionary Dictionary => _dictionary is { } dictionary ? dictionary : InitializeDictionary(); - internal override JsonNode InternalDeepClone() + internal override JsonNode DeepCloneCore() { GetUnderlyingRepresentation(out JsonPropertyDictionary? dictionary, out JsonElement? jsonElement); @@ -94,7 +88,7 @@ internal override JsonNode InternalDeepClone() foreach (KeyValuePair item in dictionary) { - jObject.Add(item.Key, item.Value?.InternalDeepClone()); + jObject.Add(item.Key, item.Value?.DeepCloneCore()); } return jObject; @@ -134,29 +128,37 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio } else { - options ??= s_defaultOptions; - writer.WriteStartObject(); - foreach (KeyValuePair item in this) + foreach (KeyValuePair entry in Dictionary) { - writer.WritePropertyName(item.Key); - JsonNodeConverter.Instance.Write(writer, item.Value, options); + writer.WritePropertyName(entry.Key); + + if (entry.Value is null) + { + writer.WriteNullValue(); + } + else + { + entry.Value.WriteTo(writer, options); + } } writer.WriteEndObject(); } } - internal override bool DeepEquals(JsonNode? node) + internal override JsonValueKind GetValueKindCore() => JsonValueKind.Object; + + internal override bool DeepEqualsCore(JsonNode? node) { switch (node) { case null or JsonArray: return false; case JsonValue value: - // JsonValueTrimmable/NonTrimmable can hold the object type so calling this method to continue the deep comparision. - return value.DeepEquals(this); + // JsonValue instances have special comparison semantics, dispatch to their implementation. + return value.DeepEqualsCore(this); case JsonObject jsonObject: JsonPropertyDictionary currentDict = Dictionary; JsonPropertyDictionary otherDict = jsonObject.Dictionary; diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.CreateOverloads.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.CreateOverloads.cs index 897502dd57aa8..824869192765f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.CreateOverloads.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.CreateOverloads.cs @@ -14,7 +14,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(bool value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.BooleanConverter); + public static JsonValue Create(bool value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.BooleanConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -22,7 +22,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(bool? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.BooleanConverter) : null; + public static JsonValue? Create(bool? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.BooleanConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -30,7 +30,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(byte value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.ByteConverter); + public static JsonValue Create(byte value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.ByteConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -38,7 +38,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(byte? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.ByteConverter) : null; + public static JsonValue? Create(byte? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.ByteConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -46,7 +46,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(char value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.CharConverter); + public static JsonValue Create(char value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.CharConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -54,7 +54,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(char? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.CharConverter) : null; + public static JsonValue? Create(char? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.CharConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -62,7 +62,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(DateTime value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.DateTimeConverter); + public static JsonValue Create(DateTime value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.DateTimeConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -70,7 +70,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(DateTime? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.DateTimeConverter) : null; + public static JsonValue? Create(DateTime? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.DateTimeConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -78,7 +78,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(DateTimeOffset value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.DateTimeOffsetConverter); + public static JsonValue Create(DateTimeOffset value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.DateTimeOffsetConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -86,7 +86,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(DateTimeOffset? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.DateTimeOffsetConverter) : null; + public static JsonValue? Create(DateTimeOffset? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.DateTimeOffsetConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -94,7 +94,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(decimal value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.DecimalConverter); + public static JsonValue Create(decimal value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.DecimalConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -102,7 +102,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(decimal? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.DecimalConverter) : null; + public static JsonValue? Create(decimal? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.DecimalConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -110,7 +110,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(double value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.DoubleConverter); + public static JsonValue Create(double value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.DoubleConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -118,7 +118,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(double? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.DoubleConverter) : null; + public static JsonValue? Create(double? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.DoubleConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -126,7 +126,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(Guid value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.GuidConverter); + public static JsonValue Create(Guid value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.GuidConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -134,7 +134,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(Guid? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.GuidConverter) : null; + public static JsonValue? Create(Guid? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.GuidConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -142,7 +142,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(short value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.Int16Converter); + public static JsonValue Create(short value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.Int16Converter); /// /// Initializes a new instance of the class that contains the specified value. @@ -150,7 +150,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(short? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.Int16Converter) : null; + public static JsonValue? Create(short? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.Int16Converter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -158,7 +158,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(int value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.Int32Converter); + public static JsonValue Create(int value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.Int32Converter); /// /// Initializes a new instance of the class that contains the specified value. @@ -166,7 +166,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(int? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.Int32Converter) : null; + public static JsonValue? Create(int? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.Int32Converter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -174,7 +174,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(long value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.Int64Converter); + public static JsonValue Create(long value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.Int64Converter); /// /// Initializes a new instance of the class that contains the specified value. @@ -182,7 +182,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(long? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.Int64Converter) : null; + public static JsonValue? Create(long? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.Int64Converter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -191,7 +191,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue Create(sbyte value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.SByteConverter); + public static JsonValue Create(sbyte value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.SByteConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -200,7 +200,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue? Create(sbyte? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.SByteConverter) : null; + public static JsonValue? Create(sbyte? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.SByteConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -208,7 +208,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue Create(float value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.SingleConverter); + public static JsonValue Create(float value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.SingleConverter); /// /// Initializes a new instance of the class that contains the specified value. @@ -216,7 +216,7 @@ public partial class JsonValue /// The underlying value of the new instance. /// Options to control the behavior. /// The new instance of the class that contains the specified value. - public static JsonValue? Create(float? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.SingleConverter) : null; + public static JsonValue? Create(float? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.SingleConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -225,7 +225,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [return: NotNullIfNotNull(nameof(value))] - public static JsonValue? Create(string? value, JsonNodeOptions? options = null) => value != null ? new JsonValueTrimmable(value, JsonMetadataServices.StringConverter) : null; + public static JsonValue? Create(string? value, JsonNodeOptions? options = null) => value != null ? new JsonValuePrimitive(value, JsonMetadataServices.StringConverter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -234,7 +234,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue Create(ushort value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.UInt16Converter); + public static JsonValue Create(ushort value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.UInt16Converter); /// /// Initializes a new instance of the class that contains the specified value. @@ -243,7 +243,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue? Create(ushort? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.UInt16Converter) : null; + public static JsonValue? Create(ushort? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.UInt16Converter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -252,7 +252,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue Create(uint value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.UInt32Converter); + public static JsonValue Create(uint value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.UInt32Converter); /// /// Initializes a new instance of the class that contains the specified value. @@ -261,7 +261,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue? Create(uint? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.UInt32Converter) : null; + public static JsonValue? Create(uint? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.UInt32Converter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -270,7 +270,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue Create(ulong value, JsonNodeOptions? options = null) => new JsonValueTrimmable(value, JsonMetadataServices.UInt64Converter); + public static JsonValue Create(ulong value, JsonNodeOptions? options = null) => new JsonValuePrimitive(value, JsonMetadataServices.UInt64Converter); /// /// Initializes a new instance of the class that contains the specified value. @@ -279,7 +279,7 @@ public partial class JsonValue /// Options to control the behavior. /// The new instance of the class that contains the specified value. [CLSCompliantAttribute(false)] - public static JsonValue? Create(ulong? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValueTrimmable(value.Value, JsonMetadataServices.UInt64Converter) : null; + public static JsonValue? Create(ulong? value, JsonNodeOptions? options = null) => value.HasValue ? new JsonValuePrimitive(value.Value, JsonMetadataServices.UInt64Converter) : null; /// /// Initializes a new instance of the class that contains the specified value. @@ -296,7 +296,7 @@ public partial class JsonValue VerifyJsonElementIsNotArrayOrObject(ref value); - return new JsonValueTrimmable(value, JsonMetadataServices.JsonElementConverter); + return new JsonValuePrimitive(value, JsonMetadataServices.JsonElementConverter); } /// @@ -320,7 +320,7 @@ public partial class JsonValue VerifyJsonElementIsNotArrayOrObject(ref element); - return new JsonValueTrimmable(element, JsonMetadataServices.JsonElementConverter); + return new JsonValuePrimitive(element, JsonMetadataServices.JsonElementConverter); } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.cs index 78ead4c98e4be..0ece122dbea17 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; namespace System.Text.Json.Nodes @@ -33,24 +32,25 @@ private protected JsonValue(JsonNodeOptions? options = null) : base(options) { } [RequiresDynamicCode(CreateDynamicCodeMessage)] public static JsonValue? Create(T? value, JsonNodeOptions? options = null) { - if (value == null) + if (value is null) { return null; } if (value is JsonElement element) { - if (element.ValueKind == JsonValueKind.Null) + if (element.ValueKind is JsonValueKind.Null) { return null; } VerifyJsonElementIsNotArrayOrObject(ref element); - return new JsonValueTrimmable(element, JsonMetadataServices.JsonElementConverter, options); + return new JsonValuePrimitive(element, JsonMetadataServices.JsonElementConverter, options); } - return new JsonValueNotTrimmable(value, options); + var jsonTypeInfo = (JsonTypeInfo)JsonSerializerOptions.Default.GetTypeInfo(typeof(T)); + return new JsonValueCustomized(value, jsonTypeInfo, options); } /// @@ -71,14 +71,14 @@ private protected JsonValue(JsonNodeOptions? options = null) : base(options) { } ThrowHelper.ThrowArgumentNullException(nameof(jsonTypeInfo)); } - if (value == null) + if (value is null) { return null; } if (value is JsonElement element) { - if (element.ValueKind == JsonValueKind.Null) + if (element.ValueKind is JsonValueKind.Null) { return null; } @@ -86,7 +86,8 @@ private protected JsonValue(JsonNodeOptions? options = null) : base(options) { } VerifyJsonElementIsNotArrayOrObject(ref element); } - return new JsonValueTrimmable(value, jsonTypeInfo, options); + jsonTypeInfo.EnsureConfigured(); + return new JsonValueCustomized(value, jsonTypeInfo, options); } internal override void GetPath(List path, JsonNode? child) @@ -96,8 +97,6 @@ internal override void GetPath(List path, JsonNode? child) Parent?.GetPath(path, this); } - internal abstract JsonValueKind GetInternalValueKind(); - /// /// Tries to obtain the current JSON value and returns a value that indicates whether the operation succeeded. /// @@ -118,7 +117,7 @@ internal override void GetPath(List path, JsonNode? child) private static void VerifyJsonElementIsNotArrayOrObject(ref JsonElement element) { // Force usage of JsonArray and JsonObject instead of supporting those in an JsonValue. - if (element.ValueKind == JsonValueKind.Object || element.ValueKind == JsonValueKind.Array) + if (element.ValueKind is JsonValueKind.Object or JsonValueKind.Array) { ThrowHelper.ThrowInvalidOperationException_NodeElementCannotBeObjectOrArray(); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueNotTrimmable.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueNotTrimmable.cs deleted file mode 100644 index c7cc8f30d7720..0000000000000 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueNotTrimmable.cs +++ /dev/null @@ -1,49 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics.CodeAnalysis; - -namespace System.Text.Json.Nodes -{ - /// - /// Not trim-safe since it calls JsonSerializer.Serialize(JsonSerializerOptions). - /// - [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] - internal sealed partial class JsonValueNotTrimmable : JsonValue - { - [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] - public JsonValueNotTrimmable(TValue value, JsonNodeOptions? options = null) : base(value, options) { } - - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "The ctor is marked RequiresUnreferencedCode.")] - public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? options = null) - { - if (writer is null) - { - ThrowHelper.ThrowArgumentNullException(nameof(writer)); - } - - JsonSerializer.Serialize(writer, _value, options); - } - - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "The ctor is marked RequiresUnreferencedCode.")] - internal override JsonNode InternalDeepClone() => JsonSerializer.SerializeToNode(_value)!; - - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "The ctor is marked RequiresUnreferencedCode.")] - internal override bool DeepEquals(JsonNode? node) - { - JsonNode? jsonNode = JsonSerializer.SerializeToNode(_value); - return DeepEquals(jsonNode, node); - } - - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "The ctor is marked RequiresUnreferencedCode.")] - internal override JsonValueKind GetInternalValueKind() - { - JsonNode? jsonNode = JsonSerializer.SerializeToNode(_value); - return jsonNode is null ? JsonValueKind.Null : jsonNode.GetValueKind(); - } - } -} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.Overrides.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.Overrides.cs deleted file mode 100644 index 6a22eb9d727f8..0000000000000 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.Overrides.cs +++ /dev/null @@ -1,150 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.Text.Json.Serialization; -using System.Text.Json.Serialization.Metadata; - -namespace System.Text.Json.Nodes -{ - /// - /// Trim-safe since it either calls the converter directly or calls the JsonSerializer.Serialize(JsonTypeInfo{TValue}). - /// - internal sealed partial class JsonValueTrimmable : JsonValue - { - private readonly JsonTypeInfo? _jsonTypeInfo; - private readonly JsonConverter? _converter; - - public JsonValueTrimmable(TValue value, JsonTypeInfo jsonTypeInfo, JsonNodeOptions? options = null) : base(value, options) - { - _jsonTypeInfo = jsonTypeInfo; - } - - public JsonValueTrimmable(TValue value, JsonConverter converter, JsonNodeOptions? options = null) : base(value, options) - { - _converter = converter; - } - - public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? options = null) - { - if (writer is null) - { - ThrowHelper.ThrowArgumentNullException(nameof(writer)); - } - - if (_converter != null) - { - options ??= s_defaultOptions; - - if (_converter.IsInternalConverterForNumberType) - { - _converter.WriteNumberWithCustomHandling(writer, _value, options.NumberHandling); - } - else - { - _converter.Write(writer, _value, options); - } - } - else - { - Debug.Assert(_jsonTypeInfo != null); - JsonSerializer.Serialize(writer, _value, _jsonTypeInfo); - } - } - - internal override JsonNode InternalDeepClone() - { - if (_converter is not null) - { - return _value is JsonElement element - ? new JsonValueTrimmable(element.Clone(), JsonMetadataServices.JsonElementConverter, Options) - : new JsonValueTrimmable(_value, _converter, Options); - } - else - { - Debug.Assert(_jsonTypeInfo != null); - return JsonSerializer.SerializeToNode(_value, _jsonTypeInfo)!; - } - } - - internal override bool DeepEquals(JsonNode? node) - { - if (node is null) - { - return false; - } - - if (_jsonTypeInfo is not null) - { - JsonNode? jsonNode = JsonSerializer.SerializeToNode(_value, _jsonTypeInfo); - return DeepEquals(jsonNode, node); - } - else - { - if (node is JsonArray || node is JsonObject) - { - return false; - } - - if (node is JsonValueTrimmable jsonElementNodeOther && _value is JsonElement jsonElementCurrent) - { - if (jsonElementNodeOther._value.ValueKind != jsonElementCurrent.ValueKind) - { - return false; - } - - switch (jsonElementCurrent.ValueKind) - { - case JsonValueKind.String: - return jsonElementCurrent.ValueEquals(jsonElementNodeOther._value.GetString()); - case JsonValueKind.True: - case JsonValueKind.False: - return jsonElementCurrent.ValueKind == jsonElementNodeOther._value.ValueKind; - case JsonValueKind.Number: - return jsonElementCurrent.GetRawValue().Span.SequenceEqual(jsonElementNodeOther._value.GetRawValue().Span); - default: - Debug.Fail("Impossible case"); - return false; - } - } - - using var currentOutput = new PooledByteBufferWriter(JsonSerializerOptions.BufferSizeDefault); - using (var writer = new Utf8JsonWriter(currentOutput, default)) - { - WriteTo(writer); - } - - using var anotherOutput = new PooledByteBufferWriter(JsonSerializerOptions.BufferSizeDefault); - using (var writer = new Utf8JsonWriter(anotherOutput, default)) - { - node.WriteTo(writer); - } - - return currentOutput.WrittenMemory.Span.SequenceEqual(anotherOutput.WrittenMemory.Span); - } - } - - internal override JsonValueKind GetInternalValueKind() - { - if (_jsonTypeInfo is not null) - { - return JsonSerializer.SerializeToElement(_value, _jsonTypeInfo).ValueKind; - } - else - { - if (_value is JsonElement element) - { - return element.ValueKind; - } - - using var output = new PooledByteBufferWriter(JsonSerializerOptions.BufferSizeDefault); - using (var writer = new Utf8JsonWriter(output, default)) - { - WriteTo(writer); - } - - return JsonElement.ParseValue(output.WrittenMemory.Span, default).ValueKind; - } - } - } -} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.cs index 2e2b1b2a2e6f4..0be2c9517a376 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfT.cs @@ -8,40 +8,32 @@ namespace System.Text.Json.Nodes { [DebuggerDisplay("{ToJsonString(),nq}")] [DebuggerTypeProxy(typeof(JsonValue<>.DebugView))] - internal abstract partial class JsonValue : JsonValue + internal abstract class JsonValue : JsonValue { - public readonly TValue _value; // keep as a field for direct access to avoid copies + internal readonly TValue Value; // keep as a field for direct access to avoid copies - public JsonValue(TValue value, JsonNodeOptions? options = null) : base(options) + protected JsonValue(TValue value, JsonNodeOptions? options = null) : base(options) { Debug.Assert(value != null); - Debug.Assert(!(value is JsonElement) || ((JsonElement)(object)value).ValueKind != JsonValueKind.Null); + Debug.Assert(value is not JsonElement or JsonElement { ValueKind: not JsonValueKind.Null }); if (value is JsonNode) { ThrowHelper.ThrowArgumentException_NodeValueNotAllowed(nameof(value)); } - _value = value; - } - - public TValue Value - { - get - { - return _value; - } + Value = value; } public override T GetValue() { // If no conversion is needed, just return the raw value. - if (_value is T returnValue) + if (Value is T returnValue) { return returnValue; } - if (_value is JsonElement) + if (Value is JsonElement) { return ConvertJsonElement(); } @@ -49,19 +41,19 @@ public override T GetValue() // Currently we do not support other conversions. // Generics (and also boxing) do not support standard cast operators say from 'long' to 'int', // so attempting to cast here would throw InvalidCastException. - throw new InvalidOperationException(SR.Format(SR.NodeUnableToConvert, _value!.GetType(), typeof(T))); + throw new InvalidOperationException(SR.Format(SR.NodeUnableToConvert, Value!.GetType(), typeof(T))); } public override bool TryGetValue([NotNullWhen(true)] out T value) { // If no conversion is needed, just return the raw value. - if (_value is T returnValue) + if (Value is T returnValue) { value = returnValue; return true; } - if (_value is JsonElement) + if (Value is JsonElement) { return TryConvertJsonElement(out value); } @@ -73,9 +65,56 @@ public override bool TryGetValue([NotNullWhen(true)] out T value) return false; } + internal sealed override JsonValueKind GetValueKindCore() + { + if (Value is JsonElement element) + { + return element.ValueKind; + } + + using PooledByteBufferWriter output = WriteToPooledBuffer(); + return JsonElement.ParseValue(output.WrittenMemory.Span, options: default).ValueKind; + } + + internal sealed override bool DeepEqualsCore(JsonNode? otherNode) + { + if (otherNode is null) + { + return false; + } + + if (Value is JsonElement thisElement && otherNode is JsonValue { Value: JsonElement otherElement }) + { + if (thisElement.ValueKind != otherElement.ValueKind) + { + return false; + } + + switch (thisElement.ValueKind) + { + case JsonValueKind.Null: + case JsonValueKind.True: + case JsonValueKind.False: + return true; + + case JsonValueKind.String: + return thisElement.ValueEquals(otherElement.GetString()); + case JsonValueKind.Number: + return thisElement.GetRawValue().Span.SequenceEqual(otherElement.GetRawValue().Span); + default: + Debug.Fail("Object and Array JsonElements cannot be contained in JsonValue."); + return false; + } + } + + using PooledByteBufferWriter thisOutput = WriteToPooledBuffer(); + using PooledByteBufferWriter otherOutput = otherNode.WriteToPooledBuffer(); + return thisOutput.WrittenMemory.Span.SequenceEqual(otherOutput.WrittenMemory.Span); + } + internal TypeToConvert ConvertJsonElement() { - JsonElement element = (JsonElement)(object)_value!; + JsonElement element = (JsonElement)(object)Value!; switch (element.ValueKind) { @@ -186,7 +225,7 @@ internal bool TryConvertJsonElement([NotNullWhen(true)] out TypeT { bool success; - JsonElement element = (JsonElement)(object)_value!; + JsonElement element = (JsonElement)(object)Value!; switch (element.ValueKind) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTCustomized.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTCustomized.cs new file mode 100644 index 0000000000000..916f7e9bcfc29 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTCustomized.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Text.Json.Serialization.Metadata; + +namespace System.Text.Json.Nodes +{ + /// + /// A JsonValue encapsulating arbitrary types using custom JsonTypeInfo metadata. + /// + internal sealed class JsonValueCustomized : JsonValue + { + private readonly JsonTypeInfo _jsonTypeInfo; + + public JsonValueCustomized(TValue value, JsonTypeInfo jsonTypeInfo, JsonNodeOptions? options = null) : base(value, options) + { + Debug.Assert(jsonTypeInfo.IsConfigured); + _jsonTypeInfo = jsonTypeInfo; + } + + public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? options = null) + { + if (writer is null) + { + ThrowHelper.ThrowArgumentNullException(nameof(writer)); + } + + JsonTypeInfo jsonTypeInfo = _jsonTypeInfo; + + if (options != null && options != jsonTypeInfo.Options) + { + options.MakeReadOnly(); + jsonTypeInfo = (JsonTypeInfo)options.GetTypeInfoInternal(typeof(TValue)); + } + + jsonTypeInfo.Serialize(writer, Value); + } + + internal override JsonNode DeepCloneCore() + { + return JsonSerializer.SerializeToNode(Value, _jsonTypeInfo)!; + } + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs new file mode 100644 index 0000000000000..b7a96c36ee1a3 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs @@ -0,0 +1,55 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; + +namespace System.Text.Json.Nodes +{ + /// + /// A JsonValue encapsulating a primitive value using a built-in converter for the type. + /// + internal sealed class JsonValuePrimitive : JsonValue + { + // Default value used when calling into the converter. + private static readonly JsonSerializerOptions s_defaultOptions = new(); + + private readonly JsonConverter _converter; + + public JsonValuePrimitive(TValue value, JsonConverter converter, JsonNodeOptions? options = null) : base(value, options) + { + Debug.Assert(converter is { IsInternalConverter: true, ConverterStrategy: ConverterStrategy.Value }); + _converter = converter; + } + + public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? options = null) + { + if (writer is null) + { + ThrowHelper.ThrowArgumentNullException(nameof(writer)); + } + + JsonConverter converter = _converter; + options ??= s_defaultOptions; + + if (converter.IsInternalConverterForNumberType) + { + converter.WriteNumberWithCustomHandling(writer, Value, options.NumberHandling); + } + else + { + converter.Write(writer, Value, options); + } + } + + internal override JsonNode DeepCloneCore() + { + // Primitive JsonValue's are generally speaking immutable so we don't to do much here. + // For the case of JsonElement clone the instance since it could be backed by pooled buffers. + return Value is JsonElement element + ? new JsonValuePrimitive(element.Clone(), JsonMetadataServices.JsonElementConverter, Options) + : new JsonValuePrimitive(Value, _converter, Options); + } + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs index d3a3ce5469079..89caeaa08f50e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs @@ -25,26 +25,13 @@ internal sealed class JsonNodeConverter : JsonConverter public override void Write(Utf8JsonWriter writer, JsonNode? value, JsonSerializerOptions options) { - if (value == null) + if (value is null) { writer.WriteNullValue(); - // Note JsonSerializer.Deserialize(JsonNode?) also calls WriteNullValue() for a null + root JsonNode. } else { - if (value is JsonValue jsonValue) - { - ValueConverter.Write(writer, jsonValue, options); - } - else if (value is JsonObject jsonObject) - { - ObjectConverter.Write(writer, jsonObject, options); - } - else - { - Debug.Assert(value is JsonArray); - ArrayConverter.Write(writer, (JsonArray)value, options); - } + value.WriteTo(writer, options); } } @@ -85,7 +72,7 @@ public override void Write(Utf8JsonWriter writer, JsonNode? value, JsonSerialize node = new JsonArray(element, options); break; default: - node = new JsonValueTrimmable(element, JsonMetadataServices.JsonElementConverter, options); + node = new JsonValuePrimitive(element, JsonMetadataServices.JsonElementConverter, options); break; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverterFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverterFactory.cs index b76d0fef05c06..cd898eb812d81 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverterFactory.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverterFactory.cs @@ -30,8 +30,6 @@ internal sealed class JsonNodeConverterFactory : JsonConverterFactory return JsonNodeConverter.Instance; } - public override bool CanConvert(Type typeToConvert) => - typeToConvert != JsonTypeInfo.ObjectType && - typeof(JsonNode).IsAssignableFrom(typeToConvert); + public override bool CanConvert(Type typeToConvert) => typeof(JsonNode).IsAssignableFrom(typeToConvert); } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs index 508e3bd61d0f0..6b09c360987dd 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs @@ -27,7 +27,7 @@ public override void Write(Utf8JsonWriter writer, JsonValue? value, JsonSerializ } JsonElement element = JsonElement.ParseValue(ref reader); - JsonValue value = new JsonValueTrimmable(element, JsonMetadataServices.JsonElementConverter, options.GetNodeOptions()); + JsonValue value = new JsonValuePrimitive(element, JsonMetadataServices.JsonElementConverter, options.GetNodeOptions()); return value; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs index 84928143a5f62..4511e150b37ee 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs @@ -131,7 +131,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, Debug.Assert(options.UnknownTypeHandling == JsonUnknownTypeHandling.JsonNode); - JsonNode node = JsonNodeConverter.Instance.Read(ref reader, typeToConvert, options)!; + JsonNode? node = JsonNodeConverter.Instance.Read(ref reader, typeToConvert, options); if (options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.Preserve && JsonSerializer.TryHandleReferenceFromJsonNode(ref reader, ref state, node, out referenceValue)) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs index df11d95a0565c..10a5f45454f2c 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Text.Json.Nodes; @@ -350,7 +351,7 @@ internal static bool TryHandleReferenceFromJsonElement( internal static bool TryHandleReferenceFromJsonNode( ref Utf8JsonReader reader, scoped ref ReadStack state, - JsonNode jsonNode, + JsonNode? jsonNode, [NotNullWhen(true)] out object? referenceValue) { bool refMetadataFound = false; @@ -359,7 +360,7 @@ internal static bool TryHandleReferenceFromJsonNode( if (jsonNode is JsonObject jsonObject) { int propertyCount = 0; - foreach (var property in jsonObject) + foreach (KeyValuePair property in jsonObject) { propertyCount++; if (refMetadataFound) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs index 201fca14143ee..ac804d6cc1159 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Caching.cs @@ -8,7 +8,6 @@ using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using System.Text.Json.Serialization; -using System.Text.Json.Serialization.Converters; using System.Text.Json.Serialization.Metadata; namespace System.Text.Json diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/Common.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/Common.cs index 573529f3e194c..0284dfe611f9c 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/Common.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/Common.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using Xunit; namespace System.Text.Json.Nodes.Tests { @@ -127,5 +128,31 @@ public static JsonObject GetManager() return manager; } } + + internal static void AssertDeepEqual(JsonNode? left, JsonNode? right) + { + // Assert Reflexivity + Assert.True(JsonNode.DeepEquals(left, left)); + Assert.True(JsonNode.DeepEquals(right, right)); + + // Core Assertion + Assert.True(JsonNode.DeepEquals(left, right)); + + // Assert Symmetry + Assert.True(JsonNode.DeepEquals(right, left)); + } + + internal static void AssertNotDeepEqual(JsonNode? left, JsonNode? right) + { + // Assert Reflexivity + Assert.True(JsonNode.DeepEquals(left, left)); + Assert.True(JsonNode.DeepEquals(right, right)); + + // Core assertion + Assert.False(JsonNode.DeepEquals(left, right)); + + // Assert Symmetry + Assert.False(JsonNode.DeepEquals(right, left)); + } } } diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs index 97dff770fe3bc..8f115d44b211f 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonArrayTests.cs @@ -509,7 +509,7 @@ public static void DeepClone() JsonArray clonedArray = array.DeepClone().AsArray(); - Assert.True(JsonNode.DeepEquals(array, clonedArray)); + JsonNodeTests.AssertDeepEqual(array, clonedArray); Assert.Equal(array.Count, clonedArray.Count); Assert.Equal(10, array[0].GetValue()); Assert.Equal("abcd", array[1].GetValue()); @@ -541,7 +541,7 @@ public static void DeepCloneFromElement() JsonArray jArray = JsonArray.Create(document.RootElement); var clone = jArray.DeepClone().AsArray(); - Assert.True(JsonNode.DeepEquals(jArray, clone)); + JsonNodeTests.AssertDeepEqual(jArray, clone); Assert.Equal(10, clone[1].GetValue()); Assert.Equal("abc", clone[0].GetValue()); } @@ -552,19 +552,14 @@ public static void DeepEquals() var array = new JsonArray() { null, 10, "str" }; var sameArray = new JsonArray() { null, 10, "str" }; - Assert.True(JsonNode.DeepEquals(array, array)); - Assert.True(JsonNode.DeepEquals(array, sameArray)); - Assert.True(JsonNode.DeepEquals(sameArray, array)); - - Assert.False(JsonNode.DeepEquals(array, null)); + JsonNodeTests.AssertDeepEqual(array, sameArray); + JsonNodeTests.AssertNotDeepEqual(array, null); var diffArray = new JsonArray() { null, 10, "s" }; - Assert.False(JsonNode.DeepEquals(array, diffArray)); - Assert.False(JsonNode.DeepEquals(diffArray, array)); + JsonNodeTests.AssertNotDeepEqual(array, diffArray); diffArray = new JsonArray() { null, 10 }; - Assert.False(JsonNode.DeepEquals(array, diffArray)); - Assert.False(JsonNode.DeepEquals(diffArray, array)); + JsonNodeTests.AssertNotDeepEqual(array, diffArray); } [Fact] @@ -576,7 +571,7 @@ public static void DeepEqualsWithJsonValueArrayType() array.Add(JsonValue.Create(4)); var value = JsonValue.Create(new long[] { 2, 3, 4 }); - Assert.True(JsonNode.DeepEquals(array, value)); + JsonNodeTests.AssertDeepEqual(array, value); } [Fact] @@ -587,11 +582,11 @@ public static void DeepEqualsFromElement() using JsonDocument document2 = JsonDocument.Parse("[1, 2, 4]"); JsonArray array2 = JsonArray.Create(document2.RootElement); - Assert.True(JsonNode.DeepEquals(array, array2)); + JsonNodeTests.AssertDeepEqual(array, array2); using JsonDocument document3 = JsonDocument.Parse("[2, 1, 4]"); JsonArray array3 = JsonArray.Create(document3.RootElement); - Assert.False(JsonNode.DeepEquals(array, array3)); + JsonNodeTests.AssertNotDeepEqual(array, array3); } [Fact] diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs index 5246739b82806..377d043f00294 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs @@ -995,7 +995,7 @@ public static void DeepClone() var clone = jObject.DeepClone().AsObject(); - Assert.True(JsonNode.DeepEquals(jObject, clone)); + JsonNodeTests.AssertDeepEqual(jObject, clone); Assert.Equal(jObject.Count, clone.Count); Assert.Equal(1, clone["One"].GetValue()); @@ -1029,7 +1029,7 @@ public static void DeepClone_FromElement() JsonObject jObject = JsonObject.Create(document.RootElement); var clone = jObject.DeepClone().AsObject(); - Assert.True(JsonNode.DeepEquals(jObject, clone)); + JsonNodeTests.AssertDeepEqual(jObject, clone); Assert.Equal(1, clone["One"].GetValue()); Assert.Equal("abc", clone["String"].GetValue()); } @@ -1045,19 +1045,13 @@ public static void DeepEquals() sameJObject["One"] = 1; sameJObject["array"] = new JsonArray() { "a", "b" }; - Assert.True(JsonNode.DeepEquals(jObject, jObject)); - Assert.True(JsonNode.DeepEquals(jObject, sameJObject)); - Assert.True(JsonNode.DeepEquals(sameJObject, jObject)); - - Assert.True(JsonNode.DeepEquals(null, null)); - Assert.False(JsonNode.DeepEquals(jObject, null)); - Assert.False(JsonNode.DeepEquals(null, jObject)); + JsonNodeTests.AssertDeepEqual(jObject, sameJObject); + JsonNodeTests.AssertNotDeepEqual(jObject, null); var diffJObject = new JsonObject(); diffJObject["One"] = 3; - Assert.False(JsonNode.DeepEquals(diffJObject, jObject)); - Assert.False(JsonNode.DeepEquals(jObject, diffJObject)); + JsonNodeTests.AssertNotDeepEqual(diffJObject, jObject); } [Fact] @@ -1083,7 +1077,7 @@ public static void DeepEquals_JsonObject_With_JsonValuePOCO() } }; - Assert.True(JsonNode.DeepEquals(jObject, JsonValue.Create(poco))); + JsonNodeTests.AssertDeepEqual(jObject, JsonValue.Create(poco)); var diffPoco = new SimpleClass() { @@ -1096,7 +1090,7 @@ public static void DeepEquals_JsonObject_With_JsonValuePOCO() } }; - Assert.False(JsonNode.DeepEquals(jObject, JsonValue.Create(diffPoco))); + JsonNodeTests.AssertNotDeepEqual(jObject, JsonValue.Create(diffPoco)); } [Fact] @@ -1114,7 +1108,7 @@ public static void DeepEquals_JsonObject_With_Dictionary() { "obj", new { } } }; - Assert.True(JsonNode.DeepEquals(jObject, JsonValue.Create(dictionary))); + JsonNodeTests.AssertDeepEqual(jObject, JsonValue.Create(dictionary)); var diffDictionary = new Dictionary() { @@ -1123,7 +1117,7 @@ public static void DeepEquals_JsonObject_With_Dictionary() { "obj", new { } } }; - Assert.False(JsonNode.DeepEquals(jObject, JsonValue.Create(diffDictionary))); + JsonNodeTests.AssertNotDeepEqual(jObject, JsonValue.Create(diffDictionary)); } private class SimpleClass @@ -1143,15 +1137,15 @@ public static void DeepEqualFromElement() using JsonDocument document2 = JsonDocument.Parse("{\"One\": 1, \"String\": \"abc\"} "); JsonObject jObject2 = JsonObject.Create(document2.RootElement); - Assert.True(JsonNode.DeepEquals(jObject, jObject2)); + JsonNodeTests.AssertDeepEqual(jObject, jObject2); using JsonDocument document3 = JsonDocument.Parse("{\"One\": 3, \"String\": \"abc\"}"); JsonObject jObject3 = JsonObject.Create(document3.RootElement); - Assert.False(JsonNode.DeepEquals(jObject, jObject3)); + JsonNodeTests.AssertNotDeepEqual(jObject, jObject3); using JsonDocument document4 = JsonDocument.Parse("{\"One\": 1, \"String\": \"abc2\"} "); JsonObject jObject4 = JsonObject.Create(document4.RootElement); - Assert.False(JsonNode.DeepEquals(jObject, jObject4)); + JsonNodeTests.AssertNotDeepEqual(jObject, jObject4); } [Fact] diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonValueTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonValueTests.cs index 47306ffd1d1eb..26cbc93555ef7 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonValueTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonValueTests.cs @@ -319,7 +319,7 @@ public static void DeepCloneNotTrimmable() JsonNode clone = jValue.DeepClone(); - Assert.True(JsonNode.DeepEquals(jValue, clone)); + JsonNodeTests.AssertDeepEqual(jValue, clone); string originalJson = jValue.ToJsonString(); string clonedJson = clone.ToJsonString(); @@ -338,7 +338,7 @@ public static void DeepCloneTrimmable(string json) JsonValue jsonValue = JsonValue.Create(document.RootElement); JsonNode clone = jsonValue.DeepClone(); - Assert.True(JsonNode.DeepEquals(jsonValue, clone)); + JsonNodeTests.AssertDeepEqual(jsonValue, clone); string originalJson = jsonValue.ToJsonString(); string clonedJson = clone.ToJsonString(); @@ -360,22 +360,22 @@ public static void DeepEqualsComplexType() jObject.Add("Id", 10); jObject.Add("Name", "test"); - Assert.True(JsonNode.DeepEquals(jValue, jObject)); - Assert.True(JsonNode.DeepEquals(jObject, jValue)); + JsonNodeTests.AssertDeepEqual(jValue, jObject); } [Fact] public static void DeepEqualsPrimitiveType() { - Assert.True(JsonNode.DeepEquals(JsonValue.Create(10), JsonValue.Create((uint)10))); - Assert.True(JsonNode.DeepEquals(JsonValue.Create(10), JsonValue.Create((ulong)10))); - Assert.True(JsonNode.DeepEquals(JsonValue.Create(10), JsonValue.Create((float)10))); - Assert.True(JsonNode.DeepEquals(JsonValue.Create(10), JsonValue.Create((decimal)10))); - Assert.True(JsonNode.DeepEquals(JsonValue.Create(10), JsonValue.Create((short)10))); - Assert.True(JsonNode.DeepEquals(JsonValue.Create(10), JsonValue.Create((ushort)10))); - Guid guid = Guid.NewGuid(); - Assert.True(JsonNode.DeepEquals(JsonValue.Create(guid), JsonValue.Create(guid.ToString()))); - Assert.False(JsonNode.DeepEquals(JsonValue.Create(10), JsonValue.Create("10"))); + JsonNodeTests.AssertDeepEqual(JsonValue.Create(10), JsonValue.Create((uint)10)); + JsonNodeTests.AssertDeepEqual(JsonValue.Create(10), JsonValue.Create((ulong)10)); + JsonNodeTests.AssertDeepEqual(JsonValue.Create(10), JsonValue.Create((float)10)); + JsonNodeTests.AssertDeepEqual(JsonValue.Create(10), JsonValue.Create((decimal)10)); + JsonNodeTests.AssertDeepEqual(JsonValue.Create(10), JsonValue.Create((short)10)); + JsonNodeTests.AssertDeepEqual(JsonValue.Create(10), JsonValue.Create((ushort)10)); + + Guid guid = Guid.Empty; + JsonNodeTests.AssertDeepEqual(JsonValue.Create(guid), JsonValue.Create(guid.ToString())); + JsonNodeTests.AssertNotDeepEqual(JsonValue.Create(10), JsonValue.Create("10")); } [Fact] @@ -385,13 +385,13 @@ public static void DeepEqualsJsonElement() JsonValue jsonValue1 = JsonValue.Create(document1.RootElement); - Assert.True(JsonNode.DeepEquals(jsonValue1, JsonValue.Create(10))); + JsonNodeTests.AssertDeepEqual(jsonValue1, JsonValue.Create(10)); JsonDocument document2 = JsonDocument.Parse("\"10\""); JsonValue jsonValue2 = JsonValue.Create(document2.RootElement); - Assert.False(JsonNode.DeepEquals(jsonValue1, jsonValue2)); - Assert.True(JsonNode.DeepEquals(jsonValue2, JsonValue.Create("10"))); + JsonNodeTests.AssertNotDeepEqual(jsonValue1, jsonValue2); + JsonNodeTests.AssertDeepEqual(jsonValue2, JsonValue.Create("10")); } [Fact] @@ -400,8 +400,8 @@ public static void DeepEqualsJsonElement_Boolean() JsonValue trueValue = JsonValue.Create(JsonDocument.Parse("true").RootElement); JsonValue falseValue = JsonValue.Create(JsonDocument.Parse("false").RootElement); - Assert.False(JsonNode.DeepEquals(trueValue, falseValue)); - Assert.True(JsonNode.DeepEquals(trueValue, trueValue.DeepClone())); + JsonNodeTests.AssertNotDeepEqual(trueValue, falseValue); + JsonNodeTests.AssertDeepEqual(trueValue, trueValue.DeepClone()); } [Fact] @@ -421,8 +421,8 @@ public static void GetValueKind() public static void DeepEquals_EscapedString() { JsonValue jsonValue = JsonValue.Create(JsonDocument.Parse("\"It\'s alright\"").RootElement); - JsonValue escapedJsonValue = JsonValue.Create(JsonDocument.Parse("\"It\u0027s alright\"").RootElement); - Assert.True(JsonNode.DeepEquals(escapedJsonValue, jsonValue)); + JsonValue escapedJsonValue = JsonValue.Create(JsonDocument.Parse("\"It\\u0027s alright\"").RootElement); + JsonNodeTests.AssertDeepEqual(escapedJsonValue, jsonValue); } private class Student From eabb5b9d0648e9b86f4d94e200dcad4c211f1e46 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 29 Jun 2023 15:54:56 +0100 Subject: [PATCH 3/4] Update src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs --- .../src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs index b7a96c36ee1a3..87be927a29222 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValueOfTPrimitive.cs @@ -45,7 +45,7 @@ public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? optio internal override JsonNode DeepCloneCore() { - // Primitive JsonValue's are generally speaking immutable so we don't to do much here. + // Primitive JsonValue's are generally speaking immutable so we don't need to do much here. // For the case of JsonElement clone the instance since it could be backed by pooled buffers. return Value is JsonElement element ? new JsonValuePrimitive(element.Clone(), JsonMetadataServices.JsonElementConverter, Options) From 02376c59cc14168ccc34946c9a7442cea354f681 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 29 Jun 2023 19:28:16 +0100 Subject: [PATCH 4/4] Avoid delegate allocation in the JsonObject setter. --- .../src/System/Text/Json/JsonPropertyDictionary.cs | 10 +++++----- .../src/System/Text/Json/Nodes/JsonObject.cs | 13 +++++++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs index 7642074b506ab..6330cf4154836 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/JsonPropertyDictionary.cs @@ -199,11 +199,11 @@ public T? this[string propertyName] set { - SetValue(propertyName, value); + SetValue(propertyName, value, out bool _); } } - public T? SetValue(string propertyName, T value, Action? assignParent = null) + public T? SetValue(string propertyName, T value, out bool valueAlreadyInDictionary) { if (IsReadOnly) { @@ -217,6 +217,7 @@ public T? this[string propertyName] CreateDictionaryIfThresholdMet(); + valueAlreadyInDictionary = false; T? existing = null; if (_propertyDictionary != null) @@ -224,7 +225,6 @@ public T? this[string propertyName] // Fast path if item doesn't exist in dictionary. if (_propertyDictionary.TryAdd(propertyName, value)) { - assignParent?.Invoke(); _propertyList.Add(new KeyValuePair(propertyName, value)); return null; } @@ -233,6 +233,7 @@ public T? this[string propertyName] if (ReferenceEquals(existing, value)) { // Ignore if the same value. + valueAlreadyInDictionary = true; return null; } } @@ -250,18 +251,17 @@ public T? this[string propertyName] if (ReferenceEquals(current.Value, value)) { // Ignore if the same value. + valueAlreadyInDictionary = true; return null; } existing = current.Value; } - assignParent?.Invoke(); _propertyList[i] = new KeyValuePair(propertyName, value); } else { - assignParent?.Invoke(); _propertyDictionary?.Add(propertyName, value); _propertyList.Add(new KeyValuePair(propertyName, value)); Debug.Assert(existing == null); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs index 6272b62a26558..043b2e246035e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs @@ -216,14 +216,19 @@ internal override void GetPath(List path, JsonNode? child) internal void SetItem(string propertyName, JsonNode? value) { - JsonNode? existing = Dictionary.SetValue(propertyName, value, () => value?.AssignParent(this)); - DetachParent(existing); + JsonNode? replacedValue = Dictionary.SetValue(propertyName, value, out bool valueAlreadyInDictionary); + + if (!valueAlreadyInDictionary) + { + value?.AssignParent(this); + } + + DetachParent(replacedValue); } private void DetachParent(JsonNode? item) { - InitializeDictionary(); - Debug.Assert(_dictionary != null); + Debug.Assert(_dictionary != null, "Cannot have detachable nodes without a materialized dictionary."); if (item != null) {