From 621dfeb51b5e963957db9d624b80138b9ab5a6e0 Mon Sep 17 00:00:00 2001 From: kakawait Date: Thu, 4 Dec 2014 23:13:46 +0100 Subject: [PATCH] Extract @ExceptionHandler method into @ControllerAdvice class in order to allow easy overriding --- .../AbstractRepositoryRestController.java | 159 +---------------- .../rest/webmvc/GlobalExceptionHandler.java | 160 ++++++++++++++++++ .../RepositoryRestMvcConfiguration.java | 6 + .../support/ControllerAdviceConfig.java | 32 ++++ .../support/ControllerAdviceWebTests.java | 29 ++++ 5 files changed, 235 insertions(+), 151 deletions(-) create mode 100644 spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/GlobalExceptionHandler.java create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceConfig.java create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceWebTests.java diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/AbstractRepositoryRestController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/AbstractRepositoryRestController.java index b5aa4f143..e58d4a2c1 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/AbstractRepositoryRestController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/AbstractRepositoryRestController.java @@ -15,53 +15,33 @@ */ package org.springframework.data.rest.webmvc; -import static org.springframework.data.rest.webmvc.ControllerUtils.*; - -import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Locale; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.context.MessageSource; -import org.springframework.context.MessageSourceAware; -import org.springframework.context.support.MessageSourceAccessor; -import org.springframework.core.convert.ConversionFailedException; -import org.springframework.dao.DataIntegrityViolationException; -import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Page; -import org.springframework.data.rest.core.RepositoryConstraintViolationException; import org.springframework.data.rest.core.mapping.ResourceMetadata; -import org.springframework.data.rest.webmvc.support.ETagDoesntMatchException; -import org.springframework.data.rest.webmvc.support.ExceptionMessage; -import org.springframework.data.rest.webmvc.support.RepositoryConstraintViolationExceptionMessage; import org.springframework.data.web.PagedResourcesAssembler; import org.springframework.hateoas.Link; import org.springframework.hateoas.Resource; import org.springframework.hateoas.Resources; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpMethod; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; -import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.util.Assert; -import org.springframework.web.HttpRequestMethodNotSupportedException; -import org.springframework.web.bind.annotation.ExceptionHandler; -import org.springframework.web.bind.annotation.ResponseBody; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.springframework.data.rest.webmvc.ControllerUtils.EMPTY_RESOURCE_LIST; /** * @author Jon Brisbin * @author Oliver Gierke + * @author Thibaud Lepretre */ @SuppressWarnings({ "rawtypes" }) -class AbstractRepositoryRestController implements MessageSourceAware { +class AbstractRepositoryRestController { private static final Logger LOG = LoggerFactory.getLogger(AbstractRepositoryRestController.class); private final PagedResourcesAssembler pagedResourcesAssembler; - private MessageSourceAccessor messageSourceAccessor; /** * Creates a new {@link AbstractRepositoryRestController} for the given {@link PagedResourcesAssembler}. @@ -74,129 +54,6 @@ public AbstractRepositoryRestController(PagedResourcesAssembler pagedRes this.pagedResourcesAssembler = pagedResourcesAssembler; } - /* - * (non-Javadoc) - * @see org.springframework.context.MessageSourceAware#setMessageSource(org.springframework.context.MessageSource) - */ - @Override - public void setMessageSource(MessageSource messageSource) { - this.messageSourceAccessor = new MessageSourceAccessor(messageSource); - } - - @ExceptionHandler({ NullPointerException.class }) - @ResponseBody - public ResponseEntity handleNPE(NullPointerException npe) { - return errorResponse(npe, HttpStatus.INTERNAL_SERVER_ERROR); - } - - @ExceptionHandler({ ResourceNotFoundException.class }) - @ResponseBody - public ResponseEntity handleNotFound() { - return notFound(); - } - - @ExceptionHandler({ HttpMessageNotReadableException.class }) - @ResponseBody - public ResponseEntity handleNotReadable(HttpMessageNotReadableException e) { - return badRequest(e); - } - - /** - * Handle failures commonly thrown from code tries to read incoming data and convert or cast it to the right type. - * - * @param t - * @return - */ - @ExceptionHandler({ InvocationTargetException.class, IllegalArgumentException.class, ClassCastException.class, - ConversionFailedException.class }) - @ResponseBody - public ResponseEntity handleMiscFailures(Throwable t) { - if (null != t.getCause() && t.getCause() instanceof ResourceNotFoundException) { - return notFound(); - } - return badRequest(t); - } - - @ExceptionHandler({ RepositoryConstraintViolationException.class }) - @ResponseBody - public ResponseEntity handleRepositoryConstraintViolationException(Locale locale, - RepositoryConstraintViolationException rcve) { - - return response(null, new RepositoryConstraintViolationExceptionMessage(rcve, messageSourceAccessor), - HttpStatus.BAD_REQUEST); - } - - /** - * Send a 409 Conflict in case of concurrent modification. - * - * @param ex - * @return - */ - @ExceptionHandler({ OptimisticLockingFailureException.class, DataIntegrityViolationException.class }) - public ResponseEntity handleConflict(Exception ex) { - return errorResponse(null, ex, HttpStatus.CONFLICT); - } - - /** - * Send {@code 405 Method Not Allowed} and include the supported {@link HttpMethod}s in the {@code Allow} header. - * - * @param o_O - * @return - */ - @ExceptionHandler - public ResponseEntity handle(HttpRequestMethodNotSupportedException o_O) { - - HttpHeaders headers = new HttpHeaders(); - headers.setAllow(o_O.getSupportedHttpMethods()); - - return new ResponseEntity(headers, HttpStatus.METHOD_NOT_ALLOWED); - } - - @ExceptionHandler - public ResponseEntity handle(ETagDoesntMatchException o_O) { - - HttpHeaders headers = o_O.getExpectedETag().addTo(new HttpHeaders()); - return new ResponseEntity(headers, HttpStatus.PRECONDITION_FAILED); - } - - protected ResponseEntity notFound() { - return notFound(null, null); - } - - protected ResponseEntity notFound(HttpHeaders headers, T body) { - return response(headers, body, HttpStatus.NOT_FOUND); - } - - protected ResponseEntity badRequest(T throwable) { - return badRequest(null, throwable); - } - - protected ResponseEntity badRequest(HttpHeaders headers, T throwable) { - return errorResponse(headers, throwable, HttpStatus.BAD_REQUEST); - } - - public ResponseEntity errorResponse(T throwable, HttpStatus status) { - return errorResponse(null, throwable, status); - } - - public ResponseEntity errorResponse(HttpHeaders headers, T throwable, - HttpStatus status) { - if (null != throwable && null != throwable.getMessage()) { - LOG.error(throwable.getMessage(), throwable); - return response(headers, new ExceptionMessage(throwable), status); - } else { - return response(headers, null, status); - } - } - - public ResponseEntity response(HttpHeaders headers, T body, HttpStatus status) { - HttpHeaders hdrs = new HttpHeaders(); - if (null != headers) { - hdrs.putAll(headers); - } - return new ResponseEntity(body, hdrs, status); - } - protected Link resourceLink(RootResourceInformation resourceLink, Resource resource) { ResourceMetadata repoMapping = resourceLink.getResourceMetadata(); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/GlobalExceptionHandler.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/GlobalExceptionHandler.java new file mode 100644 index 000000000..20ccaf17d --- /dev/null +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/GlobalExceptionHandler.java @@ -0,0 +1,160 @@ +package org.springframework.data.rest.webmvc; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.context.MessageSource; +import org.springframework.context.MessageSourceAware; +import org.springframework.context.support.MessageSourceAccessor; +import org.springframework.core.convert.ConversionFailedException; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.data.rest.core.RepositoryConstraintViolationException; +import org.springframework.data.rest.webmvc.support.ETagDoesntMatchException; +import org.springframework.data.rest.webmvc.support.ExceptionMessage; +import org.springframework.data.rest.webmvc.support.RepositoryConstraintViolationExceptionMessage; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.ResponseBody; + +import java.lang.reflect.InvocationTargetException; +import java.util.Locale; + +/** + * @author Thibaud Lepretre + */ +@ControllerAdvice +public class GlobalExceptionHandler implements MessageSourceAware { + + private static final Logger LOG = LoggerFactory.getLogger(GlobalExceptionHandler.class); + + + private MessageSourceAccessor messageSourceAccessor; + + /* + * (non-Javadoc) + * @see org.springframework.context.MessageSourceAware#setMessageSource(org.springframework.context.MessageSource) + */ + @Override + public void setMessageSource(MessageSource messageSource) { + this.messageSourceAccessor = new MessageSourceAccessor(messageSource); + } + + @ExceptionHandler({ NullPointerException.class }) + @ResponseBody + public ResponseEntity handleNPE(NullPointerException npe) { + return errorResponse(npe, HttpStatus.INTERNAL_SERVER_ERROR); + } + + @ExceptionHandler({ ResourceNotFoundException.class }) + @ResponseBody + public ResponseEntity handleNotFound() { + return notFound(); + } + + @ExceptionHandler({ HttpMessageNotReadableException.class }) + @ResponseBody + public ResponseEntity handleNotReadable(HttpMessageNotReadableException e) { + return badRequest(e); + } + + /** + * Handle failures commonly thrown from code tries to read incoming data and convert or cast it to the right type. + * + * @param t + * @return + */ + @ExceptionHandler({ InvocationTargetException.class, IllegalArgumentException.class, ClassCastException.class, + ConversionFailedException.class }) + @ResponseBody + public ResponseEntity handleMiscFailures(Throwable t) { + if (null != t.getCause() && t.getCause() instanceof ResourceNotFoundException) { + return notFound(); + } + return badRequest(t); + } + + @ExceptionHandler({ RepositoryConstraintViolationException.class }) + @ResponseBody + public ResponseEntity handleRepositoryConstraintViolationException(Locale locale, + RepositoryConstraintViolationException rcve) { + + return response(null, new RepositoryConstraintViolationExceptionMessage(rcve, messageSourceAccessor), + HttpStatus.BAD_REQUEST); + } + + /** + * Send a 409 Conflict in case of concurrent modification. + * + * @param ex + * @return HTTP Status 409 ResponseEntity + */ + @ExceptionHandler({ OptimisticLockingFailureException.class, DataIntegrityViolationException.class }) + public ResponseEntity handleConflict(Exception ex) { + return errorResponse(null, ex, HttpStatus.CONFLICT); + } + + /** + * Send {@code 405 Method Not Allowed} and include the supported {@link org.springframework.http.HttpMethod}s in the {@code Allow} header. + * + * @param o_O + * @return HTTP Status 405 ResponseEntity + */ + @ExceptionHandler + public ResponseEntity handle(HttpRequestMethodNotSupportedException o_O) { + + HttpHeaders headers = new HttpHeaders(); + headers.setAllow(o_O.getSupportedHttpMethods()); + + return new ResponseEntity(headers, HttpStatus.METHOD_NOT_ALLOWED); + } + + @ExceptionHandler + public ResponseEntity handle(ETagDoesntMatchException o_O) { + + HttpHeaders headers = o_O.getExpectedETag().addTo(new HttpHeaders()); + return new ResponseEntity(headers, HttpStatus.PRECONDITION_FAILED); + } + + protected ResponseEntity notFound() { + return notFound(null, null); + } + + protected ResponseEntity notFound(HttpHeaders headers, T body) { + return response(headers, body, HttpStatus.NOT_FOUND); + } + + protected ResponseEntity badRequest(T throwable) { + return badRequest(null, throwable); + } + + protected ResponseEntity badRequest(HttpHeaders headers, T throwable) { + return errorResponse(headers, throwable, HttpStatus.BAD_REQUEST); + } + + public ResponseEntity errorResponse(T throwable, HttpStatus status) { + return errorResponse(null, throwable, status); + } + + public ResponseEntity errorResponse(HttpHeaders headers, T throwable, + HttpStatus status) { + if (null != throwable && null != throwable.getMessage()) { + LOG.error(throwable.getMessage(), throwable); + return response(headers, new ExceptionMessage(throwable), status); + } else { + return response(headers, null, status); + } + } + + public ResponseEntity response(HttpHeaders headers, T body, HttpStatus status) { + HttpHeaders hdrs = new HttpHeaders(); + if (null != headers) { + hdrs.putAll(headers); + } + return new ResponseEntity(body, hdrs, status); + } +} diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java index b163fa9b7..7f084de69 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java @@ -68,6 +68,7 @@ import org.springframework.data.rest.webmvc.BaseUri; import org.springframework.data.rest.webmvc.BaseUriAwareController; import org.springframework.data.rest.webmvc.BaseUriAwareHandlerMapping; +import org.springframework.data.rest.webmvc.GlobalExceptionHandler; import org.springframework.data.rest.webmvc.RepositoryRestController; import org.springframework.data.rest.webmvc.RepositoryRestHandlerAdapter; import org.springframework.data.rest.webmvc.RepositoryRestHandlerMapping; @@ -547,6 +548,11 @@ public ExceptionHandlerExceptionResolver exceptionHandlerExceptionResolver() { return er; } + @Bean + public GlobalExceptionHandler globalExceptionHandler() { + return new GlobalExceptionHandler(); + } + @Bean public RepositoryInvokerFactory repositoryInvokerFactory() { return new DefaultRepositoryInvokerFactory(repositories(), defaultConversionService()); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceConfig.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceConfig.java new file mode 100644 index 000000000..710545fd8 --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceConfig.java @@ -0,0 +1,32 @@ +package org.springframework.data.rest.webmvc.support; + +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.data.rest.webmvc.jpa.JpaRepositoryConfig; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; + +/** + * @author Thibaud Lepretre + */ +@Configuration +@Import(JpaRepositoryConfig.class) +public class ControllerAdviceConfig { + @Order(Ordered.HIGHEST_PRECEDENCE) + @ControllerAdvice + public static class CustomGlobalConfiguration { + @ExceptionHandler + public ResponseEntity handle(HttpRequestMethodNotSupportedException o_O) { + HttpHeaders headers = new HttpHeaders(); + headers.setAllow(o_O.getSupportedHttpMethods()); + + return new ResponseEntity(headers, HttpStatus.INTERNAL_SERVER_ERROR); + } + } +} diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceWebTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceWebTests.java new file mode 100644 index 000000000..edd78cca2 --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/ControllerAdviceWebTests.java @@ -0,0 +1,29 @@ +package org.springframework.data.rest.webmvc.support; + +import org.junit.Test; +import org.springframework.data.rest.webmvc.AbstractWebIntegrationTests; +import org.springframework.hateoas.Link; +import org.springframework.test.context.ContextConfiguration; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * @author Thibaud Lepretre + */ +@ContextConfiguration(classes = ControllerAdviceConfig.class) +public class ControllerAdviceWebTests extends AbstractWebIntegrationTests { + @Test + public void httpRequestMethodNotSupportedExceptionShouldNowReturnHttpStatus500Over405() throws Exception { + + Link link = client.discoverUnique("addresses"); + + mvc.perform(get(link.getHref())).// + andExpect(status().isInternalServerError()); + } + + @Override + protected Iterable expectedRootLinkRels() { + return null; + } +}