diff --git a/implementation/src/main/java/io/smallrye/health/SmallRyeHealthReporter.java b/implementation/src/main/java/io/smallrye/health/SmallRyeHealthReporter.java index 9d71863e..72810de3 100644 --- a/implementation/src/main/java/io/smallrye/health/SmallRyeHealthReporter.java +++ b/implementation/src/main/java/io/smallrye/health/SmallRyeHealthReporter.java @@ -24,7 +24,6 @@ import jakarta.enterprise.inject.Instance; import jakarta.enterprise.inject.spi.Bean; import jakarta.enterprise.inject.spi.BeanManager; -import jakarta.enterprise.inject.spi.CDI; import jakarta.inject.Inject; import jakarta.json.JsonArray; import jakarta.json.JsonArrayBuilder; @@ -409,14 +408,12 @@ private List getHealthChecksWithoutHealthGroup(Class checkClass) { } private Uni getHealthAsync(Uni cachedHealth, HealthType... types) { - if (!checksInitialized) { - initChecks(); - } - if (contextPropagated) { - recreateCheckUnis(); - return computeHealth(types); + return computeHealthWithContext(types); } else { + if (!checksInitialized) { + initChecks(); + } if (additionalListsChanged(types) || additionalListsChanged || cachedHealth == null) { additionalListsChanged = false; cachedHealth = computeHealth(types); @@ -426,14 +423,29 @@ private Uni getHealthAsync(Uni cachedHealth, Hea } } - private void recreateCheckUnis() { - livenessUnis.clear(); - readinessUnis.clear(); - wellnessUnis.clear(); - startupUnis.clear(); - checksInitialized = false; - - initChecks(); + private Uni computeHealthWithContext(HealthType... types) { + List> checks = new ArrayList<>(); + for (HealthType type : types) { + switch (type) { + case LIVENESS: + initUnis(checks, livenessChecks, asyncLivenessChecks); + checks.addAll(livenessHealthRegistry.getChecks(healthChecksConfigs)); + break; + case READINESS: + initUnis(checks, readinessChecks, asyncReadinessChecks); + checks.addAll(readinessHealthRegistry.getChecks(healthChecksConfigs)); + break; + case WELLNESS: + initUnis(checks, wellnessChecks, asyncWellnessChecks); + checks.addAll(wellnessHealthRegistry.getChecks(healthChecksConfigs)); + break; + case STARTUP: + initUnis(checks, startupChecks, asyncStartupChecks); + checks.addAll(startupHealthRegistry.getChecks(healthChecksConfigs)); + break; + } + } + return getHealthAsync(checks); } private boolean additionalListsChanged(HealthType... types) { @@ -467,7 +479,6 @@ private boolean additionalListsChanged(HealthType... types) { private Uni computeHealth(HealthType[] types) { List> checks = new ArrayList<>(); - for (HealthType type : types) { switch (type) { case LIVENESS: @@ -488,7 +499,6 @@ private Uni computeHealth(HealthType[] types) { break; } } - return getHealthAsync(checks); } @@ -508,7 +518,7 @@ private Uni getHealthAsync(Collection> } return Uni.combine().all().unis(healthCheckUnis) - .combinedWith(responses -> { + .with(responses -> { JsonArrayBuilder results = jsonProvider.createArrayBuilder(); HealthCheckResponse.Status status = HealthCheckResponse.Status.UP; diff --git a/implementation/src/main/java/io/smallrye/health/registry/HealthRegistryImpl.java b/implementation/src/main/java/io/smallrye/health/registry/HealthRegistryImpl.java index 66dfedfa..50e93a76 100644 --- a/implementation/src/main/java/io/smallrye/health/registry/HealthRegistryImpl.java +++ b/implementation/src/main/java/io/smallrye/health/registry/HealthRegistryImpl.java @@ -1,10 +1,10 @@ package io.smallrye.health.registry; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.eclipse.microprofile.health.HealthCheck; import org.eclipse.microprofile.health.HealthCheckResponse; @@ -16,23 +16,24 @@ public class HealthRegistryImpl implements HealthRegistry { - Map> checks = new HashMap<>(); + Map checks = new HashMap<>(); + Map asyncChecks = new HashMap<>(); private boolean checksChanged = false; AsyncHealthCheckFactory asyncHealthCheckFactory = new AsyncHealthCheckFactory(); @Override public HealthRegistry register(String id, HealthCheck healthCheck) { - return register(id, asyncHealthCheckFactory.callSync(healthCheck)); + return register(id, healthCheck, checks); } @Override public HealthRegistry register(String id, AsyncHealthCheck asyncHealthCheck) { - return register(id, asyncHealthCheckFactory.callAsync(asyncHealthCheck)); + return register(id, asyncHealthCheck, asyncChecks); } - private HealthRegistry register(String id, Uni healthCheckResponseUni) { - checks.put(id, healthCheckResponseUni); + private HealthRegistry register(String id, T check, Map checks) { + checks.put(id, check); checksChanged = true; return this; } @@ -40,7 +41,7 @@ private HealthRegistry register(String id, Uni healthCheckR @Override public HealthRegistry remove(String id) { try { - if (checks.remove(id) == null) { + if (checks.remove(id) == null && asyncChecks.remove(id) == null) { throw new IllegalStateException(String.format("ID '%s' not found", id)); } checksChanged = true; @@ -51,9 +52,13 @@ public HealthRegistry remove(String id) { } public Collection> getChecks(Map healthChecksConfigs) { - return Collections.unmodifiableCollection(checks.entrySet().stream() + var enabledChecks = checks.entrySet().stream() .filter(e -> healthChecksConfigs.getOrDefault(e.getKey(), true)) - .map(Map.Entry::getValue).collect(Collectors.toList())); + .map(e -> asyncHealthCheckFactory.callSync(e.getValue())); + var enabledAsyncChecks = asyncChecks.entrySet().stream() + .filter(e -> healthChecksConfigs.getOrDefault(e.getKey(), true)) + .map(e -> asyncHealthCheckFactory.callAsync(e.getValue())); + return Stream.concat(enabledChecks, enabledAsyncChecks).collect(Collectors.toList()); } public boolean checksChanged() { diff --git a/testsuite/experimental/pom.xml b/testsuite/experimental/pom.xml index 84d00e69..4e020778 100644 --- a/testsuite/experimental/pom.xml +++ b/testsuite/experimental/pom.xml @@ -32,6 +32,12 @@ compile + + jakarta.servlet + jakarta.servlet-api + provided + + diff --git a/testsuite/experimental/src/test/java/io/smallrye/health/deployment/ContextHealthCheck.java b/testsuite/experimental/src/test/java/io/smallrye/health/deployment/ContextHealthCheck.java new file mode 100644 index 00000000..e2081e84 --- /dev/null +++ b/testsuite/experimental/src/test/java/io/smallrye/health/deployment/ContextHealthCheck.java @@ -0,0 +1,23 @@ +package io.smallrye.health.deployment; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; + +import org.eclipse.microprofile.health.HealthCheck; +import org.eclipse.microprofile.health.HealthCheckResponse; + +import io.smallrye.health.api.Wellness; + +@Wellness +@ApplicationScoped +public class ContextHealthCheck implements HealthCheck { + + @Inject + ContextInfo context; + + @Override + public HealthCheckResponse call() { + return HealthCheckResponse.up(context.getValue()); + } + +} diff --git a/testsuite/experimental/src/test/java/io/smallrye/health/deployment/ContextInfo.java b/testsuite/experimental/src/test/java/io/smallrye/health/deployment/ContextInfo.java new file mode 100644 index 00000000..706cb9db --- /dev/null +++ b/testsuite/experimental/src/test/java/io/smallrye/health/deployment/ContextInfo.java @@ -0,0 +1,17 @@ +package io.smallrye.health.deployment; + +import jakarta.enterprise.context.RequestScoped; + +@RequestScoped +public class ContextInfo { + + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } +} diff --git a/testsuite/experimental/src/test/java/io/smallrye/health/deployment/RequestFilter.java b/testsuite/experimental/src/test/java/io/smallrye/health/deployment/RequestFilter.java new file mode 100644 index 00000000..af0031a2 --- /dev/null +++ b/testsuite/experimental/src/test/java/io/smallrye/health/deployment/RequestFilter.java @@ -0,0 +1,29 @@ +package io.smallrye.health.deployment; + +import java.io.IOException; + +import jakarta.inject.Inject; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.annotation.WebFilter; +import jakarta.servlet.http.HttpFilter; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +@WebFilter(urlPatterns = "*", filterName = "ContextInfoFilter") +public class RequestFilter extends HttpFilter { + + public static final String TEST_CONTEXT_HEADER = "X-CONTEXT"; + + @Inject + ContextInfo contextInfo; + + @Override + protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) + throws IOException, ServletException { + var value = req.getHeader(TEST_CONTEXT_HEADER); + contextInfo.setValue(value); + super.doFilter(req, res, chain); + } + +} diff --git a/testsuite/experimental/src/test/java/io/smallrye/health/test/ContextPropagatedHealthTest.java b/testsuite/experimental/src/test/java/io/smallrye/health/test/ContextPropagatedHealthTest.java new file mode 100644 index 00000000..c917d2b4 --- /dev/null +++ b/testsuite/experimental/src/test/java/io/smallrye/health/test/ContextPropagatedHealthTest.java @@ -0,0 +1,51 @@ +package io.smallrye.health.test; + +import java.util.List; +import java.util.Map; + +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.container.test.api.RunAsClient; +import org.jboss.shrinkwrap.api.Archive; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.testng.Assert; +import org.testng.annotations.Test; + +import io.smallrye.health.deployment.ContextHealthCheck; +import io.smallrye.health.deployment.ContextInfo; +import io.smallrye.health.deployment.RequestFilter; +import io.smallrye.mutiny.Uni; +import io.smallrye.mutiny.infrastructure.Infrastructure; + +@RunAsClient +class ContextPropagatedHealthTest extends TCKBase { + + @Deployment + public static Archive getDeployment() { + return DeploymentUtils + .createWarFileWithClasses(ContextPropagatedHealthTest.class.getSimpleName(), + ContextHealthCheck.class, ContextInfo.class, RequestFilter.class, TCKBase.class) + .addAsManifestResource( + new StringAsset("io.smallrye.health.context.propagation=true"), + "microprofile-config.properties"); + } + + // got to repeat this test a couple of times to trigger potential race condition issue at least once. + @Test(invocationCount = 20) + void twoParallelRequests_shouldNotMixupContext() { + String contextValue = "test-value"; + String otherContextValue = "other-test-value"; + Uni response = Uni.createFrom().voidItem().emitOn(Infrastructure.getDefaultExecutor()).onItem() + .transform(v -> getUrlHealthContents(Map.of(RequestFilter.TEST_CONTEXT_HEADER, contextValue))); + Uni otherResponse = Uni.createFrom().voidItem().emitOn(Infrastructure.getDefaultExecutor()).onItem() + .transform(v -> getUrlHealthContents(Map.of(RequestFilter.TEST_CONTEXT_HEADER, otherContextValue))); + + List responses = Uni.join().all(response, otherResponse).andCollectFailures().await().indefinitely(); + + String responseBody = responses.get(0).getBody().orElseThrow(); + String otherResponseBody = responses.get(1).getBody().orElseThrow(); + Assert.assertNotEquals(responseBody, otherResponseBody); + Assert.assertTrue(responseBody.contains(contextValue), responseBody); + Assert.assertTrue(otherResponseBody.contains(otherContextValue), otherResponseBody); + } +} diff --git a/testsuite/experimental/src/test/java/io/smallrye/health/test/TCKBase.java b/testsuite/experimental/src/test/java/io/smallrye/health/test/TCKBase.java index 0e17b9b3..a551f716 100644 --- a/testsuite/experimental/src/test/java/io/smallrye/health/test/TCKBase.java +++ b/testsuite/experimental/src/test/java/io/smallrye/health/test/TCKBase.java @@ -27,6 +27,7 @@ import java.io.StringReader; import java.lang.reflect.Method; import java.net.URI; +import java.util.Map; import java.util.Optional; import java.util.logging.Logger; @@ -65,6 +66,10 @@ public void beforeMethod(Method method) { LOG.info(String.format("Running test: %s#%s", method.getDeclaringClass().getSimpleName(), method.getName())); } + Response getUrlHealthContents(Map headers) { + return getUrlContents(this.uri + "/health", false, true, headers); + } + Response getUrlHealthContents() { return getUrlContents(this.uri + "/health", false); } @@ -98,6 +103,10 @@ private Response getUrlContents(String theUrl, boolean useAuth) { } private Response getUrlContents(String theUrl, boolean useAuth, boolean followRedirects) { + return getUrlContents(theUrl, useAuth, followRedirects, Map.of()); + } + + private Response getUrlContents(String theUrl, boolean useAuth, boolean followRedirects, Map headers) { StringBuilder content = new StringBuilder(); int code; @@ -117,8 +126,9 @@ private Response getUrlContents(String theUrl, boolean useAuth, boolean followRe } HttpClient client = builder.build(); - - HttpResponse response = client.execute(new HttpGet(theUrl)); + var httpGet = new HttpGet(theUrl); + headers.forEach(httpGet::addHeader); + HttpResponse response = client.execute(httpGet); code = response.getStatusLine().getStatusCode(); if (response.getEntity() != null) {