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

Empty (or self-closing) Element representing List is incorrectly deserialized as null, not Empty List #252

Closed
sir4ur0n opened this issue Jul 23, 2017 · 11 comments
Milestone

Comments

@sir4ur0n
Copy link

sir4ur0n commented Jul 23, 2017

Following discussion between @allienna and @cowtowncoder on #1402 I'm creating an issue.

Hi guys,

While using Jackson with JAXB bindings, I found deserialization behaves unexpectedly on empty or self-closing elements: it sets sub-fields as null no matter what.

Here's a class to reproduce the issue (sorry if it's big):

import static org.hamcrest.core.Is.is;
import static org.hamcrest.core.IsNull.notNullValue;
import static org.junit.Assert.assertThat;

import com.fasterxml.jackson.annotation.JsonSetter;
import com.fasterxml.jackson.annotation.Nulls;
import com.fasterxml.jackson.databind.type.TypeFactory;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import com.fasterxml.jackson.module.jaxb.JaxbAnnotationIntrospector;
import java.util.ArrayList;
import java.util.List;
import javax.xml.bind.annotation.XmlAccessType;
import javax.xml.bind.annotation.XmlAccessorType;
import javax.xml.bind.annotation.XmlElement;
import javax.xml.bind.annotation.XmlRootElement;
import org.junit.Before;
import org.junit.Test;

public class Main {

  private XmlMapper mapper;

  @Before
  public void setUp() throws Exception {
    mapper = new XmlMapper();
    mapper.setDefaultSetterInfo(JsonSetter.Value.forContentNulls(Nulls.SKIP));
    mapper.setAnnotationIntrospector(new JaxbAnnotationIntrospector(TypeFactory.defaultInstance()));
  }

  @Test
  public void testWithValues() throws Exception {
    String toDeserialize = "<pojos><pojo>a</pojo><pojo>b</pojo><pojo>c</pojo></pojos>";

    Pojos pojos = mapper.readValue(toDeserialize, Pojos.class);
    assertThat(pojos, notNullValue());
    assertThat(pojos.getValues(), notNullValue());
    assertThat(pojos.getValues().size(), is(3));
  }

  @Test
  public void testWithoutValues() throws Exception {
    String toDeserialize = "<pojos></pojos>";

    Pojos pojos = mapper.readValue(toDeserialize, Pojos.class);
    assertThat(pojos, notNullValue());
    assertThat(pojos.getValues(), notNullValue());
    assertThat(pojos.getValues().size(), is(0));
  }

  @Test
  public void testSelfClosingWithoutValues() throws Exception {
    String toDeserialize = "<pojos/>";

    Pojos pojos = mapper.readValue(toDeserialize, Pojos.class);
    assertThat(pojos, notNullValue());
    assertThat(pojos.getValues(), notNullValue());
    assertThat(pojos.getValues().size(), is(0));
  }

  @Test
  public void testWrapperWithValues() throws Exception {
    String toDeserialize = "<wrapper><pojos><pojo>a</pojo><pojo>b</pojo><pojo>c</pojo></pojos></wrapper>";

    Wrapper wrapper = mapper.readValue(toDeserialize, Wrapper.class);
    assertThat(wrapper, notNullValue());
    assertThat(wrapper.getPojos(), notNullValue());
    assertThat(wrapper.getPojos().getValues(), notNullValue());
    assertThat(wrapper.getPojos().getValues().size(), is(3));
    assertThat(wrapper.getPojos().getValues().get(0).getValue(), is("a"));
  }

  @Test
  public void testWrapperWithoutValues() throws Exception {
    String toDeserialize = "<wrapper><pojos></pojos></wrapper>";

    Wrapper wrapper = mapper.readValue(toDeserialize, Wrapper.class);
    assertThat(wrapper, notNullValue());
    assertThat(wrapper.getPojos(), notNullValue());
    assertThat(wrapper.getPojos().getValues(), notNullValue());
    assertThat(wrapper.getPojos().getValues().size(), is(0));
  }

  @Test
  public void testWrapperWithSelfClosingPojos() throws Exception {
    String toDeserialize = "<wrapper><pojos/></wrapper>";

    Wrapper wrapper = mapper.readValue(toDeserialize, Wrapper.class);
    assertThat(wrapper, notNullValue());
    assertThat(wrapper.getPojos(), notNullValue());
    assertThat(wrapper.getPojos().getValues(), notNullValue());
    assertThat(wrapper.getPojos().getValues().size(), is(0));
  }

  @Test
  public void testWrapperWithoutPojos() throws Exception {
    String toDeserialize = "<wrapper></wrapper>";

    Wrapper wrapper = mapper.readValue(toDeserialize, Wrapper.class);
    assertThat(wrapper, notNullValue());
    assertThat(wrapper.getPojos(), notNullValue());
    assertThat(wrapper.getPojos().getValues(), notNullValue());
    assertThat(wrapper.getPojos().getValues().size(), is(0));
  }

  @Test
  public void testSelfClosingWrapper() throws Exception {
    String toDeserialize = "<wrapper/>";

    Wrapper wrapper = mapper.readValue(toDeserialize, Wrapper.class);
    assertThat(wrapper, notNullValue());
    assertThat(wrapper.getPojos(), notNullValue());
    assertThat(wrapper.getPojos().getValues(), notNullValue());
    assertThat(wrapper.getPojos().getValues().size(), is(0));
  }

  @XmlRootElement(name = "wrapper")
  @XmlAccessorType(XmlAccessType.FIELD)
  private static class Wrapper {
    @XmlElement(name = "pojos")
    private Pojos pojos = new Pojos();

    @java.beans.ConstructorProperties({"pojos"})
    private Wrapper(Pojos pojos) {
      this.pojos = pojos;
    }

    public Wrapper() {
    }

    public Pojos getPojos() {
      return this.pojos;
    }

    public void setPojos(Pojos pojos) {
      this.pojos = pojos;
    }

  }

  @XmlAccessorType(XmlAccessType.FIELD)
  private static class Pojos {
    @XmlElement(name = "pojo")
    private java.util.List<Pojo> values = new ArrayList<>();

    @java.beans.ConstructorProperties({"values"})
    private Pojos(List<Pojo> values) {
      this.values = values;
    }

    public Pojos() {
    }

    public List<Pojo> getValues() {
      return this.values;
    }

    public void setValues(List<Pojo> values) {
      this.values = values;
    }

  }

  @XmlAccessorType(XmlAccessType.FIELD)
  private static class Pojo {
    private String value = "";

    @java.beans.ConstructorProperties({"value"})
    private Pojo(String value) {
      this.value = value;
    }

    public Pojo() {
    }

    public String getValue() {
      return this.value;
    }

    public void setValue(String value) {
      this.value = value;
    }

  }
}

And the Maven pom.xml (without Lombok as requested by @cowtowncoder):

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>

  <groupId>test</groupId>
  <artifactId>test</artifactId>
  <version>1.0-SNAPSHOT</version>

  <properties>
    <maven.compiler.source>1.8</maven.compiler.source>
    <maven.compiler.target>1.8</maven.compiler.target>
  </properties>

  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.6.1</version>
        <configuration>
          <source>1.8</source>
          <target>1.8</target>
          <encoding>UTF-8</encoding>
        </configuration>
      </plugin>
    </plugins>
  </build>

  <dependencies>
    <dependency>
      <groupId>com.fasterxml.jackson.dataformat</groupId>
      <artifactId>jackson-dataformat-xml</artifactId>
      <version>2.9.0.pr4</version>
    </dependency>
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-core</artifactId>
      <version>2.9.0.pr4</version>
    </dependency>
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-databind</artifactId>
      <version>2.9.0.pr4</version>
    </dependency>
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-annotations</artifactId>
      <version>2.9.0.pr4</version>
    </dependency>
    <dependency>
      <groupId>com.fasterxml.jackson.module</groupId>
      <artifactId>jackson-module-jaxb-annotations</artifactId>
      <version>2.9.0.pr4</version>
    </dependency>
    <dependency>
      <groupId>org.mockito</groupId>
      <artifactId>mockito-core</artifactId>
      <version>2.7.9</version>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
      <version>4.12</version>
      <scope>test</scope>
    </dependency>
  </dependencies>

</project>

If you run it, you will see that 2 tests fail:

  • testWrapperWithoutValues
  • testWrapperWithSelfClosingPojos

Now I'm anything but a JAXB or Jackson expert, so I definitely may have done mistakes or misconfigured my mapper or POJOs, in that case, any hint is appreciated.
To my surprise, <wrapper><pojos></pojos></wrapper> returns a null Pojos object, while <wrapper></wrapper> returns an instance of Pojos (as you can see in testWrapperWithoutPojos test).
I think this is a bug.

Dependencies to ensure we test in the same conditions:
org.projectlombok:lombok:jar:1.16.14
org.mockito:mockito-core:jar:2.7.9
com.fasterxml.jackson.dataformat:jackson-dataformat-xml:jar:2.9.0.pr4
com.fasterxml.jackson.core:jackson-core:jar:2.9.0.pr4
com.fasterxml.jackson.core:jackson-annotations:jar:2.9.0.pr4
com.fasterxml.jackson.module:jackson-module-jaxb-annotations:jar:2.9.0.pr4

Note I also tried with jackson 2.8.9 version (for all Jackson-related libraries obviously) without this line:

mapper.setDefaultSetterInfo(JsonSetter.Value.forContentNulls(Nulls.SKIP));

and the problem remains.

@jetersen
Copy link

jetersen commented Dec 5, 2017

This is still an issue 😭

    val xml = """
<?xml version="1.0" encoding="utf-8"?>
<AcResponse
    Command="show depots"
    TaskId="1260">
  <Element
      Number="1"
      Name="accurev"
      Slice="1"
      exclusiveLocking="false"
      case="insensitive"
      locWidth="128" />
  <Element
      Number="2"
      Name="second accurev"
      Slice="2"
      exclusiveLocking="false"
      case="insensitive"
      locWidth="128" />
</AcResponse>
"""
    val mapper : ObjectMapper = XmlMapper()
    mapper.registerModule(KotlinModule())
    mapper.configure(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES, true)
    val response : Depots = mapper.readValue(xml)
    println(response)

@akuhtz
Copy link

akuhtz commented Apr 11, 2018

This is still an issue 😭 😭 😭

@saurabhkumar2015
Copy link

is this issue now resolved ?

@cowtowncoder
Copy link
Member

@saurabhkumar2015 what do you think, based on the fact issue is open and there havn't been comments?

And to everyone else -- please not add arbitrary "still an issue" notes unless you add some useful information: simpler reproduction, something else you think is related. Or at very least, version in which you observe it that is more recent than earlier versions mentioned.
Same goes for asking "is this still an issue": unless there is some additional potentially useful information, you are just littering and making it bit more cumbersome to follow discussion.

Conversely, if you do find issue DOES NOT occur any more on some version, adding that does add information and help.

@leluna
Copy link

leluna commented Aug 30, 2019

With Version 2.9.9, the issue can be reproduced with the following quite minimal test:

@Test
public void testSelfclosingTag() throws JsonParseException, JsonMappingException, IOException {
	XmlMapper mapper = new XmlMapper();
	TestPojo readValue = mapper.readValue("<rootelement><elements/></rootelement>", TestPojo.class);
	Assert.assertTrue(readValue.getElements() != null);
}

@Test
public void testEmptyTag() throws JsonParseException, JsonMappingException, IOException {
	XmlMapper mapper = new XmlMapper();
	TestPojo readValue = mapper.readValue("<rootelement><elements></elements></rootelement>", TestPojo.class);
	Assert.assertTrue(readValue.getElements() != null);
}

@XmlRootElement(name = "rootelement")
@XmlAccessorType(XmlAccessType.NONE)
public static class TestPojo {

	@XmlElementWrapper(name = "elements")
	@XmlElement(name = "element")
	private List<String> elements = new ArrayList<>();

	public List<String> getElements() {
		return elements;
	}
}

@cowtowncoder cowtowncoder changed the title Empty (or self-closing) element is incorrectly deserialized as null Empty (or self-closing) Element representing List is incorrectly deserialized as null, not Empty List Aug 30, 2019
@the-rockkk
Copy link

Isn't this already resolved:
#246

@hd42
Copy link

hd42 commented Feb 17, 2020

No. Setting new XmlFactoryBuilder(xmlFactory).configure(FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL, false).build(); creates a MismatchedInputException instead of creating null. An empty element(either '<elements></elements>' or '<elements/>') should be an empty list instead of null, which I would only expect if there was no XML tag.

@cowtowncoder
Copy link
Member

Currently this can be done, I think, with combination of 2 things:

  1. Passing List via Creator (constructor or static factory method annotated with @JsonCreator, parameter names annotated with @JsonProperty or auto-detected with Java 8 module)
  2. Defining "null replacement" of @JsonSetter(nulls=Nulls.AS_EMPTY) (explained in https://medium.com/@cowtowncoder/jackson-2-9-features-b2a19029e9ff )

This would make both null values and missing values to be set (due to constructors; setters are ONLY called if there is matching input value), and then map missing/null into "empty" value, which in case of Lists would be empty list.

@hd42
Copy link

hd42 commented Feb 26, 2020

@JsonSetter works for self-closing tags('<elements/>'), but an empty wrapper tag('<elements></elements>') still produces a MismatchedInputException.

@cowtowncoder
Copy link
Member

I am hoping to tackle this for Jackson 2.12, fwtw; including both handling "blank" text and empty element the same and allowing either null or "empty" List; defaulting (I think) to empty/blank become empty List, unless configured differently to produce null.

@cowtowncoder
Copy link
Member

Good news! As far as I can see, tests all pass with current 2.12 branch. Even without setting null-strategy, now that empty String by default coerces into empty values for XML.

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

8 participants