Merge pull request #1541 from guardrail-dev/revert-1532

Revert fix for #1532
guardrail-dev · Jul 22, 2022 · bad48a6 · bad48a6
2 parents 92ba7e2 + a5618d8
commit bad48a6
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 92 deletions.
diff --git a/modules/core/src/main/scala/dev/guardrail/generators/LanguageParameter.scala b/modules/core/src/main/scala/dev/guardrail/generators/LanguageParameter.scala
@@ -129,6 +129,13 @@ object LanguageParameter {
         (meta, required)                                            <- paramMeta(parameter)
         core.Resolved(paramType, _, rawDefaultType, reifiedRawType) <- core.ResolvedType.resolve[L, F](meta, protocolElems)
 
+        declType <-
+          if (!required) {
+            liftOptionalType(paramType)
+          } else {
+            paramType.pure[F]
+          }
+
         baseDefaultValue <- extractTypeName(paramType).flatMap(_.fold(rawDefaultType.traverse(_.pure[F])) { tpe =>
           protocolElems
             .flatTraverse {
@@ -143,16 +150,11 @@ object LanguageParameter {
             })
         })
 
-        (declType, defaultValue) <-
+        defaultValue <-
           if (!required) {
-            baseDefaultValue match {
-              case None =>
-                (liftOptionalType(paramType), emptyOptionalTerm().map(Option.apply _)).mapN((_, _))
-              case Some(value) =>
-                (paramType, baseDefaultValue).pure[F]
-            }
+            (baseDefaultValue.traverse(liftOptionalTerm), emptyOptionalTerm().map(Option.apply _)).mapN(_.orElse(_))
           } else {
-            (paramType, baseDefaultValue).pure[F]
+            baseDefaultValue.pure[F]
           }
 
         name <- getParameterName(parameter)

diff --git a/.../java-support/src/main/scala/dev/guardrail/generators/java/jackson/JacksonGenerator.scala b/.../java-support/src/main/scala/dev/guardrail/generators/java/jackson/JacksonGenerator.scala
@@ -896,18 +896,18 @@ class JacksonGenerator private (implicit Cl: CollectionsLibTerms[JavaLanguage, T
             Target.log.warning(s"Can't generate default value for class $clsName and property $name.") >> Target.pure(None)
           case None => Target.pure(None)
         }): Target[Option[Expression]]
-        (finalDeclType, finalDefaultValue) <- requirement match {
-          case PropertyRequirement.Required =>
-            Target.pure((tpe, expressionDefaultValue))
-          case _ =>
-            defaultValue match {
-              case Some(value) => value.toExpression.map(Option.apply).map(tpe -> _)
-              case None =>
-                for {
-                  optionalTpe <- Cl.liftOptionalType(tpe)
-                } yield (optionalTpe, Option.empty[Expression])
-            }
-        }
+        finalDefaultTypeValue <- Option(requirement)
+          .filter {
+            case PropertyRequirement.Required => true
+            case _                            => false
+          }
+          .fold[Target[(Type, Option[Expression])]](
+            for {
+              optionalTpe      <- Cl.liftOptionalType(tpe)
+              defaultValueExpr <- defaultValue.fold(Target.pure(Option.empty[Expression]))(dv => dv.toExpression.map(Option.apply))
+            } yield (optionalTpe, defaultValueExpr)
+          )(Function.const(Target.pure((tpe, expressionDefaultValue))) _)
+        (finalDeclType, finalDefaultValue) = finalDefaultTypeValue
         term <- safeParseParameter(s"final ${finalDeclType} $fieldName")
         dep     = classDep.filterNot(_.asString == clsName) // Filter out our own class name
         pattern = property.downField("pattern", _.getPattern).map(PropertyValidations)

diff --git a/modules/sample-akkaHttp/src/test/scala/core/issues/Issue1532.scala b/modules/sample-akkaHttp/src/test/scala/core/issues/Issue1532.scala
diff --git a/modules/sample/src/main/resources/issues/issue1532.yaml b/modules/sample/src/main/resources/issues/issue1532.yaml
diff --git a/modules/scala-akka-http/src/test/scala/core/issues/Issue43.scala b/modules/scala-akka-http/src/test/scala/core/issues/Issue43.scala
@@ -311,7 +311,7 @@ class Issue43 extends AnyFunSpec with Matchers with SwaggerSpecRunner {
 
     it("should generate right case class") {
       clsDog.structure shouldBe q"""case class Dog(name: String, packSize: Int = 0) extends Pet""".structure
-      clsPersianCat.structure shouldBe q"""case class PersianCat(name: String, huntingSkill: Cat.HuntingSkill = Cat.HuntingSkill.Lazy, wool: Int = 10) extends Cat""".structure
+      clsPersianCat.structure shouldBe q"""case class PersianCat(name: String, huntingSkill: Cat.HuntingSkill = Cat.HuntingSkill.Lazy, wool: Option[Int] = Option(10)) extends Cat""".structure
     }
 
     it("should generate right companion object (Dog)") {
@@ -334,7 +334,7 @@ class Issue43 extends AnyFunSpec with Matchers with SwaggerSpecRunner {
             val readOnlyKeys = _root_.scala.Predef.Set[_root_.scala.Predef.String]()
             _root_.io.circe.Encoder.AsObject.instance[PersianCat](a => _root_.io.circe.JsonObject.fromIterable(_root_.scala.Vector(("name", a.name.asJson), ("huntingSkill", a.huntingSkill.asJson), ("wool", a.wool.asJson), ("petType", _root_.io.circe.Json.fromString("PersianCat"))))).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
           }
-          implicit val decodePersianCat: _root_.io.circe.Decoder[PersianCat] = new _root_.io.circe.Decoder[PersianCat] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[PersianCat] = for (v0 <- c.downField("name").as[String]; v1 <- c.downField("huntingSkill").as[Cat.HuntingSkill]; v2 <- c.downField("wool").as[Int]) yield PersianCat(v0, v1, v2) }
+          implicit val decodePersianCat: _root_.io.circe.Decoder[PersianCat] = new _root_.io.circe.Decoder[PersianCat] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[PersianCat] = for (v0 <- c.downField("name").as[String]; v1 <- c.downField("huntingSkill").as[Cat.HuntingSkill]; v2 <- c.downField("wool").as[Option[Int]]) yield PersianCat(v0, v1, v2) }
         }
       """
       companionPersianCat.structure shouldBe companion.structure

diff --git a/modules/scala-akka-http/src/test/scala/tests/generators/akkaHttp/DefinitionSpec.scala b/modules/scala-akka-http/src/test/scala/tests/generators/akkaHttp/DefinitionSpec.scala
@@ -167,15 +167,15 @@ class DefinitionSpec extends AnyFunSuite with Matchers with SwaggerSpecRunner {
     val cmp = companionForStaticDefns(staticDefns)
 
     val definition = q"""
-      case class Sixth(defval: Int = 1, defvalOpt: Long = 2L)
+      case class Sixth(defval: Int = 1, defvalOpt: Option[Long] = Option(2L))
     """
     val companion = q"""
       object Sixth {
         implicit val encodeSixth: _root_.io.circe.Encoder.AsObject[Sixth] = {
           val readOnlyKeys = _root_.scala.Predef.Set[_root_.scala.Predef.String]()
           _root_.io.circe.Encoder.AsObject.instance[Sixth](a => _root_.io.circe.JsonObject.fromIterable(_root_.scala.Vector(("defval", a.defval.asJson), ("defval_opt", a.defvalOpt.asJson)))).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
         }
-        implicit val decodeSixth: _root_.io.circe.Decoder[Sixth] = new _root_.io.circe.Decoder[Sixth] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[Sixth] = for (v0 <- c.downField("defval").as[Int]; v1 <- c.downField("defval_opt").as[Long]) yield Sixth(v0, v1) }
+        implicit val decodeSixth: _root_.io.circe.Decoder[Sixth] = new _root_.io.circe.Decoder[Sixth] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[Sixth] = for (v0 <- c.downField("defval").as[Int]; v1 <- c.downField("defval_opt").as[Option[Long]]) yield Sixth(v0, v1) }
       }
     """
 

diff --git a/...ala-akka-http/src/test/scala/tests/generators/akkaHttp/client/DefaultParametersTest.scala b/...ala-akka-http/src/test/scala/tests/generators/akkaHttp/client/DefaultParametersTest.scala
@@ -155,7 +155,7 @@ class DefaultParametersTest extends AnyFunSuite with Matchers with SwaggerSpecRu
         val getOrderByIdOKDecoder = {
           structuredJsonEntityUnmarshaller.flatMap(_ => _ => json => io.circe.Decoder[Order].decodeJson(json).fold(FastFuture.failed, FastFuture.successful))
         }
-        def getOrderById(orderId: Long, defparmOpt: Int = 1, defparm: Int = 2, headerMeThis: String, headers: List[HttpHeader] = Nil): EitherT[Future, Either[Throwable, HttpResponse], GetOrderByIdResponse] = {
+        def getOrderById(orderId: Long, defparmOpt: Option[Int] = Option(1), defparm: Int = 2, headerMeThis: String, headers: List[HttpHeader] = Nil): EitherT[Future, Either[Throwable, HttpResponse], GetOrderByIdResponse] = {
           val allHeaders = headers ++ scala.collection.immutable.Seq[Option[HttpHeader]](Some(RawHeader("HeaderMeThis", Formatter.show(headerMeThis)))).flatten
           makeRequest(HttpMethods.GET, host + basePath + "/store/order/" + Formatter.addPath(orderId) + "?" + Formatter.addArg("defparm_opt", defparmOpt) + Formatter.addArg("defparm", defparm), allHeaders, HttpEntity.Empty, HttpProtocols.`HTTP/1.1`).flatMap(req => EitherT(httpClient(req).flatMap(resp => resp.status match {
             case StatusCodes.OK =>

diff --git a/modules/scala-http4s/src/test/scala/core/issues/Issue370.scala b/modules/scala-http4s/src/test/scala/core/issues/Issue370.scala
@@ -70,7 +70,7 @@ class Issue370 extends AnyFunSuite with Matchers with SwaggerSpecRunner {
             val readOnlyKeys = _root_.scala.Predef.Set[_root_.scala.Predef.String]()
             _root_.io.circe.Encoder.AsObject.instance[Foo](a => _root_.io.circe.JsonObject.fromIterable(_root_.scala.Vector(("value", a.value.asJson), ("value2", a.value2.asJson), ("nested", a.nested.asJson)))).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
           }
-          implicit val decodeFoo: _root_.io.circe.Decoder[Foo] = new _root_.io.circe.Decoder[Foo] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[Foo] = for (v0 <- c.downField("value").as[Foo.Value]; v1 <- c.downField("value2").as[Baz]; v2 <- c.downField("nested").as[Option[Foo.Nested]]) yield Foo(v0, v1, v2) }
+          implicit val decodeFoo: _root_.io.circe.Decoder[Foo] = new _root_.io.circe.Decoder[Foo] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[Foo] = for (v0 <- c.downField("value").as[Option[Foo.Value]]; v1 <- c.downField("value2").as[Baz]; v2 <- c.downField("nested").as[Option[Foo.Nested]]) yield Foo(v0, v1, v2) }
           sealed abstract class Value(val value: String) extends _root_.scala.Product with _root_.scala.Serializable { override def toString: String = value.toString }
           object Value {
             object members {
@@ -86,13 +86,13 @@ class Issue370 extends AnyFunSuite with Matchers with SwaggerSpecRunner {
             def from(value: String): _root_.scala.Option[Value] = values.find(_.value == value)
             implicit val order: cats.Order[Value] = cats.Order.by[Value, Int](values.indexOf)
           }
-          case class Nested(value: Foo.Nested.Value = Foo.Nested.Value.C)
+          case class Nested(value: Option[Foo.Nested.Value] = Option(Foo.Nested.Value.C))
           object Nested {
             implicit val encodeNested: _root_.io.circe.Encoder.AsObject[Nested] = {
               val readOnlyKeys = _root_.scala.Predef.Set[_root_.scala.Predef.String]()
               _root_.io.circe.Encoder.AsObject.instance[Nested](a => _root_.io.circe.JsonObject.fromIterable(_root_.scala.Vector(("value", a.value.asJson)))).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key)))
             }
-            implicit val decodeNested: _root_.io.circe.Decoder[Nested] = new _root_.io.circe.Decoder[Nested] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[Nested] = for (v0 <- c.downField("value").as[Foo.Nested.Value]) yield Nested(v0) }
+            implicit val decodeNested: _root_.io.circe.Decoder[Nested] = new _root_.io.circe.Decoder[Nested] { final def apply(c: _root_.io.circe.HCursor): _root_.io.circe.Decoder.Result[Nested] = for (v0 <- c.downField("value").as[Option[Foo.Nested.Value]]) yield Nested(v0) }
             sealed abstract class Value(val value: String) extends _root_.scala.Product with _root_.scala.Serializable { override def toString: String = value.toString }
             object Value {
               object members {
@@ -112,7 +112,7 @@ class Issue370 extends AnyFunSuite with Matchers with SwaggerSpecRunner {
         }
        """
 
-      c1.structure shouldEqual q"case class Foo(value: Foo.Value = Foo.Value.A, value2: Baz = Baz.X, nested: Option[Foo.Nested] = None)".structure
+      c1.structure shouldEqual q"case class Foo(value: Option[Foo.Value] = Option(Foo.Value.A), value2: Baz = Baz.X, nested: Option[Foo.Nested] = None)".structure
       companion.structure shouldEqual cmp.structure
     }
 

diff --git a/...es/scala-http4s/src/test/scala/tests/generators/http4s/client/DefaultParametersTest.scala b/...es/scala-http4s/src/test/scala/tests/generators/http4s/client/DefaultParametersTest.scala
@@ -145,7 +145,7 @@ class DefaultParametersTest extends AnyFunSuite with Matchers with SwaggerSpecRu
       private def parseOptionalHeader(response: Response[F], header: String): F[Option[String]] = F.pure(response.headers.get(CIString(header)).map(_.head.value))
       private def parseRequiredHeader(response: Response[F], header: String): F[String] = response.headers.get(CIString(header)).map(_.head.value).fold[F[String]](F.raiseError(ParseFailure("Missing required header.", s"HTTP header '$$header' is not present.")))(F.pure)
       private[this] val getOrderByIdOkDecoder = jsonOf[F, Order]
-      def getOrderById(orderId: Long, defparmOpt: Int = 1, defparm: Int = 2, headerMeThis: String, headers: List[Header.ToRaw] = List.empty): F[GetOrderByIdResponse] = {
+      def getOrderById(orderId: Long, defparmOpt: Option[Int] = Option(1), defparm: Int = 2, headerMeThis: String, headers: List[Header.ToRaw] = List.empty): F[GetOrderByIdResponse] = {
         val allHeaders: List[org.http4s.Header.ToRaw] = List[Header.ToRaw](org.http4s.headers.Accept(org.http4s.MediaType.application.json.withQValue(org.http4s.QValue.One))) ++ headers ++ List[Option[Header.ToRaw]](Some(("HeaderMeThis", Formatter.show(headerMeThis)))).flatten
         val req = Request[F](method = Method.GET, uri = Uri.unsafeFromString(host + basePath + "/store/order/" + Formatter.addPath(orderId) + "?" + Formatter.addArg("defparm_opt", defparmOpt) + Formatter.addArg("defparm", defparm)), headers = Headers(allHeaders))
         httpClient.run(req).use({

diff --git a/...-support/src/main/scala/dev/guardrail/generators/scala/circe/CirceProtocolGenerator.scala b/...-support/src/main/scala/dev/guardrail/generators/scala/circe/CirceProtocolGenerator.scala
@@ -213,21 +213,15 @@ class CirceProtocolGenerator private (circeVersion: CirceModelGenerator, applyVa
         pattern = property.downField("pattern", _.getPattern).map(PropertyValidations)
         presence     <- ScalaGenerator().selectTerm(NonEmptyList.ofInitLast(supportPackage, "Presence"))
         presenceType <- ScalaGenerator().selectType(NonEmptyList.ofInitLast(supportPackage, "Presence"))
-
-        (finalDeclType, finalDefaultValue) =
-          defaultValue match {
-            case Some(value) => tpe -> Some(value)
-            case None =>
-              requirement match {
-                case PropertyRequirement.Required => tpe -> None
-                case PropertyRequirement.Optional | PropertyRequirement.Configured(PropertyRequirement.Optional, PropertyRequirement.Optional) =>
-                  t"$presenceType[$tpe]" -> Some(q"$presence.Absent")
-                case _: PropertyRequirement.OptionalRequirement | _: PropertyRequirement.Configured =>
-                  t"Option[$tpe]" -> Some(q"None")
-                case PropertyRequirement.OptionalNullable =>
-                  t"$presenceType[Option[$tpe]]" -> None
-              }
-          }
+        (finalDeclType, finalDefaultValue) = requirement match {
+          case PropertyRequirement.Required => tpe -> defaultValue
+          case PropertyRequirement.Optional | PropertyRequirement.Configured(PropertyRequirement.Optional, PropertyRequirement.Optional) =>
+            t"$presenceType[$tpe]" -> defaultValue.map(t => q"$presence.Present($t)").orElse(Some(q"$presence.Absent"))
+          case _: PropertyRequirement.OptionalRequirement | _: PropertyRequirement.Configured =>
+            t"Option[$tpe]" -> defaultValue.map(t => q"Option($t)").orElse(Some(q"None"))
+          case PropertyRequirement.OptionalNullable =>
+            t"$presenceType[Option[$tpe]]" -> defaultValue.map(t => q"$presence.Present($t)")
+        }
         term = param"${Term.Name(fieldName)}: ${finalDeclType}".copy(default = finalDefaultValue)
         dep  = classDep.filterNot(_.value == clsName) // Filter out our own class name
       } yield ProtocolParameter[ScalaLanguage](

diff --git a/modules/scala-support/src/test/scala/tests/circe/ValidationTest.scala b/modules/scala-support/src/test/scala/tests/circe/ValidationTest.scala
@@ -80,7 +80,7 @@ class ValidationTest extends AnyFreeSpec with Matchers with SwaggerSpecRunner {
       staticDefns.definitions.head.toString() shouldBe """val `".*[0-9]+.*"` = _root_.shapeless.Witness(".*[0-9]+.*")"""
 
       val expected =
-        q"""case class ValidatedObject(v1: Int Refined _root_.eu.timepit.refined.numeric.Interval.Closed[_root_.shapeless.Witness.`1`.T, _root_.shapeless.Witness.`100`.T] = 10,
+        q"""case class ValidatedObject(v1: Option[Int Refined _root_.eu.timepit.refined.numeric.Interval.Closed[_root_.shapeless.Witness.`1`.T, _root_.shapeless.Witness.`100`.T]] = Option(10),
            v2: Option[String Refined _root_.eu.timepit.refined.string.MatchesRegex[ValidatedObject.`".*[0-9]+.*"`.T]] = None)"""
 
       cls.structure should equal(expected.structure)

diff --git a/project/src/main/scala/RegressionTests.scala b/project/src/main/scala/RegressionTests.scala
@@ -84,7 +84,6 @@ object RegressionTests {
     ExampleCase(sampleResource("issues/issue1138.yaml"), "issues.issue1138"),
     ExampleCase(sampleResource("issues/issue1260.yaml"), "issues.issue1260"),
     ExampleCase(sampleResource("issues/issue1218.yaml"), "issues.issue1218").frameworks("scala" -> Set("http4s", "http4s-v0.22")),
-    ExampleCase(sampleResource("issues/issue1532.yaml"), "issues.issue1532"),
     ExampleCase(sampleResource("multipart-form-data.yaml"), "multipartFormData"),
     ExampleCase(sampleResource("petstore.json"), "examples").args("--import", "examples.support.PositiveLong"),
     // ExampleCase(sampleResource("petstore-openapi-3.0.2.yaml"), "examples.petstore.openapi302").args("--import", "examples.support.PositiveLong"),

diff --git a/support/trigger-all-releases.sh b/support/trigger-all-releases.sh
@@ -1,5 +1,11 @@
 #!/usr/bin/env bash
 
+die() {
+  msg="$1"; shift
+  echo "$msg" >&2
+  exit 1
+}
+
 hash gh 2>/dev/null >&2 || die "$0 requires an authenticated gh. Ensure gh is on your path and gh auth login has been successfully completed before trying again"
 
 if [ "$1" = "go" ]; then