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