GH-2809 - Find the right id/element id function for assignedIds.

* Avoid using the outdated id function wherever possible.
* Never fall back to `toString(id((element))` (in Neo4j 4.4 scenarios)

Closes #2809

Author: meistermeier

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

@@ -401,7 +401,7 @@ private <T> T saveImpl(T instance, @Nullable Collection<PropertyFilter.Projected
 				neo4jMappingContext.getRequiredBinderFunctionFor((Class<T>) entityToBeSaved.getClass())
 		);
 		Optional<Entity> newOrUpdatedNode = neo4jClient
-				.query(() -> renderer.render(cypherGenerator.prepareSaveOf(entityMetaData, dynamicLabels)))
+				.query(() -> renderer.render(cypherGenerator.prepareSaveOf(entityMetaData, dynamicLabels, TemplateSupport.rendererRendersElementId(renderer))))
 				.bind(entityToBeSaved)
 				.with(binderFunction)
 				.fetchAs(Entity.class)
@@ -416,7 +416,7 @@ private <T> T saveImpl(T instance, @Nullable Collection<PropertyFilter.Projected
 		}
 
 		Object elementId = newOrUpdatedNode.map(node -> {
-			if (!entityMetaData.isUsingDeprecatedInternalId() && entityMetaData.isUsingInternalIds()) {
+			if (!entityMetaData.isUsingDeprecatedInternalId() && TemplateSupport.rendererRendersElementId(renderer)) {
 				return IdentitySupport.getElementId(node);
 			}
 			return node.id();
@@ -764,6 +764,7 @@ private <T> T processNestedRelations(
 			// Remove all relationships before creating all new if the entity is not new and the relationship
 			// has not been processed before.
 			// This avoids the usage of cache but might have significant impact on overall performance
+			boolean canUseElementId = TemplateSupport.rendererRendersElementId(renderer);
 			if (!isParentObjectNew && !stateMachine.hasProcessedRelationship(fromId, relationshipDescription)) {
 
 				List<Object> knownRelationshipsIds = new ArrayList<>();
@@ -780,7 +781,7 @@ private <T> T processNestedRelations(
 					}
 				}
 
-				Statement relationshipRemoveQuery = cypherGenerator.prepareDeleteOf(sourceEntity, relationshipDescription);
+				Statement relationshipRemoveQuery = cypherGenerator.prepareDeleteOf(sourceEntity, relationshipDescription, canUseElementId);
 
 				neo4jClient.query(renderer.render(relationshipRemoveQuery))
 						.bind(convertIdValues(sourceEntity.getIdProperty(), fromId)) //
@@ -858,7 +859,7 @@ private <T> T processNestedRelations(
 					// create new dynamic relationship properties
 					if (relationshipDescription.hasRelationshipProperties() && isNewRelationship && idProperty != null) {
 						CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForSingleRelationship(
-								sourceEntity, relationshipDescription, relatedValueToStore, true);
+								sourceEntity, relationshipDescription, relatedValueToStore, true, canUseElementId);
 
 						List<Object> row = Collections.singletonList(properties);
 						statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, row);
@@ -879,7 +880,7 @@ private <T> T processNestedRelations(
 					} else { // plain (new or to update) dynamic relationship or dynamic relationships with properties to update
 
 						CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForSingleRelationship(
-								sourceEntity, relationshipDescription, relatedValueToStore, false);
+								sourceEntity, relationshipDescription, relatedValueToStore, false, canUseElementId);
 
 						List<Object> row = Collections.singletonList(properties);
 						statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, row);
@@ -921,15 +922,15 @@ private <T> T processNestedRelations(
 			// batch operations
 			if (!(relationshipDescription.hasRelationshipProperties() || relationshipDescription.isDynamic() || plainRelationshipRows.isEmpty())) {
 				CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForImperativeSimpleRelationshipBatch(
-						sourceEntity, relationshipDescription, plainRelationshipRows);
+						sourceEntity, relationshipDescription, plainRelationshipRows, canUseElementId);
 				statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, plainRelationshipRows);
 				neo4jClient.query(renderer.render(statementHolder.getStatement()))
 						.bindAll(statementHolder.getProperties())
 						.run();
 			} else if (relationshipDescription.hasRelationshipProperties()) {
 				if (!relationshipPropertiesRows.isEmpty()) {
 					CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForImperativeRelationshipsWithPropertiesBatch(false,
-							sourceEntity, relationshipDescription, updateRelatedValuesToStore, relationshipPropertiesRows);
+							sourceEntity, relationshipDescription, updateRelatedValuesToStore, relationshipPropertiesRows, canUseElementId);
 					statementHolder = statementHolder.addProperty(Constants.NAME_OF_RELATIONSHIP_LIST_PARAM, relationshipPropertiesRows);
 
 					neo4jClient.query(renderer.render(statementHolder.getStatement()))
@@ -938,7 +939,7 @@ private <T> T processNestedRelations(
 				}
 				if (!newRelatedValuesToStore.isEmpty()) {
 					CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForImperativeRelationshipsWithPropertiesBatch(true,
-							sourceEntity, relationshipDescription, newRelatedValuesToStore, newRelationshipPropertiesRows);
+							sourceEntity, relationshipDescription, newRelatedValuesToStore, newRelationshipPropertiesRows, canUseElementId);
 					List<Object> all = new ArrayList<>(neo4jClient.query(renderer.render(statementHolder.getStatement()))
 							.bindAll(statementHolder.getProperties())
 							.fetchAs(Object.class)
@@ -996,7 +997,7 @@ private Entity saveRelatedNode(Object entity, NodeDescription<?> targetNodeDescr
 			return tree;
 		});
 		Optional<Entity> optionalSavedNode = neo4jClient
-				.query(() -> renderer.render(cypherGenerator.prepareSaveOf(targetNodeDescription, dynamicLabels)))
+				.query(() -> renderer.render(cypherGenerator.prepareSaveOf(targetNodeDescription, dynamicLabels, TemplateSupport.rendererRendersElementId(renderer))))
 				.bind(entity).with(binderFunction)
 				.fetchAs(Entity.class)
 				.one();

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

@@ -438,7 +438,8 @@ private <T> Mono<T> saveImpl(T instance, @Nullable Collection<PropertyFilter.Pro
 							includedProperties, entityMetaData,
 							neo4jMappingContext.getRequiredBinderFunctionFor((Class<T>) entityToBeSaved.getClass()));
 
-					Mono<Entity> idMono = this.neo4jClient.query(() -> renderer.render(cypherGenerator.prepareSaveOf(entityMetaData, dynamicLabels)))
+					boolean canUseElementId = TemplateSupport.rendererRendersElementId(renderer);
+					Mono<Entity> idMono = this.neo4jClient.query(() -> renderer.render(cypherGenerator.prepareSaveOf(entityMetaData, dynamicLabels, canUseElementId)))
 							.bind(entityToBeSaved)
 							.with(binderFunction)
 							.fetchAs(Entity.class)
@@ -452,7 +453,7 @@ private <T> Mono<T> saveImpl(T instance, @Nullable Collection<PropertyFilter.Pro
 
 					PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
 					return idMono.doOnNext(newOrUpdatedNode -> {
-						var elementId = !entityMetaData.isUsingDeprecatedInternalId() && entityMetaData.isUsingInternalIds()
+						var elementId = !entityMetaData.isUsingDeprecatedInternalId() && canUseElementId
 										? IdentitySupport.getElementId(newOrUpdatedNode)
 										: newOrUpdatedNode.id();
 						TemplateSupport.setGeneratedIdIfNecessary(entityMetaData, propertyAccessor, elementId, Optional.of(newOrUpdatedNode));
@@ -901,6 +902,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
 			// Remove all relationships before creating all new if the entity is not new and the relationship
 			// has not been processed before.
 			// This avoids the usage of cache but might have significant impact on overall performance
+			boolean canUseElementId = TemplateSupport.rendererRendersElementId(renderer);
 			if (!isParentObjectNew && !stateMachine.hasProcessedRelationship(fromId, relationshipDescription)) {
 
 				List<Object> knownRelationshipsIds = new ArrayList<>();
@@ -919,7 +921,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
 					}
 				}
 
-				Statement relationshipRemoveQuery = cypherGenerator.prepareDeleteOf(sourceEntity, relationshipDescription);
+				Statement relationshipRemoveQuery = cypherGenerator.prepareDeleteOf(sourceEntity, relationshipDescription, canUseElementId);
 
 				relationshipDeleteMonos.add(
 						neo4jClient.query(renderer.render(relationshipRemoveQuery))
@@ -990,7 +992,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
 
 									boolean isNewRelationship = idValue == null;
 									CreateRelationshipStatementHolder statementHolder = neo4jMappingContext.createStatementForSingleRelationship(
-											sourceEntity, relationshipDescription, relatedValueToStore, isNewRelationship);
+											sourceEntity, relationshipDescription, relatedValueToStore, isNewRelationship, canUseElementId);
 
 									Map<String, Object> properties = new HashMap<>();
 									properties.put(Constants.FROM_ID_PARAMETER_NAME, convertIdValues(sourceEntity.getRequiredIdProperty(), fromId));
@@ -1086,7 +1088,7 @@ private Mono<Entity> saveRelatedNode(Object relatedNode, Neo4jPersistentEntity<?
 						return tree;
 					});
 					return neo4jClient
-							.query(() -> renderer.render(cypherGenerator.prepareSaveOf(targetNodeDescription, dynamicLabels)))
+							.query(() -> renderer.render(cypherGenerator.prepareSaveOf(targetNodeDescription, dynamicLabels, TemplateSupport.rendererRendersElementId(renderer))))
 							.bind(entity).with(binderFunction)
 							.fetchAs(Entity.class)
 							.one();

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

@@ -427,10 +427,10 @@ static <T> Object retrieveOrSetRelatedId(
 	 * @return {@literal true} if renderer will use elementId
 	 */
 	static boolean rendererCanUseElementIdIfPresent(Renderer renderer, Neo4jPersistentEntity<?> targetEntity) {
-		return !targetEntity.isUsingDeprecatedInternalId() && targetEntity.isUsingInternalIds() && rendererRendersElementId(renderer);
+		return !targetEntity.isUsingDeprecatedInternalId() && rendererRendersElementId(renderer);
 	}
 
-	private static boolean rendererRendersElementId(Renderer renderer) {
+	static boolean rendererRendersElementId(Renderer renderer) {
 		return renderer.render(Cypher.returning(Functions.elementId(Cypher.anyNode("n"))).build())
 				.equals("RETURN elementId(n)");
 	}