From 1222719764749b96406c0628f4451fa8c43e9494 Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Tue, 5 Mar 2024 11:19:04 +0000 Subject: [PATCH] Fix inline schema definitions inherited from a parent. (#269) * Fix inline schema definitions inherited from a parent. When an inline schema is inherited, the child should reference the parent definition, and not build ain incompatible child version * Add test for enum --- .../generators/model/JacksonModelGenerator.kt | 47 +++++++----- .../com/cjbooms/fabrikt/model/PropertyInfo.kt | 12 ++- .../enumPolymorphicDiscriminator/api.yaml | 18 +++++ .../models/Models.kt | 74 ++++++++++++++++++- 4 files changed, 125 insertions(+), 26 deletions(-) diff --git a/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt b/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt index 9e42ef11..aff2a0f9 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt @@ -282,16 +282,20 @@ class JacksonModelGenerator( } else { when (it) { is PropertyInfo.ObjectInlinedField -> { - val props = it.schema.topLevelProperties(HTTP_SETTINGS, enclosingSchema) - val currentModel = standardDataClass( - ModelNameRegistry.getOrRegister(it.schema, enclosingSchema.toEnclosingSchemaInfo()), - it.name, - props, - it.schema.extensions, - oneOfInterfaces = emptySet(), - ) - val inlinedModels = buildInLinedModels(props, enclosingSchema, apiDocUrl) - inlinedModels + currentModel + if (it.isInherited) { + emptySet() // Rely on the parent definition + } else { + val props = it.schema.topLevelProperties(HTTP_SETTINGS, enclosingSchema) + val currentModel = standardDataClass( + ModelNameRegistry.getOrRegister(it.schema, enclosingSchema.toEnclosingSchemaInfo()), + it.name, + props, + it.schema.extensions, + oneOfInterfaces = emptySet(), + ) + val inlinedModels = buildInLinedModels(props, enclosingSchema, apiDocUrl) + inlinedModels + currentModel + } } is PropertyInfo.ObjectRefField -> emptySet() // Not an inlined definition, so do nothing @@ -314,15 +318,18 @@ class JacksonModelGenerator( } is PropertyInfo.Field -> - if (it.typeInfo is KotlinTypeInfo.Enum) { + if (it.typeInfo is KotlinTypeInfo.Enum && !it.isInherited) { setOf(buildEnumClass(it.typeInfo as KotlinTypeInfo.Enum)) } else { emptySet() } is PropertyInfo.ListField -> - buildInlinedListDefinition(it.schema, it.name, enclosingSchema, apiDocUrl) - + if (it.isInherited) { + emptySet() // Rely on the parent definition + } else { + buildInlinedListDefinition(it.schema, it.name, enclosingSchema, apiDocUrl) + } is PropertyInfo.OneOfAny -> emptySet() } } @@ -707,20 +714,20 @@ class JacksonModelGenerator( ): TypeSpec.Builder { this.forEach { it.addToClass( - schemaName, - toModelType( + schemaName = schemaName, + type = toModelType( packages.base, it.typeInfo, it.isNullable(), ), - toClassName( + parameterizedType = toClassName( packages.base, it.typeInfo, ), - classBuilder, - constructorBuilder, - classType, - validationAnnotations, + classBuilder = classBuilder, + constructorBuilder = constructorBuilder, + classSettings = classType, + validationAnnotations = validationAnnotations, ) } if (constructorBuilder.parameters.isNotEmpty() && classBuilder.modifiers.isEmpty()) { diff --git a/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt b/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt index 75842a31..b59827f9 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt @@ -186,7 +186,11 @@ sealed class PropertyInfo { val enclosingSchema: Schema? ) : PropertyInfo(), CollectionValidation { override val typeInfo: KotlinTypeInfo = - KotlinTypeInfo.from(schema, oasKey, enclosingSchema?.toEnclosingSchemaInfo()) + if (isInherited) { + KotlinTypeInfo.from(schema, oasKey, parentSchema.toEnclosingSchemaInfo()) + } else { + KotlinTypeInfo.from(schema, oasKey, enclosingSchema?.toEnclosingSchemaInfo()) + } override val minItems: Int? = schema.minItems override val maxItems: Int? = schema.maxItems } @@ -220,7 +224,11 @@ sealed class PropertyInfo { val enclosingSchema: Schema? ) : PropertyInfo() { override val typeInfo: KotlinTypeInfo = - KotlinTypeInfo.from(schema, oasKey, enclosingSchema?.toEnclosingSchemaInfo()) + if (isInherited) { + KotlinTypeInfo.from(schema, oasKey, parentSchema.toEnclosingSchemaInfo()) + } else { + KotlinTypeInfo.from(schema, oasKey, enclosingSchema?.toEnclosingSchemaInfo()) + } } data class AdditionalProperties( diff --git a/src/test/resources/examples/enumPolymorphicDiscriminator/api.yaml b/src/test/resources/examples/enumPolymorphicDiscriminator/api.yaml index f90bdc27..0b46de9d 100644 --- a/src/test/resources/examples/enumPolymorphicDiscriminator/api.yaml +++ b/src/test/resources/examples/enumPolymorphicDiscriminator/api.yaml @@ -17,6 +17,24 @@ components: properties: some_enum: $ref: '#/components/schemas/ChildDiscriminator' + inline_obj: + type: object + properties: + str: + type: string + inline_array: + type: array + items: + type: object + properties: + str: + type: string + inline_enum: + type: string + enum: + - one + - two + - three ChildDiscriminator: type: string diff --git a/src/test/resources/examples/enumPolymorphicDiscriminator/models/Models.kt b/src/test/resources/examples/enumPolymorphicDiscriminator/models/Models.kt index a7c9aac4..d3044f8a 100644 --- a/src/test/resources/examples/enumPolymorphicDiscriminator/models/Models.kt +++ b/src/test/resources/examples/enumPolymorphicDiscriminator/models/Models.kt @@ -34,10 +34,43 @@ import kotlin.collections.Map ), JsonSubTypes.Type(value = DiscriminatedChild3::class, name = "obj_three"), ) -public sealed class ChildDefinition() { +public sealed class ChildDefinition( + public open val inlineObj: ChildDefinitionInlineObj? = null, + public open val inlineArray: List? = null, + public open val inlineEnum: ChildDefinitionInlineEnum? = null, +) { public abstract val someEnum: ChildDiscriminator } +public data class ChildDefinitionInlineArray( + @param:JsonProperty("str") + @get:JsonProperty("str") + public val str: String? = null, +) + +public enum class ChildDefinitionInlineEnum( + @JsonValue + public val `value`: String, +) { + ONE("one"), + TWO("two"), + THREE("three"), + ; + + public companion object { + private val mapping: Map = + values().associateBy(ChildDefinitionInlineEnum::value) + + public fun fromValue(`value`: String): ChildDefinitionInlineEnum? = mapping[value] + } +} + +public data class ChildDefinitionInlineObj( + @param:JsonProperty("str") + @get:JsonProperty("str") + public val str: String? = null, +) + public enum class ChildDiscriminator( @JsonValue public val `value`: String, @@ -57,6 +90,17 @@ public enum class ChildDiscriminator( } public data class DiscriminatedChild1( + @param:JsonProperty("inline_obj") + @get:JsonProperty("inline_obj") + @get:Valid + override val inlineObj: ChildDefinitionInlineObj? = null, + @param:JsonProperty("inline_array") + @get:JsonProperty("inline_array") + @get:Valid + override val inlineArray: List? = null, + @param:JsonProperty("inline_enum") + @get:JsonProperty("inline_enum") + override val inlineEnum: ChildDefinitionInlineEnum? = null, @param:JsonProperty("some_prop") @get:JsonProperty("some_prop") public val someProp: String? = null, @@ -64,23 +108,45 @@ public data class DiscriminatedChild1( @get:NotNull @param:JsonProperty("some_enum") override val someEnum: ChildDiscriminator = ChildDiscriminator.OBJ_ONE_ONLY, -) : ChildDefinition() +) : ChildDefinition(inlineObj, inlineArray, inlineEnum) public data class DiscriminatedChild2( @get:JsonProperty("some_enum") @get:NotNull override val someEnum: ChildDiscriminator, + @param:JsonProperty("inline_obj") + @get:JsonProperty("inline_obj") + @get:Valid + override val inlineObj: ChildDefinitionInlineObj? = null, + @param:JsonProperty("inline_array") + @get:JsonProperty("inline_array") + @get:Valid + override val inlineArray: List? = null, + @param:JsonProperty("inline_enum") + @get:JsonProperty("inline_enum") + override val inlineEnum: ChildDefinitionInlineEnum? = null, @param:JsonProperty("some_prop") @get:JsonProperty("some_prop") public val someProp: String? = null, -) : ChildDefinition() +) : ChildDefinition(inlineObj, inlineArray, inlineEnum) public data class DiscriminatedChild3( + @param:JsonProperty("inline_obj") + @get:JsonProperty("inline_obj") + @get:Valid + override val inlineObj: ChildDefinitionInlineObj? = null, + @param:JsonProperty("inline_array") + @get:JsonProperty("inline_array") + @get:Valid + override val inlineArray: List? = null, + @param:JsonProperty("inline_enum") + @get:JsonProperty("inline_enum") + override val inlineEnum: ChildDefinitionInlineEnum? = null, @get:JsonProperty("some_enum") @get:NotNull @param:JsonProperty("some_enum") override val someEnum: ChildDiscriminator = ChildDiscriminator.OBJ_THREE, -) : ChildDefinition() +) : ChildDefinition(inlineObj, inlineArray, inlineEnum) public data class Responses( @param:JsonProperty("entries")