Skip to content

Commit b0fe9b1

Browse files
committed
Change priority for PolymorphicSerializer inside serializer() function
Now, we look up serializers for non-sealed interfaces in SerializersModule first, and only then unconditionally return PolymorphicSerializer. This is done because with old behavior, it was impossible to override interface serializer with context, as interfaces were treated as always having PolymorphicSerializer. This is a behavioral change, although impact should be minimal, as for most usecases (without modules), serializer will be the same and special workarounds are performed so cache would also work as expected. Note that KClass.serializer() function will no longer return PolymorphicSerializer for interfaces at all and return null instead. It is OK, because this function is marked with @InternalSerializationApi, and we can change its behavior. Intrinsics in plugin with changed functionality are expected to be merged in 2.0.0-RC. Fixes #2060
1 parent 54840e0 commit b0fe9b1

File tree

16 files changed

+347
-26
lines changed

16 files changed

+347
-26
lines changed

core/api/kotlinx-serialization-core.api

+2
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ public abstract interface annotation class kotlinx/serialization/Serializer : ja
120120
}
121121

122122
public final class kotlinx/serialization/SerializersKt {
123+
public static final fun moduleThenPolymorphic (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;)Lkotlinx/serialization/KSerializer;
124+
public static final fun moduleThenPolymorphic (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;[Lkotlinx/serialization/KSerializer;)Lkotlinx/serialization/KSerializer;
123125
public static final fun noCompiledSerializer (Ljava/lang/String;)Lkotlinx/serialization/KSerializer;
124126
public static final fun noCompiledSerializer (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;)Lkotlinx/serialization/KSerializer;
125127
public static final fun noCompiledSerializer (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;[Lkotlinx/serialization/KSerializer;)Lkotlinx/serialization/KSerializer;

core/commonMain/src/kotlinx/serialization/Serializers.kt

+43-8
Original file line numberDiff line numberDiff line change
@@ -189,24 +189,38 @@ private fun SerializersModule.serializerByKTypeImpl(
189189
val isNullable = type.isMarkedNullable
190190
val typeArguments = type.arguments.map(KTypeProjection::typeOrThrow)
191191

192-
val cachedSerializer = if (typeArguments.isEmpty()) {
192+
val cachedSerializer = if (this.hasInterfaceContextualSerializers) {
193+
null
194+
} else if (typeArguments.isEmpty()) {
193195
findCachedSerializer(rootClass, isNullable)
194196
} else {
195-
findParametrizedCachedSerializer(rootClass, typeArguments, isNullable).getOrNull()
197+
findParametrizedCachedSerializer(
198+
rootClass,
199+
typeArguments,
200+
isNullable
201+
).getOrNull()
202+
}
203+
if (cachedSerializer != null) return cachedSerializer
204+
var polymorphicFallback: KSerializer<Any?>? = null
205+
if (rootClass.isInterface()) {
206+
// Note that there is no check if interface is sealed, because for sealed interfaces we always have their serializers in cache
207+
// except for one rare case with named companion (see SerializersLookupNamedCompanionTest.SealedInterfaceWithExplicitAnnotation).
208+
// That case always returned incorrect PolymorphicSerializer instead of SealedClassSerializer, even before 1.9.20.
209+
polymorphicFallback = PolymorphicSerializer(rootClass).cast()
196210
}
197-
cachedSerializer?.let { return it }
198211

199212
// slow path to find contextual serializers in serializers module
200213
val contextualSerializer: KSerializer<out Any?>? = if (typeArguments.isEmpty()) {
201-
getContextual(rootClass)
214+
rootClass.serializerOrNull()
215+
?: getContextual(rootClass)
216+
?: polymorphicFallback
202217
} else {
203218
val serializers = serializersForParameters(typeArguments, failOnMissingTypeArgSerializer) ?: return null
204219
// first, we look among the built-in serializers, because the parameter could be contextual
205220
rootClass.parametrizedSerializerOrNull(serializers) { typeArguments[0].classifier }
206-
?: getContextual(
207-
rootClass,
208-
serializers
209-
)
221+
?: getContextual(rootClass, serializers)
222+
// PolymorphicSerializer always is returned even for Interface<T>, although it rarely works as expected.
223+
?: polymorphicFallback
210224
}
211225
return contextualSerializer?.cast<Any>()?.nullable(isNullable)
212226
}
@@ -376,3 +390,24 @@ internal fun noCompiledSerializer(
376390
): KSerializer<*> {
377391
return module.getContextual(kClass, argSerializers.asList()) ?: kClass.serializerNotRegistered()
378392
}
393+
394+
/**
395+
* Overloads of [moduleThenPolymorphic] should never be called directly.
396+
* Instead, compiler inserts calls to them when intrinsifying [serializer] function.
397+
*
398+
* If no request KClass is an interface, plugin performs call to [moduleThenPolymorphic] to achieve special behavior for interface serializers.
399+
* (They are only serializers that have module priority over default [PolymorphicSerializer]).
400+
*/
401+
@OptIn(ExperimentalSerializationApi::class)
402+
@Suppress("unused")
403+
@PublishedApi
404+
internal fun moduleThenPolymorphic(module: SerializersModule, kClass: KClass<*>): KSerializer<*> {
405+
return module.getContextual(kClass) ?: PolymorphicSerializer(kClass)
406+
}
407+
408+
@OptIn(ExperimentalSerializationApi::class)
409+
@Suppress("unused")
410+
@PublishedApi
411+
internal fun moduleThenPolymorphic(module: SerializersModule, kClass: KClass<*>, argSerializers: Array<KSerializer<*>>): KSerializer<*> {
412+
return module.getContextual(kClass, argSerializers.asList()) ?: PolymorphicSerializer(kClass)
413+
}

core/commonMain/src/kotlinx/serialization/SerializersCache.kt

+5-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package kotlinx.serialization
66

77
import kotlinx.serialization.builtins.nullable
8+
import kotlinx.serialization.internal.*
89
import kotlinx.serialization.internal.cast
910
import kotlinx.serialization.internal.createCache
1011
import kotlinx.serialization.internal.createParametrizedCache
@@ -18,13 +19,13 @@ import kotlin.reflect.KType
1819
* Cache for non-null non-parametrized and non-contextual serializers.
1920
*/
2021
@ThreadLocal
21-
private val SERIALIZERS_CACHE = createCache { it.serializerOrNull() }
22+
private val SERIALIZERS_CACHE = createCache { it.serializerOrNull() ?: it.polymorphicIfInterface() }
2223

2324
/**
2425
* Cache for nullable non-parametrized and non-contextual serializers.
2526
*/
2627
@ThreadLocal
27-
private val SERIALIZERS_CACHE_NULLABLE = createCache<Any?> { it.serializerOrNull()?.nullable?.cast() }
28+
private val SERIALIZERS_CACHE_NULLABLE = createCache<Any?> { (it.serializerOrNull() ?: it.polymorphicIfInterface())?.nullable?.cast() }
2829

2930
/**
3031
* Cache for non-null parametrized and non-contextual serializers.
@@ -72,3 +73,5 @@ internal fun findParametrizedCachedSerializer(
7273
PARAMETRIZED_SERIALIZERS_CACHE_NULLABLE.get(clazz, types)
7374
}
7475
}
76+
77+
internal fun KClass<*>.polymorphicIfInterface() = if (this.isInterface()) PolymorphicSerializer(this) else null

core/commonMain/src/kotlinx/serialization/internal/Platform.common.kt

+2
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ internal expect fun BooleanArray.getChecked(index: Int): Boolean
141141

142142
internal expect fun <T : Any> KClass<T>.compiledSerializerImpl(): KSerializer<T>?
143143

144+
internal expect fun <T: Any> KClass<T>.isInterface(): Boolean
145+
144146
/**
145147
* Create serializers cache for non-parametrized and non-contextual serializers.
146148
* The activity and type of cache is determined for a specific platform and a specific environment.

core/commonMain/src/kotlinx/serialization/modules/SerializersModule.kt

+13-2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ public sealed class SerializersModule {
6767
*/
6868
@ExperimentalSerializationApi
6969
public abstract fun dumpTo(collector: SerializersModuleCollector)
70+
71+
@InternalSerializationApi
72+
internal abstract val hasInterfaceContextualSerializers: Boolean
7073
}
7174

7275
/**
@@ -76,7 +79,14 @@ public sealed class SerializersModule {
7679
level = DeprecationLevel.WARNING,
7780
replaceWith = ReplaceWith("EmptySerializersModule()"))
7881
@JsName("EmptySerializersModuleLegacyJs") // Compatibility with JS
79-
public val EmptySerializersModule: SerializersModule = SerialModuleImpl(emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap())
82+
public val EmptySerializersModule: SerializersModule = SerialModuleImpl(
83+
emptyMap(),
84+
emptyMap(),
85+
emptyMap(),
86+
emptyMap(),
87+
emptyMap(),
88+
false
89+
)
8090

8191
/**
8292
* Returns a combination of two serial modules
@@ -147,7 +157,8 @@ internal class SerialModuleImpl(
147157
@JvmField val polyBase2Serializers: Map<KClass<*>, Map<KClass<*>, KSerializer<*>>>,
148158
private val polyBase2DefaultSerializerProvider: Map<KClass<*>, PolymorphicSerializerProvider<*>>,
149159
private val polyBase2NamedSerializers: Map<KClass<*>, Map<String, KSerializer<*>>>,
150-
private val polyBase2DefaultDeserializerProvider: Map<KClass<*>, PolymorphicDeserializerProvider<*>>
160+
private val polyBase2DefaultDeserializerProvider: Map<KClass<*>, PolymorphicDeserializerProvider<*>>,
161+
internal override val hasInterfaceContextualSerializers: Boolean
151162
) : SerializersModule() {
152163

153164
override fun <T : Any> getPolymorphic(baseClass: KClass<in T>, value: T): SerializationStrategy<T>? {

core/commonMain/src/kotlinx/serialization/modules/SerializersModuleBuilders.kt

+3-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
4949
private val polyBase2DefaultSerializerProvider: MutableMap<KClass<*>, PolymorphicSerializerProvider<*>> = hashMapOf()
5050
private val polyBase2NamedSerializers: MutableMap<KClass<*>, MutableMap<String, KSerializer<*>>> = hashMapOf()
5151
private val polyBase2DefaultDeserializerProvider: MutableMap<KClass<*>, PolymorphicDeserializerProvider<*>> = hashMapOf()
52+
private var hasInterfaceContextualSerializers: Boolean = false
5253

5354
/**
5455
* Adds [serializer] associated with given [kClass] for contextual serialization.
@@ -155,6 +156,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
155156
}
156157
}
157158
class2ContextualProvider[forClass] = provider
159+
if (forClass.isInterface()) hasInterfaceContextualSerializers = true
158160
}
159161

160162
@JvmName("registerDefaultPolymorphicSerializer") // Don't mangle method name for prettier stack traces
@@ -229,7 +231,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
229231

230232
@PublishedApi
231233
internal fun build(): SerializersModule =
232-
SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2DefaultSerializerProvider, polyBase2NamedSerializers, polyBase2DefaultDeserializerProvider)
234+
SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2DefaultSerializerProvider, polyBase2NamedSerializers, polyBase2DefaultDeserializerProvider, hasInterfaceContextualSerializers)
233235
}
234236

235237
/**
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/*
2+
* Copyright 2017-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.serialization
6+
7+
import kotlinx.serialization.builtins.*
8+
import kotlinx.serialization.descriptors.*
9+
import kotlinx.serialization.encoding.*
10+
import kotlinx.serialization.modules.*
11+
import kotlinx.serialization.test.*
12+
import kotlin.reflect.*
13+
import kotlin.test.*
14+
15+
// Imagine this is a 3rd party interface
16+
interface IApiError {
17+
val code: Int
18+
}
19+
20+
@Serializable(CustomSer::class)
21+
interface HasCustom
22+
23+
24+
object CustomSer: KSerializer<HasCustom> {
25+
override val descriptor: SerialDescriptor
26+
get() = TODO("Not yet implemented")
27+
28+
override fun serialize(encoder: Encoder, value: HasCustom) {
29+
TODO("Not yet implemented")
30+
}
31+
32+
override fun deserialize(decoder: Decoder): HasCustom {
33+
TODO("Not yet implemented")
34+
}
35+
}
36+
37+
@Suppress("UNCHECKED_CAST")
38+
class InterfaceContextualSerializerTest {
39+
40+
object MyApiErrorSerializer : KSerializer<IApiError> {
41+
override val descriptor: SerialDescriptor = PrimitiveSerialDescriptor("IApiError", PrimitiveKind.INT)
42+
43+
override fun serialize(encoder: Encoder, value: IApiError) {
44+
encoder.encodeInt(value.code)
45+
}
46+
47+
override fun deserialize(decoder: Decoder): IApiError {
48+
val code = decoder.decodeInt()
49+
return object : IApiError {
50+
override val code: Int = code
51+
}
52+
}
53+
}
54+
55+
private inline fun <reified T> SerializersModule.doTest(block: (KSerializer<T>) -> Unit) {
56+
block(this.serializer<T>())
57+
block(this.serializer(typeOf<T>()) as KSerializer<T>)
58+
}
59+
60+
// Native, WASM - can't retrieve serializer (no .isInterface)
61+
@Test
62+
fun testDefault() {
63+
if (isNative() || isWasm()) return
64+
assertEquals(PolymorphicKind.OPEN, serializer<IApiError>().descriptor.kind)
65+
assertEquals(PolymorphicKind.OPEN, serializer(typeOf<IApiError>()).descriptor.kind)
66+
}
67+
68+
@Test
69+
fun testCustom() {
70+
assertSame(CustomSer, serializer<HasCustom>())
71+
assertSame(CustomSer, serializer(typeOf<HasCustom>()) as KSerializer<HasCustom>)
72+
}
73+
74+
// JVM - intrinsics kick in
75+
@Test
76+
fun testContextual() {
77+
val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
78+
assertSame(MyApiErrorSerializer, module.serializer(typeOf<IApiError>()) as KSerializer<IApiError>)
79+
shouldFail<AssertionError>(beforeKotlin = "2.0.0", onJvm = true, onWasm = false, onNative = false, onJs = false ) {
80+
assertSame(MyApiErrorSerializer, module.serializer<IApiError>())
81+
}
82+
}
83+
84+
// JVM - intrinsics kick in
85+
@Test
86+
fun testInsideList() {
87+
val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
88+
assertEquals(MyApiErrorSerializer.descriptor, module.serializer(typeOf<List<IApiError>>()).descriptor.elementDescriptors.first())
89+
shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
90+
assertEquals(
91+
MyApiErrorSerializer.descriptor,
92+
module.serializer<List<IApiError>>().descriptor.elementDescriptors.first()
93+
)
94+
}
95+
}
96+
97+
@Test
98+
fun testWithNullability() {
99+
val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
100+
assertEquals(MyApiErrorSerializer.nullable.descriptor, module.serializer(typeOf<IApiError?>()).descriptor)
101+
shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
102+
assertEquals(MyApiErrorSerializer.nullable.descriptor, module.serializer<IApiError?>().descriptor)
103+
}
104+
}
105+
106+
@Test
107+
fun testWithNullabilityInsideList() {
108+
val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
109+
assertEquals(MyApiErrorSerializer.nullable.descriptor, module.serializer(typeOf<List<IApiError?>>()).descriptor.elementDescriptors.first())
110+
shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
111+
assertEquals(
112+
MyApiErrorSerializer.nullable.descriptor,
113+
module.serializer<List<IApiError?>>().descriptor.elementDescriptors.first()
114+
)
115+
}
116+
}
117+
118+
class Unrelated
119+
120+
object UnrelatedSerializer: KSerializer<Unrelated> {
121+
override val descriptor: SerialDescriptor
122+
get() = TODO("Not yet implemented")
123+
124+
override fun serialize(encoder: Encoder, value: Unrelated) {
125+
TODO("Not yet implemented")
126+
}
127+
128+
override fun deserialize(decoder: Decoder): Unrelated {
129+
TODO("Not yet implemented")
130+
}
131+
}
132+
133+
@Test
134+
fun interfaceSerializersAreCachedInsideIfModuleIsNotFilledWithInterface() = jvmOnly {
135+
// Caches are implemented on JVM
136+
val module = serializersModuleOf(Unrelated::class, UnrelatedSerializer)
137+
val p1 = module.serializer(typeOf<List<IApiError>>())
138+
assertEquals(PolymorphicKind.OPEN, p1.descriptor.elementDescriptors.first().kind)
139+
val p2 = module.serializer(typeOf<List<IApiError>>())
140+
assertSame(p1, p2)
141+
}
142+
143+
@Test
144+
fun interfaceSerializersAreCachedTopLevelIfModuleIsNotFilledWithInterface() = jvmOnly {
145+
val module = serializersModuleOf(Unrelated::class, UnrelatedSerializer)
146+
val p1 = module.serializer(typeOf<IApiError>())
147+
assertEquals(PolymorphicKind.OPEN, p1.descriptor.kind)
148+
val p2 = module.serializer(typeOf<IApiError>())
149+
assertSame(p1, p2)
150+
}
151+
152+
interface Parametrized<T> {
153+
val param: List<T>
154+
}
155+
156+
class PSer<T>(val tSer: KSerializer<T>): KSerializer<Parametrized<T>> {
157+
override val descriptor: SerialDescriptor
158+
get() = buildClassSerialDescriptor("PSer<${tSer.descriptor.serialName}>")
159+
160+
override fun serialize(encoder: Encoder, value: Parametrized<T>) {
161+
TODO("Not yet implemented")
162+
}
163+
164+
override fun deserialize(decoder: Decoder): Parametrized<T> {
165+
TODO("Not yet implemented")
166+
}
167+
}
168+
169+
@Test
170+
fun testParametrizedInterface() {
171+
if (!isNative() && !isWasm()) {
172+
assertEquals(PolymorphicKind.OPEN, serializer(typeOf<Parametrized<String>>()).descriptor.kind)
173+
}
174+
val md = SerializersModule {
175+
contextual(Parametrized::class) { PSer(it[0]) }
176+
}
177+
assertEquals("PSer<kotlin.String>", md.serializer(typeOf<Parametrized<String>>()).descriptor.serialName)
178+
shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
179+
assertEquals("PSer<kotlin.String>", md.serializer<Parametrized<String>>().descriptor.serialName)
180+
}
181+
}
182+
}

core/commonTest/src/kotlinx/serialization/SerializersLookupNamedCompanionTest.kt

+8-5
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,15 @@ class SerializersLookupNamedCompanionTest {
8989
)
9090
}
9191

92-
// should fail because annotation @NamedCompanion will be placed again by the compilation plugin
92+
// Will return incorrect `PolymorphicSerializer` on JVM as before 1.9.20
93+
// because annotation @NamedCompanion will be placed again by the compilation plugin
9394
// and they both will be placed into @Container annotation - thus they will be invisible to the runtime
94-
shouldFail<SerializationException>(sinceKotlin = "1.9.20", onJs = false, onNative = false, onWasm = false) {
95-
serializer(typeOf<SealedInterfaceWithExplicitAnnotation>())
96-
}
95+
// On other platforms, AssociatedObject is working as expected.
96+
assertEquals(
97+
if (isJvm()) PolymorphicKind.OPEN else PolymorphicKind.SEALED,
98+
serializer(typeOf<SealedInterfaceWithExplicitAnnotation>()).descriptor.kind
99+
)
97100
}
98101

99102

100-
}
103+
}

0 commit comments

Comments
 (0)