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

@JsonClassDiscriminator and other serial info annotations in sealed, polymorphic and object serializables #1515

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 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 @@ -55,7 +55,9 @@ public class ContextualSerializer<T : Any>(
public constructor(serializableClass: KClass<T>) : this(serializableClass, null, EMPTY_SERIALIZER_ARRAY)

public override val descriptor: SerialDescriptor =
buildSerialDescriptor("kotlinx.serialization.ContextualSerializer", SerialKind.CONTEXTUAL).withContext(serializableClass)
buildSerialDescriptor("kotlinx.serialization.ContextualSerializer", SerialKind.CONTEXTUAL) {
annotations = fallbackSerializer?.descriptor?.annotations.orEmpty()
}.withContext(serializableClass)

public override fun serialize(encoder: Encoder, value: T) {
encoder.encodeSerializableValue(serializer(encoder.serializersModule), value)
Expand Down
20 changes: 17 additions & 3 deletions core/commonMain/src/kotlinx/serialization/PolymorphicSerializer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,28 @@ import kotlin.reflect.*
*/
@OptIn(ExperimentalSerializationApi::class)
public class PolymorphicSerializer<T : Any>(override val baseClass: KClass<T>) : AbstractPolymorphicSerializer<T>() {
public override val descriptor: SerialDescriptor =

@PublishedApi // should we allow user access to this constructor?
internal constructor(
baseClass: KClass<T>,
classAnnotations: Array<Annotation>
) : this(baseClass) {
_descriptor.annotations = classAnnotations.asList()
}

private val _descriptor: SerialDescriptorImpl =
buildSerialDescriptor("kotlinx.serialization.Polymorphic", PolymorphicKind.OPEN) {
element("type", String.serializer().descriptor)
element(
"value",
buildSerialDescriptor("kotlinx.serialization.Polymorphic<${baseClass.simpleName}>", SerialKind.CONTEXTUAL)
buildSerialDescriptor(
"kotlinx.serialization.Polymorphic<${baseClass.simpleName}>",
SerialKind.CONTEXTUAL
)
)
}.withContext(baseClass)
} as SerialDescriptorImpl

public override val descriptor: SerialDescriptor = _descriptor.withContext(baseClass)

override fun toString(): String {
return "kotlinx.serialization.PolymorphicSerializer(baseClass: $baseClass)"
Expand Down
12 changes: 11 additions & 1 deletion core/commonMain/src/kotlinx/serialization/SealedSerializer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ public class SealedClassSerializer<T : Any>(
subclassSerializers: Array<KSerializer<out T>>
) : AbstractPolymorphicSerializer<T>() {

@PublishedApi
internal constructor(
serialName: String,
baseClass: KClass<T>,
subclasses: Array<KClass<out T>>,
subclassSerializers: Array<KSerializer<out T>>,
classAnnotations: Array<Annotation>
) : this(serialName, baseClass, subclasses, subclassSerializers) {
(this.descriptor as SerialDescriptorImpl).annotations = classAnnotations.asList()
}

override val descriptor: SerialDescriptor = buildSerialDescriptor(serialName, PolymorphicKind.SEALED) {
element("type", String.serializer().descriptor)
val elementDescriptor =
Expand All @@ -87,7 +98,6 @@ public class SealedClassSerializer<T : Any>(
}
}
element("value", elementDescriptor)

}

private val class2Serializer: Map<KClass<out T>, KSerializer<out T>>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,8 @@ internal class SerialDescriptorImpl(
builder: ClassSerialDescriptorBuilder
) : SerialDescriptor, CachedNames {

override val annotations: List<Annotation> = builder.annotations
override var annotations: List<Annotation> = builder.annotations
internal set
override val serialNames: Set<String> = builder.elementNames.toHashSet()

private val elementNames: Array<String> = builder.elementNames.toTypedArray()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ import kotlinx.serialization.encoding.*
@PublishedApi
@OptIn(ExperimentalSerializationApi::class)
internal class ObjectSerializer<T : Any>(serialName: String, private val objectInstance: T) : KSerializer<T> {
@PublishedApi
internal constructor(
serialName: String,
objectInstance: T,
classAnnotations: Array<Annotation>
) : this(serialName, objectInstance) {
(this.descriptor as SerialDescriptorImpl).annotations = classAnnotations.asList()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that this is not the best idea to do SerialDescriptorImpl almost mutable because of one issue.
Also, explicit conversion to SerialDescriptorImpl looks dangerous.
Maybe solve the problem where it appears - in serializer? Create private var property and rewrite it in the secondary constructor or other alternatives.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is, the secondary constructor is called after all init blocks and properties initializers. I considered the following alternatives:

  • Mutable (to some reasonable extent) serialDescriptorImpl — current solution. I can add buildSerialDescriptorInternal function to avoid downcast to be sure this won't break.
  • Store the builder in property, call .build on every access to public val descriptor — I don't like copying every time
  • Store descriptor in private mutable property, rebuild it from scratch (because we do not have a copying mechanism) every time. Seems reasonable, although a bit boilerplaite-ish. However, I do not see any strong objections to this solution, so I can rewrite everything to it

}

override val descriptor: SerialDescriptor = buildSerialDescriptor(serialName, StructureKind.OBJECT)

override fun serialize(encoder: Encoder, value: T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
package kotlinx.serialization

import kotlinx.serialization.descriptors.*
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.*

class SerialDescriptorAnnotationsTest {

Expand All @@ -17,7 +16,33 @@ class SerialDescriptorAnnotationsTest {
@Serializable
@SerialName("MyClass")
@CustomAnnotation("onClass")
data class WithNames(val a: Int, @CustomAnnotation("onProperty") val veryLongName: String)
data class WithNames(
val a: Int,
@CustomAnnotationWithDefault @CustomAnnotation("onProperty") val veryLongName: String
)

@SerialInfo
@Target(AnnotationTarget.PROPERTY, AnnotationTarget.CLASS)
annotation class CustomAnnotationWithDefault(val value: String = "foo")

@SerialInfo
@Target(AnnotationTarget.PROPERTY)
public annotation class JShort(val order: SByteOrder = SByteOrder.BE, val mod: SByteMod = SByteMod.Add)

public enum class SByteOrder {
BE, LE
}

public enum class SByteMod {
None, Add
}

@Serializable
public class Foo(
@JShort(SByteOrder.LE, SByteMod.None) public val bar: Short,
@JShort public val baz: Short
)


@Test
fun testSerialNameOnClass() {
Expand All @@ -40,5 +65,56 @@ class SerialDescriptorAnnotationsTest {
assertEquals("onClass", name)
}

@Test
fun testCustomAnnotationWithDefaultValue() {
val value =
WithNames.serializer().descriptor
.getElementAnnotations(1).filterIsInstance<CustomAnnotationWithDefault>().single()
assertEquals("foo", value.value)
sandwwraith marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
fun testAnnotationWithMultipleArgs() {
fun SerialDescriptor.getValues(i: Int) = getElementAnnotations(i).filterIsInstance<JShort>().single().run { order to mod }
assertEquals(SByteOrder.LE to SByteMod.None, Foo.serializer().descriptor.getValues(0))
assertEquals(SByteOrder.BE to SByteMod.Add, Foo.serializer().descriptor.getValues(1))
}

private fun List<Annotation>.getCustom() = filterIsInstance<CustomAnnotation>().single().value

@Serializable
@CustomAnnotation("sealed")
sealed class Result {
@Serializable class OK(val s: String): Result()
}

@Serializable
@CustomAnnotation("abstract")
abstract class AbstractResult {
var result: String = ""
}

@Serializable
@CustomAnnotation("object")
object ObjectResult {}

@Serializable
class Holder(val r: Result, val a: AbstractResult, val o: ObjectResult, @Contextual val names: WithNames)

private fun doTest(position: Int, expected: String) {
val desc = Holder.serializer().descriptor.getElementDescriptor(position)
assertEquals(expected, desc.annotations.getCustom())
}

@Test
fun testCustomAnnotationOnSealedClass() = doTest(0, "sealed")

@Test
fun testCustomAnnotationOnPolymorphicClass() = doTest(1, "abstract")

@Test
fun testCustomAnnotationOnObject() = doTest(2, "object")

@Test
fun testCustomAnnotationTransparentForContextual() = doTest(3, "onClass")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/

package kotlinx.serialization.json

import kotlinx.serialization.*
import kotlinx.serialization.descriptors.*
import kotlinx.serialization.encoding.*
import kotlinx.serialization.json.internal.*
import kotlin.native.concurrent.*

/**
* Indicates that the field can be represented in JSON
* with multiple possible alternative names.
* [Json] format recognizes this annotation and is able to decode
* the data using any of the alternative names.
*
* Unlike [SerialName] annotation, does not affect JSON encoding in any way.
*
* Example of usage:
* ```
* @Serializable
* data class Project(@JsonNames("title") val name: String)
*
* val project = Json.decodeFromString<Project>("""{"name":"kotlinx.serialization"}""")
* println(project)
* val oldProject = Json.decodeFromString<Project>("""{"title":"kotlinx.coroutines"}""")
* println(oldProject)
* ```
*
* This annotation has lesser priority than [SerialName].
*
* @see JsonBuilder.useAlternativeNames
*/
@SerialInfo
@Target(AnnotationTarget.PROPERTY)
@ExperimentalSerializationApi
public annotation class JsonNames(vararg val names: String)

/**
* Specifies key for class discriminator value used during polymorphic serialization in [Json].
* Provided key is used only for an annotated class, to configure global class discriminator, use [JsonBuilder.classDiscriminator]
* property.
*
* It is possible to define different class discriminators for different parts of class hierarchy.
* Pay attention to the fact that class discriminator, same as polymorphic serializer's base class, is
* determined statically.
*
* Example:
* ```
* @Serializable
* @JsonTypeDiscriminator("class")
* abstract class Base
*
* @Serializable
* @JsonTypeDiscriminator("error_class")
* abstract class ErrorClass: Base()
*
* @Serializable
* class Message(val object: Base, val error: ErrorClass?)
*
* val message = Json.decodeFromString<Message>("""{"object": {"class":"my.app.BaseMessage", "message": "not found"}, "error": {"error_class":"my.app.GenericError", "error_code": 404}}""")
* ```
*
* @see JsonBuilder.classDiscriminator
*/
@SerialInfo
@Target(AnnotationTarget.CLASS)
@ExperimentalSerializationApi
public annotation class JsonClassDiscriminator(val discriminator: String)

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,22 @@ import kotlinx.serialization.internal.*
import kotlinx.serialization.json.*

@Suppress("UNCHECKED_CAST")
internal inline fun <T> JsonEncoder.encodePolymorphically(serializer: SerializationStrategy<T>, value: T, ifPolymorphic: () -> Unit) {
internal inline fun <T> JsonEncoder.encodePolymorphically(
serializer: SerializationStrategy<T>,
value: T,
ifPolymorphic: (String) -> Unit
) {
if (serializer !is AbstractPolymorphicSerializer<*> || json.configuration.useArrayPolymorphism) {
serializer.serialize(this, value)
return
}
val actualSerializer = findActualSerializer(serializer as SerializationStrategy<Any>, value as Any)
ifPolymorphic()
actualSerializer.serialize(this, value)
}

private fun JsonEncoder.findActualSerializer(
serializer: SerializationStrategy<Any>,
value: Any
): SerializationStrategy<Any> {
val casted = serializer as AbstractPolymorphicSerializer<Any>
val actualSerializer = casted.findPolymorphicSerializer(this, value)
validateIfSealed(casted, actualSerializer, json.configuration.classDiscriminator)
val kind = actualSerializer.descriptor.kind
checkKind(kind)
return actualSerializer
val baseClassDiscriminator = serializer.descriptor.classDiscriminator(json)
val actualSerializer = casted.findPolymorphicSerializer(this, value as Any)
validateIfSealed(casted, actualSerializer, baseClassDiscriminator)
checkKind(actualSerializer.descriptor.kind)
ifPolymorphic(baseClassDiscriminator)
actualSerializer.serialize(this, value)
}

private fun validateIfSealed(
Expand Down Expand Up @@ -64,7 +60,7 @@ internal fun <T> JsonDecoder.decodeSerializableValuePolymorphic(deserializer: De
}

val jsonTree = cast<JsonObject>(decodeJsonElement(), deserializer.descriptor)
val discriminator = json.configuration.classDiscriminator
val discriminator = deserializer.descriptor.classDiscriminator(json)
val type = jsonTree[discriminator]?.jsonPrimitive?.content
val actualSerializer = deserializer.findPolymorphicSerializerOrNull(this, type)
?: throwSerializerNotFound(type, jsonTree)
Expand All @@ -79,3 +75,8 @@ private fun throwSerializerNotFound(type: String?, jsonTree: JsonObject): Nothin
else "class discriminator '$type'"
throw JsonDecodingException(-1, "Polymorphic serializer was not found for $suffix", jsonTree.toString())
}

internal fun SerialDescriptor.classDiscriminator(json: Json): String =
annotations.filterIsInstance<JsonClassDiscriminator>().singleOrNull()?.discriminator
sandwwraith marked this conversation as resolved.
Show resolved Hide resolved
?: json.configuration.classDiscriminator

Loading