From 062ffb5327e86a02ffb15b795dd391a2f59fc552 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Tue, 9 Apr 2019 13:40:53 +0200 Subject: [PATCH 01/20] Added test case for issue 222 --- build.sbt | 1 + .../src/main/resources/issues/issue222.yaml | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 modules/sample/src/main/resources/issues/issue222.yaml diff --git a/build.sbt b/build.sbt index 2b27489dbb..6897894d0e 100644 --- a/build.sbt +++ b/build.sbt @@ -40,6 +40,7 @@ val exampleCases: List[(java.io.File, String, Boolean, List[String])] = List( (sampleResource("issues/issue164.yaml"), "issues.issue164", false, List.empty), (sampleResource("issues/issue215.yaml"), "issues.issue215", false, List.empty), (sampleResource("issues/issue218.yaml"), "issues.issue218", false, List.empty), + (sampleResource("issues/issue222.yaml"), "issues.issue222", false, List.empty), (sampleResource("petstore.json"), "examples", false, List("--import", "support.PositiveLong")), (sampleResource("plain.json"), "tests.dtos", false, List.empty), (sampleResource("polymorphism.yaml"), "polymorphism", false, List.empty), diff --git a/modules/sample/src/main/resources/issues/issue222.yaml b/modules/sample/src/main/resources/issues/issue222.yaml new file mode 100644 index 0000000000..2ead418946 --- /dev/null +++ b/modules/sample/src/main/resources/issues/issue222.yaml @@ -0,0 +1,26 @@ +swagger: '2.0' +info: + title: someapp + description: someapp + version: '1' +basePath: "/v1" +schemes: + - http +produces: + - application/json +paths: {} +definitions: + Request: + description: Request fields with id + allOf: + - "$ref": "#/definitions/RequestFields" + - type: object + properties: + id: + type: string + RequestFields: + description: Request fields + type: object + properties: + state: + type: integer From 08f99328ed7b07c38c0f0dd51ba3144b8c542afe Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Fri, 12 Apr 2019 15:48:36 +0200 Subject: [PATCH 02/20] It finally compiles! Except for java :( --- .../twilio/guardrail/generators/CirceProtocolGenerator.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 68486594b2..54fa69b88c 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -373,7 +373,7 @@ object CirceProtocolGenerator { case head :: tail => definitions .collectFirst({ - case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) => + case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) && Option(e.getDiscriminator).isDefined => (clsName, e, tail.toList) :: allParents(e) }) .getOrElse(List.empty) From 28b1b6ba3449772de88cc5fd98c5fa4c7a5f076b Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Thu, 25 Apr 2019 14:30:38 +0200 Subject: [PATCH 03/20] Added test case for issue 222 --- .../generators/CirceProtocolGenerator.scala | 2 +- .../src/test/scala/core/issues/Issue222.scala | 90 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 modules/codegen/src/test/scala/core/issues/Issue222.scala diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 54fa69b88c..68486594b2 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -373,7 +373,7 @@ object CirceProtocolGenerator { case head :: tail => definitions .collectFirst({ - case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) && Option(e.getDiscriminator).isDefined => + case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) => (clsName, e, tail.toList) :: allParents(e) }) .getOrElse(List.empty) diff --git a/modules/codegen/src/test/scala/core/issues/Issue222.scala b/modules/codegen/src/test/scala/core/issues/Issue222.scala new file mode 100644 index 0000000000..1a3a8a85d9 --- /dev/null +++ b/modules/codegen/src/test/scala/core/issues/Issue222.scala @@ -0,0 +1,90 @@ +package core.issues + +import com.twilio.guardrail.generators.Http4s +import com.twilio.guardrail.languages.ScalaLanguage +import com.twilio.guardrail.{ClassDefinition, Context, ProtocolDefinitions} +import org.scalatest.{Assertion, FunSuite, Matchers} +import support.SwaggerSpecRunner + +class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { + import scala.meta._ + + val swagger: String = s""" + |swagger: '2.0' + |info: + | title: someapp + | description: someapp + | version: '1' + |basePath: "/v1" + |schemes: + | - http + |produces: + | - application/json + |paths: {} + |definitions: + | Request: + | description: Request fields with id + | allOf: + | - "$$ref": "#/definitions/RequestFields" + | - type: object + | properties: + | id: + | type: string + | RequestFields: + | description: Request fields + | type: object + | properties: + | state: + | type: integer + |""".stripMargin + + test("Ensure routes are generated for OPTIONS method") { + val (ProtocolDefinitions(List(request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage]), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) + + val List(reqEncoder, reqDecoder) = request.staticDefns.definitions + + val expectedRequestTpe = t"""Request""" + + val expectedRequestCls = q"""case class Request(state: Option[BigInt] = None, id: Option[String] = None)""" + + val expectedRequestEncoder = q""" + implicit val encodeRequest = { + val readOnlyKeys = Set[String]() + Encoder.forProduct2("id")((o: Request) => o.id).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + } + """ + val expectedRequestDecoder = q""" + implicit val decodeRequest = Decoder.forProduct1("id")(Request.apply _) + """ + + + compare(request.tpe, expectedRequestTpe) + compare(request.cls, expectedRequestCls) + compare(reqEncoder, expectedRequestEncoder) + compare(reqDecoder, expectedRequestDecoder) + + val expectedFieldsTpe = t"""RequestFields""" + val expectedFieldsCls = q"""case class RequestFields(state: Option[BigInt] = None)""" + + val List(fieldsEncoder, fieldsDecoder) = requestFields.staticDefns.definitions + + val expectedFieldsEncoder = q""" + implicit val encodeRequestFields = { + val readOnlyKeys = Set[String]() + Encoder.forProduct1("state")((o: RequestFields) => o.state).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + } + """ + val expectedFieldsDecoder = q""" + implicit val decodeRequestFields = Decoder.forProduct1("state")(RequestFields.apply _) + """ + + compare(requestFields.tpe, expectedFieldsTpe) + compare(requestFields.cls, expectedFieldsCls) + compare(fieldsEncoder, expectedFieldsEncoder) + compare(fieldsDecoder, expectedFieldsDecoder) + } + + private def compare(t1: Tree, t2: Tree): Assertion = { + t1.structure shouldEqual t2.structure + } +} From 184a5b9a4866b72a02f371fdfef10d5dc9615d71 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Thu, 25 Apr 2019 16:04:57 +0200 Subject: [PATCH 04/20] Test passes --- .../scala/com/twilio/guardrail/ProtocolGenerator.scala | 10 ++++++++-- .../guardrail/generators/CirceProtocolGenerator.scala | 5 +++-- .../protocol/terms/protocol/ModelProtocolTerm.scala | 2 +- .../protocol/terms/protocol/ModelProtocolTerms.scala | 4 ++-- .../codegen/src/test/scala/core/issues/Issue222.scala | 7 ++++--- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala index cefd26be96..ab7b4048c8 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala @@ -246,7 +246,7 @@ object ProtocolGenerator { val isRequired = requiredFields.contains(name) SwaggerUtil.propMeta[L, F](prop).flatMap(transformProperty(clsName, needCamelSnakeConversion, concreteTypes)(name, prop, _, isRequired)) }) - defn <- renderDTOClass(clsName, params, parents) + defn <- renderDTOClass(clsName, params, parents, parents.nonEmpty && Option(model.getDiscriminator).isDefined) //check for discriminator, see issue222 deps = params.flatMap(_.dep) encoder <- encodeModel(clsName, needCamelSnakeConversion, params, parents) decoder <- decodeModel(clsName, needCamelSnakeConversion, params, parents) @@ -361,6 +361,7 @@ object ProtocolGenerator { definitions.partitionEither({ case (cls, model) => + // mark ClassParents iff present and has children, otherwise left as model without hierarchies classHierarchy(cls, model).filterNot(_.children.isEmpty).toLeft((cls, model)) }) } @@ -399,7 +400,12 @@ object ProtocolGenerator { parents <- extractParents(comp, definitions, concreteTypes) model <- fromModel(clsName, comp, parents, concreteTypes) alias <- modelTypeAlias(clsName, comp) - } yield model.getOrElse(alias) + } yield { + parents + model + alias + model.getOrElse(alias) + } case arr: ArraySchema => fromArray(clsName, arr, concreteTypes) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 68486594b2..5ef51cb212 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -169,13 +169,14 @@ object CirceProtocolGenerator { dep = classDep.filterNot(_.value == clsName) // Filter out our own class name } yield ProtocolParameter[ScalaLanguage](term, name, dep, readOnlyKey, emptyToNull, finalDefaultValue) - case RenderDTOClass(clsName, selfParams, parents) => + case RenderDTOClass(clsName, selfParams, parents, renderPoly) => val discriminators = parents.flatMap(_.discriminators) val parenOpt = parents.headOption val terms = (parents.reverse.flatMap(_.params.map(_.term)) ++ selfParams.map(_.term)).filterNot( param => discriminators.contains(param.name.value) ) - val code = parenOpt + + val code = parenOpt.filter(_ => renderPoly) .fold(q"""case class ${Type.Name(clsName)}(..${terms})""")( parent => q"""case class ${Type.Name(clsName)}(..${terms}) extends ${template"..${init"${Type.Name(parent.clsName)}(...$Nil)" :: parent.interfaces diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala index 0b5f74bbaf..0f9d753083 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala @@ -16,7 +16,7 @@ case class TransformProperty[L <: LA](clsName: String, concreteTypes: List[PropMeta[L]], isRequired: Boolean) extends ModelProtocolTerm[L, ProtocolParameter[L]] -case class RenderDTOClass[L <: LA](clsName: String, params: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil) +case class RenderDTOClass[L <: LA](clsName: String, params: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil, renderPoly: Boolean) extends ModelProtocolTerm[L, L#ClassDefinition] case class EncodeModel[L <: LA](clsName: String, needCamelSnakeConversion: Boolean, params: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil) extends ModelProtocolTerm[L, Option[L#ValueDefinition]] diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala index 3a9e310ac2..6675b22748 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala @@ -17,8 +17,8 @@ class ModelProtocolTerms[L <: LA, F[_]](implicit I: InjectK[ModelProtocolTerm[L, isRequired: Boolean ): Free[F, ProtocolParameter[L]] = Free.inject[ModelProtocolTerm[L, ?], F](TransformProperty[L](clsName, name, prop, meta, needCamelSnakeConversion, concreteTypes, isRequired)) - def renderDTOClass(clsName: String, terms: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil): Free[F, L#ClassDefinition] = - Free.inject[ModelProtocolTerm[L, ?], F](RenderDTOClass[L](clsName, terms, parents)) + def renderDTOClass(clsName: String, terms: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil, renderPoly: Boolean): Free[F, L#ClassDefinition] = + Free.inject[ModelProtocolTerm[L, ?], F](RenderDTOClass[L](clsName, terms, parents, renderPoly)) def encodeModel(clsName: String, needCamelSnakeConversion: Boolean, params: List[ProtocolParameter[L]], diff --git a/modules/codegen/src/test/scala/core/issues/Issue222.scala b/modules/codegen/src/test/scala/core/issues/Issue222.scala index 1a3a8a85d9..363ddeab6f 100644 --- a/modules/codegen/src/test/scala/core/issues/Issue222.scala +++ b/modules/codegen/src/test/scala/core/issues/Issue222.scala @@ -38,7 +38,7 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { | type: integer |""".stripMargin - test("Ensure routes are generated for OPTIONS method") { + test("Ensure case-to-case inheritance is not generated") { val (ProtocolDefinitions(List(request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage]), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) val List(reqEncoder, reqDecoder) = request.staticDefns.definitions @@ -50,11 +50,11 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { val expectedRequestEncoder = q""" implicit val encodeRequest = { val readOnlyKeys = Set[String]() - Encoder.forProduct2("id")((o: Request) => o.id).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + Encoder.forProduct2("state", "id")((o: Request) => (o.state, o.id)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ val expectedRequestDecoder = q""" - implicit val decodeRequest = Decoder.forProduct1("id")(Request.apply _) + implicit val decodeRequest = Decoder.forProduct2("state", "id")(Request.apply _) """ @@ -85,6 +85,7 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { } private def compare(t1: Tree, t2: Tree): Assertion = { + println(s"$t1 | $t2") t1.structure shouldEqual t2.structure } } From 61d15f2375b911a406f156dba9f8516ec404e803 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Thu, 25 Apr 2019 16:26:39 +0200 Subject: [PATCH 05/20] Reimplemented --- .../scala/com/twilio/guardrail/ProtocolGenerator.scala | 2 +- .../guardrail/generators/CirceProtocolGenerator.scala | 8 ++++---- .../protocol/terms/protocol/ModelProtocolTerm.scala | 2 +- .../protocol/terms/protocol/ModelProtocolTerms.scala | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala index ab7b4048c8..a4fa7a4001 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala @@ -246,7 +246,7 @@ object ProtocolGenerator { val isRequired = requiredFields.contains(name) SwaggerUtil.propMeta[L, F](prop).flatMap(transformProperty(clsName, needCamelSnakeConversion, concreteTypes)(name, prop, _, isRequired)) }) - defn <- renderDTOClass(clsName, params, parents, parents.nonEmpty && Option(model.getDiscriminator).isDefined) //check for discriminator, see issue222 + defn <- renderDTOClass(clsName, params, parents) //check for discriminator, see issue222 deps = params.flatMap(_.dep) encoder <- encodeModel(clsName, needCamelSnakeConversion, params, parents) decoder <- decodeModel(clsName, needCamelSnakeConversion, params, parents) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 5ef51cb212..5f2d4abec4 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -169,14 +169,14 @@ object CirceProtocolGenerator { dep = classDep.filterNot(_.value == clsName) // Filter out our own class name } yield ProtocolParameter[ScalaLanguage](term, name, dep, readOnlyKey, emptyToNull, finalDefaultValue) - case RenderDTOClass(clsName, selfParams, parents, renderPoly) => + case RenderDTOClass(clsName, selfParams, parents) => val discriminators = parents.flatMap(_.discriminators) - val parenOpt = parents.headOption + val parentOpt = parents.headOption val terms = (parents.reverse.flatMap(_.params.map(_.term)) ++ selfParams.map(_.term)).filterNot( param => discriminators.contains(param.name.value) ) - - val code = parenOpt.filter(_ => renderPoly) + val code = parentOpt + .filter(p => p.discriminators.nonEmpty) .fold(q"""case class ${Type.Name(clsName)}(..${terms})""")( parent => q"""case class ${Type.Name(clsName)}(..${terms}) extends ${template"..${init"${Type.Name(parent.clsName)}(...$Nil)" :: parent.interfaces diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala index 0f9d753083..0b5f74bbaf 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerm.scala @@ -16,7 +16,7 @@ case class TransformProperty[L <: LA](clsName: String, concreteTypes: List[PropMeta[L]], isRequired: Boolean) extends ModelProtocolTerm[L, ProtocolParameter[L]] -case class RenderDTOClass[L <: LA](clsName: String, params: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil, renderPoly: Boolean) +case class RenderDTOClass[L <: LA](clsName: String, params: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil) extends ModelProtocolTerm[L, L#ClassDefinition] case class EncodeModel[L <: LA](clsName: String, needCamelSnakeConversion: Boolean, params: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil) extends ModelProtocolTerm[L, Option[L#ValueDefinition]] diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala index 6675b22748..3a9e310ac2 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/protocol/terms/protocol/ModelProtocolTerms.scala @@ -17,8 +17,8 @@ class ModelProtocolTerms[L <: LA, F[_]](implicit I: InjectK[ModelProtocolTerm[L, isRequired: Boolean ): Free[F, ProtocolParameter[L]] = Free.inject[ModelProtocolTerm[L, ?], F](TransformProperty[L](clsName, name, prop, meta, needCamelSnakeConversion, concreteTypes, isRequired)) - def renderDTOClass(clsName: String, terms: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil, renderPoly: Boolean): Free[F, L#ClassDefinition] = - Free.inject[ModelProtocolTerm[L, ?], F](RenderDTOClass[L](clsName, terms, parents, renderPoly)) + def renderDTOClass(clsName: String, terms: List[ProtocolParameter[L]], parents: List[SuperClass[L]] = Nil): Free[F, L#ClassDefinition] = + Free.inject[ModelProtocolTerm[L, ?], F](RenderDTOClass[L](clsName, terms, parents)) def encodeModel(clsName: String, needCamelSnakeConversion: Boolean, params: List[ProtocolParameter[L]], From ee3992114e01874377b0449dbb3a6e9a6b862e2d Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Thu, 25 Apr 2019 17:38:12 +0200 Subject: [PATCH 06/20] =?UTF-8?q?Attempt=20to=20fix=20jackson=CB=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../guardrail/generators/CirceProtocolGenerator.scala | 2 +- .../guardrail/generators/Java/JacksonGenerator.scala | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 5f2d4abec4..bf3792fe75 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -176,7 +176,7 @@ object CirceProtocolGenerator { param => discriminators.contains(param.name.value) ) val code = parentOpt - .filter(p => p.discriminators.nonEmpty) + .filter(p => p.discriminators.nonEmpty) // part of issue222 .fold(q"""case class ${Type.Name(clsName)}(..${terms})""")( parent => q"""case class ${Type.Name(clsName)}(..${terms}) extends ${template"..${init"${Type.Name(parent.clsName)}(...$Nil)" :: parent.interfaces diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala index d13da7294b..b8659bd261 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala @@ -56,9 +56,11 @@ object JacksonGenerator { parentOpt.foreach({ parent => val directParent = JavaParser.parseClassOrInterfaceType(parent.clsName) val otherParents = parent.interfaces.map(JavaParser.parseClassOrInterfaceType) - cls.setExtendedTypes( - new NodeList((directParent +: otherParents): _*) - ) + if (parent.discriminators.nonEmpty) { + cls.setExtendedTypes( + new NodeList((directParent +: otherParents): _*) + ) + } }) private def lookupTypeName(tpeName: String, concreteTypes: List[PropMeta[JavaLanguage]])(f: Type => Target[Type]): Option[Target[Type]] = From 7a9b6daf640fe592ac9fde2c5b99c6c1621d82de Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Thu, 25 Apr 2019 17:43:50 +0200 Subject: [PATCH 07/20] Removed unused values --- .../main/scala/com/twilio/guardrail/ProtocolGenerator.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala index a4fa7a4001..2060b9c961 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala @@ -401,9 +401,6 @@ object ProtocolGenerator { model <- fromModel(clsName, comp, parents, concreteTypes) alias <- modelTypeAlias(clsName, comp) } yield { - parents - model - alias model.getOrElse(alias) } From e525c60576eaadc17d3fa2848b23b7b3f2039432 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Thu, 25 Apr 2019 18:08:19 +0200 Subject: [PATCH 08/20] Scan all parents for discriminators --- .../twilio/guardrail/generators/CirceProtocolGenerator.scala | 3 ++- modules/codegen/src/test/scala/core/issues/Issue43.scala | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index bf3792fe75..1b961ce318 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -175,8 +175,9 @@ object CirceProtocolGenerator { val terms = (parents.reverse.flatMap(_.params.map(_.term)) ++ selfParams.map(_.term)).filterNot( param => discriminators.contains(param.name.value) ) + val code = parentOpt - .filter(p => p.discriminators.nonEmpty) // part of issue222 + .filter(p => parents.exists(s => s.discriminators.nonEmpty)) // part of issue222 .fold(q"""case class ${Type.Name(clsName)}(..${terms})""")( parent => q"""case class ${Type.Name(clsName)}(..${terms}) extends ${template"..${init"${Type.Name(parent.clsName)}(...$Nil)" :: parent.interfaces diff --git a/modules/codegen/src/test/scala/core/issues/Issue43.scala b/modules/codegen/src/test/scala/core/issues/Issue43.scala index 0a884e005b..69660c997b 100644 --- a/modules/codegen/src/test/scala/core/issues/Issue43.scala +++ b/modules/codegen/src/test/scala/core/issues/Issue43.scala @@ -444,7 +444,7 @@ class Issue43 extends FunSpec with Matchers with SwaggerSpecRunner { :: ADT(namePet, tpePet, trtPet, staticDefnsPet) :: ADT(nameCat, tpeCat, trtCat, staticDefnsCat) :: Nil, _, _, - _ + _3 ), _, _ From db374e49f28fb97c045a5214dfa24b29af1a49b9 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Fri, 26 Apr 2019 09:56:03 +0200 Subject: [PATCH 09/20] Cleanup --- .../main/scala/com/twilio/guardrail/ProtocolGenerator.scala | 6 ++---- modules/codegen/src/test/scala/core/issues/Issue222.scala | 1 - modules/codegen/src/test/scala/core/issues/Issue43.scala | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala index 2060b9c961..c3428ae43e 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala @@ -246,7 +246,7 @@ object ProtocolGenerator { val isRequired = requiredFields.contains(name) SwaggerUtil.propMeta[L, F](prop).flatMap(transformProperty(clsName, needCamelSnakeConversion, concreteTypes)(name, prop, _, isRequired)) }) - defn <- renderDTOClass(clsName, params, parents) //check for discriminator, see issue222 + defn <- renderDTOClass(clsName, params, parents) deps = params.flatMap(_.dep) encoder <- encodeModel(clsName, needCamelSnakeConversion, params, parents) decoder <- decodeModel(clsName, needCamelSnakeConversion, params, parents) @@ -400,9 +400,7 @@ object ProtocolGenerator { parents <- extractParents(comp, definitions, concreteTypes) model <- fromModel(clsName, comp, parents, concreteTypes) alias <- modelTypeAlias(clsName, comp) - } yield { - model.getOrElse(alias) - } + } yield model.getOrElse(alias) case arr: ArraySchema => fromArray(clsName, arr, concreteTypes) diff --git a/modules/codegen/src/test/scala/core/issues/Issue222.scala b/modules/codegen/src/test/scala/core/issues/Issue222.scala index 363ddeab6f..e1d8651443 100644 --- a/modules/codegen/src/test/scala/core/issues/Issue222.scala +++ b/modules/codegen/src/test/scala/core/issues/Issue222.scala @@ -85,7 +85,6 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { } private def compare(t1: Tree, t2: Tree): Assertion = { - println(s"$t1 | $t2") t1.structure shouldEqual t2.structure } } diff --git a/modules/codegen/src/test/scala/core/issues/Issue43.scala b/modules/codegen/src/test/scala/core/issues/Issue43.scala index 69660c997b..0a884e005b 100644 --- a/modules/codegen/src/test/scala/core/issues/Issue43.scala +++ b/modules/codegen/src/test/scala/core/issues/Issue43.scala @@ -444,7 +444,7 @@ class Issue43 extends FunSpec with Matchers with SwaggerSpecRunner { :: ADT(namePet, tpePet, trtPet, staticDefnsPet) :: ADT(nameCat, tpeCat, trtCat, staticDefnsCat) :: Nil, _, _, - _3 + _ ), _, _ From 89cc0c4c2c6abd747e75ae5e53955f2de09c7ff2 Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Fri, 26 Apr 2019 10:11:22 +0200 Subject: [PATCH 10/20] Update modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala Co-Authored-By: tomasherman --- .../twilio/guardrail/generators/CirceProtocolGenerator.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 1b961ce318..3848596b61 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -171,7 +171,7 @@ object CirceProtocolGenerator { case RenderDTOClass(clsName, selfParams, parents) => val discriminators = parents.flatMap(_.discriminators) - val parentOpt = parents.headOption + val parentOpt = if (parents.exists(s => s.discriminators.nonEmpty)) { parents.headOption } else { None } val terms = (parents.reverse.flatMap(_.params.map(_.term)) ++ selfParams.map(_.term)).filterNot( param => discriminators.contains(param.name.value) ) From d2e7b965d3c98728ca75223bfcdcd77b40941336 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Fri, 26 Apr 2019 10:20:35 +0200 Subject: [PATCH 11/20] Code review improvements #2 --- .../generators/CirceProtocolGenerator.scala | 1 - .../guardrail/generators/Java/JacksonGenerator.scala | 12 +++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 3848596b61..61831644a5 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -177,7 +177,6 @@ object CirceProtocolGenerator { ) val code = parentOpt - .filter(p => parents.exists(s => s.discriminators.nonEmpty)) // part of issue222 .fold(q"""case class ${Type.Name(clsName)}(..${terms})""")( parent => q"""case class ${Type.Name(clsName)}(..${terms}) extends ${template"..${init"${Type.Name(parent.clsName)}(...$Nil)" :: parent.interfaces diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala index b8659bd261..9daab5b014 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala @@ -56,11 +56,9 @@ object JacksonGenerator { parentOpt.foreach({ parent => val directParent = JavaParser.parseClassOrInterfaceType(parent.clsName) val otherParents = parent.interfaces.map(JavaParser.parseClassOrInterfaceType) - if (parent.discriminators.nonEmpty) { - cls.setExtendedTypes( - new NodeList((directParent +: otherParents): _*) - ) - } + cls.setExtendedTypes( + new NodeList((directParent +: otherParents): _*) + ) }) private def lookupTypeName(tpeName: String, concreteTypes: List[PropMeta[JavaLanguage]])(f: Type => Target[Type]): Option[Target[Type]] = @@ -324,7 +322,7 @@ object JacksonGenerator { case RenderDTOClass(clsName, selfParams, parents) => val dtoClassType = JavaParser.parseClassOrInterfaceType(clsName) val discriminators = parents.flatMap(_.discriminators) - val parentOpt = parents.headOption + val parentOpt = if (parents.exists(s => s.discriminators.nonEmpty)) { parents.headOption } else { None } val params = (parents.reverse.flatMap(_.params) ++ selfParams).filterNot( param => discriminators.contains(param.term.getName().getId()) ) @@ -741,7 +739,7 @@ object JacksonGenerator { Target.pure(None) case RenderSealedTrait(className, selfParams, discriminator, parents, children) => - val parentOpt = parents.headOption + val parentOpt = if (parents.exists(s => s.discriminators.nonEmpty)) { parents.headOption } else { None } val params = (parents.reverse.flatMap(_.params) ++ selfParams).filterNot(_.term.getName.getId == discriminator) val (requiredTerms, optionalTerms) = sortParams(params) val terms = requiredTerms ++ optionalTerms From 8c636a8ce0da4fb9b989e5252c2c08dad1beb4ff Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Sat, 27 Apr 2019 19:04:58 +0200 Subject: [PATCH 12/20] Removed comment --- .../src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala index c3428ae43e..cefd26be96 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/ProtocolGenerator.scala @@ -361,7 +361,6 @@ object ProtocolGenerator { definitions.partitionEither({ case (cls, model) => - // mark ClassParents iff present and has children, otherwise left as model without hierarchies classHierarchy(cls, model).filterNot(_.children.isEmpty).toLeft((cls, model)) }) } From e97efbd4b5986f30ae77ed970a92bf0d3def446c Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Wed, 1 May 2019 13:36:58 +0200 Subject: [PATCH 13/20] Added another test case and implementation --- .../generators/CirceProtocolGenerator.scala | 6 +- .../src/test/scala/core/issues/Issue222.scala | 92 ++++++++++++++++--- 2 files changed, 85 insertions(+), 13 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 61831644a5..699ce543b0 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -104,9 +104,11 @@ object CirceProtocolGenerator { (swagger match { case m: ObjectSchema => Target.pure(Option(m.getProperties)) case comp: ComposedSchema => - Target.pure(Option(comp.getAllOf()).toList.flatMap(_.asScala.toList).lastOption.flatMap(prop => Option(prop.getProperties))) + val extractedProps = Option(comp.getAllOf()).toList.flatMap(_.asScala.toList).map(e => Option(e.getProperties).map(_.asScala.toMap)).collect { case Some(e) => e } + val mergedProps = extractedProps.fold(Map.empty)(_ ++ _) + Target.pure(Option(mergedProps.asJava)) case comp: Schema[_] if Option(comp.get$ref).isDefined => - Target.error(s"Attempted to extractProperties for a ${comp.getClass()}, unsure what to do here") + Target.raiseError(s"Attempted to extractProperties for a ${comp.getClass()}, unsure what to do here") case _ => Target.pure(None) }).map(_.map(_.asScala.toList).toList.flatten) diff --git a/modules/codegen/src/test/scala/core/issues/Issue222.scala b/modules/codegen/src/test/scala/core/issues/Issue222.scala index e1d8651443..644e9b9725 100644 --- a/modules/codegen/src/test/scala/core/issues/Issue222.scala +++ b/modules/codegen/src/test/scala/core/issues/Issue222.scala @@ -36,24 +36,43 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { | properties: | state: | type: integer + | Request2: + | description: Request fields with id + | allOf: + | - "$$ref": "#/definitions/RequestFields" + | - type: object + | properties: + | id: + | type: string + | - type: object + | properties: + | id2: + | type: string + | RequestFields2: + | description: Request fields + | type: object + | properties: + | state: + | type: integer + | |""".stripMargin test("Ensure case-to-case inheritance is not generated") { - val (ProtocolDefinitions(List(request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage]), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) - + val (ProtocolDefinitions(List(request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage], _, _), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) + val List(reqEncoder, reqDecoder) = request.staticDefns.definitions val expectedRequestTpe = t"""Request""" - + val expectedRequestCls = q"""case class Request(state: Option[BigInt] = None, id: Option[String] = None)""" - + val expectedRequestEncoder = q""" implicit val encodeRequest = { val readOnlyKeys = Set[String]() Encoder.forProduct2("state", "id")((o: Request) => (o.state, o.id)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ - val expectedRequestDecoder = q""" + val expectedRequestDecoder = q""" implicit val decodeRequest = Decoder.forProduct2("state", "id")(Request.apply _) """ @@ -62,10 +81,10 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { compare(request.cls, expectedRequestCls) compare(reqEncoder, expectedRequestEncoder) compare(reqDecoder, expectedRequestDecoder) - + val expectedFieldsTpe = t"""RequestFields""" val expectedFieldsCls = q"""case class RequestFields(state: Option[BigInt] = None)""" - + val List(fieldsEncoder, fieldsDecoder) = requestFields.staticDefns.definitions val expectedFieldsEncoder = q""" @@ -74,17 +93,68 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { Encoder.forProduct1("state")((o: RequestFields) => o.state).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ - val expectedFieldsDecoder = q""" + val expectedFieldsDecoder = q""" implicit val decodeRequestFields = Decoder.forProduct1("state")(RequestFields.apply _) """ compare(requestFields.tpe, expectedFieldsTpe) compare(requestFields.cls, expectedFieldsCls) compare(fieldsEncoder, expectedFieldsEncoder) - compare(fieldsDecoder, expectedFieldsDecoder) + compare(fieldsDecoder, expectedFieldsDecoder) } - + + test("Ensure case-to-case inheritance is not generated, more complicated scenario") { + val (ProtocolDefinitions(List(_, _,request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage]), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) + + val List(reqEncoder, reqDecoder) = request.staticDefns.definitions + + val expectedRequestTpe = t"""Request2""" + + val expectedRequestCls = q"""case class Request2(state: Option[BigInt] = None, id: Option[String] = None, id2: Option[String] = None)""" + + val expectedRequestEncoder = q""" + implicit val encodeRequest2 = { + val readOnlyKeys = Set[String]() + Encoder.forProduct3("state", "id", "id2")((o: Request2) => (o.state, o.id, o.id2)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + } + """ + val expectedRequestDecoder = q""" + implicit val decodeRequest2 = Decoder.forProduct3("state", "id", "id2")(Request2.apply _) + """ + + + compare(request.tpe, expectedRequestTpe) + compare(request.cls, expectedRequestCls) + compare(reqEncoder, expectedRequestEncoder) + compare(reqDecoder, expectedRequestDecoder) + + val expectedFieldsTpe = t"""RequestFields2""" + val expectedFieldsCls = q"""case class RequestFields2(state: Option[BigInt] = None)""" + + val List(fieldsEncoder, fieldsDecoder) = requestFields.staticDefns.definitions + + val expectedFieldsEncoder = q""" + implicit val encodeRequestFields2 = { + val readOnlyKeys = Set[String]() + Encoder.forProduct1("state")((o: RequestFields2) => o.state).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + } + """ + val expectedFieldsDecoder = q""" + implicit val decodeRequestFields2 = Decoder.forProduct1("state")(RequestFields2.apply _) + """ + + compare(requestFields.tpe, expectedFieldsTpe) + compare(requestFields.cls, expectedFieldsCls) + compare(fieldsEncoder, expectedFieldsEncoder) + compare(fieldsDecoder, expectedFieldsDecoder) + } + + + + private def compare(t1: Tree, t2: Tree): Assertion = { + println(s"expected: ${t1.syntax}") + println(s"actual: ${t2.syntax}") t1.structure shouldEqual t2.structure } -} +} \ No newline at end of file From 0d54cab035a8b07a716b2f1f91f123fa8879e61c Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Wed, 1 May 2019 13:41:56 +0200 Subject: [PATCH 14/20] Improved 2nd test case scenario --- .../src/test/scala/core/issues/Issue222.scala | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/modules/codegen/src/test/scala/core/issues/Issue222.scala b/modules/codegen/src/test/scala/core/issues/Issue222.scala index 644e9b9725..224e94f210 100644 --- a/modules/codegen/src/test/scala/core/issues/Issue222.scala +++ b/modules/codegen/src/test/scala/core/issues/Issue222.scala @@ -39,7 +39,7 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { | Request2: | description: Request fields with id | allOf: - | - "$$ref": "#/definitions/RequestFields" + | - "$$ref": "#/definitions/RequestFields2" | - type: object | properties: | id: @@ -52,7 +52,7 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { | description: Request fields | type: object | properties: - | state: + | state2: | type: integer | |""".stripMargin @@ -110,16 +110,16 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { val expectedRequestTpe = t"""Request2""" - val expectedRequestCls = q"""case class Request2(state: Option[BigInt] = None, id: Option[String] = None, id2: Option[String] = None)""" + val expectedRequestCls = q"""case class Request2(state2: Option[BigInt] = None, id: Option[String] = None, id2: Option[String] = None)""" val expectedRequestEncoder = q""" implicit val encodeRequest2 = { val readOnlyKeys = Set[String]() - Encoder.forProduct3("state", "id", "id2")((o: Request2) => (o.state, o.id, o.id2)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + Encoder.forProduct3("state2", "id", "id2")((o: Request2) => (o.state2, o.id, o.id2)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ val expectedRequestDecoder = q""" - implicit val decodeRequest2 = Decoder.forProduct3("state", "id", "id2")(Request2.apply _) + implicit val decodeRequest2 = Decoder.forProduct3("state2", "id", "id2")(Request2.apply _) """ @@ -129,18 +129,18 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { compare(reqDecoder, expectedRequestDecoder) val expectedFieldsTpe = t"""RequestFields2""" - val expectedFieldsCls = q"""case class RequestFields2(state: Option[BigInt] = None)""" + val expectedFieldsCls = q"""case class RequestFields2(state2: Option[BigInt] = None)""" val List(fieldsEncoder, fieldsDecoder) = requestFields.staticDefns.definitions val expectedFieldsEncoder = q""" implicit val encodeRequestFields2 = { val readOnlyKeys = Set[String]() - Encoder.forProduct1("state")((o: RequestFields2) => o.state).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + Encoder.forProduct1("state2")((o: RequestFields2) => o.state2).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ val expectedFieldsDecoder = q""" - implicit val decodeRequestFields2 = Decoder.forProduct1("state")(RequestFields2.apply _) + implicit val decodeRequestFields2 = Decoder.forProduct1("state2")(RequestFields2.apply _) """ compare(requestFields.tpe, expectedFieldsTpe) @@ -153,8 +153,6 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { private def compare(t1: Tree, t2: Tree): Assertion = { - println(s"expected: ${t1.syntax}") - println(s"actual: ${t2.syntax}") t1.structure shouldEqual t2.structure } } \ No newline at end of file From 8aec398c14ae51dfd188d9c93570f16c3b287e92 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Wed, 1 May 2019 14:47:40 +0200 Subject: [PATCH 15/20] Added another test case --- .../generators/CirceProtocolGenerator.scala | 2 +- .../src/test/scala/core/issues/Issue222.scala | 174 ++++++++++++------ 2 files changed, 114 insertions(+), 62 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 699ce543b0..4ba66db561 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -377,7 +377,7 @@ object CirceProtocolGenerator { definitions .collectFirst({ case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) => - (clsName, e, tail.toList) :: allParents(e) + (clsName, e, tail) :: allParents(e) }) .getOrElse(List.empty) case _ => List.empty diff --git a/modules/codegen/src/test/scala/core/issues/Issue222.scala b/modules/codegen/src/test/scala/core/issues/Issue222.scala index 224e94f210..185ae2a89c 100644 --- a/modules/codegen/src/test/scala/core/issues/Issue222.scala +++ b/modules/codegen/src/test/scala/core/issues/Issue222.scala @@ -7,72 +7,91 @@ import org.scalatest.{Assertion, FunSuite, Matchers} import support.SwaggerSpecRunner class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { + import scala.meta._ - val swagger: String = s""" - |swagger: '2.0' - |info: - | title: someapp - | description: someapp - | version: '1' - |basePath: "/v1" - |schemes: - | - http - |produces: - | - application/json - |paths: {} - |definitions: - | Request: - | description: Request fields with id - | allOf: - | - "$$ref": "#/definitions/RequestFields" - | - type: object - | properties: - | id: - | type: string - | RequestFields: - | description: Request fields - | type: object - | properties: - | state: - | type: integer - | Request2: - | description: Request fields with id - | allOf: - | - "$$ref": "#/definitions/RequestFields2" - | - type: object - | properties: - | id: - | type: string - | - type: object - | properties: - | id2: - | type: string - | RequestFields2: - | description: Request fields - | type: object - | properties: - | state2: - | type: integer - | - |""".stripMargin + val swagger: String = + s""" + |swagger: '2.0' + |info: + | title: someapp + | description: someapp + | version: '1' + |basePath: "/v1" + |schemes: + | - http + |produces: + | - application/json + |paths: {} + |definitions: + | Request: + | description: Request fields with id + | allOf: + | - "$$ref": "#/definitions/RequestFields" + | - type: object + | properties: + | id: + | type: string + | RequestFields: + | description: Request fields + | type: object + | properties: + | state: + | type: integer + | required: [state] + | Request2: + | description: Request fields with id + | allOf: + | - "$$ref": "#/definitions/RequestFields2" + | - type: object + | properties: + | id: + | type: string + | - type: object + | properties: + | id2: + | type: string + | required: [id2] + | RequestFields2: + | description: Request fields + | type: object + | properties: + | state2: + | type: integer + | Request3: + | description: Request fields with id + | allOf: + | - "$$ref": "#/definitions/RequestFields" + | - type: object + | properties: + | id: + | type: string + | - type: object + | properties: + | id2: + | type: string + | required: [id2] + | - "$$ref": "#/definitions/RequestFields2" + |""".stripMargin test("Ensure case-to-case inheritance is not generated") { - val (ProtocolDefinitions(List(request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage], _, _), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) + val (ProtocolDefinitions(List(request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage], _, _, _), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) val List(reqEncoder, reqDecoder) = request.staticDefns.definitions val expectedRequestTpe = t"""Request""" - val expectedRequestCls = q"""case class Request(state: Option[BigInt] = None, id: Option[String] = None)""" + val expectedRequestCls = q"""case class Request(state: BigInt, id: Option[String] = None)""" - val expectedRequestEncoder = q""" + val expectedRequestEncoder = + q""" implicit val encodeRequest = { val readOnlyKeys = Set[String]() Encoder.forProduct2("state", "id")((o: Request) => (o.state, o.id)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ - val expectedRequestDecoder = q""" + val expectedRequestDecoder = + q""" implicit val decodeRequest = Decoder.forProduct2("state", "id")(Request.apply _) """ @@ -83,17 +102,19 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { compare(reqDecoder, expectedRequestDecoder) val expectedFieldsTpe = t"""RequestFields""" - val expectedFieldsCls = q"""case class RequestFields(state: Option[BigInt] = None)""" + val expectedFieldsCls = q"""case class RequestFields(state: BigInt)""" val List(fieldsEncoder, fieldsDecoder) = requestFields.staticDefns.definitions - val expectedFieldsEncoder = q""" + val expectedFieldsEncoder = + q""" implicit val encodeRequestFields = { val readOnlyKeys = Set[String]() Encoder.forProduct1("state")((o: RequestFields) => o.state).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ - val expectedFieldsDecoder = q""" + val expectedFieldsDecoder = + q""" implicit val decodeRequestFields = Decoder.forProduct1("state")(RequestFields.apply _) """ @@ -103,22 +124,24 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { compare(fieldsDecoder, expectedFieldsDecoder) } - test("Ensure case-to-case inheritance is not generated, more complicated scenario") { - val (ProtocolDefinitions(List(_, _,request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage]), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) + test("Ensure case-to-case inheritance is not generated, extends two objects") { + val (ProtocolDefinitions(List(_, _, request: ClassDefinition[ScalaLanguage], requestFields: ClassDefinition[ScalaLanguage], _), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) val List(reqEncoder, reqDecoder) = request.staticDefns.definitions val expectedRequestTpe = t"""Request2""" - val expectedRequestCls = q"""case class Request2(state2: Option[BigInt] = None, id: Option[String] = None, id2: Option[String] = None)""" + val expectedRequestCls = q"""case class Request2(state2: Option[BigInt] = None, id: Option[String] = None, id2: String)""" - val expectedRequestEncoder = q""" + val expectedRequestEncoder = + q""" implicit val encodeRequest2 = { val readOnlyKeys = Set[String]() Encoder.forProduct3("state2", "id", "id2")((o: Request2) => (o.state2, o.id, o.id2)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ - val expectedRequestDecoder = q""" + val expectedRequestDecoder = + q""" implicit val decodeRequest2 = Decoder.forProduct3("state2", "id", "id2")(Request2.apply _) """ @@ -133,13 +156,15 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { val List(fieldsEncoder, fieldsDecoder) = requestFields.staticDefns.definitions - val expectedFieldsEncoder = q""" + val expectedFieldsEncoder = + q""" implicit val encodeRequestFields2 = { val readOnlyKeys = Set[String]() Encoder.forProduct1("state2")((o: RequestFields2) => o.state2).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) } """ - val expectedFieldsDecoder = q""" + val expectedFieldsDecoder = + q""" implicit val decodeRequestFields2 = Decoder.forProduct1("state2")(RequestFields2.apply _) """ @@ -149,7 +174,34 @@ class Issue222 extends FunSuite with Matchers with SwaggerSpecRunner { compare(fieldsDecoder, expectedFieldsDecoder) } + test("Ensure case-to-case inheritance is not generated, extends two objects and two classes") { + val (ProtocolDefinitions(List(_, _, _, _, request: ClassDefinition[ScalaLanguage]), _, _, _), _, _) = runSwaggerSpec(swagger)(Context.empty, Http4s) + + val List(reqEncoder, reqDecoder) = request.staticDefns.definitions + + val expectedRequestTpe = t"""Request3""" + + val expectedRequestCls = q"""case class Request3(state: BigInt, state2: Option[BigInt] = None, id: Option[String] = None, id2: String)""" + val expectedRequestEncoder = + q""" + implicit val encodeRequest3 = { + val readOnlyKeys = Set[String]() + Encoder.forProduct4("state", "state2", "id", "id2")((o: Request3) => (o.state, o.state2, o.id, o.id2)).mapJsonObject(_.filterKeys(key => !(readOnlyKeys contains key))) + } + """ + val expectedRequestDecoder = + q""" + implicit val decodeRequest3 = Decoder.forProduct4("state", "state2", "id", "id2")(Request3.apply _) + """ + + + compare(request.tpe, expectedRequestTpe) + compare(request.cls, expectedRequestCls) + compare(reqEncoder, expectedRequestEncoder) + compare(reqDecoder, expectedRequestDecoder) + + } private def compare(t1: Tree, t2: Tree): Assertion = { From 83a5cfc895f7edb77a0af764ee17827a681efea2 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Wed, 1 May 2019 15:11:12 +0200 Subject: [PATCH 16/20] Added better error reporting when resolving references --- .../generators/CirceProtocolGenerator.scala | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 4ba66db561..05ac49aedb 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -369,7 +369,7 @@ object CirceProtocolGenerator { object PolyProtocolTermInterp extends (PolyProtocolTerm[ScalaLanguage, ?] ~> Target) { override def apply[A](fa: PolyProtocolTerm[ScalaLanguage, A]): Target[A] = fa match { case ExtractSuperClass(swagger, definitions) => - def allParents(model: Schema[_]): List[(String, Schema[_], List[Schema[_]])] = + def allParents(model: Schema[_]): Target[List[(String, Schema[_], List[Schema[_]])]] = model match { case elem: ComposedSchema => Option(elem.getAllOf).map(_.asScala.toList).getOrElse(List.empty) match { @@ -377,15 +377,19 @@ object CirceProtocolGenerator { definitions .collectFirst({ case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) => - (clsName, e, tail) :: allParents(e) + for { + currentParent <- Target.pure((clsName, e, tail)) + otherParents <- allParents(e) + } yield { + currentParent :: otherParents + } }) - .getOrElse(List.empty) - case _ => List.empty + .getOrElse(Target.raiseError(s"Reference ${head.get$ref()} not found among definitions")) + case _ => Target.pure(List.empty) } - case _ => List.empty + case _ => Target.pure(List.empty) } - - Target.pure(allParents(swagger)) + allParents(swagger) case RenderADTStaticDefns(clsName, discriminator, encoder, decoder) => Target.pure( From bb38c381717b7f5482945cbfaddf8c0403ba0cf0 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Wed, 1 May 2019 15:11:53 +0200 Subject: [PATCH 17/20] Scalafmt --- .../twilio/guardrail/generators/CirceProtocolGenerator.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 05ac49aedb..de429f6f5a 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -104,7 +104,8 @@ object CirceProtocolGenerator { (swagger match { case m: ObjectSchema => Target.pure(Option(m.getProperties)) case comp: ComposedSchema => - val extractedProps = Option(comp.getAllOf()).toList.flatMap(_.asScala.toList).map(e => Option(e.getProperties).map(_.asScala.toMap)).collect { case Some(e) => e } + val extractedProps = + Option(comp.getAllOf()).toList.flatMap(_.asScala.toList).map(e => Option(e.getProperties).map(_.asScala.toMap)).collect { case Some(e) => e } val mergedProps = extractedProps.fold(Map.empty)(_ ++ _) Target.pure(Option(mergedProps.asJava)) case comp: Schema[_] if Option(comp.get$ref).isDefined => @@ -379,7 +380,7 @@ object CirceProtocolGenerator { case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) => for { currentParent <- Target.pure((clsName, e, tail)) - otherParents <- allParents(e) + otherParents <- allParents(e) } yield { currentParent :: otherParents } From cb967795d7a66d0aead05b68c14581661e0a3115 Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Wed, 1 May 2019 16:41:33 +0200 Subject: [PATCH 18/20] Attempt to reduce filename sizes --- .../guardrail/generators/CirceProtocolGenerator.scala | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index de429f6f5a..937235a705 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -378,12 +378,8 @@ object CirceProtocolGenerator { definitions .collectFirst({ case (clsName, e) if Option(head.get$ref).exists(_.endsWith(s"/$clsName")) => - for { - currentParent <- Target.pure((clsName, e, tail)) - otherParents <- allParents(e) - } yield { - currentParent :: otherParents - } + val thisParent = (clsName, e, tail) + allParents(e).map(otherParents => thisParent :: otherParents) }) .getOrElse(Target.raiseError(s"Reference ${head.get$ref()} not found among definitions")) case _ => Target.pure(List.empty) From f81912b1df3df0ff5d5b1c3f0d5a2ec6e2f48dd1 Mon Sep 17 00:00:00 2001 From: Devon Stewart Date: Thu, 2 May 2019 10:29:43 +0200 Subject: [PATCH 19/20] Update modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala Co-Authored-By: tomasherman --- .../twilio/guardrail/generators/CirceProtocolGenerator.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala index 937235a705..2e022e4bb2 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/CirceProtocolGenerator.scala @@ -105,7 +105,7 @@ object CirceProtocolGenerator { case m: ObjectSchema => Target.pure(Option(m.getProperties)) case comp: ComposedSchema => val extractedProps = - Option(comp.getAllOf()).toList.flatMap(_.asScala.toList).map(e => Option(e.getProperties).map(_.asScala.toMap)).collect { case Some(e) => e } + Option(comp.getAllOf()).toList.flatMap(_.asScala.toList).flatMap(e => Option(e.getProperties).map(_.asScala.toMap)) val mergedProps = extractedProps.fold(Map.empty)(_ ++ _) Target.pure(Option(mergedProps.asJava)) case comp: Schema[_] if Option(comp.get$ref).isDefined => From fcbdddeba57e12a0d35a5976708022c4602a11ea Mon Sep 17 00:00:00 2001 From: Tomas Herman Date: Thu, 2 May 2019 22:41:45 +0200 Subject: [PATCH 20/20] Reverted changes to jackson generation --- .../twilio/guardrail/generators/Java/JacksonGenerator.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala index e1feb4f354..3b1aa1bf98 100644 --- a/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala +++ b/modules/codegen/src/main/scala/com/twilio/guardrail/generators/Java/JacksonGenerator.scala @@ -362,7 +362,7 @@ object JacksonGenerator { case RenderDTOClass(clsName, selfParams, parents) => val dtoClassType = JavaParser.parseClassOrInterfaceType(clsName) val discriminators = parents.flatMap(_.discriminators) - val parentOpt = if (parents.exists(s => s.discriminators.nonEmpty)) { parents.headOption } else { None } + val parentOpt = parents.headOption val parentParams = parents.reverse.flatMap(_.params) val parentParamNames = parentParams.map(_.name) val (parentRequiredTerms, parentOptionalTerms) = sortParams(parentParams) @@ -822,7 +822,7 @@ object JacksonGenerator { Target.pure(None) case RenderSealedTrait(className, selfParams, discriminator, parents, children) => - val parentOpt = if (parents.exists(s => s.discriminators.nonEmpty)) { parents.headOption } else { None } + val parentOpt = parents.headOption val parentParams = parents.reverse.flatMap(_.params) val parentParamNames = parentParams.map(_.name) val params = selfParams.filterNot(param => parentParamNames.contains(param.name))