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.

Author: michael-simons

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

@@ -721,12 +721,27 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Pers
 				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 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Pers
 							.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);
 				}

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

@@ -793,35 +793,48 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
 			Flux<RelationshipHandler> 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<Tuple2<Long, Long>> queryOrSave;
-							long noVersion = Long.MIN_VALUE;
+							Mono<Tuple2<Long[], Long[]>> 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 @@ private <Y> Mono<Entity> saveRelatedNode(Object relatedNode, Neo4jPersistentEnti
 
 					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(() -> {

src/main/java/org/springframework/data/neo4j/core/mapping/NestedRelationshipProcessingStateMachine.java

@@ -162,7 +162,9 @@ public void markValueAsProcessed(Object valueToStore, Long internalId) {
 		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 void markValueAsProcessed(Object valueToStore, Long internalId) {
  	 * @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 void markValueAsProcessedAs(Object relatedValueToStore, Object bean) {
 		}
 	}
 
+	@Nullable
 	public Long getInternalId(Object object) {
 		try {
 			read.lock();