Fix Polymorphic Deserialization Error with Subclass using JsonTransformingSerializer

[oungsi2000]

Fixes #3108

I have identified an issue where JsonTransformingSerializer fails
during decoding when using JsonTreeDecoder

After debugging, I found the cause of the issue in the endStructure method of JsonTreeDecoder:

// JsonTreeDecoder.endStructure
for (key in value.keys) {
    if (key !in names && key != polymorphicDiscriminator) {
        throw JsonDecodingException(
            //...
        )
    }
}

The issue is that when a JsonTreeDecoder instance is created,
the polymorphicDiscriminator is not explicitly set (remaining null).
As a result, the "type" key—which holds the polymorphic information—is treated as a regular unknown key, triggering a JsonDecodingException.

I have resolved this bug by explicitly setting the discriminator in the readJson method.

However, even though all unit tests and the build have passed (including 2 new test cases),
I am not entirely sure if this is the intended way to fix it.

Thanks

[oungsi2000]

@sandwwraith Hello! I'm a first-time contributor. Could you please review this PR? Thanks

[sandwwraith]

Thanks for your PR! As I explained in the comment, the change is too broad. The core reason for the current bug lies in the JsonTransformingSerializer.deserialize: by re-creating decoder in input.json.decodeFromJsonElement we forgo the already existing state of the JsonDecoder (which contains the class discriminator we want to know). Perhaps it can be rewritten in a way that it retrieves the discriminator itself, conceptually:

final override fun deserialize(decoder: Decoder): T {
    val input = decoder.asJsonDecoder()
    val element = input.decodeJsonElement()
    val discriminator = when(input) {
        is StreamingJsonDecoder -> input.discriminatorHolder?.discriminatorToSkip
        is AbstractJsonTreeDecoder -> input.polymorphicDiscriminator
        else -> null
    }
    return readJson(input.json, transformDeserialize(element), tSerializer, discriminator)
//        return input.json.decodeFromJsonElement(tSerializer, transformDeserialize(element))
}

Note that this is a rough draft, and I won't accept it in this form: there is still a need to account for other potential subclasses, and likely a nice API should be added to JsonDecoder to "continue" decoding elements with the current decoder instead of creating a new one.

[sandwwraith]

Please do not add irrelevant changes to the PR, especially if they're about formatting.

[sandwwraith]

Sadly, this change is too broad to fix the problem. By using classDiscriminator(json), you're adding a discriminator to every JsonTreeDecoder, even one that was not originally polymorphic. Consider this class:

@Serializable(SubExample2.PolymorphicSerializer::class)
@KeepGeneratedSerializer
@SerialName("Sub")
data class SubExample2(
    val data: String
) {
    object PolymorphicSerializer : JsonTransformingSerializer<SubExample2>(generatedSerializer())
}

It is almost identical to yours, but it does not have a superclass and therefore is not polymorphic. Therefore, we expect this test to fail:

@Test
fun testNonPolymorphicExampleShouldNotBeParsed() {
    val input = """{"type":"Sub","data":"str1"}"""
    val j = Json { ignoreUnknownKeys = false }
    println(j.decodeFromString<SubExample2>(input)) // should throw "Encountered an unknown key 'type'"
}

However, with your change, this test passes.

[oungsi2000]

@sandwwraith
Hi!
Thanks for the feedback. Before I proceed with the next commit,
I’d like to clarify a few points and share a more structured proposal for the "nice API" you mentioned.

Regarding the JsonTreeDecoder constructor,
I’m wondering if it’s okay to keep the form inside of readJson() I used in this PR,
(I will change discriminator passed down from StreamingJsonDecoder)
Given the many extension functions that create decoders,
I wanted to double-check if this approach aligns with the library's design.

Regarding the "Nice API" you mentioned, I’ve identified three main implementations of JsonDecoder: StreamingJsonDecoder, AbstractJsonTreeDecoder, and DynamicInput.

Since only StreamingJsonDecoder, AbstractJsonTreeDecoder need to hold discriminator state,
I propose introducing an internal interface, PolymorphicJsonDecoder, to unify them. This allows us to retrieve the discriminator without explicit type checking in the serializer. Here is a rough draft:

internal interface PolymorphicJsonDecoder : JsonDecoder {
    val discriminator: String?
}

internal open class StreamingJsonDecoder(...) : PolymorphicJsonDecoder, ... {
    override val discriminator get() = discriminatorHolder?.discriminatorToSkip
}

private sealed class AbstractJsonTreeDecoder(...) : PolymorphicJsonDecoder, ... {
    override val discriminator get() = polymorphicDiscriminator
}

With this, the deserialize method can be simplified as follows:

final override fun deserialize(decoder: Decoder): T {
    //use if decoder should never be a DynamicInput.
    //otherwise use: val discriminator = (input as? PolymorphicJsonDecoder)?.discriminator
    val input = decoder.asPolymorphicJsonDecoder()
    val element = input.decodeJsonElement()
    return readJson(input.json, transformDeserialize(element), tSerializer, input.discriminator)
}

Does this abstraction cover the "other potential subclasses" you had in mind, or would you prefer a different approach?
Thanks

[sandwwraith]

The idea with internal interface PolymorphicJsonDecoder is good. I also had in mind

public fun <T> decodeFromJsonElementAs(element: JsonElement, serializer: DeserializationStrategy<T>): T {
        return readJson(json, element, serializer, this)
    }

we can add to JsonDecoder. But given it will be used only in JsonTransformingSerializer, I now think it is not needed as a public API, so you don't have to add it. You can also move the extraction of the discriminator to the readJson function (note it still should work with Dynamic decoders):

public fun <T> readJson(json: Json, element: JsonElement, deserializer: DeserializationStrategy<T>, previousDecoder: JsonDecoder?): T {
    val discriminator = (previousDecoder as? PolymorphicJsonDecoder)?.discriminator
    ... 

[oungsi2000]

@sandwwraith
I've implemented the PolymorphicJsonDecoder interface and updated the logic to use readJson() instead of input.json.decodeFromJsonElement(), following your suggestion to avoid exposing a new public API.

[oungsi2000]

@sandwwraith
Hi there, I'm just checking in on this PR to see if there's anything else I should update or if you've had a chance to take a look. No rush, just wanted to keep it on the radar. Thanks!