From 64e3599f05865c0adfce95da15e1677744ea39f4 Mon Sep 17 00:00:00 2001 From: Matthew Burgess Date: Mon, 16 Mar 2020 19:01:07 -0400 Subject: [PATCH] NIFI-7260: Fix error handling and re-evaluate Module Directory property on changed for scripted controller services This closes #4147 Signed-off-by: Mike Thomsen --- .../script/AbstractScriptedRecordFactory.java | 8 ++- .../AbstractScriptedControllerService.java | 20 +++++- .../record/script/ScriptedReaderTest.groovy | 52 +++++++++++++- .../test_record_reader_load_module.groovy | 68 ++++++++++++++++++ .../src/test/resources/jar/test.jar | Bin 0 -> 1820 bytes 5 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_record_reader_load_module.groovy create mode 100644 nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/jar/test.jar diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/script/AbstractScriptedRecordFactory.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/script/AbstractScriptedRecordFactory.java index 3c694a7687..aa36ab41ac 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/script/AbstractScriptedRecordFactory.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/record/script/AbstractScriptedRecordFactory.java @@ -42,9 +42,13 @@ public abstract class AbstractScriptedRecordFactory extends AbstractScriptedC if (scriptNeedsReload.get() || recordFactory.get() == null) { if (ScriptingComponentHelper.isFile(scriptingComponentHelper.getScriptPath())) { - reloadScriptFile(scriptingComponentHelper.getScriptPath()); + if (!reloadScriptFile(scriptingComponentHelper.getScriptPath())) { + throw new ProcessException("Error during loading of script"); + } } else { - reloadScriptBody(scriptingComponentHelper.getScriptBody()); + if (!reloadScriptBody(scriptingComponentHelper.getScriptBody())) { + throw new ProcessException("Error during loading of script"); + } } scriptNeedsReload.set(false); } diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java index ee25d550af..52a9e0cfa9 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/AbstractScriptedControllerService.java @@ -103,13 +103,15 @@ public abstract class AbstractScriptedControllerService extends AbstractControll @Override public void onPropertyModified(final PropertyDescriptor descriptor, final String oldValue, final String newValue) { + validationResults.set(new HashSet<>()); + if (ScriptingComponentUtils.SCRIPT_FILE.equals(descriptor) || ScriptingComponentUtils.SCRIPT_BODY.equals(descriptor) || ScriptingComponentUtils.MODULES.equals(descriptor) || scriptingComponentHelper.SCRIPT_ENGINE.equals(descriptor)) { scriptNeedsReload.set(true); // Need to reset scriptEngine if the value has changed - if (scriptingComponentHelper.SCRIPT_ENGINE.equals(descriptor)) { + if (scriptingComponentHelper.SCRIPT_ENGINE.equals(descriptor) || ScriptingComponentUtils.MODULES.equals(descriptor)) { scriptEngine = null; } } @@ -117,7 +119,21 @@ public abstract class AbstractScriptedControllerService extends AbstractControll @Override protected Collection customValidate(ValidationContext validationContext) { - return scriptingComponentHelper.customValidate(validationContext); + + Collection commonValidationResults = super.customValidate(validationContext); + commonValidationResults.addAll(scriptingComponentHelper.customValidate(validationContext)); + + if (!commonValidationResults.isEmpty()) { + return commonValidationResults; + } + + // do not try to build processor/compile/etc until onPropertyModified clear the validation error/s + // and don't print anything into log. + if (!validationResults.get().isEmpty()) { + return validationResults.get(); + } + + return commonValidationResults; } public void onEnabled(final ConfigurationContext context) { diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/record/script/ScriptedReaderTest.groovy b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/record/script/ScriptedReaderTest.groovy index 8be0b6f2d6..5ec491777c 100644 --- a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/record/script/ScriptedReaderTest.groovy +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/groovy/org/apache/nifi/record/script/ScriptedReaderTest.groovy @@ -37,9 +37,10 @@ import org.junit.runners.JUnit4 import org.slf4j.Logger import org.slf4j.LoggerFactory -import static groovy.util.GroovyTestCase.assertEquals +import static junit.framework.TestCase.assertEquals import static org.junit.Assert.assertNotNull import static org.junit.Assert.assertNull +import static org.junit.Assert.fail import static org.mockito.Mockito.mock import static org.mockito.Mockito.when @@ -189,7 +190,56 @@ class ScriptedReaderTest { assertEquals(record.getAsInt('code'), record.getAsInt('id') * 100) } assertNull(recordReader.nextRecord()) + } + @Test + void testRecordReaderGroovyScriptChangeModuleDirectory() { + + def properties = [:] as Map + recordReaderFactory.getSupportedPropertyDescriptors().each {PropertyDescriptor descriptor -> + properties.put(descriptor, descriptor.getDefaultValue()) + } + + // Mock the ConfigurationContext for setup(...) + def configurationContext = mock(ConfigurationContext) + when(configurationContext.getProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE)) + .thenReturn(new MockPropertyValue('Groovy')) + when(configurationContext.getProperty(ScriptingComponentUtils.SCRIPT_FILE)) + .thenReturn(new MockPropertyValue('target/test/resources/groovy/test_record_reader_load_module.groovy')) + when(configurationContext.getProperty(ScriptingComponentUtils.SCRIPT_BODY)) + .thenReturn(new MockPropertyValue(null)) + when(configurationContext.getProperty(ScriptingComponentUtils.MODULES)) + .thenReturn(new MockPropertyValue(null)) + + def logger = mock(ComponentLog) + def initContext = mock(ControllerServiceInitializationContext) + when(initContext.getIdentifier()).thenReturn(UUID.randomUUID().toString()) + when(initContext.getLogger()).thenReturn(logger) + + recordReaderFactory.initialize initContext + try { + recordReaderFactory.onEnabled configurationContext + fail('Expected exception in onEnabled when script is loaded with no Module Directory set') + } catch(e) { + // Do nothing, the exception is expected as the needed class is not in the Module Directory property + } + + byte[] contentBytes = 'Flow file content not used'.bytes + InputStream inStream = new ByteArrayInputStream(contentBytes) + + def recordReader = recordReaderFactory.createRecordReader(Collections.emptyMap(), inStream, contentBytes.length, logger) + // This one is supposed to be null as the factory should fail on initialize + assertNull(recordReader) + + when(configurationContext.getProperty(ScriptingComponentUtils.MODULES)) + .thenReturn(new MockPropertyValue('target/test/resources/jar/test.jar')) + + recordReaderFactory.onPropertyModified(ScriptingComponentUtils.MODULES, '', 'target/test/resources/jar/test.jar') + + recordReaderFactory.initialize initContext + recordReaderFactory.onEnabled configurationContext + recordReader = recordReaderFactory.createRecordReader(Collections.emptyMap(), inStream, contentBytes.length, logger) + assertNotNull(recordReader) } class MockScriptedReader extends ScriptedReader implements AccessibleScriptingComponentHelper { diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_record_reader_load_module.groovy b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_record_reader_load_module.groovy new file mode 100644 index 0000000000..996cc65e65 --- /dev/null +++ b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/groovy/test_record_reader_load_module.groovy @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import org.apache.nifi.controller.AbstractControllerService +import org.apache.nifi.logging.ComponentLog +import org.apache.nifi.schema.access.SchemaNotFoundException +import org.apache.nifi.serialization.MalformedRecordException +import org.apache.nifi.serialization.RecordReader +import org.apache.nifi.serialization.RecordReaderFactory +import org.apache.nifi.serialization.SimpleRecordSchema +import org.apache.nifi.serialization.record.MapRecord +import org.apache.nifi.serialization.record.Record +import org.apache.nifi.serialization.record.RecordField +import org.apache.nifi.serialization.record.RecordFieldType +import org.apache.nifi.serialization.record.RecordSchema + +// import a test class to ensure Module Directory property is working correctly +import org.apache.nifi.script.ModulePropertyExample + + +class GroovyModuleRecordReader implements RecordReader { + + def recordSchema = new SimpleRecordSchema( + [new RecordField('id', RecordFieldType.INT.dataType), + new RecordField('name', RecordFieldType.STRING.dataType), + new RecordField('code', RecordFieldType.INT.dataType)] + ) + + def recordIterator = [ + new MapRecord(recordSchema, ['id': 1, 'name': 'John', 'code': 100]), + new MapRecord(recordSchema, ['id': 2, 'name': 'Mary', 'code': 200]), + new MapRecord(recordSchema, ['id': 3, 'name': 'Ramon', 'code': 300]) + ].iterator() + + Record nextRecord(boolean coerceTypes, boolean dropUnknown) throws IOException, MalformedRecordException { + return recordIterator.hasNext() ? recordIterator.next() : null + } + + RecordSchema getSchema() throws MalformedRecordException { + return recordSchema + } + + void close() throws IOException { + } +} + +class GroovyModuleRecordReaderFactory extends AbstractControllerService implements RecordReaderFactory { + + RecordReader createRecordReader(Map variables, InputStream inputStream, long inputLength, ComponentLog logger) throws MalformedRecordException, IOException, SchemaNotFoundException { + return new GroovyModuleRecordReader() + } +} + +reader = new GroovyModuleRecordReaderFactory() diff --git a/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/jar/test.jar b/nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/resources/jar/test.jar new file mode 100644 index 0000000000000000000000000000000000000000..2b8d3cb6c7f0b76b9a5d63a0041f23212282684f GIT binary patch literal 1820 zcmWIWW@Zs#;Nak3Sl?R^z<>le8CV#6T|*poJ^kGD|D9rBU}gyLX6FE@V1g3RX;B?EmOZZxhS)sL?7Yw5Hw9jxHb9arh9S7UZPrCFdj-7tamL4ZiIrP%7u_%{H3*?qr#P+c}4}nbz6)^Y7M{egF33 z`+oa>Yz3lvhi9ZdH;sGptL=i1a(YHzp0sPYcht?fO%)l@u8F&=%j+NAKH3y;K*GxG zr`~n0J7==_ubH>L)5=JAar*YOt-He~ov+@ny}Xx)dEYZNFNwAV+A>|Lb5@wz`o3$E zoxZzGPQT!Iq(QNmvx#E!wY6(*Jh1Ky`W-vi+bl26=;~Sbt9@JY zu4CISlw2zJh+oaHJhWt?bqhmPNQsh+lfYgP&3w20U1?jkT)4>EruS0!>Cq`RN*ClM z*E%~MRj`|qW*>C1ulUlb2|l0O95-|xiOlkQnK3QITW|M?r-2i-&e0Mbq}KJF9u(M>JVVbdN~nZBNqEuB_( zYdw#;%chGRmu%-R+PmCUv|J$nsBX>C3tpOQzNj2dov%>Kb7jLy>E8A4Eo*dZ^(W*n zzxDmZOCgt;dq3`5>9IP(C8%n@|Kjc$`qh&+T0c4+Sg6HiTPNPelqr9$|NM%BTDGs^ zbW@++QJpBf&7idM`tJ79=m6nsJ=?M;T8S^Xm-4ybGVhl&4{l5jT(nNX`SGVWY@%zf zO<8#5>}tEIzNb5X#d7$6*xQv_qs)Gozhv{hgYgHg&X%9r)ukiXy1;pbN3yF~McgcJ z&zT1=obb+^Y4dx}eW!)3Nw=02%Bat%`KoityZH8l*Q^_L#HBh+-h5rFqCai_@9AD@ zm)nvjBpfr*{V5fpcK2hzsaqRM4bw$f+B7aHr0(hb$MlJj(aNbv=h8p!kS!PgZLSfr zsccv0KlvtLZWO0`zQ9_s+S-?qOl^E09hH7r|2p)1a`MHk_YDlT3E3Vv`up*x$CrMb z3SNK2H7fL0WA_hu*i{O&f1(yt$c?^G5X@ z+s1fyv-Y1xYx*57^+q-AJ+ilL*9upaS60;mE%`l1ekjUKO`Q8hRrrd>O9_ilPA@-P zk(@T+(_;G%Tf(N_YR+@m+1DIbJn`p;?ez~oKAK-KiIaVWT1vMr?7 zhPl4gFN%M{?{6 z!oaW@STY89Gct)VpqBEmA{c}tZX1NSb$K8k%8d}D~JaGMzzr@ literal 0 HcmV?d00001