devshorts
commented
7 years ago Great feedback, I'll take a look first week in January as I'm away from a computer for a week or so. Will make all the requested changes, thanks!
On Dec 20, 2016, at 7:42 PM, Xiaodong Wang notifications@github.com wrote:
@xiaodongw commented on this pull request.
In build.gradle:
@@ -1,7 +1,7 @@ ext {
- scalaVersionMain = "2.11"
- scalaVersion = "2.11.8"
- finatraVersion = "2.6.0"
- scalaVersionMain = "2.11" Please clean up all these space changes.
In gradle/wrapper/gradle-wrapper.properties:
@@ -1,6 +1,6 @@ -#Tue Nov 29 20:30:46 PST 2016 +#Fri Dec 16 11:18:05 PST 2016 Clean up the change.
In src/main/scala/com/github/xiaodongw/swagger/finatra/FinatraSwagger.scala:
@@ -14,24 +26,199 @@ object FinatraSwagger { implicit def convertToFinatraSwagger(swagger: Swagger): FinatraSwagger = new FinatraSwagger(swagger) }
+sealed trait ModelParam {
- val name: String
- val description: String
- val required: Boolean
- val typ: Class[_] +}
+sealed trait FinatraParams I prefer the names to be FinatraParam, RouteParam, QueryParam, BodyParam, InjectParam, HeaderParam and FormParam, so they match the existing operations.
In src/main/scala/com/github/xiaodongw/swagger/finatra/FinatraSwagger.scala:
- zip(fields)
- val ast: List[Option[FinatraParams]] =
- constructorArgWithField.map { case (annotationExtractor, field) =>
- val routeParam = annotationExtractor(classOf[RouteParam])
- val queryParam = annotationExtractor(classOf[QueryParam])
- val injectJavax = annotationExtractor(classOf[JInject])
- val injectGuice = annotationExtractor(classOf[GInject])
- val header = annotationExtractor(classOf[HeaderParam])
- val form = annotationExtractor(classOf[FormParam])
- val isRequired = field.getGenericType match {
- case parameterizedType: ParameterizedType =>
- parameterizedType.getRawType.asInstanceOf[Class[]] == classOf[Option[]]
- case : Exception => I got scala.MatchError here, should be "case =>"?
In src/main/scala/com/github/xiaodongw/swagger/finatra/FinatraSwagger.scala:
- }
- else if (header.isDefined) {
- Some(Header(field.getName, typ = field.getType, required = isRequired))
- }
- else if (form.isDefined) {
- Some(Form(field.getName, typ = field.getType, required = isRequired))
- }
- else {
- Some(Body(name = field.getName, typ = field.getType))
- }
- }.toList
- ast.flatten
- }
- private def registerDynamicBody(properties: List[FinatraParams], name: Option[String] = None): Option[Parameter] = { Please change properties to List[BodyParam], the function should make it clear what's the expected input.
Make the name to just String, I do not see a None case here.
In src/main/scala/com/github/xiaodongw/swagger/finatra/FinatraSwagger.scala:
+
- ast.flatten
- }
- private def registerDynamicBody(properties: List[FinatraParams], name: Option[String] = None): Option[Parameter] = {
- val bodyElements = properties.collect { case b: Body => b }
- if (bodyElements.isEmpty) {
- return None
- }
- val bodyClass =
- // if we have more than one body element create a new runtime class
- // to fake out swagger
- if (bodyElements.size > 1) {
- val bodyEmittedClass = new ByteBuddy().subclass(classOf[Object]) Call ByteBuddy.name() here to set the class name, I think it's fine to just use the original case class name.
In src/main/scala/com/github/xiaodongw/swagger/finatra/FinatraSwagger.scala:
- val name: String
- val description: String
- val required: Boolean
- val typ: Class[_] +}
+sealed trait FinatraParams +case class Route(name: String, typ: Class[], description: String = "", required: Boolean = true) extends FinatraParams with ModelParam +case class Query(name: String, typ: Class[], description: String = "", required: Boolean = true) extends FinatraParams with ModelParam +case class Body(description: String = "", name: String, typ: Class[]) extends FinatraParams +case class RequestInject(name: String) extends FinatraParams +case class Header(name: String, required: Boolean = true, description: String = "", typ: Class[]) extends FinatraParams with ModelParam +case class Form(name: String, description: String = "", required: Boolean = true, typ: Class[_]) extends FinatraParams with ModelParam
+object Resolvers {
- class ScalaOptionResolver(objectMapper: ObjectMapper) extends ModelResolver(objectMapper) { This seems not working, the model generate from your example is wrong.
I saw the Option support actually has been fixed in swagger-scala-module (https://github.com/swagger-api/swagger-scala-module/blob/ce25f52a8839592137e414bad06db87d4332d670/src/main/scala/io/swagger/scala/converter/SwaggerScalaModelConverter.scala), we need it to publish a new release.
― You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
ccw
in relation to #25
@xiaodongw here is an initial draft of my proposal. I've done a few things here, and there is one nagging issue that I could use some help with:
request[T]operation that does full auto registration. It scans the fields of the case class and relies on the fact that case class constructor field order and reflective declared field order are the same (i really wanted to use scala reflection but I can't figure out how to get the annotations on a constructor element... :( ). Then we can get the field information and annotation information. We can add to this with our own annotations or even support the swagger annotations in full on them! However, there is an issue where if you have a body parameter that has multiple parameters they need to be registered as a single class to swagger. To do this I used bytebuddy to emit a runtime assembly class to fake out swagger.here is a screenshot if it in action:
However, the crappy part is due to the way the asm was done the model name is mangled:
If you have any suggestions on how to tweak that I would gladly do it.
Let me know what you think