From 4e94c756cba90e11ed849680aa0bade6309f577c Mon Sep 17 00:00:00 2001 From: Sneha Shivakumar Date: Tue, 27 Sep 2022 17:33:26 -0700 Subject: [PATCH 1/9] Add JsonCloneSettings to disable copy annotations --- .../Linq/AnnotationsTests.cs | 15 +++++++++++++++ Src/Newtonsoft.Json/Linq/JArray.cs | 2 +- Src/Newtonsoft.Json/Linq/JConstructor.cs | 2 +- Src/Newtonsoft.Json/Linq/JContainer.cs | 7 +++++-- Src/Newtonsoft.Json/Linq/JObject.cs | 8 ++++---- Src/Newtonsoft.Json/Linq/JProperty.cs | 2 +- Src/Newtonsoft.Json/Linq/JRaw.cs | 2 +- Src/Newtonsoft.Json/Linq/JToken.cs | 12 +++++++++++- Src/Newtonsoft.Json/Linq/JValue.cs | 12 ++++++++---- Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs | 18 ++++++++++++++++++ 10 files changed, 65 insertions(+), 15 deletions(-) create mode 100644 Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs diff --git a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs index 28af83764..e86f04810 100644 --- a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs +++ b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs @@ -308,6 +308,21 @@ public void MultipleAnnotationsAreCopied() Assert.AreEqual(annotation, t.DeepClone().Annotation()); } + [Test] + public void MultipleAnnotationsAreNotCopiedWithSetting() + { + Version version = new Version(1, 2, 3, 4); + + JObject o = new JObject(); + o.AddAnnotation("string!"); + o.AddAnnotation(version); + + JsonCloneSettings settings = new JsonCloneSettings() { CopyAnnotations = false }; + JObject o2 = (JObject)o.DeepClone(settings); + Assert.IsNull(o2.Annotation()); + Assert.AreEqual(0, o2.Annotations().Count()); + } + #if !NET20 [Test] public void Example() diff --git a/Src/Newtonsoft.Json/Linq/JArray.cs b/Src/Newtonsoft.Json/Linq/JArray.cs index 241a56045..5446a9b14 100644 --- a/Src/Newtonsoft.Json/Linq/JArray.cs +++ b/Src/Newtonsoft.Json/Linq/JArray.cs @@ -93,7 +93,7 @@ internal override bool DeepEquals(JToken node) return (node is JArray t && ContentsEqual(t)); } - internal override JToken CloneToken() + internal override JToken CloneToken(JsonCloneSettings? settings = null) { return new JArray(this); } diff --git a/Src/Newtonsoft.Json/Linq/JConstructor.cs b/Src/Newtonsoft.Json/Linq/JConstructor.cs index f2aa86182..4626470ef 100644 --- a/Src/Newtonsoft.Json/Linq/JConstructor.cs +++ b/Src/Newtonsoft.Json/Linq/JConstructor.cs @@ -147,7 +147,7 @@ internal override bool DeepEquals(JToken node) return (node is JConstructor c && _name == c.Name && ContentsEqual(c)); } - internal override JToken CloneToken() + internal override JToken CloneToken(JsonCloneSettings? settings = null) { return new JConstructor(this); } diff --git a/Src/Newtonsoft.Json/Linq/JContainer.cs b/Src/Newtonsoft.Json/Linq/JContainer.cs index f9447cd5d..565f8b3df 100644 --- a/Src/Newtonsoft.Json/Linq/JContainer.cs +++ b/Src/Newtonsoft.Json/Linq/JContainer.cs @@ -106,7 +106,7 @@ internal JContainer() { } - internal JContainer(JContainer other) + internal JContainer(JContainer other, JsonCloneSettings? settings = null) : this() { ValidationUtils.ArgumentNotNull(other, nameof(other)); @@ -118,7 +118,10 @@ internal JContainer(JContainer other) i++; } - CopyAnnotations(this, other); + if (settings?.CopyAnnotations ?? true) + { + CopyAnnotations(this, other); + } } internal void CheckReentrancy() diff --git a/Src/Newtonsoft.Json/Linq/JObject.cs b/Src/Newtonsoft.Json/Linq/JObject.cs index 872c8ae7c..96a49f2bf 100644 --- a/Src/Newtonsoft.Json/Linq/JObject.cs +++ b/Src/Newtonsoft.Json/Linq/JObject.cs @@ -92,8 +92,8 @@ public JObject() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - public JObject(JObject other) - : base(other) + public JObject(JObject other, JsonCloneSettings? settings = null) + : base(other, settings) { } @@ -244,9 +244,9 @@ internal void InternalPropertyChanging(JProperty childProperty) #endif } - internal override JToken CloneToken() + internal override JToken CloneToken(JsonCloneSettings? settings = null) { - return new JObject(this); + return new JObject(this, settings); } /// diff --git a/Src/Newtonsoft.Json/Linq/JProperty.cs b/Src/Newtonsoft.Json/Linq/JProperty.cs index 3c738cf52..d54276890 100644 --- a/Src/Newtonsoft.Json/Linq/JProperty.cs +++ b/Src/Newtonsoft.Json/Linq/JProperty.cs @@ -282,7 +282,7 @@ internal override bool DeepEquals(JToken node) return (node is JProperty t && _name == t.Name && ContentsEqual(t)); } - internal override JToken CloneToken() + internal override JToken CloneToken(JsonCloneSettings? settings = null) { return new JProperty(this); } diff --git a/Src/Newtonsoft.Json/Linq/JRaw.cs b/Src/Newtonsoft.Json/Linq/JRaw.cs index 02ca51fd1..6be6cdfa0 100644 --- a/Src/Newtonsoft.Json/Linq/JRaw.cs +++ b/Src/Newtonsoft.Json/Linq/JRaw.cs @@ -67,7 +67,7 @@ public static JRaw Create(JsonReader reader) } } - internal override JToken CloneToken() + internal override JToken CloneToken(JsonCloneSettings? settings = null) { return new JRaw(this); } diff --git a/Src/Newtonsoft.Json/Linq/JToken.cs b/Src/Newtonsoft.Json/Linq/JToken.cs index 336d748aa..f6bb7739e 100644 --- a/Src/Newtonsoft.Json/Linq/JToken.cs +++ b/Src/Newtonsoft.Json/Linq/JToken.cs @@ -131,7 +131,7 @@ public JToken Root } } - internal abstract JToken CloneToken(); + internal abstract JToken CloneToken(JsonCloneSettings? settings = null); internal abstract bool DeepEquals(JToken node); /// @@ -2446,6 +2446,16 @@ public JToken DeepClone() return CloneToken(); } + /// + /// Creates a new instance of the . All child tokens are recursively cloned. + /// + /// A flag to indicate whether to clone annotations when performing deep clone on a JToken. + /// A new instance of the . + public JToken DeepClone(JsonCloneSettings settings) + { + return CloneToken(settings); + } + /// /// Adds an object to the annotation list of this . /// diff --git a/Src/Newtonsoft.Json/Linq/JValue.cs b/Src/Newtonsoft.Json/Linq/JValue.cs index 09ed1fd42..5cb60b862 100644 --- a/Src/Newtonsoft.Json/Linq/JValue.cs +++ b/Src/Newtonsoft.Json/Linq/JValue.cs @@ -61,10 +61,14 @@ internal JValue(object? value, JTokenType type) /// Initializes a new instance of the class from another object. /// /// A object to copy from. - public JValue(JValue other) + /// A object that specifies the settings used when cloning JSON. + public JValue(JValue other, JsonCloneSettings? settings = null) : this(other.Value, other.Type) { - CopyAnnotations(this, other); + if (settings?.CopyAnnotations ?? true) + { + CopyAnnotations(this, other); + } } /// @@ -557,9 +561,9 @@ private static bool Operation(ExpressionType operation, object? objA, object? ob } #endif - internal override JToken CloneToken() + internal override JToken CloneToken(JsonCloneSettings? settings = null) { - return new JValue(this); + return new JValue(this, settings); } /// diff --git a/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs b/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs new file mode 100644 index 000000000..cae4507ee --- /dev/null +++ b/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs @@ -0,0 +1,18 @@ +using System; + +namespace Newtonsoft.Json.Linq +{ + /// + /// Specifies the settings used when selecting JSON. + /// + public class JsonCloneSettings + { + /// + /// Gets or sets a flag that indicates whether to copy annotations when cloning a JToken. + /// + /// + /// A flag that indicates whether to copy annotations when cloning a JToken. + /// + public bool CopyAnnotations { get; set; } + } +} \ No newline at end of file From cb5650ceee69edc6efb07eca8c149ab777513298 Mon Sep 17 00:00:00 2001 From: Sneha Shivakumar Date: Wed, 5 Oct 2022 17:40:00 -0700 Subject: [PATCH 2/9] Fix comment --- Src/Newtonsoft.Json/Linq/JObject.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Src/Newtonsoft.Json/Linq/JObject.cs b/Src/Newtonsoft.Json/Linq/JObject.cs index 96a49f2bf..f6727e1bd 100644 --- a/Src/Newtonsoft.Json/Linq/JObject.cs +++ b/Src/Newtonsoft.Json/Linq/JObject.cs @@ -92,6 +92,7 @@ public JObject() /// Initializes a new instance of the class from another object. /// /// A object to copy from. + /// A object indicating whether to clone annotations when cloning a JToken. public JObject(JObject other, JsonCloneSettings? settings = null) : base(other, settings) { From 6b0c863c4d37c5de20ac7ffb228daa73607a4de6 Mon Sep 17 00:00:00 2001 From: Sneha Shivakumar Date: Wed, 26 Oct 2022 15:26:37 -0700 Subject: [PATCH 3/9] Update all other class constructors to consume new settings object. --- Src/Newtonsoft.Json/Linq/JArray.cs | 9 +++++---- Src/Newtonsoft.Json/Linq/JConstructor.cs | 4 ++-- Src/Newtonsoft.Json/Linq/JProperty.cs | 6 +++--- Src/Newtonsoft.Json/Linq/JRaw.cs | 6 +++--- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/Src/Newtonsoft.Json/Linq/JArray.cs b/Src/Newtonsoft.Json/Linq/JArray.cs index 5446a9b14..29a4c692a 100644 --- a/Src/Newtonsoft.Json/Linq/JArray.cs +++ b/Src/Newtonsoft.Json/Linq/JArray.cs @@ -65,8 +65,9 @@ public JArray() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - public JArray(JArray other) - : base(other) + /// + public JArray(JArray other, JsonCloneSettings? settings = null) + : base(other, settings) { } @@ -78,7 +79,7 @@ public JArray(params object[] content) : this((object)content) { } - + /// /// Initializes a new instance of the class with the specified content. /// @@ -95,7 +96,7 @@ internal override bool DeepEquals(JToken node) internal override JToken CloneToken(JsonCloneSettings? settings = null) { - return new JArray(this); + return new JArray(this, settings); } /// diff --git a/Src/Newtonsoft.Json/Linq/JConstructor.cs b/Src/Newtonsoft.Json/Linq/JConstructor.cs index 4626470ef..937a6edb0 100644 --- a/Src/Newtonsoft.Json/Linq/JConstructor.cs +++ b/Src/Newtonsoft.Json/Linq/JConstructor.cs @@ -96,8 +96,8 @@ public JConstructor() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - public JConstructor(JConstructor other) - : base(other) + public JConstructor(JConstructor other, JsonCloneSettings? settings = null) + : base(other, settings) { _name = other.Name; } diff --git a/Src/Newtonsoft.Json/Linq/JProperty.cs b/Src/Newtonsoft.Json/Linq/JProperty.cs index d54276890..11ae7c399 100644 --- a/Src/Newtonsoft.Json/Linq/JProperty.cs +++ b/Src/Newtonsoft.Json/Linq/JProperty.cs @@ -186,8 +186,8 @@ public JToken Value /// Initializes a new instance of the class from another object. /// /// A object to copy from. - public JProperty(JProperty other) - : base(other) + public JProperty(JProperty other, JsonCloneSettings? settings = null) + : base(other, settings) { _name = other.Name; } @@ -284,7 +284,7 @@ internal override bool DeepEquals(JToken node) internal override JToken CloneToken(JsonCloneSettings? settings = null) { - return new JProperty(this); + return new JProperty(this, settings); } /// diff --git a/Src/Newtonsoft.Json/Linq/JRaw.cs b/Src/Newtonsoft.Json/Linq/JRaw.cs index 6be6cdfa0..c0694e793 100644 --- a/Src/Newtonsoft.Json/Linq/JRaw.cs +++ b/Src/Newtonsoft.Json/Linq/JRaw.cs @@ -37,8 +37,8 @@ public partial class JRaw : JValue /// Initializes a new instance of the class from another object. /// /// A object to copy from. - public JRaw(JRaw other) - : base(other) + public JRaw(JRaw other, JsonCloneSettings? settings = null) + : base(other, settings) { } @@ -69,7 +69,7 @@ public static JRaw Create(JsonReader reader) internal override JToken CloneToken(JsonCloneSettings? settings = null) { - return new JRaw(this); + return new JRaw(this, settings); } } } \ No newline at end of file From 83fbf1d35b8240b74f05cb5995c8f7fe72be3059 Mon Sep 17 00:00:00 2001 From: Sneha Shivakumar Date: Wed, 26 Oct 2022 15:54:37 -0700 Subject: [PATCH 4/9] Fixing comments. --- Src/Newtonsoft.Json/Linq/JArray.cs | 2 +- Src/Newtonsoft.Json/Linq/JConstructor.cs | 1 + Src/Newtonsoft.Json/Linq/JProperty.cs | 1 + Src/Newtonsoft.Json/Linq/JRaw.cs | 1 + Src/Newtonsoft.Json/Linq/JToken.cs | 2 +- Src/Newtonsoft.Json/Linq/JValue.cs | 2 +- 6 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Src/Newtonsoft.Json/Linq/JArray.cs b/Src/Newtonsoft.Json/Linq/JArray.cs index 29a4c692a..d427b1be2 100644 --- a/Src/Newtonsoft.Json/Linq/JArray.cs +++ b/Src/Newtonsoft.Json/Linq/JArray.cs @@ -65,7 +65,7 @@ public JArray() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// + /// A object to configure cloning settings. public JArray(JArray other, JsonCloneSettings? settings = null) : base(other, settings) { diff --git a/Src/Newtonsoft.Json/Linq/JConstructor.cs b/Src/Newtonsoft.Json/Linq/JConstructor.cs index 937a6edb0..d05f6e9e1 100644 --- a/Src/Newtonsoft.Json/Linq/JConstructor.cs +++ b/Src/Newtonsoft.Json/Linq/JConstructor.cs @@ -96,6 +96,7 @@ public JConstructor() /// Initializes a new instance of the class from another object. /// /// A object to copy from. + /// A object to configure cloning settings. public JConstructor(JConstructor other, JsonCloneSettings? settings = null) : base(other, settings) { diff --git a/Src/Newtonsoft.Json/Linq/JProperty.cs b/Src/Newtonsoft.Json/Linq/JProperty.cs index 11ae7c399..20175fbd5 100644 --- a/Src/Newtonsoft.Json/Linq/JProperty.cs +++ b/Src/Newtonsoft.Json/Linq/JProperty.cs @@ -186,6 +186,7 @@ public JToken Value /// Initializes a new instance of the class from another object. /// /// A object to copy from. + /// A object to configure cloning settings. public JProperty(JProperty other, JsonCloneSettings? settings = null) : base(other, settings) { diff --git a/Src/Newtonsoft.Json/Linq/JRaw.cs b/Src/Newtonsoft.Json/Linq/JRaw.cs index c0694e793..904b89942 100644 --- a/Src/Newtonsoft.Json/Linq/JRaw.cs +++ b/Src/Newtonsoft.Json/Linq/JRaw.cs @@ -37,6 +37,7 @@ public partial class JRaw : JValue /// Initializes a new instance of the class from another object. /// /// A object to copy from. + /// A object to configure cloning settings. public JRaw(JRaw other, JsonCloneSettings? settings = null) : base(other, settings) { diff --git a/Src/Newtonsoft.Json/Linq/JToken.cs b/Src/Newtonsoft.Json/Linq/JToken.cs index f6bb7739e..611f708ef 100644 --- a/Src/Newtonsoft.Json/Linq/JToken.cs +++ b/Src/Newtonsoft.Json/Linq/JToken.cs @@ -2449,7 +2449,7 @@ public JToken DeepClone() /// /// Creates a new instance of the . All child tokens are recursively cloned. /// - /// A flag to indicate whether to clone annotations when performing deep clone on a JToken. + /// A object to configure cloning settings. /// A new instance of the . public JToken DeepClone(JsonCloneSettings settings) { diff --git a/Src/Newtonsoft.Json/Linq/JValue.cs b/Src/Newtonsoft.Json/Linq/JValue.cs index 5cb60b862..23f2e0212 100644 --- a/Src/Newtonsoft.Json/Linq/JValue.cs +++ b/Src/Newtonsoft.Json/Linq/JValue.cs @@ -61,7 +61,7 @@ internal JValue(object? value, JTokenType type) /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object that specifies the settings used when cloning JSON. + /// A object to configure cloning settings. public JValue(JValue other, JsonCloneSettings? settings = null) : this(other.Value, other.Type) { From 57bdc99312b8a7e145c583444ee4d628e4eb6e70 Mon Sep 17 00:00:00 2001 From: Sneha Shivakumar Date: Wed, 26 Oct 2022 15:58:34 -0700 Subject: [PATCH 5/9] Matching comment to other comments. --- Src/Newtonsoft.Json/Linq/JObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/Newtonsoft.Json/Linq/JObject.cs b/Src/Newtonsoft.Json/Linq/JObject.cs index f6727e1bd..e9d9018f0 100644 --- a/Src/Newtonsoft.Json/Linq/JObject.cs +++ b/Src/Newtonsoft.Json/Linq/JObject.cs @@ -92,7 +92,7 @@ public JObject() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object indicating whether to clone annotations when cloning a JToken. + /// A object to configure cloning settings. public JObject(JObject other, JsonCloneSettings? settings = null) : base(other, settings) { From 71b1785d0aa229c84c81caf22cd079a26216565b Mon Sep 17 00:00:00 2001 From: Sneha Shivakumar Date: Mon, 31 Oct 2022 11:28:00 -0700 Subject: [PATCH 6/9] Added additional unit tests, fixed small bug --- .../Linq/AnnotationsTests.cs | 42 ++++++++++++++++++- Src/Newtonsoft.Json/Linq/JConstructor.cs | 2 +- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs index e86f04810..6fa31be27 100644 --- a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs +++ b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs @@ -312,15 +312,55 @@ public void MultipleAnnotationsAreCopied() public void MultipleAnnotationsAreNotCopiedWithSetting() { Version version = new Version(1, 2, 3, 4); + JsonCloneSettings settings = new JsonCloneSettings() { CopyAnnotations = false }; JObject o = new JObject(); o.AddAnnotation("string!"); o.AddAnnotation(version); - JsonCloneSettings settings = new JsonCloneSettings() { CopyAnnotations = false }; JObject o2 = (JObject)o.DeepClone(settings); Assert.IsNull(o2.Annotation()); Assert.AreEqual(0, o2.Annotations().Count()); + + JArray a = new JArray(); + a.AddAnnotation("string!"); + a.AddAnnotation(version); + + JArray a2 = (JArray)a.DeepClone(settings); + Assert.IsNull(a2.Annotation()); + Assert.AreEqual(0, a2.Annotations().Count()); + + JProperty p = new JProperty("test"); + p.AddAnnotation("string!"); + p.AddAnnotation(version); + + JProperty p2 = (JProperty)p.DeepClone(settings); + Assert.IsNull(p2.Annotation()); + Assert.AreEqual(0, p2.Annotations().Count()); + + JRaw r = new JRaw("test"); + r.AddAnnotation("string!"); + r.AddAnnotation(version); + + JRaw r2 = (JRaw)r.DeepClone(settings); + Assert.IsNull(r2.Annotation()); + Assert.AreEqual(0, r2.Annotations().Count()); + + JConstructor c = new JConstructor("test"); + c.AddAnnotation("string!"); + c.AddAnnotation(version); + + JConstructor c2 = (JConstructor)c.DeepClone(settings); + Assert.IsNull(c2.Annotation()); + Assert.AreEqual(0, c2.Annotations().Count()); + + JValue v = new JValue("test"); + v.AddAnnotation("string!"); + v.AddAnnotation(version); + + JValue v2 = (JValue)v.DeepClone(settings); + Assert.IsNull(v2.Annotation()); + Assert.AreEqual(0, v2.Annotations().Count()); } #if !NET20 diff --git a/Src/Newtonsoft.Json/Linq/JConstructor.cs b/Src/Newtonsoft.Json/Linq/JConstructor.cs index d05f6e9e1..a989e8117 100644 --- a/Src/Newtonsoft.Json/Linq/JConstructor.cs +++ b/Src/Newtonsoft.Json/Linq/JConstructor.cs @@ -150,7 +150,7 @@ internal override bool DeepEquals(JToken node) internal override JToken CloneToken(JsonCloneSettings? settings = null) { - return new JConstructor(this); + return new JConstructor(this, settings); } /// From fd213c53f89e38ec30ac71f8780db648ee6d6123 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 9 Nov 2022 10:20:02 +0800 Subject: [PATCH 7/9] Make new ctors internal, fix nested copies --- .../Linq/AnnotationsTests.cs | 49 +++++++++++++++++++ Src/Newtonsoft.Json.Tests/Linq/JRawTests.cs | 2 +- .../MetadataPropertyHandlingTests.cs | 2 +- Src/Newtonsoft.Json/Linq/JArray.cs | 10 ++-- Src/Newtonsoft.Json/Linq/JConstructor.cs | 9 +++- Src/Newtonsoft.Json/Linq/JContainer.cs | 49 +++++++++++-------- Src/Newtonsoft.Json/Linq/JObject.cs | 14 ++++-- Src/Newtonsoft.Json/Linq/JProperty.cs | 17 ++++--- Src/Newtonsoft.Json/Linq/JRaw.cs | 10 ++-- Src/Newtonsoft.Json/Linq/JToken.cs | 8 +-- Src/Newtonsoft.Json/Linq/JTokenWriter.cs | 2 +- Src/Newtonsoft.Json/Linq/JValue.cs | 18 ++++--- Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs | 5 ++ 13 files changed, 141 insertions(+), 54 deletions(-) diff --git a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs index 6fa31be27..6165b269e 100644 --- a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs +++ b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs @@ -303,6 +303,55 @@ public void MultipleAnnotationsAreCopied() Assert.AreEqual(0, o2.Annotations().Count()); } + [Test] + public void NestedAnnotationsAreCopied() + { + Version version = new Version(1, 2, 3, 4); + + JObject o = new JObject(); + o.AddAnnotation("string!"); + o.AddAnnotation(version); + + JValue v = new JValue(true); + v.AddAnnotation("string!"); + v.AddAnnotation(version); + + o["Item1"] = v; + + JObject o2 = (JObject)o.DeepClone(); + Assert.AreEqual("string!", o2.Annotation()); + Assert.AreEqual(version, o2.Annotation()); + + JValue v2 = (JValue)o2["Item1"]; + Assert.AreEqual("string!", v2.Annotation()); + Assert.AreEqual(version, v2.Annotation()); + } + + [Test] + public void NestedAnnotationsAreNotCopiedWithSettings() + { + Version version = new Version(1, 2, 3, 4); + JsonCloneSettings settings = new JsonCloneSettings() { CopyAnnotations = false }; + + JObject o = new JObject(); + o.AddAnnotation("string!"); + o.AddAnnotation(version); + + JValue v = new JValue(true); + v.AddAnnotation("string!"); + v.AddAnnotation(version); + + o["Item1"] = v; + + JObject o2 = (JObject)o.DeepClone(settings); + Assert.IsNull(o2.Annotation()); + Assert.AreEqual(0, o2.Annotations().Count()); + + JValue v2 = (JValue)o2["Item1"]; + Assert.IsNull(v2.Annotation()); + Assert.AreEqual(0, v2.Annotations().Count()); + } + private void AssertCloneCopy(JToken t, T annotation) where T : class { Assert.AreEqual(annotation, t.DeepClone().Annotation()); diff --git a/Src/Newtonsoft.Json.Tests/Linq/JRawTests.cs b/Src/Newtonsoft.Json.Tests/Linq/JRawTests.cs index cfeb102b3..57b7c3a88 100644 --- a/Src/Newtonsoft.Json.Tests/Linq/JRawTests.cs +++ b/Src/Newtonsoft.Json.Tests/Linq/JRawTests.cs @@ -52,7 +52,7 @@ public void RawEquals() public void RawClone() { JRaw r1 = new JRaw("raw1"); - JToken r2 = r1.CloneToken(); + JToken r2 = r1.DeepClone(); CustomAssert.IsInstanceOfType(typeof(JRaw), r2); } diff --git a/Src/Newtonsoft.Json.Tests/Serialization/MetadataPropertyHandlingTests.cs b/Src/Newtonsoft.Json.Tests/Serialization/MetadataPropertyHandlingTests.cs index 2073dbcfe..e7d00dfb1 100644 --- a/Src/Newtonsoft.Json.Tests/Serialization/MetadataPropertyHandlingTests.cs +++ b/Src/Newtonsoft.Json.Tests/Serialization/MetadataPropertyHandlingTests.cs @@ -324,7 +324,7 @@ public void DeserializeFromJToken() ]"; JToken t1 = JToken.Parse(json); - JToken t2 = t1.CloneToken(); + JToken t2 = t1.DeepClone(); List employees = t1.ToObject>(JsonSerializer.Create(new JsonSerializerSettings { diff --git a/Src/Newtonsoft.Json/Linq/JArray.cs b/Src/Newtonsoft.Json/Linq/JArray.cs index d427b1be2..1e79e7b4a 100644 --- a/Src/Newtonsoft.Json/Linq/JArray.cs +++ b/Src/Newtonsoft.Json/Linq/JArray.cs @@ -65,8 +65,12 @@ public JArray() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object to configure cloning settings. - public JArray(JArray other, JsonCloneSettings? settings = null) + public JArray(JArray other) + : base(other, settings: null) + { + } + + internal JArray(JArray other, JsonCloneSettings? settings) : base(other, settings) { } @@ -310,7 +314,7 @@ public int IndexOf(JToken item) /// public void Insert(int index, JToken item) { - InsertItem(index, item, false); + InsertItem(index, item, false, copyAnnotations: true); } /// diff --git a/Src/Newtonsoft.Json/Linq/JConstructor.cs b/Src/Newtonsoft.Json/Linq/JConstructor.cs index a989e8117..703dbbc12 100644 --- a/Src/Newtonsoft.Json/Linq/JConstructor.cs +++ b/Src/Newtonsoft.Json/Linq/JConstructor.cs @@ -96,8 +96,13 @@ public JConstructor() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object to configure cloning settings. - public JConstructor(JConstructor other, JsonCloneSettings? settings = null) + public JConstructor(JConstructor other) + : base(other, settings: null) + { + _name = other.Name; + } + + internal JConstructor(JConstructor other, JsonCloneSettings? settings) : base(other, settings) { _name = other.Name; diff --git a/Src/Newtonsoft.Json/Linq/JContainer.cs b/Src/Newtonsoft.Json/Linq/JContainer.cs index 565f8b3df..e19be3619 100644 --- a/Src/Newtonsoft.Json/Linq/JContainer.cs +++ b/Src/Newtonsoft.Json/Linq/JContainer.cs @@ -106,21 +106,23 @@ internal JContainer() { } - internal JContainer(JContainer other, JsonCloneSettings? settings = null) + internal JContainer(JContainer other, JsonCloneSettings? settings) : this() { ValidationUtils.ArgumentNotNull(other, nameof(other)); - int i = 0; - foreach (JToken child in other) + bool copyAnnotations = settings?.CopyAnnotations ?? true; + + if (copyAnnotations) { - TryAddInternal(i, child, false); - i++; + CopyAnnotations(this, other); } - if (settings?.CopyAnnotations ?? true) + int i = 0; + foreach (JToken child in other) { - CopyAnnotations(this, other); + TryAddInternal(i, child, false, copyAnnotations); + i++; } } @@ -326,7 +328,7 @@ internal bool IsMultiContent([NotNullWhen(true)]object? content) return (content is IEnumerable && !(content is string) && !(content is JToken) && !(content is byte[])); } - internal JToken EnsureParentToken(JToken? item, bool skipParentCheck) + internal JToken EnsureParentToken(JToken? item, bool skipParentCheck, bool copyAnnotations) { if (item == null) { @@ -344,7 +346,12 @@ internal JToken EnsureParentToken(JToken? item, bool skipParentCheck) // the item is being added to the root parent of itself if (item.Parent != null || item == this || (item.HasValues && Root == item)) { - item = item.CloneToken(); + // Avoid allocating settings when copy annotations is false. + JsonCloneSettings? settings = copyAnnotations + ? null + : JsonCloneSettings.SkipCopyAnnotations; + + item = item.CloneToken(settings); } return item; @@ -352,7 +359,7 @@ internal JToken EnsureParentToken(JToken? item, bool skipParentCheck) internal abstract int IndexOfItem(JToken? item); - internal virtual bool InsertItem(int index, JToken? item, bool skipParentCheck) + internal virtual bool InsertItem(int index, JToken? item, bool skipParentCheck, bool copyAnnotations) { IList children = ChildrenTokens; @@ -363,7 +370,7 @@ internal virtual bool InsertItem(int index, JToken? item, bool skipParentCheck) CheckReentrancy(); - item = EnsureParentToken(item, skipParentCheck); + item = EnsureParentToken(item, skipParentCheck, copyAnnotations); JToken? previous = (index == 0) ? null : children[index - 1]; // haven't inserted new token yet so next token is still at the inserting index @@ -493,7 +500,7 @@ internal virtual void SetItem(int index, JToken? item) CheckReentrancy(); - item = EnsureParentToken(item, false); + item = EnsureParentToken(item, false, copyAnnotations: true); ValidateToken(item, existing); @@ -638,17 +645,17 @@ internal virtual void ValidateToken(JToken o, JToken? existing) /// The content to be added. public virtual void Add(object? content) { - TryAddInternal(ChildrenTokens.Count, content, false); + TryAddInternal(ChildrenTokens.Count, content, false, copyAnnotations: true); } internal bool TryAdd(object? content) { - return TryAddInternal(ChildrenTokens.Count, content, false); + return TryAddInternal(ChildrenTokens.Count, content, false, copyAnnotations: true); } internal void AddAndSkipParentCheck(JToken token) { - TryAddInternal(ChildrenTokens.Count, token, true); + TryAddInternal(ChildrenTokens.Count, token, true, copyAnnotations: true); } /// @@ -657,10 +664,10 @@ internal void AddAndSkipParentCheck(JToken token) /// The content to be added. public void AddFirst(object? content) { - TryAddInternal(0, content, false); + TryAddInternal(0, content, false, copyAnnotations: true); } - internal bool TryAddInternal(int index, object? content, bool skipParentCheck) + internal bool TryAddInternal(int index, object? content, bool skipParentCheck, bool copyAnnotations) { if (IsMultiContent(content)) { @@ -669,7 +676,7 @@ internal bool TryAddInternal(int index, object? content, bool skipParentCheck) int multiIndex = index; foreach (object c in enumerable) { - TryAddInternal(multiIndex, c, skipParentCheck); + TryAddInternal(multiIndex, c, skipParentCheck, copyAnnotations); multiIndex++; } @@ -679,7 +686,7 @@ internal bool TryAddInternal(int index, object? content, bool skipParentCheck) { JToken item = CreateFromContent(content); - return InsertItem(index, item, skipParentCheck); + return InsertItem(index, item, skipParentCheck, copyAnnotations); } } @@ -966,7 +973,7 @@ int IList.IndexOf(JToken item) void IList.Insert(int index, JToken item) { - InsertItem(index, item, false); + InsertItem(index, item, false, copyAnnotations: true); } void IList.RemoveAt(int index) @@ -1049,7 +1056,7 @@ int IList.IndexOf(object? value) void IList.Insert(int index, object? value) { - InsertItem(index, EnsureValue(value), false); + InsertItem(index, EnsureValue(value), false, copyAnnotations: false); } bool IList.IsFixedSize => false; diff --git a/Src/Newtonsoft.Json/Linq/JObject.cs b/Src/Newtonsoft.Json/Linq/JObject.cs index e9d9018f0..fab460da4 100644 --- a/Src/Newtonsoft.Json/Linq/JObject.cs +++ b/Src/Newtonsoft.Json/Linq/JObject.cs @@ -92,8 +92,12 @@ public JObject() /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object to configure cloning settings. - public JObject(JObject other, JsonCloneSettings? settings = null) + public JObject(JObject other) + : base(other, settings: null) + { + } + + internal JObject(JObject other, JsonCloneSettings? settings) : base(other, settings) { } @@ -136,7 +140,7 @@ internal override int IndexOfItem(JToken? item) return _properties.IndexOfReference(item); } - internal override bool InsertItem(int index, JToken? item, bool skipParentCheck) + internal override bool InsertItem(int index, JToken? item, bool skipParentCheck, bool copyAnnotations) { // don't add comments to JObject, no name to reference comment by if (item != null && item.Type == JTokenType.Comment) @@ -144,7 +148,7 @@ internal override bool InsertItem(int index, JToken? item, bool skipParentCheck) return false; } - return base.InsertItem(index, item, skipParentCheck); + return base.InsertItem(index, item, skipParentCheck, copyAnnotations); } internal override void ValidateToken(JToken o, JToken? existing) @@ -245,7 +249,7 @@ internal void InternalPropertyChanging(JProperty childProperty) #endif } - internal override JToken CloneToken(JsonCloneSettings? settings = null) + internal override JToken CloneToken(JsonCloneSettings? settings) { return new JObject(this, settings); } diff --git a/Src/Newtonsoft.Json/Linq/JProperty.cs b/Src/Newtonsoft.Json/Linq/JProperty.cs index 20175fbd5..b8c7ace6a 100644 --- a/Src/Newtonsoft.Json/Linq/JProperty.cs +++ b/Src/Newtonsoft.Json/Linq/JProperty.cs @@ -173,7 +173,7 @@ public JToken Value if (_content._token == null) { - InsertItem(0, newValue, false); + InsertItem(0, newValue, false, copyAnnotations: true); } else { @@ -186,8 +186,13 @@ public JToken Value /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object to configure cloning settings. - public JProperty(JProperty other, JsonCloneSettings? settings = null) + public JProperty(JProperty other) + : base(other, settings: null) + { + _name = other.Name; + } + + internal JProperty(JProperty other, JsonCloneSettings? settings) : base(other, settings) { _name = other.Name; @@ -242,7 +247,7 @@ internal override int IndexOfItem(JToken? item) return _content.IndexOf(item); } - internal override bool InsertItem(int index, JToken? item, bool skipParentCheck) + internal override bool InsertItem(int index, JToken? item, bool skipParentCheck, bool copyAnnotations) { // don't add comments to JProperty if (item != null && item.Type == JTokenType.Comment) @@ -255,7 +260,7 @@ internal override bool InsertItem(int index, JToken? item, bool skipParentCheck) throw new JsonException("{0} cannot have multiple values.".FormatWith(CultureInfo.InvariantCulture, typeof(JProperty))); } - return base.InsertItem(0, item, false); + return base.InsertItem(0, item, false, copyAnnotations); } internal override bool ContainsItem(JToken? item) @@ -283,7 +288,7 @@ internal override bool DeepEquals(JToken node) return (node is JProperty t && _name == t.Name && ContentsEqual(t)); } - internal override JToken CloneToken(JsonCloneSettings? settings = null) + internal override JToken CloneToken(JsonCloneSettings? settings) { return new JProperty(this, settings); } diff --git a/Src/Newtonsoft.Json/Linq/JRaw.cs b/Src/Newtonsoft.Json/Linq/JRaw.cs index 904b89942..02ca96d3c 100644 --- a/Src/Newtonsoft.Json/Linq/JRaw.cs +++ b/Src/Newtonsoft.Json/Linq/JRaw.cs @@ -37,8 +37,12 @@ public partial class JRaw : JValue /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object to configure cloning settings. - public JRaw(JRaw other, JsonCloneSettings? settings = null) + public JRaw(JRaw other) + : base(other, settings: null) + { + } + + internal JRaw(JRaw other, JsonCloneSettings? settings) : base(other, settings) { } @@ -68,7 +72,7 @@ public static JRaw Create(JsonReader reader) } } - internal override JToken CloneToken(JsonCloneSettings? settings = null) + internal override JToken CloneToken(JsonCloneSettings? settings) { return new JRaw(this, settings); } diff --git a/Src/Newtonsoft.Json/Linq/JToken.cs b/Src/Newtonsoft.Json/Linq/JToken.cs index 611f708ef..27f84d8ba 100644 --- a/Src/Newtonsoft.Json/Linq/JToken.cs +++ b/Src/Newtonsoft.Json/Linq/JToken.cs @@ -131,7 +131,7 @@ public JToken Root } } - internal abstract JToken CloneToken(JsonCloneSettings? settings = null); + internal abstract JToken CloneToken(JsonCloneSettings? settings); internal abstract bool DeepEquals(JToken node); /// @@ -241,7 +241,7 @@ public void AddAfterSelf(object? content) } int index = _parent.IndexOfItem(this); - _parent.TryAddInternal(index + 1, content, false); + _parent.TryAddInternal(index + 1, content, false, copyAnnotations: true); } /// @@ -256,7 +256,7 @@ public void AddBeforeSelf(object? content) } int index = _parent.IndexOfItem(this); - _parent.TryAddInternal(index, content, false); + _parent.TryAddInternal(index, content, false, copyAnnotations: true); } /// @@ -2443,7 +2443,7 @@ object ICloneable.Clone() /// A new instance of the . public JToken DeepClone() { - return CloneToken(); + return CloneToken(settings: null); } /// diff --git a/Src/Newtonsoft.Json/Linq/JTokenWriter.cs b/Src/Newtonsoft.Json/Linq/JTokenWriter.cs index 344ae8343..618063e24 100644 --- a/Src/Newtonsoft.Json/Linq/JTokenWriter.cs +++ b/Src/Newtonsoft.Json/Linq/JTokenWriter.cs @@ -502,7 +502,7 @@ internal override void WriteToken(JsonReader reader, bool writeChildren, bool wr } } - JToken value = tokenReader.CurrentToken!.CloneToken(); + JToken value = tokenReader.CurrentToken!.CloneToken(settings: null); if (_parent != null) { diff --git a/Src/Newtonsoft.Json/Linq/JValue.cs b/Src/Newtonsoft.Json/Linq/JValue.cs index 23f2e0212..c2f03991c 100644 --- a/Src/Newtonsoft.Json/Linq/JValue.cs +++ b/Src/Newtonsoft.Json/Linq/JValue.cs @@ -57,18 +57,22 @@ internal JValue(object? value, JTokenType type) _valueType = type; } + internal JValue(JValue other, JsonCloneSettings? settings) + : this(other.Value, other.Type) + { + if (settings?.CopyAnnotations ?? true) + { + CopyAnnotations(this, other); + } + } + /// /// Initializes a new instance of the class from another object. /// /// A object to copy from. - /// A object to configure cloning settings. - public JValue(JValue other, JsonCloneSettings? settings = null) + public JValue(JValue other) : this(other.Value, other.Type) { - if (settings?.CopyAnnotations ?? true) - { - CopyAnnotations(this, other); - } } /// @@ -561,7 +565,7 @@ private static bool Operation(ExpressionType operation, object? objA, object? ob } #endif - internal override JToken CloneToken(JsonCloneSettings? settings = null) + internal override JToken CloneToken(JsonCloneSettings? settings) { return new JValue(this, settings); } diff --git a/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs b/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs index cae4507ee..c9eb195dc 100644 --- a/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs +++ b/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs @@ -7,6 +7,11 @@ namespace Newtonsoft.Json.Linq /// public class JsonCloneSettings { + internal static readonly JsonCloneSettings SkipCopyAnnotations = new JsonCloneSettings + { + CopyAnnotations = false + }; + /// /// Gets or sets a flag that indicates whether to copy annotations when cloning a JToken. /// From e9a64c672e46bdbc7c4ea28e03f6534001597159 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 9 Nov 2022 10:28:32 +0800 Subject: [PATCH 8/9] Clean up --- Src/Newtonsoft.Json/Linq/JArray.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/Newtonsoft.Json/Linq/JArray.cs b/Src/Newtonsoft.Json/Linq/JArray.cs index 1e79e7b4a..c589e211b 100644 --- a/Src/Newtonsoft.Json/Linq/JArray.cs +++ b/Src/Newtonsoft.Json/Linq/JArray.cs @@ -83,7 +83,7 @@ public JArray(params object[] content) : this((object)content) { } - + /// /// Initializes a new instance of the class with the specified content. /// From cfe40563bdd97fe42f7d79c9af50249f84b65458 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Mon, 14 Nov 2022 08:40:30 +0800 Subject: [PATCH 9/9] Updates --- .../Linq/AnnotationsTests.cs | 27 ++++++++++++++++++- Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs | 15 ++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs index 6165b269e..7a8eb44ee 100644 --- a/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs +++ b/Src/Newtonsoft.Json.Tests/Linq/AnnotationsTests.cs @@ -328,7 +328,32 @@ public void NestedAnnotationsAreCopied() } [Test] - public void NestedAnnotationsAreNotCopiedWithSettings() + public void NestedAnnotationsAreCopiedWithDefault() + { + Version version = new Version(1, 2, 3, 4); + JsonCloneSettings settings = new JsonCloneSettings(); + + JObject o = new JObject(); + o.AddAnnotation("string!"); + o.AddAnnotation(version); + + JValue v = new JValue(true); + v.AddAnnotation("string!"); + v.AddAnnotation(version); + + o["Item1"] = v; + + JObject o2 = (JObject)o.DeepClone(settings); + Assert.AreEqual("string!", o2.Annotation()); + Assert.AreEqual(version, o2.Annotation()); + + JValue v2 = (JValue)o2["Item1"]; + Assert.AreEqual("string!", v2.Annotation()); + Assert.AreEqual(version, v2.Annotation()); + } + + [Test] + public void NestedAnnotationsAreNotCopiedWithSettingsCopyAnnotationsFalse() { Version version = new Version(1, 2, 3, 4); JsonCloneSettings settings = new JsonCloneSettings() { CopyAnnotations = false }; diff --git a/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs b/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs index c9eb195dc..02f1351cc 100644 --- a/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs +++ b/Src/Newtonsoft.Json/Linq/JsonCloneSettings.cs @@ -3,7 +3,7 @@ namespace Newtonsoft.Json.Linq { /// - /// Specifies the settings used when selecting JSON. + /// Specifies the settings used when cloning JSON. /// public class JsonCloneSettings { @@ -13,10 +13,19 @@ public class JsonCloneSettings }; /// - /// Gets or sets a flag that indicates whether to copy annotations when cloning a JToken. + /// Initializes a new instance of the class. + /// + public JsonCloneSettings() + { + CopyAnnotations = true; + } + + /// + /// Gets or sets a flag that indicates whether to copy annotations when cloning a . + /// The default value is true. /// /// - /// A flag that indicates whether to copy annotations when cloning a JToken. + /// A flag that indicates whether to copy annotations when cloning a . /// public bool CopyAnnotations { get; set; } }