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 2026/05/05 16:57:03 UTC

[PR] NIFI-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

dan-s1 opened a new pull request, #11208:
URL: https://github.com/apache/nifi/pull/11208

   <!-- 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-12456](https://issues.apache.org/jira/browse/NIFI-12456)
   
   # 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 contains [commits signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) 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](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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3260859075


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   Yes since the `super.migrateProperties(config);` creates it on lines 146-153 if `AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS` is set.



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on PR #11208:
URL: https://github.com/apache/nifi/pull/11208#issuecomment-4444306720

   @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? 


-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3261113446


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   Thanks for clarifying, I see how this could get added now. Can you add a comment indicating something like, `// Remove Parsing Strategy from potential addition through the parent JsonTreeReader property migration`?



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3204174165


##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/AbstractJsonRowRecordReader.java:
##########
@@ -71,6 +73,14 @@ public abstract class AbstractJsonRowRecordReader implements RecordReader {
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
 
+    public static final PropertyDescriptor JSON_PARSE_MODE = new PropertyDescriptor.Builder()
+            .name("JSON Parse Mode")
+            .description("Specifies whether to parse the incoming JSON per the JSON specification or allow for lenient parsing")

Review Comment:
   That's a good point, I meant to convey the level of conformity to the specification, so maybe something like "Set the strategy for the level of JSON specification conformity required"



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3202233293


##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/AbstractJsonRowRecordReader.java:
##########
@@ -71,6 +73,14 @@ public abstract class AbstractJsonRowRecordReader implements RecordReader {
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
 
+    public static final PropertyDescriptor JSON_PARSE_MODE = new PropertyDescriptor.Builder()
+            .name("JSON Parse Mode")
+            .description("Specifies whether to parse the incoming JSON per the JSON specification or allow for lenient parsing")

Review Comment:
   Okay. The wording I am not sure is accurate as if lenient is chosen, it will not be according to the JSON specifications. Jackson specifically says in their Javadoc, that the leniencies are not per the JSON specification.



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3261098826


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   The `migrateProperties()` method is only needed when upgrading an existing component, so I'm not following how the `YamlTreeReader` would need to remove `PARSING_STRATEGY` when it has not yet been merged. Can you explain the relationship a bit more?



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3261117249


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   Sure!



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on PR #11208:
URL: https://github.com/apache/nifi/pull/11208#issuecomment-4481275794

   > @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.


-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on PR #11208:
URL: https://github.com/apache/nifi/pull/11208#issuecomment-4408180275

   @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 properties `Max String Length` and `Parsing Strategy` do not really make sense since for Yaml parsing it really does not support these equivalent options. Even the original `Allow Comment `does not make sense since SnakeYaml (the implementation we use for Yaml parsing) supports parsing comments that is when one plans to preserve them, but by default it ignores comments.  Hence since we are not preserving them we shouldn't need any option to keep them. This leads to awkward code as I would like exclude these property descriptors in `YamlTreeReader` but the only way I can remove them is with code like this:
   
   ```
   @Override
       protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
           
           final List<PropertyDescriptor> supportedPropertyDescriptors = new ArrayList<>(super.getSupportedPropertyDescriptors());
           supportedPropertyDescriptors.remove(AbstractJsonRowRecordReader.MAX_STRING_LENGTH);
           supportedPropertyDescriptors.remove(AbstractJsonRowRecordReader.PARSING_STRATEGY);
           
           return  supportedPropertyDescriptors;
       }
   ```
   
   The whole reason we had `YamlTreeReader`  extends `JsonTreeReader` was to not have to duplicate code as the JSON code could drive the Yaml parsing. But there are parts like these properties we do not want. Would the above code be acceptable?


-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on PR #11208:
URL: https://github.com/apache/nifi/pull/11208#issuecomment-4481113391

   @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?


-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3260859075


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   Yes since the `super.migrateProperties(config);` creates it on lines 146-153 if `AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS` is set which YamlTreeReader could have set set also.



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3260859075


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   Yes since the `super.migrateProperties(config);` creates it on lines 146-153 if `AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS` is set and which YamlTreeReader could have set set also.



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3253921690


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/json/JsonPathReader.java:
##########
@@ -152,6 +154,23 @@ protected Collection<ValidationResult> customValidate(final ValidationContext va
             .build());
     }
 
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+
+        if (config.isPropertySet(AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS)) {
+            final String allowCommentsRawValue = config.getRawPropertyValue(AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS).orElse("");

Review Comment:
   It looks like this should fall back to `"false"` to reflect the default behavior.
   ```suggestion
               final String allowCommentsRawValue = config.getRawPropertyValue(AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS).orElse(Boolean.FALSE.toString());
   ```



##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   Is this necessary? It is not part of the existing version, so it should not need to be removed in the migrate method.



##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/json/JsonTreeReader.java:
##########
@@ -142,10 +142,24 @@ public void migrateProperties(PropertyConfiguration config) {
         config.renameProperty("starting-field-name", STARTING_FIELD_NAME.getName());
         config.renameProperty("schema-application-strategy", SCHEMA_APPLICATION_STRATEGY.getName());
         config.renameProperty(OBSOLETE_SCHEMA_CACHE, SCHEMA_CACHE.getName());
+
+        if (config.isPropertySet(AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS)) {
+            final String allowCommentsRawValue = config.getRawPropertyValue(AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS).orElse("");

Review Comment:
   ```suggestion
               final String allowCommentsRawValue = config.getRawPropertyValue(AbstractJsonRowRecordReader.OBSOLETE_ALLOW_COMMENTS).orElse(Boolean.FALSE.toString());
   ```



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3202233293


##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/AbstractJsonRowRecordReader.java:
##########
@@ -71,6 +73,14 @@ public abstract class AbstractJsonRowRecordReader implements RecordReader {
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
 
+    public static final PropertyDescriptor JSON_PARSE_MODE = new PropertyDescriptor.Builder()
+            .name("JSON Parse Mode")
+            .description("Specifies whether to parse the incoming JSON per the JSON specification or allow for lenient parsing")

Review Comment:
   Okay. The wording I am not sure is accurate as if lenient is chosen it will not be according to the JSON specifications.



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3197754449


##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/JsonParseMode.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.json;
+
+import org.apache.nifi.components.DescribedValue;
+
+enum JsonParseMode implements DescribedValue {

Review Comment:
   ```suggestion
   enum ParsingStrategy implements DescribedValue {
   ```



##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/JsonParserFactory.java:
##########
@@ -42,14 +42,22 @@ public JsonParserFactory() {
      * JSON Parser Factory constructor with configurable constraints
      *
      * @param streamReadConstraints Stream Read Constraints
-     * @param allowComments Allow Comments during parsing
+     * @param lenient Allow for parsing JSON leniently
      */
-    public JsonParserFactory(final StreamReadConstraints streamReadConstraints, final boolean allowComments) {
+    public JsonParserFactory(final StreamReadConstraints streamReadConstraints, final boolean lenient) {

Review Comment:
   It looks like this should be changed to pass in the `ParsingStrategy` instead of the `boolean`



##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/AbstractJsonRowRecordReader.java:
##########
@@ -71,6 +73,14 @@ public abstract class AbstractJsonRowRecordReader implements RecordReader {
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
 
+    public static final PropertyDescriptor JSON_PARSE_MODE = new PropertyDescriptor.Builder()
+            .name("JSON Parse Mode")
+            .description("Specifies whether to parse the incoming JSON per the JSON specification or allow for lenient parsing")

Review Comment:
   I recommend naming this `Parsing Strategy` since `JSON` is implied.
   
   ```suggestion
       public static final PropertyDescriptor PARSING_STRATEGY = new PropertyDescriptor.Builder()
               .name("Parsing Strategy")
               .description("Set the strategy for handling input content according to JSON specifications")
   ```



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory merged PR #11208:
URL: https://github.com/apache/nifi/pull/11208


-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3261098826


##########
nifi-extension-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/yaml/YamlTreeReader.java:
##########
@@ -44,26 +46,28 @@
         + "See the Usage of the Controller Service for more information and examples.")
 public class YamlTreeReader extends JsonTreeReader {
 
-    private static final boolean ALLOW_COMMENTS_DISABLED = false;
+    @Override
+    public void migrateProperties(PropertyConfiguration config) {
+        super.migrateProperties(config);
+        config.removeProperty(AbstractJsonRowRecordReader.PARSING_STRATEGY.getName());

Review Comment:
   The `migrateProperties()` method is only needed when upgrading an existing component, so I'm not following how the `YamlTreeReader` would need to remove `PARSING_STRATEGY` when it has not yet been merged. Can you explain the relationship a bit more?



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3202233293


##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/AbstractJsonRowRecordReader.java:
##########
@@ -71,6 +73,14 @@ public abstract class AbstractJsonRowRecordReader implements RecordReader {
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
 
+    public static final PropertyDescriptor JSON_PARSE_MODE = new PropertyDescriptor.Builder()
+            .name("JSON Parse Mode")
+            .description("Specifies whether to parse the incoming JSON per the JSON specification or allow for lenient parsing")

Review Comment:
   Okay. The wording I am not sure is accurate as if lenient is chosen, it will not be according to the JSON specifications. Jackson specifically says in their [Javadoc](https://www.javadoc.io/static/com.fasterxml.jackson.core/jackson-core/2.21.2/com/fasterxml/jackson/core/json/JsonReadFeature.html), that the leniencies are not per the JSON specification.



-- 
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-12456 Enabled JsonTreeReader and JsonPathReader to parse JSON leniently. [nifi]

Posted by "dan-s1 (via GitHub)" <gi...@apache.org>.
dan-s1 commented on code in PR #11208:
URL: https://github.com/apache/nifi/pull/11208#discussion_r3202233293


##########
nifi-extension-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/AbstractJsonRowRecordReader.java:
##########
@@ -71,6 +73,14 @@ public abstract class AbstractJsonRowRecordReader implements RecordReader {
             .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
             .build();
 
+    public static final PropertyDescriptor JSON_PARSE_MODE = new PropertyDescriptor.Builder()
+            .name("JSON Parse Mode")
+            .description("Specifies whether to parse the incoming JSON per the JSON specification or allow for lenient parsing")

Review Comment:
   Okay. The wording I am not sure is accurate as if lenient is chosen, it will not be according to the JSON specifications. Jackson specifically says in their Javadoc, that the lenieincies are not per the JSON specification.



-- 
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