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.

Sign up for GitHub

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

Jump to bottom

Allow FilteringParserDelegate to skip last elements in array #883

Merged
merged 5 commits into from
Jan 8, 2023
Merged

Allow FilteringParserDelegate to skip last elements in array #883

merged 5 commits into from
Jan 8, 2023

Conversation

pgomulka
Copy link
Contributor

FilteringParserDelegate when parsing ID_END_ARRAY or ID_END_OBJECT of the skipped element (when _headContext.isStartHandled() == false should be able to exit the loop and continue parsing next tokens.

This logic was changed accidentally removed in 7db467d#diff-f6642caef61e0c403f51a6150ecf45263034fca5002782fd02eacd01e53fe549L694

closes #882

@pgomulka pgomulka mentioned this pull request Dec 29, 2022
Closed
@cowtowncoder cowtowncoder added the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Jan 6, 2023
@cowtowncoder
Copy link
Member

cowtowncoder commented Jan 6, 2023 •
edited
Loading

Ok, one quick question first: would it make sense to rebase against 2.14 instead of 2.15, or would this be too risky a change? It seems like it might be safe for a patch, and that way would be released earlier (2.15.0 is still bit out).
I am fine either way.

Also: before merging I'd need CLA (unless I have received one before -- apologies if I missed it).

CLA is from here: https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf
(and there is also alternative Corporate CLA that is sometimes used although most contributions are with individual CLA).

The usual way is to print the doc, fill & sign, scan/photo, email to info at fasterxml dot com.
Once I receive it I can merge this to whichever branch we want (I can also merge it to 2.15 and cherry-pick in 2.14 if necessary).

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

@tvernum
Copy link

tvernum commented Jan 6, 2023

@cowtowncoder I believe you have a corporate CLA for Elastic.
I'll arrange to have @pgomulka and I added to schedule A.

@pgomulka
Copy link
Contributor Author

pgomulka commented Jan 6, 2023

Ok, one quick question first: would it make sense to rebase against 2.14 instead of 2.15, or would this be too risky a change? It seems like it might be safe for a patch, and that way would be released earlier (2.15.0 is still bit out).

@cowtowncoder I can either rebase this PR against 2.14 and forward-port it to 2.15
or we could backport this commit to 2.14 after it is merged to 2.15.

What is the preferred way in fasterxml project?

@pjfanning
Copy link
Member

target 2.14 and then we'll merge to 2.15 and master

pgomulka and others added 5 commits January 6, 2023 15:47
@pgomulka
draft, do not allow the parser to consume all tokens after the last o…
b587a4c 
…bject of array was excluded
@tvernum @pgomulka
Adjust tests to conform with Jackson style
25d0010 
Adds 1 new test to BasicParserFilteringTest, and switches the
implementation of all testExcludeObject* methods to build on existing
Jackson testing infrastructure.
@pgomulka
a fix for a last array inside an array
da357c7
@pgomulka
fix for array uncomment
951bbc9
@pgomulka
empty lines
476c48e
@pgomulka pgomulka force-pushed the skipping_last_elements_of_array branch from 2eb2fc9 to 476c48e Compare January 6, 2023 14:47
@pgomulka pgomulka changed the base branch from 2.15 to 2.14 January 6, 2023 14:47
@cowtowncoder
Copy link
Member

@cowtowncoder I believe you have a corporate CLA for Elastic. I'll arrange to have @pgomulka and I added to schedule A.

I had a feeling I should have CCLA for Elastic but for some reason I don't seem to be able to locate it.
If you could send an updated version (with schedule A list) I'll make sure to check this into repository.

@cowtowncoder cowtowncoder removed the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Jan 8, 2023
@cowtowncoder
Copy link
Member

Given that there should be CLA in effect & there will be updated one sent, I will merge this now.

@cowtowncoder cowtowncoder merged commit 9fd899d into FasterXML:2.14 Jan 8, 2023
@cowtowncoder cowtowncoder added this to the 2.14.2 milestone Jan 8, 2023
@pgomulka pgomulka mentioned this pull request Jan 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cowtowncoder cowtowncoder cowtowncoder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.14.2
Development

Successfully merging this pull request may close these issues.

Allow TokenFIlter to skip last elements in arrays
4 participants
@pgomulka @cowtowncoder @tvernum @pjfanning