Kotlin / kotlinx.serialization

Kotlin multiplatform / multi-format serialization
Apache License 2.0
5.37k stars 620 forks source link

Support nullable types in JsonTransformingSerializer #1927

Open marcosalis opened 2 years ago

marcosalis commented 2 years ago

What is your use-case and why do you need this feature? Based on this comment, a custom JsonTransformingSerializer doesn't support nullable types.

There is currently no straightforward way (except cumbersome usage of null "marker objects" in data) to handle custom deserialization of a null JSON field when one needs to directly read from a JsonElements to create a decoded model.

A pretty common use case would be setting a property to null or to a default type in a model when an unexpected value (or set of values) is encountered while reading the JSON element in a legacy API.

Describe the solution you'd like As suggested in the comment above, it would be enough for JsonTransformingSerializer to support nullable types:

object ModelSerializer : JsonTransformingSerializer<Model?>(Model.serializer().nullable) {

    override fun transformDeserialize(element: JsonElement): JsonElement {
         if (condition) {
             return JsonNull
         }
        ...
    }
}

Right now, if I attempt that the deserialization fails with:

kotlinx.serialization.json.internal.JsonDecodingException: Expected class kotlinx.serialization.json.JsonObject as the serialized body of XXX, but had class kotlinx.serialization.json.JsonNull

holdbetter commented 1 year ago

Absolutely need in this!

At 1.4.1 version there is another IDE error:

image

String used as simple example. Looks like fix is simple JsonTransformingSerializer<T : Any> should be JsonTransformingSerializer<T : Any?>

holdbetter commented 1 year ago

My usage the same. I'm using 3rd party API and at deserialization process I'm not sure that there is expected value at my property, then I want to provide default null value

holdbetter commented 1 year ago

Well, as I said fix is quiet simple at the source level, but I'm not sure that is appropriate way to change JsonTransformingSerializer class, so I didn't create pull request with my solution, but I would like to share it with you here:

It's gross, because it wasn't well-tested. I just checked my needs and it's working.

There is nothing extra added, I just replaced upper bound to nullable Any and transformed all non-null type to nullable, also moved 2 internal cast methods to be accessed.

import kotlinx.serialization.InternalSerializationApi
import kotlinx.serialization.KSerializer
import kotlinx.serialization.descriptors.SerialDescriptor
import kotlinx.serialization.encoding.Decoder
import kotlinx.serialization.encoding.Encoder
import kotlinx.serialization.json.*
import kotlinx.serialization.json.internal.writeJson

abstract class NullableJsonTransformingSerializer<T : Any?>(
    private val tSerializer: KSerializer<T?>
) : KSerializer<T?> {

    /**
     * A descriptor for this transformation.
     * By default, it delegates to [tSerializer]'s descriptor.
     *
     * However, this descriptor can be overridden to achieve better representation of the resulting JSON shape
     * for schema generating or introspection purposes.
     */
    override val descriptor: SerialDescriptor get() = tSerializer.descriptor

    @OptIn(InternalSerializationApi::class)
    final override fun serialize(encoder: Encoder, value: T?) {
        val output = encoder.asJsonEncoder()
        var element = output.json.writeJson(value, tSerializer)
        element = transformSerialize(element)
        output.encodeJsonElement(element)
    }

    final override fun deserialize(decoder: Decoder): T? {
        val input = decoder.asJsonDecoder()
        val element = input.decodeJsonElement()
        return input.json.decodeFromJsonElement(tSerializer, transformDeserialize(element))
    }

    /**
     * Transformation that happens during [deserialize] call.
     * Does nothing by default.
     *
     * During deserialization, a value from JSON is firstly decoded to a [JsonElement],
     * user transformation in [transformDeserialize] is applied,
     * and then resulting [JsonElement] is deserialized to [T] with [tSerializer].
     */
    protected open fun transformDeserialize(element: JsonElement): JsonElement = element

    /**
     * Transformation that happens during [serialize] call.
     * Does nothing by default.
     *
     * During serialization, a value of type [T] is serialized with [tSerializer] to a [JsonElement],
     * user transformation in [transformSerialize] is applied, and then resulting [JsonElement] is encoded to a JSON string.
     */
    protected open fun transformSerialize(element: JsonElement): JsonElement = element

    private fun Encoder.asJsonEncoder() = this as? JsonEncoder ?: throw IllegalStateException(
        "This serializer can be used only with Json format." +
                "Expected Encoder to be JsonEncoder, got ${this::class}"
    )

    private fun Decoder.asJsonDecoder() = this as? JsonDecoder ?: throw IllegalStateException(
        "This serializer can be used only with Json format." +
                "Expected Decoder to be JsonDecoder, got ${this::class}"
    )
}
holdbetter commented 1 year ago

Or even it could be just T : Any? without other changes. I'll check, write tests and maybe do PR later. Please, use above solution carefully

corbella83 commented 1 year ago

I think that the solution is simpler than that. We don't need any extra class.

Just change the definition of JsonTransformingSerializer<T : Any> to JsonTransformingSerializer<T>

There is no need to force T to be of type "Any". T type includes nullable and non nullable types

holdbetter commented 1 year ago

Yep. I am not suppose to do extra class. It's solution for now without any PRs.

Also, T : Any? is the same as T as I know. So yeah if that simple change wouldn't crash current tests, it could be the solution. If not it needs more research and maybe nullable class is the idea here