TSK-1543: Throw expressive error when REST parameter cannot be converted

rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/TaskanaRestExceptionHandler.java

@@ -3,8 +3,10 @@ package pro.taskana.common.rest;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
 import org.springframework.dao.DuplicateKeyException;
+import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
+import org.springframework.validation.BindException;
 import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.context.request.WebRequest;
@@ -33,113 +35,113 @@ import pro.taskana.workbasket.api.exceptions.WorkbasketInUseException;
 public class TaskanaRestExceptionHandler extends ResponseEntityExceptionHandler {
 
   @ExceptionHandler(InvalidArgumentException.class)
-  protected ResponseEntity<TaskanaErrorData> handleInvalidArgument(
+  protected ResponseEntity<Object> handleInvalidArgument(
       InvalidArgumentException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.BAD_REQUEST, false);
   }
 
   @ExceptionHandler(NotAuthorizedException.class)
-  protected ResponseEntity<TaskanaErrorData> handleNotAuthorized(
-      NotAuthorizedException ex, WebRequest req) {
+  protected ResponseEntity<Object> handleNotAuthorized(NotAuthorizedException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.FORBIDDEN);
   }
 
   @ExceptionHandler(NotFoundException.class)
-  protected ResponseEntity<TaskanaErrorData> handleTaskNotFound(
-      NotFoundException ex, WebRequest req) {
+  protected ResponseEntity<Object> handleTaskNotFound(NotFoundException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.NOT_FOUND);
   }
 
   @ExceptionHandler(TaskAlreadyExistException.class)
-  protected ResponseEntity<TaskanaErrorData> handleTaskAlreadyExist(
+  protected ResponseEntity<Object> handleTaskAlreadyExist(
       TaskAlreadyExistException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(NotAuthorizedToQueryWorkbasketException.class)
-  protected ResponseEntity<TaskanaErrorData> handleNotAuthorizedToQueryWorkbasket(
+  protected ResponseEntity<Object> handleNotAuthorizedToQueryWorkbasket(
       NotAuthorizedToQueryWorkbasketException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.FORBIDDEN);
   }
 
   @ExceptionHandler(InvalidStateException.class)
-  protected ResponseEntity<TaskanaErrorData> handleInvalidState(
-      InvalidStateException ex, WebRequest req) {
+  protected ResponseEntity<Object> handleInvalidState(InvalidStateException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(InvalidOwnerException.class)
-  protected ResponseEntity<TaskanaErrorData> handleInvalidOwner(
-      InvalidOwnerException ex, WebRequest req) {
+  protected ResponseEntity<Object> handleInvalidOwner(InvalidOwnerException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(ClassificationAlreadyExistException.class)
-  protected ResponseEntity<TaskanaErrorData> handleClassificationAlreadyExist(
+  protected ResponseEntity<Object> handleClassificationAlreadyExist(
       ClassificationAlreadyExistException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(DuplicateKeyException.class)
-  protected ResponseEntity<TaskanaErrorData> handleDuplicateKey(
-      DuplicateKeyException ex, WebRequest req) {
+  protected ResponseEntity<Object> handleDuplicateKey(DuplicateKeyException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(ConcurrencyException.class)
-  protected ResponseEntity<TaskanaErrorData> handleConcurrencyException(
+  protected ResponseEntity<Object> handleConcurrencyException(
       ConcurrencyException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(WorkbasketInUseException.class)
-  protected ResponseEntity<TaskanaErrorData> handleWorkbasketInUse(
+  protected ResponseEntity<Object> handleWorkbasketInUse(
       WorkbasketInUseException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.LOCKED);
   }
 
   @ExceptionHandler(WorkbasketAlreadyExistException.class)
-  protected ResponseEntity<TaskanaErrorData> handleWorkbasketAlreadyExist(
+  protected ResponseEntity<Object> handleWorkbasketAlreadyExist(
       WorkbasketAlreadyExistException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(WorkbasketAccessItemAlreadyExistException.class)
-  protected ResponseEntity<TaskanaErrorData> handleWorkbasketAccessItemAlreadyExist(
+  protected ResponseEntity<Object> handleWorkbasketAccessItemAlreadyExist(
       WorkbasketAccessItemAlreadyExistException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.CONFLICT);
   }
 
   @ExceptionHandler(InvalidWorkbasketException.class)
-  protected ResponseEntity<TaskanaErrorData> handleInvalidWorkbasket(
+  protected ResponseEntity<Object> handleInvalidWorkbasket(
       InvalidWorkbasketException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.BAD_REQUEST);
   }
 
   @ExceptionHandler(DomainNotFoundException.class)
-  protected ResponseEntity<TaskanaErrorData> handleDomainNotFound(
+  protected ResponseEntity<Object> handleDomainNotFound(
       DomainNotFoundException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.BAD_REQUEST);
   }
 
   @ExceptionHandler(MaxUploadSizeExceededException.class)
-  protected ResponseEntity<TaskanaErrorData> handleMaxUploadSizeExceededException(
+  protected ResponseEntity<Object> handleMaxUploadSizeExceededException(
       MaxUploadSizeExceededException ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.PAYLOAD_TOO_LARGE);
   }
 
+  @Override
+  protected ResponseEntity<Object> handleBindException(
+      BindException ex, HttpHeaders headers, HttpStatus status, WebRequest request) {
+    return buildResponse(ex, request, HttpStatus.BAD_REQUEST);
+  }
+
   @ExceptionHandler(Exception.class)
-  protected ResponseEntity<TaskanaErrorData> handleGeneralException(Exception ex, WebRequest req) {
+  protected ResponseEntity<Object> handleGeneralException(Exception ex, WebRequest req) {
     return buildResponse(ex, req, HttpStatus.BAD_REQUEST);
   }
 
-  private ResponseEntity<TaskanaErrorData> buildResponse(
-      Exception ex, WebRequest req, HttpStatus status) {
+  private ResponseEntity<Object> buildResponse(Throwable ex, WebRequest req, HttpStatus status) {
     return buildResponse(ex, req, status, true);
   }
 
-  private ResponseEntity<TaskanaErrorData> buildResponse(
-      Exception ex, WebRequest req, HttpStatus status, boolean logExceptionOnError) {
+  private ResponseEntity<Object> buildResponse(
+      Throwable ex, WebRequest req, HttpStatus status, boolean logExceptionOnError) {
     TaskanaErrorData errorData = new TaskanaErrorData(status, ex, req);
     if (logExceptionOnError) {
       logger.error(

rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/models/TaskanaErrorData.java

@@ -14,7 +14,7 @@ public class TaskanaErrorData {
   private final String message;
   private String path;
 
-  public TaskanaErrorData(HttpStatus stat, Exception ex, WebRequest req) {
+  public TaskanaErrorData(HttpStatus stat, Throwable ex, WebRequest req) {
     this.timestamp = new Date();
     this.status = stat.value();
     this.error = stat.name();

rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/GeneralExceptionHandlingTest.java

@@ -10,15 +10,21 @@ import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpMethod;
 import org.springframework.web.client.HttpClientErrorException;
+import org.springframework.web.client.HttpClientErrorException.BadRequest;
 
 import pro.taskana.classification.rest.models.ClassificationSummaryPagedRepresentationModel;
 import pro.taskana.common.test.rest.RestHelper;
 import pro.taskana.common.test.rest.TaskanaSpringBootTest;
+import pro.taskana.workbasket.rest.models.WorkbasketSummaryPagedRepresentationModel;
 
 /** Test general Exception Handling. */
 @TaskanaSpringBootTest
 class GeneralExceptionHandlingTest {
 
+  private static final ParameterizedTypeReference<WorkbasketSummaryPagedRepresentationModel>
+      WORKBASKET_SUMMARY_PAGE_MODEL_TYPE =
+          new ParameterizedTypeReference<WorkbasketSummaryPagedRepresentationModel>() {};
+
   private final RestHelper restHelper;
 
   @Autowired
@@ -45,4 +51,33 @@ class GeneralExceptionHandlingTest {
         .isInstanceOf(HttpClientErrorException.class)
         .hasMessageContaining("non-existing-id");
   }
+
+  @Test
+  void should_ThrowExpressiveError_When_InvalidEnumValueIsProvided() {
+    String url = restHelper.toUrl(RestEndpoints.URL_WORKBASKET) + "?type=GROU";
+    HttpEntity<String> auth = new HttpEntity<>(restHelper.getHeadersAdmin());
+
+    ThrowingCallable httpCall =
+        () -> {
+          TEMPLATE.exchange(url, HttpMethod.GET, auth, WORKBASKET_SUMMARY_PAGE_MODEL_TYPE);
+        };
+
+    // Inside the complete message of the exception, thrown when querying the REST controller with
+    // a wrong enum value (for example 'GROU' instead of 'GROUP'), there will be found following
+    // information about this issue:
+    //
+    //     nested exception is org.springframework.core.convert.ConversionFailedException: Failed to
+    //     convert from type [java.lang.String] to type [pro.taskana.workbasket.api.WorkbasketType]
+    //     for value 'GROU'; nested exception is java.lang.IllegalArgumentException: No enum
+    //     constant pro.taskana.workbasket.api.WorkbasketType.GROU
+    //
+    // Unfortunately the message of the exception thrown here is cut and thus there is no info
+    // about this problem. In case of querying the REST controller directly there will.
+    assertThatThrownBy(httpCall)
+        .isInstanceOf(BadRequest.class)
+        .hasMessageContaining(
+            "\"status\":400,\"error\":\"BAD_REQUEST\",\"exception\":\"org.springframework.web."
+                + "method.annotation.ModelAttributeMethodProcessor$1\",\"message\":\"org."
+                + "springframework");
+  }
 }