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.

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;
 

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<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.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<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 @@ public class AbstractMongoQueryUnitTests {
 		@org.springframework.data.mongodb.repository.Query("{}")
 		Page<Person> findByAnnotatedQuery(String firstnanme, Pageable pageable);
 
+		/** @see DATAMONGO-1057 */
+		Slice<Person> findByLastname(String lastname, Pageable page);
+
 	}
 }