DATAREST-906 - Consider default Pageable even if Sort is empty.

We now resolve default configured Pageable without dropping them if their Sort is empty. Previously, any default Pageable was dropped if its Sort was empty which caused null being handed to controller methods of @RepositoryRestController instances. Original pull request: #231.
2016-09-23 10:23:05 +02:00
parent 1f89fc8fd1
commit b4210049d6
4 changed files with 380 additions and 1 deletions
--- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaDefaultPageableWebTests.java
+++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaDefaultPageableWebTests.java
@@ -0,0 +1,137 @@
+/*
+ * Copyright 2016 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.rest.webmvc.jpa;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+import java.util.Collections;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.PageImpl;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
+import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
+import org.springframework.data.rest.tests.AbstractWebIntegrationTests;
+import org.springframework.data.rest.webmvc.RepositoryRestController;
+import org.springframework.data.rest.webmvc.config.RepositoryRestConfigurerAdapter;
+import org.springframework.data.rest.webmvc.config.RepositoryRestMvcConfiguration;
+import org.springframework.hateoas.Link;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.ResponseBody;
+
+/**
+ * Web integration tests specific to default {@link Pageable} handling.
+ * 
+ * @author Mark Paluch
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@WebAppConfiguration
+@ContextConfiguration(classes = JpaDefaultPageableWebTests.Config.class)
+public class JpaDefaultPageableWebTests extends AbstractWebIntegrationTests {
+
+	@Configuration
+	@Import({ RepositoryRestMvcConfiguration.class, JpaRepositoryConfig.class })
+	@EnableJpaRepositories(considerNestedRepositories = true)
+	static class Config extends RepositoryRestConfigurerAdapter {
+
+		public void configureRepositoryRestConfiguration(RepositoryRestConfiguration config) {
+			config.setDefaultPageSize(1);
+		}
+	}
+
+	@RepositoryRestController
+	public static class MyRestController {
+
+		@RequestMapping(method = RequestMethod.GET, path = "books/default-pageable")
+		@ResponseBody
+		Page<Book> getDefaultPageable(Pageable pageable) {
+
+			if (pageable != null) {
+				return new PageImpl<Book>(Collections.singletonList(new Book()), pageable, 1);
+			}
+
+			return new PageImpl<Book>(Collections.emptyList(), pageable, 0);
+		}
+	}
+
+	@Autowired TestDataPopulator loader;
+	@Autowired ApplicationContext context;
+
+	@Before
+	public void setUp() {
+		loader.populateRepositories();
+		super.setUp();
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void executesSearchThatTakesAMappedSortProperty() throws Exception {
+
+		Link findBySortedLink = client.discoverUnique("books", "search", "find-spring-books-sorted");
+
+		// Assert sort options advertised
+		assertThat(findBySortedLink.isTemplated(), is(true));
+		assertThat(findBySortedLink.getVariableNames(), hasItems("sort", "projection"));
+
+		// Assert results returned as specified
+		client.follow(findBySortedLink.expand()).//
+				andExpect(jsonPath("$._embedded.books[0].title").exists()).//
+				andExpect(jsonPath("$._embedded.books[1].title").doesNotExist());
+
+		client.follow(findBySortedLink.expand("sales,desc")).//
+				andExpect(jsonPath("$._embedded.books[0].title").exists()).//
+				andExpect(jsonPath("$._embedded.books[1].title").doesNotExist());
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void shouldApplyDefaultPageable() throws Exception {
+
+		mvc.perform(get("/books/default-pageable")).andDo(print()) //
+				.andExpect(jsonPath("$.content[0].sales").value(0)) //
+				.andExpect(jsonPath("$.size").value(1));
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void shouldOverrideDefaultPageable() throws Exception {
+
+		mvc.perform(get("/books/default-pageable?size=10")).andDo(print()) //
+				.andExpect(jsonPath("$.content[0].sales").value(0)) //
+				.andExpect(jsonPath("$.size").value(10));
+	}
+}
--- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolver.java
+++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolver.java
@@ -80,7 +80,7 @@ public class MappingAwarePageableArgumentResolver implements HandlerMethodArgume
 		Pageable pageable = delegate.resolveArgument(parameter, mavContainer, webRequest, binderFactory);

 		if (pageable == null || pageable.getSort() == null) {
-			return null;
+			return pageable;
 		}

 		Sort translated = translator.translateSort(pageable.getSort(), parameter, webRequest);
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaDefaultPageableWebTests.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaDefaultPageableWebTests.java
@@ -0,0 +1,137 @@
+/*
+ * Copyright 2016 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.rest.webmvc.jpa;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+import java.util.Collections;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+import org.springframework.data.domain.Page;
+import org.springframework.data.domain.PageImpl;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
+import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
+import org.springframework.data.rest.webmvc.AbstractWebIntegrationTests;
+import org.springframework.data.rest.webmvc.RepositoryRestController;
+import org.springframework.data.rest.webmvc.config.RepositoryRestConfigurerAdapter;
+import org.springframework.data.rest.webmvc.config.RepositoryRestMvcConfiguration;
+import org.springframework.hateoas.Link;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.ResponseBody;
+
+/**
+ * Web integration tests specific to default {@link Pageable} handling.
+ * 
+ * @author Mark Paluch
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@WebAppConfiguration
+@ContextConfiguration(classes = JpaDefaultPageableWebTests.Config.class)
+public class JpaDefaultPageableWebTests extends AbstractWebIntegrationTests {
+
+	@Configuration
+	@Import({ RepositoryRestMvcConfiguration.class, JpaRepositoryConfig.class })
+	@EnableJpaRepositories(considerNestedRepositories = true)
+	static class Config extends RepositoryRestConfigurerAdapter {
+
+		public void configureRepositoryRestConfiguration(RepositoryRestConfiguration config) {
+			config.setDefaultPageSize(1);
+		}
+	}
+
+	@RepositoryRestController
+	public static class MyRestController {
+
+		@RequestMapping(method = RequestMethod.GET, value = "books/default-pageable")
+		@ResponseBody
+		Page<Book> getDefaultPageable(Pageable pageable) {
+
+			if (pageable != null) {
+				return new PageImpl<Book>(Collections.singletonList(new Book()), pageable, 1);
+			}
+
+			return new PageImpl<Book>(Collections.<Book>emptyList(), pageable, 0);
+		}
+	}
+
+	@Autowired TestDataPopulator loader;
+	@Autowired ApplicationContext context;
+
+	@Before
+	public void setUp() {
+		loader.populateRepositories();
+		super.setUp();
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void executesSearchThatTakesAMappedSortProperty() throws Exception {
+
+		Link findBySortedLink = client.discoverUnique("books", "search", "find-spring-books-sorted");
+
+		// Assert sort options advertised
+		assertThat(findBySortedLink.isTemplated(), is(true));
+		assertThat(findBySortedLink.getVariableNames(), hasItems("sort", "projection"));
+
+		// Assert results returned as specified
+		client.follow(findBySortedLink.expand()).//
+				andExpect(jsonPath("$._embedded.books[0].title").exists()).//
+				andExpect(jsonPath("$._embedded.books[1].title").doesNotExist());
+
+		client.follow(findBySortedLink.expand("sales,desc")).//
+				andExpect(jsonPath("$._embedded.books[0].title").exists()).//
+				andExpect(jsonPath("$._embedded.books[1].title").doesNotExist());
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void shouldApplyDefaultPageable() throws Exception {
+
+		mvc.perform(get("/books/default-pageable")).andDo(print()) //
+				.andExpect(jsonPath("$.content[0].sales").value(0)) //
+				.andExpect(jsonPath("$.size").value(1));
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void shouldOverrideDefaultPageable() throws Exception {
+
+		mvc.perform(get("/books/default-pageable?size=10")).andDo(print()) //
+				.andExpect(jsonPath("$.content[0].sales").value(0)) //
+				.andExpect(jsonPath("$.size").value(10));
+	}
+}
--- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolverUnitTests.java
+++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolverUnitTests.java
@@ -0,0 +1,105 @@
+/*
+ * Copyright 2016 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.rest.webmvc.json;
+
+import static org.hamcrest.MatcherAssert.*;
+import static org.hamcrest.Matchers.*;
+import static org.mockito.Mockito.*;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.core.MethodParameter;
+import org.springframework.data.domain.PageRequest;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
+import org.springframework.data.domain.Sort.Direction;
+import org.springframework.data.web.PageableHandlerMethodArgumentResolver;
+import org.springframework.web.bind.support.WebDataBinderFactory;
+import org.springframework.web.context.request.NativeWebRequest;
+import org.springframework.web.method.support.ModelAndViewContainer;
+
+/**
+ * Unit tests for {@link MappingAwarePageableArgumentResolver}.
+ * 
+ * @author Mark Paluch
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class MappingAwarePageableArgumentResolverUnitTests {
+
+	@Mock JacksonMappingAwareSortTranslator translator;
+	@Mock PageableHandlerMethodArgumentResolver delegate;
+	@Mock MethodParameter parameter;
+	@Mock NativeWebRequest webRequest;
+	@Mock ModelAndViewContainer modelAndViewContainer;
+	@Mock WebDataBinderFactory binderFactory;
+
+	MappingAwarePageableArgumentResolver resolver;
+
+	@Before
+	public void setUp() {
+		resolver = new MappingAwarePageableArgumentResolver(translator, delegate);
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void resolveArgumentShouldReturnTranslatedPageable() throws Exception {
+
+		Sort translated = new Sort("world");
+		Pageable pageable = new PageRequest(0, 1, Direction.ASC, "hello");
+
+		when(delegate.resolveArgument(parameter, modelAndViewContainer, webRequest, binderFactory)).thenReturn(pageable);
+		when(translator.translateSort(pageable.getSort(), parameter, webRequest)).thenReturn(translated);
+
+		Pageable result = resolver.resolveArgument(parameter, modelAndViewContainer, webRequest, binderFactory);
+
+		assertThat(result.getPageSize(), is(1));
+		assertThat(result.getPageNumber(), is(0));
+		assertThat(result.getSort(), is(equalTo(translated)));
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void resolveArgumentShouldReturnPageableWithoutSort() throws Exception {
+
+		Pageable pageable = new PageRequest(0, 1);
+
+		when(delegate.resolveArgument(parameter, modelAndViewContainer, webRequest, binderFactory)).thenReturn(pageable);
+
+		Pageable result = resolver.resolveArgument(parameter, modelAndViewContainer, webRequest, binderFactory);
+
+		assertThat(result.getPageSize(), is(1));
+		assertThat(result.getPageNumber(), is(0));
+		assertThat(result.getSort(), is(nullValue()));
+	}
+
+	/**
+	 * @see DATAREST-906
+	 */
+	@Test
+	public void resolveArgumentShouldReturnNoPageable() throws Exception {
+
+		Pageable result = resolver.resolveArgument(parameter, modelAndViewContainer, webRequest, binderFactory);
+
+		assertThat(result, is(nullValue()));
+	}
+}