Idiomatic Builder Pattern

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java

@@ -20,7 +20,7 @@
 import tech.pegasys.pantheon.cli.PantheonControllerBuilder;
 import tech.pegasys.pantheon.controller.KeyPairUtil;
 import tech.pegasys.pantheon.controller.PantheonController;
-import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration.Builder;
+import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -42,7 +42,6 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner {
   private final Map<String, Runner> pantheonRunners = new HashMap<>();
   private ExecutorService nodeExecutor = Executors.newCachedThreadPool();
 
-  @SuppressWarnings("rawtypes")
   @Override
   public void startNode(final PantheonNode node) {
     if (nodeExecutor == null || nodeExecutor.isShutdown()) {
@@ -53,35 +52,36 @@ public void startNode(final PantheonNode node) {
     final EthNetworkConfig ethNetworkConfig =
         node.ethNetworkConfig()
             .orElse(new EthNetworkConfig.Builder(mainnet()).setNetworkId(NETWORK_ID).build());
-    PantheonController<?> pantheonController;
+    final PantheonController<?> pantheonController;
     try {
       pantheonController =
-          builder.build(
-              new Builder().build(),
-              node.homeDirectory(),
-              ethNetworkConfig,
-              false,
-              node.getMiningParameters(),
-              node.isDevMode(),
-              KeyPairUtil.getDefaultKeyFile(node.homeDirectory()));
+          builder
+              .synchronizerConfiguration(new SynchronizerConfiguration.Builder().build())
+              .homePath(node.homeDirectory())
+              .ethNetworkConfig(ethNetworkConfig)
+              .syncWithOttoman(false)
+              .miningParameters(node.getMiningParameters())
+              .devMode(node.isDevMode())
+              .nodePrivateKeyFile(KeyPairUtil.getDefaultKeyFile(node.homeDirectory()))
+              .build();
     } catch (final IOException e) {
       throw new RuntimeException("Error building PantheonController", e);
     }
 
     final Runner runner =
         new RunnerBuilder()
-            .build(
-                Vertx.vertx(),
-                pantheonController,
-                true,
-                node.bootnodes(),
-                node.hostName(),
-                node.p2pPort(),
-                25,
-                node.jsonRpcConfiguration(),
-                node.webSocketConfiguration(),
-                node.homeDirectory(),
-                Collections.emptySet());
+            .vertx(Vertx.vertx())
+            .pantheonController(pantheonController)
+            .discovery(true)
+            .bootstrapPeers(node.bootnodes())
+            .discoveryHost(node.hostName())
+            .discoveryPort(node.p2pPort())
+            .maxPeers(25)
+            .jsonRpcConfiguration(node.jsonRpcConfiguration())
+            .webSocketConfiguration(node.webSocketConfiguration())
+            .dataDir(node.homeDirectory())
+            .bannedNodeIds(Collections.emptySet())
+            .build();
 
     nodeExecutor.submit(runner::execute);
 

pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java

@@ -67,18 +67,74 @@
 
 public class RunnerBuilder {
 
-  public Runner build(
-      final Vertx vertx,
-      final PantheonController<?> pantheonController,
-      final boolean discovery,
-      final Collection<?> bootstrapPeers,
-      final String discoveryHost,
-      final int listenPort,
-      final int maxPeers,
-      final JsonRpcConfiguration jsonRpcConfiguration,
-      final WebSocketConfiguration webSocketConfiguration,
-      final Path dataDir,
-      final Collection<String> bannedNodeIds) {
+  private Vertx vertx;
+  private PantheonController<?> pantheonController;
+  private boolean discovery;
+  private Collection<?> bootstrapPeers;
+  private String discoveryHost;
+  private int listenPort;
+  private int maxPeers;
+  private JsonRpcConfiguration jsonRpcConfiguration;
+  private WebSocketConfiguration webSocketConfiguration;
+  private Path dataDir;
+  private Collection<String> bannedNodeIds;
+
+  public RunnerBuilder vertx(final Vertx vertx) {
+    this.vertx = vertx;
+    return this;
+  }
+
+  public RunnerBuilder pantheonController(final PantheonController<?> pantheonController) {
+    this.pantheonController = pantheonController;
+    return this;
+  }
+
+  public RunnerBuilder discovery(final boolean discovery) {
+    this.discovery = discovery;
+    return this;
+  }
+
+  public RunnerBuilder bootstrapPeers(final Collection<?> bootstrapPeers) {
+    this.bootstrapPeers = bootstrapPeers;
+    return this;
+  }
+
+  public RunnerBuilder discoveryHost(final String discoveryHost) {
+    this.discoveryHost = discoveryHost;
+    return this;
+  }
+
+  public RunnerBuilder discoveryPort(final int listenPort) {
+    this.listenPort = listenPort;
+    return this;
+  }
+
+  public RunnerBuilder maxPeers(final int maxPeers) {
+    this.maxPeers = maxPeers;
+    return this;
+  }
+
+  public RunnerBuilder jsonRpcConfiguration(final JsonRpcConfiguration jsonRpcConfiguration) {
+    this.jsonRpcConfiguration = jsonRpcConfiguration;
+    return this;
+  }
+
+  public RunnerBuilder webSocketConfiguration(final WebSocketConfiguration webSocketConfiguration) {
+    this.webSocketConfiguration = webSocketConfiguration;
+    return this;
+  }
+
+  public RunnerBuilder dataDir(final Path dataDir) {
+    this.dataDir = dataDir;
+    return this;
+  }
+
+  public RunnerBuilder bannedNodeIds(final Collection<String> bannedNodeIds) {
+    this.bannedNodeIds = bannedNodeIds;
+    return this;
+  }
+
+  public Runner build() {
 
     Preconditions.checkNotNull(pantheonController);
 
@@ -187,7 +243,7 @@ public Runner build(
               filterManager);
 
       final SubscriptionManager subscriptionManager =
-          createSubscriptionManager(vertx, context.getBlockchain(), transactionPool);
+          createSubscriptionManager(vertx, transactionPool);
 
       createLogsSubscriptionService(
           context.getBlockchain(), context.getWorldStateArchive(), subscriptionManager);
@@ -251,7 +307,7 @@ private Map<String, JsonRpcMethod> jsonRpcMethods(
   }
 
   private SubscriptionManager createSubscriptionManager(
-      final Vertx vertx, final Blockchain blockchain, final TransactionPool transactionPool) {
+      final Vertx vertx, final TransactionPool transactionPool) {
     final SubscriptionManager subscriptionManager = new SubscriptionManager();
     final PendingTransactionSubscriptionService pendingTransactions =
         new PendingTransactionSubscriptionService(subscriptionManager);
@@ -261,7 +317,7 @@ private SubscriptionManager createSubscriptionManager(
     return subscriptionManager;
   }
 
-  private LogsSubscriptionService createLogsSubscriptionService(
+  private void createLogsSubscriptionService(
       final Blockchain blockchain,
       final WorldStateArchive worldStateArchive,
       final SubscriptionManager subscriptionManager) {
@@ -270,16 +326,14 @@ private LogsSubscriptionService createLogsSubscriptionService(
             subscriptionManager, new BlockchainQueries(blockchain, worldStateArchive));
 
     blockchain.observeBlockAdded(logsSubscriptionService);
-
-    return logsSubscriptionService;
   }
 
-  private SyncingSubscriptionService createSyncingSubscriptionService(
+  private void createSyncingSubscriptionService(
       final Synchronizer synchronizer, final SubscriptionManager subscriptionManager) {
-    return new SyncingSubscriptionService(subscriptionManager, synchronizer);
+    new SyncingSubscriptionService(subscriptionManager, synchronizer);
   }
 
-  private NewBlockHeadersSubscriptionService createNewBlockHeadersSubscriptionService(
+  private void createNewBlockHeadersSubscriptionService(
       final Blockchain blockchain,
       final WorldStateArchive worldStateArchive,
       final SubscriptionManager subscriptionManager) {
@@ -288,8 +342,6 @@ private NewBlockHeadersSubscriptionService createNewBlockHeadersSubscriptionServ
             subscriptionManager, new BlockchainQueries(blockchain, worldStateArchive));
 
     blockchain.observeBlockAdded(newBlockHeadersSubscriptionService);
-
-    return newBlockHeadersSubscriptionService;
   }
 
   private WebSocketService createWebsocketService(

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java

@@ -45,8 +45,8 @@
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
@@ -217,7 +217,7 @@ public static class RpcApisConversionException extends Exception {
     split = ",",
     arity = "1..*"
   )
-  private final Collection<String> bannedNodeIds = null;
+  private final Collection<String> bannedNodeIds = new ArrayList<>();
 
   // TODO: Re-enable as per NC-1057/NC-1681
   //  @Option(
@@ -447,14 +447,16 @@ public void run() {
 
   PantheonController<?> buildController() {
     try {
-      return controllerBuilder.build(
-          buildSyncConfig(syncMode),
-          dataDir,
-          ethNetworkConfig(),
-          syncWithOttoman,
-          new MiningParameters(coinbase, minTransactionGasPrice, extraData, isMiningEnabled),
-          isDevMode,
-          getNodePrivateKeyFile());
+      return controllerBuilder
+          .synchronizerConfiguration(buildSyncConfig(syncMode))
+          .homePath(dataDir)
+          .ethNetworkConfig(ethNetworkConfig())
+          .syncWithOttoman(syncWithOttoman)
+          .miningParameters(
+              new MiningParameters(coinbase, minTransactionGasPrice, extraData, isMiningEnabled))
+          .devMode(isDevMode)
+          .nodePrivateKeyFile(getNodePrivateKeyFile())
+          .build();
     } catch (final InvalidConfigurationException e) {
       throw new ExecutionException(new CommandLine(this), e.getMessage());
     } catch (final IOException e) {
@@ -504,20 +506,21 @@ private void synchronize(
 
     checkNotNull(runnerBuilder);
 
-    // BEWARE: Peer discovery boolean must be inverted as it's negated in the options !
     final Runner runner =
-        runnerBuilder.build(
-            Vertx.vertx(),
-            controller,
-            !noPeerDiscovery,
-            bootstrapNodes,
-            discoveryHostAndPort.getHost(),
-            discoveryHostAndPort.getPort(),
-            maxPeers,
-            jsonRpcConfiguration,
-            webSocketConfiguration,
-            dataDir,
-            bannedNodeIds == null ? Collections.emptySet() : bannedNodeIds);
+        runnerBuilder
+            .vertx(Vertx.vertx())
+            .pantheonController(controller)
+            // BEWARE: Peer discovery boolean must be inverted as it's negated in the options !
+            .discovery(!noPeerDiscovery)
+            .bootstrapPeers(bootstrapNodes)
+            .discoveryHost(discoveryHostAndPort.getHost())
+            .discoveryPort(discoveryHostAndPort.getPort())
+            .maxPeers(maxPeers)
+            .jsonRpcConfiguration(jsonRpcConfiguration)
+            .webSocketConfiguration(webSocketConfiguration)
+            .dataDir(dataDir)
+            .bannedNodeIds(bannedNodeIds)
+            .build();
 
     addShutdownHook(runner);
     runner.execute();
@@ -530,7 +533,7 @@ private void addShutdownHook(final Runner runner) {
                 () -> {
                   try {
                     runner.close();
-                  } catch (Exception e) {
+                  } catch (final Exception e) {
                     throw new RuntimeException(e);
                   }
                 }));
@@ -634,7 +637,7 @@ private EthNetworkConfig updateNetworkConfig(final EthNetworkConfig ethNetworkCo
   private String genesisConfig() {
     try {
       return Resources.toString(genesisFile.toURI().toURL(), UTF_8);
-    } catch (IOException e) {
+    } catch (final IOException e) {
       throw new ParameterException(
           new CommandLine(this), String.format("Unable to load genesis file %s.", genesisFile), e);
     }

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonControllerBuilder.java

@@ -31,22 +31,58 @@
 
 public class PantheonControllerBuilder {
 
-  public PantheonController<?> build(
-      final SynchronizerConfiguration synchronizerConfiguration,
-      final Path homePath,
-      final EthNetworkConfig ethNetworkConfig,
-      final boolean syncWithOttoman,
-      final MiningParameters miningParameters,
-      final boolean isDevMode,
-      final File nodePrivateKeyFile)
-      throws IOException {
+  private SynchronizerConfiguration synchronizerConfiguration;
+  private Path homePath;
+  private EthNetworkConfig ethNetworkConfig;
+  private boolean syncWithOttoman;
+  private MiningParameters miningParameters;
+  private boolean devMode;
+  private File nodePrivateKeyFile;
+
+  public PantheonControllerBuilder synchronizerConfiguration(
+      final SynchronizerConfiguration synchronizerConfiguration) {
+    this.synchronizerConfiguration = synchronizerConfiguration;
+    return this;
+  }
+
+  public PantheonControllerBuilder homePath(final Path homePath) {
+    this.homePath = homePath;
+    return this;
+  }
+
+  public PantheonControllerBuilder ethNetworkConfig(final EthNetworkConfig ethNetworkConfig) {
+    this.ethNetworkConfig = ethNetworkConfig;
+    return this;
+  }
+
+  public PantheonControllerBuilder syncWithOttoman(final boolean syncWithOttoman) {
+    this.syncWithOttoman = syncWithOttoman;
+    return this;
+  }
+
+  public PantheonControllerBuilder miningParameters(final MiningParameters miningParameters) {
+    this.miningParameters = miningParameters;
+    return this;
+  }
+
+  public PantheonControllerBuilder devMode(final boolean devMode) {
+    this.devMode = devMode;
+    return this;
+  }
+
+  public PantheonControllerBuilder nodePrivateKeyFile(final File nodePrivateKeyFile) {
+    this.nodePrivateKeyFile = nodePrivateKeyFile;
+    return this;
+  }
+
+  public PantheonController<?> build() throws IOException {
     // instantiate a controller with mainnet config if no genesis file is defined
     // otherwise use the indicated genesis file
     final KeyPair nodeKeys = loadKeyPair(nodePrivateKeyFile);
 
     final StorageProvider storageProvider =
         RocksDbStorageProvider.create(homePath.resolve(DATABASE_PATH));
-    if (isDevMode) {
+    if (devMode) {
       final GenesisConfigFile genesisConfig = GenesisConfigFile.development();
       return MainnetPantheonController.init(
           storageProvider,