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

ParametricTypeNamesTransformer doesn't update $ref fields in body parameters #355

Open
gavares opened this issue Nov 4, 2020 · 5 comments

Comments

@gavares
Copy link

gavares commented Nov 4, 2020

play-swagger version: 0.10.2-PLAY2.8

Given a routes file like:

###
# summary: send a payload
#  parameters:
#   - name: body
#     schema:
#       $ref: '#/components/schemas/model.CustomResource[model.MySpec]'
###
POST /myroute     

The generated swagger.json does not properly replace the parameterized CustomResource[model.MySpec] in the body parameter. The generated schema definition does have the parameterized definition rendered properly. The output looks like:

"definitions" : {
    "model.CustomResource-domain.MySpec" : {
       ...
    }
},
...
    "/myroute": {
        "post": {
           "parameters" : [ {
              "in" : "body",
              "name" : "body",
              "schema" : {
                "$ref" : "#/components/schemas/model.CustomResource[model.MySpec]"
              }
            } ],
        }
     }

I've looked into the code for ParametricTypeNamesTransformer the tf function is defined as:

 private def tf(obj: JsObject): JsObject = JsObject {
    obj.fields.map {
      case (key, value: JsObject)  (normalize(key), tf(value))
      case (key, JsString(value))  (normalize(key), JsString(normalize(value)))
      case (key, other)            (normalize(key), other)
      case e                       e
    }
  }

Because the parameters section is defined as an array, they will never get normalizeed by the tf function as defined above. This seems to be the desired behavior as specified in ParametricTypeNamesTransformerSpec.

The result is swagger that cannot be rendered by the swagger ui and, I suspect, won't be useable by code generators as well.

@gavares
Copy link
Author

gavares commented Nov 4, 2020

Assuming I've identified an actual bug in the impl, I went ahead and made a locally modified version which seems to solve my issue:

class MyParametricTypeNamesTransformer extends OutputTransformer with LazyLogging {
  override def apply(obj: JsObject): Try[JsObject] = Success(tf(obj))

  private def tf(obj: JsObject): JsObject = {
    val normalizedFields = obj.fields.map {
      case (key, obj: JsObject) => (normalize(key), tf(obj))
      case (key, JsString(value)) => (normalize(key), JsString(normalize(value)))
      case (key, ary: JsArray) => (normalize(key), normalizeAry(ary))
      case (key, other) => (normalize(key), other)
      case e => e
    }
    JsObject( normalizedFields )
  }

  private def normalizeAry(ary: JsArray): JsArray = {
    val normalized: Seq[JsValue] = ary.value.map {
      case o: JsObject => tf(o)
      case a: JsArray => normalizeAry(a)
      case JsString(s) => JsString(normalize(s))
      case other => other
    }.toSeq
    JsArray(normalized)
  }

  private final def normalize(s: String): String = {
    logger.info(s"normalize: ${s}")
    s match {
      case ParametricType.ParametricTypeClassName(className, argsGroup) =>
        val normalizedArgs =
          argsGroup
            .split(",")
            .iterator
            .map(_.trim)
            .map(normalize)
            .mkString("_")
        s"$className-$normalizedArgs"
      case n => n
    }
  }
}

With this change, my swagger UI seems to be happy and all my models appear to be rendering correctly.

I can submit a pull request for this if I'm not misunderstanding something.

@pviral
Copy link

pviral commented Nov 11, 2020

+1, I see the same issue with play-java. In my case, it is not able to generate any parameters for GET.
routes

###
#  summary: Retrieve message
#  parameters:
#     content:
#       application/json:
#         schema:
#           $ref: '#/definitions/models.Message'
#  responses:
#    200:
#      schema:
#        $ref: '#/definitions/models.Message'
###
GET     /v1/message                controllers.HomeController.message

Message.java

package models;

public class Message {
    public String msg = "";
    public Message(String msg) {
        this.msg = msg;
    }
// ...
}

sbt.build

swaggerDomainNameSpaces := Seq("models")
//swaggerV3 := true
swaggerPlayJava := true
swaggerPrettyJson := true

Screen Shot 2020-11-11 at 2 40 30 PM

@gavares
Do you think it might be same issue?

@pviral
Copy link

pviral commented Nov 12, 2020

Assuming I've identified an actual bug in the impl, I went ahead and made a locally modified version which seems to solve my issue:

class MyParametricTypeNamesTransformer extends OutputTransformer with LazyLogging {
  override def apply(obj: JsObject): Try[JsObject] = Success(tf(obj))

  private def tf(obj: JsObject): JsObject = {
    val normalizedFields = obj.fields.map {
      case (key, obj: JsObject) => (normalize(key), tf(obj))
      case (key, JsString(value)) => (normalize(key), JsString(normalize(value)))
      case (key, ary: JsArray) => (normalize(key), normalizeAry(ary))
      case (key, other) => (normalize(key), other)
      case e => e
    }
    JsObject( normalizedFields )
  }

  private def normalizeAry(ary: JsArray): JsArray = {
    val normalized: Seq[JsValue] = ary.value.map {
      case o: JsObject => tf(o)
      case a: JsArray => normalizeAry(a)
      case JsString(s) => JsString(normalize(s))
      case other => other
    }.toSeq
    JsArray(normalized)
  }

  private final def normalize(s: String): String = {
    logger.info(s"normalize: ${s}")
    s match {
      case ParametricType.ParametricTypeClassName(className, argsGroup) =>
        val normalizedArgs =
          argsGroup
            .split(",")
            .iterator
            .map(_.trim)
            .map(normalize)
            .mkString("_")
        s"$className-$normalizedArgs"
      case n => n
    }
  }
}

With this change, my swagger UI seems to be happy and all my models appear to be rendering correctly.

I can submit a pull request for this if I'm not misunderstanding something.

you can submit PR directly :)

@gavares
Copy link
Author

gavares commented Nov 14, 2020

Sure, I suppose I was worried because there is the explicit test to ensure that arrays are ignored in ParametricTypeNamesTransformerSpec. I'll create a PR.

@Javakky-pxv
Copy link
Collaborator

@gavares
Has this PR already been submitted? I have received the sample code, so if I have time, I might be able to address it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants