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

objectMapper.writeValueAsString does not properly serialize Kotlin data class variables that begin with "is" #575

Closed
skwirskj opened this issue Jul 22, 2022 · 9 comments
Labels

Comments

@skwirskj
Copy link

skwirskj commented Jul 22, 2022

Describe the bug

When calling the objectMapper's writeValueAsString and passing a Kotlin data class to the function, any of the variables that begin with "is" such as "isDayTime" and that is a string type, do not retain its values after serialization. This was observed after using the serialized body in a GET request to a private API where the API threw an error due to missing required parameters. This does work though if the variable is a boolean.

To Reproduce

  1. Create a Kotlin data class with at least one variable of String type, starting with "is"
  2. Create an instance of the data class and an instance of the Jackson Object Mapper
  3. Serialize the data and pass to an API. API should come back with some error if the value is required.

Expected behavior

The objectMapper would serialize the data class as expected and would retain the value, regardless of the naming convention of a variable.

Versions
Kotlin: 1.7.10
Jackson-module-kotlin: 2.13.3
Jackson-databind:

Additional context
Add any other context about the problem here.

@skwirskj skwirskj added the bug label Jul 22, 2022
@jgorgen
Copy link

jgorgen commented Sep 2, 2022

Reproduced in Jackson-module-kotlin: 2.12.1

import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper

        class TestMapperClass(temp: String){
            val isDebug = "no"
            val a = "a"
            val b = "b"
            val c = "c"
        }
        val objectMapper = jacksonObjectMapper()
        val result = objectMapper.writeValueAsString(TestMapperClass("foo"))
        System.out.println(result)

produces

{"a":"a","b":"b","c":"c"}

instead of the expected

{"isDebug":"no","a":"a","b":"b","c":"c"}

@cowtowncoder
Copy link
Member

@jgorgen 2.12.1 is an old version, so at very least should test with 2.12.7 (latest from 2.12).
But ideally with latest release 2.13.4.

I assume reproduction is still valid but this is an important consideration in many other cases.

@zigzago
Copy link

zigzago commented Sep 26, 2022

reproduced with 2.13.4 ;)

@Richie94
Copy link
Contributor

Richie94 commented Oct 3, 2022

I had a look at it and it is filtered out here https://github.com/FasterXML/jackson-databind/blob/2.14/src/main/java/com/fasterxml/jackson/databind/introspect/DefaultAccessorNamingStrategy.java#L73

Seems like the getters starting with is are only allowed with boolean type.

I am not sure, on what is the desired behaviour for this...

@cowtowncoder
Copy link
Member

Thank you @Richie94. Yes, that would be problematic; for original Java Beans there is this limitation -- but Kotlin does not impose it so ideally Jackson would not either, in case of Kotlin classes.
The challenge is that of how to configure this part of behavior: unfortunately it is deep within code and does not get configuration/context passed.

@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 4, 2022

Another thought here: does Kotlin module provide custom AccessorNamingStrategy (added in Jackson 2.12)?
If it doesn't (and I think it doesn't), it could provide one to customize handling for Kotlin types specifically.
This is done by registering AccessorNamingStrategy.Provider which could apply separate strategy just to Kotlin types.
But that could implement whatever logic Kotlin specifically defines.

This could be complementary to:

FasterXML/jackson-databind#3609

which would allow exposing non-boolean-returning-is-getters more generally.

The main reason why I think it'd be good to do this Kotlin-specific override is if it allows solving other Kotlin-naming issues (wrt Java POJO naming Jackson assumes).
General allow of all "is-setters" can be useful for others tho so it need not be either-or case.

@Richie94
Copy link
Contributor

Richie94 commented Oct 4, 2022

I also think your suggestion doing it for all would be the better way, because you could also build these combinations with java code.

@cowtowncoder
Copy link
Member

The problem with doing it for all is that semantics of JAva POJOs, Scala Objects and Kotlin Objects vary to some degree. So the applies-to-all solution will never work exactly right by default for everything.

That said, the basic logic for whether strict "Boolean-returning" is needed for is-getter is something that makes enough sense. It's just that if anything more fine-grained is needed, that should go via AccessorNamingStrategy (or some other pluggable mechanism).

@k163377
Copy link
Contributor

k163377 commented Mar 3, 2023

This issue will be fixed by #630 and therefore closed as a duplicate.

@k163377 k163377 closed this as completed Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants