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

Can't serialize instances of class hierarchy with constructor parameters

Open rnikander opened this issue 6 years ago • 35 comments

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

I have a web app that displays some complex reports. There is a class hierarchy of "queries" that I'd like to serialize to/from server. But something like this is not serializable:

abstract class Query(val type: QueryType) { ... }
abstract class QueryBase(type: QueryType) :Query(type) { ... }
class SpecificQuery1() : QueryBase(QueryType.Specific1) { ... }

The error is:

This class is not serializable automatically because it has primary constructor parameters of which are not properties

The type sort of is a property - it becomes a property once it's passed up to the root class.

I put a custom serializer on QueryBase, with method bodies of TODO("not implemented"), because I'm not sure yet how I could do it, or if it's possible.

@Serializable(with = QueryBaseSerializer::class)
abstract class QueryBase(queryType: QueryType)

I then get another error on SpecificQuery1, suggesting that even if I figured out how to write a serializer for QueryBase, I will have more problems.

Impossible to make this class serializable because its parent is not serializable and does not have exactly one constructor without parameters

Describe the solution you'd like

I'd like the auto-generated serializers to handle it. If that's not possible, I'd like documentation on how to implement custom serializers to handle it. If I need to write custom serializers, it would be nice if there were a way to use some of the plugin's code-generation abilities, so that I don't have to remember to manually serialize each property and keep that code in sync if I add properties to a class. In other words, maybe the plugin can't generate the whole serializer for me, but it could generate something to serialize the properties that it can handle, so I could call that from my serializer once I handled the custom part.

rnikander avatar Nov 07 '19 10:11 rnikander

I'd like to bump this feature request, it would be really useful in my case.

I have configuration files that are shared between multiple services. All services configurations have a lot of values in common, stored in one abstract class, then each service defines a subclass adding some specific properties.

It is not a polymorphic serialization in the sense that I know which subclass I am deserializing.

SeekDaSky avatar May 13 '20 12:05 SeekDaSky

Hi, do you plan to support this use case at some point? Thank you for your answer. :smiley:

SeekDaSky avatar Aug 12 '20 12:08 SeekDaSky

Hi, Unfortunately, handling arbitrary arguments that are not properties is not an easy task — because they may be involved in arbitrary expressions that should be default values or superclass constructor arguments (e.g. you have class QueryBase(type: QueryType) :Query(type + "myQuery"))

We usually recommend follow this pattern when serializing class hierarchies: make vals abstract in base classes and override them in concrete classes, like this:

abstract class Query { 
    abstract val type: QueryType
}
abstract class QueryBase(override val type: QueryType) :Query() { ... }

sandwwraith avatar Aug 12 '20 14:08 sandwwraith

@sandwwraith Is there anything in the works for this? While the provided solution compiles, it doesn't produce the cleanest code.

jgandert avatar Oct 13 '20 20:10 jgandert

Hello @CreamyCookie, what about this snippet?

import kotlinx.serialization.Serializable

@Serializable
sealed class TimerState {
    abstract val millis: Long

    @Serializable
    class STOPPED(override val millis: Long) : TimerState()

    @Serializable
    class TICKING(override val millis: Long) : TimerState()
}

abd3lraouf avatar Dec 28 '20 06:12 abd3lraouf

@AbdElraoufSabri Huh, isn't that just an implementation of the pattern suggested by @sandwwraith ?

jgandert avatar Dec 28 '20 10:12 jgandert

@CreamyCookie exactly 😂

abd3lraouf avatar Dec 28 '20 10:12 abd3lraouf

I'm getting this error with a sealed class hierarchy:

Can't serialize instances of class hierarchy with constructor parameters

I can apply the abstract val strategy as suggested by @sandwwraith , but in some cases that doesn't make sense from a data representation / encapsulation point of view, because the object creator can override a set value in the constructor.

I think the following should work correctly:

@Serializable(with = ShapeSerializer::class)
sealed class Shape(val type: String) {

    @Serializable // this is where I get the error
    data class Rect(...) : Shape("rectangle")

    @Serializable
    data class Circle(...) : Shape("circle")
}

Am I doing something wrong?

marcosalis avatar May 11 '22 14:05 marcosalis

any update for this?

yangwuan55 avatar Nov 28 '22 10:11 yangwuan55

My case:

@Serializable
data class UserRoleDto(
    @SerialName("name")
    val name: String,
    @SerialName("description")
    val description: String,
    @SerialName("id")
    override var id: Long? = null,
) : BaseDto<Long>(id) {}
abstract class BaseDto<T>(
    open var id: T? = null,
)

or

@Serializable
abstract class BaseDto<T>(
    open var id: T? = null,
)

Solution:

@Serializable
abstract class BaseDto<T> {
    abstract var id: T?
}

To be honest, I already doubt the need to use kotlinx.serialization, because jackson does not have such problems. Yes, jackson does not support some things from Kotlin, but at the moment kotlinx.serialization creates more problems than it solves. Oh yes, I have a multiplatform ... Okay, I'll have to use it.

TalosDx avatar Jan 28 '23 16:01 TalosDx

Honestly I'm really surprised that this wasn't part of the original architecture of serialization. I expected more from JetBrains. It's an obvious and really common use case. Not sure how you expect the library to be relevant at all if class hierarchies are not handled gracefully.

mattiasflodin avatar Mar 09 '23 10:03 mattiasflodin

@mattiasflodin I understand yours and everyone else's disappointment, but please remember that this library is open source, and is given for free. When reading your comment, the developers of kotlinx serialization won't feel good, and it will probably make them less motivated. I'm sure that if this issue could be solved easily, they would solve it. I can see you are an open source maintainer yourself, so you probably know how it feels.

Just a friendly reminder for everyone to be polite 🙂

acmpo6ou avatar Jun 24 '23 16:06 acmpo6ou

Note that "secondary constructors only" also works. There must be a way for serialised values to be passed to the constructor (without side-effects). Primary constructors block that in certain cases.

pdvrieze avatar Jun 26 '23 12:06 pdvrieze

@mattiasflodin I understand yours and everyone else's disappointment, but please remember that this library is open source, and is given for free. When reading your comment, the developers of kotlinx serialization won't feel good, and it will probably make them less motivated. I'm sure that if this issue could be solved easily, they would solve it. I can see you are an open source maintainer yourself, so you probably know how it feels.

Just a friendly reminder for everyone to be polite 🙂

What is wrong with his post? Do you expect him to be sunshine and rainbows when something this garbage pops up?

This is a clear problem that is fundemental and should have been addressed at the start.

"Feeling good" has nothing to do with it. He was nothing but polite in his post, respectfully stating his dissapointment. As he should. You want to see unpolite,

Here you go:

This is, trash. A programmer should not be forced to redo his code because a damn SERIALIZER cant serialize. Being opinionated is fine but being opinionated to the point of not working is a high horse that you have to get out of. Jetbrains is a great company and this is clearly a mistake or oversight. We should call out these things when we see them, and celebrate the things that we like. Thats how it works , cry more.

rom4ster avatar Nov 16 '23 09:11 rom4ster

What is wrong with his post? Do you expect him to be sunshine and rainbows when something this garbage pops up?

I expect people to understand that not everything is easily possible to be achieved. Different people have different needs, different problems and wishes. It's not possible to achieve everything everyone wants. The devs try to help as many people as possible. But things like this will come up.

I know it's frustrating. But no need to get too emotional about this, and hurt others' feelings. Especially the feelings of people that made this for free.

He was nothing but polite in his post, respectfully stating his dissapointment. As he should.

I'm not sure if "Not sure how you expect the library to be relevant at all if class hierarchies are not handled gracefully." sounds polite.

You want to see unpolite,

No 😅, this is exactly the opposite of what I want to see.

Jetbrains is a great company and this is clearly a mistake or oversight. We should call out these things when we see them, and celebrate the things that we like. Thats how it works , cry more.

It is true, we should call them out, but we shouldn't hurt anyone when we do this. And we should always remember that this was given to us for free. Also, it's always easy to say "this doesn't work!", and it's another thing to actually fix the problem.

acmpo6ou avatar Nov 16 '23 10:11 acmpo6ou

Just to clarify: calling something 'garbage' or 'trash' does not raise issue priority :)

sandwwraith avatar Nov 16 '23 11:11 sandwwraith

What is wrong with his post? Do you expect him to be sunshine and rainbows when something this garbage pops up?

I expect people to understand that not everything is easily possible to be achieved. Different people have different needs, different problems and wishes. It's not possible to achieve everything everyone wants. The devs try to help as many people as possible. But things like this will come up.

I know it's frustrating. But no need to get too emotional about this, and hurt others' feelings. Especially the feelings of people that made this for free.

He was nothing but polite in his post, respectfully stating his dissapointment. As he should.

I'm not sure if "Not sure how you expect the library to be relevant at all if class hierarchies are not handled gracefully." sounds polite.

You want to see unpolite,

No 😅, this is exactly the opposite of what I want to see.

Jetbrains is a great company and this is clearly a mistake or oversight. We should call out these things when we see them, and celebrate the things that we like. Thats how it works , cry more.

It is true, we should call them out, but we shouldn't hurt anyone when we do this. And we should always remember that this was given to us for free. Also, it's always easy to say "this doesn't work!", and it's another thing to actually fix the problem.

Dude if you think "i expected more from this company" is "hurting people's feelings" Wow you got problems. Do you have the same logic with google? they make tons of stuff for free but I dont think you will be talking to them this way....

Regardless, companies are not people, and when you insult them, people need to learn to detach themselves. Jetbrains is not these devs' life, and these devs' skill is not jetbrains skill. Jebrains is seperate from the devs.

But even if you consider them the same. He is just stating dissapointment in a way that is real. So many SJWs around ....

rom4ster avatar Nov 17 '23 04:11 rom4ster

Just to clarify: calling something 'garbage' or 'trash' does not raise issue priority :)

You work at jetbrains?

But no you are mostly right. I just ran into this last night and it wasted like 5 hrs of my time and i was pissed. Then the other guy saying "be nice" pissed me off more soo.... cry

rom4ster avatar Nov 17 '23 04:11 rom4ster

Fundamentally kotlinx.serialization works differently from other serialization such as done by java serialization in that it doesn't just do underwater magic to put the data into the type. This means it needs to have a way to construct the type (and to call the parent constructor).

To bring things to a more productive perspective, there are a number of options:

  • Make the superclass not have a primary constructor, in that case it is possible for the serialization plugin to inject a correct constructor.
  • Use a custom serializer that will use the knowledge of the type to instantiate it appropriately
  • Have a generator that creates custom serializers that can then be adjusted manually (perhaps also including some checking on changes in the types)
  • Allow for "split" serialization where only a SerializationStrategy is generated, but not a DeserializationStrategy (as that requires the constructors).

pdvrieze avatar Nov 17 '23 13:11 pdvrieze

What I dont get is why does the primary constructor matter? Why can it not handle a primary constructor but handle the non primary ones? If i have a subclass , and I call the constructor it will auto call the primary constructor with the right value so this is quite confusing to me

rom4ster avatar Nov 17 '23 17:11 rom4ster

I'm not sure if "Not sure how you expect the library to be relevant at all if class hierarchies are not handled gracefully." sounds polite.

For the record, I did not mean for this to come off as impolite. As you know, JetBrains is a commercial company which is in the business of producing software that has value to people. I'm one of the people who are paying for that software. The idea is that they invest a certain amount of money developing something, and then they earn that back with a profit margin.

In this case, they have been spending the money on an architecture that was broken before the first line of code was written, and they keep spending more money on it. My comment was an attempt to articulate how they will not be reaching their business goals with this venture and that they need to go back to the drawing board. I have a vested interest in this not just because I want a good serialization library, but because as a customer I don't want to pay for an ongoing development effort that is doomed to fail.

If you have a better way to express this that doesn't sound impolite, I'm always willing to learn.

mattiasflodin avatar Nov 22 '23 08:11 mattiasflodin

@mattiasflodin So you're paying for kotlin serialization library? I thought it was free.

acmpo6ou avatar Nov 22 '23 09:11 acmpo6ou

@mattiasflodin So you're paying for kotlin serialization library? I thought it was free.

Are we still speaking in good faith here? Because it seems to me you are intentionally pretending not to see the bigger picture. But I'll humor you: The people who develop the kotlin serialization library have a salary, and without that salary the serialization library would not have seen the light of day. The salary is paid for with the sale of JetBrains products. Shareholders are happy with this arrangement because the library (if designed properly) could make the ecosystem more popular, which feeds back into more sales of JetBrains products.

However, if JetBrains develops libraries that are not useful then the investment will not lead to more sales, possibly even reduced sales because poorly designed libraries causes customers to lose trust in the company. This could lead to an increased price relative to the value you get for JetBrains products, because a disproportionate amount of the cost is wasted on software that does not increase the value of the products or even harms their value.

I have a very high trust in JetBrains. It has some of the best developers I can think of, the entire company including the support staff has an impressive technical skill, and their company processes seem to be a very well-oiled machine. So when they make mistakes like this, it makes me reevaluate that trust. Did they drop the ball? Are they going to go back and fix it, or just dismiss people who bring up issues and press on?

The best way I can think of to influence JetBrains to stay on track and not fall into the enshittification trap, is to provide feedback such as this issue. The day I stop providing feedback is the day I have lost too much trust and have given up on them.

mattiasflodin avatar Nov 22 '23 09:11 mattiasflodin

@mattiasflodin The fact they get paid for this doesn't change the fact the library is provided for free.

You should always provide the feedback, but it's important to do it carefully, and without harm.

I think my message is very simple.

Let's stop discussing this since at this point it's just off topic.

acmpo6ou avatar Nov 22 '23 10:11 acmpo6ou

Is this still an issue? I can't seem to serialise this subclass (names have been replaced by Copilot for privacy):

@Serializable sealed class A(val a: String) {
    @Serializable @SerialName("tb") data object B : A("tb")
    @Serializable sealed class C(a: String) : A(a) {
        @Serializable @SerialName("cb") data object D : C("cb")
        @Serializable @SerialName("cb1") data object E : C("cb1")
        @Serializable @SerialName("cb2") data object F : C("cb2")
    }
    data object G : A("tu")
    data object H : A("g")

EDIT: the error is: This class is not serializable automatically because it has primary constructor parameters that are not properties referring to @Serializable sealed class C(a: String) : A(a) {

ecastilla95 avatar Apr 03 '24 08:04 ecastilla95

Is this still an issue?

Yes, it is a fundamental design limitation. The fix is to not have a primary constructor (so the serialization code can create its own constructor).

pdvrieze avatar Apr 08 '24 16:04 pdvrieze