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

Required attribute of @JsonProperty is ignored when deserializing from XML #538

Open
johandeschutterGET opened this issue Jun 30, 2022 · 7 comments
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Milestone

Comments

@johandeschutterGET
Copy link

When JSON is parsed, there is a check on properties that are annotated with JsonProperty with attibute required to true.
If the required property is missing, a MismatchedInputException with message "Missing required creator property" is thrown.

When XML is parsed, there is no check on required properties/element. If the required property/element is missing, there is no exception.
Adding DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES doesn't make any difference.

There is constructor is annotated with JsonCreator and with the required property annotated with JsonProperty.

It worked when jackson-dataformat-xml version 2.11.4 was used, it doesn't work since version 2.12.0

Do I need to use another annotation for a required XML element?

Example class:

@JsonRootName(value = "bar")
public class Bar
{
    @JsonProperty(value = "foo", required = true)
    private int foo;

    public Bar()
    {}

    @JsonCreator
    public Bar(@JsonProperty(value = "foo", required = true) final int foo)
    {
        this.foo = foo;
    }

    public int getFoo()
    {
        return this.foo;
    }

    @Override
    public int hashCode()
    {
        return Objects.hash(this.foo);
    }

    @Override
    public boolean equals(final Object obj)
    {
        if (this == obj)
        {
            return true;
        }
        if (!(obj instanceof Bar))
        {
            return false;
        }
        final Bar other = (Bar) obj;
        return this.foo == other.foo;
    }

    @Override
    public String toString()
    {
        return "Bar: " + Objects.toString(this.foo);
    }
}

Example of unit test on JSON, this unit test throws an exception as expected, because property foo is missing.

    @Test(expected = JsonProcessingException.class)
    public void deserializeJson() throws Exception
    {
        final ObjectMapper jsonMapper = new ObjectMapper();

        final Bar expected = new Bar(123);
        final Bar actual = jsonMapper.readValue("{}", Bar.class); // missing property foo in JSON string
    }

Example of unit test on XML, this unit test does not throw an exception

  @Test(expected = JsonProcessingException.class)
   public void deserializeXml() throws Exception
   {
       final XmlMapper xmlMapper = new XmlMapper();
       xmlMapper.configure(DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES, true);

       final Bar expected = new Bar(123);
       final Bar actual = xmlMapper.readValue("<bar></bar>", Bar.class); // missing element foo in XML string

       Assert.fail("Expected com.fasterxml.jackson.databind.exc.MismatchedInputException: Missing required creator property 'foo' (index 0)");
   }
@cowtowncoder
Copy link
Member

Hmmh. That should work, unless I am missing something. Perhaps this is because of the oddities of the root element handling, in context of "empty element"; it could be that processing is unintentionally short-cut to go through default constructor or something.

I hope to look into this in near future (but there's a bit of a backlog): thank you for reporting the issue, and especially thank you for providing the test case!

@cowtowncoder cowtowncoder added the has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue label Jul 3, 2022
@johandeschutterGET
Copy link
Author

The parsing of an empty XML element(1 tag, no start and end tag) is correct, if the feature FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL is enabled.
The result is a NULL.

    @Test
    public void deserializeEmptyTag() throws Exception
    {
        final XmlMapper xmlMapper = new XmlMapper();
        xmlMapper.configure(FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL, true);
        final Bar actual = xmlMapper.readValue("<bar/>", Bar.class);
        Assert.assertNull(actual);
    }

The parsing of XML element with value empty string (start and end tag with empty string between), with feature FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL enabled, fails. The result is an object (Bar: 0) created by the default constructor, the required property foo is zero (= default value). There is no MismatchedInputException thrown for the missing foo property.

    @Test
    public void deserializeTagEmptyString() throws Exception
    {
        final XmlMapper xmlMapper = new XmlMapper();
        xmlMapper.configure(FromXmlParser.Feature.EMPTY_ELEMENT_AS_NULL, true);
        final Bar actual = xmlMapper.readValue("<bar></bar>", Bar.class);
        Assert.assertNull(actual); // java.lang.AssertionError: expected null, but was:<Bar: 0>
    }

The parsing of XML element with value empty string (start and end tag with empty string between) is correct, if the coercion configuration is defined for type Bar in order to process an empty string input as null.

    @Test
    public void deserializeTagEmptyStringCorrected() throws Exception
    {
        final XmlMapper xmlMapper = new XmlMapper();
        xmlMapper.coercionConfigFor(Bar.class).setCoercion(CoercionInputShape.EmptyString, CoercionAction.AsNull);
        final Bar actual = xmlMapper.readValue("<bar></bar>", Bar.class);
        Assert.assertNull(actual);
    }

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 4, 2022

@johandeschutterGET Yes, you are right. Apologies for slow follow up here, I am finally catching up here.

Ok. Looking at this, what happens is that this is essentially coercion from Empty String into "empty" value of POJO.
With other formats -- say, JSON -- empty String value would use different logic than Object value and thus there should be no exception.
Unfortunately XML is special: due to structure being what it is, empty Element looks either as "Object with no properties" or "empty String".

To further complicate things, however, from databind perspective (format-agnostic), value at this point definitely looks like String value, not Object.

But maybe I can figure out something...

@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 4, 2022

Also: I think this is specific to Root value as well: it would not be (as) problematic for anything referenced by some other value.

Unfortunately I am not sure of how to tackle this at this point in time -- while it would be possible to detect existence of "property-based" Creator (via ValueInstantiator); and even more, properties involved including requiredness; it's not very easy to backtrack from the point of "empty String" handling back to make things look like Object.
Ideally I suspect this should be handled by XML module itself to expose empty element as START_OBJECT / END_OBJECT pair.

Unfortunately I think behavior was actually changed in 2.12 from such a setup to exposing as JSON String to support root-level scalars... So going back to that might be tricky to pull off. Unless we could somehow figure out target for root value is POJO, based on deserializer?

@cowtowncoder
Copy link
Member

Thinking about this more: translation is within initialize() of XmlTokenStream, called when FromXmlParser is being constructed. It'd be easy to change shape of tokens here.

The problem is that at this point we do not (yet, sort of) have information on JsonDeserializer being used.

So likely it'd have to be done, instead, from readRootValue() of XmlDeserializationContext which does have this information. It could probably determine that we are using a BeanDeserializer, and if token was JsonToken.VALUE_STRING with empty/blank contents, force re-initialization of FromXmlParser/XmlTokenStream to instead offer START_OBJECT/END_OBJECT sequence. Phew.
It could additionally even check if there's ValueInstantiator to see what creators exist. Or maybe that there is no From-String instantiator.

It seems somewhat doable, just a bit complicated and convoluted :)

@cowtowncoder
Copy link
Member

As per above note, I think #491 is due to the same root problem. So solving this issue would resolve at least 2 reported issues.

@cowtowncoder cowtowncoder changed the title Required attribute of JsonProperty annotation is ignored when deserializing from XML to Java. Required attribute of @JsonProperty is ignored when deserializing from XML Sep 7, 2022
@cowtowncoder cowtowncoder added this to the 2.14.0 milestone Sep 7, 2022
cowtowncoder added a commit that referenced this issue Sep 7, 2022
@cowtowncoder
Copy link
Member

Ok, I was able to figure out a much simpler approach, essentially reverting some of the changes done in 2.12 for coercion root-element textual content. But without breaking what that change was initially added to fix.

So should work as expected in 2.14.0; hoping to release 2.14.0-rc1 within a week or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

2 participants