diff --git a/pom.xml b/pom.xml index 657607148e..3a0995dc62 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 2.2.0.BUILD-SNAPSHOT + 2.2.0.DATAMONGO-2195-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index c2ff37b35c..2a407e3243 100644 --- a/spring-data-mongodb-benchmarks/pom.xml +++ b/spring-data-mongodb-benchmarks/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-mongodb-parent - 2.2.0.BUILD-SNAPSHOT + 2.2.0.DATAMONGO-2195-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-cross-store/pom.xml b/spring-data-mongodb-cross-store/pom.xml index fd36f227c0..b3b7b7ff34 100644 --- a/spring-data-mongodb-cross-store/pom.xml +++ b/spring-data-mongodb-cross-store/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-mongodb-parent - 2.2.0.BUILD-SNAPSHOT + 2.2.0.DATAMONGO-2195-SNAPSHOT ../pom.xml @@ -50,7 +50,7 @@ org.springframework.data spring-data-mongodb - 2.2.0.BUILD-SNAPSHOT + 2.2.0.DATAMONGO-2195-SNAPSHOT diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index fc8d28a2b6..917730a3aa 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-mongodb-parent - 2.2.0.BUILD-SNAPSHOT + 2.2.0.DATAMONGO-2195-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index f3c85a046a..8231920bf1 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -11,7 +11,7 @@ org.springframework.data spring-data-mongodb-parent - 2.2.0.BUILD-SNAPSHOT + 2.2.0.DATAMONGO-2195-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java index 7560b5bdef..e990ce587c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java @@ -199,6 +199,16 @@ interface Entity { */ Query getByIdQuery(); + /** + * Returns the {@link Query} to remove an entity by its {@literal id} and if applicable {@literal version}. + * + * @return the {@link Query} to use for removing the entity. Never {@literal null}. + * @since 2.2 + */ + default Query getRemoveByQuery() { + return isVersionedEntity() ? getQueryForVersion() : getByIdQuery(); + } + /** * Returns the {@link Query} to find the entity in its current version. * @@ -229,9 +239,11 @@ default boolean isVersionedEntity() { } /** - * Returns the value of the version if the entity has a version property, {@literal null} otherwise. + * Returns the value of the version if the entity {@link #isVersionedEntity() has a version property}. * - * @return + * @return the entity version. Can be {@literal null}. + * @throws IllegalStateException if the entity does not define a {@literal version} property. Make sure to check + * {@link #isVersionedEntity()}. */ @Nullable Object getVersion(); @@ -287,8 +299,8 @@ interface AdaptibleEntity extends Entity { /** * Returns the current version value if the entity has a version property. * - * @return the current version or {@literal null} in case it's uninitialized or the entity doesn't expose a version - * property. + * @return the current version or {@literal null} in case it's uninitialized. + * @throws IllegalStateException if the entity does not define a {@literal version} property. */ @Nullable Number getVersion(); @@ -490,10 +502,10 @@ public Query getByIdQuery() { public Query getQueryForVersion() { MongoPersistentProperty idProperty = entity.getRequiredIdProperty(); - MongoPersistentProperty property = entity.getRequiredVersionProperty(); + MongoPersistentProperty versionProperty = entity.getRequiredVersionProperty(); return new Query(Criteria.where(idProperty.getName()).is(getId())// - .and(property.getName()).is(getVersion())); + .and(versionProperty.getName()).is(getVersion())); } /* 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 2d66986d60..85f3d411ac 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 @@ -1350,7 +1350,10 @@ T findAndReplace(Query query, S replacement, FindAndReplaceOptions option UpdateResult updateMulti(Query query, Update update, Class entityClass, String collectionName); /** - * Remove the given object from the collection by id. + * Remove the given object from the collection by {@literal id} and (if applicable) its + * {@link org.springframework.data.annotation.Version}.
+ * Use {@link DeleteResult#getDeletedCount()} for insight whether an {@link DeleteResult#wasAcknowledged() + * acknowledged} remove operation was successful or not. * * @param object must not be {@literal null}. * @return the {@link DeleteResult} which lets you access the results of the previous delete. @@ -1358,7 +1361,10 @@ T findAndReplace(Query query, S replacement, FindAndReplaceOptions option DeleteResult remove(Object object); /** - * Removes the given object from the given collection. + * Removes the given object from the given collection by {@literal id} and (if applicable) its + * {@link org.springframework.data.annotation.Version}.
+ * Use {@link DeleteResult#getDeletedCount()} for insight whether an {@link DeleteResult#wasAcknowledged() + * acknowledged} remove operation was successful or not. * * @param object must not be {@literal null}. * @param collectionName name of the collection where the objects will removed, must not be {@literal null} or empty. 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 94fe71d4ca..2bd0e75696 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 @@ -68,7 +68,16 @@ import org.springframework.data.mongodb.core.aggregation.Fields; import org.springframework.data.mongodb.core.aggregation.TypeBasedAggregationOperationContext; import org.springframework.data.mongodb.core.aggregation.TypedAggregation; -import org.springframework.data.mongodb.core.convert.*; +import org.springframework.data.mongodb.core.convert.DbRefResolver; +import org.springframework.data.mongodb.core.convert.DefaultDbRefResolver; +import org.springframework.data.mongodb.core.convert.JsonSchemaMapper; +import org.springframework.data.mongodb.core.convert.MappingMongoConverter; +import org.springframework.data.mongodb.core.convert.MongoConverter; +import org.springframework.data.mongodb.core.convert.MongoCustomConversions; +import org.springframework.data.mongodb.core.convert.MongoJsonSchemaMapper; +import org.springframework.data.mongodb.core.convert.MongoWriter; +import org.springframework.data.mongodb.core.convert.QueryMapper; +import org.springframework.data.mongodb.core.convert.UpdateMapper; import org.springframework.data.mongodb.core.index.IndexOperations; import org.springframework.data.mongodb.core.index.IndexOperationsProvider; import org.springframework.data.mongodb.core.index.MongoMappingEventPublisher; @@ -1638,9 +1647,7 @@ public DeleteResult remove(Object object) { Assert.notNull(object, "Object must not be null!"); - Query query = operations.forEntity(object).getByIdQuery(); - - return remove(query, object.getClass()); + return remove(object, operations.determineCollectionName(object.getClass())); } @Override @@ -1649,7 +1656,7 @@ public DeleteResult remove(Object object, String collectionName) { Assert.notNull(object, "Object must not be null!"); Assert.hasText(collectionName, "Collection name must not be null or empty!"); - Query query = operations.forEntity(object).getByIdQuery(); + Query query = operations.forEntity(object).getRemoveByQuery(); return doRemove(collectionName, query, object.getClass(), false); } @@ -1721,7 +1728,7 @@ public DeleteResult doInCollection(MongoCollection collection) : collection; DeleteResult result = multi ? collectionToUse.deleteMany(removeQuery, options) - : collection.deleteOne(removeQuery, options); + : collectionToUse.deleteOne(removeQuery, options); maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName)); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index 73b9ea6b55..f92cd7632c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java @@ -1688,7 +1688,7 @@ private boolean containsVersionProperty(Document document, @Nullable MongoPersis return false; } - return document.containsKey(persistentEntity.getRequiredIdProperty().getFieldName()); + return document.containsKey(persistentEntity.getRequiredVersionProperty().getFieldName()); } /* @@ -1717,7 +1717,7 @@ public Mono remove(Object object) { Assert.notNull(object, "Object must not be null!"); - return remove(operations.forEntity(object).getByIdQuery(), object.getClass()); + return remove(operations.forEntity(object).getRemoveByQuery(), object.getClass()); } /* @@ -1729,7 +1729,7 @@ public Mono remove(Object object, String collectionName) { Assert.notNull(object, "Object must not be null!"); Assert.hasText(collectionName, "Collection name must not be null or empty!"); - return doRemove(collectionName, operations.forEntity(object).getByIdQuery(), object.getClass()); + return doRemove(collectionName, operations.forEntity(object).getRemoveByQuery(), object.getClass()); } private void assertUpdateableIdIfNotSet(Object value) { @@ -1787,15 +1787,14 @@ protected Mono doRemove(String collectionName, Query query, @N Document queryObject = query.getQueryObject(); MongoPersistentEntity entity = getPersistentEntity(entityClass); + Document removeQuery = queryMapper.getMappedObject(queryObject, entity); return execute(collectionName, collection -> { - Document removeQuey = queryMapper.getMappedObject(queryObject, entity); - - maybeEmitEvent(new BeforeDeleteEvent<>(removeQuey, entityClass, collectionName)); + maybeEmitEvent(new BeforeDeleteEvent<>(removeQuery, entityClass, collectionName)); MongoAction mongoAction = new MongoAction(writeConcern, MongoActionOperation.REMOVE, collectionName, entityClass, - null, removeQuey); + null, removeQuery); DeleteOptions deleteOptions = new DeleteOptions(); query.getCollation().map(Collation::toMongoCollation).ifPresent(deleteOptions::collation); @@ -1805,13 +1804,13 @@ protected Mono doRemove(String collectionName, Query query, @N if (LOGGER.isDebugEnabled()) { LOGGER.debug("Remove using query: {} in collection: {}.", - new Object[] { serializeToJsonSafely(removeQuey), collectionName }); + new Object[] { serializeToJsonSafely(removeQuery), collectionName }); } if (query.getLimit() > 0 || query.getSkip() > 0) { FindPublisher cursor = new QueryFindPublisherPreparer(query, entityClass) - .prepare(collection.find(removeQuey)) // + .prepare(collection.find(removeQuery)) // .projection(MappedDocument.getIdOnlyProjection()); return Flux.from(cursor) // @@ -1822,10 +1821,10 @@ protected Mono doRemove(String collectionName, Query query, @N return collectionToUse.deleteMany(MappedDocument.getIdIn(val), deleteOptions); }); } else { - return collectionToUse.deleteMany(removeQuey, deleteOptions); + return collectionToUse.deleteMany(removeQuery, deleteOptions); } - }).doOnNext(deleteResult -> maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName))) + }).doOnNext(it -> maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName))) // .next(); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoEntityInformation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoEntityInformation.java index 71caf6bfbf..c58732f589 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoEntityInformation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoEntityInformation.java @@ -16,11 +16,13 @@ package org.springframework.data.mongodb.repository.query; import org.springframework.data.repository.core.EntityInformation; +import org.springframework.lang.Nullable; /** * Mongo specific {@link EntityInformation}. * * @author Oliver Gierke + * @author Mark Paluch */ public interface MongoEntityInformation extends EntityInformation { @@ -37,4 +39,26 @@ public interface MongoEntityInformation extends EntityInformation * @return */ String getIdAttribute(); + + /** + * Returns whether the entity uses optimistic locking. + * + * @return true if the entity defines a {@link org.springframework.data.annotation.Version} property. + * @since 2.2 + */ + default boolean isVersioned() { + return false; + } + + /** + * Returns the version value for the entity or {@literal null} if the entity is not {@link #isVersioned() versioned}. + * + * @param entity must not be {@literal null} + * @return can be {@literal null}. + * @since 2.2 + */ + @Nullable + default Object getVersion(T entity) { + return null; + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MappingMongoEntityInformation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MappingMongoEntityInformation.java index 4d4fc6de9a..e9c1814edf 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MappingMongoEntityInformation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/MappingMongoEntityInformation.java @@ -16,6 +16,7 @@ package org.springframework.data.mongodb.repository.support; import org.bson.types.ObjectId; +import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.repository.query.MongoEntityInformation; import org.springframework.data.repository.core.support.PersistentEntityInformation; @@ -87,14 +88,16 @@ private MappingMongoEntityInformation(MongoPersistentEntity entity, @Nullable this.fallbackIdType = idType != null ? idType : (Class) ObjectId.class; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.mongodb.repository.MongoEntityInformation#getCollectionName() */ public String getCollectionName() { return customCollectionName == null ? entityMetadata.getCollection() : customCollectionName; } - /* (non-Javadoc) + /* + * (non-Javadoc) * @see org.springframework.data.mongodb.repository.MongoEntityInformation#getIdAttribute() */ public String getIdAttribute() { @@ -106,7 +109,6 @@ public String getIdAttribute() { * @see org.springframework.data.repository.core.support.PersistentEntityInformation#getIdType() */ @Override - @SuppressWarnings("unchecked") public Class getIdType() { if (this.entityMetadata.hasIdProperty()) { @@ -115,4 +117,30 @@ public Class getIdType() { return fallbackIdType; } + + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.repository.MongoEntityInformation#isVersioned() + */ + @Override + public boolean isVersioned() { + return this.entityMetadata.hasVersionProperty(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.repository.MongoEntityInformation#getVersion(Object) + */ + @Override + public Object getVersion(T entity) { + + if (!isVersioned()) { + return null; + } + + PersistentPropertyAccessor accessor = this.entityMetadata.getPropertyAccessor(entity); + + return accessor.getProperty(this.entityMetadata.getRequiredVersionProperty()); + } + } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepository.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepository.java index de52950c54..cccfcea6f4 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepository.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepository.java @@ -23,6 +23,7 @@ import java.util.Optional; import java.util.stream.Collectors; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Example; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; @@ -40,6 +41,8 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import com.mongodb.client.result.DeleteResult; + /** * Repository base implementation for Mongo. * @@ -161,7 +164,14 @@ public void delete(T entity) { Assert.notNull(entity, "The given entity must not be null!"); - deleteById(entityInformation.getRequiredId(entity)); + DeleteResult deleteResult = mongoOperations.remove(entity, entityInformation.getCollectionName()); + + if (entityInformation.isVersioned() && deleteResult.wasAcknowledged() && deleteResult.getDeletedCount() == 0) { + throw new OptimisticLockingFailureException(String.format( + "The entity with id %s with version %s in %s cannot be deleted! Was it modified or deleted in the meantime?", + entityInformation.getId(entity), entityInformation.getVersion(entity), + entityInformation.getCollectionName())); + } } /* diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepository.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepository.java index 6802a20c91..019fdd3ec1 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepository.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepository.java @@ -28,6 +28,7 @@ import org.reactivestreams.Publisher; import org.springframework.dao.IncorrectResultSizeDataAccessException; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Example; import org.springframework.data.domain.Sort; import org.springframework.data.mongodb.core.ReactiveMongoOperations; @@ -39,6 +40,8 @@ import org.springframework.data.util.Streamable; import org.springframework.util.Assert; +import com.mongodb.client.result.DeleteResult; + /** * Reactive repository base implementation for Mongo. * @@ -355,7 +358,24 @@ public Mono delete(T entity) { Assert.notNull(entity, "The given entity must not be null!"); - return deleteById(entityInformation.getRequiredId(entity)); + Mono remove = mongoOperations.remove(entity, entityInformation.getCollectionName()); + + if (entityInformation.isVersioned()) { + + remove = remove.handle((deleteResult, sink) -> { + + if (deleteResult.wasAcknowledged() && deleteResult.getDeletedCount() == 0) { + sink.error(new OptimisticLockingFailureException(String.format( + "The entity with id %s with version %s in %s cannot be deleted! Was it modified or deleted in the meantime?", + entityInformation.getId(entity), entityInformation.getVersion(entity), + entityInformation.getCollectionName()))); + } else { + sink.next(deleteResult); + } + }); + } + + return remove.then(); } /* @@ -367,7 +387,7 @@ public Mono deleteAll(Iterable entities) { Assert.notNull(entities, "The given Iterable of entities must not be null!"); - return Flux.fromIterable(entities).flatMap(entity -> deleteById(entityInformation.getRequiredId(entity))).then(); + return Flux.fromIterable(entities).flatMap(this::delete).then(); } /* diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index bb073c87c2..435a8e3370 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -43,9 +43,9 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +import org.assertj.core.api.Assertions; import org.bson.types.ObjectId; import org.hamcrest.collection.IsMapContaining; -import org.hamcrest.core.IsSame; import org.joda.time.DateTime; import org.junit.After; import org.junit.Before; @@ -1646,6 +1646,24 @@ public void initializesVersionOnInsert() { assertThat(person.version, is(0)); } + @Test // DATAMONGO-2195 + public void removeVersionedEntityConsidersVersion() { + + PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); + person.firstName = "Dave"; + + template.insert(person); + assertThat(person.version, is(0)); + template.update(PersonWithVersionPropertyOfTypeInteger.class).matching(query(where("id").is(person.id))) + .apply(new Update().set("firstName", "Walter")).first(); + + DeleteResult deleteResult = template.remove(person); + + assertThat(deleteResult.wasAcknowledged()).isTrue(); + assertThat(deleteResult.getDeletedCount()).isZero(); + assertThat(template.count(new Query(), PersonWithVersionPropertyOfTypeInteger.class)).isOne(); + } + @Test // DATAMONGO-588 public void initializesVersionOnBatchInsert() { 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 480dbebc00..41699e6414 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 @@ -83,6 +83,7 @@ import com.mongodb.MongoException; import com.mongodb.MongoNamespace; import com.mongodb.ReadPreference; +import com.mongodb.WriteConcern; import com.mongodb.client.AggregateIterable; import com.mongodb.client.FindIterable; import com.mongodb.client.MapReduceIterable; @@ -96,6 +97,7 @@ import com.mongodb.client.model.MapReduceAction; import com.mongodb.client.model.ReplaceOptions; import com.mongodb.client.model.UpdateOptions; +import com.mongodb.client.result.DeleteResult; import com.mongodb.client.result.UpdateResult; /** @@ -114,11 +116,13 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { @Mock MongoClient mongo; @Mock MongoDatabase db; @Mock MongoCollection collection; + @Mock MongoCollection collectionWithWriteConcern; @Mock MongoCursor cursor; @Mock FindIterable findIterable; @Mock AggregateIterable aggregateIterable; @Mock MapReduceIterable mapReduceIterable; @Mock UpdateResult updateResult; + @Mock DeleteResult deleteResult; Document commandResultDocument = new Document(); @@ -141,6 +145,8 @@ public void setUp() { when(collection.aggregate(any(List.class), any())).thenReturn(aggregateIterable); when(collection.withReadPreference(any())).thenReturn(collection); when(collection.replaceOne(any(), any(), any(ReplaceOptions.class))).thenReturn(updateResult); + when(collection.withWriteConcern(any())).thenReturn(collectionWithWriteConcern); + when(collectionWithWriteConcern.deleteOne(any(Bson.class), any())).thenReturn(deleteResult); when(findIterable.projection(any())).thenReturn(findIterable); when(findIterable.sort(any(org.bson.Document.class))).thenReturn(findIterable); when(findIterable.collation(any())).thenReturn(findIterable); @@ -731,6 +737,19 @@ public void findAndRemoveShouldUseCollationWhenPresent() { assertThat(options.getValue().getCollation().getLocale(), is("fr")); } + @Test // DATAMONGO-2196 + public void removeShouldApplyWriteConcern() { + + Person person = new Person(); + person.id = "id-1"; + + template.setWriteConcern(WriteConcern.UNACKNOWLEDGED); + template.remove(person); + + verify(collection).withWriteConcern(eq(WriteConcern.UNACKNOWLEDGED)); + verify(collectionWithWriteConcern).deleteOne(any(Bson.class), any()); + } + @Test // DATAMONGO-1518 public void findAndRemoveManyShouldUseCollationWhenPresent() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java index e1856704c7..7f67c69f86 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java @@ -15,10 +15,10 @@ */ package org.springframework.data.mongodb.core; +import static org.assertj.core.api.Assertions.*; import static org.springframework.data.mongodb.core.aggregation.Aggregation.*; import static org.springframework.data.mongodb.core.query.Criteria.*; import static org.springframework.data.mongodb.core.query.Query.*; -import static org.springframework.data.mongodb.test.util.Assertions.*; import lombok.AllArgsConstructor; import lombok.Data; @@ -704,6 +704,32 @@ public void removeWithMonoAndCollectionShouldDeleteElement() { StepVerifier.create(template.count(new Query(), Sample.class)).expectNext(2L).verifyComplete(); } + @Test // DATAMONGO-2195 + public void removeVersionedEntityConsidersVersion() { + + PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); + person.firstName = "Dave"; + + template.insert(person).as(StepVerifier::create).expectNextCount(1).verifyComplete(); + assertThat(person.version).isZero(); + template.update(PersonWithVersionPropertyOfTypeInteger.class).matching(query(where("id").is(person.id))) + .apply(new Update().set("firstName", "Walter")).first() // + .as(StepVerifier::create) // + .expectNextCount(1) // + .verifyComplete(); + + template.remove(person).as(StepVerifier::create) // + .consumeNextWith(actual -> { + + assertThat(actual.wasAcknowledged()).isTrue(); + assertThat(actual.getDeletedCount()).isZero(); + }).verifyComplete(); + template.count(new Query(), PersonWithVersionPropertyOfTypeInteger.class) // + .as(StepVerifier::create) // + .expectNext(1L) // + .verifyComplete(); + } + @Test // DATAMONGO-1444 public void optimisticLockingHandling() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTransactionTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTransactionTests.java index d98757d3ae..c2ece77e84 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTransactionTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTransactionTests.java @@ -83,6 +83,12 @@ public void setUp() { .expectNext(Success.SUCCESS) // .verifyComplete(); + StepVerifier + .create( + MongoTestUtils.createOrReplaceCollection(DATABASE_NAME, "personWithVersionPropertyOfTypeInteger", client)) + .expectNext(Success.SUCCESS) // + .verifyComplete(); + StepVerifier.create(template.insert(DOCUMENT, COLLECTION_NAME)).expectNextCount(1).verifyComplete(); template.insertAll(Arrays.asList(AHMANN, ARLEN, LEESHA, RENNA)) // @@ -270,4 +276,50 @@ public void errorInFlowOutsideTransactionDoesNotAbortIt() { .expectNext(2L) // .verifyComplete(); } + + @Test // DATAMONGO-2195 + public void deleteWithMatchingVersion() { + + PersonWithVersionPropertyOfTypeInteger rojer = new PersonWithVersionPropertyOfTypeInteger(); + rojer.firstName = "rojer"; + + PersonWithVersionPropertyOfTypeInteger saved = template.insert(rojer).block(); + + template.inTransaction().execute(action -> action.remove(saved)) // + .as(StepVerifier::create) // + .consumeNextWith(result -> assertThat(result.getDeletedCount()).isOne()) // + .verifyComplete(); + } + + @Test // DATAMONGO-2195 + public void deleteWithVersionMismatch() { + + PersonWithVersionPropertyOfTypeInteger rojer = new PersonWithVersionPropertyOfTypeInteger(); + rojer.firstName = "rojer"; + + PersonWithVersionPropertyOfTypeInteger saved = template.insert(rojer).block(); + saved.version = 5; + + template.inTransaction().execute(action -> action.remove(saved)) // + .as(StepVerifier::create) // + .consumeNextWith(actual -> { + + assertThat(actual.wasAcknowledged()).isTrue(); + assertThat(actual.getDeletedCount()).isZero(); + }).verifyComplete(); + } + + @Test // DATAMONGO-2195 + public void deleteNonExistingWithVersion() { + + PersonWithVersionPropertyOfTypeInteger rojer = new PersonWithVersionPropertyOfTypeInteger(); + rojer.id = "deceased"; + rojer.firstName = "rojer"; + rojer.version = 5; + + template.inTransaction().execute(action -> action.remove(rojer)) // + .as(StepVerifier::create) // + .consumeNextWith(result -> assertThat(result.getDeletedCount()).isZero()) // + .verifyComplete(); + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/VersionedPerson.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/VersionedPerson.java new file mode 100644 index 0000000000..049e801e4d --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/VersionedPerson.java @@ -0,0 +1,47 @@ +/* + * Copyright 2019 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.repository; + +import lombok.Data; +import lombok.NoArgsConstructor; + +import org.springframework.data.annotation.Version; +import org.springframework.data.mongodb.core.mapping.Document; +import org.springframework.lang.Nullable; + +/** + * @author Christoph Strobl + */ +@Document +@Data +@NoArgsConstructor +public class VersionedPerson extends Contact { + + private String firstname; + private @Nullable String lastname; + + private @Version Long version; + + public VersionedPerson(String firstname) { + this(firstname, null); + } + + public VersionedPerson(String firstname, @Nullable String lastname) { + + this.firstname = firstname; + this.lastname = lastname; + } +} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepositoryVersionedEntityTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepositoryVersionedEntityTests.java new file mode 100644 index 0000000000..99b0cd088d --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepositoryVersionedEntityTests.java @@ -0,0 +1,176 @@ +/* + * Copyright 2019 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.repository.support; + +import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.Assumptions.*; +import static org.springframework.data.mongodb.core.query.Criteria.*; +import static org.springframework.data.mongodb.core.query.Query.*; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Configuration; +import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.data.mongodb.MongoTransactionManager; +import org.springframework.data.mongodb.config.AbstractMongoClientConfiguration; +import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.core.mapping.BasicMongoPersistentEntity; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.repository.VersionedPerson; +import org.springframework.data.mongodb.repository.query.MongoEntityInformation; +import org.springframework.data.mongodb.test.util.MongoVersion; +import org.springframework.data.mongodb.test.util.ReplicaSet; +import org.springframework.data.util.ClassTypeInformation; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.transaction.support.TransactionTemplate; + +import com.mongodb.client.MongoClient; +import com.mongodb.client.MongoClients; + +/** + * @author Christoph Strobl + * @author Mark Paluch + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration +public class SimpleMongoRepositoryVersionedEntityTests { + + @Configuration + static class Config extends AbstractMongoClientConfiguration { + + @Override + public MongoClient mongoClient() { + return MongoClients.create(); + } + + @Override + protected String getDatabaseName() { + return "database"; + } + } + + @Autowired // + private MongoTemplate template; + + private MongoEntityInformation personEntityInformation; + private SimpleMongoRepository repository; + + private VersionedPerson sarah; + + @Before + public void setUp() { + + MongoPersistentEntity entity = template.getConverter().getMappingContext() + .getRequiredPersistentEntity(VersionedPerson.class); + + personEntityInformation = new MappingMongoEntityInformation(entity); + repository = new SimpleMongoRepository<>(personEntityInformation, template); + repository.deleteAll(); + + sarah = repository.save(new VersionedPerson("Sarah", "Connor")); + } + + @Test // DATAMONGO-2195 + public void deleteWithMatchingVersion() { + + repository.delete(sarah); + + assertThat(template.count(query(where("id").is(sarah.getId())), VersionedPerson.class)).isZero(); + } + + @Test // DATAMONGO-2195 + @MongoVersion(asOf = "4.0") + public void deleteWithMatchingVersionInTx() { + + assumeThat(ReplicaSet.required().runsAsReplicaSet()).isTrue(); + + long countBefore = repository.count(); + + initTxTemplate().execute(status -> { + + VersionedPerson t800 = repository.save(new VersionedPerson("T-800")); + repository.delete(t800); + + return Void.TYPE; + }); + + assertThat(repository.count()).isEqualTo(countBefore); + } + + @Test // DATAMONGO-2195 + public void deleteWithVersionMismatch() { + + sarah.setVersion(5L); + + assertThatExceptionOfType(OptimisticLockingFailureException.class).isThrownBy(() -> repository.delete(sarah)); + + assertThat(template.count(query(where("id").is(sarah.getId())), VersionedPerson.class)).isOne(); + } + + @Test // DATAMONGO-2195 + @MongoVersion(asOf = "4.0") + public void deleteWithVersionMismatchInTx() { + + assumeThat(ReplicaSet.required().runsAsReplicaSet()).isTrue(); + + long countBefore = repository.count(); + + assertThatExceptionOfType(OptimisticLockingFailureException.class) + .isThrownBy(() -> initTxTemplate().execute(status -> { + + VersionedPerson t800 = repository.save(new VersionedPerson("T-800")); + t800.setVersion(5L); + repository.delete(t800); + + return Void.TYPE; + })); + + assertThat(repository.count()).isEqualTo(countBefore); + } + + @Test // DATAMONGO-2195 + public void deleteNonExisting() { + assertThatThrownBy(() -> repository.delete(new VersionedPerson("T-800"))) + .isInstanceOf(OptimisticLockingFailureException.class); + } + + @Test // DATAMONGO-2195 + @MongoVersion(asOf = "4.0") + public void deleteNonExistingInTx() { + + assumeThat(ReplicaSet.required().runsAsReplicaSet()).isTrue(); + + initTxTemplate().execute(status -> { + + assertThatThrownBy(() -> repository.delete(new VersionedPerson("T-800"))) + .isInstanceOf(OptimisticLockingFailureException.class); + + return Void.TYPE; + }); + } + + TransactionTemplate initTxTemplate() { + + MongoTransactionManager txmgr = new MongoTransactionManager(template.getMongoDbFactory()); + TransactionTemplate tt = new TransactionTemplate(txmgr); + tt.afterPropertiesSet(); + + return tt; + } +} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepositoryVersionedEntityTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepositoryVersionedEntityTests.java new file mode 100644 index 0000000000..b4c2f0e30c --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepositoryVersionedEntityTests.java @@ -0,0 +1,112 @@ +/* + * Copyright 2019 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.repository.support; + +import static org.springframework.data.mongodb.core.query.Criteria.*; +import static org.springframework.data.mongodb.core.query.Query.*; + +import reactor.test.StepVerifier; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Configuration; +import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.data.mongodb.config.AbstractReactiveMongoConfiguration; +import org.springframework.data.mongodb.core.ReactiveMongoTemplate; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.repository.VersionedPerson; +import org.springframework.data.mongodb.repository.query.MongoEntityInformation; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import com.mongodb.reactivestreams.client.MongoClient; +import com.mongodb.reactivestreams.client.MongoClients; + +/** + * @author Mark Paluch + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration +public class SimpleReactiveMongoRepositoryVersionedEntityTests { + + @Configuration + static class Config extends AbstractReactiveMongoConfiguration { + + @Override + public MongoClient reactiveMongoClient() { + return MongoClients.create(); + } + + @Override + protected String getDatabaseName() { + return "database"; + } + } + + @Autowired // + private ReactiveMongoTemplate template; + + private MongoEntityInformation personEntityInformation; + private SimpleReactiveMongoRepository repository; + + private VersionedPerson sarah; + + @Before + public void setUp() { + + MongoPersistentEntity entity = template.getConverter().getMappingContext() + .getRequiredPersistentEntity(VersionedPerson.class); + + personEntityInformation = new MappingMongoEntityInformation(entity); + repository = new SimpleReactiveMongoRepository<>(personEntityInformation, template); + repository.deleteAll().as(StepVerifier::create).verifyComplete(); + + sarah = repository.save(new VersionedPerson("Sarah", "Connor")).block(); + } + + @Test // DATAMONGO-2195 + public void deleteWithMatchingVersion() { + + repository.delete(sarah).as(StepVerifier::create).verifyComplete(); + + template.count(query(where("id").is(sarah.getId())), VersionedPerson.class) // + .as(StepVerifier::create) // + .expectNext(0L).verifyComplete(); + } + + @Test // DATAMONGO-2195 + public void deleteWithVersionMismatch() { + + sarah.setVersion(5L); + + repository.delete(sarah).as(StepVerifier::create).verifyError(OptimisticLockingFailureException.class); + + template.count(query(where("id").is(sarah.getId())), VersionedPerson.class) // + .as(StepVerifier::create) // + .expectNextCount(1) // + .verifyComplete(); + } + + @Test // DATAMONGO-2195 + public void deleteNonExisting() { + + repository.delete(new VersionedPerson("T-800")).as(StepVerifier::create) + .verifyError(OptimisticLockingFailureException.class); + } + +} diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc index 80ec95981e..b3eedd7ce5 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -6,6 +6,8 @@ * <> * <> via `ReactiveQuerydslPredicateExecutor`. * <>. +* Template API delete by entity considers the version property in delete queries. +* Repository deletes now throw `OptimisticLockingFailureException` when a versioned entity cannot be deleted. [[new-features.2-1-0]] == What's New in Spring Data MongoDB 2.1 diff --git a/src/main/asciidoc/reference/mongodb.adoc b/src/main/asciidoc/reference/mongodb.adoc index 0f5a11211d..3d7fa2a784 100644 --- a/src/main/asciidoc/reference/mongodb.adoc +++ b/src/main/asciidoc/reference/mongodb.adoc @@ -1080,6 +1080,12 @@ template.save(tmp); // throws OptimisticLockingFailureException IMPORTANT: Optimistic Locking requires to set the `WriteConcern` to `ACKNOWLEDGED`. Otherwise `OptimisticLockingFailureException` can be silently swallowed. +NOTE: As of Version 2.2 `MongoOperations` also includes the `@Version` property when removing an entity from the database. +To remove a `Document` without version check use `MongoOperations#remove(Query,...)` instead of `MongoOperations#remove(Object)`. + +NOTE: As of Version 2.2 repositories check for the outcome of acknowledged deletes when removing versioned entities. +An `OptimisticLockingFailureException` is raised if a versioned entity cannot be deleted through `CrudRepository.delete(Object)`. In such case, the version was changed or the object was deleted in the meantime. Use `CrudRepository.deleteById(ID)` to bypass optimistic locking functionality and delete objects regardless of their version. + [[mongo.query]] == Querying Documents