diff --git a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java index 8e16515d0..890ddcecf 100644 --- a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java @@ -234,10 +234,10 @@ public class PageableHandlerMethodArgumentResolver implements HandlerMethodArgum String pageString = webRequest.getParameter(getParameterNameToUse(pageParameterName, methodParameter)); String pageSizeString = webRequest.getParameter(getParameterNameToUse(sizeParameterName, methodParameter)); - int page = StringUtils.hasText(pageString) ? Integer.parseInt(pageString) - (oneIndexedParameters ? 1 : 0) - : defaultOrFallback.getPageNumber(); - int pageSize = StringUtils.hasText(pageSizeString) ? Integer.parseInt(pageSizeString) : defaultOrFallback - .getPageSize(); + int page = StringUtils.hasText(pageString) ? parseAndApplyBoundaries(pageString, 0, Integer.MAX_VALUE) + - (oneIndexedParameters ? 1 : 0) : defaultOrFallback.getPageNumber(); + int pageSize = StringUtils.hasText(pageSizeString) ? parseAndApplyBoundaries(pageSizeString, 0, maxPageSize) + : defaultOrFallback.getPageSize(); // Limit lower bound pageSize = pageSize < 1 ? defaultOrFallback.getPageSize() : pageSize; @@ -301,4 +301,23 @@ public class PageableHandlerMethodArgumentResolver implements HandlerMethodArgum return new PageRequest(defaultPageNumber, defaultPageSize, defaults.direction(), defaults.sort()); } + + /** + * Tries to parse the given {@link String} into an integer and applies the given boundaries. Will return the lower + * boundary if the {@link String} cannot be parsed. + * + * @param parameter + * @param lower + * @param upper + * @return + */ + private static int parseAndApplyBoundaries(String parameter, int lower, int upper) { + + try { + int parsed = Integer.parseInt(parameter); + return parsed < lower ? lower : parsed > upper ? upper : parsed; + } catch (NumberFormatException e) { + return lower; + } + } } diff --git a/src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolver.java index 228a60736..e9361e83a 100644 --- a/src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/SortHandlerMethodArgumentResolver.java @@ -251,7 +251,13 @@ public class SortHandlerMethodArgumentResolver implements HandlerMethodArgumentR continue; } - allOrders.add(new Order(direction, elements[i])); + String property = elements[i]; + + if (!StringUtils.hasText(property)) { + continue; + } + + allOrders.add(new Order(direction, property)); } } diff --git a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java index 4b14801f2..0e27ed6b6 100644 --- a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java @@ -15,6 +15,8 @@ */ package org.springframework.data.web; +import static org.springframework.data.web.PageableHandlerMethodArgumentResolver.*; + import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Qualifier; @@ -97,8 +99,7 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa request.addParameter("page", "0"); request.addParameter("size", "0"); - assertSupportedAndResult(supportedMethodParameter, PageableHandlerMethodArgumentResolver.DEFAULT_PAGE_REQUEST, - request); + assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request); } /** @@ -112,7 +113,43 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa exception.expect(IllegalStateException.class); exception.expectMessage("invalidDefaultPageSize"); - assertSupportedAndResult(parameter, PageableHandlerMethodArgumentResolver.DEFAULT_PAGE_REQUEST); + assertSupportedAndResult(parameter, DEFAULT_PAGE_REQUEST); + } + + /** + * @see DATACMNS-408 + */ + @Test + public void fallsBackToFirstPageIfNegativePageNumberIsGiven() throws Exception { + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("page", "-1"); + + assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request); + } + + /** + * @see DATACMNS-408 + */ + @Test + public void pageParamIsNotNumeric() throws Exception { + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("page", "a"); + + assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request); + } + + /** + * @see DATACMNS-408 + */ + @Test + public void sizeParamIsNotNumeric() throws Exception { + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("size", "a"); + + assertSupportedAndResult(supportedMethodParameter, DEFAULT_PAGE_REQUEST, request); } @Override diff --git a/src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java index ed0a80ea9..0d5dab5e7 100644 --- a/src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/SortHandlerMethodArgumentResolverUnitTests.java @@ -17,7 +17,11 @@ package org.springframework.data.web; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; +import static org.springframework.data.domain.Sort.Direction.*; +import javax.servlet.http.HttpServletRequest; + +import org.junit.BeforeClass; import org.junit.Test; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.core.MethodParameter; @@ -40,6 +44,13 @@ import org.springframework.web.context.request.ServletWebRequest; */ public class SortHandlerMethodArgumentResolverUnitTests extends SortDefaultUnitTests { + static MethodParameter PARAMETER; + + @BeforeClass + public static void setUp() throws Exception { + PARAMETER = new MethodParameter(Controller.class.getMethod("supportedMethod", Sort.class), 0); + } + /** * @see DATACMNS-351 */ @@ -126,6 +137,61 @@ public class SortHandlerMethodArgumentResolverUnitTests extends SortDefaultUnitT assertThat(result, is(new Sort(Direction.ASC, "firstname", "lastname"))); } + /** + * @see DATACMNS-408 + */ + @Test + public void parsesEmptySortToNull() throws Exception { + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("sort", ""); + + assertThat(resolveSort(request, PARAMETER), is(nullValue())); + } + + /** + * @see DATACMNS-408 + */ + @Test + public void sortParamIsInvalidProperty() throws Exception { + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("sort", ",DESC"); + + assertThat(resolveSort(request, PARAMETER), is(nullValue())); + } + + /** + * @see DATACMNS-408 + */ + @Test + public void sortParamIsInvalidPropertyWhenMultiProperty() throws Exception { + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("sort", "property1,,DESC"); + + assertThat(resolveSort(request, PARAMETER), is(new Sort(DESC, "property1"))); + } + + /** + * @see DATACMNS-408 + */ + @Test + public void sortParamIsEmptyWhenMultiParams() throws Exception { + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("sort", "property,DESC"); + request.addParameter("sort", ""); + + assertThat(resolveSort(request, PARAMETER), is(new Sort(DESC, "property"))); + } + + private static Sort resolveSort(HttpServletRequest request, MethodParameter parameter) throws Exception { + + SortHandlerMethodArgumentResolver resolver = new SortHandlerMethodArgumentResolver(); + return resolver.resolveArgument(parameter, null, new ServletWebRequest(request), null); + } + private static void assertSupportedAndResolvedTo(NativeWebRequest request, MethodParameter parameter, Sort sort) { SortHandlerMethodArgumentResolver resolver = new SortHandlerMethodArgumentResolver();