You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2019/02/25 18:12:38 UTC

[geode] branch develop updated: GEODE-6174: Don't catch IllegalArgumentException from the validator (#3226)

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

jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new c10fe07  GEODE-6174: Don't catch IllegalArgumentException from the validator (#3226)
c10fe07 is described below

commit c10fe0700f44ce772ade016419ac582fc8671977
Author: Kenneth Howe <kh...@pivotal.io>
AuthorDate: Mon Feb 25 10:12:28 2019 -0800

    GEODE-6174: Don't catch IllegalArgumentException from the validator (#3226)
    
    Added a handler for IllegalArgumentException in the ManagementControllerAdvice to return
    HttpStatus 400.
---
 .../management/internal/api/LocatorClusterManagementService.java   | 6 +-----
 .../internal/api/LocatorClusterManagementServiceTest.java          | 7 +++----
 .../internal/rest/controllers/ManagementControllerAdvice.java      | 6 ++++++
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
index e18a4eb..8bf25fb 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
@@ -88,11 +88,7 @@ public class LocatorClusterManagementService implements ClusterManagementService
 
     ConfigurationValidator validator = validators.get(config.getClass());
     if (validator != null) {
-      try {
-        validator.validate(config);
-      } catch (IllegalArgumentException e) {
-        return new ClusterManagementResult(false, e.getMessage());
-      }
+      validator.validate(config);
     }
 
 
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
index 8947235..cbe664f 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
@@ -78,10 +78,9 @@ public class LocatorClusterManagementServiceTest {
 
   @Test
   public void validationFailed() throws Exception {
-    result = service.create(regionConfig, "cluster");
-    assertThat(result.isSuccessful()).isFalse();
-    assertThat(result.getPersistenceStatus().getMessage())
-        .contains("Name of the region has to be specified");
+    assertThatThrownBy(() -> service.create(regionConfig, "cluster"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("Name of the region has to be specified");
   }
 
   @Test
diff --git a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
index 9e8265d..937b412 100644
--- a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
+++ b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
@@ -67,6 +67,12 @@ public class ManagementControllerAdvice {
         HttpStatus.BAD_REQUEST);
   }
 
+  @ExceptionHandler(IllegalArgumentException.class)
+  public ResponseEntity<ClusterManagementResult> badRequest(final IllegalArgumentException e) {
+    return new ResponseEntity<>(new ClusterManagementResult(false, e.getMessage()),
+        HttpStatus.BAD_REQUEST);
+  }
+
   @ExceptionHandler(InstanceNotFoundException.class)
   public ResponseEntity<ClusterManagementResult> notFound(final InstanceNotFoundException e) {
     return new ResponseEntity<>(new ClusterManagementResult(false, e.getMessage()),