Merge pull request #3330 from jGauravGupta/PAYARA-3068

PAYARA-3068 MP Healthcheck fails if no name is supplied
payara · Oct 29, 2018 · 137f88a · 137f88a
2 parents f4b2f2b + 52440ff
commit 137f88a
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 15 deletions.
diff --git a/...le/healthcheck/src/main/java/fish/payara/microprofile/healthcheck/HealthCheckService.java b/...le/healthcheck/src/main/java/fish/payara/microprofile/healthcheck/HealthCheckService.java
@@ -47,7 +47,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
@@ -57,6 +56,7 @@
 import javax.servlet.http.HttpServletResponse;
 
 import fish.payara.microprofile.healthcheck.config.MetricsHealthCheckConfiguration;
+import static java.util.logging.Level.WARNING;
 import org.eclipse.microprofile.health.HealthCheck;
 import org.eclipse.microprofile.health.HealthCheckResponse;
 import org.glassfish.api.StartupRunLevel;
@@ -90,6 +90,8 @@ public class HealthCheckService implements EventListener, ConfigListener {
     @Inject
     MetricsHealthCheckConfiguration configuration;
 
+    private static final Logger LOG = Logger.getLogger(HealthCheckService.class.getName());
+
     private final Map<String, Set<HealthCheck>> healthChecks = new ConcurrentHashMap<>();
     private final Map<String, ClassLoader> applicationClassLoaders = new ConcurrentHashMap<>();
 
@@ -182,8 +184,7 @@ public void performHealthChecks(HttpServletResponse response) throws IOException
                         Thread.currentThread().setContextClassLoader(originalClassLoader);
                     }
                 } catch (Exception ex) {
-                    Logger.getLogger(HealthCheckService.class.getName()).log(Level.WARNING,
-                            "Exception executing HealthCheck: " + healthCheck.getClass().getCanonicalName(), ex);
+                    LOG.log(WARNING, "Exception executing HealthCheck : " + healthCheck.getClass().getCanonicalName(), ex);
                     // If there's any issue, set the response to an error
                     response.setStatus(500);
                 }

diff --git a/...in/java/fish/payara/microprofile/healthcheck/response/HealthCheckResponseBuilderImpl.java b/...in/java/fish/payara/microprofile/healthcheck/response/HealthCheckResponseBuilderImpl.java
@@ -45,6 +45,7 @@
 import org.eclipse.microprofile.health.HealthCheckResponse;
 import org.eclipse.microprofile.health.HealthCheckResponse.State;
 import org.eclipse.microprofile.health.HealthCheckResponseBuilder;
+import static org.glassfish.common.util.StringHelper.isEmpty;
 
 /**
  * Base Implementation of HealthCheckResponseBuilder.
@@ -54,12 +55,12 @@ public class HealthCheckResponseBuilderImpl extends HealthCheckResponseBuilder {
 
     private String name;
     private State state;
-    private Optional<Map<String, Object>> data = Optional.of(new HashMap<>());
+    private final Optional<Map<String, Object>> data = Optional.of(new HashMap<>());
 
     @Override
     public HealthCheckResponseBuilder name(String name) {
         // If the provided string isn't empty or null, set it as the name, otherwise throw an exception.
-        if (!stringEmptyOrNull(name)) {
+        if (!isEmpty(name)) {
             this.name = name;
             return this;
         } else {
@@ -70,7 +71,7 @@ public HealthCheckResponseBuilder name(String name) {
     @Override
     public HealthCheckResponseBuilder withData(String key, String value) {
         // If the provided string isn't empty or null, enter it into the Map, otherwise throw an exception.
-        if (!stringEmptyOrNull(name)) {
+        if (!isEmpty(key)) {
             data.get().put(key, value);
             return this;
         } else {
@@ -81,7 +82,7 @@ public HealthCheckResponseBuilder withData(String key, String value) {
     @Override
     public HealthCheckResponseBuilder withData(String key, long value) {
         // If the provided string isn't empty or null, enter it into the Map, otherwise throw an exception.
-        if (!stringEmptyOrNull(name)) {
+        if (!isEmpty(key)) {
             data.get().put(key, value);
             return this;
         } else {
@@ -92,7 +93,7 @@ public HealthCheckResponseBuilder withData(String key, long value) {
     @Override
     public HealthCheckResponseBuilder withData(String key, boolean value) {
         // If the provided string isn't empty or null, enter it into the Map, otherwise throw an exception.
-        if (!stringEmptyOrNull(name)) {
+        if (!isEmpty(key)) {
             data.get().put(key, value);
             return this;
         } else {
@@ -125,18 +126,19 @@ public HealthCheckResponseBuilder state(boolean up) {
 
     @Override
     public HealthCheckResponse build() {
+         validate();
         // Just use the basic HealthCheckResponse implementation
         HealthCheckResponse healthCheckResponse = new HealthCheckResponseImpl(name, state, data);
         return healthCheckResponse;
     }
 
-    /**
-     * Helper method that checks whether or not the provided String is empty or null
-     * @param string The String to evaluate
-     * @return True if the String is empty or null
-     */
-    private boolean stringEmptyOrNull(String string) {
-        return (string == null || string.isEmpty());
+    private void validate() {
+        if (isEmpty(name)) {
+            throw new IllegalArgumentException("Healthcheck name is not defined");
+        }
+        if (state == null) {
+            throw new IllegalArgumentException(String.format("Healthcheck [%s] state is not defined", name));
+        }
     }
 
 }