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

Deserializing @JacksonXmlElementWrapper(useWrapping = false) fails on missing (optional) attribute #64

Closed
nezda opened this issue May 13, 2013 · 14 comments

Comments

@nezda
Copy link

nezda commented May 13, 2013

Attempting to deserialize an unwrapped collection field (@JacksonXmlElementWrapper(useWrapping = false)) fails if element is missing an attribute. For example, given:

static class Optional {
    @JacksonXmlText
    public String number = "NOT SET";
    public String type = "NOT SET";
}
static class MultiOptional {
    @JacksonXmlElementWrapper(useWrapping = false)
    public List<Optional> optional;
}

XmlMapper can handle <MultiOptional><optional type='work'>123-456-7890</optional></MultiOptional>
but not <MultiOptional><optional>123-456-7890</optional></MultiOptional> (since it is missing type property).

Here are some related additions I made to com.fasterxml.jackson.dataformat.xml.failing.TestDeserialization to refine this issue:

        private static class StateAttrLocalName
    {
        @JacksonXmlProperty(localName="state", isAttribute=true)
        public String stateAttr = "NOT SET";

                @Override public String toString() { return "StateAttrLocalName: stateAttr: " + stateAttr; }
    }

        public void testAttrLocalName() throws Exception
    {
        StateAttrLocalName ob = MAPPER.readValue("<Wrapper state='stateAttr'></Wrapper>",
                StateAttrLocalName.class);
        assertNotNull(ob);
                System.err.println("ob: " + ob);
        assertEquals("stateAttr", ob.stateAttr);
    }


        private static class StateAttr
    {
        @JacksonXmlProperty(localName="state", isAttribute=true)
        public String state = "NOT SET";

                @Override public String toString() { return "StateAttr: state: " + state; }
    }

        public void testAttr() throws Exception
    {
        StateAttr ob = MAPPER.readValue("<Wrapper state='stateAttr'></Wrapper>",
                StateAttr.class);
        assertNotNull(ob);
                System.err.println("ob: " + ob);
        assertEquals("stateAttr", ob.state);
    }

        private static class TwoStates
    {
        @JacksonXmlProperty(localName="state", isAttribute=true)
        public String stateAttr = "NOT SET";
        public String state = "NOT SET";

                @Override public String toString() { return "TwoStates: stateAttr: " + stateAttr + " state: " + state; }
    }

    public void testAttrElementConflict() throws Exception
    {
        TwoStates ob = MAPPER.readValue("<Wrapper state='stateAttr'><state>stateElement</state></Wrapper>",
                TwoStates.class);
        assertNotNull(ob);
                System.err.println("ob: " + ob);
        assertEquals("stateElement", ob.state);
        assertEquals("stateAttr", ob.stateAttr); // attribute stateAttr is ignored
    }

        private static class Optional
    {
                @JacksonXmlText
        public String number = "NOT SET";
        //@JacksonXmlProperty(isAttribute=true)
                public String type = "NOT SET";

                @Override public String toString() { return "Optional: number: " + number + " type: " + type; }
    }

    public void testOptionalAttr() throws Exception
    {
        Optional ob = MAPPER.readValue("<phone type='work'>123-456-7890</phone>",
                Optional.class);
        assertNotNull(ob);
                System.err.println("ob: " + ob);
        assertEquals("123-456-7890", ob.number);
        assertEquals("work", ob.type);
    }

    public void testMissingOptionalAttr() throws Exception
    {
        Optional ob = MAPPER.readValue("<phone>123-456-7890</phone>",
                Optional.class);
        assertNotNull(ob);
                System.err.println("ob: " + ob);
        assertEquals("123-456-7890", ob.number);
        assertEquals("NOT SET", ob.type);
    }

        private static class MultiOptional
    {
                @JacksonXmlElementWrapper(useWrapping = false)
                public List<Optional> optional;

                @Override public String toString() { return "MultiOptional: optional's: " + optional; }
    }

        public void testMultiOptional() throws Exception
    {
        MultiOptional ob = MAPPER.readValue("<MultiOptional><optional type='work'>123-456-7890</optional></MultiOptional>",
                MultiOptional.class);
        assertNotNull(ob);
                System.err.println("ob: " + ob); // works fine
//        assertEquals("123-456-7890", ob.number);
//        assertEquals("NOT SET", ob.type);
    }

        public void testMultiOptionalWithMissingType() throws Exception
    {
            MultiOptional ob = MAPPER.readValue("<MultiOptional><optional>123-456-7890</optional></MultiOptional>",
                MultiOptional.class);
        assertNotNull(ob);
                System.err.println("ob: " + ob); // fails: 
//        assertEquals("123-456-7890", ob.number);
//        assertEquals("NOT SET", ob.type);
    }
@cowtowncoder
Copy link
Member

Thanks, sounds like a bug.

@nezda
Copy link
Author

nezda commented May 14, 2013

Would be awesome to not have to use JAXB or JiBX for this project.

@cowtowncoder
Copy link
Member

Yes. I hope to find time to fix this for 2.2.2 release. Thank you for providing the test case!

@ShijunK
Copy link

ShijunK commented May 21, 2013

something may or may not related
unwrapped property's XMLStreamReader is not reset/cleanup properly before being reused for a different property

test case:

  @JacksonXmlRootElement(localName = "root")
    public static class Root {

        @JacksonXmlProperty(localName = "unwrapped")
        @JacksonXmlElementWrapper(useWrapping = false)
        private List<UnwrappedElement> unwrapped;

        @JacksonXmlProperty
        private String name;

        public static class UnwrappedElement {
            @JacksonXmlProperty(isAttribute = true)
            private String id;

            @JacksonXmlProperty(isAttribute = true)
            private String type;
        }
    }

    @Test
    public void testUnwrappedWithOptionalAttribute() throws Exception {
        Root twoAttributes = xmlMapper.readValue("<root><unwrapped id=\"1\" type=\"string\"/><unwrapped id=\"2\" type=\"string\"/><name>text</name></root>", Root.class);
        assertThat(twoAttributes.name, equalTo("test"));
    }

exception

java.lang.IllegalArgumentException: Invalid index 0; current element has only 0 attributes
    at com.ctc.wstx.sr.AttributeCollector.throwIndex(AttributeCollector.java:1018)
    at com.ctc.wstx.sr.AttributeCollector.getLocalName(AttributeCollector.java:325)
    at com.ctc.wstx.sr.BasicStreamReader.getAttributeLocalName(BasicStreamReader.java:576)
    at com.fasterxml.jackson.dataformat.xml.deser.XmlTokenStream._next(XmlTokenStream.java:306)
    at com.fasterxml.jackson.dataformat.xml.deser.XmlTokenStream.next(XmlTokenStream.java:162)
    at com.fasterxml.jackson.dataformat.xml.deser.FromXmlParser.nextToken(FromXmlParser.java:451)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:291)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:121)
    at com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer.deserialize(WrapperHandlingDeserializer.java:109)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:2888)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2034)

@nezda
Copy link
Author

nezda commented May 21, 2013

@ShijunK if you are using

<dependency>
  <groupId>org.codehaus.woodstox</groupId>
  <artifactId>woodstox-core-asl</artifactId>
  <version>4.x</version>
</dependency>

and you can remove it (i.e., use default), see if problem goes away - that solved this problem for me. I don't think that problem is related to this issue.

@ShijunK
Copy link

ShijunK commented May 21, 2013

@nezda , thanks, that's does not fix the problem. I was using 4.1.5, tried the latest 4.2.0. still get the same exception.

@nezda
Copy link
Author

nezda commented May 21, 2013

@ShijunK did you remove the dependency as I suggested or upgrade the version?

@ShijunK
Copy link

ShijunK commented May 21, 2013

@nezda thanks. I misread your comment.

after removing the woodstox dependency, com.sun's implementation works.
thank you

@cowtowncoder
Copy link
Member

No, please do NOT remove Woodstox dependency: Stax parser included in JDK (SJSXP) is known to have issues and even if it happened to work around one, you most likely hit something else. While I have submitted a dozen fixes for SJSXP, it is lagging far behind Woodstox in about all aspects, including both correctness and performance.

@ShijunK
Copy link

ShijunK commented May 21, 2013

@cowtowncoder , thanks for the advice. I did double check other test cases. and I did hit some strange exceptions after removing woodstox. For now, I will just do the dirty work to use DOM parse xml for my use case.

one strange exception after removing woodstox dependency:

public static class Node {

        @JacksonXmlProperty(localName = "unwrapped")
        @JacksonXmlElementWrapper(useWrapping = false)
        private List<UnwrappedElement> unwrapped;

        @JacksonXmlProperty
        private String name;

        @JacksonXmlProperty(localName = "node")
        @JacksonXmlElementWrapper(useWrapping = false)
        private List<Node> node;

        public static class UnwrappedElement {
            @JacksonXmlProperty(isAttribute = true)
            private String id;

            @JacksonXmlProperty(isAttribute = true)
            private String type;
        }
    }

    @Test
    public void testUnwrappedWithOptionalAttribute() throws Exception {
        xmlMapper.configure(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY, true);
        Node node = xmlMapper.readValue(
                "<node>\n" +
                        "    <unwrapped id=\"1\"/>\n" +
                        "    <unwrapped id=\"2\" type=\"string\"/>\n" +
                        "    <name>parent</name>\n" +
                        "    <node>\n" +
                        "        <unwrapped id=\"3\"></unwrapped>\n" +
                        "        <name>child</name>\n" +
                        "    </node>\n" +
                        "</node>",
                Node.class);
        assertThat(node.name, equalTo("parent"));
        assertThat(node.node.get(0).name, equalTo("child"));
    }
Caused by: java.lang.IllegalStateException: Current state not XML_START_ELEMENT (1) but 6
    at com.fasterxml.jackson.dataformat.xml.deser.XmlTokenStream.repeatStartElement(XmlTokenStream.java:228)
    at com.fasterxml.jackson.dataformat.xml.deser.FromXmlParser.addVirtualWrapping(FromXmlParser.java:280)
    at com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer._configureParser(WrapperHandlingDeserializer.java:140)
    at com.fasterxml.jackson.dataformat.xml.deser.WrapperHandlingDeserializer.deserialize(WrapperHandlingDeserializer.java:108)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.handleNonArray(CollectionDeserializer.java:270)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:217)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:207)
    at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:23)
    at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:449)
    at com.fasterxml.jackson.databind.deser.impl.FieldProperty.deserializeAndSet(FieldProperty.java:107)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:295)
    ... 32 more

@cowtowncoder
Copy link
Member

You can also try JAXB -- it is fairly solid, and much faster than playing with DOM.

@cowtowncoder
Copy link
Member

FWIW, while I am happy to accept patches to make module work better with default Stax parser from JDK, I will not be investigating problems specific to it (i.e. things that work with Woodstox). Others are free to pursue these aspects of course.

@cowtowncoder
Copy link
Member

Another note: each bug entry should be for specific problem; new ones should be filed for other problems, unless they are known to be directly related. I mention this since unit test included seems to point out different issues.
I will focus on one initially mentioned; problems with missing attributes, on (un)wrapper lists.

@cowtowncoder
Copy link
Member

I believe this is finally properly fixed. The problem turned out to be related to "auto-detection" of leaf-level String values: in case where this may occur, need to add explicit support for serializing POJO from a String value.

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