Skip to content

Commit 1718c7b

Browse files
meistermeiermeistermeier
committed
GH-2175 - Fix Pageable and Sort for cyclic domains.
1 parent 839d321 commit 1718c7b

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

-25
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

+1-1
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ private Optional<Neo4jClient.RecordFetchSpec<T>> createFetchSpec() {
663663
if (genericQueryAndParameters.isEmpty()) {
664664
return Optional.empty();
665665
}
666-
cypherQuery = renderer.render(GenericQueryAndParameters.STATEMENT);
666+
cypherQuery = renderer.render(queryFragments.toGenericStatement());
667667
finalParameters = genericQueryAndParameters.getParameters();
668668
} else {
669669
cypherQuery = renderer.render(queryFragments.toStatement());

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ private <T> Mono<ExecutableQuery<T>> createExecutableQuery(Class<T> domainType,
452452
if (containsPossibleCircles && !queryFragments.isScalarValueReturn()) {
453453
return createQueryAndParameters(entityMetaData, queryFragments, parameters)
454454
.flatMap(finalQueryAndParameters ->
455-
createExecutableQuery(domainType, renderer.render(GenericQueryAndParameters.STATEMENT),
455+
createExecutableQuery(domainType, renderer.render(queryFragments.toGenericStatement()),
456456
finalQueryAndParameters.getParameters()));
457457
}
458458

@@ -747,7 +747,7 @@ public <T> Mono<ExecutableQuery<T>> toExecutableQuery(PreparedQuery<T> preparedQ
747747
if (containsPossibleCircles && !queryFragments.isScalarValueReturn()) {
748748
return createQueryAndParameters(entityMetaData, queryFragments, parameters)
749749
.map(genericQueryAndParameters -> {
750-
ReactiveNeo4jClient.MappingSpec<T> mappingSpec = this.neo4jClient.query(renderer.render(GenericQueryAndParameters.STATEMENT))
750+
ReactiveNeo4jClient.MappingSpec<T> mappingSpec = this.neo4jClient.query(renderer.render(queryFragments.toGenericStatement()))
751751
.in(databaseName.getValue()).bindAll(genericQueryAndParameters.getParameters()).fetchAs(resultType);
752752

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

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

+47-5
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 toGenericStatement() {
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

+7-22
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;
@@ -59,14 +55,11 @@ public class SimpleNeo4jRepository<T, ID> implements PagingAndSortingRepository<
5955

6056
private final Neo4jPersistentEntity<T> entityMetaData;
6157

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

6660
this.neo4jOperations = neo4jOperations;
6761
this.entityInformation = entityInformation;
6862
this.entityMetaData = this.entityInformation.getEntityMetaData();
69-
this.cypherGenerator = CypherGenerator.INSTANCE;
7063
}
7164

7265
@Override
@@ -90,25 +83,17 @@ public List<T> findAll() {
9083
@Override
9184
public List<T> findAll(Sort sort) {
9285

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

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

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

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

+4-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.neo4j.repository.support;
1717

1818
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
19+
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
1920
import reactor.core.publisher.Flux;
2021
import reactor.core.publisher.Mono;
2122

@@ -24,13 +25,11 @@
2425
import java.util.stream.StreamSupport;
2526

2627
import org.apiguardian.api.API;
27-
import org.neo4j.cypherdsl.core.Statement;
2828
import org.reactivestreams.Publisher;
2929
import org.springframework.data.domain.Sort;
3030
import org.springframework.data.neo4j.core.ReactiveNeo4jOperations;
3131
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
3232
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
33-
import org.springframework.data.neo4j.repository.query.CypherAdapterUtils;
3433
import org.springframework.data.repository.reactive.ReactiveSortingRepository;
3534
import org.springframework.stereotype.Repository;
3635
import org.springframework.transaction.annotation.Transactional;
@@ -97,11 +96,9 @@ public Flux<T> findAll() {
9796

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

107104
@Override

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

+60
Original file line numberDiff line numberDiff line change
@@ -1142,6 +1142,62 @@ void findAndMapMultipleLevelsOfSimpleRelationships(@Autowired SimpleEntityWithRe
11421142
assertThat(entityA.getBs().get(0).getCs()).hasSize(1);
11431143
}
11441144

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

11471203
@Nested
@@ -3906,6 +3962,10 @@ interface RelationshipRepository extends Neo4jRepository<PersonWithRelationship,
39063962

39073963
PersonWithRelationship findByName(String name);
39083964

3965+
Page<PersonWithRelationship> findByName(String name, Pageable pageable);
3966+
3967+
List<PersonWithRelationship> findByName(String name, Sort sort);
3968+
39093969
PersonWithRelationship findByHobbiesNameOrPetsName(String hobbyName, String petName);
39103970

39113971
PersonWithRelationship findByHobbiesNameAndPetsName(String hobbyName, String petName);

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

+32
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
@@ -2517,6 +2547,8 @@ interface ReactiveRelationshipRepository extends ReactiveNeo4jRepository<PersonW
25172547
Mono<PersonWithRelationship> findByPetsHobbiesName(String hobbyName);
25182548

25192549
Mono<PersonWithRelationship> findByPetsFriendsName(String petName);
2550+
2551+
Flux<PersonWithRelationship> findByName(String name, Sort sort);
25202552
}
25212553

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

0 commit comments

Comments
 (0)