From dec820d6c233988341b7d1f0af220218c7cf3f30 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 15 Mar 2017 16:57:33 +0100 Subject: [PATCH] DATACMNS-867 - Removed handling of null Pageables. Pageable now exposes a dedicated null-object representing the absence of pagination so that all places that previously handled null values can now rather be more relaxed and assume that a non-null value is given. --- .../springframework/data/domain/Pageable.java | 13 ++++++++++++- .../querydsl/QuerydslPredicateExecutor.java | 2 +- .../PageableHandlerMethodArgumentResolver.java | 14 +++++++------- .../data/web/PageableDefaultUnitTests.java | 4 ++-- ...HandlerMethodArgumentResolverUnitTests.java | 18 ++++++++---------- 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/springframework/data/domain/Pageable.java b/src/main/java/org/springframework/data/domain/Pageable.java index 5d07cf348..978434533 100644 --- a/src/main/java/org/springframework/data/domain/Pageable.java +++ b/src/main/java/org/springframework/data/domain/Pageable.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2013 the original author or authors. + * Copyright 2008-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ */ package org.springframework.data.domain; +import java.util.Optional; + import org.springframework.util.Assert; /** @@ -120,4 +122,13 @@ public interface Pageable { * @return */ boolean hasPrevious(); + + /** + * Returns an {@link Optional} so that it can easily be mapped on. + * + * @return + */ + default Optional toOptional() { + return isUnpaged() ? Optional.empty() : Optional.of(this); + } } diff --git a/src/main/java/org/springframework/data/querydsl/QuerydslPredicateExecutor.java b/src/main/java/org/springframework/data/querydsl/QuerydslPredicateExecutor.java index 66f82a5bc..e394f281f 100644 --- a/src/main/java/org/springframework/data/querydsl/QuerydslPredicateExecutor.java +++ b/src/main/java/org/springframework/data/querydsl/QuerydslPredicateExecutor.java @@ -83,7 +83,7 @@ public interface QuerydslPredicateExecutor { * {@link Page} is returned. * * @param predicate can be {@literal null}. - * @param pageable can be {@literal null}. + * @param pageable must not be {@literal null}. * @return a {@link Page} of entities matching the given {@link Predicate}. */ Page findAll(Predicate predicate, Pageable pageable); diff --git a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java index c66c336f4..18f0300b0 100644 --- a/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/PageableHandlerMethodArgumentResolver.java @@ -54,7 +54,7 @@ public class PageableHandlerMethodArgumentResolver implements PageableArgumentRe private static final int DEFAULT_MAX_PAGE_SIZE = 2000; static final Pageable DEFAULT_PAGE_REQUEST = PageRequest.of(0, 20); - private Optional fallbackPageable = Optional.of(DEFAULT_PAGE_REQUEST); + private Pageable fallbackPageable = DEFAULT_PAGE_REQUEST; private SortArgumentResolver sortResolver; private String pageParameterName = DEFAULT_PAGE_PARAMETER; private String sizeParameterName = DEFAULT_SIZE_PARAMETER; @@ -101,7 +101,7 @@ public class PageableHandlerMethodArgumentResolver implements PageableArgumentRe * * @param fallbackPageable the {@link Pageable} to be used as general fallback. */ - public void setFallbackPageable(Optional fallbackPageable) { + public void setFallbackPageable(Pageable fallbackPageable) { Assert.notNull(fallbackPageable, "Fallback Pageable must not be null!"); @@ -240,7 +240,7 @@ public class PageableHandlerMethodArgumentResolver implements PageableArgumentRe assertPageableUniqueness(methodParameter); - Optional defaultOrFallback = getDefaultFromAnnotationOrFallback(methodParameter); + Optional defaultOrFallback = getDefaultFromAnnotationOrFallback(methodParameter).toOptional(); String pageString = webRequest.getParameter(getParameterNameToUse(pageParameterName, methodParameter)); String pageSizeString = webRequest.getParameter(getParameterNameToUse(sizeParameterName, methodParameter)); @@ -252,8 +252,8 @@ public class PageableHandlerMethodArgumentResolver implements PageableArgumentRe return null; } - int p = page.orElseGet( - () -> defaultOrFallback.map(Pageable::getPageNumber).orElseThrow(IllegalStateException::new)); + int p = page + .orElseGet(() -> defaultOrFallback.map(Pageable::getPageNumber).orElseThrow(IllegalStateException::new)); int ps = pageSize .orElseGet(() -> defaultOrFallback.map(Pageable::getPageSize).orElseThrow(IllegalStateException::new)); @@ -288,10 +288,10 @@ public class PageableHandlerMethodArgumentResolver implements PageableArgumentRe return builder.append(source).toString(); } - private Optional getDefaultFromAnnotationOrFallback(MethodParameter methodParameter) { + private Pageable getDefaultFromAnnotationOrFallback(MethodParameter methodParameter) { if (methodParameter.hasParameterAnnotation(PageableDefault.class)) { - return Optional.of(getDefaultPageRequestFrom(methodParameter)); + return getDefaultPageRequestFrom(methodParameter); } return fallbackPageable; diff --git a/src/test/java/org/springframework/data/web/PageableDefaultUnitTests.java b/src/test/java/org/springframework/data/web/PageableDefaultUnitTests.java index 6c2cef895..ffe77171c 100755 --- a/src/test/java/org/springframework/data/web/PageableDefaultUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageableDefaultUnitTests.java @@ -69,8 +69,8 @@ public abstract class PageableDefaultUnitTests { @Test public void returnsDefaultIfNoRequestParametersAndNoDefault() throws Exception { - assertSupportedAndResult(getParameterOfMethod("supportedMethod"), ReflectionTestUtils - .invokeMethod(ReflectionTestUtils.getField(getResolver(), "fallbackPageable"), "get")); + assertSupportedAndResult(getParameterOfMethod("supportedMethod"), + (Pageable) ReflectionTestUtils.getField(getResolver(), "fallbackPageable")); } @Test diff --git a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java index aa01381d5..36da66e4a 100755 --- a/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageableHandlerMethodArgumentResolverUnitTests.java @@ -18,8 +18,6 @@ package org.springframework.data.web; import static org.assertj.core.api.Assertions.*; import static org.springframework.data.web.PageableHandlerMethodArgumentResolver.*; -import java.util.Optional; - import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Qualifier; @@ -140,20 +138,20 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa } @Test // DATACMNS-477 - public void returnsNullIfFallbackIsNullAndNoParametersGiven() throws Exception { + public void returnsNullIfFallbackIsUnpagedAndNoParametersGiven() throws Exception { PageableHandlerMethodArgumentResolver resolver = getResolver(); - resolver.setFallbackPageable(Optional.empty()); + resolver.setFallbackPageable(Pageable.unpaged()); assertSupportedAndResult(supportedMethodParameter, null, new ServletWebRequest(new MockHttpServletRequest()), resolver); } @Test // DATACMNS-477 - public void returnsNullIfFallbackIsNullAndOnlyPageIsGiven() throws Exception { + public void returnsNullIfFallbackIsUnpagedAndOnlyPageIsGiven() throws Exception { PageableHandlerMethodArgumentResolver resolver = getResolver(); - resolver.setFallbackPageable(Optional.empty()); + resolver.setFallbackPageable(Pageable.unpaged()); MockHttpServletRequest request = new MockHttpServletRequest(); request.addParameter("page", "20"); @@ -162,10 +160,10 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa } @Test // DATACMNS-477 - public void returnsNullIfFallbackIsNullAndOnlySizeIsGiven() throws Exception { + public void returnsNullIfFallbackIsUnpagedAndOnlySizeIsGiven() throws Exception { PageableHandlerMethodArgumentResolver resolver = getResolver(); - resolver.setFallbackPageable(Optional.empty()); + resolver.setFallbackPageable(Pageable.unpaged()); MockHttpServletRequest request = new MockHttpServletRequest(); request.addParameter("size", "10"); @@ -191,7 +189,7 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa public void usesNullSortIfNoDefaultIsConfiguredAndPageAndSizeAreGiven() { PageableHandlerMethodArgumentResolver resolver = getResolver(); - resolver.setFallbackPageable(Optional.empty()); + resolver.setFallbackPageable(Pageable.unpaged()); MockHttpServletRequest request = new MockHttpServletRequest(); request.addParameter("page", "0"); @@ -236,7 +234,7 @@ public class PageableHandlerMethodArgumentResolverUnitTests extends PageableDefa public void detectsFallbackPageableIfNullOneIsConfigured() { PageableHandlerMethodArgumentResolver resolver = getResolver(); - resolver.setFallbackPageable(Optional.empty()); + resolver.setFallbackPageable(Pageable.unpaged()); assertThat(resolver.isFallbackPageable(null)).isFalse(); assertThat(resolver.isFallbackPageable(PageRequest.of(0, 10))).isFalse();