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

Databind regression with Kotlin module in use, between 2.8.6 and 2.8.7 #1572

Closed
apatrida opened this issue Mar 24, 2017 · 9 comments
Closed
Milestone

Comments

@apatrida
Copy link
Member

A user reported: FasterXML/jackson-module-kotlin#61
to the Jackson-Kotlin module.

In their simple test case, provided here as: https://github.com/mhlz/jackson-module-kotlin-issue

The code works fine with 2.8.6 of Jackson databind, but fails with 2.8.7

The problem is when the JsonIgnore is on one of the constructor parameters, the implied naming of the parameters and implied JsonCreator doesn't work anymore and databind cannot find the properties in the constructor. I do not know if it switches to the default constructor or if it is confused in some other way.

I know this is not a java-only reproducible case (maybe with Java 8 parameter names? but that doesn't imply the JsonCreator) but it is quite easy to debug this into the Java.

Load the pom.xml into Intellij community edition which has the Kotlin plugin (plugin 1.1.0 or 1.1.1 which are the current versions), and debug the main in Main.kt file. You can then debug and see what is going on in data bind.

@cowtowncoder
Copy link
Member

Looking at release notes, probably related to #1317.

@cowtowncoder
Copy link
Member

For convenience will paste here:

import com.fasterxml.jackson.annotation.JsonIgnore
import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.kotlin.registerKotlinModule

annotation class NoArg

data class InnerTest(
        val str: String,

        val otherStr: String
)

@NoArg
@JsonInclude(JsonInclude.Include.NON_ABSENT)
data class Test(
        val innerTest: InnerTest,

        @field:JsonIgnore
        val otherOtherStr: String = ""
)

val testJson = """
{
    "innerTest": {
        "str": "str",
        "otherStr": "otherStr"
    }
}
"""

fun main(args: Array<String>) {
    val mapper = ObjectMapper().registerKotlinModule()
    val test = mapper.readValue(testJson, Test::class.java)
    println(test)
}

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 29, 2017

I'll try to create java-only version; I need regression test anyway, and I think I understand where the problem comes.

But I suspect that this case working may have been more due to luck than design: having creator parameters that are ignored is not really a use case from Java side. But I can understand it may be different story with Kotlin.

... actually, never mind. No, @JsonIgnore can't be added to parameter anyway, gets added to field. Interesting juxtaposition nonetheless....

cowtowncoder added a commit that referenced this issue Mar 29, 2017
@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 29, 2017

Hmmh. I thought I was able to reproduce the issue but no. Guessing this might be difference between explicit @JsonProperty I am using and implicit creator name... so may try that approach instead to tease out the problem.

cowtowncoder added a commit that referenced this issue Mar 29, 2017
@cowtowncoder
Copy link
Member

Alas, no go. I can not really make the test fail either on 2.8(.8-SNAPSHOT) or master...
At this point I think I'd really need to what Kotlin class looks like wrt fields and constructors, and annotations.

For what it is worth, new not-failing tests is

src/test/java/com/fasterxml/jackson/failing/IgnoredCreatorProperty1572Test.java

@cowtowncoder
Copy link
Member

@apatrida Trying this now with 2.8.8-SNAPSHOT (from 2.8) branch, code passes; but going with 2.8.7 I can see failure. I suspect this is same as #1345 which I just fixed; would appreciate someone else double-checking. If fixed, I am thinking of releasing 2.8.8 quite soon since there are quite a few fixes:

https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.8.8

@apatrida
Copy link
Member Author

apatrida commented Apr 3, 2017

@cowtowncoder I'll give it a shot, thanks.

@apatrida
Copy link
Member Author

apatrida commented Apr 3, 2017

@cowtowncoder the code succeeds with 2.8.8-SNAPSHOT

@cowtowncoder cowtowncoder added this to the 2.8.8 milestone Apr 3, 2017
@cowtowncoder
Copy link
Member

@apatrida Ok good. I wish I understood nature of regression (as in: why did it NOT fail in 2.8.x before 2.8.7), but at least I understand the fix.
There is one more issue I hope to resolve before 2.8.8 (related to protobuf encoding) but hope to get 2.8.8 out quite soon.

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

2 participants