From fa554a4bbcf3ebd6f98f6fc4b96657b9f9a0de90 Mon Sep 17 00:00:00 2001 From: Astrid Jahn Date: Fri, 12 Apr 2024 11:06:36 +0200 Subject: [PATCH] Switch out uPickle for Circe in codegen This is a preliminary change for being able to support smithy-build.json as input configuration objects; parsing these in a future-proof way is relatively complicated and difficult to support in uPickle without hand-writing the entirety of the parsing logic. Since Circe is already used elsewhere in the project, we replace the uPickle with Circe, including the implementation of creating the smithy-build.json file from settings. Note that the special requirement of merging two JSON files while concatenating and deduplicating their arrays is not supported out of the box by Circe's deepMerge and still has to be handled manually (cf https://github.com/circe/circe/issues/271) --- build.sbt | 4 +- .../smithy4s/codegen/SmithyBuildJson.scala | 57 ++++++---------- .../codegen/internals/SmithyBuild.scala | 12 ++-- .../codegen/internals/SmithyBuildSpec.scala | 67 +++++++++++++++---- 4 files changed, 84 insertions(+), 56 deletions(-) diff --git a/build.sbt b/build.sbt index 6479ae281..0d034dc5d 100644 --- a/build.sbt +++ b/build.sbt @@ -373,7 +373,9 @@ lazy val codegen = projectMatrix Dependencies.Alloy.openapi, Dependencies.Smithytranslate.proto, "com.lihaoyi" %% "os-lib" % "0.9.3", - "com.lihaoyi" %% "upickle" % "3.2.0", + Dependencies.Circe.core.value, + Dependencies.Circe.parser.value, + Dependencies.Circe.generic.value, Dependencies.collectionsCompat.value, "org.scala-lang" % "scala-reflect" % scalaVersion.value, "io.get-coursier" %% "coursier" % "2.1.9" diff --git a/modules/codegen/src/smithy4s/codegen/SmithyBuildJson.scala b/modules/codegen/src/smithy4s/codegen/SmithyBuildJson.scala index c23488310..f871f2d60 100644 --- a/modules/codegen/src/smithy4s/codegen/SmithyBuildJson.scala +++ b/modules/codegen/src/smithy4s/codegen/SmithyBuildJson.scala @@ -19,10 +19,9 @@ package smithy4s.codegen import smithy4s.codegen.internals.SmithyBuild import smithy4s.codegen.internals.SmithyBuildMaven import smithy4s.codegen.internals.SmithyBuildMavenRepository -import upickle.default._ +import io.circe.{Json, parser} private[codegen] object SmithyBuildJson { - def toJson( imports: Seq[String], dependencies: Seq[String], @@ -43,47 +42,29 @@ private[codegen] object SmithyBuildJson { def merge( json1: String, json2: String - ): String = { - val j1 = read[ujson.Value](json1) - val j2 = read[ujson.Value](json2) - val merged = mergeJs(j1, j2) - val finalJs = removeArrayDuplicates(merged) - finalJs.render(indent = 4) - } + ): String = (for { + j1 <- parser.parse(json1) + j2 <- parser.parse(json2) + merged = mergeJs(j1, j2) + } yield merged).left.map(err => throw err).merge.spaces4 private def mergeJs( - v1: ujson.Value, - v2: ujson.Value - ): ujson.Value = { - (v1, v2) match { - case (ujson.Obj(obj1), ujson.Obj(obj2)) => - val result = obj2.foldLeft(obj1.toMap) { - case (elements, (key, value2)) => - val value = elements.get(key) match { - case None => - value2 - case Some(value1) => - mergeJs(value1, value2) + v1: Json, + v2: Json + ): Json = { + (v1.asObject, v2.asObject, v1.asArray, v2.asArray) match { + // copied from circe's deepMerge method, however in order to handle concat + deduplication on arrays we need to do it manually here + case (Some(lhs), Some(rhs), _, _) => + Json.fromJsonObject( + lhs.toIterable.foldLeft(rhs) { case (acc, (key, value)) => + rhs(key).fold(acc.add(key, value)) { r => + acc.add(key, mergeJs(value, r)) } - elements.updated(key, value) - } - ujson.Obj.from(result) - case (arr1: ujson.Arr, arr2: ujson.Arr) => - ujson.Arr(arr1.arr ++ arr2.arr) - case (_, _) => v1 - } - } - - private def removeArrayDuplicates(js: ujson.Value): ujson.Value = { - js match { - case ujson.Obj(obj1) => - ujson.Obj.from( - obj1.toList.map { case (key, value) => - key -> removeArrayDuplicates(value) } ) - case (arr1: ujson.Arr) => arr1.arr.distinct - case x => x + case (_, _, Some(arr1), Some(arr2)) => + Json.arr((arr1 ++ arr2).distinct: _*) + case _ => v1 } } } diff --git a/modules/codegen/src/smithy4s/codegen/internals/SmithyBuild.scala b/modules/codegen/src/smithy4s/codegen/internals/SmithyBuild.scala index b600ed5d4..3cf6e4828 100644 --- a/modules/codegen/src/smithy4s/codegen/internals/SmithyBuild.scala +++ b/modules/codegen/src/smithy4s/codegen/internals/SmithyBuild.scala @@ -17,7 +17,9 @@ package smithy4s.codegen package internals -import upickle.default._ +import io.circe.Codec +import io.circe.generic.semiauto._ +import io.circe.syntax._ private[internals] final case class SmithyBuild( version: String, @@ -25,8 +27,8 @@ private[internals] final case class SmithyBuild( maven: SmithyBuildMaven ) private[codegen] object SmithyBuild { - implicit val codecs: ReadWriter[SmithyBuild] = macroRW - def writeJson(sb: SmithyBuild): String = write(sb, indent = 4) + implicit val codecs: Codec[SmithyBuild] = deriveCodec + def writeJson(sb: SmithyBuild): String = sb.asJson.spaces4 } private[internals] final case class SmithyBuildMaven( @@ -34,12 +36,12 @@ private[internals] final case class SmithyBuildMaven( repositories: Seq[SmithyBuildMavenRepository] ) private[codegen] object SmithyBuildMaven { - implicit val codecs: ReadWriter[SmithyBuildMaven] = macroRW + implicit val codecs: Codec[SmithyBuildMaven] = deriveCodec } private[internals] final case class SmithyBuildMavenRepository( url: String ) private[codegen] object SmithyBuildMavenRepository { - implicit val codecs: ReadWriter[SmithyBuildMavenRepository] = macroRW + implicit val codecs: Codec[SmithyBuildMavenRepository] = deriveCodec } diff --git a/modules/codegen/test/src/smithy4s/codegen/internals/SmithyBuildSpec.scala b/modules/codegen/test/src/smithy4s/codegen/internals/SmithyBuildSpec.scala index d8b8801ed..a9ce64144 100644 --- a/modules/codegen/test/src/smithy4s/codegen/internals/SmithyBuildSpec.scala +++ b/modules/codegen/test/src/smithy4s/codegen/internals/SmithyBuildSpec.scala @@ -30,17 +30,17 @@ final class SmithyBuildSpec extends munit.FunSuite { assertEquals( actual, """|{ - | "version": "1.0", - | "imports": [ + | "version" : "1.0", + | "imports" : [ | "src/" | ], - | "maven": { - | "dependencies": [ + | "maven" : { + | "dependencies" : [ | "dep" | ], - | "repositories": [ + | "repositories" : [ | { - | "url": "repo" + | "url" : "repo" | } | ] | } @@ -73,18 +73,61 @@ final class SmithyBuildSpec extends munit.FunSuite { assertEquals( actual, """|{ - | "version": "1.0", - | "imports": [ + | "version" : "1.0", + | "imports" : [ | "src/main/smithy" | ], - | "maven": { - | "dependencies": [ + | "maven" : { + | "dependencies" : [ | "oterh", | "dep1" | ], - | "repositories": [] + | "repositories" : [ + | ] + | }, + | "custom" : "attribute" + |}""".stripMargin + ) + } + + test("merge two json de-duplicating arrays") { + val actual = SmithyBuildJson.merge( + """|{ + | "version": "1.0", + | "imports": ["src/main/smithy"], + | "maven": { + | "dependencies": ["oterh", "dep1"], + | "repositories": [] + | } + |} + |""".stripMargin, + """|{ + | "version": "1.0", + | "imports": ["src/main/smithy"], + | "maven": { + | "dependencies": ["dep1"], + | "repositories": [] + | }, + | "custom": "attribute" + |} + |""".stripMargin + ) + assertEquals( + actual, + """|{ + | "version" : "1.0", + | "imports" : [ + | "src/main/smithy" + | ], + | "maven" : { + | "dependencies" : [ + | "oterh", + | "dep1" + | ], + | "repositories" : [ + | ] | }, - | "custom": "attribute" + | "custom" : "attribute" |}""".stripMargin ) }