[GitHub] [nifi] mattyb149 commented on a change in pull request #5444: NIFI-9286: Add expression language to JOLT processors and fixes the custom module implementation to use custom jars in the processor

[GitBox <gi...@apache.org>]

mattyb149 commented on a change in pull request #5444:
URL: https://github.com/apache/nifi/pull/5444#discussion_r737714808



##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/JoltTransformJSON.java
##########
@@ -242,7 +251,8 @@
                     }
                 }
             } catch (final Exception e) {
-                getLogger().info("Processor is not valid - " + e.toString());
+//                getLogger().info("Processor is not valid - " + e.toString());

Review comment:
       Nitpick to remove dead code

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/JoltTransformJSON.java
##########
@@ -208,7 +209,7 @@
             final ClassLoader customClassLoader;
 
             try {
-                if (modulePath != null) {
+                if (modulePath != null && !validationContext.isExpressionLanguagePresent(modulePath)) {

Review comment:
       Should this check also be added to JoltTransformRecord.validate()?

##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/JoltTransformJSON.java
##########
@@ -306,26 +316,33 @@ public void process(OutputStream out) throws IOException {
         logger.info("Transformed {}", new Object[]{original});
     }
 
-    private JoltTransform getTransform(final ProcessContext context, final FlowFile flowFile) throws Exception {
+    private JoltTransform getTransform(final ProcessContext context, final FlowFile flowFile) {
         final Optional<String> specString;
         if (context.getProperty(JOLT_SPEC).isSet()) {
             specString = Optional.of(context.getProperty(JOLT_SPEC).evaluateAttributeExpressions(flowFile).getValue());
         } else {
             specString = Optional.empty();
         }
 
-        return transformCache.get(specString);
+        return transformCache.get(specString, currString -> {
+            try {
+                return createTransform(context, currString.orElse(null), flowFile);
+            } catch (Exception e) {
+                getLogger().error("Problem getting transform", e);
+            }
+            return null;
+        });
     }
 
     @OnScheduled
     public void setup(final ProcessContext context) {
         int maxTransformsToCache = context.getProperty(TRANSFORM_CACHE_SIZE).asInteger();
         transformCache = Caffeine.newBuilder()
                 .maximumSize(maxTransformsToCache)
-                .build(specString -> createTransform(context, specString.orElse(null)));
+                .build();
 
         try {
-            if (context.getProperty(MODULES).isSet()) {
+            if (context.getProperty(MODULES).isSet() && !context.getProperty(MODULES).isExpressionLanguagePresent()) {

Review comment:
       Is this necessary? I would think the property would be invalid if there was EL present, and if we decide to support EL we would remove this and add `evaluateAttributeExpression()`




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