Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't instantiate new instances of Kotlin singleton objects #225

Closed
Dico200 opened this issue May 4, 2019 · 6 comments
Closed

Don't instantiate new instances of Kotlin singleton objects #225

Dico200 opened this issue May 4, 2019 · 6 comments

Comments

@Dico200
Copy link

Dico200 commented May 4, 2019

Same topic as the problem described by #141, #196, #159.

On #141, @apatrida said:

This is outside the scope of the module, you cannot replace an object singleton in Kotlin, and if it is immutable values the content could not be set. Jackson does not "bind into existing instances" for any case and the definition of readValue is specifically to create new objects created from the contents of the JSON. The Kotlin module only provides information to the data binder and does not control when it decides to create an instance only in how it knows how to create that instance.

By his logic, Jackson should be instantiating enum values too, because they can contain state. But obviously, Jackson doesn't do that, because it enums are singletons by definition.
Kotlin singleton objects fall into the exact same category as enum values. They can contain state, but that state should not be serialized, just like enum values' state. There should only ever be one instance, and Jackson instantiating new ones can break applications.

Jackson does not "bind into existing instances" for any case

Except for enum values. And it should do the same for Kotlin objects, as per my argument above.

It is a very common Pattern in Kotlin to use objects in place of enums because they can be used as constant instances of a sealed class, where other subclasses can hold state because they are instantiable. It is hurting very much that the Jackson Kotlin module cannot handle this properly, because object singletons are not comparable by == operator by default, and programmers expect === to work.

@apatrida
Copy link
Member

apatrida commented May 5, 2019

@Dico200 What is your proposal here? A singleton has a contract that the instance will not change. You propose that we violate that and swap in another instance? would it be a thread-safe swap? How so when the access isn't protected? Would we merge into the instance? How so when it wouldn't be thread safe? There is a contract here with Kotlin objects that we would violate to replace the instance.

The best that could be done here is a merge, but again many objects are designed to have properties set later.

What is your proposed solution here?

@apatrida
Copy link
Member

apatrida commented May 5, 2019

@Dico200 What is the expected behavior of the serialization and deserialization of these objects? to include the actual values when serializing the JSON? Then when deserializing what if the values do not match?

@Dico200
Copy link
Author

Dico200 commented May 5, 2019

@apatrida The exact same behaviour as enum constants is the expected behaviour. How you would implement it is up to you.

@Dico200
Copy link
Author

Dico200 commented May 5, 2019

This pull request looks to have implemented something for this: link

However, it will require an accompanying serializer to indeed avoid any of the object's state being serialized, as you hinted on that pull request @apatrida

@cowtowncoder cowtowncoder changed the title Don't instantiate new instances of kotlin singleton objects. Don't instantiate new instances of Kotlin singleton objects Oct 21, 2019
cowtowncoder pushed a commit that referenced this issue Oct 21, 2019
@apatrida
Copy link
Member

This is fixed by #244
yes?

@Dico200
Copy link
Author

Dico200 commented Oct 27, 2019

Great! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants