DATADOC-238 - Fixed applying pagination for manually defined queries.

BasicQuery accidentally shadowed limit and skip fields of Query and introduced setters instead of builder style mutators. This caused the getters not returning the values set throughout the mutators which essentially turned off pagination for manually defined queries.

Removed the shadowing and created test case. Refactored constructors.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java

@@ -15,29 +15,28 @@
  */
 package org.springframework.data.mongodb.core.query;
 
+import com.mongodb.BasicDBObject;
 import com.mongodb.DBObject;
 import com.mongodb.util.JSON;
 
+/**
+ * Custom {@link Query} implementation to setup a basic query from some arbitrary JSON query string.
+ *
+ * @author Thomas Risberg
+ * @author Oliver Gierke
+ */
 public class BasicQuery extends Query {
 
-	private DBObject queryObject = null;
-
-	private DBObject fieldsObject = null;
-
-	private DBObject sortObject = null;
-
-	private int skip;
-
-	private int limit;
+	private final DBObject queryObject;
+	private final DBObject fieldsObject;
+	private DBObject sortObject;
 
 	public BasicQuery(String query) {
-		super();
-		this.queryObject = (DBObject) JSON.parse(query);
+		this((DBObject) JSON.parse(query));
 	}
 
 	public BasicQuery(DBObject queryObject) {
-		super();
-		this.queryObject = queryObject;
+		this(queryObject, null);
 	}
 
 	public BasicQuery(String query, String fields) {
@@ -56,36 +55,33 @@ public class BasicQuery extends Query {
 		return this;
 	}
 
+	@Override
 	public DBObject getQueryObject() {
 		return this.queryObject;
 	}
 
+	@Override
 	public DBObject getFieldsObject() {
 		return fieldsObject;
 	}
 
+	@Override
 	public DBObject getSortObject() {
-		return sortObject;
+		
+		BasicDBObject result = new BasicDBObject();
+		if (sortObject != null) {
+			result.putAll(sortObject);
+		}
+		
+		DBObject overrides = super.getSortObject();
+		if (overrides != null) {
+			result.putAll(overrides);
+		}
+		
+		return result;
 	}
 
 	public void setSortObject(DBObject sortObject) {
 		this.sortObject = sortObject;
 	}
-
-	public int getSkip() {
-		return skip;
-	}
-
-	public void setSkip(int skip) {
-		this.skip = skip;
-	}
-
-	public int getLimit() {
-		return this.limit;
-	}
-
-	public void setLimit(int limit) {
-		this.limit = limit;
-	}
-
 }

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

@@ -324,6 +324,13 @@ public class MongoRepositoryFactoryBean<T extends Repository<S, ID>, S, ID exten
 				Order order = toOrder(sort, property);
 				index.on(property, order);
 			}
+			
+			// Add fixed sorting criteria to index
+			if (sort != null) {
+				for (Sort.Order order : sort) {
+					index.on(order.getProperty(), QueryUtils.toOrder(order));
+				}
+			}
 
 			MongoEntityInformation<?, ?> metadata = query.getQueryMethod().getEntityInformation();
 			operations.ensureIndex(index, metadata.getCollectionName());

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

@@ -68,11 +68,14 @@ abstract class QueryUtils {
 		org.springframework.data.mongodb.core.query.Sort bSort = query.sort();
 
 		for (Order order : sort) {
-			bSort.on(order.getProperty(),
-					order.isAscending() ? org.springframework.data.mongodb.core.query.Order.ASCENDING
-							: org.springframework.data.mongodb.core.query.Order.DESCENDING);
+			bSort.on(order.getProperty(), toOrder(order));
 		}
 
 		return query;
 	}
+	
+	public static org.springframework.data.mongodb.core.query.Order toOrder(Order order) {
+		return order.isAscending() ? org.springframework.data.mongodb.core.query.Order.ASCENDING
+				: org.springframework.data.mongodb.core.query.Order.DESCENDING;
+	}
 }

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/BasicQueryUnitTests.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2011 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.mongodb.core.query;
+
+import static org.junit.Assert.*;
+import static org.hamcrest.CoreMatchers.*;
+import static org.springframework.data.mongodb.core.query.Criteria.*;
+
+import org.junit.Test;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.DBObject;
+
+/**
+ * Unit tests for {@link BasicQuery}.
+ *
+ * @author Oliver Gierke
+ */
+public class BasicQueryUnitTests {
+	
+	@Test
+	public void createsQueryFromPlainJson() {
+		Query q = new BasicQuery("{ \"name\" : \"Thomas\"}");
+		DBObject reference = new BasicDBObject("name", "Thomas");
+		assertThat(q.getQueryObject(), is(reference));
+	}
+
+	@Test
+	public void addsCriteriaCorrectly() {
+		Query q = new BasicQuery("{ \"name\" : \"Thomas\"}").addCriteria(where("age").lt(80));
+		DBObject reference = new BasicDBObject("name", "Thomas");
+		reference.put("age", new BasicDBObject("$lt", 80));
+		assertThat(q.getQueryObject(), is(reference));
+	}
+	
+	@Test
+	public void overridesSortCorrectly() {
+		
+		BasicQuery query = new BasicQuery("{}");
+		query.setSortObject(new BasicDBObject("name", -1));
+		query.sort().on("lastname", Order.ASCENDING);
+		
+		DBObject sortReference = new BasicDBObject("name", -1);
+		sortReference.put("lastname", 1);
+		assertThat(query.getSortObject(), is(sortReference));
+	}
+}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java

@@ -86,13 +86,6 @@ public class QueryTests {
 		Assert.assertEquals(expectedFields, q.getFieldsObject().toString());
 	}
 
-	@Test
-	public void testBasicQuery() {
-		Query q = new BasicQuery("{ \"name\" : \"Thomas\"}").addCriteria(where("age").lt(80));
-		String expected = "{ \"name\" : \"Thomas\" , \"age\" : { \"$lt\" : 80}}";
-		Assert.assertEquals(expected, q.getQueryObject().toString());
-	}
-
 	@Test
 	public void testSimpleQueryWithChainedCriteria() {
 		Query q = new Query(where("name").is("Thomas").and("age").lt(80));

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

@@ -33,7 +33,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests {
 
 	@Autowired
 	protected PersonRepository repository;
-	Person dave, carter, boyd, stefan, leroi, alicia;
+	Person dave, oliver, carter, boyd, stefan, leroi, alicia;
 	QPerson person;
 	
 	List<Person> all;
@@ -44,6 +44,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests {
 		repository.deleteAll();
 
 		dave = new Person("Dave", "Matthews", 42);
+		oliver = new Person("Oliver August", "Matthews", 4);
 		carter = new Person("Carter", "Beauford", 49);
 		boyd = new Person("Boyd", "Tinsley", 45);
 		stefan = new Person("Stefan", "Lessard", 34);
@@ -53,7 +54,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests {
 
 		person = new QPerson("person");
 
-		all = repository.save(Arrays.asList(dave, carter, boyd, stefan, leroi, alicia));
+		all = repository.save(Arrays.asList(oliver, dave, carter, boyd, stefan, leroi, alicia));
 	}
 
 	@Test
@@ -119,17 +120,26 @@ public abstract class AbstractPersonRepositoryIntegrationTests {
 	@Test
 	public void findsPagedPersons() throws Exception {
 
-		Page<Person> result = repository.findAll(new PageRequest(1, 2, Direction.ASC, "lastname"));
+		Page<Person> result = repository.findAll(new PageRequest(1, 2, Direction.ASC, "lastname", "firstname"));
 		assertThat(result.isFirstPage(), is(false));
 		assertThat(result.isLastPage(), is(false));
 		assertThat(result, hasItems(dave, stefan));
-		System.out.println(result);
 	}
 
 	@Test
 	public void executesPagedFinderCorrectly() throws Exception {
 
-		Page<Person> page = repository.findByLastnameLike("*a*", new PageRequest(0, 2, Direction.ASC, "lastname"));
+		Page<Person> page = repository.findByLastnameLike("*a*", new PageRequest(0, 2, Direction.ASC, "lastname", "firstname"));
+		assertThat(page.isFirstPage(), is(true));
+		assertThat(page.isLastPage(), is(false));
+		assertThat(page.getNumberOfElements(), is(2));
+		assertThat(page, hasItems(carter, stefan));
+	}
+	
+	@Test
+	public void executesPagedFinderWithAnnotatedQueryCorrectly() throws Exception {
+
+		Page<Person> page = repository.findByLastnameLikeWithPageable(".*a.*", new PageRequest(0, 2, Direction.ASC, "lastname", "firstname"));
 		assertThat(page.isFirstPage(), is(true));
 		assertThat(page.isLastPage(), is(false));
 		assertThat(page.getNumberOfElements(), is(2));
@@ -280,4 +290,12 @@ public abstract class AbstractPersonRepositoryIntegrationTests {
 		
 		repository.save(daveSyer);
 	}
+
+//	@Test
+	public void findsPeopleByLastnameAndOrdersCorrectly() {
+		List<Person> result = repository.findByLastnameOrderByFirstnameAsc("Matthews");
+		assertThat(result.size(), is(2));
+		assertThat(result.get(0), is(dave));
+		assertThat(result.get(1), is(oliver));
+	}
 }

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

@@ -42,6 +42,14 @@ public interface PersonRepository extends MongoRepository<Person, String>, Query
 	 * @return
 	 */
 	List<Person> findByLastname(String lastname);
+	
+	/**
+	 * Returns all {@link Person}s with the given lastname ordered by their firstname.
+	 * 
+	 * @param lastname
+	 * @return
+	 */
+	List<Person> findByLastnameOrderByFirstnameAsc(String lastname);
 
 	/**
 	 * Returns the {@link Person}s with the given firstname. Uses {@link Query} annotation to define the query to be
@@ -70,6 +78,9 @@ public interface PersonRepository extends MongoRepository<Person, String>, Query
 	 */
 	Page<Person> findByLastnameLike(String lastname, Pageable pageable);
 
+	@Query("{ 'lastname' : { '$regex' : ?0, '$options' : ''}}")
+	Page<Person> findByLastnameLikeWithPageable(String lastname, Pageable pageable);
+	
 	/**
 	 * Returns all {@link Person}s with a firstname contained in the given varargs.
 	 * 

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

@@ -49,20 +49,7 @@ public class RepositoryIndexCreationIntegrationTests {
 	
 	@After
 	public void tearDown() {
-		operations.execute(Person.class, new CollectionCallback<Void>() {
-
-			public Void doInCollection(DBCollection collection) throws MongoException, DataAccessException {
-				
-				for (DBObject index : collection.getIndexInfo()) {
-					String indexName = index.get("name").toString();
-					if (indexName.startsWith("find")) {
-						collection.dropIndex(indexName);
-					}
-				}
-				
-				return null;
-			}
-		});
+		operations.dropCollection(Person.class);
 	}
 	
 	@Test
@@ -71,7 +58,7 @@ public class RepositoryIndexCreationIntegrationTests {
 
 			public Void doInCollection(DBCollection collection) throws MongoException, DataAccessException {
 				List<DBObject> indexInfo = collection.getIndexInfo();
-				
+
 				assertThat(indexInfo.isEmpty(), is(false));
 				assertThat(indexInfo.size(), is(greaterThan(2)));
 				assertThat(getIndexNamesFrom(indexInfo), hasItems("findByLastname", "findByFirstnameNotIn"));