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

DATAMONGO-2108 - Fixed broken auditing for entities using optimistic locking.

Browse Source 
The previous implementation of MongoTemplate.doSaveVersioned(…) 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.

Added integration tests for the auditing via MongoOperations and repositories.
This commit is contained in:
Oliver Drotbohm
2018-10-18 16:54:54 +02:00
parent 5982ee84f7
commit 2253d3e301
3 changed files with 137 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java
View File

@@ -242,6 +242,14 @@ class EntityOperations {
* @return
*/
T getBean();
/**
* Returns whether the entity is considered to be new.
*
* @return
* @since 2.1.2
*/
boolean isNew();
}
/**
@@ -387,6 +395,15 @@ class EntityOperations {
public T getBean() {
return map;
}
/*
* (non-Javadoc)
* @see org.springframework.data.mongodb.core.EntityOperations.Entity#isNew()
*/
@Override
public boolean isNew() {
return map.get(ID_FIELD) != null;
}
}
private static class SimpleMappedEntity<T extends Map<String, Object>> extends UnmappedEntity<T> {
@@ -549,6 +566,15 @@ class EntityOperations {
public T getBean() {
return propertyAccessor.getBean();
}
/*
* (non-Javadoc)
* @see org.springframework.data.mongodb.core.EntityOperations.Entity#isNew()
*/
@Override
public boolean isNew() {
return entity.isNew(propertyAccessor.getBean());
}
}
private static class AdaptibleMappedEntity<T> extends MappedEntity<T> implements AdaptibleEntity<T> {
@@ -631,7 +657,9 @@ class EntityOperations {
return propertyAccessor.getBean();
}
propertyAccessor.setProperty(entity.getRequiredVersionProperty(), 0);
MongoPersistentProperty versionProperty = entity.getRequiredVersionProperty();
propertyAccessor.setProperty(versionProperty, versionProperty.getType().isPrimitive() ? 1 : 0);
return propertyAccessor.getBean();
}

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

@@ -1212,20 +1212,19 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
protected <T> T doInsert(String collectionName, T objectToSave, MongoWriter<T> writer) {
AdaptibleEntity<T> entity = operations.forEntity(objectToSave, mongoConverter.getConversionService());
T toSave = entity.initializeVersionProperty();
BeforeConvertEvent<T> event = new BeforeConvertEvent<>(toSave, collectionName);
toSave = maybeEmitEvent(event).getSource();
BeforeConvertEvent<T> event = new BeforeConvertEvent<>(objectToSave, collectionName);
T toConvert = maybeEmitEvent(event).getSource();
AdaptibleEntity<T> entity = operations.forEntity(toConvert, mongoConverter.getConversionService());
entity.assertUpdateableIdIfNotSet();
T initialized = entity.initializeVersionProperty();
Document dbDoc = entity.toMappedDocument(writer).getDocument();
maybeEmitEvent(new BeforeSaveEvent<>(toSave, dbDoc, collectionName));
Object id = insertDocument(collectionName, dbDoc, toSave.getClass());
maybeEmitEvent(new BeforeSaveEvent<>(initialized, dbDoc, collectionName));
Object id = insertDocument(collectionName, dbDoc, initialized.getClass());
T saved = populateIdIfNecessary(toSave, id);
T saved = populateIdIfNecessary(initialized, id);
maybeEmitEvent(new AfterSaveEvent<>(saved, dbDoc, collectionName));
return saved;
@@ -1357,38 +1356,36 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
@SuppressWarnings("unchecked")
private <T> T doSaveVersioned(AdaptibleEntity<T> source, String collectionName) {
Number number = source.getVersion();
if (number != null) {
// Create query for entity with the id and old version
Query query = source.getQueryForVersion();
// Bump version number
T toSave = source.incrementVersion();
toSave = maybeEmitEvent(new BeforeConvertEvent<T>(toSave, collectionName)).getSource();
source.assertUpdateableIdIfNotSet();
MappedDocument mapped = source.toMappedDocument(mongoConverter);
maybeEmitEvent(new BeforeSaveEvent<>(toSave, mapped.getDocument(), collectionName));
Update update = mapped.updateWithoutId();
UpdateResult result = doUpdate(collectionName, query, update, toSave.getClass(), false, false);
if (result.getModifiedCount() == 0) {
throw new OptimisticLockingFailureException(
String.format("Cannot save entity %s with version %s to collection %s. Has it been modified meanwhile?",
source.getId(), number, collectionName));
}
maybeEmitEvent(new AfterSaveEvent<>(toSave, mapped.getDocument(), collectionName));
return toSave;
if (source.isNew()) {
return (T) doInsert(collectionName, source.getBean(), this.mongoConverter);
}
return (T) doInsert(collectionName, source.getBean(), this.mongoConverter);
// Create query for entity with the id and old version
Query query = source.getQueryForVersion();
// Bump version number
T toSave = source.incrementVersion();
toSave = maybeEmitEvent(new BeforeConvertEvent<T>(toSave, collectionName)).getSource();
source.assertUpdateableIdIfNotSet();
MappedDocument mapped = source.toMappedDocument(mongoConverter);
maybeEmitEvent(new BeforeSaveEvent<>(toSave, mapped.getDocument(), collectionName));
Update update = mapped.updateWithoutId();
UpdateResult result = doUpdate(collectionName, query, update, toSave.getClass(), false, false);
if (result.getModifiedCount() == 0) {
throw new OptimisticLockingFailureException(
String.format("Cannot save entity %s with version %s to collection %s. Has it been modified meanwhile?",
source.getId(), source.getVersion(), collectionName));
}
maybeEmitEvent(new AfterSaveEvent<>(toSave, mapped.getDocument(), collectionName));
return toSave;
}
protected <T> T doSave(String collectionName, T objectToSave, MongoWriter<T> writer) {

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

@@ -15,11 +15,13 @@
*/
package org.springframework.data.mongodb.config;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.*;
import java.util.Optional;
import java.util.function.Function;
import org.junit.Before;
import org.junit.Test;
@@ -28,15 +30,18 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.annotation.Version;
import org.springframework.data.domain.AuditorAware;
import org.springframework.data.mongodb.core.AuditablePerson;
import org.springframework.data.mongodb.core.MongoOperations;
import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
import org.springframework.data.mongodb.repository.MongoRepository;
import org.springframework.data.mongodb.repository.config.EnableMongoRepositories;
import org.springframework.stereotype.Repository;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import com.mongodb.Mongo;
import com.mongodb.MongoClient;
/**
@@ -51,6 +56,9 @@ public class AuditingViaJavaConfigRepositoriesTests {
@Autowired AuditablePersonRepository auditablePersonRepository;
@Autowired AuditorAware<AuditablePerson> auditorAware;
@Autowired MongoMappingContext context;
@Autowired MongoOperations operations;
AuditablePerson auditor;
@Configuration
@@ -107,6 +115,61 @@ public class AuditingViaJavaConfigRepositoriesTests {
new AnnotationConfigApplicationContext(SimpleConfig.class);
}
@Test // DATAMONGO-2139
public void auditingWorksForVersionedEntityWithWrapperVersion() {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
auditablePersonRepository::save, //
null, 0L, 1L);
}
@Test // DATAMONGO-2139
public void auditingWorksForVersionedEntityWithSimpleVersion() {
verifyAuditingViaVersionProperty(new SimpleVersionedAuditablePerson(), //
it -> it.version, //
auditablePersonRepository::save, //
0L, 1L, 2L);
}
@Test // DATAMONGO-2139
public void auditingWorksForVersionedEntityWithWrapperVersionOnTemplate() {
verifyAuditingViaVersionProperty(new VersionedAuditablePerson(), //
it -> it.version, //
operations::save, //
null, 0L, 1L);
}
@Test // DATAMONGO-2139
public void auditingWorksForVersionedEntityWithSimpleVersionOnTemplate() {
verifyAuditingViaVersionProperty(new SimpleVersionedAuditablePerson(), //
it -> it.version, //
operations::save, //
0L, 1L, 2L);
}
private <T extends AuditablePerson> void verifyAuditingViaVersionProperty(T instance,
Function<T, Object> versionExtractor, Function<T, T> persister, Object... expectedValues) {
MongoPersistentEntity<?> entity = context.getRequiredPersistentEntity(instance.getClass());
assertThat(versionExtractor.apply(instance)).isEqualTo(expectedValues[0]);
assertThat(entity.isNew(instance)).isTrue();
instance = auditablePersonRepository.save(instance);
assertThat(versionExtractor.apply(instance)).isEqualTo(expectedValues[1]);
assertThat(entity.isNew(instance)).isFalse();
instance = auditablePersonRepository.save(instance);
assertThat(versionExtractor.apply(instance)).isEqualTo(expectedValues[2]);
assertThat(entity.isNew(instance)).isFalse();
}
@Repository
static interface AuditablePersonRepository extends MongoRepository<AuditablePerson, String> {}
@@ -128,4 +191,12 @@ public class AuditingViaJavaConfigRepositoriesTests {
return "database";
}
}
static class VersionedAuditablePerson extends AuditablePerson {
@Version Long version;
}
static class SimpleVersionedAuditablePerson extends AuditablePerson {
@Version long version;
}
}