Missing empty constructor issue: deserialization breaking change from 2.17 to 2.18

[baylrock]

Search before asking

• I searched in the issues and found nothing similar.
• I have confirmed that the same problem is not reproduced if I exclude the KotlinModule.
• I searched in the issues of databind and other modules used and found nothing similar.
• I have confirmed that the problem does not reproduce in Java and only occurs when using Kotlin and KotlinModule.

Describe the bug

It appears that 2.18 introduced a change to the constructor detection, causing existing use cases to fail. I observed it in a case where a class extending a Map without an empty constructor can no longer be instantiated. See the test case example.

To Reproduce

With jackson 2.18.0 onboard:

    @Test
    fun test() {
        assertThrows<MissingKotlinParameterException> {
            jacksonObjectMapper().readValue<Old>("""{ "key":"value" }""")
        }

        assertDoesNotThrow {
            jacksonObjectMapper().readValue<New>("""{ "key":"value" }""")
        }
    }

    // what was working prior to 2.18
    class Old : TreeMap<String, String> {
        constructor(map: Map<String, String>) : super(map)
    }

    // what has to be changed to work with 2.18
    class New : TreeMap<String, String> {
        constructor() : super()
        constructor(map: Map<String, String>) : super(map)
    }

Expected behavior

Changes should be backward-compatible per the versioning standard (minor version changed).

Versions

Kotlin:
Jackson-module-kotlin: 2.18.0
Jackson-databind: 2.18.0

Additional context

I'm not 100% sure if this is a Kotlin module issue or not, as I operate in Kotlin codebase only and not invested enough to test it in plane java.
Generally, the fix to the issue is simple; the questionable part is that, if this is intentional, this is technically a breaking change in a minor version change.

[cowtowncoder]

Needs to be checked against 2.18.1 at least (ideally also 2.18.2-SNAPSHOT, build from 2.18 branch, but that's optional) as there are multiple relevant jackson-databind fixes.

[fischl-viesure]

I can confirm the bug with these versions of jackson-module-kotlin:

• 2.18.1
• 2.18.2-SNAPSHOT (commit 9889989)

I also tested with version 2.17.2 and the behavior to assure that the MissingKotlinParameterException is not thrown with the older version, as described.

[k163377]

The root cause of this also seemed to be the same as #841 (FasterXML/jackson-databind#4777).

I first checked how the Old constructor is handled in KotrinamesAnnotationIntrospector and both were determined to be creator with mode=DEFAULT.
To be precise, 2.17 uses findCreatorAnnotation and 2.18 uses findDefaultCreator, but since the mode is the same, there is probably no difference.

On the other hand, as a result of StdValueInstantiator.createFromObjectSettings now registering withArgsCreator, valueInstantiator.canCreateFromObjectWith now returns true.
This changes the processing path of MapDeserializer.deserialize, causing KotlinValueInstantiator.createFromObjectWith to be called unexpectedly, resulting in an error.

[cowtowncoder]

@baylrock Please include actual exception message (with at least some of the stack trace).

[cowtowncoder]

It looks like intent was for Constructor to use "Delegating" mode; and if so perhaps it is necessary to use annotation

@JsonCreator(mode = JsonCreator.Mode.DELEGATING)

since I don't see any good heuristics for databind to determine this intent (constructor parameter name and underlying property. Not sure why in 2.17 that would be mode selected.

EDIT: Actually, I do have a guess. Perhaps it is due to type of single-argument being Map, tilting heuristics toward delegating.

[baylrock]

@cowtowncoder issue was found when migrating from 2.12 to 2.18. So it shouldn't be about 2.17 being weird.

[cowtowncoder]

@baylrock That is assuming behavior as of 2.17 was correct, which while not unreasonable assumption is not always true. In case of refactoring of property introspection for 2.18, logic was rewritten to address actual flaws for some well-known cases, but making sure all unit tests pass. Problem here is that there is an unlimited number of combinations, edge cases, and so coverage is always incomplete. While case presented here is not very complex, it seems likely it wasn't covered, and change in logic was not detected.

The question, then, is which of behaviors is more correct: that of 2.17, or that of 2.18.
Put another way: just because 2.17 behaves in certain way is not a guarantee it is the intended ("correct") way.

Having said that, I do now have one idea wrt why Creater mode heuristics may have changed with 2.18 -- I vaguely remember there being some logic to consider some types (specifically Map) as indicating preference towards choosing Mode.DELEGATING.

But with all of this said: I would suggest adding explicit

@JsonCreator(mode = JsonCreator.Mode.DELEGATING)

(with whatever mechanism Kotlin requires)
for the intended Constructor. That will eliminate any need to use heuristics on work with any 2.x version o Jackson.

[fischl-viesure]

@baylrock Please include actual exception message (with at least some of the stack trace).

I take the liberty and step in for baylrock, 'cause I just happened to see the updates to this ticket now.

This is the exception when instantiating class Old with version 2.18.2:

com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException: Instantiation of [map type; class JacksonTest$Old, [simple type, class java.lang.String] -> [simple type, class java.lang.String]] value failed for JSON property map due to missing (therefore NULL) value for creator parameter map which is a non-nullable type
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 17] (through reference chain: JacksonTest$Old["N/A"]->JacksonTest$Old["map"])

	at com.fasterxml.jackson.module.kotlin.KotlinValueInstantiator.createFromObjectWith(KotlinValueInstantiator.kt:97)
	at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:214)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer._deserializeUsingCreator(MapDeserializer.java:711)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:432)
	at com.fasterxml.jackson.databind.deser.std.MapDeserializer.deserialize(MapDeserializer.java:32)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4917)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3843)
	at JacksonTest.test(JacksonTest.kt:34)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)

Also, annotating the constructor, as you described, with…

@JsonCreator(mode = JsonCreator.Mode.DELEGATING)

…made the exception go away 🥳 (anyway, my problem turned out to not being the one described here 😊).

Thank you cowtowncoder for helping us along!