Skip to content

Commit d84991b

Browse files
meistermeiermeistermeier
committed
GH-2175 - Fix Pageable and Sort for cyclic domains.
1 parent 03b0898 commit d84991b

File tree

8 files changed

+153
-62
lines changed

8 files changed

+153
-62
lines changed

‎src/main/java/org/springframework/data/neo4j/core/GenericQueryAndParameters.java

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,6 @@
1515
*/
1616
package org.springframework.data.neo4j.core;
1717

18-
import org.neo4j.cypherdsl.core.Cypher;
19-
import org.neo4j.cypherdsl.core.Functions;
20-
import org.neo4j.cypherdsl.core.Node;
21-
import org.neo4j.cypherdsl.core.Relationship;
22-
import org.neo4j.cypherdsl.core.Statement;
23-
import org.springframework.data.neo4j.core.mapping.Constants;
24-
2518
import java.util.Collection;
2619
import java.util.Collections;
2720
import java.util.HashMap;
@@ -34,7 +27,6 @@ final class GenericQueryAndParameters {
3427
private final static String RELATIONSHIP_IDS = "relationshipIds";
3528
private final static String RELATED_NODE_IDS = "relatedNodeIds";
3629

37-
final static Statement STATEMENT = createStatement();
3830
final static GenericQueryAndParameters EMPTY =
3931
new GenericQueryAndParameters(Collections.emptySet(), Collections.emptySet(), Collections.emptySet());
4032

@@ -64,21 +56,4 @@ boolean isEmpty() {
6456
return parameters.get(ROOT_NODE_IDS).isEmpty();
6557
}
6658

67-
private static Statement createStatement() {
68-
Node rootNodes = Cypher.anyNode(ROOT_NODE_IDS);
69-
Node relatedNodes = Cypher.anyNode(RELATED_NODE_IDS);
70-
Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(RELATIONSHIP_IDS);
71-
72-
return Cypher.match(rootNodes)
73-
.where(Functions.id(rootNodes).in(Cypher.parameter(ROOT_NODE_IDS)))
74-
.optionalMatch(relationships)
75-
.where(Functions.id(relationships).in(Cypher.parameter(RELATIONSHIP_IDS)))
76-
.optionalMatch(relatedNodes)
77-
.where(Functions.id(relatedNodes).in(Cypher.parameter(RELATED_NODE_IDS)))
78-
.returning(
79-
rootNodes.as(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE),
80-
Functions.collectDistinct(relationships).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
81-
Functions.collectDistinct(relatedNodes).as(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)
82-
).build();
83-
}
8459
}

‎src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ private Optional<Neo4jClient.RecordFetchSpec<T>> createFetchSpec() {
668668
if (genericQueryAndParameters.isEmpty()) {
669669
return Optional.empty();
670670
}
671-
cypherQuery = renderer.render(GenericQueryAndParameters.STATEMENT);
671+
cypherQuery = renderer.render(queryFragments.generateGenericStatement());
672672
finalParameters = genericQueryAndParameters.getParameters();
673673
} else {
674674
cypherQuery = renderer.render(queryFragments.toStatement());

‎src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ private <T> Mono<ExecutableQuery<T>> createExecutableQuery(Class<T> domainType,
451451
if (containsPossibleCircles && !queryFragments.isScalarValueReturn()) {
452452
return createQueryAndParameters(entityMetaData, queryFragments, parameters)
453453
.flatMap(finalQueryAndParameters ->
454-
createExecutableQuery(domainType, renderer.render(GenericQueryAndParameters.STATEMENT),
454+
createExecutableQuery(domainType, renderer.render(queryFragments.generateGenericStatement()),
455455
finalQueryAndParameters.getParameters()));
456456
}
457457

@@ -738,7 +738,7 @@ public <T> Mono<ExecutableQuery<T>> toExecutableQuery(PreparedQuery<T> preparedQ
738738
if (containsPossibleCircles && !queryFragments.isScalarValueReturn()) {
739739
return createQueryAndParameters(entityMetaData, queryFragments, parameters)
740740
.map(genericQueryAndParameters -> {
741-
ReactiveNeo4jClient.MappingSpec<T> mappingSpec = this.neo4jClient.query(renderer.render(GenericQueryAndParameters.STATEMENT))
741+
ReactiveNeo4jClient.MappingSpec<T> mappingSpec = this.neo4jClient.query(renderer.render(queryFragments.generateGenericStatement()))
742742
.bindAll(genericQueryAndParameters.getParameters()).fetchAs(resultType);
743743

744744
ReactiveNeo4jClient.RecordFetchSpec<T> fetchSpec = preparedQuery.getOptionalMappingFunction()

‎src/main/java/org/springframework/data/neo4j/repository/query/QueryFragmentsAndParameters.java

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@
2020
import org.neo4j.cypherdsl.core.Conditions;
2121
import org.neo4j.cypherdsl.core.Cypher;
2222
import org.neo4j.cypherdsl.core.Expression;
23+
import org.neo4j.cypherdsl.core.Functions;
24+
import org.neo4j.cypherdsl.core.Node;
2325
import org.neo4j.cypherdsl.core.PatternElement;
26+
import org.neo4j.cypherdsl.core.Relationship;
2427
import org.neo4j.cypherdsl.core.SortItem;
2528
import org.neo4j.cypherdsl.core.Statement;
2629
import org.neo4j.cypherdsl.core.StatementBuilder;
@@ -41,6 +44,7 @@
4144
import java.util.HashSet;
4245
import java.util.List;
4346
import java.util.Map;
47+
import java.util.Optional;
4448
import java.util.Set;
4549

4650
import static org.neo4j.cypherdsl.core.Cypher.parameter;
@@ -59,7 +63,8 @@ public final class QueryFragmentsAndParameters {
5963
private final QueryFragments queryFragments;
6064
private final String cypherQuery;
6165

62-
public QueryFragmentsAndParameters(NodeDescription<?> nodeDescription, QueryFragments queryFragments, Map<String, Object> parameters) {
66+
public QueryFragmentsAndParameters(NodeDescription<?> nodeDescription, QueryFragments queryFragments,
67+
@Nullable Map<String, Object> parameters) {
6368
this.nodeDescription = nodeDescription;
6469
this.queryFragments = queryFragments;
6570
this.parameters = parameters;
@@ -150,8 +155,19 @@ static QueryFragmentsAndParameters forExample(Neo4jMappingContext mappingContext
150155
Map<String, Object> parameters = predicate.getParameters();
151156
Condition condition = predicate.getCondition();
152157

153-
Neo4jPersistentEntity<?> entityMetaData = mappingContext.getPersistentEntity(example.getProbeType());
158+
return getQueryFragmentsAndParameters(mappingContext.getPersistentEntity(example.getProbeType()), pageable,
159+
sort, parameters, condition);
160+
}
161+
162+
public static QueryFragmentsAndParameters forPageableAndSort(Neo4jPersistentEntity<?> neo4jPersistentEntity,
163+
@Nullable Pageable pageable, @Nullable Sort sort) {
154164

165+
return getQueryFragmentsAndParameters(neo4jPersistentEntity, pageable, sort, Collections.emptyMap(), null);
166+
}
167+
168+
private static QueryFragmentsAndParameters getQueryFragmentsAndParameters(
169+
Neo4jPersistentEntity<?> entityMetaData, @Nullable Pageable pageable, @Nullable Sort sort,
170+
@Nullable Map<String, Object> parameters, @Nullable Condition condition) {
155171

156172
Expression[] returnStatement = cypherGenerator.createReturnStatementForMatch(entityMetaData);
157173

@@ -172,7 +188,6 @@ static QueryFragmentsAndParameters forExample(Neo4jMappingContext mappingContext
172188
}
173189

174190
return new QueryFragmentsAndParameters(entityMetaData, queryFragments, parameters);
175-
176191
}
177192

178193
/**
@@ -204,8 +219,8 @@ public List<PatternElement> getMatchOn() {
204219
return matchOn;
205220
}
206221

207-
public void setCondition(Condition condition) {
208-
this.condition = condition;
222+
public void setCondition(@Nullable Condition condition) {
223+
this.condition = Optional.ofNullable(condition).orElse(Conditions.noCondition());
209224
}
210225

211226
public Condition getCondition() {
@@ -260,6 +275,33 @@ private SortItem[] getOrderBy() {
260275
return orderBy != null ? orderBy : new SortItem[]{};
261276
}
262277

278+
public Statement generateGenericStatement() {
279+
String rootNodeIds = "rootNodeIds";
280+
String relationshipIds = "relationshipIds";
281+
String relatedNodeIds = "relatedNodeIds";
282+
Node rootNodes = Cypher.anyNode(rootNodeIds);
283+
Node relatedNodes = Cypher.anyNode(relatedNodeIds);
284+
Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(relationshipIds);
285+
return Cypher.match(rootNodes)
286+
.where(Functions.id(rootNodes).in(Cypher.parameter(rootNodeIds)))
287+
.optionalMatch(relationships)
288+
.where(Functions.id(relationships).in(Cypher.parameter(relationshipIds)))
289+
.optionalMatch(relatedNodes)
290+
.where(Functions.id(relatedNodes).in(Cypher.parameter(relatedNodeIds)))
291+
.with(
292+
rootNodes.as(Constants.NAME_OF_ROOT_NODE.getValue()),
293+
Functions.collectDistinct(relationships).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
294+
Functions.collectDistinct(relatedNodes).as(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES))
295+
.orderBy(getOrderBy())
296+
.returning(
297+
Constants.NAME_OF_ROOT_NODE.as(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE),
298+
Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
299+
Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)
300+
)
301+
.skip(skip)
302+
.limit(limit).build();
303+
}
304+
263305
public Statement toStatement() {
264306

265307
StatementBuilder.OngoingReadingWithoutWhere match = null;

‎src/main/java/org/springframework/data/neo4j/repository/support/SimpleNeo4jRepository.java

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,13 @@
2222
import java.util.stream.StreamSupport;
2323

2424
import org.apiguardian.api.API;
25-
import org.neo4j.cypherdsl.core.Statement;
26-
import org.neo4j.cypherdsl.core.StatementBuilder;
27-
import org.neo4j.cypherdsl.core.StatementBuilder.OngoingReadingAndReturn;
2825
import org.springframework.data.domain.Page;
2926
import org.springframework.data.domain.Pageable;
3027
import org.springframework.data.domain.Sort;
3128
import org.springframework.data.neo4j.core.Neo4jOperations;
32-
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
3329
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
3430
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
35-
import org.springframework.data.neo4j.repository.query.CypherAdapterUtils;
31+
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
3632
import org.springframework.data.repository.PagingAndSortingRepository;
3733
import org.springframework.data.support.PageableExecutionUtils;
3834
import org.springframework.stereotype.Repository;
@@ -60,14 +56,11 @@ public class SimpleNeo4jRepository<T, ID> implements PagingAndSortingRepository<
6056

6157
private final Neo4jPersistentEntity<T> entityMetaData;
6258

63-
private final CypherGenerator cypherGenerator;
64-
6559
protected SimpleNeo4jRepository(Neo4jOperations neo4jOperations, Neo4jEntityInformation<T, ID> entityInformation) {
6660

6761
this.neo4jOperations = neo4jOperations;
6862
this.entityInformation = entityInformation;
6963
this.entityMetaData = this.entityInformation.getEntityMetaData();
70-
this.cypherGenerator = CypherGenerator.INSTANCE;
7164
}
7265

7366
@Override
@@ -91,25 +84,17 @@ public List<T> findAll() {
9184
@Override
9285
public List<T> findAll(Sort sort) {
9386

94-
Statement statement = cypherGenerator.prepareMatchOf(entityMetaData)
95-
.returning(cypherGenerator.createReturnStatementForMatch(entityMetaData))
96-
.orderBy(CypherAdapterUtils.toSortItems(entityMetaData, sort)).build();
97-
98-
return this.neo4jOperations.findAll(statement, entityInformation.getJavaType());
87+
return this.neo4jOperations.toExecutableQuery(entityInformation.getJavaType(),
88+
QueryFragmentsAndParameters.forPageableAndSort(entityMetaData, null, sort))
89+
.getResults();
9990
}
10091

10192
@Override
10293
public Page<T> findAll(Pageable pageable) {
94+
List<T> allResult = this.neo4jOperations.toExecutableQuery(entityInformation.getJavaType(),
95+
QueryFragmentsAndParameters.forPageableAndSort(entityMetaData, pageable, null))
96+
.getResults();
10397

104-
OngoingReadingAndReturn returning = cypherGenerator.prepareMatchOf(entityMetaData)
105-
.returning(cypherGenerator.createReturnStatementForMatch(entityMetaData));
106-
107-
StatementBuilder.BuildableStatement returningWithPaging = CypherAdapterUtils.addPagingParameter(entityMetaData,
108-
pageable, returning);
109-
110-
Statement statement = returningWithPaging.build();
111-
112-
List<T> allResult = this.neo4jOperations.findAll(statement, entityInformation.getJavaType());
11398
LongSupplier totalCountSupplier = this::count;
11499
return PageableExecutionUtils.getPage(allResult, pageable, totalCountSupplier);
115100
}

‎src/main/java/org/springframework/data/neo4j/repository/support/SimpleReactiveNeo4jRepository.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,14 @@
2323
import java.util.stream.StreamSupport;
2424

2525
import org.apiguardian.api.API;
26-
import org.neo4j.cypherdsl.core.Statement;
2726
import org.reactivestreams.Publisher;
2827

2928
import org.springframework.data.domain.Sort;
3029
import org.springframework.data.neo4j.core.ReactiveNeo4jOperations;
3130
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
3231
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
3332
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
34-
import org.springframework.data.neo4j.repository.query.CypherAdapterUtils;
33+
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
3534
import org.springframework.data.repository.reactive.ReactiveSortingRepository;
3635
import org.springframework.stereotype.Repository;
3736
import org.springframework.transaction.annotation.Transactional;
@@ -99,11 +98,9 @@ public Flux<T> findAll() {
9998

10099
@Override
101100
public Flux<T> findAll(Sort sort) {
102-
Statement statement = cypherGenerator.prepareMatchOf(entityMetaData)
103-
.returning(cypherGenerator.createReturnStatementForMatch(entityMetaData))
104-
.orderBy(CypherAdapterUtils.toSortItems(entityMetaData, sort)).build();
105-
106-
return neo4jOperations.findAll(statement, this.entityInformation.getJavaType());
101+
return this.neo4jOperations.toExecutableQuery(entityInformation.getJavaType(),
102+
QueryFragmentsAndParameters.forPageableAndSort(entityMetaData, null, sort))
103+
.flatMapMany(ReactiveNeo4jOperations.ExecutableQuery::getResults);
107104
}
108105

109106
@Override

‎src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,62 @@ void findAndMapMultipleLevelsOfSimpleRelationships(@Autowired SimpleEntityWithRe
11441144
assertThat(entityA.getBs().get(0).getCs()).hasSize(1);
11451145
}
11461146

1147+
@Test // GH-2175
1148+
void findCyclicWithPageable(@Autowired RelationshipRepository repository) {
1149+
try (Session session = createSession()) {
1150+
session.run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h1:Hobby{name:'Music'}), "
1151+
+ "(n)-[:Has]->(p1:Pet{name: 'Jerry'}), (n)-[:Has]->(p2:Pet{name: 'Tom'}), "
1152+
+ "(n)<-[:Has]-(c:Club{name:'ClownsClub'}), " + "(p1)-[:Has]->(h2:Hobby{name:'sleeping'}), "
1153+
+ "(p1)-[:Has]->(p2)")
1154+
.consume();
1155+
}
1156+
1157+
Page<PersonWithRelationship> peoplePage = repository.findAll(PageRequest.of(0, 1));
1158+
assertThat(peoplePage.getTotalElements()).isEqualTo(1);
1159+
}
1160+
1161+
@Test // GH-2175
1162+
void findCyclicWithSort(@Autowired RelationshipRepository repository) {
1163+
try (Session session = createSession()) {
1164+
session.run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h1:Hobby{name:'Music'}), "
1165+
+ "(n)-[:Has]->(p1:Pet{name: 'Jerry'}), (n)-[:Has]->(p2:Pet{name: 'Tom'}), "
1166+
+ "(n)<-[:Has]-(c:Club{name:'ClownsClub'}), " + "(p1)-[:Has]->(h2:Hobby{name:'sleeping'}), "
1167+
+ "(p1)-[:Has]->(p2)")
1168+
.consume();
1169+
}
1170+
1171+
List<PersonWithRelationship> people = repository.findAll(Sort.by("name"));
1172+
assertThat(people).hasSize(1);
1173+
}
1174+
1175+
@Test // GH-2175
1176+
void cyclicDerivedFinderWithPageable(@Autowired RelationshipRepository repository) {
1177+
try (Session session = createSession()) {
1178+
session.run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h1:Hobby{name:'Music'}), "
1179+
+ "(n)-[:Has]->(p1:Pet{name: 'Jerry'}), (n)-[:Has]->(p2:Pet{name: 'Tom'}), "
1180+
+ "(n)<-[:Has]-(c:Club{name:'ClownsClub'}), " + "(p1)-[:Has]->(h2:Hobby{name:'sleeping'}), "
1181+
+ "(p1)-[:Has]->(p2)")
1182+
.consume();
1183+
}
1184+
1185+
Page<PersonWithRelationship> peoplePage = repository.findByName("Freddie", PageRequest.of(0, 1));
1186+
assertThat(peoplePage.getTotalElements()).isEqualTo(1);
1187+
}
1188+
1189+
@Test // GH-2175
1190+
void cyclicDerivedFinderWithSort(@Autowired RelationshipRepository repository) {
1191+
try (Session session = createSession()) {
1192+
session.run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h1:Hobby{name:'Music'}), "
1193+
+ "(n)-[:Has]->(p1:Pet{name: 'Jerry'}), (n)-[:Has]->(p2:Pet{name: 'Tom'}), "
1194+
+ "(n)<-[:Has]-(c:Club{name:'ClownsClub'}), " + "(p1)-[:Has]->(h2:Hobby{name:'sleeping'}), "
1195+
+ "(p1)-[:Has]->(p2)")
1196+
.consume();
1197+
}
1198+
1199+
List<PersonWithRelationship> people = repository.findByName("Freddie", Sort.by("name"));
1200+
assertThat(people).hasSize(1);
1201+
}
1202+
11471203
}
11481204

11491205
@Nested
@@ -3958,6 +4014,10 @@ interface RelationshipRepository extends Neo4jRepository<PersonWithRelationship,
39584014

39594015
PersonWithRelationship findByName(String name);
39604016

4017+
Page<PersonWithRelationship> findByName(String name, Pageable pageable);
4018+
4019+
List<PersonWithRelationship> findByName(String name, Sort sort);
4020+
39614021
PersonWithRelationship findByHobbiesNameOrPetsName(String hobbyName, String petName);
39624022

39634023
PersonWithRelationship findByHobbiesNameAndPetsName(String hobbyName, String petName);

‎src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveRepositoryIT.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,6 +1032,36 @@ private long createFriendlyPets() {
10321032
+ "-[:Has]->(:Pet{name:'Tom'})" + "RETURN id(luna) as id").single().get("id").asLong();
10331033
}
10341034
}
1035+
1036+
@Test // GH-2175
1037+
void findCyclicWithSort(@Autowired ReactiveRelationshipRepository repository) {
1038+
try (Session session = createSession()) {
1039+
session.run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h1:Hobby{name:'Music'}), "
1040+
+ "(n)-[:Has]->(p1:Pet{name: 'Jerry'}), (n)-[:Has]->(p2:Pet{name: 'Tom'}), "
1041+
+ "(n)<-[:Has]-(c:Club{name:'ClownsClub'}), " + "(p1)-[:Has]->(h2:Hobby{name:'sleeping'}), "
1042+
+ "(p1)-[:Has]->(p2)")
1043+
.consume();
1044+
}
1045+
1046+
StepVerifier.create(repository.findAll(Sort.by("name")))
1047+
.expectNextCount(1)
1048+
.verifyComplete();
1049+
}
1050+
1051+
@Test // GH-2175
1052+
void cyclicDerivedFinderWithSort(@Autowired ReactiveRelationshipRepository repository) {
1053+
try (Session session = createSession()) {
1054+
session.run("CREATE (n:PersonWithRelationship{name:'Freddie'})-[:Has]->(h1:Hobby{name:'Music'}), "
1055+
+ "(n)-[:Has]->(p1:Pet{name: 'Jerry'}), (n)-[:Has]->(p2:Pet{name: 'Tom'}), "
1056+
+ "(n)<-[:Has]-(c:Club{name:'ClownsClub'}), " + "(p1)-[:Has]->(h2:Hobby{name:'sleeping'}), "
1057+
+ "(p1)-[:Has]->(p2)")
1058+
.consume();
1059+
}
1060+
1061+
StepVerifier.create(repository.findByName("Freddie", Sort.by("name")))
1062+
.expectNextCount(1)
1063+
.verifyComplete();
1064+
}
10351065
}
10361066

10371067
@Nested
@@ -2524,6 +2554,8 @@ interface ReactiveRelationshipRepository extends ReactiveNeo4jRepository<PersonW
25242554
Mono<PersonWithRelationship> findByPetsHobbiesName(String hobbyName);
25252555

25262556
Mono<PersonWithRelationship> findByPetsFriendsName(String petName);
2557+
2558+
Flux<PersonWithRelationship> findByName(String name, Sort sort);
25272559
}
25282560

25292561
interface ReactiveSimilarThingRepository extends ReactiveCrudRepository<SimilarThing, Long> {}

0 commit comments

Comments
 (0)