DATAMONGO-1057 - Fix SliceExecution skipping elements.

We now directly set the offset to use instead of reading it from the used pageable. This asserts that every single element is read from the store.
Prior to this change the altered pageSize lead to an unintended increase of the number of elements to skip.

Original pull request: #226.

Author: christophstrobl

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java

@@ -21,7 +21,6 @@
 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 Object execute(Query query) {
 	 * {@link Execution} for {@link Slice} query methods.
 	 * 
 	 * @author Oliver Gierke
+	 * @author Christoph Strobl
 	 * @since 1.5
 	 */
 
@@ -232,9 +232,9 @@ Object execute(Query query) {
 
 			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;
 

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java

@@ -19,6 +19,7 @@
 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 void executesSingleEntityQueryWithProjectionCorrectly() {
 		assertThat(result, is(notNullValue()));
 		assertThat(result.firstname, is("Carter"));
 		assertThat(result.lastname, is("Beauford"));
+	}
+
+	/**
+	 * @see DATAMONGO-1057
+	 */
+	@Test
+	public void sliceShouldTraverseElementsWithoutSkippingOnes() {
+
+		repository.deleteAll();
+
+		List<Person> persons = new ArrayList<Person>(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<Person> 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()));
 	}
 }

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstractMongoQueryUnitTests.java

@@ -36,6 +36,8 @@
 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.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 void metadataShouldBeAddedToStringBasedQueryCorrectly() {
 		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<Query> 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<Query> 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<Query> 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 @@ private interface Repo extends MongoRepository<Person, Long> {
 		@org.springframework.data.mongodb.repository.Query("{}")
 		Page<Person> findByAnnotatedQuery(String firstnanme, Pageable pageable);
 
+		/** @see DATAMONGO-1057 */
+		Slice<Person> findByLastname(String lastname, Pageable page);
+
 	}
 }