You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "mattyb149 (via GitHub)" <gi...@apache.org> on 2023/07/20 22:56:40 UTC
[GitHub] [nifi] mattyb149 opened a new pull request, #7509: NIFI-11830: Add ability to apply JSLT transform to each record
mattyb149 opened a new pull request, #7509:
URL: https://github.com/apache/nifi/pull/7509
# Summary
[NIFI-11830](https://issues.apache.org/jira/browse/NIFI-11830) This PR adds a "Transformation Strategy" property to JSLTTransformJSON to allow the user to choose whether to apply the JSLT transform to the entire JSON input or to each JSON object in a top-level array.
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-11830`
- [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-11830`
### Pull Request Formatting
- [x] Pull Request based on current revision of the `main` branch
- [x] 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`
- [x] JDK 17
### 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
- [x] 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
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7509: NIFI-11830: Add ability to apply JSLT transform to each record
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory closed pull request #7509: NIFI-11830: Add ability to apply JSLT transform to each record
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory closed pull request #7509: NIFI-11830: Add ability to apply JSLT transform to each record
URL: https://github.com/apache/nifi/pull/7509
--
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
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7509: NIFI-11830: Add ability to apply JSLT transform to each record
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #7509:
URL: https://github.com/apache/nifi/pull/7509#discussion_r1272582318
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -202,40 +219,73 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
return;
}
+ final TransformStrategy transformStrategy = TransformStrategy.valueOf(context.getProperty(TRANSFORM_STRATEGY).getValue());
final StopWatch stopWatch = new StopWatch(true);
- final JsonNode jsonNode;
- try (final InputStream in = session.read(original)) {
- jsonNode = readJson(in);
- } catch (final Exception e) {
- getLogger().error("JSLT Transform failed {}", original, e);
- session.transfer(original, REL_FAILURE);
- return;
- }
-
final PropertyValue transformProperty = context.getProperty(JSLT_TRANSFORM);
+ FlowFile transformed;
+ final JsonFactory jsonFactory = new JsonFactory();
try {
final String transform = readTransform(transformProperty, original);
final Expression jsltExpression = transformCache.get(transform, currString -> Parser.compileString(transform));
+ final boolean prettyPrint = context.getProperty(PRETTY_PRINT).asBoolean();
- final JsonNode transformedJson = jsltExpression.apply(jsonNode);
- final ObjectWriter writer = context.getProperty(PRETTY_PRINT).asBoolean() ? jsonObjectMapper.writerWithDefaultPrettyPrinter() : jsonObjectMapper.writer();
- final Object outputObject;
- if (transformedJson == null || transformedJson.isNull()) {
- getLogger().warn("JSLT Transform resulted in no data {}", original);
- outputObject = null;
- } else {
- outputObject = transformedJson;
- }
- FlowFile transformed = session.write(original, out -> {
- if (outputObject != null) {
- writer.writeValue(out, outputObject);
+ transformed = session.write(original, (inputStream, outputStream) -> {
+ boolean topLevelArray = false;
+ JsonParser jsonParser;
+ JsonNode firstJsonNode;
+ if (APPLY_TRANSFORM_TO_EACH_OBJECT.equals(transformStrategy)) {
+ jsonParser = jsonFactory.createParser(inputStream);
+ jsonParser.setCodec(jsonObjectMapper);
+
+ JsonToken token = jsonParser.nextToken();
+ if (token == JsonToken.START_ARRAY) {
+ token = jsonParser.nextToken(); // advance to START_OBJECT token
+ topLevelArray = true;
+ }
+ if (token == JsonToken.START_OBJECT) { // could be END_ARRAY also
+ firstJsonNode = jsonParser.readValueAsTree();
+ } else {
+ firstJsonNode = null;
+ }
+ } else {
+ firstJsonNode = readJson(inputStream);
+ jsonParser = null; // This will not be used when applying the transform to the entire FlowFile
+ }
+
+ final ObjectWriter writer = prettyPrint ? jsonObjectMapper.writerWithDefaultPrettyPrinter() : jsonObjectMapper.writer();
+ final JsonGenerator jsonGenerator = writer.createGenerator(outputStream);
+
+ Object outputObject;
+ JsonNode nextNode;
+
+ if (topLevelArray) {
+ jsonGenerator.writeStartArray();
}
+ nextNode = firstJsonNode;
+ do {
+ final JsonNode transformedJson = jsltExpression.apply(nextNode);
+ if (transformedJson == null || transformedJson.isNull()) {
+ getLogger().warn("JSLT Transform resulted in no data {}", original);
Review Comment:
This could be repetitive with a large number of records, perhaps set a flag an log once after the loop?
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -277,4 +327,68 @@ private String readTransform(final PropertyValue propertyValue) {
throw new UncheckedIOException("Read JSLT Transform failed", e);
}
}
-}
+
+
+ protected JsonNode getNextJsonNode(final TransformStrategy transformStrategy, final JsonParser jsonParser) throws IOException {
+
+ if (APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE.equals(transformStrategy)) {
+ return null;
+ }
+ return getJsonNode(jsonParser);
+ }
+
+ private JsonNode getJsonNode(JsonParser jsonParser) throws IOException {
+ while (true) {
+ final JsonToken token = jsonParser.nextToken();
+ if (token == null) {
+ return null;
+ }
+
+ switch (token) {
+ case START_ARRAY:
+ case END_ARRAY:
+ case END_OBJECT:
+ break;
+ case START_OBJECT:
+ return jsonParser.readValueAsTree();
+ default:
+ throw new IOException("Expected to get a JSON Object but got a token of type " + token.name());
+ }
+ }
+ }
+
+ /**
+ * Enumeration of supported Output Strategies
+ */
+ enum TransformStrategy implements DescribedValue {
+ APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE("APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE", "Entire FlowFile", "Entire FlowFile"),
+ APPLY_TRANSFORM_TO_EACH_OBJECT("APPLY_TRANSFORM_TO_EACH_OBJECT", "Each JSON Object", "Each JSON Object");
+
+ private final String value;
+
+ private final String displayName;
+
+ private final String description;
+
+ TransformStrategy(final String value, final String displayName, final String description) {
+ this.value = value;
+ this.displayName = displayName;
+ this.description = description;
+ }
+
+ @Override
+ public String getValue() {
+ return value;
Review Comment:
```suggestion
return name();
```
##########
nifi-nar-bundles/nifi-jslt-bundle/nifi-jslt-processors/src/main/java/org/apache/nifi/processors/jslt/JSLTTransformJSON.java:
##########
@@ -277,4 +327,68 @@ private String readTransform(final PropertyValue propertyValue) {
throw new UncheckedIOException("Read JSLT Transform failed", e);
}
}
-}
+
+
+ protected JsonNode getNextJsonNode(final TransformStrategy transformStrategy, final JsonParser jsonParser) throws IOException {
+
+ if (APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE.equals(transformStrategy)) {
+ return null;
+ }
+ return getJsonNode(jsonParser);
+ }
+
+ private JsonNode getJsonNode(JsonParser jsonParser) throws IOException {
+ while (true) {
+ final JsonToken token = jsonParser.nextToken();
+ if (token == null) {
+ return null;
+ }
+
+ switch (token) {
+ case START_ARRAY:
+ case END_ARRAY:
+ case END_OBJECT:
+ break;
+ case START_OBJECT:
+ return jsonParser.readValueAsTree();
+ default:
+ throw new IOException("Expected to get a JSON Object but got a token of type " + token.name());
+ }
+ }
+ }
+
+ /**
+ * Enumeration of supported Output Strategies
+ */
+ enum TransformStrategy implements DescribedValue {
+ APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE("APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE", "Entire FlowFile", "Entire FlowFile"),
+ APPLY_TRANSFORM_TO_EACH_OBJECT("APPLY_TRANSFORM_TO_EACH_OBJECT", "Each JSON Object", "Each JSON Object");
+
+ private final String value;
Review Comment:
The value string does not need to be repeated and the `name()` method can be used inside `getValue()`:
```suggestion
APPLY_TRANSFORM_TO_ENTIRE_FLOWFILE("Entire FlowFile", "Entire FlowFile"),
APPLY_TRANSFORM_TO_EACH_OBJECT("Each JSON Object", "Each JSON Object");
```
--
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
[GitHub] [nifi] mattyb149 commented on a diff in pull request #7509: NIFI-11830: Add ability to apply JSLT transform to each record
Posted by "mattyb149 (via GitHub)" <gi...@apache.org>.
mattyb149 commented on code in PR #7509:
URL: https://github.com/apache/nifi/pull/7509#discussion_r1270879547
##########
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:
Not sure, I was trying different things when getting the per-object part working. I will remove it and see what happens :)
--
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