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

Deserialization of class inheritance depends on attributes order #242

Closed
khovanskiy opened this issue Jun 11, 2017 · 10 comments
Closed

Deserialization of class inheritance depends on attributes order #242

khovanskiy opened this issue Jun 11, 2017 · 10 comments
Milestone

Comments

@khovanskiy
Copy link
Contributor

When I try to deserialize this xml, XmlMapper creates empty list of action-elements:

<widget id="4" type="Transition">
    <attributes>
        <event/>
        <action name="a"/>
        <action name="b"/>
        <code/>
        <guard/>
    </attributes>
</widget>

But if I put the type attribute on the first position, the list contains 2 elements.

Parent class Widget:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type", defaultImpl = TransitionWidget.class)
@JsonSubTypes({
        @JsonSubTypes.Type(value = StateWidget.class, name = "State"),
        @JsonSubTypes.Type(value = TransitionWidget.class, name = "Transition")
})
public abstract class Widget {
    private Integer id;

    protected Widget(Integer id) {
        this.id = id;
    }

    @JacksonXmlProperty(isAttribute = true)
    public Integer getId() {
        return id;
    }
}

Child class TransitionWidget:

@JacksonXmlRootElement(localName = "widget")
public class TransitionWidget extends Widget {
    private final TransitionAttributes attributes;

    @JsonCreator
    public TransitionWidget(@JsonProperty("id") Integer id, @JsonProperty("attributes") TransitionAttributes attributes) {
        super(id);
        this.attributes = attributes;
    }

    @JacksonXmlProperty(localName = "attributes")
    public TransitionAttributes getAttributes() {
        return attributes;
    }
}

And its inner class:

@JacksonXmlRootElement(localName = "attributes")
public class TransitionAttributes {
    private Event event;
    private List<Action> actions;
    private String code;
    private String guard;

    @JacksonXmlProperty(localName = "action")
    @JacksonXmlElementWrapper(useWrapping = false)
    public List<Action> getActions() {
        return actions;
    }
}

Method "getActions" also returns empty list, when type-attribute of widget is missing while defaultImpl is specified.

This is checked with Jackson 2.8.8 and 2.9.0.pr3.

@cowtowncoder
Copy link
Member

Given that you include most pieces, would it be possible to then just add small unit test method that verifies the problem?

@khovanskiy
Copy link
Contributor Author

khovanskiy commented Jun 11, 2017

I can simplify the example, but the structure is needed to reproduce.

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type", defaultImpl = B.class)
@JsonSubTypes({
        @JsonSubTypes.Type(value = B.class, name = "B")
})
static abstract class A {
    public Integer id;
}

static class Attr {
    @JacksonXmlElementWrapper(useWrapping = false)
    public List<Param> param;
}

static class Param {
    public String name;
}

static class B extends A {
    public Attr attr;
}

And the test:

XmlMapper MAPPER = new XmlMapper();
String content1 = "<A type=\"B\" id=\"1\"><attr><param name=\"1\"/><param name=\"2\"/></attr></A>";
B b1 = (B) MAPPER.readValue(content1, A.class);
assertEquals(2, b1.attr.param.size());
String content2 = "<A id=\"1\" type=\"B\"><attr><param name=\"1\"/><param name=\"2\"/></attr></A>";
B b2 = (B) MAPPER.readValue(content2, A.class);
assertEquals(2, b2.attr.param.size()); // throws exception

I am not familiar the open-source development process. Could I create a pull request with unsuccessful test-case?

@cowtowncoder
Copy link
Member

@khovanskiy Yes, failing test case is fine. There is package failing, under which failing tests do not block successful Maven build. This is where such tests are initially located, and then moved out to another package once issue has been fixed.

khovanskiy added a commit to khovanskiy/jackson-dataformat-xml that referenced this issue Jun 12, 2017
cowtowncoder added a commit that referenced this issue Jun 21, 2017
@jimirocks
Copy link

Two years opened, is there any plan to fix this BUG? It basically prevents usage of polymorphism when parsing XML produced by another party. Which basically prevetns usage of this library :(

@cowtowncoder
Copy link
Member

@jimirocks This is an open source projects, so contributions are welcome. I do not think anyone currently is working actively on this issue.

As to there being a plan... typical plan for things like this follows a pattern of someone:

  1. Figures out what is going on,
  2. Figures out how to fix the issue
  3. Fixes it.

At this point there is a reproduction to help with (1). It is also worth noting that there may be more than person doing the steps.

If you feel so strongly this should be fixed, perhaps you help fix the problem: or at least figure out what is happening.
This would be much more useful than indicating severity of the issue.

@cowtowncoder
Copy link
Member

Looking at the problem a bit. So, the underlying trigger is that in passing case type id comes first, and thereby polymorphic deserialization finds it and does not have to buffer anything. In second case there is another attribute (id) first, requiring buffering of at least that data. This triggers the problem: for some reason logical elements for param are seen as empty.

An immediate concern is that TokenBuffer is format-agnostic and may not be able to retain information from XML. This may or may not be relevant directly. Similarly combining of TokenBuffer with streaming source (JsonParserSequence) has same challenges.
The particular concern is that implementation of isExpectedStartArrayToken() does not (can not) trigger override handling same way as XML-backed FromXmlParser.

I'll dig a little more to see what exactly causes discrepancy, and perhaps it can be resolved.

@cowtowncoder
Copy link
Member

Some more info: isExpectedStartArrayToken() itself not problematic, but check by WrapperHandlingDeserializer is: due to buffering, parser is JsonParserDelegate, not FromXmlParser, so _configureParser() can not add virtual wrapping. This might be something I can work around... we'll see.

@cowtowncoder cowtowncoder added this to the 2.10.0 milestone Sep 6, 2019
@cowtowncoder
Copy link
Member

Figured out a way to solve the specific problem in this case.

@jimirocks
Copy link

@cowtowncoder Many thanks for the fix! Also sorry for not reacting while on vacation ...

@cowtowncoder
Copy link
Member

@jimirocks Np. Glad I went ahead and problem turned out relatively easy to fix.

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