Spring/spring-data-neo4j
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

GH-2879 - Use elementId in derive queries.

Browse Source 
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
This commit is contained in:
Gerrit Meier
2024-04-02 16:48:39 +02:00
parent 2b28ac28ad
commit 58c184a7a8
5 changed files with 83 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -483,11 +483,17 @@ final class CypherQueryCreator extends AbstractQueryCreator<QueryFragmentsAndPar
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());
}

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

@@ -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 @@ abstract class AbstractElementIdTestBase {
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)"));
}
}

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

@@ -50,9 +50,13 @@ import org.springframework.transaction.annotation.EnableTransactionManagement;
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 @@ public class ImperativeElementIdIT extends AbstractElementIdTestBase {
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 @@ 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)

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

@@ -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<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 @@ public class ReactiveElementIdIT extends AbstractElementIdTestBase {
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 @@ 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)

3
src/test/java/org/springframework/data/neo4j/test/LogbackCapture.java
View File

@@ -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);
}