NIFI-6304 added trim, toLowerCase and toUpperCase to record path oper…#3478
NIFI-6304 added trim, toLowerCase and toUpperCase to record path oper…#3478MikeThomsen wants to merge 6 commits intoapache:masterfrom
Conversation
ijokarumawak
left a comment
There was a problem hiding this comment.
Thanks for the improvements @MikeThomsen !
I think we should separate Concat and these added functions. Please see my comments for detail.
...commons/nifi-record-path/src/main/java/org/apache/nifi/record/path/functions/TrimString.java
Outdated
Show resolved
Hide resolved
...ommons/nifi-record-path/src/main/java/org/apache/nifi/record/path/functions/ToUpperCase.java
Outdated
Show resolved
Hide resolved
...ommons/nifi-record-path/src/main/java/org/apache/nifi/record/path/functions/ToLowerCase.java
Outdated
Show resolved
Hide resolved
...-record-path/src/main/java/org/apache/nifi/record/path/functions/AbstractStringFunction.java
Outdated
Show resolved
Hide resolved
...-record-path/src/main/java/org/apache/nifi/record/path/functions/AbstractStringFunction.java
Outdated
Show resolved
Hide resolved
|
Moved this comment to here |
...ons/nifi-record-path/src/main/java/org/apache/nifi/record/path/paths/RecordPathCompiler.java
Outdated
Show resolved
Hide resolved
|
@ijokarumawak I think all of your concerns have been addressed now. |
There was a problem hiding this comment.
Thanks @MikeThomsen for the updates and documentation!
I still think these functions should convert multiple FieldValues as multiple FieldValues. Applying function to each fields passed through the stream. To explain what exact I'm imagining, I've submitted a PR to your branch. Please take a look. MikeThomsen#3 (Also, the PR addresses all of following comments)
...ons/nifi-record-path/src/main/java/org/apache/nifi/record/path/paths/RecordPathCompiler.java
Show resolved
Hide resolved
nifi-commons/nifi-record-path/src/main/java/org/apache/nifi/record/path/functions/Concat.java
Show resolved
Hide resolved
...ommons/nifi-record-path/src/main/java/org/apache/nifi/record/path/functions/ToLowerCase.java
Outdated
Show resolved
Hide resolved
NIFI-6304 Refactored to make it simpler
|
@ijokarumawak merged your changes and reverted |
|
@MikeThomsen LGTM +1. Thanks for the updates, all work as expected including combined functions such as |
…ations.
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.