From ef781424cffb969e88234873d61b2f307a21f430 Mon Sep 17 00:00:00 2001
From: Tomas Langer <tomas.langer@oracle.com>
Date: Tue, 17 Dec 2024 22:58:02 +0100
Subject: [PATCH] Fixed service supply. Now we try to use all matchin service
 descriptors even for a single instance, as any may return an empty optional
 (if appropriate supplier type), and we would not yield an instance, even when
 we should. As a result the method "explodeFilterAndSort" was going through
 all service manager and collected all instances - which resulted in errors if
 a provider with lower weight was not capable of creating an intance because
 of unsatisfied dependency.

---
 integrations/oci/tls-certificates/pom.xml     |  1 -
 .../OciCertificatesTlsManagerTest.java        |  2 +-
 .../service/registry/ServiceSupplies.java     | 35 ++++++++++++++-----
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/integrations/oci/tls-certificates/pom.xml b/integrations/oci/tls-certificates/pom.xml
index 6e584f9932a..4e8dd495662 100644
--- a/integrations/oci/tls-certificates/pom.xml
+++ b/integrations/oci/tls-certificates/pom.xml
@@ -182,7 +182,6 @@
                         <CA_OCID>SETME</CA_OCID>
                         <SERVER_CERT_OCID>SETME</SERVER_CERT_OCID>
                         <SERVER_KEY_OCID>SETME</SERVER_KEY_OCID>
-
                         <oci.real.usage>false</oci.real.usage>
                     </systemPropertyVariables>
                 </configuration>
diff --git a/integrations/oci/tls-certificates/src/test/java/io/helidon/integrations/oci/tls/certificates/OciCertificatesTlsManagerTest.java b/integrations/oci/tls-certificates/src/test/java/io/helidon/integrations/oci/tls/certificates/OciCertificatesTlsManagerTest.java
index 88ee911e7c7..7ca9df86b2e 100644
--- a/integrations/oci/tls-certificates/src/test/java/io/helidon/integrations/oci/tls/certificates/OciCertificatesTlsManagerTest.java
+++ b/integrations/oci/tls-certificates/src/test/java/io/helidon/integrations/oci/tls/certificates/OciCertificatesTlsManagerTest.java
@@ -74,7 +74,7 @@ void configIsMonitoredForChange() throws Exception {
         assertThat("sanity",
                    caCertDownloadCountBaseline0,
                    equalTo(0));
-        assertThat("santiy",
+        assertThat("sanity",
                    pkDownloadCountBaseLine0,
                    equalTo(0));
 
diff --git a/service/registry/src/main/java/io/helidon/service/registry/ServiceSupplies.java b/service/registry/src/main/java/io/helidon/service/registry/ServiceSupplies.java
index a34521f81c1..94e58e29724 100644
--- a/service/registry/src/main/java/io/helidon/service/registry/ServiceSupplies.java
+++ b/service/registry/src/main/java/io/helidon/service/registry/ServiceSupplies.java
@@ -33,6 +33,29 @@ final class ServiceSupplies {
     private ServiceSupplies() {
     }
 
+    private static <T> Optional<ServiceInstance<T>> oneInstance(Lookup lookup,
+                                                                List<ServiceManager<T>> serviceManagers) {
+        // we may have more than one service manager, as the first one(s) may not provide a value
+        traceLookup(lookup, "explode, filter, and sort");
+
+        for (ServiceManager<T> serviceManager : serviceManagers) {
+            Optional<ServiceInstance<T>> thisManager = serviceManager.activator()
+                    .instances(lookup)
+                    .stream()
+                    .flatMap(List::stream)
+                    .map(it -> serviceManager.registryInstance(lookup, it))
+                    .findFirst();
+
+            traceLookupInstance(lookup, serviceManager, thisManager.map(List::of).orElseGet(List::of));
+
+            if (thisManager.isPresent()) {
+                return thisManager;
+            }
+        }
+
+        return Optional.empty();
+    }
+
     private static <T> List<ServiceInstance<T>> explodeFilterAndSort(Lookup lookup,
                                                                      List<ServiceManager<T>> serviceManagers) {
         // this method is called when we resolve instances, so we can safely assume any scope is active
@@ -133,9 +156,7 @@ static class ServiceSupply<T> extends ServiceSupplyBase<T> implements Supplier<T
 
             Supplier<T> supplier;
 
-            supplier = () -> explodeFilterAndSort(lookup, managers)
-                    .stream()
-                    .findFirst()
+            supplier = () -> oneInstance(lookup, managers)
                     .map(ServiceInstance::get)
                     .orElseThrow(() -> new ServiceRegistryException(
                             "Neither of matching services could provide a value. Descriptors: " + managers + ", "
@@ -158,9 +179,7 @@ static class ServiceSupplyOptional<T> extends ServiceSupplyBase<T> implements Su
 
         @Override
         public Optional<T> get() {
-            Optional<ServiceInstance<T>> first = explodeFilterAndSort(super.lookup, super.managers)
-                    .stream()
-                    .findFirst();
+            Optional<ServiceInstance<T>> first = oneInstance(super.lookup, super.managers);
             return first.map(Supplier::get);
         }
     }
@@ -173,9 +192,7 @@ static class ServiceInstanceSupplyOptional<T> extends ServiceSupplyBase<T> imple
 
         @Override
         public Optional<ServiceInstance<T>> get() {
-            return explodeFilterAndSort(super.lookup, super.managers)
-                    .stream()
-                    .findFirst();
+            return oneInstance(super.lookup, super.managers);
         }
     }