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

`ClassDiscriminatorMode` in 3rd party formats (move to kxs-core, or rename w/ `Json` prefix)

Open BenWoodworth opened this issue 1 year ago • 3 comments

What is your use-case and why do you need this feature?

I have a 3rd party format (knbt)[https://github.com/BenWoodworth/knbt] and a request in slack to implement a class discriminator mode similar to JSON's.

If ClassDiscriminatorMode were in kotlinx-serialization-core, then I could use the existing enum (and maybe throw an error during configuration if a specific mode)

It's awkward/confusing for me to add a similarly named enum into my library, notably if my library and kxs-json are both used in the same project (which is the situation for the person requesting the discriminator mode in my library)

Describe the solution you'd like

I couldn't find any discussion about why ClassDiscriminatorMode was added or the decisions behind it, but I think making it available as kotlinx.serialization.ClassDiscriminatorMode in kxs-core instead of in the json package would be convenient for other format libraries that want to add the functionality. (And since it's still marked as experimental in kxs-json, I think this might still be an option).

That could also make it more standard, though I can see issues if kxs wants to add a new mode to the enum later and libraries don't update their code to handle it.

Otherwise, maybe renaming it to be JsonClassDiscriminatorMode, Json-prefixed like other json-specific features, might be another option. Though I'm leaning towards moving this to core as a standard configuration option that formats could support

BenWoodworth avatar Mar 18 '24 04:03 BenWoodworth

There are several reasons for this enum to be json-specific. First, other formats in serialization repo do not place class discriminator inside objects at all; they use 'default polymorphism', which is represented as [type, object] array. Second, not all enum entries are meaningful for all formats. While NEVER surely can be used in every format, there is also ALL_JSON_OBJECTS entry (which already contains JSON in its name). I'm not sure it will entirely fit other possible formats. For example, if you use array polymorphism, you can even make primitives polymorphic — ["kotlin.Int", 3], and they're obviously neither Json objects nor "[StructureKind.CLASS] or [StructureKind.OBJECT]".

To sum up, to me, it looks like only NEVER mode is actually meaningful for all formats universally. So, probably, you can get away with adding writeClassDiscriminator: Boolean flag for your format.

However, renaming ClassDiscriminatorMode to Json(Class)DiscriminatorMode indeed makes sense if you want to implement a similar enum for your format. We definitely should consider this.

sandwwraith avatar Mar 20 '24 13:03 sandwwraith

As an example, XML has a completely different way of dealing with polymorphism. It supports the [type, object] mode, it supports using the type derived tag name as "discriminator" (this implies that this type can only map to a single child - at least until I add support for order-sensitive decoding), and finally it allows using an attribute to indicate the type (by default xsi:type). It is significant here that the xml format, by default, ignores the property name for child tags (and uses type names instead). As to renaming, it makes some sense, but may also be a compatibility issue.

pdvrieze avatar Mar 20 '24 17:03 pdvrieze

Thanks for the insight! It does sound like renaming would be the way to go, if changing anything at all.

In my mind I was thinking that a discriminator (and its mode) could work for any format, basically encoding the Kotlin classes as if there was an extra property added in the first place. Having the classes translate all the same. That, and ALL_CLASSES being a better name for the mode (or just ALL, since classes might be implied). Then for serialization configured without discriminators (array polymorphism, no support at all, ...) could disregard the ClassDiscriminatorMode enum entirely.

Even if that's all true though, and any discriminator-supporting format could implement these options, there might be additional modes that a format might want to add. And @pdvrieze, similar to what you mentioned, encoding the discriminator as an attribute instead of an entry (or in other formats, perhaps as metadata instead of an additional field). Thanks for bringing up the XML example :)

My format is very similar to JSON in structure, so for me, I'll probably include my own NbtClassDiscriminatorMode to loosely mirror JSON's. And if we're open to renaming it to JsonClassDiscriminatorMode, I could also play around with doing so in a binary compatible and auto-migratable way, and open up a PR if I'm getting anywhere with it. Let me know!

BenWoodworth avatar Mar 27 '24 22:03 BenWoodworth