Update JsonDeltaFormatterUnitTests.cs

Fix obscure bug with array sorting

Src/JsonDiffPatchDotNet.UnitTests/DiffUnitTests.cs

@@ -225,9 +225,9 @@ namespace JsonDiffPatchDotNet.UnitTests
 		[Test]
 		public void Diff_EfficientArrayDiffSameLengthNested_ValidDiff()
 		{
-			var jdp = new JsonDiffPatch(new Options { ArrayDiff = ArrayDiffMode.Efficient });
-			var left = JToken.Parse(@"[1,2,{""p"":false},4]");
-			var right = JToken.Parse(@"[1,2,{""p"":true},4]");
+			var jdp = new JsonDiffPatch(new Options { ArrayDiff = ArrayDiffMode.Efficient, ObjectHash = (jObj) => jObj["Id"].Value<string>() });
+			var left = JToken.Parse(@"[1,2,{""Id"" : ""F12B21EF-F57D-4958-ADDC-A3F52EC25EC8"", ""p"":false},4]");
+			var right = JToken.Parse(@"[1,2,{""Id"" : ""F12B21EF-F57D-4958-ADDC-A3F52EC25EC8"", ""p"":true},4]");
 
 			JObject diff = jdp.Diff(left, right) as JObject;
 
@@ -236,6 +236,36 @@ namespace JsonDiffPatchDotNet.UnitTests
 			Assert.IsNotNull(diff["2"]);
 		}
 
+        [Test]
+        public void Diff_EfficientArrayDiffWithComplexObject_ValidDiff()
+        {
+            var jdp = new JsonDiffPatch(new Options { ArrayDiff = ArrayDiffMode.Efficient, ObjectHash = (jObj) => jObj["Id"].Value<string>() });
+            //var jdp = new JsonDiffPatch(new Options { ArrayDiff = ArrayDiffMode.Efficient });
+            var left = JToken.Parse(@"[{""Id"" : ""F12B21EF-F57D-4958-ADDC-A3F52EC25EC8"", ""p"":false}, {""Id"" : ""F12B21EF-F57D-4958-ADDC-A3F52EC25EC9"", ""p"":true}]");
+            var right = JToken.Parse(@"[{""Id"" : ""F12B21EF-F57D-4958-ADDC-A3F52EC25EC8"", ""p"":true}, {""Id"" : ""F12B21EF-F57D-4958-ADDC-A3F52EC25EC10"", ""p"":false}]");
+
+            JObject diff = jdp.Diff(left, right) as JObject;
+
+            Assert.IsNotNull(diff);
+            Assert.AreEqual(4, diff.Properties().Count());
+        }
+
+		[Test]
+		public void Diff_EfficientArrayDiffWithComplexObjectHash_ValidDiff()
+		{
+			var jdp = new JsonDiffPatch(new Options { ArrayDiff = ArrayDiffMode.Efficient, ObjectHash = (jObj) => jObj["Id"].Value<string>() });
+			var left = JToken.Parse(@"[{""Id"": ""22ff56c7-2307-414b-8a3a-9bf1cdba2095"",""city"":""São Paulo""},{""Id"":""3fca9cdb-dd9b-4b7c-afc1-587751e25bd6"",""city"":""abc""},{""Id"":""1fe6a0f9-3974-427f-81cb-6004748cb179"",""city"":""xyz""}]");
+			var right = JToken.Parse(@"[{""Id"":""3fca9cdb-dd9b-4b7c-afc1-587751e25bd6"",""city"":""abc""},{""Id"":""1fe6a0f9-3974-427f-81cb-6004748cb179"",""city"":""xyz""}, {""Id"":""3fca9cdb-dd9b-4b7c-afc1-587751e25b44"",""city"":""new""}]");
+
+			JObject diff = jdp.Diff(left, right) as JObject;
+
+			Assert.IsNotNull(diff);
+			Assert.AreEqual(3, diff.Properties().Count());
+			Assert.IsNotNull(diff["_0"]);
+			Assert.IsNotNull(diff["2"]);
+			Assert.AreEqual(((JArray)diff["2"])[0].Value<string>("city"), "new");
+		}
+
 		[Test]
 		public void Diff_EfficientArrayDiffSameWithObject_NoDiff()
 		{

Src/JsonDiffPatchDotNet.UnitTests/JsonDeltaFormatterUnitTests.cs

@@ -1,6 +1,8 @@
+using System.Linq;
 using JsonDiffPatchDotNet.Formatters.JsonPatch;
 using Newtonsoft.Json.Linq;
 using NUnit.Framework;
+using System.Linq;
 
 namespace JsonDiffPatchDotNet.UnitTests
 {
@@ -121,6 +123,64 @@ namespace JsonDiffPatchDotNet.UnitTests
 			AssertOperation(operations[1], OperationTypes.Remove, "/1");
 		}
 
+		[Test]
+		public void Format_SortsRemoveOperations_Success()
+		{
+			const string patchJson = @"
+{
+	""a"": {
+		""a"": [0,0,0],
+		""b"": [0,0,0],
+		""c"": {
+			""a"": {
+				""a"": [0,0,0],
+				""b"": [0,0,0],
+				""c"": [0,0,0],
+				""d"": [0,0,0],
+				""e"": [0,0,0],
+				""f"": [0,0,0]
+			}
+		}
+	},
+	""b"": [0,0,0],
+	""c"": [0,0,0],
+	""d"": [0,0,0],
+	""e"": [0,0,0],
+	""f"": [0,0,0],
+	""g"": [0,0,0],
+	""h"": [0,0,0],
+	""i"": {
+		""a"": {
+			""a"": {
+				""_t"": ""a"",
+				""_0"": [0,0,0],
+				""_1"": [0,0,0]
+			}
+		}
+	}
+}
+";
+			var patch = JToken.Parse(patchJson);
+			var operations = Formatter.Format(patch);
+
+			var paths = operations.Select(o => o.Path).ToList();
+			// removal of the array item at index 1 should come before the item at index 0
+			Assert.Less(paths.IndexOf("/i/a/a/1"), paths.IndexOf("/i/a/a/0"));
+		}
+
+		public void Format_EscapeOfJsonPointer_Success()
+		{
+			var left = JObject.Parse(@"{ ""a/b"": ""a"", ""a~b"": ""ab"", ""a/~b"": ""abb"",""a/b/c~"": ""abc"" }");
+			var right = JObject.Parse(@"{ ""a/b"": ""ab"", ""a~b"": ""ba"", ""a/~b"": ""bba"",""a/b/c~"": ""cba""  }");
+			var patch = Differ.Diff(left, right);
+			var operations = Formatter.Format(patch);
+
+			Assert.IsTrue(operations.Any(x => x.Path.Equals("/a~1b") && x.Value.ToString().Equals("ab")));
+			Assert.IsTrue(operations.Any(x => x.Path.Equals("/a~0b") && x.Value.ToString().Equals("ba")));
+			Assert.IsTrue(operations.Any(x => x.Path.Equals("/a~1~0b") && x.Value.ToString().Equals("bba")));
+			Assert.IsTrue(operations.Any(x => x.Path.Equals("/a~1b~1c~0") && x.Value.ToString().Equals("cba")));
+		}
+
 		private void AssertOperation(Operation operation, string expectedOp, string expectedPath, JValue expectedValue = null)
 		{
 			Assert.AreEqual(expectedOp, operation.Op);

Src/JsonDiffPatchDotNet/ArrayDiffMode.cs

@@ -16,6 +16,6 @@
 		/// the entire left and entire right arrays are added to the patch document as a simple
 		/// JSON token replace. If they are the same, then token is skipped in the patch document.
 		/// </summary>
-		Simple,
+		Simple
 	}
 }

Src/JsonDiffPatchDotNet/DefaultItemMatch.cs

@@ -0,0 +1,17 @@
+using Newtonsoft.Json.Linq;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace JsonDiffPatchDotNet
+{
+    public class DefaultItemMatch : ItemMatch
+    {
+        public DefaultItemMatch(Func<JToken, object> objectHash):base(objectHash)
+        {
+            
+        }
+    }
+}

Src/JsonDiffPatchDotNet/Formatters/JsonPatch/JsonDeltaFormatter.cs

@@ -51,7 +51,7 @@ namespace JsonDiffPatchDotNet.Formatters.JsonPatch
 
 		protected override void NodeBegin(JsonFormatContext context, string key, string leftKey, DeltaType type, NodeType nodeType, bool isLast)
 		{
-			context.Path.Add(leftKey);
+			context.Path.Add(Escape(leftKey));
 		}
 
 		protected override void NodeEnd(JsonFormatContext context, string key, string leftKey, DeltaType type, NodeType nodeType, bool isLast)
@@ -64,6 +64,13 @@ namespace JsonDiffPatchDotNet.Formatters.JsonPatch
 
 		protected override void RootEnd(JsonFormatContext context, DeltaType type, NodeType nodeType) { }
 
+		private string Escape(string key)
+		{
+			if (string.IsNullOrEmpty(key)) return key;
+			return key.Replace("~", "~0")
+				.Replace("/", "~1");
+		}
+
 		private void FormatNode(JsonFormatContext context, JToken delta, JToken left)
 		{
 			FormatDeltaChildren(context, delta, left);
@@ -94,8 +101,8 @@ namespace JsonDiffPatchDotNet.Formatters.JsonPatch
 			var removeOpsOtherOps = PartitionRemoveOps(result);
 			var removeOps = removeOpsOtherOps[0];
 			var otherOps = removeOpsOtherOps[1];
-			Array.Sort(removeOps, new RemoveOperationComparer());
-			return removeOps.Concat(otherOps).ToList();
+			var removeOpsReverse = removeOps.OrderBy(x => x.Path, new PathComparer());
+			return removeOpsReverse.Concat(otherOps).ToList();
 		}
 
 		private IList<Operation[]> PartitionRemoveOps(IList<Operation> result)
@@ -109,15 +116,15 @@ namespace JsonDiffPatchDotNet.Formatters.JsonPatch
 			return new List<Operation[]> {left.ToArray(), right.ToArray()};
 		}
 
-		private class RemoveOperationComparer : IComparer<Operation>
+		private class PathComparer : IComparer<string>
 		{
-			public int Compare(Operation a, Operation b)
+			public int Compare(string a, string b)
 			{
 				if (a == null) throw new ArgumentNullException(nameof(a));
 				if (b == null) throw new ArgumentNullException(nameof(b));
 
-				var splitA = a.Path.Split('/');
-				var splitB = b.Path.Split('/');
+				var splitA = a.Split('/');
+				var splitB = b.Split('/');
 
 				return splitA.Length != splitB.Length
 					? splitA.Length - splitB.Length

Src/JsonDiffPatchDotNet/ItemMatch.cs

@@ -0,0 +1,49 @@
+using Newtonsoft.Json.Linq;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace JsonDiffPatchDotNet
+{
+    public abstract class ItemMatch
+    {
+        internal Func<JToken, object> ObjectHash;
+
+        protected ItemMatch()
+        {
+            
+        }
+
+        protected ItemMatch(Func<JToken, object> objectHash)
+        {
+            ObjectHash = objectHash;
+        }
+
+        public virtual bool Match(JToken object1, JToken object2)
+        {
+            return Match(object1, object2, ObjectHash);
+        }
+
+        public virtual bool Match(JToken object1, JToken object2, Func<JToken, object> objectHash)
+        {
+            if(objectHash == null || object1.Type != JTokenType.Object)
+            {
+                return JToken.DeepEquals(object1, object2);
+            }
+
+            var hash1 = objectHash.Invoke(object1);
+            if(hash1 == null)
+            {
+                return false;
+            }
+            var hash2 = objectHash.Invoke(object2);
+            if(hash2 == null)
+            {
+                return false;
+            }
+            return hash1.Equals(hash2);
+        }
+    }
+}

Src/JsonDiffPatchDotNet/JsonDiffPatch.cs

@@ -38,6 +38,11 @@ namespace JsonDiffPatchDotNet
 		/// <returns>JSON Patch Document</returns>
 		public JToken Diff(JToken left, JToken right)
 		{
+		    
+		    var objectHash = this._options.ObjectHash;
+            var itemMatch = new DefaultItemMatch(objectHash);
+		
+		
 			if (left == null)
 				left = new JValue("");
 			if (right == null)
@@ -67,7 +72,7 @@ namespace JsonDiffPatchDotNet
 					: null;
 			}
 
-			if (!JToken.DeepEquals(left, right))
+			if (!itemMatch.Match(left, right))
 			{
 				return new JArray(left, right);
 			}				
@@ -372,6 +377,8 @@ namespace JsonDiffPatchDotNet
 
 		private JObject ArrayDiff(JArray left, JArray right)
         {
+            var objectHash = this._options.ObjectHash;
+            var itemMatch = new DefaultItemMatch(objectHash);
             var result = JObject.Parse(@"{ ""_t"": ""a"" }");
 
             int commonHead = 0;
@@ -380,19 +387,34 @@ namespace JsonDiffPatchDotNet
 			if (JToken.DeepEquals(left, right))
 				return null;
 
+            var childContext = new List<JToken>();
+
             // Find common head
             while (commonHead < left.Count
                 && commonHead < right.Count
-				&& JToken.DeepEquals(left[commonHead], right[commonHead]))
+                && itemMatch.Match(left[commonHead], right[commonHead]))
             {
+                var index = commonHead;
+                var child = Diff(left[index], right[index]);
+                if(child != null)
+                {
+                    result[$"{index}"] = child;
+                }
                 commonHead++;
             }
 
             // Find common tail
             while (commonTail + commonHead < left.Count
                 && commonTail + commonHead < right.Count
-				&& JToken.DeepEquals(left[left.Count - 1 - commonTail], right[right.Count - 1 - commonTail]))
+                && itemMatch.Match(left[left.Count - 1 - commonTail], right[right.Count - 1 - commonTail]))
             {
+                var index1 = left.Count - 1 - commonTail;
+                var index2 = right.Count - 1 - commonTail;
+                var child = Diff(left[index1], right[index2]);
+                if(child != null)
+                {
+                    result[$"{index2}"] = child;
+                }
                 commonTail++;
             }
 
@@ -411,6 +433,10 @@ namespace JsonDiffPatchDotNet
                 // Trivial case, a block (1 or more consecutive items) was removed
                 for (int index = commonHead; index < left.Count - commonTail; ++index)
                 {
+                    if (result.ContainsKey(index.ToString()))
+                    {
+                        result.Remove(index.ToString());
+                    }
                     result[$"_{index}"] = new JArray(left[index], 0, (int)DiffOperation.Deleted);
                 }
 
@@ -420,13 +446,17 @@ namespace JsonDiffPatchDotNet
             // Complex Diff, find the LCS (Longest Common Subsequence)
             List<JToken> trimmedLeft = left.ToList().GetRange(commonHead, left.Count - commonTail - commonHead);
             List<JToken> trimmedRight = right.ToList().GetRange(commonHead, right.Count - commonTail - commonHead);
-			Lcs lcs = Lcs.Get(trimmedLeft, trimmedRight);
+            Lcs lcs = Lcs.Get(trimmedLeft, trimmedRight, itemMatch);
 
             for (int index = commonHead; index < left.Count - commonTail; ++index)
             {
                 if (lcs.Indices1.IndexOf(index - commonHead) < 0)
                 {
                     // Removed
+                    if (result.ContainsKey(index.ToString()))
+                    {
+                        result.Remove(index.ToString());
+                    }
                     result[$"_{index}"] = new JArray(left[index], 0, (int)DiffOperation.Deleted);
                 }
             }

Src/JsonDiffPatchDotNet/JsonDiffPatchDotNet.csproj

@@ -25,6 +25,6 @@
     <Version>2.3.0.0</Version>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="Newtonsoft.Json" Version="11.0.1" />
+    <PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
   </ItemGroup>
 </Project>

Src/JsonDiffPatchDotNet/Lcs.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using Newtonsoft.Json.Linq;
 
@@ -19,14 +19,14 @@ namespace JsonDiffPatchDotNet
             Indices2 = new List<int>();
         }
 
-		internal static Lcs Get(List<JToken> left, List<JToken> right)
+        internal static Lcs Get(List<JToken> left, List<JToken> right, ItemMatch match)
         {
-			var matrix = LcsInternal(left, right);
-			var result = Backtrack(matrix, left, right, left.Count, right.Count);
+            var matrix = LcsInternal(left, right, match);
+            var result = Backtrack(matrix, left, right, left.Count, right.Count, match);
             return result;
         }
 
-		private static int[,] LcsInternal(List<JToken> left, List<JToken> right)
+        private static int[,] LcsInternal(List<JToken> left, List<JToken> right, ItemMatch match)
         {
             var arr = new int[left.Count + 1, right.Count + 1];
 
@@ -34,7 +34,7 @@ namespace JsonDiffPatchDotNet
             {
                 for (int j = 1; j <= right.Count; j++)
                 {
-					if (JToken.DeepEquals(left[i - 1], right[j - 1]))
+                    if (match.Match(left[i - 1], right[j - 1]))
                     {
                         arr[i, j] = arr[i - 1, j - 1] + 1;
                     }
@@ -48,33 +48,36 @@ namespace JsonDiffPatchDotNet
             return arr;
         }
 
-        private static Lcs Backtrack(int[,] matrix, List<JToken> left, List<JToken> right, int li, int ri)
+        private static Lcs Backtrack(int[,] matrix, List<JToken> left, List<JToken> right, int li, int ri, ItemMatch match)
         {
+            var index1 = li;
+            var index2 = ri;
             var result = new Lcs();
-            for (int i = 1, j = 1; i <= li && j <= ri;)
-            {
-                // If the JSON tokens at the same position are both Objects or both Arrays, we just say they 
-                // are the same even if they are not, because we can package smaller deltas than an entire 
-                // object or array replacement by doing object to object or array to array diff.
-                if (JToken.DeepEquals(left[i - 1], right[j - 1])
-                || left[i - 1].Type == JTokenType.Object && right[j - 1].Type == JTokenType.Object
-                || left[i - 1].Type == JTokenType.Array && right[j - 1].Type == JTokenType.Array)
-                {
-                    result.Sequence.Add(left[i - 1]);
-                    result.Indices1.Add(i - 1);
-                    result.Indices2.Add(j - 1);
-                    i++;
-                    j++;
-                    continue;
-                }
 
-                if (matrix[i, j - 1] > matrix[i - 1, j])
+            while (index1 != 0 && index2 != 0)
             {
-                    i++;
+                var sameLetter = match.Match(left[index1 - 1], right[index2 - 1]);
+
+                if (sameLetter)
+                {
+                    result.Sequence.Add(left[index1 - 1]);
+                    result.Indices1.Add(index1 - 1);
+                    result.Indices2.Add(index2 - 1);
+                    --index1;
+                    --index2;
                 }
                 else
                 {
-                    j++;
+                    var valueAtMatrixAbove = matrix[index1, index2 - 1];
+                    var valueAtMatrixLeft = matrix[index1 - 1, index2];
+                    if (valueAtMatrixAbove > valueAtMatrixLeft)
+                    {
+                        --index2;
+                    }
+                    else
+                    {
+                        --index1;
+                    }
                 }
             }
 

Src/JsonDiffPatchDotNet/Options.cs

@@ -1,3 +1,4 @@
+using Newtonsoft.Json.Linq;
 using System;
 using System.Collections;
 using System.Collections.Generic;
@@ -38,5 +39,17 @@ namespace JsonDiffPatchDotNet
 		/// Specifies behaviors to apply to the diff patch set
 		/// </summary>
 		public DiffBehavior DiffBehaviors { get; set; }
+		
+        /// <summary>
+        /// for LCS to work, it needs a way to match items between previous/original (or left/right) arrays. In traditional text diff tools this is trivial, as two lines of text are compared char 
+        /// char.
+        /// When no matches by reference or value are found, array diffing fallbacks to a dumb behavior: matching items by position.
+        /// Matching by position is not the most efficient option (eg. if an item is added at the first position, all the items below will be considered modified), but it produces expected results
+        /// in most trivial cases.This is good enough as soon as movements/insertions/deletions only happen near the bottom of the array.
+        /// This is because if 2 objects are not equal by reference(ie.the same object) both objects are considered different values, as there is no trivial solution to compare two arbitrary objects
+        /// in JavaScript.
+        /// To improve the results leveraging the power of LCS(and position move detection) you need to provide a way to compare 2 objects.
+        /// </summary>
+        public Func<JToken, object> ObjectHash { get; set; }
     }
 }