Skip to content

Commit 166bd1d

Browse files
michael-simonsmichael-simons
committed
GH-2259 - Explicitly acquire locks on nodes when @Version is used.
This change brings back the original locking behaviour from Neo4j-OGM: The version is incremented inside the database and with it, a physical lock on the Node is acquired as described in https://neo4j.com/docs/java-reference/current/transaction-management/ under "Explicitly acquire a write lock". That incremented version is than used in a where clause after pipelining the matched node. The changes makes it necessary to retrieve the changed property from the database and apply it to the domain entity afterwards. The callbacks used before became superflous. This commit brings the ability to fetch single Nodes, Relationships or in general (driver) entities via the Neo4j client, allowing us to fetch the changed structure without additional mapping functions. This fixes #2259.
1 parent 04b0185 commit 166bd1d

13 files changed

+196
-212
lines changed

‎src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java

+20-11
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.neo4j.driver.exceptions.NoSuchRecordException;
4747
import org.neo4j.driver.summary.ResultSummary;
4848
import org.neo4j.driver.summary.SummaryCounters;
49+
import org.neo4j.driver.types.Entity;
4950
import org.springframework.beans.BeansException;
5051
import org.springframework.beans.factory.BeanClassLoaderAware;
5152
import org.springframework.beans.factory.BeanFactory;
@@ -336,26 +337,28 @@ private <T> T saveImpl(T instance, @Nullable List<PropertyDescriptor> includedPr
336337
return tree;
337338
});
338339
}
339-
Optional<Long> optionalInternalId = neo4jClient
340+
Optional<Entity> newOrUpdatedNode = neo4jClient
340341
.query(() -> renderer.render(cypherGenerator.prepareSaveOf(entityMetaData, dynamicLabels)))
341342
.bind(entityToBeSaved)
342343
.with(binderFunction)
343-
.fetchAs(Long.class).one();
344+
.fetchAs(Entity.class)
345+
.one();
344346

345-
if (!optionalInternalId.isPresent()) {
347+
if (!newOrUpdatedNode.isPresent()) {
346348
if (entityMetaData.hasVersionProperty()) {
347349
throw new OptimisticLockingFailureException(OPTIMISTIC_LOCKING_ERROR_MESSAGE);
348350
}
349351
// defensive exception throwing
350352
throw new IllegalStateException("Could not retrieve an internal id while saving.");
351353
}
352354

353-
Long internalId = optionalInternalId.get();
355+
Long internalId = newOrUpdatedNode.get().id();
354356

355357
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
356358
if (entityMetaData.isUsingInternalIds()) {
357359
propertyAccessor.setProperty(entityMetaData.getRequiredIdProperty(), internalId);
358360
}
361+
TemplateSupport.updateVersionPropertyIfPossible(entityMetaData, propertyAccessor, newOrUpdatedNode.get());
359362
processRelations(entityMetaData, instance, internalId, propertyAccessor, isEntityNew, includeProperty);
360363

361364
return propertyAccessor.getBean();
@@ -373,7 +376,7 @@ private <T> DynamicLabels determineDynamicLabels(T entityToBeSaved, Neo4jPersist
373376

374377
if (entityMetaData.hasVersionProperty()) {
375378
runnableQuery = runnableQuery
376-
.bind((Long) propertyAccessor.getProperty(entityMetaData.getRequiredVersionProperty()) - 1)
379+
.bind((Long) propertyAccessor.getProperty(entityMetaData.getRequiredVersionProperty()))
377380
.to(Constants.NAME_OF_VERSION_PARAM);
378381
}
379382

@@ -691,11 +694,13 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Pers
691694
: eventSupport.maybeCallBeforeBind(relatedObjectBeforeCallbacksApplied);
692695

693696
Long relatedInternalId;
697+
Entity savedEntity = null;
694698
// No need to save values if processed
695699
if (stateMachine.hasProcessedValue(relatedValueToStore)) {
696700
relatedInternalId = stateMachine.getInternalId(relatedObjectBeforeCallbacksApplied);
697701
} else {
698-
relatedInternalId = saveRelatedNode(newRelatedObject, targetEntity);
702+
savedEntity = saveRelatedNode(newRelatedObject, targetEntity);
703+
relatedInternalId = savedEntity.id();
699704
}
700705
stateMachine.markValueAsProcessed(relatedValueToStore, relatedInternalId);
701706

@@ -730,6 +735,9 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Pers
730735
if (targetEntity.isUsingInternalIds()) {
731736
targetPropertyAccessor.setProperty(targetEntity.getRequiredIdProperty(), relatedInternalId);
732737
}
738+
if (savedEntity != null) {
739+
TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity);
740+
}
733741
stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean());
734742

735743
if (processState != ProcessState.PROCESSED_ALL_VALUES) {
@@ -751,20 +759,21 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Pers
751759
return (T) propertyAccessor.getBean();
752760
}
753761

754-
private <Y> Long saveRelatedNode(Object entity, NodeDescription targetNodeDescription) {
762+
private <Y> Entity saveRelatedNode(Object entity, NodeDescription targetNodeDescription) {
755763

756764
DynamicLabels dynamicLabels = determineDynamicLabels(entity, (Neo4jPersistentEntity) targetNodeDescription);
757765
Class<Y> entityType = (Class<Y>) ((Neo4jPersistentEntity<?>) targetNodeDescription).getType();
758-
Optional<Long> optionalSavedNodeId = neo4jClient
766+
Optional<Entity> optionalSavedNode = neo4jClient
759767
.query(() -> renderer.render(cypherGenerator.prepareSaveOf(targetNodeDescription, dynamicLabels)))
760768
.bind((Y) entity).with(neo4jMappingContext.getRequiredBinderFunctionFor(entityType))
761-
.fetchAs(Long.class).one();
769+
.fetchAs(Entity.class)
770+
.one();
762771

763-
if (((Neo4jPersistentEntity) targetNodeDescription).hasVersionProperty() && !optionalSavedNodeId.isPresent()) {
772+
if (((Neo4jPersistentEntity) targetNodeDescription).hasVersionProperty() && !optionalSavedNode.isPresent()) {
764773
throw new OptimisticLockingFailureException(OPTIMISTIC_LOCKING_ERROR_MESSAGE);
765774
}
766775

767-
return optionalSavedNodeId.get();
776+
return optionalSavedNode.get();
768777
}
769778

770779
@Override

‎src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java

+33-15
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.neo4j.cypherdsl.core.Statement;
4848
import org.neo4j.cypherdsl.core.renderer.Renderer;
4949
import org.neo4j.driver.summary.SummaryCounters;
50+
import org.neo4j.driver.types.Entity;
5051
import org.reactivestreams.Publisher;
5152
import org.springframework.beans.BeansException;
5253
import org.springframework.beans.factory.BeanClassLoaderAware;
@@ -307,7 +308,7 @@ public <T, R> Mono<R> saveAs(T instance, Class<R> resultType) {
307308

308309
private <T> Mono<T> saveImpl(T instance, @Nullable List<PropertyDescriptor> includedProperties) {
309310

310-
Neo4jPersistentEntity entityMetaData = neo4jMappingContext.getPersistentEntity(instance.getClass());
311+
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(instance.getClass());
311312
boolean isNewEntity = entityMetaData.isNew(instance);
312313
return Mono.just(instance).flatMap(eventSupport::maybeCallBeforeBind)
313314
.flatMap(entityToBeSaved -> determineDynamicLabels(entityToBeSaved, entityMetaData)).flatMap(t -> {
@@ -331,9 +332,11 @@ private <T> Mono<T> saveImpl(T instance, @Nullable List<PropertyDescriptor> incl
331332
});
332333
}
333334

334-
Mono<Long> idMono = this.neo4jClient.query(() -> renderer.render(cypherGenerator.prepareSaveOf(entityMetaData, dynamicLabels)))
335-
.bind(entityToBeSaved).with(binderFunction)
336-
.fetchAs(Long.class).one()
335+
Mono<Entity> idMono = this.neo4jClient.query(() -> renderer.render(cypherGenerator.prepareSaveOf(entityMetaData, dynamicLabels)))
336+
.bind(entityToBeSaved)
337+
.with(binderFunction)
338+
.fetchAs(Entity.class)
339+
.one()
337340
.switchIfEmpty(Mono.defer(() -> {
338341
if (entityMetaData.hasVersionProperty()) {
339342
return Mono.error(() -> new OptimisticLockingFailureException(OPTIMISTIC_LOCKING_ERROR_MESSAGE));
@@ -342,14 +345,18 @@ private <T> Mono<T> saveImpl(T instance, @Nullable List<PropertyDescriptor> incl
342345
}));
343346

344347
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
345-
return idMono.doOnNext(internalId -> {
348+
return idMono.doOnNext(newOrUpdatedNode -> {
346349
if (entityMetaData.isUsingInternalIds()) {
347-
propertyAccessor.setProperty(entityMetaData.getRequiredIdProperty(), internalId);
350+
propertyAccessor.setProperty(entityMetaData.getRequiredIdProperty(), newOrUpdatedNode.id());
348351
}
349-
}).flatMap(internalId -> processRelations(entityMetaData, instance, internalId, propertyAccessor, isNewEntity, includeProperty));
352+
TemplateSupport.updateVersionPropertyIfPossible(entityMetaData, propertyAccessor, newOrUpdatedNode);
353+
}).map(Entity::id)
354+
.flatMap(internalId -> processRelations(entityMetaData, instance, internalId, propertyAccessor, isNewEntity, includeProperty));
350355
});
351356
}
352357

358+
359+
353360
private <T> Mono<Tuple2<T, DynamicLabels>> determineDynamicLabels(T entityToBeSaved,
354361
Neo4jPersistentEntity<?> entityMetaData) {
355362
return entityMetaData.getDynamicLabelsProperty().map(p -> {
@@ -362,7 +369,7 @@ private <T> Mono<Tuple2<T, DynamicLabels>> determineDynamicLabels(T entityToBeSa
362369

363370
if (entityMetaData.hasVersionProperty()) {
364371
runnableQuery = runnableQuery
365-
.bind((Long) propertyAccessor.getProperty(entityMetaData.getRequiredVersionProperty()) - 1)
372+
.bind((Long) propertyAccessor.getProperty(entityMetaData.getRequiredVersionProperty()))
366373
.to(Constants.NAME_OF_VERSION_PARAM);
367374
}
368375

@@ -775,20 +782,30 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
775782
.flatMap(newRelatedObject -> {
776783
Neo4jPersistentEntity<?> targetEntity = neo4jMappingContext.getPersistentEntity(relatedObjectBeforeCallbacksApplied.getClass());
777784

778-
Mono<Long> queryOrSave;
785+
Mono<Tuple2<Long, Long>> queryOrSave;
786+
long noVersion = Long.MIN_VALUE;
779787
if (stateMachine.hasProcessedValue(relatedValueToStore)) {
780-
queryOrSave = Mono.just(stateMachine.getInternalId(relatedObjectBeforeCallbacksApplied));
788+
queryOrSave = Mono.just(stateMachine.getInternalId(relatedObjectBeforeCallbacksApplied))
789+
.map(id -> Tuples.of(id, noVersion));
781790
} else {
782-
queryOrSave = saveRelatedNode(newRelatedObject, targetEntity);
791+
queryOrSave = saveRelatedNode(newRelatedObject, targetEntity)
792+
.map(entity -> Tuples.of(entity.id(), targetEntity.hasVersionProperty() ?
793+
entity.get(targetEntity.getVersionProperty().getPropertyName())
794+
.asLong() :
795+
noVersion));
783796
}
784-
return queryOrSave.flatMap(relatedInternalId -> {
797+
return queryOrSave.flatMap(idAndVersion -> {
798+
long relatedInternalId = idAndVersion.getT1();
785799
stateMachine.markValueAsProcessed(relatedValueToStore, relatedInternalId);
786800
// if an internal id is used this must be set to link this entity in the next iteration
787801
PersistentPropertyAccessor<?> targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject);
788802
if (targetEntity.isUsingInternalIds()) {
789803
targetPropertyAccessor.setProperty(targetEntity.getRequiredIdProperty(), relatedInternalId);
790804
stateMachine.markValueAsProcessedAs(newRelatedObject, targetPropertyAccessor.getBean());
791805
}
806+
if (targetEntity.hasVersionProperty() && idAndVersion.getT2() != noVersion) {
807+
targetPropertyAccessor.setProperty(targetEntity.getVersionProperty(), idAndVersion.getT2());
808+
}
792809

793810
Object idValue = idProperty != null
794811
? relationshipContext
@@ -855,18 +872,19 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
855872

856873
}
857874

858-
private <Y> Mono<Long> saveRelatedNode(Object relatedNode, Neo4jPersistentEntity<?> targetNodeDescription) {
875+
private <Y> Mono<Entity> saveRelatedNode(Object relatedNode, Neo4jPersistentEntity<?> targetNodeDescription) {
859876

860877
return determineDynamicLabels((Y) relatedNode, targetNodeDescription)
861878
.flatMap(t -> {
862879
Y entity = t.getT1();
863-
Class<Y> entityType = (Class<Y>) ((Neo4jPersistentEntity<?>) targetNodeDescription).getType();
880+
Class<Y> entityType = (Class<Y>) targetNodeDescription.getType();
864881
DynamicLabels dynamicLabels = t.getT2();
865882

866883
return neo4jClient
867884
.query(() -> renderer.render(cypherGenerator.prepareSaveOf(targetNodeDescription, dynamicLabels)))
868885
.bind((Y) entity).with(neo4jMappingContext.getRequiredBinderFunctionFor(entityType))
869-
.fetchAs(Long.class).one();
886+
.fetchAs(Entity.class)
887+
.one();
870888
}).switchIfEmpty(Mono.defer(() -> {
871889
if (targetNodeDescription.hasVersionProperty()) {
872890
return Mono.error(() -> new OptimisticLockingFailureException(OPTIMISTIC_LOCKING_ERROR_MESSAGE));

‎src/main/java/org/springframework/data/neo4j/core/TemplateSupport.java

+14
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@
3434
import org.neo4j.cypherdsl.core.Node;
3535
import org.neo4j.cypherdsl.core.Relationship;
3636
import org.neo4j.cypherdsl.core.Statement;
37+
import org.neo4j.driver.types.Entity;
38+
import org.springframework.data.mapping.PersistentPropertyAccessor;
3739
import org.springframework.data.neo4j.core.mapping.Constants;
40+
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
3841
import org.springframework.data.neo4j.repository.query.QueryFragments;
3942
import org.springframework.lang.Nullable;
4043

@@ -111,6 +114,17 @@ static Predicate<String> computeIncludePropertyPredicate(List<PropertyDescriptor
111114
}
112115
}
113116

117+
static void updateVersionPropertyIfPossible(
118+
Neo4jPersistentEntity<?> entityMetaData,
119+
PersistentPropertyAccessor<?> propertyAccessor,
120+
Entity newOrUpdatedNode
121+
) {
122+
if (entityMetaData.hasVersionProperty()) {
123+
propertyAccessor.setProperty(
124+
entityMetaData.getVersionProperty(), newOrUpdatedNode.get(entityMetaData.getVersionProperty().getPropertyName()).asLong());
125+
}
126+
}
127+
114128
/**
115129
* Merges statement and explicit parameters. Statement parameters have a higher precedence
116130
*

‎src/main/java/org/springframework/data/neo4j/core/convert/AdditionalTypes.java

+6
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@
4040
import org.neo4j.driver.Value;
4141
import org.neo4j.driver.Values;
4242
import org.neo4j.driver.exceptions.value.LossyCoercion;
43+
import org.neo4j.driver.types.Entity;
44+
import org.neo4j.driver.types.Node;
45+
import org.neo4j.driver.types.Relationship;
4346
import org.springframework.core.convert.TypeDescriptor;
4447
import org.springframework.core.convert.converter.ConditionalConverter;
4548
import org.springframework.core.convert.converter.ConverterRegistry;
@@ -98,6 +101,9 @@ final class AdditionalTypes {
98101
hlp.add(ConverterBuilder.reading(Value.class, URI.class, AdditionalTypes::asURI).andWriting(AdditionalTypes::value));
99102
hlp.add(ConverterBuilder.reading(Value.class, TimeZone.class, AdditionalTypes::asTimeZone).andWriting(AdditionalTypes::value));
100103
hlp.add(ConverterBuilder.reading(Value.class, ZoneId.class, AdditionalTypes::asZoneId).andWriting(AdditionalTypes::value));
104+
hlp.add(ConverterBuilder.reading(Value.class, Entity.class, Value::asEntity));
105+
hlp.add(ConverterBuilder.reading(Value.class, Node.class, Value::asNode));
106+
hlp.add(ConverterBuilder.reading(Value.class, Relationship.class, Value::asRelationship));
101107

102108
CONVERTERS = Collections.unmodifiableList(hlp);
103109
}

0 commit comments

Comments
 (0)