From cb5ce0621010af4d1d4ff55e3e40e2d91aa3cb30 Mon Sep 17 00:00:00 2001 From: Gerrit Meier Date: Tue, 2 Apr 2024 16:48:39 +0200 Subject: [PATCH] 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 --- .../repository/query/CypherQueryCreator.java | 14 +++++-- .../AbstractElementIdTestBase.java | 14 ++++++- .../ImperativeElementIdIT.java | 35 +++++++++++++++--- .../pure_element_id/ReactiveElementIdIT.java | 37 +++++++++++++++---- .../data/neo4j/test/LogbackCapture.java | 3 +- 5 files changed, 83 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java index f17fbdb6e..924e5eec7 100644 --- a/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java +++ b/src/main/java/org/springframework/data/neo4j/repository/query/CypherQueryCreator.java @@ -483,11 +483,17 @@ final class CypherQueryCreator extends AbstractQueryCreator 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)")); + } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java index 3b3ea94db..713090eb9 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ImperativeElementIdIT.java @@ -50,9 +50,13 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; public class ImperativeElementIdIT extends AbstractElementIdTestBase { interface Repo1 extends Neo4jRepository { + + NodeWithGeneratedId1 findByIdIn(List ids); } interface Repo2 extends Neo4jRepository { + + NodeWithGeneratedId2 findByRelatedNodesIdIn(List ids); } interface Repo3 extends Neo4jRepository { @@ -61,6 +65,31 @@ public class ImperativeElementIdIT extends AbstractElementIdTestBase { interface Repo4 extends Neo4jRepository { } + @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 @@ public class ImperativeElementIdIT extends AbstractElementIdTestBase { } } - 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) diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java index f6e14cbc1..935be06d5 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/pure_element_id/ReactiveElementIdIT.java +++ b/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.data.neo4j.test.Neo4jReactiveTestConfiguration; 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 { + Mono findByIdIn(List ids); } interface Repo2 extends ReactiveNeo4jRepository { + Mono findByRelatedNodesIdIn(List ids); } interface Repo3 extends ReactiveNeo4jRepository { @@ -66,6 +69,30 @@ public class ReactiveElementIdIT extends AbstractElementIdTestBase { interface Repo4 extends ReactiveNeo4jRepository { } + @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 @@ public class ReactiveElementIdIT extends AbstractElementIdTestBase { } } - 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) diff --git a/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java b/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java index c7d0afc9d..998af5f84 100644 --- a/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java +++ b/src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java @@ -63,12 +63,13 @@ public final class LogbackCapture implements ExtensionContext.Store.CloseableRes } void clear() { + this.resetLogLevel(); this.listAppender.list.clear(); } @Override public void close() { - resetLogLevel(); + this.resetLogLevel(); this.listAppender.stop(); this.logger.detachAppender(listAppender); }