From 8d26e68c666e4ca66b0f0d676b296265af4dc012 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 17:08:48 +0100 Subject: [PATCH 1/9] Benchmark LCS diff implementation --- .gitignore | 16 ++++- .../scala/diffson/PatienceBenchmarks.scala | 58 +++++++++++++++++++ build.sbt | 13 +++++ project/plugins.sbt | 1 + 4 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 benchmarks/src/main/scala/diffson/PatienceBenchmarks.scala diff --git a/.gitignore b/.gitignore index f9ff931..11aa8d0 100644 --- a/.gitignore +++ b/.gitignore @@ -20,11 +20,9 @@ syntax: glob .idea *.iml -#bsp -.bsp - # building target +out build null tmp* @@ -51,3 +49,15 @@ build.log #ensime .ensime* ensime.sbt + +# website +site/content/api +site/content/documentation/ +site/output + +.metals/ +.bloop/ +.bsp/ +metals.sbt + +.vscode/settings.json diff --git a/benchmarks/src/main/scala/diffson/PatienceBenchmarks.scala b/benchmarks/src/main/scala/diffson/PatienceBenchmarks.scala new file mode 100644 index 0000000..8c7be60 --- /dev/null +++ b/benchmarks/src/main/scala/diffson/PatienceBenchmarks.scala @@ -0,0 +1,58 @@ +package diffson + +import org.openjdk.jmh.annotations.State +import org.openjdk.jmh.annotations.Scope +import org.openjdk.jmh.annotations.BenchmarkMode +import org.openjdk.jmh.annotations.Mode +import org.openjdk.jmh.annotations.Fork +import org.openjdk.jmh.annotations.Warmup +import org.openjdk.jmh.annotations.Measurement + +import diffson.circe._ +import diffson.jsonpatch.lcsdiff._ +import diffson.lcs._ + +import io.circe.syntax._ +import io.circe.Json +import org.openjdk.jmh.annotations.Benchmark + +@BenchmarkMode(Array(Mode.Throughput)) +@State(Scope.Benchmark) +@Fork(value = 1) +@Warmup(iterations = 3, time = 2) +@Measurement(iterations = 5, time = 2) +class PatienceBenchmarks { + + implicit val lcs = new Patience[Json] + + private def createJson(depth: Int, arrayStep: Int) = + List + .range(depth, 0, -1) + .foldLeft(Json.obj("array" := List.range(0, 1000, arrayStep).map(n => Json.obj("n" := n, "other" := "common")))) { + (acc, idx) => + Json.obj(s"key$idx" := acc, "other" := arrayStep) + } + + def array(size: Int, step: Int) = + Json.obj("array" := List.range(0, size, step)) + + val deep1 = + createJson(100, 1) + + val deep2 = + createJson(100, 2) + + val array1 = + array(1000, 2) + + val array2 = + array(1000, 1) + + @Benchmark + def diffArray() = + diff(array1, array2) + + @Benchmark + def diffDeep() = + diff(deep1, deep2) +} diff --git a/build.sbt b/build.sbt index 97c5c91..f363f73 100644 --- a/build.sbt +++ b/build.sbt @@ -90,3 +90,16 @@ lazy val circe = crossProject(JSPlatform, JVMPlatform, NativePlatform) ) ) .dependsOn(core, testkit % Test) + +lazy val benchmarks = crossProject(JVMPlatform) + .crossType(CrossType.Pure) + .in(file("benchmarks")) + .enablePlugins(NoPublishPlugin, JmhPlugin) + .settings(commonSettings: _*) + .settings( + name := "diffson-benchmarks", + libraryDependencies ++= Seq( + "io.circe" %% "circe-literal" % circeVersion + ) + ) + .dependsOn(circe) diff --git a/project/plugins.sbt b/project/plugins.sbt index 5666df0..42dcf55 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -2,3 +2,4 @@ addSbtPlugin("org.typelevel" % "sbt-typelevel" % "0.4.18") addSbtPlugin("org.scala-js" % "sbt-scalajs" % "1.13.0") addSbtPlugin("org.scala-native" % "sbt-scala-native" % "0.4.10") addSbtPlugin("org.portable-scala" % "sbt-scala-native-crossproject" % "1.2.0") +addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.4") From 5437d1d56af52914115bc18e70400e9b9b6b8fa3 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 17:11:11 +0100 Subject: [PATCH 2/9] Avoid list traversal to get indices --- core/src/main/scala/diffson/lcs/Patience.scala | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/diffson/lcs/Patience.scala b/core/src/main/scala/diffson/lcs/Patience.scala index a33252a..20b22a8 100644 --- a/core/src/main/scala/diffson/lcs/Patience.scala +++ b/core/src/main/scala/diffson/lcs/Patience.scala @@ -20,7 +20,6 @@ import cats.Eq import cats.implicits._ import scala.annotation.tailrec -import scala.collection.SortedMap import scala.collection.immutable.TreeMap import scala.collection.compat._ @@ -43,17 +42,17 @@ class Patience[T: Eq](withFallback: Boolean = true) extends Lcs[T] { /** Returns occurrences that appear only once in the list, associated with their index */ private def uniques(l: List[T]): Map[T, Int] = { @tailrec - def loop(l: List[Occurrence], acc: Map[T, Int]): Map[T, Int] = l match { - case (value, idx) :: tl => + def loop(l: List[T], idx: Int, acc: Map[T, Int]): Map[T, Int] = l match { + case value :: tl => if (acc.contains(value)) // not unique, remove it from the accumulator and go further - loop(tl, acc - value) + loop(tl, idx + 1, acc - value) else - loop(tl, acc + (value -> idx)) + loop(tl, idx + 1, acc.updated(value, idx)) case Nil => acc } - loop(l.zipWithIndex, Map.empty) + loop(l, 0, Map.empty) } /** Takes all occurences from the first sequence and order them as in the second sequence if it is present */ From 731e6926c06b817aee3726cca0ff5f9c4cf74a89 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 17:12:53 +0100 Subject: [PATCH 3/9] Make sure to use the hash caching LCS implementations --- core/src/main/scala/diffson/jsonpatch/package.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/diffson/jsonpatch/package.scala b/core/src/main/scala/diffson/jsonpatch/package.scala index 05df62e..3ce2af7 100644 --- a/core/src/main/scala/diffson/jsonpatch/package.scala +++ b/core/src/main/scala/diffson/jsonpatch/package.scala @@ -23,10 +23,10 @@ package object jsonpatch { object lcsdiff { object remembering { implicit def JsonDiffDiff[Json: Jsony: Lcs]: Diff[Json, JsonPatch[Json]] = - new JsonDiff[Json](true, true) + new JsonDiff[Json](true, true)(implicitly, implicitly[Lcs[Json]].savedHashes) } implicit def JsonDiffDiff[Json: Jsony: Lcs]: Diff[Json, JsonPatch[Json]] = - new JsonDiff[Json](true, false) + new JsonDiff[Json](true, false)(implicitly, implicitly[Lcs[Json]].savedHashes) } object simplediff { From 9ae2e0e60b88e45a39df42da4a1ec75c8cc8d2f6 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 17:13:36 +0100 Subject: [PATCH 4/9] Use chain for more efficient concatenation --- .../scala/diffson/jsonpatch/JsonDiff.scala | 67 ++++++++++--------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala index a98c4e7..a89c19b 100644 --- a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala +++ b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala @@ -23,70 +23,72 @@ import jsonpointer._ import cats.implicits._ import scala.annotation.tailrec +import cats.data.Chain class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony[Json], Lcs: Lcs[Json]) extends Diff[Json, JsonPatch[Json]] { def diff(json1: Json, json2: Json): JsonPatch[Json] = - JsonPatch(diff(json1, json2, Pointer.Root)) + JsonPatch(diff(json1, json2, Pointer.Root).toList) - private def diff(json1: Json, json2: Json, pointer: Pointer): List[Operation[Json]] = + private def diff(json1: Json, json2: Json, pointer: Pointer): Chain[Operation[Json]] = if (json1 === json2) // if they are equal, this one is easy... - Nil + Chain.empty else (json1, json2) match { case (JsObject(fields1), JsObject(fields2)) => fieldsDiff(fields1.toList, fields2.toList, pointer) case (JsArray(arr1), JsArray(arr2)) if diffArray => arraysDiff(arr1.toList, arr2.toList, pointer) - case (_, _) => List(Replace(pointer, json2, if (rememberOld) Some(json1) else None)) + case (_, _) => Chain.one(Replace(pointer, json2, if (rememberOld) Some(json1) else None)) } private def fieldsDiff(fields1: List[(String, Json)], fields2: List[(String, Json)], - path: Pointer): List[Operation[Json]] = { + path: Pointer): Chain[Operation[Json]] = { // sort fields by name in both objects val sorted1 = fields1.sortBy(_._1) val sorted2 = fields2.sortBy(_._1) @tailrec def associate(fields1: List[(String, Json)], fields2: List[(String, Json)], - acc: List[(Option[(String, Json)], Option[(String, Json)])]) - : List[(Option[(String, Json)], Option[(String, Json)])] = (fields1, fields2) match { + acc: Chain[(Option[(String, Json)], Option[(String, Json)])]) + : Chain[(Option[(String, Json)], Option[(String, Json)])] = (fields1, fields2) match { case (f1 :: t1, f2 :: t2) if f1._1 == f2._1 => // same name, associate both - associate(t1, t2, (Some(f1), Some(f2)) :: acc) + associate(t1, t2, acc.prepend((Some(f1), Some(f2)))) case (f1 :: t1, f2 :: _) if f1._1 < f2._1 => // the first field is not present in the second object - associate(t1, fields2, (Some(f1), None) :: acc) + associate(t1, fields2, acc.prepend((Some(f1), None))) case (_ :: _, f2 :: t2) => // the second field is not present in the first object - associate(fields1, t2, (None, Some(f2)) :: acc) + associate(fields1, t2, acc.prepend((None, Some(f2)))) case (_, Nil) => - fields1.map(Some(_) -> None) ::: acc + Chain.fromSeq(fields1.map(Some(_) -> None)) ++ acc case (Nil, _) => - fields2.map(None -> Some(_)) ::: acc + Chain.fromSeq(fields2.map(None -> Some(_))) ++ acc } + @tailrec def fields(fs: List[(Option[(String, Json)], Option[(String, Json)])], - acc: List[Operation[Json]]): List[Operation[Json]] = fs match { + acc: Chain[Operation[Json]]): Chain[Operation[Json]] = fs match { case (Some(f1), Some(f2)) :: tl if f1 == f2 => // all right, nothing changed fields(tl, acc) case (Some(f1), Some(f2)) :: tl => // same field name, different values - fields(tl, diff(f1._2, f2._2, path / f1._1) ::: acc) + fields(tl, diff(f1._2, f2._2, path / f1._1) ++ acc) case (Some(f1), None) :: tl => // the field was deleted - fields(tl, Remove[Json](path / f1._1, if (rememberOld) Some(f1._2) else None) :: acc) + fields(tl, acc.prepend(Remove[Json](path / f1._1, if (rememberOld) Some(f1._2) else None))) case (None, Some(f2)) :: tl => // the field was added - fields(tl, Add(path / f2._1, f2._2) :: acc) + fields(tl, acc.prepend(Add(path / f2._1, f2._2))) case _ => acc } - fields(associate(sorted1, sorted2, Nil), Nil) + fields(associate(sorted1, sorted2, Chain.empty).toList, Chain.empty) } - private def arraysDiff(arr1: List[Json], arr2: List[Json], path: Pointer): List[Operation[Json]] = { + private def arraysDiff(arr1: List[Json], arr2: List[Json], path: Pointer): Chain[Operation[Json]] = { // get the longest common subsequence in the array val lcs = Lcs.lcs(arr1, arr2) @@ -104,15 +106,16 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony // add a bunch of values to an array starting at the specified index @tailrec - def add(arr: List[Json], idx: Int, acc: List[Operation[Json]]): List[Operation[Json]] = arr match { - case v :: tl => add(tl, idx + 1, Add(path / idx, v) :: acc) - case Nil => acc.reverse + def add(arr: List[Json], idx: Int, acc: Chain[Operation[Json]]): Chain[Operation[Json]] = arr match { + case v :: tl => add(tl, idx + 1, acc.append(Add(path / idx, v))) + case Nil => acc } // remove a bunch of array elements starting by the last one in the range - def remove(from: Int, until: Int, shift: Int, arr: List[Json]): List[Operation[Json]] = - (for (idx <- until to from by -1) - yield Remove[Json](path / idx, if (rememberOld) Some(arr(idx - shift)) else None)).toList + def remove(from: Int, until: Int, shift: Int, arr: List[Json]): Chain[Operation[Json]] = + Chain.fromSeq( + for (idx <- until to from by -1) + yield Remove[Json](path / idx, if (rememberOld) Some(arr(idx - shift)) else None)) // now iterate over the first array to computes what was added, what was removed and what was modified @tailrec @@ -123,8 +126,8 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony shift1: Int, // current index shift in the first array (due to elements being add or removed) idx2: Int, // current index in the second array lcs: List[(Int, Int)], // the list of remaining matching indices - acc: List[Operation[Json]] // the already accumulated result - ): List[Operation[Json]] = (arr1, arr2) match { + acc: Chain[Operation[Json]] // the already accumulated result + ): Chain[Operation[Json]] = (arr1, arr2) match { case (_ :: tl1, _) if isCommon1(idx1, lcs) => // all values in arr2 were added until the index of common value val until = lcs.head._2 @@ -134,7 +137,7 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony shift1 + until - idx2, until + 1, lcs.tail, - add(arr2.take(until - idx2), idx1 + shift1, Nil) reverse_::: acc) + acc ++ add(arr2.take(until - idx2), idx1 + shift1, Chain.empty)) case (_, _ :: tl2) if isCommon2(idx2, lcs) => // all values in arr1 were removed until the index of common value val until = lcs.head._1 @@ -144,18 +147,18 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony shift1 - (until - idx1), idx2 + 1, lcs.tail, - remove(idx1 + shift1, until - 1 + shift1, idx1 + shift1, arr1) reverse_::: acc) + acc ++ remove(idx1 + shift1, until - 1 + shift1, idx1 + shift1, arr1)) case (v1 :: tl1, v2 :: tl2) => // values are different, recursively compute the diff of these values - loop(tl1, tl2, idx1 + 1, shift1, idx2 + 1, lcs, diff(v1, v2, path / (idx1 + shift1)) reverse_::: acc) + loop(tl1, tl2, idx1 + 1, shift1, idx2 + 1, lcs, acc ++ diff(v1, v2, path / (idx1 + shift1))) case (_, Nil) => // all subsequent values in arr1 were removed - remove(idx1 + shift1, idx1 + arr1.size - 1 + shift1, idx1 + shift1, arr1) reverse_::: acc + acc ++ remove(idx1 + shift1, idx1 + arr1.size - 1 + shift1, idx1 + shift1, arr1) case (Nil, _) => // all subsequent value in arr2 were added - arr2.map(Add(path / "-", _)) reverse_::: acc + acc ++ Chain.fromSeq(arr2.map(Add(path / "-", _))) } - loop(arr1, arr2, 0, 0, 0, lcs, Nil).reverse + loop(arr1, arr2, 0, 0, 0, lcs, Chain.empty) } } From a0400eb330a94b0383e4e83fbf42d6874d9d6b63 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 18:02:22 +0100 Subject: [PATCH 5/9] Avoid systematic potentially heavy equality check at first --- .scalafmt.conf | 2 +- .../scala/diffson/jsonpatch/JsonDiff.scala | 48 +++++++++---------- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/.scalafmt.conf b/.scalafmt.conf index b302c12..a58a8d0 100644 --- a/.scalafmt.conf +++ b/.scalafmt.conf @@ -1,4 +1,4 @@ -version = "3.7.2" +version = "3.7.1" maxColumn = 120 danglingParentheses.preset = false align.preset = some diff --git a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala index a89c19b..b96c9b5 100644 --- a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala +++ b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala @@ -20,30 +20,30 @@ package jsonpatch import lcs._ import jsonpointer._ -import cats.implicits._ +import cats.syntax.all._ +import cats.data.Chain +import cats.Eval import scala.annotation.tailrec -import cats.data.Chain class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony[Json], Lcs: Lcs[Json]) extends Diff[Json, JsonPatch[Json]] { def diff(json1: Json, json2: Json): JsonPatch[Json] = - JsonPatch(diff(json1, json2, Pointer.Root).toList) + JsonPatch(diff(json1, json2, Pointer.Root).value.toList) - private def diff(json1: Json, json2: Json, pointer: Pointer): Chain[Operation[Json]] = - if (json1 === json2) - // if they are equal, this one is easy... - Chain.empty - else - (json1, json2) match { - case (JsObject(fields1), JsObject(fields2)) => fieldsDiff(fields1.toList, fields2.toList, pointer) - case (JsArray(arr1), JsArray(arr2)) if diffArray => arraysDiff(arr1.toList, arr2.toList, pointer) - case (_, _) => Chain.one(Replace(pointer, json2, if (rememberOld) Some(json1) else None)) - } + private def diff(json1: Json, json2: Json, pointer: Pointer): Eval[Chain[Operation[Json]]] = + (json1, json2) match { + case (JsObject(fields1), JsObject(fields2)) => fieldsDiff(fields1.toList, fields2.toList, pointer) + case (JsArray(arr1), JsArray(arr2)) if diffArray => arraysDiff(arr1.toList, arr2.toList, pointer) + case _ if json1 === json2 => + // if they are equal, this one is easy... + Eval.now(Chain.empty) + case _ => Eval.now(Chain.one(Replace(pointer, json2, if (rememberOld) Some(json1) else None))) + } private def fieldsDiff(fields1: List[(String, Json)], fields2: List[(String, Json)], - path: Pointer): Chain[Operation[Json]] = { + path: Pointer): Eval[Chain[Operation[Json]]] = { // sort fields by name in both objects val sorted1 = fields1.sortBy(_._1) val sorted2 = fields2.sortBy(_._1) @@ -67,15 +67,14 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony Chain.fromSeq(fields2.map(None -> Some(_))) ++ acc } - @tailrec def fields(fs: List[(Option[(String, Json)], Option[(String, Json)])], - acc: Chain[Operation[Json]]): Chain[Operation[Json]] = fs match { - case (Some(f1), Some(f2)) :: tl if f1 == f2 => + acc: Chain[Operation[Json]]): Eval[Chain[Operation[Json]]] = fs match { + case (Some(f1), Some(f2)) :: tl if f1._2 === f2._2 => // all right, nothing changed fields(tl, acc) case (Some(f1), Some(f2)) :: tl => // same field name, different values - fields(tl, diff(f1._2, f2._2, path / f1._1) ++ acc) + diff(f1._2, f2._2, path / f1._1).flatMap(d => fields(tl, d ++ acc)) case (Some(f1), None) :: tl => // the field was deleted fields(tl, acc.prepend(Remove[Json](path / f1._1, if (rememberOld) Some(f1._2) else None))) @@ -83,12 +82,12 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony // the field was added fields(tl, acc.prepend(Add(path / f2._1, f2._2))) case _ => - acc + Eval.now(acc) } fields(associate(sorted1, sorted2, Chain.empty).toList, Chain.empty) } - private def arraysDiff(arr1: List[Json], arr2: List[Json], path: Pointer): Chain[Operation[Json]] = { + private def arraysDiff(arr1: List[Json], arr2: List[Json], path: Pointer): Eval[Chain[Operation[Json]]] = { // get the longest common subsequence in the array val lcs = Lcs.lcs(arr1, arr2) @@ -118,7 +117,6 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony yield Remove[Json](path / idx, if (rememberOld) Some(arr(idx - shift)) else None)) // now iterate over the first array to computes what was added, what was removed and what was modified - @tailrec def loop( arr1: List[Json], // the first array arr2: List[Json], // the second array @@ -127,7 +125,7 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony idx2: Int, // current index in the second array lcs: List[(Int, Int)], // the list of remaining matching indices acc: Chain[Operation[Json]] // the already accumulated result - ): Chain[Operation[Json]] = (arr1, arr2) match { + ): Eval[Chain[Operation[Json]]] = (arr1, arr2) match { case (_ :: tl1, _) if isCommon1(idx1, lcs) => // all values in arr2 were added until the index of common value val until = lcs.head._2 @@ -150,13 +148,13 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony acc ++ remove(idx1 + shift1, until - 1 + shift1, idx1 + shift1, arr1)) case (v1 :: tl1, v2 :: tl2) => // values are different, recursively compute the diff of these values - loop(tl1, tl2, idx1 + 1, shift1, idx2 + 1, lcs, acc ++ diff(v1, v2, path / (idx1 + shift1))) + diff(v1, v2, path / (idx1 + shift1)).flatMap(d => loop(tl1, tl2, idx1 + 1, shift1, idx2 + 1, lcs, acc ++ d)) case (_, Nil) => // all subsequent values in arr1 were removed - acc ++ remove(idx1 + shift1, idx1 + arr1.size - 1 + shift1, idx1 + shift1, arr1) + Eval.now(acc ++ remove(idx1 + shift1, idx1 + arr1.size - 1 + shift1, idx1 + shift1, arr1)) case (Nil, _) => // all subsequent value in arr2 were added - acc ++ Chain.fromSeq(arr2.map(Add(path / "-", _))) + Eval.now(acc ++ Chain.fromSeq(arr2.map(Add(path / "-", _)))) } loop(arr1, arr2, 0, 0, 0, lcs, Chain.empty) From 62eba218bf1fe24be4ba4410c9f4912ea4ba712f Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 18:44:04 +0100 Subject: [PATCH 6/9] Avoid deep recursive object equalities --- .../scala/diffson/jsonpatch/JsonDiff.scala | 59 +++++-------------- .../src/main/scala/diffson/TestJsonDiff.scala | 14 +++-- .../main/scala/diffson/TestSimpleDiff.scala | 14 ++--- 3 files changed, 30 insertions(+), 57 deletions(-) diff --git a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala index b96c9b5..76bb4f0 100644 --- a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala +++ b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala @@ -33,7 +33,7 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony private def diff(json1: Json, json2: Json, pointer: Pointer): Eval[Chain[Operation[Json]]] = (json1, json2) match { - case (JsObject(fields1), JsObject(fields2)) => fieldsDiff(fields1.toList, fields2.toList, pointer) + case (JsObject(fields1), JsObject(fields2)) => fieldsDiff(fields1.toList, fields2, pointer) case (JsArray(arr1), JsArray(arr2)) if diffArray => arraysDiff(arr1.toList, arr2.toList, pointer) case _ if json1 === json2 => // if they are equal, this one is easy... @@ -42,51 +42,22 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony } private def fieldsDiff(fields1: List[(String, Json)], - fields2: List[(String, Json)], - path: Pointer): Eval[Chain[Operation[Json]]] = { - // sort fields by name in both objects - val sorted1 = fields1.sortBy(_._1) - val sorted2 = fields2.sortBy(_._1) - @tailrec - def associate(fields1: List[(String, Json)], - fields2: List[(String, Json)], - acc: Chain[(Option[(String, Json)], Option[(String, Json)])]) - : Chain[(Option[(String, Json)], Option[(String, Json)])] = (fields1, fields2) match { - case (f1 :: t1, f2 :: t2) if f1._1 == f2._1 => - // same name, associate both - associate(t1, t2, acc.prepend((Some(f1), Some(f2)))) - case (f1 :: t1, f2 :: _) if f1._1 < f2._1 => - // the first field is not present in the second object - associate(t1, fields2, acc.prepend((Some(f1), None))) - case (_ :: _, f2 :: t2) => - // the second field is not present in the first object - associate(fields1, t2, acc.prepend((None, Some(f2)))) - case (_, Nil) => - Chain.fromSeq(fields1.map(Some(_) -> None)) ++ acc - case (Nil, _) => - Chain.fromSeq(fields2.map(None -> Some(_))) ++ acc + fields2: Map[String, Json], + path: Pointer): Eval[Chain[Operation[Json]]] = + fields1 match { + case (fld, value1) :: fields1 => + fields2.get(fld) match { + case Some(value2) => + fieldsDiff(fields1, fields2.removed(fld), path).flatMap(d => diff(value1, value2, path / fld).map(_ ++ d)) + case None => + // field is not in the second object, delete it + fieldsDiff(fields1, fields2, path).map( + _.prepend(Remove(path / fld, if (rememberOld) Some(value1) else None))) + } + case Nil => + Eval.now(Chain.fromSeq(fields2.toList).map { case (fld, value) => Add(path / fld, value) }) } - def fields(fs: List[(Option[(String, Json)], Option[(String, Json)])], - acc: Chain[Operation[Json]]): Eval[Chain[Operation[Json]]] = fs match { - case (Some(f1), Some(f2)) :: tl if f1._2 === f2._2 => - // all right, nothing changed - fields(tl, acc) - case (Some(f1), Some(f2)) :: tl => - // same field name, different values - diff(f1._2, f2._2, path / f1._1).flatMap(d => fields(tl, d ++ acc)) - case (Some(f1), None) :: tl => - // the field was deleted - fields(tl, acc.prepend(Remove[Json](path / f1._1, if (rememberOld) Some(f1._2) else None))) - case (None, Some(f2)) :: tl => - // the field was added - fields(tl, acc.prepend(Add(path / f2._1, f2._2))) - case _ => - Eval.now(acc) - } - fields(associate(sorted1, sorted2, Chain.empty).toList, Chain.empty) - } - private def arraysDiff(arr1: List[Json], arr2: List[Json], path: Pointer): Eval[Chain[Operation[Json]]] = { // get the longest common subsequence in the array val lcs = Lcs.lcs(arr1, arr2) diff --git a/testkit/shared/src/main/scala/diffson/TestJsonDiff.scala b/testkit/shared/src/main/scala/diffson/TestJsonDiff.scala index 7fe6edd..9c6dbc4 100644 --- a/testkit/shared/src/main/scala/diffson/TestJsonDiff.scala +++ b/testkit/shared/src/main/scala/diffson/TestJsonDiff.scala @@ -51,9 +51,11 @@ abstract class TestJsonDiff[Json](implicit Json: Jsony[Json]) val json3 = parseJson("""{"lbl": 32, "new1": false, "new2": null}""") val json4 = parseJson("""{"a": 3, "b": {"a": true }}""") val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""") - diff(json1, json2) should be(JsonPatch[Json](Add(Pointer("new"), false: Json))) - diff(json1, json3) should be(JsonPatch[Json](Add(Pointer("new2"), Json.Null), Add(Pointer("new1"), false: Json))) - diff(json4, json5) should be(JsonPatch[Json](Add(Pointer("b", "b"), 43: Json), Add(Pointer("c"), Json.Null))) + diff(json1, json2).ops should contain theSameElementsAs List(Add(Pointer("new"), false: Json)) + diff(json1, json3).ops should contain theSameElementsAs List(Add(Pointer("new2"), Json.Null), + Add(Pointer("new1"), false: Json)) + diff(json4, json5).ops should contain theSameElementsAs List(Add(Pointer("b", "b"), 43: Json), + Add(Pointer("c"), Json.Null)) } it should "contain a remove operation for each removed field" in { @@ -62,9 +64,9 @@ abstract class TestJsonDiff[Json](implicit Json: Jsony[Json]) val json3 = parseJson("""{"lbl": 32, "old1": false, "old2": null}""") val json4 = parseJson("""{"a": 3, "b": {"a": true }}""") val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""") - diff(json2, json1) should be(JsonPatch[Json](Remove(Pointer("old")))) - diff(json3, json1) should be(JsonPatch[Json](Remove(Pointer("old2")), Remove(Pointer("old1")))) - diff(json5, json4) should be(JsonPatch[Json](Remove(Pointer("b", "b")), Remove(Pointer("c")))) + diff(json2, json1).ops should contain theSameElementsAs List(Remove(Pointer("old"))) + diff(json3, json1).ops should contain theSameElementsAs List(Remove(Pointer("old2")), Remove(Pointer("old1"))) + diff(json5, json4).ops should contain theSameElementsAs List(Remove(Pointer("b", "b")), Remove(Pointer("c"))) } it should "correctly handle array diffs in objects" in { diff --git a/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala b/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala index 281b2aa..0ddca67 100644 --- a/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala +++ b/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala @@ -40,15 +40,15 @@ abstract class TestSimpleDiff[Json](implicit val Json: Jsony[Json]) diff(parseJson("true"), parseJson("13")) should be(JsonPatch[Json](Replace(Pointer.Root, 13: Json))) } - it should "contain an add operation for each added field" in { + it should "conta be be bein an add operation for each added field" in { val json1 = parseJson("""{"lbl": 32}""") val json2 = parseJson("""{"lbl": 32, "new": false}""") val json3 = parseJson("""{"lbl": 32, "new1": false, "new2": null}""") val json4 = parseJson("""{"a": 3, "b": {"a": true }}""") val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""") - diff(json1, json2) should be(JsonPatch[Json](Add(Pointer("new"), false: Json))) - diff(json1, json3) should be(JsonPatch[Json](Add(Pointer("new2"), Json.Null), Add(Pointer("new1"), false: Json))) - diff(json4, json5) should be(JsonPatch[Json](Add(Pointer("b", "b"), 43: Json), Add(Pointer("c"), Json.Null))) + diff(json1, json2).ops should contain theSameElementsAs List(Add(Pointer("new"), false: Json)) + diff(json1, json3).ops should contain theSameElementsAs List(Add(Pointer("new2"), Json.Null), Add(Pointer("new1"), false: Json)) + diff(json4, json5).ops should contain theSameElementsAs List(Add(Pointer("b", "b"), 43: Json), Add(Pointer("c"), Json.Null)) } it should "contain a remove operation for each removed field" in { @@ -57,9 +57,9 @@ abstract class TestSimpleDiff[Json](implicit val Json: Jsony[Json]) val json3 = parseJson("""{"lbl": 32, "old1": false, "old2": null}""") val json4 = parseJson("""{"a": 3, "b": {"a": true }}""") val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""") - diff(json2, json1) should be(JsonPatch[Json](Remove(Pointer("old")))) - diff(json3, json1) should be(JsonPatch[Json](Remove(Pointer("old2")), Remove(Pointer("old1")))) - diff(json5, json4) should be(JsonPatch[Json](Remove(Pointer("b", "b")), Remove(Pointer("c")))) + diff(json2, json1).ops should contain theSameElementsAs List(Remove(Pointer("old"))) + diff(json3, json1).ops should contain theSameElementsAs List(Remove(Pointer("old2")), Remove(Pointer("old1"))) + diff(json5, json4).ops should contain theSameElementsAs List(Remove(Pointer("b", "b")), Remove(Pointer("c"))) } it should "correctly handle array diffs in objects (i.e. just replaced)" in { From 3bbbfffe2c410f6e8f1edff33882fea46eafe2a0 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 18:54:40 +0100 Subject: [PATCH 7/9] Update CI workflows --- .github/workflows/ci.yml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4a895d..d4d5c8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,11 +94,11 @@ jobs: - name: Make target directories if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main') - run: mkdir -p circe/jvm/target testkit/native/target target testkit/js/target .js/target core/.native/target playJson/jvm/target sprayJson/target core/.js/target circe/js/target core/.jvm/target .jvm/target .native/target circe/native/target playJson/js/target testkit/jvm/target project/target + run: mkdir -p circe/jvm/target testkit/native/target target testkit/js/target .js/target core/.native/target playJson/jvm/target benchmarks/.jvm/target sprayJson/target core/.js/target circe/js/target core/.jvm/target .jvm/target .native/target circe/native/target playJson/js/target testkit/jvm/target project/target - name: Compress target directories if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main') - run: tar cf targets.tar circe/jvm/target testkit/native/target target testkit/js/target .js/target core/.native/target playJson/jvm/target sprayJson/target core/.js/target circe/js/target core/.jvm/target .jvm/target .native/target circe/native/target playJson/js/target testkit/jvm/target project/target + run: tar cf targets.tar circe/jvm/target testkit/native/target target testkit/js/target .js/target core/.native/target playJson/jvm/target benchmarks/.jvm/target sprayJson/target core/.js/target circe/js/target core/.jvm/target .jvm/target .native/target circe/native/target playJson/js/target testkit/jvm/target project/target - name: Upload target directories if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main') @@ -181,32 +181,32 @@ jobs: tar xf targets.tar rm targets.tar - - name: Download target directories (2.13.9, rootJS) + - name: Download target directories (2.13.10, rootJS) uses: actions/download-artifact@v3 with: - name: target-${{ matrix.os }}-${{ matrix.java }}-2.13.9-rootJS + name: target-${{ matrix.os }}-${{ matrix.java }}-2.13.10-rootJS - - name: Inflate target directories (2.13.9, rootJS) + - name: Inflate target directories (2.13.10, rootJS) run: | tar xf targets.tar rm targets.tar - - name: Download target directories (2.13.9, rootJVM) + - name: Download target directories (2.13.10, rootJVM) uses: actions/download-artifact@v3 with: - name: target-${{ matrix.os }}-${{ matrix.java }}-2.13.9-rootJVM + name: target-${{ matrix.os }}-${{ matrix.java }}-2.13.10-rootJVM - - name: Inflate target directories (2.13.9, rootJVM) + - name: Inflate target directories (2.13.10, rootJVM) run: | tar xf targets.tar rm targets.tar - - name: Download target directories (2.13.9, rootNative) + - name: Download target directories (2.13.10, rootNative) uses: actions/download-artifact@v3 with: - name: target-${{ matrix.os }}-${{ matrix.java }}-2.13.9-rootNative + name: target-${{ matrix.os }}-${{ matrix.java }}-2.13.10-rootNative - - name: Inflate target directories (2.13.9, rootNative) + - name: Inflate target directories (2.13.10, rootNative) run: | tar xf targets.tar rm targets.tar From 4f63e6bf0d69eca133b9967c3345a82d71bb76fa Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 18:55:11 +0100 Subject: [PATCH 8/9] Format tests --- testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala b/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala index 0ddca67..728152a 100644 --- a/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala +++ b/testkit/shared/src/main/scala/diffson/TestSimpleDiff.scala @@ -47,8 +47,10 @@ abstract class TestSimpleDiff[Json](implicit val Json: Jsony[Json]) val json4 = parseJson("""{"a": 3, "b": {"a": true }}""") val json5 = parseJson("""{"a": 3, "b": {"a": true, "b": 43}, "c": null}""") diff(json1, json2).ops should contain theSameElementsAs List(Add(Pointer("new"), false: Json)) - diff(json1, json3).ops should contain theSameElementsAs List(Add(Pointer("new2"), Json.Null), Add(Pointer("new1"), false: Json)) - diff(json4, json5).ops should contain theSameElementsAs List(Add(Pointer("b", "b"), 43: Json), Add(Pointer("c"), Json.Null)) + diff(json1, json3).ops should contain theSameElementsAs List(Add(Pointer("new2"), Json.Null), + Add(Pointer("new1"), false: Json)) + diff(json4, json5).ops should contain theSameElementsAs List(Add(Pointer("b", "b"), 43: Json), + Add(Pointer("c"), Json.Null)) } it should "contain a remove operation for each removed field" in { From 97aee07c53e1e121aee3407470d1b13abb1dcae1 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 2 Mar 2023 19:01:16 +0100 Subject: [PATCH 9/9] Make Scala native compile --- core/src/main/scala/diffson/jsonpatch/JsonDiff.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala index 76bb4f0..44c3ae0 100644 --- a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala +++ b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala @@ -48,7 +48,7 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony case (fld, value1) :: fields1 => fields2.get(fld) match { case Some(value2) => - fieldsDiff(fields1, fields2.removed(fld), path).flatMap(d => diff(value1, value2, path / fld).map(_ ++ d)) + fieldsDiff(fields1, fields2 - fld, path).flatMap(d => diff(value1, value2, path / fld).map(_ ++ d)) case None => // field is not in the second object, delete it fieldsDiff(fields1, fields2, path).map(