From 3e89fafe510afa2733fd05b76abe51c4b1da93f1 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 6 Sep 2020 14:19:35 +0200 Subject: [PATCH 1/5] try to fix issues with case class required fields --- .../SwaggerScalaModelConverter.scala | 48 +++++++++++++++++++ .../converter/ModelPropertyParserTest.scala | 3 +- 2 files changed, 49 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 40692ec4..855a4965 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -5,6 +5,7 @@ import java.util.Iterator import com.fasterxml.jackson.databind.JavaType import com.fasterxml.jackson.databind.`type`.ReferenceType +import com.fasterxml.jackson.module.scala.introspect.{BeanIntrospector, PropertyDescriptor} import com.fasterxml.jackson.module.scala.{DefaultScalaModule, JsonScalaEnumeration} import io.swagger.v3.core.converter._ import io.swagger.v3.core.jackson.ModelResolver @@ -27,6 +28,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte private val SetClass = classOf[scala.collection.Set[_]] private val BigDecimalClass = classOf[BigDecimal] private val BigIntClass = classOf[BigInt] + private val ProductClass = classOf[Product] override def resolve(`type`: AnnotatedType, context: ModelConverterContext, chain: Iterator[ModelConverter]): Schema[_] = { val javaType = _mapper.constructType(`type`.getType) @@ -40,6 +42,8 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte resolve(nextType(baseType, `type`, javaType), context, chain) } else if (!annotatedOverrides.headOption.getOrElse(true)) { resolve(nextType(new AnnotatedTypeForOption(), `type`, javaType), context, chain) + } else if (isCaseClass(cls)) { + caseClassSchema(cls, `type`, context, chain).getOrElse(None.orNull) } else if (chain.hasNext) { val nextResolved = Option(chain.next().resolve(`type`, context, chain)) nextResolved match { @@ -68,6 +72,22 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte } } + private def caseClassSchema(cls: Class[_], `type`: AnnotatedType, context: ModelConverterContext, + chain: Iterator[ModelConverter]): Option[Schema[_]] = { + if (chain.hasNext) { + Option(chain.next().resolve(`type`, context, chain)).map { schema => + val introspector = BeanIntrospector(cls) + introspector.properties.foreach { property => + val propertyClass = getPropertyClass(property) + if (!isOption(propertyClass)) addRequiredItem(schema, property.name) + } + schema + } + } else { + None + } + } + private def getRequiredSettings(`type`: AnnotatedType): Seq[Boolean] = `type` match { case _: AnnotatedTypeForOption => Seq.empty case _ => { @@ -183,8 +203,36 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte else None } + private def getPropertyClass(property: PropertyDescriptor): Class[_] = { + property.param match { + case Some(constructorParameter) => { + val types = constructorParameter.constructor.getParameterTypes + if (constructorParameter.index > types.size) { + classOf[Any] + } else { + types(constructorParameter.index) + } + } + 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 _ => classOf[Any] + } + } + } + } + } + private def isOption(cls: Class[_]): Boolean = cls == OptionClass private def isIterable(cls: Class[_]): Boolean = IterableClass.isAssignableFrom(cls) + private def isCaseClass(cls: Class[_]): Boolean = ProductClass.isAssignableFrom(cls) private def nullSafeList[T](array: Array[T]): List[T] = Option(array) match { case None => List.empty[T] 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 6273aef1..bed3f5e4 100644 --- a/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala +++ b/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala @@ -347,8 +347,7 @@ class ModelPropertyParserTest extends AnyFlatSpec with Matchers with OptionValue val1Field shouldBe a [IntegerSchema] val val2Field = model.value.getProperties.get("val2") val2Field shouldBe a [IntegerSchema] - //TODO try to fix this - //model.value.getRequired().asScala shouldEqual Seq("val1", "val2") + model.value.getRequired().asScala shouldEqual Seq("val1", "val2") } private def findModel(schemas: Map[String, Schema[_]], name: String): Option[Schema[_]] = { From 305d6d20de5072130308d801174ed340c8e28d1a Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 6 Sep 2020 14:28:34 +0200 Subject: [PATCH 2/5] tidy up --- .../swagger/scala/converter/SwaggerScalaModelConverter.scala | 5 +++-- 1 file changed, 3 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 855a4965..605eaca3 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -29,6 +29,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte private val BigDecimalClass = classOf[BigDecimal] private val BigIntClass = classOf[BigInt] private val ProductClass = classOf[Product] + private val AnyClass = classOf[Any] override def resolve(`type`: AnnotatedType, context: ModelConverterContext, chain: Iterator[ModelConverter]): Schema[_] = { val javaType = _mapper.constructType(`type`.getType) @@ -208,7 +209,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte case Some(constructorParameter) => { val types = constructorParameter.constructor.getParameterTypes if (constructorParameter.index > types.size) { - classOf[Any] + AnyClass } else { types(constructorParameter.index) } @@ -223,7 +224,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte case Some(setter) if setter.getParameterCount == 1 => { setter.getParameterTypes()(0) } - case _ => classOf[Any] + case _ => AnyClass } } } From 18973a21e672a5ed3c9aad2fe6d3e0edd2785574 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 6 Sep 2020 14:46:52 +0200 Subject: [PATCH 3/5] wip --- .../SwaggerScalaModelConverter.scala | 53 +++++++++++++++++-- 1 file changed, 49 insertions(+), 4 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 605eaca3..bc558bf5 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -79,8 +79,18 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte Option(chain.next().resolve(`type`, context, chain)).map { schema => val introspector = BeanIntrospector(cls) introspector.properties.foreach { property => - val propertyClass = getPropertyClass(property) - if (!isOption(propertyClass)) addRequiredItem(schema, property.name) + getPropertyAnnotatedType(property) match { + case Some(annotatedPropertyType) => { + val propertyClass = getPropertyClass(property) + val required = getRequiredSettings(annotatedPropertyType).headOption + .getOrElse(!isOption(propertyClass)) + if (required) addRequiredItem(schema, property.name) + } + case _ => { + val propertyClass = getPropertyClass(property) + if (!isOption(propertyClass)) addRequiredItem(schema, property.name) + } + } } schema } @@ -89,10 +99,10 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte } } - private def getRequiredSettings(`type`: AnnotatedType): Seq[Boolean] = `type` match { + private def getRequiredSettings(annotatedType: AnnotatedType): Seq[Boolean] = annotatedType match { case _: AnnotatedTypeForOption => Seq.empty case _ => { - nullSafeList(`type`.getCtxAnnotations).collect { + nullSafeList(annotatedType.getCtxAnnotations).collect { case p: Parameter => p.required() case s: SchemaAnnotation => s.required() case a: ArraySchema => a.arraySchema().required() @@ -100,6 +110,14 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte } } + private def getRequiredSettings(annotatedType: java.lang.reflect.AnnotatedType): Seq[Boolean] = { + nullSafeList(annotatedType.getAnnotations).collect { + case p: Parameter => p.required() + case s: SchemaAnnotation => s.required() + case a: ArraySchema => a.arraySchema().required() + } + } + private def matchScalaPrimitives(`type`: AnnotatedType, nullableClass: Class[_]): Option[Schema[_]] = { val annotations = Option(`type`.getCtxAnnotations).map(_.toSeq).getOrElse(Seq.empty) annotations.collectFirst { case ann: SchemaAnnotation => ann } match { @@ -231,6 +249,33 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte } } + private def getPropertyAnnotatedType(property: PropertyDescriptor): Option[java.lang.reflect.AnnotatedType] = { + property.param match { + case Some(constructorParameter) => { + val types = constructorParameter.constructor.getAnnotatedParameterTypes + if (constructorParameter.index > types.size) { + None + } else { + Some(types(constructorParameter.index)) + } + } + case _ => property.field match { + case Some(field) => Some(field.getAnnotatedType) + case _ => property.setter match { + case Some(setter) if setter.getParameterCount == 1 => { + Some(setter.getAnnotatedParameterTypes()(0)) + } + case _ => property.beanSetter match { + case Some(setter) if setter.getParameterCount == 1 => { + Some(setter.getAnnotatedParameterTypes()(0)) + } + case _ => None + } + } + } + } + } + private def isOption(cls: Class[_]): Boolean = cls == OptionClass private def isIterable(cls: Class[_]): Boolean = IterableClass.isAssignableFrom(cls) private def isCaseClass(cls: Class[_]): Boolean = ProductClass.isAssignableFrom(cls) From 7d96a85910439d8b3eba3ddb580f65b21bf2ebb5 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 6 Sep 2020 15:14:44 +0200 Subject: [PATCH 4/5] fix tests --- .../SwaggerScalaModelConverter.scala | 36 +++++++++---------- .../converter/ModelPropertyParserTest.scala | 2 +- 2 files changed, 19 insertions(+), 19 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 bc558bf5..bb682d62 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -1,5 +1,6 @@ package com.github.swagger.scala.converter +import java.lang.annotation.Annotation import java.lang.reflect.ParameterizedType import java.util.Iterator @@ -79,17 +80,16 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte Option(chain.next().resolve(`type`, context, chain)).map { schema => val introspector = BeanIntrospector(cls) introspector.properties.foreach { property => - getPropertyAnnotatedType(property) match { - case Some(annotatedPropertyType) => { - val propertyClass = getPropertyClass(property) - val required = getRequiredSettings(annotatedPropertyType).headOption - .getOrElse(!isOption(propertyClass)) - if (required) addRequiredItem(schema, property.name) - } - case _ => { + getPropertyAnnotations(property) match { + case Seq() => { val propertyClass = getPropertyClass(property) if (!isOption(propertyClass)) addRequiredItem(schema, property.name) } + case annotations => { + val required = getRequiredSettings(annotations).headOption + .getOrElse(!isOption(getPropertyClass(property))) + if (required) addRequiredItem(schema, property.name) + } } } schema @@ -110,8 +110,8 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte } } - private def getRequiredSettings(annotatedType: java.lang.reflect.AnnotatedType): Seq[Boolean] = { - nullSafeList(annotatedType.getAnnotations).collect { + private def getRequiredSettings(annotations: Seq[Annotation]): Seq[Boolean] = { + annotations.collect { case p: Parameter => p.required() case s: SchemaAnnotation => s.required() case a: ArraySchema => a.arraySchema().required() @@ -249,27 +249,27 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte } } - private def getPropertyAnnotatedType(property: PropertyDescriptor): Option[java.lang.reflect.AnnotatedType] = { + private def getPropertyAnnotations(property: PropertyDescriptor): Seq[Annotation] = { property.param match { case Some(constructorParameter) => { - val types = constructorParameter.constructor.getAnnotatedParameterTypes + val types = constructorParameter.constructor.getParameterAnnotations if (constructorParameter.index > types.size) { - None + Seq.empty } else { - Some(types(constructorParameter.index)) + types(constructorParameter.index).toSeq } } case _ => property.field match { - case Some(field) => Some(field.getAnnotatedType) + case Some(field) => field.getAnnotations.toSeq case _ => property.setter match { case Some(setter) if setter.getParameterCount == 1 => { - Some(setter.getAnnotatedParameterTypes()(0)) + setter.getAnnotations().toSeq } case _ => property.beanSetter match { case Some(setter) if setter.getParameterCount == 1 => { - Some(setter.getAnnotatedParameterTypes()(0)) + setter.getAnnotations().toSeq } - case _ => None + case _ => Seq.empty } } } 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 bed3f5e4..4bf523e9 100644 --- a/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala +++ b/src/test/scala/com/github/swagger/scala/converter/ModelPropertyParserTest.scala @@ -366,7 +366,7 @@ class ModelPropertyParserTest extends AnyFlatSpec with Matchers with OptionValue val schemas = converter.readAll(classOf[ModelWStringSeq]).asScala.toMap val model = findModel(schemas, "ModelWStringSeq") model should be(defined) - nullSafeList(model.value.getRequired) shouldEqual Seq() + nullSafeList(model.value.getRequired) shouldBe empty } it should "process Array-Model with forced required Scala Option Seq" in { From 0b38566eb1cdd7ef5d423e6f632b4b8cb223b524 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 6 Sep 2020 15:17:06 +0200 Subject: [PATCH 5/5] tidy up --- .../scala/converter/SwaggerScalaModelConverter.scala | 8 +------- 1 file changed, 1 insertion(+), 7 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 bb682d62..f902adca 100644 --- a/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala +++ b/src/main/scala/com/github/swagger/scala/converter/SwaggerScalaModelConverter.scala @@ -101,13 +101,7 @@ class SwaggerScalaModelConverter extends ModelResolver(SwaggerScalaModelConverte private def getRequiredSettings(annotatedType: AnnotatedType): Seq[Boolean] = annotatedType match { case _: AnnotatedTypeForOption => Seq.empty - case _ => { - nullSafeList(annotatedType.getCtxAnnotations).collect { - case p: Parameter => p.required() - case s: SchemaAnnotation => s.required() - case a: ArraySchema => a.arraySchema().required() - } - } + case _ => getRequiredSettings(nullSafeList(annotatedType.getCtxAnnotations)) } private def getRequiredSettings(annotations: Seq[Annotation]): Seq[Boolean] = {