Skip to content

Commit 7d4bb2a

Browse files
sandwwraithsandwwraith
authored
Improve polymorphic deserialization optimization: (#2481)
Previously, when discriminator was found as the first key in Json, but there was no deserializer for it, we still fell back to a slow path with JsonTree. It was actually meaningless because a slow path always throws exception when a serializer is not found. Such behavior led to unnecessary memory pressure & consumption in exceptional cases (see linked ticket for details). Also make polymorphic deserialization exception messages more meaningful and make them more consistent with serialization ones. Also fix behavior when the actual discriminator value is JsonNull (it should be treated as missing, not as "null" string). Fixes #2478
1 parent cf57414 commit 7d4bb2a

File tree

9 files changed

+134
-59
lines changed

9 files changed

+134
-59
lines changed

‎core/commonMain/src/kotlinx/serialization/internal/AbstractPolymorphicSerializer.kt‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ public abstract class AbstractPolymorphicSerializer<T : Any> internal constructo
5858
}
5959
else -> throw SerializationException(
6060
"Invalid index in polymorphic deserialization of " +
61-
(klassName ?: "unknown class") +
62-
"\n Expected 0, 1 or DECODE_DONE(-1), but found $index"
61+
(klassName ?: "unknown class") +
62+
"\n Expected 0, 1 or DECODE_DONE(-1), but found $index"
6363
)
6464
}
6565
}
@@ -98,14 +98,14 @@ public abstract class AbstractPolymorphicSerializer<T : Any> internal constructo
9898

9999
@JvmName("throwSubtypeNotRegistered")
100100
internal fun throwSubtypeNotRegistered(subClassName: String?, baseClass: KClass<*>): Nothing {
101-
val scope = "in the scope of '${baseClass.simpleName}'"
101+
val scope = "in the polymorphic scope of '${baseClass.simpleName}'"
102102
throw SerializationException(
103103
if (subClassName == null)
104-
"Class discriminator was missing and no default polymorphic serializers were registered $scope"
104+
"Class discriminator was missing and no default serializers were registered $scope."
105105
else
106-
"Class '$subClassName' is not registered for polymorphic serialization $scope.\n" +
107-
"To be registered automatically, class '$subClassName' has to be '@Serializable', and the base class '${baseClass.simpleName}' has to be sealed and '@Serializable'.\n" +
108-
"Alternatively, register the serializer for '$subClassName' explicitly in a corresponding SerializersModule."
106+
"Serializer for subclass '$subClassName' is not found $scope.\n" +
107+
"Check if class with serial name '$subClassName' exists and serializer is registered in a corresponding SerializersModule.\n" +
108+
"To be registered automatically, class '$subClassName' has to be '@Serializable', and the base class '${baseClass.simpleName}' has to be sealed and '@Serializable'."
109109
)
110110
}
111111

‎docs/polymorphism.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ fun main() {
123123
This is close to the best design for a serializable hierarchy of classes, but running it produces the following error:
124124

125125
```text
126-
Exception in thread "main" kotlinx.serialization.SerializationException: Class 'OwnedProject' is not registered for polymorphic serialization in the scope of 'Project'.
126+
Exception in thread "main" kotlinx.serialization.SerializationException: Serializer for subclass 'OwnedProject' is not found in the polymorphic scope of 'Project'.
127+
Check if class with serial name 'OwnedProject' exists and serializer is registered in a corresponding SerializersModule.
127128
To be registered automatically, class 'OwnedProject' has to be '@Serializable', and the base class 'Project' has to be sealed and '@Serializable'.
128-
Alternatively, register the serializer for 'OwnedProject' explicitly in a corresponding SerializersModule.
129129
```
130130

131131
<!--- TEST LINES_START -->
@@ -832,7 +832,8 @@ fun main() {
832832
We get the following exception.
833833

834834
```text
835-
Exception in thread "main" kotlinx.serialization.json.internal.JsonDecodingException: Polymorphic serializer was not found for class discriminator 'unknown'
835+
Exception in thread "main" kotlinx.serialization.json.internal.JsonDecodingException: Unexpected JSON token at offset 0: Serializer for subclass 'unknown' is not found in the polymorphic scope of 'Project' at path: $
836+
Check if class with serial name 'unknown' exists and serializer is registered in a corresponding SerializersModule.
836837
```
837838

838839
<!--- TEST LINES_START -->

‎formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphicDeserializationErrorMessagesTest.kt‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
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.features
6+
7+
import kotlinx.serialization.*
8+
import kotlinx.serialization.json.*
9+
import kotlin.test.*
10+
11+
class PolymorphicDeserializationErrorMessagesTest : JsonTestBase() {
12+
@Serializable
13+
class DummyData(@Polymorphic val a: Any)
14+
15+
@Serializable
16+
class Holder(val d: DummyData)
17+
18+
// TODO: remove this after #2480 is merged
19+
private fun checkSerializationException(action: () -> Unit, assertions: SerializationException.(String) -> Unit) {
20+
val e = assertFailsWith(SerializationException::class, action)
21+
assertNotNull(e.message)
22+
e.assertions(e.message!!)
23+
}
24+
25+
@Test
26+
fun testNotRegisteredMessage() = parametrizedTest { mode ->
27+
val input = """{"d":{"a":{"type":"my.Class", "value":42}}}"""
28+
checkSerializationException({
29+
default.decodeFromString<Holder>(input, mode)
30+
}, { message ->
31+
// ReaderJsonLexer.peekLeadingMatchingValue is not implemented, so first-key optimization is not working for streaming yet.
32+
if (mode == JsonTestingMode.STREAMING)
33+
assertContains(message, "Unexpected JSON token at offset 10: Serializer for subclass 'my.Class' is not found in the polymorphic scope of 'Any' at path: \$.d.a")
34+
else
35+
assertContains(message, "Serializer for subclass 'my.Class' is not found in the polymorphic scope of 'Any'")
36+
})
37+
}
38+
39+
@Test
40+
fun testDiscriminatorMissingNoDefaultMessage() = parametrizedTest { mode ->
41+
val input = """{"d":{"a":{"value":42}}}"""
42+
checkSerializationException({
43+
default.decodeFromString<Holder>(input, mode)
44+
}, { message ->
45+
// Always slow path when discriminator is missing, so no position and path
46+
assertContains(message, "Class discriminator was missing and no default serializers were registered in the polymorphic scope of 'Any'")
47+
})
48+
}
49+
50+
@Test
51+
fun testClassDiscriminatorIsNull() = parametrizedTest { mode ->
52+
val input = """{"d":{"a":{"type":null, "value":42}}}"""
53+
checkSerializationException({
54+
default.decodeFromString<Holder>(input, mode)
55+
}, { message ->
56+
// Always slow path when discriminator is missing, so no position and path
57+
assertContains(message, "Class discriminator was missing and no default serializers were registered in the polymorphic scope of 'Any'")
58+
})
59+
}
60+
}

‎formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphismWithAnyTest.kt‎

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
package kotlinx.serialization.features
66

77
import kotlinx.serialization.*
8-
import kotlinx.serialization.json.Json
8+
import kotlinx.serialization.json.*
99
import kotlinx.serialization.modules.*
1010
import kotlinx.serialization.modules.plus
1111
import kotlinx.serialization.test.assertStringFormAndRestored
1212
import kotlin.test.*
1313

14-
class PolymorphismWithAnyTest {
14+
class PolymorphismWithAnyTest: JsonTestBase() {
1515

1616
@Serializable
1717
data class MyPolyData(val data: Map<String, @Polymorphic Any>)
@@ -28,19 +28,20 @@ class PolymorphismWithAnyTest {
2828
val className = className.substringAfterLast('.')
2929
val scopeName = scopeName.substringAfterLast('.')
3030
val expectedText =
31-
"Class '$className' is not registered for polymorphic serialization in the scope of '$scopeName'"
31+
"Serializer for subclass '$className' is not found in the polymorphic scope of '$scopeName'"
3232
assertTrue(exception.message!!.startsWith(expectedText),
3333
"Found $exception, but expected to start with: $expectedText")
3434
}
3535

3636
@Test
37-
fun testFailWithoutModulesWithCustomClass() {
37+
fun testFailWithoutModulesWithCustomClass() = parametrizedTest { mode ->
3838
checkNotRegisteredMessage(
3939
"kotlinx.serialization.IntData", "kotlin.Any",
4040
assertFailsWith<SerializationException>("not registered") {
4141
Json.encodeToString(
4242
MyPolyData.serializer(),
43-
MyPolyData(mapOf("a" to IntData(42)))
43+
MyPolyData(mapOf("a" to IntData(42))),
44+
mode
4445
)
4546
}
4647
)
@@ -51,26 +52,27 @@ class PolymorphismWithAnyTest {
5152
val json = Json {
5253
serializersModule = SerializersModule { polymorphic(Any::class) { subclass(IntData.serializer()) } }
5354
}
54-
assertStringFormAndRestored(
55+
assertJsonFormAndRestored(
5556
expected = """{"data":{"a":{"type":"kotlinx.serialization.IntData","intV":42}}}""",
56-
original = MyPolyData(mapOf("a" to IntData(42))),
57+
data = MyPolyData(mapOf("a" to IntData(42))),
5758
serializer = MyPolyData.serializer(),
58-
format = json
59+
json = json
5960
)
6061
}
6162

6263
/**
6364
* This test should fail because PolyDerived registered in the scope of PolyBase, not kotlin.Any
6465
*/
6566
@Test
66-
fun testFailWithModulesNotInAnyScope() {
67+
fun testFailWithModulesNotInAnyScope() = parametrizedTest { mode ->
6768
val json = Json { serializersModule = BaseAndDerivedModule }
6869
checkNotRegisteredMessage(
6970
"kotlinx.serialization.PolyDerived", "kotlin.Any",
7071
assertFailsWith<SerializationException> {
7172
json.encodeToString(
7273
MyPolyData.serializer(),
73-
MyPolyData(mapOf("a" to PolyDerived("foo")))
74+
MyPolyData(mapOf("a" to PolyDerived("foo"))),
75+
mode
7476
)
7577
}
7678
)
@@ -86,19 +88,19 @@ class PolymorphismWithAnyTest {
8688
@Test
8789
fun testRebindModules() {
8890
val json = Json { serializersModule = baseAndDerivedModuleAtAny }
89-
assertStringFormAndRestored(
91+
assertJsonFormAndRestored(
9092
expected = """{"data":{"a":{"type":"kotlinx.serialization.PolyDerived","id":1,"s":"foo"}}}""",
91-
original = MyPolyData(mapOf("a" to PolyDerived("foo"))),
93+
data = MyPolyData(mapOf("a" to PolyDerived("foo"))),
9294
serializer = MyPolyData.serializer(),
93-
format = json
95+
json = json
9496
)
9597
}
9698

9799
/**
98100
* This test should fail because PolyDerived registered in the scope of kotlin.Any, not PolyBase
99101
*/
100102
@Test
101-
fun testFailWithModulesNotInParticularScope() {
103+
fun testFailWithModulesNotInParticularScope() = parametrizedTest { mode ->
102104
val json = Json { serializersModule = baseAndDerivedModuleAtAny }
103105
checkNotRegisteredMessage(
104106
"kotlinx.serialization.PolyDerived", "kotlinx.serialization.PolyBase",
@@ -108,7 +110,8 @@ class PolymorphismWithAnyTest {
108110
MyPolyDataWithPolyBase(
109111
mapOf("a" to PolyDerived("foo")),
110112
PolyDerived("foo")
111-
)
113+
),
114+
mode
112115
)
113116
}
114117
)
@@ -117,17 +120,30 @@ class PolymorphismWithAnyTest {
117120
@Test
118121
fun testBindModules() {
119122
val json = Json { serializersModule = (baseAndDerivedModuleAtAny + BaseAndDerivedModule) }
120-
assertStringFormAndRestored(
123+
assertJsonFormAndRestored(
121124
expected = """{"data":{"a":{"type":"kotlinx.serialization.PolyDerived","id":1,"s":"foo"}},
122125
|"polyBase":{"type":"kotlinx.serialization.PolyDerived","id":1,"s":"foo"}}""".trimMargin().lines().joinToString(
123126
""
124127
),
125-
original = MyPolyDataWithPolyBase(
128+
data = MyPolyDataWithPolyBase(
126129
mapOf("a" to PolyDerived("foo")),
127130
PolyDerived("foo")
128131
),
129132
serializer = MyPolyDataWithPolyBase.serializer(),
130-
format = json
133+
json = json
131134
)
132135
}
136+
137+
@Test
138+
fun testTypeKeyLastInInput() = parametrizedTest { mode ->
139+
val json = Json { serializersModule = (baseAndDerivedModuleAtAny + BaseAndDerivedModule) }
140+
val input = """{"data":{"a":{"id":1,"s":"foo","type":"kotlinx.serialization.PolyDerived"}},
141+
|"polyBase":{"id":1,"s":"foo","type":"kotlinx.serialization.PolyDerived"}}""".trimMargin().lines().joinToString(
142+
"")
143+
val data = MyPolyDataWithPolyBase(
144+
mapOf("a" to PolyDerived("foo")),
145+
PolyDerived("foo")
146+
)
147+
assertEquals(data, json.decodeFromString(MyPolyDataWithPolyBase.serializer(), input, mode))
148+
}
133149
}

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonExceptions.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ internal fun UnknownKeyException(key: String, input: String) = JsonDecodingExcep
8282
"Current input: ${input.minify()}"
8383
)
8484

85-
private fun CharSequence.minify(offset: Int = -1): CharSequence {
85+
internal fun CharSequence.minify(offset: Int = -1): CharSequence {
8686
if (length < 200) return this
8787
if (offset == -1) {
8888
val start = this.length - 60

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,15 @@ internal fun <T> JsonDecoder.decodeSerializableValuePolymorphic(deserializer: De
6363
val discriminator = deserializer.descriptor.classDiscriminator(json)
6464

6565
val jsonTree = cast<JsonObject>(decodeJsonElement(), deserializer.descriptor)
66-
val type = jsonTree[discriminator]?.jsonPrimitive?.content
67-
val actualSerializer = deserializer.findPolymorphicSerializerOrNull(this, type)
68-
?: throwSerializerNotFound(type, jsonTree)
69-
66+
val type = jsonTree[discriminator]?.jsonPrimitive?.contentOrNull // differentiate between `"type":"null"` and `"type":null`.
7067
@Suppress("UNCHECKED_CAST")
71-
return json.readPolymorphicJson(discriminator, jsonTree, actualSerializer as DeserializationStrategy<T>)
72-
}
73-
74-
@JvmName("throwSerializerNotFound")
75-
internal fun throwSerializerNotFound(type: String?, jsonTree: JsonObject): Nothing {
76-
val suffix =
77-
if (type == null) "missing class discriminator ('null')"
78-
else "class discriminator '$type'"
79-
throw JsonDecodingException(-1, "Polymorphic serializer was not found for $suffix", jsonTree.toString())
68+
val actualSerializer =
69+
try {
70+
deserializer.findPolymorphicSerializer(this, type)
71+
} catch (it: SerializationException) { // Wrap SerializationException into JsonDecodingException to preserve input
72+
throw JsonDecodingException(-1, it.message!!, jsonTree.toString())
73+
} as DeserializationStrategy<T>
74+
return json.readPolymorphicJson(discriminator, jsonTree, actualSerializer)
8075
}
8176

8277
internal fun SerialDescriptor.classDiscriminator(json: Json): String {

‎formats/json/commonMain/src/kotlinx/serialization/json/internal/StreamingJsonDecoder.kt‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,22 @@ internal open class StreamingJsonDecoder(
7171

7272
val discriminator = deserializer.descriptor.classDiscriminator(json)
7373
val type = lexer.peekLeadingMatchingValue(discriminator, configuration.isLenient)
74-
var actualSerializer: DeserializationStrategy<Any>? = null
75-
if (type != null) {
76-
actualSerializer = deserializer.findPolymorphicSerializerOrNull(this, type)
77-
}
78-
if (actualSerializer == null) {
79-
// Fallback if we haven't found discriminator or serializer
74+
?: // Fallback to slow path if we haven't found discriminator on first try
8075
return decodeSerializableValuePolymorphic<T>(deserializer as DeserializationStrategy<T>)
81-
}
8276

83-
discriminatorHolder = DiscriminatorHolder(discriminator)
8477
@Suppress("UNCHECKED_CAST")
85-
val result = actualSerializer.deserialize(this) as T
86-
return result
78+
val actualSerializer = try {
79+
deserializer.findPolymorphicSerializer(this, type)
80+
} catch (it: SerializationException) { // Wrap SerializationException into JsonDecodingException to preserve position, path, and input.
81+
// Split multiline message from private core function:
82+
// core/commonMain/src/kotlinx/serialization/internal/AbstractPolymorphicSerializer.kt:102
83+
val message = it.message!!.substringBefore('\n').removeSuffix(".")
84+
val hint = it.message!!.substringAfter('\n', missingDelimiterValue = "")
85+
lexer.fail(message, hint = hint)
86+
} as DeserializationStrategy<T>
87+
88+
discriminatorHolder = DiscriminatorHolder(discriminator)
89+
return actualSerializer.deserialize(this)
8790

8891
} catch (e: MissingFieldException) {
8992
// Add "at path" if and only if we've just caught an exception and it hasn't been augmented yet

‎guide/test/PolymorphismTest.kt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ class PolymorphismTest {
2323
@Test
2424
fun testExamplePoly03() {
2525
captureOutput("ExamplePoly03") { example.examplePoly03.main() }.verifyOutputLinesStart(
26-
"Exception in thread \"main\" kotlinx.serialization.SerializationException: Class 'OwnedProject' is not registered for polymorphic serialization in the scope of 'Project'.",
27-
"To be registered automatically, class 'OwnedProject' has to be '@Serializable', and the base class 'Project' has to be sealed and '@Serializable'.",
28-
"Alternatively, register the serializer for 'OwnedProject' explicitly in a corresponding SerializersModule."
26+
"Exception in thread \"main\" kotlinx.serialization.SerializationException: Serializer for subclass 'OwnedProject' is not found in the polymorphic scope of 'Project'.",
27+
"Check if class with serial name 'OwnedProject' exists and serializer is registered in a corresponding SerializersModule.",
28+
"To be registered automatically, class 'OwnedProject' has to be '@Serializable', and the base class 'Project' has to be sealed and '@Serializable'."
2929
)
3030
}
3131

@@ -133,7 +133,8 @@ class PolymorphismTest {
133133
@Test
134134
fun testExamplePoly18() {
135135
captureOutput("ExamplePoly18") { example.examplePoly18.main() }.verifyOutputLinesStart(
136-
"Exception in thread \"main\" kotlinx.serialization.json.internal.JsonDecodingException: Polymorphic serializer was not found for class discriminator 'unknown'"
136+
"Exception in thread \"main\" kotlinx.serialization.json.internal.JsonDecodingException: Unexpected JSON token at offset 0: Serializer for subclass 'unknown' is not found in the polymorphic scope of 'Project' at path: $",
137+
"Check if class with serial name 'unknown' exists and serializer is registered in a corresponding SerializersModule."
137138
)
138139
}
139140

‎integration-test/src/commonTest/kotlin/sample/JsonTest.kt‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import kotlinx.serialization.modules.*
1212
import kotlin.reflect.*
1313
import kotlin.test.*
1414

15-
public val jsonWithDefaults = Json { encodeDefaults = true }
15+
val jsonWithDefaults = Json { encodeDefaults = true }
1616

1717
class JsonTest {
1818

@@ -129,10 +129,9 @@ class JsonTest {
129129
assertEquals("""Derived2(state1='foo')""", restored2.toString())
130130
}
131131

132-
@Suppress("NAME_SHADOWING")
133132
private fun checkNotRegisteredMessage(exception: SerializationException) {
134133
val expectedText =
135-
"is not registered for polymorphic serialization in the scope of"
134+
"is not found in the polymorphic scope of"
136135
assertEquals(true, exception.message?.contains(expectedText))
137136
}
138137

0 commit comments

Comments
 (0)