Skip to content

NIFI-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. - #11208

Merged
exceptionfactory merged 8 commits into
apache:mainfrom
dan-s1:NIFI-12456
Jun 2, 2026
Merged

NIFI-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently.#11208
exceptionfactory merged 8 commits into
apache:mainfrom
dan-s1:NIFI-12456

Conversation

@dan-s1

@dan-s1 dan-s1 commented May 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

NIFI-12456
The changes in this PR now allow for JsonTreeReader and JsonPathReader to parse incoming JSON leniently which includes allowing

  • Java comments (/**/ and //)
  • Yaml comments (start of line begins with #)
  • Leading plus sign in numbers (e.g. +123)
  • Leading zeros in numbers (e.g. 0001)
  • "missing" decimal numbers to end with a decimal point (e.g. 123.)
  • "missing value" in an array (i.e. sequence of two commas, without value in-between e.g. ["A",,"C"]),
  • Trailing comma in an array or member in an object
  • Use of single quotes for quoting strings (i.e. use of an apostrophe)
  • Use of unquoted field names
  • Use of unescaped control characters (ASCII characters with value less than 32)

Tracking

Please complete the following tracking steps prior to pull request creation.

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 contains commits signed with a registered key indicating Verified status

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

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using ./mvnw clean install -P contrib-check
    • JDK 21
    • JDK 25

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@exceptionfactory
exceptionfactory self-requested a review May 5, 2026 17:42
exceptionfactory
exceptionfactory requested changes May 6, 2026
View reviewed changes

@exceptionfactory exceptionfactory left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this configuration option @dan-s1. The general approach looks clear, I provided some recommendations on naming.

Comment thread ...s/nifi-json-record-utils/src/main/java/org/apache/nifi/json/AbstractJsonRowRecordReader.java Outdated
Comment thread ...fi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/JsonParseMode.java Outdated
Comment thread ...ecord-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/JsonParserFactory.java Outdated
@dan-s1

dan-s1 commented May 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@exceptionfactory I have made all the changes you requested. I am concerned though that the configuration for the YamlTreeReader is now awkward since it extends JsonTreeReader. The property Parsing Strategy is not relevant to YamlTreeReader since Yaml parsing does not support these features. Even the original Allow Comment does not make sense since SnakeYaml (the implementation we use for Yaml parsing) supports parsing comments when one plans to preserve them, but by default it ignores comments as seen in the javadocs. Hence since we are not preserving them we shouldn't need any option to keep them.
The whole reason we had YamlTreeReader extend JsonTreeReader was to not have to duplicate code as the JSON code could drive the Yaml parsing. But the property Parsing Strategy should not be included.

Is the only way possible to remove Parsing Strategy in getSupportedPropertyDescriptors() with code like below?

@Override
    protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
        
        final List<PropertyDescriptor> supportedPropertyDescriptors = new ArrayList<>(super.getSupportedPropertyDescriptors());
        supportedPropertyDescriptors.remove(AbstractJsonRowRecordReader.PARSING_STRATEGY);
        
        return  supportedPropertyDescriptors;
    }

I am assuming I cannot simply remove them in migrateProperties.

So I added that code. Please let me know if that is okay.

@dan-s1
dan-s1 requested a review from exceptionfactory May 11, 2026 13:11
@dan-s1

dan-s1 commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

@exceptionfactory I believe I addressed all the issues you had brought up. Is there anything else you would like me to change for this PR?

exceptionfactory
exceptionfactory requested changes May 17, 2026
View reviewed changes

@exceptionfactory exceptionfactory left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the updates and also adjusting the YAML Reader. The general approach looks good. I noted a few remaining minor things, and then this should be ready to go.

Comment thread ...le/nifi-record-serialization-services/src/main/java/org/apache/nifi/json/JsonPathReader.java Outdated
Comment thread ...le/nifi-record-serialization-services/src/main/java/org/apache/nifi/json/JsonTreeReader.java Outdated
Comment thread ...le/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java
@dan-s1
dan-s1 requested a review from exceptionfactory May 18, 2026 17:44
@dan-s1

dan-s1 commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@exceptionfactory On the ticket in Jira, there was a request to also add ALLOW_UNESCAPED_CONTROL_CHARS. Is that something you think we should also add?

@exceptionfactory

exceptionfactory commented May 18, 2026

Copy link
Copy Markdown
Contributor

@exceptionfactory On the ticket in Jira, there was a request to also add ALLOW_UNESCAPED_CONTROL_CHARS. Is that something you think we should also add?

Yes, that seems worth adding as part of this initial implementation.

exceptionfactory
exceptionfactory approved these changes Jun 2, 2026
View reviewed changes

@exceptionfactory exceptionfactory left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working through the feedback @dan-s1, the latest version looks good. +1 merging

@exceptionfactory
exceptionfactory merged commit 041ae48 into apache:main Jun 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@exceptionfactory exceptionfactory exceptionfactory approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dan-s1 @exceptionfactory