Skip to content

Commit 62eba21

Browse files
satabinsatabin
committed
Avoid deep recursive object equalities
1 parent a0400eb commit 62eba21

File tree

3 files changed

+30
-57
lines changed

3 files changed

+30
-57
lines changed

‎core/src/main/scala/diffson/jsonpatch/JsonDiff.scala

Lines changed: 15 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony
3333

3434
private def diff(json1: Json, json2: Json, pointer: Pointer): Eval[Chain[Operation[Json]]] =
3535
(json1, json2) match {
36-
case (JsObject(fields1), JsObject(fields2)) => fieldsDiff(fields1.toList, fields2.toList, pointer)
36+
case (JsObject(fields1), JsObject(fields2)) => fieldsDiff(fields1.toList, fields2, pointer)
3737
case (JsArray(arr1), JsArray(arr2)) if diffArray => arraysDiff(arr1.toList, arr2.toList, pointer)
3838
case _ if json1 === json2 =>
3939
// if they are equal, this one is easy...
@@ -42,51 +42,22 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony
4242
}
4343

4444
private def fieldsDiff(fields1: List[(String, Json)],
45-
fields2: List[(String, Json)],
46-
path: Pointer): Eval[Chain[Operation[Json]]] = {
47-
// sort fields by name in both objects
48-
val sorted1 = fields1.sortBy(_._1)
49-
val sorted2 = fields2.sortBy(_._1)
50-
@tailrec
51-
def associate(fields1: List[(String, Json)],
52-
fields2: List[(String, Json)],
53-
acc: Chain[(Option[(String, Json)], Option[(String, Json)])])
54-
: Chain[(Option[(String, Json)], Option[(String, Json)])] = (fields1, fields2) match {
55-
case (f1 :: t1, f2 :: t2) if f1._1 == f2._1 =>
56-
// same name, associate both
57-
associate(t1, t2, acc.prepend((Some(f1), Some(f2))))
58-
case (f1 :: t1, f2 :: _) if f1._1 < f2._1 =>
59-
// the first field is not present in the second object
60-
associate(t1, fields2, acc.prepend((Some(f1), None)))
61-
case (_ :: _, f2 :: t2) =>
62-
// the second field is not present in the first object
63-
associate(fields1, t2, acc.prepend((None, Some(f2))))
64-
case (_, Nil) =>
65-
Chain.fromSeq(fields1.map(Some(_) -> None)) ++ acc
66-
case (Nil, _) =>
67-
Chain.fromSeq(fields2.map(None -> Some(_))) ++ acc
45+
fields2: Map[String, Json],
46+
path: Pointer): Eval[Chain[Operation[Json]]] =
47+
fields1 match {
48+
case (fld, value1) :: fields1 =>
49+
fields2.get(fld) match {
50+
case Some(value2) =>
51+
fieldsDiff(fields1, fields2.removed(fld), path).flatMap(d => diff(value1, value2, path / fld).map(_ ++ d))
52+
case None =>
53+
// field is not in the second object, delete it
54+
fieldsDiff(fields1, fields2, path).map(
55+
_.prepend(Remove(path / fld, if (rememberOld) Some(value1) else None)))
56+
}
57+
case Nil =>
58+
Eval.now(Chain.fromSeq(fields2.toList).map { case (fld, value) => Add(path / fld, value) })
6859
}
6960

70-
def fields(fs: List[(Option[(String, Json)], Option[(String, Json)])],
71-
acc: Chain[Operation[Json]]): Eval[Chain[Operation[Json]]] = fs match {
72-
case (Some(f1), Some(f2)) :: tl if f1._2 === f2._2 =>
73-
// all right, nothing changed
74-
fields(tl, acc)
75-
case (Some(f1), Some(f2)) :: tl =>
76-
// same field name, different values
77-
diff(f1._2, f2._2, path / f1._1).flatMap(d => fields(tl, d ++ acc))
78-
case (Some(f1), None) :: tl =>
79-
// the field was deleted
80-
fields(tl, acc.prepend(Remove[Json](path / f1._1, if (rememberOld) Some(f1._2) else None)))
81-
case (None, Some(f2)) :: tl =>
82-
// the field was added
83-
fields(tl, acc.prepend(Add(path / f2._1, f2._2)))
84-
case _ =>
85-
Eval.now(acc)
86-
}
87-
fields(associate(sorted1, sorted2, Chain.empty).toList, Chain.empty)
88-
}
89-
9061
private def arraysDiff(arr1: List[Json], arr2: List[Json], path: Pointer): Eval[Chain[Operation[Json]]] = {
9162
// get the longest common subsequence in the array
9263
val lcs = Lcs.lcs(arr1, arr2)

‎testkit/shared/src/main/scala/diffson/TestJsonDiff.scala

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ abstract class TestJsonDiff[Json](implicit Json: Jsony[Json])
5151
val json3 = parseJson("""{"lbl": 32, "new1": false, "new2": null}""")
5252
val json4 = parseJson("""{"a": 3, "b": {"a": true }}""")
5353
val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""")
54-
diff(json1, json2) should be(JsonPatch[Json](Add(Pointer("new"), false: Json)))
55-
diff(json1, json3) should be(JsonPatch[Json](Add(Pointer("new2"), Json.Null), Add(Pointer("new1"), false: Json)))
56-
diff(json4, json5) should be(JsonPatch[Json](Add(Pointer("b", "b"), 43: Json), Add(Pointer("c"), Json.Null)))
54+
diff(json1, json2).ops should contain theSameElementsAs List(Add(Pointer("new"), false: Json))
55+
diff(json1, json3).ops should contain theSameElementsAs List(Add(Pointer("new2"), Json.Null),
56+
Add(Pointer("new1"), false: Json))
57+
diff(json4, json5).ops should contain theSameElementsAs List(Add(Pointer("b", "b"), 43: Json),
58+
Add(Pointer("c"), Json.Null))
5759
}
5860

5961
it should "contain a remove operation for each removed field" in {
@@ -62,9 +64,9 @@ abstract class TestJsonDiff[Json](implicit Json: Jsony[Json])
6264
val json3 = parseJson("""{"lbl": 32, "old1": false, "old2": null}""")
6365
val json4 = parseJson("""{"a": 3, "b": {"a": true }}""")
6466
val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""")
65-
diff(json2, json1) should be(JsonPatch[Json](Remove(Pointer("old"))))
66-
diff(json3, json1) should be(JsonPatch[Json](Remove(Pointer("old2")), Remove(Pointer("old1"))))
67-
diff(json5, json4) should be(JsonPatch[Json](Remove(Pointer("b", "b")), Remove(Pointer("c"))))
67+
diff(json2, json1).ops should contain theSameElementsAs List(Remove(Pointer("old")))
68+
diff(json3, json1).ops should contain theSameElementsAs List(Remove(Pointer("old2")), Remove(Pointer("old1")))
69+
diff(json5, json4).ops should contain theSameElementsAs List(Remove(Pointer("b", "b")), Remove(Pointer("c")))
6870
}
6971

7072
it should "correctly handle array diffs in objects" in {

‎testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ abstract class TestSimpleDiff[Json](implicit val Json: Jsony[Json])
4040
diff(parseJson("true"), parseJson("13")) should be(JsonPatch[Json](Replace(Pointer.Root, 13: Json)))
4141
}
4242

43-
it should "contain an add operation for each added field" in {
43+
it should "conta be be bein an add operation for each added field" in {
4444
val json1 = parseJson("""{"lbl": 32}""")
4545
val json2 = parseJson("""{"lbl": 32, "new": false}""")
4646
val json3 = parseJson("""{"lbl": 32, "new1": false, "new2": null}""")
4747
val json4 = parseJson("""{"a": 3, "b": {"a": true }}""")
4848
val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""")
49-
diff(json1, json2) should be(JsonPatch[Json](Add(Pointer("new"), false: Json)))
50-
diff(json1, json3) should be(JsonPatch[Json](Add(Pointer("new2"), Json.Null), Add(Pointer("new1"), false: Json)))
51-
diff(json4, json5) should be(JsonPatch[Json](Add(Pointer("b", "b"), 43: Json), Add(Pointer("c"), Json.Null)))
49+
diff(json1, json2).ops should contain theSameElementsAs List(Add(Pointer("new"), false: Json))
50+
diff(json1, json3).ops should contain theSameElementsAs List(Add(Pointer("new2"), Json.Null), Add(Pointer("new1"), false: Json))
51+
diff(json4, json5).ops should contain theSameElementsAs List(Add(Pointer("b", "b"), 43: Json), Add(Pointer("c"), Json.Null))
5252
}
5353

5454
it should "contain a remove operation for each removed field" in {
@@ -57,9 +57,9 @@ abstract class TestSimpleDiff[Json](implicit val Json: Jsony[Json])
5757
val json3 = parseJson("""{"lbl": 32, "old1": false, "old2": null}""")
5858
val json4 = parseJson("""{"a": 3, "b": {"a": true }}""")
5959
val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""")
60-
diff(json2, json1) should be(JsonPatch[Json](Remove(Pointer("old"))))
61-
diff(json3, json1) should be(JsonPatch[Json](Remove(Pointer("old2")), Remove(Pointer("old1"))))
62-
diff(json5, json4) should be(JsonPatch[Json](Remove(Pointer("b", "b")), Remove(Pointer("c"))))
60+
diff(json2, json1).ops should contain theSameElementsAs List(Remove(Pointer("old")))
61+
diff(json3, json1).ops should contain theSameElementsAs List(Remove(Pointer("old2")), Remove(Pointer("old1")))
62+
diff(json5, json4).ops should contain theSameElementsAs List(Remove(Pointer("b", "b")), Remove(Pointer("c")))
6363
}
6464

6565
it should "correctly handle array diffs in objects (i.e. just replaced)" in {

0 commit comments

Comments
 (0)