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