GEODE-6174: Don't catch IllegalArgumentException from the validator (#…

…3226) Added a handler for IllegalArgumentException in the ManagementControllerAdvice to return HttpStatus 400.
apache · Feb 25, 2019 · c10fe07 · c10fe07
1 parent affee69
commit c10fe07
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/...c/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java b/...c/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
@@ -88,11 +88,7 @@ public ClusterManagementResult create(CacheElement config, String group) {
 
     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/...st/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java b/...st/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
@@ -78,10 +78,9 @@ public void elementAlreadyExist() throws Exception {
 
   @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/...ava/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java b/...ava/org/apache/geode/management/internal/rest/controllers/ManagementControllerAdvice.java
@@ -67,6 +67,12 @@ public ResponseEntity<ClusterManagementResult> badRequest(final MalformedObjectN
         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()),