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 85a60dbaa..ae08fc0b7 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 @@ -372,6 +372,10 @@ public abstract class AbstractMongoQuery implements RepositoryQuery { */ final class DeleteExecution extends Execution { + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery.Execution#execute(org.springframework.data.mongodb.core.query.Query) + */ @Override Object execute(Query query) { @@ -382,20 +386,11 @@ public abstract class AbstractMongoQuery implements RepositoryQuery { private Object deleteAndConvertResult(Query query, MongoEntityMetadata metadata) { if (method.isCollectionQuery()) { - return findAndRemove(query, metadata); + return operations.findAllAndRemove(query, metadata.getJavaType()); } - WriteResult writeResult = remove(query, metadata); + WriteResult writeResult = operations.remove(query, metadata.getCollectionName()); return writeResult != null ? writeResult.getN() : 0L; } - - private List findAndRemove(Query query, MongoEntityMetadata metadata) { - return operations.findAllAndRemove(query, metadata.getJavaType()); - } - - private WriteResult remove(Query query, MongoEntityMetadata metadata) { - return operations.remove(query, metadata.getCollectionName()); - } } - } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java index 622e02ee5..07b8f4e2a 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java @@ -34,6 +34,7 @@ import com.mongodb.util.JSON; */ public class StringBasedMongoQuery extends AbstractMongoQuery { + private static final String COUND_AND_DELETE = "Manually defined query for %s cannot be both a count and delete query at the same time!"; private static final Pattern PLACEHOLDER = Pattern.compile("\\?(\\d+)"); private static final Logger LOG = LoggerFactory.getLogger(StringBasedMongoQuery.class); @@ -43,7 +44,18 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { private final boolean isDeleteQuery; /** - * Creates a new {@link StringBasedMongoQuery}. + * Creates a new {@link StringBasedMongoQuery} for the given {@link MongoQueryMethod} and {@link MongoOperations}. + * + * @param method must not be {@literal null}. + * @param mongoOperations must not be {@literal null}. + */ + public StringBasedMongoQuery(MongoQueryMethod method, MongoOperations mongoOperations) { + this(method.getAnnotatedQuery(), method, mongoOperations); + } + + /** + * Creates a new {@link StringBasedMongoQuery} for the given {@link String}, {@link MongoQueryMethod} and + * {@link MongoOperations}. * * @param method must not be {@literal null}. * @param template must not be {@literal null}. @@ -56,10 +68,10 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { this.fieldSpec = method.getFieldSpecification(); this.isCountQuery = method.hasAnnotatedQuery() ? method.getQueryAnnotation().count() : false; this.isDeleteQuery = method.hasAnnotatedQuery() ? method.getQueryAnnotation().delete() : false; - } - public StringBasedMongoQuery(MongoQueryMethod method, MongoOperations mongoOperations) { - this(method.getAnnotatedQuery(), method, mongoOperations); + if (isCountQuery && isDeleteQuery) { + throw new IllegalArgumentException(String.format(COUND_AND_DELETE, method)); + } } /* @@ -98,6 +110,10 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { return isCountQuery; } + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#isDeleteQuery() + */ @Override protected boolean isDeleteQuery() { return this.isDeleteQuery; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java index 6bd058403..2d04c285b 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java @@ -319,6 +319,9 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { assertThat((Object[]) idField.get("$in"), is(new Object[] { Integer.valueOf(0), Integer.valueOf(1) })); } + /** + * @see DATAMONGO-566 + */ @Test public void findAllAndRemoveShouldNotTriggerRemoveIfFindResultIsEmpty() { 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 5c480d769..cceaeae0b 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 @@ -781,6 +781,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests { repository.save(alicia); Page result = repository.findByHavingCreator(new PageRequest(0, 100)); + assertThat(result.getNumberOfElements(), is(1)); assertThat(result.getContent().get(0), is(alicia)); } @@ -792,6 +793,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests { public void deleteByShouldReturnListOfDeletedElementsWhenRetunTypeIsCollectionLike() { List result = repository.deleteByLastname("Beauford"); + assertThat(result, hasItem(carter)); assertThat(result, hasSize(1)); } @@ -803,6 +805,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests { public void deleteByShouldRemoveElementsMatchingDerivedQuery() { repository.deleteByLastname("Beauford"); + assertThat(operations.count(new BasicQuery("{'lastname':'Beauford'}"), Person.class), is(0L)); } @@ -837,6 +840,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests { public void deleteByUsingAnnotatedQueryShouldReturnListOfDeletedElementsWhenRetunTypeIsCollectionLike() { List result = repository.removeByLastnameUsingAnnotatedQuery("Beauford"); + assertThat(result, hasItem(carter)); assertThat(result, hasSize(1)); } @@ -848,6 +852,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests { public void deleteByUsingAnnotatedQueryShouldRemoveElementsMatchingDerivedQuery() { repository.removeByLastnameUsingAnnotatedQuery("Beauford"); + assertThat(operations.count(new BasicQuery("{'lastname':'Beauford'}"), Person.class), is(0L)); } @@ -858,5 +863,4 @@ public abstract class AbstractPersonRepositoryIntegrationTests { public void deleteByUsingAnnotatedQueryShouldReturnNumberOfDocumentsRemovedIfReturnTypeIsLong() { assertThat(repository.removePersonByLastnameUsingAnnotatedQuery("Beauford"), is(1L)); } - } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstracMongoQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstracMongoQueryUnitTests.java index a9d86f927..b70c102d8 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstracMongoQueryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstracMongoQueryUnitTests.java @@ -15,6 +15,7 @@ */ package org.springframework.data.mongodb.repository.query; +import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; import static org.mockito.Mockito.*; @@ -47,33 +48,30 @@ import org.springframework.data.repository.core.RepositoryMetadata; import com.mongodb.WriteResult; /** + * Unit tests for {@link AbstractMongoQuery}. + * * @author Christoph Strobl + * @author Oliver Gierke */ @RunWith(MockitoJUnitRunner.class) -public class MongoQueryExecutionUnitTests { +public class AbstracMongoQueryUnitTests { - private @Mock RepositoryMetadata metadataMock; + @Mock RepositoryMetadata metadataMock; + @Mock MongoOperations mongoOperationsMock; + @Mock @SuppressWarnings("rawtypes") BasicMongoPersistentEntity persitentEntityMock; + @Mock MongoMappingContext mappingContextMock; + @Mock WriteResult writeResultMock; - private @Mock MongoOperations mongoOperationsMock; - - @SuppressWarnings("rawtypes")// - private @Mock BasicMongoPersistentEntity persitentEntityMock; - - private @Mock MongoMappingContext mappingContextMock; - - private @Mock WriteResult writeResultMock; - - @SuppressWarnings({ "unchecked", "rawtypes" }) @Before + @SuppressWarnings({ "unchecked", "rawtypes" }) public void setUp() { when(metadataMock.getDomainType()).thenReturn((Class) Person.class); when(metadataMock.getReturnedDomainClass(Matchers.any(Method.class))).thenReturn((Class) Person.class); when(persitentEntityMock.getCollection()).thenReturn("persons"); - when(mappingContextMock.getPersistentEntity(Matchers.any(Class.class))).thenReturn(persitentEntityMock); - when(persitentEntityMock.getType()).thenReturn(Person.class); + DbRefResolver dbRefResolver = new DefaultDbRefResolver(mock(MongoDbFactory.class)); MappingMongoConverter converter = new MappingMongoConverter(dbRefResolver, mappingContextMock); converter.afterPropertiesSet(); @@ -89,6 +87,7 @@ public class MongoQueryExecutionUnitTests { public void testDeleteExecutionCallsRemoveCorreclty() { createQueryForMethod("deletePersonByLastname", String.class).setDeleteQuery(true).execute(new Object[] { "booh" }); + verify(this.mongoOperationsMock, times(1)).remove(Matchers.any(Query.class), Matchers.eq("persons")); verify(this.mongoOperationsMock, times(0)).find(Matchers.any(Query.class), Matchers.any(Class.class), Matchers.anyString()); @@ -134,14 +133,19 @@ public class MongoQueryExecutionUnitTests { MongoQueryFake query = createQueryForMethod("deletePersonByLastname", String.class); query.setDeleteQuery(true); - assertThat(query.execute(new Object[] { "fake" }), Is. is(100L)); - + assertThat(query.execute(new Object[] { "fake" }), is((Object) 100L)); verify(this.mongoOperationsMock, times(1)).remove(Matchers.any(Query.class), Matchers.eq("persons")); } private MongoQueryFake createQueryForMethod(String methodName, Class... paramTypes) { + try { - return this.createQueryForMethod(Repo.class.getMethod(methodName, paramTypes)); + + Method method = Repo.class.getMethod(methodName, paramTypes); + MongoQueryMethod queryMethod = new MongoQueryMethod(method, metadataMock, mappingContextMock); + + return new MongoQueryFake(queryMethod, mongoOperationsMock); + } catch (NoSuchMethodException e) { throw new IllegalArgumentException(e.getMessage(), e); } catch (SecurityException e) { @@ -149,23 +153,15 @@ public class MongoQueryExecutionUnitTests { } } - private MongoQueryFake createQueryForMethod(Method method) { - return new MongoQueryFake(createMongoQueryMethodFrom(method), this.mongoOperationsMock); - } - - private MongoQueryMethod createMongoQueryMethodFrom(Method method) { - return new MongoQueryMethod(method, metadataMock, this.mappingContextMock); - } + private static class MongoQueryFake extends AbstractMongoQuery { - class MongoQueryFake extends AbstractMongoQuery { + private boolean isCountQuery; + private boolean isDeleteQuery; public MongoQueryFake(MongoQueryMethod method, MongoOperations operations) { super(method, operations); } - private boolean isCountQuery; - private boolean isDeleteQuery; - @Override protected Query createQuery(ConvertingParameterAccessor accessor) { return new BasicQuery("{'foo':'bar'}"); @@ -181,11 +177,6 @@ public class MongoQueryExecutionUnitTests { return isDeleteQuery; } - public MongoQueryFake setCountQuery(boolean isCountQuery) { - this.isCountQuery = isCountQuery; - return this; - } - public MongoQueryFake setDeleteQuery(boolean isDeleteQuery) { this.isDeleteQuery = isDeleteQuery; return this; @@ -198,5 +189,4 @@ public class MongoQueryExecutionUnitTests { Long deletePersonByLastname(String lastname); } - } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java index 3f9d6f198..775f25bfb 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java @@ -150,6 +150,14 @@ public class StringBasedMongoQueryUnitTests { assertThat(mongoQuery.isDeleteQuery(), is(true)); } + /** + * @see DATAMONGO-566 + */ + @Test(expected = IllegalArgumentException.class) + public void preventsDeleteAndCountFlagAtTheSameTime() throws Exception { + createQueryForMethod("invalidMethod", String.class); + } + private StringBasedMongoQuery createQueryForMethod(String name, Class... parameters) throws Exception { Method method = SampleRepository.class.getMethod(name, parameters); @@ -174,5 +182,7 @@ public class StringBasedMongoQueryUnitTests { @Query(value = "{ 'lastname' : ?0 }", delete = true) void removeByLastname(String lastname); + @Query(value = "{ 'lastname' : ?0 }", delete = true, count = true) + void invalidMethod(String lastname); } }