Jvm global serializers (UUID) #1184
Conversation
There was a problem hiding this comment.
I think building a library of common custom serializers (especially for more frequently used JVM types) is worthwhile, however I don't think that it should be in the core module, rather in a support module that users can opt in to.
The way that the UUID serializer is hooked up is incorrect (in the wrong place, and not supporting compile time serializer selection). In addition it would very quickly break scalability and library parity.
|
|
||
| @OptIn(ExperimentalSerializationApi::class) | ||
| internal object UUIDSerializer: KSerializer<UUID> { | ||
| override val descriptor = PrimitiveSerialDescriptor("java.util.UUID", PrimitiveKind.STRING) | ||
| override fun serialize(encoder: Encoder, value: UUID) = encoder.encodeString(value.toString()) | ||
| override fun deserialize(decoder: Decoder): UUID = UUID.fromString(decoder.decodeString()) | ||
| } |
There was a problem hiding this comment.
While this is one of the valid ways to serialize a UUID it is not the only one (for non-text formats a binary representation would be better, it is after all just a 128bit number).
More significantly, I think that it is important to be very careful with providing serializers for types that are not extremely standard in the default library/module (there are way too many to think about). Those would be better put into a support module that can be loaded when needed. I would be in support of such an optional "common serializers" module, and UUID is a good candidate for inclusion there.
There was a problem hiding this comment.
I cannot thumbs-up this comment enough. Unless a type has an intrinsic representation in the underlying serialization format (like, for example, a string or a number or a list) providing a default means encoding an opinion on how a type is serialized that will not be universal.
As an existing example, the library has built-in support for pair and triple that I wish were not included. For pair, one may want to encode an array that is always two elements.
So I too strongly believe these should not be provided in the core. Especially given how trivial it is to create them yourself where the responsibility of making the decision about the serialized form.
(If you need a really, really terrible example of this, look at Gson's built-in support for serializing dates and note Moshi's intentional complete absence of that as a built-in feature and reliance on user-supplied adapters)
There was a problem hiding this comment.
Agree. At least, this one should be named as UUIDAsStringSerializer, but we can also imagine Base64UUIDSerializer or UUIDAsByteArraySerializer. Ultimately, users should choose what serializer to pick on their own.
| private fun <T> jvmGlobalSerializer(jClass: Class<T>): KSerializer<T>? = | ||
| when (jClass) { | ||
| UUID::class.java -> UUIDSerializer as? KSerializer<T> | ||
| else -> null | ||
| } |
There was a problem hiding this comment.
This is not a scalable solution, more significantly it doesn't work in a way that keeps parity between standard and custom serializers. In addition, this code path is only for reflectively resolving serializers. In that case the serialmodule is available. The method is mainly for the code path with type arguments, which UUID doesn't have. As such it will result in surprising and inconsistent behaviour: serializer(typeOf<UUID>()) will not return the serializer because it has no type arguments. The place that you would add the additional built-in would be in Primitives.kt line 69, builtinSerializerOrNull. But frankly the approach I would take would be to create a different method that uses serializerOrNull and then adds whatever you want to add as alternative. Maybe even better is to just use a ContextSerializer and then you create a module which adds all these defaults.
Of course kotlinx.serialization is primarily focused on statically (at compile time) resolving serializers and for that you need to announce the serializers to the compiler with the right annotations. It may be worthwhile to allow libraries to declare serializers in a way that they are automatically picked up by the compiler but that is a whole different thing.
There was a problem hiding this comment.
Actually, it will work with serializer<UUID>() (as it shown in tests), because this method resolves all serializers.
However, the bigger problem that this method used only for top-level reflective resolving (serializer<> function) and it does not affect static compile-time resolving. That is, any class that contains UUID (e.g. @Serializable data class MyData(val uuid: UUID)) will report compile-time exception that no serializer is found, require using @UseSerializers/SerializersModule, so original issue is kinda not solved.
| else -> null | ||
| } | ||
|
|
||
| private fun <T : Any> Class<T>.isNotAnnotated(): Boolean { |
There was a problem hiding this comment.
This has a spurious space after the T, invalid change
| } | ||
|
|
||
| @Test | ||
| fun testUUID() { |
There was a problem hiding this comment.
This method uses a reflective approach to resolve the serializer. Also add one that uses UUID.serializer, You will find that will not work currently.
There was a problem hiding this comment.
Thanks for your contribution and time!
However, we do not have plans to include any pre-defined serializers for Java classes directly into the core module. There are several reasons to it:
- Core module should be kept lightweight — not all users need serializers for Java classes, especially users that use library in multiplatform context.
- Opinionated serializers — as @jw and @pdvrieze correctly noticed, there are a lot of different approaches to serialize UUIDs, Dates, and so on.
We have a plan to provide a separate jvm-only module for serializers for java classes. However, there are still big design questions around that: should we provide several serializing strategies for one class out of the box for most popular use-cases? How these serializers should be connected to compile-time resolving, is @UseSerializers annotation enough? I think answers for these questions will be provided in future releases of kotlinx.serialization. For now, the most efficient way to serialize UUIDs/Dates is to copy-paste and tweak serializer to your particular needs.
Just follow up on #507
I'm not sure what's you thinking right now about this approach, but I decided to show the way how to make it under control, but let contributors extend platform standard types serializers
Looking forward to see your opinion