From 9cc609af97157bda197f4dcd5b7653bf985ee40c Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 17 Apr 2015 14:52:05 +0200 Subject: [PATCH] DATAREST-522 - Improved exception handling in DelegatingHandlerMapping. We now also handle HttpRequestMethodNotSupportedException and UnsatisfiedServletRequestParameterException and traverse the chain before eventually rejecting the mapping attempt with the last exception found. Related tickets: DATAREST-409. --- .../support/DelegatingHandlerMapping.java | 9 ++++-- .../DelegatingHandlerMappingUnitTests.java | 30 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java index eec4d7ca0..9790842de 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMapping.java @@ -21,8 +21,9 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.core.Ordered; import org.springframework.util.Assert; -import org.springframework.web.HttpMediaTypeException; import org.springframework.web.HttpMediaTypeNotAcceptableException; +import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.UnsatisfiedServletRequestParameterException; import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.HandlerMapping; @@ -65,7 +66,7 @@ public class DelegatingHandlerMapping implements HandlerMapping, Ordered { @Override public HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception { - HttpMediaTypeException exception = null; + Exception exception = null; for (HandlerMapping delegate : delegates) { @@ -79,6 +80,10 @@ public class DelegatingHandlerMapping implements HandlerMapping, Ordered { } catch (HttpMediaTypeNotAcceptableException o_O) { exception = o_O; + } catch (HttpRequestMethodNotSupportedException o_O) { + exception = o_O; + } catch (UnsatisfiedServletRequestParameterException o_O) { + exception = o_O; } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java index 7771bcba3..6f6460455 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/support/DelegatingHandlerMappingUnitTests.java @@ -15,6 +15,7 @@ */ package org.springframework.data.rest.webmvc.support; +import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.*; @@ -27,6 +28,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.web.HttpMediaTypeNotAcceptableException; +import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.UnsatisfiedServletRequestParameterException; import org.springframework.web.servlet.HandlerMapping; /** @@ -42,23 +45,34 @@ public class DelegatingHandlerMappingUnitTests { @Mock HttpServletRequest request; /** - * @see DATAREST-490 + * @see DATAREST-490, DATAREST-522 */ @Test + public void consultsAllHandlerMappingsAndThrowsExceptionEventually() throws Exception { + + DelegatingHandlerMapping mapping = new DelegatingHandlerMapping(Arrays.asList(first, second)); + + assertHandlerTriedButExceptionThrown(mapping, HttpMediaTypeNotAcceptableException.class); + assertHandlerTriedButExceptionThrown(mapping, HttpRequestMethodNotSupportedException.class); + assertHandlerTriedButExceptionThrown(mapping, UnsatisfiedServletRequestParameterException.class); + } + @SuppressWarnings("unchecked") - public void testname() throws Exception { + private final void assertHandlerTriedButExceptionThrown(HandlerMapping mapping, Class type) + throws Exception { - HandlerMapping handlerMapping = new DelegatingHandlerMapping(Arrays.asList(first, second)); - - when(first.getHandler(request)).thenThrow(HttpMediaTypeNotAcceptableException.class); + when(first.getHandler(request)).thenThrow(type); try { - handlerMapping.getHandler(request); - fail(String.format("Expected %s!", HttpMediaTypeNotAcceptableException.class.getSimpleName())); + mapping.getHandler(request); + fail(String.format("Expected %s!", type.getSimpleName())); - } catch (HttpMediaTypeNotAcceptableException o_O) { + } catch (Exception o_O) { + assertThat(o_O, is(instanceOf(type))); verify(second, times(1)).getHandler(request); + } finally { + reset(first, second); } } }