exceptionfactory commented on code in PR #7509:
URL: https://github.com/apache/nifi/pull/7509#discussion_r1270634538
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -122,11 +139,18 @@ public class JSLTTransformJSON extends AbstractProcessor {
private static final List<PropertyDescriptor> descriptors;
private static final Set<Relationship> relationships;
private static final ObjectMapper jsonObjectMapper = new ObjectMapper();
+ private static final ObjectMapper jsonObjectMapperPerObject = new
ObjectMapper();
Review Comment:
Is there a reason for this second ObjectMapper?
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -122,11 +139,18 @@ public class JSLTTransformJSON extends AbstractProcessor {
private static final List<PropertyDescriptor> descriptors;
private static final Set<Relationship> relationships;
private static final ObjectMapper jsonObjectMapper = new ObjectMapper();
+ private static final ObjectMapper jsonObjectMapperPerObject = new
ObjectMapper();
+
+ private static final JsonFactory jsonFactory = new JsonFactory();
+ private JsonParser jsonParser;
+ private JsonNode firstJsonNode;
+ private boolean firstObjectConsumed = false;
Review Comment:
This seems like it could introduce thread-safety issues with multiple
concurrent tasks. These first JSON Node and object consumed properties should
be local to methods and passed around.
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -277,4 +329,36 @@ private String readTransform(final PropertyValue
propertyValue) {
throw new UncheckedIOException("Read JSLT Transform failed", e);
}
}
-}
+
+
+ protected JsonNode getNextJsonNode(final String transformStrategy) throws
IOException {
+ if (!firstObjectConsumed) {
+ firstObjectConsumed = true;
+ return firstJsonNode;
+ }
+ if
(APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE.getValue().equals(transformStrategy)) {
+ return null;
+ }
+ return getJsonNode();
+ }
+
+ private JsonNode getJsonNode() throws IOException {
Review Comment:
The JsonParser instance should be passed to this method.
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -79,6 +84,9 @@
+ "fails, the original FlowFile is routed to the 'failure'
relationship.")
public class JSLTTransformJSON extends AbstractProcessor {
+ public static final AllowableValue APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE =
new AllowableValue("Entire FlowFile", "Entire FlowFile", "Entire FlowFile");
+ public static final AllowableValue APPLY_TRANSFORM_TO_EACH_OBJECT = new
AllowableValue("Each JSON Object", "Each JSON Object", "Each JSON Object");
Review Comment:
Using an enum implementing DescribedValue can make it easier to compare
values later on, so it might be worth changing the options, although this is
limited in scope and works as it stands.
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -122,11 +139,18 @@ public class JSLTTransformJSON extends AbstractProcessor {
private static final List<PropertyDescriptor> descriptors;
private static final Set<Relationship> relationships;
private static final ObjectMapper jsonObjectMapper = new ObjectMapper();
+ private static final ObjectMapper jsonObjectMapperPerObject = new
ObjectMapper();
+
+ private static final JsonFactory jsonFactory = new JsonFactory();
+ private JsonParser jsonParser;
Review Comment:
This JsonParser usage is not thread-safe.
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -277,4 +329,36 @@ private String readTransform(final PropertyValue
propertyValue) {
throw new UncheckedIOException("Read JSLT Transform failed", e);
}
}
-}
+
+
+ protected JsonNode getNextJsonNode(final String transformStrategy) throws
IOException {
Review Comment:
This is where it would be helpful to use the `enum` value for the selected
strategy.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
