GH-2879 - Use elementId in derive queries.

There is a convenient function if the property in derived queries
refers to the internal id to use the `id` / `elementId` function.
This was never taken into consideration when the elementId support
got introduced.

The fix is straight forward in line with the existing call to
`Cypher.call("id")` to avoid introducing more changes to the
infrastructure.

Also includes fixture for the logging capture based tests around
elementId/id to catch the right log output again.

Closes #2879

Author: meistermeier

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

@@ -483,11 +483,17 @@ private Expression toCypherProperty(PersistentPropertyPath<Neo4jPersistentProper
 
 		String containerName = getContainerName(path, owner);
 		if (owner.equals(this.nodeDescription) && path.getLength() == 1) {
-			expression = leafProperty.isInternalIdProperty() ?
-					Cypher.call("id").withArgs(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription)).asFunction() :
-					Cypher.property(containerName, leafProperty.getPropertyName());
-		} else if (leafProperty.isInternalIdProperty()) {
+			if (leafProperty.isInternalIdProperty() && owner.isUsingDeprecatedInternalId()) {
+				expression = Cypher.call("id").withArgs(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription)).asFunction();
+			} else if (leafProperty.isInternalIdProperty()) {
+				expression = Cypher.call("elementId").withArgs(Constants.NAME_OF_TYPED_ROOT_NODE.apply(nodeDescription)).asFunction();
+			} else {
+				expression = Cypher.property(containerName, leafProperty.getPropertyName());
+			}
+		} else if (leafProperty.isInternalIdProperty() && owner.isUsingDeprecatedInternalId()) {
 			expression = Cypher.call("id").withArgs(Cypher.name(containerName)).asFunction();
+		} else if (leafProperty.isInternalIdProperty()) {
+			expression = Cypher.call("elementId").withArgs(Cypher.name(containerName)).asFunction();
 		} else {
 			expression = Cypher.property(containerName, leafProperty.getPropertyName());
 		}

src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/AbstractElementIdTestBase.java

@@ -15,26 +15,32 @@
  */
 package org.springframework.data.neo4j.integration.issues.pure_element_id;
 
+import java.util.List;
 import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
 
+import ch.qos.logback.classic.Level;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Tag;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.Session;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.neo4j.test.BookmarkCapture;
+import org.springframework.data.neo4j.test.LogbackCapture;
 import org.springframework.data.neo4j.test.Neo4jExtension;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 @Tag(Neo4jExtension.NEEDS_VERSION_SUPPORTING_ELEMENT_ID)
 abstract class AbstractElementIdTestBase {
 
 	protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
 
 	@BeforeEach
-	void setupData(@Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture) {
+	void setupData(LogbackCapture logbackCapture, @Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture) {
 
+		logbackCapture.addLogger("org.springframework.data.neo4j.cypher.deprecation", Level.WARN);
 		try (Session session = driver.session()) {
 			session.run("MATCH (n) DETACH DELETE n").consume();
 			bookmarkCapture.seedWith(session.lastBookmarks());
@@ -68,4 +74,10 @@ static String adaptQueryTo44IfNecessary(String query) {
 		return query;
 	}
 
+	static void assertThatLogMessageDoNotIndicateIDUsage(LogbackCapture logbackCapture) {
+		List<String> formattedMessages = logbackCapture.getFormattedMessages();
+		assertThat(formattedMessages)
+				.noneMatch(s -> s.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning") ||
+						s.contains("The query used a deprecated function. ('id' is no longer supported)"));
+	}
 }

src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java

@@ -50,9 +50,13 @@
 public class ImperativeElementIdIT extends AbstractElementIdTestBase {
 
 	interface Repo1 extends Neo4jRepository<NodeWithGeneratedId1, String> {
+
+		NodeWithGeneratedId1 findByIdIn(List<String> ids);
 	}
 
 	interface Repo2 extends Neo4jRepository<NodeWithGeneratedId2, String> {
+
+		NodeWithGeneratedId2 findByRelatedNodesIdIn(List<String> ids);
 	}
 
 	interface Repo3 extends Neo4jRepository<NodeWithGeneratedId3, String> {
@@ -61,6 +65,31 @@ interface Repo3 extends Neo4jRepository<NodeWithGeneratedId3, String> {
 	interface Repo4 extends Neo4jRepository<NodeWithGeneratedId4, String> {
 	}
 
+	@Test
+	void dontCallIdForDerivedQueriesWithInClause(LogbackCapture logbackCapture, @Autowired Repo1 repo1) {
+
+		var node = repo1.save(new NodeWithGeneratedId1("testValue"));
+		String id = node.getId();
+
+		repo1.findByIdIn(List.of(id));
+
+		assertThatLogMessageDoNotIndicateIDUsage(logbackCapture);
+	}
+
+	@Test
+	void dontCallIdForDerivedQueriesWithRelatedInClause(LogbackCapture logbackCapture, @Autowired Repo2 repo2) {
+		var node1 = new NodeWithGeneratedId1("testValue");
+		var node2 = new NodeWithGeneratedId2("testValue");
+		node2.setRelatedNodes(List.of(node1));
+		var savedNode2 = repo2.save(node2);
+
+		String id = savedNode2.getRelatedNodes().get(0).getId();
+
+		repo2.findByRelatedNodesIdIn(List.of(id));
+
+		assertThatLogMessageDoNotIndicateIDUsage(logbackCapture);
+	}
+
 	@Test
 	void simpleNodeCreationShouldFillIdAndNotUseIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1) {
 
@@ -322,12 +351,6 @@ RETURN count(*)"""),
 		}
 	}
 
-	private static void assertThatLogMessageDoNotIndicateIDUsage(LogbackCapture logbackCapture) {
-		assertThat(logbackCapture.getFormattedMessages())
-				.noneMatch(s -> s.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning") ||
-						s.contains("The query used a deprecated function. ('id' is no longer supported)"));
-	}
-
 	@Configuration
 	@EnableTransactionManagement
 	@EnableNeo4jRepositories(considerNestedRepositories = true)

src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java

@@ -15,8 +15,6 @@
  */
 package org.springframework.data.neo4j.integration.issues.pure_element_id;
 
-import static org.assertj.core.api.Assertions.assertThat;
-
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -40,6 +38,9 @@
 import org.springframework.lang.NonNull;
 import org.springframework.transaction.ReactiveTransactionManager;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
+import reactor.core.publisher.Mono;
+
+import static org.assertj.core.api.Assertions.assertThat;
 
 /**
  * Assertions that no {@code id()} calls are generated when no deprecated id types are present.
@@ -55,9 +56,11 @@ public class ReactiveElementIdIT extends AbstractElementIdTestBase {
 
 
 	interface Repo1 extends ReactiveNeo4jRepository<NodeWithGeneratedId1, String> {
+		Mono<NodeWithGeneratedId1> findByIdIn(List<String> ids);
 	}
 
 	interface Repo2 extends ReactiveNeo4jRepository<NodeWithGeneratedId2, String> {
+		Mono<NodeWithGeneratedId2> findByRelatedNodesIdIn(List<String> ids);
 	}
 
 	interface Repo3 extends ReactiveNeo4jRepository<NodeWithGeneratedId3, String> {
@@ -66,6 +69,30 @@ interface Repo3 extends ReactiveNeo4jRepository<NodeWithGeneratedId3, String> {
 	interface Repo4 extends ReactiveNeo4jRepository<NodeWithGeneratedId4, String> {
 	}
 
+	@Test
+	void dontCallIdForDerivedQueriesWithInClause(LogbackCapture logbackCapture, @Autowired Repo1 repo1) {
+
+		var node = repo1.save(new NodeWithGeneratedId1("testValue")).block();
+		String id = node.getId();
+
+		repo1.findByIdIn(List.of(id)).block();
+
+		assertThatLogMessageDoNotIndicateIDUsage(logbackCapture);
+	}
+
+	@Test
+	void dontCallIdForDerivedQueriesWithRelatedInClause(LogbackCapture logbackCapture, @Autowired Repo2 repo2) {
+		var node1 = new NodeWithGeneratedId1("testValue");
+		var node2 = new NodeWithGeneratedId2("testValue");
+		node2.setRelatedNodes(List.of(node1));
+		var savedNode2 = repo2.save(node2).block();
+
+		String id = savedNode2.getRelatedNodes().get(0).getId();
+
+		repo2.findByRelatedNodesIdIn(List.of(id)).block();
+
+		assertThatLogMessageDoNotIndicateIDUsage(logbackCapture);
+	}
 
 	@Test
 	void simpleNodeCreationShouldFillIdAndNotUseIdFunction(LogbackCapture logbackCapture, @Autowired Repo1 repo1) {
@@ -336,12 +363,6 @@ RETURN count(*)"""),
 		}
 	}
 
-	private static void assertThatLogMessageDoNotIndicateIDUsage(LogbackCapture logbackCapture) {
-		assertThat(logbackCapture.getFormattedMessages())
-				.noneMatch(s -> s.contains("Neo.ClientNotification.Statement.FeatureDeprecationWarning") ||
-						s.contains("The query used a deprecated function. ('id' is no longer supported)"));
-	}
-
 	@Configuration
 	@EnableTransactionManagement
 	@EnableReactiveNeo4jRepositories(considerNestedRepositories = true)

src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java

@@ -63,12 +63,13 @@ void start() {
 	}
 
 	void clear() {
+		this.resetLogLevel();
 		this.listAppender.list.clear();
 	}
 
 	@Override
 	public void close() {
-		resetLogLevel();
+		this.resetLogLevel();
 		this.listAppender.stop();
 		this.logger.detachAppender(listAppender);
 	}