Fix bug when adding / appending string values (Azure#53025)

m-nash · web-flow · commit eea773978a78 · 2025-10-07T10:12:38.000-07:00
diff --git a/sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPatch.Implementation.cs b/sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPatch.Implementation.cs
@@ -453,6 +453,11 @@ private void SetInternal(ReadOnlySpan<byte> jsonPath, EncodedValue encodedValue)
                     nextPath = localPath.GetFirstProperty();
                     parentPath = "$"u8;
                 }
+                if (nextPath.IsArrayIndex() && !encodedValue.Kind.HasFlag(ValueKind.ArrayItemAppend))
+                {
+                    nextPath = parentPath;
+                    kind |= ValueKind.ArrayItemAppend;
+                }
             }
             else
             {
@@ -470,16 +475,15 @@ private void SetInternal(ReadOnlySpan<byte> jsonPath, EncodedValue encodedValue)
                 {
                     nextPath = pathReader.GetParsedPath();
                     parentPath = nextPath.GetParent();
+                    if (nextPath.IsArrayIndex() && !encodedValue.Kind.HasFlag(ValueKind.ArrayItemAppend))
+                    {
+                        nextPath = parentPath;
+                        kind |= ValueKind.ArrayItemAppend;
+                    }
                 }
             }
         }
 
-        if (nextPath.IsArrayIndex() && !encodedValue.Kind.HasFlag(ValueKind.ArrayItemAppend))
-        {
-            nextPath = parentPath;
-            kind |= ValueKind.ArrayItemAppend;
-        }
-
         if (nextPath.SequenceEqual(localPath))
         {
             kind = encodedValue.Kind;
diff --git a/sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPatch.Serialization.cs b/sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPatch.Serialization.cs
@@ -179,20 +179,23 @@ public void WriteTo(Utf8JsonWriter writer)
                 }
                 else
                 {
-                    if (kvp.Key.IsArrayIndex())
-                    {
-                        newJson = newJson.InsertAt(kvp.Key, kvp.Value.Value);
-                    }
-                    else
-                    {
-                        newJson = newJson.Set(kvp.Key, kvp.Value.Value);
-                    }
+                    newJson = newJson.Set(kvp.Key, GetEncodedBytes(kvp.Value));
                 }
             }
             writer.WriteRawValue(newJson.Span);
         }
     }
 
+    private static ReadOnlyMemory<byte> GetEncodedBytes(EncodedValue value)
+    {
+        ValueKind kind = value.Kind;
+        if (kind.HasFlag(ValueKind.Utf8String) || kind.HasFlag(ValueKind.DateTime) || kind.HasFlag(ValueKind.Guid) || kind.HasFlag(ValueKind.TimeSpan))
+        {
+            return new([(byte)'"', .. value.Value.Span, (byte)'"']);
+        }
+        return value.Value;
+    }
+
     private static void WriteEncodedValueAsJson(Utf8JsonWriter writer, ReadOnlySpan<byte> propertyName, EncodedValue encodedValue)
     {
         if (encodedValue.Value.Length == 0)
diff --git a/sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPathExtensions.cs b/sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPathExtensions.cs
@@ -428,7 +428,9 @@ private static bool IsFirstProperty(ReadOnlyMemory<byte> json, ReadOnlySpan<byte
     public static byte[] Set(this ReadOnlyMemory<byte> json, ReadOnlySpan<byte> jsonPath, ReadOnlyMemory<byte> jsonReplacement)
     {
         bool found = TryFind(json.Span, jsonPath, out Utf8JsonReader jsonReader);
-        return jsonReader.SetCurrentValue(found, jsonPath.GetPropertyName(), json, jsonReplacement);
+        return !found && jsonPath.IsArrayIndex()
+            ? json.InsertAt(jsonPath, jsonReplacement)
+            : jsonReader.SetCurrentValue(found, jsonPath.GetPropertyName(), json, jsonReplacement);
     }
 
     /// <summary>
diff --git a/sdk/core/System.ClientModel/tests/ModelReaderWriterTests/JsonPatchTests.cs b/sdk/core/System.ClientModel/tests/ModelReaderWriterTests/JsonPatchTests.cs
@@ -461,5 +461,21 @@ public void NullException_Format()
             Assert.AreEqual("Value cannot be null. (Parameter 'format')", ex!.Message);
 #endif
         }
+
+        [Test]
+        public void AddStringPropertyToComplexObject()
+        {
+            JsonPatch jp = new("{\"a\":{\"b\":{\"bProp\":1},\"c\":{\"cProp\":true}}}"u8.ToArray());
+            jp.Set("$.a.new"u8, "newValue");
+            Assert.AreEqual("{\"a\":{\"b\":{\"bProp\":1},\"c\":{\"cProp\":true},\"new\":\"newValue\"}}", jp.ToString("J"));
+        }
+
+        [Test]
+        public void AddStringArrayElement()
+        {
+            JsonPatch jp = new("{\"a\":[\"one\",\"two\",\"three\"]}"u8.ToArray());
+            jp.Set("$.a[1]"u8, "newTwo");
+            Assert.AreEqual("{\"a\":[\"one\",\"newTwo\",\"three\"]}", jp.ToString("J"));
+        }
     }
 }
diff --git a/sdk/core/System.ClientModel/tests/internal/ModelReaderWriter/JsonPathExtensionsTests.cs b/sdk/core/System.ClientModel/tests/internal/ModelReaderWriter/JsonPathExtensionsTests.cs
@@ -260,6 +260,24 @@ public void GetFirstProperty(string jsonPathStr, string expected)
             Assert.AreEqual(expected, Encoding.UTF8.GetString(first.ToArray()));
         }
 
+        [TestCase("{\"a\":1,\"b\":2}", "$.b", "100", "{\"a\":1,\"b\":100}")]
+        [TestCase("{\"a\":{\"b\":{\"bProp\":1},\"c\":{\"cProp\":true}}}", "$.a.new", "\"stringValue\"", "{\"a\":{\"b\":{\"bProp\":1},\"c\":{\"cProp\":true},\"new\":\"stringValue\"}}")]
+        [TestCase("{\"a\":{\"b\":{\"bProp\":1},\"c\":{\"cProp\":true}}}", "$.a.new", "100", "{\"a\":{\"b\":{\"bProp\":1},\"c\":{\"cProp\":true},\"new\":100}}")]
+        [TestCase("{\"a\":{\"b\":[\"1\",\"2\",\"3\"]}}", "$.a.b[0]", "\"new\"", "{\"a\":{\"b\":[\"new\",\"2\",\"3\"]}}")]
+        [TestCase("{\"a\":{\"b\":[\"1\",\"2\",\"3\"]}}", "$.a.b[1]", "\"new\"", "{\"a\":{\"b\":[\"1\",\"new\",\"3\"]}}")]
+        [TestCase("{\"a\":{\"b\":[\"1\",\"2\",\"3\"]}}", "$.a.b[2]", "\"new\"", "{\"a\":{\"b\":[\"1\",\"2\",\"new\"]}}")]
+        [TestCase("{\"a\":{\"b\":[1,2,3]}}", "$.a.b[0]", "10", "{\"a\":{\"b\":[10,2,3]}}")]
+        [TestCase("{\"a\":{\"b\":[1,2,3]}}", "$.a.b[1]", "10", "{\"a\":{\"b\":[1,10,3]}}")]
+        [TestCase("{\"a\":{\"b\":[1,2,3]}}", "$.a.b[2]", "10", "{\"a\":{\"b\":[1,2,10]}}")]
+        public void Set(string jsonStr, string jsonPathStr, string newValue, string expected)
+        {
+            ReadOnlyMemory<byte> json = Encoding.UTF8.GetBytes(jsonStr);
+            ReadOnlySpan<byte> jsonPath = Encoding.UTF8.GetBytes(jsonPathStr);
+            ReadOnlyMemory<byte> value = Encoding.UTF8.GetBytes(newValue);
+            var result = json.Set(jsonPath, value);
+            Assert.AreEqual(expected, Encoding.UTF8.GetString(result));
+        }
+
         [TestCase("{\"a\":1,\"b\":2}", "$.c", "3", false, "{\"a\":1,\"b\":2,\"c\":3}")]
         [TestCase("{\"a\":1}", "$.a", "2", true, "{\"a\":2}")]
         [TestCase("{\"arr\":[1,2]}", "$.arr[1]", "10", true, "{\"arr\":[1,10]}")]

Original file line number	Diff line number	Diff line change
`@@ -453,6 +453,11 @@ private void SetInternal(ReadOnlySpan<byte> jsonPath, EncodedValue encodedValue)`
`453`	`453`	`nextPath = localPath.GetFirstProperty();`
`454`	`454`	`parentPath = "$"u8;`
`455`	`455`	`}`
	`456`	`+ if (nextPath.IsArrayIndex() && !encodedValue.Kind.HasFlag(ValueKind.ArrayItemAppend))`
	`457`	`+ {`
	`458`	`+ nextPath = parentPath;`
	`459`	`+ kind \|= ValueKind.ArrayItemAppend;`
	`460`	`+ }`
`456`	`461`	`}`
`457`	`462`	`else`
`458`	`463`	`{`
`@@ -470,16 +475,15 @@ private void SetInternal(ReadOnlySpan<byte> jsonPath, EncodedValue encodedValue)`
`470`	`475`	`{`
`471`	`476`	`nextPath = pathReader.GetParsedPath();`
`472`	`477`	`parentPath = nextPath.GetParent();`
	`478`	`+ if (nextPath.IsArrayIndex() && !encodedValue.Kind.HasFlag(ValueKind.ArrayItemAppend))`
	`479`	`+ {`
	`480`	`+ nextPath = parentPath;`
	`481`	`+ kind \|= ValueKind.ArrayItemAppend;`
	`482`	`+ }`
`473`	`483`	`}`
`474`	`484`	`}`
`475`	`485`	`}`
`476`	`486`
`477`		`- if (nextPath.IsArrayIndex() && !encodedValue.Kind.HasFlag(ValueKind.ArrayItemAppend))`
`478`		`- {`
`479`		`- nextPath = parentPath;`
`480`		`- kind \|= ValueKind.ArrayItemAppend;`
`481`		`- }`
`482`		`-`
`483`	`487`	`if (nextPath.SequenceEqual(localPath))`
`484`	`488`	`{`
`485`	`489`	`kind = encodedValue.Kind;`
Original file line number	Diff line number	Diff line change
`@@ -179,20 +179,23 @@ public void WriteTo(Utf8JsonWriter writer)`
`179`	`179`	`}`
`180`	`180`	`else`
`181`	`181`	`{`
`182`		`- if (kvp.Key.IsArrayIndex())`
`183`		`- {`
`184`		`- newJson = newJson.InsertAt(kvp.Key, kvp.Value.Value);`
`185`		`- }`
`186`		`- else`
`187`		`- {`
`188`		`- newJson = newJson.Set(kvp.Key, kvp.Value.Value);`
`189`		`- }`
	`182`	`+ newJson = newJson.Set(kvp.Key, GetEncodedBytes(kvp.Value));`
`190`	`183`	`}`
`191`	`184`	`}`
`192`	`185`	`writer.WriteRawValue(newJson.Span);`
`193`	`186`	`}`
`194`	`187`	`}`
`195`	`188`
	`189`	`+ private static ReadOnlyMemory<byte> GetEncodedBytes(EncodedValue value)`
	`190`	`+ {`
	`191`	`+ ValueKind kind = value.Kind;`
	`192`	`+ if (kind.HasFlag(ValueKind.Utf8String) \|\| kind.HasFlag(ValueKind.DateTime) \|\| kind.HasFlag(ValueKind.Guid) \|\| kind.HasFlag(ValueKind.TimeSpan))`
	`193`	`+ {`
	`194`	`+ return new([(byte)'"', .. value.Value.Span, (byte)'"']);`
	`195`	`+ }`
	`196`	`+ return value.Value;`
	`197`	`+ }`
	`198`	`+`
`196`	`199`	`private static void WriteEncodedValueAsJson(Utf8JsonWriter writer, ReadOnlySpan<byte> propertyName, EncodedValue encodedValue)`
`197`	`200`	`{`
`198`	`201`	`if (encodedValue.Value.Length == 0)`
Original file line number	Diff line number	Diff line change
`@@ -428,7 +428,9 @@ private static bool IsFirstProperty(ReadOnlyMemory<byte> json, ReadOnlySpan<byte`
`428`	`428`	`public static byte[] Set(this ReadOnlyMemory<byte> json, ReadOnlySpan<byte> jsonPath, ReadOnlyMemory<byte> jsonReplacement)`
`429`	`429`	`{`
`430`	`430`	`bool found = TryFind(json.Span, jsonPath, out Utf8JsonReader jsonReader);`
`431`		`- return jsonReader.SetCurrentValue(found, jsonPath.GetPropertyName(), json, jsonReplacement);`
	`431`	`+ return !found && jsonPath.IsArrayIndex()`
	`432`	`+ ? json.InsertAt(jsonPath, jsonReplacement)`
	`433`	`+ : jsonReader.SetCurrentValue(found, jsonPath.GetPropertyName(), json, jsonReplacement);`
`432`	`434`	`}`
`433`	`435`
`434`	`436`	`/// <summary>`