From 71d43c37abed011c22b265b38d9bd3daa68c47a9 Mon Sep 17 00:00:00 2001 From: SamTheisens <1911436+SamTheisens@users.noreply.github.com> Date: Sun, 7 Aug 2022 11:02:10 +0800 Subject: [PATCH 1/4] Prioritize explicitly specified format over the one inferred from the primitive type --- .../scala/converter/SwaggerScalaModelConverter.scala | 4 +++- .../scala/converter/ModelPropertyParserTest.scala | 7 +++++++ src/test/scala/models/ModelWOptionString.scala | 9 +++++---- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala index e8e0f8f2..7e2c936d 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -137,7 +137,9 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte val propAsString = objectMapper.writeValueAsString(itemSchema) val correctedSchema = objectMapper.readValue(propAsString, primitiveProperty.getClass) correctedSchema.setType(primitiveProperty.getType) - correctedSchema.setFormat(primitiveProperty.getFormat) + if (itemSchema.getFormat == null) { + correctedSchema.setFormat(primitiveProperty.getFormat) + } correctedSchema } diff --git a/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala b/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala index 56fad8df..5c2aaec4 100644 --- a/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala +++ b/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala @@ -38,6 +38,13 @@ class ModelPropertyParserTest extends AnyFlatSpec with Matchers with OptionValue stringWithDataType should not be (null) stringWithDataType shouldBe a [StringSchema] nullSafeList(stringWithDataType.getRequired) shouldBe empty + + val ipAddress = model.value.getProperties().get("ipAddress") + ipAddress should not be (null) + ipAddress shouldBe a[StringSchema] + ipAddress.getDescription shouldBe "An IP address" + ipAddress.getFormat shouldBe "IPv4 or IPv6" + nullSafeList(ipAddress.getRequired) shouldBe empty } it should "process Option[Model] as Model" in { diff --git a/src/test/scala/models/ModelWOptionString.scala b/src/test/scala/models/ModelWOptionString.scala index 3d4ec927..6a80a987 100644 --- a/src/test/scala/models/ModelWOptionString.scala +++ b/src/test/scala/models/ModelWOptionString.scala @@ -3,10 +3,11 @@ package models import io.swagger.v3.oas.annotations.Parameter import io.swagger.v3.oas.annotations.media.Schema -case class ModelWOptionString ( - stringOpt: Option[String], - stringWithDataTypeOpt: Option[String]) - +case class ModelWOptionString (stringOpt: Option[String], + stringWithDataTypeOpt: Option[String], + @Schema(description = "An IP address", format = "IPv4 or IPv6") + ipAddress: Option[String] + ) case class ModelWOptionModel (modelOpt: Option[ModelWOptionString]) From 4bb123dda98ef37a1d4b0c47a60f78db71de579f Mon Sep 17 00:00:00 2001 From: SamTheisens <1911436+SamTheisens@users.noreply.github.com> Date: Sun, 7 Aug 2022 16:14:12 +0800 Subject: [PATCH 2/4] Handle ambiguous apply methods --- .../swagger/scala/converter/ErasureHelper.scala | 16 ++++++++-------- src/test/scala/models/ModelWOptionInt.scala | 8 ++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/main/scala-2/com/github/swagger/scala/converter/ErasureHelper.scala b/src/main/scala-2/com/github/swagger/scala/converter/ErasureHelper.scala index 350cc163..a6196bde 100644 --- a/src/main/scala-2/com/github/swagger/scala/converter/ErasureHelper.scala +++ b/src/main/scala-2/com/github/swagger/scala/converter/ErasureHelper.scala @@ -1,6 +1,7 @@ package com.github.swagger.scala.converter import scala.reflect.runtime.universe +import scala.util.Try object ErasureHelper { @@ -10,14 +11,13 @@ object ErasureHelper { val moduleSymbol = mirror.moduleSymbol(Class.forName(cls.getName)) val ConstructorName = "apply" val companion: universe.Symbol = moduleSymbol.typeSignature.member(universe.TermName(ConstructorName)) - val properties = if (companion.fullName.endsWith(ConstructorName)) { - companion.asMethod.paramLists.flatten - } else { - val sym = mirror.staticClass(cls.getName) - sym.selfType.members - .filterNot(_.isMethod) - .filterNot(_.isClass) - } + val properties = + Try(companion.asTerm.alternatives.head.asMethod.paramLists.flatten).getOrElse { + val sym = mirror.staticClass(cls.getName) + sym.selfType.members + .filterNot(_.isMethod) + .filterNot(_.isClass) + } properties.flatMap { prop: universe.Symbol => val maybeClass: Option[Class[_]] = prop.typeSignature.typeArgs.headOption.flatMap { signature => diff --git a/src/test/scala/models/ModelWOptionInt.scala b/src/test/scala/models/ModelWOptionInt.scala index ee3a6c01..4576b5fa 100644 --- a/src/test/scala/models/ModelWOptionInt.scala +++ b/src/test/scala/models/ModelWOptionInt.scala @@ -6,6 +6,14 @@ case class ModelWOptionInt(optInt: Option[Int]) object NestingObject { case class NestedModelWOptionInt(optInt: Option[Int]) + + object NestedModelWOptionInt { + + def apply(nonOptional: Int): NestedModelWOptionInt = { + NestedModelWOptionInt(Some(nonOptional)) + } + } + case class NestedModelWOptionIntSchemaOverride(@Schema(description = "This is an optional int") optInt: Option[Int]) } From b4f213bfe93df2e5b31043b940a70f7ea20e9e36 Mon Sep 17 00:00:00 2001 From: SamTheisens <1911436+SamTheisens@users.noreply.github.com> Date: Sun, 7 Aug 2022 16:15:22 +0800 Subject: [PATCH 3/4] Avoid NPE on empty case class branch: --- .../scala/converter/SwaggerScalaModelConverter.scala | 2 +- .../scala/converter/ModelPropertyParserTest.scala | 12 +++++++++++- src/test/scala/models/ModelWOptionInt.scala | 3 +++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala index 7e2c936d..4ff610bb 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -86,7 +86,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte val schemaOverrideClass = propertyAnnotations.collectFirst { case s: SchemaAnnotation if s.implementation() != VoidClass => s.implementation() } - if (schemaOverrideClass.isEmpty) { + if (schemaOverrideClass.isEmpty && schema.getProperties != null) { erasedProperties.get(property.name).foreach { erasedType => val primitiveType = PrimitiveType.fromType(erasedType) if (primitiveType != null && isOptional) { diff --git a/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala b/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala index 5c2aaec4..b92cb9ff 100644 --- a/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala +++ b/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala @@ -3,7 +3,7 @@ package com.github.swagger.scala.converter import io.swagger.v3.core.converter._ import io.swagger.v3.core.util.Json import io.swagger.v3.oas.models.media._ -import models.NestingObject.NestedModelWOptionInt +import models.NestingObject.{NestedModelWOptionInt, NoProperties} import models._ import org.scalatest.OptionValues import org.scalatest.flatspec.AnyFlatSpec @@ -142,6 +142,16 @@ class ModelPropertyParserTest extends AnyFlatSpec with Matchers with OptionValue nullSafeList(model.value.getRequired) shouldBe empty } + it should "process Model without any properties" in { + val converter = ModelConverters.getInstance() + val schemas = converter.readAll(classOf[NoProperties]).asScala.toMap + val model = schemas.get("NoProperties") + model should be(defined) + model.value.getProperties should be (null) + model.get shouldBe a[Schema[_]] + model.get.getDescription shouldBe "An empty case class" + } + it should "process Model with nested Scala Option Int with Schema Override" in { val converter = ModelConverters.getInstance() val schemas = converter.readAll(classOf[ModelWOptionIntSchemaOverride]).asScala.toMap diff --git a/src/test/scala/models/ModelWOptionInt.scala b/src/test/scala/models/ModelWOptionInt.scala index 4576b5fa..7f23ee2c 100644 --- a/src/test/scala/models/ModelWOptionInt.scala +++ b/src/test/scala/models/ModelWOptionInt.scala @@ -7,6 +7,9 @@ case class ModelWOptionInt(optInt: Option[Int]) object NestingObject { case class NestedModelWOptionInt(optInt: Option[Int]) + @Schema(description = "An empty case class") + case class NoProperties() + object NestedModelWOptionInt { def apply(nonOptional: Int): NestedModelWOptionInt = { From 7c5d8a24e8fcd5860413b08e0f380bd97df9e207 Mon Sep 17 00:00:00 2001 From: SamTheisens <1911436+SamTheisens@users.noreply.github.com> Date: Sun, 7 Aug 2022 16:43:24 +0800 Subject: [PATCH 4/4] Remove duplication of logic in getting a property's class and annotations --- .../SwaggerScalaModelConverter.scala | 61 ++++++------------- 1 file changed, 17 insertions(+), 44 deletions(-) diff --git a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala index 4ff610bb..77ebce46 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -2,8 +2,7 @@ package com.github.swagger.scala.converter import java.lang.annotation.Annotation import java.lang.reflect.ParameterizedType -import java.util.Iterator -import com.fasterxml.jackson.databind.JavaType +import com.fasterxml.jackson.databind.{JavaType, ObjectMapper} import com.fasterxml.jackson.databind.`type`.ReferenceType import com.fasterxml.jackson.module.scala.introspect.{BeanIntrospector, PropertyDescriptor} import com.fasterxml.jackson.module.scala.{DefaultScalaModule, JsonScalaEnumeration} @@ -22,11 +21,10 @@ import scala.util.control.NonFatal class AnnotatedTypeForOption extends AnnotatedType object SwaggerScalaModelConverter { - val objectMapper = Json.mapper().registerModule(DefaultScalaModule) + val objectMapper: ObjectMapper = Json.mapper().registerModule(DefaultScalaModule) } class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverter.objectMapper) { - SwaggerScalaModelConverter private val logger = LoggerFactory.getLogger(classOf[SwaggerScalaModelConverter]) private val VoidClass = classOf[Void] @@ -40,7 +38,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte private val ProductClass = classOf[Product] private val AnyClass = classOf[Any] - override def resolve(`type`: AnnotatedType, context: ModelConverterContext, chain: Iterator[ModelConverter]): Schema[_] = { + override def resolve(`type`: AnnotatedType, context: ModelConverterContext, chain: util.Iterator[ModelConverter]): Schema[_] = { val javaType = _mapper.constructType(`type`.getType) val cls = javaType.getRawClass @@ -80,9 +78,8 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte val introspector = BeanIntrospector(cls) val erasedProperties = ErasureHelper.erasedOptionalPrimitives(cls) introspector.properties.foreach { property => - val propertyClass = getPropertyClass(property) + val (propertyClass, propertyAnnotations) = getPropertyClassAndAnnotations(property) val isOptional = isOption(propertyClass) - val propertyAnnotations = getPropertyAnnotations(property) val schemaOverrideClass = propertyAnnotations.collectFirst { case s: SchemaAnnotation if s.implementation() != VoidClass => s.implementation() } @@ -263,54 +260,30 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte } } - private def getPropertyClass(property: PropertyDescriptor): Class[_] = { + private def getPropertyClassAndAnnotations(property: PropertyDescriptor): (Class[_], Seq[Annotation]) = { property.param match { - case Some(constructorParameter) => { + case Some(constructorParameter) => val types = constructorParameter.constructor.getParameterTypes - if (constructorParameter.index > types.size) { - AnyClass + val annotations = constructorParameter.constructor.getParameterAnnotations + val index = constructorParameter.index + if (index > types.size) { + (AnyClass, Seq.empty) } else { - types(constructorParameter.index) + val onlyType = types(index) + val onlyAnnotations = if (index > annotations.size) { Seq.empty[Annotation] } else { annotations(index).toIndexedSeq } + (onlyType, onlyAnnotations) } - } - case _ => property.field match { - case Some(field) => field.getType - case _ => property.setter match { - case Some(setter) if setter.getParameterCount == 1 => { - setter.getParameterTypes()(0) - } - case _ => property.beanSetter match { - case Some(setter) if setter.getParameterCount == 1 => { - setter.getParameterTypes()(0) - } - case _ => AnyClass - } - } - } - } - } - - private def getPropertyAnnotations(property: PropertyDescriptor): Seq[Annotation] = { - property.param match { - case Some(constructorParameter) => { - val types = constructorParameter.constructor.getParameterAnnotations - if (constructorParameter.index > types.size) { - Seq.empty - } else { - types(constructorParameter.index).toSeq - } - } case _ => property.field match { - case Some(field) => field.getAnnotations.toSeq + case Some(field) => (field.getType, field.getAnnotations.toSeq) case _ => property.setter match { case Some(setter) if setter.getParameterCount == 1 => { - setter.getAnnotations().toSeq + (setter.getParameterTypes()(0), setter.getAnnotations.toSeq) } case _ => property.beanSetter match { case Some(setter) if setter.getParameterCount == 1 => { - setter.getAnnotations().toSeq + (setter.getParameterTypes()(0), setter.getAnnotations.toSeq) } - case _ => Seq.empty + case _ => (AnyClass, Seq.empty) } } }