test: avoid getFreePort collisions (#4366)

core/common/lib/util-lib/src/main/java/org/eclipse/edc/util/io/Ports.java

@@ -16,32 +16,32 @@
 
 import java.io.IOException;
 import java.net.ServerSocket;
+import java.util.HashSet;
 import java.util.Random;
-
-import static java.lang.String.format;
+import java.util.Set;
 
 /**
  * Utilities for assigning ports.
  */
 public final class Ports {
     public static final int MAX_TCP_PORT = 65_535;
     private static final Random RANDOM = new Random();
+    private static final Set<Integer> ALREADY_RETURNED = new HashSet<>();
 
     /**
-     * Gets a free port in the range 1024 - 65535 by trying them in ascending order.
+     * Gets a free port in the range 1024 - 65535 by trying them randomly.
      *
-     * @return the first free port
+     * @return the lower bound.
      * @throws IllegalArgumentException if no free port is available
      */
     public static int getFreePort() {
-        var rnd = 1024 + RANDOM.nextInt(MAX_TCP_PORT - 1024);
-        return getFreePort(rnd);
+        return getFreePort(1024);
     }
 
     /**
-     * Gets a free port in the range lowerBound - 65535 by trying them in ascending order.
+     * Gets a free port in the range lowerBound - 65535 by trying them randomly.
      *
-     * @return the first free port
+     * @return the lower bound.
      * @throws IllegalArgumentException if no free port is available
      */
     public static int getFreePort(int lowerBound) {
@@ -52,7 +52,7 @@ public static int getFreePort(int lowerBound) {
     }
 
     /**
-     * Gets a free port in the range lowerBound - upperBound by trying them in ascending order.
+     * Gets a free port in the range lowerBound - upperBound randomly. Will not return ports already returned.
      *
      * @return the first free port
      * @throws IllegalArgumentException if no free port is available or if the bounds are invalid.
@@ -65,25 +65,21 @@ public static int getFreePort(int lowerBound, int upperBound) {
         if (upperBound > MAX_TCP_PORT) {
             throw new IllegalArgumentException("Upper bound must be < " + MAX_TCP_PORT);
         }
-        var port = lowerBound;
-        boolean found = false;
 
-        while (!found && port <= upperBound) {
-            try (ServerSocket serverSocket = new ServerSocket(port)) {
-                serverSocket.setReuseAddress(true);
-                port = serverSocket.getLocalPort();
+        do {
+            var tryPort = lowerBound + RANDOM.nextInt(upperBound - lowerBound);
+            if (!ALREADY_RETURNED.contains(tryPort)) {
+                ALREADY_RETURNED.add(tryPort);
 
-                found = true;
-            } catch (IOException e) {
-                found = false;
-                port++;
-            }
-        }
+                try (var serverSocket = new ServerSocket(tryPort)) {
+                    serverSocket.setReuseAddress(true);
 
-        if (!found) {
-            throw new IllegalArgumentException(format("No free ports in the range [%d - %d]", lowerBound, upperBound));
-        }
-        return port;
+                    return tryPort;
+                } catch (IOException ignored) {
+                    // port already used by external service, try another one
+                }
+            }
+        } while (true);
     }
 
     private Ports() {

core/common/lib/util-lib/src/test/java/org/eclipse/edc/util/io/PortsTest.java

@@ -19,6 +19,7 @@
 import java.io.IOException;
 import java.net.ServerSocket;
 
+import static java.util.stream.IntStream.range;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -51,12 +52,23 @@ void getFreePort_withUpperAndLowerBound() {
         assertThatThrownBy(() -> Ports.getFreePort(5000, 0)).isInstanceOf(IllegalArgumentException.class);
     }
 
+    @Test
+    void getFreePortAvoidDuplicated() {
+        var lowerBound = 6000;
+        var portsToBeGenerated = 10;
+        var ports = range(0, portsToBeGenerated)
+                .mapToObj(i -> Ports.getFreePort(lowerBound, lowerBound + portsToBeGenerated))
+                .distinct().toList();
+
+        assertThat(ports.size()).isEqualTo(portsToBeGenerated);
+    }
+
     @Test
     void getFreePort_whenOccupied() throws IOException {
         var port = Ports.getFreePort(MIN_PORT);
 
         try (var socket = new ServerSocket(port)) {
-            assertThat(Ports.getFreePort(MIN_PORT)).describedAs("Next free port").isGreaterThan(socket.getLocalPort());
+            assertThat(Ports.getFreePort(MIN_PORT)).describedAs("Next free port").isNotEqualTo(socket.getLocalPort());
         }
     }
-}
+}