Behavior of when `FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL` enabled different after 2.12.0

[lazyr1ch]

I have a class

@Data
public class FooView {
    private String str;
}

and enabled feature FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL for XmlMapper. Then i trying to call FooView fooView = xmlMapper.readValue(content, FooView.class) and set content as: `

As result i've had a NP: java.lang.NullPointerException: Cannot invoke method getStr() on null object `

Before 2.12.0 returned result object (fooView) was not null object.

Version information 2.12.1 and greater. 2.12.0 gets another bug too.

[cowtowncoder]

This is not enough to reproduce your issue: I'd need one to be able to help. So just a unit test that specifies exact calls (description has most of it but I am not sure how you get NPE)

Note, too, that reproduction can not use other frameworks like Lombok: they need to be stand-alone.

[cowtowncoder]

Also, wrong repo, transferred to jackson-dataformat-xml

[cowtowncoder]

No information to reproduce; may be re-opened/re-filed with test case

[lazyr1ch]

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
import com.fasterxml.jackson.dataformat.xml.deser.FromXmlParser;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Objects;

import static org.junit.Assert.assertNotNull;

public class TestMappingOfInputView {
    final static Logger logger = LoggerFactory.getLogger(TestMappingOfInputView.class);

    public static class TestView {
        private String value = "defaultValue";

        public String getValue() {
            return value;
        }

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

        @Override
        public String toString() {
            return "TestView{" +
                    "value='" + value + '\'' +
                    '}';
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;

            TestView testView = (TestView) o;

            return Objects.equals(value, testView.value);
        }

        @Override
        public int hashCode() {
            return value != null ? value.hashCode() : 0;
        }
    }

    @Test
    public void test1() throws Exception {
        ObjectMapper mapper = makeXmlMapper();

        String xml = "<Foo/>";
        TestView testView = mapper.readValue(xml, TestView.class);

        assertNotNull(testView);
    }

    private ObjectMapper makeXmlMapper() {
        XmlMapper mapper = new XmlMapper();
        mapper.enable(FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL);
        return mapper;
    }
}

[lazyr1ch]

@cowtowncoder Hi! I added test for my case. For jackson version < 2.12.0 test complete sucessfully, but for >=2.12.0 test is failed.

[cowtowncoder]

@pavelturch Ok so do you expect testView to be null or not? Assertion suggests:

 assertNotNull(testView);

not but since 2.15 seems to produce non-null Object I am bit confused about this bug report (as test as written does not fail).

Which versions have you tested?

[lazyr1ch]

@pavelturch Ok so do you expect testView to be null or not? Assertion suggests:

 assertNotNull(testView);

not but since 2.15 seems to produce non-null Object I am bit confused about this bug report (as test as written does not fail).

Which versions have you tested?

I'm expecting that testView is not null. I was tested it on 2.12.1, 2.12.7, 2.13.5, 2.14.3, 2.15.1 and I got null object in assertion. Runs under OpenJDK 1.8. My maven project have a parent "spring-boot-starter-parent 2.5.14" and I override jackson-bom.version property. I checked dependency conflicts and make sure that overrided version is used. What else can affect?

[cowtowncoder]

I'll have to first try to reproduce this, but... I also think that from name EMPTY_ELEMENT_AS_NULL it seems like it actually should produce null, not an "empty" Object. Regardless of how Jackson 2.12.x behaved.

[lazyr1ch]

Main problem in a root node. Nested empty nodes can be a null-obects. In my case I got xml response from API that contains empty root node. Or another case I need send empty xml root node, but body serializes to null.

вт, 23 мая 2023 г., 00:43 Tatu Saloranta @.***>:

I'll have to first try to reproduce this, but... I also think that from name EMPTY_ELEMENT_AS_NULL it seems like it actually should produce null, not an "empty" Object. Regardless of how Jackson 2.12.x behaved.

— Reply to this email directly, view it on GitHub https://github.com/FasterXML/jackson-dataformat-xml/issues/579#issuecomment-1558059657, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4FWZLQE5PT7K7BBZ4HNYLXHPMXFANCNFSM6AAAAAAVQOPFAQ . You are receiving this because you were mentioned.Message ID: @.***>

[cowtowncoder]

@pavelturch I think I should re-phrase my question: do you really need to want to enable EMPTY_ELEMENT_AS_NULL? There should typically be no need for that unless you really want nulls. So it'd make sense to just disable it in your case.

[lazyr1ch]

@cowtowncoder I really need a backward compatibility for our API :) And yes, I want to enable EMPTY_ELEMENT_AS_NULL.

[cowtowncoder]

Ok. Realistically speaking if all versions since 2.12.1 return null, I think it is the case that right now behavior seen is the standard expected behavior: changing it to not return null would be backwards incompatible change despite it once upon a time behaving in a different way.

Looking at release notes, I think #435 is the fix that changed behavior. As far I can see feature description explains expected semantics. I will still need to add an actual test case (hope to get to that tomorrow after resolving other issues) but it looks like I might consider this "behaves as intended" and close.

NOTE: one possibility for supporting alternate behavior while retaining compatibility is to add more Feature choices. This is a possibility to separate handling at root value level.

[lazyr1ch]

Ok. I understand, thank you.

Here is another case with mapper.enable(FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL). For version 2.11.x in test1 I get result1 is non-null, result2 is non-null object too, but result3.getValue() is null-object.
For version >=2.12.1 in test1 I get result1 is null, result2 is null, result 3 is null.
It seems that I won't be able to use the new version until I refactor the code and disable EMPTY_ELEMENT_AS_NULL. But the risks still remain.

` @Test public void test1() throws Exception { ObjectMapper mapper = makeXmlMapper();

    String xml = "<Foo/>";
    TestView result1 = mapper.readValue(xml, TestView.class);

    assertNotNull(result1);
}

@Test
public void test2() throws Exception {
    ObjectMapper mapper = makeXmlMapper();

    String xml = "<Foo/>";
    List<TestView> result2 = mapper.readValue(xml, new TypeReference<List<TestView>>() {
    });

    assertNotNull(result2);
}

@Test
public void test3() throws Exception {
    ObjectMapper mapper = makeXmlMapper();

    String xml = "<Foo><value/></Foo>";
    TestView result3 = mapper.readValue(xml, TestView.class);

    assertNull(result3.getValue());
}

`

[cowtowncoder]

Yes, in general with this setting any and all empty XML elements (<elem/>) would be exposed as null tokens so in that sense your result is consistent with intent of this flag.

That said I think it's not a great feature, partly as in XML there is no semantic distinction between <elem/> and <elem></elem> -- yet this feature introduces distinction. So perhaps it should be deprecated (or maybe even removed) from Jackson 3.0 altogether. Its semantics are unexpected and implementation brittle.

But for what it is worth I don't think behavior wrt root element will be changed at this point.

[cowtowncoder]

Ok: this works as intended. I added a test to verify the behavior as sort of documentation.

[lazyr1ch]

@cowtowncoder Ok, thanks

[cowtowncoder]

@lazyr1ch no problem & apologies for the mess we got here. I don't like introducing backwards-incompatible changes like this. Thank you for your understanding.