From 7500ba18fdfc63a09fdeca21f045a66ddd7b97e2 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 6 Feb 2019 11:29:17 +0100 Subject: [PATCH] DATAMONGO-2195 - Throw OptimisticLockingFailureException on delete only in repositories. OptimisticLockingFailureException is now thrown only when deleting an entity through a repository and no longer when using the Template API. Original pull request: #641. --- .../data/mongodb/core/MongoTemplate.java | 26 ---- .../mongodb/core/ReactiveMongoTemplate.java | 29 ----- .../data/mongodb/core/ResultOperations.java | 76 ------------ .../query/MongoEntityInformation.java | 23 ++++ .../MappingMongoEntityInformation.java | 34 +++++- .../support/SimpleMongoRepository.java | 12 +- .../SimpleReactiveMongoRepository.java | 24 +++- .../data/mongodb/core/MongoTemplateTests.java | 48 +------- .../core/ReactiveMongoTemplateTests.java | 60 ++-------- ...ReactiveMongoTemplateTransactionTests.java | 8 +- ...leMongoRepositoryVersionedEntityTests.java | 20 ++-- ...veMongoRepositoryVersionedEntityTests.java | 112 ++++++++++++++++++ src/main/asciidoc/new-features.adoc | 2 + src/main/asciidoc/reference/mongodb.adoc | 7 +- 14 files changed, 233 insertions(+), 248 deletions(-) delete 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/support/SimpleReactiveMongoRepositoryVersionedEntityTests.java 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 15ba3d16b..2bd0e7569 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 @@ -1730,36 +1730,10 @@ 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 17b180724..f92cd7632 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 @@ -1824,35 +1824,6 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati return collectionToUse.deleteMany(removeQuery, deleteOptions); } - }).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 deleted file mode 100644 index da43ff0fa..000000000 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ResultOperations.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * 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/query/MongoEntityInformation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoEntityInformation.java index 71caf6bfb..e1f9ae8ee 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,25 @@ public interface MongoEntityInformation extends EntityInformation * @return */ String getIdAttribute(); + + /** + * Returns whether the entity uses optimistic locking. + * + * @return + * @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 + */ + @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 4d4fc6de9..e9c1814ed 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 @@ public class MappingMongoEntityInformation extends PersistentEntityInform 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 class MappingMongoEntityInformation extends PersistentEntityInform * @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 MappingMongoEntityInformation extends PersistentEntityInform 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 c2fc4935c..cccfcea6f 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.List; 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.data.util.Streamable; 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 class SimpleMongoRepository implements MongoRepository { Assert.notNull(entity, "The given entity must not be null!"); - mongoOperations.remove(entity, entityInformation.getCollectionName()); + 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 928255da1..019fdd3ec 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 java.util.stream.Collectors; 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.StreamUtils; 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 class SimpleReactiveMongoRepository implement Assert.notNull(entity, "The given entity must not be null!"); - return mongoOperations.remove(entity, entityInformation.getCollectionName()).then(); + 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 class SimpleReactiveMongoRepository implement 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 eb271caf2..ee2d8d360 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 @@ -1648,20 +1648,7 @@ public class MongoTemplateTests { } @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() { + public void removeVersionedEntityConsidersVersion() { PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); person.firstName = "Dave"; @@ -1671,38 +1658,13 @@ public class MongoTemplateTests { 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"); + DeleteResult deleteResult = template.remove(person); + + assertThat(deleteResult.wasAcknowledged()).isTrue(); + assertThat(deleteResult.getDeletedCount()).isZero(); 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 6cd2b9dd2..7f67c69f8 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; @@ -705,23 +705,7 @@ public class ReactiveMongoTemplateTests { } @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() { + public void removeVersionedEntityConsidersVersion() { PersonWithVersionPropertyOfTypeInteger person = new PersonWithVersionPropertyOfTypeInteger(); person.firstName = "Dave"; @@ -734,42 +718,12 @@ public class ReactiveMongoTemplateTests { .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(); - } + template.remove(person).as(StepVerifier::create) // + .consumeNextWith(actual -> { - @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(); + assertThat(actual.wasAcknowledged()).isTrue(); + assertThat(actual.getDeletedCount()).isZero(); + }).verifyComplete(); template.count(new Query(), PersonWithVersionPropertyOfTypeInteger.class) // .as(StepVerifier::create) // .expectNext(1L) // 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 2a2bd1357..c2ece77e8 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,7 +31,6 @@ 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; @@ -303,8 +302,11 @@ public class ReactiveMongoTemplateTransactionTests { template.inTransaction().execute(action -> action.remove(saved)) // .as(StepVerifier::create) // - .expectError(OptimisticLockingFailureException.class) // - .verify(); + .consumeNextWith(actual -> { + + assertThat(actual.wasAcknowledged()).isTrue(); + assertThat(actual.getDeletedCount()).isZero(); + }).verifyComplete(); } @Test // DATAMONGO-2195 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 index fc003b6f3..99b0cd088 100644 --- 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 @@ -16,7 +16,7 @@ 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.assertj.core.api.Assumptions.*; import static org.springframework.data.mongodb.core.query.Criteria.*; import static org.springframework.data.mongodb.core.query.Query.*; @@ -45,6 +45,7 @@ import com.mongodb.client.MongoClients; /** * @author Christoph Strobl + * @author Mark Paluch */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration @@ -64,9 +65,6 @@ public class SimpleMongoRepositoryVersionedEntityTests { } } - private static final MongoPersistentEntity ENTITY = new BasicMongoPersistentEntity( - ClassTypeInformation.from(VersionedPerson.class)); - @Autowired // private MongoTemplate template; @@ -78,7 +76,10 @@ public class SimpleMongoRepositoryVersionedEntityTests { @Before public void setUp() { - personEntityInformation = new MappingMongoEntityInformation(ENTITY); + MongoPersistentEntity entity = template.getConverter().getMappingContext() + .getRequiredPersistentEntity(VersionedPerson.class); + + personEntityInformation = new MappingMongoEntityInformation(entity); repository = new SimpleMongoRepository<>(personEntityInformation, template); repository.deleteAll(); @@ -117,8 +118,7 @@ public class SimpleMongoRepositoryVersionedEntityTests { sarah.setVersion(5L); - assertThatExceptionOfType(OptimisticLockingFailureException.class).isThrownBy(() -> repository.delete(sarah)) // - .withMessageContaining("Expected version 5 but was 0"); + assertThatExceptionOfType(OptimisticLockingFailureException.class).isThrownBy(() -> repository.delete(sarah)); assertThat(template.count(query(where("id").is(sarah.getId())), VersionedPerson.class)).isOne(); } @@ -146,7 +146,8 @@ public class SimpleMongoRepositoryVersionedEntityTests { @Test // DATAMONGO-2195 public void deleteNonExisting() { - repository.delete(new VersionedPerson("T-800")); + assertThatThrownBy(() -> repository.delete(new VersionedPerson("T-800"))) + .isInstanceOf(OptimisticLockingFailureException.class); } @Test // DATAMONGO-2195 @@ -157,7 +158,8 @@ public class SimpleMongoRepositoryVersionedEntityTests { initTxTemplate().execute(status -> { - repository.delete(new VersionedPerson("T-800")); + assertThatThrownBy(() -> repository.delete(new VersionedPerson("T-800"))) + .isInstanceOf(OptimisticLockingFailureException.class); return Void.TYPE; }); 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 000000000..b4c2f0e30 --- /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 a742ce0e2..df0ed829d 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -7,6 +7,8 @@ * <> via `ReactiveQuerydslPredicateExecutor`. * <>. * Extended SpEL aggregation support for MongoDB 3.4 and MongoDB 4.0 operators (see <>). +* 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 11b9095c1..3d7fa2a78 100644 --- a/src/main/asciidoc/reference/mongodb.adoc +++ b/src/main/asciidoc/reference/mongodb.adoc @@ -1080,11 +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, 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. +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