exceptionfactory commented on code in PR #9785:
URL: https://github.com/apache/nifi/pull/9785#discussion_r2001029968
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -55,11 +57,31 @@
@Tags({"json", "jolt", "transform", "shiftr", "chainr", "defaultr", "removr",
"cardinality", "sort"})
@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
@WritesAttribute(attribute = "mime.type", description = "Always set to
application/json")
-@CapabilityDescription("Applies a list of Jolt specifications to the flowfile
JSON payload. A new FlowFile is created "
- + "with transformed content and is routed to the 'success'
relationship. If the JSON transform "
- + "fails, the original FlowFile is routed to the 'failure'
relationship.")
+@CapabilityDescription("Applies a list of JOLT specifications to either the
FlowFile JSON content or a specified FlowFile JSON attribute. "
+ + "When 'Json Source' is set to FLOW_FILE, the FlowFile content is
transformed and the modified FlowFile is routed to the 'success' relationship. "
+ + "When 'Json Source' is set to ATTRIBUTE, the specified attribute's
value is transformed and updated in place, with the FlowFile routed to the
'success' relationship. "
+ + "If the JSON transform fails, the original FlowFile is routed to the
'failure' relationship.")
@RequiresInstanceClassLoading
public class JoltTransformJSON extends AbstractJoltTransform {
+
+ public static final PropertyDescriptor JSON_SOURCE = new
PropertyDescriptor.Builder()
+ .name("Json Source")
Review Comment:
`JSON` should be all uppercase in property names:
```suggestion
.name("JSON Source")
```
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -55,11 +57,31 @@
@Tags({"json", "jolt", "transform", "shiftr", "chainr", "defaultr", "removr",
"cardinality", "sort"})
@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
@WritesAttribute(attribute = "mime.type", description = "Always set to
application/json")
-@CapabilityDescription("Applies a list of Jolt specifications to the flowfile
JSON payload. A new FlowFile is created "
- + "with transformed content and is routed to the 'success'
relationship. If the JSON transform "
- + "fails, the original FlowFile is routed to the 'failure'
relationship.")
+@CapabilityDescription("Applies a list of JOLT specifications to either the
FlowFile JSON content or a specified FlowFile JSON attribute. "
+ + "When 'Json Source' is set to FLOW_FILE, the FlowFile content is
transformed and the modified FlowFile is routed to the 'success' relationship. "
+ + "When 'Json Source' is set to ATTRIBUTE, the specified attribute's
value is transformed and updated in place, with the FlowFile routed to the
'success' relationship. "
+ + "If the JSON transform fails, the original FlowFile is routed to the
'failure' relationship.")
@RequiresInstanceClassLoading
public class JoltTransformJSON extends AbstractJoltTransform {
+
+ public static final PropertyDescriptor JSON_SOURCE = new
PropertyDescriptor.Builder()
+ .name("Json Source")
+ .description("Specifies whether the JOLT transformation is applied
to FlowFile JSON content or to specified FlowFile JSON attribute.")
+ .required(true)
+ .allowableValues(SourceStrategy.class)
+ .defaultValue(SourceStrategy.FLOW_FILE)
+ .build();
+
+ public static final PropertyDescriptor JSON_SOURCE_ATTRIBUTE = new
PropertyDescriptor.Builder()
+ .name("Json Source Attribute")
Review Comment:
```suggestion
.name("JSON Source Attribute")
```
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -55,11 +57,31 @@
@Tags({"json", "jolt", "transform", "shiftr", "chainr", "defaultr", "removr",
"cardinality", "sort"})
@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
@WritesAttribute(attribute = "mime.type", description = "Always set to
application/json")
-@CapabilityDescription("Applies a list of Jolt specifications to the flowfile
JSON payload. A new FlowFile is created "
- + "with transformed content and is routed to the 'success'
relationship. If the JSON transform "
- + "fails, the original FlowFile is routed to the 'failure'
relationship.")
+@CapabilityDescription("Applies a list of JOLT specifications to either the
FlowFile JSON content or a specified FlowFile JSON attribute. "
+ + "When 'Json Source' is set to FLOW_FILE, the FlowFile content is
transformed and the modified FlowFile is routed to the 'success' relationship. "
+ + "When 'Json Source' is set to ATTRIBUTE, the specified attribute's
value is transformed and updated in place, with the FlowFile routed to the
'success' relationship. "
Review Comment:
This description is duplicative of the property description, so I recommend
removing it.
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -122,14 +146,34 @@ public void onTrigger(final ProcessContext context,
ProcessSession session) thro
final ComponentLog logger = getLogger();
final StopWatch stopWatch = new StopWatch(true);
-
final Object inputJson;
- try (final InputStream in = session.read(original)) {
- inputJson = jsonUtil.jsonToObject(in);
- } catch (final Exception e) {
- logger.error("JSON parsing failed for {}", original, e);
- session.transfer(original, REL_FAILURE);
- return;
+ final boolean isSourceFlowFileContent = SourceStrategy.FLOW_FILE ==
context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);
Review Comment:
Minor renaming recommendation:
```suggestion
final boolean sourceStrategyFlowFile = SourceStrategy.FLOW_FILE ==
context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);
```
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -55,11 +57,31 @@
@Tags({"json", "jolt", "transform", "shiftr", "chainr", "defaultr", "removr",
"cardinality", "sort"})
@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
@WritesAttribute(attribute = "mime.type", description = "Always set to
application/json")
-@CapabilityDescription("Applies a list of Jolt specifications to the flowfile
JSON payload. A new FlowFile is created "
- + "with transformed content and is routed to the 'success'
relationship. If the JSON transform "
- + "fails, the original FlowFile is routed to the 'failure'
relationship.")
+@CapabilityDescription("Applies a list of JOLT specifications to either the
FlowFile JSON content or a specified FlowFile JSON attribute. "
+ + "When 'Json Source' is set to FLOW_FILE, the FlowFile content is
transformed and the modified FlowFile is routed to the 'success' relationship. "
+ + "When 'Json Source' is set to ATTRIBUTE, the specified attribute's
value is transformed and updated in place, with the FlowFile routed to the
'success' relationship. "
+ + "If the JSON transform fails, the original FlowFile is routed to the
'failure' relationship.")
@RequiresInstanceClassLoading
public class JoltTransformJSON extends AbstractJoltTransform {
+
+ public static final PropertyDescriptor JSON_SOURCE = new
PropertyDescriptor.Builder()
+ .name("Json Source")
+ .description("Specifies whether the JOLT transformation is applied
to FlowFile JSON content or to specified FlowFile JSON attribute.")
Review Comment:
```suggestion
.description("Specifies whether the Jolt transformation is
applied to FlowFile JSON content or to specified FlowFile JSON attribute.")
```
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -122,14 +146,34 @@ public void onTrigger(final ProcessContext context,
ProcessSession session) thro
final ComponentLog logger = getLogger();
final StopWatch stopWatch = new StopWatch(true);
-
final Object inputJson;
- try (final InputStream in = session.read(original)) {
- inputJson = jsonUtil.jsonToObject(in);
- } catch (final Exception e) {
- logger.error("JSON parsing failed for {}", original, e);
- session.transfer(original, REL_FAILURE);
- return;
+ final boolean isSourceFlowFileContent = SourceStrategy.FLOW_FILE ==
context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);
+ String jsonSourceAttributeName = null;
+
+ if (isSourceFlowFileContent) {
+ try (final InputStream in = session.read(original)) {
+ inputJson = jsonUtil.jsonToObject(in);
+ } catch (final Exception e) {
+ logger.error("JSON parsing failed on FlowFile content for {}",
original, e);
+ session.transfer(original, REL_FAILURE);
+ return;
+ }
+ } else {
+ jsonSourceAttributeName =
context.getProperty(JSON_SOURCE_ATTRIBUTE).evaluateAttributeExpressions(original).getValue();
Review Comment:
Support for Expression Language raises as important question. Evaluating the
expression to return an attribute name could be confusing, since
`${attributeName}` would return the value JSON, not the attribute name itself.
One option is to remove support for Expression Language. The other option is to
change the property name to describe the JSON Source itself. This also impacts
the `JSON Source` property options. The options could be `FLOW_FILE` and
`SOURCE_REFERENCE` or similar, with `JSON Source Reference` supporting
Expression Language. The property naming needs some further consideration, as
I'm not sure `Source Reference` is as clear as it should be. Perhaps `JSON
Source Content`.
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -122,14 +146,34 @@ public void onTrigger(final ProcessContext context,
ProcessSession session) thro
final ComponentLog logger = getLogger();
final StopWatch stopWatch = new StopWatch(true);
-
final Object inputJson;
- try (final InputStream in = session.read(original)) {
- inputJson = jsonUtil.jsonToObject(in);
- } catch (final Exception e) {
- logger.error("JSON parsing failed for {}", original, e);
- session.transfer(original, REL_FAILURE);
- return;
+ final boolean isSourceFlowFileContent = SourceStrategy.FLOW_FILE ==
context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);
+ String jsonSourceAttributeName = null;
+
+ if (isSourceFlowFileContent) {
+ try (final InputStream in = session.read(original)) {
+ inputJson = jsonUtil.jsonToObject(in);
+ } catch (final Exception e) {
+ logger.error("JSON parsing failed on FlowFile content for {}",
original, e);
+ session.transfer(original, REL_FAILURE);
+ return;
+ }
+ } else {
+ jsonSourceAttributeName =
context.getProperty(JSON_SOURCE_ATTRIBUTE).evaluateAttributeExpressions(original).getValue();
+ final String jsonSourceAttributeValue =
original.getAttribute(jsonSourceAttributeName);
+ if (StringUtils.isBlank(jsonSourceAttributeValue)) {
+ logger.error("FlowFile attribute value was blank");
Review Comment:
The attribute name should be included in the message:
```suggestion
logger.error("FlowFile attribute [{}] value is blank",
jsonSourceAttributeName);
```
##########
nifi-extension-bundles/nifi-jolt-bundle/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java:
##########
@@ -55,11 +57,31 @@
@Tags({"json", "jolt", "transform", "shiftr", "chainr", "defaultr", "removr",
"cardinality", "sort"})
@InputRequirement(InputRequirement.Requirement.INPUT_REQUIRED)
@WritesAttribute(attribute = "mime.type", description = "Always set to
application/json")
-@CapabilityDescription("Applies a list of Jolt specifications to the flowfile
JSON payload. A new FlowFile is created "
- + "with transformed content and is routed to the 'success'
relationship. If the JSON transform "
- + "fails, the original FlowFile is routed to the 'failure'
relationship.")
+@CapabilityDescription("Applies a list of JOLT specifications to either the
FlowFile JSON content or a specified FlowFile JSON attribute. "
+ + "When 'Json Source' is set to FLOW_FILE, the FlowFile content is
transformed and the modified FlowFile is routed to the 'success' relationship. "
+ + "When 'Json Source' is set to ATTRIBUTE, the specified attribute's
value is transformed and updated in place, with the FlowFile routed to the
'success' relationship. "
+ + "If the JSON transform fails, the original FlowFile is routed to the
'failure' relationship.")
@RequiresInstanceClassLoading
public class JoltTransformJSON extends AbstractJoltTransform {
+
+ public static final PropertyDescriptor JSON_SOURCE = new
PropertyDescriptor.Builder()
+ .name("Json Source")
+ .description("Specifies whether the JOLT transformation is applied
to FlowFile JSON content or to specified FlowFile JSON attribute.")
+ .required(true)
+ .allowableValues(SourceStrategy.class)
+ .defaultValue(SourceStrategy.FLOW_FILE)
+ .build();
+
+ public static final PropertyDescriptor JSON_SOURCE_ATTRIBUTE = new
PropertyDescriptor.Builder()
+ .name("Json Source Attribute")
+ .description("The FlowFile attribute containing JSON to be
transformed. "
+ + "This property is required only when 'Json Source' is
set to ATTRIBUTE.")
Review Comment:
This can be specified as a multiline string, however, it is not necessary to
include the second sentence, since the documentation rendering automatically
describes dependent properties.
--
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]
