Repository: smallrye/smallrye-stork
Pull request #606 (2023-06-27 11:14:07)

Allow to start without any address configured in static-list service
Now it's possible to register services addresses using static list so it's useful to allow Stork Static service discovery to start without any address in the list.


Patch:
diff --git a/service-discovery/kubernetes/src/main/resources/META-INF/services/io.smallrye.stork.spi.ServiceDiscoveryProvider b/service-discovery/kubernetes/src/main/resources/META-INF/services/io.smallrye.stork.spi.ServiceDiscoveryProvider
deleted file mode 100644
index c9c0ffb1..00000000
--- a/service-discovery/kubernetes/src/main/resources/META-INF/services/io.smallrye.stork.spi.ServiceDiscoveryProvider
+++ /dev/null
@@ -1,1 +0,0 @@
-io.smallrye.stork.servicediscovery.kubernetes.KubernetesServiceDiscoveryProvider
\ No newline at end of file
diff --git a/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscovery.java b/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscovery.java
index 52f7de8c..4d015e14 100644
--- a/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscovery.java
+++ b/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscovery.java
@@ -34,19 +34,22 @@ public final class StaticListServiceDiscovery implements ServiceDiscovery {
     @Override
     public Uni<List<ServiceInstance>> getServiceInstances() {
         List<String> addresses = StaticAddressesBackend.getAddresses(serviceName);
-        for (String address : addresses) {
-            try {
-                HostAndPort hostAndPort = StorkAddressUtils.parseToHostAndPort(address, 80, "service");
-                DefaultServiceInstance serviceInstance = new DefaultServiceInstance(ServiceInstanceIds.next(), hostAndPort.host,
-                        hostAndPort.port,
-                        hostAndPort.path, false);
-                if (!instances.contains(serviceInstance)) {
-                    instances
-                            .add(serviceInstance);
+        if (addresses != null && !addresses.isEmpty()) {
+            for (String address : addresses) {
+                try {
+                    HostAndPort hostAndPort = StorkAddressUtils.parseToHostAndPort(address, 80, "service");
+                    DefaultServiceInstance serviceInstance = new DefaultServiceInstance(ServiceInstanceIds.next(),
+                            hostAndPort.host,
+                            hostAndPort.port,
+                            hostAndPort.path, false);
+                    if (!instances.contains(serviceInstance)) {
+                        instances
+                                .add(serviceInstance);
+                    }
+                } catch (Exception e) {
+                    throw new IllegalArgumentException(
+                            "Address not parseable to URL: " + address + " for service " + serviceName);
                 }
-            } catch (Exception e) {
-                throw new IllegalArgumentException(
-                        "Address not parseable to URL: " + address + " for service " + serviceName);
             }
         }
 
diff --git a/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProvider.java b/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProvider.java
index b9118a5c..a3852b9a 100644
--- a/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProvider.java
+++ b/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProvider.java
@@ -33,26 +33,25 @@ public class StaticListServiceDiscoveryProvider
     public ServiceDiscovery createServiceDiscovery(StaticConfiguration config, String serviceName,
             ServiceConfig serviceConfig, StorkInfrastructure storkInfrastructure) {
         String addresses = config.getAddressList();
-        if (addresses == null || addresses.isBlank()) {
-            throw new IllegalArgumentException("No address list defined for service " + serviceName);
-        }
         List<DefaultServiceInstance> addressList = new ArrayList<>();
-        for (String address : addresses.split(",")) {
-            address = address.trim();
-            try {
-                HostAndPort hostAndPort = StorkAddressUtils.parseToHostAndPort(address, 80, "service '" + serviceName + "'");
-                addressList
-                        .add(new DefaultServiceInstance(ServiceInstanceIds.next(), hostAndPort.host, hostAndPort.port,
-                                hostAndPort.path, isSecure(config.getSecure(), hostAndPort.port)));
-                StaticAddressesBackend.add(serviceName, address);
-            } catch (Exception e) {
-                throw new IllegalArgumentException(
-                        "Address not parseable to URL: " + address + " for service " + serviceName);
+        if (addresses != null && !addresses.isBlank()) {
+            for (String address : addresses.split(",")) {
+                address = address.trim();
+                try {
+                    HostAndPort hostAndPort = StorkAddressUtils.parseToHostAndPort(address, 80,
+                            "service '" + serviceName + "'");
+                    addressList
+                            .add(new DefaultServiceInstance(ServiceInstanceIds.next(), hostAndPort.host, hostAndPort.port,
+                                    hostAndPort.path, isSecure(config.getSecure(), hostAndPort.port)));
+                    StaticAddressesBackend.add(serviceName, address);
+                } catch (Exception e) {
+                    throw new IllegalArgumentException(
+                            "Address not parseable to URL: " + address + " for service " + serviceName);
+                }
+            }
+            if (Boolean.parseBoolean(config.getShuffle())) {
+                Collections.shuffle(addressList);
             }
-        }
-
-        if (Boolean.parseBoolean(config.getShuffle())) {
-            Collections.shuffle(addressList);
         }
 
         return new StaticListServiceDiscovery(serviceName, addressList);
diff --git a/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceRegistrar.java b/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceRegistrar.java
index 8bdb3fa5..914dc240 100644
--- a/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceRegistrar.java
+++ b/service-discovery/static-list/src/main/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceRegistrar.java
@@ -1,7 +1,6 @@
 package io.smallrye.stork.servicediscovery.staticlist;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -38,27 +37,24 @@ public class StaticListServiceRegistrar implements ServiceRegistrar<Metadata.Def
 
     public static final class StaticAddressesBackend {
 
-        private static Map<String, List<String>> backend;
+        private static Map<String, List<String>> backend = new HashMap<>();
 
         public static List<String> getAddresses(String serviceName) {
-            if (backend == null) {
-                backend = new HashMap<>();
-                backend.put(serviceName, Collections.emptyList());
-            }
             return backend.get(serviceName);
         }
 
         public static void add(String serviceName, String address) {
-            if (backend == null) {
-                backend = new HashMap<>();
-                backend.put(serviceName, new ArrayList<>());
-            } else if (backend.get(serviceName) == null) {
-                List<String> adresses = new ArrayList<>();
-                adresses.add(address);
-                backend.put(serviceName, adresses);
-            } else if (!backend.get(serviceName).contains(address)) {
-                backend.get(serviceName).add(address);
-
+            if (serviceName == null || serviceName.length() == 0) {
+                throw new IllegalArgumentException("No service name provided for address " + address);
+            }
+            if (backend.get(serviceName) != null) {
+                if (!backend.get(serviceName).contains(address)) {
+                    backend.get(serviceName).add(address);
+                }
+            } else {
+                List<String> addresses = new ArrayList<>();
+                addresses.add(address);
+                backend.put(serviceName, addresses);
             }
         }
 
@@ -67,6 +63,10 @@ public class StaticListServiceRegistrar implements ServiceRegistrar<Metadata.Def
                 backend.remove(serviceName);
             }
         }
+
+        public static void clearAll() {
+            backend.clear();
+        }
     }
 
 }


Test patch:
diff --git a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticAddressesBackendTest.java b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticAddressesBackendTest.java
new file mode 100644
index 00000000..5c556b30
--- /dev/null
+++ b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticAddressesBackendTest.java
@@ -0,0 +1,64 @@
+package io.smallrye.stork.servicediscovery.staticlist;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import io.smallrye.stork.servicediscovery.staticlist.StaticListServiceRegistrar.StaticAddressesBackend;
+
+public class StaticAddressesBackendTest {
+
+    @BeforeEach
+    public void clearAddresses() {
+        StaticAddressesBackend.clearAll();
+    }
+
+    @Test
+    public void shouldNotAddAddressAlreadyPresent() {
+        StaticAddressesBackend.add("my-service", "localhost:8080");
+        StaticAddressesBackend.add("my-service", "localhost:8080");
+
+        List<String> addresses = StaticAddressesBackend.getAddresses("my-service");
+
+        assertThat(addresses).isNotEmpty();
+        assertThat(addresses.size()).isEqualTo(1);
+        assertThat(addresses).contains("localhost:8080");
+
+    }
+
+    @Test
+    public void shouldAddAFewAddress() {
+        StaticAddressesBackend.add("my-service", "localhost:8080");
+        StaticAddressesBackend.add("my-service", "localhost:8081");
+
+        List<String> addresses = StaticAddressesBackend.getAddresses("my-service");
+
+        assertThat(addresses).isNotEmpty();
+        assertThat(addresses.size()).isEqualTo(2);
+        assertThat(addresses).containsExactlyInAnyOrder("localhost:8080", "localhost:8081");
+
+        StaticAddressesBackend.clear("my-service");
+        addresses = StaticAddressesBackend.getAddresses("my-service");
+        assertThat(addresses).isNull();
+    }
+
+    @Test
+    public void shouldAddAFewAddressThenClearList() {
+
+        Exception exception = assertThrows(IllegalArgumentException.class, () -> {
+            StaticAddressesBackend.add("", "localhost:8080");
+        });
+
+        String expectedMessage = "No service name provided for address localhost:8080";
+        String actualMessage = exception.getMessage();
+
+        assertTrue(actualMessage.contains(expectedMessage));
+
+    }
+
+}
diff --git a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryCDITest.java b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryCDITest.java
index 3d7b41a3..f8274f10 100644
--- a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryCDITest.java
+++ b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryCDITest.java
@@ -46,6 +46,8 @@ public class StaticListServiceDiscoveryCDITest {
                 null, Map.of("address-list", "localhost:8083/foo, localhost:8083/bar"));
         config.addServiceConfig("secured-service", null, "static",
                 null, Map.of("address-list", "localhost:443, localhost"));
+        config.addServiceConfig("empty-list-service", null, "static", null,
+                null, Map.of("address-list", ""), null);
 
         this.stork = StorkTestUtils.getNewStorkInstance();
 
@@ -65,6 +67,14 @@ public class StaticListServiceDiscoveryCDITest {
         assertThat(instances.get(0).isSecure()).isTrue();
     }
 
+    @Test
+    void testEmptyDetection() {
+        List<ServiceInstance> instances = stork.getService("empty-list-service").getInstances().await()
+                .atMost(Duration.ofSeconds(5));
+
+        assertThat(instances).hasSize(0);
+    }
+
     @Test
     void shouldGetAllServiceInstances() {
         List<ServiceInstance> serviceInstances = stork.getService("first-service")
diff --git a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiCDITest.java b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiCDITest.java
index 45314576..7f0744a8 100644
--- a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiCDITest.java
+++ b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiCDITest.java
@@ -52,7 +52,17 @@ public class StaticListServiceDiscoveryProgrammaticApiCDITest {
                         new StaticConfiguration().withAddressList("localhost:443, localhost")))
                 .defineIfAbsent("shuffle-service", ServiceDefinition.of(
                         new StaticConfiguration()
-                                .withAddressList("localhost:8080, localhost:8081").withShuffle("true")));
+                                .withAddressList("localhost:8080, localhost:8081").withShuffle("true")))
+                .defineIfAbsent("empty-list-service", ServiceDefinition.of(new StaticConfiguration().withAddressList(null)));
+        ;
+    }
+
+    @Test
+    void testEmptyDetection() {
+        List<ServiceInstance> instances = stork.getService("empty-list-service").getInstances().await()
+                .atMost(Duration.ofSeconds(5));
+
+        assertThat(instances).hasSize(0);
     }
 
     @Test
diff --git a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiTest.java b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiTest.java
index b718e063..0fc11874 100644
--- a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiTest.java
+++ b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryProgrammaticApiTest.java
@@ -38,7 +38,16 @@ public class StaticListServiceDiscoveryProgrammaticApiTest {
                         new StaticConfiguration().withAddressList("localhost:443, localhost")))
                 .defineIfAbsent("shuffle-service", ServiceDefinition.of(
                         new StaticConfiguration()
-                                .withAddressList("localhost:8080, localhost:8081").withShuffle("true")));
+                                .withAddressList("localhost:8080, localhost:8081").withShuffle("true")))
+                .defineIfAbsent("empty-list-service", ServiceDefinition.of(new StaticConfiguration().withAddressList(null)));
+    }
+
+    @Test
+    void testEmptyDetection() {
+        List<ServiceInstance> instances = stork.getService("empty-list-service").getInstances().await()
+                .atMost(Duration.ofSeconds(5));
+
+        assertThat(instances).hasSize(0);
     }
 
     @Test
@@ -61,14 +70,6 @@ public class StaticListServiceDiscoveryProgrammaticApiTest {
                 .getInstances()
                 .await().atMost(Duration.ofSeconds(5));
 
-        for (ServiceInstance si : serviceInstances) {
-            System.out.println("----- instances: \n");
-            System.out.println("----- si host: " + si.getHost());
-            System.out.println("----- si port: " + si.getPort());
-            System.out.println("----- si id: " + si.getId());
-            System.out.println("----- si path: " + si.getPath());
-        }
-
         assertThat(serviceInstances).hasSize(2);
 
         List<String> list = serviceInstances.stream().map(si -> si.getHost() + ":" + si.getPort()).collect(Collectors.toList());
diff --git a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryTest.java b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryTest.java
index 34fa0115..0b0ee5c4 100644
--- a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryTest.java
+++ b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticListServiceDiscoveryTest.java
@@ -38,6 +38,9 @@ public class StaticListServiceDiscoveryTest {
         TestConfigProvider.addServiceConfig("scheme-service", null, "static", null,
                 null, Map.of("address-list", "http://localhost:8080, https://localhost:8081"), null);
 
+        TestConfigProvider.addServiceConfig("empty-list-service", null, "static", null,
+                null, Map.of("address-list", ""), null);
+
         stork = StorkTestUtils.getNewStorkInstance();
     }
 
@@ -81,6 +84,14 @@ public class StaticListServiceDiscoveryTest {
         assertThat(serviceInstances.stream().map(ServiceInstance::isSecure)).allSatisfy(b -> assertThat(b).isFalse());
     }
 
+    @Test
+    void testEmptyDetection() {
+        List<ServiceInstance> instances = stork.getService("empty-list-service").getInstances().await()
+                .atMost(Duration.ofSeconds(5));
+
+        assertThat(instances).hasSize(0);
+    }
+
     @Test
     void shouldFailOnMissingService() {
         assertThatThrownBy(() -> stork.getService("missing")).isInstanceOf(NoSuchServiceDefinitionException.class);
diff --git a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticServiceRegistrationTest.java b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticServiceRegistrationTest.java
index 26c1390a..29a1c735 100644
--- a/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticServiceRegistrationTest.java
+++ b/service-discovery/static-list/src/test/java/io/smallrye/stork/servicediscovery/staticlist/StaticServiceRegistrationTest.java
@@ -25,11 +25,8 @@ public class StaticServiceRegistrationTest {
     @BeforeEach
     void setUp() {
         TestConfigProvider.clear();
-        TestConfigProvider.addServiceConfig("first-service", null, "static", "static",
-                null, Map.of("address-list", "localhost:8080, localhost:8081"),
-                Map.of("address-list", "localhost:8080, localhost:8081"));
+        StaticAddressesBackend.clearAll();
 
-        stork = StorkTestUtils.getNewStorkInstance();
     }
 
     @AfterEach
@@ -39,6 +36,11 @@ public class StaticServiceRegistrationTest {
 
     @Test
     void shouldGetAllServiceInstances() {
+        TestConfigProvider.addServiceConfig("first-service", null, "static", "static",
+                null, Map.of("address-list", "localhost:8080, localhost:8081"), null);
+
+        stork = StorkTestUtils.getNewStorkInstance();
+
         String serviceName = "first-service";
         List<ServiceInstance> serviceInstances = stork.getService(serviceName)
                 .getInstances()
@@ -65,4 +67,28 @@ public class StaticServiceRegistrationTest {
         assertThat(serviceInstances.stream().map(ServiceInstance::getPort)).containsExactlyInAnyOrder(8080,
                 8081, 9090);
     }
+
+    @Test
+    void shouldGetAllServiceInstancesWithoutConfig() {
+        TestConfigProvider.addServiceConfig("first-service", null, "static", "static",
+                null, null, null);
+
+        stork = StorkTestUtils.getNewStorkInstance();
+
+        String serviceName = "first-service";
+
+        ServiceRegistrar<Metadata.DefaultMetadataKey> staticRegistrar = stork.getService(serviceName).getServiceRegistrar();
+
+        staticRegistrar.registerServiceInstance(serviceName, "localhost", 8080);
+
+        List<ServiceInstance> serviceInstances = stork.getService(serviceName)
+                .getInstances()
+                .await().atMost(Duration.ofSeconds(5));
+
+        assertThat(serviceInstances).hasSize(1);
+        assertThat(serviceInstances.stream().map(ServiceInstance::getHost)).contains("localhost");
+        assertThat(serviceInstances.stream().map(ServiceInstance::getPort)).contains(8080);
+        assertThat(serviceInstances.stream().map(ServiceInstance::isSecure)).allSatisfy(b -> assertThat(b).isFalse());
+
+    }
 }