From 35f180f999f68cf071b28b533206cdb67534f1ee Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 15 Aug 2011 16:53:15 +0200 Subject: [PATCH] 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. --- .../data/mongodb/core/query/BasicQuery.java | 58 +++++++++--------- .../MongoRepositoryFactoryBean.java | 7 +++ .../data/mongodb/repository/QueryUtils.java | 9 ++- .../core/query/BasicQueryUnitTests.java | 60 +++++++++++++++++++ .../data/mongodb/core/query/QueryTests.java | 7 --- ...tractPersonRepositoryIntegrationTests.java | 28 +++++++-- .../mongodb/repository/PersonRepository.java | 11 ++++ ...positoryIndexCreationIntegrationTests.java | 17 +----- 8 files changed, 136 insertions(+), 61 deletions(-) create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/BasicQueryUnitTests.java diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java index f691aa2f4..91b8ea300 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java +++ b/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; - } - } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/MongoRepositoryFactoryBean.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/MongoRepositoryFactoryBean.java index 3c30c434d..8ed3c86b4 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/MongoRepositoryFactoryBean.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/MongoRepositoryFactoryBean.java @@ -324,6 +324,13 @@ public class MongoRepositoryFactoryBean, 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()); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/QueryUtils.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/QueryUtils.java index b24edf54d..19485d548 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/QueryUtils.java +++ b/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; + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/BasicQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/BasicQueryUnitTests.java new file mode 100644 index 000000000..ca0e288fc --- /dev/null +++ b/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)); + } +} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java index 8d4139b7e..0a3a18a34 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java +++ b/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)); 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 65e519735..f4928ec4d 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 @@ -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 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 result = repository.findAll(new PageRequest(1, 2, Direction.ASC, "lastname")); + Page 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 page = repository.findByLastnameLike("*a*", new PageRequest(0, 2, Direction.ASC, "lastname")); + Page 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 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 result = repository.findByLastnameOrderByFirstnameAsc("Matthews"); + assertThat(result.size(), is(2)); + assertThat(result.get(0), is(dave)); + assertThat(result.get(1), is(oliver)); + } } \ No newline at end of file diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java index e4e9b90cd..20512e44a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java @@ -42,6 +42,14 @@ public interface PersonRepository extends MongoRepository, Query * @return */ List findByLastname(String lastname); + + /** + * Returns all {@link Person}s with the given lastname ordered by their firstname. + * + * @param lastname + * @return + */ + List 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, Query */ Page findByLastnameLike(String lastname, Pageable pageable); + @Query("{ 'lastname' : { '$regex' : ?0, '$options' : ''}}") + Page findByLastnameLikeWithPageable(String lastname, Pageable pageable); + /** * Returns all {@link Person}s with a firstname contained in the given varargs. * diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/RepositoryIndexCreationIntegrationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/RepositoryIndexCreationIntegrationTests.java index 7cbabeed7..b1add200d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/RepositoryIndexCreationIntegrationTests.java +++ b/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() { - - 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 indexInfo = collection.getIndexInfo(); - + assertThat(indexInfo.isEmpty(), is(false)); assertThat(indexInfo.size(), is(greaterThan(2))); assertThat(getIndexNamesFrom(indexInfo), hasItems("findByLastname", "findByFirstnameNotIn"));