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
561 stars 221 forks source link

First element in unwrapped XML array is ignored during deserialization to base class #567

Open tetradon opened 1 year ago

tetradon commented 1 year ago

During Jackson migration, I discovered an issue that looks like a regression in 2.11.1. In this version, the first element in XML array is ignored when I deserialize to the base class.

@JsonSubTypes({
        @JsonSubTypes.Type(value = Wrapper.class, name = "wrapper")
})
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public class Base {
}
@JacksonXmlRootElement(localName = "wrapper")
public class Wrapper extends Base {

    @JacksonXmlProperty(localName = "item")
    @JacksonXmlElementWrapper(useWrapping = false)
    private List<Item> items = new ArrayList<>();

    public Wrapper(List<Item> items) {
        this.items = items;
    }

    public Wrapper() {
    }

    public List<Item> getItems() {
        return items;
    }

    public void setItems(List<Item> items) {
        this.items = items;
    }

    @Override
    public String toString() {
        return "Wrapper{" +
                "items=" + items +
                '}';
    }
}
@JacksonXmlRootElement(localName = "item")
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonIgnoreProperties(ignoreUnknown = true)
public class Item {

    private String id;

    public Item(String id) {
        this.id = id;
    }

    public Item() {
    }

    public String getId() {
        return id;
    }

    public void setId(String id) {
        this.id = id;
    }

    @Override
    public String toString() {
        return "Item{" +
                "id='" + id + '\'' +
                '}';
    }
}
public class Main {
    public static void main(String[] args) throws JsonProcessingException {
        String xmlString = """
                <?xml version="1.0" encoding="UTF-8"?>
                <wrapper type="wrapper">
                    <item><id>1</id></item>
                    <item><id>2</id></item>
                    <item><id>3</id></item>
                </wrapper>
                """;
        Base base = new XmlMapper().readValue(xmlString, Base.class);
        System.out.println(base);
    }
}

Output:

Version 2.11.0 image

Version 2.11.1+ image

Workarounds:

  1. Deserizalize to the concrete Wrapper class instead of Base: new XmlMapper().readValue(xmlString, Wrapper.class);
  2. Add following setter for the Wrapper class
    
    @JsonSetter
    public void setItems(Item item) {
        this.items.add(item);
    }

Unofortunately workarounds doesn't suit my needs as this is library code base that is used in many other projects :(

Link to the code in GitHub: 
https://github.com/tetradon/jackson-xml-deserialization-issue
cowtowncoder commented 1 year ago

Version 2.11.1 is very old so I would strongly recommend upgrading to a later version, like 2.14.1. There is a chance the issue may have been resolved as there have been many fixes between these minor versions. (although to be honest polymorphic deserialization is tricky with XML using Jackson so there may well be edge cases)

tetradon commented 1 year ago

Version 2.11.1 is very old so I would strongly recommend upgrading to a later version, like 2.14.1. There is a chance the issue may have been resolved as there have been many fixes between these minor versions. (although to be honest polymorphic deserialization is tricky with XML using Jackson so there may well be edge cases)

Yeah, the goal is to migrate to the latest, my point is that the issue was introduced in 2.11.1 and actual for all versions after it, including the latest 2.14.1

cowtowncoder commented 1 year ago

Ok, as long as it still fail on 2.14.1 that's all I needed; I wasn't sure from the description.

Other than that couple of suggestions:

tetradon commented 1 year ago

I've removed @JsonIgnoreProperties(ignoreUnknown = true) and added unit tests to the project with the original case and roundtrip, both are failing

https://github.com/tetradon/jackson-xml-deserialization-issue/blob/main/src/test/java/JacksonXmlTest.java

cowtowncoder commented 1 year ago

Ok. So the issue is likely combination of problems due to use of polymorphism (for both wrapper and Lists elements), unwrapped Lists and buffering: legal combination of things, but unfortunately something that is known to be problematic (I think #525 may be due to same root cause -- although there is no reordering needed here; possibly #426 too).

I don't know how this could have worked in earlier versions, nor exactly what is going on here. But at least there is a test that I will try to add under failing tests of this module.

cowtowncoder commented 1 year ago

Ok, I added a failing test (reproduction) in project so at least there's slightly less set up work for investigation.

But I also realized that while I suspect buffering (and inability for token stream to handle "unwrapped" case when reading from buffer) is related, this does look bit different from other failing cases, as there's "first one missing" symptom. That is probably related to handling of unwrapped-aspect (sequence of items is not contained in one scope but two for some reason).