From c4ef564825c50b6a60f5f7e43ccb9ce01eab6c0d Mon Sep 17 00:00:00 2001
From: Abdelhamid Bakhta <45264458+abdelhamidbakhta@users.noreply.github.com>
Date: Mon, 7 Dec 2020 18:29:55 +0100
Subject: [PATCH] Accept to use default port values if not in use. (#1673)

* Accept to use default port values if not in use.

Signed-off-by: Abdelhamid Bakhta <abdelhamid.bakhta@consensys.net>
---
 .../org/hyperledger/besu/cli/BesuCommand.java | 46 ++++++++++++++-----
 .../hyperledger/besu/cli/BesuCommandTest.java | 30 +++++++++++-
 2 files changed, 63 insertions(+), 13 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 766b40d6eb..22cb5118c7 100644
--- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
+++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -2366,21 +2366,12 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   }
 
   private void checkPortClash() {
-    // List of port parameters
-    final List<Integer> ports =
-        asList(
-            p2pPort,
-            graphQLHttpPort,
-            rpcHttpPort,
-            rpcWsPort,
-            metricsPort,
-            metricsPushPort,
-            stratumPort);
-    ports.stream()
+    getEffectivePorts().stream()
         .filter(Objects::nonNull)
+        .filter(port -> port > 0)
         .forEach(
             port -> {
-              if (port != 0 && !allocatedPorts.add(port)) {
+              if (!allocatedPorts.add(port)) {
                 throw new ParameterException(
                     commandLine,
                     "Port number '"
@@ -2390,6 +2381,37 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
             });
   }
 
+  /**
+   * * Gets the list of effective ports (ports that are enabled).
+   *
+   * @return The list of effective ports
+   */
+  private List<Integer> getEffectivePorts() {
+    final List<Integer> effectivePorts = new ArrayList<>();
+    addPortIfEnabled(effectivePorts, p2pPort, p2pEnabled);
+    addPortIfEnabled(effectivePorts, graphQLHttpPort, isGraphQLHttpEnabled);
+    addPortIfEnabled(effectivePorts, rpcHttpPort, isRpcHttpEnabled);
+    addPortIfEnabled(effectivePorts, rpcWsPort, isRpcWsEnabled);
+    addPortIfEnabled(effectivePorts, metricsPort, isMetricsEnabled);
+    addPortIfEnabled(effectivePorts, metricsPushPort, isMetricsPushEnabled);
+    addPortIfEnabled(effectivePorts, stratumPort, iStratumMiningEnabled);
+    return effectivePorts;
+  }
+
+  /**
+   * Adds port in the passed list only if enabled.
+   *
+   * @param ports The list of ports
+   * @param port The port value
+   * @param enabled true if enabled, false otherwise
+   */
+  private void addPortIfEnabled(
+      final List<Integer> ports, final Integer port, final boolean enabled) {
+    if (enabled) {
+      ports.add(port);
+    }
+  }
+
   private void checkGoQuorumCompatibilityConfig(final EthNetworkConfig ethNetworkConfig) {
     if (isGoQuorumCompatibilityMode) {
       final GenesisConfigOptions genesisConfigOptions = readGenesisConfigOptions();
diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
index 4562b4253b..ba02f6c9f4 100644
--- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
+++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -3853,7 +3853,12 @@ public class BesuCommandTest extends CommandTestAbstract {
 
   @Test
   public void assertThatDuplicatePortSpecifiedFails() {
-    parseCommand("--p2p-port=9", "--rpc-http-port=10", "--rpc-ws-port=10");
+    parseCommand(
+        "--p2p-port=9",
+        "--rpc-http-enabled",
+        "--rpc-http-port=10",
+        "--rpc-ws-port=10",
+        "--rpc-ws-enabled");
     assertThat(commandErrorOutput.toString())
         .contains("Port number '10' has been specified multiple times.");
   }
@@ -3999,4 +4004,27 @@ public class BesuCommandTest extends CommandTestAbstract {
     assertThat(commandErrorOutput.toString())
         .contains("GoQuorum compatibility mode (enabled) cannot be used on Mainnet");
   }
+
+  @Test
+  public void assertThatCheckPortClashAcceptsAsExpected() throws Exception {
+    // use WS port for HTTP
+    final int port = 8546;
+    parseCommand("--rpc-http-enabled", "--rpc-http-port", String.valueOf(port));
+    verify(mockRunnerBuilder).jsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
+    verify(mockRunnerBuilder).build();
+    assertThat(jsonRpcConfigArgumentCaptor.getValue().getPort()).isEqualTo(port);
+    assertThat(commandOutput.toString()).isEmpty();
+    assertThat(commandErrorOutput.toString()).isEmpty();
+  }
+
+  @Test
+  public void assertThatCheckPortClashRejectsAsExpected() throws Exception {
+    // use WS port for HTTP
+    final int port = 8546;
+    parseCommand("--rpc-http-enabled", "--rpc-http-port", String.valueOf(port), "--rpc-ws-enabled");
+    assertThat(commandOutput.toString()).isEmpty();
+    assertThat(commandErrorOutput.toString())
+        .contains(
+            "Port number '8546' has been specified multiple times. Please review the supplied configuration.");
+  }
 }