Skip to content

Commit 8e44e10

Browse files
committed
GH-2858 - Fix following paths in projections.
If an entity has already been loaded by any relationship, it gets marked as processed. But this is not a valid state if there are multiple relationships to this entity and it is loaded via different projections for each relationship. In those cases SDN will just stop to find other relationships. This commit fixes this behaviour by also taking the relationship the entity got loaded with into account.
1 parent 07b64cb commit 8e44e10

File tree

2 files changed

+47
-32
lines changed

2 files changed

+47
-32
lines changed

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

+26-16
Original file line numberDiff line numberDiff line change
@@ -1195,8 +1195,8 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy
11951195
return NodesAndRelationshipsByIdStatementProvider.EMPTY;
11961196
}
11971197
// load first level relationships
1198-
final Set<String> relationshipIds = new HashSet<>();
1199-
final Set<String> relatedNodeIds = new HashSet<>();
1198+
// final Set<String> relationshipIds = new HashSet<>();
1199+
final Map<String, Set<String>> relationshipsToRelatedNodeIds = new HashMap<>();
12001200

12011201
for (RelationshipDescription relationshipDescription : entityMetaData.getRelationshipsInHierarchy(queryFragments::includeField)) {
12021202

@@ -1210,14 +1210,14 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy
12101210
.bindAll(usedParameters)
12111211
.fetch()
12121212
.one()
1213-
.ifPresent(iterateAndMapNextLevel(relationshipIds, relatedNodeIds, relationshipDescription, PropertyPathWalkStep.empty()));
1213+
.ifPresent(iterateAndMapNextLevel(relationshipsToRelatedNodeIds, relationshipDescription, PropertyPathWalkStep.empty()));
12141214
}
12151215

1216-
return new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, relationshipIds, relatedNodeIds, queryFragments, elementIdOrIdFunction);
1216+
return new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, relationshipsToRelatedNodeIds.keySet(), relationshipsToRelatedNodeIds.values().stream().flatMap(Collection::stream).toList(), queryFragments, elementIdOrIdFunction);
12171217
}
12181218

1219-
private void iterateNextLevel(Collection<String> nodeIds, RelationshipDescription sourceRelationshipDescription, Set<String> relationshipIds,
1220-
Set<String> relatedNodeIds, PropertyPathWalkStep currentPathStep) {
1219+
private void iterateNextLevel(Collection<String> nodeIds, RelationshipDescription sourceRelationshipDescription,
1220+
Map<String, Set<String>> relationshipsToRelatedNodes, PropertyPathWalkStep currentPathStep) {
12211221

12221222
Neo4jPersistentEntity<?> target = (Neo4jPersistentEntity<?>) sourceRelationshipDescription.getTarget();
12231223

@@ -1251,32 +1251,42 @@ private void iterateNextLevel(Collection<String> nodeIds, RelationshipDescriptio
12511251
.bindAll(Collections.singletonMap(Constants.NAME_OF_IDS, TemplateSupport.convertToLongIdOrStringElementId(nodeIds)))
12521252
.fetch()
12531253
.one()
1254-
.ifPresent(iterateAndMapNextLevel(relationshipIds, relatedNodeIds, relationshipDescription, nextPathStep));
1254+
.ifPresent(iterateAndMapNextLevel(relationshipsToRelatedNodes, relationshipDescription, nextPathStep));
12551255
}
12561256
}
12571257

12581258
@NonNull
1259-
private Consumer<Map<String, Object>> iterateAndMapNextLevel(Set<String> relationshipIds,
1260-
Set<String> relatedNodeIds,
1259+
private Consumer<Map<String, Object>> iterateAndMapNextLevel(Map<String, Set<String>> relationshipsToRelatedNodes,
12611260
RelationshipDescription relationshipDescription,
12621261
PropertyPathWalkStep currentPathStep) {
12631262

12641263
return record -> {
1264+
1265+
Map<String, Set<String>> relatedNodesVisited = new HashMap<>(relationshipsToRelatedNodes);
12651266
@SuppressWarnings("unchecked")
12661267
List<String> newRelationshipIds = ((List<Object>) record.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS)).stream().map(TemplateSupport::convertIdOrElementIdToString).toList();
1267-
relationshipIds.addAll(newRelationshipIds);
1268-
12691268
@SuppressWarnings("unchecked")
1270-
List<String> newRelatedNodeIds = ((List<Object>) record.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)).stream().map(TemplateSupport::convertIdOrElementIdToString).toList();
1269+
Set<String> relatedIds = new HashSet<>(((List<Object>) record.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)).stream().map(TemplateSupport::convertIdOrElementIdToString).toList());
12711270

1272-
Set<String> relatedIds = new HashSet<>(newRelatedNodeIds);
12731271
// use this list to get down the road
12741272
// 1. remove already visited ones;
1275-
relatedIds.removeAll(relatedNodeIds);
1276-
relatedNodeIds.addAll(relatedIds);
1273+
// we don't know which id came with which node, so we need to assume that a relationshipId connects to all related nodes
1274+
for (String newRelationshipId : newRelationshipIds) {
1275+
relatedNodesVisited.put(newRelationshipId, relatedIds);
1276+
Set<String> knownRelatedNodesBefore = relationshipsToRelatedNodes.get(newRelationshipId);
1277+
if (knownRelatedNodesBefore != null) {
1278+
Set<String> mergedKnownRelatedNodes = new HashSet<>(knownRelatedNodesBefore);
1279+
// there are already existing nodes in there for this relationship
1280+
mergedKnownRelatedNodes.addAll(relatedIds);
1281+
relatedNodesVisited.put(newRelationshipId, mergedKnownRelatedNodes);
1282+
relatedIds.removeAll(knownRelatedNodesBefore);
1283+
}
1284+
}
1285+
1286+
relationshipsToRelatedNodes.putAll(relatedNodesVisited);
12771287
// 2. for the rest start the exploration
12781288
if (!relatedIds.isEmpty()) {
1279-
iterateNextLevel(relatedIds, relationshipDescription, relationshipIds, relatedNodeIds, currentPathStep);
1289+
iterateNextLevel(relatedIds, relationshipDescription, relationshipsToRelatedNodes, currentPathStep);
12801290
}
12811291
};
12821292
}

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

+21-16
Original file line numberDiff line numberDiff line change
@@ -720,8 +720,7 @@ private Mono<NodesAndRelationshipsByIdStatementProvider> createNodesAndRelations
720720
Class<?> rootClass = entityMetaData.getUnderlyingClass();
721721

722722
Set<String> rootNodeIds = ctx.get("rootNodes");
723-
Set<String> processedRelationshipIds = ctx.get("processedRelationships");
724-
Set<String> processedNodeIds = ctx.get("processedNodes");
723+
Map<String, Set<String>> relationshipsToRelatedNodeIds = ctx.get("relationshipsToRelatedNodeIds");
725724
return Flux.fromIterable(entityMetaData.getRelationshipsInHierarchy(queryFragments::includeField))
726725
.concatMap(relationshipDescription -> {
727726

@@ -748,12 +747,11 @@ private Mono<NodesAndRelationshipsByIdStatementProvider> createNodesAndRelations
748747
})
749748
.expand(iterateAndMapNextLevel(relationshipDescription, queryFragments, rootClass, PropertyPathWalkStep.empty()));
750749
})
751-
.then(Mono.fromSupplier(() -> new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, processedRelationshipIds, processedNodeIds, queryFragments, elementIdOrIdFunction)));
750+
.then(Mono.fromSupplier(() -> new NodesAndRelationshipsByIdStatementProvider(rootNodeIds, relationshipsToRelatedNodeIds.keySet(), relationshipsToRelatedNodeIds.values().stream().flatMap(Collection::stream).toList(), queryFragments, elementIdOrIdFunction)));
752751
})
753752
.contextWrite(ctx -> ctx
754753
.put("rootNodes", ConcurrentHashMap.newKeySet())
755-
.put("processedNodes", ConcurrentHashMap.newKeySet())
756-
.put("processedRelationships", ConcurrentHashMap.newKeySet()));
754+
.put("relationshipsToRelatedNodeIds", new ConcurrentHashMap<>()));
757755

758756
}
759757

@@ -812,22 +810,29 @@ Publisher<Tuple2<Collection<String>, Collection<String>>>> iterateAndMapNextLeve
812810

813811
return newRelationshipAndRelatedNodeIds ->
814812
Flux.deferContextual(ctx -> {
815-
Set<String> relationshipIds = ctx.get("processedRelationships");
816-
Set<String> processedNodeIds = ctx.get("processedNodes");
813+
Map<String, Set<String>> relationshipsToRelatedNodeIds = ctx.get("relationshipsToRelatedNodeIds");
814+
Map<String, Set<String>> relatedNodesVisited = new HashMap<>(relationshipsToRelatedNodeIds);
817815

818816
Collection<String> newRelationshipIds = newRelationshipAndRelatedNodeIds.getT1();
819-
Set<String> tmpProcessedRels = ConcurrentHashMap.newKeySet(newRelationshipIds.size());
820-
tmpProcessedRels.addAll(newRelationshipIds);
821-
tmpProcessedRels.removeAll(relationshipIds);
822-
relationshipIds.addAll(newRelationshipIds);
823817

824818
Collection<String> newRelatedNodeIds = newRelationshipAndRelatedNodeIds.getT2();
825-
Set<String> tmpProcessedNodes = ConcurrentHashMap.newKeySet(newRelatedNodeIds.size());
826-
tmpProcessedNodes.addAll(newRelatedNodeIds);
827-
tmpProcessedNodes.removeAll(processedNodeIds);
828-
processedNodeIds.addAll(newRelatedNodeIds);
819+
Set<String> relatedIds = ConcurrentHashMap.newKeySet(newRelatedNodeIds.size());
820+
relatedIds.addAll(newRelatedNodeIds);
821+
822+
for (String newRelationshipId : newRelationshipIds) {
823+
relatedNodesVisited.put(newRelationshipId, relatedIds);
824+
Set<String> knownRelatedNodesBefore = relationshipsToRelatedNodeIds.get(newRelationshipId);
825+
if (knownRelatedNodesBefore != null) {
826+
Set<String> mergedKnownRelatedNodes = new HashSet<>(knownRelatedNodesBefore);
827+
// there are already existing nodes in there for this relationship
828+
mergedKnownRelatedNodes.addAll(relatedIds);
829+
relatedNodesVisited.put(newRelationshipId, mergedKnownRelatedNodes);
830+
relatedIds.removeAll(knownRelatedNodesBefore);
831+
}
832+
}
833+
relationshipsToRelatedNodeIds.putAll(relatedNodesVisited);
829834

830-
if (tmpProcessedRels.isEmpty() && tmpProcessedNodes.isEmpty()) {
835+
if (relatedIds.isEmpty()) {
831836
return Mono.empty();
832837
}
833838

0 commit comments

Comments
 (0)