From 343d0ff66c385050478007a6326c5f6b377dabae Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 29 Feb 2024 13:52:57 +1100 Subject: [PATCH 01/34] Add option to only load known plugins Signed-off-by: Gabriel-Trintinalia --- .../org/hyperledger/besu/cli/BesuCommand.java | 64 +++++++++++-------- .../besu/cli/DefaultCommandValues.java | 3 + .../cli/converter/PluginInfoConverter.java | 31 +++++++++ .../stable/PluginsConfigurationOptions.java | 36 +++++++++++ .../besu/cli/util/CommandLineUtils.java | 33 ++++++++++ .../util/ConfigOptionSearchAndRunHandler.java | 12 +--- .../besu/services/BesuPluginContextImpl.java | 21 +++++- .../ConfigOptionSearchAndRunHandlerTest.java | 10 +-- 8 files changed, 164 insertions(+), 46 deletions(-) create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java create mode 100644 besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index da9f086eeb0..de43ef54493 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -44,6 +44,7 @@ import org.hyperledger.besu.cli.config.ProfileName; import org.hyperledger.besu.cli.converter.MetricCategoryConverter; import org.hyperledger.besu.cli.converter.PercentageConverter; +import org.hyperledger.besu.cli.converter.PluginInfoConverter; import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty; import org.hyperledger.besu.cli.error.BesuExecutionExceptionHandler; import org.hyperledger.besu.cli.error.BesuParameterExceptionHandler; @@ -58,6 +59,7 @@ import org.hyperledger.besu.cli.options.stable.NodePrivateKeyFileOption; import org.hyperledger.besu.cli.options.stable.P2PTLSConfigOptions; import org.hyperledger.besu.cli.options.stable.PermissionsOptions; +import org.hyperledger.besu.cli.options.stable.PluginsConfigurationOptions; import org.hyperledger.besu.cli.options.stable.RpcWebsocketOptions; import org.hyperledger.besu.cli.options.unstable.ChainPruningOptions; import org.hyperledger.besu.cli.options.unstable.DnsOptions; @@ -892,6 +894,10 @@ static class MetricsOptionGroup { @Mixin private PkiBlockCreationOptions pkiBlockCreationOptions; + // Plugins Configuration Option Group + @CommandLine.ArgGroup(validate = false, heading = "@|bold Plugin Configuration Options|@%n") + PluginsConfigurationOptions pluginsConfigurationOptions = new PluginsConfigurationOptions(); + private EthNetworkConfig ethNetworkConfig; private JsonRpcConfiguration jsonRpcConfiguration; private JsonRpcConfiguration engineJsonRpcConfiguration; @@ -1050,10 +1056,21 @@ public int parse( handleUnstableOptions(); preparePlugins(); - final int exitCode = - parse(resultHandler, executionExceptionHandler, parameterExceptionHandler, args); + final ConfigOptionSearchAndRunHandler configParsingHandler = + new ConfigOptionSearchAndRunHandler( + (parseResult -> { + registerPlugins(parseResult); + commandLine.setExecutionStrategy(resultHandler); + commandLine.execute(parseResult.originalArgs().toArray(new String[0])); + return 0; + }), + environment); - return exitCode; + return commandLine + .setExecutionStrategy(configParsingHandler) + .setParameterExceptionHandler(parameterExceptionHandler) + .setExecutionExceptionHandler(executionExceptionHandler) + .execute(args); } /** Used by Dagger to parse all options into a commandline instance. */ @@ -1218,8 +1235,6 @@ private void preparePlugins() { rocksDBPlugin.register(besuPluginContext); new InMemoryStoragePlugin().register(besuPluginContext); - besuPluginContext.registerPlugins(pluginsDir()); - metricCategoryRegistry .getMetricCategories() .forEach(metricCategoryConverter::addRegistryCategory); @@ -1233,6 +1248,25 @@ private SecurityModule defaultSecurityModule() { return new KeyPairSecurityModule(loadKeyPair(nodePrivateKeyFileOption.getNodePrivateKeyFile())); } + private void registerPlugins(final CommandLine.ParseResult parseResult) { + var pluginsStrictRegistration = + CommandLineUtils.getOptionValueOrDefault( + parseResult.commandSpec().commandLine(), + DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, + Boolean::valueOf); + + List plugins = List.of(); + if (Boolean.TRUE.equals(pluginsStrictRegistration)) { + plugins = + CommandLineUtils.getOptionValueOrDefault( + parseResult.commandSpec().commandLine(), + DEFAULT_PLUGINS_OPTION_NAME, + new PluginInfoConverter()); + logger.info("Plugins to register: {}", plugins); + } + besuPluginContext.registerPlugins(pluginsDir(), plugins); + } + // loadKeyPair() is public because it is accessed by subcommands /** @@ -1245,26 +1279,6 @@ public KeyPair loadKeyPair(final File nodePrivateKeyFile) { return KeyPairUtil.loadKeyPair(resolveNodePrivateKeyFile(nodePrivateKeyFile)); } - private int parse( - final CommandLine.IExecutionStrategy resultHandler, - final BesuExecutionExceptionHandler besuExecutionExceptionHandler, - final BesuParameterExceptionHandler besuParameterExceptionHandler, - final String... args) { - // Create a handler that will search for a config file option and use it for - // default values - // and eventually it will run regular parsing of the remaining options. - - final ConfigOptionSearchAndRunHandler configParsingHandler = - new ConfigOptionSearchAndRunHandler( - resultHandler, besuParameterExceptionHandler, environment); - - return commandLine - .setExecutionStrategy(configParsingHandler) - .setParameterExceptionHandler(besuParameterExceptionHandler) - .setExecutionExceptionHandler(besuExecutionExceptionHandler) - .execute(args); - } - private void preSynchronization() { preSynchronizationTaskRunner.runTasks(besuController); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java index e62d76baf38..e38a470c708 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java @@ -96,6 +96,9 @@ public interface DefaultCommandValues { /** The Default tls protocols. */ List DEFAULT_TLS_PROTOCOLS = List.of("TLSv1.3", "TLSv1.2"); + String DEFAULT_PLUGINS_OPTION_NAME = "--plugins"; + String DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME = "--plugins-strict-registration-enabled"; + /** * Gets default besu data path. * diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java new file mode 100644 index 00000000000..786d6a848be --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java @@ -0,0 +1,31 @@ +package org.hyperledger.besu.cli.converter; + +import java.util.Arrays; +import java.util.List; + +import picocli.CommandLine; + +public class PluginInfoConverter + implements CommandLine.ITypeConverter> { + + @Override + public List convert(final String value) { + return Arrays.stream(value.split(",")).map(this::toPluginInfo).toList(); + } + + private PluginInfo toPluginInfo(final String pluginName) { + return new PluginInfo(pluginName); + } + + public static class PluginInfo { + String name; + + PluginInfo(final String name) { + this.name = name; + } + + public String getName() { + return name; + } + } +} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java new file mode 100644 index 00000000000..aa153ab7f19 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -0,0 +1,36 @@ +package org.hyperledger.besu.cli.options.stable; + +import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_OPTION_NAME; +import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME; + +import org.hyperledger.besu.cli.converter.PluginInfoConverter; + +import java.util.List; + +import picocli.CommandLine; + +public class PluginsConfigurationOptions { + @CommandLine.Option( + names = {DEFAULT_PLUGINS_OPTION_NAME}, + description = "Comma separated list of plugins.", + split = ",", + converter = PluginInfoConverter.class, + arity = "0..*") + @SuppressWarnings({ + "FieldCanBeFinal", + "FieldMayBeFinal", + "UnusedVariable" + }) // PicoCLI requires non-final Strings. + private List plugins = null; + + @CommandLine.Option( + names = {DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME}, + defaultValue = "false", + description = "Comma separated list of plugins.") + @SuppressWarnings({ + "FieldCanBeFinal", + "FieldMayBeFinal", + "UnusedVariable" + }) // PicoCLI requires non-final Strings. + private boolean requireExplicitPlugins = false; +} diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java index 397592459c6..6731b00a193 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java @@ -261,4 +261,37 @@ public static boolean isOptionSet(final CommandLine commandLine, final String op .filter(optionSpec -> Arrays.stream(optionSpec.names()).anyMatch(optionName::equals)) .anyMatch(CommandLineUtils::isOptionSet); } + + public static T getOptionValueOrDefault( + final CommandLine commandLine, + final String optionName, + final CommandLine.ITypeConverter converter) { + + // Attempt to get the option's value if it was provided by the user + T value = commandLine.getParseResult().matchedOptionValue(optionName, null); + + if (value != null) { + // User provided a value for the option + return value; + } else { + // Option was not set by the user, attempt to get the default value + CommandLine.Model.OptionSpec optionSpec = commandLine.getCommandSpec().findOption(optionName); + if (optionSpec != null) { + String defaultValueString = null; + try { + defaultValueString = commandLine.getDefaultValueProvider().defaultValue(optionSpec); + if (defaultValueString != null) { + // Convert the default value string to the option's type + return converter.convert(defaultValueString); + } + } catch (Exception e) { + throw new RuntimeException( + "Failed to convert default value for option " + optionName + ": " + e.getMessage(), + e); + } + } + } + // No value was provided by the user, and no default is available + return null; + } } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java b/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java index 52b67652822..146e1d2d918 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java @@ -25,30 +25,23 @@ import picocli.CommandLine; import picocli.CommandLine.IDefaultValueProvider; import picocli.CommandLine.IExecutionStrategy; -import picocli.CommandLine.IParameterExceptionHandler; import picocli.CommandLine.ParameterException; import picocli.CommandLine.ParseResult; -/** Custom Config option search and run handler. */ public class ConfigOptionSearchAndRunHandler extends CommandLine.RunLast { private final IExecutionStrategy resultHandler; - private final IParameterExceptionHandler parameterExceptionHandler; private final Map environment; /** * Instantiates a new Config option search and run handler. * * @param resultHandler the result handler - * @param parameterExceptionHandler the parameter exception handler * @param environment the environment variables map */ public ConfigOptionSearchAndRunHandler( - final IExecutionStrategy resultHandler, - final IParameterExceptionHandler parameterExceptionHandler, - final Map environment) { - this.resultHandler = resultHandler; - this.parameterExceptionHandler = parameterExceptionHandler; + final IExecutionStrategy resultHandler, final Map environment) { this.environment = environment; + this.resultHandler = resultHandler; } @Override @@ -62,7 +55,6 @@ public List handle(final ParseResult parseResult) throws ParameterExcept new ProfileFinder().findConfiguration(environment, parseResult))); commandLine.setExecutionStrategy(resultHandler); - commandLine.setParameterExceptionHandler(parameterExceptionHandler); commandLine.execute(parseResult.originalArgs().toArray(new String[0])); return new ArrayList<>(); diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index a35c3e0a87a..793d975c19c 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -17,6 +17,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; +import org.hyperledger.besu.cli.converter.PluginInfoConverter; import org.hyperledger.besu.plugin.BesuContext; import org.hyperledger.besu.plugin.BesuPlugin; import org.hyperledger.besu.plugin.services.BesuService; @@ -99,12 +100,17 @@ public Optional getService(final Class serviceType return Optional.ofNullable((T) serviceRegistry.get(serviceType)); } + public void registerPlugins(final Path pluginsDir) { + registerPlugins(pluginsDir, List.of()); + } + /** * Register plugins. * * @param pluginsDir the plugins dir */ - public void registerPlugins(final Path pluginsDir) { + public void registerPlugins( + final Path pluginsDir, List pluginsTolLoad) { lines.add("Plugins:"); checkState( state == Lifecycle.UNINITIALIZED, @@ -120,11 +126,20 @@ public void registerPlugins(final Path pluginsDir) { int pluginsCount = 0; for (final BesuPlugin plugin : serviceLoader) { + String pluginName = plugin.getClass().getSimpleName(); + String pluginVersion = getPluginVersion(plugin); + // Check if the plugin is in the list of plugins to load + boolean shouldLoad = pluginsTolLoad.stream().anyMatch(p -> p.getName().equals(pluginName)); + + if (!shouldLoad) { + lines.add(String.format("%s (Skipped)", plugin.getClass().getSimpleName())); + continue; + } + pluginsCount++; try { plugin.register(this); LOG.info("Registered plugin of type {}.", plugin.getClass().getName()); - String pluginVersion = getPluginVersion(plugin); pluginVersions.add(pluginVersion); lines.add(String.format("%s (%s)", plugin.getClass().getSimpleName(), pluginVersion)); } catch (final Exception e) { @@ -142,7 +157,7 @@ public void registerPlugins(final Path pluginsDir) { LOG.debug("Plugin registration complete."); lines.add( String.format( - "TOTAL = %d of %d plugins successfully loaded", plugins.size(), pluginsCount)); + "TOTAL = %d of %d plugins successfully registered", plugins.size(), pluginsCount)); lines.add(String.format("from %s", pluginsDir.toAbsolutePath())); state = Lifecycle.REGISTERED; diff --git a/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java b/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java index fbc672dc0a1..7d6b2914a9a 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandlerTest.java @@ -84,9 +84,7 @@ public void initMocks() { lenient().when(mockConfigOptionSpec.getter()).thenReturn(mockConfigOptionGetter); levelOption = new LoggingLevelOption(); levelOption.setLogLevel("INFO"); - configParsingHandler = - new ConfigOptionSearchAndRunHandler( - resultHandler, mockParameterExceptionHandler, environment); + configParsingHandler = new ConfigOptionSearchAndRunHandler(resultHandler, environment); } @Test @@ -108,7 +106,6 @@ public void handleWithEnvironmentVariable() throws IOException { final ConfigOptionSearchAndRunHandler environmentConfigFileParsingHandler = new ConfigOptionSearchAndRunHandler( resultHandler, - mockParameterExceptionHandler, singletonMap( "BESU_CONFIG_FILE", Files.createFile(temp.resolve("tmp")).toFile().getAbsolutePath())); @@ -131,9 +128,7 @@ public void handleWithCommandLineOptionShouldRaiseExceptionIfNoFileParam() throw public void handleWithEnvironmentVariableOptionShouldRaiseExceptionIfNoFileParam() { final ConfigOptionSearchAndRunHandler environmentConfigFileParsingHandler = new ConfigOptionSearchAndRunHandler( - resultHandler, - mockParameterExceptionHandler, - singletonMap("BESU_CONFIG_FILE", "not_found.toml")); + resultHandler, singletonMap("BESU_CONFIG_FILE", "not_found.toml")); when(mockParseResult.hasMatchedOption(CONFIG_FILE_OPTION_NAME)).thenReturn(false); @@ -166,7 +161,6 @@ public void handleThrowsErrorWithWithEnvironmentVariableAndCommandLineSpecified( final ConfigOptionSearchAndRunHandler environmentConfigFileParsingHandler = new ConfigOptionSearchAndRunHandler( resultHandler, - mockParameterExceptionHandler, singletonMap("BESU_CONFIG_FILE", temp.resolve("tmp").toFile().getAbsolutePath())); when(mockParseResult.hasMatchedOption(CONFIG_FILE_OPTION_NAME)).thenReturn(true); From 4b354f2da1288116754b960210830ecfe7085d06 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Fri, 1 Mar 2024 15:43:05 +1100 Subject: [PATCH 02/34] changes to plugin logic Signed-off-by: Gabriel-Trintinalia --- .../org/hyperledger/besu/cli/BesuCommand.java | 54 +++----- .../cli/converter/PluginInfoConverter.java | 17 +-- .../stable/PluginsConfigurationOptions.java | 18 ++- .../util/ConfigOptionSearchAndRunHandler.java | 2 +- .../besu/services/BesuPluginContextImpl.java | 126 +++++++++++------- .../core/plugins/PluginConfiguration.java | 32 +++++ .../ethereum/core/plugins/PluginInfo.java | 13 ++ 7 files changed, 163 insertions(+), 99 deletions(-) create mode 100644 ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java create mode 100644 ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index de43ef54493..fadd57d0ae1 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -44,7 +44,6 @@ import org.hyperledger.besu.cli.config.ProfileName; import org.hyperledger.besu.cli.converter.MetricCategoryConverter; import org.hyperledger.besu.cli.converter.PercentageConverter; -import org.hyperledger.besu.cli.converter.PluginInfoConverter; import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty; import org.hyperledger.besu.cli.error.BesuExecutionExceptionHandler; import org.hyperledger.besu.cli.error.BesuParameterExceptionHandler; @@ -123,6 +122,7 @@ import org.hyperledger.besu.ethereum.core.MiningParametersMetrics; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.core.VersionMetadata; +import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; @@ -1057,20 +1057,20 @@ public int parse( preparePlugins(); final ConfigOptionSearchAndRunHandler configParsingHandler = - new ConfigOptionSearchAndRunHandler( - (parseResult -> { - registerPlugins(parseResult); - commandLine.setExecutionStrategy(resultHandler); - commandLine.execute(parseResult.originalArgs().toArray(new String[0])); - return 0; - }), - environment); + new ConfigOptionSearchAndRunHandler( + (parseResult -> { + registerPlugins(parseResult); + commandLine.setExecutionStrategy(resultHandler); + commandLine.execute(parseResult.originalArgs().toArray(new String[0])); + return 0; + }), + environment); return commandLine - .setExecutionStrategy(configParsingHandler) - .setParameterExceptionHandler(parameterExceptionHandler) - .setExecutionExceptionHandler(executionExceptionHandler) - .execute(args); + .setExecutionStrategy(configParsingHandler) + .setParameterExceptionHandler(parameterExceptionHandler) + .setExecutionExceptionHandler(executionExceptionHandler) + .execute(args); } /** Used by Dagger to parse all options into a commandline instance. */ @@ -1249,22 +1249,11 @@ private SecurityModule defaultSecurityModule() { } private void registerPlugins(final CommandLine.ParseResult parseResult) { - var pluginsStrictRegistration = - CommandLineUtils.getOptionValueOrDefault( - parseResult.commandSpec().commandLine(), - DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, - Boolean::valueOf); - List plugins = List.of(); - if (Boolean.TRUE.equals(pluginsStrictRegistration)) { - plugins = - CommandLineUtils.getOptionValueOrDefault( - parseResult.commandSpec().commandLine(), - DEFAULT_PLUGINS_OPTION_NAME, - new PluginInfoConverter()); - logger.info("Plugins to register: {}", plugins); - } - besuPluginContext.registerPlugins(pluginsDir(), plugins); + PluginConfiguration configuration = + PluginsConfigurationOptions.fromCommandLine(parseResult.commandSpec().commandLine()); + + besuPluginContext.registerPlugins(configuration); } // loadKeyPair() is public because it is accessed by subcommands @@ -2401,15 +2390,6 @@ public Path dataDir() { return dataPath.toAbsolutePath(); } - private Path pluginsDir() { - final String pluginsDir = System.getProperty("besu.plugins.dir"); - if (pluginsDir == null) { - return new File(System.getProperty("besu.home", "."), "plugins").toPath(); - } else { - return new File(pluginsDir).toPath(); - } - } - private SecurityModule securityModule() { return securityModuleService .getByName(securityModuleName) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java index 786d6a848be..ad83300e203 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java @@ -1,12 +1,13 @@ package org.hyperledger.besu.cli.converter; +import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; + import java.util.Arrays; import java.util.List; import picocli.CommandLine; -public class PluginInfoConverter - implements CommandLine.ITypeConverter> { +public class PluginInfoConverter implements CommandLine.ITypeConverter> { @Override public List convert(final String value) { @@ -16,16 +17,4 @@ public List convert(final String value) { private PluginInfo toPluginInfo(final String pluginName) { return new PluginInfo(pluginName); } - - public static class PluginInfo { - String name; - - PluginInfo(final String name) { - this.name = name; - } - - public String getName() { - return name; - } - } } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index aa153ab7f19..44c81675b97 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -4,12 +4,16 @@ import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME; import org.hyperledger.besu.cli.converter.PluginInfoConverter; +import org.hyperledger.besu.cli.util.CommandLineUtils; +import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; +import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; import java.util.List; import picocli.CommandLine; public class PluginsConfigurationOptions { + @CommandLine.Option( names = {DEFAULT_PLUGINS_OPTION_NAME}, description = "Comma separated list of plugins.", @@ -21,7 +25,7 @@ public class PluginsConfigurationOptions { "FieldMayBeFinal", "UnusedVariable" }) // PicoCLI requires non-final Strings. - private List plugins = null; + private List plugins = null; @CommandLine.Option( names = {DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME}, @@ -33,4 +37,16 @@ public class PluginsConfigurationOptions { "UnusedVariable" }) // PicoCLI requires non-final Strings. private boolean requireExplicitPlugins = false; + + public static PluginConfiguration fromCommandLine(CommandLine commandLine) { + Boolean pluginsStrictRegistration = + CommandLineUtils.getOptionValueOrDefault( + commandLine, DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, Boolean::valueOf); + + List plugins = + CommandLineUtils.getOptionValueOrDefault( + commandLine, DEFAULT_PLUGINS_OPTION_NAME, new PluginInfoConverter()); + + return new PluginConfiguration(plugins, Boolean.TRUE.equals(pluginsStrictRegistration)); + } } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java b/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java index 146e1d2d918..129ab9ee1ae 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/ConfigOptionSearchAndRunHandler.java @@ -40,8 +40,8 @@ public class ConfigOptionSearchAndRunHandler extends CommandLine.RunLast { */ public ConfigOptionSearchAndRunHandler( final IExecutionStrategy resultHandler, final Map environment) { - this.environment = environment; this.resultHandler = resultHandler; + this.environment = environment; } @Override diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index 793d975c19c..8c28e19070d 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -17,7 +17,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; -import org.hyperledger.besu.cli.converter.PluginInfoConverter; +import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; import org.hyperledger.besu.plugin.BesuContext; import org.hyperledger.besu.plugin.BesuPlugin; import org.hyperledger.besu.plugin.services.BesuService; @@ -75,6 +75,9 @@ private enum Lifecycle { private Lifecycle state = Lifecycle.UNINITIALIZED; private final Map, ? super BesuService> serviceRegistry = new HashMap<>(); private final List plugins = new ArrayList<>(); + + private int registeredPlugins = 0; + private final List pluginVersions = new ArrayList<>(); final List lines = new ArrayList<>(); @@ -100,69 +103,100 @@ public Optional getService(final Class serviceType return Optional.ofNullable((T) serviceRegistry.get(serviceType)); } - public void registerPlugins(final Path pluginsDir) { - registerPlugins(pluginsDir, List.of()); + public void registerPlugins(final Path path) { + PluginConfiguration configuration = new PluginConfiguration(List.of(), false); + registerPlugins(configuration); } - /** - * Register plugins. - * - * @param pluginsDir the plugins dir - */ - public void registerPlugins( - final Path pluginsDir, List pluginsTolLoad) { - lines.add("Plugins:"); + public void registerPlugins(final PluginConfiguration config) { + List detectedPlugins = findPlugins(config); + List pluginsToRegister; + + if (config.isStrictPluginRegistrationEnabled()) { + pluginsToRegister = filterPluginsBasedOnConfiguration(detectedPlugins, config); + } else { + pluginsToRegister = detectedPlugins; + } + + registerPlugins(pluginsToRegister); + logPluginRegistrationSummary(pluginsToRegister, config); + } + + private List filterPluginsBasedOnConfiguration( + List detectedPlugins, PluginConfiguration config) { + return detectedPlugins.stream() + .filter( + plugin -> + config.getPluginInfos().stream() + .anyMatch( + pluginInfo -> + pluginInfo.getName().equals(plugin.getClass().getSimpleName()))) + .collect(Collectors.toList()); + } + + private void logPluginRegistrationSummary( + List registeredPlugins, PluginConfiguration config) { + LOG.debug("Plugin registration complete."); + LOG.debug( + String.format( + "TOTAL = %d of %d plugins successfully registered", + registeredPlugins.size(), plugins.size())); + LOG.debug(String.format("from %s", config.pluginsDir().toAbsolutePath())); + } + + private List findPlugins(final PluginConfiguration config) { checkState( state == Lifecycle.UNINITIALIZED, "Besu plugins have already been registered. Cannot register additional plugins."); final ClassLoader pluginLoader = - pluginDirectoryLoader(pluginsDir).orElse(this.getClass().getClassLoader()); - - state = Lifecycle.REGISTERING; + pluginDirectoryLoader(config.pluginsDir()).orElse(this.getClass().getClassLoader()); final ServiceLoader serviceLoader = ServiceLoader.load(BesuPlugin.class, pluginLoader); - int pluginsCount = 0; + List plugins = new ArrayList<>(); for (final BesuPlugin plugin : serviceLoader) { - String pluginName = plugin.getClass().getSimpleName(); - String pluginVersion = getPluginVersion(plugin); - // Check if the plugin is in the list of plugins to load - boolean shouldLoad = pluginsTolLoad.stream().anyMatch(p -> p.getName().equals(pluginName)); - - if (!shouldLoad) { - lines.add(String.format("%s (Skipped)", plugin.getClass().getSimpleName())); - continue; - } - - pluginsCount++; - try { - plugin.register(this); - LOG.info("Registered plugin of type {}.", plugin.getClass().getName()); - pluginVersions.add(pluginVersion); - lines.add(String.format("%s (%s)", plugin.getClass().getSimpleName(), pluginVersion)); - } catch (final Exception e) { - LOG.error( - "Error registering plugin of type " - + plugin.getClass().getName() - + ", start and stop will not be called.", - e); - lines.add(String.format("ERROR %s", plugin.getClass().getSimpleName())); - continue; - } plugins.add(plugin); } + return plugins; + } - LOG.debug("Plugin registration complete."); - lines.add( - String.format( - "TOTAL = %d of %d plugins successfully registered", plugins.size(), pluginsCount)); - lines.add(String.format("from %s", pluginsDir.toAbsolutePath())); - + /** + * Register plugins. + * + * @param serviceLoader the serviceLoader + */ + private void registerPlugins(final List serviceLoader) { + state = Lifecycle.REGISTERING; + lines.add("Plugins:"); + for (final BesuPlugin plugin : serviceLoader) { + if (registerPlugin(plugin)) { + plugins.add(plugin); + } + } state = Lifecycle.REGISTERED; } + private boolean registerPlugin(BesuPlugin plugin) { + try { + plugin.register(this); + LOG.info("Registered plugin of type {}.", plugin.getClass().getName()); + String pluginVersion = getPluginVersion(plugin); + pluginVersions.add(pluginVersion); + lines.add(String.format("%s (%s)", plugin.getClass().getSimpleName(), pluginVersion)); + } catch (final Exception e) { + LOG.error( + "Error registering plugin of type " + + plugin.getClass().getName() + + ", start and stop will not be called.", + e); + lines.add(String.format("ERROR %s", plugin.getClass().getSimpleName())); + return false; + } + return true; + } + /** * get the summary log, as a list of string lines * diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java new file mode 100644 index 00000000000..912a97fff72 --- /dev/null +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java @@ -0,0 +1,32 @@ +package org.hyperledger.besu.ethereum.core.plugins; + +import java.io.File; +import java.nio.file.Path; +import java.util.List; + +public class PluginConfiguration { + final List pluginInfos; + final boolean strictPluginRegistrationEnabled; + + public PluginConfiguration(List pluginInfos, boolean pluginsStrictRegistration) { + this.pluginInfos = pluginInfos; + this.strictPluginRegistrationEnabled = pluginsStrictRegistration; + } + + public List getPluginInfos() { + return pluginInfos; + } + + public boolean isStrictPluginRegistrationEnabled() { + return strictPluginRegistrationEnabled; + } + + public Path pluginsDir() { + final String pluginsDir = System.getProperty("besu.plugins.dir"); + if (pluginsDir == null) { + return new File(System.getProperty("besu.home", "."), "plugins").toPath(); + } else { + return new File(pluginsDir).toPath(); + } + } +} diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java new file mode 100644 index 00000000000..4eb6dc545ca --- /dev/null +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java @@ -0,0 +1,13 @@ +package org.hyperledger.besu.ethereum.core.plugins; + +public class PluginInfo { + String name; + + public PluginInfo(final String name) { + this.name = name; + } + + public String getName() { + return name; + } +} From f380874559bf860add4b376385383138b6f075d1 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Mon, 4 Mar 2024 14:38:41 +1100 Subject: [PATCH 03/34] changes to plugin logic Signed-off-by: Gabriel-Trintinalia --- .../cli/converter/PluginInfoConverter.java | 3 ++ .../stable/PluginsConfigurationOptions.java | 5 ++- .../besu/services/BesuPluginContextImpl.java | 39 +++++++++++-------- .../core/plugins/PluginConfiguration.java | 3 +- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java index ad83300e203..3b407ae8f77 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java @@ -11,6 +11,9 @@ public class PluginInfoConverter implements CommandLine.ITypeConverter convert(final String value) { + if (value == null || value.isEmpty()) { + return List.of(); + } return Arrays.stream(value.split(",")).map(this::toPluginInfo).toList(); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index 44c81675b97..32589b4b000 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -38,7 +38,7 @@ public class PluginsConfigurationOptions { }) // PicoCLI requires non-final Strings. private boolean requireExplicitPlugins = false; - public static PluginConfiguration fromCommandLine(CommandLine commandLine) { + public static PluginConfiguration fromCommandLine(final CommandLine commandLine) { Boolean pluginsStrictRegistration = CommandLineUtils.getOptionValueOrDefault( commandLine, DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, Boolean::valueOf); @@ -47,6 +47,7 @@ public static PluginConfiguration fromCommandLine(CommandLine commandLine) { CommandLineUtils.getOptionValueOrDefault( commandLine, DEFAULT_PLUGINS_OPTION_NAME, new PluginInfoConverter()); - return new PluginConfiguration(plugins, Boolean.TRUE.equals(pluginsStrictRegistration)); + return new PluginConfiguration( + plugins != null ? plugins : List.of(), Boolean.TRUE.equals(pluginsStrictRegistration)); } } diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index 8c28e19070d..b05aa66b53c 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkState; import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; +import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; import org.hyperledger.besu.plugin.BesuContext; import org.hyperledger.besu.plugin.BesuPlugin; import org.hyperledger.besu.plugin.services.BesuService; @@ -38,6 +39,7 @@ import java.util.Map; import java.util.Optional; import java.util.ServiceLoader; +import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -76,8 +78,6 @@ private enum Lifecycle { private final Map, ? super BesuService> serviceRegistry = new HashMap<>(); private final List plugins = new ArrayList<>(); - private int registeredPlugins = 0; - private final List pluginVersions = new ArrayList<>(); final List lines = new ArrayList<>(); @@ -119,29 +119,36 @@ public void registerPlugins(final PluginConfiguration config) { } registerPlugins(pluginsToRegister); - logPluginRegistrationSummary(pluginsToRegister, config); + logPluginRegistrationSummary(pluginsToRegister, detectedPlugins); } private List filterPluginsBasedOnConfiguration( List detectedPlugins, PluginConfiguration config) { + Set configuredPluginNames = + config.getPluginInfos().stream().map(PluginInfo::getName).collect(Collectors.toSet()); + return detectedPlugins.stream() - .filter( - plugin -> - config.getPluginInfos().stream() - .anyMatch( - pluginInfo -> - pluginInfo.getName().equals(plugin.getClass().getSimpleName()))) + .filter(plugin -> configuredPluginNames.contains(plugin.getClass().getSimpleName())) .collect(Collectors.toList()); } private void logPluginRegistrationSummary( - List registeredPlugins, PluginConfiguration config) { - LOG.debug("Plugin registration complete."); - LOG.debug( - String.format( - "TOTAL = %d of %d plugins successfully registered", - registeredPlugins.size(), plugins.size())); - LOG.debug(String.format("from %s", config.pluginsDir().toAbsolutePath())); + List registeredPlugins, List allDetectedPlugins) { + lines.add("Plugin registration summary:"); + + Set registeredPluginNames = + registeredPlugins.stream() + .map(plugin -> plugin.getClass().getSimpleName()) + .collect(Collectors.toSet()); + + for (BesuPlugin plugin : allDetectedPlugins) { + String pluginName = plugin.getClass().getSimpleName(); + if (registeredPluginNames.contains(pluginName)) { + lines.add(pluginName + " (Registered)"); + } else { + lines.add(pluginName + " (Skipped)"); + } + } } private List findPlugins(final PluginConfiguration config) { diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java index 912a97fff72..cfc9964d28a 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java @@ -8,7 +8,8 @@ public class PluginConfiguration { final List pluginInfos; final boolean strictPluginRegistrationEnabled; - public PluginConfiguration(List pluginInfos, boolean pluginsStrictRegistration) { + public PluginConfiguration( + final List pluginInfos, final boolean pluginsStrictRegistration) { this.pluginInfos = pluginInfos; this.strictPluginRegistrationEnabled = pluginsStrictRegistration; } From 568127555bd23edd3a86d06511b5f317499be343 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 10:53:16 +1100 Subject: [PATCH 04/34] Refactoring and unit tests Signed-off-by: Gabriel-Trintinalia --- .../besu/cli/util/CommandLineUtils.java | 42 ++++---- .../cli/CommandLineUtilsDefaultsTest.java | 95 +++++++++++++++++++ .../besu/cli/CommandLineUtilsTest.java | 1 - 3 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java index 6731b00a193..8a0826d0710 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java @@ -269,29 +269,31 @@ public static T getOptionValueOrDefault( // Attempt to get the option's value if it was provided by the user T value = commandLine.getParseResult().matchedOptionValue(optionName, null); - if (value != null) { // User provided a value for the option return value; - } else { - // Option was not set by the user, attempt to get the default value - CommandLine.Model.OptionSpec optionSpec = commandLine.getCommandSpec().findOption(optionName); - if (optionSpec != null) { - String defaultValueString = null; - try { - defaultValueString = commandLine.getDefaultValueProvider().defaultValue(optionSpec); - if (defaultValueString != null) { - // Convert the default value string to the option's type - return converter.convert(defaultValueString); - } - } catch (Exception e) { - throw new RuntimeException( - "Failed to convert default value for option " + optionName + ": " + e.getMessage(), - e); - } - } } - // No value was provided by the user, and no default is available - return null; + + // Attempt to get the default value if the option was not set by the user + return getDefaultOptionValue(commandLine, optionName, converter); + } + + private static T getDefaultOptionValue( + final CommandLine commandLine, + final String optionName, + final CommandLine.ITypeConverter converter) { + + CommandLine.Model.OptionSpec optionSpec = commandLine.getCommandSpec().findOption(optionName); + if (optionSpec == null || commandLine.getDefaultValueProvider() == null) { + return null; + } + + try { + String defaultValueString = commandLine.getDefaultValueProvider().defaultValue(optionSpec); + return defaultValueString != null ? converter.convert(defaultValueString) : null; + } catch (Exception e) { + throw new RuntimeException( + "Failed to convert default value for option " + optionName + ": " + e.getMessage(), e); + } } } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java new file mode 100644 index 00000000000..e2aa690f290 --- /dev/null +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java @@ -0,0 +1,95 @@ +package org.hyperledger.besu.cli; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.hyperledger.besu.cli.util.CommandLineUtils; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import picocli.CommandLine; + +/** + * Unit tests for {@link CommandLineUtils} focusing on the retrieval of option values + * (getOptionValueOrDefault). + */ +public class CommandLineUtilsDefaultsTest { + private CommandLine commandLine; + private CommandLine.ParseResult parseResult; + private CommandLine.Model.CommandSpec commandSpec; + private CommandLine.Model.OptionSpec optionSpec; + + private CommandLine.IDefaultValueProvider defaultValueProvider; + + @BeforeEach + public void setUp() { + commandLine = mock(CommandLine.class); + parseResult = mock(CommandLine.ParseResult.class); + commandSpec = mock(CommandLine.Model.CommandSpec.class); + optionSpec = mock(CommandLine.Model.OptionSpec.class); + defaultValueProvider = mock(CommandLine.IDefaultValueProvider.class); + + when(commandLine.getParseResult()).thenReturn(parseResult); + when(commandLine.getCommandSpec()).thenReturn(commandSpec); + when(commandLine.getDefaultValueProvider()).thenReturn(defaultValueProvider); + } + + @Test + public void testGetOptionValueOrDefault_UserProvidedValue() { + when(parseResult.matchedOptionValue("option", null)).thenReturn("optionValue"); + String result = + CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); + assertThat(result).isEqualTo("optionValue"); + } + + @Test + public void testGetOptionValueOrDefault_DefaultValue() throws Exception { + when(commandSpec.findOption("option")).thenReturn(optionSpec); + when(defaultValueProvider.defaultValue(optionSpec)).thenReturn("defaultValue"); + String result = + CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); + assertThat(result).isEqualTo("defaultValue"); + } + + @Test + public void userOptionOverridesDefaultValue() throws Exception { + when(parseResult.matchedOptionValue("option", null)).thenReturn("optionValue"); + when(defaultValueProvider.defaultValue(optionSpec)).thenReturn("defaultValue"); + String result = + CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); + assertThat(result).isEqualTo("optionValue"); + } + + @Test + public void testGetOptionValueOrDefault_NoValueOrDefault() { + when(commandSpec.findOption("option")).thenReturn(null); + String result = + CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); + assertThat(result).isNull(); + } + + @Test + public void testGetOptionValueOrDefault_ConversionFailure() throws Exception { + when(commandSpec.findOption("option")).thenReturn(optionSpec); + when(commandLine.getDefaultValueProvider()).thenReturn(defaultValueProvider); + when(defaultValueProvider.defaultValue(optionSpec)).thenReturn("defaultValue"); + + CommandLine.ITypeConverter failingConverter = + value -> { + throw new Exception("Conversion failed"); + }; + + String actualMessage = + assertThrows( + RuntimeException.class, + () -> + CommandLineUtils.getOptionValueOrDefault( + commandLine, "option", failingConverter)) + .getMessage(); + final String expectedMessage = + "Failed to convert default value for option option: Conversion failed"; + assertThat(actualMessage).isEqualTo(expectedMessage); + } +} diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsTest.java index d8e8912eefa..24b840dc74b 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsTest.java @@ -69,7 +69,6 @@ private abstract static class AbstractTestCommand implements Runnable { commandLine.setDefaultValueProvider(new EnvironmentVariableDefaultProvider(environment)); } - // Completely disables p2p within Besu. @Option( names = {"--option-enabled"}, arity = "1") From d4adb711f37fa706c76b161ae3e6d4529331fdc3 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 11:56:32 +1100 Subject: [PATCH 05/34] Refactoring and unit tests Signed-off-by: Gabriel-Trintinalia --- .../besu/cli/util/CommandLineUtils.java | 12 +--- .../cli/CommandLineUtilsDefaultsTest.java | 56 +++++++++---------- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java index 8a0826d0710..bfd1d54eb63 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java @@ -267,15 +267,9 @@ public static T getOptionValueOrDefault( final String optionName, final CommandLine.ITypeConverter converter) { - // Attempt to get the option's value if it was provided by the user - T value = commandLine.getParseResult().matchedOptionValue(optionName, null); - if (value != null) { - // User provided a value for the option - return value; - } - - // Attempt to get the default value if the option was not set by the user - return getDefaultOptionValue(commandLine, optionName, converter); + return commandLine + .getParseResult() + .matchedOptionValue(optionName, getDefaultOptionValue(commandLine, optionName, converter)); } private static T getDefaultOptionValue( diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java index e2aa690f290..ab39c670ba6 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandLineUtilsDefaultsTest.java @@ -1,7 +1,10 @@ package org.hyperledger.besu.cli; import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.cli.util.CommandLineUtils.getOptionValueOrDefault; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -16,65 +19,64 @@ * (getOptionValueOrDefault). */ public class CommandLineUtilsDefaultsTest { + private static final String OPTION_NAME = "option"; + private static final String OPTION_VALUE = "optionValue"; + private static final String DEFAULT_VALUE = "defaultValue"; + public final CommandLine.ITypeConverter converter = String::valueOf; private CommandLine commandLine; - private CommandLine.ParseResult parseResult; - private CommandLine.Model.CommandSpec commandSpec; private CommandLine.Model.OptionSpec optionSpec; - private CommandLine.IDefaultValueProvider defaultValueProvider; + private CommandLine.ParseResult parseResult; @BeforeEach public void setUp() { commandLine = mock(CommandLine.class); parseResult = mock(CommandLine.ParseResult.class); - commandSpec = mock(CommandLine.Model.CommandSpec.class); + CommandLine.Model.CommandSpec commandSpec = mock(CommandLine.Model.CommandSpec.class); optionSpec = mock(CommandLine.Model.OptionSpec.class); defaultValueProvider = mock(CommandLine.IDefaultValueProvider.class); - when(commandLine.getParseResult()).thenReturn(parseResult); when(commandLine.getCommandSpec()).thenReturn(commandSpec); when(commandLine.getDefaultValueProvider()).thenReturn(defaultValueProvider); + when(parseResult.matchedOptionValue(anyString(), any())).thenCallRealMethod(); + when(commandSpec.findOption(OPTION_NAME)).thenReturn(optionSpec); } @Test public void testGetOptionValueOrDefault_UserProvidedValue() { - when(parseResult.matchedOptionValue("option", null)).thenReturn("optionValue"); - String result = - CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); - assertThat(result).isEqualTo("optionValue"); + when(parseResult.matchedOption(OPTION_NAME)).thenReturn(optionSpec); + when(optionSpec.getValue()).thenReturn(OPTION_VALUE); + + String result = getOptionValueOrDefault(commandLine, OPTION_NAME, converter); + assertThat(result).isEqualTo(OPTION_VALUE); } @Test public void testGetOptionValueOrDefault_DefaultValue() throws Exception { - when(commandSpec.findOption("option")).thenReturn(optionSpec); - when(defaultValueProvider.defaultValue(optionSpec)).thenReturn("defaultValue"); - String result = - CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); - assertThat(result).isEqualTo("defaultValue"); + when(defaultValueProvider.defaultValue(optionSpec)).thenReturn(DEFAULT_VALUE); + String result = getOptionValueOrDefault(commandLine, OPTION_NAME, converter); + assertThat(result).isEqualTo(DEFAULT_VALUE); } @Test public void userOptionOverridesDefaultValue() throws Exception { - when(parseResult.matchedOptionValue("option", null)).thenReturn("optionValue"); - when(defaultValueProvider.defaultValue(optionSpec)).thenReturn("defaultValue"); - String result = - CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); - assertThat(result).isEqualTo("optionValue"); + when(parseResult.matchedOption(OPTION_NAME)).thenReturn(optionSpec); + when(optionSpec.getValue()).thenReturn(OPTION_VALUE); + + when(defaultValueProvider.defaultValue(optionSpec)).thenReturn(DEFAULT_VALUE); + String result = getOptionValueOrDefault(commandLine, OPTION_NAME, converter); + assertThat(result).isEqualTo(OPTION_VALUE); } @Test public void testGetOptionValueOrDefault_NoValueOrDefault() { - when(commandSpec.findOption("option")).thenReturn(null); - String result = - CommandLineUtils.getOptionValueOrDefault(commandLine, "option", String::valueOf); + String result = getOptionValueOrDefault(commandLine, OPTION_NAME, converter); assertThat(result).isNull(); } @Test public void testGetOptionValueOrDefault_ConversionFailure() throws Exception { - when(commandSpec.findOption("option")).thenReturn(optionSpec); - when(commandLine.getDefaultValueProvider()).thenReturn(defaultValueProvider); - when(defaultValueProvider.defaultValue(optionSpec)).thenReturn("defaultValue"); + when(defaultValueProvider.defaultValue(optionSpec)).thenReturn(DEFAULT_VALUE); CommandLine.ITypeConverter failingConverter = value -> { @@ -84,9 +86,7 @@ public void testGetOptionValueOrDefault_ConversionFailure() throws Exception { String actualMessage = assertThrows( RuntimeException.class, - () -> - CommandLineUtils.getOptionValueOrDefault( - commandLine, "option", failingConverter)) + () -> getOptionValueOrDefault(commandLine, OPTION_NAME, failingConverter)) .getMessage(); final String expectedMessage = "Failed to convert default value for option option: Conversion failed"; From a439c660781617fe9034bce79ef89c1f174afe8c Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 14:58:28 +1100 Subject: [PATCH 06/34] Refactoring and unit tests Signed-off-by: Gabriel-Trintinalia --- .../services/BesuPluginContextImplTest.java | 39 ++-- .../stable/PluginsConfigurationOptions.java | 10 +- .../besu/services/BesuPluginContextImpl.java | 213 +++++++++--------- .../core/plugins/PluginConfiguration.java | 25 +- .../hyperledger/besu/plugin/BesuPlugin.java | 20 ++ 5 files changed, 177 insertions(+), 130 deletions(-) diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index 57797462739..2ee1d7612c8 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -53,11 +53,12 @@ public void clearTestPluginState() { public void verifyEverythingGoesSmoothly() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); - assertThat(contextImpl.getPlugins()).isEmpty(); + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(new File(".").toPath()); - assertThat(contextImpl.getPlugins()).isNotEmpty(); + assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); - final Optional testPluginOptional = findTestPlugin(contextImpl.getPlugins()); + final Optional testPluginOptional = + findTestPlugin(contextImpl.getRegisteredPlugins()); Assertions.assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); @@ -75,18 +76,18 @@ public void registrationErrorsHandledSmoothly() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER"); - assertThat(contextImpl.getPlugins()).isEmpty(); + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(new File(".").toPath()); - assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.beforeExternalServices(); - assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.startPlugins(); - assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.stopPlugins(); - assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); } @Test @@ -94,11 +95,14 @@ public void startErrorsHandledSmoothly() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART"); - assertThat(contextImpl.getPlugins()).isEmpty(); + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(new File(".").toPath()); - assertThat(contextImpl.getPlugins()).extracting("class").contains(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()) + .extracting("class") + .contains(TestPicoCLIPlugin.class); - final Optional testPluginOptional = findTestPlugin(contextImpl.getPlugins()); + final Optional testPluginOptional = + findTestPlugin(contextImpl.getRegisteredPlugins()); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); @@ -106,10 +110,10 @@ public void startErrorsHandledSmoothly() { contextImpl.beforeExternalServices(); contextImpl.startPlugins(); assertThat(testPicoCLIPlugin.getState()).isEqualTo("failstart"); - assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.stopPlugins(); - assertThat(contextImpl.getPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); } @Test @@ -117,11 +121,14 @@ public void stopErrorsHandledSmoothly() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP"); - assertThat(contextImpl.getPlugins()).isEmpty(); + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(new File(".").toPath()); - assertThat(contextImpl.getPlugins()).extracting("class").contains(TestPicoCLIPlugin.class); + assertThat(contextImpl.getRegisteredPlugins()) + .extracting("class") + .contains(TestPicoCLIPlugin.class); - final Optional testPluginOptional = findTestPlugin(contextImpl.getPlugins()); + final Optional testPluginOptional = + findTestPlugin(contextImpl.getRegisteredPlugins()); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index 32589b4b000..5364b7ff6a2 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -39,15 +39,19 @@ public class PluginsConfigurationOptions { private boolean requireExplicitPlugins = false; public static PluginConfiguration fromCommandLine(final CommandLine commandLine) { - Boolean pluginsStrictRegistration = + PluginConfiguration.DetectionType detectionType = CommandLineUtils.getOptionValueOrDefault( - commandLine, DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, Boolean::valueOf); + commandLine, + DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, + PluginConfiguration.DetectionType::valueOf); List plugins = CommandLineUtils.getOptionValueOrDefault( commandLine, DEFAULT_PLUGINS_OPTION_NAME, new PluginInfoConverter()); return new PluginConfiguration( - plugins != null ? plugins : List.of(), Boolean.TRUE.equals(pluginsStrictRegistration)); + plugins != null ? plugins : List.of(), + detectionType, + PluginConfiguration.defaultPluginsDir()); } } diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index b05aa66b53c..5198ba25096 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -40,9 +40,9 @@ import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; -import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; +import java.util.stream.StreamSupport; import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; @@ -76,7 +76,11 @@ private enum Lifecycle { private Lifecycle state = Lifecycle.UNINITIALIZED; private final Map, ? super BesuService> serviceRegistry = new HashMap<>(); - private final List plugins = new ArrayList<>(); + + private List detectedPlugins = new ArrayList<>(); + private List requestedPlugins = new ArrayList<>(); + + private final List registeredPlugins = new ArrayList<>(); private final List pluginVersions = new ArrayList<>(); final List lines = new ArrayList<>(); @@ -103,83 +107,73 @@ public Optional getService(final Class serviceType return Optional.ofNullable((T) serviceRegistry.get(serviceType)); } + /** + * Registers plugins located at the specified path. This method constructs a plugin configuration + * with the default settings and the provided path, then delegates to the overloaded {@code + * registerPlugins} method that accepts a {@link PluginConfiguration} object. + * + * @param path The path where plugins are located. This path will be used to search for plugin + * implementations to be registered. + */ public void registerPlugins(final Path path) { - PluginConfiguration configuration = new PluginConfiguration(List.of(), false); + PluginConfiguration configuration = + new PluginConfiguration(List.of(), PluginConfiguration.DetectionType.ALL, path); registerPlugins(configuration); } + /** + * Registers plugins based on the provided {@link PluginConfiguration}. This method finds plugins + * according to the configuration settings, filters them if necessary (based on the detection + * type), and then registers the filtered or found plugins. It also logs a summary of the plugin + * registration process. + * + * @param config The configuration settings used to find and filter plugins for registration. The + * configuration includes the plugin directory, detection type, and any explicit plugin + * identifiers if applicable. + */ public void registerPlugins(final PluginConfiguration config) { - List detectedPlugins = findPlugins(config); - List pluginsToRegister; - - if (config.isStrictPluginRegistrationEnabled()) { - pluginsToRegister = filterPluginsBasedOnConfiguration(detectedPlugins, config); - } else { - pluginsToRegister = detectedPlugins; - } - - registerPlugins(pluginsToRegister); - logPluginRegistrationSummary(pluginsToRegister, detectedPlugins); - } - - private List filterPluginsBasedOnConfiguration( - List detectedPlugins, PluginConfiguration config) { - Set configuredPluginNames = - config.getPluginInfos().stream().map(PluginInfo::getName).collect(Collectors.toSet()); - - return detectedPlugins.stream() - .filter(plugin -> configuredPluginNames.contains(plugin.getClass().getSimpleName())) - .collect(Collectors.toList()); - } + checkState( + state == Lifecycle.UNINITIALIZED, + "Besu plugins have already been registered. Cannot register additional plugins."); + state = Lifecycle.REGISTERING; - private void logPluginRegistrationSummary( - List registeredPlugins, List allDetectedPlugins) { - lines.add("Plugin registration summary:"); + detectedPlugins = findPlugins(config); + if (config.getDetectionType() == PluginConfiguration.DetectionType.EXPLICIT) { + // Extract the set of plugin names from the configuration + requestedPlugins = + config.getPluginInfos().stream().map(PluginInfo::getName).collect(Collectors.toList()); - Set registeredPluginNames = - registeredPlugins.stream() - .map(plugin -> plugin.getClass().getSimpleName()) - .collect(Collectors.toSet()); + // Filter the detected Besu plugins to include only those explicitly configured + List registeringPlugins = + detectedPlugins.stream() + .filter(plugin -> requestedPlugins.contains(plugin.getClass().getSimpleName())) + .toList(); + registerPlugins(registeringPlugins); - for (BesuPlugin plugin : allDetectedPlugins) { - String pluginName = plugin.getClass().getSimpleName(); - if (registeredPluginNames.contains(pluginName)) { - lines.add(pluginName + " (Registered)"); - } else { - lines.add(pluginName + " (Skipped)"); - } + } else { + registerPlugins(detectedPlugins); } } private List findPlugins(final PluginConfiguration config) { - checkState( - state == Lifecycle.UNINITIALIZED, - "Besu plugins have already been registered. Cannot register additional plugins."); - - final ClassLoader pluginLoader = - pluginDirectoryLoader(config.pluginsDir()).orElse(this.getClass().getClassLoader()); - - final ServiceLoader serviceLoader = - ServiceLoader.load(BesuPlugin.class, pluginLoader); - - List plugins = new ArrayList<>(); - for (final BesuPlugin plugin : serviceLoader) { - plugins.add(plugin); - } - return plugins; + ClassLoader pluginLoader = + pluginDirectoryLoader(config.getPluginsDir()).orElse(getClass().getClassLoader()); + ServiceLoader serviceLoader = ServiceLoader.load(BesuPlugin.class, pluginLoader); + return StreamSupport.stream(serviceLoader.spliterator(), false).collect(Collectors.toList()); } /** - * Register plugins. + * Registers a list of plugins if the system is uninitialized. Updates system state throughout the + * registration process and adds successfully registered plugins to the system. * - * @param serviceLoader the serviceLoader + * @param pluginsToRegister The list of plugins to be registered. + * @throws IllegalStateException if the system is not in the UNINITIALIZED state. */ - private void registerPlugins(final List serviceLoader) { - state = Lifecycle.REGISTERING; - lines.add("Plugins:"); - for (final BesuPlugin plugin : serviceLoader) { + private void registerPlugins(final List pluginsToRegister) { + + for (final BesuPlugin plugin : pluginsToRegister) { if (registerPlugin(plugin)) { - plugins.add(plugin); + registeredPlugins.add(plugin); } } state = Lifecycle.REGISTERED; @@ -189,9 +183,7 @@ private boolean registerPlugin(BesuPlugin plugin) { try { plugin.register(this); LOG.info("Registered plugin of type {}.", plugin.getClass().getName()); - String pluginVersion = getPluginVersion(plugin); - pluginVersions.add(pluginVersion); - lines.add(String.format("%s (%s)", plugin.getClass().getSimpleName(), pluginVersion)); + pluginVersions.add(plugin.getVersion()); } catch (final Exception e) { LOG.error( "Error registering plugin of type " @@ -204,28 +196,6 @@ private boolean registerPlugin(BesuPlugin plugin) { return true; } - /** - * get the summary log, as a list of string lines - * - * @return the summary - */ - public List getPluginsSummaryLog() { - return lines; - } - - private String getPluginVersion(final BesuPlugin plugin) { - final Package pluginPackage = plugin.getClass().getPackage(); - final String implTitle = - Optional.ofNullable(pluginPackage.getImplementationTitle()) - .filter(Predicate.not(String::isBlank)) - .orElse(plugin.getClass().getSimpleName()); - final String implVersion = - Optional.ofNullable(pluginPackage.getImplementationVersion()) - .filter(Predicate.not(String::isBlank)) - .orElse(""); - return implTitle + "/v" + implVersion; - } - /** Before external services. */ public void beforeExternalServices() { checkState( @@ -234,7 +204,7 @@ public void beforeExternalServices() { Lifecycle.REGISTERED, state); state = Lifecycle.BEFORE_EXTERNAL_SERVICES_STARTED; - final Iterator pluginsIterator = plugins.iterator(); + final Iterator pluginsIterator = registeredPlugins.iterator(); while (pluginsIterator.hasNext()) { final BesuPlugin plugin = pluginsIterator.next(); @@ -265,7 +235,7 @@ public void startPlugins() { Lifecycle.BEFORE_EXTERNAL_SERVICES_FINISHED, state); state = Lifecycle.BEFORE_MAIN_LOOP_STARTED; - final Iterator pluginsIterator = plugins.iterator(); + final Iterator pluginsIterator = registeredPlugins.iterator(); while (pluginsIterator.hasNext()) { final BesuPlugin plugin = pluginsIterator.next(); @@ -296,7 +266,7 @@ public void stopPlugins() { state); state = Lifecycle.STOPPING; - for (final BesuPlugin plugin : plugins) { + for (final BesuPlugin plugin : registeredPlugins) { try { plugin.stop(); LOG.debug("Stopped plugin of type {}.", plugin.getClass().getName()); @@ -309,11 +279,6 @@ public void stopPlugins() { state = Lifecycle.STOPPED; } - @Override - public Collection getPluginVersions() { - return Collections.unmodifiableList(pluginVersions); - } - private static URL pathToURIOrNull(final Path p) { try { return p.toUri().toURL(); @@ -322,16 +287,6 @@ private static URL pathToURIOrNull(final Path p) { } } - /** - * Gets plugins. - * - * @return the plugins - */ - @VisibleForTesting - List getPlugins() { - return Collections.unmodifiableList(plugins); - } - private Optional pluginDirectoryLoader(final Path pluginsDir) { if (pluginsDir != null && pluginsDir.toFile().isDirectory()) { LOG.debug("Searching for plugins in {}", pluginsDir.toAbsolutePath()); @@ -355,14 +310,62 @@ private Optional pluginDirectoryLoader(final Path pluginsDir) { return Optional.empty(); } + @Override + public Collection getPluginVersions() { + return Collections.unmodifiableList(pluginVersions); + } + + /** + * Gets plugins. + * + * @return the plugins + */ + @VisibleForTesting + List getRegisteredPlugins() { + return Collections.unmodifiableList(registeredPlugins); + } + /** * Gets named plugins. * * @return the named plugins */ public Map getNamedPlugins() { - return plugins.stream() + return registeredPlugins.stream() .filter(plugin -> plugin.getName().isPresent()) .collect(Collectors.toMap(plugin -> plugin.getName().get(), plugin -> plugin, (a, b) -> b)); } + + /** + * Generates a summary log of plugin registration. The summary includes registered plugins, + * detected but not registered (skipped) plugins, and requested but not found plugins. + * + * @return A list of strings, each representing a line in the summary log. + */ + public List getPluginsSummaryLog() { + List summary = new ArrayList<>(); + summary.add("Plugin registration summary:"); + + // Log registered plugins with their names and versions + registeredPlugins.forEach( + plugin -> summary.add(String.format("%s %s", plugin.getName(), plugin.getVersion()))); + + // Log detected but not registered (skipped) plugins + detectedPlugins.stream() + .filter(plugin -> !registeredPlugins.contains(plugin)) + .forEach(plugin -> summary.add(String.format(" %s (Skipped)", plugin.getName()))); + + // Collect names of detected plugins for comparison + Set detectedPluginNames = + detectedPlugins.stream() + .map(BesuPlugin::getName) + .flatMap(Optional::stream) + .collect(Collectors.toSet()); + + // Log requested but not found plugins + requestedPlugins.stream() + .filter(plugin -> !detectedPluginNames.contains(plugin)) + .forEach(plugin -> summary.add(String.format("%s (Not Found)", plugin))); + return summary; + } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java index cfc9964d28a..f735886544c 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java @@ -6,23 +6,31 @@ public class PluginConfiguration { final List pluginInfos; - final boolean strictPluginRegistrationEnabled; + final DetectionType detectionType; + final Path pluginsDir; public PluginConfiguration( - final List pluginInfos, final boolean pluginsStrictRegistration) { + final List pluginInfos, + final DetectionType pluginsStrictRegistration, + final Path pluginsDir) { this.pluginInfos = pluginInfos; - this.strictPluginRegistrationEnabled = pluginsStrictRegistration; + this.detectionType = pluginsStrictRegistration; + this.pluginsDir = pluginsDir; } public List getPluginInfos() { return pluginInfos; } - public boolean isStrictPluginRegistrationEnabled() { - return strictPluginRegistrationEnabled; + public DetectionType getDetectionType() { + return detectionType; } - public Path pluginsDir() { + public Path getPluginsDir() { + return pluginsDir; + } + + public static Path defaultPluginsDir() { final String pluginsDir = System.getProperty("besu.plugins.dir"); if (pluginsDir == null) { return new File(System.getProperty("besu.home", "."), "plugins").toPath(); @@ -30,4 +38,9 @@ public Path pluginsDir() { return new File(pluginsDir).toPath(); } } + + public enum DetectionType { + ALL, + EXPLICIT + } } diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/BesuPlugin.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/BesuPlugin.java index 5a54808a6c1..913b14b793c 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/BesuPlugin.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/BesuPlugin.java @@ -84,4 +84,24 @@ default CompletableFuture reloadConfiguration() { * started. */ void stop(); + + /** + * Retrieves the version information of the plugin. It constructs a version string using the + * implementation title and version from the package information. If either the title or version + * is not available, it defaults to the class's simple name and "", respectively. + * + * @return A string representing the plugin's version information, formatted as "Title/vVersion". + */ + default String getVersion() { + Package pluginPackage = this.getClass().getPackage(); + String implTitle = + Optional.ofNullable(pluginPackage.getImplementationTitle()) + .filter(title -> !title.isBlank()) + .orElseGet(() -> this.getClass().getSimpleName()); + String implVersion = + Optional.ofNullable(pluginPackage.getImplementationVersion()) + .filter(version -> !version.isBlank()) + .orElse(""); + return implTitle + "/v" + implVersion; + } } From 001c6e42aa321ef777042d11ee1804928b371f04 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 15:11:41 +1100 Subject: [PATCH 07/34] Refactoring and unit tests Signed-off-by: Gabriel-Trintinalia --- .../cli/converter/PluginInfoConverter.java | 28 +++++++-- .../besu/cli/util/CommandLineUtils.java | 26 ++++++++ .../besu/services/BesuPluginContextImpl.java | 18 +++--- .../core/plugins/PluginConfiguration.java | 63 +++++++++++++++---- .../ethereum/core/plugins/PluginInfo.java | 45 +++++++++++-- 5 files changed, 149 insertions(+), 31 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java index 3b407ae8f77..750784f38c7 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/converter/PluginInfoConverter.java @@ -2,22 +2,42 @@ import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; -import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; import picocli.CommandLine; +/** + * Converts a comma-separated string into a list of {@link PluginInfo} objects. This converter is + * intended for use with PicoCLI to process command line arguments that specify plugin information. + */ public class PluginInfoConverter implements CommandLine.ITypeConverter> { + /** + * Converts a comma-separated string into a list of {@link PluginInfo}. + * + * @param value The comma-separated string representing plugin names. + * @return A list of {@link PluginInfo} objects created from the provided string. + */ @Override public List convert(final String value) { - if (value == null || value.isEmpty()) { + if (value == null || value.trim().isEmpty()) { return List.of(); } - return Arrays.stream(value.split(",")).map(this::toPluginInfo).toList(); + return Stream.of(value.split(",")) + .map(String::trim) + .map(this::toPluginInfo) + .collect(Collectors.toList()); } + /** + * Creates a {@link PluginInfo} object from a plugin name. + * + * @param pluginName The name of the plugin. + * @return A {@link PluginInfo} object representing the plugin. + */ private PluginInfo toPluginInfo(final String pluginName) { - return new PluginInfo(pluginName); + return new PluginInfo(pluginName.trim()); } } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java index bfd1d54eb63..8394160e2e5 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java @@ -262,6 +262,18 @@ public static boolean isOptionSet(final CommandLine commandLine, final String op .anyMatch(CommandLineUtils::isOptionSet); } + /** + * Retrieves the value of a specified command line option, converting it to its appropriate type, + * or returns the default value if the option was not specified. + * + * @param The type of the option value. + * @param commandLine The {@link CommandLine} instance containing the parsed command line options. + * @param optionName The name of the option whose value is to be retrieved. + * @param converter A converter that converts the option's string value to its appropriate type. + * @return The value of the specified option converted to its type, or the default value if the + * option was not specified. Returns {@code null} if the option does not exist or if there is + * no default value and the option was not specified. + */ public static T getOptionValueOrDefault( final CommandLine commandLine, final String optionName, @@ -272,6 +284,20 @@ public static T getOptionValueOrDefault( .matchedOptionValue(optionName, getDefaultOptionValue(commandLine, optionName, converter)); } + /** + * Retrieves the default value for a specified command line option, converting it to its + * appropriate type. + * + * @param The type of the option value. + * @param commandLine The {@link CommandLine} instance containing the parsed command line options. + * @param optionName The name of the option whose default value is to be retrieved. + * @param converter A converter that converts the option's default string value to its appropriate + * type. + * @return The default value of the specified option converted to its type, or {@code null} if the + * option does not exist, does not have a default value, or if an error occurs during + * conversion. + * @throws RuntimeException if there is an error converting the default value string to its type. + */ private static T getDefaultOptionValue( final CommandLine commandLine, final String optionName, diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index 5198ba25096..c31d52f6f4b 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -107,6 +107,13 @@ public Optional getService(final Class serviceType return Optional.ofNullable((T) serviceRegistry.get(serviceType)); } + private List detectPlugins(final PluginConfiguration config) { + ClassLoader pluginLoader = + pluginDirectoryLoader(config.getPluginsDir()).orElse(getClass().getClassLoader()); + ServiceLoader serviceLoader = ServiceLoader.load(BesuPlugin.class, pluginLoader); + return StreamSupport.stream(serviceLoader.spliterator(), false).collect(Collectors.toList()); + } + /** * Registers plugins located at the specified path. This method constructs a plugin configuration * with the default settings and the provided path, then delegates to the overloaded {@code @@ -137,11 +144,11 @@ public void registerPlugins(final PluginConfiguration config) { "Besu plugins have already been registered. Cannot register additional plugins."); state = Lifecycle.REGISTERING; - detectedPlugins = findPlugins(config); + detectedPlugins = detectPlugins(config); if (config.getDetectionType() == PluginConfiguration.DetectionType.EXPLICIT) { // Extract the set of plugin names from the configuration requestedPlugins = - config.getPluginInfos().stream().map(PluginInfo::getName).collect(Collectors.toList()); + config.getPluginInfos().stream().map(PluginInfo::name).collect(Collectors.toList()); // Filter the detected Besu plugins to include only those explicitly configured List registeringPlugins = @@ -155,13 +162,6 @@ public void registerPlugins(final PluginConfiguration config) { } } - private List findPlugins(final PluginConfiguration config) { - ClassLoader pluginLoader = - pluginDirectoryLoader(config.getPluginsDir()).orElse(getClass().getClassLoader()); - ServiceLoader serviceLoader = ServiceLoader.load(BesuPlugin.class, pluginLoader); - return StreamSupport.stream(serviceLoader.spliterator(), false).collect(Collectors.toList()); - } - /** * Registers a list of plugins if the system is uninitialized. Updates system state throughout the * registration process and adds successfully registered plugins to the system. diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java index f735886544c..3d7cc22c8c1 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java @@ -1,21 +1,36 @@ package org.hyperledger.besu.ethereum.core.plugins; -import java.io.File; import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; import java.util.List; +import java.util.Objects; +/** + * Configuration for managing plugins, including their information, detection type, and directory. + */ public class PluginConfiguration { - final List pluginInfos; - final DetectionType detectionType; - final Path pluginsDir; + private final List pluginInfos; + private final DetectionType detectionType; + private final Path pluginsDir; + /** + * Constructs a new PluginConfiguration with the specified plugin information, detection type, and + * plugins directory. + * + * @param pluginInfos List of {@link PluginInfo} objects representing the plugins. + * @param detectionType The {@link DetectionType} indicating how plugins should be detected. + * @param pluginsDir The directory where plugins are located. + */ public PluginConfiguration( final List pluginInfos, - final DetectionType pluginsStrictRegistration, + final DetectionType detectionType, final Path pluginsDir) { - this.pluginInfos = pluginInfos; - this.detectionType = pluginsStrictRegistration; - this.pluginsDir = pluginsDir; + this.pluginInfos = + Collections.unmodifiableList( + Objects.requireNonNull(pluginInfos, "pluginInfos cannot be null")); + this.detectionType = Objects.requireNonNull(detectionType, "detectionType cannot be null"); + this.pluginsDir = Objects.requireNonNull(pluginsDir, "pluginsDir cannot be null"); } public List getPluginInfos() { @@ -30,17 +45,41 @@ public Path getPluginsDir() { return pluginsDir; } + /** + * Returns the default plugins directory based on system properties. + * + * @return The default {@link Path} to the plugin's directory. + */ public static Path defaultPluginsDir() { - final String pluginsDir = System.getProperty("besu.plugins.dir"); - if (pluginsDir == null) { - return new File(System.getProperty("besu.home", "."), "plugins").toPath(); + final String pluginsDirProperty = System.getProperty("besu.plugins.dir"); + if (pluginsDirProperty == null) { + return Paths.get(System.getProperty("besu.home", "."), "plugins"); } else { - return new File(pluginsDir).toPath(); + return Paths.get(pluginsDirProperty); } } + /** + * Enumerates the types of plugin detection mechanisms. + * + *

This enum defines how plugins should be detected and loaded by the system. + */ public enum DetectionType { + /** + * Indicates that all discoverable plugins should be loaded. + * + *

When set to {@code ALL}, the system will attempt to load all plugins found within the + * specified plugins directory, without requiring explicit configuration for each plugin. + */ ALL, + + /** + * Indicates that only explicitly specified plugins should be loaded. + * + *

When set to {@code EXPLICIT}, the system will only load plugins that have been explicitly + * listed in the configuration. This allows for more controlled and selective loading of + * plugins. + */ EXPLICIT } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java index 4eb6dc545ca..c887527361f 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java @@ -1,13 +1,46 @@ package org.hyperledger.besu.ethereum.core.plugins; +/** Represents information about a plugin, including its name. */ +public record PluginInfo(String name) { + /** + * Constructs a new PluginInfo instance with the specified name. + * + * @param name The name of the plugin. Cannot be null or empty. + * @throws IllegalArgumentException if the name is null or empty. + */ + public PluginInfo { + if (name == null || name.trim().isEmpty()) { + throw new IllegalArgumentException("Plugin name cannot be null or empty."); + } + } + + /** + * Returns the name of the plugin. + * + * @return The name of the plugin. + */ + @Override + public String name() { + return name; + } -public class PluginInfo { - String name; + // Optionally, override toString, equals, and hashCode methods - public PluginInfo(final String name) { - this.name = name; + @Override + public String toString() { + return "PluginInfo{name='" + name + "'}"; } - public String getName() { - return name; + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + PluginInfo that = (PluginInfo) o; + + return name.equals(that.name); } } From 586ae9f6a5b27ec48c828da61599b5334dbc069f Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 15:16:16 +1100 Subject: [PATCH 08/34] Simplify Signed-off-by: Gabriel-Trintinalia --- .../ethereum/core/plugins/PluginInfo.java | 35 ++++--------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java index c887527361f..0d76a83185f 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginInfo.java @@ -1,46 +1,23 @@ package org.hyperledger.besu.ethereum.core.plugins; + /** Represents information about a plugin, including its name. */ -public record PluginInfo(String name) { +public final class PluginInfo { + private final String name; + /** * Constructs a new PluginInfo instance with the specified name. * * @param name The name of the plugin. Cannot be null or empty. * @throws IllegalArgumentException if the name is null or empty. */ - public PluginInfo { + public PluginInfo(final String name) { if (name == null || name.trim().isEmpty()) { throw new IllegalArgumentException("Plugin name cannot be null or empty."); } + this.name = name; } - /** - * Returns the name of the plugin. - * - * @return The name of the plugin. - */ - @Override public String name() { return name; } - - // Optionally, override toString, equals, and hashCode methods - - @Override - public String toString() { - return "PluginInfo{name='" + name + "'}"; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - PluginInfo that = (PluginInfo) o; - - return name.equals(that.name); - } } From 58317dee40f8850ee86f1b35cec8749db2fc5a4c Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 15:29:50 +1100 Subject: [PATCH 09/34] Simplify Signed-off-by: Gabriel-Trintinalia --- .../org/hyperledger/besu/cli/BesuCommand.java | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 19fcbebfe5b..d311d86e7e6 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1056,14 +1056,7 @@ public int parse( preparePlugins(); final ConfigOptionSearchAndRunHandler configParsingHandler = - new ConfigOptionSearchAndRunHandler( - (parseResult -> { - registerPlugins(parseResult); - commandLine.setExecutionStrategy(resultHandler); - commandLine.execute(parseResult.originalArgs().toArray(new String[0])); - return 0; - }), - environment); + getConfigOptionSearchAndRunHandler(resultHandler); return commandLine .setExecutionStrategy(configParsingHandler) @@ -1072,6 +1065,28 @@ public int parse( .execute(args); } + /** + * Creates a handler for searching and running configuration options with plugin registration. + * + * @param nextHandler The next execution strategy to be used after plugin registration. + * @return A {@link ConfigOptionSearchAndRunHandler} configured to register plugins and then + * delegate to the next handler. + */ + private ConfigOptionSearchAndRunHandler getConfigOptionSearchAndRunHandler( + IExecutionStrategy nextHandler) { + final IExecutionStrategy pluginRegistrationTask = + parseResult -> { + // Extract PluginConfiguration from command line arguments and register plugins. + PluginConfiguration configuration = + PluginsConfigurationOptions.fromCommandLine(parseResult.commandSpec().commandLine()); + besuPluginContext.registerPlugins(configuration); + + commandLine.setExecutionStrategy(nextHandler); + return commandLine.execute(parseResult.originalArgs().toArray(new String[0])); + }; + return new ConfigOptionSearchAndRunHandler(pluginRegistrationTask, environment); + } + /** Used by Dagger to parse all options into a commandline instance. */ public void toCommandLine() { commandLine = @@ -1247,14 +1262,6 @@ private SecurityModule defaultSecurityModule() { return new KeyPairSecurityModule(loadKeyPair(nodePrivateKeyFileOption.getNodePrivateKeyFile())); } - private void registerPlugins(final CommandLine.ParseResult parseResult) { - - PluginConfiguration configuration = - PluginsConfigurationOptions.fromCommandLine(parseResult.commandSpec().commandLine()); - - besuPluginContext.registerPlugins(configuration); - } - // loadKeyPair() is public because it is accessed by subcommands /** From bab565d2e8c5cadd037a9837b2cd2e2753898bdd Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 16:46:07 +1100 Subject: [PATCH 10/34] Add tests Signed-off-by: Gabriel-Trintinalia --- .../services/BesuPluginContextImplTest.java | 68 +++++++++++++++++-- .../org/hyperledger/besu/cli/BesuCommand.java | 2 +- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index 2ee1d7612c8..4501807e6b7 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -17,13 +17,17 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; +import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; import org.hyperledger.besu.plugin.BesuPlugin; +import org.hyperledger.besu.tests.acceptance.plugins.TestBesuEventsPlugin; import org.hyperledger.besu.tests.acceptance.plugins.TestPicoCLIPlugin; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import java.util.Optional; @@ -58,7 +62,7 @@ public void verifyEverythingGoesSmoothly() { assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); final Optional testPluginOptional = - findTestPlugin(contextImpl.getRegisteredPlugins()); + findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); Assertions.assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); @@ -102,7 +106,7 @@ public void startErrorsHandledSmoothly() { .contains(TestPicoCLIPlugin.class); final Optional testPluginOptional = - findTestPlugin(contextImpl.getRegisteredPlugins()); + findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); @@ -128,7 +132,7 @@ public void stopErrorsHandledSmoothly() { .contains(TestPicoCLIPlugin.class); final Optional testPluginOptional = - findTestPlugin(contextImpl.getRegisteredPlugins()); + findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); assertThat(testPluginOptional).isPresent(); final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); @@ -165,9 +169,63 @@ public void lifecycleExceptions() throws Throwable { assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins); } - private Optional findTestPlugin(final List plugins) { + @Test + public void shouldRegisterAllPluginsWhenDetectionTypeIsAll() { + final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); + final PluginConfiguration config = + new PluginConfiguration( + List.of(), // Explicitly include our test plugin + PluginConfiguration.DetectionType.ALL, + Paths.get(".")); + + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); + contextImpl.registerPlugins(config); + final Optional testPluginOptional = + findTestPlugin(contextImpl.getRegisteredPlugins(), TestPicoCLIPlugin.class); + Assertions.assertThat(testPluginOptional).isPresent(); + final TestPicoCLIPlugin testPicoCLIPlugin = testPluginOptional.get(); + assertThat(testPicoCLIPlugin.getState()).isEqualTo("registered"); + } + + @Test + public void shouldRegisterOnlySpecifiedPluginWhenDetectionTypeIsExplicit() { + final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); + final PluginConfiguration config = + new PluginConfiguration( + List.of(new PluginInfo("TestPicoCLIPlugin")), // Explicitly include our test plugin + PluginConfiguration.DetectionType.EXPLICIT, + Paths.get(".")); + + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); + contextImpl.registerPlugins(config); + + final Optional nonRequestedPlugin = + findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class); + + Assertions.assertThat(nonRequestedPlugin).isEmpty(); + } + + @Test + public void shouldNotRegisterUnspecifiedPluginsWhenDetectionTypeIsExplicit() { + final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); + final PluginConfiguration config = + new PluginConfiguration( + List.of(new PluginInfo("TestPicoCLIPlugin")), // Explicitly include our test plugin + PluginConfiguration.DetectionType.EXPLICIT, + Paths.get(".")); + + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); + contextImpl.registerPlugins(config); + + final Optional nonRequestedPlugin = + findTestPlugin(contextImpl.getRegisteredPlugins(), TestBesuEventsPlugin.class); + Assertions.assertThat(nonRequestedPlugin).isEmpty(); + } + + private Optional findTestPlugin( + final List plugins, final Class type) { return plugins.stream() - .filter(p -> p instanceof TestPicoCLIPlugin) + .filter(p -> type.equals(p.getClass())) .map(p -> (TestPicoCLIPlugin) p) .findFirst(); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index d311d86e7e6..4d8bb4eef1c 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1073,7 +1073,7 @@ public int parse( * delegate to the next handler. */ private ConfigOptionSearchAndRunHandler getConfigOptionSearchAndRunHandler( - IExecutionStrategy nextHandler) { + final IExecutionStrategy nextHandler) { final IExecutionStrategy pluginRegistrationTask = parseResult -> { // Extract PluginConfiguration from command line arguments and register plugins. From e03a8de78dd91608394475a594055379aae42fab Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 17:00:24 +1100 Subject: [PATCH 11/34] Add tests Signed-off-by: Gabriel-Trintinalia --- .../services/BesuPluginContextImplTest.java | 29 ++++++++++++---- .../besu/services/BesuPluginContextImpl.java | 33 +++++++++++++++++-- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index 4501807e6b7..e031c4e53c6 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -16,6 +16,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.junit.jupiter.api.Assertions.assertThrows; import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; @@ -29,6 +30,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; +import java.util.NoSuchElementException; import java.util.Optional; import org.assertj.core.api.Assertions; @@ -173,10 +175,7 @@ public void lifecycleExceptions() throws Throwable { public void shouldRegisterAllPluginsWhenDetectionTypeIsAll() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final PluginConfiguration config = - new PluginConfiguration( - List.of(), // Explicitly include our test plugin - PluginConfiguration.DetectionType.ALL, - Paths.get(".")); + new PluginConfiguration(List.of(), PluginConfiguration.DetectionType.ALL, Paths.get(".")); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(config); @@ -192,7 +191,7 @@ public void shouldRegisterOnlySpecifiedPluginWhenDetectionTypeIsExplicit() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final PluginConfiguration config = new PluginConfiguration( - List.of(new PluginInfo("TestPicoCLIPlugin")), // Explicitly include our test plugin + List.of(new PluginInfo("TestPicoCLIPlugin")), PluginConfiguration.DetectionType.EXPLICIT, Paths.get(".")); @@ -210,7 +209,7 @@ public void shouldNotRegisterUnspecifiedPluginsWhenDetectionTypeIsExplicit() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final PluginConfiguration config = new PluginConfiguration( - List.of(new PluginInfo("TestPicoCLIPlugin")), // Explicitly include our test plugin + List.of(new PluginInfo("TestPicoCLIPlugin")), PluginConfiguration.DetectionType.EXPLICIT, Paths.get(".")); @@ -222,6 +221,24 @@ public void shouldNotRegisterUnspecifiedPluginsWhenDetectionTypeIsExplicit() { Assertions.assertThat(nonRequestedPlugin).isEmpty(); } + @Test + void shouldThrowExceptionIfExplicitlySpecifiedPluginNotFound() { + final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); + PluginConfiguration config = + new PluginConfiguration( + List.of(new PluginInfo("NonExistentPlugin")), + PluginConfiguration.DetectionType.EXPLICIT, + Paths.get(".")); + + String exceptionMessage = + assertThrows(NoSuchElementException.class, () -> contextImpl.registerPlugins(config)) + .getMessage(); + final String expectedMessage = + "The following requested plugins were not found: NonExistentPlugin"; + assertThat(exceptionMessage).isEqualTo(expectedMessage); + assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); + } + private Optional findTestPlugin( final List plugins, final Class type) { return plugins.stream() diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index c31d52f6f4b..566c3b1661b 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -37,6 +37,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; @@ -152,9 +153,8 @@ public void registerPlugins(final PluginConfiguration config) { // Filter the detected Besu plugins to include only those explicitly configured List registeringPlugins = - detectedPlugins.stream() - .filter(plugin -> requestedPlugins.contains(plugin.getClass().getSimpleName())) - .toList(); + matchAndValidateRequestedPlugins(requestedPlugins, detectedPlugins); + registerPlugins(registeringPlugins); } else { @@ -162,6 +162,33 @@ public void registerPlugins(final PluginConfiguration config) { } } + private List matchAndValidateRequestedPlugins( + final List requestedPluginNames, final List detectedPlugins) + throws NoSuchElementException { + + // Filter detected plugins to include only those that match the requested names + List matchingPlugins = + detectedPlugins.stream() + .filter(plugin -> requestedPluginNames.contains(plugin.getClass().getSimpleName())) + .collect(Collectors.toList()); + + // Check if all requested plugins were found among the detected plugins + if (matchingPlugins.size() != requestedPluginNames.size()) { + // Find which requested plugins were not matched to throw a detailed exception + Set matchedPluginNames = + matchingPlugins.stream() + .map(plugin -> plugin.getClass().getSimpleName()) + .collect(Collectors.toSet()); + String missingPlugins = + requestedPluginNames.stream() + .filter(name -> !matchedPluginNames.contains(name)) + .collect(Collectors.joining(", ")); + throw new NoSuchElementException( + "The following requested plugins were not found: " + missingPlugins); + } + return matchingPlugins; + } + /** * Registers a list of plugins if the system is uninitialized. Updates system state throughout the * registration process and adds successfully registered plugins to the system. From e44856f4ba1a3f18bd48486d247f5ff28a18188f Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 17:12:41 +1100 Subject: [PATCH 12/34] improve code Signed-off-by: Gabriel-Trintinalia --- .../dsl/node/ThreadBesuNodeRunner.java | 3 ++- .../besu/services/BesuPluginContextImplTest.java | 13 +++++++------ .../besu/services/BesuPluginContextImpl.java | 14 -------------- .../core/plugins/PluginConfiguration.java | 16 +++++++++++----- 4 files changed, 20 insertions(+), 26 deletions(-) diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java index 1c10f82e3bf..e189d3b2d46 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java @@ -30,6 +30,7 @@ import org.hyperledger.besu.ethereum.GasLimitCalculator; import org.hyperledger.besu.ethereum.api.graphql.GraphQLConfiguration; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; @@ -117,7 +118,7 @@ private BesuPluginContextImpl buildPluginContext( } else { pluginsPath = Path.of(pluginDir); } - besuPluginContext.registerPlugins(pluginsPath); + besuPluginContext.registerPlugins(new PluginConfiguration(pluginsPath)); commandLine.parseArgs(node.getConfiguration().getExtraCLIOptions().toArray(new String[0])); diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index e031c4e53c6..ee8459cf3e9 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -24,7 +24,6 @@ import org.hyperledger.besu.tests.acceptance.plugins.TestBesuEventsPlugin; import org.hyperledger.besu.tests.acceptance.plugins.TestPicoCLIPlugin; -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -41,6 +40,8 @@ public class BesuPluginContextImplTest { + private static final Path PATH = Paths.get("."); + @BeforeAll public static void createFakePluginDir() throws IOException { if (System.getProperty("besu.plugins.dir") == null) { @@ -60,7 +61,7 @@ public void verifyEverythingGoesSmoothly() { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new File(".").toPath()); + contextImpl.registerPlugins(new PluginConfiguration(PATH)); assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); final Optional testPluginOptional = @@ -83,7 +84,7 @@ public void registrationErrorsHandledSmoothly() { System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new File(".").toPath()); + contextImpl.registerPlugins(new PluginConfiguration(PATH)); assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.beforeExternalServices(); @@ -102,7 +103,7 @@ public void startErrorsHandledSmoothly() { System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new File(".").toPath()); + contextImpl.registerPlugins(new PluginConfiguration(PATH)); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -128,7 +129,7 @@ public void stopErrorsHandledSmoothly() { System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new File(".").toPath()); + contextImpl.registerPlugins(new PluginConfiguration(PATH)); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -151,7 +152,7 @@ public void stopErrorsHandledSmoothly() { public void lifecycleExceptions() throws Throwable { final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final ThrowableAssert.ThrowingCallable registerPlugins = - () -> contextImpl.registerPlugins(new File(".").toPath()); + () -> contextImpl.registerPlugins(new PluginConfiguration(PATH)); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::startPlugins); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins); diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index 566c3b1661b..9b959dc7cec 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -115,20 +115,6 @@ private List detectPlugins(final PluginConfiguration config) { return StreamSupport.stream(serviceLoader.spliterator(), false).collect(Collectors.toList()); } - /** - * Registers plugins located at the specified path. This method constructs a plugin configuration - * with the default settings and the provided path, then delegates to the overloaded {@code - * registerPlugins} method that accepts a {@link PluginConfiguration} object. - * - * @param path The path where plugins are located. This path will be used to search for plugin - * implementations to be registered. - */ - public void registerPlugins(final Path path) { - PluginConfiguration configuration = - new PluginConfiguration(List.of(), PluginConfiguration.DetectionType.ALL, path); - registerPlugins(configuration); - } - /** * Registers plugins based on the provided {@link PluginConfiguration}. This method finds plugins * according to the configuration settings, filters them if necessary (based on the detection diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java index 3d7cc22c8c1..5870efe56b2 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java @@ -1,10 +1,11 @@ package org.hyperledger.besu.ethereum.core.plugins; +import static java.util.Objects.requireNonNull; + import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collections; import java.util.List; -import java.util.Objects; /** * Configuration for managing plugins, including their information, detection type, and directory. @@ -27,10 +28,15 @@ public PluginConfiguration( final DetectionType detectionType, final Path pluginsDir) { this.pluginInfos = - Collections.unmodifiableList( - Objects.requireNonNull(pluginInfos, "pluginInfos cannot be null")); - this.detectionType = Objects.requireNonNull(detectionType, "detectionType cannot be null"); - this.pluginsDir = Objects.requireNonNull(pluginsDir, "pluginsDir cannot be null"); + Collections.unmodifiableList(requireNonNull(pluginInfos, "pluginInfos cannot be null")); + this.detectionType = requireNonNull(detectionType, "detectionType cannot be null"); + this.pluginsDir = requireNonNull(pluginsDir, "pluginsDir cannot be null"); + } + + public PluginConfiguration(final Path pluginsDir) { + this.pluginInfos = List.of(); + this.detectionType = DetectionType.ALL; + this.pluginsDir = requireNonNull(pluginsDir, "pluginsDir cannot be null"); } public List getPluginInfos() { From 5f86fabb2e9c437cbae35dbd9363e3b732fccf1d Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 17:20:34 +1100 Subject: [PATCH 13/34] improve code Signed-off-by: Gabriel-Trintinalia --- .../services/BesuPluginContextImplTest.java | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index ee8459cf3e9..cb63b6acb03 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -36,11 +36,12 @@ import org.assertj.core.api.ThrowableAssert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; public class BesuPluginContextImplTest { - private static final Path PATH = Paths.get("."); + private BesuPluginContextImpl contextImpl; @BeforeAll public static void createFakePluginDir() throws IOException { @@ -56,10 +57,13 @@ public void clearTestPluginState() { System.clearProperty("testPicoCLIPlugin.testOption"); } + @BeforeEach + void setup() { + contextImpl = new BesuPluginContextImpl(); + } + @Test public void verifyEverythingGoesSmoothly() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); - assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(new PluginConfiguration(PATH)); assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); @@ -80,7 +84,6 @@ public void verifyEverythingGoesSmoothly() { @Test public void registrationErrorsHandledSmoothly() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); @@ -99,7 +102,6 @@ public void registrationErrorsHandledSmoothly() { @Test public void startErrorsHandledSmoothly() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); @@ -125,7 +127,6 @@ public void startErrorsHandledSmoothly() { @Test public void stopErrorsHandledSmoothly() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); @@ -150,7 +151,6 @@ public void stopErrorsHandledSmoothly() { @Test public void lifecycleExceptions() throws Throwable { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final ThrowableAssert.ThrowingCallable registerPlugins = () -> contextImpl.registerPlugins(new PluginConfiguration(PATH)); @@ -174,7 +174,6 @@ public void lifecycleExceptions() throws Throwable { @Test public void shouldRegisterAllPluginsWhenDetectionTypeIsAll() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final PluginConfiguration config = new PluginConfiguration(List.of(), PluginConfiguration.DetectionType.ALL, Paths.get(".")); @@ -189,7 +188,6 @@ public void shouldRegisterAllPluginsWhenDetectionTypeIsAll() { @Test public void shouldRegisterOnlySpecifiedPluginWhenDetectionTypeIsExplicit() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final PluginConfiguration config = new PluginConfiguration( List.of(new PluginInfo("TestPicoCLIPlugin")), @@ -207,7 +205,6 @@ public void shouldRegisterOnlySpecifiedPluginWhenDetectionTypeIsExplicit() { @Test public void shouldNotRegisterUnspecifiedPluginsWhenDetectionTypeIsExplicit() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); final PluginConfiguration config = new PluginConfiguration( List.of(new PluginInfo("TestPicoCLIPlugin")), @@ -224,7 +221,6 @@ public void shouldNotRegisterUnspecifiedPluginsWhenDetectionTypeIsExplicit() { @Test void shouldThrowExceptionIfExplicitlySpecifiedPluginNotFound() { - final BesuPluginContextImpl contextImpl = new BesuPluginContextImpl(); PluginConfiguration config = new PluginConfiguration( List.of(new PluginInfo("NonExistentPlugin")), From b443f9306d0b1dc5217adadbc6feadc5cc81b6ab Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 21:10:15 +1100 Subject: [PATCH 14/34] improve code Signed-off-by: Gabriel-Trintinalia --- .../services/BesuPluginContextImplTest.java | 43 +++++------- .../stable/PluginsConfigurationOptions.java | 35 ++++------ .../besu/services/BesuPluginContextImpl.java | 12 ++-- .../core/plugins/PluginConfiguration.java | 69 +++++++------------ 4 files changed, 65 insertions(+), 94 deletions(-) diff --git a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java index cb63b6acb03..3798e87e655 100644 --- a/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java +++ b/acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java @@ -40,7 +40,7 @@ import org.junit.jupiter.api.Test; public class BesuPluginContextImplTest { - private static final Path PATH = Paths.get("."); + private static final Path DEFAULT_PLUGIN_DIRECTORY = Paths.get("."); private BesuPluginContextImpl contextImpl; @BeforeAll @@ -65,7 +65,7 @@ void setup() { @Test public void verifyEverythingGoesSmoothly() { assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(PATH)); + contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty(); final Optional testPluginOptional = @@ -87,7 +87,7 @@ public void registrationErrorsHandledSmoothly() { System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(PATH)); + contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class); contextImpl.beforeExternalServices(); @@ -105,7 +105,7 @@ public void startErrorsHandledSmoothly() { System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(PATH)); + contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -130,7 +130,7 @@ public void stopErrorsHandledSmoothly() { System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); - contextImpl.registerPlugins(new PluginConfiguration(PATH)); + contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); assertThat(contextImpl.getRegisteredPlugins()) .extracting("class") .contains(TestPicoCLIPlugin.class); @@ -152,7 +152,7 @@ public void stopErrorsHandledSmoothly() { @Test public void lifecycleExceptions() throws Throwable { final ThrowableAssert.ThrowingCallable registerPlugins = - () -> contextImpl.registerPlugins(new PluginConfiguration(PATH)); + () -> contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY)); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::startPlugins); assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins); @@ -174,8 +174,7 @@ public void lifecycleExceptions() throws Throwable { @Test public void shouldRegisterAllPluginsWhenDetectionTypeIsAll() { - final PluginConfiguration config = - new PluginConfiguration(List.of(), PluginConfiguration.DetectionType.ALL, Paths.get(".")); + final PluginConfiguration config = createConfigurationForAllPlugins(); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(config); @@ -188,11 +187,7 @@ public void shouldRegisterAllPluginsWhenDetectionTypeIsAll() { @Test public void shouldRegisterOnlySpecifiedPluginWhenDetectionTypeIsExplicit() { - final PluginConfiguration config = - new PluginConfiguration( - List.of(new PluginInfo("TestPicoCLIPlugin")), - PluginConfiguration.DetectionType.EXPLICIT, - Paths.get(".")); + final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(config); @@ -205,12 +200,7 @@ public void shouldRegisterOnlySpecifiedPluginWhenDetectionTypeIsExplicit() { @Test public void shouldNotRegisterUnspecifiedPluginsWhenDetectionTypeIsExplicit() { - final PluginConfiguration config = - new PluginConfiguration( - List.of(new PluginInfo("TestPicoCLIPlugin")), - PluginConfiguration.DetectionType.EXPLICIT, - Paths.get(".")); - + final PluginConfiguration config = createConfigurationForSpecificPlugin("TestPicoCLIPlugin"); assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); contextImpl.registerPlugins(config); @@ -221,11 +211,7 @@ public void shouldNotRegisterUnspecifiedPluginsWhenDetectionTypeIsExplicit() { @Test void shouldThrowExceptionIfExplicitlySpecifiedPluginNotFound() { - PluginConfiguration config = - new PluginConfiguration( - List.of(new PluginInfo("NonExistentPlugin")), - PluginConfiguration.DetectionType.EXPLICIT, - Paths.get(".")); + PluginConfiguration config = createConfigurationForSpecificPlugin("NonExistentPlugin"); String exceptionMessage = assertThrows(NoSuchElementException.class, () -> contextImpl.registerPlugins(config)) @@ -236,6 +222,15 @@ void shouldThrowExceptionIfExplicitlySpecifiedPluginNotFound() { assertThat(contextImpl.getRegisteredPlugins()).isEmpty(); } + private PluginConfiguration createConfigurationForAllPlugins() { + return new PluginConfiguration(null, false, DEFAULT_PLUGIN_DIRECTORY); + } + + private PluginConfiguration createConfigurationForSpecificPlugin(final String pluginName) { + return new PluginConfiguration( + List.of(new PluginInfo(pluginName)), true, DEFAULT_PLUGIN_DIRECTORY); + } + private Optional findTestPlugin( final List plugins, final Class type) { return plugins.stream() diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index 5364b7ff6a2..823584f1cef 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -12,38 +12,33 @@ import picocli.CommandLine; +@SuppressWarnings("UnusedVariable") public class PluginsConfigurationOptions { @CommandLine.Option( names = {DEFAULT_PLUGINS_OPTION_NAME}, - description = "Comma separated list of plugins.", + description = "Comma-separated list of plugins.", split = ",", converter = PluginInfoConverter.class, arity = "0..*") - @SuppressWarnings({ - "FieldCanBeFinal", - "FieldMayBeFinal", - "UnusedVariable" - }) // PicoCLI requires non-final Strings. - private List plugins = null; + private List plugins; @CommandLine.Option( names = {DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME}, defaultValue = "false", - description = "Comma separated list of plugins.") - @SuppressWarnings({ - "FieldCanBeFinal", - "FieldMayBeFinal", - "UnusedVariable" - }) // PicoCLI requires non-final Strings. - private boolean requireExplicitPlugins = false; - + description = "Enables strict registration of plugins.") + private boolean strictRegistration; + + /** + * Constructs a {@link PluginConfiguration} instance based on the command line options. + * + * @param commandLine The command line instance containing parsed options. + * @return A new {@link PluginConfiguration} instance. + */ public static PluginConfiguration fromCommandLine(final CommandLine commandLine) { - PluginConfiguration.DetectionType detectionType = + boolean strictRegistration = CommandLineUtils.getOptionValueOrDefault( - commandLine, - DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, - PluginConfiguration.DetectionType::valueOf); + commandLine, DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, Boolean::valueOf); List plugins = CommandLineUtils.getOptionValueOrDefault( @@ -51,7 +46,7 @@ public static PluginConfiguration fromCommandLine(final CommandLine commandLine) return new PluginConfiguration( plugins != null ? plugins : List.of(), - detectionType, + strictRegistration, PluginConfiguration.defaultPluginsDir()); } } diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index 9b959dc7cec..73ee3f6581e 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkState; import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; -import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; import org.hyperledger.besu.plugin.BesuContext; import org.hyperledger.besu.plugin.BesuPlugin; import org.hyperledger.besu.plugin.services.BesuService; @@ -132,18 +131,17 @@ public void registerPlugins(final PluginConfiguration config) { state = Lifecycle.REGISTERING; detectedPlugins = detectPlugins(config); - if (config.getDetectionType() == PluginConfiguration.DetectionType.EXPLICIT) { - // Extract the set of plugin names from the configuration - requestedPlugins = - config.getPluginInfos().stream().map(PluginInfo::name).collect(Collectors.toList()); + if (config.isStrictRegistration()) { + // Register only the plugins that were explicitly requested and validated + requestedPlugins = config.getRequestedPlugins(); - // Filter the detected Besu plugins to include only those explicitly configured + // Match and validate the requested plugins against the detected plugins List registeringPlugins = matchAndValidateRequestedPlugins(requestedPlugins, detectedPlugins); registerPlugins(registeringPlugins); - } else { + // If strict registration is not enabled, register all detected plugins registerPlugins(detectedPlugins); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java index 5870efe56b2..60eb7b3c624 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java @@ -6,45 +6,52 @@ import java.nio.file.Paths; import java.util.Collections; import java.util.List; +import java.util.stream.Collectors; /** * Configuration for managing plugins, including their information, detection type, and directory. */ public class PluginConfiguration { - private final List pluginInfos; - private final DetectionType detectionType; + private final List requestedPlugins; + private final boolean strictRegistration; private final Path pluginsDir; /** * Constructs a new PluginConfiguration with the specified plugin information, detection type, and - * plugins directory. + * requestedPlugins directory. * - * @param pluginInfos List of {@link PluginInfo} objects representing the plugins. - * @param detectionType The {@link DetectionType} indicating how plugins should be detected. - * @param pluginsDir The directory where plugins are located. + * @param requestedPlugins List of {@link PluginInfo} objects representing the requestedPlugins. + * @param strictRegistration strictRegistration + * @param pluginsDir The directory where requestedPlugins are located. */ public PluginConfiguration( - final List pluginInfos, - final DetectionType detectionType, + final List requestedPlugins, + final boolean strictRegistration, final Path pluginsDir) { - this.pluginInfos = - Collections.unmodifiableList(requireNonNull(pluginInfos, "pluginInfos cannot be null")); - this.detectionType = requireNonNull(detectionType, "detectionType cannot be null"); - this.pluginsDir = requireNonNull(pluginsDir, "pluginsDir cannot be null"); + this.requestedPlugins = requestedPlugins; + this.strictRegistration = strictRegistration; + this.pluginsDir = pluginsDir; } public PluginConfiguration(final Path pluginsDir) { - this.pluginInfos = List.of(); - this.detectionType = DetectionType.ALL; - this.pluginsDir = requireNonNull(pluginsDir, "pluginsDir cannot be null"); + this.requestedPlugins = null; + this.strictRegistration = false; + this.pluginsDir = requireNonNull(pluginsDir); } - public List getPluginInfos() { - return pluginInfos; + /** + * Returns the names of requested plugins, or an empty list if none. + * + * @return List of requested plugin names, never {@code null}. + */ + public List getRequestedPlugins() { + return requestedPlugins == null + ? Collections.emptyList() + : requestedPlugins.stream().map(PluginInfo::name).collect(Collectors.toList()); } - public DetectionType getDetectionType() { - return detectionType; + public boolean isStrictRegistration() { + return strictRegistration; } public Path getPluginsDir() { @@ -64,28 +71,4 @@ public static Path defaultPluginsDir() { return Paths.get(pluginsDirProperty); } } - - /** - * Enumerates the types of plugin detection mechanisms. - * - *

This enum defines how plugins should be detected and loaded by the system. - */ - public enum DetectionType { - /** - * Indicates that all discoverable plugins should be loaded. - * - *

When set to {@code ALL}, the system will attempt to load all plugins found within the - * specified plugins directory, without requiring explicit configuration for each plugin. - */ - ALL, - - /** - * Indicates that only explicitly specified plugins should be loaded. - * - *

When set to {@code EXPLICIT}, the system will only load plugins that have been explicitly - * listed in the configuration. This allows for more controlled and selective loading of - * plugins. - */ - EXPLICIT - } } From b1349c4830053d0ec82ad437e9dbe671c3787714 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Thu, 7 Mar 2024 21:50:45 +1100 Subject: [PATCH 15/34] improve code Signed-off-by: Gabriel-Trintinalia --- .../stable/PluginsConfigurationOptions.java | 4 +- .../besu/cli/util/CommandLineUtils.java | 4 +- .../besu/services/BesuPluginContextImpl.java | 49 +++++++++++-------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index 823584f1cef..b628483b585 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -45,8 +45,6 @@ public static PluginConfiguration fromCommandLine(final CommandLine commandLine) commandLine, DEFAULT_PLUGINS_OPTION_NAME, new PluginInfoConverter()); return new PluginConfiguration( - plugins != null ? plugins : List.of(), - strictRegistration, - PluginConfiguration.defaultPluginsDir()); + plugins, strictRegistration, PluginConfiguration.defaultPluginsDir()); } } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java index 8394160e2e5..c1bb5cb109f 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/util/CommandLineUtils.java @@ -310,7 +310,9 @@ private static T getDefaultOptionValue( try { String defaultValueString = commandLine.getDefaultValueProvider().defaultValue(optionSpec); - return defaultValueString != null ? converter.convert(defaultValueString) : null; + return defaultValueString != null + ? converter.convert(defaultValueString) + : optionSpec.getValue(); } catch (Exception e) { throw new RuntimeException( "Failed to convert default value for option " + optionName + ": " + e.getMessage(), e); diff --git a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java index 73ee3f6581e..b863435c0e1 100644 --- a/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java +++ b/besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java @@ -83,7 +83,6 @@ private enum Lifecycle { private final List registeredPlugins = new ArrayList<>(); private final List pluginVersions = new ArrayList<>(); - final List lines = new ArrayList<>(); /** * Add service. @@ -201,7 +200,6 @@ private boolean registerPlugin(BesuPlugin plugin) { + plugin.getClass().getName() + ", start and stop will not be called.", e); - lines.add(String.format("ERROR %s", plugin.getClass().getSimpleName())); return false; } return true; @@ -355,28 +353,39 @@ public Map getNamedPlugins() { */ public List getPluginsSummaryLog() { List summary = new ArrayList<>(); - summary.add("Plugin registration summary:"); + summary.add("Plugin Registration Summary:"); // Log registered plugins with their names and versions - registeredPlugins.forEach( - plugin -> summary.add(String.format("%s %s", plugin.getName(), plugin.getVersion()))); - - // Log detected but not registered (skipped) plugins - detectedPlugins.stream() - .filter(plugin -> !registeredPlugins.contains(plugin)) - .forEach(plugin -> summary.add(String.format(" %s (Skipped)", plugin.getName()))); + if (registeredPlugins.isEmpty()) { + summary.add("No plugins have been registered."); + } else { + summary.add("Registered Plugins:"); + registeredPlugins.forEach( + plugin -> + summary.add( + String.format( + " - %s (Version: %s)", + plugin.getClass().getSimpleName(), plugin.getVersion()))); + } - // Collect names of detected plugins for comparison - Set detectedPluginNames = + // Identify and log detected but not registered (skipped) plugins + List skippedPlugins = detectedPlugins.stream() - .map(BesuPlugin::getName) - .flatMap(Optional::stream) - .collect(Collectors.toSet()); - - // Log requested but not found plugins - requestedPlugins.stream() - .filter(plugin -> !detectedPluginNames.contains(plugin)) - .forEach(plugin -> summary.add(String.format("%s (Not Found)", plugin))); + .filter(plugin -> !registeredPlugins.contains(plugin)) + .map(plugin -> plugin.getClass().getSimpleName()) + .toList(); + + if (!skippedPlugins.isEmpty()) { + summary.add("Skipped Plugins:"); + skippedPlugins.forEach( + pluginName -> + summary.add(String.format(" - %s (Detected but not registered)", pluginName))); + } + summary.add( + String.format( + "TOTAL = %d of %d plugins successfully registered.", + registeredPlugins.size(), detectedPlugins.size())); + return summary; } } From 41f815f2904c15b3441e6fc2ff5bc025c29ee9aa Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Fri, 8 Mar 2024 10:51:33 +1100 Subject: [PATCH 16/34] Implement CLIOptions interface Signed-off-by: Gabriel-Trintinalia --- .../org/hyperledger/besu/cli/BesuCommand.java | 5 ++++ .../stable/PluginsConfigurationOptions.java | 30 +++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 4d8bb4eef1c..e72f7dd45cf 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1468,6 +1468,11 @@ private void validateOptions() { validateApiOptions(); p2pTLSConfigOptions.checkP2PTLSOptionsDependencies(logger, commandLine); pkiBlockCreationOptions.checkPkiBlockCreationOptionsDependencies(logger, commandLine); + validatePluginOptions(logger, commandLine); + } + + private void validatePluginOptions(final Logger logger, final CommandLine commandLine) { + pluginsConfigurationOptions.validate(logger, commandLine); } private void validateApiOptions() { diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index b628483b585..e2c26229b9a 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -4,17 +4,17 @@ import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME; import org.hyperledger.besu.cli.converter.PluginInfoConverter; +import org.hyperledger.besu.cli.options.CLIOptions; import org.hyperledger.besu.cli.util.CommandLineUtils; import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration; import org.hyperledger.besu.ethereum.core.plugins.PluginInfo; import java.util.List; +import org.slf4j.Logger; import picocli.CommandLine; -@SuppressWarnings("UnusedVariable") -public class PluginsConfigurationOptions { - +public class PluginsConfigurationOptions implements CLIOptions { @CommandLine.Option( names = {DEFAULT_PLUGINS_OPTION_NAME}, description = "Comma-separated list of plugins.", @@ -29,6 +29,30 @@ public class PluginsConfigurationOptions { description = "Enables strict registration of plugins.") private boolean strictRegistration; + public void validate(final Logger logger, final CommandLine commandLine) { + this.checkDependencies(logger, commandLine); + } + + private void checkDependencies(final Logger logger, final CommandLine commandLine) { + CommandLineUtils.checkOptionDependencies( + logger, + commandLine, + DEFAULT_PLUGINS_STRICT_REGISTRATION_OPTION_NAME, + !strictRegistration, + List.of("--plugins")); + } + + @Override + public PluginConfiguration toDomainObject() { + return new PluginConfiguration( + plugins, strictRegistration, PluginConfiguration.defaultPluginsDir()); + } + + @Override + public List getCLIOptions() { + return CommandLineUtils.getCLIOptions(this, new PluginsConfigurationOptions()); + } + /** * Constructs a {@link PluginConfiguration} instance based on the command line options. * From 3659b1901e14112f30dbbedcf1676d5720108bc2 Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Fri, 8 Mar 2024 11:19:53 +1100 Subject: [PATCH 17/34] Create another constructor and simplify code Signed-off-by: Gabriel-Trintinalia --- .../stable/PluginsConfigurationOptions.java | 13 ++++----- .../core/plugins/PluginConfiguration.java | 27 +++++++++++++++++-- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java index e2c26229b9a..5f0e61905e2 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java @@ -17,8 +17,9 @@ public class PluginsConfigurationOptions implements CLIOptions { @CommandLine.Option( names = {DEFAULT_PLUGINS_OPTION_NAME}, - description = "Comma-separated list of plugins.", + description = "Comma-separated list of plugin names", split = ",", + hidden = true, converter = PluginInfoConverter.class, arity = "0..*") private List plugins; @@ -26,7 +27,9 @@ public class PluginsConfigurationOptions implements CLIOptions