Skip to content

Commit

Permalink
NIFI-11944 Removed validation warning from ExecuteStreamCommand
Browse files Browse the repository at this point in the history
- Removed customValidate() which logged an incorrect warning every 5 seconds based on strategy string instance evaluation

Signed-off-by: Matt Burgess <[email protected]>

This closes apache#7607
  • Loading branch information
exceptionfactory authored and mattyb149 committed Aug 14, 2023
1 parent b55b712 commit 71abd77
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.apache.nifi.components.PropertyDescriptor;
import org.apache.nifi.components.PropertyValue;
import org.apache.nifi.components.RequiredPermission;
import org.apache.nifi.components.ValidationContext;
import org.apache.nifi.components.ValidationResult;
import org.apache.nifi.components.Validator;
import org.apache.nifi.expression.AttributeExpression.ResultType;
Expand Down Expand Up @@ -64,7 +63,6 @@
import java.io.OutputStream;
import java.lang.ProcessBuilder.Redirect;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -190,13 +188,11 @@ public class ExecuteStreamCommand extends AbstractProcessor {
private final static Set<Relationship> ATTRIBUTE_RELATIONSHIP_SET;

private static final Pattern COMMAND_ARGUMENT_PATTERN = Pattern.compile("command\\.argument\\.(?<commandIndex>[0-9]+)$");
public static final String executionArguments = "Command Arguments Property";
public static final String dynamicArguements = "Dynamic Property Arguments";

static final AllowableValue EXECUTION_ARGUMENTS_PROPERTY_STRATEGY = new AllowableValue(executionArguments, executionArguments,
static final AllowableValue COMMAND_ARGUMENTS_PROPERTY_STRATEGY = new AllowableValue("Command Arguments Property", "Command Arguments Property",
"Arguments to be supplied to the executable are taken from the Command Arguments property");

static final AllowableValue DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY = new AllowableValue(dynamicArguements, dynamicArguements,
static final AllowableValue DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY = new AllowableValue("Dynamic Property Arguments", "Dynamic Property Arguments",
"Arguments to be supplied to the executable are taken from dynamic properties with pattern of 'command.argument.<commandIndex>'");

static final PropertyDescriptor WORKING_DIR = new PropertyDescriptor.Builder()
Expand All @@ -222,14 +218,14 @@ public class ExecuteStreamCommand extends AbstractProcessor {
.description("Strategy for configuring arguments to be supplied to the command.")
.expressionLanguageSupported(ExpressionLanguageScope.NONE)
.required(false)
.allowableValues(EXECUTION_ARGUMENTS_PROPERTY_STRATEGY, DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY)
.defaultValue(EXECUTION_ARGUMENTS_PROPERTY_STRATEGY.getValue())
.allowableValues(COMMAND_ARGUMENTS_PROPERTY_STRATEGY, DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY)
.defaultValue(COMMAND_ARGUMENTS_PROPERTY_STRATEGY.getValue())
.build();

static final PropertyDescriptor EXECUTION_ARGUMENTS = new PropertyDescriptor.Builder()
.name("Command Arguments")
.description("The arguments to supply to the executable delimited by the ';' character.")
.dependsOn(ARGUMENTS_STRATEGY, EXECUTION_ARGUMENTS_PROPERTY_STRATEGY)
.dependsOn(ARGUMENTS_STRATEGY, COMMAND_ARGUMENTS_PROPERTY_STRATEGY)
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
.addValidator((subject, input, context) -> {
ValidationResult result = new ValidationResult.Builder()
Expand All @@ -248,7 +244,7 @@ public class ExecuteStreamCommand extends AbstractProcessor {
static final PropertyDescriptor ARG_DELIMITER = new PropertyDescriptor.Builder()
.name("Argument Delimiter")
.description("Delimiter to use to separate arguments for a command [default: ;]. Must be a single character")
.dependsOn(ARGUMENTS_STRATEGY, EXECUTION_ARGUMENTS_PROPERTY_STRATEGY)
.dependsOn(ARGUMENTS_STRATEGY, COMMAND_ARGUMENTS_PROPERTY_STRATEGY)
.addValidator(StandardValidators.SINGLE_CHAR_VALIDATOR)
.required(true)
.defaultValue(";")
Expand Down Expand Up @@ -356,29 +352,6 @@ protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String
}
}

@Override
protected Collection<ValidationResult> customValidate(ValidationContext validationContext) {
final List<ValidationResult> validationResults = new ArrayList<>(super.customValidate(validationContext));

final String argumentStrategy = validationContext.getProperty(ARGUMENTS_STRATEGY).getValue();
if (DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY.getValue() != argumentStrategy) {
for (final PropertyDescriptor propertyDescriptor : validationContext.getProperties().keySet()) {
if (!propertyDescriptor.isDynamic()) {
continue;
}

final String propertyName = propertyDescriptor.getName();
final Matcher matcher = COMMAND_ARGUMENT_PATTERN.matcher(propertyName);
if (matcher.matches()) {
logger.warn("[{}] should be set to [{}] when command arguments are supplied as Dynamic Properties. The property [{}] will be ignored.",
ARGUMENTS_STRATEGY.getDisplayName(), DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY.getDisplayName(), propertyName);
}
}
}

return validationResults;
}

@Override
public void onTrigger(ProcessContext context, final ProcessSession session) throws ProcessException {
FlowFile inputFlowFile = session.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.nifi.components.PropertyDescriptor;
import org.apache.nifi.expression.ExpressionLanguageScope;
import org.apache.nifi.processors.standard.util.ArgumentUtils;
import org.apache.nifi.util.LogMessage;
import org.apache.nifi.util.MockFlowFile;
import org.apache.nifi.util.TestRunner;
import org.apache.nifi.util.TestRunners;
Expand Down Expand Up @@ -49,6 +50,34 @@
@DisabledOnOs(value = OS.WINDOWS, disabledReason = "Test only runs on *nix")
public class TestExecuteStreamCommand {

@Test
public void testDynamicPropertyArgumentsStrategyValid() {
final TestRunner runner = TestRunners.newTestRunner(ExecuteStreamCommand.class);

runner.setProperty(ExecuteStreamCommand.ARGUMENTS_STRATEGY, ExecuteStreamCommand.DYNAMIC_PROPERTY_ARGUMENTS_STRATEGY.getValue());
runner.setProperty(ExecuteStreamCommand.EXECUTION_COMMAND, "java");
runner.setProperty("command.argument.1", "-version");

runner.assertValid();

final List<LogMessage> warnMessages = runner.getLogger().getWarnMessages();
assertTrue(warnMessages.isEmpty(), "Warning Log Messages found");
}

@Test
public void testCommandArgumentsPropertyStrategyValid() {
final TestRunner runner = TestRunners.newTestRunner(ExecuteStreamCommand.class);

runner.setProperty(ExecuteStreamCommand.ARGUMENTS_STRATEGY, ExecuteStreamCommand.COMMAND_ARGUMENTS_PROPERTY_STRATEGY.getValue());
runner.setProperty(ExecuteStreamCommand.EXECUTION_COMMAND, "java");
runner.setProperty("RUNTIME_VERSION", "version-1");

runner.assertValid();

final List<LogMessage> warnMessages = runner.getLogger().getWarnMessages();
assertTrue(warnMessages.isEmpty(), "Warning Log Messages found");
}

@Test
public void testExecuteJar() throws Exception {
File exJar = new File("src/test/resources/ExecuteCommand/TestSuccess.jar");
Expand Down

0 comments on commit 71abd77

Please sign in to comment.