Skip to content

Commit c4835f8

Browse files
michael-simonsmichael-simons
committed
GH-1907 - Clean-up relationships querying code.
See #1907. Not an implementation or a fix, but improvement of overall readability.
1 parent 722df84 commit c4835f8

8 files changed

+304
-285
lines changed

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

-59
This file was deleted.

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

+24-22
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import org.springframework.data.mapping.AssociationHandler;
5757
import org.springframework.data.mapping.PersistentPropertyAccessor;
5858
import org.springframework.data.mapping.callback.EntityCallbacks;
59+
import org.springframework.data.neo4j.core.TemplateSupport.NodesAndRelationshipsByIdStatementProvider;
5960
import org.springframework.data.neo4j.core.mapping.Constants;
6061
import org.springframework.data.neo4j.core.mapping.CreateRelationshipStatementHolder;
6162
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
@@ -70,6 +71,7 @@
7071
import org.springframework.data.neo4j.core.mapping.RelationshipDescription;
7172
import org.springframework.data.neo4j.core.mapping.callback.EventSupport;
7273
import org.springframework.data.neo4j.repository.NoResultException;
74+
import org.springframework.data.neo4j.repository.query.QueryFragments;
7375
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
7476
import org.springframework.data.projection.ProjectionFactory;
7577
import org.springframework.data.projection.ProjectionInformation;
@@ -362,7 +364,7 @@ private <T> T saveImpl(T instance, @Nullable List<PropertyDescriptor> includedPr
362364
private <T> DynamicLabels determineDynamicLabels(T entityToBeSaved, Neo4jPersistentEntity<?> entityMetaData) {
363365
return entityMetaData.getDynamicLabelsProperty().map(p -> {
364366

365-
PersistentPropertyAccessor propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
367+
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(entityToBeSaved);
366368
Neo4jClient.RunnableSpecTightToDatabase runnableQuery = neo4jClient
367369
.query(() -> renderer.render(cypherGenerator.createStatementReturningDynamicLabels(entityMetaData)))
368370
.bind(propertyAccessor.getProperty(entityMetaData.getRequiredIdProperty()))
@@ -410,14 +412,14 @@ private <T> List<T> saveAllImpl(Iterable<T> instances, @Nullable List<PropertyDe
410412
}
411413

412414
class Tuple3<T> {
413-
T t1;
414-
boolean t2;
415-
T t3;
416-
417-
Tuple3(T t1, boolean t2, T t3) {
418-
this.t1 = t1;
419-
this.t2 = t2;
420-
this.t3 = t3;
415+
final T originalInstance;
416+
final boolean wasNew;
417+
final T modifiedInstance;
418+
419+
Tuple3(T originalInstance, boolean wasNew, T modifiedInstance) {
420+
this.originalInstance = originalInstance;
421+
this.wasNew = wasNew;
422+
this.modifiedInstance = modifiedInstance;
421423
}
422424
}
423425

@@ -427,7 +429,7 @@ class Tuple3<T> {
427429

428430
// Save roots
429431
Function<T, Map<String, Object>> binderFunction = neo4jMappingContext.getRequiredBinderFunctionFor(domainClass);
430-
List<Map<String, Object>> entityList = entitiesToBeSaved.stream().map(h -> h.t3).map(binderFunction)
432+
List<Map<String, Object>> entityList = entitiesToBeSaved.stream().map(h -> h.modifiedInstance).map(binderFunction)
431433
.collect(Collectors.toList());
432434
ResultSummary resultSummary = neo4jClient
433435
.query(() -> renderer.render(cypherGenerator.prepareSaveOfMultipleInstancesOf(entityMetaData)))
@@ -441,8 +443,8 @@ class Tuple3<T> {
441443

442444
// Save related
443445
return entitiesToBeSaved.stream().map(t -> {
444-
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(t.t3);
445-
return processRelations(entityMetaData, t.t1, propertyAccessor, t.t2, TemplateSupport.computeIncludePropertyPredicate(includedProperties));
446+
PersistentPropertyAccessor<T> propertyAccessor = entityMetaData.getPropertyAccessor(t.modifiedInstance);
447+
return processRelations(entityMetaData, t.originalInstance, propertyAccessor, t.wasNew, TemplateSupport.computeIncludePropertyPredicate(includedProperties));
446448
}).collect(Collectors.toList());
447449
}
448450

@@ -848,21 +850,21 @@ private Optional<Neo4jClient.RecordFetchSpec<T>> createFetchSpec() {
848850
String cypherQuery = queryFragmentsAndParameters.getCypherQuery();
849851
Map<String, Object> finalParameters = queryFragmentsAndParameters.getParameters();
850852

851-
QueryFragmentsAndParameters.QueryFragments queryFragments = queryFragmentsAndParameters.getQueryFragments();
853+
QueryFragments queryFragments = queryFragmentsAndParameters.getQueryFragments();
852854
Neo4jPersistentEntity<?> entityMetaData = (Neo4jPersistentEntity<?>) queryFragmentsAndParameters.getNodeDescription();
853855

854856
boolean containsPossibleCircles = entityMetaData != null && entityMetaData.containsPossibleCircles(queryFragments::includeField);
855857
if (cypherQuery == null || containsPossibleCircles) {
856858

857859
if (containsPossibleCircles && !queryFragments.isScalarValueReturn()) {
858-
GenericQueryAndParameters genericQueryAndParameters =
859-
createQueryAndParameters(entityMetaData, queryFragments, queryFragmentsAndParameters.getParameters());
860+
NodesAndRelationshipsByIdStatementProvider nodesAndRelationshipsById =
861+
createNodesAndRelationshipsByIdStatementProvider(entityMetaData, queryFragments, queryFragmentsAndParameters.getParameters());
860862

861-
if (genericQueryAndParameters.isEmpty()) {
863+
if (nodesAndRelationshipsById.hasRootNodeIds()) {
862864
return Optional.empty();
863865
}
864-
cypherQuery = renderer.render(queryFragments.generateGenericStatement());
865-
finalParameters = genericQueryAndParameters.getParameters();
866+
cypherQuery = renderer.render(nodesAndRelationshipsById.toStatement());
867+
finalParameters = nodesAndRelationshipsById.getParameters();
866868
} else {
867869
Statement statement = queryFragments.toStatement();
868870
cypherQuery = renderer.render(statement);
@@ -876,8 +878,8 @@ private Optional<Neo4jClient.RecordFetchSpec<T>> createFetchSpec() {
876878
.map(f -> newMappingSpec.mappedBy(f)).orElse(newMappingSpec));
877879
}
878880

879-
private GenericQueryAndParameters createQueryAndParameters(Neo4jPersistentEntity<?> entityMetaData,
880-
QueryFragmentsAndParameters.QueryFragments queryFragments, Map<String, Object> parameters) {
881+
private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsByIdStatementProvider(Neo4jPersistentEntity<?> entityMetaData,
882+
QueryFragments queryFragments, Map<String, Object> parameters) {
881883

882884
// first check if the root node(s) exist(s) at all
883885
Statement rootNodesStatement = cypherGenerator
@@ -896,7 +898,7 @@ private GenericQueryAndParameters createQueryAndParameters(Neo4jPersistentEntity
896898

897899
if (rootNodeIds.isEmpty()) {
898900
// fast return if no matching root node(s) are found
899-
return GenericQueryAndParameters.EMPTY;
901+
return NodesAndRelationshipsByIdStatementProvider.EMPTY;
900902
}
901903
// load first level relationships
902904
final Set<Long> relationshipIds = new HashSet<>();
@@ -917,7 +919,7 @@ private GenericQueryAndParameters createQueryAndParameters(Neo4jPersistentEntity
917919
.ifPresent(iterateAndMapNextLevel(relationshipIds, relatedNodeIds, relationshipDescription));
918920
}
919921

920-
return new GenericQueryAndParameters(rootNodeIds, relationshipIds, relatedNodeIds);
922+
return new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, relationshipIds, relatedNodeIds, queryFragments);
921923
}
922924

923925
private void iterateNextLevel(Collection<Long> nodeIds, Neo4jPersistentEntity<?> target, Set<Long> relationshipIds,

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

+18-19
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.springframework.data.mapping.AssociationHandler;
5959
import org.springframework.data.mapping.PersistentPropertyAccessor;
6060
import org.springframework.data.mapping.callback.ReactiveEntityCallbacks;
61+
import org.springframework.data.neo4j.core.TemplateSupport.NodesAndRelationshipsByIdStatementProvider;
6162
import org.springframework.data.neo4j.core.mapping.Constants;
6263
import org.springframework.data.neo4j.core.mapping.CreateRelationshipStatementHolder;
6364
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
@@ -71,6 +72,7 @@
7172
import org.springframework.data.neo4j.core.mapping.NodeDescription;
7273
import org.springframework.data.neo4j.core.mapping.RelationshipDescription;
7374
import org.springframework.data.neo4j.core.mapping.callback.ReactiveEventSupport;
75+
import org.springframework.data.neo4j.repository.query.QueryFragments;
7476
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
7577
import org.springframework.data.projection.ProjectionFactory;
7678
import org.springframework.data.projection.ProjectionInformation;
@@ -547,21 +549,21 @@ private <T> Mono<ExecutableQuery<T>> createExecutableQuery(Class<T> domainType,
547549
QueryFragmentsAndParameters queryFragmentsAndParameters) {
548550

549551
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(domainType);
550-
QueryFragmentsAndParameters.QueryFragments queryFragments = queryFragmentsAndParameters.getQueryFragments();
552+
QueryFragments queryFragments = queryFragmentsAndParameters.getQueryFragments();
551553

552554
boolean containsPossibleCircles = entityMetaData != null && entityMetaData.containsPossibleCircles(queryFragments::includeField);
553555
if (containsPossibleCircles && !queryFragments.isScalarValueReturn()) {
554-
return createQueryAndParameters(entityMetaData, queryFragments, queryFragmentsAndParameters.getParameters())
556+
return createNodesAndRelationshipsByIdStatementProvider(entityMetaData, queryFragments, queryFragmentsAndParameters.getParameters())
555557
.flatMap(finalQueryAndParameters ->
556-
createExecutableQuery(domainType, renderer.render(queryFragments.generateGenericStatement()),
558+
createExecutableQuery(domainType, renderer.render(finalQueryAndParameters.toStatement()),
557559
finalQueryAndParameters.getParameters()));
558560
}
559561

560562
return createExecutableQuery(domainType, queryFragments.toStatement(), queryFragmentsAndParameters.getParameters());
561563
}
562564

563-
private Mono<GenericQueryAndParameters> createQueryAndParameters(Neo4jPersistentEntity<?> entityMetaData,
564-
QueryFragmentsAndParameters.QueryFragments queryFragments, Map<String, Object> parameters) {
565+
private Mono<NodesAndRelationshipsByIdStatementProvider> createNodesAndRelationshipsByIdStatementProvider(Neo4jPersistentEntity<?> entityMetaData,
566+
QueryFragments queryFragments, Map<String, Object> parameters) {
565567

566568
return Mono.deferContextual(ctx -> {
567569
Set<Long> rootNodeIds = ctx.get("rootNodes");
@@ -590,16 +592,12 @@ private Mono<GenericQueryAndParameters> createQueryAndParameters(Neo4jPersistent
590592
})
591593
.expand(iterateAndMapNextLevel(relationshipDescription));
592594
})
593-
.collect(GenericQueryAndParameters::new, (genericQueryAndParameters, _not_used2) ->
594-
genericQueryAndParameters.with(rootNodeIds, processedRelationshipIds, processedNodeIds)
595-
);
595+
.then(Mono.fromSupplier(() -> new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, processedRelationshipIds, processedNodeIds, queryFragments)));
596596
})
597-
.contextWrite(ctx -> {
598-
return ctx
599-
.put("rootNodes", ConcurrentHashMap.newKeySet())
600-
.put("processedNodes", ConcurrentHashMap.newKeySet())
601-
.put("processedRelationships", ConcurrentHashMap.newKeySet());
602-
});
597+
.contextWrite(ctx -> ctx
598+
.put("rootNodes", ConcurrentHashMap.newKeySet())
599+
.put("processedNodes", ConcurrentHashMap.newKeySet())
600+
.put("processedRelationships", ConcurrentHashMap.newKeySet()));
603601

604602
}
605603

@@ -886,17 +884,18 @@ public <T> Mono<ExecutableQuery<T>> toExecutableQuery(PreparedQuery<T> preparedQ
886884
String cypherQuery = queryFragmentsAndParameters.getCypherQuery();
887885
Map<String, Object> finalParameters = queryFragmentsAndParameters.getParameters();
888886

889-
QueryFragmentsAndParameters.QueryFragments queryFragments = queryFragmentsAndParameters.getQueryFragments();
887+
QueryFragments queryFragments = queryFragmentsAndParameters.getQueryFragments();
890888
Neo4jPersistentEntity<?> entityMetaData = (Neo4jPersistentEntity<?>) queryFragmentsAndParameters.getNodeDescription();
891889

892890
boolean containsPossibleCircles = entityMetaData != null && entityMetaData.containsPossibleCircles(queryFragments::includeField);
893891
if (cypherQuery == null || containsPossibleCircles) {
894892

895893
if (containsPossibleCircles && !queryFragments.isScalarValueReturn()) {
896-
return createQueryAndParameters(entityMetaData, queryFragments, finalParameters)
897-
.map(genericQueryAndParameters -> {
898-
ReactiveNeo4jClient.MappingSpec<T> mappingSpec = this.neo4jClient.query(renderer.render(queryFragments.generateGenericStatement()))
899-
.bindAll(genericQueryAndParameters.getParameters()).fetchAs(resultType);
894+
return createNodesAndRelationshipsByIdStatementProvider(entityMetaData, queryFragments, finalParameters)
895+
.map(nodesAndRelationshipsById -> {
896+
ReactiveNeo4jClient.MappingSpec<T> mappingSpec = this.neo4jClient.query(renderer.render(
897+
nodesAndRelationshipsById.toStatement()))
898+
.bindAll(nodesAndRelationshipsById.getParameters()).fetchAs(resultType);
900899

901900
ReactiveNeo4jClient.RecordFetchSpec<T> fetchSpec = preparedQuery.getOptionalMappingFunction()
902901
.map(mappingFunction -> mappingSpec.mappedBy(mappingFunction)).orElse(mappingSpec);

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

+69
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
import java.beans.PropertyDescriptor;
1919
import java.util.Arrays;
20+
import java.util.Collection;
21+
import java.util.Collections;
2022
import java.util.HashMap;
2123
import java.util.HashSet;
2224
import java.util.List;
@@ -27,7 +29,13 @@
2729
import java.util.stream.StreamSupport;
2830

2931
import org.apiguardian.api.API;
32+
import org.neo4j.cypherdsl.core.Cypher;
33+
import org.neo4j.cypherdsl.core.Functions;
34+
import org.neo4j.cypherdsl.core.Node;
35+
import org.neo4j.cypherdsl.core.Relationship;
3036
import org.neo4j.cypherdsl.core.Statement;
37+
import org.springframework.data.neo4j.core.mapping.Constants;
38+
import org.springframework.data.neo4j.repository.query.QueryFragments;
3139
import org.springframework.lang.Nullable;
3240

3341
/**
@@ -119,6 +127,67 @@ static Map<String, Object> mergeParameters(Statement statement, @Nullable Map<St
119127
return mergedParameters;
120128
}
121129

130+
/**
131+
* Parameter holder class for a query with the return pattern of `rootNodes, relationships, relatedNodes`.
132+
* The parameter values must be internal node or relationship ids.
133+
*/
134+
static final class NodesAndRelationshipsByIdStatementProvider {
135+
136+
private final static String ROOT_NODE_IDS = "rootNodeIds";
137+
private final static String RELATIONSHIP_IDS = "relationshipIds";
138+
private final static String RELATED_NODE_IDS = "relatedNodeIds";
139+
140+
final static NodesAndRelationshipsByIdStatementProvider EMPTY =
141+
new NodesAndRelationshipsByIdStatementProvider(Collections.emptySet(), Collections.emptySet(), Collections.emptySet(), new QueryFragments());
142+
143+
private final Map<String, Collection<Long>> parameters = new HashMap<>(3);
144+
private final QueryFragments queryFragments;
145+
146+
NodesAndRelationshipsByIdStatementProvider(Collection<Long> rootNodeIds, Collection<Long> relationshipsIds, Collection<Long> relatedNodeIds, QueryFragments queryFragments) {
147+
148+
this.parameters.put(ROOT_NODE_IDS, rootNodeIds);
149+
this.parameters.put(RELATIONSHIP_IDS, relationshipsIds);
150+
this.parameters.put(RELATED_NODE_IDS, relatedNodeIds);
151+
this.queryFragments = queryFragments;
152+
}
153+
154+
Map<String, Object> getParameters() {
155+
return Collections.unmodifiableMap(parameters);
156+
}
157+
158+
boolean hasRootNodeIds() {
159+
return parameters.get(ROOT_NODE_IDS).isEmpty();
160+
}
161+
162+
Statement toStatement() {
163+
164+
String rootNodeIds = "rootNodeIds";
165+
String relationshipIds = "relationshipIds";
166+
String relatedNodeIds = "relatedNodeIds";
167+
Node rootNodes = Cypher.anyNode(rootNodeIds);
168+
Node relatedNodes = Cypher.anyNode(relatedNodeIds);
169+
Relationship relationships = Cypher.anyNode().relationshipBetween(Cypher.anyNode()).named(relationshipIds);
170+
return Cypher.match(rootNodes)
171+
.where(Functions.id(rootNodes).in(Cypher.parameter(rootNodeIds)))
172+
.optionalMatch(relationships)
173+
.where(Functions.id(relationships).in(Cypher.parameter(relationshipIds)))
174+
.optionalMatch(relatedNodes)
175+
.where(Functions.id(relatedNodes).in(Cypher.parameter(relatedNodeIds)))
176+
.with(
177+
rootNodes.as(Constants.NAME_OF_ROOT_NODE.getValue()),
178+
Functions.collectDistinct(relationships).as(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
179+
Functions.collectDistinct(relatedNodes).as(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES))
180+
.orderBy(queryFragments.getOrderBy())
181+
.returning(
182+
Constants.NAME_OF_ROOT_NODE.as(Constants.NAME_OF_SYNTHESIZED_ROOT_NODE),
183+
Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATIONS),
184+
Cypher.name(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)
185+
)
186+
.skip(queryFragments.getSkip())
187+
.limit(queryFragments.getLimit()).build();
188+
}
189+
}
190+
122191
private TemplateSupport() {
123192
}
124193
}

‎src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public void write(Object source, Map<String, Object> parameters) {
175175
Neo4jPersistentEntity<?> nodeDescription = (Neo4jPersistentEntity<?>) nodeDescriptionStore
176176
.getNodeDescription(source.getClass());
177177

178-
PersistentPropertyAccessor propertyAccessor = nodeDescription.getPropertyAccessor(source);
178+
PersistentPropertyAccessor<Object> propertyAccessor = nodeDescription.getPropertyAccessor(source);
179179
nodeDescription.doWithProperties((Neo4jPersistentProperty p) -> {
180180

181181
// Skip the internal properties, we don't want them to end up stored as properties

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -256,16 +256,16 @@ protected Condition or(Condition base, Condition condition) {
256256
@Override
257257
protected QueryFragmentsAndParameters complete(@Nullable Condition condition, Sort sort) {
258258

259-
QueryFragmentsAndParameters.QueryFragments queryFragments = createQueryFragments(condition, sort);
259+
QueryFragments queryFragments = createQueryFragments(condition, sort);
260260

261261
Map<String, Object> convertedParameters = this.boundedParameters.stream()
262262
.collect(Collectors.toMap(p -> p.nameOrIndex, p -> parameterConversion.apply(p.value, p.conversionOverride)));
263263
return new QueryFragmentsAndParameters(nodeDescription, queryFragments, convertedParameters);
264264
}
265265

266266
@NonNull
267-
private QueryFragmentsAndParameters.QueryFragments createQueryFragments(@Nullable Condition condition, Sort sort) {
268-
QueryFragmentsAndParameters.QueryFragments queryFragments = new QueryFragmentsAndParameters.QueryFragments();
267+
private QueryFragments createQueryFragments(@Nullable Condition condition, Sort sort) {
268+
QueryFragments queryFragments = new QueryFragments();
269269

270270
// all the ways we could query for
271271
Node startNode = Cypher.node(nodeDescription.getPrimaryLabel(), nodeDescription.getAdditionalLabels())

0 commit comments

Comments
 (0)