From 17c895bbb10ad81acf6a450d6d9c4fb68b3f55d7 Mon Sep 17 00:00:00 2001 From: Eric Spiegelberg Date: Wed, 30 Aug 2017 09:51:25 -0500 Subject: [PATCH] Return correct results when paging and filtering on relationship property Replace "with n" for "with distinct n" when filtering on a relationship property. Fixes #384 --- .../cypher/query/PagingAndSortingQuery.java | 5 +++-- .../ogm/session/request/NodeQueryBuilder.java | 8 +++++++ .../strategy/impl/NodeQueryStatements.java | 2 +- .../impl/NodeQueryStatementsTest.java | 22 +++++++++---------- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/neo4j/ogm/cypher/query/PagingAndSortingQuery.java b/core/src/main/java/org/neo4j/ogm/cypher/query/PagingAndSortingQuery.java index 198d07ebc5..e59b4849b6 100644 --- a/core/src/main/java/org/neo4j/ogm/cypher/query/PagingAndSortingQuery.java +++ b/core/src/main/java/org/neo4j/ogm/cypher/query/PagingAndSortingQuery.java @@ -12,10 +12,10 @@ */ package org.neo4j.ogm.cypher.query; -import org.neo4j.ogm.session.request.NodeQueryBuilder; - import java.util.Map; +import org.apache.commons.lang3.StringUtils; + /** * Extends {@link CypherQuery} with additional functionality for Paging and Sorting. * Only used by queries that return actual nodes and/or relationships from the graph. Other queries @@ -67,6 +67,7 @@ public String getStatement() { StringBuilder sb = new StringBuilder(); String returnClause = this.returnClause; + sb.append(matchClause); if (!sorting.isEmpty()) { sb.append(sorting.replace("$", "n")); diff --git a/core/src/main/java/org/neo4j/ogm/session/request/NodeQueryBuilder.java b/core/src/main/java/org/neo4j/ogm/session/request/NodeQueryBuilder.java index b8b63eaaec..d3c298fd9a 100644 --- a/core/src/main/java/org/neo4j/ogm/session/request/NodeQueryBuilder.java +++ b/core/src/main/java/org/neo4j/ogm/session/request/NodeQueryBuilder.java @@ -35,6 +35,7 @@ public class NodeQueryBuilder { private Map parameters; private int matchClauseId; private boolean built = false; + private boolean hasRelationshipMatch = false; public NodeQueryBuilder(String principalLabel, Iterable filters) { this.principalClause = new PrincipalNodeMatchClause(principalLabel); @@ -56,6 +57,7 @@ public FilteredQuery build() { } if (filter.isNested()) { appendNestedFilter(filter); + hasRelationshipMatch = true; } else { //If the filter is not nested, it belongs to the node we're returning principalClause().append(filter); @@ -132,6 +134,12 @@ private StringBuilder toCypher() { stringBuilder.append(matchClause.toCypher()); } + if (hasRelationshipMatch) { + stringBuilder.append("WITH DISTINCT n"); + } else { + stringBuilder.append("WITH n"); + } + return stringBuilder; } } diff --git a/core/src/main/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatements.java b/core/src/main/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatements.java index 2e50eac578..d3b671c6ae 100644 --- a/core/src/main/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatements.java +++ b/core/src/main/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatements.java @@ -92,7 +92,7 @@ public PagingAndSortingQuery findByType(String label, int depth) { @Override public PagingAndSortingQuery findByType(String label, Filters parameters, int depth) { FilteredQuery filteredQuery = FilteredQueryBuilder.buildNodeQuery(label, parameters); - String matchClause = filteredQuery.statement()+ "WITH n"; + String matchClause = filteredQuery.statement(); String returnClause = loadClauseBuilder.build(label, depth); return new PagingAndSortingQuery(matchClause, returnClause, filteredQuery.parameters(), depth != 0, true); } diff --git a/test/src/test/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatementsTest.java b/test/src/test/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatementsTest.java index c9ca28f920..95950f4a89 100644 --- a/test/src/test/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatementsTest.java +++ b/test/src/test/java/org/neo4j/ogm/session/request/strategy/impl/NodeQueryStatementsTest.java @@ -378,7 +378,7 @@ public void testFindByNestedPropertyOutgoing() { planetParam.setNestedEntityTypeLabel("Planet"); planetParam.setRelationshipType("COLLIDES"); planetParam.setRelationshipDirection("OUTGOING"); - assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /** @@ -391,7 +391,7 @@ public void testFindByNestedPropertySameEntityType() { filter.setNestedEntityTypeLabel("Asteroid"); filter.setRelationshipType("COLLIDES"); filter.setRelationshipDirection("OUTGOING"); - assertThat(queryStatements.findByType("Asteroid", new Filters().add(filter), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Asteroid`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + assertThat(queryStatements.findByType("Asteroid", new Filters().add(filter), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Asteroid`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /** @@ -404,7 +404,7 @@ public void testFindByNestedPropertyIncoming() { planetParam.setNestedEntityTypeLabel("Planet"); planetParam.setRelationshipType("COLLIDES"); planetParam.setRelationshipDirection("INCOMING"); - assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)<-[:`COLLIDES`]-(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)<-[:`COLLIDES`]-(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /** @@ -417,7 +417,7 @@ public void testFindByNestedPropertyUndirected() { planetParam.setNestedEntityTypeLabel("Planet"); planetParam.setRelationshipType("COLLIDES"); planetParam.setRelationshipDirection("UNDIRECTED"); - assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]-(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } MATCH (n)-[:`COLLIDES`]-(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /** @@ -433,7 +433,7 @@ public void testFindByMultipleNestedProperties() { planetParam.setNestedEntityTypeLabel("Planet"); planetParam.setRelationshipType("COLLIDES"); planetParam.setRelationshipDirection("OUTGOING"); - assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /** @@ -449,7 +449,7 @@ public void testFindByMultipleNestedPropertiesInfiniteDepth() { planetParam.setNestedEntityTypeLabel("Planet"); planetParam.setRelationshipType("COLLIDES"); planetParam.setRelationshipDirection("OUTGOING"); - assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), -1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..]-(m) RETURN p, ID(n)"); + assertThat(queryStatements.findByType("Asteroid", new Filters().add(diameterParam).add(planetParam), -1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) WHERE n.`diameter` > { `diameter_0` } MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..]-(m) RETURN p, ID(n)"); } /** @@ -502,7 +502,7 @@ public void testFindByNestedREProperty() { assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (n)-[r0:`COLLIDES`]->(m0) " + "WHERE r0.`totalDestructionProbability` = { `collision_totalDestructionProbability_0` } " + - "WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + "WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /** @@ -528,7 +528,7 @@ public void testFindByMultipleNestedREProperty() { satelliteParam.setNestedRelationshipEntity(true); assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam).add(satelliteParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (n)-[r0:`COLLIDES`]->(m0) WHERE r0.`totalDestructionProbability` = { `collision_totalDestructionProbability_0` } " + - "MATCH (n)<-[r1:`MONITORED_BY`]-(m1) WHERE r1.`signalStrength` >= { `monitoringSatellites_signalStrength_1` } WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + "MATCH (n)<-[r1:`MONITORED_BY`]-(m1) WHERE r1.`signalStrength` >= { `monitoringSatellites_signalStrength_1` } WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } @@ -554,7 +554,7 @@ public void testFindByNestedBaseAndREProperty() { assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam, moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (n)-[r0:`COLLIDES`]->(m0) " + "WHERE r0.`totalDestructionProbability` = { `collision_totalDestructionProbability_0` } " + - "MATCH (m1:`Moon`) WHERE m1.`name` = { `moon_name_1` } MATCH (n)<-[:`ORBITS`]-(m1) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + "MATCH (m1:`Moon`) WHERE m1.`name` = { `moon_name_1` } MATCH (n)<-[:`ORBITS`]-(m1) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } @@ -578,7 +578,7 @@ public void testFindByDifferentNestedPropertiesAnded() { assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam).add(moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } " + "MATCH (m1:`Moon`) WHERE m1.`name` = { `moon_name_1` } MATCH (n)-[:`COLLIDES`]->(m0) " + - "MATCH (n)<-[:`ORBITS`]-(m1) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + "MATCH (n)<-[:`ORBITS`]-(m1) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /** @@ -620,7 +620,7 @@ public void testFindByMultipleNestedPropertiesAnded() { moonParam.setRelationshipType("COLLIDES"); moonParam.setRelationshipDirection("OUTGOING"); moonParam.setBooleanOperator(BooleanOperator.AND); - assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam).add(moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } AND m0.`size` = { `collidesWith_size_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); + assertThat(queryStatements.findByType("Asteroid", new Filters().add(planetParam).add(moonParam), 1).getStatement()).isEqualTo("MATCH (n:`Asteroid`) MATCH (m0:`Planet`) WHERE m0.`name` = { `collidesWith_name_0` } AND m0.`size` = { `collidesWith_size_1` } MATCH (n)-[:`COLLIDES`]->(m0) WITH DISTINCT n MATCH p=(n)-[*0..1]-(m) RETURN p, ID(n)"); } /**