XmlMapper/UntypedObjectDeserializer swallows duplicated elements in XML documents

[joaovarandas]

Hello guys.

I think I have already seen this issue around, but, for UntypedObjectDeserializer the solution maybe a bit different.

Entity

<?xml version="1.0" encoding="UTF-8"?>
<person>
   <name>John</name>
   <parent>Jose</parent>
   <parent>Maria</parent>
   <dogs>
      <count>3</count>
      <dog>
         <name>Spike</name>
         <age>12</age>
      </dog>
      <dog>
         <name>Brutus</name>
         <age>9</age>
      </dog>
      <dog>
         <name>Bob</name>
         <age>14</age>
      </dog>
   </dogs>
</person>

Code

new XmlMapper().readValue(xml, Object.class);       

Output

{
  "name" : "John",
  "parent" : "Maria",
  "dogs" : {
    "count" : "3",
    "dog" : {
      "name" : "Bob",
      "age" : "14"
    }
  }
}

Problem

Duplicated elements in the entity get swallowed by current UntypedObjectDeserializer implementation.

I can't use Typed Objects. In my use case, I don't have any typed objects, because I don't know how objects are sent to me.

Possible Solution

While creating the Map for the data, check if there are duplicated keys, and start an Array, with this approach, the output would be:

{
  "name" : "John",
  "parent" : [ "Jose", "Maria" ],
  "dogs" : {
    "count" : "3",
    "dog" : [ {
      "name" : "Spike",
      "age" : "12"
    }, {
      "name" : "Brutus",
      "age" : "9"
    }, {
      "name" : "Bob",
      "age" : "14"
    } ]
  }
}

How to Reproduce

Gist

• https://gist.github.com/joaovarandas/1543e792ed6204f0cf5fe860cb7d58ed

Artifacts:

• jackson-core
• jackson-databind
• jackson-dataformat-xml

Version:

• 2.7.4

Conclusion

The gist implements the solution using an extended version of the UntypedObjectDeserializer.

If not the default behavior, what about creating a new DeserializationFeature to enable this(default or not)?

Should you guys like/aprove this solution, I can always fork the project and submit a pull request with the full solution as a feature or default behavior.

Thanks!
Jp

[joaovarandas]

The new test 'testDuplicatedElementsSwallowing' in class 'XmlParserTest' ensures it is working as proposed.

Let me know if you want me to pull this.

[cowtowncoder]

I don't think this should be the default behavior, but I would be fine with a feature that allows doing this.

One question here is regarding how it should be enabled; at least two possibilities:

1. Straight setter/getter in XmlMapper (similar to setDefaultUseWrapper)
2. FromXmlParser.Feature

Of these I would otherwise favor (2) since it's easier to expand, but one problem is that this feature can not be changed on per-call basis and as such should not be a (format-specific) DeserializationFeature. So perhaps (1) is the better way to go.

Alternatively perhaps it would be better to add option (3); add XmlMapper.Feature.

So if you can do a PR for feature itself (with simple setter, for now), and I can retrofit XmlMapper.Feature?

[joaovarandas]

The problem with adding as a "optional" feature (or setter) is that "unaware" developers could lose data deserialized from XML. That's what happened to me: - Users found out the bug weeks after it has been deployed to production, since during the tests there were no repeated elements.

Anyway, sure I can do that. And it fits my use case as well.
Thanks!

[cowtowncoder]

Another option would be to explicitly fail on any attempt to bind to "untyped" (java.lang.Object), to make it explicitly clear that failure occurs. I am open to doing this, although that would have to go in next minor version. It would probably also have to apply to Map...

In the end I do not think there are good options: ideally XML is not being mapped to either Object or Map. Trying to auto-detect between List of things and single thing does not work well with Java style typing -- access to such values is awkward. In fact, I might go as far as claiming that it might make more sense to use DOM (and alternatives, JDOM, DOM4J) instead of (Jackson) databind.

[joaovarandas]

Yeah... I agree with you, but:

I'm currently building a facade for the Apache Components/HTTPClient, and invoking SOAP services... Then I just "transform" the return object to a simple Map/List, and invoke JavaScript code (using Nashorn).

In that use case, it doesn't matter if the object is an Array or a Map since I access them using simple JavaScript bindings like: data["attribute-key"] or data[0] for an array ...

Probably for Scala or other languages inside JVM it would be the same.

I think failing is good too, to fail when there are duplicated elements in a default UntypedObject deser ...

It's not final yet, but if you wanna take a look at the project:
https://github.com/inpaas/inpaas-httpclient

[joaovarandas]

@cowtowncoder how could I "deregister" a Module or the deserializer?

See the code below:

public XmlMapper setUseXmlUntypedObjectDeserModule(boolean state) {
    if (state == true) {
        Module xmlUntypedObjectDeserModule = new SimpleModule().addDeserializer(Object.class, new XmlUntypedObjectDeserializer());
        registerModule(xmlUntypedObjectDeserModule);
    } else {
        // is there anyway to rollback the state?
    }

    return this;
}

[cowtowncoder]

There is no way to deregister a module, since registration really is just a callback to get a chance to register other things. Further, there is no metadata on what registered any given handler (no back-references to module). So answer is you can not. You can just register more handlers.

So question would rather become "what are you trying to do". It is possible to add suitable Serializers / Deserializers handler to -- for example -- avoid return custom serializer/deserializer depending configuration.

[Paradoxia]

@joaovarandas Thank you, your gist solution saved my day.

[arakelian]

@joaovarandas Saved me, too. Thanks.

[perezmlal]

@joaovarandas thanks, your gist solution works perfect.

[gonzalogarciajaubert]

Thanks @joaovarandas for your gist

[isipkh]

Thanks @joaovarandas for the solution in gist

[dadoonet]

Thanks @joaovarandas for the gist. Worked perfectly.

[vy]

I linked this gem to a relevant StackOverflow post. Thanks @joaovarandas for the report and the temporary fix!

[Musikolo]

@cowtowncoder, could you please clarify whether this issue has been solved?
I wonder whether there is a way to support XML with duplicate child nodes?
In the worst case scenario, is there any way to get all XML child node loaded?

Using version 2.9.4 currently.

Thank you!

[cowtowncoder]

@Musikolo it hasn't, and I don't see a way to handle it via Untyped object deserialization. Jackson XML module does not offer general-purpose node approach to XML -- problem is that XML simply has no native difference that would allow separating Arrays (lists) from Objects (maps), and I really really dislike heuristics that try to separate single-value from multiple-values, as this has proven not to be reliable or stable handling for anything.

[sschuberth]

Excuse my probably naive question, but why can't we simply use a MultiMap instead of a Map and be good?

BTW: The problem with the approach suggested in this issue is that it does not work if you also need to parse the attributes of the keys / tags with the identical names.

[cowtowncoder]

@sschuberth Because "untyped" databinding is defined as using basic Maps, Lists, Numbers, Strings and booleans. It can not be changed to use MultiMaps (and I don't think JDK has MultiMaps at this point, so API shouldn't be tied to specific external library that provides type). Since JSON does not allow multiple values per property it would also not make much sense to complicate handling for data formats other than XML, at least not at this level.

One possibility, I think, could be to make XMLMapper expose different type of tree model.
But I am not sure how valuable that would be over someone just using DOM values.

[goodlifeinc]

Absolute life saver!

In order to throw my 5 cents on the topic, for me it was crucial to readValue to Object.class instead of TypeReference of Map<String, Object>.

Thanks for the workaround provided!

[ST-DDT]

Is there a workaround for the (XML) JsonNode way as well?

[ViniciusArnhold]

@cowtowncoder Is this (Handling duplicate elements natively) targeted for 3.0?

[cowtowncoder]

I hope it can be addressed, although I do not really have solid plans. But if there was, I think it'd be one
of "Big Ideas", to be described here:

https://github.com/FasterXML/jackson-future-ideas/wiki/JSTEP

with a new JSTEP id, description of basic idea. I wish I had some concrete ideas, as I agree with everyone that availability of a tree model is a HUGE win, and that with XML there has to be a way to indicate element sequences, element/attribute distinction. But still isolating it from some of complexities of XML, while keeping model interoperable with other representations.

I added tentative placeholder (looks like it'd be `JSTEP-6') on page linked-to above.

[cowtowncoder]

One update: created FasterXML/jackson-databind#2732 to help with JsonNode to -- I hope -- essentially implement "coerce as Array if you see "duplicate" property.
This could also pave the way to address "untyped" deserialization case.

[cowtowncoder]

I ended up implementing suggestion, so that repeated elements result in automatic "upgrade" into List. This is done automatically when reading xml content.

Will be in 2.12(.0).

[taojoe]

@cowtowncoder does 2.12 release in a few days?

[cowtowncoder]

@taojoe not quite that fast: I am hoping to get the first release candidate (rc1) out within a week, and then bit of time is needed for testing. So earliest I think would be in maybe 4 weeks.

[taojoe]

@cowtowncoder rc1 is ok for me. Thanks a lot. ' repeated elements result in automatic "upgrade" into List' is the key part in my project. It'll save a lot of time.

[taojoe]

@cowtowncoder when will rc1 release ?

[cowtowncoder]

Missed doing it last weekend, now hoping to have time over upcoming weekend, in 4-5 days.

[taojoe]

@cowtowncoder thanks

[yairlenga]

I added a ticket to improve handling of repeated elements, see [https://github.com//issues/495]

For those with Perl background: current solution is similar to XML::Simple forceArray = true, the addition will follow the forceArray = [ tag1, tag2, …] approach. For parsing in Perl, it was a life saver.