From 6f0ca8730411d01e5c5c2f845a5387020aee60ba Mon Sep 17 00:00:00 2001 From: Matt Burgess Date: Tue, 23 Aug 2022 13:53:12 -0400 Subject: [PATCH] NIFI-10387: Fixed negative logic bug in scripted components (#6325) --- .../script/BaseScriptedLookupService.java | 4 +- .../script/InvokeScriptedProcessor.java | 4 +- .../sink/script/ScriptedRecordSink.java | 4 +- .../engine/script/ScriptedRulesEngine.java | 4 +- .../script/ScriptedActionHandler.java | 4 +- .../script/TestScriptedLookupService.groovy | 39 +++++++++++++++++++ 6 files changed, 49 insertions(+), 10 deletions(-) diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java index 623796eb4b..3c34af2ad8 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/lookup/script/BaseScriptedLookupService.java @@ -241,9 +241,9 @@ public class BaseScriptedLookupService extends AbstractScriptedControllerService public void setup() { if (scriptNeedsReload.get() || lookupService.get() == null) { if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) { - scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath())); + scriptNeedsReload.set(!reloadScriptFile(scriptingComponentHelper.getScriptPath())); } else { - scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody())); + scriptNeedsReload.set(!reloadScriptBody(scriptingComponentHelper.getScriptBody())); } } } diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java index 5cc4d65861..80abbcadd1 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/InvokeScriptedProcessor.java @@ -225,9 +225,9 @@ public class InvokeScriptedProcessor extends AbstractSessionFactoryProcessor { public void setup() { if (scriptNeedsReload.get() || processor.get() == null) { if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) { - scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath())); + scriptNeedsReload.set(!reloadScriptFile(scriptingComponentHelper.getScriptPath())); } else { - scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody())); + scriptNeedsReload.set(!reloadScriptBody(scriptingComponentHelper.getScriptBody())); } } } diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java index f17cb0e95f..0f5748ad9b 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/sink/script/ScriptedRecordSink.java @@ -107,9 +107,9 @@ public class ScriptedRecordSink extends AbstractScriptedControllerService implem public void setup() { if (scriptNeedsReload.get() || recordSink.get() == null) { if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) { - scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath())); + scriptNeedsReload.set(!reloadScriptFile(scriptingComponentHelper.getScriptPath())); } else { - scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody())); + scriptNeedsReload.set(!reloadScriptBody(scriptingComponentHelper.getScriptBody())); } } } diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java index 5239761c25..b30b9ef2c9 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/engine/script/ScriptedRulesEngine.java @@ -78,9 +78,9 @@ public class ScriptedRulesEngine extends AbstractScriptedControllerService imple public void setup() { if (scriptNeedsReload.get() || rulesEngine.get() == null) { if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) { - scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath())); + scriptNeedsReload.set(!reloadScriptFile(scriptingComponentHelper.getScriptPath())); } else { - scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody())); + scriptNeedsReload.set(!reloadScriptBody(scriptingComponentHelper.getScriptBody())); } } } diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java index f81ade4ffb..4e5efd3b26 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/rules/handlers/script/ScriptedActionHandler.java @@ -80,9 +80,9 @@ public class ScriptedActionHandler extends AbstractScriptedControllerService imp public void setup() { if (scriptNeedsReload.get() || actionHandler.get() == null) { if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) { - scriptNeedsReload.set(reloadScriptFile(scriptingComponentHelper.getScriptPath())); + scriptNeedsReload.set(!reloadScriptFile(scriptingComponentHelper.getScriptPath())); } else { - scriptNeedsReload.set(reloadScriptBody(scriptingComponentHelper.getScriptBody())); + scriptNeedsReload.set(!reloadScriptBody(scriptingComponentHelper.getScriptBody())); } } } diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/lookup/script/TestScriptedLookupService.groovy b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/lookup/script/TestScriptedLookupService.groovy index abb3daf024..01f4454032 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/lookup/script/TestScriptedLookupService.groovy +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/lookup/script/TestScriptedLookupService.groovy @@ -46,8 +46,11 @@ import static org.junit.jupiter.api.Assertions.assertTrue */ class TestScriptedLookupService { private static final String GROOVY_SCRIPT = "test_lookup_inline.groovy" + private static final String ALTERNATE_GROOVY_SCRIPT = "test_simple_lookup_inline.groovy" private static final Path SOURCE_PATH = Paths.get("src/test/resources/groovy", GROOVY_SCRIPT) + private static final Path ALTERNATE_SOURCE_PATH = Paths.get("src/test/resources/groovy", ALTERNATE_GROOVY_SCRIPT) private static final Path TARGET_PATH = Paths.get("target", GROOVY_SCRIPT) + private static final Path ALTERNATE_TARGET_PATH = Paths.get("target", ALTERNATE_GROOVY_SCRIPT) private static final Logger logger = LoggerFactory.getLogger(TestScriptedLookupService) ScriptedLookupService scriptedLookupService def scriptingComponent @@ -59,7 +62,9 @@ class TestScriptedLookupService { logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}") } Files.copy(SOURCE_PATH, TARGET_PATH, StandardCopyOption.REPLACE_EXISTING) + Files.copy(ALTERNATE_SOURCE_PATH, ALTERNATE_TARGET_PATH, StandardCopyOption.REPLACE_EXISTING) TARGET_PATH.toFile().deleteOnExit() + ALTERNATE_TARGET_PATH.toFile().deleteOnExit() } @BeforeEach @@ -96,6 +101,40 @@ class TestScriptedLookupService { assertFalse(opt.present) } + @Test + void testLookupServiceScriptReload() { + final TestRunner runner = TestRunners.newTestRunner(new AbstractProcessor() { + @Override + public void onTrigger(final ProcessContext context, final ProcessSession session) throws ProcessException { + } + }); + + runner.addControllerService("lookupService", scriptedLookupService) + runner.setProperty(scriptedLookupService, "Script Engine", "Groovy") + runner.setProperty(scriptedLookupService, ScriptingComponentUtils.SCRIPT_BODY, (String) null) + runner.setProperty(scriptedLookupService, ScriptingComponentUtils.SCRIPT_FILE, ALTERNATE_TARGET_PATH.toString()) + runner.setProperty(scriptedLookupService, ScriptingComponentUtils.MODULES, (String) null) + // This call to setup should fail loading the script but should mark that the script should be reloaded + scriptedLookupService.setup() + // This prevents the (lookupService == null) check from passing, in order to force the reload from the + // scriptNeedsReload variable specifically + scriptedLookupService.lookupService.set(new MockScriptedLookupService()) + + runner.enableControllerService(scriptedLookupService) + + MockFlowFile mockFlowFile = new MockFlowFile(1L) + InputStream inStream = new ByteArrayInputStream('Flow file content not used'.bytes) + + Optional opt = scriptedLookupService.lookup(['key':'Hello']) + assertTrue(opt.present) + assertEquals('Hi', opt.get()) + opt = scriptedLookupService.lookup(['key':'World']) + assertTrue(opt.present) + assertEquals('there', opt.get()) + opt = scriptedLookupService.lookup(['key':'Not There']) + assertFalse(opt.present) + } + class MockScriptedLookupService extends ScriptedLookupService implements AccessibleScriptingComponentHelper { @Override