From bf8629a96bd2e0b31bde9320eb18e0947e303174 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Thu, 10 Oct 2024 18:10:32 +0200 Subject: [PATCH 1/4] Do not check kind or discriminator collisions for subclasses' polymorphic serializers if Json.classDiscriminatorMode is set to NONE. This should simplify the code in situations where JSON is expected to be only sent and not parsed. Fixes #2753 Fixes #1486 --- .../JsonClassDiscriminatorModeTest.kt | 40 +++++++++++++++++++ .../src/kotlinx/serialization/json/Json.kt | 2 +- ...r.kt => JsonSerializersModuleValidator.kt} | 13 ++++-- .../json/internal/Polymorphic.kt | 6 ++- 4 files changed, 54 insertions(+), 7 deletions(-) rename formats/json/commonMain/src/kotlinx/serialization/json/internal/{PolymorphismValidator.kt => JsonSerializersModuleValidator.kt} (86%) diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt index b2f4713727..56b29b64c4 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt @@ -4,7 +4,9 @@ package kotlinx.serialization.json.polymorphic +import kotlinx.serialization.* import kotlinx.serialization.json.* +import kotlinx.serialization.modules.* import kotlin.test.* class ClassDiscriminatorModeAllObjectsTest : @@ -80,5 +82,43 @@ class ClassDiscriminatorModeNoneTest : @Test fun testNullable() = testNullable("""{"sb":null,"sc":null}""") + + interface CommandType + + enum class Modify : CommandType { + CREATE, DELETE + } + + @Serializable + class Command(val cmd: CommandType) + + @Test + fun testNoneModeAllowsPolymorphicEnums() { + val module = SerializersModule { + polymorphic(CommandType::class) { + subclass(Modify::class) + } + } + val j = Json { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE } + parametrizedTest { mode -> + assertEquals("""{"cmd":"CREATE"}""", j.encodeToString(Command(Modify.CREATE), mode)) + } + } + + @Serializable + class SomeCommand(val type: String) : CommandType + + @Test + fun testNoneModeAllowsDiscriminatorClash() { + val module = SerializersModule { + polymorphic(CommandType::class) { + subclass(SomeCommand::class) + } + } + val j = Json { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE } + parametrizedTest { mode -> + assertEquals("""{"cmd":{"type":"foo"}}""", j.encodeToString(Command(SomeCommand("foo")), mode)) + } + } } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt index fe6b094d88..bf3a651eea 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt @@ -669,7 +669,7 @@ private class JsonImpl(configuration: JsonConfiguration, module: SerializersModu private fun validateConfiguration() { if (serializersModule == EmptySerializersModule()) return // Fast-path for in-place JSON allocations - val collector = PolymorphismValidator(configuration.useArrayPolymorphism, configuration.classDiscriminator) + val collector = JsonSerializersModuleValidator(configuration) serializersModule.dumpTo(collector) } } diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/PolymorphismValidator.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonSerializersModuleValidator.kt similarity index 86% rename from formats/json/commonMain/src/kotlinx/serialization/json/internal/PolymorphismValidator.kt rename to formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonSerializersModuleValidator.kt index e4606fae05..0b00f9da28 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/PolymorphismValidator.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonSerializersModuleValidator.kt @@ -6,15 +6,19 @@ package kotlinx.serialization.json.internal import kotlinx.serialization.* import kotlinx.serialization.descriptors.* +import kotlinx.serialization.json.* import kotlinx.serialization.modules.* import kotlin.reflect.* @OptIn(ExperimentalSerializationApi::class) -internal class PolymorphismValidator( - private val useArrayPolymorphism: Boolean, - private val discriminator: String +internal class JsonSerializersModuleValidator( + configuration: JsonConfiguration, ) : SerializersModuleCollector { + private val discriminator: String = configuration.classDiscriminator + private val useArrayPolymorphism: Boolean = configuration.useArrayPolymorphism + private val isDiscriminatorRequired = configuration.classDiscriminatorMode != ClassDiscriminatorMode.NONE + override fun contextual( kClass: KClass, provider: (typeArgumentsSerializers: List>) -> KSerializer<*> @@ -29,7 +33,7 @@ internal class PolymorphismValidator( ) { val descriptor = actualSerializer.descriptor checkKind(descriptor, actualClass) - if (!useArrayPolymorphism) { + if (!useArrayPolymorphism && isDiscriminatorRequired) { // Collisions with "type" can happen only for JSON polymorphism checkDiscriminatorCollisions(descriptor, actualClass) } @@ -43,6 +47,7 @@ internal class PolymorphismValidator( } if (useArrayPolymorphism) return + if (!isDiscriminatorRequired) return /* * For this kind we can't intercept the JSON object {} in order to add "type: ...". * Except for maps that just can clash and accidentally overwrite the type. diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt b/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt index acc0bf4737..26d752661f 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt @@ -37,8 +37,10 @@ internal inline fun JsonEncoder.encodePolymorphically( val casted = serializer as AbstractPolymorphicSerializer requireNotNull(value) { "Value for serializer ${serializer.descriptor} should always be non-null. Please report issue to the kotlinx.serialization tracker." } val actual = casted.findPolymorphicSerializer(this, value) - if (baseClassDiscriminator != null) validateIfSealed(serializer, actual, baseClassDiscriminator) - checkKind(actual.descriptor.kind) + if (baseClassDiscriminator != null) { + validateIfSealed(serializer, actual, baseClassDiscriminator) + checkKind(actual.descriptor.kind) + } actual as SerializationStrategy } else serializer From 87ccd8033b670825aa6b3cdcff1b918be7d0182a Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Thu, 10 Oct 2024 18:12:19 +0200 Subject: [PATCH 2/4] fixup! Do not check kind or discriminator collisions --- .../json/polymorphic/JsonClassDiscriminatorModeTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt index 56b29b64c4..8e75f9dbdc 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt @@ -99,7 +99,7 @@ class ClassDiscriminatorModeNoneTest : subclass(Modify::class) } } - val j = Json { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE } + val j = Json(default) { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE } parametrizedTest { mode -> assertEquals("""{"cmd":"CREATE"}""", j.encodeToString(Command(Modify.CREATE), mode)) } @@ -115,7 +115,7 @@ class ClassDiscriminatorModeNoneTest : subclass(SomeCommand::class) } } - val j = Json { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE } + val j = Json(default) { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE } parametrizedTest { mode -> assertEquals("""{"cmd":{"type":"foo"}}""", j.encodeToString(Command(SomeCommand("foo")), mode)) } From 629a70a5c4a479719acc989256fa059ebf91fe20 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Fri, 11 Oct 2024 11:25:05 +0200 Subject: [PATCH 3/4] fixup! Do not check kind or discriminator collisions --- .../json/polymorphic/JsonClassDiscriminatorModeTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt index 8e75f9dbdc..d55e7559ed 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt @@ -85,6 +85,7 @@ class ClassDiscriminatorModeNoneTest : interface CommandType + @Serializable // For Kotlin/JS enum class Modify : CommandType { CREATE, DELETE } @@ -96,7 +97,7 @@ class ClassDiscriminatorModeNoneTest : fun testNoneModeAllowsPolymorphicEnums() { val module = SerializersModule { polymorphic(CommandType::class) { - subclass(Modify::class) + subclass(Modify::class, Modify.serializer()) } } val j = Json(default) { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE } From d61c197d604857aace26c3a5eb920b1ec2498ed9 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Mon, 28 Oct 2024 19:28:24 +0100 Subject: [PATCH 4/4] ~add documentation --- .../commonMain/src/kotlinx/serialization/json/Json.kt | 9 +++++++++ .../src/kotlinx/serialization/json/JsonConfiguration.kt | 3 +++ 2 files changed, 12 insertions(+) diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt index bf3a651eea..a0fc7b1f8d 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt @@ -495,6 +495,13 @@ public class JsonBuilder internal constructor(json: Json) { /** * Name of the class descriptor property for polymorphic serialization. * `type` by default. + * + * Note that if your class has any serial names that are equal to [classDiscriminator] + * (e.g., `@Serializable class Foo(val type: String)`), an [IllegalArgumentException] will be thrown from `Json {}` builder. + * You can disable this check and class discriminator inclusion with [ClassDiscriminatorMode.NONE], but kotlinx.serialization will not be + * able to deserialize such data back. + * + * @see classDiscriminatorMode */ public var classDiscriminator: String = json.configuration.classDiscriminator @@ -504,6 +511,8 @@ public class JsonBuilder internal constructor(json: Json) { * * Other modes are generally intended to produce JSON for consumption by third-party libraries, * therefore, this setting does not affect the deserialization process. + * + * @see classDiscriminator */ @ExperimentalSerializationApi public var classDiscriminatorMode: ClassDiscriminatorMode = json.configuration.classDiscriminatorMode diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt b/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt index ade53a6a44..3be703a3c5 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/JsonConfiguration.kt @@ -81,6 +81,9 @@ public enum class ClassDiscriminatorMode { * This mode is generally intended to produce JSON for consumption by third-party libraries. * kotlinx.serialization is unable to deserialize [polymorphic classes][POLYMORPHIC] without class discriminators, * so it is impossible to deserialize JSON produced in this mode if a data model has polymorphic classes. + * + * Using this mode relaxes several configuration checks in [Json]. In particular, it is possible to serialize enums and primitives + * as polymorphic subclasses in this mode, since it is no longer required for them to have outer `{}` object to include class discriminator. */ NONE,