Skip to content

Commit

Permalink
Omit discriminator property for Kotlin serialization
Browse files Browse the repository at this point in the history
Generating the discriminator property causes a name conflict with Kotlin Serialization's class discriminator.

Example: `Sealed class 'subclass' cannot be serialized as base class 'com.example.models.Superclass' because it has property name that conflicts with JSON class discriminator 'type'.`
  • Loading branch information
ulrikandersen committed Dec 20, 2024
1 parent 89f6d46 commit 63176bf
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 21 deletions.
6 changes: 6 additions & 0 deletions end2end-tests/models-kotlinx/openapi/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,24 @@ components:
LandlinePhone:
type: object
required:
- type
- number
- area_code
properties:
type:
type: string
number:
type: string
area_code:
type: string
MobilePhone:
type: object
required:
- type
- number
properties:
type:
type: string
number:
type: string
Error:
Expand Down
29 changes: 29 additions & 0 deletions src/main/kotlin/com/cjbooms/fabrikt/generators/GeneratorUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -219,4 +219,33 @@ object GeneratorUtils {
}

private fun isNullable(parameter: Parameter): Boolean = !parameter.isRequired && parameter.schema.default == null

/**
* Converts a TypeSpec to an object by copying over all properties, functions, etc.
*/
fun TypeSpec.toObjectTypeSpec(): TypeSpec {
require(name != null) { "Name must be set to convert to object" }

val objectBuilder = TypeSpec.objectBuilder(name!!)
.addAnnotations(annotations)
.addModifiers(modifiers)
.superclass(superclass)
.addProperties(propertySpecs)
.addFunctions(funSpecs)
.addKdoc(kdoc)

for ((typeName, _) in superinterfaces) {
objectBuilder.addSuperinterface(typeName)
}

if (initializerBlock.isNotEmpty()) {
objectBuilder.addInitializerBlock(initializerBlock)
}

for (nestedType in typeSpecs) {
objectBuilder.addType(nestedType)
}

return objectBuilder.build()
}
}
26 changes: 15 additions & 11 deletions src/main/kotlin/com/cjbooms/fabrikt/generators/PropertyUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,22 @@ object PropertyUtils {
if (isDiscriminatorFieldWithSingleKnownValue(classSettings, schemaName)) {
this as PropertyInfo.Field
if (classSettings.polymorphyType in listOf(ClassSettings.PolymorphyType.SUB, ClassSettings.PolymorphyType.ONE_OF)) {
property.initializer(name)
serializationAnnotations.addParameter(property, oasKey)
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)
if (!serializationAnnotations.supportsBackingPropertyForDiscriminator) {
return // Skip adding the property to the class
} else {
property.initializer(name)
serializationAnnotations.addParameter(property, oasKey)
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())
}
constructorBuilder.addParameter(constructorParameter.build())
}
} else {
property.initializer(name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.cjbooms.fabrikt.cli.ModelCodeGenOptionType.SEALED_INTERFACES_FOR_ONE_
import com.cjbooms.fabrikt.configurations.Packages
import com.cjbooms.fabrikt.generators.ClassSettings
import com.cjbooms.fabrikt.generators.GeneratorUtils.toClassName
import com.cjbooms.fabrikt.generators.GeneratorUtils.toObjectTypeSpec
import com.cjbooms.fabrikt.generators.MutableSettings
import com.cjbooms.fabrikt.generators.PropertyUtils.addToClass
import com.cjbooms.fabrikt.generators.PropertyUtils.isNullable
Expand Down Expand Up @@ -498,7 +499,14 @@ class ModelGenerator(

serializationAnnotations.addClassAnnotation(classBuilder)

return classBuilder.build()
val classTypeSpec = classBuilder.build()

return if (classBuilder.propertySpecs.isNotEmpty()) {
classTypeSpec
} else {
// properties have been filtered out in generation process so return an object instead
classTypeSpec.toObjectTypeSpec()
}
}

private fun polymorphicSuperSubType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeSpec

object JacksonAnnotations : SerializationAnnotations {
override val supportsBackingPropertyForDiscriminator = true
override val supportsAdditionalProperties = true
override fun addIgnore(propertySpecBuilder: PropertySpec.Builder) =
propertySpecBuilder.addAnnotation(JacksonMetadata.ignore)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

object KotlinxSerializationAnnotations : SerializationAnnotations {
/**
* Polymorphic class discriminators are added as annotations in kotlinx serialization.
* Including them in the class definition causes compilation errors since the property name
* will conflict with the class discriminator name.
*/
override val supportsBackingPropertyForDiscriminator = false

/**
* Supporting "additionalProperties: true" for kotlinx serialization requires additional
* research and work due to Any type in the map (val properties: MutableMap<String, Any?>)
Expand All @@ -18,6 +25,7 @@ object KotlinxSerializationAnnotations : SerializationAnnotations {
* See also https://github.com/Kotlin/kotlinx.serialization/issues/1978
*/
override val supportsAdditionalProperties = false

override fun addIgnore(propertySpecBuilder: PropertySpec.Builder) =
propertySpecBuilder // not applicable

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import com.squareup.kotlinpoet.TypeName
import com.squareup.kotlinpoet.TypeSpec

sealed interface SerializationAnnotations {
/**
* Whether to include backing property for a polymorphic discriminator
*/
val supportsBackingPropertyForDiscriminator: Boolean

/**
* Whether the annotation supports OpenAPI's additional properties
* https://spec.openapis.org/oas/v3.0.0.html#model-with-map-dictionary-properties
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
package examples.discriminatedOneOf.models

import javax.validation.constraints.NotNull
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@SerialName("a")
@Serializable
public data class StateA(
@SerialName("status")
@get:NotNull
public val status: Status = Status.A,
) : State
public object StateA : State
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,4 @@ public data class StateB(
@SerialName("mode")
@get:NotNull
public val mode: StateBMode,
@SerialName("status")
@get:NotNull
public val status: Status = Status.B,
) : State

0 comments on commit 63176bf

Please sign in to comment.