Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue #222 #234

Merged
merged 36 commits into from May 3, 2019
Merged

issue #222 #234

Changes from 20 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
062ffb5
Added test case for issue 222
Apr 9, 2019
08f9932
It finally compiles! Except for java :(
Apr 12, 2019
2ebdf07
Merge branch 'master' into issue222
tomasherman Apr 12, 2019
28b1b6b
Added test case for issue 222
Apr 25, 2019
cc1cc1b
Merge branch 'issue222' of github.com:avast/guardrail into issue222
Apr 25, 2019
184a5b9
Test passes
Apr 25, 2019
e014a04
Merge branch 'master' into issue222
tomasherman Apr 25, 2019
61d15f2
Reimplemented
Apr 25, 2019
5cd28fb
Merge branch 'issue222' of github.com:avast/guardrail into issue222
Apr 25, 2019
ee39921
Attempt to fix jacksonˆ
Apr 25, 2019
7a9b6da
Removed unused values
Apr 25, 2019
e525c60
Scan all parents for discriminators
Apr 25, 2019
db374e4
Cleanup
Apr 26, 2019
89cc0c4
Update modules/codegen/src/main/scala/com/twilio/guardrail/generators…
blast-hardcheese Apr 26, 2019
d2e7b96
Code review improvements #2
Apr 26, 2019
f244636
Merge branch 'master' into issue222
tomasherman Apr 26, 2019
cf0ddf9
Merge branch 'master' into issue222
tomasherman Apr 27, 2019
8c636a8
Removed comment
Apr 27, 2019
c3a8d6f
Merge branch 'issue222' of github.com:avast/guardrail into issue222
Apr 27, 2019
8411ede
Merge branch 'master' into issue222
tomasherman Apr 30, 2019
0efb82e
Merge branch 'master' of https://github.com/twilio/guardrail into iss…
May 1, 2019
e97efbd
Added another test case and implementation
May 1, 2019
0d54cab
Improved 2nd test case scenario
May 1, 2019
2a7e2fc
Merge branch 'issue222' of github.com:avast/guardrail into issue222
May 1, 2019
8aec398
Added another test case
May 1, 2019
83a5cfc
Added better error reporting when resolving references
May 1, 2019
bb38c38
Scalafmt
May 1, 2019
cb96779
Attempt to reduce filename sizes
May 1, 2019
f81912b
Update modules/codegen/src/main/scala/com/twilio/guardrail/generators…
blast-hardcheese May 2, 2019
5a75f7a
Merge branch 'master' of https://github.com/twilio/guardrail into iss…
May 2, 2019
20ad1f6
Merge branch 'issue222' of github.com:avast/guardrail into issue222
May 2, 2019
6c37f8a
Merge branch 'master' into issue222
tomasherman May 2, 2019
c0b3ddd
Merge branch 'master' into issue222
tomasherman May 2, 2019
fcbddde
Reverted changes to jackson generation
May 2, 2019
b5bfc35
Merge branch 'master' into issue222
tomasherman May 3, 2019
aa2ba14
Merge branch 'master' into issue222
blast-hardcheese May 3, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -41,6 +41,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("issues/issue249.yaml"), "issues.issue249", false, List.empty),
(sampleResource("multipart-form-data.yaml"), "multipartFormData", false, List.empty),
(sampleResource("petstore.json"), "examples", false, List("--import", "support.PositiveLong")),
@@ -171,11 +171,12 @@ object CirceProtocolGenerator {

case RenderDTOClass(clsName, selfParams, parents) =>
val discriminators = parents.flatMap(_.discriminators)
val parenOpt = 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)
)
val code = parenOpt

val code = parentOpt
.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
@@ -324,7 +324,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())
)
@@ -801,7 +801,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
@@ -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 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

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"""
implicit val decodeRequest = Decoder.forProduct2("state", "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)))
This conversation was marked as resolved by tomasherman

This comment has been minimized.

Copy link
@blast-hardcheese

blast-hardcheese Apr 30, 2019

Collaborator

Isn't "id" missing from the encoder?

This comment has been minimized.

Copy link
@tomasherman

tomasherman Apr 30, 2019

Author Contributor

what do you mean? there is no id in request fields, it's only in request itself ... unless im missing something?

This comment has been minimized.

Copy link
@blast-hardcheese

blast-hardcheese Apr 30, 2019

Collaborator

Ah, my apologies -- I encountered the error I mentioned below, and I was hasty in looking for an example of the undesired behaviour in what you already had here.

}
"""
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
}
}
@@ -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
This conversation was marked as resolved by tomasherman

This comment has been minimized.

Copy link
@blast-hardcheese

blast-hardcheese Apr 30, 2019

Collaborator

This doesn't render both id and id2

Suggested change
type: string
type: string
- type: object
properties:
id2:
type: string

This doesn't render parameters from any object literals, so id is missing

Suggested change
type: string
type: string
- "$ref": "#/definitions/RequestFields2"
RequestFields:
description: Request fields
type: object
properties:
state:
type: integer
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.