From 39637c034c9a657479b31c7bd7d8f6b717b5acb7 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Thu, 17 Jun 2021 17:53:30 +0200 Subject: [PATCH] GH-2295 - Ensure same order of operations in both `Neo4jTemplate` and `ReactiveNeo4jTemplate`. - Use the same order of operations in both templates - Make sure to check with the same objects whether an entity has been seen - Brace for null values of related internal ids (check before overwriting an existing id and in case, check whether it has been seen through another object) This closes #2295. --- .../data/neo4j/core/Neo4jTemplate.java | 29 +++++++----- .../neo4j/core/ReactiveNeo4jTemplate.java | 47 ++++++++++++------- ...tedRelationshipProcessingStateMachine.java | 7 ++- 3 files changed, 52 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java index 8b71bd3a7..08bf1264d 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java @@ -721,12 +721,27 @@ public final class Neo4jTemplate implements Neo4jOperations, FluentNeo4jOperatio Entity savedEntity = null; // No need to save values if processed if (stateMachine.hasProcessedValue(relatedValueToStore)) { - relatedInternalId = stateMachine.getInternalId(relatedObjectBeforeCallbacksApplied); + relatedInternalId = stateMachine.getInternalId(relatedValueToStore); } else { savedEntity = saveRelatedNode(newRelatedObject, targetEntity); relatedInternalId = savedEntity.id(); + stateMachine.markValueAsProcessed(relatedValueToStore, relatedInternalId); } - stateMachine.markValueAsProcessed(relatedValueToStore, relatedInternalId); + + PersistentPropertyAccessor targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject); + // if an internal id is used this must be set to link this entity in the next iteration + if (targetEntity.isUsingInternalIds()) { + Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty(); + if (relatedInternalId == null && targetPropertyAccessor.getProperty(requiredIdProperty) != null) { + relatedInternalId = (Long) targetPropertyAccessor.getProperty(requiredIdProperty); + } else if (targetPropertyAccessor.getProperty(requiredIdProperty) == null) { + targetPropertyAccessor.setProperty(requiredIdProperty, relatedInternalId); + } + } + if (savedEntity != null) { + TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity); + } + stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean()); Object idValue = idProperty != null ? relationshipContext @@ -754,16 +769,6 @@ public final class Neo4jTemplate implements Neo4jOperations, FluentNeo4jOperatio .setProperty(idProperty, relationshipInternalId.get()); } - PersistentPropertyAccessor targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject); - // if an internal id is used this must be set to link this entity in the next iteration - if (targetEntity.isUsingInternalIds()) { - targetPropertyAccessor.setProperty(targetEntity.getRequiredIdProperty(), relatedInternalId); - } - if (savedEntity != null) { - TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity); - } - stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean()); - if (processState != ProcessState.PROCESSED_ALL_VALUES) { processNestedRelations(targetEntity, targetPropertyAccessor, isEntityNew, stateMachine, s -> true); } diff --git a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java index cced118cf..8db4c1d2b 100644 --- a/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java @@ -793,35 +793,48 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Rea Flux relationshipCreation = Flux.fromIterable(relatedValuesToStore).concatMap(relatedValueToStore -> { Object relatedObjectBeforeCallbacksApplied = relationshipContext.identifyAndExtractRelationshipTargetNode(relatedValueToStore); - return Mono.deferContextual(ctx -> eventSupport - .maybeCallBeforeBind(relatedObjectBeforeCallbacksApplied) + return Mono.deferContextual(ctx -> + + (stateMachine.hasProcessedValue(relatedObjectBeforeCallbacksApplied) + ? Mono.just(stateMachine.getProcessedAs(relatedObjectBeforeCallbacksApplied)) + : eventSupport.maybeCallBeforeBind(relatedObjectBeforeCallbacksApplied)) + .flatMap(newRelatedObject -> { Neo4jPersistentEntity targetEntity = neo4jMappingContext.getPersistentEntity(relatedObjectBeforeCallbacksApplied.getClass()); - Mono> queryOrSave; - long noVersion = Long.MIN_VALUE; + Mono> queryOrSave; if (stateMachine.hasProcessedValue(relatedValueToStore)) { - queryOrSave = Mono.just(stateMachine.getInternalId(relatedObjectBeforeCallbacksApplied)) - .map(id -> Tuples.of(id, noVersion)); + queryOrSave = Mono.just(new Long[] {stateMachine.getInternalId(relatedValueToStore)}) + .map(id -> Tuples.of(id, new Long[1])); } else { queryOrSave = saveRelatedNode(newRelatedObject, targetEntity) - .map(entity -> Tuples.of(entity.id(), targetEntity.hasVersionProperty() ? - entity.get(targetEntity.getVersionProperty().getPropertyName()) - .asLong() : - noVersion)); + .doOnNext(entity -> stateMachine.markValueAsProcessed(relatedValueToStore, entity.id())) + .map(entity -> { + Long version = targetEntity.hasVersionProperty() ? + entity.get(targetEntity.getVersionProperty().getPropertyName()).asLong() : + null; + return Tuples.of( + new Long[] { entity.id() }, + new Long[] { version }); + }); } return queryOrSave.flatMap(idAndVersion -> { - long relatedInternalId = idAndVersion.getT1(); - stateMachine.markValueAsProcessed(relatedValueToStore, relatedInternalId); + Long relatedInternalId = idAndVersion.getT1()[0]; // if an internal id is used this must be set to link this entity in the next iteration PersistentPropertyAccessor targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject); if (targetEntity.isUsingInternalIds()) { - targetPropertyAccessor.setProperty(targetEntity.getRequiredIdProperty(), relatedInternalId); - stateMachine.markValueAsProcessedAs(newRelatedObject, targetPropertyAccessor.getBean()); + Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty(); + if (relatedInternalId == null + && targetPropertyAccessor.getProperty(requiredIdProperty) != null) { + relatedInternalId = (Long) targetPropertyAccessor.getProperty(requiredIdProperty); + } else if (targetPropertyAccessor.getProperty(requiredIdProperty) == null) { + targetPropertyAccessor.setProperty(requiredIdProperty, relatedInternalId); + } } - if (targetEntity.hasVersionProperty() && idAndVersion.getT2() != noVersion) { - targetPropertyAccessor.setProperty(targetEntity.getVersionProperty(), idAndVersion.getT2()); + if (targetEntity.hasVersionProperty() && idAndVersion.getT2()[0] != null) { + targetPropertyAccessor.setProperty(targetEntity.getVersionProperty(), idAndVersion.getT2()[0]); } + stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean()); Object idValue = idProperty != null ? relationshipContext @@ -898,7 +911,7 @@ public final class ReactiveNeo4jTemplate implements ReactiveNeo4jOperations, Rea return neo4jClient .query(() -> renderer.render(cypherGenerator.prepareSaveOf(targetNodeDescription, dynamicLabels))) - .bind((Y) entity).with(neo4jMappingContext.getRequiredBinderFunctionFor(entityType)) + .bind(entity).with(neo4jMappingContext.getRequiredBinderFunctionFor(entityType)) .fetchAs(Entity.class) .one(); }).switchIfEmpty(Mono.defer(() -> { diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java b/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java index 272450ae8..6a267d8de 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java @@ -162,7 +162,9 @@ public final class NestedRelationshipProcessingStateMachine { try { write.lock(); this.processedObjects.add(valueToStore); - this.processedObjectsIds.put(valueToStore, internalId); + if (internalId != null) { + this.processedObjectsIds.put(valueToStore, internalId); + } } finally { write.unlock(); } @@ -175,7 +177,7 @@ public final class NestedRelationshipProcessingStateMachine { * @return processed yes (true) / no (false) */ public boolean hasProcessedValue(Object value) { - return processedObjects.contains(value); + return processedObjects.contains(value) || processedObjectsAlias.containsKey(value); } /** @@ -200,6 +202,7 @@ public final class NestedRelationshipProcessingStateMachine { } } + @Nullable public Long getInternalId(Object object) { try { read.lock();