DATAGRAPH-1427 - Optimistic locking check on delete.

This commit is contained in:
Gerrit Meier
2020-11-12 15:47:44 +01:00
parent 85e7e808f2
commit e7989df04f
9 changed files with 245 additions and 4 deletions

30
etc/adr/adr-007.adoc Normal file
View File

@@ -0,0 +1,30 @@
== ADR 7: Optimistic locking on delete
=== Status
accepted
=== Context
At the moment there is no optimistic locking check at all when it comes to entity deletion via `delete...` API calls.
=== Decision
For `deleteById`:
It won't make any sense with just an identifier provided, to load the current data from the database and compare it on save with the retrieved version.
In 99.999(...)% of the use-cases it will be the same version.
For the rest it would mean that the data has just been modified between loading the version and the delete call.
If the latter would have been the only call against the database, it would have removed the entity correctly.
For `delete(entity)`:
There should be a check against the version of the previously loaded entity.
As a consequent throw a `OptimisticLockingFailureException` if the versions have a mismatch on delete.
Non-existing version parameters during `delete(entity)`:
Fall back to `OR <versionField> IS NULL` to provide a relaxed mode.
Otherwise, we would initialize the version actively to just delete the node.
=== Consequences
A clean line between what is supposed to work with optimistic locking and what does not.
Additionally, this aligns the Neo4j module with the other Spring Data modules.

View File

@@ -22,6 +22,7 @@ import java.util.Optional;
import org.apiguardian.api.API;
import org.neo4j.cypherdsl.core.Statement;
import org.springframework.dao.IncorrectResultSizeDataAccessException;
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
import org.springframework.data.neo4j.repository.NoResultException;
/**
@@ -196,6 +197,8 @@ public interface Neo4jOperations {
*/
<T> void deleteById(Object id, Class<T> domainType);
<T> void deleteByIdWithVersion(Object id, Class<T> domainType, Neo4jPersistentProperty versionProperty, Object versionValue);
/**
* Deletes all entities with one of the given ids, including all entities related to that entity.
*

View File

@@ -21,6 +21,7 @@ import static org.neo4j.cypherdsl.core.Cypher.parameter;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -30,6 +31,7 @@ import java.util.stream.Collectors;
import org.apache.commons.logging.LogFactory;
import org.apiguardian.api.API;
import org.neo4j.cypherdsl.core.Condition;
import org.neo4j.cypherdsl.core.Cypher;
import org.neo4j.cypherdsl.core.Functions;
import org.neo4j.cypherdsl.core.Statement;
import org.neo4j.cypherdsl.core.renderer.Renderer;
@@ -343,6 +345,32 @@ public final class Neo4jTemplate implements Neo4jOperations, BeanFactoryAware {
summary.counters().relationshipsDeleted()));
}
@Override
public <T> void deleteByIdWithVersion(Object id, Class<T> domainType, Neo4jPersistentProperty versionProperty,
Object versionValue) {
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(domainType);
String nameOfParameter = "id";
Condition condition = entityMetaData.getIdExpression().isEqualTo(parameter(nameOfParameter))
.and(Cypher.property(Constants.NAME_OF_ROOT_NODE, versionProperty.getPropertyName())
.isEqualTo(parameter(Constants.NAME_OF_VERSION_PARAM))
.or(Cypher.property(Constants.NAME_OF_ROOT_NODE, versionProperty.getPropertyName()).isNull()));
Statement statement = cypherGenerator.prepareMatchOf(entityMetaData, condition)
.returning(Constants.NAME_OF_ROOT_NODE).build();
Map<String, Object> parameters = new HashMap<>();
parameters.put(nameOfParameter, convertIdValues(entityMetaData.getRequiredIdProperty(), id));
parameters.put(Constants.NAME_OF_VERSION_PARAM, versionValue);
createExecutableQuery(domainType, statement, parameters).getSingleResult().orElseThrow(
() -> new OptimisticLockingFailureException(OPTIMISTIC_LOCKING_ERROR_MESSAGE)
);
deleteById(id, domainType);
}
@Override
public <T> void deleteAllById(Iterable<?> ids, Class<T> domainType) {

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.data.neo4j.core;
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@@ -195,6 +196,9 @@ public interface ReactiveNeo4jOperations {
*/
<T> Mono<Void> deleteById(Object id, Class<T> domainType);
<T> Mono<Void> deleteByIdWithVersion(Object id, Class<T> domainType, Neo4jPersistentProperty versionProperty,
Object versionValue);
/**
* Deletes all entities with one of the given ids, including all entities related to that entity.
*

View File

@@ -18,6 +18,7 @@ package org.springframework.data.neo4j.core;
import static org.neo4j.cypherdsl.core.Cypher.asterisk;
import static org.neo4j.cypherdsl.core.Cypher.parameter;
import org.neo4j.cypherdsl.core.Cypher;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.util.function.Tuple2;
@@ -26,6 +27,7 @@ import reactor.util.function.Tuples;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
@@ -348,6 +350,36 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Bea
.to(nameOfParameter).run().then());
}
@Override
public <T> Mono<Void> deleteByIdWithVersion(Object id, Class<T> domainType, Neo4jPersistentProperty versionProperty,
Object versionValue) {
String nameOfParameter = "id";
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(domainType);
Condition condition = entityMetaData.getIdExpression().isEqualTo(parameter(nameOfParameter))
.and(Cypher.property(Constants.NAME_OF_ROOT_NODE, versionProperty.getPropertyName())
.isEqualTo(parameter(Constants.NAME_OF_VERSION_PARAM))
.or(Cypher.property(Constants.NAME_OF_ROOT_NODE, versionProperty.getPropertyName()).isNull()));
Statement statement = cypherGenerator.prepareMatchOf(entityMetaData, condition)
.returning(Constants.NAME_OF_ROOT_NODE).build();
Map<String, Object> parameters = new HashMap<>();
parameters.put(nameOfParameter, convertIdValues(entityMetaData.getRequiredIdProperty(), id));
parameters.put(Constants.NAME_OF_VERSION_PARAM, versionValue);
return getDatabaseName().flatMap(databaseName -> this.neo4jClient.query(() -> renderer.render(statement))
.in(databaseName.getValue())
.bindAll(parameters)
.fetch().one().switchIfEmpty(Mono.defer(() -> {
if (entityMetaData.hasVersionProperty()) {
return Mono.error(() -> new OptimisticLockingFailureException(OPTIMISTIC_LOCKING_ERROR_MESSAGE));
}
return Mono.empty();
})))
.then(deleteById(id, domainType));
}
@Override
public Mono<Void> deleteAll(Class<?> domainType) {

View File

@@ -29,8 +29,9 @@ import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.neo4j.core.Neo4jOperations;
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
import org.springframework.data.neo4j.repository.query.CypherAdapterUtils;
import org.springframework.data.repository.PagingAndSortingRepository;
import org.springframework.data.repository.support.PageableExecutionUtils;
@@ -149,7 +150,13 @@ public class SimpleNeo4jRepository<T, ID> implements PagingAndSortingRepository<
public void delete(T entity) {
ID id = this.entityInformation.getId(entity);
this.deleteById(id);
if (entityMetaData.hasVersionProperty()) {
Neo4jPersistentProperty versionProperty = entityMetaData.getRequiredVersionProperty();
Object versionValue = entityMetaData.getPropertyAccessor(entity).getProperty(versionProperty);
this.neo4jOperations.deleteByIdWithVersion(id, this.entityInformation.getJavaType(), versionProperty, versionValue);
} else {
this.deleteById(id);
}
}
@Override

View File

@@ -15,6 +15,7 @@
*/
package org.springframework.data.neo4j.repository.support;
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@@ -170,9 +171,17 @@ public class SimpleReactiveNeo4jRepository<T, ID> implements ReactiveSortingRepo
@Override
@Transactional
public Mono<Void> delete(T entity) {
Assert.notNull(entity, "The given entity must not be null!");
return deleteById(this.entityInformation.getId(entity));
ID id = this.entityInformation.getId(entity);
if (entityMetaData.hasVersionProperty()) {
Neo4jPersistentProperty versionProperty = entityMetaData.getRequiredVersionProperty();
Object versionValue = entityMetaData.getPropertyAccessor(entity).getProperty(versionProperty);
return this.neo4jOperations.deleteByIdWithVersion(id, this.entityInformation.getJavaType(), versionProperty, versionValue);
} else {
return this.deleteById(id);
}
}
/*

View File

@@ -205,6 +205,71 @@ class OptimisticLockingIT {
}
@Test
void shouldNotFailOnDeleteByIdWithNullVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx -> tx.run("CREATE (v:VersionedThingWithAssignedId {id:1})").consume());
}
repository.deleteById(1L);
try (Session session = driver.session()) {
long count = session.readTransaction(tx ->
tx.run("MATCH (v:VersionedThingWithAssignedId) return count(v) as vCount").next()
.get("vCount").asLong());
assertThat(count).isEqualTo(0);
}
}
@Test
void shouldNotFailOnDeleteByEntityWithNullVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx -> tx.run("CREATE (v:VersionedThingWithAssignedId {id:1})").consume());
}
VersionedThingWithAssignedId thing = repository.findById(1L).get();
repository.delete(thing);
try (Session session = driver.session()) {
long count = session.readTransaction(tx ->
tx.run("MATCH (v:VersionedThingWithAssignedId) return count(v) as vCount").next()
.get("vCount").asLong());
assertThat(count).isEqualTo(0);
}
}
@Test
void shouldNotFailOnDeleteByIdWithAnyVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx ->
tx.run("CREATE (v:VersionedThingWithAssignedId {id:1, myVersion:3})").consume());
}
repository.deleteById(1L);
try (Session session = driver.session()) {
long count = session.readTransaction(tx ->
tx.run("MATCH (v:VersionedThingWithAssignedId) return count(v) as vCount").next()
.get("vCount").asLong());
assertThat(count).isEqualTo(0);
}
}
@Test
void shouldFailOnDeleteByEntityWithWrongVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx ->
tx.run("CREATE (v:VersionedThingWithAssignedId {id:1, myVersion:2})").consume());
}
VersionedThingWithAssignedId thing = repository.findById(1L).get();
thing.setMyVersion(3L);
assertThatExceptionOfType(OptimisticLockingFailureException.class).isThrownBy(() -> repository.delete(thing));
}
interface VersionedThingRepository extends Neo4jRepository<VersionedThing, Long> {}
interface VersionedThingWithAssignedIdRepository extends Neo4jRepository<VersionedThingWithAssignedId, Long> {}

View File

@@ -204,6 +204,69 @@ class ReactiveOptimisticLockingIT {
}
@Test
void shouldNotFailOnDeleteByIdWithNullVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx -> tx.run("CREATE (v:VersionedThingWithAssignedId {id:1})").consume());
}
StepVerifier.create(repository.deleteById(1L))
.verifyComplete();
try (Session session = driver.session()) {
long count = session.readTransaction(tx ->
tx.run("MATCH (v:VersionedThingWithAssignedId) return count(v) as vCount").next()
.get("vCount").asLong());
assertThat(count).isEqualTo(0);
}
}
@Test
void shouldNotFailOnDeleteByEntityWithNullVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx -> tx.run("CREATE (v:VersionedThingWithAssignedId {id:1})").consume());
}
StepVerifier.create(repository.findById(1L).map(thing -> repository.deleteById(1L)))
.expectNextCount(1L)
.verifyComplete();
}
@Test
void shouldNotFailOnDeleteByIdWithAnyVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx ->
tx.run("CREATE (v:VersionedThingWithAssignedId {id:1, myVersion:3})").consume());
}
StepVerifier.create(repository.deleteById(1L))
.verifyComplete();
try (Session session = driver.session()) {
long count = session.readTransaction(tx ->
tx.run("MATCH (v:VersionedThingWithAssignedId) return count(v) as vCount").next()
.get("vCount").asLong());
assertThat(count).isEqualTo(0);
}
}
@Test
void shouldFailOnDeleteByEntityWithWrongVersion(@Autowired VersionedThingWithAssignedIdRepository repository) {
try (Session session = driver.session()) {
session.writeTransaction(tx -> tx.run("CREATE (v:VersionedThingWithAssignedId {id:1, myVersion:2})").consume());
}
StepVerifier.create(repository.findById(1L)
.flatMap(thing -> {
thing.setMyVersion(3L);
return repository.delete(thing);
})).verifyError(OptimisticLockingFailureException.class);
}
interface VersionedThingRepository extends ReactiveNeo4jRepository<VersionedThing, Long> {}
interface VersionedThingWithAssignedIdRepository