From 0b76599c31b3fa367dc172272e81cee87a634ba9 Mon Sep 17 00:00:00 2001
From: David <davidfallah1@gmail.com>
Date: Mon, 2 Oct 2017 09:54:55 +0100
Subject: [PATCH] Improve conformance of replace operations to spec

This ensures that JSON patch "replace" operations are functionally
equivalent to "remove" operations followed by "add" operations at the
same path, as RFC 6902 specifies.

Addresses #110
---
 .../Internal/DynamicObjectAdapter.cs          |   5 +
 .../Internal/DynamicObjectAdapterTest.cs      |   4 +-
 .../WriteOnceDynamicTestObject.cs             | 117 ++++++++++++++++++
 3 files changed, 124 insertions(+), 2 deletions(-)
 create mode 100644 test/Microsoft.AspNetCore.JsonPatch.Test/WriteOnceDynamicTestObject.cs

diff --git a/src/Microsoft.AspNetCore.JsonPatch/Internal/DynamicObjectAdapter.cs b/src/Microsoft.AspNetCore.JsonPatch/Internal/DynamicObjectAdapter.cs
index dc3c48266f..5b3d7d8bdd 100644
--- a/src/Microsoft.AspNetCore.JsonPatch/Internal/DynamicObjectAdapter.cs
+++ b/src/Microsoft.AspNetCore.JsonPatch/Internal/DynamicObjectAdapter.cs
@@ -96,6 +96,11 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal
                 return false;
             }
 
+            if (!TryRemove(target, segment, contractResolver, out errorMessage))
+            {
+                return false;
+            }
+
             if (!TrySetDynamicObjectProperty(target, contractResolver, segment, convertedValue, out errorMessage))
             {
                 return false;
diff --git a/test/Microsoft.AspNetCore.JsonPatch.Test/Internal/DynamicObjectAdapterTest.cs b/test/Microsoft.AspNetCore.JsonPatch.Test/Internal/DynamicObjectAdapterTest.cs
index 96b1aee935..9d5eb2595c 100644
--- a/test/Microsoft.AspNetCore.JsonPatch.Test/Internal/DynamicObjectAdapterTest.cs
+++ b/test/Microsoft.AspNetCore.JsonPatch.Test/Internal/DynamicObjectAdapterTest.cs
@@ -130,11 +130,11 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal
         }
 
         [Fact]
-        public void TryReplace_ReplacesPropertyValue()
+        public void TryReplace_RemovesExistingValue_BeforeAddingNewValue()
         {
             // Arrange
             var adapter = new DynamicObjectAdapter();
-            dynamic target = new DynamicTestObject();
+            dynamic target = new WriteOnceDynamicTestObject();
             target.NewProperty = new object();
             var segment = "NewProperty";
             var resolver = new DefaultContractResolver();
diff --git a/test/Microsoft.AspNetCore.JsonPatch.Test/WriteOnceDynamicTestObject.cs b/test/Microsoft.AspNetCore.JsonPatch.Test/WriteOnceDynamicTestObject.cs
new file mode 100644
index 0000000000..769ddcc154
--- /dev/null
+++ b/test/Microsoft.AspNetCore.JsonPatch.Test/WriteOnceDynamicTestObject.cs
@@ -0,0 +1,117 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Dynamic;
+
+namespace Microsoft.AspNetCore.JsonPatch.Internal
+{
+    /// <remarks>
+    /// <para>
+    /// This class is used specifically to test that JSON patch "replace" operations are functionally equivalent to
+    /// "add" and "remove" operations applied sequentially using the same path.
+    /// </para>
+    /// <para>
+    /// This is done by asserting that  no value exists for a particular key before setting its value. To replace the
+    /// value for a key, the key must first be removed, and then re-added with the new value.
+    /// </para>
+    /// <para>
+    /// See JsonPatch#110 for further details.
+    /// </para>
+    /// </remarks>
+    public class WriteOnceDynamicTestObject : DynamicObject
+    {
+        private Dictionary<string, object> _dictionary = new Dictionary<string, object>();
+
+        public object this[string key] { get => ((IDictionary<string, object>)_dictionary)[key]; set => SetValueForKey(key, value); }
+
+        public ICollection<string> Keys => ((IDictionary<string, object>)_dictionary).Keys;
+
+        public ICollection<object> Values => ((IDictionary<string, object>)_dictionary).Values;
+
+        public int Count => ((IDictionary<string, object>)_dictionary).Count;
+
+        public bool IsReadOnly => ((IDictionary<string, object>)_dictionary).IsReadOnly;
+
+        public void Add(string key, object value)
+        {
+            SetValueForKey(key, value);
+        }
+
+        public void Add(KeyValuePair<string, object> item)
+        {
+            SetValueForKey(item.Key, item.Value);
+        }
+
+        public void Clear()
+        {
+            ((IDictionary<string, object>)_dictionary).Clear();
+        }
+
+        public bool Contains(KeyValuePair<string, object> item)
+        {
+            return ((IDictionary<string, object>)_dictionary).Contains(item);
+        }
+
+        public bool ContainsKey(string key)
+        {
+            return ((IDictionary<string, object>)_dictionary).ContainsKey(key);
+        }
+
+        public void CopyTo(KeyValuePair<string, object>[] array, int arrayIndex)
+        {
+            ((IDictionary<string, object>)_dictionary).CopyTo(array, arrayIndex);
+        }
+
+        public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
+        {
+            return ((IDictionary<string, object>)_dictionary).GetEnumerator();
+        }
+
+        public bool Remove(string key)
+        {
+            return ((IDictionary<string, object>)_dictionary).Remove(key);
+        }
+
+        public bool Remove(KeyValuePair<string, object> item)
+        {
+            return ((IDictionary<string, object>)_dictionary).Remove(item);
+        }
+
+        public bool TryGetValue(string key, out object value)
+        {
+            return ((IDictionary<string, object>)_dictionary).TryGetValue(key, out value);
+        }
+
+        public override bool TryGetMember(GetMemberBinder binder, out object result)
+        {
+            var name = binder.Name;
+
+            return TryGetValue(name, out result);
+        }
+
+        public override bool TrySetMember(SetMemberBinder binder, object value)
+        {
+            SetValueForKey(binder.Name, value);
+
+            return true;
+        }
+
+        private void SetValueForKey(string key, object value)
+        {
+            if (value == null)
+            {
+                _dictionary.Remove(key);
+                return;
+            }
+
+            if (_dictionary.ContainsKey(key))
+            {
+                throw new ArgumentException($"Value for {key} already exists");
+            }
+
+            _dictionary[key] = value;
+        }
+    }
+}