microprofile-health-128 org.eclipse.microprofile.health.HealthCheckResponse leaks

fixes #128 with multiple providers support addition

Signed-off-by: Antoine Sabot-Durand <antoine@sabot-durand.net>

Author: antoinesd

api/pom.xml

@@ -37,6 +37,10 @@
             <artifactId>org.osgi.annotation.versioning</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>org.testng</groupId>
+            <artifactId>testng</artifactId>
+        </dependency>
     </dependencies>
 
     <build>
@@ -97,13 +101,28 @@
                  </configuration>
                 <executions>
                     <execution>
-                        <id>baseline</id> 
+                        <id>baseline</id>
                         <goals>
                             <goal>baseline</goal>
                         </goals>
                     </execution>
                 </executions>
             </plugin>
+            <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-surefire-plugin</artifactId>
+            <version>${maven-surefire-plugin.version}</version>
+            <executions>
+            <execution>
+                <id>default-test</id>
+                <configuration>
+                    <systemPropertyVariables>
+                        <serviceDir>${project.build.testOutputDirectory}/META-INF/services/</serviceDir>
+                    </systemPropertyVariables>
+                </configuration>
+            </execution>
+            </executions>
+            </plugin>
         </plugins>
     </build>
 </project>

api/src/main/java/org/eclipse/microprofile/health/HealthCheckResponse.java

@@ -24,19 +24,15 @@
 
 import org.eclipse.microprofile.health.spi.HealthCheckResponseProvider;
 
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.Map;
 import java.util.Optional;
-import java.util.ServiceLoader;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
  * The response to a health check invocation.
  * <p>
  * The {@link HealthCheckResponse} class is reserved for an extension by implementation providers.
- * An application should use one of the static methods to create a Response instance using a 
+ * An application should use one of the static methods to create a Response instance using a
  * {@link HealthCheckResponseBuilder}.
  * When used on the consuming end, The class can also be instantiated directly.
  * </p>
@@ -45,8 +41,6 @@ public class HealthCheckResponse {
 
     private static final Logger LOGGER = Logger.getLogger(HealthCheckResponse.class.getName());
 
-    private static volatile HealthCheckResponseProvider provider = null;
-
     private final String name;
 
     private final Status status;
@@ -78,9 +72,10 @@ public HealthCheckResponse() {
      * Used by OSGi environment where the service loader pattern is not supported.
      *
      * @param provider the provider instance to use.
+     * @deprecated use {{@link HealthCheckResponseProviderResolver#setProvider}} instead
      */
     public static void setResponseProvider(HealthCheckResponseProvider provider) {
-        HealthCheckResponse.provider = provider;
+        HealthCheckResponseProviderResolver.setProvider(provider);
     }
 
     /**
@@ -91,7 +86,7 @@ public static void setResponseProvider(HealthCheckResponseProvider provider) {
      */
     public static HealthCheckResponseBuilder named(String name) {
 
-        return getProvider().createResponseBuilder().name(name);
+        return HealthCheckResponseProviderResolver.getProvider().createResponseBuilder().name(name);
     }
 
     /**
@@ -102,12 +97,12 @@ public static HealthCheckResponseBuilder named(String name) {
      * @return a new, empty health check builder
      */
     public static HealthCheckResponseBuilder builder() {
-        return getProvider().createResponseBuilder();
+        return HealthCheckResponseProviderResolver.getProvider().createResponseBuilder();
     }
 
     /**
      * Creates a successful health check with a name.
-     * 
+     *
      * @param name the check name
      * @return a new sucessful health check response with a name
      */
@@ -117,33 +112,14 @@ public static HealthCheckResponse up(String name) {
 
     /**
      * Creates a failed health check with a name.
-     * 
+     *
      * @param name the check name
      * @return a new failed health check response with a name
      */
     public static HealthCheckResponse down(String name) {
         return HealthCheckResponse.named(name).down().build();
     }
 
-    private static HealthCheckResponseProvider getProvider() {
-        if (provider == null) {
-            synchronized (HealthCheckResponse.class) {
-                if (provider != null) {
-                    return provider;
-                }
-
-                HealthCheckResponseProvider newInstance = find(HealthCheckResponseProvider.class);
-
-                if (newInstance == null) {
-                    throw new IllegalStateException("No HealthCheckResponseProvider implementation found!");
-                }
-
-                provider = newInstance;
-            }
-        }
-        return provider;
-    }
-
     // the actual contract
 
     public enum Status {UP, DOWN}
@@ -160,61 +136,4 @@ public Optional<Map<String, Object>> getData() {
         return data;
     }
 
-    private static <T> T find(Class<T> service) {
-
-        T serviceInstance = find(service, HealthCheckResponse.getContextClassLoader());
-
-        // alternate classloader
-        if (null == serviceInstance) {
-            serviceInstance = find(service, HealthCheckResponse.class.getClassLoader());
-        }
-
-        // service cannot be found
-        if (null == serviceInstance) {
-            throw new IllegalStateException("Unable to find service " + service.getName());
-        }
-
-        return serviceInstance;
-    }
-
-    private static <T> T find(Class<T> service, ClassLoader cl) {
-
-        T serviceInstance = null;
-
-        try {
-            ServiceLoader<T> services = ServiceLoader.load(service, cl);
-
-            for (T spi : services) {
-                if (serviceInstance != null) {
-                    throw new IllegalStateException(
-                            "Multiple service implementations found: "
-                                    + spi.getClass().getName() + " and "
-                                    + serviceInstance.getClass().getName());
-                }
-                serviceInstance = spi;
-            }
-        }
-        catch (Throwable t) {
-            LOGGER.log(Level.SEVERE, "Error loading service " + service.getName() + ".", t);
-        }
-
-        return serviceInstance;
-    }
-
-
-    private static ClassLoader getContextClassLoader() {
-        return AccessController.doPrivileged((PrivilegedAction<ClassLoader>) () -> {
-            ClassLoader cl = null;
-            try {
-                cl = Thread.currentThread().getContextClassLoader();
-            }
-            catch (SecurityException ex) {
-                LOGGER.log(
-                        Level.WARNING,
-                        "Unable to get context classloader instance.",
-                        ex);
-            }
-            return cl;
-        });
-    }
 }

api/src/main/java/org/eclipse/microprofile/health/HealthCheckResponseProviderResolver.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2020 Contributors to the Eclipse Foundation
+ *
+ * See the NOTICES file(s) distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * You may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ */
+
+package org.eclipse.microprofile.health;
+
+import java.util.Collections;
+import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.function.Predicate;
+
+import org.eclipse.microprofile.health.spi.HealthCheckResponseProvider;
+
+
+/**
+ * This class an helper to retrieve the Microprofile HealthCheck implementation
+ * by using Java {@link ServiceLoader} api.
+ * It allows the usage of multiple implementations at the same time.
+ *
+ * @author Antoine Sabot-Durand
+ * @since 3.0
+ */
+public class HealthCheckResponseProviderResolver {
+
+    private static final Object LOCK = new Object();
+
+    private static volatile HealthCheckResponseProvider provider = null;
+
+    private static volatile Set<HealthCheckResponseProvider> discoveredProviders = null;
+
+    private static volatile Predicate<HealthCheckResponseProvider> providerPredicate = p -> true;
+
+    protected HealthCheckResponseProviderResolver() {
+    }
+
+    /**
+     * @return the selected {@link HealthCheckResponseProvider} in cache. If not available it tries to resolve it first
+     */
+    public static HealthCheckResponseProvider getProvider() {
+        if (provider == null) {
+            if (discoveredProviders == null) {
+                synchronized (LOCK) {
+                    if (discoveredProviders == null) {
+                        findAllProviders();
+                    }
+                }
+            }
+            provider = discoveredProviders.stream()
+                .filter(providerPredicate)
+                .findFirst().orElseThrow(() -> new IllegalStateException("No HealthCheckResponseProvider implementation found!"));
+        }
+        return provider;
+    }
+
+    /**
+     *
+     * Used to manually set the {@link HealthCheckResponseProvider}.
+     * Also used by OSGi environment where the service loader pattern is not supported.
+     *
+     * @param provider the provider instance to use.
+     */
+    public static void setProvider(HealthCheckResponseProvider provider) {
+        HealthCheckResponseProviderResolver.provider = provider;
+    }
+
+    /**
+     *
+     * Other way than {@link #setProvider} to set the {@link HealthCheckResponseProvider}
+     *
+     * @param providerPredicate a predicate to choose the matching provider
+     */
+    public static void setProviderPredicate(Predicate<HealthCheckResponseProvider> providerPredicate) {
+        HealthCheckResponseProviderResolver.providerPredicate = providerPredicate;
+        setProvider(null);
+    }
+
+    private static void findAllProviders() {
+        ServiceLoader<HealthCheckResponseProvider> providerLoader;
+        Set<HealthCheckResponseProvider> providers = new HashSet<>();
+
+        Class<HealthCheckResponseProvider> clazz = HealthCheckResponseProvider.class;
+        ClassLoader cl = SecurityActions.getContextClassLoader();
+        if (cl == null) {
+            cl = clazz.getClassLoader();
+        }
+        providerLoader = SecurityActions.loadService(HealthCheckResponseProvider.class, cl);
+
+        if (!providerLoader.iterator().hasNext()) {
+            throw new IllegalStateException("Unable to locate HealthCheckResponseProvider");
+        }
+
+        try {
+            providerLoader.forEach(providers::add);
+        }
+        catch (ServiceConfigurationError e) {
+            throw new IllegalStateException(e);
+        }
+        discoveredProviders = Collections.unmodifiableSet(providers);
+    }
+
+    protected static void reset() {
+        provider = null;
+        discoveredProviders =null;
+        providerPredicate = p -> true;
+
+
+    }
+
+}