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

Deserialization with XmlMapper and DeserializationFeature.UNWRAP_ROOT_VALUE no longer works in 2.12 #485

Closed
ionel-sirbu-crunch opened this issue Jul 16, 2021 · 12 comments
Milestone

Comments

@ionel-sirbu-crunch
Copy link

I've recently updated a project at work to Spring-boot 2.5.2 & one of the tests started failing. As it turns out, the root cause lies in the deserialisation of XML data.
We use the root unwrapping feature & have the data model class annotated with @JsonRootName.

I've created a very simple project to give you an example of our usage here.

Old, working version: 2.11.4
New, non-working version: 2.12.3

Any advice is much appreciated.
Thanks!

@cowtowncoder
Copy link
Member

Looks like the linked-to repository does not exist any more?

@ionel-sirbu-crunch
Copy link
Author

Looks like the linked-to repository does not exist any more?

Apologies, the repo was private. Could you please try it now?

@marcospassos
Copy link

marcospassos commented Aug 27, 2021

Same problem here. We just updated our libraries to 2.12.5, and one of them uses the XML mapper. It looks like the UNWRAP_ROOT_VALUE deserialization flag is being completely ignored.

@cowtowncoder
Copy link
Member

cowtowncoder commented Aug 27, 2021

@ionel-sirbu-crunch yes, I can access that.

@marcospassos This is not a bug but feature, as per #374: due to the way XML works, the outermost element is effectively ignored regardless of this settings. That ticket explains rationale.

If this is due to #374, it would not be considered a bug, but fix to inconsistent/wrong behavior. It is unfortunate if there is existing code that counted on that behavior, however.
I am not sure what would be the best way forward here.

@marcospassos
Copy link

Yes, but it's a breaking change, isn't it?

@marcospassos
Copy link

Jacksons simply can't deserialize anything anymore, so all properties are null now.

@cowtowncoder
Copy link
Member

cowtowncoder commented Aug 30, 2021

This was not known to be a use case; no tests covering it. Unless I misunderstand usage, it was not supported to begin with -- and was confusing for users since serialization and deserialization for XML were asymmetric: things simply would not work if you serialized something, then tried to read it back.
And since Jackson XML modules stated goal is to specifically "read what it writes": that is, round-tripping in that way is the main goal, older behavior still seems wrong, fundamentally.

This was not reported as an issue during Jackson 2.12 release candidate phase, as far as I know, meaning that 2.12.0 was released with the change as intended.

But if and when it turns out there is existing usage I do want to figure out how to resolve that, for 2.13. 2.12 is something we can do little about because its API is fixed (no additions) and since behavior of .0 was changed purposefully.

I am a little bit hesitant to try to undo change and allow UNWRAP_ROOT_VALUE, given the noted problems.
I could instead include a new feature -- ToXmlGenerator.Feature.XXX -- although that also seems a bit redundant.

Regardless I really want to resolve this for 2.13.0. Perhaps I do need to undo the change.

@marcospassos
Copy link

We'll try to adjust on our side to not bring more maintenance for something that isn't officially supported. So, for us, no need to spend time on this.

@cowtowncoder
Copy link
Member

@marcospassos I appreciate your flexibility and am sorry there was this breakage. I will still consider revert here; I simply did not realize the feature was used (and considered useful) on XML side.

@ionel-sirbu-crunch
Copy link
Author

@cowtowncoder Thanks for looking into this! As I mentioned in the initial description, we only deserialize the XML data (it comes from an external service), so I wasn't aware of the asymmetry of the UNWRAP_ROOT_VALUE feature.
For us this is no longer a matter of urgency though, as I've already implemented a workaround. Luckily for us, the root element has the same shape every time, so it was easy to work around it.

Should you decide to leave this feature as it is, may I suggest adding a matrix of unsupported ObjectMapper features to the readme so that it's a little more transparent to the developers what they can & cannot use?

Many thanks!

@cowtowncoder cowtowncoder changed the title Deserialization no longer works with XmlMapper and DeserializationFeature.UNWRAP_ROOT_VALUE in 2.12.3 Deserialization no longer works with XmlMapper and DeserializationFeature.UNWRAP_ROOT_VALUE in 2.12 Sep 4, 2021
@cowtowncoder
Copy link
Member

cowtowncoder commented Sep 4, 2021

@ionel-sirbu-crunch I fully agree! And yes, I can see why in deserialize-only case this functionality can be useful (although as per original issue, there may be quirks with some content).

Looking at code change(s) involved, I think I can do a revert here. Just wondering it there should be another switch for XML module to (optionally) just ignore that setting altogether.

Regardless, I think the idea of mentioning complications for UNWRAP_ROOT_VALUE is valid to note in core jackson-databind Javadocs. But even more so, WRAP_ROOT_VALUE if and when it won't work for XML.

@cowtowncoder cowtowncoder added this to the 2.13.0 milestone Sep 19, 2021
@cowtowncoder cowtowncoder changed the title Deserialization no longer works with XmlMapper and DeserializationFeature.UNWRAP_ROOT_VALUE in 2.12 Deserialization with XmlMapper and DeserializationFeature.UNWRAP_ROOT_VALUE no longer works in 2.12 Sep 19, 2021
@cowtowncoder
Copy link
Member

Created #494 for possible follow-up work.

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

3 participants