diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java index 75c86b025..b65104660 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java @@ -21,7 +21,6 @@ import java.util.List; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.domain.PageImpl; -import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.domain.SliceImpl; @@ -211,6 +210,7 @@ public abstract class AbstractMongoQuery implements RepositoryQuery { * {@link Execution} for {@link Slice} query methods. * * @author Oliver Gierke + * @author Christoph Strobl * @since 1.5 */ @@ -232,9 +232,9 @@ public abstract class AbstractMongoQuery implements RepositoryQuery { MongoEntityMetadata metadata = method.getEntityInformation(); int pageSize = pageable.getPageSize(); - Pageable slicePageable = new PageRequest(pageable.getPageNumber(), pageSize + 1, pageable.getSort()); - List result = operations.find(query.with(slicePageable), metadata.getJavaType(), metadata.getCollectionName()); + List result = operations.find(query.skip(pageable.getOffset()).limit(pageSize + 1).with(pageable.getSort()), + metadata.getJavaType(), metadata.getCollectionName()); boolean hasNext = result.size() > pageSize; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java index 3ca40debc..910f59ccd 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java @@ -19,6 +19,7 @@ import static java.util.Arrays.*; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -1023,6 +1024,28 @@ public abstract class AbstractPersonRepositoryIntegrationTests { assertThat(result, is(notNullValue())); assertThat(result.firstname, is("Carter")); assertThat(result.lastname, is("Beauford")); + } + /** + * @see DATAMONGO-1057 + */ + @Test + public void sliceShouldTraverseElementsWithoutSkippingOnes() { + + repository.deleteAll(); + + List persons = new ArrayList(100); + for (int i = 0; i < 100; i++) { + // format firstname to assert sorting retains proper order + persons.add(new Person(String.format("%03d", i), "ln" + 1, 100)); + } + + repository.save(persons); + + Slice slice = repository.findByAgeGreaterThan(50, new PageRequest(0, 20, Direction.ASC, "firstname")); + assertThat(slice, contains(persons.subList(0, 20).toArray())); + + slice = repository.findByAgeGreaterThan(50, slice.nextPageable()); + assertThat(slice, contains(persons.subList(20, 40).toArray())); } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstractMongoQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstractMongoQueryUnitTests.java index 8735367ca..9ffd7efb9 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstractMongoQueryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstractMongoQueryUnitTests.java @@ -36,6 +36,8 @@ import org.mockito.runners.MockitoJUnitRunner; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Slice; +import org.springframework.data.domain.Sort; import org.springframework.data.mongodb.MongoDbFactory; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.Person; @@ -50,6 +52,8 @@ import org.springframework.data.mongodb.repository.Meta; import org.springframework.data.mongodb.repository.MongoRepository; import org.springframework.data.repository.core.RepositoryMetadata; +import com.mongodb.BasicDBObjectBuilder; +import com.mongodb.DBObject; import com.mongodb.WriteResult; /** @@ -211,6 +215,73 @@ public class AbstractMongoQueryUnitTests { assertThat(captor.getValue().getMeta().getComment(), is("comment")); } + /** + * @see DATAMONGO-1057 + */ + @Test + public void slicedExecutionShouldRetainNrOfElementsToSkip() { + + MongoQueryFake query = createQueryForMethod("findByLastname", String.class, Pageable.class); + Pageable page1 = new PageRequest(0, 10); + Pageable page2 = page1.next(); + + query.execute(new Object[] { "fake", page1 }); + query.execute(new Object[] { "fake", page2 }); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); + + verify(this.mongoOperationsMock, times(2)) + .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + + assertThat(captor.getAllValues().get(0).getSkip(), is(0)); + assertThat(captor.getAllValues().get(1).getSkip(), is(10)); + } + + /** + * @see DATAMONGO-1057 + */ + @Test + public void slicedExecutionShouldIncrementLimitByOne() { + + MongoQueryFake query = createQueryForMethod("findByLastname", String.class, Pageable.class); + Pageable page1 = new PageRequest(0, 10); + Pageable page2 = page1.next(); + + query.execute(new Object[] { "fake", page1 }); + query.execute(new Object[] { "fake", page2 }); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); + + verify(this.mongoOperationsMock, times(2)) + .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + + assertThat(captor.getAllValues().get(0).getLimit(), is(11)); + assertThat(captor.getAllValues().get(1).getLimit(), is(11)); + } + + /** + * @see DATAMONGO-1057 + */ + @Test + public void slicedExecutionShouldRetainSort() { + + MongoQueryFake query = createQueryForMethod("findByLastname", String.class, Pageable.class); + Pageable page1 = new PageRequest(0, 10, Sort.Direction.DESC, "bar"); + Pageable page2 = page1.next(); + + query.execute(new Object[] { "fake", page1 }); + query.execute(new Object[] { "fake", page2 }); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); + + verify(this.mongoOperationsMock, times(2)) + .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + + DBObject expectedSortObject = new BasicDBObjectBuilder().add("bar", -1).get(); + assertThat(captor.getAllValues().get(0).getSortObject(), is(expectedSortObject)); + assertThat(captor.getAllValues().get(1).getSortObject(), is(expectedSortObject)); + } + private MongoQueryFake createQueryForMethod(String methodName, Class... paramTypes) { try { @@ -272,5 +343,8 @@ public class AbstractMongoQueryUnitTests { @org.springframework.data.mongodb.repository.Query("{}") Page findByAnnotatedQuery(String firstnanme, Pageable pageable); + /** @see DATAMONGO-1057 */ + Slice findByLastname(String lastname, Pageable page); + } }