GH-2750 - Avoid toString in relationship creation.

Without the string conversion Neo4j will either to an (element)id or
label seek instead of a full node scan.
Also: In some corner case adding labels to the generic cyclic query
can improve the performance of the query.
On the other hand it cannot make it worse.

Closes #2750

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

@@ -409,7 +409,12 @@ public final class Neo4jTemplate implements
 			throw new IllegalStateException("Could not retrieve an internal id while saving");
 		}
 
-		String elementId = newOrUpdatedNode.map(IdentitySupport::getElementId).get();
+		Object elementId = newOrUpdatedNode.map(node -> {
+			if (!entityMetaData.isUsingDeprecatedInternalId() && entityMetaData.isUsingInternalIds()) {
+				return IdentitySupport.getElementId(node);
+			}
+			return node.id();
+		}).get();
 
 		PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
 		TemplateSupport.setGeneratedIdIfNecessary(entityMetaData, propertyAccessor, elementId, newOrUpdatedNode);
@@ -807,14 +812,14 @@ public final class Neo4jTemplate implements
 						? stateMachine.getProcessedAs(relatedObjectBeforeCallbacksApplied)
 						: eventSupport.maybeCallBeforeBind(relatedObjectBeforeCallbacksApplied);
 
-				String relatedInternalId;
+				Object relatedInternalId;
 				Entity savedEntity = null;
 				// No need to save values if processed
 				if (stateMachine.hasProcessedValue(relatedValueToStore)) {
 					relatedInternalId = stateMachine.getObjectId(relatedValueToStore);
 				} else {
 					savedEntity = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath);
-					relatedInternalId = TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? savedEntity.elementId() : Long.toString(savedEntity.id());
+					relatedInternalId = TemplateSupport.rendererCanUseElementIdIfPresent(renderer, targetEntity) ? savedEntity.elementId() : savedEntity.id();
 					stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);
 					if (relatedValueToStore instanceof MappingSupport.RelationshipPropertiesWithEntityHolder) {
 						Object entity = ((MappingSupport.RelationshipPropertiesWithEntityHolder) relatedValueToStore).getRelatedEntity();

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

@@ -447,7 +447,9 @@ public final class ReactiveNeo4jTemplate implements
 
 					PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
 					return idMono.doOnNext(newOrUpdatedNode -> {
-						var elementId = IdentitySupport.getElementId(newOrUpdatedNode);
+						var elementId = !entityMetaData.isUsingDeprecatedInternalId() && entityMetaData.isUsingInternalIds()
+										? IdentitySupport.getElementId(newOrUpdatedNode)
+										: newOrUpdatedNode.id();
 						TemplateSupport.setGeneratedIdIfNecessary(entityMetaData, propertyAccessor, elementId, Optional.of(newOrUpdatedNode));
 						TemplateSupport.updateVersionPropertyIfPossible(entityMetaData, propertyAccessor, newOrUpdatedNode);
 						finalStateMachine.markEntityAsProcessed(instance, elementId);
@@ -942,17 +944,17 @@ public final class ReactiveNeo4jTemplate implements
 						.flatMap(newRelatedObject -> {
 							Neo4jPersistentEntity<?> targetEntity = neo4jMappingContext.getRequiredPersistentEntity(relatedObjectBeforeCallbacksApplied.getClass());
 
-							Mono<Tuple2<AtomicReference<String>, AtomicReference<Entity>>> queryOrSave;
+							Mono<Tuple2<AtomicReference<Object>, AtomicReference<Entity>>> queryOrSave;
 							if (stateMachine.hasProcessedValue(relatedValueToStore)) {
-								AtomicReference<String> relatedInternalId = new AtomicReference<>();
+								AtomicReference<Object> relatedInternalId = new AtomicReference<>();
-								String possibleValue = stateMachine.getObjectId(relatedValueToStore);
+								Object possibleValue = stateMachine.getObjectId(relatedValueToStore);
 								if (possibleValue != null) {
 									relatedInternalId.set(possibleValue);
 								}
 								queryOrSave = Mono.just(Tuples.of(relatedInternalId, new AtomicReference<>()));
 							} else {
 								queryOrSave = saveRelatedNode(newRelatedObject, targetEntity, includeProperty, currentPropertyPath)
-										.map(entity -> Tuples.of(new AtomicReference<>(TemplateSupport.rendererCanUseElementIdIfPresent(renderer) ? entity.elementId() : Long.toString(entity.id())), new AtomicReference<>(entity)))
+										.map(entity -> Tuples.of(new AtomicReference<>((Object) (TemplateSupport.rendererCanUseElementIdIfPresent(renderer, targetEntity) ? entity.elementId() : entity.id())), new AtomicReference<>(entity)))
 										.doOnNext(t -> {
 											var relatedInternalId = t.getT1().get();
 											stateMachine.markEntityAsProcessed(relatedValueToStore, relatedInternalId);
@@ -963,7 +965,7 @@ public final class ReactiveNeo4jTemplate implements
 										});
 							}
 							return queryOrSave.flatMap(idAndEntity -> {
-									String relatedInternalId = idAndEntity.getT1().get();
+									Object relatedInternalId = idAndEntity.getT1().get();
 									Entity savedEntity = idAndEntity.getT2().get();
 									Neo4jPersistentProperty requiredIdProperty = targetEntity.getRequiredIdProperty();
 									PersistentPropertyAccessor<?> targetPropertyAccessor = targetEntity.getPropertyAccessor(newRelatedObject);

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

@@ -204,28 +204,26 @@ public final class TemplateSupport {
 
 		Statement toStatement(NodeDescription<?> nodeDescription) {
 
-			String rootNodeIds = "rootNodeIds";
+			String primaryLabel = nodeDescription.getPrimaryLabel();
-			String relationshipIds = "relationshipIds";
+			Node rootNodes = Cypher.node(primaryLabel).named(ROOT_NODE_IDS);
-			String relatedNodeIds = "relatedNodeIds";
+			Node relatedNodes = Cypher.anyNode(RELATED_NODE_IDS);
-			Node rootNodes = Cypher.anyNode(rootNodeIds);
+			Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(RELATIONSHIP_IDS);
-			Node relatedNodes = Cypher.anyNode(relatedNodeIds);
-			Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(relationshipIds);
 			return Cypher.match(rootNodes)
-					.where(Functions.elementId(rootNodes).in(Cypher.parameter(rootNodeIds)))
+					.where(Functions.elementId(rootNodes).in(Cypher.parameter(ROOT_NODE_IDS)))
 					.with(Functions.collect(rootNodes).as(Constants.NAME_OF_ROOT_NODE))
 					.optionalMatch(relationships)
-					.where(Functions.elementId(relationships).in(Cypher.parameter(relationshipIds)))
+					.where(Functions.elementId(relationships).in(Cypher.parameter(RELATIONSHIP_IDS)))
 					.with(Constants.NAME_OF_ROOT_NODE, Functions.collectDistinct(relationships).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS))
 					.optionalMatch(relatedNodes)
-					.where(Functions.elementId(relatedNodes).in(Cypher.parameter(relatedNodeIds)))
+					.where(Functions.elementId(relatedNodes).in(Cypher.parameter(RELATED_NODE_IDS)))
 					.with(
 							Constants.NAME_OF_ROOT_NODE,
 							Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATIONS).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
 							Functions.collectDistinct(relatedNodes).as(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)
 					)
-					.unwind(Constants.NAME_OF_ROOT_NODE).as(rootNodeIds)
+					.unwind(Constants.NAME_OF_ROOT_NODE).as(ROOT_NODE_IDS)
 					.with(
-							Cypher.name(rootNodeIds).as(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription).getValue()),
+							Cypher.name(ROOT_NODE_IDS).as(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription).getValue()),
 							Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
 							Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES))
 					.orderBy(queryFragments.getOrderBy())
@@ -353,7 +351,7 @@ public final class TemplateSupport {
 	static <T> void setGeneratedIdIfNecessary(
 			Neo4jPersistentEntity<?> entityMetaData,
 			PersistentPropertyAccessor<T> propertyAccessor,
-			String elementId,
+			Object elementId,
 			Optional<Entity> databaseEntity
 	) {
 		if (!entityMetaData.isUsingInternalIds()) {
@@ -381,11 +379,11 @@ public final class TemplateSupport {
 	 * @param <T> The type of the entity
 	 * @return The actual related internal id being used.
 	 */
-	static <T> String retrieveOrSetRelatedId(
+	static <T> Object retrieveOrSetRelatedId(
 			Neo4jPersistentEntity<?> entityMetadata,
 			PersistentPropertyAccessor<T> propertyAccessor,
 			Optional<Entity> databaseEntity,
-			@Nullable String relatedInternalId
+			@Nullable Object relatedInternalId
 	) {
 		if (!entityMetadata.isUsingInternalIds()) {
 			return Objects.requireNonNull(relatedInternalId);
@@ -403,7 +401,7 @@ public final class TemplateSupport {
 			}
 		} else {
 			if (relatedInternalId == null && current != null) {
-				relatedInternalId = (String) current;
+				relatedInternalId = current;
 			} else if (current == null) {
 				propertyAccessor.setProperty(requiredIdProperty, relatedInternalId);
 			}
@@ -415,8 +413,8 @@ public final class TemplateSupport {
 	 * Checks if the renderer is configured in such a way that it will use element id or apply toString(id(n)) workaround.
 	 * @return {@literal true} if renderer will use elementId
 	 */
-	static boolean rendererCanUseElementIdIfPresent(Renderer renderer) {
+	static boolean rendererCanUseElementIdIfPresent(Renderer renderer, Neo4jPersistentEntity<?> targetEntity) {
-		return renderer.render(Cypher.returning(Functions.elementId(Cypher.anyNode("n"))).build())
+		return !targetEntity.isUsingDeprecatedInternalId() && targetEntity.isUsingInternalIds() && renderer.render(Cypher.returning(Functions.elementId(Cypher.anyNode("n"))).build())
 				.equals("RETURN elementId(n)");
 	}
 

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

@@ -430,13 +430,14 @@ public enum CypherGenerator {
 		var startNodeIdFunction = getNodeIdFunction(neo4jPersistentEntity);
 		return match(startNode)
 				.where(startNodeIdFunction.apply(startNode).isEqualTo(idParameter))
-				.match(endNode).where(Functions.elementId(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)))
+				.match(endNode)
+				.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)))
 				.merge(relationshipFragment)
 				.returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment))
 				.build();
 	}
 
-	private static Function<Node, Expression> getNodeIdFunction(Neo4jPersistentEntity<?> entity) {
+	private static Function<Node, Expression> getNodeIdFunction(@Nullable Neo4jPersistentEntity<?> entity) {
 
 		Function<Node, Expression> startNodeIdFunction;
 		var idProperty = entity.getRequiredIdProperty();
@@ -452,6 +453,20 @@ public enum CypherGenerator {
 		return startNodeIdFunction;
 	}
 
+	private static Function<Node, Expression> getEndNodeIdFunction(@Nullable Neo4jPersistentEntity<?> entity) {
+
+		Function<Node, Expression> startNodeIdFunction;
+		if (entity == null) {
+			return Functions::elementId;
+		}
+		if (!entity.isUsingDeprecatedInternalId() && entity.isUsingInternalIds()) {
+			startNodeIdFunction = Functions::elementId;
+		} else {
+			startNodeIdFunction = Functions::id;
+		}
+		return startNodeIdFunction;
+	}
+
 	private static Function<Relationship, Expression> getRelationshipIdFunction(RelationshipDescription relationshipDescription) {
 
 		Function<Relationship, Expression> result = Functions::elementId;
@@ -488,7 +503,8 @@ public enum CypherGenerator {
 				.with(row)
 				.match(startNode)
 				.where(getNodeIdFunction(neo4jPersistentEntity).apply(startNode).isEqualTo(idProperty))
-				.match(endNode).where(Functions.elementId(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
+				.match(endNode)
+				.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
 				.merge(relationshipFragment)
 				.returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment))
 				.build();
@@ -521,7 +537,8 @@ public enum CypherGenerator {
 
 		StatementBuilder.OngoingReadingWithWhere startAndEndNodeMatch = match(startNode)
 				.where(nodeIdFunction.apply(startNode).isEqualTo(idParameter))
-				.match(endNode).where(Functions.elementId(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)));
+				.match(endNode)
+				.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)));
 
 		StatementBuilder.ExposesSet createOrMatch = isNew
 				? startAndEndNodeMatch.create(relationshipFragment)
@@ -567,7 +584,7 @@ public enum CypherGenerator {
 					.match(startNode)
 					.where(nodeIdFunction.apply(startNode).isEqualTo(idProperty))
 					.match(endNode)
-					.where(endNode.elementId().isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
+					.where(getEndNodeIdFunction((Neo4jPersistentEntity<?>) relationship.getTarget()).apply(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
 					.create(relationshipFragment)
 					.mutate(RELATIONSHIP_NAME, relationshipProperties)
 					.returning(getReturnedIdExpressionsForRelationship(relationship, relationshipFragment))

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

@@ -64,7 +64,7 @@ public final class NestedRelationshipProcessingStateMachine {
 	 * A map pointing from a processed object to the internal id.
 	 * This will be useful during the persistence to avoid another DB network round-trip.
 	 */
-	private final Map<Integer, String> processedObjectsIds = new HashMap<>();
+	private final Map<Integer, Object> processedObjectsIds = new HashMap<>();
 
 	public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext) {
 
@@ -73,7 +73,7 @@ public final class NestedRelationshipProcessingStateMachine {
 		this.mappingContext = mappingContext;
 	}
 
-	public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext, Object initialObject, String elementId) {
+	public NestedRelationshipProcessingStateMachine(final Neo4jMappingContext mappingContext, Object initialObject, Object elementId) {
 		this(mappingContext);
 
 		Assert.notNull(initialObject, "Initial object must not be null");
@@ -143,7 +143,7 @@ public final class NestedRelationshipProcessingStateMachine {
 	 * @param valueToStore If not {@literal null}, all non-null values will be marked as processed
 	 * @param elementId    The internal id of the value processed
 	 */
-	public void markEntityAsProcessed(Object valueToStore, String elementId) {
+	public void markEntityAsProcessed(Object valueToStore, Object elementId) {
 
 		final long stamp = lock.writeLock();
 		try {
@@ -154,7 +154,7 @@ public final class NestedRelationshipProcessingStateMachine {
 		}
 	}
 
-	private void doMarkValueAsProcessed(Object valueToStore, String elementId) {
+	private void doMarkValueAsProcessed(Object valueToStore, Object elementId) {
 
 		Object value = extractRelatedValueFromRelationshipProperties(valueToStore);
 		storeHashedVersionInProcessedObjectsIds(valueToStore, elementId);
@@ -191,7 +191,7 @@ public final class NestedRelationshipProcessingStateMachine {
 						.findAny();
 				if (alreadyProcessedObject.isPresent()) { // Skip the show the next time around.
 					processed = true;
-					String internalId = getObjectId(alreadyProcessedObject.get());
+					Object internalId = getObjectId(alreadyProcessedObject.get());
 					if (internalId != null) {
 						stamp = lock.tryConvertToWriteLock(stamp);
 						doMarkValueAsProcessed(valueToCheck, internalId);
@@ -239,11 +239,11 @@ public final class NestedRelationshipProcessingStateMachine {
 	 * @return The objects id
 	 */
 	@Nullable
-	public String getObjectId(Object object) {
+	public Object getObjectId(Object object) {
 		final long stamp = lock.readLock();
 		try {
 			Object valueToCheck = extractRelatedValueFromRelationshipProperties(object);
-			String possibleId = getProcessedObjectIds(valueToCheck);
+			Object possibleId = getProcessedObjectIds(valueToCheck);
 			return possibleId != null ? possibleId : getProcessedObjectIds(getProcessedAs(valueToCheck));
 		} finally {
 			lock.unlock(stamp);
@@ -261,7 +261,7 @@ public final class NestedRelationshipProcessingStateMachine {
 	}
 
 	@Nullable
-	private String getProcessedObjectIds(@Nullable Object entity) {
+	private Object getProcessedObjectIds(@Nullable Object entity) {
 		if (entity == null) {
 			return null;
 		}
@@ -282,7 +282,7 @@ public final class NestedRelationshipProcessingStateMachine {
 	/*
 	 * Convenience wrapper functions to avoid exposing the System.identityHashCode "everywhere" in this class.
 	 */
-	private void storeHashedVersionInProcessedObjectsIds(Object initialObject, String elementId) {
+	private void storeHashedVersionInProcessedObjectsIds(Object initialObject, Object elementId) {
 		processedObjectsIds.put(System.identityHashCode(initialObject), elementId);
 	}