When using sealed classes: Discriminator not included in serialized object

[ulrikandersen]

Hi,

When using fabrikt with SEALED_INTERFACES_FOR_ONE_OF I am experiencing issues with the serialised object: It does not contain the discriminator property.

The issue exists in when using fabrikt in my own code, but I have also tried using the models from the example and see the same issues:

• The generated JSON does not contain status to identify the sub class.
• When trying to parse JSON that does contains the status it fails parsing.

I am using Jackson with no extra configuration.

Example:

class JacksonParsingTest {
    @Test
    fun writeAndRead() {
        // write
        val obj = SomeObj(state = StateA)
        val objAsString = ObjectMapper().writeValueAsString(obj)
        println("Serialized object: $objAsString")

        // read
        val parsedObj = ObjectMapper().readValue("""
            {
              "state": {
                "status": "a"
              }
            }
        """.trimIndent(), SomeObj::class.java)
        println("Deserialized object: $parsedObj")
    }

    public data class SomeObj(
        @param:JsonProperty("state")
        @get:JsonProperty("state")
        @get:NotNull
        @get:Valid
        public val state: State,
    )

    @JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        include = JsonTypeInfo.As.EXISTING_PROPERTY,
        property = "status",
        visible = true,
    )
    @JsonSubTypes(
        JsonSubTypes.Type(value = StateA::class, name = "a"),
        JsonSubTypes.Type(
            value =
            StateB::class,
            name = "b",
        ),
    )
    public sealed interface State

    public object StateA : State

    public object StateB : State

    public enum class Status(
        @JsonValue
        public val `value`: String,
    ) {
        A("a"),
        B("b"),
        ;

        public companion object {
            private val mapping: Map<String, Status> = values().associateBy(Status::value)

            public fun fromValue(`value`: String): Status? = mapping[value]
        }
    }
}

Output:

Serialized object: {"state":{}}

com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve subtype of [simple type, class JacksonParsingTest$State]: missing type id property 'status' (for POJO property 'state')
 at [Source: (String)"{"state":{}}"; line: 1, column: 11] (through reference chain: JacksonParsingTest$SomeObj["state"])

Example with added status:

If I add the status property it works:

class JacksonParsingTest {
    @Test
    fun writeAndRead() {
        // write
        val obj = SomeObj(state = StateA(status = Status.A))
        val objAsString = ObjectMapper().writeValueAsString(obj)
        println("Serialized object: $objAsString")

        // read
        val parsedObj = ObjectMapper().readValue("""
            {
              "state": {
                "status": "a"
              }
            }
        """.trimIndent(), SomeObj::class.java)
        println("Deserialized object: $parsedObj")
    }

    public data class SomeObj(
        @param:JsonProperty("state")
        @get:JsonProperty("state")
        @get:NotNull
        @get:Valid
        public val state: State,
    )

    @JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        include = JsonTypeInfo.As.EXISTING_PROPERTY,
        property = "status",
        visible = true,
    )
    @JsonSubTypes(
        JsonSubTypes.Type(value = StateA::class, name = "a"),
        JsonSubTypes.Type(
            value =
            StateB::class,
            name = "b",
        ),
    )
    public sealed interface State {
        @get:JsonProperty("status")
        @get:NotNull
        public val status: Status
    }

    public data class StateA(
        @param:JsonProperty("status")
        override val status: Status,
    ) : State

    public data class StateB(
        @param:JsonProperty("status")
        override val status: Status,
    ) : State

    public enum class Status(
        @JsonValue
        public val `value`: String,
    ) {
        A("a"),
        B("b"),
        ;

        public companion object {
            private val mapping: Map<String, Status> = values().associateBy(Status::value)

            public fun fromValue(`value`: String): Status? = mapping[value]
        }
    }
}

Output:

Serialized object: {"state":{"status":"a"}}
Deserialized object: SomeObj(state=StateA(status=A))

I am unsure if this is a Jackson issue or an issue with the generated code.

Do you have an idea?

[cjbooms]

So full disclosure, I have never used this feature. But, I remember raising this with when @pschichtel added this feature.
#125 (comment)

I think is answer then was that the json needs to contain the discriminator field, and Jackson can read it and decide what polymorphic type to use, but it does not need to actually be modelled in the data classes

[cjbooms]

Oh actually, I think I see what you are saying. That feature is somehow stripping the status field. SateA and StateB look like they should contain a status property

[pschichtel]

@cjbooms This was intentional. It strips the property that is being declared as the discriminator. Mainly to prevent accidental misconfiguration of the class when generating json. If you keep the discriminator property part of the data class, then you can construct a value of one class with the discriminator value of the other class.

I'm pretty sure I used this successfully in the project I used fabrikt in. Both for parsing and generating. I might be able to publish the openapi files or at least the relevant sections as an example. I'll check next week.

[pschichtel]

I checked the project. I definitely used this feature with success. Maybe it worked for me because I had the kotlin module enabled in jackson? Or maybe it's because the data classes are turned into objects here since no fields are left. The cases where this happened in my spec are never parsed, only generated.

Does it work with additional fields in the objects? Does it work when you enable the kotlin module in the ObjectMapper?

[ulrikandersen]

Deserialization

When I enable the Kotlin module deserialization in the first example works.

Although Jackson is able to determine the type I need to configure FAIL_ON_UNKNOWN_PROPERTIES = false to avoid Jackson complaining that the status field is not known on the class (UnrecognizedPropertyException: Unrecognized field "status").

val mapper = ObjectMapper()
    .registerKotlinModule()
    .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)

val parsedObj = mapper.readValue("""
    {
      "state": {
        "status": "a"
      }
    }
""".trimIndent(), SomeObj::class.java)
println("Deserialized object: $parsedObj") // SomeObj(state=JacksonParsingTest$StateA@129fed45)

So; adding the Kotlin module makes the generated models work for deserialization.

Serialization

Configuring the Kotlin module does not seem to change how the model is serialized and we still get JSON without the type info: {"state":{}} which would not be parsable by Jackson.

[cjbooms]

Thanks @pschichtel and @ulrikandersen
I think we should adopt the same approach as used in polymorphism with allOf. Include the discriminator property, but default it to the correct value as follows, eg:

public sealed interface State

public data class StateA(
    @get:JsonProperty("status")
    @get:NotNull
    @param:JsonProperty("status")
    public val status: Status = Status.A,
) : State

public data class StateB(
    @get:JsonProperty("status")
    @get:NotNull
    @param:JsonProperty("status")
    public val status: Status = Status.B,
) : State

That should fix this issue, and help avoid (but not prevent) constructing a value of one class with the discriminator value of the other class.

See updated PR:
https://github.com/cjbooms/fabrikt/pull/268/files

[cjbooms]

Fixed in 13.0.0

[ulrikandersen]

@cjbooms I think #268 introduced a new issue: If additional properties are present in the model the default enum value is set to the discriminator value.

#270 contains the updated spec with an additional property. The tests will to fail because the generated model differs from the expected.

I would be happy to assist fixing it, but I would need some pointers as to where to look.

[ulrikandersen]

#270 now contains a fix for the issue. Let me know what you think 🙏

[cjbooms]

Thanks for the fix, looks good. Released in 13.0.1. Should sync to maven central in the next 30 minutes or so