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

Do not check kind or discriminator collisions #2833

Merged
merged 4 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 :
Expand Down Expand Up @@ -80,5 +82,44 @@ class ClassDiscriminatorModeNoneTest :

@Test
fun testNullable() = testNullable("""{"sb":null,"sc":null}""")

interface CommandType

@Serializable // For Kotlin/JS
enum class Modify : CommandType {
CREATE, DELETE
}

@Serializable
class Command(val cmd: CommandType)

@Test
fun testNoneModeAllowsPolymorphicEnums() {
val module = SerializersModule {
polymorphic(CommandType::class) {
subclass(Modify::class, Modify.serializer())
}
}
val j = Json(default) { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE }
parametrizedTest { mode ->
assertEquals("""{"cmd":"CREATE"}""", j.encodeToString(Command(Modify.CREATE), mode))
}
}

@Serializable
class SomeCommand(val type: String) : CommandType
sandwwraith marked this conversation as resolved.
Show resolved Hide resolved

@Test
fun testNoneModeAllowsDiscriminatorClash() {
val module = SerializersModule {
polymorphic(CommandType::class) {
subclass(SomeCommand::class)
}
}
val j = Json(default) { serializersModule = module; classDiscriminatorMode = ClassDiscriminatorMode.NONE }
parametrizedTest { mode ->
assertEquals("""{"cmd":{"type":"foo"}}""", j.encodeToString(Command(SomeCommand("foo")), mode))
}
}
}

11 changes: 10 additions & 1 deletion formats/json/commonMain/src/kotlinx/serialization/json/Json.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -669,7 +678,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)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <T : Any> contextual(
kClass: KClass<T>,
provider: (typeArgumentsSerializers: List<KSerializer<*>>) -> KSerializer<*>
Expand All @@ -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)
}
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ internal inline fun <T> JsonEncoder.encodePolymorphically(
val casted = serializer as AbstractPolymorphicSerializer<Any>
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<T>
} else serializer

Expand Down