Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@JsonClassDiscriminator should work on interfaces #2181

Open
endorh opened this issue Jan 31, 2023 · 2 comments
Open

@JsonClassDiscriminator should work on interfaces #2181

endorh opened this issue Jan 31, 2023 · 2 comments
Labels

Comments

@endorh
Copy link

endorh commented Jan 31, 2023

What is your use-case and why do you need this feature?
I want to use polymorphic deserialization with an interface as the root class, using a different class discriminator from "type", but changing the classDiscriminator setting globally is limiting and undesirable.

Describe the solution you'd like
The @JsonClassDiscriminator annotation should not be ignored on interfaces.

Note that annotating each subclass with @JsonClassDiscriminator still doesn't help.

Workaround
It's possible to create a custom serializer which properly recognizes the @JsonClassDiscriminator annotation on interfaces, and then use it with contextual in the module descriptor, as long as the interface is annotated with @Serializable(with = KSerializer::class). Using this approach, it's even possible to support non-string discriminators (Add support for numeric json class discriminators #2142), by using custom annotations (e.g., @IntTypeDiscriminator), as well as multiple nested type discriminators in the same hierarchy (see the example).

Example workaround
import kotlinx.serialization.*
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.descriptors.StructureKind
import kotlinx.serialization.descriptors.buildSerialDescriptor
import kotlinx.serialization.encoding.Decoder
import kotlinx.serialization.encoding.Encoder
import kotlinx.serialization.json.*
import kotlinx.serialization.modules.*
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
import java.lang.IllegalStateException
import java.util.*
import kotlin.reflect.KClass
import kotlin.reflect.full.*

// Instead of @JsonClassDiscriminator we use our own annotation
annotation class TypeDiscriminator(val name: String = "type")
annotation class IntTypeDiscriminator(val name: String = "type")

/**
 * Equivalent of [SerialName] for [Int] discriminators.
 */
annotation class IntSerialName(val value: Int)

// For use in interfaces (not an extension of `Any` because it pollutes completion)
inline fun <reified T: Any> serialNameOf(`this`: Any) =
  `this`::class.findInheritableAnnotationUnder<SerialName, T>()?.value
  ?: throw IllegalStateException("Hierarchy of ${`this`::class.simpleName} is missing @SerialName annotation")

inline fun <reified T: Any> intSerialNameOf(`this`: Any) =
  `this`::class.findInheritableAnnotationUnder<IntSerialName, T>()?.value
  ?: throw IllegalStateException("Hierarchy of ${`this`::class.simpleName} is missing @IntSerialName annotation")

// Reflection utils
@Suppress("UNCHECKED_CAST")
fun <A : Annotation> KClass<*>.findInheritableAnnotation(kls: KClass<A>): A? =
  annotations.firstOrNull { kls.isInstance(it) } as? A
  ?: superclasses.firstNotNullOfOrNull { it.findInheritableAnnotation(kls) }
inline fun <reified A : Annotation> KClass<*>.findInheritableAnnotation() = findInheritableAnnotation(A::class)

@Suppress("UNCHECKED_CAST")
fun <A : Annotation> KClass<*>.findInheritableAnnotationUnder(a: KClass<A>, top: KClass<*>): A? =
  allSuperclasses.filter { top.isSuperclassOf(it) && top != it }.sortedWith { l, r ->
      if (l.isSuperclassOf(r)) -1 else if (r.isSuperclassOf(l)) 1 else 0
  }.firstNotNullOfOrNull { k -> k.annotations.firstOrNull { a.isInstance(it) } } as? A
inline fun <reified A : Annotation, reified T : Any> KClass<*>.findInheritableAnnotationUnder(): A? =
  findInheritableAnnotationUnder(A::class, T::class)

// Custom module descriptor wrapper, because `polyBase2Serializers` is private in [SerializersModule] :)
interface IntrospectableSerializersModule {
    val poly2serializersMap: Map<KClass<*>, Map<KClass<*>, KSerializer<*>>>
    val serializersModule: SerializersModule
    
    companion object {
        operator fun invoke(builder: IntrospectableSerializersModuleBuilder.() -> Unit) =
          IntrospectableSerializersModuleBuilder().apply(builder).build()
    }
}

open class IntrospectableSerializersModuleImpl protected constructor(
  override val poly2serializersMap: Map<KClass<*>, Map<KClass<*>, KSerializer<*>>>,
  override val serializersModule: SerializersModule
) : IntrospectableSerializersModule {
    companion object {
        internal fun of(
          poly2serializersMap: Map<KClass<*>, Map<KClass<*>, KSerializer<*>>>,
          serializersModule: SerializersModule
        ) = IntrospectableSerializersModuleImpl(poly2serializersMap, serializersModule)
    }
}

// Builder to wrap SerializersModule with IntrospectableSerializersModule
class IntrospectableSerializersModuleBuilder internal constructor() {
    private val poly2serializersMap = mutableMapOf<KClass<*>, MutableMap<KClass<*>, KSerializer<*>>>()
    private var serializersModule = EmptySerializersModule()
    
    // In case we want to add more types of polymorphic serializers (e.g., structure based polymorphism)
    private fun <T : Any> basePolymorphic(
      baseClass: KClass<T>, serializer: KSerializer<T>, builder: SubClassBuilder<T>
    ) {
        poly2serializersMap.getOrPut(baseClass) { mutableMapOf() }.putAll(builder.map)
        module {
            @Suppress("UNCHECKED_CAST")
            fun <S : T> PolymorphicModuleBuilder<T>.addSubClass(klass: KClass<S>, serializer: KSerializer<*>) {
                subclass(klass, serializer as KSerializer<S>) // Capture 'em all types
            }
    
            // Only works if `baseClass` is annotated with `@Serializable`, which, for non-sealed interfaces
            //   needs to be `@Serializable(with = KSerializer::class)` to avoid the compiler error
            // Doesn't work either if `baseClass` is annotated with `@Polymorphic`
            // It's also not possible to annotate `baseClass` with `@Contextual`, as it's only
            //   applicable to type usages
            contextual(baseClass, serializer)
            polymorphic(baseClass) {
                builder.map.forEach { (k, v) ->
                    addSubClass(k, v)
                }
            }
        }
    }
    
    fun <T : Any> discriminatedPolymorphic(
      baseClass: KClass<T>, builder: SubClassBuilder<T>
    ) = basePolymorphic(baseClass, DiscriminatedPolymorphicSerializer(baseClass), builder)
    
    inline fun <T : Any> discriminatedPolymorphic(
      baseClass: KClass<T>, builder: SubClassBuilder<T>.() -> Unit
    ) = discriminatedPolymorphic(baseClass, SubClassBuilder<T>().apply(builder))
    
    @OptIn(InternalSerializationApi::class)
    inner class SubClassBuilder<T : Any> {
        internal val map: MutableMap<KClass<out T>, KSerializer<out T>> = mutableMapOf()
        fun <S : T> subClass(subClass: KClass<out S>, serializer: KSerializer<out S> = subClass.serializer()) {
            map[subClass] = serializer
        }
        // We could even add a SubClassBuilder<T>.() -> Unit argument to register the nested subclasses in a nested block
        fun <S : T> subPolymorphic(subClass: KClass<out S>) =
          subClass(subClass, DiscriminatedPolymorphicSerializer(subClass))
    }
    
    fun combineModule(module: SerializersModule) {
        serializersModule += module
    }
    
    inline fun module(builder: SerializersModuleBuilder.() -> Unit) {
        combineModule(SerializersModule(builder))
    }
    
    internal fun build(): IntrospectableSerializersModule =
      IntrospectableSerializersModuleImpl.of(poly2serializersMap, serializersModule)
}

// Wrapped JSON format
/**
 * Result **MUST** be cached.
 */
@Suppress("JSON_FORMAT_REDUNDANT") // It'd be nice if this could be propagated, or if this behaviour could be expressed with a contract
fun IntrospectableJson(module: IntrospectableSerializersModule, builder: (JsonBuilder.() -> Unit)? = null): Json = Json {
    builder?.let { it() }
    serializersModule += module.serializersModule
}.also {
    IntrospectableModulesByJson[it] = module
}

// Since we cannot add custom settings to the Json format
internal val IntrospectableModulesByJson = IdentityHashMap<Json, IntrospectableSerializersModule>()

@Suppress("UNCHECKED_CAST")
fun <T: Any> Json.getPolymorphic(baseClass: KClass<T>): Map<KClass<T>, KSerializer<T>> =
  IntrospectableModulesByJson[this]?.poly2serializersMap?.get(baseClass) as? Map<KClass<T>, KSerializer<T>>
  ?: throw SerializationException(
      "Class ${baseClass.simpleName} is not registered as a discriminated polymorphic base class " +
      "in an introspectable Json format!")

// Custom serializer
@OptIn(ExperimentalSerializationApi::class, InternalSerializationApi::class) // buildSerialDescriptor is internal OptIn
open class DiscriminatedPolymorphicSerializer<T : Any>(val baseClass: KClass<T>) : KSerializer<T> {
    override val descriptor: SerialDescriptor = buildSerialDescriptor(
        "DiscriminatedPolymorphicSerializer<${baseClass.qualifiedName}>",
        StructureKind.OBJECT // Blatant lie to trick the JSON format into using this serializer even if it's not concrete
        // PolymorphicKind.OPEN // Not everyone can handle the truth, this probably messes up schema generation
    )
    
    // Unoptimized implementation, ideally the inheritable annotations should be cached
    override fun serialize(encoder: Encoder, value: T) {
        val jsonEncoder = encoder as? JsonEncoder ?: throw SerializationException(
            "DiscriminatedPolymorphicSerializer only supports JSON format")
        val classMap = jsonEncoder.json.getPolymorphic(baseClass)
        // We cannot assume that value::class is one of the subclasses,
        // if we want to support nested hierarchies
        val cls = if (value::class in classMap) {
            value::class
        } else classMap.keys.firstOrNull { it.isInstance(value) } ?: throw SerializationException(
            "Value of class ${value::class.qualifiedName} is not a known subclass of ${baseClass.qualifiedName}")
        val serializer = classMap[cls]!!
        val obj = jsonEncoder.json.encodeToJsonElement(serializer, value)
        val res = baseClass.findAnnotation<IntTypeDiscriminator>()?.let { k ->
            val id = cls.findInheritableAnnotation<IntSerialName>()?.value ?: throw SerializationException(
                "Class ${cls.qualifiedName} is registered for discriminated polymorphic " +
                "serialization with an Int discriminator in the scope of ${baseClass.qualifiedName}," +
                "but is missing an @IntSerialName annotation.")
            JsonObject(mapOf(k.name to JsonPrimitive(id)) + obj.jsonObject)
        } ?: (baseClass.findAnnotation<TypeDiscriminator>()?.name ?: "type").let { name ->
            val id = cls.findInheritableAnnotation<SerialName>()?.value ?: cls.qualifiedName!!
            JsonObject(mapOf(name to JsonPrimitive(id)) + obj.jsonObject)
        }
        encoder.encodeJsonElement(res)
    }
    
    override fun deserialize(decoder: Decoder): T {
        val jsonDecoder = decoder as? JsonDecoder ?: throw SerializationException(
            "ContentBasedPolymorphicSerializer only supports JSON format")
        val classMap = jsonDecoder.json.getPolymorphic(baseClass)
        val element = jsonDecoder.decodeJsonElement()
        val (kls, obj) = selectDescriptor(classMap.keys, element.jsonObject)
        val serializer =
          decoder.serializersModule.getPolymorphic(baseClass, classMap[kls]!!.descriptor.serialName)
          ?: throw SerializationException(
              "Polymorphic serializer for '${kls.qualifiedName}' under ${baseClass::class.qualifiedName} is missing!")
        return decoder.json.decodeFromJsonElement(serializer, obj)
    }
    
    open fun selectDescriptor(classes: Set<KClass<T>>, value: JsonObject): Pair<KClass<out T>, JsonObject> {
        val obj = value.jsonObject
        baseClass.findAnnotation<IntTypeDiscriminator>()?.name?.let { name ->
            val id = obj[name]?.jsonPrimitive?.intOrNull ?: throw SerializationException(
                "Missing int type discriminator \"$name\" for polymorphic type ${baseClass.simpleName} in $obj!")
            return classes.firstOrNull { cls ->
                val clsId = cls.findInheritableAnnotation<IntSerialName>()?.value ?: throw SerializationException(
                    "Class ${value::class.qualifiedName} is registered for discriminated polymorphic " +
                    "serialization with an Int discriminator in the scope of ${baseClass.qualifiedName}," +
                    "but is missing an @IntSerialName annotation.")
                clsId == id
            }?.let {
                it to JsonObject(obj - name)
            } ?: // TODO: Here would be the place to fallback to a default deserializer (but wdyk... it's also private in SerializersModule :))
            throw SerializationException(
                "Unsupported polymorphic discriminator for class ${baseClass.simpleName}: $id")
        } ?: (baseClass.findAnnotation<TypeDiscriminator>()?.name ?: "type").let { name ->
            // Inconsistent API (why no stringOrNull?)
            val discriminator = obj[name]?.jsonPrimitive
            // A different error could be reported if the discriminator is of the wrong type
            if (discriminator?.isString != true) throw SerializationException(
                "Missing type discriminator \"$name\" for polymorphic type ${baseClass.simpleName} in $obj!")
            val serialName = discriminator.content
            return classes.firstOrNull { cls ->
                val clsSerialName = cls.findInheritableAnnotation<SerialName>()?.value ?: cls.qualifiedName!!
                clsSerialName == serialName
            }?.let {
                it to JsonObject(obj - name)
            } ?: // TODO: Here would again be the place to fallback to a default deserializer
            throw SerializationException(
                "Unsupported polymorphic discriminator for class ${baseClass.simpleName}: $serialName")
        }
    }
}

// Sample usage

// Would use the default polymorphic serializer, which doesn't support @JsonClassDiscriminator in interfaces
// @Polymorphic
// @JsonClassDiscriminator("kind")

// Not passing the `with` results in a compiler error, but passing the default value (KSerializer::class)
// allows us to replace the default polymorphic serializer with `contextual` in the module descriptor
// (not complaining, but why?)
@Serializable(with = KSerializer::class)
@TypeDiscriminator("kind") // Our own `@JsonClassDiscriminator` annotation
interface ResourceOperation {
    // This property could be omitted if we didn't want it in the interface
    val kind: String get() = serialNameOf<ResourceOperation>(this)
}

@SerialName("create") // Since our serializer looks for @SerialName in superclasses we may add it here
interface CreateOperation : ResourceOperation {
    val uri: String
}

@SerialName("rename")
interface RenameOperation : ResourceOperation {
    val oldUri: String
    val newUri: String
}

// Nested polymorphic type with its own subtype discriminator
@TypeDiscriminator("modifyKind")
@SerialName("modify") // Serial name for the parent type discriminator
interface ModifyOperation : ResourceOperation {
    val modifyKind: String get() = serialNameOf<ModifyOperation>(this)
    val uri: String
}

@SerialName("replaceContent") // Serial name for `modifyKind` (the closest in the hierarchy)
interface ReplaceContentOperation : ModifyOperation {
    val newContent: String
}

// Implementations
@Serializable // No other annotation is needed
data class CreateOperationImpl(override val uri: String) : CreateOperation

@Serializable
data class RenameOperationImpl(
  override val oldUri: String, override val newUri: String
) : RenameOperation

@Serializable
data class ReplaceContentOperationImpl(
  override val uri: String, override val newContent: String
) : ReplaceContentOperation

// Custom module descriptor
val INTROSPECTABLE_MODULE = IntrospectableSerializersModule {
    discriminatedPolymorphic(ResourceOperation::class) {
        subClass(CreateOperationImpl::class)
        subClass(RenameOperationImpl::class)
        // Shorthand for subClass(ModifyOperation::class, DiscriminatedPolymorphicSerializer(ModifyOperation::class))
        subPolymorphic(ModifyOperation::class)
    }
    
    discriminatedPolymorphic(ModifyOperation::class) {
        subClass(ReplaceContentOperationImpl::class)
    }
    
    module {
        // Normal module configuration...
    }
}

// Wrapped Json format
val FORMAT = IntrospectableJson(INTROSPECTABLE_MODULE) {
    // Normal Json configuration...
}

// Demo usage
class Test {
    @Test fun test() {
        val create: ResourceOperation = FORMAT.decodeFromString("""{"kind": "create", "uri": "foo"}""")
        assertTrue(create is CreateOperation)
        assertEquals("""{"kind":"create","uri":"foo"}""", FORMAT.encodeToString(create))
        assertEquals("create", create.kind)
        val rename: ResourceOperation = FORMAT.decodeFromString("""{"kind": "rename", "oldUri": "foo", "newUri": "bar"}""")
        assertTrue(rename is RenameOperation)
        assertEquals("""{"kind":"rename","oldUri":"foo","newUri":"bar"}""", FORMAT.encodeToString(rename))
        assertEquals("rename", rename.kind)
        val replace: ResourceOperation = FORMAT.decodeFromString("""{"kind": "modify", "modifyKind": "replaceContent", "uri": "foo", "newContent": "bar"}""")
        assertTrue(replace is ReplaceContentOperation)
        assertEquals("""{"kind":"modify","modifyKind":"replaceContent","uri":"foo","newContent":"bar"}""", FORMAT.encodeToString(replace))
        assertEquals("replaceContent", (replace as ModifyOperation).modifyKind)
        assertEquals("modify", replace.kind)
    }
}

However, this requires the custom serializer to access the polyBase2Serializers property from the SerializersModule (which is not part of the public API), or use a custom module descriptor that wraps SerializersModule exposing the polymorphic hierarchy information (which also requires wrapping the Json format, so its custom module descriptor is accessible from a global identity map (at least this approach does not rely on internal API)).

Furthermore, the contextual setting in the SerializersModule for the interface is only used if the interface itself is annotated with @Serializable, but, since interfaces cannot be annotated with @Serializable without a with argument (because it's a compile error to clarify intent), it's necessary to annotate the interface with @Serializable(with = KSerializer::class), as any other argument will result in a compiler warning (a serializer of Any is not applicable to an interface type).

I imagine that the use of @Serializable(with = KSerializer::class) is unintended and probably an overlook of the compile error that prevents using @Serializable without arguments on interfaces, but it's the cleanest way I've found to get the interface to be deserialized with the serializer specified as contextual in the module descriptor. Note that using @Serializable(with = DiscriminatedPolymorphicSerializer::class) not only produces a compiler warning, but it's both pointless and wrong, since the passed serializer class should have a constructor accepting type argument serializers, if anything, which is not the case of the serializer in this example.

As shown, this workaround is actually even more powerful than the requested feature. It'd be nice if @Polymorphic types could be configured with a different serializer than the built-in PolymorphicSerializer (e.g. a structure-based serializer).

Additional thoughts
I'd like to propose exposing the polyBase2Serializers map (indirectly) with a function on SerializersModule to retrieve the (immutable) map from subclasses of a given class to their serializers, so at least this workaround wouldn't be as sketchy as it currently is.

This function could be an overload of the already existing getPolymorphic functions, that wouldn't require an instance or the serial name of a subclass:

fun <T : Any> getPolymorphic(baseClass: KClass<in T>): Map<KClass<in T>, KSerializer<*>>

It'd also be ideal if the polyBase2DefaultDeserializerProvider map was somehow accessible. Why isn't all the SerializersModule information public anyways? It's incredibly frustrating to develop custom serializers that reuse module configuration.

Additionally, the idea of a custom module descriptor would at least be easier to implement if the Json format (and others) could be configured with custom settings (e.g., stored in a map by settings class).

data class MyCustomFormatSettings(val setting: Boolean = false)
val format = Json {
    addCustomOptions(MyCustomFormatSettings())
}

// Custom serializers would then access these options like this
format.customOptions[MyCustomFormatSettings::class]?.setting

This could be pretty useful to allow custom serializers to use format-specific configuration.

Finally, if supporting @JsonClassDiscriminator for interfaces is not possible, then at least the annotation should be reported as an error when applied to an interface, since this behavior is unexpected (even if the annotation contains Class in its name).

@endorh endorh added the feature label Jan 31, 2023
@sandwwraith
Copy link
Member

Thanks for this detailed write-up! You've mentioned several problems here:

  1. @JsonClassDiscriminator is not stored on serializable interfaces — it is indeed a bug. I believe we should have fixed it for abstract classes, sealed classes/interfaces and objects (Annotations are not collected for object kinds #708, Serial descriptor for sealed classes does not provide class annotations #1240), but for some reason it is still actual for interfaces.

  2. It is impossible to override the default serializer for an interface with a contextual one without tricks. Really, @Serializable(KSerializer::class) shouldn't work this way, it caught me by surprise. There's Instruct Kotlin serialization to always serialize a third party interface using a specific serializer #2060 for that.

  3. The rest of the issue, as far as I understood, is about implementing a hand-rolled PolymorphicSerializer for cases when the default one is not enough. That's why you need access to SerializersModule internal maps, correct? There's also your comment about it: deduction based polymorphism #1392 (comment)

So, there are several existing ways to introspect the module: SerializersModule.getPolymorphicDescriptors, but it works only with descriptors from the library's PolymorphicSerializer. Does making SerialDescriptor.withContext(context: KClass<*>) public solve this problem for custom serializers? A descriptor can contain all annotations you need, although I'm not sure if it has enough information to find KClass to use getPolymorphic afterward.

Another option (which we usually forget about) is to use SerializersModule.dumpTo. From the documentation, it looks like it fits your purposes:

[SerializersModuleCollector] can introspect and accumulate content of any [SerializersModule] via [SerializersModule.dumpTo], using a visitor-like pattern: [contextual] and [polymorphic] functions are invoked for each registered serializer.

It is a bit clumsy to use, but with polymorphic(baseClass, actualClass, actualSerializer), it seems to be possible to build polyBase2Serializers map of your own. Given that module is immutable, you can store it where you want. Does dumpTo solve your problem?

Regarding points 1 and 2: They're mostly connected with the fact that non-sealed interfaces are serializable automatically and do not have any generated code on their own — a polymorphic serializer is always inserted in the place where we meet them. It seemed a good design decision at that time; however, maybe it is time to reconsider it.

@endorh
Copy link
Author

endorh commented Feb 3, 2023

Thanks for such a detailed answer.

Indeed, making SerialDescriptor.withContext(context: KClass<*>) public would allow custom serializers to access at least the descriptors of the subclasses of a polymorphic type, which would be enough in this case.

On another hand, I had totally overlooked SerializersModule.dumpTo. Despite requiring a preprocessing step, I prefer this solution, since it can even access the default serializers.
I may post an updated workaround using dumpTo if I have the time.


Regarding points 1 and 2, I agree it'd be useful generating code for interfaces, if it'd allow replacing the default polymorphic serializer without trickery.

The use of @Serializable(KSerializer::class) also caught me by surprise.

I found it because I mistakenly thought using @Serializable(DiscriminatedPolymorphicSerializer::class) would work. When I then added contextual(ResourceOperation::class, DiscriminatedPolymorphicSerializer(ResourceOperation::class)) it started working.
Some time after, I tried removing the @Serializable annotation, but that made it stop working. Thus, since the @Serializable annotation could not be used without arguments, neither @Contextual nor @Polymorphic could be used in its place, and it produced a compiler warning about the serializer type mismatch, because an Any serializer is not technically applicable to an interface type (does that make any sense?), I tried passing instead a Nothing serializer (which still produced the warning), and in the end I had the idea of using the default value of KSerializer::class, which surprisingly kept the trick working while avoiding all warnings (I assume due to an overlook in the check done by the compiler plugin).
I agree this is clearly an unintended use of the annotation, and using KSerializer::class as an argument should probably be marked as an error/warning by the compiler.

While the default polymorphic serializer may be good for most use-cases, having the freedom to serialize open/3rd party interfaces with custom logic is a feature that I feel is missing or quite arduous to achieve, in the context of migrating from other established JVM serialization libraries (e.g., GSON, Jackson) in which this is common practice (from my experience, at least).

I really like how simple to use is this library thanks to the compiler plugin, but, when requirements become more complex, it can seem harder to configure or not flexible enough (without trickery) at times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants