jackson-module-kotlin
jackson-module-kotlin copied to clipboard
FasterXML
Jackson skips isXXX properties with Int type
Describe the bug
The Kotlin properties with signature isXXX: Int omits default values even with option JsonInclude.Include.ALWAYS
private data class ProblematicType(val id: Int, val default: Int)
To Reproduce
- Prepare mapper:
val mapper = jacksonObjectMapper().setSerializationInclusion(JsonInclude.Include.ALWAYS)
- Prepare classes:
private data class Correct(val id: Int, val default: Int)
private data class Problematic(val id: Int, val isDefault: Int)
- Serialize both of them via writeValueAsString:
mapper.writeValueAsString(...)
- Compare the results
Expected behavior Both classes have the same result semantic, so jsons are like this:
{id: 1, default: 0}{id: 1, isDefault: 0}
Actual result
isDefault is ignored when it has 0:
{id: 1, default: 0}{id: 1}<--- 0 is ignored for Int type
Versions Kotlin: 1.3.72 Jackson-module-kotlin: 2.11 Jackson-databind: 2.11
Additional context This is issue of 2.11 only. I didn't see it with previous versions (2.10.1 - 2.10.4). Version downgrading helps.
Spring Boots also adds his dependencies. Probably, this is result of version conflict. Actual runtime dependencies are:
+--- com.fasterxml.jackson.core:jackson-databind:2.10.4 -> 2.11.0 (*)
+--- com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.10.4
| +--- com.fasterxml.jackson.core:jackson-core:2.10.4 -> 2.11.0
| \--- com.fasterxml.jackson.core:jackson-databind:2.10.4 -> 2.11.0 (*)
+--- com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.10.4
| +--- com.fasterxml.jackson.core:jackson-annotations:2.10.4 -> 2.11.0
| +--- com.fasterxml.jackson.core:jackson-core:2.10.4 -> 2.11.0
| \--- com.fasterxml.jackson.core:jackson-databind:2.10.4 -> 2.11.0 (*)
@imanushin thanks for your contribution. Workaround for you case is:
private data class Problematic(val id: Int, @get:JsonProperty("isDefault") val isDefault: Int)
@cowtowncoder the problem related to https://github.com/FasterXML/jackson-module-kotlin/issues/80. Based on Kotlin docs:
If the name of the property starts with is, a different name mapping rule is used: the name of the getter will be the same as the property name, and the name of the setter will be obtained by replacing is with set. For example, for a property isOpen, the getter will be called isOpen() and the setter will be called setOpen(). This rule applies for properties of any type, not just Boolean.
The current implementation doesn't take into account a case where a property has a non-boolean type. At this point, jackson-databind tries to fetch proper getter name, but BeanUtil.okNameForIsGetter can properly handle only cases where is applied only for boolean fields.
Could we add a method this check it to AnnotationIntrospector and let override it?
Yeah to me using "is-getter" for non-boolean values seems kind of wrong, but I know Kotlin specifies that as legit use case.
I changed handling in 2.11 to be more configurable, but it is probably true that addition of JacksonAnnotationIntrospector.findRenameByField() would not fully work with non-boolean getter. Adding a MapperFeature would be reasonable, as BeanUtil can not really call AnnotationIntrospector (nor would I want it to be added). But caller would need to take that into accont.
But basically I would not object to having a setting to enable discovery of "other kinds" of is-getters.
@cowtowncoder adding this feature makes sense because for java it doesn't work also:
public class Github337Java {
/* com.fasterxml.jackson.databind.exc.InvalidDefinitionException:
No serializer found for class com.fasterxml.jackson.module.kotlin.test.github.Correct and no properties discovered to create BeanSerializer
(to avoid exception, disable SerializationFeature.FAIL_ON_EMPTY_BEANS) */
@Test
public void serialization() throws JsonProcessingException {
ObjectMapper mapper = new ObjectMapper();
String value = mapper.writeValueAsString(new Correct());
assertNotNull(value);
}
}
class Correct {
private final Integer def = 1;
public Integer isDef() {
return def;
}
}
Yes, but my understanding is that Bean specification would only consider boolean/Boolean valued is getters. But if you or anyone else have link(s) to different interpretation would be happy to be proven wrong?
But even if so, I guess I am not totally against detecting non-boolean-valued is-getters. Change could expose previously unseen properties, if changed for default handling so there's minor backwards compatibility concern.
Based on Java Beans specification (8.3 Design Patterns for Properties) is-getters are valid only for boolean properties.
Have you ever had a request for such a feature in Jackson for Java/Scala? I don't think that this feature makes sense in Java/Scala (it's not covered by spec), it's only Kotlin-specific feature.
@viartemev I do not remember having this request for Java or Scala. The reason I mention MapperFeature is simply as an easy integration point to let Kotlin module (or possibly user) change the setting.
However, I think it is a fair point that a global setting that would also affect non-Kotlin types could be confusing.
I am open to other styles of configuration and if refactoring in jackson-databind is needed that's fine too. Introspection of naming convention is too hard-coded at this point, being quite old cold from pre-1.0 days (added around 0.9.5 or so, when databind was created :) ).
So this might be just one aspect in need of improvements: other examples include (more) configurable "with"-methods for builders.
Unfortunately I haven't had time to dig deeper into this issue, so help would be appreciated.
I added some failing tests, and one working test that shows @viartemev 's workaround using use-site targets.
https://github.com/FasterXML/jackson-module-kotlin/blob/2.12/src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/github/failing/GitHub337.kt
