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

XmlMapper 2.12 regression: no default no-arg ctor found #491

Closed
vy opened this issue Aug 26, 2021 · 18 comments
Closed

XmlMapper 2.12 regression: no default no-arg ctor found #491

vy opened this issue Aug 26, 2021 · 18 comments
Milestone

Comments

@vy
Copy link
Contributor

vy commented Aug 26, 2021

As described first in FasterXML/jackson-module-kotlin#396, 2.12.0 has introduced a regression to the XmlMapper. I have managed to reproduce the regression in the below Java snippet, which works on 2.11.4, but fails on 2.12.{0..4}:

package com.vlkan.jackson;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonRootName;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.nio.charset.StandardCharsets;

import static org.junit.jupiter.api.Assertions.assertEquals;

class MixinTest {

    interface Problem {

        String DEFAULT_TYPE = "about:blank";

        int DEFAULT_STATUS = 500;

        String getType();

        int getStatus();

    }

    static class DefaultProblem implements Problem {

        private final String type;

        private final int status;

        /**
         * This is required to workaround Jackson's missing support for static
         * {@link JsonCreator}s in mix-ins. That is, we need to define the
         * creator on a constructor in the mix-in that is matching with a
         * constructor here too.
         *
         * @see <a href="https://github.com/FasterXML/jackson-databind/issues/1820">jackson-databind issue 1820</a>
         */
        DefaultProblem(String type, Integer status) {
            this.type = type != null ? type : Problem.DEFAULT_TYPE;
            this.status = status != null ? status : Problem.DEFAULT_STATUS;
        }

        @Override
        public String getType() {
            return type;
        }

        @Override
        public int getStatus() {
            return status;
        }

    }

    @JsonTypeInfo(
            use = JsonTypeInfo.Id.NAME,
            include = JsonTypeInfo.As.EXISTING_PROPERTY,
            property = "type",
            defaultImpl = DefaultProblem.class,
            visible = true)
    @JsonInclude(JsonInclude.Include.NON_EMPTY)
    @JsonRootName("problem")
    interface ProblemMixIn extends Problem {

        @Override
        @JsonProperty("type")
        String getType();

        @Override
        @JsonProperty("status")
        int getStatus();

    }

    abstract static class DefaultProblemMixIn extends DefaultProblem {

        @JsonCreator
        DefaultProblemMixIn(
                @JsonProperty("type") String type,
                @JsonProperty("status") Integer status) {
            super(type, status);
            throw new IllegalStateException(
                    "mix-in constructor is there only for extracting the JSON mapping, " +
                            "it should not have been called");
        }

    }

    static class ProblemModule extends SimpleModule {

        @Override
        public void setupModule(SetupContext context) {
            super.setupModule(context);
            registerMixIns(context);
        }

        private static void registerMixIns(SetupContext context) {
            context.setMixInAnnotations(DefaultProblem.class, DefaultProblemMixIn.class);
            context.setMixInAnnotations(Problem.class, ProblemMixIn.class);
        }

    }

    private static final ProblemModule MODULE = new ProblemModule();

    private static final ObjectMapper JSON_MAPPER = new ObjectMapper().registerModule(MODULE);

    private static final XmlMapper XML_MAPPER = (XmlMapper) new XmlMapper().registerModule(MODULE);

    @Test
    void test_empty_Problem_JSON_deserialization() throws IOException {
        byte[] problemJsonBytes = "{}".getBytes(StandardCharsets.UTF_8);
        Problem problem = JSON_MAPPER.readValue(problemJsonBytes, Problem.class);
        assertEquals(Problem.DEFAULT_TYPE, problem.getType());
        assertEquals(Problem.DEFAULT_STATUS, problem.getStatus());
    }

    @Test
    void test_empty_Problem_XML_deserialization() throws IOException {
        byte[] problemXmlBytes = "<problem/>".getBytes(StandardCharsets.UTF_8);
        Problem problem = XML_MAPPER.readValue(problemXmlBytes, Problem.class);
        assertEquals(Problem.DEFAULT_TYPE, problem.getType());
        assertEquals(Problem.DEFAULT_STATUS, problem.getStatus());
    }

}

Both tests pass on 2.11.4, whereas, on 2.12.{0..4}, test_empty_Problem_JSON_deserialization() passes and
test_empty_Problem_XML_deserialization() fails with the following message:

com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `com.vlkan.jackson.MixinTest$DefaultProblem` (although at least one Creator exists): no default no-arguments constructor found
 at [Source: (byte[])"<problem/>"; line: 1, column: 1]

	at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
	at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1588)
	at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1213)
	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:1042)
	at com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromEmptyString(StdDeserializer.java:322)
	at com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromString(StdDeserializer.java:270)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromString(BeanDeserializerBase.java:1495)
	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.jsontype.impl.AsPropertyTypeDeserializer._deserializeTypedUsingDefaultImpl(AsPropertyTypeDeserializer.java:194)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer.deserializeTypedFromObject(AsPropertyTypeDeserializer.java:96)
	at com.fasterxml.jackson.databind.deser.AbstractDeserializer.deserializeWithType(AbstractDeserializer.java:263)
	at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
	at com.fasterxml.jackson.dataformat.xml.deser.XmlDeserializationContext.readRootValue(XmlDeserializationContext.java:91)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4593)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3609)
	at com.vlkan.jackson.MixinTest.test_empty_Problem_XML_deserialization(MixinTest.java:129)
        ...

In order to make test_empty_Problem_XML_deserialization() pass on 2.12.{0..4}, one needs to add a no-arg ctor to DefaultProblem.

@cowtowncoder
Copy link
Member

cowtowncoder commented Aug 26, 2021

Do you think this might be due to:

FasterXML/jackson-databind#3220

which I am planning on releasing as part of 2.12.5? I am trying to figure out when to release that patch as there are only 2 fixes all in all -- but this fix is sort of critical.

Also: is this separate from JSON handling?

@vy
Copy link
Contributor Author

vy commented Aug 27, 2021

I am able to reproduce the failure with the following dependencies:

  • jackson-annotations 2.13.0-rc1
  • jackson-databind 2.13.0-SNAPSHOT (locally built from the 2.13 branch of the repo)
  • jackson-dataformat-xml 2.13.0-rc1

Is this separate from JSON handling?

Yes, ObjectMapper isn't affected from this regression. It is only XmlMapper misbehaving.

@cowtowncoder
Copy link
Member

Given the snapshot, I assume 2.13.0-rc2 has the same behavior.

But how about 2.12.5 that was just released last night?

Also: is it possible to reproduce this without mix-ins?

@vy
Copy link
Contributor Author

vy commented Aug 30, 2021

  • I can reproduce the failure in 2.12.5.
  • I don't know of a way to reproduce this without the mix-ins, sorry.

vy added a commit to vy/jackson-dataformat-xml that referenced this issue Aug 31, 2021
cowtowncoder pushed a commit that referenced this issue Sep 1, 2021
… (#492)

Co-authored-by: Volkan Yazıcı <volkan@yazi.ci>
@cowtowncoder
Copy link
Member

Thank you for providing the test: I will try to get to this as soon as possible; hectic week at work but this is on my radar.

@cowtowncoder
Copy link
Member

Sigh. Things get rather complicated, with root-level polymorphic value. Will try to see what gives; looks like the default empty String ValueInstantiator is involved here. Will try to see if I can tease out mix-in aspect.

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 4, 2021

Ok, yes, mix-ins are not related here. Some notes:

  1. The issue is related to empty (root) element; taken to mean "Empty String" as content -- passing, <problem><status>500</status></problem>, for example, would work
  2. Probably related to improvements aimed at making it possible to read such empty elements as "empty" POJOs. But the issue here is either use of non-default constructor (which ought to be fine of course), or, perhaps, polymorphic handling.
  3. Work-around on short term would be adding the default constructor (need not be public) -- that will get called ok, I tested it.

So while I will try to address this, I would suggest adding such constructor for this specific case, to work-around the problem.

@cowtowncoder cowtowncoder added 2.14 and removed 2.13 labels Sep 4, 2021
@cowtowncoder
Copy link
Member

Ok. I added some notes on the failing test. This is not an easy problem to solve; basically there seems to be 2 ways to work around it. Either:

  1. Change interpretation of <root></root> (empty element) to produce START_OBJECT / END_OBJECT pair (instead of VALUE_STRING). This would likely cause other regression, but is a possibility.
  2. Change StdDeserializer handling of "empty value" to allow use of Properties-based Creator, passing "all absent" values. Actually code goes to BeanDeserializerBase.getEmptyValue(), which is where this would need to be done.

Of these (2) would work better and would probably be the way to go. I don't have time to pursue this quite now, but wanted to add a note if anyone else might want to try it.

Other than this, adding the no-arguments constructor also works around the issue.

@henrik242
Copy link

mvn test -Dtest=Issue491NoArgCtorDeserRegressionTest still fails in 2.13.1.

@cowtowncoder
Copy link
Member

@henrik242 Yes. Why wouldn't it? This is an open issue, with failing test.

@henrik242
Copy link

@cowtowncoder Do you know if there are plans on fixing this issue? We're still on 2.11.4 :/

@cowtowncoder
Copy link
Member

@henrik242 I haven't had any time to work on this. But there is a reproduction (failing unit test), and it looks like this might have same root cause as #538; so I understand the issue but have no good plan for solving it.

@cowtowncoder cowtowncoder added this to the 2.14.0 milestone Sep 7, 2022
@cowtowncoder cowtowncoder changed the title 2.12.0 XmlMapper regression: no default no-arg ctor found XmlMapper 2.12 regression: no default no-arg ctor found Sep 7, 2022
@cowtowncoder
Copy link
Member

@henrik242 Oh wow. We got lucky -- I was able to figure out a simpleish way to fix this, in a way reverting handling of root-element to pre-2.12 state now that 2.13 has other features allow coercion. I hope there aren't other regressions for things without test coverage, but this particular issue will be fixed in 2.14.0.
Hoping to get 2.14.0-rc1 out within a week.

@henrik242
Copy link

Amazing! Thanks for the effort!

@cowtowncoder
Copy link
Member

You are welcome! Thank you for the ping, was well timed :)

henrik242 added a commit to henrik242/jackson-xml-problem that referenced this issue Sep 26, 2022
@henrik242
Copy link

@cowtowncoder I've found an additional issue where this fails. Check should_parse_empty_tag_without_default_constructor() in https://github.com/henrik242/jackson-xml-problem/blob/no-string-argument/src/test/java/jackson/xml/NoStringArgumentTest.java#L37

This runs fine in 2.11.3, but fails in 2.14.0-rc1

@cowtowncoder
Copy link
Member

@henrik242 Could you please file a new separate issue? I typically do not re-open issues if there's a released version marked with a fix.

@henrik242
Copy link

henrik242 commented Sep 27, 2022

@cowtowncoder Sure, posted in #547

EDIT: By the way, I just realized that this is the exact same issue that I reported in FasterXML/jackson-module-kotlin#396 two years ago. This is still covered by https://github.com/FasterXML/jackson-integration-tests/blob/master/src/test/kotlin/com/fasterxml/jackson/failing/Jackson212MissingConstructorTest.kt

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

3 participants