Spring/spring-data-mongodb
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATAMONGO-2179 - Fixed broken auditing for entities using optimistic locking via batch save.

Browse Source 
The previous implementation of (Reactive)MongoTemplate.doInsertBatch(…) prematurely initialized the version property so that the entity wasn't considered new by the auditing subsystem. Even worse, for primitive version properties, the initialization kept the property at a value of 0, so that the just persisted entity was still considered new. This mean that via the repository route, inserts are triggered even for subsequent attempts to save an entity which caused duplicate key exceptions.

We now make sure we fire the BeforeConvertEvent before the version property is initialized or updated. Also, the initialization of the property now sets primitive properties to 1 initially.

Related tickets: DATAMONGO-2139, DATAMONGO-2150.

Original Pull Request: #632
This commit is contained in:
Mark Paluch
2019-01-07 08:42:43 +01:00
committed by Christoph Strobl
parent 8a34bc46a2
commit 723b481f82
4 changed files with 59 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java
View File

@@ -1309,17 +1309,18 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
List<T> initializedBatchToSave = new ArrayList<>(batchToSave.size());
for (T uninitialized : batchToSave) {
AdaptibleEntity<T> entity = operations.forEntity(uninitialized, mongoConverter.getConversionService());
T toSave = entity.initializeVersionProperty();
BeforeConvertEvent<T> event = new BeforeConvertEvent<>(uninitialized, collectionName);
T toConvert = maybeEmitEvent(event).getSource();
BeforeConvertEvent<T> event = new BeforeConvertEvent<>(toSave, collectionName);
toSave = maybeEmitEvent(event).getSource();
AdaptibleEntity<T> entity = operations.forEntity(toConvert, mongoConverter.getConversionService());
entity.assertUpdateableIdIfNotSet();
T initialized = entity.initializeVersionProperty();
Document document = entity.toMappedDocument(writer).getDocument();
maybeEmitEvent(new BeforeSaveEvent<>(initialized, document, collectionName));
maybeEmitEvent(new BeforeSaveEvent<>(toSave, document, collectionName));
documentList.add(document);
initializedBatchToSave.add(toSave);
initializedBatchToSave.add(initialized);
}
List<Object> ids = insertDocumentList(collectionName, documentList);

24
spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java
View File

@@ -1239,9 +1239,10 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati
BeforeConvertEvent<T> event = new BeforeConvertEvent<>(objectToSave, collectionName);
T toConvert = maybeEmitEvent(event).getSource();
AdaptibleEntity<T> entity = operations.forEntity(toConvert, mongoConverter.getConversionService());
AdaptibleEntity<T> entity = operations.forEntity(toConvert, mongoConverter.getConversionService());
entity.assertUpdateableIdIfNotSet();
T initialized = entity.initializeVersionProperty();
Document dbDoc = entity.toMappedDocument(writer).getDocument();
@@ -1314,19 +1315,22 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati
Assert.notNull(writer, "MongoWriter must not be null!");
Mono<List<Tuple2<AdaptibleEntity<T>, Document>>> prepareDocuments = Flux.fromIterable(batchToSave).map(o -> {
Mono<List<Tuple2<AdaptibleEntity<T>, Document>>> prepareDocuments = Flux.fromIterable(batchToSave)
.map(uninitialized -> {
AdaptibleEntity<T> entity = operations.forEntity(o, mongoConverter.getConversionService());
T toSave = entity.initializeVersionProperty();
BeforeConvertEvent<T> event = new BeforeConvertEvent<>(uninitialized, collectionName);
T toConvert = maybeEmitEvent(event).getSource();
BeforeConvertEvent<T> event = new BeforeConvertEvent<>(toSave, collectionName);
toSave = maybeEmitEvent(event).getSource();
AdaptibleEntity<T> entity = operations.forEntity(toConvert, mongoConverter.getConversionService());
entity.assertUpdateableIdIfNotSet();
Document dbDoc = entity.toMappedDocument(writer).getDocument();
T initialized = entity.initializeVersionProperty();
Document dbDoc = entity.toMappedDocument(writer).getDocument();
maybeEmitEvent(new BeforeSaveEvent<>(toSave, dbDoc, collectionName));
return Tuples.of(entity, dbDoc);
}).collectList();
maybeEmitEvent(new BeforeSaveEvent<>(initialized, dbDoc, collectionName));
return Tuples.of(entity, dbDoc);
}).collectList();
Flux<Tuple2<AdaptibleEntity<T>, Document>> insertDocuments = prepareDocuments.flatMapMany(tuples -> {

20
spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/AuditingViaJavaConfigRepositoriesTests.java
View File

@@ -20,6 +20,7 @@ import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.*;
import java.util.Collections;
import java.util.Optional;
import java.util.function.Function;
@@ -121,15 +122,27 @@ public class AuditingViaJavaConfigRepositoriesTests {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
auditablePersonRepository::save, //
null, 0L, 1L);
}
@Test // DATAMONGO-2179
public void auditingWorksForVersionedEntityBatchWithWrapperVersion() {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
s -> auditablePersonRepository.saveAll(Collections.singletonList(s)).get(0), //
null, 0L, 1L);
}
@Test // DATAMONGO-2139
public void auditingWorksForVersionedEntityWithSimpleVersion() {
verifyAuditingViaVersionProperty(new SimpleVersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
auditablePersonRepository::save, //
0L, 1L, 2L);
}
@@ -139,6 +152,7 @@ public class AuditingViaJavaConfigRepositoriesTests {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
operations::save, //
null, 0L, 1L);
}
@@ -148,21 +162,25 @@ public class AuditingViaJavaConfigRepositoriesTests {
verifyAuditingViaVersionProperty(new SimpleVersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
operations::save, //
0L, 1L, 2L);
}
private <T extends AuditablePerson> void verifyAuditingViaVersionProperty(T instance,
Function<T, Object> versionExtractor, Function<T, T> persister, Object... expectedValues) {
Function<T, Object> versionExtractor, Function<T, Object> createdDateExtractor, Function<T, T> persister,
Object... expectedValues) {
MongoPersistentEntity<?> entity = context.getRequiredPersistentEntity(instance.getClass());
assertThat(versionExtractor.apply(instance)).isEqualTo(expectedValues[0]);
assertThat(createdDateExtractor.apply(instance)).isNull();
assertThat(entity.isNew(instance)).isTrue();
instance = persister.apply(instance);
assertThat(versionExtractor.apply(instance)).isEqualTo(expectedValues[1]);
assertThat(createdDateExtractor.apply(instance)).isNotNull();
assertThat(entity.isNew(instance)).isFalse();
instance = persister.apply(instance);

20
spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/ReactiveAuditingTests.java
View File

@@ -21,6 +21,7 @@ import static org.mockito.Mockito.*;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
@@ -84,15 +85,27 @@ public class ReactiveAuditingTests {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
auditablePersonRepository::save, //
null, 0L, 1L);
}
@Test // DATAMONGO-2179
public void auditingWorksForVersionedEntityBatchWithWrapperVersion() {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
s -> auditablePersonRepository.saveAll(Collections.singletonList(s)).next(), //
null, 0L, 1L);
}
@Test // DATAMONGO-2139, DATAMONGO-2150
public void auditingWorksForVersionedEntityWithSimpleVersion() {
verifyAuditingViaVersionProperty(new SimpleVersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
auditablePersonRepository::save, //
0L, 1L, 2L);
}
@@ -102,6 +115,7 @@ public class ReactiveAuditingTests {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
operations::save, //
null, 0L, 1L);
}
@@ -110,17 +124,20 @@ public class ReactiveAuditingTests {
public void auditingWorksForVersionedEntityWithSimpleVersionOnTemplate() {
verifyAuditingViaVersionProperty(new SimpleVersionedAuditablePerson(), //
it -> it.version, //
AuditablePerson::getCreatedAt, //
operations::save, //
0L, 1L, 2L);
}
private <T extends AuditablePerson> void verifyAuditingViaVersionProperty(T instance,
Function<T, Object> versionExtractor, Function<T, Mono<T>> persister, Object... expectedValues) {
Function<T, Object> versionExtractor, Function<T, Object> createdDateExtractor, Function<T, Mono<T>> persister,
Object... expectedValues) {
AtomicReference<T> instanceHolder = new AtomicReference<>(instance);
MongoPersistentEntity<?> entity = context.getRequiredPersistentEntity(instance.getClass());
assertThat(versionExtractor.apply(instance)).isEqualTo(expectedValues[0]);
assertThat(createdDateExtractor.apply(instance)).isNull();
assertThat(entity.isNew(instance)).isTrue();
persister.apply(instanceHolder.get()) //
@@ -129,6 +146,7 @@ public class ReactiveAuditingTests {
instanceHolder.set(actual);
assertThat(versionExtractor.apply(actual)).isEqualTo(expectedValues[1]);
assertThat(createdDateExtractor.apply(instance)).isNotNull();
assertThat(entity.isNew(actual)).isFalse();
}).verifyComplete();