dan-s1 commented on code in PR #7665:
URL: https://github.com/apache/nifi/pull/7665#discussion_r1367170067


##########
nifi-nar-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/JsonRecordSource.java:
##########
@@ -29,6 +30,11 @@
 
 public class JsonRecordSource implements RecordSource<JsonNode> {
     private static final Logger logger = 
LoggerFactory.getLogger(JsonRecordSource.class);

Review Comment:
   If the added static final private variables below are uppercase, this should 
also be for consistency.



##########
nifi-nar-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/JsonParserFactory.java:
##########
@@ -23,27 +23,20 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Objects;
 
-public class JsonParserFactory implements TokenParserFactory{
-    private static final JsonFactory JSON_FACTORY = new JsonFactory();
-    private static final ObjectMapper JSON_MAPPER = new ObjectMapper();
-
+public class JsonParserFactory implements TokenParserFactory {
     @Override
-    public JsonParser getJsonParser(InputStream in) throws IOException {
-        JsonParser jsonParser = JSON_FACTORY.createParser(in);
-        jsonParser.setCodec(JSON_MAPPER);
-
-        return jsonParser;
-    }
+    public JsonParser getJsonParser(final InputStream in, final 
StreamReadConstraints streamReadConstraints, final boolean allowComments) 
throws IOException {
+        Objects.requireNonNull(in, "Input Stream required");
+        Objects.requireNonNull(streamReadConstraints, "Stream Read Constraints 
required");
 
-    @Override
-    public ObjectMapper createCodec(boolean allowComments, 
StreamReadConstraints streamReadConstraints) {
-        ObjectMapper codec = new ObjectMapper();
-        if(allowComments) {
-            codec.enable(JsonParser.Feature.ALLOW_COMMENTS);
+        final ObjectMapper objectMapper = new ObjectMapper();
+        
objectMapper.getFactory().setStreamReadConstraints(streamReadConstraints);
+        if (allowComments) {
+            objectMapper.enable(JsonParser.Feature.ALLOW_COMMENTS);
         }
-        codec.getFactory().setStreamReadConstraints(streamReadConstraints);
-
-        return codec;
+        final JsonFactory jsonFactory = objectMapper.getFactory();
+        return jsonFactory.createParser(in);

Review Comment:
   The previous code in `AbstractJsonRowRecordReader.java` formally on line 146 
when creating the jsonParser also called `setCodec`.  Shouldn't you have 
something similar here?
   Instead of
   ` return jsonFactory.createParser(in);`
   
   change to
   ```
   final JsonParser jsonParser = jsonFactory.createParser(in);
   jsonParser.setCodec(objectMapper);
   return jsonParser;
   ```



##########
nifi-nar-bundles/nifi-extension-utils/nifi-record-utils/nifi-json-record-utils/src/main/java/org/apache/nifi/json/TokenParserFactory.java:
##########
@@ -18,13 +18,19 @@
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.StreamReadConstraints;
-import com.fasterxml.jackson.databind.ObjectMapper;
 
 import java.io.IOException;
 import java.io.InputStream;
 
 public interface TokenParserFactory {
-    JsonParser getJsonParser(InputStream in) throws IOException;
-
-    ObjectMapper createCodec(boolean allowComments, StreamReadConstraints 
streamReadConstraints);
+    /**
+     * Get JSON Parser implementation for provided Input Stream with 
configured settings
+     *
+     * @param in Input Stream to be parsed
+     * @param streamReadConstraints Stream Read Constraints applied
+     * @param allowComments Whether to allow comments when parsing
+     * @return JSON Parser
+     * @throws IOException Thrown on failures ot read the Input Stream

Review Comment:
   Instead of
   `@throws IOException Thrown on failures ot read the Input Stream`
   this should be
   `@throws IOException Thrown on failures to read the Input Stream`



-- 
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-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to