kotlinx.serialization icon indicating copy to clipboard operation
kotlinx.serialization copied to clipboard

Experimental UuidSerializer breaks projects with custom UUID serializer

Open nielsvanvelzen opened this issue 1 year ago • 1 comments

Describe the bug

A project using a custom UUID serializer with the name of the serial descriptor set to UUID (casing does not matter) will break with the recent 1.7.2 release. Even though the new serializer is not used, the serialization library will throw the following error:

The name of serial descriptor should uniquely identify associated serializer.
For serial name UUID there already exist UuidSerializer.
Please refer to SerialDescriptor documentation for additional information.
java.lang.IllegalArgumentException: The name of serial descriptor should uniquely identify associated serializer.
For serial name UUID there already exist UuidSerializer.
Please refer to SerialDescriptor documentation for additional information.

While I would argue this is fine to happen in a 1.8 release, it's not in a patch release. In this specific case, the serializer is added in an SDK that is used by multiple apps. If one of those apps were to update to kotlinx.serialization 1.7.3 they would also be affected by this issue.

To Reproduce

  • Create a project with a custom UUID serializer. Sample code can be taken from the Jellyfin Kotlin SDK: https://github.com/jellyfin/jellyfin-sdk-kotlin/blob/aee65c0d314de5ca6115d37c9a4d5721ad20c1c7/jellyfin-model/src/jvmMain/kotlin/org/jellyfin/sdk/model/serializer/UUIDSerializer.kt

Expected behavior

In any 1.7.z release a custom serializer should just work.

Environment

  • Kotlin version: 2.0.20
  • Library version: 1.7.2
  • Kotlin platforms: JVM / Android
  • Gradle version: 8.5.2
  • IDE version (if bug is related to the IDE) IntellijIDEA 2024.2.0.2

nielsvanvelzen avatar Aug 28 '24 20:08 nielsvanvelzen

I forgot to mention, our current UUID serializer is for the java.util.UUID class. So it's not the same type that the serializer is made for. In the linked repository we have a typealias for that so it may not be obvious from reading the code.

nielsvanvelzen avatar Aug 28 '24 20:08 nielsvanvelzen

It wasn't our intention to break users' serializers, especially those that work with java.util.UUID. It's just a bug, and even patch releases may contain them :(

I still want to point out that providing a library with a serializer that is named just "UUID" maybe not reliable since any of your clients can declare a serializer with the same short name, which may possibly lead to bugs

sandwwraith avatar Aug 29 '24 14:08 sandwwraith

Thanks for picking it up! Regarding the names, I wasn't that familiar with the serialization library when I first wrote that uuid serializer. The documentation example also uses a simple name (just "Color") so I never thought much about it.

I'll update our serializers so they're prefixes to avoid any future problems. Thanks for the tip!

nielsvanvelzen avatar Aug 29 '24 15:08 nielsvanvelzen

@sandwwraith could you please fix the example from the guide as well?

qwwdfsad avatar Aug 29 '24 15:08 qwwdfsad

Just ran into this too, downgraded back to 1.7.1

chrisjenx avatar Aug 29 '24 19:08 chrisjenx

I still want to point out that providing a library with a serializer that is named just "UUID" maybe not reliable since any of your clients can declare a serializer with the same short name, which may possibly lead to bugs

Wait so I can write a serialize with the same name as another one to override a library one? And it's case insensitive? That seems like a little bit of an oversight? They should at least be package name sensitive?

chrisjenx avatar Aug 29 '24 19:08 chrisjenx

Oh for reference my issue is similar but different, I just get UUIDSerializer class not found so not sure what happens but I guess the 1.7.2 version nukes my custom one so then it can't be found anymore?

chrisjenx avatar Aug 29 '24 19:08 chrisjenx

@sandwwraith we have run into the same issue, and our serializer class has a custom name. Serializer class name that conflict are:

UUIDSerializer
MscoUUIDSerializer

Both of them cause conflict with the built-in serializer in 1.7.2

Caused by: java.lang.IllegalArgumentException: The name of serial descriptor should uniquely identify associated serializer.
For serial name UUID there already exist UuidSerializer.
Please refer to SerialDescriptor documentation for additional information.
at kotlinx.serialization.internal.PrimitivesKt.checkName(Primitives.kt:83)
at kotlinx.serialization.internal.PrimitivesKt.PrimitiveDescriptorSafe(Primitives.kt:73)
at kotlinx.serialization.descriptors.SerialDescriptorsKt.PrimitiveSerialDescriptor(SerialDescriptors.kt:91)
at com.myapplication.serializer.MscoUUIDSerializer.<clinit>(MscoUUIDSerializer.kt

We are also deserializing the java.util.UUID. ~~And what seems to be in conflict here, ist the name of the class to be serialized (in this case UUID), not the name of the serializer itself.~~

lukaszkalnik avatar Sep 13 '24 15:09 lukaszkalnik

Ah ok, I see: we have to change the name in the PrimitiveSerialDescriptor("UUID", PrimitiveKind.STRING) to something else than just UUID.

lukaszkalnik avatar Sep 13 '24 15:09 lukaszkalnik

Ah ok, I see: we have to change the name in the PrimitiveSerialDescriptor("UUID", PrimitiveKind.STRING) to something else than just UUID.

Yes, the name is used by formats for caching (and resolving polymorphic children), and thus needs to be unique.

pdvrieze avatar Sep 13 '24 15:09 pdvrieze

I feel dumb as the error is pretty clear, but it only just clicked it's the string name of the descriptor not the object SerializerName..

chrisjenx avatar Sep 16 '24 16:09 chrisjenx