From f3d2ae366ea455ba6cd5e4e92e415964a7650e7d Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 17 Dec 2014 18:35:39 +0100 Subject: [PATCH] DATAMONGO-1120 - Fix execution of query methods using pagination and field mapping customizations. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repository queries that used pagination and referred to a field that was customized were failing as the count query executed was not mapped correctly in MongoOperations. This result from the fix for DATAMONGO-1080 which removed the premature field name translation from AbstractMongoQuery and thus lead to unmapped field names being used for the count query. We now expose the previously existing, but not public count(…) method on MongoOperations that takes both an entity type as well as an explicit collection name to be able to count-query a dedicated collection but still get the query mapping applied for a certain type. Related ticket: DATAMONGO-1080. --- .../data/mongodb/core/MongoOperations.java | 16 ++++++++- .../data/mongodb/core/MongoTemplate.java | 6 +++- .../repository/query/AbstractMongoQuery.java | 6 ++-- .../data/mongodb/repository/Person.java | 3 +- .../query/AbstractMongoQueryUnitTests.java | 36 ++++++++----------- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java index 0575c8a26..f48d6b128 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoOperations.java @@ -652,14 +652,28 @@ public interface MongoOperations { long count(Query query, Class entityClass); /** - * Returns the number of documents for the given {@link Query} querying the given collection. + * Returns the number of documents for the given {@link Query} querying the given collection. The given {@link Query} + * must solely consist of document field references as we lack type information to map potential property references + * onto document fields. TO make sure the query gets mapped, use {@link #count(Query, Class, String)}. * * @param query * @param collectionName must not be {@literal null} or empty. * @return + * @see #count(Query, Class, String) */ long count(Query query, String collectionName); + /** + * Returns the number of documents for the given {@link Query} by querying the given collection using the given entity + * class to map the given {@link Query}. + * + * @param query + * @param entityClass must not be {@literal null}. + * @param collectionName must not be {@literal null} or empty. + * @return + */ + long count(Query query, Class entityClass, String collectionName); + /** * Insert the object into the collection for the entity type of the object to save. *

diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 629402bd7..f8f391c3d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -642,7 +642,11 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { return count(query, null, collectionName); } - private long count(Query query, Class entityClass, String collectionName) { + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.MongoOperations#count(org.springframework.data.mongodb.core.query.Query, java.lang.Class, java.lang.String) + */ + public long count(Query query, Class entityClass, String collectionName) { Assert.hasText(collectionName); final DBObject dbObject = query == null ? null : queryMapper.getMappedObject(query.getQueryObject(), 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 f1c434372..f6676fecb 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 @@ -259,9 +259,11 @@ public abstract class AbstractMongoQuery implements RepositoryQuery { Object execute(Query query) { MongoEntityMetadata metadata = method.getEntityInformation(); + String collectionName = metadata.getCollectionName(); + Class type = metadata.getJavaType(); int overallLimit = query.getLimit(); - long count = operations.count(query, metadata.getCollectionName()); + long count = operations.count(query, type, collectionName); count = overallLimit != 0 ? Math.min(count, query.getLimit()) : count; boolean pageableOutOfScope = pageable.getOffset() > count; @@ -278,7 +280,7 @@ public abstract class AbstractMongoQuery implements RepositoryQuery { query.limit(overallLimit - pageable.getOffset()); } - List result = operations.find(query, metadata.getJavaType(), metadata.getCollectionName()); + List result = operations.find(query, type, collectionName); return new PageImpl(result, pageable, count); } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/Person.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/Person.java index 689b2e455..1b47b2b68 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/Person.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/Person.java @@ -25,6 +25,7 @@ import org.springframework.data.mongodb.core.index.GeoSpatialIndexed; import org.springframework.data.mongodb.core.index.Indexed; import org.springframework.data.mongodb.core.mapping.DBRef; import org.springframework.data.mongodb.core.mapping.Document; +import org.springframework.data.mongodb.core.mapping.Field; /** * Sample domain class. @@ -50,7 +51,7 @@ public class Person extends Contact { @GeoSpatialIndexed private Point location; - private Address address; + private @Field("add") Address address; private Set

shippingAddresses; @DBRef User creator; 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 e721d3ea1..f7399ae7d 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 @@ -15,7 +15,7 @@ */ package org.springframework.data.mongodb.repository.query; -import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; @@ -100,8 +100,7 @@ public class AbstractMongoQueryUnitTests { createQueryForMethod("deletePersonByLastname", String.class).setDeleteQuery(true).execute(new Object[] { "booh" }); - verify(this.mongoOperationsMock, times(1)).remove(Matchers.any(Query.class), Matchers.eq(Person.class), - Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(1)).remove(Matchers.any(Query.class), eq(Person.class), eq("persons")); verify(this.mongoOperationsMock, times(0)).find(Matchers.any(Query.class), Matchers.any(Class.class), Matchers.anyString()); } @@ -119,8 +118,8 @@ public class AbstractMongoQueryUnitTests { createQueryForMethod("deleteByLastname", String.class).setDeleteQuery(true).execute(new Object[] { "booh" }); - verify(this.mongoOperationsMock, times(1)).findAllAndRemove(Matchers.any(Query.class), Matchers.eq(Person.class), - Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(1)).findAllAndRemove(Matchers.any(Query.class), eq(Person.class), + eq("persons")); } /** @@ -143,15 +142,14 @@ public class AbstractMongoQueryUnitTests { public void testDeleteExecutionReturnsNrDocumentsDeletedFromWriteResult() { when(writeResultMock.getN()).thenReturn(100); - when(this.mongoOperationsMock.remove(Matchers.any(Query.class), Matchers.eq(Person.class), Matchers.eq("persons"))) - .thenReturn(writeResultMock); + when(this.mongoOperationsMock.remove(Matchers.any(Query.class), eq(Person.class), eq("persons"))).thenReturn( + writeResultMock); MongoQueryFake query = createQueryForMethod("deletePersonByLastname", String.class); query.setDeleteQuery(true); assertThat(query.execute(new Object[] { "fake" }), is((Object) 100L)); - verify(this.mongoOperationsMock, times(1)).remove(Matchers.any(Query.class), Matchers.eq(Person.class), - Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(1)).remove(Matchers.any(Query.class), eq(Person.class), eq("persons")); } /** @@ -165,8 +163,7 @@ public class AbstractMongoQueryUnitTests { ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); - verify(this.mongoOperationsMock, times(1)) - .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(1)).find(captor.capture(), eq(Person.class), eq("persons")); assertThat(captor.getValue().getMeta().getComment(), nullValue()); } @@ -182,8 +179,7 @@ public class AbstractMongoQueryUnitTests { ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); - verify(this.mongoOperationsMock, times(1)) - .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(1)).find(captor.capture(), eq(Person.class), eq("persons")); assertThat(captor.getValue().getMeta().getComment(), is("comment")); } @@ -198,7 +194,7 @@ public class AbstractMongoQueryUnitTests { ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); - verify(this.mongoOperationsMock, times(1)).count(captor.capture(), Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(1)).count(captor.capture(), eq(Person.class), eq("persons")); assertThat(captor.getValue().getMeta().getComment(), is("comment")); } @@ -213,8 +209,7 @@ public class AbstractMongoQueryUnitTests { ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); - verify(this.mongoOperationsMock, times(1)) - .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(1)).find(captor.capture(), eq(Person.class), eq("persons")); assertThat(captor.getValue().getMeta().getComment(), is("comment")); } @@ -233,8 +228,7 @@ public class AbstractMongoQueryUnitTests { ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); - verify(this.mongoOperationsMock, times(2)) - .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(2)).find(captor.capture(), eq(Person.class), eq("persons")); assertThat(captor.getAllValues().get(0).getSkip(), is(0)); assertThat(captor.getAllValues().get(1).getSkip(), is(10)); @@ -255,8 +249,7 @@ public class AbstractMongoQueryUnitTests { ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); - verify(this.mongoOperationsMock, times(2)) - .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(2)).find(captor.capture(), eq(Person.class), eq("persons")); assertThat(captor.getAllValues().get(0).getLimit(), is(11)); assertThat(captor.getAllValues().get(1).getLimit(), is(11)); @@ -277,8 +270,7 @@ public class AbstractMongoQueryUnitTests { ArgumentCaptor captor = ArgumentCaptor.forClass(Query.class); - verify(this.mongoOperationsMock, times(2)) - .find(captor.capture(), Matchers.eq(Person.class), Matchers.eq("persons")); + verify(this.mongoOperationsMock, times(2)).find(captor.capture(), eq(Person.class), eq("persons")); DBObject expectedSortObject = new BasicDBObjectBuilder().add("bar", -1).get(); assertThat(captor.getAllValues().get(0).getSortObject(), is(expectedSortObject));