Skip to content

Commit ce2878e

Browse files
authored
Prioritize explicitly specified format (#171)
* Prioritize explicitly specified format over the one inferred from the primitive type * Handle ambiguous apply methods * Avoid NPE on empty case class branch: * Remove duplication of logic in getting a property's class and annotations
1 parent 8ea4744 commit ce2878e

File tree

5 files changed

+63
-59
lines changed

5 files changed

+63
-59
lines changed

src/main/scala-2/com/github/swagger/scala/converter/ErasureHelper.scala

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.github.swagger.scala.converter
22

33
import scala.reflect.runtime.universe
4+
import scala.util.Try
45

56
object ErasureHelper {
67

@@ -10,14 +11,13 @@ object ErasureHelper {
1011
val moduleSymbol = mirror.moduleSymbol(Class.forName(cls.getName))
1112
val ConstructorName = "apply"
1213
val companion: universe.Symbol = moduleSymbol.typeSignature.member(universe.TermName(ConstructorName))
13-
val properties = if (companion.fullName.endsWith(ConstructorName)) {
14-
companion.asMethod.paramLists.flatten
15-
} else {
16-
val sym = mirror.staticClass(cls.getName)
17-
sym.selfType.members
18-
.filterNot(_.isMethod)
19-
.filterNot(_.isClass)
20-
}
14+
val properties =
15+
Try(companion.asTerm.alternatives.head.asMethod.paramLists.flatten).getOrElse {
16+
val sym = mirror.staticClass(cls.getName)
17+
sym.selfType.members
18+
.filterNot(_.isMethod)
19+
.filterNot(_.isClass)
20+
}
2121

2222
properties.flatMap { prop: universe.Symbol =>
2323
val maybeClass: Option[Class[_]] = prop.typeSignature.typeArgs.headOption.flatMap { signature =>

src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala

Lines changed: 21 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ package com.github.swagger.scala.converter
22

33
import java.lang.annotation.Annotation
44
import java.lang.reflect.ParameterizedType
5-
import java.util.Iterator
6-
import com.fasterxml.jackson.databind.JavaType
5+
import com.fasterxml.jackson.databind.{JavaType, ObjectMapper}
76
import com.fasterxml.jackson.databind.`type`.ReferenceType
87
import com.fasterxml.jackson.module.scala.introspect.{BeanIntrospector, PropertyDescriptor}
98
import com.fasterxml.jackson.module.scala.{DefaultScalaModule, JsonScalaEnumeration}
@@ -22,11 +21,10 @@ import scala.util.control.NonFatal
2221
class AnnotatedTypeForOption extends AnnotatedType
2322

2423
object SwaggerScalaModelConverter {
25-
val objectMapper = Json.mapper().registerModule(DefaultScalaModule)
24+
val objectMapper: ObjectMapper = Json.mapper().registerModule(DefaultScalaModule)
2625
}
2726

2827
class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverter.objectMapper) {
29-
SwaggerScalaModelConverter
3028

3129
private val logger = LoggerFactory.getLogger(classOf[SwaggerScalaModelConverter])
3230
private val VoidClass = classOf[Void]
@@ -40,7 +38,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte
4038
private val ProductClass = classOf[Product]
4139
private val AnyClass = classOf[Any]
4240

43-
override def resolve(`type`: AnnotatedType, context: ModelConverterContext, chain: Iterator[ModelConverter]): Schema[_] = {
41+
override def resolve(`type`: AnnotatedType, context: ModelConverterContext, chain: util.Iterator[ModelConverter]): Schema[_] = {
4442
val javaType = _mapper.constructType(`type`.getType)
4543
val cls = javaType.getRawClass
4644

@@ -80,13 +78,12 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte
8078
val introspector = BeanIntrospector(cls)
8179
val erasedProperties = ErasureHelper.erasedOptionalPrimitives(cls)
8280
introspector.properties.foreach { property =>
83-
val propertyClass = getPropertyClass(property)
81+
val (propertyClass, propertyAnnotations) = getPropertyClassAndAnnotations(property)
8482
val isOptional = isOption(propertyClass)
85-
val propertyAnnotations = getPropertyAnnotations(property)
8683
val schemaOverrideClass = propertyAnnotations.collectFirst {
8784
case s: SchemaAnnotation if s.implementation() != VoidClass => s.implementation()
8885
}
89-
if (schemaOverrideClass.isEmpty) {
86+
if (schemaOverrideClass.isEmpty && schema.getProperties != null) {
9087
erasedProperties.get(property.name).foreach { erasedType =>
9188
val primitiveType = PrimitiveType.fromType(erasedType)
9289
if (primitiveType != null && isOptional) {
@@ -137,7 +134,9 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte
137134
val propAsString = objectMapper.writeValueAsString(itemSchema)
138135
val correctedSchema = objectMapper.readValue(propAsString, primitiveProperty.getClass)
139136
correctedSchema.setType(primitiveProperty.getType)
140-
correctedSchema.setFormat(primitiveProperty.getFormat)
137+
if (itemSchema.getFormat == null) {
138+
correctedSchema.setFormat(primitiveProperty.getFormat)
139+
}
141140
correctedSchema
142141
}
143142

@@ -261,54 +260,30 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte
261260
}
262261
}
263262

264-
private def getPropertyClass(property: PropertyDescriptor): Class[_] = {
263+
private def getPropertyClassAndAnnotations(property: PropertyDescriptor): (Class[_], Seq[Annotation]) = {
265264
property.param match {
266-
case Some(constructorParameter) => {
265+
case Some(constructorParameter) =>
267266
val types = constructorParameter.constructor.getParameterTypes
268-
if (constructorParameter.index > types.size) {
269-
AnyClass
267+
val annotations = constructorParameter.constructor.getParameterAnnotations
268+
val index = constructorParameter.index
269+
if (index > types.size) {
270+
(AnyClass, Seq.empty)
270271
} else {
271-
types(constructorParameter.index)
272+
val onlyType = types(index)
273+
val onlyAnnotations = if (index > annotations.size) { Seq.empty[Annotation] } else { annotations(index).toIndexedSeq }
274+
(onlyType, onlyAnnotations)
272275
}
273-
}
274-
case _ => property.field match {
275-
case Some(field) => field.getType
276-
case _ => property.setter match {
277-
case Some(setter) if setter.getParameterCount == 1 => {
278-
setter.getParameterTypes()(0)
279-
}
280-
case _ => property.beanSetter match {
281-
case Some(setter) if setter.getParameterCount == 1 => {
282-
setter.getParameterTypes()(0)
283-
}
284-
case _ => AnyClass
285-
}
286-
}
287-
}
288-
}
289-
}
290-
291-
private def getPropertyAnnotations(property: PropertyDescriptor): Seq[Annotation] = {
292-
property.param match {
293-
case Some(constructorParameter) => {
294-
val types = constructorParameter.constructor.getParameterAnnotations
295-
if (constructorParameter.index > types.size) {
296-
Seq.empty
297-
} else {
298-
types(constructorParameter.index).toSeq
299-
}
300-
}
301276
case _ => property.field match {
302-
case Some(field) => field.getAnnotations.toSeq
277+
case Some(field) => (field.getType, field.getAnnotations.toSeq)
303278
case _ => property.setter match {
304279
case Some(setter) if setter.getParameterCount == 1 => {
305-
setter.getAnnotations().toSeq
280+
(setter.getParameterTypes()(0), setter.getAnnotations.toSeq)
306281
}
307282
case _ => property.beanSetter match {
308283
case Some(setter) if setter.getParameterCount == 1 => {
309-
setter.getAnnotations().toSeq
284+
(setter.getParameterTypes()(0), setter.getAnnotations.toSeq)
310285
}
311-
case _ => Seq.empty
286+
case _ => (AnyClass, Seq.empty)
312287
}
313288
}
314289
}

src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package com.github.swagger.scala.converter
33
import io.swagger.v3.core.converter._
44
import io.swagger.v3.core.util.Json
55
import io.swagger.v3.oas.models.media._
6-
import models.NestingObject.NestedModelWOptionInt
6+
import models.NestingObject.{NestedModelWOptionInt, NoProperties}
77
import models._
88
import org.scalatest.OptionValues
99
import org.scalatest.flatspec.AnyFlatSpec
@@ -38,6 +38,13 @@ class ModelPropertyParserTest extends AnyFlatSpec with Matchers with OptionValue
3838
stringWithDataType should not be (null)
3939
stringWithDataType shouldBe a [StringSchema]
4040
nullSafeList(stringWithDataType.getRequired) shouldBe empty
41+
42+
val ipAddress = model.value.getProperties().get("ipAddress")
43+
ipAddress should not be (null)
44+
ipAddress shouldBe a[StringSchema]
45+
ipAddress.getDescription shouldBe "An IP address"
46+
ipAddress.getFormat shouldBe "IPv4 or IPv6"
47+
nullSafeList(ipAddress.getRequired) shouldBe empty
4148
}
4249

4350
it should "process Option[Model] as Model" in {
@@ -135,6 +142,16 @@ class ModelPropertyParserTest extends AnyFlatSpec with Matchers with OptionValue
135142
nullSafeList(model.value.getRequired) shouldBe empty
136143
}
137144

145+
it should "process Model without any properties" in {
146+
val converter = ModelConverters.getInstance()
147+
val schemas = converter.readAll(classOf[NoProperties]).asScala.toMap
148+
val model = schemas.get("NoProperties")
149+
model should be(defined)
150+
model.value.getProperties should be (null)
151+
model.get shouldBe a[Schema[_]]
152+
model.get.getDescription shouldBe "An empty case class"
153+
}
154+
138155
it should "process Model with nested Scala Option Int with Schema Override" in {
139156
val converter = ModelConverters.getInstance()
140157
val schemas = converter.readAll(classOf[ModelWOptionIntSchemaOverride]).asScala.toMap

src/test/scala/models/ModelWOptionInt.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ case class ModelWOptionInt(optInt: Option[Int])
66

77
object NestingObject {
88
case class NestedModelWOptionInt(optInt: Option[Int])
9+
10+
@Schema(description = "An empty case class")
11+
case class NoProperties()
12+
13+
object NestedModelWOptionInt {
14+
15+
def apply(nonOptional: Int): NestedModelWOptionInt = {
16+
NestedModelWOptionInt(Some(nonOptional))
17+
}
18+
}
19+
920
case class NestedModelWOptionIntSchemaOverride(@Schema(description = "This is an optional int") optInt: Option[Int])
1021
}
1122

src/test/scala/models/ModelWOptionString.scala

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ package models
33
import io.swagger.v3.oas.annotations.Parameter
44
import io.swagger.v3.oas.annotations.media.Schema
55

6-
case class ModelWOptionString (
7-
stringOpt: Option[String],
8-
stringWithDataTypeOpt: Option[String])
9-
6+
case class ModelWOptionString (stringOpt: Option[String],
7+
stringWithDataTypeOpt: Option[String],
8+
@Schema(description = "An IP address", format = "IPv4 or IPv6")
9+
ipAddress: Option[String]
10+
)
1011

1112
case class ModelWOptionModel (modelOpt: Option[ModelWOptionString])
1213

0 commit comments

Comments
 (0)