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

2 files changed, 94 insertions, 18 deletions

diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphicDeserializationErrorMessagesTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphicDeserializationErrorMessagesTest.kt
new file mode 100644
index 00000000..2b2f1f70
--- /dev/null
+++ b/formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphicDeserializationErrorMessagesTest.kt
@@ -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'")
+        })
+    }
+}
diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphismWithAnyTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphismWithAnyTest.kt
index e1d38fdd..07b6e31a 100644
--- a/formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphismWithAnyTest.kt
+++ b/formats/json-tests/commonTest/src/kotlinx/serialization/features/PolymorphismWithAnyTest.kt
@@ -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>)
@@ -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
                 )
             }
         )
@@ -51,11 +52,11 @@ 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
         )
     }
 
@@ -63,14 +64,15 @@ class PolymorphismWithAnyTest {
      * 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
                 )
             }
         )
@@ -86,11 +88,11 @@ 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
         )
     }
 
@@ -98,7 +100,7 @@ class PolymorphismWithAnyTest {
      * 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",
@@ -108,7 +110,8 @@ class PolymorphismWithAnyTest {
                     MyPolyDataWithPolyBase(
                         mapOf("a" to PolyDerived("foo")),
                         PolyDerived("foo")
-                    )
+                    ),
+                    mode
                 )
             }
         )
@@ -117,17 +120,30 @@ class PolymorphismWithAnyTest {
     @Test
     fun testBindModules() {
         val json = Json { serializersModule = (baseAndDerivedModuleAtAny + BaseAndDerivedModule) }
-        assertStringFormAndRestored(
+        assertJsonFormAndRestored(
             expected = """{"data":{"a":{"type":"kotlinx.serialization.PolyDerived","id":1,"s":"foo"}},
                 |"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
         )
     }
+
+    @Test
+    fun testTypeKeyLastInInput() = parametrizedTest { mode ->
+        val json = Json { serializersModule = (baseAndDerivedModuleAtAny + BaseAndDerivedModule) }
+        val input = """{"data":{"a":{"id":1,"s":"foo","type":"kotlinx.serialization.PolyDerived"}},
+                |"polyBase":{"id":1,"s":"foo","type":"kotlinx.serialization.PolyDerived"}}""".trimMargin().lines().joinToString(
+            "")
+        val data = MyPolyDataWithPolyBase(
+            mapOf("a" to PolyDerived("foo")),
+            PolyDerived("foo")
+        )
+        assertEquals(data, json.decodeFromString(MyPolyDataWithPolyBase.serializer(), input, mode))
+    }
 }