GH-2681 - Sanitize label replacements.

This requires an update of the CypherDSL to use the built-in
function instead of copying code.

Author: meistermeier

pom.xml

@@ -75,7 +75,7 @@
 		<byte-buddy.version>1.11.0</byte-buddy.version>
 		<cdi>2.0.SP1</cdi>
 		<checkstyle.version>8.40</checkstyle.version>
-		<cypher-dsl.version>2022.4.0</cypher-dsl.version>
+		<cypher-dsl.version>2022.8.5</cypher-dsl.version>
 		<dist.id>spring-data-neo4j</dist.id>
 		<dist.key>SDNEO4J</dist.key>
 		<flatten-maven-plugin.version>1.2.5</flatten-maven-plugin.version>

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

@@ -39,6 +39,7 @@
 import org.neo4j.cypherdsl.core.Conditions;
 import org.neo4j.cypherdsl.core.Cypher;
 import org.neo4j.cypherdsl.core.Expression;
+import org.neo4j.cypherdsl.core.FunctionInvocation;
 import org.neo4j.cypherdsl.core.Functions;
 import org.neo4j.cypherdsl.core.MapProjection;
 import org.neo4j.cypherdsl.core.Node;
@@ -54,6 +55,7 @@
 import org.neo4j.cypherdsl.core.SymbolicName;
 import org.neo4j.cypherdsl.core.renderer.Configuration;
 import org.neo4j.cypherdsl.core.renderer.Renderer;
+import org.neo4j.cypherdsl.core.utils.Assertions;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mapping.PersistentProperty;
@@ -62,6 +64,8 @@
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
+import javax.lang.model.SourceVersion;
+
 /**
  * A generator based on the schema defined by node and relationship descriptions. Most methods return renderable Cypher
  * statements.
@@ -348,7 +352,7 @@ public Statement prepareSaveOf(NodeDescription<?> nodeDescription,
 				Property versionProperty = rootNode.property(((Neo4jPersistentEntity<?>) nodeDescription).getRequiredVersionProperty().getName());
 
 				createIfNew = updateDecorator.apply(optionalMatch(possibleExistingNode)
-						.where(possibleExistingNode.internalId().isEqualTo(idParameter))
+						.where(idFunction(possibleExistingNode).isEqualTo(idParameter))
 						.with(possibleExistingNode)
 						.where(possibleExistingNode.isNull())
 						.create(rootNode.withProperties(versionProperty, literalOf(0)))
@@ -358,7 +362,7 @@ public Statement prepareSaveOf(NodeDescription<?> nodeDescription,
 						.build();
 
 				updateIfExists = updateDecorator.apply(match(rootNode)
-						.where(rootNode.internalId().isEqualTo(idParameter))
+						.where(idFunction(rootNode).isEqualTo(idParameter))
 						.and(versionProperty.isEqualTo(parameter(Constants.NAME_OF_VERSION_PARAM))) // Initial check
 						.set(versionProperty.to(versionProperty.add(literalOf(1)))) // Acquire lock
 						.with(rootNode)
@@ -368,19 +372,23 @@ public Statement prepareSaveOf(NodeDescription<?> nodeDescription,
 						.returning(rootNode).build();
 			} else {
 				createIfNew = updateDecorator
-						.apply(optionalMatch(possibleExistingNode).where(possibleExistingNode.internalId().isEqualTo(idParameter))
+						.apply(optionalMatch(possibleExistingNode).where(idFunction(possibleExistingNode).isEqualTo(idParameter))
 								.with(possibleExistingNode).where(possibleExistingNode.isNull()).create(rootNode)
 								.set(rootNode, parameter(Constants.NAME_OF_PROPERTIES_PARAM)))
 						.returning(rootNode).build();
 
-				updateIfExists = updateDecorator.apply(match(rootNode).where(rootNode.internalId().isEqualTo(idParameter))
+				updateIfExists = updateDecorator.apply(match(rootNode).where(idFunction(rootNode).isEqualTo(idParameter))
 						.mutate(rootNode, parameter(Constants.NAME_OF_PROPERTIES_PARAM))).returning(rootNode).build();
 			}
 
 			return Cypher.union(createIfNew, updateIfExists);
 		}
 	}
 
+	private static FunctionInvocation idFunction(Node rootNode) {
+		return Functions.id(rootNode);
+	}
+
 	public Statement prepareSaveOfMultipleInstancesOf(NodeDescription<?> nodeDescription) {
 
 		Assert.isTrue(!nodeDescription.isUsingInternalIds(),
@@ -398,7 +406,7 @@ public Statement prepareSaveOfMultipleInstancesOf(NodeDescription<?> nodeDescrip
 		return Cypher.unwind(parameter(Constants.NAME_OF_ENTITY_LIST_PARAM)).as(row)
 				.merge(rootNode.withProperties(nameOfIdProperty, Cypher.property(row, Constants.NAME_OF_ID)))
 				.mutate(rootNode, Cypher.property(row, Constants.NAME_OF_PROPERTIES_PARAM))
-				.returning(rootNode.internalId().as(Constants.NAME_OF_INTERNAL_ID), rootNode.property(nameOfIdProperty).as(Constants.NAME_OF_ID))
+				.returning(idFunction(rootNode).as(Constants.NAME_OF_INTERNAL_ID), rootNode.property(nameOfIdProperty).as(Constants.NAME_OF_ID))
 				.build();
 	}
 
@@ -420,9 +428,9 @@ public Statement prepareSaveOfRelationship(Neo4jPersistentEntity<?> neo4jPersist
 				startNode.relationshipFrom(endNode, type)).named(RELATIONSHIP_NAME);
 
 		return match(startNode)
-				.where(neo4jPersistentEntity.isUsingInternalIds() ? startNode.internalId().isEqualTo(idParameter)
+				.where(neo4jPersistentEntity.isUsingInternalIds() ? idFunction(startNode).isEqualTo(idParameter)
 						: startNode.property(idPropertyName).isEqualTo(idParameter))
-				.match(endNode).where(endNode.internalId().isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)))
+				.match(endNode).where(idFunction(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)))
 				.merge(relationshipFragment)
 				.returning(Functions.id(relationshipFragment))
 				.build();
@@ -450,9 +458,9 @@ public Statement prepareSaveOfRelationships(Neo4jPersistentEntity<?> neo4jPersis
 				.with(row)
 				.match(startNode)
 				.where(neo4jPersistentEntity.isUsingInternalIds()
-						? startNode.internalId().isEqualTo(idProperty)
+						? idFunction(startNode).isEqualTo(idProperty)
 						: startNode.property(idPropertyName).isEqualTo(idProperty))
-				.match(endNode).where(endNode.internalId().isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
+				.match(endNode).where(idFunction(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
 				.merge(relationshipFragment)
 				.returning(Functions.id(relationshipFragment))
 				.build();
@@ -482,9 +490,9 @@ public Statement prepareSaveOfRelationshipWithProperties(Neo4jPersistentEntity<?
 				.named(RELATIONSHIP_NAME);
 
 		StatementBuilder.OngoingReadingWithWhere startAndEndNodeMatch = match(startNode)
-				.where(neo4jPersistentEntity.isUsingInternalIds() ? startNode.internalId().isEqualTo(idParameter)
+				.where(neo4jPersistentEntity.isUsingInternalIds() ? idFunction(startNode).isEqualTo(idParameter)
 						: startNode.property(idPropertyName).isEqualTo(idParameter))
-				.match(endNode).where(endNode.internalId().isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)));
+				.match(endNode).where(idFunction(endNode).isEqualTo(parameter(Constants.TO_ID_PARAMETER_NAME)));
 
 		StatementBuilder.ExposesSet createOrMatch = isNew
 				? startAndEndNodeMatch.create(relationshipFragment)
@@ -527,10 +535,10 @@ public Statement prepareUpdateOfRelationshipsWithProperties(Neo4jPersistentEntit
 					.match(startNode)
 					.where(
 							neo4jPersistentEntity.isUsingInternalIds()
-									? startNode.internalId().isEqualTo(idProperty)
+									? idFunction(startNode).isEqualTo(idProperty)
 									: startNode.property(idPropertyName).isEqualTo(idProperty)
 					)
-					.match(endNode).where(endNode.internalId().isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
+					.match(endNode).where(idFunction(endNode).isEqualTo(Cypher.property(row, Constants.TO_ID_PARAMETER_NAME)))
 					.create(relationshipFragment)
 					.mutate(RELATIONSHIP_NAME, relationshipProperties)
 					.returning(Functions.id(relationshipFragment)).build();
@@ -566,7 +574,7 @@ public Statement prepareDeleteOf(
 
 		Parameter<?> idParameter = parameter(Constants.FROM_ID_PARAMETER_NAME);
 		return match(relationship)
-				.where(neo4jPersistentEntity.isUsingInternalIds() ? startNode.internalId().isEqualTo(idParameter)
+				.where(neo4jPersistentEntity.isUsingInternalIds() ? idFunction(startNode).isEqualTo(idParameter)
 						: startNode.property(idPropertyName).isEqualTo(idParameter))
 				.and(Functions.id(relationship).in(Cypher.parameter(Constants.NAME_OF_KNOWN_RELATIONSHIPS_PARAM)).not())
 				.delete(relationship.getRequiredSymbolicName())
@@ -613,6 +621,7 @@ public Collection<Expression> createReturnStatementForMatch(Neo4jPersistentEntit
 						}
 						expression = Cypher.property(property.substring(0, firstDot), tail);
 					} else {
+						Assertions.isTrue(SourceVersion.isIdentifier(property), "Name must be a valid identifier.");
 						expression = Cypher.name(property);
 					}
 					if (order.isIgnoreCase()) {

src/main/java/org/springframework/data/neo4j/repository/query/Neo4jSpelSupport.java

@@ -26,6 +26,7 @@
 
 import org.apache.commons.logging.LogFactory;
 import org.apiguardian.api.API;
+import org.neo4j.cypherdsl.core.internal.SchemaNames;
 import org.springframework.core.log.LogAccessor;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
@@ -111,7 +112,9 @@ private static LiteralReplacement labels(@Nullable Object arg, String joinOn) {
 
 	private static String joinStrings(Object arg, String joinOn) {
 		if (arg instanceof Collection) {
-			return ((Collection<?>) arg).stream().map(Object::toString).collect(Collectors.joining(joinOn));
+			return ((Collection<?>) arg).stream()
+					.map(o -> SchemaNames.sanitize(o.toString()).get())
+					.collect(Collectors.joining(joinOn));
 		}
 
 		// we are so kind and also accept plain strings instead of collection<string>