From a9aed66803a55d89c48a1024ea933b885faa3df8 Mon Sep 17 00:00:00 2001 From: nscuro Date: Wed, 29 Mar 2023 21:13:47 +0200 Subject: [PATCH 1/3] Implement health endpoint based on MicroProfile Health Signed-off-by: nscuro --- alpine-server/pom.xml | 9 ++ .../health/HealthCheckResponseBuilder.java | 94 +++++++++++ .../health/HealthCheckResponseProvider.java | 34 ++++ .../alpine/server/health/HealthCheckType.java | 53 +++++++ .../health/checks/DatabaseHealthCheck.java | 101 ++++++++++++ .../server/servlets/HealthCheckServlet.java | 149 ++++++++++++++++++ ...rg.eclipse.microprofile.health.HealthCheck | 1 + ...ile.health.spi.HealthCheckResponseProvider | 1 + .../HealthCheckResponseBuilderTest.java | 62 ++++++++ .../HealthCheckResponseProviderTest.java | 33 ++++ .../checks/DatabaseHealthCheckTest.java | 114 ++++++++++++++ pom.xml | 12 ++ 12 files changed, 663 insertions(+) create mode 100644 alpine-server/src/main/java/alpine/server/health/HealthCheckResponseBuilder.java create mode 100644 alpine-server/src/main/java/alpine/server/health/HealthCheckResponseProvider.java create mode 100644 alpine-server/src/main/java/alpine/server/health/HealthCheckType.java create mode 100644 alpine-server/src/main/java/alpine/server/health/checks/DatabaseHealthCheck.java create mode 100644 alpine-server/src/main/java/alpine/server/servlets/HealthCheckServlet.java create mode 100644 alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck create mode 100644 alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.spi.HealthCheckResponseProvider create mode 100644 alpine-server/src/test/java/alpine/server/health/HealthCheckResponseBuilderTest.java create mode 100644 alpine-server/src/test/java/alpine/server/health/HealthCheckResponseProviderTest.java create mode 100644 alpine-server/src/test/java/alpine/server/health/checks/DatabaseHealthCheckTest.java diff --git a/alpine-server/pom.xml b/alpine-server/pom.xml index 92e9b5c8..2bff660d 100644 --- a/alpine-server/pom.xml +++ b/alpine-server/pom.xml @@ -189,6 +189,10 @@ com.fasterxml.jackson.jaxrs jackson-jaxrs-json-provider + + com.fasterxml.jackson.datatype + jackson-datatype-jdk8 + @@ -211,6 +215,11 @@ org.owasp security-logging-logback + + + org.eclipse.microprofile.health + microprofile-health-api + org.owasp.encoder diff --git a/alpine-server/src/main/java/alpine/server/health/HealthCheckResponseBuilder.java b/alpine-server/src/main/java/alpine/server/health/HealthCheckResponseBuilder.java new file mode 100644 index 00000000..064078d5 --- /dev/null +++ b/alpine-server/src/main/java/alpine/server/health/HealthCheckResponseBuilder.java @@ -0,0 +1,94 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health; + +import org.apache.commons.lang3.StringUtils; +import org.eclipse.microprofile.health.HealthCheckResponse; +import org.eclipse.microprofile.health.HealthCheckResponse.Status; + +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +import static java.util.function.Predicate.not; + +/** + * Implementation of the MicroProfile Health SPI. + * + * @see MicroProfile Health SPI Usage + * @since 2.3.0 + */ +class HealthCheckResponseBuilder extends org.eclipse.microprofile.health.HealthCheckResponseBuilder { + + private String name; + private Status status = Status.DOWN; + private final Map data = new HashMap<>(); + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder name(final String name) { + this.name = name; + return this; + } + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder withData(final String key, final String value) { + data.put(key, value); + return this; + } + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder withData(final String key, final long value) { + data.put(key, value); + return this; + } + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder withData(final String key, final boolean value) { + data.put(key, value); + return this; + } + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder up() { + status = Status.UP; + return this; + } + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder down() { + status = Status.DOWN; + return this; + } + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder status(final boolean up) { + status = up ? Status.UP : Status.DOWN; + return this; + } + + @Override + public HealthCheckResponse build() { + if (StringUtils.isBlank(name)) { + throw new IllegalArgumentException("Health check responses must provide a name"); + } + + return new HealthCheckResponse(name, status, Optional.of(data).filter(not(Map::isEmpty))); + } + +} diff --git a/alpine-server/src/main/java/alpine/server/health/HealthCheckResponseProvider.java b/alpine-server/src/main/java/alpine/server/health/HealthCheckResponseProvider.java new file mode 100644 index 00000000..0b4a0f0f --- /dev/null +++ b/alpine-server/src/main/java/alpine/server/health/HealthCheckResponseProvider.java @@ -0,0 +1,34 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health; + +/** + * Implementation of the MicroProfile Health SPI. + * + * @see MicroProfile Health SPI Usage + * @since 2.3.0 + */ +public class HealthCheckResponseProvider implements org.eclipse.microprofile.health.spi.HealthCheckResponseProvider { + + @Override + public org.eclipse.microprofile.health.HealthCheckResponseBuilder createResponseBuilder() { + return new HealthCheckResponseBuilder(); + } + +} diff --git a/alpine-server/src/main/java/alpine/server/health/HealthCheckType.java b/alpine-server/src/main/java/alpine/server/health/HealthCheckType.java new file mode 100644 index 00000000..8673ed7d --- /dev/null +++ b/alpine-server/src/main/java/alpine/server/health/HealthCheckType.java @@ -0,0 +1,53 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health; + +/** + * Defines types of health checks supported by MicroProfile Health. + * + * @see MicroProfile Health Specification + * @see Probes in Kubernetes + * @since 2.3.0 + */ +public enum HealthCheckType { + + /** + * Liveness probes may be used by service orchestrators to evaluate + * whether a service instance needs to be restarted. + */ + LIVENESS, + + /** + * Readiness probes may be used by service orchestrators to evaluate + * whether a service instance is ready to accept traffic. + */ + READINESS, + + /** + * Startup probes may be used by service orchestrators to evaluate + * whether a service instance has started. + */ + STARTUP, + + /** + * Probes that either do not specify their type, or apply to all types. + */ + ALL + +} diff --git a/alpine-server/src/main/java/alpine/server/health/checks/DatabaseHealthCheck.java b/alpine-server/src/main/java/alpine/server/health/checks/DatabaseHealthCheck.java new file mode 100644 index 00000000..943a0a65 --- /dev/null +++ b/alpine-server/src/main/java/alpine/server/health/checks/DatabaseHealthCheck.java @@ -0,0 +1,101 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health.checks; + +import alpine.common.logging.Logger; +import alpine.server.persistence.PersistenceManagerFactory; +import org.eclipse.microprofile.health.HealthCheck; +import org.eclipse.microprofile.health.HealthCheckResponse; +import org.eclipse.microprofile.health.HealthCheckResponse.Status; +import org.eclipse.microprofile.health.Readiness; + +import javax.jdo.PersistenceManager; +import javax.jdo.Query; +import javax.jdo.Transaction; + +/** + * @since 2.3.0 + */ +@Readiness +public class DatabaseHealthCheck implements HealthCheck { + + private static final Logger LOGGER = Logger.getLogger(DatabaseHealthCheck.class); + + @Override + public HealthCheckResponse call() { + final var responseBuilder = HealthCheckResponse.named("database"); + + try (final PersistenceManager pm = PersistenceManagerFactory.createPersistenceManager()) { + // DataNucleus maintains different connection pools for transactional and + // non-transactional operations. Check both of them by executing a test query + // in a transactional and non-transactional context. + final Status nonTransactionalStatus = checkNonTransactionalConnectionPool(pm); + final Status transactionalStatus = checkTransactionalConnectionPool(pm); + + responseBuilder + .status(nonTransactionalStatus == Status.UP && transactionalStatus == Status.UP) + .withData("nontx_connection_pool", nonTransactionalStatus.name()) + .withData("tx_connection_pool", transactionalStatus.name()); + } catch (Exception e) { + LOGGER.error("Executing database health check failed", e); + responseBuilder.down() + .withData("exception_message", e.getMessage()); + } + + return responseBuilder.build(); + } + + private Status checkNonTransactionalConnectionPool(final PersistenceManager pm) { + LOGGER.debug("Checking non-transactional connection pool"); + try { + return executeQuery(pm); + } catch (Exception e) { + LOGGER.error("Checking non-transactional connection pool failed", e); + return Status.DOWN; + } + } + + private Status checkTransactionalConnectionPool(final PersistenceManager pm) { + LOGGER.debug("Checking transactional connection pool"); + final Transaction trx = pm.currentTransaction(); + trx.setRollbackOnly(); + try { + trx.begin(); + return executeQuery(pm); + } catch (Exception e) { + LOGGER.error("Checking transactional connection pool failed", e); + return Status.DOWN; + } finally { + if (trx.isActive()) { + trx.rollback(); + } + } + } + + private Status executeQuery(final PersistenceManager pm) { + final Query query = pm.newQuery(Query.SQL, "SELECT 1"); + try { + query.executeResultUnique(Long.class); + return Status.UP; + } finally { + query.closeAll(); + } + } + +} diff --git a/alpine-server/src/main/java/alpine/server/servlets/HealthCheckServlet.java b/alpine-server/src/main/java/alpine/server/servlets/HealthCheckServlet.java new file mode 100644 index 00000000..0cabb1e0 --- /dev/null +++ b/alpine-server/src/main/java/alpine/server/servlets/HealthCheckServlet.java @@ -0,0 +1,149 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.servlets; + +import alpine.common.logging.Logger; +import alpine.server.health.HealthCheckType; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; +import org.eclipse.microprofile.health.HealthCheck; +import org.eclipse.microprofile.health.HealthCheckResponse; +import org.eclipse.microprofile.health.Liveness; +import org.eclipse.microprofile.health.Readiness; +import org.eclipse.microprofile.health.Startup; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.core.HttpHeaders; +import javax.ws.rs.core.MediaType; +import java.io.IOException; +import java.util.ArrayList; +import java.util.ServiceLoader; + +/** + * A {@link HttpServlet} exposing health information, following the MicroProfile Health specification. + *

+ * Health checks can be added by implementing the {@link HealthCheck} interface, and registering + * implementations as providers of {@code org.eclipse.microprofile.health.HealthCheck} in {@code META-INF/services}. + *

+ * {@link HealthCheck} implementations must be annotated with either {@link Liveness}, {@link Readiness}, or {@link Startup}. + * Checks without any of those annotations will be ignored. + * + * @see MicroProfile Health Specification + * @since 2.3.0 + */ +public class HealthCheckServlet extends HttpServlet { + + private static final Logger LOGGER = Logger.getLogger(HealthCheckServlet.class); + + private ServiceLoader healthCheckServiceLoader; + private ObjectMapper objectMapper; + + @Override + public void init() throws ServletException { + healthCheckServiceLoader = ServiceLoader.load(HealthCheck.class); + objectMapper = new ObjectMapper() + // HealthCheckResponse#data is of type Optional. + // We need this module to correctly serialize Optional values. + .registerModule(new Jdk8Module()) + .setSerializationInclusion(JsonInclude.Include.NON_NULL); + } + + @Override + protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) { + final HealthCheckType requestCheckType = determineHealthCheckType(req); + + final var checkResponses = new ArrayList(); + try { + for (final HealthCheck healthCheck : healthCheckServiceLoader) { + final HealthCheckType checkType = determineHealthCheckType(healthCheck); + if (checkType != null + && (requestCheckType == HealthCheckType.ALL || requestCheckType == checkType)) { + LOGGER.debug("Calling health check: " + healthCheck.getClass().getName()); + checkResponses.add(healthCheck.call()); + } + } + } catch (Exception e) { + LOGGER.error("Failed to execute health checks", e); + resp.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return; + } + + // The overall UP status is determined by logical conjunction of all check statuses. + final HealthCheckResponse.Status overallStatus = checkResponses.stream() + .map(HealthCheckResponse::getStatus) + .filter(HealthCheckResponse.Status.DOWN::equals) + .findFirst() + .orElse(HealthCheckResponse.Status.UP); + + final JsonNode responseJson = JsonNodeFactory.instance.objectNode() + .put("status", overallStatus.name()) + .putPOJO("checks", checkResponses); + + try { + resp.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); + objectMapper.writeValue(resp.getWriter(), responseJson); + } catch (IOException e) { + LOGGER.error("Failed to write health response", e); + resp.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return; + } + + if (overallStatus == HealthCheckResponse.Status.UP) { + resp.setStatus(HttpServletResponse.SC_OK); + } else { + resp.setStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE); + } + } + + private HealthCheckType determineHealthCheckType(final HttpServletRequest req) { + final String requestPath = req.getPathInfo(); + if (requestPath == null) { + return HealthCheckType.ALL; + } + + return switch (requestPath) { + case "/live" -> HealthCheckType.LIVENESS; + case "/ready" -> HealthCheckType.READINESS; + case "/started" -> HealthCheckType.STARTUP; + default -> HealthCheckType.ALL; + }; + } + + private HealthCheckType determineHealthCheckType(final HealthCheck healthCheck) { + final Class checkClass = healthCheck.getClass(); + if (checkClass.isAnnotationPresent(Liveness.class)) { + return HealthCheckType.LIVENESS; + } else if (checkClass.isAnnotationPresent(Readiness.class)) { + return HealthCheckType.READINESS; + } else if (checkClass.isAnnotationPresent(Startup.class)) { + return HealthCheckType.STARTUP; + } + + // Checks without classification are supposed to be + // ignored according to the spec. + return null; + } + +} diff --git a/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck b/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck new file mode 100644 index 00000000..bf0bc270 --- /dev/null +++ b/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck @@ -0,0 +1 @@ +alpine.server.health.checks.DatabaseHealthCheck \ No newline at end of file diff --git a/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.spi.HealthCheckResponseProvider b/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.spi.HealthCheckResponseProvider new file mode 100644 index 00000000..d697591b --- /dev/null +++ b/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.spi.HealthCheckResponseProvider @@ -0,0 +1 @@ +alpine.server.health.HealthCheckResponseProvider \ No newline at end of file diff --git a/alpine-server/src/test/java/alpine/server/health/HealthCheckResponseBuilderTest.java b/alpine-server/src/test/java/alpine/server/health/HealthCheckResponseBuilderTest.java new file mode 100644 index 00000000..787ceb7c --- /dev/null +++ b/alpine-server/src/test/java/alpine/server/health/HealthCheckResponseBuilderTest.java @@ -0,0 +1,62 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health; + +import org.eclipse.microprofile.health.HealthCheckResponse; +import org.junit.Test; + +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +public class HealthCheckResponseBuilderTest { + + @Test + public void test() { + final HealthCheckResponse response = new HealthCheckResponseBuilder() + .name("foobar") + .status(true) + .withData("foo", "bar") + .withData("baz", 666L) + .build(); + assertThat(response.getName()).isEqualTo("foobar"); + assertThat(response.getStatus()).isEqualTo(HealthCheckResponse.Status.UP); + assertThat(response.getData()).isPresent(); + assertThat(response.getData().get()).containsAllEntriesOf(Map.of( + "foo", "bar", + "baz", 666L + )); + } + + @Test + public void testDefaults() { + final HealthCheckResponse response = new HealthCheckResponseBuilder().name("foobar").build(); + assertThat(response.getName()).isEqualTo("foobar"); + assertThat(response.getStatus()).isEqualTo(HealthCheckResponse.Status.DOWN); + assertThat(response.getData()).isNotPresent(); + } + + @Test + public void testWithNoName() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> new HealthCheckResponseBuilder().build()); + } + +} \ No newline at end of file diff --git a/alpine-server/src/test/java/alpine/server/health/HealthCheckResponseProviderTest.java b/alpine-server/src/test/java/alpine/server/health/HealthCheckResponseProviderTest.java new file mode 100644 index 00000000..92ef94eb --- /dev/null +++ b/alpine-server/src/test/java/alpine/server/health/HealthCheckResponseProviderTest.java @@ -0,0 +1,33 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class HealthCheckResponseProviderTest { + + @Test + public void testCreateResponseBuilder() { + assertThat(new HealthCheckResponseProvider().createResponseBuilder()) + .isInstanceOf(HealthCheckResponseBuilder.class); + } + +} \ No newline at end of file diff --git a/alpine-server/src/test/java/alpine/server/health/checks/DatabaseHealthCheckTest.java b/alpine-server/src/test/java/alpine/server/health/checks/DatabaseHealthCheckTest.java new file mode 100644 index 00000000..a6585d7b --- /dev/null +++ b/alpine-server/src/test/java/alpine/server/health/checks/DatabaseHealthCheckTest.java @@ -0,0 +1,114 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health.checks; + +import alpine.Config; +import alpine.server.persistence.PersistenceManagerFactory; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.datanucleus.api.jdo.JDOPersistenceManagerFactory; +import org.datanucleus.store.connection.ConnectionFactory; +import org.datanucleus.store.connection.ConnectionManagerImpl; +import org.eclipse.microprofile.health.HealthCheckResponse; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; + +import javax.jdo.PersistenceManager; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DatabaseHealthCheckTest { + + @BeforeClass + public static void setUpClass() { + Config.enableUnitTests(); + } + + @After + public void tearDown() { + PersistenceManagerFactory.tearDown(); + } + + @Test + public void testWithAllConnectionFactoriesUp() { + final HealthCheckResponse response = new DatabaseHealthCheck().call(); + assertThat(response.getName()).isEqualTo("database"); + assertThat(response.getStatus()).isEqualTo(HealthCheckResponse.Status.UP); + assertThat(response.getData()).isPresent(); + assertThat(response.getData().get()).containsAllEntriesOf(Map.of( + "nontx_connection_pool", "UP", + "tx_connection_pool", "UP" + )); + } + + @Test + public void testWithPersistenceManagerFactoryClosed() { + try (final PersistenceManager pm = PersistenceManagerFactory.createPersistenceManager()) { + pm.getPersistenceManagerFactory().close(); + } + + final HealthCheckResponse response = new DatabaseHealthCheck().call(); + assertThat(response.getName()).isEqualTo("database"); + assertThat(response.getStatus()).isEqualTo(HealthCheckResponse.Status.DOWN); + assertThat(response.getData()).isPresent(); + assertThat(response.getData().get()).containsAllEntriesOf(Map.of( + "exception_message", "Cant access or use PMF after it has been closed." + )); + } + + @Test + public void testWithTransactionalConnectionFactoryDown() throws Exception { + try (final PersistenceManager pm = PersistenceManagerFactory.createPersistenceManager()) { + final var pmf = (JDOPersistenceManagerFactory) pm.getPersistenceManagerFactory(); + final var connectionManager = (ConnectionManagerImpl) pmf.getNucleusContext().getStoreManager().getConnectionManager(); + final var primaryConnectionFactory = (ConnectionFactory) FieldUtils.readField(connectionManager, "primaryConnectionFactory", true); + primaryConnectionFactory.close(); + } + + final HealthCheckResponse response = new DatabaseHealthCheck().call(); + assertThat(response.getName()).isEqualTo("database"); + assertThat(response.getStatus()).isEqualTo(HealthCheckResponse.Status.DOWN); + assertThat(response.getData()).isPresent(); + assertThat(response.getData().get()).containsAllEntriesOf(Map.of( + "nontx_connection_pool", "UP", + "tx_connection_pool", "DOWN" + )); + } + + @Test + public void testWithSecondaryConnectionFactoryDown() throws Exception { + try (final PersistenceManager pm = PersistenceManagerFactory.createPersistenceManager()) { + final var pmf = (JDOPersistenceManagerFactory) pm.getPersistenceManagerFactory(); + final var connectionManager = (ConnectionManagerImpl) pmf.getNucleusContext().getStoreManager().getConnectionManager(); + final var secondaryConnectionFactory = (ConnectionFactory) FieldUtils.readField(connectionManager, "secondaryConnectionFactory", true); + secondaryConnectionFactory.close(); + } + + final HealthCheckResponse response = new DatabaseHealthCheck().call(); + assertThat(response.getName()).isEqualTo("database"); + assertThat(response.getStatus()).isEqualTo(HealthCheckResponse.Status.DOWN); + assertThat(response.getData()).isPresent(); + assertThat(response.getData().get()).containsAllEntriesOf(Map.of( + "nontx_connection_pool", "DOWN", + "tx_connection_pool", "UP" + )); + } + +} \ No newline at end of file diff --git a/pom.xml b/pom.xml index 462c0fc9..30c06c6d 100644 --- a/pom.xml +++ b/pom.xml @@ -185,6 +185,7 @@ 1.2.5 1.2.11 1.9.4 + 3.1 9.43.1 1.2.3 1.1.7 @@ -377,12 +378,23 @@ jackson-jaxrs-json-provider ${lib.jackson.version} + + com.fasterxml.jackson.datatype + jackson-datatype-jdk8 + ${lib.jackson.version} + io.micrometer micrometer-registry-prometheus ${lib.micrometer.version} + + + org.eclipse.microprofile.health + microprofile-health-api + ${lib.microprofile-health-api.version} + From 5e0a4fed74bf061cca7d6a2e0513fdb8d916d1e4 Mon Sep 17 00:00:00 2001 From: nscuro Date: Fri, 31 Mar 2023 20:36:09 +0200 Subject: [PATCH 2/3] Use global registry instead of service providers Also: * Allow checks to have multiple types, not just one * Rename `HealthCheckServlet` to `HealthServlet` * Add tests for servlet Signed-off-by: nscuro --- alpine-server/pom.xml | 5 + .../server/health/HealthCheckRegistry.java | 56 +++ ...thCheckServlet.java => HealthServlet.java} | 52 ++- ...rg.eclipse.microprofile.health.HealthCheck | 1 - .../server/servlets/HealthServletTest.java | 375 ++++++++++++++++++ pom.xml | 7 + 6 files changed, 473 insertions(+), 23 deletions(-) create mode 100644 alpine-server/src/main/java/alpine/server/health/HealthCheckRegistry.java rename alpine-server/src/main/java/alpine/server/servlets/{HealthCheckServlet.java => HealthServlet.java} (77%) delete mode 100644 alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck create mode 100644 alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java diff --git a/alpine-server/pom.xml b/alpine-server/pom.xml index 2bff660d..99f0ed1f 100644 --- a/alpine-server/pom.xml +++ b/alpine-server/pom.xml @@ -270,6 +270,11 @@ assertj-core test + + net.javacrumbs.json-unit + json-unit-assertj + test + com.github.tomakehurst wiremock-jre8-standalone diff --git a/alpine-server/src/main/java/alpine/server/health/HealthCheckRegistry.java b/alpine-server/src/main/java/alpine/server/health/HealthCheckRegistry.java new file mode 100644 index 00000000..7d1323ab --- /dev/null +++ b/alpine-server/src/main/java/alpine/server/health/HealthCheckRegistry.java @@ -0,0 +1,56 @@ +/* + * This file is part of Alpine. + * + * 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 + * Copyright (c) Steve Springett. All Rights Reserved. + */ +package alpine.server.health; + +import org.eclipse.microprofile.health.HealthCheck; + +import java.util.Collections; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * A global registry for {@link HealthCheck}s. + *

+ * Used by {@link alpine.server.servlets.HealthServlet} to lookup registered checks. + * + * @since 2.3.0 + */ +public class HealthCheckRegistry { + + private static final HealthCheckRegistry INSTANCE = new HealthCheckRegistry(); + + private final Map checks; + + public HealthCheckRegistry() { + checks = new ConcurrentHashMap<>(); + } + + public static HealthCheckRegistry getInstance() { + return INSTANCE; + } + + public Map getChecks() { + return Collections.unmodifiableMap(checks); + } + + public void register(final String name, final HealthCheck check) { + checks.put(name, check); + } + +} diff --git a/alpine-server/src/main/java/alpine/server/servlets/HealthCheckServlet.java b/alpine-server/src/main/java/alpine/server/servlets/HealthServlet.java similarity index 77% rename from alpine-server/src/main/java/alpine/server/servlets/HealthCheckServlet.java rename to alpine-server/src/main/java/alpine/server/servlets/HealthServlet.java index 0cabb1e0..6dd87662 100644 --- a/alpine-server/src/main/java/alpine/server/servlets/HealthCheckServlet.java +++ b/alpine-server/src/main/java/alpine/server/servlets/HealthServlet.java @@ -19,6 +19,7 @@ package alpine.server.servlets; import alpine.common.logging.Logger; +import alpine.server.health.HealthCheckRegistry; import alpine.server.health.HealthCheckType; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.JsonNode; @@ -39,30 +40,36 @@ import javax.ws.rs.core.MediaType; import java.io.IOException; import java.util.ArrayList; -import java.util.ServiceLoader; /** * A {@link HttpServlet} exposing health information, following the MicroProfile Health specification. *

* Health checks can be added by implementing the {@link HealthCheck} interface, and registering - * implementations as providers of {@code org.eclipse.microprofile.health.HealthCheck} in {@code META-INF/services}. + * implementations with the global {@link HealthCheckRegistry} instance. *

- * {@link HealthCheck} implementations must be annotated with either {@link Liveness}, {@link Readiness}, or {@link Startup}. - * Checks without any of those annotations will be ignored. + * {@link HealthCheck} implementations must be annotated with either {@link Liveness}, {@link Readiness}, {@link Startup}, + * or any combination of the same. Checks without any of those annotations will be ignored. * * @see MicroProfile Health Specification * @since 2.3.0 */ -public class HealthCheckServlet extends HttpServlet { +public class HealthServlet extends HttpServlet { - private static final Logger LOGGER = Logger.getLogger(HealthCheckServlet.class); + private static final Logger LOGGER = Logger.getLogger(HealthServlet.class); - private ServiceLoader healthCheckServiceLoader; + private final HealthCheckRegistry checkRegistry; private ObjectMapper objectMapper; + public HealthServlet() { + this(HealthCheckRegistry.getInstance()); + } + + HealthServlet(final HealthCheckRegistry checkRegistry) { + this.checkRegistry = checkRegistry; + } + @Override public void init() throws ServletException { - healthCheckServiceLoader = ServiceLoader.load(HealthCheck.class); objectMapper = new ObjectMapper() // HealthCheckResponse#data is of type Optional. // We need this module to correctly serialize Optional values. @@ -72,14 +79,12 @@ public void init() throws ServletException { @Override protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) { - final HealthCheckType requestCheckType = determineHealthCheckType(req); + final HealthCheckType requestedCheckType = determineHealthCheckType(req); final var checkResponses = new ArrayList(); try { - for (final HealthCheck healthCheck : healthCheckServiceLoader) { - final HealthCheckType checkType = determineHealthCheckType(healthCheck); - if (checkType != null - && (requestCheckType == HealthCheckType.ALL || requestCheckType == checkType)) { + for (final HealthCheck healthCheck : checkRegistry.getChecks().values()) { + if (matchesCheckType(healthCheck, requestedCheckType)) { LOGGER.debug("Calling health check: " + healthCheck.getClass().getName()); checkResponses.add(healthCheck.call()); } @@ -131,19 +136,22 @@ private HealthCheckType determineHealthCheckType(final HttpServletRequest req) { }; } - private HealthCheckType determineHealthCheckType(final HealthCheck healthCheck) { - final Class checkClass = healthCheck.getClass(); - if (checkClass.isAnnotationPresent(Liveness.class)) { - return HealthCheckType.LIVENESS; - } else if (checkClass.isAnnotationPresent(Readiness.class)) { - return HealthCheckType.READINESS; - } else if (checkClass.isAnnotationPresent(Startup.class)) { - return HealthCheckType.STARTUP; + private boolean matchesCheckType(final HealthCheck check, final HealthCheckType requestedType) { + final Class checkClass = check.getClass(); + if (checkClass.isAnnotationPresent(Liveness.class) + && (requestedType == HealthCheckType.ALL || requestedType == HealthCheckType.LIVENESS)) { + return true; + } else if (checkClass.isAnnotationPresent(Readiness.class) + && (requestedType == HealthCheckType.ALL || requestedType == HealthCheckType.READINESS)) { + return true; + } else if (checkClass.isAnnotationPresent(Startup.class) + && (requestedType == HealthCheckType.ALL || requestedType == HealthCheckType.STARTUP)) { + return true; } // Checks without classification are supposed to be // ignored according to the spec. - return null; + return false; } } diff --git a/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck b/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck deleted file mode 100644 index bf0bc270..00000000 --- a/alpine-server/src/main/resources/META-INF/services/org.eclipse.microprofile.health.HealthCheck +++ /dev/null @@ -1 +0,0 @@ -alpine.server.health.checks.DatabaseHealthCheck \ No newline at end of file diff --git a/alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java b/alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java new file mode 100644 index 00000000..cd8ea593 --- /dev/null +++ b/alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java @@ -0,0 +1,375 @@ +package alpine.server.servlets; + +import alpine.server.health.HealthCheckRegistry; +import net.javacrumbs.jsonunit.core.Option; +import org.eclipse.microprofile.health.HealthCheck; +import org.eclipse.microprofile.health.HealthCheckResponse; +import org.eclipse.microprofile.health.Liveness; +import org.eclipse.microprofile.health.Readiness; +import org.eclipse.microprofile.health.Startup; +import org.junit.Before; +import org.junit.Test; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.ByteArrayOutputStream; +import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; +import java.util.function.Supplier; + +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class HealthServletTest { + + private HttpServletRequest requestMock; + private HttpServletResponse responseMock; + private ByteArrayOutputStream responseOutputStream; + private PrintWriter responseWriter; + + @Before + public void setUp() throws Exception { + requestMock = mock(HttpServletRequest.class); + responseMock = mock(HttpServletResponse.class); + responseOutputStream = new ByteArrayOutputStream(); + responseWriter = new PrintWriter(responseOutputStream); + when(responseMock.getWriter()).thenReturn(responseWriter); + } + + @Test + public void shouldReportStatusUpWhenNoChecksAreRegistered() throws Exception { + final var servlet = new HealthServlet(); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(200)); + verify(responseMock).setHeader(eq("Content-Type"), eq("application/json")); + assertThatJson(responseOutputStream.toString(StandardCharsets.UTF_8)) + .isEqualTo(""" + { + "status": "UP", + "checks": [] + } + """); + } + + @Test + public void shouldReportStatusUpWhenAllChecksAreUp() throws Exception { + final var checkA = new MockReadinessCheck(() -> HealthCheckResponse.up("foo")); + final var checkB = new MockReadinessCheck(() -> HealthCheckResponse.up("bar")); + + final var checkRegistry = new HealthCheckRegistry(); + checkRegistry.register("foo", checkA); + checkRegistry.register("bar", checkB); + + final var servlet = new HealthServlet(checkRegistry); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(200)); + verify(responseMock).setHeader(eq("Content-Type"), eq("application/json")); + assertThatJson(responseOutputStream.toString(StandardCharsets.UTF_8)) + .when(Option.IGNORING_ARRAY_ORDER) + .isEqualTo(""" + { + "status": "UP", + "checks": [ + { + "name": "foo", + "status": "UP", + "data": null + }, + { + "name": "bar", + "status": "UP", + "data": null + } + ] + } + """); + } + + @Test + public void shouldReportStatusDownWhenAtLeastOneCheckIsDown() throws Exception { + final var checkUp = new MockReadinessCheck(() -> HealthCheckResponse.up("foo")); + final var checkDown = new MockReadinessCheck(() -> HealthCheckResponse.down("bar")); + + final var checkRegistry = new HealthCheckRegistry(); + checkRegistry.register("foo", checkUp); + checkRegistry.register("bar", checkDown); + + final var servlet = new HealthServlet(checkRegistry); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(503)); + verify(responseMock).setHeader(eq("Content-Type"), eq("application/json")); + assertThatJson(responseOutputStream.toString(StandardCharsets.UTF_8)) + .when(Option.IGNORING_ARRAY_ORDER) + .isEqualTo(""" + { + "status": "DOWN", + "checks": [ + { + "name": "foo", + "status": "UP", + "data": null + }, + { + "name": "bar", + "status": "DOWN", + "data": null + } + ] + } + """); + } + + @Test + public void shouldNotReportAnythingWhenCallingAtLeastOneCheckFailed() throws Exception { + final var checkUp = new MockReadinessCheck(() -> HealthCheckResponse.up("foo")); + final var checkFail = new MockReadinessCheck(() -> { + throw new IllegalStateException("Simulated check exception"); + }); + + final var checkRegistry = new HealthCheckRegistry(); + checkRegistry.register("foo", checkUp); + checkRegistry.register("bar", checkFail); + + final var servlet = new HealthServlet(checkRegistry); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(500)); + verify(responseMock, never()).setHeader(eq("Content-Type"), anyString()); + assertThat(responseOutputStream.size()).isZero(); + } + + @Test + public void shouldIncludeLivenessCheckWhenLivenessIsRequested() throws Exception { + final var livenessCheck = new MockLivenessCheck(() -> HealthCheckResponse.up("live")); + final var readinessCheck = new MockReadinessCheck(() -> HealthCheckResponse.up("ready")); + final var startupCheck = new MockStartupCheck(() -> HealthCheckResponse.up("start")); + final var allTypesCheck = new MockAllTypesCheck(() -> HealthCheckResponse.up("all")); + + final var checkRegistry = new HealthCheckRegistry(); + checkRegistry.register("foo", livenessCheck); + checkRegistry.register("bar", readinessCheck); + checkRegistry.register("baz", startupCheck); + checkRegistry.register("qux", allTypesCheck); + + when(requestMock.getPathInfo()).thenReturn("/live"); + + final var servlet = new HealthServlet(checkRegistry); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(200)); + verify(responseMock).setHeader(eq("Content-Type"), eq("application/json")); + assertThatJson(responseOutputStream.toString(StandardCharsets.UTF_8)) + .when(Option.IGNORING_ARRAY_ORDER) + .isEqualTo(""" + { + "status": "UP", + "checks": [ + { + "name": "live", + "status": "UP", + "data": null + }, + { + "name": "all", + "status": "UP", + "data": null + } + ] + } + """); + } + + @Test + public void shouldIncludeReadinessCheckWhenReadinessIsRequested() throws Exception { + final var livenessCheck = new MockLivenessCheck(() -> HealthCheckResponse.up("live")); + final var readinessCheck = new MockReadinessCheck(() -> HealthCheckResponse.up("ready")); + final var startupCheck = new MockStartupCheck(() -> HealthCheckResponse.up("start")); + final var allTypesCheck = new MockAllTypesCheck(() -> HealthCheckResponse.up("all")); + + final var checkRegistry = new HealthCheckRegistry(); + checkRegistry.register("foo", livenessCheck); + checkRegistry.register("bar", readinessCheck); + checkRegistry.register("baz", startupCheck); + checkRegistry.register("qux", allTypesCheck); + + when(requestMock.getPathInfo()).thenReturn("/ready"); + + final var servlet = new HealthServlet(checkRegistry); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(200)); + verify(responseMock).setHeader(eq("Content-Type"), eq("application/json")); + assertThatJson(responseOutputStream.toString(StandardCharsets.UTF_8)) + .when(Option.IGNORING_ARRAY_ORDER) + .isEqualTo(""" + { + "status": "UP", + "checks": [ + { + "name": "ready", + "status": "UP", + "data": null + }, + { + "name": "all", + "status": "UP", + "data": null + } + ] + } + """); + } + + @Test + public void shouldIncludeStartupCheckWhenStartupIsRequested() throws Exception { + final var livenessCheck = new MockLivenessCheck(() -> HealthCheckResponse.up("live")); + final var readinessCheck = new MockReadinessCheck(() -> HealthCheckResponse.up("ready")); + final var startupCheck = new MockStartupCheck(() -> HealthCheckResponse.up("start")); + final var allTypesCheck = new MockAllTypesCheck(() -> HealthCheckResponse.up("all")); + + final var checkRegistry = new HealthCheckRegistry(); + checkRegistry.register("foo", livenessCheck); + checkRegistry.register("bar", readinessCheck); + checkRegistry.register("baz", startupCheck); + checkRegistry.register("qux", allTypesCheck); + + when(requestMock.getPathInfo()).thenReturn("/started"); + + final var servlet = new HealthServlet(checkRegistry); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(200)); + verify(responseMock).setHeader(eq("Content-Type"), eq("application/json")); + assertThatJson(responseOutputStream.toString(StandardCharsets.UTF_8)) + .when(Option.IGNORING_ARRAY_ORDER) + .isEqualTo(""" + { + "status": "UP", + "checks": [ + { + "name": "start", + "status": "UP", + "data": null + }, + { + "name": "all", + "status": "UP", + "data": null + } + ] + } + """); + } + + @Test + public void shouldIncludeAllChecksWhenAllAreRequested() throws Exception { + final var livenessCheck = new MockLivenessCheck(() -> HealthCheckResponse.up("live")); + final var readinessCheck = new MockReadinessCheck(() -> HealthCheckResponse.up("ready")); + final var startupCheck = new MockStartupCheck(() -> HealthCheckResponse.up("start")); + final var allTypesCheck = new MockAllTypesCheck(() -> HealthCheckResponse.up("all")); + + final var checkRegistry = new HealthCheckRegistry(); + checkRegistry.register("foo", livenessCheck); + checkRegistry.register("bar", readinessCheck); + checkRegistry.register("baz", startupCheck); + checkRegistry.register("qux", allTypesCheck); + + when(requestMock.getPathInfo()).thenReturn("/"); + + final var servlet = new HealthServlet(checkRegistry); + servlet.init(); + servlet.doGet(requestMock, responseMock); + + verify(responseMock).setStatus(eq(200)); + verify(responseMock).setHeader(eq("Content-Type"), eq("application/json")); + assertThatJson(responseOutputStream.toString(StandardCharsets.UTF_8)) + .when(Option.IGNORING_ARRAY_ORDER) + .isEqualTo(""" + { + "status": "UP", + "checks": [ + { + "name": "live", + "status": "UP", + "data": null + }, + { + "name": "ready", + "status": "UP", + "data": null + }, + { + "name": "start", + "status": "UP", + "data": null + }, + { + "name": "all", + "status": "UP", + "data": null + } + ] + } + """); + } + + private abstract static class AbstractMockCheck implements HealthCheck { + private final Supplier responseSupplier; + + private AbstractMockCheck(final Supplier responseSupplier) { + this.responseSupplier = responseSupplier; + } + + @Override + public HealthCheckResponse call() { + return responseSupplier.get(); + } + } + + @Liveness + private static class MockLivenessCheck extends AbstractMockCheck { + private MockLivenessCheck(final Supplier responseSupplier) { + super(responseSupplier); + } + } + + @Readiness + private static class MockReadinessCheck extends AbstractMockCheck { + private MockReadinessCheck(final Supplier responseSupplier) { + super(responseSupplier); + } + } + + @Startup + private static class MockStartupCheck extends AbstractMockCheck { + private MockStartupCheck(final Supplier responseSupplier) { + super(responseSupplier); + } + } + + @Liveness + @Readiness + @Startup + private static class MockAllTypesCheck extends AbstractMockCheck { + private MockAllTypesCheck(final Supplier responseSupplier) { + super(responseSupplier); + } + } + +} \ No newline at end of file diff --git a/pom.xml b/pom.xml index 30c06c6d..16212146 100644 --- a/pom.xml +++ b/pom.xml @@ -177,6 +177,7 @@ 2.3.6 3.2.1 2.38 + 2.37.0 2.2 0.9.1 3.0.2 @@ -482,6 +483,12 @@ ${lib.assertj.version} test + + net.javacrumbs.json-unit + json-unit-assertj + ${lib.json-unit.version} + test + com.github.tomakehurst wiremock-jre8-standalone From ca397274751cda5e3f82e621fc273ae670326ef0 Mon Sep 17 00:00:00 2001 From: nscuro Date: Fri, 31 Mar 2023 21:11:12 +0200 Subject: [PATCH 3/3] Fix 503 status not being set after response body was written Signed-off-by: nscuro --- .../alpine/server/servlets/HealthServlet.java | 19 +++++++++---------- .../server/servlets/HealthServletTest.java | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/alpine-server/src/main/java/alpine/server/servlets/HealthServlet.java b/alpine-server/src/main/java/alpine/server/servlets/HealthServlet.java index 6dd87662..202dfb9a 100644 --- a/alpine-server/src/main/java/alpine/server/servlets/HealthServlet.java +++ b/alpine-server/src/main/java/alpine/server/servlets/HealthServlet.java @@ -78,7 +78,7 @@ public void init() throws ServletException { } @Override - protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) { + protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) throws IOException { final HealthCheckType requestedCheckType = determineHealthCheckType(req); final var checkResponses = new ArrayList(); @@ -91,7 +91,7 @@ protected void doGet(final HttpServletRequest req, final HttpServletResponse res } } catch (Exception e) { LOGGER.error("Failed to execute health checks", e); - resp.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); return; } @@ -106,19 +106,18 @@ protected void doGet(final HttpServletRequest req, final HttpServletResponse res .put("status", overallStatus.name()) .putPOJO("checks", checkResponses); + if (overallStatus == HealthCheckResponse.Status.UP) { + resp.setStatus(HttpServletResponse.SC_OK); + } else { + resp.setStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE); + } + try { resp.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); objectMapper.writeValue(resp.getWriter(), responseJson); } catch (IOException e) { LOGGER.error("Failed to write health response", e); - resp.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - return; - } - - if (overallStatus == HealthCheckResponse.Status.UP) { - resp.setStatus(HttpServletResponse.SC_OK); - } else { - resp.setStatus(HttpServletResponse.SC_SERVICE_UNAVAILABLE); + resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } diff --git a/alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java b/alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java index cd8ea593..70c451dd 100644 --- a/alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java +++ b/alpine-server/src/test/java/alpine/server/servlets/HealthServletTest.java @@ -146,7 +146,7 @@ public void shouldNotReportAnythingWhenCallingAtLeastOneCheckFailed() throws Exc servlet.init(); servlet.doGet(requestMock, responseMock); - verify(responseMock).setStatus(eq(500)); + verify(responseMock).sendError(eq(500)); verify(responseMock, never()).setHeader(eq("Content-Type"), anyString()); assertThat(responseOutputStream.size()).isZero(); }