FasterXML / jackson-dataformat-xml

Extension for Jackson JSON processor that adds support for serializing POJOs as XML (and deserializing from XML) as an alternative to JSON
Apache License 2.0
573 stars 222 forks source link

XML wrapper doesn't work with java records #517

Open Antolius opened 2 years ago

Antolius commented 2 years ago

Using @JacksonXmlElementWrapper and @JacksonXmlProperty annotations with Java records results in deserialization exception.

For example, I'd expect following XML:

<Request>
  <messages>
    <message>
      <text>given text</text>
    </message>
  </messages>
</Request>

to deserialize into following java records:

public record Request(
        @JacksonXmlElementWrapper(localName = "messages")
        @JacksonXmlProperty(localName = "message")
        List<Message> messages
) {
    public Request {
    }

    private Request() {
        this(null);
    }
}

public record Message(String text) {
    public Message {
    }

    private  Message() {
        this(null);
    }
}

However, it results in an exception:

com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid definition for property 'messages' (of type `Request`): Could not find creator property with name 'messages' (known Creator properties: [message])
 at [Source: (StringReader); line: 1, column: 10]

This is unexpected because deserialization works with equivalent java classes.

This happens with Java 17 and Jackson version 2.13.2, which is the latest one as of this writing.

I've put together a minimal project which showcases the issue: jackson-record-showcase.

magott commented 1 year ago

The same seem to go for @JacksonXmlText

pjfanning commented 1 year ago

Try v2.15.0-rc3. This version has better Record support.

magott commented 1 year ago

@pjfanning did not help for @JacksonXmlElementWrapper nor @JacksonXmlText. When used on kotlin data classes at least. If it works for Records, then maybe #153 needs to be re-opened?

cowtowncoder commented 1 year ago

@magott Kotlin Data classes are different from Records (wrt implementations); Kotlin module applies tons of magic unfortunately (some of it necessary). So if there's issue with that it may need to be opened.

One practical challenge here is tho that XML module tests cannot depend on Kotlin nor vice versa.

Refers to issue #153 is probably wrong; maybe that's for Kotlin repo issue?

magott commented 1 year ago

@cowtowncoder yes, my bad. It was https://github.com/FasterXML/jackson-module-kotlin/issues/153 If the fix for java and kotlin is different from each other. That will need to be reopened?

cowtowncoder commented 1 year ago

@magott Added notE on Kotlin module issue. As per my note there, reproduction needs to be added to jackson-integration-tests repo as that can refer to both modules and is meant as a place for problems with combinations of modules.

MikePryadko commented 1 year ago

Excuse me, but WTF?! :pout: :pout: :pout: version: 2.15.0-rc3 Java 17

code:

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonRootName;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper;
import com.fasterxml.jackson.dataformat.xml.ser.ToXmlGenerator;

import java.util.List;

public class Main {

    @JsonRootName("root")
    record VerySimple (
            @JsonProperty("text") String text,
            @JsonProperty("msg") @JacksonXmlElementWrapper(localName = "msgList") List<String> msg
    ) { }

    public static void main(String[] args) throws JsonProcessingException {
        var mapper = new XmlMapper();
        mapper.enable(SerializationFeature.INDENT_OUTPUT);
        mapper.enable(ToXmlGenerator.Feature.WRITE_XML_DECLARATION);

        System.out.println(mapper.writeValueAsString(new VerySimple("text", List.of("hello","world"))));
        var xml = """
                <?xml version='1.0' encoding='UTF-8'?>
                <root>
                  <text>text</text>
                  <msgList>
                    <msg>hello</msg>
                    <msg>world</msg>
                  </msgList>
                </root>""";
        var obj = mapper.readValue(xml, VerySimple.class);
        System.out.println(obj);
    }
}

output:

<?xml version='1.0' encoding='UTF-8'?>
<root>
  <text>text</text>
  <msgList>
    <msg>hello</msg>
    <msg>world</msg>
  </msgList>
</root>

Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid definition for property 'msgList' (of type `Main$VerySimple`): Could not find creator property with name 'msgList' (known Creator properties: [text, msg])
 at [Source: (StringReader); line: 2, column: 1]
    at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:62)
    at com.fasterxml.jackson.databind.DeserializationContext.reportBadPropertyDefinition(DeserializationContext.java:1910)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.addBeanProps(BeanDeserializerFactory.java:636)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:277)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:151)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:415)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:350)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
    at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
    at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
    at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4861)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4731)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3677)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3645)
    at Main.main(Main.java:34)
cowtowncoder commented 1 year ago

@MikePryadko Uh? WTF what?

MikePryadko commented 1 year ago

@cowtowncoder seriously? Well, let's state the obvious: serialization works fine, deserialization fails. :expressionless: Do you think it's Ok?

MikePryadko commented 1 year ago

With plain class result is same

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonRootName;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper;
import com.fasterxml.jackson.dataformat.xml.ser.ToXmlGenerator;

import java.util.List;

public class Main {

    @JsonRootName("root")
    static class Test {
        @JsonProperty("text")
        public final String text;
        @JsonProperty("msg")
        @JacksonXmlElementWrapper(localName = "msgList")
        public final List<String> msg;

        public Test(@JsonProperty("text") String text, @JsonProperty("msg") @JacksonXmlElementWrapper(localName = "msgList") List<String> msg) {
            this.text = text;
            this.msg = msg;
        }
    }

    public static void main(String[] args) throws JsonProcessingException {
        var mapper = new XmlMapper();
        mapper.enable(SerializationFeature.INDENT_OUTPUT);
        mapper.enable(ToXmlGenerator.Feature.WRITE_XML_DECLARATION);

        System.out.println(mapper.writeValueAsString(new Test("text", List.of("hello","world"))));
        var xml = """
                <?xml version='1.0' encoding='UTF-8'?>
                <root>
                  <text>text</text>
                  <msgList>
                    <msg>hello</msg>
                    <msg>world</msg>
                  </msgList>
                </root>""";
        var obj = mapper.readValue(xml, Test.class);
        System.out.println(obj);
    }
}
<?xml version='1.0' encoding='UTF-8'?>
<root>
  <text>text</text>
  <msgList>
    <msg>hello</msg>
    <msg>world</msg>
  </msgList>
</root>

Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Invalid definition for property 'msgList' (of type `Main$Test`): Could not find creator property with name 'msgList' (known Creator properties: [text, msg])
 at [Source: (StringReader); line: 2, column: 1]
    at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:62)
    at com.fasterxml.jackson.databind.DeserializationContext.reportBadPropertyDefinition(DeserializationContext.java:1910)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.addBeanProps(BeanDeserializerFactory.java:636)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:277)
    at com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:151)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:415)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:350)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:264)
    at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
    at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
    at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
    at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4861)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4731)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3677)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3645)
    at Main.main(Main.java:42)
MikePryadko commented 1 year ago

I found: deserialization crushes when field name in @JacksonXmlElementWrapper and @JsonProperty are different :neutral_face:

Works fine:

record VerySimple (
            @JsonProperty("text") String text,
            @JsonProperty("msg") @JacksonXmlElementWrapper(localName = "msg") List<String> msg
    ) { }

Fails:

record VerySimple (
            @JsonProperty("text") String text,
            @JsonProperty("msg") @JacksonXmlElementWrapper(localName = "msgList") List<String> msg
    ) { }

Can anyone fix this please..?

cowtowncoder commented 1 year ago

@cowtowncoder seriously? Well, let's state the obvious: serialization works fine, deserialization fails. 😑 Do you think it's Ok?

I don't think I have indicated that is good: of course we'd rather things worked.

So it sounds like WTF here was meant as `+1 to fix this' or something. I was just surprised by indications of anger over a bug report, comments regarding related (but different) bug reports for Kotlin module.

MikePryadko commented 1 year ago

@cowtowncoder sorry for aggression - it was my sincere reaction: it's 2023 outside, and annotation @JacksonXmlElementWrapper exists for 12 years, but proper deserialization still not works... 😐 why?

Or this situation is a bug and previous versions worked fine?

cowtowncoder commented 1 year ago

@MikePryadko @JacksonXmlElementWrapper works fine for POJOs; although one thing worth noting is that Jackson is not -- and never planned to be, nor will be -- actual JAXB implementation. Support for JAXB annotations is for interoperability, best effort. What is supported are Jackson's annotations; a subset of JAXB annotations work.

But there seem to be issues specific to use with Java Records and Kotlin Data classes. They have their own peculiarities wrt handling.

I don't think handling has worked better before. You can consider issues an incomplete support for specific combination of things.

And all help in actually solving the problem is welcome. But less so for commentary on "why isn't it STILL FIXED IN 2023". It could be 2048 and if no one has figured out how to make things work it wouldn't work. OSS projects do not really work well with Shaming as incentive.

Walnussbaer commented 1 month ago

I just encountered the same issue when using a custom constructor for a simple POJO (https://stackoverflow.com/questions/79055046/deserialize-list-of-objects-from-xml-to-java-pojos-using-jackson/79061736#79061736)

I didn't want a default constructor to be present, so noone can create shallow objects of my POJO classes. But setting the default constructor to protected is a workaround I can live with somehow.

Still a bit weird that a structure like this

<items>
   <item></item>
   <item></item>
</items>

is such a pain to be deserialized with Jackson + XML. Esspecially because when deserializing JSON, this kind of structure (with a wrapper element) is the default way to go and no problem at all for Jackson.

cowtowncoder commented 1 month ago

Quick note: Jackson 2.18.0 had a significant refactoring related to handling of constructors, and specifically helping with handling of Record constructors wrt annotations.

So this issue could be easier to resolve (or possible even resolved, although not super likely). If anyone has time to see if behavior differs with 2.18.0, that'd be useful.

Another thing that'd be good to do would be to figure out a way to be able to optionally build and run Record-based tests, similar to how jackson-databind does it. That way we could add reproduction, instead of manual verification. Problem being that Jackson 2.x still requires just Java 8, so by default we cannot use Record types. The way jackson-databind handles this is by using different test source directory (src/test-jdk17/java/) and Maven profile to optionally include it -- basically, including and running tests if (and only if) Maven runs on JDK 17 or above.

cowtowncoder commented 1 month ago

As per #671, we can now add XML + Record tests on this project!

JooHyukKim commented 2 weeks ago

Hello @cowtowncoder, I tried poking around the error. And I was wondering if you could give some directions on solution?

So currently, this validation in BeanDeserializerFactory comparing creatorProp name vs beanProperty name is failing.

                for (SettableBeanProperty cp : creatorProps) {
                    if (name.equals(cp.getName()) && (cp instanceof CreatorProperty)) {
                        cprop = (CreatorProperty) cp;
                        break;
                    }
                }

Do you think it's feasible to overwrite the creatorProp to match ? Or any other extension point to hook into creatorProp or something?

cowtowncoder commented 2 weeks ago

@JooHyukKim could you elaborate on how that is failing? Not matching when it should? I am not sure what kind of extension/override would be needed.

JooHyukKim commented 2 weeks ago

So specifically, creatorProp has name 'message' while beanProp has name 'messages'.

cowtowncoder commented 2 weeks ago

Ok: so handling of

        @JacksonXmlElementWrapper(localName = "messages")
        @JacksonXmlProperty(localName = "message")

has missed a renaming somewhere, probably. Place where it is encountered is not, however, where anything can be done any more (i.e. we should not try to reverse engineer what might have happened; it would be possible to also have false match, not just mismatch) -- so property name must be kept in sync at an earlier point.