From 2a3106bbc85c50dc2b81deee701753dee7f37de4 Mon Sep 17 00:00:00 2001 From: Adrian Fluturel Date: Thu, 9 Jan 2025 06:29:19 +0100 Subject: [PATCH 1/8] Implement TryAdd and TryAdd with index out parameter for JsonObject --- .../Text/Json/Nodes/JsonObject.IDictionary.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) 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 3de8d933a1b070..5e7c091ca3f5a0 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 @@ -34,6 +34,43 @@ public void Add(string propertyName, JsonNode? value) value?.AssignParent(this); } + /// + /// Adds an element with the provided name and value to the , if a property named doesn't already exist.. + /// + /// The property name of the element to add. + /// The value of the element to add. + /// is null. + /// + /// if the property didn't exist and the element was added; otherwise, . + /// + public bool TryAdd(string propertyName, JsonNode? value) => TryAdd(propertyName, value, out _); + + /// + /// Adds an element with the provided name and value to the , if a property named doesn't already exist. + /// + /// The property name of the element to add. + /// The value of the element to add. + /// The index of the added or existing . This is always a valid index into the . + /// is null. + /// + /// if the property didn't exist and the element was added; otherwise, . + /// + public bool TryAdd(string propertyName, JsonNode? value, out int index) + { + if (propertyName is null) + { + ThrowHelper.ThrowArgumentNullException(nameof(propertyName)); + } + + var success = Dictionary.TryAdd(propertyName, value, out index); + if (success) + { + value?.AssignParent(this); + } + + return success; + } + /// /// Adds the specified property to the . /// From 2252001d915d399339c71a0df575be393880b233 Mon Sep 17 00:00:00 2001 From: Adrian Fluturel Date: Thu, 9 Jan 2025 10:14:05 +0100 Subject: [PATCH 2/8] Add exception to docs for TryGetPropertyValue without out index --- .../System.Text.Json/src/System/Text/Json/Nodes/JsonObject.cs | 3 +++ 1 file changed, 3 insertions(+) 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 9844bb59207cf4..406d694cbe3d89 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 @@ -115,6 +115,9 @@ internal string GetPropertyName(JsonNode? node) /// /// The name of the property to return. /// The JSON value of the property with the specified name. + /// + /// is . + /// /// /// if a property with the specified name was found; otherwise, . /// From 76c0f514820d95a3b9f0963faa9ae8cf8a251028 Mon Sep 17 00:00:00 2001 From: Adrian Fluturel Date: Thu, 9 Jan 2025 10:14:41 +0100 Subject: [PATCH 3/8] Add overload for TryGetPropertyValue with out index parameter --- .../src/System/Text/Json/Nodes/JsonObject.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) 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 406d694cbe3d89..8488e0d478760e 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 @@ -131,6 +131,31 @@ public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode) return Dictionary.TryGetValue(propertyName, out jsonNode); } + /// + /// Gets the value associated with the specified property name. + /// + /// The property name of the value to get. + /// + /// When this method returns, it contains the value associated with the specified property name, if the property name is found; + /// otherwise . + /// + /// The index of if found; otherwise, -1. + /// + /// is . + /// + /// + /// if the contains an element with the specified property name; otherwise, . + /// + public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode, out int index) + { + if (propertyName is null) + { + ThrowHelper.ThrowArgumentNullException(nameof(propertyName)); + } + + return Dictionary.TryGetValue(propertyName, out jsonNode, out index); + } + /// public override void WriteTo(Utf8JsonWriter writer, JsonSerializerOptions? options = null) { From 6121c38df22063d5e53ddb855b32a7f662ebb3bf Mon Sep 17 00:00:00 2001 From: Adrian Fluturel Date: Thu, 9 Jan 2025 10:23:25 +0100 Subject: [PATCH 4/8] Modify TryGetPropertyValue to use its overload with the index param --- .../src/System/Text/Json/Nodes/JsonObject.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) 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 8488e0d478760e..32b3e7456e238f 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 @@ -121,15 +121,7 @@ internal string GetPropertyName(JsonNode? node) /// /// if a property with the specified name was found; otherwise, . /// - public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode) - { - if (propertyName is null) - { - ThrowHelper.ThrowArgumentNullException(nameof(propertyName)); - } - - return Dictionary.TryGetValue(propertyName, out jsonNode); - } + public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode) => TryGetPropertyValue(propertyName, out jsonNode, out _); /// /// Gets the value associated with the specified property name. From 3ded48514d1b6fce216c450cf3942a039e1dca4a Mon Sep 17 00:00:00 2001 From: Adrian Fluturel Date: Sat, 11 Jan 2025 03:46:23 +0100 Subject: [PATCH 5/8] Add new functions to ref --- src/libraries/System.Text.Json/ref/System.Text.Json.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs index 469ea5e4f07c89..b9640d8990d953 100644 --- a/src/libraries/System.Text.Json/ref/System.Text.Json.cs +++ b/src/libraries/System.Text.Json/ref/System.Text.Json.cs @@ -842,6 +842,8 @@ public sealed partial class JsonObject : System.Text.Json.Nodes.JsonNode, System System.Collections.Generic.KeyValuePair System.Collections.Generic.IList>.this[int index] { get { throw null; } set { } } public void Add(System.Collections.Generic.KeyValuePair property) { } public void Add(string propertyName, System.Text.Json.Nodes.JsonNode? value) { } + public bool TryAdd(string propertyName, JsonNode? value) { throw null; } + public bool TryAdd(string propertyName, JsonNode? value, out int index) { throw null; } public void Clear() { } public bool ContainsKey(string propertyName) { throw null; } public static System.Text.Json.Nodes.JsonObject? Create(System.Text.Json.JsonElement element, System.Text.Json.Nodes.JsonNodeOptions? options = default(System.Text.Json.Nodes.JsonNodeOptions?)) { throw null; } @@ -862,6 +864,7 @@ public void SetAt(int index, System.Text.Json.Nodes.JsonNode? value) { } void System.Collections.Generic.IList>.RemoveAt(int index) { } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } public bool TryGetPropertyValue(string propertyName, out System.Text.Json.Nodes.JsonNode? jsonNode) { throw null; } + public bool TryGetPropertyValue(string propertyName, out System.Text.Json.Nodes.JsonNode? jsonNode, out int index) { throw null; } public override void WriteTo(System.Text.Json.Utf8JsonWriter writer, System.Text.Json.JsonSerializerOptions? options = null) { } } public abstract partial class JsonValue : System.Text.Json.Nodes.JsonNode From 1672a5b73433d2bb33f66f0af50c52cadd11ee3f Mon Sep 17 00:00:00 2001 From: Adrian Fluturel Date: Sat, 11 Jan 2025 03:50:47 +0100 Subject: [PATCH 6/8] Add unit tests --- .../JsonNode/JsonObjectTests.cs | 63 +++++++++++++++++++ .../JsonNode/ParseTests.cs | 54 ++++++++++++++++ 2 files changed, 117 insertions(+) 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 ac8258de22d71e..fa8ceb005148c8 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 @@ -1604,6 +1604,69 @@ public static void JsonObject_IsIList() Assert.Equal(-1, jObject.IndexOf("Two")); } + [Fact] + public static void TryAdd_NewKey_EmptyJsonObject() + { + JsonObject jObject = new(); + + Assert.True(jObject.TryAdd("First", "value", out var index)); + Assert.Equal(0, index); + } + + [Fact] + public static void TryAdd_NewKey() + { + JsonObject jObject = new() + { + ["One"] = 1, + ["Two"] = "str", + ["Three"] = null, + }; + + Assert.True(jObject.TryAdd("Four", 33, out var index)); + Assert.Equal(3, index); + } + + [Fact] + public static void TryAdd_ExistingKey() + { + JsonObject jObject = new() + { + ["One"] = 1, + ["Two"] = "str", + ["Three"] = null, + }; + + Assert.False(jObject.TryAdd("Two", 33, out var index)); + Assert.Equal(1, index); + } + + [Fact] + public static void TryAdd_ThrowsArgumentNullException() + { + JsonObject jObject = new() + { + ["One"] = 1, + ["Two"] = "str", + ["Three"] = null, + }; + + Assert.Throws(() => { jObject.TryAdd(null, 33); }); + } + + [Fact] + public static void TryGetPropertyValue_ThrowsArgumentNullException() + { + JsonObject jObject = new() + { + ["One"] = 1, + ["Two"] = "str", + ["Three"] = null, + }; + + Assert.Throws(() => { jObject.TryGetPropertyValue(null, out var jsonNode, out var index); }); + } + [Theory] [InlineData(10_000)] [InlineData(50_000)] diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/ParseTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/ParseTests.cs index 7c6e82db5d30bd..81619d89665c9b 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/ParseTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/ParseTests.cs @@ -84,6 +84,60 @@ public static void Parse_TryGetPropertyValue() Assert.IsType(node); } + [Fact] + public static void Parse_TryGetPropertyValueWithIndex() + { + JsonObject jObject = JsonNode.Parse(JsonNodeTests.ExpectedDomJson).AsObject(); + + JsonNode? node; + int index; + + Assert.True(jObject.TryGetPropertyValue("MyString", out node, out index)); + Assert.Equal("Hello!", node.GetValue()); + Assert.Equal(0, index); + + Assert.True(jObject.TryGetPropertyValue("MyNull", out node, out index)); + Assert.Null(node); + Assert.Equal(1, index); + + Assert.True(jObject.TryGetPropertyValue("MyBoolean", out node, out index)); + Assert.False(node.GetValue()); + Assert.Equal(2, index); + + Assert.True(jObject.TryGetPropertyValue("MyArray", out node, out index)); + Assert.IsType(node); + Assert.Equal(3, index); + + Assert.True(jObject.TryGetPropertyValue("MyInt", out node, out index)); + Assert.Equal(43, node.GetValue()); + Assert.Equal(4, index); + + Assert.True(jObject.TryGetPropertyValue("MyDateTime", out node, out index)); + Assert.Equal("2020-07-08T00:00:00", node.GetValue()); + Assert.Equal(5, index); + + Assert.True(jObject.TryGetPropertyValue("MyGuid", out node, out index)); + Assert.Equal("ed957609-cdfe-412f-88c1-02daca1b4f51", node.AsValue().GetValue().ToString()); + Assert.Equal(6, index); + + Assert.True(jObject.TryGetPropertyValue("MyObject", out node, out index)); + Assert.IsType(node); + Assert.Equal(7, index); + } + + [Fact] + public static void Parse_TryGetPropertyValueFail() + { + JsonObject jObject = JsonNode.Parse(JsonNodeTests.ExpectedDomJson).AsObject(); + + JsonNode? node; + int index; + + Assert.False(jObject.TryGetPropertyValue("NonExistentKey", out node, out index)); + Assert.Null(node); + Assert.Equal(-1, index); + } + [Fact] public static void Parse_TryGetValue() { From 1fa1a6dba465c345f9a91d0d9b0f476fe1de45da Mon Sep 17 00:00:00 2001 From: Adrian Fluturel Date: Mon, 13 Jan 2025 19:05:04 +0100 Subject: [PATCH 7/8] Fix build for .NET 9 --- .../src/System/Text/Json/Nodes/JsonObject.IDictionary.cs | 8 ++++++-- .../src/System/Text/Json/Nodes/JsonObject.cs | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) 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 5e7c091ca3f5a0..6a7e8e017a71d2 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 @@ -35,7 +35,7 @@ public void Add(string propertyName, JsonNode? value) } /// - /// Adds an element with the provided name and value to the , if a property named doesn't already exist.. + /// Adds an element with the provided name and value to the , if a property named doesn't already exist. /// /// The property name of the element to add. /// The value of the element to add. @@ -62,12 +62,16 @@ public bool TryAdd(string propertyName, JsonNode? value, out int index) ThrowHelper.ThrowArgumentNullException(nameof(propertyName)); } +#if NET10_0_OR_GREATER var success = Dictionary.TryAdd(propertyName, value, out index); +#else + var success = Dictionary.TryAdd(propertyName, value); + index = Dictionary.IndexOf(propertyName); +#endif if (success) { value?.AssignParent(this); } - return success; } 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 32b3e7456e238f..f59914307687d0 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 @@ -145,7 +145,12 @@ public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode, out ThrowHelper.ThrowArgumentNullException(nameof(propertyName)); } +#if NET10_0_OR_GREATER return Dictionary.TryGetValue(propertyName, out jsonNode, out index); +#else + index = Dictionary.IndexOf(propertyName); + return Dictionary.TryGetValue(propertyName, out jsonNode); +#endif } /// From 556a847f11dfe162e9321feae63dc3cb0a15ed1b Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Tue, 1 Jul 2025 14:54:21 +0300 Subject: [PATCH 8/8] Address remaining feedback. --- .../Text/Json/Nodes/JsonObject.IDictionary.cs | 9 ++++----- .../src/System/Text/Json/Nodes/JsonObject.cs | 15 +++++++++++---- .../Converters/Node/JsonObjectConverter.cs | 10 ++-------- 3 files changed, 17 insertions(+), 17 deletions(-) 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 f91922a9ab49c6..f86362c9eea2ca 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 @@ -58,12 +58,11 @@ public bool TryAdd(string propertyName, JsonNode? value, out int index) { ThrowHelper.ThrowArgumentNullException(nameof(propertyName)); } - -#if NET10_0_OR_GREATER - var success = Dictionary.TryAdd(propertyName, value, out index); +#if NET9_0 + bool success = Dictionary.TryAdd(propertyName, value); + index = success ? Dictionary.Count - 1 : Dictionary.IndexOf(propertyName); #else - var success = Dictionary.TryAdd(propertyName, value); - index = Dictionary.IndexOf(propertyName); + bool success = Dictionary.TryAdd(propertyName, value, out index); #endif if (success) { 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 285c240d9a1960..ebab8d554de25e 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 @@ -142,11 +142,18 @@ public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode, out { ArgumentNullException.ThrowIfNull(propertyName); -#if NET10_0_OR_GREATER - return Dictionary.TryGetValue(propertyName, out jsonNode, out index); -#else +#if NET9_0 index = Dictionary.IndexOf(propertyName); - return Dictionary.TryGetValue(propertyName, out jsonNode); + if (index < 0) + { + jsonNode = null; + return false; + } + + jsonNode = Dictionary.GetAt(index).Value; + return true; +#else + return Dictionary.TryGetValue(propertyName, out jsonNode, out index); #endif } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs index c824dd97952198..fb97edcf75f54d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs @@ -35,15 +35,9 @@ internal override void ReadElementAndSetProperty( { jObject[propertyName] = jNodeValue; } - else + else if (!jObject.TryAdd(propertyName, jNodeValue)) { - // TODO: Use TryAdd once https://github.com/dotnet/runtime/issues/110244 is resolved. - if (jObject.ContainsKey(propertyName)) - { - ThrowHelper.ThrowJsonException_DuplicatePropertyNotAllowed(propertyName); - } - - jObject.Add(propertyName, jNodeValue); + ThrowHelper.ThrowJsonException_DuplicatePropertyNotAllowed(propertyName); } }