Dagger plugin context (#7465)

* Eliminates dependency from Controller on BesuComponent --------- Signed-off-by: Justin Florentine <justin+github@florentine.us> Co-authored-by: garyschulte <garyschulte@gmail.com>
3 months ago · 563afdb3ec
parent 6d33164742
commit 563afdb3ec
8 changed files with 67 additions and 66 deletions
--- 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
@ -93,7 +93,6 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.inject.Inject;
 import javax.inject.Named;
@ -449,7 +448,6 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner {
    }

    @Provides
-    @Named("besuPluginContext")
    public BesuPluginContextImpl providePluginContext(
        final StorageServiceImpl storageService,
        final SecurityModuleServiceImpl securityModuleService,
@ -549,17 +547,17 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner {
  static class MockBesuCommandModule {

    @Provides
-    BesuCommand provideBesuCommand(final AcceptanceTestBesuComponent component) {
+    BesuCommand provideBesuCommand(final BesuPluginContextImpl pluginContext) {
      final BesuCommand besuCommand =
          new BesuCommand(
-              component,
              RlpBlockImporter::new,
              JsonBlockImporter::new,
              RlpBlockExporter::new,
              new RunnerBuilder(),
              new BesuController.Builder(),
-              Optional.ofNullable(component.getBesuPluginContext()).orElse(null),
-              System.getenv());
+              pluginContext,
+              System.getenv(),
+              LoggerFactory.getLogger(MockBesuCommandModule.class));
      besuCommand.toCommandLine();
      return besuCommand;
    }
--- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
+++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@ -84,7 +84,6 @@ import org.hyperledger.besu.cli.util.BesuCommandCustomFactory;
 import org.hyperledger.besu.cli.util.CommandLineUtils;
 import org.hyperledger.besu.cli.util.ConfigDefaultValueProviderStrategy;
 import org.hyperledger.besu.cli.util.VersionProvider;
-import org.hyperledger.besu.components.BesuComponent;
 import org.hyperledger.besu.config.CheckpointConfigOptions;
 import org.hyperledger.besu.config.GenesisConfigFile;
 import org.hyperledger.besu.config.GenesisConfigOptions;
@ -865,13 +864,9 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
  private BesuController besuController;
  private BesuConfigurationImpl pluginCommonConfiguration;

-  private BesuComponent besuComponent;
  private final Supplier<ObservableMetricsSystem> metricsSystem =
-      Suppliers.memoize(
-          () ->
-              besuComponent == null || besuComponent.getObservableMetricsSystem() == null
-                  ? MetricsSystemFactory.create(metricsConfiguration())
-                  : besuComponent.getObservableMetricsSystem());
+      Suppliers.memoize(() -> MetricsSystemFactory.create(metricsConfiguration()));
+
  private Vertx vertx;
  private EnodeDnsConfiguration enodeDnsConfiguration;
  private KeyValueStorageProvider keyValueStorageProvider;
@ -879,7 +874,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
  /**
   * Besu command constructor.
   *
-   * @param besuComponent BesuComponent which acts as our application context
   * @param rlpBlockImporter RlpBlockImporter supplier
   * @param jsonBlockImporterFactory instance of {@code Function<BesuController, JsonBlockImporter>}
   * @param rlpBlockExporterFactory instance of {@code Function<Blockchain, RlpBlockExporter>}
@ -887,18 +881,18 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   * @param controllerBuilder instance of BesuController.Builder
   * @param besuPluginContext instance of BesuPluginContextImpl
   * @param environment Environment variables map
+   * @param commandLogger instance of Logger for outputting to the CLI
   */
  public BesuCommand(
-      final BesuComponent besuComponent,
      final Supplier<RlpBlockImporter> rlpBlockImporter,
      final Function<BesuController, JsonBlockImporter> jsonBlockImporterFactory,
      final Function<Blockchain, RlpBlockExporter> rlpBlockExporterFactory,
      final RunnerBuilder runnerBuilder,
      final BesuController.Builder controllerBuilder,
      final BesuPluginContextImpl besuPluginContext,
-      final Map<String, String> environment) {
+      final Map<String, String> environment,
+      final Logger commandLogger) {
    this(
-        besuComponent,
        rlpBlockImporter,
        jsonBlockImporterFactory,
        rlpBlockExporterFactory,
@ -914,13 +908,13 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
        new TransactionSelectionServiceImpl(),
        new TransactionPoolValidatorServiceImpl(),
        new TransactionSimulationServiceImpl(),
-        new BlockchainServiceImpl());
+        new BlockchainServiceImpl(),
+        commandLogger);
  }

  /**
   * Overloaded Besu command constructor visible for testing.
   *
-   * @param besuComponent BesuComponent which acts as our application context
   * @param rlpBlockImporter RlpBlockImporter supplier
   * @param jsonBlockImporterFactory instance of {@code Function<BesuController, JsonBlockImporter>}
   * @param rlpBlockExporterFactory instance of {@code Function<Blockchain, RlpBlockExporter>}
@ -937,10 +931,10 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   * @param transactionValidatorServiceImpl instance of TransactionValidatorServiceImpl
   * @param transactionSimulationServiceImpl instance of TransactionSimulationServiceImpl
   * @param blockchainServiceImpl instance of BlockchainServiceImpl
+   * @param commandLogger instance of Logger for outputting to the CLI
   */
  @VisibleForTesting
  protected BesuCommand(
-      final BesuComponent besuComponent,
      final Supplier<RlpBlockImporter> rlpBlockImporter,
      final Function<BesuController, JsonBlockImporter> jsonBlockImporterFactory,
      final Function<Blockchain, RlpBlockExporter> rlpBlockExporterFactory,
@ -956,9 +950,10 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
      final TransactionSelectionServiceImpl transactionSelectionServiceImpl,
      final TransactionPoolValidatorServiceImpl transactionValidatorServiceImpl,
      final TransactionSimulationServiceImpl transactionSimulationServiceImpl,
-      final BlockchainServiceImpl blockchainServiceImpl) {
-    this.besuComponent = besuComponent;
-    this.logger = besuComponent.getBesuCommandLogger();
+      final BlockchainServiceImpl blockchainServiceImpl,
+      final Logger commandLogger) {
+
+    this.logger = commandLogger;
    this.rlpBlockImporter = rlpBlockImporter;
    this.rlpBlockExporterFactory = rlpBlockExporterFactory;
    this.jsonBlockImporterFactory = jsonBlockImporterFactory;
@ -970,8 +965,13 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
    this.securityModuleService = securityModuleService;
    this.permissioningService = permissioningService;
    this.privacyPluginService = privacyPluginService;
-    this.pluginCommonConfiguration = new BesuConfigurationImpl();
-    besuPluginContext.addService(BesuConfiguration.class, pluginCommonConfiguration);
+    if (besuPluginContext.getService(BesuConfigurationImpl.class).isPresent()) {
+      this.pluginCommonConfiguration =
+          besuPluginContext.getService(BesuConfigurationImpl.class).get();
+    } else {
+      this.pluginCommonConfiguration = new BesuConfigurationImpl();
+      besuPluginContext.addService(BesuConfiguration.class, this.pluginCommonConfiguration);
+    }
    this.rpcEndpointServiceImpl = rpcEndpointServiceImpl;
    this.transactionSelectionServiceImpl = transactionSelectionServiceImpl;
    this.transactionValidatorServiceImpl = transactionValidatorServiceImpl;
@ -1821,9 +1821,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   */
  public BesuController buildController() {
    try {
-      return this.besuComponent == null
-          ? getControllerBuilder().build()
-          : getControllerBuilder().besuComponent(this.besuComponent).build();
+      return setupControllerBuilder().build();
    } catch (final Exception e) {
      throw new ExecutionException(this.commandLine, e.getMessage(), e);
    }
@ -1834,7 +1832,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   *
   * @return instance of BesuControllerBuilder
   */
-  public BesuControllerBuilder getControllerBuilder() {
+  public BesuControllerBuilder setupControllerBuilder() {
    pluginCommonConfiguration
        .init(dataDir(), dataDir().resolve(DATABASE_PATH), getDataStorageConfiguration())
        .withMiningParameters(miningParametersSupplier.get())
@ -2800,7 +2798,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
    builder.setTxPoolImplementation(buildTransactionPoolConfiguration().getTxPoolImplementation());
    builder.setWorldStateUpdateMode(unstableEvmOptions.toDomainObject().worldUpdaterMode());

-    builder.setPluginContext(besuComponent.getBesuPluginContext());
+    builder.setPluginContext(this.besuPluginContext);

    return builder.build();
  }
--- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/blocks/BlocksSubCommand.java
+++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/blocks/BlocksSubCommand.java
@ -254,7 +254,7 @@ public class BlocksSubCommand implements Runnable {
        // Set some defaults
        return parentCommand
            .parentCommand
-            .getControllerBuilder()
+            .setupControllerBuilder()
            // set to mainnet genesis block so validation rules won't reject it.
            .clock(Clock.fixed(Instant.ofEpochSecond(startTime), ZoneOffset.UTC))
            .miningParameters(getMiningParameters())
@ -374,7 +374,7 @@ public class BlocksSubCommand implements Runnable {
    private BesuController createBesuController() {
      return parentCommand
          .parentCommand
-          .getControllerBuilder()
+          .setupControllerBuilder()
          .miningParameters(MiningParameters.newDefault())
          .build();
    }
--- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java
+++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/storage/TrieLogSubCommand.java
@ -87,7 +87,7 @@ public class TrieLogSubCommand implements Runnable {
    // disable limit trie logs to avoid preloading during subcommand execution
    return parentCommand
        .besuCommand
-        .getControllerBuilder()
+        .setupControllerBuilder()
        .dataStorageConfiguration(
            ImmutableDataStorageConfiguration.copyOf(config).withBonsaiLimitTrieLogsEnabled(false))
        .build();
--- a/besu/src/main/java/org/hyperledger/besu/components/BesuCommandModule.java
+++ b/besu/src/main/java/org/hyperledger/besu/components/BesuCommandModule.java
@ -22,8 +22,8 @@ import org.hyperledger.besu.chainimport.RlpBlockImporter;
 import org.hyperledger.besu.cli.BesuCommand;
 import org.hyperledger.besu.controller.BesuController;
 import org.hyperledger.besu.metrics.prometheus.MetricsConfiguration;
+import org.hyperledger.besu.services.BesuPluginContextImpl;

-import java.util.Optional;
 import javax.inject.Named;
 import javax.inject.Singleton;

@ -42,17 +42,19 @@ public class BesuCommandModule {

  @Provides
  @Singleton
-  BesuCommand provideBesuCommand(final BesuComponent besuComponent) {
+  BesuCommand provideBesuCommand(
+      final BesuPluginContextImpl pluginContext,
+      final @Named("besuCommandLogger") Logger commandLogger) {
    final BesuCommand besuCommand =
        new BesuCommand(
-            besuComponent,
            RlpBlockImporter::new,
            JsonBlockImporter::new,
            RlpBlockExporter::new,
            new RunnerBuilder(),
            new BesuController.Builder(),
-            Optional.ofNullable(besuComponent.getBesuPluginContext()).orElse(null),
-            System.getenv());
+            pluginContext,
+            System.getenv(),
+            commandLogger);
    besuCommand.toCommandLine();
    return besuCommand;
  }
--- a/besu/src/main/java/org/hyperledger/besu/components/BesuComponent.java
+++ b/besu/src/main/java/org/hyperledger/besu/components/BesuComponent.java
@ -75,7 +75,6 @@ public interface BesuComponent {
   *
   * @return BesuPluginContextImpl
   */
-  @Named("besuPluginContext")
  BesuPluginContextImpl getBesuPluginContext();

  /**
--- a/besu/src/main/java/org/hyperledger/besu/components/BesuPluginContextModule.java
+++ b/besu/src/main/java/org/hyperledger/besu/components/BesuPluginContextModule.java
@ -14,9 +14,10 @@
 */
 package org.hyperledger.besu.components;

+import org.hyperledger.besu.plugin.services.BesuConfiguration;
+import org.hyperledger.besu.services.BesuConfigurationImpl;
 import org.hyperledger.besu.services.BesuPluginContextImpl;

-import javax.inject.Named;
 import javax.inject.Singleton;

 import dagger.Module;
@ -29,15 +30,23 @@ public class BesuPluginContextModule {
  /** Default constructor. */
  public BesuPluginContextModule() {}

+  @Provides
+  @Singleton
+  BesuConfigurationImpl provideBesuPluginConfig() {
+    return new BesuConfigurationImpl();
+  }
+
  /**
   * Creates a BesuPluginContextImpl, used for plugin service discovery.
   *
+   * @param pluginConfig the BesuConfigurationImpl
   * @return the BesuPluginContext
   */
  @Provides
-  @Named("besuPluginContext")
  @Singleton
-  public BesuPluginContextImpl provideBesuPluginContext() {
-    return new BesuPluginContextImpl();
+  public BesuPluginContextImpl provideBesuPluginContext(final BesuConfigurationImpl pluginConfig) {
+    BesuPluginContextImpl retval = new BesuPluginContextImpl();
+    retval.addService(BesuConfiguration.class, pluginConfig);
+    return retval;
  }
 }
--- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
+++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
@ -229,9 +229,6 @@ public abstract class CommandTestAbstract {
  @Mock
  protected Logger mockLogger;

-  @Mock(lenient = true)
-  protected BesuComponent mockBesuComponent;
-
  @Captor protected ArgumentCaptor<Collection<Bytes>> bytesCollectionCollector;
  @Captor protected ArgumentCaptor<Path> pathArgumentCaptor;
  @Captor protected ArgumentCaptor<String> stringArgumentCaptor;
@ -384,8 +381,6 @@ public abstract class CommandTestAbstract {
    lenient()
        .when(mockBesuPluginContext.getService(TransactionSelectionService.class))
        .thenReturn(Optional.of(txSelectionService));
-
-    when(mockBesuComponent.getBesuCommandLogger()).thenReturn(mockLogger);
  }

  @BeforeEach
@ -470,7 +465,6 @@ public abstract class CommandTestAbstract {
    switch (testType) {
      case REQUIRED_OPTION:
        return new TestBesuCommandWithRequiredOption(
-            mockBesuComponent,
            () -> rlpBlockImporter,
            this::jsonBlockImporterFactory,
            (blockchain) -> rlpBlockExporter,
@ -480,10 +474,10 @@ public abstract class CommandTestAbstract {
            environment,
            storageService,
            securityModuleService,
-            privacyPluginService);
+            privacyPluginService,
+            mockLogger);
      case PORT_CHECK:
        return new TestBesuCommand(
-            mockBesuComponent,
            () -> rlpBlockImporter,
            this::jsonBlockImporterFactory,
            (blockchain) -> rlpBlockExporter,
@ -493,10 +487,10 @@ public abstract class CommandTestAbstract {
            environment,
            storageService,
            securityModuleService,
-            privacyPluginService);
+            privacyPluginService,
+            mockLogger);
      default:
        return new TestBesuCommandWithoutPortCheck(
-            mockBesuComponent,
            () -> rlpBlockImporter,
            this::jsonBlockImporterFactory,
            (blockchain) -> rlpBlockExporter,
@ -506,7 +500,8 @@ public abstract class CommandTestAbstract {
            environment,
            storageService,
            securityModuleService,
-            privacyPluginService);
+            privacyPluginService,
+            mockLogger);
    }
  }

@ -536,7 +531,6 @@ public abstract class CommandTestAbstract {
    private Vertx vertx;

    TestBesuCommand(
-        final BesuComponent besuComponent,
        final Supplier<RlpBlockImporter> mockBlockImporter,
        final Function<BesuController, JsonBlockImporter> jsonBlockImporterFactory,
        final Function<Blockchain, RlpBlockExporter> rlpBlockExporterFactory,
@ -546,9 +540,9 @@ public abstract class CommandTestAbstract {
        final Map<String, String> environment,
        final StorageServiceImpl storageService,
        final SecurityModuleServiceImpl securityModuleService,
-        final PrivacyPluginServiceImpl privacyPluginService) {
+        final PrivacyPluginServiceImpl privacyPluginService,
+        final Logger commandLogger) {
      super(
-          besuComponent,
          mockBlockImporter,
          jsonBlockImporterFactory,
          rlpBlockExporterFactory,
@ -564,7 +558,8 @@ public abstract class CommandTestAbstract {
          new TransactionSelectionServiceImpl(),
          new TransactionPoolValidatorServiceImpl(),
          new TransactionSimulationServiceImpl(),
-          new BlockchainServiceImpl());
+          new BlockchainServiceImpl(),
+          commandLogger);
    }

    @Override
@ -635,7 +630,6 @@ public abstract class CommandTestAbstract {
    private final Boolean acceptTermsAndConditions = false;

    TestBesuCommandWithRequiredOption(
-        final BesuComponent besuComponent,
        final Supplier<RlpBlockImporter> mockBlockImporter,
        final Function<BesuController, JsonBlockImporter> jsonBlockImporterFactory,
        final Function<Blockchain, RlpBlockExporter> rlpBlockExporterFactory,
@ -645,9 +639,9 @@ public abstract class CommandTestAbstract {
        final Map<String, String> environment,
        final StorageServiceImpl storageService,
        final SecurityModuleServiceImpl securityModuleService,
-        final PrivacyPluginServiceImpl privacyPluginService) {
+        final PrivacyPluginServiceImpl privacyPluginService,
+        final Logger commandLogger) {
      super(
-          besuComponent,
          mockBlockImporter,
          jsonBlockImporterFactory,
          rlpBlockExporterFactory,
@ -657,7 +651,8 @@ public abstract class CommandTestAbstract {
          environment,
          storageService,
          securityModuleService,
-          privacyPluginService);
+          privacyPluginService,
+          commandLogger);
    }

    public Boolean getAcceptTermsAndConditions() {
@ -669,7 +664,6 @@ public abstract class CommandTestAbstract {
  public static class TestBesuCommandWithoutPortCheck extends TestBesuCommand {

    TestBesuCommandWithoutPortCheck(
-        final BesuComponent context,
        final Supplier<RlpBlockImporter> mockBlockImporter,
        final Function<BesuController, JsonBlockImporter> jsonBlockImporterFactory,
        final Function<Blockchain, RlpBlockExporter> rlpBlockExporterFactory,
@ -679,9 +673,9 @@ public abstract class CommandTestAbstract {
        final Map<String, String> environment,
        final StorageServiceImpl storageService,
        final SecurityModuleServiceImpl securityModuleService,
-        final PrivacyPluginServiceImpl privacyPluginService) {
+        final PrivacyPluginServiceImpl privacyPluginService,
+        final Logger commandLogger) {
      super(
-          context,
          mockBlockImporter,
          jsonBlockImporterFactory,
          rlpBlockExporterFactory,
@ -691,7 +685,8 @@ public abstract class CommandTestAbstract {
          environment,
          storageService,
          securityModuleService,
-          privacyPluginService);
+          privacyPluginService,
+          commandLogger);
    }

    @Override