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

2.12.x regression: no default no-arguments constructor found #396

Closed
henrik242 opened this issue Oct 13, 2020 · 52 comments
Closed

2.12.x regression: no default no-arguments constructor found #396

henrik242 opened this issue Oct 13, 2020 · 52 comments
Labels
xml Related to XML handling
Milestone

Comments

@henrik242
Copy link

henrik242 commented Oct 13, 2020

I have this Kotlin object which parses an XML into a data class:

object XmlTool {

    @JvmStatic
    fun parseXml(xml: String?): Product {
        val xmlIn = XMLInputFactory.newInstance()
        val factory = XmlFactory(xmlIn)
        val xmlModule = JacksonXmlModule()

        val mapper = XmlMapper(factory, xmlModule).registerKotlinModule()

        return mapper.readValue(xml, Product::class.java)
    }

    data class Stuff(val str: String?)
    data class Product(val stuff: Stuff?)
}

And this Java test:

class Jackson212MissingConstructorTest {

    @Test
    void fails_with_jackson_2_12() throws Exception {
        String xml = "<product><stuff></stuff></product>";

        Product product = XmlTool.parseXml(xml);

        assertEquals(new Product(null), product);
    }
}

Parsing this in Jackson 2.12.0 fails with com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of jackson.xml.XmlTool$Stuff (although at least one Creator exists): no default no-arguments constructor found.

Jackson 2.11.3 works just fine.

The full exception is:

jackson.xml.Jackson212MissingConstructorTest > fails_with_jackson_2_12() 
    com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `jackson.xml.XmlTool$Stuff` (although at least one Creator exists): no default no-arguments constructor found
     at [Source: (StringReader); line: 1, column: 17] (through reference chain: jackson.xml.XmlTool$Product["stuff"])
        at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
        at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1590)
        at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1215)
        at com.fasterxml.jackson.databind.deser.ValueInstantiator.createUsingDefault(ValueInstantiator.java:248)
        at com.fasterxml.jackson.databind.deser.std.StdValueInstantiator.createUsingDefault(StdValueInstantiator.java:275)
        at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.getEmptyValue(BeanDeserializerBase.java:1027)
        at com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromEmptyString(StdDeserializer.java:322)
        at com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromString(StdDeserializer.java:271)
        at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromString(BeanDeserializerBase.java:1480)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:207)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:197)
        at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:542)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:565)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:449)
        at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1390)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:362)
        at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:195)
        at com.fasterxml.jackson.dataformat.xml.deser.XmlDeserializationContext.readRootValue(XmlDeserializationContext.java:91)
        at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4568)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3523)
        at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3491)
        at jackson.xml.XmlTool.parseXml(XmlTool.kt:19)
        at jackson.xml.Jackson212MissingConstructorTest.fails_with_jackson_2_12(Jackson212MissingConstructorTest.java:14)

Here's a test project: https://github.com/henrik242/jackson-xml-problem/tree/2-12-empty-constructor

@henrik242
Copy link
Author

@cowtowncoder Have you seen this? Looking forward to rc2 😄

@kupci
Copy link
Member

kupci commented Nov 10, 2020

@henrik242 To make sure this is categorized correctly, are you sure this should be here in jackson-dataformat-xml? Or should it go under jackson-module-kotlin?

dependencies {
    implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-xml:$jacksonVersion")
    implementation**("com.fasterxml.jackson.module:jackson-module-kotlin:$jacksonVersion")**

    testImplementation("org.junit.jupiter:junit-jupiter-api:$junitVersion")
    testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:$junitVersion")
}

@cowtowncoder
Copy link
Member

cowtowncoder commented Nov 10, 2020

@henrik242 Had not noticed it. As per @kupci's comment I suspect this might not Kotlin-specific, so I'll try to see if I can reproduce with XML and Java.

I would expect test to probably fail for different reason: instead of null, should give empty String to constructor. But we'll see.

@cowtowncoder
Copy link
Member

cowtowncoder commented Nov 10, 2020

Actually, no, I would need a plain Java reproduction -- I am guessing this may well be due to problem with @JsonCreator handling (properties vs delegating), and/or not using @JacksonXmlText. Alternative would be to try to reproduce this with json and move to Kotlin but that I don't think would work well.

Test itself could be added under jackson-integration-tests which can combine Kotlin and XML modules (neither of which is allowed to depend on each other, even for tests, since doing so would lead to cyclic dependency that's gnarly for builds).

So, the issue very likely has to do with the way Kotlin module's mapping of data classes to creators; in this case creator would have to be "delegating", not properties-based, looking at structure. I don't know how it might have worked before but I do not think there is a way to make that structure map as expected with properties-style constructors.

@cowtowncoder
Copy link
Member

Example works with Java definitions like:

    static class Stuff427 {
        String str;

        @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
        public Stuff427(String s) { str = s; }
    }

    static class Product427 {
        Stuff427 stuff;

        public Product427(@JsonProperty("stuff") Stuff427 s) { stuff = s; }
    }

    public void testEmptyIssue427() throws Exception
    {
        String xml = "<product><stuff></stuff></product>";
        Product427 product = MAPPER.readValue(xml, Product427.class);
        assertNotNull(product);
        assertNotNull(product.stuff);
        assertEquals("", product.stuff.str);
    }

and the distinction is that the outer POJO requires properties-based creator (since there is propert "stuff"), whereas inner POJO requires delegate-based as it must map straight from String value.

So I don't see a bug in XML format module, and suspect that challenge is that of annotating Stuff to delegate String value.

cowtowncoder referenced this issue in FasterXML/jackson-dataformat-xml Nov 10, 2020
@henrik242
Copy link
Author

@cowtowncoder Should I re-post this issue in the Kotlin module? Or add a PR to https://github.com/FasterXML/jackson-integration-tests ? Or maybe both?

@cowtowncoder
Copy link
Member

@henrik242 I think I'll first transfer this to Kotlin module, for maintainers to see.

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-dataformat-xml Nov 11, 2020
@cowtowncoder cowtowncoder added the xml Related to XML handling label Nov 11, 2020
@henrik242
Copy link
Author

@cowtowncoder @dinomite The 2.12 release fails with this issue now.

@henrik242 henrik242 changed the title 2.12.0-rc1 regression: no default no-arguments constructor found 2.12.0 regression: no default no-arguments constructor found Nov 30, 2020
@dinomite dinomite added this to the 2.12.1 milestone Dec 1, 2020
@michaelbrewer
Copy link

What is the timeline for 2.12.1 release and can we add this to the test coverage to prevent regression

@cowtowncoder
Copy link
Member

Typically first .1 patch follows in about 1 month of the .0, depending on accumulation of fixes.
I am trying to take it easy over December anyway, but I think it is likely that 2.12.1 would be released around end of 2020, either last week of Dec or first of Jan 2021.

As to regression test: if the test requires both XML and Kotlin modules, it should to go in:

https://github.com/FasterXML/jackson-integration-tests

to avoid adding cross-component dependencies (beyond existing compile-time dependencies to core components). This assuming it is not possible to reproduce this without xml module (or kotlin module).

@henrik242
Copy link
Author

@cowtowncoder @michaelbrewer @dinomite I've added an integration test for this now

@dinomite
Copy link
Member

@cowtowncoder We actually already have the jackson-dataformat-xml module in the Kotlin module's test dependencies, so I added @henrik242 's test to this module's test suite.

I haven't dug into what's actually going wrong, happy to take tips or PRs against that branch for a fix.

@cowtowncoder
Copy link
Member

@dinomite Ok. I guess that's fine then -- means that I will need to make sure to always publish XML module before Kotlin one, but that is reasonable from ordering perspective.

@dinomite
Copy link
Member

Hmm, that's a good point so maybe it should be in the integration tests. Is the error only happening with the XML module in conjunction with the Kotlin module?

@henrik242
Copy link
Author

henrik242 commented Dec 12, 2020

Is the error only happening with the XML module in conjunction with the Kotlin module?

Yes, AFAIK

@dinomite
Copy link
Member

Just dropping in to mention that the branch is open with the test case, awaiting a fix: #401

@dinomite dinomite added the 2.12 label Dec 19, 2020
@cowtowncoder
Copy link
Member

@dinomite it should be possible to add currently failing tests under src/test/kotlin/com/fasterxml/jackson/module/kotlin/test/failing -- that allows running manually without breaking build?

@dinomite
Copy link
Member

Good idea, I'll move that test into the failing tests group

@dinomite
Copy link
Member

@henrik242 Thanks for keeping an eye on this, please report back once you've had a chance to check out 2.12.5

@cowtowncoder
Copy link
Member

2.12.5 is out, but if if 2.13.0-rc2 fails, I suspect the issue is still there.

@hrkfdn
Copy link

hrkfdn commented Aug 30, 2021

For me the issue is still there with 2.12.5

@henrik242
Copy link
Author

henrik242 commented Aug 31, 2021

No reason for everyone to run their own personal tests, just run the one in jackson-integration-tests:

$ git checkout 2.12
$ mvn test -Dtest=Jackson212MissingConstructorTest
...
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.657 s <<< FAILURE! - in com.fasterxml.jackson.failing.Jackson212MissingConstructorTest
[ERROR] testMissingConstructor(com.fasterxml.jackson.failing.Jackson212MissingConstructorTest)  Time elapsed: 0.599 s  <<< ERROR!
com.fasterxml.jackson.databind.exc.MismatchedInputException: 
Cannot construct instance of `com.fasterxml.jackson.failing.Jackson212MissingConstructorTest$Stuff` (although at least one Creator exists): no default no-arguments constructor found
 at [Source: (StringReader); line: 1, column: 17] (through reference chain: com.fasterxml.jackson.failing.Jackson212MissingConstructorTest$Product["stuff"])
	at com.fasterxml.jackson.failing.Jackson212MissingConstructorTest.testMissingConstructor(Jackson212MissingConstructorTest.kt:22)

@henrik242
Copy link
Author

henrik242 commented Aug 31, 2021

@vy I created a pull request with your test in jackson-integration-tests, hope that's ok! FasterXML/jackson-integration-tests#12

EDIT: Never mind, I saw just noticed that you added it yourself in FasterXML/jackson-integration-tests#10 Sorry! :)

@dinomite
Copy link
Member

dinomite commented Sep 2, 2021

Test added in jackson-module-xml FasterXML/jackson-dataformat-xml#492

@dinomite dinomite closed this as completed Sep 2, 2021
@henrik242
Copy link
Author

Just as a note to myself (and others following this), this bug is now being tracked in FasterXML/jackson-dataformat-xml#491

It can be reproduced with mvn test -Dtest=Issue491NoArgCtorDeserRegressionTest in the jackson-dataformat-xml repo.

@dinomite
Copy link
Member

dinomite commented Oct 6, 2021

@henrik242 Thanks for following up with that—such links of information are very helpful.

@cowtowncoder
Copy link
Member

Fixed in 2.15.0:

FasterXML/jackson-dataformat-xml#547

need to change "failing failing test" now

@henrik242
Copy link
Author

henrik242 commented Jan 16, 2023

@cowtowncoder
Copy link
Member

@henrik242 I think release will be sooner than I originally planned, due to one particular CVE. But I think we are still out by at least a month since first release candidate. Still, lots of good fixes esp. for Java record types, for 2.15.

henrik242 added a commit to henrik242/jackson-integration-tests that referenced this issue Jan 17, 2023
@henrik242
Copy link
Author

@cowtowncoder No problem :) I see that the jackson-integration-tests test is still failing, by the way. Maybe I'm trying the wrong way? Here's the change: FasterXML/jackson-integration-tests@master...henrik242:jackson-integration-tests:2.15-missing-constructor-fail

(I also had to do a sbt publishM2 in jackson-module-scala to make the build run)

@cowtowncoder
Copy link
Member

@henrik242 Hmmh. Ok, I got Scala module snapshot resolved (should now push snapshots automatically with fix @pjfanning did). But I have always hard time running Kotlin-tests from Eclipse... not sure how to get that to work.

@henrik242
Copy link
Author

henrik242 commented Jan 18, 2023

@cowtowncoder Cool, I can probably make an issue of it then. Can't you just make Eclipse trigger mvn to run the tests?

@cowtowncoder
Copy link
Member

I can't get Eclipse to read Kotlin projects in a way that it recognizes Kotlin classes, unfortunately.
Running test might work somehow but wouldn't help a lot. Well, I guess it would at least show the failure I guess.

@henrik242
Copy link
Author

henrik242 commented Jan 19, 2023

Weird. It works out of the box in IntelliJ IDEA. Haven't used Eclipse in ages, so I can't help there :/

EDIT: I tried the latest Eclipse IDE, but couldn't get it to work. The kotlin plugin had problems that I was unable to resolve, and the scala plugin isn't supported anymore.

@cowtowncoder
Copy link
Member

cowtowncoder commented Jan 19, 2023

Understood. I can try with Idea, I have a free copy I think.
The only issue I've had where Eclipse is better is ability to work with multiple modules (like breakpoints in jackson-databind as well as Kotlin module); never figured out how that'd work with Idea. Not a problem here tho.

Then again even if I can reproduce the failure not sure I can do much to fix it, but just to acknowledge. I guess I could at least re-open this issue & remove "fixed" entry from release notes.
I suspect this has to do with Kotlin doing its own Creator discovery.

@cowtowncoder
Copy link
Member

Forgot to update: had absolutely no problem running Kotlin test from Idea.

I added notes on integration test over at:

FasterXML/jackson-integration-tests#14

but basically I think things actually work as expected from Jackson perspective (no more exception, outcomes as expected I think); so what need is:

  1. To know what is the desired behavior for specific use case; and from that
  2. Ensure that annotations -- if necessary -- are added, mostly wrt @JsonCreator(mode = ), to make sure desired outcome

@henrik242 Let me know what you think.

@cowtowncoder
Copy link
Member

Sounds like things are now resolved, as per testing, verification. So wrt 2.15.0 (not yet released) this should work.

@henrik242
Copy link
Author

Related problem: #721

Fails as of 2.12.3 and later (including 2.16.0-rc1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
xml Related to XML handling
Projects
None yet
Development

No branches or pull requests

8 participants