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

Boolean property name starting with 'is' not serialized/deserialized properly #80

Closed
tbvh opened this issue Jul 12, 2017 · 36 comments
Closed
Labels
is-prefix Issues relating to is-prefixed fields
Milestone

Comments

@tbvh
Copy link

tbvh commented Jul 12, 2017

Hi,
I'm experiencing issues with a boolean property which name starts with 'is'.
While the getter doesn't get prefixed by kotlin, it seems that jackson still strips away 'is' as though it was a prefix. The resulting json doesn't contain the 'is', and deserialization fails.

This test illustrates the problem:

class SerializationTest {
    @Test
    fun testIsBool() {
        val mapper = ObjectMapper()
        mapper.registerModule(KotlinModule())

        val example = IsBoolExample(true)
        val json = mapper.writeValueAsString(example) 
        // json contains: {"trueOrFalse":true}

        val deserialized = mapper.readValue(json, IsBoolExample::class.java)

        Assert.assertEquals(example.isTrueOrFalse, deserialized.isTrueOrFalse)
    }
}
class IsBoolExample(val isTrueOrFalse: Boolean)

This fails on mapper.readValue(..) with:

com.fasterxml.jackson.databind.JsonMappingException: Can not construct instance of com.bol.service.reptile.spring.IsBoolExample: no suitable constructor found, can not deserialize from Object value (missing default constructor or creator, or perhaps need to add/enable type information?)
at [Source: {"trueOrFalse":true}; line: 1, column: 2]

@tbvh
Copy link
Author

tbvh commented Jul 12, 2017

Note that the following test succeeds as expected:

class SerializationTest {
    @Test
    fun testBool() {
        val mapper = ObjectMapper()
        mapper.registerModule(KotlinModule())

        val example = BoolExample(true)
        val json = mapper.writeValueAsString(example)
        // json contains: {"trueOrFalse":true}

        val deserialized = mapper.readValue(json, BoolExample::class.java)

        Assert.assertEquals(example.trueOrFalse, deserialized.trueOrFalse)
    }
}
class BoolExample(val trueOrFalse: Boolean)

And finally, if another field is present (so a constructor can be matched) and FAIL_ON_UNKNOWN_PROPERTIES is false, then the boolean parameter stays (defaults to) false.

class SerializationTest {
    @Test
    fun testStrIsBoolExample() {
        val mapper = ObjectMapper()
        mapper.registerModule(KotlinModule())
        mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)

        val example = StrIsBoolExample(true, "example")
        val json = mapper.writeValueAsString(example)
        // json contains: {"str":"example","trueOrFalse":true}

        val deserialized = mapper.readValue(json, StrIsBoolExample::class.java)

        Assert.assertEquals(example.isTrueOrFalse, deserialized.isTrueOrFalse)
    }
}
class StrIsBoolExample(val isTrueOrFalse: Boolean,val str: String)

Here the assert fails.

@tbvh tbvh changed the title Boolean property name starting with 'is' cannot be deserialized Boolean property name starting with 'is' not serialized/deserialized properly Jul 12, 2017
@hizhengfu
Copy link

Encountered the same problem,
It was also found that pId was serialized to pid

@DavidRigglemanININ
Copy link
Contributor

I'm seeing this issue as well. One thing I noticed is that if I flip the val to a var, then it works as expected. So I'm guessing the issue is that the val generates a Java getter but not the setter, and that's what's causing the problem here.

@apatrida
Copy link
Member

Jackson expects JavaBean naming patterns, Kotlin does not always generate things that way and therefore creates an unwinnable situation. I'm still looking at options, but there is a workaround for some cases.

This works by adding a JsonProperty annotation:

  @Test
    fun testGithub80_FailOnBoolWith_Is() {
        val mapper = jacksonObjectMapper()

        val example = IsBoolExample(true)
        val json = mapper.writeValueAsString(example)
        // json contains: {"trueOrFalse":true}

        val deserialized = mapper.readValue<IsBoolExample>(json)

        assertEquals(example.isTrueOrFalse, deserialized.isTrueOrFalse)
    }

    class IsBoolExample(@JsonProperty("trueOrFalse") val isTrueOrFalse: Boolean)

@raderio
Copy link

raderio commented May 15, 2018

If in Kotlin I have
class IsBoolExample(val isTrueOrFalse: Boolean)
I expect to have isTrueOrFalse in json, not trueOrFalse, if I need trueOrFalse I will have trueOrFalse property in class.

So I think it will be ok to add @JsonProperty with values for boolean properties by default by jackson-module-kotlin, or at least to be configurable by property in module.

@jdorleans
Copy link

I have a similar problem for Boolean property when its name starts with is*. In my case, it is already a var and it doesn't work as mentioned above. The only trivial way I found was to add @JsonProperty, since I want its name to be recognised as it is and not configure the module to ignore it.
Here's my example:

class Product {
  var isNew: Boolean? = null
}
...
val map = mapOf("isNew" to true) // Deserialized Product JSON in a Map<String, Any?>
mapper.convertValue(map, Product::class.java) // FAIL 

The error message says property isNew is not recognised and the only known property is called: new.

@dynamics3
Copy link

dynamics3 commented Jun 20, 2018

We have encountered this thing as well. When we receive json that has a boolean, which starts with "is", then it fails on jacksonObjectMapper().readValue<TestData>(jsonString) throwing UnrecognizedPropertyException. Example: data class TestData(val isOk: Boolean = false)

The funny thing is that when another property is added to data class, then it works flawlessly, e.g.
data class TestData(val addedProp: String = "", val isOk: Boolean = false).

Adding @get:JsonProperty("isOk") annotation fixes the issue. However, it would be great if the appropriate solution is added.

@ahmetgeymen
Copy link

I have also encountered with this issue with my String property. I overcame with using annotation @get:JsonProperty("isSomething"). Though I think we should not use any annotation, Jackson should serialize properly.

See also:
(#52)

@sschuberth
Copy link

sschuberth commented Sep 20, 2018

Another incarnation of the problem seems to be that @JsonIgnore does not work on such properties. For code

@JsonIgnore
val isWholeProjectExcluded = true

the property still gets serialized (as "wholeProjectExcluded").

A work-around is to write the code as

val isWholeProjectExcluded
        @JsonIgnore
        get() = true

or shorter, as @Tapac points out, as

@get:JsonIgnore
val isWholeProjectExcluded = true

@tjsnell
Copy link

tjsnell commented Oct 30, 2018

Just wasted a bunch of time tracking this down. Any plans to fix it?

@Tapac
Copy link

Tapac commented Nov 12, 2018

@sschuberth, you can use get as annotation target (see more here)
@get:JsonIgnore val isWholeProjectExcluded = true

@sschuberth
Copy link

Thanks @Tapac, will use that!

@brokenthorn
Copy link

We have encountered this thing as well. When we receive json that has a boolean, which starts with "is", then it fails on jacksonObjectMapper().readValue<TestData>(jsonString) throwing UnrecognizedPropertyException. Example: data class TestData(val isOk: Boolean = false)

The funny thing is that when another property is added to data class, then it works flawlessly, e.g.
data class TestData(val addedProp: String = "", val isOk: Boolean = false).

Adding @JsonProperty("isOk") annotation fixes the issue. However, it would be great if the appropriate solution is added.

@JsonProperty does not work for me.

My code actually looks like this:

@JsonProperty("isSubstance")
val isSubstance: Boolean = false,

and it still serializes as substance.

@raderio
Copy link

raderio commented Dec 5, 2018

@brokenthorn you need to use @get:JsonProperty("isSubstance")

@dynamics3
Copy link

We have encountered this thing as well. When we receive json that has a boolean, which starts with "is", then it fails on jacksonObjectMapper().readValue<TestData>(jsonString) throwing UnrecognizedPropertyException. Example: data class TestData(val isOk: Boolean = false)
The funny thing is that when another property is added to data class, then it works flawlessly, e.g.
data class TestData(val addedProp: String = "", val isOk: Boolean = false).
Adding @JsonProperty("isOk") annotation fixes the issue. However, it would be great if the appropriate solution is added.

@JsonProperty does not work for me.

My code actually looks like this:

@JsonProperty("isSubstance")
val isSubstance: Boolean = false,

and it still serializes as substance.

As @raderio has already mentioned, you need to use @get:JsonPropert("isSubstance"). Sorry about that, I have edited my comment as well to reflect this. Somehow, made a mistake when wrote that post. Good luck with your code!!!

@milosmns
Copy link

But why is this happening? There is no straight answer. Booleans even get compiled to fields in Java prefixed with "is". There is absolutely no reason to strip the "is", please fix this.

@brokenthorn
Copy link

But why is this happening? There is no straight answer. Booleans even get compiled to fields in Java prefixed with "is". There is absolutely no reason to strip the "is", please fix this.

Totally agree. This seems very inconsistent and makes implementing serialization from other platforms that follow a more expected format, harder.

@daliborfilus
Copy link

Oh my...

@cowtowncoder
Copy link
Member

Not sure why this would happen with Kotlin module, but core Jackson databind ONLY considers "is"-prefix for getters and setters -- field names (and constructor parameter names if available) are used as-is and do not infer any special handling.

@DavidRigglemanININ
Copy link
Contributor

Could it be that a kotlin val/var has a auto generated getter/setter for Java compatibility purposes that is prefixed with "is"? For example, if I have a kotlin property "done", I believe it will generate a getter/setter where you can call isDone() from Java.

@brokenthorn
Copy link

Could it be that a kotlin val/var has a auto generated getter/setter for Java compatibility purposes that is prefixed with "is"? For example, if I have a kotlin property "done", I believe it will generate a getter/setter where you can call isDone() from Java.

Makes no sense then to remove the is part when something like isDone is defined in Kotlin, not Java.

@DavidRigglemanININ
Copy link
Contributor

DavidRigglemanININ commented Feb 27, 2019

So I did a deep dive and figured out what was going on. My general assumption was mostly correct in that it was an issue with the generated getter/setters, although just in a slightly different way than I originally thought. The way that Kotlin works is that if you have a val boolean field isTrueOrFalse, Kotlin will generate a getter (and a setter if it's a var). For booleans, Kotlin appears to generate the getter/setters with the same name (it seems to skip adding the "is" prefix if the boolean field already is prefixed by "is"; interestingly, the same behavior does not appear to happen in non boolean getters where the field name starts with get). So now we are left with having both a field and method for a given property. Now inside the Jackson data bind code (so not part of kotlin module), there is logic to get rid of the field (BeanSerializaerFactory.removeIgnorableTypes), leaving only the method left. Now there is code later on to find the implicit property name. But because it's a method and not a field, it calls BeanUtil.okNameForIsGetter. This logic in turn strips out the "is" prefix because it's working under the standard Java naming convention for beans where the underlying field does not have the "is" but the getter does. So where does this leave us? I'm not sure what the best solution may be, but here's one proposal I'm currently testing locally that only requires changes to this module code.
In KotlinModule.kt, update the code to contain the following

internal class ReflectionCache(reflectionCacheSize: Int) {
    private val javaClassToKotlin = LRUMap<Class<Any>, KClass<Any>>(reflectionCacheSize, reflectionCacheSize)
    private val javaConstructorToKotlin = LRUMap<Constructor<Any>, KFunction<Any>>(reflectionCacheSize, reflectionCacheSize)
    private val javaMethodToKotlin = LRUMap<Method, KFunction<*>>(reflectionCacheSize, reflectionCacheSize)
    private val javaConstructorIsCreatorAnnotated = LRUMap<AnnotatedConstructor, Boolean>(reflectionCacheSize, reflectionCacheSize)
    private val kotlinGeneratedBooleanMethod = LRUMap<AnnotatedMethod, Boolean>(reflectionCacheSize, reflectionCacheSize)

    fun kotlinFromJava(key: Class<Any>): KClass<Any> = javaClassToKotlin.get(key) ?: key.kotlin.let { javaClassToKotlin.putIfAbsent(key, it) ?: it }
    fun kotlinFromJava(key: Constructor<Any>): KFunction<Any>? = javaConstructorToKotlin.get(key) ?: key.kotlinFunction?.let { javaConstructorToKotlin.putIfAbsent(key, it) ?: it }
    fun kotlinFromJava(key: Method): KFunction<*>? = javaMethodToKotlin.get(key) ?: key.kotlinFunction?.let { javaMethodToKotlin.putIfAbsent(key, it) ?: it }
    fun checkConstructorIsCreatorAnnotated(key: AnnotatedConstructor, calc: (AnnotatedConstructor) -> Boolean): Boolean = javaConstructorIsCreatorAnnotated.get(key) ?: calc(key).let { javaConstructorIsCreatorAnnotated.putIfAbsent(key, it) ?: it }
    fun isKotlinGeneratedBooleanMethod(key: AnnotatedMethod, calc: (AnnotatedMethod) -> Boolean): Boolean = kotlinGeneratedBooleanMethod.get(key) ?: calc(key).let { kotlinGeneratedBooleanMethod.putIfAbsent(key, it) ?: it }
}

...

override fun findImplicitPropertyName(member: AnnotatedMember): String? {
        if (member is AnnotatedParameter) {
            return findKotlinParameterName(member)
        } else if (member is AnnotatedMethod) {
            if (member.declaringClass.isKotlinClass() && (member.rawReturnType == Boolean::class.java || member.rawReturnType == java.lang.Boolean::class.java)) {
                if (cache.isKotlinGeneratedBooleanMethod(member, { it.declaringClass.declaredFields.any { f -> f.name == member.name } })) {
                    return member.name
                }
            }
        }
        return null
    }

What I am doing here is basically checking if there is a field with the same name as the method, then use the method name (without any name mangling). I've limited it to kotlin, boolean properties, and backed with a cache to keep the impact as small as possible and also for performance reasons since the check involves reflection.

Note that my solution will ignore JsonProperty annotations explicitly setting it to a different name. However, from my testing, I've found that JsonProperty doesn't even work if you have a "isTrueOrFalse" property name with JsonProperty("foo") annotation (serializing works but deserializing fails). If you did want to honor the property, you could add a check of !member.hasAnnotation(JsonProperty::class.java) right at the beginning of if(cache.isKotlinGeneratedBooleanMethod...

@adhesivee
Copy link

When will this be fixed? I still have this issue with 2.9.9 and is causing some unexpected behavior.

@cowtowncoder cowtowncoder added this to the 2.10.1 milestone Oct 26, 2019
cowtowncoder added a commit that referenced this issue Oct 26, 2019
@cowtowncoder
Copy link
Member

@ElbowBaggins thanks!

@cowtowncoder
Copy link
Member

Merged @LipatovAndrey 's fix -- will be in 2.10.1

And also contemplating actually adding databind MapperFeature in 2.11, to allow this out of the box (possibly to further improve in 3.0).

pdelagrave pushed a commit to pdelagrave/orca that referenced this issue Dec 12, 2019
* jackson-module-kotlin changed the serialization behaviour for fields starting with 'is': FasterXML/jackson-module-kotlin#80
* jackson 2.10's objectMapper.convertValue() now converts everything even if the source is already from the target type. The fixed test was expected a value from a not fully converted Map.
mergify bot pushed a commit to spinnaker/orca that referenced this issue Dec 17, 2019
* fix(templated-pipelines-v1): Expected artifacts correctly inherited/merged

TemplateMerge.mergeDistinct() was merging 2 list of different types into one. Upcoming Jackson library update  would have broken compilation of TemplatedPipelineModelMutator if left unmodified.

* chore(dependencies): Upgrade Spring Boot to 2.2.1

Adjusting code for Jedis 3, Jackson 2.10 upgrades.

* fix(artifacts): Always persisting `pipeline.expectedArtifacts.boundArtifact`

Artifact resolution wasn't always persisting `pipeline.expectedArtifacts.boundArtifact` depending on the expectedArtifact's type when passed to `ArtifactResolver.resolveArtifacts()`.
Only `ExpectedArtifact` would get their `boundArtifact` updated; if a Map was used then a copy of that Map would be updated instead and this isn't returned to the caller.
The update of Jackson to 2.10 made it so that `boundArtifact` would always be updated on a copy of the passed `expectedArtifact` no matter its type, this commit is an attempt to fix both problems.
A better fix would be to make it clear what type is accepted and expected, but that's a much larger refactor.

* chore(dependencies): Upgrade Spring Boot to 2.2.1 cont'd

* jackson-module-kotlin changed the serialization behaviour for fields starting with 'is': FasterXML/jackson-module-kotlin#80
* jackson 2.10's objectMapper.convertValue() now converts everything even if the source is already from the target type. The fixed test was expected a value from a not fully converted Map.

* chore(dependencies): bump kork and keiko
cowtowncoder added a commit that referenced this issue Dec 28, 2019
@FabrizioBilleciUNICT
Copy link

@get:PropertyName("isSomething") var isSomething works also for me.

@LifeIsStrange
Copy link

@cowtowncoder I thought this would be fixed out of the box for jackson-module-kotlin 2.11 but I am using the latest stable version
through implementation("com.fasterxml.jackson.module:jackson-module-kotlin") so I believe I should have 2.11 on my system and am still hitting this issue. (yes I manually solve it through @get:JsonProperty)

@MaksimRT
Copy link

@cowtowncoder I thought this would be fixed out of the box for jackson-module-kotlin 2.11 but I am using the latest stable version
through implementation("com.fasterxml.jackson.module:jackson-module-kotlin") so I believe I should have 2.11 on my system and am still hitting this issue. (yes I manually solve it through @get:JsonProperty)

Have you tried to add jackson-databind module as well?)

It will cause an issue without it(

        <dependency>
            <groupId>com.fasterxml.jackson.module</groupId>
            <artifactId>jackson-module-kotlin</artifactId>
            <version>${jackson-module-kotlin.version}</version>
        </dependency>
        <dependency>
            <groupId>com.fasterxml.jackson.core</groupId>
            <artifactId>jackson-databind</artifactId>
            <version>${jackson-databind.version}</version>
        </dependency>

@cowtowncoder
Copy link
Member

Yes, both Kotlin module and jackson-databind would need to be 2.11(.2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is-prefix Issues relating to is-prefixed fields
Projects
None yet
Development

No branches or pull requests