You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2019/08/29 07:26:13 UTC

[james-project] 04/05: JAMES-2526 Make cause compulsory for degraded & unhealthy status

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit f3821641694d2366405d7d484f3006fcc49321a0
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue Aug 27 11:22:51 2019 +0700

    JAMES-2526 Make cause compulsory for degraded & unhealthy status
---
 .../cassandra/utils/CassandraHealthCheck.java      |  2 +-
 .../backend/rabbitmq/RabbitMQHealthCheck.java      |  7 ++-
 .../org/apache/james/core/healthcheck/Result.java  |  8 ---
 .../apache/james/core/healthcheck/ResultTest.java  | 72 +++++++++-------------
 .../james/jpa/healthcheck/JPAHealthCheck.java      |  4 +-
 .../webadmin/routes/HealthCheckRoutesTest.java     | 11 ++--
 6 files changed, 42 insertions(+), 62 deletions(-)

diff --git a/backends-common/cassandra/src/main/java/org/apache/james/backends/cassandra/utils/CassandraHealthCheck.java b/backends-common/cassandra/src/main/java/org/apache/james/backends/cassandra/utils/CassandraHealthCheck.java
index 7023519..cc78894 100644
--- a/backends-common/cassandra/src/main/java/org/apache/james/backends/cassandra/utils/CassandraHealthCheck.java
+++ b/backends-common/cassandra/src/main/java/org/apache/james/backends/cassandra/utils/CassandraHealthCheck.java
@@ -60,7 +60,7 @@ public class CassandraHealthCheck implements HealthCheck {
             return Result.healthy(COMPONENT_NAME);
         } catch (Exception e) {
             LOGGER.error("Error checking cassandra backend", e);
-            return Result.unhealthy(COMPONENT_NAME);
+            return Result.unhealthy(COMPONENT_NAME, e.getMessage());
         }
     }
 }
diff --git a/backends-common/rabbitmq/src/main/java/org/apache/james/backend/rabbitmq/RabbitMQHealthCheck.java b/backends-common/rabbitmq/src/main/java/org/apache/james/backend/rabbitmq/RabbitMQHealthCheck.java
index 0290a47..a3b02a4 100644
--- a/backends-common/rabbitmq/src/main/java/org/apache/james/backend/rabbitmq/RabbitMQHealthCheck.java
+++ b/backends-common/rabbitmq/src/main/java/org/apache/james/backend/rabbitmq/RabbitMQHealthCheck.java
@@ -49,12 +49,13 @@ public class RabbitMQHealthCheck implements HealthCheck {
             if (rabbitChannelPoolImpl.tryConnection()) {
                 return Result.healthy(COMPONENT_NAME);
             } else {
-                LOGGER.error("The created connection was not opened");
-                return Result.unhealthy(COMPONENT_NAME);
+                String message = "The created connection was not opened";
+                LOGGER.error(message);
+                return Result.unhealthy(COMPONENT_NAME, message);
             }
         } catch (Exception e) {
             LOGGER.error("Unhealthy RabbitMQ instances: could not establish a connection", e);
-            return Result.unhealthy(COMPONENT_NAME);
+            return Result.unhealthy(COMPONENT_NAME, e.getMessage());
         }
     }
 }
diff --git a/core/src/main/java/org/apache/james/core/healthcheck/Result.java b/core/src/main/java/org/apache/james/core/healthcheck/Result.java
index fe9657a..e204069 100644
--- a/core/src/main/java/org/apache/james/core/healthcheck/Result.java
+++ b/core/src/main/java/org/apache/james/core/healthcheck/Result.java
@@ -30,18 +30,10 @@ public class Result {
         return new Result(componentName, ResultStatus.UNHEALTHY, Optional.of(cause));
     }
 
-    public static Result unhealthy(ComponentName componentName) {
-        return new Result(componentName, ResultStatus.UNHEALTHY, Optional.empty());
-    }
-
     public static Result degraded(ComponentName componentName, String cause) {
         return new Result(componentName, ResultStatus.DEGRADED, Optional.of(cause));
     }
 
-    public static Result degraded(ComponentName componentName) {
-        return new Result(componentName, ResultStatus.DEGRADED, Optional.empty());
-    }
-
     private final ComponentName componentName;
     private final ResultStatus status;
     private final Optional<String> cause;
diff --git a/core/src/test/java/org/apache/james/core/healthcheck/ResultTest.java b/core/src/test/java/org/apache/james/core/healthcheck/ResultTest.java
index ece974a..e2e96fe 100644
--- a/core/src/test/java/org/apache/james/core/healthcheck/ResultTest.java
+++ b/core/src/test/java/org/apache/james/core/healthcheck/ResultTest.java
@@ -23,88 +23,81 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import org.junit.jupiter.api.Test;
 
-public class ResultTest {
+class ResultTest {
 
     private static final ComponentName COMPONENT_NAME = new ComponentName("component");
 
     @Test
-    public void componentNameShouldBeKeptWhenHealthy() {
+    void componentNameShouldBeKeptWhenHealthy() {
         Result result = Result.healthy(COMPONENT_NAME);
 
         assertThat(result.getComponentName()).isEqualTo(COMPONENT_NAME);
     }
 
     @Test
-    public void componentNameShouldBeKeptWhenUnhealthy() {
-        Result result = Result.unhealthy(COMPONENT_NAME);
+    void componentNameShouldBeKeptWhenUnhealthy() {
+        Result result = Result.unhealthy(COMPONENT_NAME, "cause");
 
         assertThat(result.getComponentName()).isEqualTo(COMPONENT_NAME);
     }
 
     @Test
-    public void componentNameShouldBeKeptWhenDegraded() {
-        Result result = Result.degraded(COMPONENT_NAME);
+    void componentNameShouldBeKeptWhenDegraded() {
+        Result result = Result.degraded(COMPONENT_NAME, "cause");
 
         assertThat(result.getComponentName()).isEqualTo(COMPONENT_NAME);
     }
 
     @Test
-    public void statusShouldBeHealthyWhenHealthy() {
+    void statusShouldBeHealthyWhenHealthy() {
         Result result = Result.healthy(COMPONENT_NAME);
 
         assertThat(result.getStatus()).isEqualTo(ResultStatus.HEALTHY);
     }
 
     @Test
-    public void causeShouldBeEmptyWhenHealthy() {
+    void causeShouldBeEmptyWhenHealthy() {
         Result result = Result.healthy(COMPONENT_NAME);
 
         assertThat(result.getCause()).isEmpty();
     }
 
     @Test
-    public void isHealthyShouldBeTrueWhenHealthy() {
+    void isHealthyShouldBeTrueWhenHealthy() {
         Result result = Result.healthy(COMPONENT_NAME);
 
         assertThat(result.isHealthy()).isTrue();
     }
 
     @Test
-    public void isDegradedShouldBeFalseWhenHealthy() {
+    void isDegradedShouldBeFalseWhenHealthy() {
         Result result = Result.healthy(COMPONENT_NAME);
 
         assertThat(result.isDegraded()).isFalse();
     }
 
     @Test
-    public void isUnhealthyShouldBeFalseWhenHealthy() {
+    void isUnhealthyShouldBeFalseWhenHealthy() {
         Result result = Result.healthy(COMPONENT_NAME);
 
         assertThat(result.isUnHealthy()).isFalse();
     }
 
     @Test
-    public void statusShouldBeDegradedWhenDegraded() {
+    void statusShouldBeDegradedWhenDegraded() {
         Result result = Result.degraded(COMPONENT_NAME, "cause");
 
         assertThat(result.getStatus()).isEqualTo(ResultStatus.DEGRADED);
     }
 
     @Test
-    public void causeMayBeEmptyWhenDegraded() {
-        Result result = Result.degraded(COMPONENT_NAME);
-
-        assertThat(result.getCause()).isEmpty();
-    }
-
-    @Test
-    public void degradedShouldThrowWhenNullCause() {
+    void degradedShouldThrowWhenNullCause() {
         assertThatThrownBy(() -> Result.degraded(COMPONENT_NAME, null))
             .isInstanceOf(NullPointerException.class);
     }
 
     @Test
-    public void causeShouldBeKeptWhenNotDegraded() {
+    void causeShouldBeKeptWhenNotDegraded() {
         String cause = "cause";
         Result result = Result.degraded(COMPONENT_NAME, cause);
 
@@ -112,42 +105,35 @@ public class ResultTest {
     }
 
     @Test
-    public void isHealthyShouldBeFalseWhenDegraded() {
-        Result result = Result.degraded(COMPONENT_NAME);
+    void isHealthyShouldBeFalseWhenDegraded() {
+        Result result = Result.degraded(COMPONENT_NAME, "cause");
 
         assertThat(result.isHealthy()).isFalse();
     }
 
     @Test
-    public void isDegradedShouldBeFalseWhenDegraded() {
-        Result result = Result.degraded(COMPONENT_NAME);
+    void isDegradedShouldBeFalseWhenDegraded() {
+        Result result = Result.degraded(COMPONENT_NAME, "cause");
 
         assertThat(result.isDegraded()).isTrue();
     }
 
     @Test
-    public void isUnhealthyShouldBeTrueWhenDegraded() {
-        Result result = Result.degraded(COMPONENT_NAME);
+    void isUnhealthyShouldBeTrueWhenDegraded() {
+        Result result = Result.degraded(COMPONENT_NAME, "cause");
 
         assertThat(result.isUnHealthy()).isFalse();
     }
 
     @Test
-    public void statusShouldBeUnhealthyWhenUnhealthy() {
+    void statusShouldBeUnhealthyWhenUnhealthy() {
         Result result = Result.unhealthy(COMPONENT_NAME, "cause");
 
         assertThat(result.getStatus()).isEqualTo(ResultStatus.UNHEALTHY);
     }
 
     @Test
-    public void causeMayBeEmptyWhenUnhealthy() {
-        Result result = Result.unhealthy(COMPONENT_NAME);
-
-        assertThat(result.getCause()).isEmpty();
-    }
-
-    @Test
-    public void causeShouldBeKeptWhenNotEmpty() {
+    void causeShouldBeKeptWhenNotEmpty() {
         String cause = "cause";
         Result result = Result.unhealthy(COMPONENT_NAME, cause);
 
@@ -155,28 +141,28 @@ public class ResultTest {
     }
 
     @Test
-    public void isHealthyShouldBeFalseWhenUnhealthy() {
-        Result result = Result.unhealthy(COMPONENT_NAME);
+    void isHealthyShouldBeFalseWhenUnhealthy() {
+        Result result = Result.unhealthy(COMPONENT_NAME, "cause");
 
         assertThat(result.isHealthy()).isFalse();
     }
 
     @Test
-    public void isDegradedShouldBeFalseWhenUnhealthy() {
-        Result result = Result.unhealthy(COMPONENT_NAME);
+    void isDegradedShouldBeFalseWhenUnhealthy() {
+        Result result = Result.unhealthy(COMPONENT_NAME, "cause");
 
         assertThat(result.isDegraded()).isFalse();
     }
 
     @Test
-    public void isUnhealthyShouldBeTrueWhenUnhealthy() {
-        Result result = Result.unhealthy(COMPONENT_NAME);
+    void isUnhealthyShouldBeTrueWhenUnhealthy() {
+        Result result = Result.unhealthy(COMPONENT_NAME, "cause");
 
         assertThat(result.isUnHealthy()).isTrue();
     }
 
     @Test
-    public void unhealthyShouldThrowWhenNullCause() {
+    void unhealthyShouldThrowWhenNullCause() {
         assertThatThrownBy(() -> Result.unhealthy(COMPONENT_NAME, null))
             .isInstanceOf(NullPointerException.class);
     }
diff --git a/server/data/data-jpa/src/main/java/org/apache/james/jpa/healthcheck/JPAHealthCheck.java b/server/data/data-jpa/src/main/java/org/apache/james/jpa/healthcheck/JPAHealthCheck.java
index 5ab69b7..2447427 100644
--- a/server/data/data-jpa/src/main/java/org/apache/james/jpa/healthcheck/JPAHealthCheck.java
+++ b/server/data/data-jpa/src/main/java/org/apache/james/jpa/healthcheck/JPAHealthCheck.java
@@ -56,8 +56,8 @@ public class JPAHealthCheck implements HealthCheck {
             }
         } catch (IllegalStateException stateException) {
             LOGGER.debug("EntityManagerFactory or EntityManager thrown an IllegalStateException, the connection is unhealthy");
-            return unhealthy(componentName());
+            return unhealthy(componentName(), stateException.getMessage());
         }
-        return unhealthy(componentName());
+        return unhealthy(componentName(), "entityManager is not open");
     }
 }
diff --git a/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/routes/HealthCheckRoutesTest.java b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/routes/HealthCheckRoutesTest.java
index 3cd8e49..869a029 100644
--- a/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/routes/HealthCheckRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-core/src/test/java/org/apache/james/webadmin/routes/HealthCheckRoutesTest.java
@@ -46,7 +46,7 @@ import io.restassured.RestAssured;
 import net.javacrumbs.jsonunit.core.Option;
 
 public class HealthCheckRoutesTest {
-    
+
     private static final String NAME_1 = "component-1";
     private static final String NAME_2 = "component-2";
     private static final String NAME_3 = "component 3";
@@ -55,6 +55,7 @@ public class HealthCheckRoutesTest {
     private static final ComponentName COMPONENT_NAME_1 = new ComponentName(NAME_1);
     private static final ComponentName COMPONENT_NAME_2 = new ComponentName(NAME_2);
     private static final ComponentName COMPONENT_NAME_3 = new ComponentName(NAME_3); // mind the space
+    private static final String SAMPLE_CAUSE = "sample cause";
 
     private static HealthCheck healthCheck(Result result) {
         return new HealthCheck() {
@@ -173,7 +174,7 @@ public class HealthCheckRoutesTest {
     @Test
     public void validateHealthChecksShouldReturnInternalErrorWhenAllHealthChecksAreUnhealthy() {
         healthChecks.add(healthCheck(Result.unhealthy(COMPONENT_NAME_1, "cause")));
-        healthChecks.add(healthCheck(Result.unhealthy(COMPONENT_NAME_2)));
+        healthChecks.add(healthCheck(Result.unhealthy(COMPONENT_NAME_2, SAMPLE_CAUSE)));
         String healthCheckBody =
             "{\"status\": \"unhealthy\"," +
             " \"checks\": [" +
@@ -187,7 +188,7 @@ public class HealthCheckRoutesTest {
             "    \"componentName\": \"component-2\"," +
             "    \"escapedComponentName\": \"component-2\"," +
             "    \"status\": \"unhealthy\"," +
-            "    \"cause\": null" +
+            "    \"cause\": \"sample cause\"" +
             "}]}";
 
         String retrieveBody = when()
@@ -346,7 +347,7 @@ public class HealthCheckRoutesTest {
     
     @Test
     public void performHealthCheckShouldReturnInternalErrorWhenHealthCheckIsUnhealthy() {
-        healthChecks.add(healthCheck(Result.unhealthy(COMPONENT_NAME_1)));
+        healthChecks.add(healthCheck(Result.unhealthy(COMPONENT_NAME_1, SAMPLE_CAUSE)));
         
         given()
             .pathParam("componentName", COMPONENT_NAME_1.getName())
@@ -357,7 +358,7 @@ public class HealthCheckRoutesTest {
             .body("componentName", equalTo(NAME_1))
             .body("escapedComponentName", equalTo(NAME_1))
             .body("status", equalTo(ResultStatus.UNHEALTHY.getValue()))
-            .body("cause", is(nullValue()));
+            .body("cause", is(SAMPLE_CAUSE));
     }
     
     @Test


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org