From 680560284d298f6c0ed43b664981355a317c33cf Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Fri, 1 Mar 2024 22:02:13 +0000 Subject: [PATCH 1/4] Do not strip discriminator property in oneOf generation. --- .../generators/model/JacksonModelGenerator.kt | 32 ++++++++----------- .../fabrikt/generators/ModelGeneratorTest.kt | 4 +-- 2 files changed, 15 insertions(+), 21 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 aff2a0f9..139f88cd 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt @@ -165,7 +165,10 @@ class JacksonModelGenerator( .filterNot { it.schema.isOneOfPolymorphicTypes() } .flatMap { val properties = it.schema.topLevelProperties(HTTP_SETTINGS, it.schema) - if (properties.isNotEmpty() || it.typeInfo is KotlinTypeInfo.Enum) { + if (properties.isNotEmpty() || + it.typeInfo is KotlinTypeInfo.Enum || + findOneOfSuperInterface(schemas, it, options).isNotEmpty() + ) { val primaryModel = buildPrimaryModel(api, it, properties, schemas) val inlinedModels = buildInLinedModels(properties, it.schema, it.schema.getDocumentUrl()) listOf(primaryModel) + inlinedModels @@ -469,22 +472,8 @@ class JacksonModelGenerator( extensions: Map, oneOfInterfaces: Set, ): TypeSpec { - val filteredProperties = if (oneOfInterfaces.size == 1) { - val oneOfInterface = oneOfInterfaces.first() - val discriminatorProp = oneOfInterface.schema.discriminator?.propertyName - val mappingCount = - oneOfInterface.schema.discriminator?.mappings?.values?.count { it.endsWith("/$modelName") } - if (discriminatorProp != null && mappingCount == 1) { - properties.filterNot { it.name == discriminatorProp } - } else { - properties - } - } else { - properties - } - val name = generatedType(packages.base, modelName) - val generateObject = properties.isNotEmpty() && filteredProperties.isEmpty() + val generateObject = properties.isEmpty() val builder = if (generateObject) { TypeSpec.objectBuilder(name) @@ -503,7 +492,7 @@ class JacksonModelGenerator( } if (!generateObject) { - filteredProperties.addToClass( + properties.addToClass( schemaName = schemaName, classBuilder = classBuilder, classType = ClassSettings(ClassSettings.PolymorphyType.NONE, extensions.hasJsonMergePatchExtension), @@ -809,9 +798,14 @@ class JacksonModelGenerator( private fun List.filterByExternalRefResolutionMode( externalReferences: Map.Entry>, ) = when (externalRefResolutionMode) { - ExternalReferencesResolutionMode.TARGETED -> this.filter { apiSchema -> externalReferences.value.contains(apiSchema.name) } - else -> this + ExternalReferencesResolutionMode.TARGETED -> this.filter { apiSchema -> + externalReferences.value.contains( + apiSchema.name + ) } + + else -> this + } } private val Map.hasJsonMergePatchExtension diff --git a/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt b/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt index 2b26c8c3..c9f67ff5 100644 --- a/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt +++ b/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt @@ -65,8 +65,8 @@ class ModelGeneratorTest { ModelNameRegistry.clear() } - // @Test - // fun `debug single test`() = `correct models are generated for different OpenApi Specifications`("insert test case") + @Test + fun `debug single test`() = `correct models are generated for different OpenApi Specifications`("discriminatedOneOf") @ParameterizedTest @MethodSource("testCases") From 7a2ed04a81fe4f3874ac6746c6b80b6d2afe0f3e Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Fri, 1 Mar 2024 22:05:32 +0000 Subject: [PATCH 2/4] revert test and formatting changes --- .../fabrikt/generators/model/JacksonModelGenerator.kt | 9 ++------- .../com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt | 4 ++-- 2 files changed, 4 insertions(+), 9 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 139f88cd..831781be 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt @@ -798,14 +798,9 @@ class JacksonModelGenerator( private fun List.filterByExternalRefResolutionMode( externalReferences: Map.Entry>, ) = when (externalRefResolutionMode) { - ExternalReferencesResolutionMode.TARGETED -> this.filter { apiSchema -> - externalReferences.value.contains( - apiSchema.name - ) + ExternalReferencesResolutionMode.TARGETED -> this.filter { apiSchema -> externalReferences.value.contains(apiSchema.name) } + else -> this } - - else -> this - } } private val Map.hasJsonMergePatchExtension diff --git a/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt b/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt index c9f67ff5..2b26c8c3 100644 --- a/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt +++ b/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt @@ -65,8 +65,8 @@ class ModelGeneratorTest { ModelNameRegistry.clear() } - @Test - fun `debug single test`() = `correct models are generated for different OpenApi Specifications`("discriminatedOneOf") + // @Test + // fun `debug single test`() = `correct models are generated for different OpenApi Specifications`("insert test case") @ParameterizedTest @MethodSource("testCases") From d260c2bf445faf5460df11a1277b501197be65bf Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Fri, 1 Mar 2024 22:07:38 +0000 Subject: [PATCH 3/4] correct test --- .../examples/discriminatedOneOf/models/Models.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/test/resources/examples/discriminatedOneOf/models/Models.kt b/src/test/resources/examples/discriminatedOneOf/models/Models.kt index 7b73d7f4..43cc27c8 100644 --- a/src/test/resources/examples/discriminatedOneOf/models/Models.kt +++ b/src/test/resources/examples/discriminatedOneOf/models/Models.kt @@ -33,9 +33,19 @@ public data class SomeObj( ) public sealed interface State -public object StateA : State +public data class StateA( + @param:JsonProperty("status") + @get:JsonProperty("status") + @get:NotNull + public val status: Status, +) : State -public object StateB : State +public data class StateB( + @param:JsonProperty("status") + @get:JsonProperty("status") + @get:NotNull + public val status: Status, +) : State public enum class Status( @JsonValue From 0eb7b7cdb2a3c18054d84d7e5a7bd47a0ef3997d Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Mon, 4 Mar 2024 21:57:28 +0000 Subject: [PATCH 4/4] Change oneOf so that it defaults to the correct discriminator value --- .../fabrikt/generators/PropertyUtils.kt | 8 +- .../generators/model/JacksonModelGenerator.kt | 87 ++++++++----------- .../com/cjbooms/fabrikt/model/PropertyInfo.kt | 22 +++-- .../fabrikt/util/KaizenParserExtensions.kt | 53 ++++++++++- .../fabrikt/generators/ModelGeneratorTest.kt | 3 + .../discriminatedOneOf/models/Models.kt | 8 +- 6 files changed, 110 insertions(+), 71 deletions(-) diff --git a/src/main/kotlin/com/cjbooms/fabrikt/generators/PropertyUtils.kt b/src/main/kotlin/com/cjbooms/fabrikt/generators/PropertyUtils.kt index f23b6721..1a313d6a 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/generators/PropertyUtils.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/generators/PropertyUtils.kt @@ -21,6 +21,7 @@ data class ClassSettings( NONE, SUPER, SUB, + ONE_OF, } } @@ -102,11 +103,16 @@ object PropertyUtils { property.addAnnotation(JacksonMetadata.jacksonPropertyAnnotation(oasKey)) property.addValidationAnnotations(this, validationAnnotations) } + + ClassSettings.PolymorphyType.ONE_OF -> { + property.addAnnotation(JacksonMetadata.jacksonPropertyAnnotation(oasKey)) + property.addValidationAnnotations(this, validationAnnotations) + } } if (isDiscriminatorFieldWithSingleKnownValue(classSettings, schemaName)) { this as PropertyInfo.Field - if (classSettings.polymorphyType == ClassSettings.PolymorphyType.SUB) { + if (classSettings.polymorphyType in listOf(ClassSettings.PolymorphyType.SUB, ClassSettings.PolymorphyType.ONE_OF)) { property.initializer(name) property.addAnnotation(JacksonMetadata.jacksonParameterAnnotation(oasKey)) val constructorParameter: ParameterSpec.Builder = ParameterSpec.builder(name, wrappedType) 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 831781be..2fc6953d 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/generators/model/JacksonModelGenerator.kt @@ -29,6 +29,7 @@ import com.cjbooms.fabrikt.model.PropertyInfo.Companion.topLevelProperties import com.cjbooms.fabrikt.model.SchemaInfo import com.cjbooms.fabrikt.model.SourceApi import com.cjbooms.fabrikt.model.toEnclosingSchemaInfo +import com.cjbooms.fabrikt.util.KaizenParserExtensions.findOneOfSuperInterface import com.cjbooms.fabrikt.util.KaizenParserExtensions.getDiscriminatorForInLinedObjectUnderAllOf import com.cjbooms.fabrikt.util.KaizenParserExtensions.getSchemaRefName import com.cjbooms.fabrikt.util.KaizenParserExtensions.getSuperType @@ -164,10 +165,10 @@ class JacksonModelGenerator( .filterNot { it.schema.isSimpleType() } .filterNot { it.schema.isOneOfPolymorphicTypes() } .flatMap { - val properties = it.schema.topLevelProperties(HTTP_SETTINGS, it.schema) + val properties = it.schema.topLevelProperties(HTTP_SETTINGS, api, it.schema) if (properties.isNotEmpty() || it.typeInfo is KotlinTypeInfo.Enum || - findOneOfSuperInterface(schemas, it, options).isNotEmpty() + it.schema.findOneOfSuperInterface(schemas.map { it.schema }).isNotEmpty() ) { val primaryModel = buildPrimaryModel(api, it, properties, schemas) val inlinedModels = buildInLinedModels(properties, it.schema, it.schema.getDocumentUrl()) @@ -199,7 +200,7 @@ class JacksonModelGenerator( schemaInfo.schema.discriminator, allSchemas, schemaInfo.schema.oneOfSchemas, - findOneOfSuperInterface(allSchemas, schemaInfo, options), + schemaInfo.schema.findOneOfSuperInterface(allSchemas.map { it.schema }), ) schemaInfo.schema.isPolymorphicSuperType() && schemaInfo.schema.isPolymorphicSubType(api) -> @@ -210,7 +211,7 @@ class JacksonModelGenerator( checkNotNull(schemaInfo.schema.getDiscriminatorForInLinedObjectUnderAllOf()), schemaInfo.schema.getSuperType(api)!!.let { SchemaInfo(it.name, it) }, schemaInfo.schema.extensions, - findOneOfSuperInterface(allSchemas, schemaInfo, options), + schemaInfo.schema.findOneOfSuperInterface(allSchemas.map { it.schema }), allSchemas, ) @@ -220,7 +221,7 @@ class JacksonModelGenerator( properties, schemaInfo.schema.discriminator, schemaInfo.schema.extensions, - findOneOfSuperInterface(allSchemas, schemaInfo, options), + schemaInfo.schema.findOneOfSuperInterface(allSchemas.map { it.schema }), allSchemas, ) @@ -230,7 +231,7 @@ class JacksonModelGenerator( properties, schemaInfo.schema.getSuperType(api)!!.let { SchemaInfo(it.name, it) }, schemaInfo.schema.extensions, - findOneOfSuperInterface(allSchemas, schemaInfo, options), + schemaInfo.schema.findOneOfSuperInterface(allSchemas.map { it.schema }), ) schemaInfo.typeInfo is KotlinTypeInfo.Enum -> buildEnumClass(schemaInfo.typeInfo) @@ -239,40 +240,12 @@ class JacksonModelGenerator( schemaName = schemaName, properties = properties, extensions = schemaInfo.schema.extensions, - oneOfInterfaces = findOneOfSuperInterface(allSchemas, schemaInfo, options), + oneOfInterfaces = schemaInfo.schema.findOneOfSuperInterface(allSchemas.map { it.schema }), ) } } - private fun findOneOfSuperInterface( - allSchemas: List, - schema: SchemaInfo, - options: Set, - ): Set { - if (SEALED_INTERFACES_FOR_ONE_OF !in options) { - return emptySet() - } - return allSchemas - .filter { it.schema.discriminator != null && it.schema.oneOfSchemas.isNotEmpty() } - .mapNotNull { info -> - info.schema.discriminator.mappings - .toList() - .find { (_, ref) -> - ref.endsWith("/${schema.name}") - } - ?.let { (key, _) -> - Pair(key!!, info) - } - } - .map { (_, parent) -> - val field = parent.schema.discriminator.propertyName!! - if (!schema.schema.properties.containsKey(field)) { - throw IllegalArgumentException("schema $schema did not have discriminator property") - } - parent - } - .toSet() - } + private fun buildInLinedModels( topLevelProperties: Collection, @@ -288,7 +261,7 @@ class JacksonModelGenerator( if (it.isInherited) { emptySet() // Rely on the parent definition } else { - val props = it.schema.topLevelProperties(HTTP_SETTINGS, enclosingSchema) + val props = it.schema.topLevelProperties(HTTP_SETTINGS, sourceApi.openApi3, enclosingSchema) val currentModel = standardDataClass( ModelNameRegistry.getOrRegister(it.schema, enclosingSchema.toEnclosingSchemaInfo()), it.name, @@ -311,7 +284,7 @@ class JacksonModelGenerator( standardDataClass( modelName = ModelNameRegistry.getOrRegister(it.schema, valueSuffix = it.schema.isInlinedTypedAdditionalProperties()), schemaName = it.name, - properties = it.schema.topLevelProperties(HTTP_SETTINGS, enclosingSchema), + properties = it.schema.topLevelProperties(HTTP_SETTINGS, sourceApi.openApi3, enclosingSchema), extensions = it.schema.extensions, oneOfInterfaces = emptySet(), ), @@ -350,7 +323,7 @@ class JacksonModelGenerator( ?: enclosingSchema.toEnclosingSchemaInfo() when { items.isInlinedObjectDefinition() -> - items.topLevelProperties(HTTP_SETTINGS, enclosingSchema).let { props -> + items.topLevelProperties(HTTP_SETTINGS, sourceApi.openApi3, enclosingSchema).let { props -> buildInLinedModels( topLevelProperties = props, enclosingSchema = enclosingSchema, @@ -457,7 +430,7 @@ class JacksonModelGenerator( standardDataClass( modelName = ModelNameRegistry.getOrRegister(schema, valueSuffix = schema.isInlinedTypedAdditionalProperties()), schemaName = schema.safeName(), - properties = mapField.schema.additionalPropertiesSchema.topLevelProperties(HTTP_SETTINGS), + properties = mapField.schema.additionalPropertiesSchema.topLevelProperties(HTTP_SETTINGS, sourceApi.openApi3), extensions = mapField.schema.extensions, oneOfInterfaces = emptySet(), ) @@ -470,7 +443,7 @@ class JacksonModelGenerator( schemaName: String, properties: Collection, extensions: Map, - oneOfInterfaces: Set, + oneOfInterfaces: Set, ): TypeSpec { val name = generatedType(packages.base, modelName) val generateObject = properties.isEmpty() @@ -488,15 +461,23 @@ class JacksonModelGenerator( .addCompanionObject() for (oneOfInterface in oneOfInterfaces) { classBuilder - .addSuperinterface(generatedType(packages.base, ModelNameRegistry.getOrRegister(oneOfInterface.schema))) + .addSuperinterface(generatedType(packages.base, ModelNameRegistry.getOrRegister(oneOfInterface))) } if (!generateObject) { - properties.addToClass( - schemaName = schemaName, - classBuilder = classBuilder, - classType = ClassSettings(ClassSettings.PolymorphyType.NONE, extensions.hasJsonMergePatchExtension), - ) + if (oneOfInterfaces.size == 1) { + properties.addToClass( + schemaName = schemaName, + classBuilder = classBuilder, + classType = ClassSettings(ClassSettings.PolymorphyType.ONE_OF, extensions.hasJsonMergePatchExtension), + ) + } else { + properties.addToClass( + schemaName = schemaName, + classBuilder = classBuilder, + classType = ClassSettings(ClassSettings.PolymorphyType.NONE, extensions.hasJsonMergePatchExtension), + ) + } } return classBuilder.build() } @@ -508,7 +489,7 @@ class JacksonModelGenerator( discriminator: Discriminator, superType: SchemaInfo, extensions: Map, - oneOfSuperInterfaces: Set, + oneOfSuperInterfaces: Set, allSchemas: List, ): TypeSpec = with(FunSpec.constructorBuilder()) { TypeSpec.classBuilder(generatedType(packages.base, modelName)) @@ -538,7 +519,7 @@ class JacksonModelGenerator( discriminator: Discriminator, allSchemas: List, members: List, - oneOfSuperInterfaces: Set, + oneOfSuperInterfaces: Set, ): TypeSpec { val interfaceBuilder = TypeSpec.interfaceBuilder(generatedType(packages.base, modelName)) .addModifiers(KModifier.SEALED) @@ -578,7 +559,7 @@ class JacksonModelGenerator( properties: Collection, discriminator: Discriminator, extensions: Map, - oneOfSuperInterfaces: Set, + oneOfSuperInterfaces: Set, allSchemas: List, ): TypeSpec = TypeSpec.classBuilder(generatedType(packages.base, modelName)) .buildPolymorphicSuperType( @@ -598,7 +579,7 @@ class JacksonModelGenerator( properties: Collection, discriminator: Discriminator, extensions: Map, - oneOfSuperInterfaces: Set, + oneOfSuperInterfaces: Set, allSchemas: List, constructorBuilder: FunSpec.Builder = FunSpec.constructorBuilder(), ): TypeSpec.Builder { @@ -650,7 +631,7 @@ class JacksonModelGenerator( properties: Collection, superType: SchemaInfo, extensions: Map, - oneOfSuperInterfaces: Set, + oneOfSuperInterfaces: Set, ): TypeSpec = TypeSpec.classBuilder(generatedType(packages.base, modelName)) .buildPolymorphicSubType(schemaName, properties, superType, extensions, oneOfSuperInterfaces).build() @@ -659,7 +640,7 @@ class JacksonModelGenerator( allProperties: Collection, superType: SchemaInfo, extensions: Map, - oneOfSuperInterfaces: Set, + oneOfSuperInterfaces: Set, constructorBuilder: FunSpec.Builder = FunSpec.constructorBuilder(), ): TypeSpec.Builder { this.addSerializableInterface() diff --git a/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt b/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt index b59827f9..8315e5c7 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/model/PropertyInfo.kt @@ -15,6 +15,7 @@ import com.cjbooms.fabrikt.util.KaizenParserExtensions.safeName import com.cjbooms.fabrikt.util.KaizenParserExtensions.safeType import com.cjbooms.fabrikt.util.NormalisedString.camelCase import com.cjbooms.fabrikt.util.NormalisedString.toEnumName +import com.reprezen.kaizen.oasparser.model3.OpenApi3 import com.reprezen.kaizen.oasparser.model3.Schema sealed class PropertyInfo { @@ -38,7 +39,7 @@ sealed class PropertyInfo { val HTTP_SETTINGS = Settings() - fun Schema.topLevelProperties(settings: Settings, enclosingSchema: Schema? = null): Collection { + fun Schema.topLevelProperties(settings: Settings, api: OpenApi3, enclosingSchema: Schema? = null): Collection { val results = mutableListOf() + allOfSchemas.flatMap { it.topLevelProperties( @@ -47,12 +48,13 @@ sealed class PropertyInfo { enclosingSchema, it ), + api, this ) } + (if (oneOfSchemas.isEmpty()) emptyList() else listOf(OneOfAny(oneOfSchemas.first()))) + - anyOfSchemas.flatMap { it.topLevelProperties(settings.copy(markAllOptional = true), this) } + - getInLinedProperties(settings, enclosingSchema) + anyOfSchemas.flatMap { it.topLevelProperties(settings.copy(markAllOptional = true), api, this) } + + getInLinedProperties(settings, api, enclosingSchema) return results.distinctBy { it.oasKey } } @@ -68,13 +70,14 @@ sealed class PropertyInfo { private fun Schema.getInLinedProperties( settings: Settings, + api: OpenApi3, enclosingSchema: Schema? = null ): Collection { val mainProperties: List = properties.map { property -> when (property.value.safeType()) { OasType.Array.type -> ListField( - isRequired(property, settings.markReadWriteOnlyOptional, settings.markAllOptional), + isRequired(api, property, settings.markReadWriteOnlyOptional, settings.markAllOptional), property.key, property.value, settings.markAsInherited, @@ -87,6 +90,7 @@ sealed class PropertyInfo { if (property.value.isSimpleMapDefinition() || property.value.isSchemaLess()) MapField( isRequired = isRequired( + api, property, settings.markReadWriteOnlyOptional, settings.markAllOptional @@ -99,7 +103,7 @@ sealed class PropertyInfo { else if (property.value.isInlinedObjectDefinition()) ObjectInlinedField( isRequired = isRequired( - property, settings.markReadWriteOnlyOptional, settings.markAllOptional + api, property, settings.markReadWriteOnlyOptional, settings.markAllOptional ), oasKey = property.key, schema = property.value, @@ -109,7 +113,7 @@ sealed class PropertyInfo { ) else ObjectRefField( - isRequired(property, settings.markReadWriteOnlyOptional, settings.markAllOptional), + isRequired(api, property, settings.markReadWriteOnlyOptional, settings.markAllOptional), property.key, property.value, settings.markAsInherited, @@ -120,13 +124,13 @@ sealed class PropertyInfo { null } else { Field( - isRequired(property, settings.markReadWriteOnlyOptional, settings.markAllOptional), + isRequired(api, property, settings.markReadWriteOnlyOptional, settings.markAllOptional), oasKey = property.key, schema = property.value, isInherited = settings.markAsInherited, - isPolymorphicDiscriminator = isDiscriminatorProperty(property), + isPolymorphicDiscriminator = isDiscriminatorProperty(api, property), maybeDiscriminator = enclosingSchema?.let { - this.getKeyIfSingleDiscriminatorValue(property, it) + this.getKeyIfSingleDiscriminatorValue(api, property, it) }, enclosingSchema = if (property.value.isInlinedEnumDefinition()) this else null ) diff --git a/src/main/kotlin/com/cjbooms/fabrikt/util/KaizenParserExtensions.kt b/src/main/kotlin/com/cjbooms/fabrikt/util/KaizenParserExtensions.kt index 26d8fb08..6cab6c81 100644 --- a/src/main/kotlin/com/cjbooms/fabrikt/util/KaizenParserExtensions.kt +++ b/src/main/kotlin/com/cjbooms/fabrikt/util/KaizenParserExtensions.kt @@ -138,6 +138,7 @@ object KaizenParserExtensions { api.schemas.values.firstOrNull { it.name == safeName() } fun Schema.isRequired( + api: OpenApi3, prop: Map.Entry, markReadWriteOnlyOptional: Boolean, markAllOptional: Boolean, @@ -145,19 +146,50 @@ object KaizenParserExtensions { if (markAllOptional || (prop.value.isReadOnly && markReadWriteOnlyOptional) || (prop.value.isWriteOnly && markReadWriteOnlyOptional)) { false } else { - requiredFields.contains(prop.key) || isDiscriminatorProperty(prop) // A discriminator property should be required + requiredFields.contains(prop.key) || isDiscriminatorProperty(api, prop) // A discriminator property should be required } fun Schema.getSchemaRefName() = Overlay.of(this).jsonReference.split("/").last() - fun Schema.isDiscriminatorProperty(prop: Map.Entry): Boolean = - discriminator?.propertyName == prop.key + fun Schema.isDiscriminatorProperty(api: OpenApi3, prop: Map.Entry): Boolean = + discriminator?.propertyName == prop.key || + findOneOfSuperInterface(api.schemas.values.toList()).any { oneOf -> + oneOf.discriminator?.mappings?.values?.any { it.endsWith("/$name") } ?: false + } + + fun Schema.findOneOfSuperInterface(allSchemas: List): Set { + if (ModelCodeGenOptionType.SEALED_INTERFACES_FOR_ONE_OF !in MutableSettings.modelOptions()) { + return emptySet() + } + return allSchemas + .filter { it.discriminator != null && it.oneOfSchemas.isNotEmpty() } + .mapNotNull { schema -> + schema.discriminator.mappings + .toList() + .find { (_, ref) -> + ref.endsWith("/${name}") + } + ?.let { (key, _) -> + Pair(key!!, schema) + } + } + .map { (_, parent) -> + val field = parent.discriminator.propertyName!! + if (!properties.containsKey(field)) { + throw IllegalArgumentException("schema $name did not have discriminator property") + } + parent + } + .toSet() + } fun Schema.getKeyIfSingleDiscriminatorValue( + api: OpenApi3, prop: Map.Entry, enclosingSchema: Schema, ): Map? = - if (isDiscriminatorProperty(prop) && discriminator.mappingKeys(enclosingSchema).isNotEmpty()) { + if (isDiscriminatorProperty(api, prop)) { + val discriminator = findDiscriminator(api) discriminator.mappingKeys(enclosingSchema).map { if (prop.value.isEnumDefinition()) { it.key to PropertyInfo.DiscriminatorKey.EnumKey(it.key, it.value) @@ -169,6 +201,18 @@ object KaizenParserExtensions { null } + private fun Schema.findDiscriminator(api: OpenApi3): Discriminator { + val bestDiscriminator = if (this.hasDiscriminator()) { + this.discriminator + } else { + val oneOfDiscriminator = findOneOfSuperInterface(api.schemas.values.toList()).firstOrNull { oneOfInterface -> + oneOfInterface.hasDiscriminator() + }?.discriminator + oneOfDiscriminator ?: this.discriminator + } + return bestDiscriminator + } + fun Discriminator.mappingKeys(enclosingSchema: Schema): Map { val discriminatorMappings = mappings?.map { it.key to it.value.split("/").last() }?.toMap() return if (discriminatorMappings.isNullOrEmpty()) { @@ -187,6 +231,7 @@ object KaizenParserExtensions { } fun Schema.hasNoDiscriminator(): Boolean = this.discriminator.propertyName == null + fun Schema.hasDiscriminator(): Boolean = !hasNoDiscriminator() fun Schema.safeName(): String = when { diff --git a/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt b/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt index 2b26c8c3..941b8980 100644 --- a/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt +++ b/src/test/kotlin/com/cjbooms/fabrikt/generators/ModelGeneratorTest.kt @@ -76,6 +76,9 @@ class ModelGeneratorTest { if (testCaseName == "instantDateTime") { MutableSettings.addOption(CodeGenTypeOverride.DATETIME_AS_INSTANT) } + if (testCaseName == "discriminatedOneOf") { + MutableSettings.addOption(ModelCodeGenOptionType.SEALED_INTERFACES_FOR_ONE_OF) + } val basePackage = "examples.${testCaseName.replace("/", ".")}" val apiLocation = javaClass.getResource("/examples/$testCaseName/api.yaml")!! val sourceApi = SourceApi(apiLocation.readText(), baseDir = Paths.get(apiLocation.toURI())) diff --git a/src/test/resources/examples/discriminatedOneOf/models/Models.kt b/src/test/resources/examples/discriminatedOneOf/models/Models.kt index 43cc27c8..3b2ffdc4 100644 --- a/src/test/resources/examples/discriminatedOneOf/models/Models.kt +++ b/src/test/resources/examples/discriminatedOneOf/models/Models.kt @@ -34,17 +34,17 @@ public data class SomeObj( public sealed interface State public data class StateA( - @param:JsonProperty("status") @get:JsonProperty("status") @get:NotNull - public val status: Status, + @param:JsonProperty("status") + public val status: Status = Status.A, ) : State public data class StateB( - @param:JsonProperty("status") @get:JsonProperty("status") @get:NotNull - public val status: Status, + @param:JsonProperty("status") + public val status: Status = Status.B, ) : State public enum class Status(