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

Jackson deserializes kotlin.Unit singleton as new instances. #196

Closed
LDVSOFT opened this issue Dec 3, 2018 · 9 comments
Closed

Jackson deserializes kotlin.Unit singleton as new instances. #196

LDVSOFT opened this issue Dec 3, 2018 · 9 comments

Comments

@LDVSOFT
Copy link

LDVSOFT commented Dec 3, 2018

Kotlin has builtin support for singleton classes in form of object-s. The most trivial one is Unit from standard library and is used like void in Java: every method that returns nothing actually returns Unit type, and empty return statement returns Unit. In most cases you would not see it as it's represented as Java void, but in some cases it will be represented with singleton instance.

As stated in #141, it's not in Jackson power to understand which classes are only used as singletones, but object ones should be considered those, especially, for example, standard Unit.

For example, let's condider that user serializes a generic structure that happens to store several Unit values. After serialization, they all are represented by {}, that may not be the best form, but it's OK. But when deserialized, they all become Unit instanses that are not equal to each one nor the Unit one. The shortest form to test this is:

println(jacksonObjectMapper().readValue<Unit>("{}") === Unit)

Kotlin does expose information about object classes via KClass::objectInstance property. Maybe, then Jackson should (at least for stantard library classes) use it?

@apatrida
Copy link
Member

apatrida commented May 5, 2019

I'm surprised by the use case of storing the value of Unit ... when is this needed?

@LDVSOFT
Copy link
Author

LDVSOFT commented May 13, 2019

I'm surprised by the use case of storing the value of Unit ... when is this needed?

I used it when one our Map-like structure was needed with fake missing values, but not null-s. Unit does it's job. It's not the finest value, maybe, but good enough for now.

@apatrida
Copy link
Member

@LDVSOFT can you check this against the 2.10 branch, there was a fix for object classes, and if Unit is defined in a similar way it would work for that as well. Check and I'll re-open if you tag a comment here that it is still a problem.

@schmist
Copy link
Contributor

schmist commented Jan 15, 2021

The issues seems to be back since 2.12.0 (2.11.4 is still working):

jacksonObjectMapper().readValue<Unit>("{}") === Unit

results in false.

@hudson155
Copy link

I'm seeing the same. This issue has returned. This threw me for a pretty good runaround today! 😂

@schmist
Copy link
Contributor

schmist commented Feb 18, 2021

Is there a plan to fix this in the future or should we find another way (e.g., not use {})?

@cowtowncoder
Copy link
Member

@schmist @hudson155 adding comments on closed issues is less visible than filing a new issue with link to earlier one, so if this is still problematic, please file a new issue.

I don't have context here to know if there has been regression: if so, providing a PR for failing test case(s) (can be put under src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/failing) would be a good idea.
Also wondering if this was related to #281 where there only seemed to be not-great solutions.

dinomite added a commit that referenced this issue Feb 19, 2021
@dinomite
Copy link
Member

Unfortunately this didn't have a test so it's hard to pinpoint where it broke—someone spending time with git bisect should be able to track it down. I added a failing tests so we'll know if this gets fixed in the future and that test can be trivially repurposed to protect against future regression.

@kylooh
Copy link

kylooh commented Oct 30, 2023

The issues seems to be back since 2.12.0 (2.11.4 is still working):

jacksonObjectMapper().readValue<Unit>("{}") === Unit

results in false.

With the latest Jasckon version 2.16.0-rc1, and Kotlin version 1.9.10. Object deserialize still not work, the answer of deserializing object from String, is not equals to the original object

Here is a simplest example

object Data

fun main() {
    val jsonMapper = jacksonObjectMappe()
    assert(jsonMapper.readValue<Data>("{}") == Data) // the answer is not equal
}

And the simplest way to fix this in Kotlin 1.9 and above, is make object as a data class. For other version's Kotlin which is below 1.9. There is no solution for this problem now.

data  object Data

Also, I test the mentioned version 2.10 with com.fasterxml.jackson:jackson-bom:2.10.5.20201202, it actually works. So, this behavior break might be imported manually. And according to this comment #196 (comment). It might be imported after version 2.12.

Then I checked code, I found there is a class SingletonSupport. As it description says, the CANONICALIZE option should fix #225's problem. Also it say it is default after 2.10. But I checked KotlinFeature.SingletonSupport, it says the singleton feature is not enabled by default. Is this could be some mistakes?

I tried debuging. Found that is version 2.10, class KotlinBeanDeserializerModifier think the registered bean is not Kotlin class, so it returns the original instance beanClass.kotlin.objectInstance. So this one is the main logic for object deserializing.

And I traced this code, found that in KotlinMoudle#setupModule field singletonSupport is DISABLE, which is not follow the description in SingletonSupport. But the KotlinModule's description did saied singletonSupport is disable by default

Then I found it be disabled by this comment #307 . Which leads us to #281 . So the current way to solve this problem is

val jsonMapper = jsonMapper {
        addModule(
            kotlinModule {
                configure(KotlinFeature.SingletonSupport, true)
            }
        )
    }

Hope this comment will help you guys.

Also make object enable as default, In my option, should be true.

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

7 participants