You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "dan-s1 (via GitHub)" <gi...@apache.org> on 2025/03/06 18:05:12 UTC
[PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON. [nifi]
dan-s1 opened a new pull request, #9781:
URL: https://github.com/apache/nifi/pull/9781
<!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
<!-- contributor license agreements. See the NOTICE file distributed with -->
<!-- this work for additional information regarding copyright ownership. -->
<!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
<!-- (the "License"); you may not use this file except in compliance with -->
<!-- the License. You may obtain a copy of the License at -->
<!-- http://www.apache.org/licenses/LICENSE-2.0 -->
<!-- Unless required by applicable law or agreed to in writing, software -->
<!-- distributed under the License is distributed on an "AS IS" BASIS, -->
<!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
<!-- See the License for the specific language governing permissions and -->
<!-- limitations under the License. -->
# Summary
[NIFI-14331](https://issues.apache.org/jira/browse/NIFI-14331)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### 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
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 21
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #9781:
URL: https://github.com/apache/nifi/pull/9781#discussion_r2060470067
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestConvertRecord.java:
##########
@@ -434,7 +435,10 @@ public void testJSONDroppingUnknownFields() throws InitializationException, IOEx
runner.assertAllFlowFilesTransferred(ConvertRecord.REL_SUCCESS, 1);
final MockFlowFile flowFile = runner.getFlowFilesForRelationship(ConvertRecord.REL_SUCCESS).getFirst();
- assertFalse(flowFile.getContent().contains("fieldThatShouldBeRemoved"));
+
+ // This covers all the cases
+ // "undefinedKey", "undefinedObjectArray", "undefinedObject", "undefinedScalarArray"
Review Comment:
@mosermw Thanks for pointing that out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #9781:
URL: https://github.com/apache/nifi/pull/9781#discussion_r1985145491
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml:
##########
@@ -632,6 +632,7 @@
<exclude>src/test/resources/TestConvertRecord/input/person_dropfield.json</exclude>
Review Comment:
@pvillard31 Good catch! I just made the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on PR #9781:
URL: https://github.com/apache/nifi/pull/9781#issuecomment-2721369252
@pvillard31 When you have a chance can you please review? Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on PR #9781:
URL: https://github.com/apache/nifi/pull/9781#issuecomment-2761409555
@exceptionfactory Can you please take a look at this PR? My team has a flow which is experiencing this bug and it would be great to have this fix. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "pvillard31 (via GitHub)" <gi...@apache.org>.
pvillard31 commented on code in PR #9781:
URL: https://github.com/apache/nifi/pull/9781#discussion_r1985069844
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml:
##########
@@ -632,6 +632,7 @@
<exclude>src/test/resources/TestConvertRecord/input/person_dropfield.json</exclude>
Review Comment:
Not sure why this is not raised by the RAT check but this should be updated.
```suggestion
<exclude>src/test/resources/TestConvertRecord/input/personJob_dropfield.json</exclude>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "mosermw (via GitHub)" <gi...@apache.org>.
mosermw commented on code in PR #9781:
URL: https://github.com/apache/nifi/pull/9781#discussion_r2060435674
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestConvertRecord.java:
##########
@@ -434,7 +435,10 @@ public void testJSONDroppingUnknownFields() throws InitializationException, IOEx
runner.assertAllFlowFilesTransferred(ConvertRecord.REL_SUCCESS, 1);
final MockFlowFile flowFile = runner.getFlowFilesForRelationship(ConvertRecord.REL_SUCCESS).getFirst();
- assertFalse(flowFile.getContent().contains("fieldThatShouldBeRemoved"));
+
+ // This covers all the cases
+ // "undefinedKey", "undefinedObjectArray", "undefinedObject", "undefinedScalarArray"
Review Comment:
Thanks for the updates @dan-s1 but this list is missing "undefinedKeyInObject".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "mosermw (via GitHub)" <gi...@apache.org>.
mosermw commented on PR #9781:
URL: https://github.com/apache/nifi/pull/9781#issuecomment-2831098631
I've been racking my brain wondering if the JsonNode `deepCopy()` is necessary and I cannot think of any reason. And since this PR removes it and that resolves the described problem, I'm in favor of removing it.
So +1 from me, I will merge once my local build passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on PR #9781:
URL: https://github.com/apache/nifi/pull/9781#issuecomment-2831115809
@mosermw Thank you for taking the time to review and approve!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "mosermw (via GitHub)" <gi...@apache.org>.
mosermw commented on code in PR #9781:
URL: https://github.com/apache/nifi/pull/9781#discussion_r2059261481
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestConvertRecord.java:
##########
@@ -434,7 +435,10 @@ public void testJSONDroppingUnknownFields() throws InitializationException, IOEx
runner.assertAllFlowFilesTransferred(ConvertRecord.REL_SUCCESS, 1);
final MockFlowFile flowFile = runner.getFlowFilesForRelationship(ConvertRecord.REL_SUCCESS).getFirst();
- assertFalse(flowFile.getContent().contains("fieldThatShouldBeRemoved"));
+ final List<String> undefinedTopLevelKeys = List.of("undefinedKey", "undefinedObjectArray", "undefinedObject", "undefinedScalarArray");
+
+ undefinedTopLevelKeys.forEach(undefinedTopLevelKey -> assertFalse(flowFile.getContent().contains(undefinedTopLevelKey)));
Review Comment:
Could this be simplified to a single assert that the string "undefined" is not in the result? Perhaps you could add a comment stating this covers all of the cases "undefinedKey", "undefinedObjectArray", "undefinedObject", "undefinedScalarArray", "undefinedKeyInObject"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter. [nifi]
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #9781: NIFI-14331 Allow for removing all undefined elements (not only top level keys) from the JSON when using JsonTreeReader and JsonRecordSetWriter.
URL: https://github.com/apache/nifi/pull/9781
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org