Creating a custom SerialModule for Json seems useless

[gregorbg]

Describe the bug
When creating my own SerialModule implementation to pass along as context, I have to overwrite several methods with names such as getPolymorphic -- which to me implies that these methods will be called/invoked at runtime if I use my context and something tries to (de-)serialize polymorphic values.

In reality however, this does not happen because the Json constructor invokes a plus operation and the modules are combined via dumpTo which only supports statically determined seralizer mappings.

To Reproduce
Minimal-ish example to show my point:

import kotlinx.serialization.KSerializer
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import kotlinx.serialization.modules.EmptyModule
import kotlinx.serialization.modules.SerialModule
import kotlin.reflect.KClass

@Serializable
sealed class Data {
    abstract val id: Int

    @Serializable
    @SerialName("something")
    data class Something(val message: String, override val id: Int) : Data()
    @Serializable
    @SerialName("something_else")
    data class SomethingElse(val number: Int, override val id: Int) : Data()
}

object MyModule : SerialModule by EmptyModule {
    override fun <T : Any> getPolymorphic(baseClass: KClass<T>, serializedClassName: String): KSerializer<out T>? {
        println("do something dynamic depending on $serializedClassName!")
        return null
    }
}

fun main() {
    val parser = Json(context = MyModule)
    val serial = """{"id": 42, "message":"Hello, World", "type": "something"}"""

    val parsed = parser.parse(Data.serializer(), serial)
    println(parsed)
}

Expected behavior
I expect the line do something dynamic depending on something! to be printed when executing. This does not happen -- in fact, nothing gets printed at all before the final println(parsed) statement.

I've taken a look at the code and it seems that the dumpTo method uses a SerialModuleCollector which is backed by a default SerialModuleImpl. This is unfortunate because it statically collects all pre-registered serialisers into one big map and then allows no dynamic invocation at runtime because it only uses Map#get.

It also defies the intuition that overriding any method other than dumpTo in the SerialModule interface would achieve anything, which makes me wonder why these methods are there in the first place.

For more context, please see my description and code sample at #448. I have a sealed class hierarchy of extension objects that are relevant for me, but I also need to parse "unknown" extensions (that were created by other people) into some form of "fallback extension". That is only possible, however, if I can dynamically intercept the serialiser lookup at runtime.

Thanks for your help and your amazing library! :)

Environment

• Kotlin version: 1.3.61
• Library version: 0.14.0
• Kotlin platforms: JVM only
• Gradle version: 6.1.1
• IDE version (if bug is related to the IDE) not related to IDE

[gregorbg]

I have a basic idea in mind where the dumpTo method would become completely obsolete. Instead the operator fun plus returns a rather generic SerialModule that keeps references to both plus operands and invokes them like going through a list, effectively delegating the method calls.

Would you be interested in receiving PRs for that?

[qwwdfsad]

@suushiemaniac Thanks for the offer, but we already have the solution (and had a lot of discussions to come up with it) in mind and are starting to prototype it.

Chaining (keeping references and invoking both methods) was one of the first candidates, but
there are a lot of problems with it : order of invocations is typically not clear, especially in the case of complex concatenation (e.g. ((m1 + m2) + m3) + (m4 + m5), it's not clear how (and if at all) detect and resolve conflicts, which situations are possible and which are not. Chaining introduces a lot of cognitive loads, "hard-to-tell-without-running" behaviour, complicates reasoning and performance will be far from perfect.

The problem is, in fact, that + was supposed to concatenate collection of serializers, but, currently, looks like it concatenates behaviours (aka plain code, aka user-overridden methods). This looks like a road to nowhere for the general-purpose solution.

What we intend to do is the following:

• Prohibit implementing SerialModule directly (e.g. by making it a sealed class)
• Provide a new API to SerialModuleBuilder: in addition to polymorphic(clz) { subclass<Foo> } it will be possible to register "default serializer provider" (name TBD): polymorphic(clz) { missingSerializer { clazzName -> ... } }. It will enable use-cases like "allow to skip serializer or lookup a non-registering one". Such lambda is bound to the base-class and will behave in the same manner as regular serializers (exceptions on conflicts, etc.)

It would be really nice if you could elaborate on your particular use-case. Why do you need to override getPolymorphic in the first place? What problem do you want to solve by it?

[gregorbg]

Thanks for the quick and detailed reply, @qwwdfsad ! Great explanation on why chaining is a bad idea 👍

Actually, my use case is described and discussed in #448. I have a list of objects where for some of them, I want to "skip" deserializing.

I realise that entirely skipping them may be difficult (EDIT: as per #514), so it is a conceivable option to provide a Serializer (as part of the missingSerializer solution that you outlined) that parses them into some default fallback class.

Longer version: There's an API providing a list of objects, that a lot of people have write access to. There is only this one "master" list, and I want to be able to read and write my parts of the cake (ie the ones I have serializers for) without interfering with what other people possibly stored in the API as well.