From ac648fefc8806be8642f40a5618632996e4fc55b Mon Sep 17 00:00:00 2001 From: Martijn Kamphuis Date: Fri, 15 Mar 2019 12:08:33 +0100 Subject: [PATCH] Improved Id equality check Id equality in elements used to only be able to match on an 'id' property. The functionality has been changed to configure the property to compare equality. Also the equality check used to only allow comparison of simple types. This has been improved to compare complex 'id' objects. --- JsonDiffPatch/CompareOptions.cs | 18 ++++ JsonDiffPatch/JsonDiffer.cs | 101 ++++++++++-------- JsonDiffPatchTests/DiffTests2.cs | 67 +++++++++++- JsonDiffPatchTests/JsonDiffPatch.Tests.csproj | 76 +++++++++++-- .../Samples/scene1a_complex_id.json | 19 ++++ .../Samples/scene1a_otherid.json | 16 +++ .../Samples/scene1b_complex_id.json | 16 +++ .../Samples/scene1b_otherid.json | 13 +++ .../Samples/scene2a_complex_id.json | 13 +++ .../Samples/scene2a_otherid.json | 13 +++ .../Samples/scene2b_complex_id.json | 22 ++++ .../Samples/scene2b_otherid.json | 22 ++++ .../Samples/scene3a_complex_id.json | 6 ++ .../Samples/scene3a_otherid.json | 6 ++ .../Samples/scene3b_complex_id.json | 6 ++ .../Samples/scene3b_otherid.json | 6 ++ 16 files changed, 366 insertions(+), 54 deletions(-) create mode 100644 JsonDiffPatch/CompareOptions.cs create mode 100644 JsonDiffPatchTests/Samples/scene1a_complex_id.json create mode 100644 JsonDiffPatchTests/Samples/scene1a_otherid.json create mode 100644 JsonDiffPatchTests/Samples/scene1b_complex_id.json create mode 100644 JsonDiffPatchTests/Samples/scene1b_otherid.json create mode 100644 JsonDiffPatchTests/Samples/scene2a_complex_id.json create mode 100644 JsonDiffPatchTests/Samples/scene2a_otherid.json create mode 100644 JsonDiffPatchTests/Samples/scene2b_complex_id.json create mode 100644 JsonDiffPatchTests/Samples/scene2b_otherid.json create mode 100644 JsonDiffPatchTests/Samples/scene3a_complex_id.json create mode 100644 JsonDiffPatchTests/Samples/scene3a_otherid.json create mode 100644 JsonDiffPatchTests/Samples/scene3b_complex_id.json create mode 100644 JsonDiffPatchTests/Samples/scene3b_otherid.json diff --git a/JsonDiffPatch/CompareOptions.cs b/JsonDiffPatch/CompareOptions.cs new file mode 100644 index 0000000..4a80e24 --- /dev/null +++ b/JsonDiffPatch/CompareOptions.cs @@ -0,0 +1,18 @@ +using System; + +namespace JsonDiffPatch +{ + public class CompareOptions + { + public static readonly CompareOptions Default = new CompareOptions(); + + public bool EnableIdentifierCheck { get; } + public string IdentifierProperty { get; } + + public CompareOptions(bool enableIdentifierCheck = false, string identifierProperty = "id") + { + EnableIdentifierCheck = enableIdentifierCheck; + IdentifierProperty = identifierProperty; + } + } +} diff --git a/JsonDiffPatch/JsonDiffer.cs b/JsonDiffPatch/JsonDiffer.cs index 8a118ad..d87deaf 100644 --- a/JsonDiffPatch/JsonDiffer.cs +++ b/JsonDiffPatch/JsonDiffer.cs @@ -45,8 +45,7 @@ internal static Operation Replace(string path, string key, JToken value) return Build("replace", path, key, value); } - internal static IEnumerable CalculatePatch(JToken left, JToken right, bool useIdToDetermineEquality, - string path = "") + internal static IEnumerable CalculatePatch(JToken left, JToken right, CompareOptions compareOptions, string path = "") { if (left.Type != right.Type) { @@ -57,7 +56,7 @@ internal static IEnumerable CalculatePatch(JToken left, JToken right, if (left.Type == JTokenType.Array) { Operation prev = null; - foreach (var operation in ProcessArray(left, right, path, useIdToDetermineEquality)) + foreach (var operation in ProcessArray(left, right, path, compareOptions)) { var prevRemove = prev as RemoveOperation; var add = operation as AddOperation; @@ -98,7 +97,7 @@ internal static IEnumerable CalculatePatch(JToken left, JToken right, foreach (var match in zipped) { string newPath = path + "/" + match.key; - foreach (var patch in CalculatePatch(match.left, match.right, useIdToDetermineEquality, newPath)) + foreach (var patch in CalculatePatch(match.left, match.right, compareOptions, newPath)) yield return patch; } yield break; @@ -114,13 +113,9 @@ internal static IEnumerable CalculatePatch(JToken left, JToken right, } } - private static IEnumerable ProcessArray(JToken left, JToken right, string path, - bool useIdPropertyToDetermineEquality) + private static IEnumerable ProcessArray(JToken left, JToken right, string path, CompareOptions compareOptions) { - var comparer = new CustomCheckEqualityComparer(useIdPropertyToDetermineEquality, new JTokenEqualityComparer()); - - - + var comparer = new CustomCheckEqualityComparer(compareOptions, new JTokenEqualityComparer()); int commonHead = 0; int commonTail = 0; @@ -134,7 +129,7 @@ private static IEnumerable ProcessArray(JToken left, JToken right, st if (comparer.Equals(array1[commonHead], array2[commonHead]) == false) break; //diff and yield objects here - foreach (var operation in CalculatePatch(array1[commonHead], array2[commonHead], useIdPropertyToDetermineEquality, path + "/" + commonHead)) + foreach (var operation in CalculatePatch(array1[commonHead], array2[commonHead], compareOptions, path + "/" + commonHead)) { yield return operation; } @@ -149,7 +144,7 @@ private static IEnumerable ProcessArray(JToken left, JToken right, st var index1 = len1 - 1 - commonTail; var index2 = len2 - 1 - commonTail; - foreach (var operation in CalculatePatch(array1[index1], array2[index2], useIdPropertyToDetermineEquality, path + "/" + index1)) + foreach (var operation in CalculatePatch(array1[index1], array2[index2], compareOptions, path + "/" + index1)) { yield return operation; } @@ -175,7 +170,7 @@ private static IEnumerable ProcessArray(JToken left, JToken right, st { for (int i = 0; i < leftMiddle.Length; i++) { - foreach (var operation in CalculatePatch(leftMiddle[i], rightMiddle[i], useIdPropertyToDetermineEquality, path + "/" + commonHead)) + foreach (var operation in CalculatePatch(leftMiddle[i], rightMiddle[i], compareOptions, path + "/" + commonHead)) { yield return operation; } @@ -332,67 +327,89 @@ public int GetHashCode(KeyValuePair obj) /// /// /// - /// + /// + /// + /// + public PatchDocument Diff(JToken @from, JToken to) + { + return new PatchDocument(CalculatePatch(@from, to, CompareOptions.Default).ToArray()); + } + + /// + /// + /// + /// + /// + /// Use id comparison on array members to determine equality + /// + public PatchDocument Diff(JToken @from, JToken to, CompareOptions compareOptions) + { + if (compareOptions == null) + throw new ArgumentNullException(nameof(compareOptions)); + + return new PatchDocument(CalculatePatch(@from, to, compareOptions).ToArray()); + } + + /// + /// + /// + /// /// - /// Use id propety on array members to determine equality + /// Use id property on array members to determine equality /// + [Obsolete("Use Diff(@from, to, compareOptions) instead. Kept for backwards compatibility only.")] public PatchDocument Diff(JToken @from, JToken to, bool useIdPropertyToDetermineEquality) { - return new PatchDocument(CalculatePatch(@from, to, useIdPropertyToDetermineEquality).ToArray()); + CompareOptions compareOptions = new CompareOptions(useIdPropertyToDetermineEquality); + + return new PatchDocument(CalculatePatch(@from, to, compareOptions).ToArray()); } } internal class CustomCheckEqualityComparer : IEqualityComparer { - private readonly bool _enableIdCheck; + private readonly CompareOptions _compareOptions; private readonly IEqualityComparer _inner; - public CustomCheckEqualityComparer(bool enableIdCheck, IEqualityComparer inner) + public CustomCheckEqualityComparer(CompareOptions compareOptions, IEqualityComparer inner) { - _enableIdCheck = enableIdCheck; + _compareOptions = compareOptions; _inner = inner; } public bool Equals(JToken x, JToken y) { - if (_enableIdCheck && x.Type == JTokenType.Object && y.Type == JTokenType.Object) + if (_compareOptions.EnableIdentifierCheck && x.Type == JTokenType.Object && y.Type == JTokenType.Object) { - var xIdToken = x["id"]; - var yIdToken = y["id"]; + var xIdToken = x[_compareOptions.IdentifierProperty]; + var yIdToken = y[_compareOptions.IdentifierProperty]; - var xId = xIdToken != null ? xIdToken.Value() : null; - var yId = yIdToken != null ? yIdToken.Value() : null; - if (xId != null && xId == yId) + if (xIdToken != null && yIdToken != null) { - return true; + if (xIdToken.HasValues && y.HasValues) + return _inner.Equals(xIdToken, yIdToken); + + if (xIdToken.HasValues != yIdToken.HasValues) + return false; } + + var xId = xIdToken?.Value(); + var yId = yIdToken?.Value(); + + return (xId != null && xId == yId); } return _inner.Equals(x, y); } public int GetHashCode(JToken obj) { - if (_enableIdCheck && obj.Type == JTokenType.Object) + if (_compareOptions.EnableIdentifierCheck && obj.Type == JTokenType.Object) { - var xIdToken = obj["id"]; + var xIdToken = obj[_compareOptions.IdentifierProperty]; var xId = xIdToken != null && xIdToken.HasValues ? xIdToken.Value() : null; if (xId != null) return xId.GetHashCode() + _inner.GetHashCode(obj); } return _inner.GetHashCode(obj); } - - public static bool HaveEqualIds(JToken x, JToken y) - { - if (x.Type == JTokenType.Object && y.Type == JTokenType.Object) - { - var xIdToken = x["id"]; - var yIdToken = y["id"]; - - var xId = xIdToken != null ? xIdToken.Value() : null; - var yId = yIdToken != null ? yIdToken.Value() : null; - return xId != null && xId == yId; - } - return false; - } } } \ No newline at end of file diff --git a/JsonDiffPatchTests/DiffTests2.cs b/JsonDiffPatchTests/DiffTests2.cs index 8291074..72c030d 100644 --- a/JsonDiffPatchTests/DiffTests2.cs +++ b/JsonDiffPatchTests/DiffTests2.cs @@ -1,4 +1,6 @@ -using System.IO; +using System; +using System.IO; +using System.Reflection; using JsonDiffPatch; using Newtonsoft.Json.Linq; using NUnit.Framework; @@ -10,25 +12,80 @@ public class DiffTests2 { [Test] public void ComplexExampleWithDeepArrayChange() { - - var leftPath = @".\samples\scene{0}a.json"; - var rightPath = @".\samples\scene{0}b.json"; + var currentDir = Path.GetDirectoryName(Assembly.GetCallingAssembly().CodeBase); + var leftPath = Path.Combine(currentDir, @"Samples\scene{0}a.json").Replace("file://", "").Replace("file:\\", ""); + var rightPath = Path.Combine(currentDir, @"Samples\scene{0}b.json").Replace("file://", "").Replace("file:\\", ""); var i = 1; + var filesFound = false; while(File.Exists(string.Format(leftPath, i))) { + filesFound = true; + var scene1Text = File.ReadAllText(string.Format(leftPath, i)); var scene1 = JToken.Parse(scene1Text); var scene2Text = File.ReadAllText(string.Format(rightPath, i)); var scene2 = JToken.Parse(scene2Text); - var patchDoc = new JsonDiffer().Diff(scene1, scene2, true); + var patchDoc = new JsonDiffer().Diff(scene1, scene2, new CompareOptions(true)); //Assert.AreEqual("[{\"op\":\"remove\",\"path\":\"/items/0/entities/1\"}]", var patcher = new JsonPatcher(); patcher.Patch(ref scene1, patchDoc); Assert.True(JToken.DeepEquals(scene1, scene2)); i++; } + + Assert.IsTrue(filesFound); + } + + [Test] + public void ComplexExampleWithDeepArrayChangeOtherIdProperty() + { + var currentDir = Path.GetDirectoryName(Assembly.GetCallingAssembly().CodeBase); + var leftPath = Path.Combine(currentDir, @"Samples\scene{0}a_otherid.json").Replace("file://", "").Replace("file:\\", ""); + var rightPath = Path.Combine(currentDir, @"Samples\scene{0}b_otherid.json").Replace("file://", "").Replace("file:\\", ""); + var i = 1; + var filesFound = false; + while (File.Exists(Path.Combine(currentDir, string.Format(leftPath, i)))) + { + filesFound = true; + + var scene1Text = File.ReadAllText(Path.Combine(currentDir, string.Format(leftPath, i))); + var scene1 = JToken.Parse(scene1Text); + var scene2Text = File.ReadAllText(Path.Combine(currentDir, string.Format(rightPath, i))); + var scene2 = JToken.Parse(scene2Text); + var patchDoc = new JsonDiffer().Diff(scene1, scene2, new CompareOptions(true, "otherId")); + var patcher = new JsonPatcher(); + patcher.Patch(ref scene1, patchDoc); + Assert.True(JToken.DeepEquals(scene1, scene2)); + i++; + } + + Assert.IsTrue(filesFound); } + [Test] + public void ComplexExampleWithDeepArrayChangeComplexIdProperty() + { + var currentDir = Path.GetDirectoryName(Assembly.GetCallingAssembly().CodeBase); + var leftPath = Path.Combine(currentDir, @"Samples\scene{0}a_complex_id.json").Replace("file://", "").Replace("file:\\", ""); + var rightPath = Path.Combine(currentDir, @"Samples\scene{0}b_complex_id.json").Replace("file://", "").Replace("file:\\", ""); + var i = 1; + var filesFound = false; + while (File.Exists(string.Format(leftPath, i))) + { + filesFound = true; + + var scene1Text = File.ReadAllText(string.Format(leftPath, i)); + var scene1 = JToken.Parse(scene1Text); + var scene2Text = File.ReadAllText(string.Format(rightPath, i)); + var scene2 = JToken.Parse(scene2Text); + var patchDoc = new JsonDiffer().Diff(scene1, scene2, new CompareOptions(true)); + var patcher = new JsonPatcher(); + patcher.Patch(ref scene1, patchDoc); + Assert.True(JToken.DeepEquals(scene1, scene2)); + i++; + } + Assert.IsTrue(filesFound); + } } } \ No newline at end of file diff --git a/JsonDiffPatchTests/JsonDiffPatch.Tests.csproj b/JsonDiffPatchTests/JsonDiffPatch.Tests.csproj index 2483eda..4e50e57 100644 --- a/JsonDiffPatchTests/JsonDiffPatch.Tests.csproj +++ b/JsonDiffPatchTests/JsonDiffPatch.Tests.csproj @@ -7,21 +7,83 @@ + + + + + + + + + + + + - - - - - - - + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + + + Always + diff --git a/JsonDiffPatchTests/Samples/scene1a_complex_id.json b/JsonDiffPatchTests/Samples/scene1a_complex_id.json new file mode 100644 index 0000000..797ee01 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene1a_complex_id.json @@ -0,0 +1,19 @@ +{ + "items": [ + { + "id": { + "external": "viewpoint", + "internal": "1234" + }, + "entities": [ + { + "name": "Donn Crimmins (sdf)" + }, + { + "name": "Chester Westberg" + } + ], + "type": "Viewpoint" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene1a_otherid.json b/JsonDiffPatchTests/Samples/scene1a_otherid.json new file mode 100644 index 0000000..42bfb48 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene1a_otherid.json @@ -0,0 +1,16 @@ +{ + "items": [ + { + "otherId": "viewpoint", + "entities": [ + { + "name": "Donn Crimmins (sdf)" + }, + { + "name": "Chester Westberg" + } + ], + "type": "Viewpoint" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene1b_complex_id.json b/JsonDiffPatchTests/Samples/scene1b_complex_id.json new file mode 100644 index 0000000..563f8a4 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene1b_complex_id.json @@ -0,0 +1,16 @@ +{ + "items": [ + { + "id": { + "internal": "1234", + "external": "viewpoint" + }, + "entities": [ + { + "name": "Donn Crimmins (sdf)" + } + ], + "type": "Viewpoint" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene1b_otherid.json b/JsonDiffPatchTests/Samples/scene1b_otherid.json new file mode 100644 index 0000000..a238cf9 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene1b_otherid.json @@ -0,0 +1,13 @@ +{ + "items": [ + { + "otherId": "viewpoint", + "entities": [ + { + "name": "Donn Crimmins (sdf)" + } + ], + "type": "Viewpoint" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene2a_complex_id.json b/JsonDiffPatchTests/Samples/scene2a_complex_id.json new file mode 100644 index 0000000..c30d34c --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene2a_complex_id.json @@ -0,0 +1,13 @@ +{ + "incidents": [ + { + "when": "2015-09-08T12:15:25+01:00", + "id": { "internal": "7f468da7-3788-40c0-90fd-b06bc61c86dd" }, + "where": "Fey St", + "what": [ + "Franklyn Sanfilippo (aaaa) says \"1\"" + ], + "type": "Incident" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene2a_otherid.json b/JsonDiffPatchTests/Samples/scene2a_otherid.json new file mode 100644 index 0000000..c78bc1b --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene2a_otherid.json @@ -0,0 +1,13 @@ +{ + "incidents": [ + { + "when": "2015-09-08T12:15:25+01:00", + "otherId": "7f468da7-3788-40c0-90fd-b06bc61c86dd", + "where": "Fey St", + "what": [ + "Franklyn Sanfilippo (aaaa) says \"1\"" + ], + "type": "Incident" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene2b_complex_id.json b/JsonDiffPatchTests/Samples/scene2b_complex_id.json new file mode 100644 index 0000000..173c763 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene2b_complex_id.json @@ -0,0 +1,22 @@ +{ + "incidents": [ + { + "when": "2015-09-08T12:15:46+01:00", + "id": "dab4a570-2dde-458e-92b1-f71e42605f90", + "where": "Fey St", + "what": [ + "Franklyn Sanfilippo (aaaa) says \"2\"" + ], + "type": "Incident" + }, + { + "when": "2015-09-08T12:15:25+01:00", + "id": { "internal": "7f468da7-3788-40c0-90fd-b06bc61c86dd" }, + "where": "Fey St", + "what": [ + "Franklyn Sanfilippo (aaaa) says \"1\"" + ], + "type": "Incident" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene2b_otherid.json b/JsonDiffPatchTests/Samples/scene2b_otherid.json new file mode 100644 index 0000000..1a52a27 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene2b_otherid.json @@ -0,0 +1,22 @@ +{ + "incidents": [ + { + "when": "2015-09-08T12:15:46+01:00", + "otherId": "dab4a570-2dde-458e-92b1-f71e42605f90", + "where": "Fey St", + "what": [ + "Franklyn Sanfilippo (aaaa) says \"2\"" + ], + "type": "Incident" + }, + { + "when": "2015-09-08T12:15:25+01:00", + "otherId": "7f468da7-3788-40c0-90fd-b06bc61c86dd", + "where": "Fey St", + "what": [ + "Franklyn Sanfilippo (aaaa) says \"1\"" + ], + "type": "Incident" + } + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene3a_complex_id.json b/JsonDiffPatchTests/Samples/scene3a_complex_id.json new file mode 100644 index 0000000..031cabc --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene3a_complex_id.json @@ -0,0 +1,6 @@ +{ + "description": [ + "cash: 400", + "product: 100" + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene3a_otherid.json b/JsonDiffPatchTests/Samples/scene3a_otherid.json new file mode 100644 index 0000000..031cabc --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene3a_otherid.json @@ -0,0 +1,6 @@ +{ + "description": [ + "cash: 400", + "product: 100" + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene3b_complex_id.json b/JsonDiffPatchTests/Samples/scene3b_complex_id.json new file mode 100644 index 0000000..713d9b2 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene3b_complex_id.json @@ -0,0 +1,6 @@ +{ + "description": [ + "cash: 700", + "product: 000" + ] +} \ No newline at end of file diff --git a/JsonDiffPatchTests/Samples/scene3b_otherid.json b/JsonDiffPatchTests/Samples/scene3b_otherid.json new file mode 100644 index 0000000..713d9b2 --- /dev/null +++ b/JsonDiffPatchTests/Samples/scene3b_otherid.json @@ -0,0 +1,6 @@ +{ + "description": [ + "cash: 700", + "product: 000" + ] +} \ No newline at end of file