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

Improve polymorphic deserialization optimization: #2481

Merged
merged 5 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ public abstract class AbstractPolymorphicSerializer<T : Any> internal constructo
}
else -> throw SerializationException(
"Invalid index in polymorphic deserialization of " +
(klassName ?: "unknown class") +
"\n Expected 0, 1 or DECODE_DONE(-1), but found $index"
(klassName ?: "unknown class") +
"\n Expected 0, 1 or DECODE_DONE(-1), but found $index"
)
}
}
Expand Down Expand Up @@ -98,14 +98,14 @@ public abstract class AbstractPolymorphicSerializer<T : Any> internal constructo

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

Expand Down
7 changes: 4 additions & 3 deletions docs/polymorphism.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ fun main() {
This is close to the best design for a serializable hierarchy of classes, but running it produces the following error:

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

<!--- TEST LINES_START -->
Expand Down Expand Up @@ -832,7 +832,8 @@ fun main() {
We get the following exception.

```text
Exception in thread "main" kotlinx.serialization.json.internal.JsonDecodingException: Polymorphic serializer was not found for class discriminator 'unknown'
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: $
Check if class with serial name 'unknown' exists and serializer is registered in a corresponding SerializersModule.
```

<!--- TEST LINES_START -->
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright 2017-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

package kotlinx.serialization.features

import kotlinx.serialization.*
import kotlinx.serialization.json.*
import kotlin.test.*

class PolymorphicDeserializationErrorMessagesTest : JsonTestBase() {
@Serializable
class DummyData(@Polymorphic val a: Any)

@Serializable
class Holder(val d: DummyData)

// TODO: remove this after #2480 is merged
private fun checkSerializationException(action: () -> Unit, assertions: SerializationException.(String) -> Unit) {
val e = assertFailsWith(SerializationException::class, action)
assertNotNull(e.message)
e.assertions(e.message!!)
}

@Test
fun testNotRegisteredMessage() = parametrizedTest { mode ->
val input = """{"d":{"a":{"type":"my.Class", "value":42}}}"""
checkSerializationException({
default.decodeFromString<Holder>(input, mode)
}, { message ->
// ReaderJsonLexer.peekLeadingMatchingValue is not implemented, so first-key optimization is not working for streaming yet.
if (mode == JsonTestingMode.STREAMING)
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")
else
assertContains(message, "Serializer for subclass 'my.Class' is not found in the polymorphic scope of 'Any'")
})
}

@Test
fun testDiscriminatorMissingNoDefaultMessage() = parametrizedTest { mode ->
val input = """{"d":{"a":{"value":42}}}"""
checkSerializationException({
default.decodeFromString<Holder>(input, mode)
}, { message ->
// Always slow path when discriminator is missing, so no position and path
assertContains(message, "Class discriminator was missing and no default serializers were registered in the polymorphic scope of 'Any'")
})
}

@Test
fun testClassDiscriminatorIsNull() = parametrizedTest { mode ->
val input = """{"d":{"a":{"type":null, "value":42}}}"""
checkSerializationException({
default.decodeFromString<Holder>(input, mode)
}, { message ->
// Always slow path when discriminator is missing, so no position and path
assertContains(message, "Class discriminator was missing and no default serializers were registered in the polymorphic scope of 'Any'")
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
package kotlinx.serialization.features

import kotlinx.serialization.*
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.*
import kotlinx.serialization.modules.*
import kotlinx.serialization.modules.plus
import kotlinx.serialization.test.assertStringFormAndRestored
import kotlin.test.*

class PolymorphismWithAnyTest {
class PolymorphismWithAnyTest: JsonTestBase() {

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

@Test
fun testFailWithoutModulesWithCustomClass() {
fun testFailWithoutModulesWithCustomClass() = parametrizedTest { mode ->
checkNotRegisteredMessage(
"kotlinx.serialization.IntData", "kotlin.Any",
assertFailsWith<SerializationException>("not registered") {
Json.encodeToString(
MyPolyData.serializer(),
MyPolyData(mapOf("a" to IntData(42)))
MyPolyData(mapOf("a" to IntData(42))),
mode
)
}
)
Expand All @@ -51,26 +52,27 @@ class PolymorphismWithAnyTest {
val json = Json {
serializersModule = SerializersModule { polymorphic(Any::class) { subclass(IntData.serializer()) } }
}
assertStringFormAndRestored(
assertJsonFormAndRestored(
expected = """{"data":{"a":{"type":"kotlinx.serialization.IntData","intV":42}}}""",
original = MyPolyData(mapOf("a" to IntData(42))),
data = MyPolyData(mapOf("a" to IntData(42))),
serializer = MyPolyData.serializer(),
format = json
json = json
)
}

/**
* This test should fail because PolyDerived registered in the scope of PolyBase, not kotlin.Any
*/
@Test
fun testFailWithModulesNotInAnyScope() {
fun testFailWithModulesNotInAnyScope() = parametrizedTest { mode ->
val json = Json { serializersModule = BaseAndDerivedModule }
checkNotRegisteredMessage(
"kotlinx.serialization.PolyDerived", "kotlin.Any",
assertFailsWith<SerializationException> {
json.encodeToString(
MyPolyData.serializer(),
MyPolyData(mapOf("a" to PolyDerived("foo")))
MyPolyData(mapOf("a" to PolyDerived("foo"))),
mode
)
}
)
Expand All @@ -86,19 +88,19 @@ class PolymorphismWithAnyTest {
@Test
fun testRebindModules() {
val json = Json { serializersModule = baseAndDerivedModuleAtAny }
assertStringFormAndRestored(
assertJsonFormAndRestored(
expected = """{"data":{"a":{"type":"kotlinx.serialization.PolyDerived","id":1,"s":"foo"}}}""",
original = MyPolyData(mapOf("a" to PolyDerived("foo"))),
data = MyPolyData(mapOf("a" to PolyDerived("foo"))),
serializer = MyPolyData.serializer(),
format = json
json = json
)
}

/**
* This test should fail because PolyDerived registered in the scope of kotlin.Any, not PolyBase
*/
@Test
fun testFailWithModulesNotInParticularScope() {
fun testFailWithModulesNotInParticularScope() = parametrizedTest { mode ->
val json = Json { serializersModule = baseAndDerivedModuleAtAny }
checkNotRegisteredMessage(
"kotlinx.serialization.PolyDerived", "kotlinx.serialization.PolyBase",
Expand All @@ -108,7 +110,8 @@ class PolymorphismWithAnyTest {
MyPolyDataWithPolyBase(
mapOf("a" to PolyDerived("foo")),
PolyDerived("foo")
)
),
mode
)
}
)
Expand All @@ -117,17 +120,17 @@ class PolymorphismWithAnyTest {
@Test
fun testBindModules() {
val json = Json { serializersModule = (baseAndDerivedModuleAtAny + BaseAndDerivedModule) }
assertStringFormAndRestored(
assertJsonFormAndRestored(
expected = """{"data":{"a":{"type":"kotlinx.serialization.PolyDerived","id":1,"s":"foo"}},
sandwwraith marked this conversation as resolved.
Show resolved Hide resolved
|"polyBase":{"type":"kotlinx.serialization.PolyDerived","id":1,"s":"foo"}}""".trimMargin().lines().joinToString(
""
),
original = MyPolyDataWithPolyBase(
data = MyPolyDataWithPolyBase(
mapOf("a" to PolyDerived("foo")),
PolyDerived("foo")
),
serializer = MyPolyDataWithPolyBase.serializer(),
format = json
json = json
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ internal fun UnknownKeyException(key: String, input: String) = JsonDecodingExcep
"Current input: ${input.minify()}"
)

private fun CharSequence.minify(offset: Int = -1): CharSequence {
internal fun CharSequence.minify(offset: Int = -1): CharSequence {
if (length < 200) return this
if (offset == -1) {
val start = this.length - 60
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,15 @@ internal fun <T> JsonDecoder.decodeSerializableValuePolymorphic(deserializer: De
val discriminator = deserializer.descriptor.classDiscriminator(json)

val jsonTree = cast<JsonObject>(decodeJsonElement(), deserializer.descriptor)
val type = jsonTree[discriminator]?.jsonPrimitive?.content
val actualSerializer = deserializer.findPolymorphicSerializerOrNull(this, type)
?: throwSerializerNotFound(type, jsonTree)

val type = jsonTree[discriminator]?.jsonPrimitive?.contentOrNull // differentiate between `"type":"null"` and `"type":null`.
@Suppress("UNCHECKED_CAST")
return json.readPolymorphicJson(discriminator, jsonTree, actualSerializer as DeserializationStrategy<T>)
}

@JvmName("throwSerializerNotFound")
internal fun throwSerializerNotFound(type: String?, jsonTree: JsonObject): Nothing {
val suffix =
if (type == null) "missing class discriminator ('null')"
else "class discriminator '$type'"
throw JsonDecodingException(-1, "Polymorphic serializer was not found for $suffix", jsonTree.toString())
val actualSerializer =
try {
deserializer.findPolymorphicSerializer(this, type)
} catch (it: SerializationException) { // Wrap SerializationException into JsonDecodingException to preserve input
throw JsonDecodingException(-1, it.message!!, jsonTree.toString())
} as DeserializationStrategy<T>
return json.readPolymorphicJson(discriminator, jsonTree, actualSerializer)
}

internal fun SerialDescriptor.classDiscriminator(json: Json): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,20 @@ internal open class StreamingJsonDecoder(

val discriminator = deserializer.descriptor.classDiscriminator(json)
val type = lexer.peekLeadingMatchingValue(discriminator, configuration.isLenient)
var actualSerializer: DeserializationStrategy<Any>? = null
if (type != null) {
actualSerializer = deserializer.findPolymorphicSerializerOrNull(this, type)
}
if (actualSerializer == null) {
// Fallback if we haven't found discriminator or serializer
?: // Fallback to slow path if we haven't found discriminator on first try
return decodeSerializableValuePolymorphic<T>(deserializer as DeserializationStrategy<T>)
}

discriminatorHolder = DiscriminatorHolder(discriminator)
@Suppress("UNCHECKED_CAST")
val result = actualSerializer.deserialize(this) as T
return result
val actualSerializer = try {
deserializer.findPolymorphicSerializer(this, type)
} catch (it: SerializationException) { // Wrap SerializationException into JsonDecodingException to preserve position, path, and input.
val message = it.message!!.substringBefore('\n').removeSuffix(".")
sandwwraith marked this conversation as resolved.
Show resolved Hide resolved
val hint = it.message!!.substringAfter('\n', missingDelimiterValue = "")
lexer.fail(message, hint = hint)
} as DeserializationStrategy<T>

discriminatorHolder = DiscriminatorHolder(discriminator)
return actualSerializer.deserialize(this)

} catch (e: MissingFieldException) {
// Add "at path" if and only if we've just caught an exception and it hasn't been augmented yet
Expand Down
9 changes: 5 additions & 4 deletions guide/test/PolymorphismTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ class PolymorphismTest {
@Test
fun testExamplePoly03() {
captureOutput("ExamplePoly03") { example.examplePoly03.main() }.verifyOutputLinesStart(
"Exception in thread \"main\" kotlinx.serialization.SerializationException: Class 'OwnedProject' is not registered for polymorphic serialization in the scope of 'Project'.",
"To be registered automatically, class 'OwnedProject' has to be '@Serializable', and the base class 'Project' has to be sealed and '@Serializable'.",
"Alternatively, register the serializer for 'OwnedProject' explicitly in a corresponding SerializersModule."
"Exception in thread \"main\" kotlinx.serialization.SerializationException: Serializer for subclass 'OwnedProject' is not found in the polymorphic scope of 'Project'.",
"Check if class with serial name 'OwnedProject' exists and serializer is registered in a corresponding SerializersModule.",
"To be registered automatically, class 'OwnedProject' has to be '@Serializable', and the base class 'Project' has to be sealed and '@Serializable'."
)
}

Expand Down Expand Up @@ -133,7 +133,8 @@ class PolymorphismTest {
@Test
fun testExamplePoly18() {
captureOutput("ExamplePoly18") { example.examplePoly18.main() }.verifyOutputLinesStart(
"Exception in thread \"main\" kotlinx.serialization.json.internal.JsonDecodingException: Polymorphic serializer was not found for class discriminator 'unknown'"
"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: $",
"Check if class with serial name 'unknown' exists and serializer is registered in a corresponding SerializersModule."
)
}

Expand Down
2 changes: 1 addition & 1 deletion integration-test/src/commonTest/kotlin/sample/JsonTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class JsonTest {
@Suppress("NAME_SHADOWING")
private fun checkNotRegisteredMessage(exception: SerializationException) {
val expectedText =
"is not registered for polymorphic serialization in the scope of"
"is not found in the polymorphic scope of"
assertEquals(true, exception.message?.contains(expectedText))
}

Expand Down