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

Fix mismatch in setNextIsUnwrapped(boolean) in XmlBeanSerializerBase#serializeFieldsFiltered() #616

Merged
merged 2 commits into from
Dec 2, 2023

Conversation

vmi
Copy link
Contributor

@vmi vmi commented Nov 28, 2023

In serializeFields() of XmlBeanSerializerBase, setNextIsUnwrapped(true) is executed in L.200-203, and setNextIsUnwrapped(false) is executed in L.215-219.
However, in serializeFieldsFiltered(), setNextIsUnwrapped(true) is executed in L.282-285, but there is no corresponding setNextIsUnwrapped(false).

@pjfanning
Copy link
Member

Could you provide a test case?

@vmi
Copy link
Contributor Author

vmi commented Nov 28, 2023

I have no test cases.
I will make it if necessary, but please give me some time.

@cowtowncoder
Copy link
Member

This looks correct, but I agree with @pjfanning that it'd be really useful to have a test case to show the fix.

We can also backport the fix into 2.16 for 2.16.1, I think. I can cherry-pick merge unless you want to re-base (not sure if Github UI has a good way to do that).

@cowtowncoder
Copy link
Member

@vmi One other process thing: since this is your first (but hopefully not last! :) ) contribution, I think we will need CLA from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and it's usually easiest to fill, sign, print & scan/photo, email to cla at fasterxml dot com.
This only needs to be done once, before the first contribution and is good for all other contributions.
Apologies for this extra step but it is something we need as OSS project used by corporations.

Thank you again for contributing this -- looking forward to merging it!

@vmi
Copy link
Contributor Author

vmi commented Dec 2, 2023

I added unit test, and sent signed CLA.

@cowtowncoder
Copy link
Member

Excellent! CLA successfully received, test looks good; will merge after CI passes.

@cowtowncoder cowtowncoder merged commit 06c3a43 into FasterXML:2.17 Dec 2, 2023
3 checks passed
@cowtowncoder cowtowncoder changed the title Fix mismatch in setNextIsUnwrapped(true/false) in XmlBeanSerializerBase#serializeFieldsFiltered() Fix mismatch in setNextIsUnwrapped(boolean) in XmlBeanSerializerBase#serializeFieldsFiltered() Dec 2, 2023
cowtowncoder pushed a commit that referenced this pull request Dec 2, 2023
cowtowncoder added a commit that referenced this pull request Dec 2, 2023
@cowtowncoder cowtowncoder added this to the 2.16.1 milestone Dec 2, 2023
@cowtowncoder
Copy link
Member

Since this seems like a safe fix, I backported it in 2.16.1 as well so will get released sooner (we only just started 2.17 branch).

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

Successfully merging this pull request may close these issues.

3 participants