NIFI-8730: Invalidate (don't evaluate) empty script in scripting components

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes #5184.

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java

@@ -241,11 +241,10 @@ public class BaseScriptedLookupService extends AbstractScriptedControllerService
     public void setup() {
         if (scriptNeedsReload.get() || lookupService.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) {
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java

@@ -215,11 +215,10 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor {
     public void setup() {
         if (scriptNeedsReload.get() || processor.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) {
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 
@@ -326,6 +325,10 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor {
      * @return Whether the script was successfully reloaded
      */
     private boolean reloadScript(final String scriptBody) {
+        if (StringUtils.isEmpty(scriptBody)) {
+            return true;
+        }
+
         // note we are starting here with a fresh listing of validation
         // results since we are (re)loading a new/updated script. any
         // existing validation results are not relevant
@@ -431,15 +434,14 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor {
         // store the updated validation results
         validationResults.set(results);
 
-        // return whether there was any issues loading the configured script
+        // return whether there were any issues loading the configured script
-        return results.isEmpty();
+        return !results.isEmpty();
     }
 
     /**
      * Invokes the validate() routine provided by the script, allowing for
-     * custom validation code. This method assumes there is a valid Processor
+     * custom validation code, if there is a valid Processor
-     * defined in the script and it has been loaded by the
+     * defined in the script and it has been loaded by the processor
-     * InvokeScriptedProcessor processor
      *
      * @param context The validation context to be passed into the custom
      * validate method
@@ -460,6 +462,12 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor {
             return validationResults.get();
         }
 
+        Collection<ValidationResult> scriptingComponentHelperResults = scriptingComponentHelper.customValidate(context);
+        if (scriptingComponentHelperResults != null && !scriptingComponentHelperResults.isEmpty()) {
+            validationResults.set(scriptingComponentHelperResults);
+            return scriptingComponentHelperResults;
+        }
+
         scriptingComponentHelper.setScriptEngineName(context.getProperty(scriptingComponentHelper.SCRIPT_ENGINE).getValue());
         scriptingComponentHelper.setScriptPath(context.getProperty(ScriptingComponentUtils.SCRIPT_FILE).evaluateAttributeExpressions().getValue());
         scriptingComponentHelper.setScriptBody(context.getProperty(ScriptingComponentUtils.SCRIPT_BODY).getValue());
@@ -476,7 +484,7 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor {
             try {
                 // defer to the underlying processor for validation, without the
                 // invokescriptedprocessor properties
-                final Set<PropertyDescriptor> innerPropertyDescriptor = new HashSet<PropertyDescriptor>(scriptingComponentHelper.getDescriptors());
+                final Set<PropertyDescriptor> innerPropertyDescriptor = new HashSet<>(scriptingComponentHelper.getDescriptors());
 
                 ValidationContext innerValidationContext = new FilteredPropertiesValidationContextAdapter(context, innerPropertyDescriptor);
                 final Collection<ValidationResult> instanceResults = instance.validate(innerValidationContext);
@@ -568,7 +576,10 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor {
 
     @OnStopped
     public void stop(ProcessContext context) {
-        invokeScriptedProcessorMethod("onStopped", context);
+        // If the script needs to be reloaded at this point, it is because it was empty
+        if (!scriptNeedsReload.get()) {
+            invokeScriptedProcessorMethod("onStopped", context);
+        }
         scriptingComponentHelper.stop();
         processor.set(null);
         scriptRunner = null;

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ScriptedTransformRecord.java

@@ -168,7 +168,7 @@ public class ScriptedTransformRecord extends AbstractProcessor implements Search
     @OnScheduled
     public void setup(final ProcessContext context) throws IOException {
         if (!scriptingComponentHelper.isInitialized.get()) {
-            scriptingComponentHelper.createResources();
+            scriptingComponentHelper.createResources(false);
         }
 
         scriptingComponentHelper.setupVariables(context);

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java

@@ -107,11 +107,10 @@ public class ScriptedRecordSink extends AbstractScriptedControllerService implem
     public void setup() {
         if (scriptNeedsReload.get() || recordSink.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) {
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java

@@ -78,11 +78,10 @@ public class ScriptedRulesEngine extends AbstractScriptedControllerService imple
     public void setup() {
         if (scriptNeedsReload.get() || rulesEngine.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) {
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java

@@ -80,11 +80,10 @@ public class ScriptedActionHandler extends AbstractScriptedControllerService imp
     public void setup() {
         if (scriptNeedsReload.get() || actionHandler.get() == null) {
             if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) {
-                reloadScriptFile(scriptingComponentHelper.getScriptPath());
+                scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath()));
             } else {
-                reloadScriptBody(scriptingComponentHelper.getScriptBody());
+                scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody()));
             }
-            scriptNeedsReload.set(false);
         }
     }
 

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java

@@ -119,7 +119,7 @@ public abstract class AbstractScriptedControllerService extends AbstractControll
     @Override
     protected Collection<ValidationResult> customValidate(ValidationContext validationContext) {
 
-        Collection<ValidationResult> commonValidationResults = super.customValidate(validationContext);
+        Collection<ValidationResult> commonValidationResults = new ArrayList<>(super.customValidate(validationContext));
         commonValidationResults.addAll(scriptingComponentHelper.customValidate(validationContext));
 
         if (!commonValidationResults.isEmpty()) {
@@ -173,7 +173,7 @@ public abstract class AbstractScriptedControllerService extends AbstractControll
         validationResults.set(results);
 
         // return whether there was any issues loading the configured script
-        return results.isEmpty();
+        return !results.isEmpty();
     }
 
     /**

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestInvokeJavascript.java

@@ -52,7 +52,6 @@ public class TestInvokeJavascript extends BaseScriptTest {
      */
     @Test
     public void testReadFlowFileContentAndStoreInFlowFileAttribute() throws Exception {
-        runner.setValidateExpressionUsage(false);
         runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE, "ECMAScript");
         runner.setProperty(ScriptingComponentUtils.SCRIPT_FILE, "target/test/resources/javascript/test_reader.js");
         runner.setProperty(ScriptingComponentUtils.MODULES, "target/test/resources/javascript");
@@ -146,7 +145,6 @@ public class TestInvokeJavascript extends BaseScriptTest {
     @Test(expected = AssertionError.class)
     public void testInvokeScriptCausesException() throws Exception {
         final TestRunner runner = TestRunners.newTestRunner(new InvokeScriptedProcessor());
-        runner.setValidateExpressionUsage(false);
         runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE, "ECMAScript");
         runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY, getFileContentsAsString(
                 TEST_RESOURCE_LOCATION + "javascript/testInvokeScriptCausesException.js")
@@ -164,7 +162,6 @@ public class TestInvokeJavascript extends BaseScriptTest {
      */
     @Test
     public void testScriptRoutesToFailure() throws Exception {
-        runner.setValidateExpressionUsage(false);
         runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE, "ECMAScript");
         runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY, getFileContentsAsString(
                 TEST_RESOURCE_LOCATION + "javascript/testScriptRoutesToFailure.js")
@@ -177,4 +174,16 @@ public class TestInvokeJavascript extends BaseScriptTest {
         final List<MockFlowFile> result = runner.getFlowFilesForRelationship("FAILURE");
         assertFalse(result.isEmpty());
     }
+
+    /**
+     * Tests an empty script with Nashorn (which throws an NPE if it is loaded), this test verifies an empty script is not attempted to be loaded.
+     *
+     * @throws Exception Any error encountered while testing
+     */
+    @Test
+    public void testEmptyScript() throws Exception {
+        runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE, "ECMAScript");
+        runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY, "");
+        runner.assertNotValid();
+    }
 }

nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_reader.groovy

@@ -57,7 +57,7 @@ class GroovyProcessor implements Processor {
     @Override
     void onTrigger(ProcessContext context, ProcessSessionFactory sessionFactory) throws ProcessException {
         def session = sessionFactory.createSession()
-        def flowFile = session.get();
+        def flowFile = session.get()
         if (flowFile == null) {
             return
         }