From 4ecf20ce4cfb6c87f9b7eaf02009ec3e913dfec7 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 4 Feb 2019 12:48:02 +0100 Subject: [PATCH] DATAMONGO-2195 - Consider version when removing an entity. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now consider a potential @Version when removing an entity. MongoOperations#remove(Object) and MongoOperations#remove(Object, String) include the version of the object to remove if the entity is versioned. Opposed to save(Object), remove(Object) does not throw OptimisticLockingFailureException if a versioned entity could not be removed. This behavior is subject to be changed in a future release. Throwing OptimisticLockingFailureException on failed delete on Template API level was not introduced to not break existing application code. MongoRepository now also considers the entities version, following the very same logic as MongoOperations. To remove an entity without version check use MongoOperations#remove(Query,…) or MongoRepository#deleteById(…). Original pull request: #641. --- .../data/mongodb/core/EntityOperations.java | 14 +- .../data/mongodb/core/MongoOperations.java | 14 +- .../data/mongodb/core/MongoTemplate.java | 43 ++++- .../mongodb/core/ReactiveMongoTemplate.java | 50 +++-- .../data/mongodb/core/ResultOperations.java | 76 ++++++++ .../support/SimpleMongoRepository.java | 2 +- .../SimpleReactiveMongoRepository.java | 2 +- .../data/mongodb/core/MongoTemplateTests.java | 57 ++++++ .../core/ReactiveMongoTemplateTests.java | 72 ++++++++ ...ReactiveMongoTemplateTransactionTests.java | 50 +++++ .../mongodb/repository/VersionedPerson.java | 47 +++++ ...leMongoRepositoryVersionedEntityTests.java | 174 ++++++++++++++++++ src/main/asciidoc/reference/mongodb.adoc | 5 + 13 files changed, 584 insertions(+), 22 deletions(-) create mode 100644 spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ResultOperations.java create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/VersionedPerson.java create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepositoryVersionedEntityTests.java 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 7560b5bde..cb7870d25 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 @@ class EntityOperations { */ 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. * @@ -490,10 +500,10 @@ class EntityOperations { 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 2d66986d6..89f693d15 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,19 +1350,29 @@ public interface MongoOperations extends FluentMongoOperations { 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}. * * @param object must not be {@literal null}. * @return the {@link DeleteResult} which lets you access the results of the previous delete. + * @throws org.springframework.dao.OptimisticLockingFailureException if the given {@literal object} is + * {@link org.springframework.data.annotation.Version versioned} and the current version does not match the + * one within the store. If no document with matching {@literal _id} exists in the collection the operation + * completes without error. */ 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}. * * @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. * @return the {@link DeleteResult} which lets you access the results of the previous delete. + * @throws org.springframework.dao.OptimisticLockingFailureException if the given {@literal object} is + * {@link org.springframework.data.annotation.Version versioned} and the current version does not match the + * one within the store. If no document with matching {@literal _id} exists in the collection the operation + * completes without error. */ DeleteResult remove(Object object, String collectionName); 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 7ed5a1883..15ba3d16b 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.AggregationResults; 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 class MongoTemplate implements MongoOperations, ApplicationContextAware, 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 class MongoTemplate implements MongoOperations, ApplicationContextAware, 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); } @@ -1723,10 +1730,36 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, DeleteResult result = multi ? collectionToUse.deleteMany(removeQuery, options) : collectionToUse.deleteOne(removeQuery, options); + checkForOptimisticLockingFailures(result, removeQuery, collectionToUse); + maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName)); return result; } + + private void checkForOptimisticLockingFailures(DeleteResult result, Document removeQuery, + MongoCollection collectionToUse) { + + if (multi || !ResultOperations.isUndecidedDeleteResult(result, removeQuery, entity)) { + return; + } + + String versionFieldName = entity.getVersionProperty().getFieldName(); + Document idQuery = new Document(removeQuery); + idQuery.remove(versionFieldName); + + Iterator it = collectionToUse.find(idQuery).projection(Projections.include("_id", versionFieldName)) + .limit(1).iterator(); + + if (it.hasNext()) { + + Document source = it.next(); + throw ResultOperations.newDeleteVersionedOptimisticLockingException(source.get("_id"), collectionName, + removeQuery.get(versionFieldName), source.get(versionFieldName)); + + } + + } }); } 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 73b9ea6b5..17b180724 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 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati return false; } - return document.containsKey(persistentEntity.getRequiredIdProperty().getFieldName()); + return document.containsKey(persistentEntity.getRequiredVersionProperty().getFieldName()); } /* @@ -1717,7 +1717,7 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati 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 class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati 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 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati 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 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati 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,39 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati 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))) + }).flatMap(deleteResult -> { + + if (!ResultOperations.isUndecidedDeleteResult(deleteResult, removeQuery, entity)) { + return Mono.just(deleteResult); + } + + return execute(collectionName, (coll -> { + + String versionFieldName = entity.getVersionProperty().getFieldName(); + + Document queryWithoutVersion = new Document(removeQuery); + queryWithoutVersion.remove(versionFieldName); + + Publisher publisher = coll.find(queryWithoutVersion) + .projection(Projections.include("_id", entity.getVersionProperty().getFieldName())).limit(1).first(); + + return Mono.from(publisher) // + .defaultIfEmpty(new Document()) // + .flatMap(it -> { + + if (it.isEmpty()) { + return Mono.just(deleteResult); + } + + return Mono.error(ResultOperations.newDeleteVersionedOptimisticLockingException(it.get("_id"), + collectionName, removeQuery.get(versionFieldName), it.get(versionFieldName))); + }); + })).next(); + + }).doOnNext(it -> maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName))) // .next(); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ResultOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ResultOperations.java new file mode 100644 index 000000000..da43ff0fa --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ResultOperations.java @@ -0,0 +1,76 @@ +/* + * 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.core; + +import org.bson.Document; +import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.lang.Nullable; + +import com.mongodb.client.result.DeleteResult; +import com.mongodb.client.result.UpdateResult; + +/** + * {@link ResultOperations} is intended to share logic depending on {@link DeleteResult} and + * {@link com.mongodb.client.result.UpdateResult} between reactive and imperative implementations. + * + * @author Christoph Strobl + * @since 2.2 + */ +class ResultOperations { + + /** + * Decide if a given {@link DeleteResult} needs further inspection. Returns {@literal true} if the delete of a + * {@link org.springframework.data.annotation.Version versioned} entity did not remove any documents although the + * operation has been {@link DeleteResult#wasAcknowledged() acknowledged}. + * + * @param deleteResult must not be {@literal null}. + * @param query the actual query used for the delete operation. + * @param entity can be {@literal null}. + * @return {@literal true} if it cannot be decided if nothing got deleted because of a potential + * {@link org.springframework.data.annotation.Version version} mismatch, or if the document did not exist in + * first place. + */ + static boolean isUndecidedDeleteResult(DeleteResult deleteResult, org.bson.Document query, + @Nullable MongoPersistentEntity entity) { + + if (!deleteResult.wasAcknowledged() || deleteResult.getDeletedCount() > 0) { + return false; + } + + if (entity == null) { + return false; + } + + if (!entity.hasVersionProperty()) { + return false; + } + + return containsVersionProperty(query, entity); + } + + static OptimisticLockingFailureException newDeleteVersionedOptimisticLockingException(Object id, + String collectionName, Object expectedVersion, Object actualVersion) { + + throw new OptimisticLockingFailureException( + String.format("The entity with id %s in %s has changed and cannot be deleted! " + System.lineSeparator() + // + "Expected version %s but was %s.", id, collectionName, expectedVersion, actualVersion)); + } + + private static boolean containsVersionProperty(Document document, MongoPersistentEntity entity) { + return document.containsKey(entity.getRequiredVersionProperty().getFieldName()); + } +} 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 de52950c5..c2fc4935c 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 @@ -161,7 +161,7 @@ public class SimpleMongoRepository implements MongoRepository { Assert.notNull(entity, "The given entity must not be null!"); - deleteById(entityInformation.getRequiredId(entity)); + mongoOperations.remove(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 6802a20c9..928255da1 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 @@ -355,7 +355,7 @@ public class SimpleReactiveMongoRepository implement Assert.notNull(entity, "The given entity must not be null!"); - return deleteById(entityInformation.getRequiredId(entity)); + return mongoOperations.remove(entity, entityInformation.getCollectionName()).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 2e72afa49..eb271caf2 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,6 +43,7 @@ import java.util.concurrent.atomic.AtomicReference; 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.joda.time.DateTime; @@ -1646,6 +1647,62 @@ public class MongoTemplateTests { assertThat(person.version, is(0)); } + @Test // DATAMONGO-2195 + public void removeEntityWithMatchingVersion() { + + PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); + person.firstName = "Dave"; + + template.insert(person); + assertThat(person.version, is(0)); + + template.remove(person); + assertThat(template.count(new Query(), Person.class)).isZero(); + } + + @Test // DATAMONGO-2195 + public void removeEntityWithoutMatchingVersionThrowsOptimisticLockingFailureException() { + + 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(); + + Assertions.assertThatThrownBy(() -> template.remove(person)).isInstanceOf(OptimisticLockingFailureException.class) + .hasMessageContaining("Expected version 0 but was 1"); + assertThat(template.count(new Query(), PersonWithVersionPropertyOfTypeInteger.class)).isOne(); + } + + @Test // DATAMONGO-2195 + public void removeNonExistingVersionedEntityJustPasses() { + + PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); + person.id = "id-1"; + person.firstName = "Dave"; + person.version = 10; + + assertThat(template.remove(person).getDeletedCount()).isZero(); + } + + @Test // DATAMONGO-2195 + @DirtiesContext + public void removeEntityWithoutMatchingVersionWhenUsingUnaknowledgedWriteDoesNotThrowsOptimisticLockingFailureException() { + + 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(); + + template.setWriteConcern(WriteConcern.UNACKNOWLEDGED); + assertThat(template.remove(person).wasAcknowledged()).isFalse(); + } + @Test // DATAMONGO-588 public void initializesVersionOnBatchInsert() { 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 e1856704c..6cd2b9dd2 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 @@ -704,6 +704,78 @@ public class ReactiveMongoTemplateTests { StepVerifier.create(template.count(new Query(), Sample.class)).expectNext(2L).verifyComplete(); } + @Test // DATAMONGO-2195 + public void removeEntityWithMatchingVersion() { + + PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); + person.firstName = "Dave"; + + template.insert(person).as(StepVerifier::create).expectNextCount(1).verifyComplete(); + assertThat(person.version).isZero(); + + template.remove(person).as(StepVerifier::create).expectNextCount(1).verifyComplete(); + template.count(new Query(), PersonWithVersionPropertyOfTypeInteger.class) // + .as(StepVerifier::create) // + .expectNext(0L) // + .verifyComplete(); + } + + @Test // DATAMONGO-2195 + public void removeEntityWithoutMatchingVersionThrowsOptimisticLockingFailureException() { + + 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).expectError(OptimisticLockingFailureException.class).verify(); + template.count(new Query(), PersonWithVersionPropertyOfTypeInteger.class) // + .as(StepVerifier::create) // + .expectNext(1L) // + .verifyComplete(); + } + + @Test // DATAMONGO-2195 + public void removeNonExistingVersionedEntityJustPasses() { + + PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); + person.id = "id-1"; + person.firstName = "Dave"; + person.version = 10; + + template.remove(person).as(StepVerifier::create).expectNextCount(1).verifyComplete(); + } + + @Test // DATAMONGO-2195 + @DirtiesContext + public void removeEntityWithoutMatchingVersionWhenUsingUnaknowledgedWriteDoesNotThrowsOptimisticLockingFailureException() { + + 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.setWriteConcern(WriteConcern.UNACKNOWLEDGED); + + template.remove(person).as(StepVerifier::create).expectNextCount(1).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 d98757d3a..2a2bd1357 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 @@ -31,6 +31,7 @@ import org.junit.ClassRule; import org.junit.Test; import org.junit.rules.TestRule; import org.reactivestreams.Publisher; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.domain.Sort; import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.test.util.MongoTestUtils; @@ -83,6 +84,12 @@ public class ReactiveMongoTemplateTransactionTests { .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 +277,47 @@ public class ReactiveMongoTemplateTransactionTests { .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) // + .expectError(OptimisticLockingFailureException.class) // + .verify(); + } + + @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 000000000..049e801e4 --- /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 000000000..fc003b6f3 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepositoryVersionedEntityTests.java @@ -0,0 +1,174 @@ +/* + * 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.assumeThat; +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 + */ +@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"; + } + } + + private static final MongoPersistentEntity ENTITY = new BasicMongoPersistentEntity( + ClassTypeInformation.from(VersionedPerson.class)); + + @Autowired // + private MongoTemplate template; + + private MongoEntityInformation personEntityInformation; + private SimpleMongoRepository repository; + + private VersionedPerson sarah; + + @Before + public void setUp() { + + 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)) // + .withMessageContaining("Expected version 5 but was 0"); + + 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() { + repository.delete(new VersionedPerson("T-800")); + } + + @Test // DATAMONGO-2195 + @MongoVersion(asOf = "4.0") + public void deleteNonExistingInTx() { + + assumeThat(ReplicaSet.required().runsAsReplicaSet()).isTrue(); + + initTxTemplate().execute(status -> { + + repository.delete(new VersionedPerson("T-800")); + + return Void.TYPE; + }); + } + + TransactionTemplate initTxTemplate() { + + MongoTransactionManager txmgr = new MongoTransactionManager(template.getMongoDbFactory()); + TransactionTemplate tt = new TransactionTemplate(txmgr); + tt.afterPropertiesSet(); + + return tt; + } +} diff --git a/src/main/asciidoc/reference/mongodb.adoc b/src/main/asciidoc/reference/mongodb.adoc index 0f5a11211..11b9095c1 100644 --- a/src/main/asciidoc/reference/mongodb.adoc +++ b/src/main/asciidoc/reference/mongodb.adoc @@ -1080,6 +1080,11 @@ 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, raising an +`OptimisticLockingFailureException` if a document with matching `_id` but a `version` mismatch shall be deleted. In cases +where no Document with matching `_id` can be found the operation passes without error. +To remove a `Document` without version check use `MongoOperations#remove(Query,...)` instead of `MongoOperations#remove(Object)`. + [[mongo.query]] == Querying Documents