From a7ad81da351e0a516ba978700f8464b9d1d525b1 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Tue, 29 Jan 2019 18:13:08 +0100 Subject: [PATCH] NC-1737 Enabled warning on CLI dependent options (#679) fixes NC-1737 by adding an option dependency mechanism for "enabled" options This mechanism is aimed at preventing the use of options if the associated service is disabled. This is not a generic system as our need is simple here. But in future version of PicoCLI some options dependency mechanism may be implemented that could replace this. See https://github.com/remkop/picocli/issues/295 Our current mechanism apply to p2p, rpc (http+ws), mining, privacy and metrics. It logs a warning wherever some options are used that require another one to be enabled. Also renamed inconsistent isRpcHttpEnabled option variable and moved --privacy-enabled option to be on top of other privacy options. injects a Logger in PantheonCommand constructor to log and also provides injection capabilities for a mock in tests. adds unit tests updates tests logger name to TEST_LOGGER to prevent confusion with the mock remove --max-trailing-peers after merge with master where it was deleted updated rules for metrics Signed-off-by: Adrian Sutton --- .../java/tech/pegasys/pantheon/Pantheon.java | 2 + .../pantheon/cli/CommandLineUtils.java | 71 +++++ .../pegasys/pantheon/cli/PantheonCommand.java | 104 ++++++- .../pegasys/pantheon/util/StringUtils.java | 38 +++ .../pantheon/cli/CommandLineUtilsTest.java | 186 +++++++++++++ .../pantheon/cli/CommandTestAbstract.java | 21 +- .../pantheon/cli/PantheonCommandTest.java | 261 ++++++++++++++---- .../pantheon/util/StringUtilsTest.java | 47 ++++ 8 files changed, 660 insertions(+), 70 deletions(-) create mode 100644 pantheon/src/main/java/tech/pegasys/pantheon/cli/CommandLineUtils.java create mode 100644 pantheon/src/main/java/tech/pegasys/pantheon/util/StringUtils.java create mode 100644 pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandLineUtilsTest.java create mode 100644 pantheon/src/test/java/tech/pegasys/pantheon/util/StringUtilsTest.java diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/Pantheon.java b/pantheon/src/main/java/tech/pegasys/pantheon/Pantheon.java index e1f5b6f363..5bb6c4ba00 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/Pantheon.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/Pantheon.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon; +import static org.apache.logging.log4j.LogManager.getLogger; import static picocli.CommandLine.defaultExceptionHandler; import tech.pegasys.pantheon.cli.PantheonCommand; @@ -29,6 +30,7 @@ public final class Pantheon { final PantheonCommand pantheonCommand = new PantheonCommand( + getLogger(), new BlockImporter(), new RunnerBuilder(), new PantheonControllerBuilder(), diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/CommandLineUtils.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/CommandLineUtils.java new file mode 100644 index 0000000000..38b9b62c96 --- /dev/null +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/CommandLineUtils.java @@ -0,0 +1,71 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed 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. + */ +package tech.pegasys.pantheon.cli; + +import tech.pegasys.pantheon.util.StringUtils; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.logging.log4j.Logger; +import picocli.CommandLine; + +class CommandLineUtils { + /** + * Check if options are passed that require an option to be true to have any effect and log a + * warning with the list of affected options. + * + *

Note that in future version of PicoCLI some options dependency mechanism may be implemented + * that could replace this. See https://github.com/remkop/picocli/issues/295 + * + * @param logger the logger instance used to log the warning + * @param commandLine the command line containing the options we want to check + * @param mainOptionName the name of the main option to test dependency against. Only used for + * display. + * @param isMainOptionCondition the condition to test the options dependencies, if true will test + * if not won't + * @param dependentOptionsNames a list of option names that can't be used if condition is met. + * Example: if --miner-coinbase is in the list and condition is that --miner-enabled should + * not be false, we log a warning. + */ + static void checkOptionDependencies( + final Logger logger, + final CommandLine commandLine, + final String mainOptionName, + final boolean isMainOptionCondition, + final List dependentOptionsNames) { + if (isMainOptionCondition) { + String affectedOptions = + commandLine + .getCommandSpec() + .options() + .stream() + .filter( + option -> + Arrays.stream(option.names()).anyMatch(dependentOptionsNames::contains) + && !option.stringValues().isEmpty()) + .map(option -> option.names()[0]) + .collect( + Collectors.collectingAndThen( + Collectors.toList(), StringUtils.joiningWithLastDelimiter(", ", " and "))); + + if (!affectedOptions.isEmpty()) { + logger.warn( + "{} will have no effect unless {} is defined on the command line.", + affectedOptions, + mainOptionName); + } + } + } +} diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 23eb193b43..e93c4859d8 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -62,6 +62,7 @@ import java.net.URI; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Optional; @@ -74,6 +75,7 @@ import com.google.common.net.HostSpecifier; import io.vertx.core.Vertx; import io.vertx.core.json.DecodeException; import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.config.Configurator; import picocli.CommandLine; import picocli.CommandLine.AbstractParseResultHandler; @@ -100,6 +102,10 @@ import picocli.CommandLine.ParameterException; ) public class PantheonCommand implements DefaultCommandValues, Runnable { + private final Logger logger; + + private CommandLine commandLine; + public static class RpcApisConverter implements ITypeConverter { @Override @@ -251,7 +257,7 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { names = {"--rpc-http-enabled"}, description = "Set if the JSON-RPC service should be started (default: ${DEFAULT-VALUE})" ) - private final Boolean isHttpRpcEnabled = false; + private final Boolean isRpcHttpEnabled = false; @Option( names = {"--rpc-http-host"}, @@ -471,6 +477,12 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { ) private final Boolean permissionsAccountsEnabled = false; + @Option( + names = {"--privacy-enabled"}, + description = "Set if private transaction should be enabled (default: ${DEFAULT-VALUE})" + ) + private final Boolean privacyEnabled = false; + @Option( names = {"--privacy-url"}, description = "The URL on which enclave is running " @@ -483,12 +495,6 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { ) private final File privacyPublicKeyFile = null; - @Option( - names = {"--privacy-enabled"}, - description = "Set if private transaction should be enabled (default: ${DEFAULT-VALUE})" - ) - private final Boolean privacyEnabled = false; - @Option( names = {"--privacy-precompiled-address"}, description = @@ -497,10 +503,12 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { private final Integer privacyPrecompiledAddress = Address.PRIVACY; public PantheonCommand( + final Logger logger, final BlockImporter blockImporter, final RunnerBuilder runnerBuilder, final PantheonControllerBuilder controllerBuilder, final SynchronizerConfiguration.Builder synchronizerConfigurationBuilder) { + this.logger = logger; this.blockImporter = blockImporter; this.runnerBuilder = runnerBuilder; this.controllerBuilder = controllerBuilder; @@ -514,7 +522,7 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { final DefaultExceptionHandler> exceptionHandler, final String... args) { - final CommandLine commandLine = new CommandLine(this); + commandLine = new CommandLine(this); commandLine.setCaseInsensitiveEnumValuesAllowed(true); @@ -553,10 +561,26 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { Configurator.setAllLevels("", logLevel); } - if (!p2pEnabled && (bootNodes != null && !bootNodes.isEmpty())) { - throw new ParameterException( - new CommandLine(this), "Unable to specify bootnodes if p2p is disabled."); - } + // Check that p2p options are able top work or send an error + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--p2p-enabled", + !p2pEnabled, + Arrays.asList( + "--bootnodes", + "--discovery-enabled", + "--max-peers", + "--banned-node-id", + "--banned-node-ids")); + + // Check that mining options are able top work or send an error + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--miner-enabled", + !isMiningEnabled, + Arrays.asList("--miner-coinbase", "--min-gas-price", "--miner-extra-data")); //noinspection ConstantConditions if (isMiningEnabled && coinbase == null) { @@ -627,8 +651,21 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { } private JsonRpcConfiguration jsonRpcConfiguration() { + + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--rpc-http-enabled", + !isRpcHttpEnabled, + Arrays.asList( + "--rpc-http-api", + "--rpc-http-apis", + "--rpc-http-cors-origins", + "--rpc-http-host", + "--rpc-http-port")); + final JsonRpcConfiguration jsonRpcConfiguration = JsonRpcConfiguration.createDefault(); - jsonRpcConfiguration.setEnabled(isHttpRpcEnabled); + jsonRpcConfiguration.setEnabled(isRpcHttpEnabled); jsonRpcConfiguration.setHost(rpcHttpHost.toString()); jsonRpcConfiguration.setPort(rpcHttpPort); jsonRpcConfiguration.setCorsAllowedDomains(rpcHttpCorsAllowedOrigins); @@ -638,6 +675,19 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { } private WebSocketConfiguration webSocketConfiguration() { + + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--rpc-ws-enabled", + !isRpcWsEnabled, + Arrays.asList( + "--rpc-ws-api", + "--rpc-ws-apis", + "--rpc-ws-refresh-delay", + "--rpc-ws-host", + "--rpc-ws-port")); + final WebSocketConfiguration webSocketConfiguration = WebSocketConfiguration.createDefault(); webSocketConfiguration.setEnabled(isRpcWsEnabled); webSocketConfiguration.setHost(rpcWsHost.toString()); @@ -655,6 +705,24 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { + "time. Please refer to CLI reference for more details about this constraint."); } + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--metrics-enabled", + !isMetricsEnabled, + Arrays.asList("--metrics-host", "--metrics-port")); + + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--metrics-push-enabled", + !isMetricsPushEnabled, + Arrays.asList( + "--metrics-push-host", + "--metrics-push-port", + "--metrics-push-interval", + "--metrics-push-prometheus-job")); + final MetricsConfiguration metricsConfiguration = createDefault(); metricsConfiguration.setEnabled(isMetricsEnabled); metricsConfiguration.setHost(metricsHost.toString()); @@ -683,6 +751,16 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { } private PrivacyParameters orionConfiguration() { + + // Check that mining options are able top work or send an error + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--privacy-enabled", + !privacyEnabled, + Arrays.asList( + "--privacy-url", "--privacy-public-key-file", "--privacy-precompiled-address")); + final PrivacyParameters privacyParameters = PrivacyParameters.noPrivacy(); privacyParameters.setEnabled(privacyEnabled); privacyParameters.setUrl(privacyUrl.toString()); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/util/StringUtils.java b/pantheon/src/main/java/tech/pegasys/pantheon/util/StringUtils.java new file mode 100644 index 0000000000..67ad700725 --- /dev/null +++ b/pantheon/src/main/java/tech/pegasys/pantheon/util/StringUtils.java @@ -0,0 +1,38 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed 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. + */ +package tech.pegasys.pantheon.util; + +import java.util.List; +import java.util.function.Function; + +/** some useful tools to display strings in command line help or error messages */ +public class StringUtils { + + /** + * Joins a list into string elements with a delimiter but having a last different delimiter + * Example: "this thing, that thing and this other thing" + * + * @param delimiter delimiter for all the items except before the last one + * @param lastDelimiter delimiter before the last item + * @return a delimited string representation of the list + */ + public static Function, String> joiningWithLastDelimiter( + final String delimiter, final String lastDelimiter) { + return list -> { + final int last = list.size() - 1; + if (last < 1) return String.join(delimiter, list); + return String.join( + lastDelimiter, String.join(delimiter, list.subList(0, last)), list.get(last)); + }; + } +} diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandLineUtilsTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandLineUtilsTest.java new file mode 100644 index 0000000000..c62b4400a4 --- /dev/null +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandLineUtilsTest.java @@ -0,0 +1,186 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed 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. + */ +package tech.pegasys.pantheon.cli; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static picocli.CommandLine.defaultExceptionHandler; + +import java.util.ArrayList; +import java.util.Arrays; + +import org.apache.logging.log4j.Logger; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import picocli.CommandLine; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; +import picocli.CommandLine.RunLast; + +@RunWith(MockitoJUnitRunner.class) +public class CommandLineUtilsTest { + @Mock Logger mockLogger; + + @Command(description = "This command is for testing.", name = "testcommand") + private abstract static class AbstractTestCommand implements Runnable { + + final Logger logger; + final CommandLine commandLine; + + AbstractTestCommand(final Logger logger) { + this.logger = logger; + commandLine = new CommandLine(this); + } + + // Completely disables p2p within Pantheon. + @Option( + names = {"--option-enabled"}, + arity = "1" + ) + final Boolean optionEnabled = true; + + @Option(names = {"--option2"}) + private final Integer option2 = 2; + + @Option(names = {"--option3"}) + private final Integer option3 = 3; + + @Option(names = {"--option4"}) + private final Integer option4 = 4; + } + + private static class TestCommandWithDeps extends AbstractTestCommand { + + TestCommandWithDeps(final Logger logger) { + super(logger); + } + + @Override + public void run() { + // Check that mining options are able top work or send an error + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + "--option-enabled", + !optionEnabled, + Arrays.asList("--option2", "--option3")); + } + } + + private static class TestCommandWithoutDeps extends AbstractTestCommand { + + TestCommandWithoutDeps(final Logger logger) { + super(logger); + } + + @Override + public void run() { + // Check that mining options are able top work or send an error + CommandLineUtils.checkOptionDependencies( + logger, commandLine, "--option-enabled", !optionEnabled, new ArrayList<>()); + } + } + + @Test + public void optionsAreNotExpected() { + AbstractTestCommand testCommand = new TestCommandWithDeps(mockLogger); + testCommand.commandLine.parseWithHandlers( + new RunLast(), + defaultExceptionHandler(), + "--option-enabled", + "false", + "--option2", + "20", + "--option3", + "30", + "--option4", + "40"); + verifyOptionsConstraintLoggerCall(mockLogger, "--option2 and --option3", "--option-enabled"); + } + + @Test + public void optionIsNotExpected() { + AbstractTestCommand testCommand = new TestCommandWithDeps(mockLogger); + testCommand.commandLine.parseWithHandlers( + new RunLast(), + defaultExceptionHandler(), + "--option-enabled", + "false", + "--option2", + "20", + "--option4", + "40"); + verifyOptionsConstraintLoggerCall(mockLogger, "--option2", "--option-enabled"); + } + + @Test + public void optionsAreExpected() { + AbstractTestCommand testCommand = new TestCommandWithDeps(mockLogger); + testCommand.commandLine.parseWithHandlers( + new RunLast(), + defaultExceptionHandler(), + "--option2", + "20", + "--option3", + "30", + "--option4", + "40"); + verifyNoMoreInteractions(mockLogger); + } + + @Test + public void noDependencies() { + AbstractTestCommand testCommand = new TestCommandWithoutDeps(mockLogger); + testCommand.commandLine.parseWithHandlers( + new RunLast(), + defaultExceptionHandler(), + "--option-enabled", + "false", + "--option2", + "20", + "--option3", + "30", + "--option4", + "40"); + verifyNoMoreInteractions(mockLogger); + } + + /** + * Check logger calls + * + *

Here we check the calls to logger and not the result of the log line as we don't test the + * logger itself but the fact that we call it. + * + * @param dependentOptions the string representing the list of dependent options names + * @param mainOption the main option name + */ + private void verifyOptionsConstraintLoggerCall( + final Logger logger, final String dependentOptions, final String mainOption) { + + ArgumentCaptor stringArgumentCaptor = ArgumentCaptor.forClass(String.class); + + verify(logger) + .warn( + stringArgumentCaptor.capture(), + stringArgumentCaptor.capture(), + stringArgumentCaptor.capture()); + assertThat(stringArgumentCaptor.getAllValues().get(0)) + .isEqualTo("{} will have no effect unless {} is defined on the command line."); + assertThat(stringArgumentCaptor.getAllValues().get(1)).isEqualTo(dependentOptions); + assertThat(stringArgumentCaptor.getAllValues().get(2)).isEqualTo(mainOption); + } +} diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java index de45e2171e..7bd60b9291 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java @@ -56,7 +56,7 @@ import picocli.CommandLine.RunLast; @RunWith(MockitoJUnitRunner.class) public abstract class CommandTestAbstract { - private final Logger LOG = LogManager.getLogger(); + private final Logger TEST_LOGGER = LogManager.getLogger(); final ByteArrayOutputStream commandOutput = new ByteArrayOutputStream(); private final PrintStream outPrintStream = new PrintStream(commandOutput); @@ -71,6 +71,7 @@ public abstract class CommandTestAbstract { @Mock SynchronizerConfiguration mockSyncConf; @Mock PantheonController mockController; @Mock BlockImporter mockBlockImporter; + @Mock Logger mockLogger; @Captor ArgumentCaptor> stringListArgumentCaptor; @Captor ArgumentCaptor pathArgumentCaptor; @@ -128,15 +129,19 @@ public abstract class CommandTestAbstract { // Display outputs for debug purpose @After public void displayOutput() { - LOG.info("Standard output {}", commandOutput.toString()); - LOG.info("Standard error {}", commandErrorOutput.toString()); + TEST_LOGGER.info("Standard output {}", commandOutput.toString()); + TEST_LOGGER.info("Standard error {}", commandErrorOutput.toString()); } CommandLine.Model.CommandSpec parseCommand(final String... args) { final TestPantheonCommand pantheonCommand = new TestPantheonCommand( - mockBlockImporter, mockRunnerBuilder, mockControllerBuilder, mockSyncConfBuilder); + mockLogger, + mockBlockImporter, + mockRunnerBuilder, + mockControllerBuilder, + mockSyncConfBuilder); // parse using Ansi.OFF to be able to assert on non formatted output results pantheonCommand.parse( @@ -151,11 +156,17 @@ public abstract class CommandTestAbstract { @CommandLine.Spec CommandLine.Model.CommandSpec spec; TestPantheonCommand( + final Logger mockLogger, final BlockImporter mockBlockImporter, final RunnerBuilder mockRunnerBuilder, final PantheonControllerBuilder mockControllerBuilder, final SynchronizerConfiguration.Builder mockSyncConfBuilder) { - super(mockBlockImporter, mockRunnerBuilder, mockControllerBuilder, mockSyncConfBuilder); + super( + mockLogger, + mockBlockImporter, + mockRunnerBuilder, + mockControllerBuilder, + mockSyncConfBuilder); } } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index aa519be34c..845f2fb8d2 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -592,12 +592,28 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void p2pEnabledOptionFalseValueCannotAlsoHaveBootnodesSpecified() { - parseCommand("--p2p-enabled", "false", "--bootnodes", String.join(",", validENodeStrings)); + public void p2pOptionsRequiresServiceToBeEnabled() { + final String[] nodes = {"0001", "0002", "0003"}; + + parseCommand( + "--p2p-enabled", + "false", + "--bootnodes", + String.join(",", validENodeStrings), + "--discovery-enabled", + "false", + "--max-peers", + "42", + "--banned-node-id", + String.join(",", nodes), + "--banned-node-ids", + String.join(",", nodes)); + + verifyOptionsConstraintLoggerCall( + "--discovery-enabled, --bootnodes, --max-peers and --banned-node-ids", "--p2p-enabled"); assertThat(commandOutput.toString()).isEmpty(); - assertThat(commandErrorOutput.toString()) - .contains("Unable to specify bootnodes if p2p is disabled."); + assertThat(commandErrorOutput.toString()).isEmpty(); } @Test @@ -686,8 +702,8 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void callingWithRefreshDelayWithValueMustUseValue() { - parseCommand("--rpc-ws-refresh-delay", "2000"); + public void rpcWsRefreshDelayMustBeUsed() { + parseCommand("--rpc-ws-enabled", "--rpc-ws-refresh-delay", "2000"); verify(mockRunnerBuilder).webSocketConfiguration(wsRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -699,8 +715,8 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void callingWithRefreshDelayWithNegativeValueMustError() { - parseCommand("--rpc-ws-refresh-delay", "-2000"); + public void rpcWsRefreshDelayWithNegativeValueMustError() { + parseCommand("--rpc-ws-enabled", "--rpc-ws-refresh-delay", "-2000"); assertThat(commandOutput.toString()).isEmpty(); final String expectedErrorMsg = "Refresh delay must be a positive integer between"; @@ -769,7 +785,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcEnabledPropertyDefaultIsFalse() { + public void rpcHttpEnabledPropertyDefaultIsFalse() { parseCommand(); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); @@ -782,7 +798,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcEnabledPropertyMustBeUsed() { + public void rpcHttpEnabledPropertyMustBeUsed() { parseCommand("--rpc-http-enabled"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); @@ -796,7 +812,7 @@ public class PantheonCommandTest extends CommandTestAbstract { @Test public void rpcApisPropertyMustBeUsed() { - parseCommand("--rpc-http-api", "ETH,NET"); + parseCommand("--rpc-http-api", "ETH,NET", "--rpc-http-enabled"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -808,6 +824,26 @@ public class PantheonCommandTest extends CommandTestAbstract { assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void rpcHttpOptionsRequiresServiceToBeEnabled() { + parseCommand( + "--rpc-http-api", + "ETH,NET", + "--rpc-http-host", + "0.0.0.0", + "--rpc-http-port", + "1234", + "--rpc-http-cors-origins", + "all"); + + verifyOptionsConstraintLoggerCall( + "--rpc-http-host, --rpc-http-port, --rpc-http-cors-origins and --rpc-http-api", + "--rpc-http-enabled"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Test public void rpcApisPropertyWithInvalidEntryMustDisplayError() { parseCommand("--rpc-http-api", "BOB"); @@ -821,11 +857,12 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostAndPortOptionsMustBeUsed() { + public void rpcHttpHostAndPortOptionsMustBeUsed() { final String host = "1.2.3.4"; final int port = 1234; - parseCommand("--rpc-http-host", host, "--rpc-http-port", String.valueOf(port)); + parseCommand( + "--rpc-http-enabled", "--rpc-http-host", host, "--rpc-http-port", String.valueOf(port)); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -838,9 +875,9 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsTwoDomainsMustBuildListWithBothDomains() { + public void rpcHttpCorsOriginsTwoDomainsMustBuildListWithBothDomains() { final String[] origins = {"http://domain1.com", "https://domain2.com"}; - parseCommand("--rpc-http-cors-origins", String.join(",", origins)); + parseCommand("--rpc-http-enabled", "--rpc-http-cors-origins", String.join(",", origins)); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -853,9 +890,9 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsDoubleCommaFilteredOut() { + public void rpcHttpCorsOriginsDoubleCommaFilteredOut() { final String[] origins = {"http://domain1.com", "https://domain2.com"}; - parseCommand("--rpc-http-cors-origins", String.join(",,", origins)); + parseCommand("--rpc-http-enabled", "--rpc-http-cors-origins", String.join(",,", origins)); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -868,9 +905,9 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsWithWildcardMustBuildListWithWildcard() { + public void rpcHttpCorsOriginsWithWildcardMustBuildListWithWildcard() { final String[] origins = {"*"}; - parseCommand("--rpc-http-cors-origins", String.join(",", origins)); + parseCommand("--rpc-http-enabled", "--rpc-http-cors-origins", String.join(",", origins)); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -883,8 +920,8 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsWithAllMustBuildListWithWildcard() { - parseCommand("--rpc-http-cors-origins", "all"); + public void rpcHttpCorsOriginsWithAllMustBuildListWithWildcard() { + parseCommand("--rpc-http-enabled", "--rpc-http-cors-origins", "all"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -896,9 +933,9 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsWithNoneMustBuildEmptyList() { + public void rpcHttpCorsOriginsWithNoneMustBuildEmptyList() { final String[] origins = {"none"}; - parseCommand("--rpc-http-cors-origins", String.join(",", origins)); + parseCommand("--rpc-http-enabled", "--rpc-http-cors-origins", String.join(",", origins)); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -910,7 +947,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsNoneWithAnotherDomainMustFail() { + public void rpcHttpCorsOriginsNoneWithAnotherDomainMustFail() { final String[] origins = {"http://domain1.com", "none"}; parseCommand("--rpc-http-cors-origins", String.join(",", origins)); @@ -922,7 +959,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsNoneWithAnotherDomainMustFailNoneFirst() { + public void rpcHttpCorsOriginsNoneWithAnotherDomainMustFailNoneFirst() { final String[] origins = {"none", "http://domain1.com"}; parseCommand("--rpc-http-cors-origins", String.join(",", origins)); @@ -934,7 +971,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsAllWithAnotherDomainMustFail() { + public void rpcHttpCorsOriginsAllWithAnotherDomainMustFail() { parseCommand("--rpc-http-cors-origins=http://domain1.com,all"); verifyZeroInteractions(mockRunnerBuilder); @@ -945,7 +982,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsAllWithAnotherDomainMustFailAsFlags() { + public void rpcHttpCorsOriginsAllWithAnotherDomainMustFailAsFlags() { parseCommand("--rpc-http-cors-origins=http://domain1.com", "--rpc-http-cors-origins=all"); verifyZeroInteractions(mockRunnerBuilder); @@ -956,7 +993,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsWildcardWithAnotherDomainMustFail() { + public void rpcHttpCorsOriginsWildcardWithAnotherDomainMustFail() { parseCommand("--rpc-http-cors-origins=http://domain1.com,*"); verifyZeroInteractions(mockRunnerBuilder); @@ -967,7 +1004,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsWildcardWithAnotherDomainMustFailAsFlags() { + public void rpcHttpCorsOriginsWildcardWithAnotherDomainMustFailAsFlags() { parseCommand("--rpc-http-cors-origins=http://domain1.com", "--rpc-http-cors-origins=*"); verifyZeroInteractions(mockRunnerBuilder); @@ -978,7 +1015,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsInvalidRegexShouldFail() { + public void rpcHttpCorsOriginsInvalidRegexShouldFail() { final String[] origins = {"**"}; parseCommand("--rpc-http-cors-origins", String.join(",", origins)); @@ -990,7 +1027,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcCorsOriginsEmtyValueFails() { + public void rpcHttpCorsOriginsEmtyValueFails() { parseCommand("--rpc-http-cors-origins="); verifyZeroInteractions(mockRunnerBuilder); @@ -1001,7 +1038,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistAcceptsSingleArgument() { + public void rpcHttpHostWhitelistAcceptsSingleArgument() { parseCommand("--host-whitelist", "a"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); @@ -1017,7 +1054,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistAcceptsMultipleArguments() { + public void rpcHttpHostWhitelistAcceptsMultipleArguments() { parseCommand("--host-whitelist", "a,b"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); @@ -1033,7 +1070,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistAcceptsDoubleComma() { + public void rpcHttpHostWhitelistAcceptsDoubleComma() { parseCommand("--host-whitelist", "a,,b"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); @@ -1049,7 +1086,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistAcceptsMultipleFlags() { + public void rpcHttpHostWhitelistAcceptsMultipleFlags() { parseCommand("--host-whitelist=a", "--host-whitelist=b"); verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture()); @@ -1065,7 +1102,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistStarWithAnotherHostnameMustFail() { + public void rpcHttpHostWhitelistStarWithAnotherHostnameMustFail() { final String[] origins = {"friend", "*"}; parseCommand("--host-whitelist", String.join(",", origins)); @@ -1077,7 +1114,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistStarWithAnotherHostnameMustFailStarFirst() { + public void rpcHttpHostWhitelistStarWithAnotherHostnameMustFailStarFirst() { final String[] origins = {"*", "friend"}; parseCommand("--host-whitelist", String.join(",", origins)); @@ -1089,7 +1126,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistAllWithAnotherHostnameMustFail() { + public void rpcHttpHostWhitelistAllWithAnotherHostnameMustFail() { final String[] origins = {"friend", "all"}; parseCommand("--host-whitelist", String.join(",", origins)); @@ -1101,7 +1138,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistWithNoneMustBuildEmptyList() { + public void rpcHttpHostWhitelistWithNoneMustBuildEmptyList() { final String[] origins = {"none"}; parseCommand("--host-whitelist", String.join(",", origins)); @@ -1115,7 +1152,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistNoneWithAnotherDomainMustFail() { + public void rpcHttpHostWhitelistNoneWithAnotherDomainMustFail() { final String[] origins = {"http://domain1.com", "none"}; parseCommand("--host-whitelist", String.join(",", origins)); @@ -1127,7 +1164,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistNoneWithAnotherDomainMustFailNoneFirst() { + public void rpcHttpHostWhitelistNoneWithAnotherDomainMustFailNoneFirst() { final String[] origins = {"none", "http://domain1.com"}; parseCommand("--host-whitelist", String.join(",", origins)); @@ -1139,7 +1176,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void jsonRpcHostWhitelistEmptyValueFails() { + public void rpcHttpHostWhitelistEmptyValueFails() { parseCommand("--host-whitelist="); verifyZeroInteractions(mockRunnerBuilder); @@ -1150,7 +1187,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void wsRpcEnabledPropertyDefaultIsFalse() { + public void rpcWsRpcEnabledPropertyDefaultIsFalse() { parseCommand(); verify(mockRunnerBuilder).webSocketConfiguration(wsRpcConfigArgumentCaptor.capture()); @@ -1163,7 +1200,7 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void wsRpcEnabledPropertyMustBeUsed() { + public void rpcWsRpcEnabledPropertyMustBeUsed() { parseCommand("--rpc-ws-enabled"); verify(mockRunnerBuilder).webSocketConfiguration(wsRpcConfigArgumentCaptor.capture()); @@ -1176,8 +1213,28 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void wsApiPropertyMustBeUsed() { - parseCommand("--rpc-ws-api", "ETH, NET"); + public void rpcWsOptionsRequiresServiceToBeEnabled() { + parseCommand( + "--rpc-ws-api", + "ETH,NET", + "--rpc-ws-host", + "0.0.0.0", + "--rpc-ws-port", + "1234", + "--rpc-ws-refresh-delay", + "2"); + + verifyOptionsConstraintLoggerCall( + "--rpc-ws-host, --rpc-ws-port, --rpc-ws-api and --rpc-ws-refresh-delay", + "--rpc-ws-enabled"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + + @Test + public void rpcWsApiPropertyMustBeUsed() { + parseCommand("--rpc-ws-enabled", "--rpc-ws-api", "ETH, NET"); verify(mockRunnerBuilder).webSocketConfiguration(wsRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -1190,10 +1247,10 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void wsRpcHostAndPortOptionMustBeUsed() { + public void rpcWsHostAndPortOptionMustBeUsed() { final String host = "1.2.3.4"; final int port = 1234; - parseCommand("--rpc-ws-host", host, "--rpc-ws-port", String.valueOf(port)); + parseCommand("--rpc-ws-enabled", "--rpc-ws-host", host, "--rpc-ws-port", String.valueOf(port)); verify(mockRunnerBuilder).webSocketConfiguration(wsRpcConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -1231,11 +1288,42 @@ public class PantheonCommandTest extends CommandTestAbstract { assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void metricsPushOptionsRequiresPushToBeEnabled() { + parseCommand( + "--metrics-push-host", + "0.0.0.0", + "--metrics-push-port", + "1234", + "--metrics-push-interval", + "2", + "--metrics-push-prometheus-job", + "job-name"); + + verifyOptionsConstraintLoggerCall( + "--metrics-push-host, --metrics-push-port, --metrics-push-interval and --metrics-push-prometheus-job", + "--metrics-push-enabled"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + + @Test + public void metricsOptionsRequiresPullMetricsToBeEnabled() { + parseCommand("--metrics-host", "0.0.0.0", "--metrics-port", "1234"); + + verifyOptionsConstraintLoggerCall("--metrics-host and --metrics-port", "--metrics-enabled"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Test public void metricsHostAndPortOptionMustBeUsed() { final String host = "1.2.3.4"; final int port = 1234; - parseCommand("--metrics-host", host, "--metrics-port", String.valueOf(port)); + parseCommand( + "--metrics-enabled", "--metrics-host", host, "--metrics-port", String.valueOf(port)); verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -1264,7 +1352,12 @@ public class PantheonCommandTest extends CommandTestAbstract { public void metricsPushHostAndPushPortOptionMustBeUsed() { final String host = "1.2.3.4"; final int port = 1234; - parseCommand("--metrics-push-host", host, "--metrics-push-port", String.valueOf(port)); + parseCommand( + "--metrics-push-enabled", + "--metrics-push-host", + host, + "--metrics-push-port", + String.valueOf(port)); verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -1278,7 +1371,7 @@ public class PantheonCommandTest extends CommandTestAbstract { @Test public void metricsPushIntervalMustBeUsed() { - parseCommand("--metrics-push-interval", "42"); + parseCommand("--metrics-push-enabled", "--metrics-push-interval", "42"); verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -1291,7 +1384,8 @@ public class PantheonCommandTest extends CommandTestAbstract { @Test public void metricsPrometheusJobMustBeUsed() { - parseCommand("--metrics-push-prometheus-job", "pantheon-command-test"); + parseCommand( + "--metrics-push-enabled", "--metrics-push-prometheus-job", "pantheon-command-test"); verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); @@ -1345,12 +1439,32 @@ public class PantheonCommandTest extends CommandTestAbstract { .isEqualTo(Optional.of(Address.fromHexString(coinbaseStr))); } + @Test + public void miningOptionsRequiresServiceToBeEnabled() { + + final Address requestedCoinbase = Address.fromHexString("0000011111222223333344444"); + parseCommand( + "--miner-coinbase", + requestedCoinbase.toString(), + "--min-gas-price", + "42", + "--miner-extra-data", + "0x1122334455667788990011223344556677889900112233445566778899001122"); + + verifyOptionsConstraintLoggerCall( + "--miner-coinbase, --min-gas-price and --miner-extra-data", "--miner-enabled"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Test public void miningParametersAreCaptured() throws Exception { final Address requestedCoinbase = Address.fromHexString("0000011111222223333344444"); final String extraDataString = "0x1122334455667788990011223344556677889900112233445566778899001122"; parseCommand( + "--miner-enabled", "--miner-coinbase=" + requestedCoinbase.toString(), "--min-gas-price=15", "--miner-extra-data=" + extraDataString); @@ -1527,6 +1641,27 @@ public class PantheonCommandTest extends CommandTestAbstract { assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void privacyOptionsRequiresServiceToBeEnabled() { + + final File file = new File("./specific/public_key"); + + parseCommand( + "--privacy-url", + ORION_URI, + "--privacy-public-key-file", + file.getPath(), + "--privacy-precompiled-address", + String.valueOf(Byte.MAX_VALUE - 1)); + + verifyOptionsConstraintLoggerCall( + "--privacy-url, --privacy-public-key-file and --privacy-precompiled-address", + "--privacy-enabled"); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Test public void mustVerifyPrivacyIsDisabled() throws IOException { parseCommand(); @@ -1559,4 +1694,26 @@ public class PantheonCommandTest extends CommandTestAbstract { private static String escapeTomlString(final String s) { return StringEscapeUtils.escapeJava(s); } + + /** + * Check logger calls + * + *

Here we check the calls to logger and not the result of the log line as we don't test the + * logger itself but the fact that we call it. + * + * @param dependentOptions the string representing the list of dependent options names + * @param mainOption the main option name + */ + private void verifyOptionsConstraintLoggerCall( + final String dependentOptions, final String mainOption) { + verify(mockLogger) + .warn( + stringArgumentCaptor.capture(), + stringArgumentCaptor.capture(), + stringArgumentCaptor.capture()); + assertThat(stringArgumentCaptor.getAllValues().get(0)) + .isEqualTo("{} will have no effect unless {} is defined on the command line."); + assertThat(stringArgumentCaptor.getAllValues().get(1)).isEqualTo(dependentOptions); + assertThat(stringArgumentCaptor.getAllValues().get(2)).isEqualTo(mainOption); + } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/util/StringUtilsTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/util/StringUtilsTest.java new file mode 100644 index 0000000000..a2c4657b42 --- /dev/null +++ b/pantheon/src/test/java/tech/pegasys/pantheon/util/StringUtilsTest.java @@ -0,0 +1,47 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed 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. + */ +package tech.pegasys.pantheon.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.stream.Collectors; + +import org.junit.Test; + +public class StringUtilsTest { + + @Test + public void joiningWithLastDelimiter() { + Map, String> testCases = new HashMap<>(); + //noinspection ArraysAsListWithZeroOrOneArgument + testCases.put(Arrays.asList("item1"), "item1"); + testCases.put(Arrays.asList("item1", "item2"), "item1 and item2"); + testCases.put(Arrays.asList("item1", "item2", "item3"), "item1, item2 and item3"); + + for (Entry, String> entry : testCases.entrySet()) { + String joinedResult = + entry + .getKey() + .stream() + .collect( + Collectors.collectingAndThen( + Collectors.toList(), StringUtils.joiningWithLastDelimiter(", ", " and "))); + assertThat(joinedResult).isEqualTo(entry.getValue()); + } + } +}