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

(close by end of Nov 2016) XmlElementRef behavior is not correct #136

Closed Poorman65 closed 7 years ago

Poorman65 commented 9 years ago

I have a situation where I need to output subtypes. My real world scenario requires them to be in a List, but it also does not appear to work even with a simple derived class as a property. In the example below I first output a list of Root subclasses and then an individual Root subclass. This sample works fine with JAXB directly.

Here is the Root class:

@XmlAccessorType(XmlAccessType.FIELD)
@XmlRootElement(name="theRoot")
public class Root {
    private String rootVar = "Root var data";

    public String getRootVar() {
        return rootVar;
    }

    public void setRootVar(String rootVar) {
        this.rootVar = rootVar;
    }
}

Here is one of the Derived classes:

@XmlAccessorType(XmlAccessType.FIELD)
@XmlRootElement(name="theDerived1")
public class Derived1 extends Root {
    private String derivedVar = "Derived value in 1";

    public String getDerivedVar() {
        return derivedVar;
    }

    public void setDerivedVar(String derivedVar) {
        this.derivedVar = derivedVar;
    }
}

Here is the Thing class that uses the Root class:

@XmlAccessorType(XmlAccessType.FIELD)
@XmlRootElement(name="theThing")
public class Thing {
    @XmlElementWrapper(name="listOfRootsWrapper")
    @XmlAnyElement
    @XmlElementRefs
    ( 
      { 
        @XmlElementRef(type=Derived1.class, name="derived1"), 
        @XmlElementRef(type=Derived2.class, name="derived2")
      } 
    ) 
    private List<Root> listOfRoots = new ArrayList<Root>();

    @XmlElementRef
    private Root derived;

    public void addToListOfRoots(Root root) {
        this.listOfRoots.add(root);
        this.derived = root;
    }

   // Getters and Setters
}

Here is the output using JAXB:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<theThing>
    <listOfRootsWrapper>
        <theDerived1>
            <rootVar>Root var data</rootVar>
            <derivedVar>Derived value in 1</derivedVar>
        </theDerived1>
        <theDerived2>
            <rootVar>Root var data</rootVar>
            <derivedVar>Derived value in 2</derivedVar>
        </theDerived2>
    </listOfRootsWrapper>
    <theDerived2>
        <rootVar>Root var data</rootVar>
        <derivedVar>Derived value in 2</derivedVar>
    </theDerived2>
</theThing>

Here is the output using Jackson 2.4.4 (Note the "listOfRoots" wrapping each entry in the list and "theRoot" instead of the actual derived class:

<?xml version='1.1' encoding='UTF-8'?><theThing><listOfRootsWrapper>
    <listOfRoots>
      <derived1>
        <rootVar>Root var data</rootVar>
        <derivedVar>Derived value in 1</derivedVar>
      </derived1>
    </listOfRoots>
    <listOfRoots>
      <derived2>
        <rootVar>Root var data</rootVar>
        <derivedVar>Derived value in 2</derivedVar>
      </derived2>
    </listOfRoots>
  </listOfRootsWrapper>
  <theRoot>
    <rootVar>Root var data</rootVar>
    <derivedVar>Derived value in 2</derivedVar>
  </theRoot>
</theThing>
Poorman65 commented 9 years ago

I have also tried to write a custom serializer to get past this but I get an ArrayIndexOutOfBounds when pretty printing is turned on. It works fine without pretty printing but I have to have formatted output.

Here is my serializer:

public class JaxbElementSerializer extends JsonSerializer<Thing> {
    @Override
    public void serialize(Thing thing, JsonGenerator jgen,
            SerializerProvider provider) throws IOException,
            JsonProcessingException {
        jgen.writeStartObject();
        jgen.writeObjectFieldStart("myWrapper");
        JAXBElement jaxbElement;

        for (Object object : thing.getListOfRoots()) {
            if (object.getClass().isAssignableFrom(JAXBElement.class)) {
                jaxbElement = (JAXBElement) object;
                jgen.writeStringField(jaxbElement.getName().toString(),
                        jaxbElement.getValue().toString());
            } else {
                jgen.writeObjectField(object.getClass().getSimpleName(), object);
            }
        }
        jgen.writeEndObject();
        jgen.writeEndObject();
    }
}
cowtowncoder commented 9 years ago

Ok. The extra listOfRoots entry is because Jackson defaults to "wrapped" style of serializing lists, whereas JAXB defaults to "unwrapped". So although Jackson can recognize JAXB annotations, its defaults differ here (due to historical reasons; unwrapped inclusion was not supported before 2.3 or so). You can change the inclusion either by using annotation

@JacksonXmlElementWrapper(useWrapping = false)

for the property, or globally by configuring XmlModule (which is often implicitly created unless settings need to be changed):

JacksonXmlModule module = new JacksonXmlModule();
module.setDefaultUseWrapper(false);
XmlMapper mapper = new XmlMapper(module);
Poorman65 commented 9 years ago

Isn't @JacksonXmlElementWrapper analogous to @XmlElementWrapper(name="listOfRootsWrapper")?

I use that because I want the containing wrapper around the entire list. It's the wrapper around the items in the list that I am trying to get rid of.

If I remove the @XmlElementWrapper and use @JacksonXmlElementWrapper, then I am able to control the outer wrapper. It has no impact on the wrapper (listOfRoots) on the individual elements. Also, I need to have the JAXB annotations so that I can generate a schema. It's fine if I have Jackson annotations as well though.

In my example the outer wrapper is "listOfRootsWrapper" as shown here:

===>Want these<===<listOfRootsWrapper>
===>Don't want these<===<listOfRoots>
      <derived1>
        <rootVar>Root var data</rootVar>
        <derivedVar>Derived value in 1</derivedVar>
      </derived1>
===>Don't want these<===</listOfRoots>
===>Don't want these<===<listOfRoots>
      <derived2>
        <rootVar>Root var data</rootVar>
        <derivedVar>Derived value in 2</derivedVar>
      </derived2>
===>Don't want these<===</listOfRoots>
===>Want these<===</listOfRootsWrapper>

If I get rid of the JAXB annotations and just use the @JacksonXmlElementWrapper then I get the following (No outer wrapper but still has individual item wrappers):

<?xml version='1.1' encoding='UTF-8'?><theThing>
  <listOfRoots>
    <rootVar>Root var data</rootVar>
    <derivedVar>Derived value in 1</derivedVar>
  </listOfRoots>
  <listOfRoots>
    <rootVar>Root var data</rootVar>
    <derivedVar>Derived value in 2</derivedVar>
  </listOfRoots>
  <theRoot>
    <rootVar>Root var data</rootVar>
    <derivedVar>Derived value in 2</derivedVar>
  </theRoot>
</theThing>
cowtowncoder commented 9 years ago

Actually you are absolutely right regarding wrapper, and I got confused myself. And behavior does sound like a bug.

One thing you could test just to eliminate problems is to use Jackson annotation that would otherwise used for polymorphic types: @JsonTypeInfo, with inclusion type of As.WRAPPER_OBJECT. That should result in similar problem: if it does, then problem is not with JAXB annotations but XML handling. If output differs, it would be more likely a problem with JAXB annotation support.

Poorman65 commented 9 years ago

Same problem using @JsonTypeInfo!

Can you help me with a workaround? I have a deliverable today.

If you look at my comment that I posted right after the issue above, I was trying to roll my own serializer. It works as expected if I turn off pretty printing, but I get an ArrayIndexOutOfBounds with pretty printing on. The formatting is not correct and when I debug through the code it ends up with a -1 for _nesting in DefaultXmlPrettyPrinter.writeEndObject.

By the way, this only seems to throw the exception if I use writeObjectField. I used writeStringField as a test and the exception didn't occur.

Poorman65 commented 9 years ago

I think that the main problem is in ObjectMapper. The following code changes the pretty printer to use the default and is replacing the one that is already active. I think it should be checking to see if there is already a pretty printer before setting the default.

I can write up another issue for this but I desperately need to get a workaround so that I can get my code checked in today.

public void writeValue(JsonGenerator jgen, Object value)
    ...
    if (config.isEnabled(SerializationFeature.INDENT_OUTPUT)) {
        jgen.useDefaultPrettyPrinter();
    }
Poorman65 commented 9 years ago

I'm able to get this to work properly by overriding XmlMapper and overriding the writeValue method. I think this same bug may be repeated in quite a few places. I only changed the one I was hitting by I see the same thing being done more times in the ObjectMapper. I can't override those because they are final.

The useDefaultPrettyPrinter() method is a bit of a misnomer. It doesn't simply use the default pretty printer, it creates a new instance.

    if (jgen.getPrettyPrinter() == null && config.isEnabled(SerializationFeature.INDENT_OUTPUT)) {
        jgen.useDefaultPrettyPrinter();
    }

This may be related to another bug I reported: "Missing newline after XML header #131" This exact fix doesn't correct it, but it may be caused by one of the other locations where the same code is used.

cowtowncoder commented 9 years ago

Interesting. I am not sure this is related to newline part, since that does require different handling (XML generators consider whitespace outside root element to be somewhat different thing than whitespace within actual document). But definitely sounds like use of indenter may be fixed; I just need to create a unit test to reproduce that issue, and verify whether it is also occurring with 2.5.

cowtowncoder commented 9 years ago

Actually I think the problem is with override by ToXmlGenerator. Whereas GeneratorBase has this:

    @Override public JsonGenerator useDefaultPrettyPrinter() {
        /* 28-Sep-2012, tatu: As per [Issue#84], should not override a
         *  pretty printer if one already assigned.
         */
        if (getPrettyPrinter() != null) {
            return this;
        }
        return setPrettyPrinter(new DefaultPrettyPrinter());
    }

its override does not have similar handling. So I can add that there.

Poorman65 commented 9 years ago

That looks like a good fix.

viftodi commented 8 years ago

Has this issue died? It seems there is a fix, will it ever be merged?

cowtowncoder commented 7 years ago

@viftodi At this point there are too many conflicting statements here for me to know exactly what problem is being discussed; and no unit test to show the problem.

So what I think is needed is the reproduction of remaining problem(s), if any; and ideally I think filing a new issue to make it easy to see what has been fixed and what hasn't.

Fix I mentioned earlier was included and so pretty-printing part should work and has been included earlier (in 2.6 at least I think).

I'll close this issue soon, but leave open for a bit for visibility.