From 81c70b7fc74ea1c9ca1d51b8a817e0ff943bfa4b Mon Sep 17 00:00:00 2001 From: Diana Freitas Date: Wed, 20 Nov 2024 14:58:55 +0000 Subject: [PATCH 1/5] Make fromFullPaths tail recursive --- .../main/scala/com/kevel/apso/circe/Implicits.scala | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala b/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala index 79699b18..32eb82ca 100644 --- a/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala +++ b/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala @@ -1,5 +1,6 @@ package com.kevel.apso.circe +import scala.annotation.tailrec import scala.util.Try import io.circe._ @@ -110,10 +111,13 @@ object Implicits { * the sequence of dot-separated (or other separator) paths * @param separatorRegex * regex to use to separate fields + * @param acc + * accumulator for the resulting Json object * @return * the resulting Json object */ - def fromFullPaths(paths: Seq[(String, Json)], separatorRegex: String = "\\."): Json = { + @tailrec + def fromFullPaths(paths: Seq[(String, Json)], separatorRegex: String = "\\.", acc: Json = Json.obj()): Json = { def createJson(keys: Seq[String], value: Json): Json = { keys match { case Nil => value @@ -122,9 +126,10 @@ object Implicits { } paths match { - case Nil => Json.obj() + case Nil => acc case (path, value) :: rem => - createJson(path.split(separatorRegex).toList, value).deepMerge(fromFullPaths(rem, separatorRegex)) + val newAcc = acc.deepMerge(createJson(path.split(separatorRegex).toList, value)) + fromFullPaths(rem, separatorRegex, newAcc) } } From a58c57f224aa7a23cc9a5bec3f7c7eb42a1bc534 Mon Sep 17 00:00:00 2001 From: Diana Freitas Date: Wed, 20 Nov 2024 15:45:28 +0000 Subject: [PATCH 2/5] Use internal tail recursive function --- .../com/kevel/apso/circe/Implicits.scala | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala b/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala index 32eb82ca..7e989c6d 100644 --- a/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala +++ b/circe/src/main/scala/com/kevel/apso/circe/Implicits.scala @@ -111,13 +111,10 @@ object Implicits { * the sequence of dot-separated (or other separator) paths * @param separatorRegex * regex to use to separate fields - * @param acc - * accumulator for the resulting Json object * @return * the resulting Json object */ - @tailrec - def fromFullPaths(paths: Seq[(String, Json)], separatorRegex: String = "\\.", acc: Json = Json.obj()): Json = { + def fromFullPaths(paths: Seq[(String, Json)], separatorRegex: String = "\\."): Json = { def createJson(keys: Seq[String], value: Json): Json = { keys match { case Nil => value @@ -125,12 +122,17 @@ object Implicits { } } - paths match { - case Nil => acc - case (path, value) :: rem => - val newAcc = acc.deepMerge(createJson(path.split(separatorRegex).toList, value)) - fromFullPaths(rem, separatorRegex, newAcc) + @tailrec + def fromFullPathsRec(paths: Seq[(String, Json)], acc: Json): Json = { + paths match { + case Nil => acc + case (path, value) :: rem => + val newAcc = acc.deepMerge(createJson(path.split(separatorRegex).toList, value)) + fromFullPathsRec(rem, newAcc) + } } + + fromFullPathsRec(paths, Json.obj()) } final implicit class ApsoJsonEncoder[A](val encoder: Encoder[A]) extends AnyVal { From b6b6cc49bcbf9e30bfce699f7c8c32ab6b97907a Mon Sep 17 00:00:00 2001 From: Diana Freitas Date: Wed, 20 Nov 2024 16:04:58 +0000 Subject: [PATCH 3/5] Test that fromFullPaths does not throw a StackOverflowError --- .../src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala b/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala index 254efea7..a49a9956 100644 --- a/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala +++ b/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala @@ -26,6 +26,11 @@ class ImplicitsSpec extends Specification { res mustEqual expectedJson } + + "not throw a StackOverflowError for large lists of paths" in { + val paths = (1 to 10000).map(i => (s"a.v$i", i.asJson)).toList + fromFullPaths(paths, ".") must not(throwAn[StackOverflowError]) + } } "provide a method to get the key set of a JSON Object" in { From 26164ce6a4f7ccd64e487574c9b5e5c9d9557cf3 Mon Sep 17 00:00:00 2001 From: Diana Freitas Date: Wed, 20 Nov 2024 16:28:02 +0000 Subject: [PATCH 4/5] Test that last path values take precedence over previous values --- .../src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala b/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala index a49a9956..fd4085fe 100644 --- a/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala +++ b/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala @@ -31,6 +31,11 @@ class ImplicitsSpec extends Specification { val paths = (1 to 10000).map(i => (s"a.v$i", i.asJson)).toList fromFullPaths(paths, ".") must not(throwAn[StackOverflowError]) } + + "giving precedence to the last path value if duplicate paths exist" in { + val res = fromFullPaths(List("a" -> 1.asJson, "a" -> 2.asJson, "a" -> 3.asJson)) + res mustEqual json"""{"a": 3}""" + } } "provide a method to get the key set of a JSON Object" in { From d177737fd818a91b827de450a2de4c1c978623d7 Mon Sep 17 00:00:00 2001 From: Diana Freitas <56595843+dianaamfr@users.noreply.github.com> Date: Thu, 21 Nov 2024 09:23:26 +0000 Subject: [PATCH 5/5] Test precedence with duplicate paths at different depths Co-authored-by: Joao Azevedo --- .../test/scala/com/kevel/apso/circe/ImplicitsSpec.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala b/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala index fd4085fe..810e2504 100644 --- a/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala +++ b/circe/src/test/scala/com/kevel/apso/circe/ImplicitsSpec.scala @@ -33,8 +33,11 @@ class ImplicitsSpec extends Specification { } "giving precedence to the last path value if duplicate paths exist" in { - val res = fromFullPaths(List("a" -> 1.asJson, "a" -> 2.asJson, "a" -> 3.asJson)) - res mustEqual json"""{"a": 3}""" + val json1 = fromFullPaths(List("a" -> 1.asJson, "a" -> 2.asJson, "a" -> 3.asJson)) + json1 mustEqual json"""{"a": 3}""" + + val json2 = fromFullPaths(List("a.b.c" -> 1.asJson, "a.b" -> 2.asJson, "a" -> 3.asJson)) + json2 mustEqual json"""{"a": 3}""" } }