Move discriminated field to constructor. (#217)

* Move discriminated field to constructor. This is necessary even when there is only one value to have it used as part of the equals function * fix tests * move override value to make it a little clearer * improve readability * simplify * remove debug statement
cjbooms · Jun 13, 2023 · 575d3cf · 575d3cf
1 parent 55c0ece
commit 575d3cf
Show file tree

Hide file tree

Showing 15 changed files with 130 additions and 163 deletions.
diff --git a/src/main/kotlin/com/cjbooms/fabrikt/generators/PropertyUtils.kt b/src/main/kotlin/com/cjbooms/fabrikt/generators/PropertyUtils.kt
@@ -86,18 +86,6 @@ object PropertyUtils {
                 ClassSettings.PolymorphyType.SUB -> {
                     if (this is PropertyInfo.Field && isPolymorphicDiscriminator) {
                         property.addModifiers(KModifier.OVERRIDE)
-                        val discriminators = maybeDiscriminator.getDiscriminatorMappings(schemaName)
-                        if (discriminators.size == 1) {
-                            when (val discriminator = discriminators.first()) {
-                                is PropertyInfo.DiscriminatorKey.EnumKey ->
-                                    property.initializer("%T.%L", wrappedType, discriminator.enumKey)
-
-                                is PropertyInfo.DiscriminatorKey.StringKey ->
-                                    property.initializer("%S", discriminator.stringValue)
-                            }
-                        } else {
-                            property.addAnnotation(JacksonMetadata.jacksonParameterAnnotation(oasKey))
-                        }
                     } else {
                         if (isInherited) {
                             property.addModifiers(KModifier.OVERRIDE)
@@ -116,21 +104,33 @@ object PropertyUtils {
                 }
             }
 
-            if (this !is PropertyInfo.Field ||
-                !isPolymorphicDiscriminator ||
-                isSubTypeDiscriminatorWithNoValue(classSettings) ||
-                isSubTypeDiscriminatorWithMultipleValues(classSettings, schemaName)
-            ) {
+            if (isDiscriminatorFieldWithSingleKnownValue(classSettings, schemaName)) {
+                this as PropertyInfo.Field
+                if (classSettings.polymorphyType == ClassSettings.PolymorphyType.SUB) {
+                    property.initializer(name)
+                    val constructorParameter: ParameterSpec.Builder = ParameterSpec.builder(name, wrappedType)
+                    val discriminators = maybeDiscriminator.getDiscriminatorMappings(schemaName)
+                    when (val discriminator = discriminators.first()) {
+                        is PropertyInfo.DiscriminatorKey.EnumKey ->
+                            constructorParameter.defaultValue("%T.%L", wrappedType, discriminator.enumKey)
+
+                        is PropertyInfo.DiscriminatorKey.StringKey ->
+                            constructorParameter.defaultValue("%S", discriminator.stringValue)
+                    }
+                    constructorBuilder.addParameter(constructorParameter.build())
+                }
+            } else {
                 property.initializer(name)
                 val constructorParameter: ParameterSpec.Builder = ParameterSpec.builder(name, wrappedType)
                 val oasDefault = getDefaultValue(this, parameterizedType)
                 if (!isRequired) {
                     if (oasDefault != null) {
                         val wrappedDefault =
-                            if (classSettings.isMergePatchPattern)
+                            if (classSettings.isMergePatchPattern) {
                                 OasDefault.JsonNullableValue(oasDefault)
-                            else
+                            } else {
                                 oasDefault
+                            }
                         constructorParameter.defaultValue(wrappedDefault.getDefault())
                     } else {
                         val undefinedDefault = if (classSettings.isMergePatchPattern) {
@@ -148,6 +148,14 @@ object PropertyUtils {
         classBuilder.addProperty(property.build())
     }
 
+    private fun PropertyInfo.isDiscriminatorFieldWithSingleKnownValue(
+        classSettings: ClassSettings,
+        schemaName: String,
+    ) = this is PropertyInfo.Field &&
+        isPolymorphicDiscriminator &&
+        !isSubTypeDiscriminatorWithNoValue(classSettings) &&
+        !isSubTypeDiscriminatorWithMultipleValues(classSettings, schemaName)
+
     private fun Map<String, PropertyInfo.DiscriminatorKey>?.getDiscriminatorMappings(
         schemaName: String,
     ): List<PropertyInfo.DiscriminatorKey> =

diff --git a/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt b/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt
@@ -726,9 +726,16 @@ class JacksonModelGenerator(
                 KModifier.DATA,
             )
         }
+        sortConstructorParameters(constructorBuilder, classType)
         return classBuilder.primaryConstructor(constructorBuilder.build())
     }
 
+    private fun sortConstructorParameters(constructorBuilder: FunSpec.Builder, classType: ClassSettings) {
+        if (classType.polymorphyType != ClassSettings.PolymorphyType.NONE) {
+            constructorBuilder.parameters.sortBy { it.defaultValue?.toString() != "null" && it.defaultValue != null }
+        }
+    }
+
     private fun Discriminator.getDiscriminatorMappings(schemaInfo: SchemaInfo): Map<String, TypeName> =
         mappingKeys(schemaInfo.schema)
             .filter { it.value == schemaInfo.schema.name || it.key == schemaInfo.schema.name }

diff --git a/src/test/resources/examples/companionObject/models/Models.kt b/src/test/resources/examples/companionObject/models/Models.kt
@@ -28,12 +28,11 @@ data class Cat(
     val hunts: Boolean? = null,
     @param:JsonProperty("age")
     @get:JsonProperty("age")
-    val age: Int? = null
-) : Pet(id, name, tag) {
+    val age: Int? = null,
     @get:JsonProperty("petType")
     @get:NotNull
     override val petType: String = "Cat"
-
+) : Pet(id, name, tag) {
     companion object
 }
 
@@ -54,12 +53,11 @@ data class Dog(
     val bark: Boolean? = null,
     @param:JsonProperty("breed")
     @get:JsonProperty("breed")
-    val breed: DogBreed? = null
-) : Pet(id, name, tag) {
+    val breed: DogBreed? = null,
     @get:JsonProperty("petType")
     @get:NotNull
     override val petType: String = "Dog"
-
+) : Pet(id, name, tag) {
     companion object
 }
 

diff --git a/src/test/resources/examples/enumPolymorphicDiscriminator/models/Models.kt b/src/test/resources/examples/enumPolymorphicDiscriminator/models/Models.kt
@@ -12,31 +12,28 @@ import kotlin.collections.Map
 data class ConcreteImplOne(
     @param:JsonProperty("some_prop")
     @get:JsonProperty("some_prop")
-    val someProp: String? = null
-) : PolymorphicEnumDiscriminator() {
+    val someProp: String? = null,
     @get:JsonProperty("some_enum")
     @get:NotNull
     override val someEnum: EnumDiscriminator = EnumDiscriminator.OBJ_ONE_ONLY
-}
+) : PolymorphicEnumDiscriminator()
 
 data class ConcreteImplOneRef(
     @param:JsonProperty("some_prop")
     @get:JsonProperty("some_prop")
-    val someProp: String? = null
-) : PolymorphicEnumDiscriminator() {
+    val someProp: String? = null,
     @get:JsonProperty("some_enum")
     @get:NotNull
     override val someEnum: EnumDiscriminator = EnumDiscriminator.OBJ_ONE_ONLY
-}
+) : PolymorphicEnumDiscriminator()
 
-class ConcreteImplThree() : PolymorphicEnumDiscriminator() {
+data class ConcreteImplThree(
     @get:JsonProperty("some_enum")
     @get:NotNull
     override val someEnum: EnumDiscriminator = EnumDiscriminator.OBJ_THREE
-}
+) : PolymorphicEnumDiscriminator()
 
 data class ConcreteImplTwo(
-    @param:JsonProperty("some_enum")
     @get:JsonProperty("some_enum")
     @get:NotNull
     override val someEnum: EnumDiscriminator,

diff --git a/src/test/resources/examples/externalReferences/models/Models.kt b/src/test/resources/examples/externalReferences/models/Models.kt
@@ -87,22 +87,20 @@ data class ExternalObjectTwo(
 data class OneOfOne(
     @param:JsonProperty("oneOfOne")
     @get:JsonProperty("oneOfOne")
-    val oneOfOne: String? = null
-) : ParentOneOf() {
+    val oneOfOne: String? = null,
     @get:JsonProperty("discriminator")
     @get:NotNull
     override val discriminator: String = "OneOfOne"
-}
+) : ParentOneOf()
 
 data class OneOfTwo(
     @param:JsonProperty("oneOfTwo")
     @get:JsonProperty("oneOfTwo")
-    val oneOfTwo: String? = null
-) : ParentOneOf() {
+    val oneOfTwo: String? = null,
     @get:JsonProperty("discriminator")
     @get:NotNull
     override val discriminator: String = "OneOfTwo"
-}
+) : ParentOneOf()
 
 @JsonTypeInfo(
     use = JsonTypeInfo.Id.NAME,

diff --git a/src/test/resources/examples/javaSerializableModels/models/Models.kt b/src/test/resources/examples/javaSerializableModels/models/Models.kt
@@ -96,12 +96,11 @@ data class FirstModel(
     override val etag: String? = null,
     @param:JsonProperty("extra_first_attr")
     @get:JsonProperty("extra_first_attr")
-    val extraFirstAttr: List<String>? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag), Serializable {
+    val extraFirstAttr: List<String>? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.FIRST_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag), Serializable
 
 data class QueryResult(
     @param:JsonProperty("items")
@@ -133,12 +132,11 @@ data class SecondModel(
     val extraFirstAttr: String? = null,
     @param:JsonProperty("extra_second_attr")
     @get:JsonProperty("extra_second_attr")
-    val extraSecondAttr: Boolean? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag), Serializable {
+    val extraSecondAttr: Boolean? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.SECOND_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag), Serializable
 
 data class ThirdModel(
     @param:JsonProperty("id")
@@ -161,9 +159,8 @@ data class ThirdModel(
     val extraFirstAttr: OffsetDateTime? = null,
     @param:JsonProperty("extra_second_attr")
     @get:JsonProperty("extra_second_attr")
-    val extraSecondAttr: Int? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag), Serializable {
+    val extraSecondAttr: Int? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.THIRD_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag), Serializable
diff --git a/src/test/resources/examples/micronautIntrospectedModels/models/Models.kt b/src/test/resources/examples/micronautIntrospectedModels/models/Models.kt
@@ -100,12 +100,11 @@ data class FirstModel(
     override val etag: String? = null,
     @param:JsonProperty("extra_first_attr")
     @get:JsonProperty("extra_first_attr")
-    val extraFirstAttr: List<String>? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag) {
+    val extraFirstAttr: List<String>? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.FIRST_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag)
 
 @Introspected
 data class QueryResult(
@@ -139,12 +138,11 @@ data class SecondModel(
     val extraFirstAttr: String? = null,
     @param:JsonProperty("extra_second_attr")
     @get:JsonProperty("extra_second_attr")
-    val extraSecondAttr: Boolean? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag) {
+    val extraSecondAttr: Boolean? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.SECOND_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag)
 
 @Introspected
 data class ThirdModel(
@@ -168,9 +166,8 @@ data class ThirdModel(
     val extraFirstAttr: OffsetDateTime? = null,
     @param:JsonProperty("extra_second_attr")
     @get:JsonProperty("extra_second_attr")
-    val extraSecondAttr: Int? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag) {
+    val extraSecondAttr: Int? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.THIRD_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag)
diff --git a/src/test/resources/examples/micronautReflectionModels/models/Models.kt b/src/test/resources/examples/micronautReflectionModels/models/Models.kt
@@ -100,12 +100,11 @@ data class FirstModel(
     override val etag: String? = null,
     @param:JsonProperty("extra_first_attr")
     @get:JsonProperty("extra_first_attr")
-    val extraFirstAttr: List<String>? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag) {
+    val extraFirstAttr: List<String>? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.FIRST_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag)
 
 @ReflectiveAccess
 data class QueryResult(
@@ -139,12 +138,11 @@ data class SecondModel(
     val extraFirstAttr: String? = null,
     @param:JsonProperty("extra_second_attr")
     @get:JsonProperty("extra_second_attr")
-    val extraSecondAttr: Boolean? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag) {
+    val extraSecondAttr: Boolean? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.SECOND_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag)
 
 @ReflectiveAccess
 data class ThirdModel(
@@ -168,9 +166,8 @@ data class ThirdModel(
     val extraFirstAttr: OffsetDateTime? = null,
     @param:JsonProperty("extra_second_attr")
     @get:JsonProperty("extra_second_attr")
-    val extraSecondAttr: Int? = null
-) : Content(id, firstAttr, secondAttr, thirdAttr, etag) {
+    val extraSecondAttr: Int? = null,
     @get:JsonProperty("model_type")
     @get:NotNull
     override val modelType: ContentModelType = ContentModelType.THIRD_MODEL
-}
+) : Content(id, firstAttr, secondAttr, thirdAttr, etag)