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.

Sign up for GitHub

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

Add polymorphic default serializers (as opposed to deserializers) #1686

Merged
merged 4 commits into from
Nov 22, 2021
Merged

Add polymorphic default serializers (as opposed to deserializers) #1686

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5385952
Add polymorphic serializers
Earthcomputer Sep 18, 2021
d0bd405
Address review comments
Earthcomputer Oct 11, 2021
9824466
Remove PolymorphicModuleBuilder.defaultSerializer
Earthcomputer Nov 11, 2021
5bed0db
Address review comments
Earthcomputer Nov 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions core/api/kotlinx-serialization-core.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,8 @@ public final class kotlinx/serialization/modules/PolymorphicModuleBuilder {
public fun <init> (Lkotlin/reflect/KClass;Lkotlinx/serialization/KSerializer;)V
public final fun buildTo (Lkotlinx/serialization/modules/SerializersModuleBuilder;)V
public final fun default (Lkotlin/jvm/functions/Function1;)V
public final fun defaultDeserializer (Lkotlin/jvm/functions/Function1;)V
public final fun defaultSerializer (Lkotlin/jvm/functions/Function1;)V
public final fun subclass (Lkotlin/reflect/KClass;Lkotlinx/serialization/KSerializer;)V
}

Expand All @@ -1195,6 +1197,8 @@ public final class kotlinx/serialization/modules/SerializersModuleBuilder : kotl
public final fun include (Lkotlinx/serialization/modules/SerializersModule;)V
public fun polymorphic (Lkotlin/reflect/KClass;Lkotlin/reflect/KClass;Lkotlinx/serialization/KSerializer;)V
public fun polymorphicDefault (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;)V
public fun polymorphicDeserializerDefault (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;)V
public fun polymorphicSerializerDefault (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;)V
}

public final class kotlinx/serialization/modules/SerializersModuleBuildersKt {
Expand All @@ -1209,10 +1213,13 @@ public abstract interface class kotlinx/serialization/modules/SerializersModuleC
public abstract fun contextual (Lkotlin/reflect/KClass;Lkotlinx/serialization/KSerializer;)V
public abstract fun polymorphic (Lkotlin/reflect/KClass;Lkotlin/reflect/KClass;Lkotlinx/serialization/KSerializer;)V
public abstract fun polymorphicDefault (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;)V
public abstract fun polymorphicDeserializerDefault (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;)V
public abstract fun polymorphicSerializerDefault (Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;)V
}

public final class kotlinx/serialization/modules/SerializersModuleCollector$DefaultImpls {
public static fun contextual (Lkotlinx/serialization/modules/SerializersModuleCollector;Lkotlin/reflect/KClass;Lkotlinx/serialization/KSerializer;)V
public static fun polymorphicDefault (Lkotlinx/serialization/modules/SerializersModuleCollector;Lkotlin/reflect/KClass;Lkotlin/jvm/functions/Function1;)V
}

public final class kotlinx/serialization/modules/SerializersModuleKt {
Expand Down
3 changes: 2 additions & 1 deletion core/commonMain/src/kotlinx/serialization/descriptors/ContextAware.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public fun SerializersModule.getContextualDescriptor(descriptor: SerialDescripto
/**
* Retrieves a collection of descriptors which serializers are registered for polymorphic serialization in [this]
* with base class equal to [descriptor]'s [SerialDescriptor.capturedKClass].
* This method does not retrieve serializers registered with [PolymorphicModuleBuilder.default].
* This method does not retrieve serializers registered with [PolymorphicModuleBuilder.defaultDeserializer]
* or [PolymorphicModuleBuilder.defaultSerializer].
*
* @see SerializersModule.getPolymorphic
* @see SerializersModuleBuilder.polymorphic
Expand Down
68 changes: 57 additions & 11 deletions core/commonMain/src/kotlinx/serialization/modules/PolymorphicModuleBuilder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ public class PolymorphicModuleBuilder<in Base : Any> @PublishedApi internal cons
private val baseSerializer: KSerializer<Base>? = null
) {
private val subclasses: MutableList<Pair<KClass<out Base>, KSerializer<out Base>>> = mutableListOf()
private var defaultSerializerProvider: ((String?) -> DeserializationStrategy<out Base>?)? = null
private var defaultSerializerProvider: ((Base) -> SerializationStrategy<Base>?)? = null
private var defaultDeserializerProvider: ((String?) -> DeserializationStrategy<out Base>?)? = null

/**
* Registers a [subclass] [serializer] in the resulting module under the [base class][Base].
Expand All @@ -29,26 +30,66 @@ public class PolymorphicModuleBuilder<in Base : Any> @PublishedApi internal cons
subclasses.add(subclass to serializer)
}

/**
* Adds a default deserializers provider associated with the given [baseClass] to the resulting module.
* [defaultSerializerProvider] is invoked when no polymorphic serializers for `value` were found.
*
* [defaultSerializerProvider] can be stateful and lookup a serializer for the missing type dynamically.
*
* Default serializers provider affects only serialization process.
*/
@Suppress("UNCHECKED_CAST")
public fun <T : Base> defaultSerializer(defaultSerializerProvider: (value: T) -> SerializationStrategy<T>?) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need type parameter? I think defaultSerializerProvider: (value: Base) -> SerializationStrategy<Base>? is more appropriate here.

Imagine the situation: we have Base; A: Base(); B:Base(). If we register defaultSerializer<A> { return A.serializer() } which is allowed by this signature, when we try to serialize B, we'd get ClassCastException, because our lambda can accept only A type. Therefore, default serializer should be able to select between all subclasses of base, i.e. accept value: Base

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to use @UnsafeVariance for that, but sure.

Copy link
Member

@sandwwraith sandwwraith Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the problem is more complicated than it looks at the first sight. @UnsafeVariance is needed here because PolymorphicModuleBuilder has IN variance: <in Base : Any>. Why does it need it? The answer lies in this sample: https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/polymorphism.md#registering-multiple-superclasses

If you have a vast hierarchy (in the sample, it is Any and Project, but instead of Any, it can be different multiple super-interfaces), it is logical to register subclasses of the lowest common interface for polymorphic serialization in all bases (registerProjectSubclasses method in the sample). Naturally, it should accept PolymorphicModuleBuilder<Project>, because any of the Project subclasses can be serialized and deserialized in bigger scopes (say Any or other super-interface). It is also possible to return some deserializer as default — if it always returns a subclass of Project, it is possible to assign it into any variable up to Any.

However, it is not the case for the default serializer: since it accepts an instance, if we register it using some lambda that accepts a Project, we can't accept arbitrary Any. SerializationStrategy would also break: we know how to serialize Project, but not our super-interface. Both problems will lead to ClassCastException. If we add this function with @UnsafeVariance, the following code is error-prone:

val module = SerializersModule {
    fun PolymorphicModuleBuilder<Project>.registerProjectSubclasses() {
        subclass(OwnedProject::class)
        defaultSerializer { it: Project -> SomeDefaultProjectSerializer } // will throw ClassCastException if we serialize String as Any
    }
    polymorphic(Any::class) { registerProjectSubclasses() }
    polymorphic(Project::class) { registerProjectSubclasses() }
}

There are multiple ways to solve this problem:

  1. Leave @UnsafeVariance and document that this function may cause problems, probably annotate it with special opt-in annotation. Not a clean solution, since most people don't read the documentation. It probably would be more helpful if we can suppress CCE and throw SerializationException instead about smth like 'serializer not found, default serializer is not applicable', but I'm not sure if this can be done accurately — need further investigation. In any case, stacktrace of the exception won't pinpoint the actual line with the problem.
  2. Remove in Base: Any in polymorphic module builder. It would solve the problem, because Kotlin compiler is smart enough. We still can declare the helper function as fun PolymorphicModuleBuilder<in Project>.registerProjectSubclasses() (use-site variance), but compiler would infer that actual Base in defaultSerializer is Any and thus would require to accept Any and return SerializationStrategy<Any>. This is a good solution, but unfortunately removing variance is a source-incompatible breaking change we can't afford to do. (In the sample, polymorphic(Any::class) { registerProjectSubclasses() } would not compile with 'Unresolved reference' )
  3. Make defaultSerializer with fixed types, e.g. defaultSerializer(defaultSerializerProvider: (value: Any) -> SerializationStrategy<Any?>?). Possible and type-safe, but very inconvenient to use.
  4. Do not provide defaultSerializer at all. Note that polymorphicDefaultSerializer on the regular SerializersModuleBuilder is still a thing as it doesn't have such problems. By doing this, we're causing minor inconvenience — people are forced to write polymorphicDefaultSerializer outside of polymorphic {} scope, but we're saving them from accidental exceptions that are hard to grasp.

I think that option 4 is the way to go, despite all inconveniences. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, sorry I read your comment and then got sidetracked and forgot about it. I agree, 4 is the best solution

require(this.defaultSerializerProvider == null) {
"Default serializer provider is already registered for class $baseClass: ${this.defaultSerializerProvider}"
}
this.defaultSerializerProvider = defaultSerializerProvider as ((Base) -> SerializationStrategy<Base>?)
}

/**
* Adds a default serializers provider associated with the given [baseClass] to the resulting module.
* [defaultSerializerProvider] is invoked when no polymorphic serializers associated with the `className`
* [defaultDeserializerProvider] is invoked when no polymorphic serializers associated with the `className`
* were found. `className` could be `null` for formats that support nullable class discriminators
* (currently only [Json] with [useArrayPolymorphism][JsonBuilder.useArrayPolymorphism] set to `false`)
*
* [defaultSerializerProvider] can be stateful and lookup a serializer for the missing type dynamically.
* [defaultDeserializerProvider] can be stateful and lookup a serializer for the missing type dynamically.
*
* Typically, if the class is not registered in advance, it is not possible to know the structure of the unknown
* type and have a precise serializer, so the default serializer has limited capabilities.
* To have a structural access to the unknown data, it is recommended to use [JsonTransformingSerializer]
* or [JsonContentPolymorphicSerializer] classes.
*
* Default serializers provider affects only deserialization process.
* Default deserializers provider affects only deserialization process.
*/
public fun default(defaultSerializerProvider: (className: String?) -> DeserializationStrategy<out Base>?) {
require(this.defaultSerializerProvider == null) {
"Default serializer provider is already registered for class $baseClass: ${this.defaultSerializerProvider}"
public fun defaultDeserializer(defaultDeserializerProvider: (className: String?) -> DeserializationStrategy<out Base>?) {
require(this.defaultDeserializerProvider == null) {
"Default deserializer provider is already registered for class $baseClass: ${this.defaultDeserializerProvider}"
}
this.defaultSerializerProvider = defaultSerializerProvider
this.defaultDeserializerProvider = defaultDeserializerProvider
}

/**
* Adds a default deserializers provider associated with the given [baseClass] to the resulting module.
* [defaultDeserializerProvider] is invoked when no polymorphic serializers associated with the `className`
* were found. `className` could be `null` for formats that support nullable class discriminators
* (currently only [Json] with [useArrayPolymorphism][JsonBuilder.useArrayPolymorphism] set to `false`)
*
* [defaultDeserializerProvider] can be stateful and lookup a serializer for the missing type dynamically.
*
* Typically, if the class is not registered in advance, it is not possible to know the structure of the unknown
* type and have a precise serializer, so the default serializer has limited capabilities.
* To have a structural access to the unknown data, it is recommended to use [JsonTransformingSerializer]
* or [JsonContentPolymorphicSerializer] classes.
*
* Default deserializers provider affects only deserialization process.
*
* @see defaultDeserializer
*/
@Deprecated("Specify whether it's a default serializer/deserializer",
ReplaceWith("defaultDeserializer(defaultSerializerProvider)")
)
public fun default(defaultDeserializerProvider: (className: String?) -> DeserializationStrategy<out Base>?) {
defaultDeserializer(defaultDeserializerProvider)
}

@Suppress("UNCHECKED_CAST")
Expand All @@ -63,9 +104,14 @@ public class PolymorphicModuleBuilder<in Base : Any> @PublishedApi internal cons
)
}

val default = defaultSerializerProvider
if (default != null) {
builder.registerDefaultPolymorphicSerializer(baseClass, default, false)
val defaultSerializer = defaultSerializerProvider
if (defaultSerializer != null) {
builder.registerDefaultPolymorphicSerializer(baseClass, defaultSerializer, false)
}

val defaultDeserializer = defaultDeserializerProvider
if (defaultDeserializer != null) {
builder.registerDefaultPolymorphicDeserializer(baseClass, defaultDeserializer, false)
}
}
}
Expand Down
35 changes: 26 additions & 9 deletions core/commonMain/src/kotlinx/serialization/modules/SerializersModule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public sealed class SerializersModule {
*/
@SharedImmutable
@ExperimentalSerializationApi
public val EmptySerializersModule: SerializersModule = SerialModuleImpl(emptyMap(), emptyMap(), emptyMap(), emptyMap())
public val EmptySerializersModule: SerializersModule = SerialModuleImpl(emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap())

/**
* Returns a combination of two serial modules
Expand Down Expand Up @@ -113,12 +113,19 @@ public infix fun SerializersModule.overwriteWith(other: SerializersModule): Seri
registerPolymorphicSerializer(baseClass, actualClass, actualSerializer, allowOverwrite = true)
}

override fun <Base : Any> polymorphicDefault(
override fun <Base : Any> polymorphicSerializerDefault(
baseClass: KClass<Base>,
defaultSerializerProvider: (className: String?) -> DeserializationStrategy<out Base>?
defaultSerializerProvider: (value: Base) -> SerializationStrategy<Base>?
) {
registerDefaultPolymorphicSerializer(baseClass, defaultSerializerProvider, allowOverwrite = true)
}

override fun <Base : Any> polymorphicDeserializerDefault(
baseClass: KClass<Base>,
defaultDeserializerProvider: (className: String?) -> DeserializationStrategy<out Base>?
) {
registerDefaultPolymorphicDeserializer(baseClass, defaultDeserializerProvider, allowOverwrite = true)
}
})
}

Expand All @@ -133,21 +140,26 @@ public infix fun SerializersModule.overwriteWith(other: SerializersModule): Seri
internal class SerialModuleImpl(
private val class2ContextualFactory: Map<KClass<*>, ContextualProvider>,
@JvmField val polyBase2Serializers: Map<KClass<*>, Map<KClass<*>, KSerializer<*>>>,
private val polyBase2DefaultSerializerProvider: Map<KClass<*>, PolymorphicSerializerProvider<*>>,
private val polyBase2NamedSerializers: Map<KClass<*>, Map<String, KSerializer<*>>>,
private val polyBase2DefaultProvider: Map<KClass<*>, PolymorphicProvider<*>>
private val polyBase2DefaultDeserializerProvider: Map<KClass<*>, PolymorphicDeserializerProvider<*>>
) : SerializersModule() {

override fun <T : Any> getPolymorphic(baseClass: KClass<in T>, value: T): SerializationStrategy<T>? {
if (!value.isInstanceOf(baseClass)) return null
return polyBase2Serializers[baseClass]?.get(value::class) as? SerializationStrategy<T>
// Registered
val registered = polyBase2Serializers[baseClass]?.get(value::class) as? SerializationStrategy<T>
if (registered != null) return registered
// Default
return (polyBase2DefaultSerializerProvider[baseClass] as? PolymorphicSerializerProvider<T>)?.invoke(value)
}

override fun <T : Any> getPolymorphic(baseClass: KClass<in T>, serializedClassName: String?): DeserializationStrategy<out T>? {
// Registered
val registered = polyBase2NamedSerializers[baseClass]?.get(serializedClassName) as? KSerializer<out T>
if (registered != null) return registered
// Default
return (polyBase2DefaultProvider[baseClass] as? PolymorphicProvider<T>)?.invoke(serializedClassName)
return (polyBase2DefaultDeserializerProvider[baseClass] as? PolymorphicDeserializerProvider<T>)?.invoke(serializedClassName)
}

override fun <T : Any> getContextual(kClass: KClass<T>, typeArgumentsSerializers: List<KSerializer<*>>): KSerializer<T>? {
Expand Down Expand Up @@ -175,13 +187,18 @@ internal class SerialModuleImpl(
}
}

polyBase2DefaultProvider.forEach { (baseClass, provider) ->
collector.polymorphicDefault(baseClass as KClass<Any>, provider as (PolymorphicProvider<out Any>))
polyBase2DefaultSerializerProvider.forEach { (baseClass, provider) ->
collector.polymorphicSerializerDefault(baseClass as KClass<Any>, provider as (PolymorphicSerializerProvider<Any>))
}

polyBase2DefaultDeserializerProvider.forEach { (baseClass, provider) ->
collector.polymorphicDeserializerDefault(baseClass as KClass<Any>, provider as (PolymorphicDeserializerProvider<out Any>))
}
}
}

internal typealias PolymorphicProvider<Base> = (className: String?) -> DeserializationStrategy<out Base>?
internal typealias PolymorphicDeserializerProvider<Base> = (className: String?) -> DeserializationStrategy<out Base>?
internal typealias PolymorphicSerializerProvider<Base> = (value: Base) -> SerializationStrategy<Base>?

/** This class is needed to support re-registering the same static (argless) serializers:
*
Expand Down
47 changes: 37 additions & 10 deletions core/commonMain/src/kotlinx/serialization/modules/SerializersModuleBuilders.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ public inline fun SerializersModule(builderAction: SerializersModuleBuilder.() -
public class SerializersModuleBuilder @PublishedApi internal constructor() : SerializersModuleCollector {
private val class2ContextualProvider: MutableMap<KClass<*>, ContextualProvider> = hashMapOf()
private val polyBase2Serializers: MutableMap<KClass<*>, MutableMap<KClass<*>, KSerializer<*>>> = hashMapOf()
private val polyBase2DefaultSerializerProvider: MutableMap<KClass<*>, PolymorphicSerializerProvider<*>> = hashMapOf()
private val polyBase2NamedSerializers: MutableMap<KClass<*>, MutableMap<String, KSerializer<*>>> = hashMapOf()
private val polyBase2DefaultProvider: MutableMap<KClass<*>, PolymorphicProvider<*>> = hashMapOf()
private val polyBase2DefaultDeserializerProvider: MutableMap<KClass<*>, PolymorphicDeserializerProvider<*>> = hashMapOf()

/**
* Adds [serializer] associated with given [kClass] for contextual serialization.
Expand Down Expand Up @@ -91,17 +92,30 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser

/**
* Adds a default serializers provider associated with the given [baseClass] to the resulting module.
* [defaultSerializerProvider] is invoked when no polymorphic serializers associated with the `className`
* [defaultSerializerProvider] is invoked when no polymorphic serializers for `value` were found.
*
* @see PolymorphicModuleBuilder.defaultSerializer
*/
public override fun <Base : Any> polymorphicSerializerDefault(
baseClass: KClass<Base>,
defaultSerializerProvider: (value: Base) -> SerializationStrategy<Base>?
) {
registerDefaultPolymorphicSerializer(baseClass, defaultSerializerProvider, false)
}

/**
* Adds a default deserializers provider associated with the given [baseClass] to the resulting module.
* [defaultDeserializerProvider] is invoked when no polymorphic serializers associated with the `className`
* were found. `className` could be `null` for formats that support nullable class discriminators
* (currently only `Json` with `useArrayPolymorphism` set to `false`)
*
* @see PolymorphicModuleBuilder.default
* @see PolymorphicModuleBuilder.defaultDeserializer
*/
public override fun <Base : Any> polymorphicDefault(
public override fun <Base : Any> polymorphicDeserializerDefault(
baseClass: KClass<Base>,
defaultSerializerProvider: (className: String?) -> DeserializationStrategy<out Base>?
defaultDeserializerProvider: (className: String?) -> DeserializationStrategy<out Base>?
) {
registerDefaultPolymorphicSerializer(baseClass, defaultSerializerProvider, false)
registerDefaultPolymorphicDeserializer(baseClass, defaultDeserializerProvider, false)
}

/**
Expand Down Expand Up @@ -132,14 +146,27 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
@JvmName("registerDefaultPolymorphicSerializer") // Don't mangle method name for prettier stack traces
internal fun <Base : Any> registerDefaultPolymorphicSerializer(
baseClass: KClass<Base>,
defaultSerializerProvider: (className: String?) -> DeserializationStrategy<out Base>?,
defaultSerializerProvider: (value: Base) -> SerializationStrategy<Base>?,
allowOverwrite: Boolean
) {
val previous = polyBase2DefaultProvider[baseClass]
val previous = polyBase2DefaultDeserializerProvider[baseClass]
if (previous != null && previous != defaultSerializerProvider && !allowOverwrite) {
throw IllegalArgumentException("Default serializers provider for class $baseClass is already registered: $previous")
}
polyBase2DefaultProvider[baseClass] = defaultSerializerProvider
polyBase2DefaultSerializerProvider[baseClass] = defaultSerializerProvider
}

@JvmName("registerDefaultPolymorphicDeserializer") // Don't mangle method name for prettier stack traces
internal fun <Base : Any> registerDefaultPolymorphicDeserializer(
baseClass: KClass<Base>,
defaultDeserializerProvider: (className: String?) -> DeserializationStrategy<out Base>?,
allowOverwrite: Boolean
) {
val previous = polyBase2DefaultDeserializerProvider[baseClass]
if (previous != null && previous != defaultDeserializerProvider && !allowOverwrite) {
throw IllegalArgumentException("Default deserializers provider for class $baseClass is already registered: $previous")
}
polyBase2DefaultDeserializerProvider[baseClass] = defaultDeserializerProvider
}

@JvmName("registerPolymorphicSerializer") // Don't mangle method name for prettier stack traces
Expand Down Expand Up @@ -188,7 +215,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser

@PublishedApi
internal fun build(): SerializersModule =
SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2NamedSerializers, polyBase2DefaultProvider)
SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2DefaultSerializerProvider, polyBase2NamedSerializers, polyBase2DefaultDeserializerProvider)
}

/**
Expand Down
Loading