NIFI-14337 - Enhance JoltTransformJSON to Support JOLT Transformation… #9785

Srilatha-ramreddy · 2025-03-07T06:39:24Z

…s on Attributes

Summary

NIFI-14337

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Apache NiFi Jira issue created

Pull Request Tracking

Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

Pull Request based on current revision of the main branch
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
- JDK 21

Licensing

New dependencies are compatible with the Apache License 2.0 according to the License Policy
New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

Documentation formatting appears as expected in rendered files

exceptionfactory

Thanks for proposing this improvement @Srilatha-ramreddy.

Although having an optional property works, it is not immediately clear that this would alter the behavior. To make the implementation clearer, it would be helpful to introduce an additional strategy property. The property could be named JSON Source and could have values of Attribute or FlowFile using an enum that implements DescribedValue to bound the supported options. The property would be required, and would default to FlowFile, maintaining the current behavior. The new FlowFile Attribute property would depend on this strategy property.

Although that approach means introducing an additional property, it would make the configured behavior of the Processor much clearer. Feel free to raise any questions about this strategy.

exceptionfactory · 2025-03-10T14:52:45Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+            .name("Json Attribute")
+            .displayName("Json Attribute")


The displayName() method is not needed when the name() is the same. I also recommend renaming the property to FlowFile Attribute.

Suggested change

.name("Json Attribute")

.displayName("Json Attribute")

.name("FlowFile Attribute")

dan-s1 · 2025-03-13T12:31:17Z

...ifi-jolt-processors/src/test/java/org/apache/nifi/processors/jolt/TestJoltTransformJSON.java

+        final Map<String, String> attributes = Collections.singletonMap("jsonAttr",
+                "{\"rating\":{\"primary\":{\"value\":3},\"series\":{\"value\":[5,4]},\"quality\":{\"value\":}}}");


You could simply use Map.of like you do on line 497. Similar comment for line 511.

Suggested change

final Map<String, String> attributes = Collections.singletonMap("jsonAttr",

"{\"rating\":{\"primary\":{\"value\":3},\"series\":{\"value\":[5,4]},\"quality\":{\"value\":}}}");

final Map<String, String> attributes = Map.of("jsonAttr",

"{\"rating\":{\"primary\":{\"value\":3},\"series\":{\"value\":[5,4]},\"quality\":{\"value\":}}}");

dan-s1 · 2025-03-13T12:45:07Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+        + "When 'Json Source' is set to FLOW_FILE, the FlowFile content is transformed and the modified FlowFile is routed to '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 'success' relationship. "


Suggested change

+ "When 'Json Source' is set to FLOW_FILE, the FlowFile content is transformed and the modified FlowFile is routed to '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 'success' relationship. "

+ "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. "

dan-s1 · 2025-03-13T12:53:19Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+            jsonSourceAttributeName = context.getProperty(JSON_SOURCE_ATTRIBUTE).evaluateAttributeExpressions(original).getValue();
+            final String jsonSourceAttributeValue = original.getAttribute(jsonSourceAttributeName);
+            if (StringUtils.isBlank(jsonSourceAttributeValue)) {
+                logger.error("FlowFile attribute value evaluated to null");


StringUtils.isBlank is not only when a string is null. It can even be when the string is empty or only has white space.

Suggested change

logger.error("FlowFile attribute value evaluated to null");

logger.error("FlowFile attribute value was blank);

dan-s1 · 2025-03-13T12:54:50Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+        final boolean isSourceFlowFileContent  = SourceStrategy.FLOW_FILE == context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);
+        String jsonSourceAttributeName = null;
+
+        if (isSourceFlowFileContent ) {


Suggested change

if (isSourceFlowFileContent ) {

if (isSourceFlowFileContent) {

dan-s1 · 2025-03-13T13:09:51Z

...ifi-jolt-processors/src/test/java/org/apache/nifi/processors/jolt/TestJoltTransformJSON.java

+    @Test
+    void testJsonAttributeNotInitialised() throws IOException {
+        runner.setProperty(JoltTransformJSON.JSON_SOURCE, SourceStrategy.ATTRIBUTE);
+        runner.setProperty(JoltTransformJSON.JOLT_SPEC, "./src/test/resources/specs/shiftrSpec.json");


I see this value "./src/test/resources/specs/shiftrSpec.json" being used a total of three times by you on this line, line 481, and line 494. Another test uses the same value on line 225. In addition there is another form of this same file used "src/test/resources/specs/shiftrSpec.json" without the leading ./ on lines 214 and 255. Please make a private static final String variable with one of these values and use it in all six places.

dan-s1 · 2025-03-13T13:25:12Z

...ifi-jolt-processors/src/test/java/org/apache/nifi/processors/jolt/TestJoltTransformJSON.java

+        runner.setProperty(JoltTransformJSON.JSON_SOURCE, SourceStrategy.ATTRIBUTE);
+        runner.setProperty(JoltTransformJSON.JOLT_SPEC, "./src/test/resources/specs/shiftrSpec.json");
+        runner.setProperty(JoltTransformJSON.JOLT_TRANSFORM, JoltTransformStrategy.SHIFTR);
+        runner.setProperty(JoltTransformJSON.JSON_SOURCE_ATTRIBUTE, "jsonAttr");


I see this value "jsonAttr" being used 8 times for defining the name of the attribute (this line and on lines 483, 484, 498, 510, 511, 523 and 525). Please make a private static final String variable and use it in each of those places.

dan-s1 · 2025-03-13T13:39:43Z

...ifi-jolt-processors/src/test/java/org/apache/nifi/processors/jolt/TestJoltTransformJSON.java

@@ -464,6 +464,71 @@ void testJoltSpecInvalidEL() throws IOException {
        runner.assertNotValid();
    }

+    @Test


@exceptionfactory I see 4 new unit tests which are configured very similarly. Do you think these four tests should be folded into a JUnit5 ParamaterizedTest and use the MethodSource annotation to define a method which would return the necessary arguments for each of these tests?

…on Attributes

Srilatha-ramreddy · 2025-03-13T23:55:57Z

@exceptionfactory @dan-s1 Thanks for the feedback. Review comments are now addressed and is Ready for review. Thanks

exceptionfactory · 2025-03-15T00:10:22Z

Thanks for the updates @Srilatha-ramreddy, please review the Checkstyle warnings. I will take a closer look at the latest version soon.

Warning:  src/test/java/org/apache/nifi/processors/jolt/TestJoltTransformJSON.java:[478] (sizes) LineLength: Line is longer than 200 characters (found 213).

dan-s1 · 2025-03-17T14:38:56Z

...ifi-jolt-processors/src/test/java/org/apache/nifi/processors/jolt/TestJoltTransformJSON.java

+        return Stream.of(
+                Arguments.of(JSON_SOURCE_ATTR_NAME, null, SHIFTR_SPEC_PATH,
+                        JoltTransformStrategy.SHIFTR, false, null),
+                Arguments.of(JSON_SOURCE_ATTR_NAME, Map.of(JSON_SOURCE_ATTR_NAME, INVALID_INPUT_JSON), SHIFTR_SPEC_PATH,
+                        JoltTransformStrategy.SHIFTR, false, null),
+                Arguments.of("${dynamicJsonAttr}", Map.of("dynamicJsonAttr", JSON_SOURCE_ATTR_NAME, JSON_SOURCE_ATTR_NAME, EXPECTED_JSON), SHIFTR_SPEC_PATH,
+                        JoltTransformStrategy.SHIFTR, true, SHIFTR_JSON_OUTPUT),
+                Arguments.of(JSON_SOURCE_ATTR_NAME, Map.of(JSON_SOURCE_ATTR_NAME, EXPECTED_JSON), CHAINR_SPEC_PATH,
+                        JoltTransformStrategy.CHAINR, true, CHAINR_JSON_OUTPUT)
+        );


@Srilatha-ramreddy Thanks for adding the ParamaterizedTest. That is what I had in mind. I am requesting one minor change, instead of using Arguments.of please use Arguments.argumentSet so you can give a meaningful name to each of the tests. I was going to start from the original names of the unit tests you had started with although I no longer see that commit as it seems you squashed it. Please note in general once the PR has been submitted no squashes should be done. Thanks!

@dan-s1 Thanks for the feedback. Please review the latest commit and will also not squash the commits anymore.

exceptionfactory

Thanks for the updates @Srilatha-ramreddy. I noted a few minor adjustments, and one larger question regarding Expression Language evaluation. As mentioned, support for Expression Language opens up some additional possibilities that should be considered.

exceptionfactory · 2025-03-18T13:18:26Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

 @RequiresInstanceClassLoading
 public class JoltTransformJSON extends AbstractJoltTransform {
+
+    public static final PropertyDescriptor JSON_SOURCE = new PropertyDescriptor.Builder()
+            .name("Json Source")


JSON should be all uppercase in property names:

Suggested change

.name("Json Source")

.name("JSON Source")

exceptionfactory · 2025-03-18T13:18:35Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+
+    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.")


Suggested change

.description("Specifies whether the JOLT transformation is applied to FlowFile JSON content or to specified FlowFile JSON attribute.")

.description("Specifies whether the Jolt transformation is applied to FlowFile JSON content or to specified FlowFile JSON attribute.")

exceptionfactory · 2025-03-18T13:18:45Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+            .build();
+
+    public static final PropertyDescriptor JSON_SOURCE_ATTRIBUTE = new PropertyDescriptor.Builder()
+            .name("Json Source Attribute")


Suggested change

.name("Json Source Attribute")

.name("JSON Source Attribute")

exceptionfactory · 2025-03-18T13:19:11Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+        + "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. "


This description is duplicative of the property description, so I recommend removing it.

exceptionfactory · 2025-03-18T13:19:46Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+            .description("The FlowFile attribute containing JSON to be transformed. "
+                    + "This property is required only when 'Json Source' is set to ATTRIBUTE.")


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.

exceptionfactory · 2025-03-18T13:21:54Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

-            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);


Minor renaming recommendation:

Suggested change

final boolean isSourceFlowFileContent = SourceStrategy.FLOW_FILE == context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);

final boolean sourceStrategyFlowFile = SourceStrategy.FLOW_FILE == context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);

exceptionfactory · 2025-03-18T13:23:25Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+            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");


The attribute name should be included in the message:

Suggested change

logger.error("FlowFile attribute value was blank");

logger.error("FlowFile attribute [{}] value is blank", jsonSourceAttributeName);

exceptionfactory · 2025-03-18T13:30:30Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+                return;
+            }
+        } else {
+            jsonSourceAttributeName = context.getProperty(JSON_SOURCE_ATTRIBUTE).evaluateAttributeExpressions(original).getValue();


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.

@exceptionfactory I agree with you, EL support was just to support any edge case scenarios but there can be workarounds to achieve what is intended. Happy to remove the expression language support.

@exceptionfactory Can you please review.

…me more cleanup

dan-s1 · 2025-03-21T15:02:04Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+                try {
+                    inputJson = jsonUtil.jsonToObject(jsonSourceAttributeValue);
+                } catch (final Exception e) {
+                    logger.error("JSON parsing failed on FlowFile attribute for {}", original, e);


It might be a good idea to include the name of the attribute like you did above.

Suggested change

logger.error("JSON parsing failed on FlowFile attribute for {}", original, e);

logger.error("JSON parsing failed on attribute '{}' of FlowFile {}", jsonSourceAttributeName, original, e);

dan-s1 · 2025-03-21T15:29:02Z

...le/nifi-jolt-processors/src/main/java/org/apache/nifi/processors/jolt/JoltTransformJSON.java

+            logger.info("Transform completed on FlowFile attribute for {}", original);
+        }


Again, it may be beneficial to name the attribute which was transformed so it is clear in the logs

Suggested change

logger.info("Transform completed on FlowFile attribute for {}", original);

}

logger.info("Transform completed on attribute {} of FlowFile {}", sonSourceAttributeName, original);

}

exceptionfactory requested changes Mar 10, 2025

View reviewed changes

Srilatha-ramreddy requested a review from exceptionfactory March 12, 2025 00:07

dan-s1 reviewed Mar 13, 2025

View reviewed changes

NIFI-14337 Enhance JoltTransformJSON to Support JOLT Transformations …

e9ef7ed

…on Attributes

Srilatha-ramreddy force-pushed the NIFI-14337 branch from 7ab8446 to e9ef7ed Compare March 13, 2025 23:53

Srilatha-ramreddy requested a review from dan-s1 March 13, 2025 23:54

NIFI-14337 Fix Checkstyle Warning

566fc4f

dan-s1 reviewed Mar 17, 2025

View reviewed changes

dan-s1 mentioned this pull request Mar 17, 2025

NIFI-14375 - Kafka refactor - partitioning, uncommitted time, inputstream, tests, etc #9807

Open

10 tasks

NIFI-14337 Use Arguments.argumentSet to define the testcase names

f3fa31e

exceptionfactory requested changes Mar 18, 2025

View reviewed changes

NIFI-14337 - Remove JSON Attribute expression language support and so…

3b91741

…me more cleanup

dan-s1 reviewed Mar 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NIFI-14337 - Enhance JoltTransformJSON to Support JOLT Transformation… #9785

NIFI-14337 - Enhance JoltTransformJSON to Support JOLT Transformation… #9785

Srilatha-ramreddy commented Mar 7, 2025 •

edited

Loading

exceptionfactory left a comment

exceptionfactory Mar 10, 2025

dan-s1 Mar 13, 2025

dan-s1 Mar 13, 2025

dan-s1 Mar 13, 2025

dan-s1 Mar 13, 2025

dan-s1 Mar 13, 2025

dan-s1 Mar 13, 2025 •

edited

Loading

dan-s1 Mar 13, 2025 •

edited

Loading

Srilatha-ramreddy commented Mar 13, 2025

exceptionfactory commented Mar 15, 2025

dan-s1 Mar 17, 2025 •

edited

Loading

Srilatha-ramreddy Mar 17, 2025

exceptionfactory left a comment

exceptionfactory Mar 18, 2025

exceptionfactory Mar 18, 2025

exceptionfactory Mar 18, 2025

exceptionfactory Mar 18, 2025

exceptionfactory Mar 18, 2025

exceptionfactory Mar 18, 2025

exceptionfactory Mar 18, 2025

exceptionfactory Mar 18, 2025

Srilatha-ramreddy Mar 18, 2025

Srilatha-ramreddy Mar 20, 2025

dan-s1 Mar 21, 2025

dan-s1 Mar 21, 2025 •

edited

Loading

	.name("Json Attribute")
	.displayName("Json Attribute")
	.name("FlowFile Attribute")

		final Map<String, String> attributes = Collections.singletonMap("jsonAttr",
		"{\"rating\":{\"primary\":{\"value\":3},\"series\":{\"value\":[5,4]},\"quality\":{\"value\":}}}");

		+ "When 'Json Source' is set to FLOW_FILE, the FlowFile content is transformed and the modified FlowFile is routed to '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 'success' relationship. "

	logger.error("FlowFile attribute value evaluated to null");
	logger.error("FlowFile attribute value was blank);

	if (isSourceFlowFileContent ) {
	if (isSourceFlowFileContent) {

	.description("Specifies whether the JOLT transformation is applied to FlowFile JSON content or to specified FlowFile JSON attribute.")
	.description("Specifies whether the Jolt transformation is applied to FlowFile JSON content or to specified FlowFile JSON attribute.")

	.name("Json Source Attribute")
	.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.")

	final boolean isSourceFlowFileContent = SourceStrategy.FLOW_FILE == context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);
	final boolean sourceStrategyFlowFile = SourceStrategy.FLOW_FILE == context.getProperty(JSON_SOURCE).asAllowableValue(SourceStrategy.class);

	logger.error("FlowFile attribute value was blank");
	logger.error("FlowFile attribute [{}] value is blank", jsonSourceAttributeName);

	logger.error("JSON parsing failed on FlowFile attribute for {}", original, e);
	logger.error("JSON parsing failed on attribute '{}' of FlowFile {}", jsonSourceAttributeName, original, e);

		logger.info("Transform completed on FlowFile attribute for {}", original);
		}

NIFI-14337 - Enhance JoltTransformJSON to Support JOLT Transformation… #9785

Are you sure you want to change the base?

NIFI-14337 - Enhance JoltTransformJSON to Support JOLT Transformation… #9785

Conversation

Srilatha-ramreddy commented Mar 7, 2025 • edited Loading

Summary

Tracking

Issue Tracking

Pull Request Tracking

Pull Request Formatting

Verification

Build

Licensing

Documentation

exceptionfactory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dan-s1 Mar 13, 2025 • edited Loading

Choose a reason for hiding this comment

dan-s1 Mar 13, 2025 • edited Loading

Choose a reason for hiding this comment

Srilatha-ramreddy commented Mar 13, 2025

exceptionfactory commented Mar 15, 2025

dan-s1 Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

exceptionfactory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dan-s1 Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Srilatha-ramreddy commented Mar 7, 2025 •

edited

Loading

dan-s1 Mar 13, 2025 •

edited

Loading

dan-s1 Mar 13, 2025 •

edited

Loading

dan-s1 Mar 17, 2025 •

edited

Loading

dan-s1 Mar 21, 2025 •

edited

Loading