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

NIFI-9677 Fixed LookupRecord issue that an empty JSON array caused mismatch #8266

Closed
wants to merge 3 commits into from

Conversation

jrsteinebrey
Copy link
Contributor

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

I added two new unit tests to verify my fix.
I also manually tested the new code works as intended in a local build of NiFi editor.

Build

  • Build completed using mvn clean install -P contrib-check
    • JDK 21

Licensing

N/A

Documentation

N/A

… considered unmatched even though it should be considered as a match.
@mattyb149 mattyb149 self-requested a review January 18, 2024 20:03
@mattyb149
Copy link
Contributor

Reviewing...

@greyp9 greyp9 requested a review from mattyb149 January 22, 2024 18:31
@mattyb149
Copy link
Contributor

When running the unit tests with Java 8 (for a support/nifi-1.x backport), I get an error because the foo entry shows up as null in the output of testLookupMissingJsonField():

[{"foo":null,"unmentioned":{"foo":"original"}}]

I checked some commits and everything looks the same, but I'm guessing there was some change made to the record-based stuff in main that didn't make its way back to the 1.x branch. Either way JSON is not guaranteed to have "ordered" fields, so we should figure out why the "foo": null is in there (if it shouldn't be) or check each field for its contents rather than the whole output FlowFile.

@mattyb149
Copy link
Contributor

+1 LGTM, thanks for the fix! Merging to support/nifi-1.x and main

@mattyb149 mattyb149 closed this in 59ff1b6 Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants