Skip to content

Commit caea3f1

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 9a6a408 commit caea3f1

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
@@ -1202,8 +1202,8 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy
12021202
return NodesAndRelationshipsByIdStatementProvider.EMPTY;
12031203
}
12041204
// load first level relationships
1205-
final Set<String> relationshipIds = new HashSet<>();
1206-
final Set<String> relatedNodeIds = new HashSet<>();
1205+
// final Set<String> relationshipIds = new HashSet<>();
1206+
final Map<String, Set<String>> relationshipsToRelatedNodeIds = new HashMap<>();
12071207

12081208
for (RelationshipDescription relationshipDescription : entityMetaData.getRelationshipsInHierarchy(queryFragments::includeField)) {
12091209

@@ -1217,14 +1217,14 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy
12171217
.bindAll(usedParameters)
12181218
.fetch()
12191219
.one()
1220-
.ifPresent(iterateAndMapNextLevel(relationshipIds, relatedNodeIds, relationshipDescription, PropertyPathWalkStep.empty()));
1220+
.ifPresent(iterateAndMapNextLevel(relationshipsToRelatedNodeIds, relationshipDescription, PropertyPathWalkStep.empty()));
12211221
}
12221222

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

1226-
private void iterateNextLevel(Collection<String> nodeIds, RelationshipDescription sourceRelationshipDescription, Set<String> relationshipIds,
1227-
Set<String> relatedNodeIds, PropertyPathWalkStep currentPathStep) {
1226+
private void iterateNextLevel(Collection<String> nodeIds, RelationshipDescription sourceRelationshipDescription,
1227+
Map<String, Set<String>> relationshipsToRelatedNodes, PropertyPathWalkStep currentPathStep) {
12281228

12291229
Neo4jPersistentEntity<?> target = (Neo4jPersistentEntity<?>) sourceRelationshipDescription.getTarget();
12301230

@@ -1258,32 +1258,42 @@ private void iterateNextLevel(Collection<String> nodeIds, RelationshipDescriptio
12581258
.bindAll(Collections.singletonMap(Constants.NAME_OF_IDS, TemplateSupport.convertToLongIdOrStringElementId(nodeIds)))
12591259
.fetch()
12601260
.one()
1261-
.ifPresent(iterateAndMapNextLevel(relationshipIds, relatedNodeIds, relationshipDescription, nextPathStep));
1261+
.ifPresent(iterateAndMapNextLevel(relationshipsToRelatedNodes, relationshipDescription, nextPathStep));
12621262
}
12631263
}
12641264

12651265
@NonNull
1266-
private Consumer<Map<String, Object>> iterateAndMapNextLevel(Set<String> relationshipIds,
1267-
Set<String> relatedNodeIds,
1266+
private Consumer<Map<String, Object>> iterateAndMapNextLevel(Map<String, Set<String>> relationshipsToRelatedNodes,
12681267
RelationshipDescription relationshipDescription,
12691268
PropertyPathWalkStep currentPathStep) {
12701269

12711270
return record -> {
1271+
1272+
Map<String, Set<String>> relatedNodesVisited = new HashMap<>(relationshipsToRelatedNodes);
12721273
@SuppressWarnings("unchecked")
12731274
List<String> newRelationshipIds = ((List<Object>) record.get(Constants.NAME_OF_SYNTHESIZED_RELATIONS)).stream().map(TemplateSupport::convertIdOrElementIdToString).toList();
1274-
relationshipIds.addAll(newRelationshipIds);
1275-
12761275
@SuppressWarnings("unchecked")
1277-
List<String> newRelatedNodeIds = ((List<Object>) record.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)).stream().map(TemplateSupport::convertIdOrElementIdToString).toList();
1276+
Set<String> relatedIds = new HashSet<>(((List<Object>) record.get(Constants.NAME_OF_SYNTHESIZED_RELATED_NODES)).stream().map(TemplateSupport::convertIdOrElementIdToString).toList());
12781277

1279-
Set<String> relatedIds = new HashSet<>(newRelatedNodeIds);
12801278
// use this list to get down the road
12811279
// 1. remove already visited ones;
1282-
relatedIds.removeAll(relatedNodeIds);
1283-
relatedNodeIds.addAll(relatedIds);
1280+
// we don't know which id came with which node, so we need to assume that a relationshipId connects to all related nodes
1281+
for (String newRelationshipId : newRelationshipIds) {
1282+
relatedNodesVisited.put(newRelationshipId, relatedIds);
1283+
Set<String> knownRelatedNodesBefore = relationshipsToRelatedNodes.get(newRelationshipId);
1284+
if (knownRelatedNodesBefore != null) {
1285+
Set<String> mergedKnownRelatedNodes = new HashSet<>(knownRelatedNodesBefore);
1286+
// there are already existing nodes in there for this relationship
1287+
mergedKnownRelatedNodes.addAll(relatedIds);
1288+
relatedNodesVisited.put(newRelationshipId, mergedKnownRelatedNodes);
1289+
relatedIds.removeAll(knownRelatedNodesBefore);
1290+
}
1291+
}
1292+
1293+
relationshipsToRelatedNodes.putAll(relatedNodesVisited);
12841294
// 2. for the rest start the exploration
12851295
if (!relatedIds.isEmpty()) {
1286-
iterateNextLevel(relatedIds, relationshipDescription, relationshipIds, relatedNodeIds, currentPathStep);
1296+
iterateNextLevel(relatedIds, relationshipDescription, relationshipsToRelatedNodes, currentPathStep);
12871297
}
12881298
};
12891299
}

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)