Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAYARA-3068 MP Healthcheck fails if no name is supplied #3330

Merged
merged 1 commit into from
Oct 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<>();

Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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));
}
}

}