Skip to content

Commit 9d6acf9

Browse files
meistermeiermeistermeier
committed
GH-2600 - Fix relproperties and simple relationship hydration.
Misses dynamic relationships right now. Closes #2600
1 parent 590f748 commit 9d6acf9

File tree

1 file changed

+128
-38
lines changed

1 file changed

+128
-38
lines changed

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

Lines changed: 128 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ private <R> MapAccessor determineQueryRoot(MapAccessor mapAccessor, @Nullable Ne
144144
Node node = value.asNode();
145145
if (primaryLabels.stream().anyMatch(node::hasLabel)) { // it has a matching label
146146
// We haven't seen this node yet, so we take it
147-
if (knownObjects.getObject(node.id()) == null) {
147+
if (knownObjects.getObject("N" + node.id()) == null) {
148148
matchingNodes.add(node);
149149
} else {
150150
seenMatchingNodes.add(node);
@@ -263,22 +263,23 @@ private static MapAccessor mergeRootNodeWithRecord(Node node, MapAccessor record
263263
private <ET> ET map(MapAccessor queryResult, MapAccessor allValues, Neo4jPersistentEntity<ET> nodeDescription) {
264264
Collection<Relationship> relationshipsFromResult = extractRelationships(allValues);
265265
Collection<Node> nodesFromResult = extractNodes(allValues);
266-
return map(queryResult, nodeDescription, null, relationshipsFromResult, nodesFromResult);
266+
return map(queryResult, nodeDescription, null, null, relationshipsFromResult, nodesFromResult);
267267
}
268268

269269
private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescription,
270-
@Nullable Object lastMappedEntity, Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult) {
270+
@Nullable Object lastMappedEntity, @Nullable RelationshipDescription relationshipDescription, Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult) {
271271

272272
// if the given result does not contain an identifier to the mapped object cannot get temporarily saved
273-
Long internalId = getInternalId(queryResult);
273+
String direction = relationshipDescription != null ? relationshipDescription.getDirection().name() : null;
274+
String internalId = getInternalId(queryResult, direction);
274275

275276
Supplier<ET> mappedObjectSupplier = () -> {
276277
if (knownObjects.isInCreation(internalId)) {
277278
throw new MappingException(
278279
String.format(
279280
"The node with id %s has a logical cyclic mapping dependency. " +
280281
"Its creation caused the creation of another node that has a reference to this.",
281-
internalId)
282+
internalId.substring(1))
282283
);
283284
}
284285
knownObjects.setInCreation(internalId);
@@ -326,7 +327,7 @@ private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescr
326327
}
327328

328329

329-
private <ET> void populateProperties(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescription, Long internalId,
330+
private <ET> void populateProperties(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescription, String internalId,
330331
ET mappedObject, @Nullable Object lastMappedEntity,
331332
Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult, boolean objectAlreadyMapped) {
332333

@@ -364,12 +365,30 @@ private <ET> void populateProperties(MapAccessor queryResult, Neo4jPersistentEnt
364365
}
365366

366367
@Nullable
367-
private Long getInternalId(@NonNull MapAccessor queryResult) {
368-
return queryResult instanceof Node
369-
? (Long) ((Node) queryResult).id()
370-
: queryResult.get(Constants.NAME_OF_INTERNAL_ID) == null || queryResult.get(Constants.NAME_OF_INTERNAL_ID).isNull()
371-
? null
372-
: queryResult.get(Constants.NAME_OF_INTERNAL_ID).asLong();
368+
private String getInternalId(@NonNull MapAccessor queryResult, @Nullable String seed) {
369+
if (queryResult instanceof Node) {
370+
return "N" + ((Node) queryResult).id();
371+
} else if (queryResult instanceof Relationship) {
372+
Relationship relationshipValue = (Relationship) queryResult;
373+
return "R" + seed + relationshipValue.id();
374+
} else if (!(queryResult.get(Constants.NAME_OF_INTERNAL_ID) == null || queryResult.get(Constants.NAME_OF_INTERNAL_ID).isNull())) {
375+
return "N" + queryResult.get(Constants.NAME_OF_INTERNAL_ID).asLong();
376+
}
377+
378+
return null;
379+
}
380+
381+
@Nullable
382+
private Long getInternalIdAsLong(@NonNull MapAccessor queryResult) {
383+
if (queryResult instanceof Node) {
384+
return ((Node) queryResult).id();
385+
} else if (queryResult instanceof Relationship) {
386+
return ((Relationship) queryResult).id();
387+
} else if (!(queryResult.get(Constants.NAME_OF_INTERNAL_ID) == null || queryResult.get(Constants.NAME_OF_INTERNAL_ID).isNull())) {
388+
return queryResult.get(Constants.NAME_OF_INTERNAL_ID).asLong();
389+
}
390+
391+
return null;
373392
}
374393

375394
@NonNull
@@ -538,6 +557,23 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
538557
boolean populatedScalarValue = !persistentProperty.isCollectionLike() && !persistentProperty.isMap()
539558
&& propertyValueNotNull;
540559

560+
if (populatedCollection) {
561+
createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, relationshipsFromResult, nodesFromResult, false)
562+
.ifPresent(value -> {
563+
Collection<?> providedCollection = (Collection<?>) value;
564+
Collection<?> existingValue = (Collection<?>) propertyValue;
565+
Collection<Object> newValue = CollectionFactory.createCollection(existingValue.getClass(), providedCollection.size() + existingValue.size());
566+
567+
RelationshipDescription relationshipDescription = (RelationshipDescription) association;
568+
Map<Object, Object> mergedValues = new HashMap<>();
569+
mergeCollections(relationshipDescription, existingValue, mergedValues);
570+
mergeCollections(relationshipDescription, providedCollection, mergedValues);
571+
572+
newValue.addAll(mergedValues.values());
573+
propertyAccessor.setProperty(persistentProperty, newValue);
574+
});
575+
}
576+
541577
boolean propertyAlreadyPopulated = populatedCollection || populatedMap || populatedScalarValue;
542578

543579
// avoid unnecessary re-assignment of values
@@ -550,9 +586,33 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
550586
};
551587
}
552588

589+
private void mergeCollections(RelationshipDescription relationshipDescription, Collection<?> values, Map<Object, Object> mergedValues) {
590+
for (Object existingValueInCollection : values) {
591+
if (relationshipDescription.hasRelationshipProperties()) {
592+
Object existingIdPropertyValue = ((Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity())
593+
.getPropertyAccessor(existingValueInCollection)
594+
.getProperty(((Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity()).getIdProperty());
595+
596+
mergedValues.put(existingIdPropertyValue, existingValueInCollection);
597+
} else if (!relationshipDescription.isDynamic()) { // should not happen because this is all inside populatedCollection (but better safe than sorry)
598+
Object existingIdPropertyValue = ((Neo4jPersistentEntity<?>) relationshipDescription.getTarget())
599+
.getPropertyAccessor(existingValueInCollection)
600+
.getProperty(((Neo4jPersistentEntity<?>) relationshipDescription.getTarget()).getIdProperty());
601+
602+
mergedValues.put(existingIdPropertyValue, existingValueInCollection);
603+
}
604+
}
605+
}
606+
607+
private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty persistentProperty, MapAccessor values,
608+
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, Collection<Relationship> relationshipsFromResult,
609+
Collection<Node> nodesFromResult) {
610+
return createInstanceOfRelationships(persistentProperty, values, relationshipDescription, baseDescription, relationshipsFromResult, nodesFromResult, true);
611+
}
612+
553613
private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty persistentProperty, MapAccessor values,
554614
RelationshipDescription relationshipDescription, NodeDescription<?> baseDescription, Collection<Relationship> relationshipsFromResult,
555-
Collection<Node> nodesFromResult) {
615+
Collection<Node> nodesFromResult, boolean fetchMore) {
556616

557617
String typeOfRelationship = relationshipDescription.getType();
558618
String targetLabel = relationshipDescription.getTarget().getPrimaryLabel();
@@ -592,7 +652,7 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
592652
List<Object> relationshipsAndProperties = new ArrayList<>();
593653

594654
if (Values.NULL.equals(list)) {
595-
Long sourceNodeId = getInternalId(values);
655+
Long sourceNodeId = getInternalIdAsLong(values);
596656

597657
Function<Relationship, Long> sourceIdSelector = relationshipDescription.isIncoming() ? Relationship::endNodeId : Relationship::startNodeId;
598658
Function<Relationship, Long> targetIdSelector = relationshipDescription.isIncoming() ? Relationship::startNodeId : Relationship::endNodeId;
@@ -619,22 +679,37 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
619679
// If this relationship got processed twice (OUTGOING, INCOMING), it is never needed again
620680
// and therefor should not be in the list.
621681
// Otherwise, for highly linked data it could potentially cause a StackOverflowError.
622-
if (knownObjects.hasProcessedRelationshipCompletely(possibleRelationship.id())) {
682+
String direction = relationshipDescription.getDirection().name();
683+
if (knownObjects.hasProcessedRelationshipCompletely("R" + direction + possibleRelationship.id())) {
623684
relationshipsFromResult.remove(possibleRelationship);
624685
}
625686
// If the target is the same(equal) node, get the related object from the cache.
626687
// Avoiding the call to the map method also breaks an endless cycle of trying to finish
627688
// the property population of _this_ object.
628689
// The initial population will happen at the end of this mapping. This is sufficient because
629690
// it only affects properties not changing the instance of the object.
630-
Object mappedObject = sourceNodeId != null && sourceNodeId.equals(targetNodeId)
631-
? knownObjects.getObject(sourceNodeId)
632-
: map(possibleValueNode, concreteTargetNodeDescription, null, relationshipsFromResult, nodesFromResult);
633-
if (relationshipDescription.hasRelationshipProperties()) {
691+
Object mappedObject;
692+
if (fetchMore) {
693+
mappedObject = sourceNodeId != null && sourceNodeId.equals(targetNodeId)
694+
? knownObjects.getObject("N" + sourceNodeId)
695+
: map(possibleValueNode, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
696+
} else {
697+
Object objectFromStore = knownObjects.getObject("N" + targetNodeId);
698+
mappedObject = objectFromStore != null
699+
? objectFromStore
700+
: map(possibleValueNode, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
701+
}
634702

635-
Object relationshipProperties = map(possibleRelationship,
636-
(Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(),
637-
mappedObject, relationshipsFromResult, nodesFromResult);
703+
if (relationshipDescription.hasRelationshipProperties()) {
704+
Object relationshipProperties;
705+
if (fetchMore) {
706+
relationshipProperties = map(possibleRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
707+
} else {
708+
Object objectFromStore = knownObjects.getObject(getInternalId(possibleRelationship, relationshipDescription.getDirection().name()));
709+
relationshipProperties = objectFromStore != null
710+
? objectFromStore
711+
: map(possibleRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), mappedObject, relationshipDescription, relationshipsFromResult, nodesFromResult);
712+
}
638713
relationshipsAndProperties.add(relationshipProperties);
639714
mappedObjectHandler.accept(possibleRelationship.type(), relationshipProperties);
640715
} else {
@@ -651,7 +726,15 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
651726
Neo4jPersistentEntity<?> concreteTargetNodeDescription =
652727
getMostConcreteTargetNodeDescription(genericTargetNodeDescription, relatedEntity);
653728

654-
Object valueEntry = map(relatedEntity, concreteTargetNodeDescription, null, relationshipsFromResult, nodesFromResult);
729+
Object valueEntry;
730+
if (fetchMore) {
731+
valueEntry = map(relatedEntity, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
732+
} else {
733+
Object objectFromStore = knownObjects.getObject(getInternalId(relatedEntity, null));
734+
valueEntry = objectFromStore != null
735+
? objectFromStore
736+
: map(relatedEntity, concreteTargetNodeDescription, null, null, relationshipsFromResult, nodesFromResult);
737+
}
655738

656739
if (relationshipDescription.hasRelationshipProperties()) {
657740
String sourceLabel = relationshipDescription.getSource().getMostAbstractParentLabel(baseDescription);
@@ -660,9 +743,16 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
660743
Relationship relatedEntityRelationship = relatedEntity.get(relationshipSymbolicName)
661744
.asRelationship();
662745

663-
Object relationshipProperties = map(relatedEntityRelationship,
664-
(Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(),
665-
valueEntry, relationshipsFromResult, nodesFromResult);
746+
Object relationshipProperties;
747+
if (fetchMore) {
748+
relationshipProperties = map(relatedEntityRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
749+
} else {
750+
Object objectFromStore = knownObjects.getObject(getInternalId(relatedEntityRelationship, relationshipDescription.getDirection().name()));
751+
relationshipProperties = objectFromStore != null
752+
? objectFromStore
753+
: map(relatedEntityRelationship, (Neo4jPersistentEntity<?>) relationshipDescription.getRelationshipPropertiesEntity(), valueEntry, relationshipDescription, relationshipsFromResult, nodesFromResult);
754+
}
755+
666756
relationshipsAndProperties.add(relationshipProperties);
667757
mappedObjectHandler.accept(relatedEntity.get(RelationshipDescription.NAME_OF_RELATIONSHIP_TYPE).asString(), relationshipProperties);
668758
} else {
@@ -779,14 +869,14 @@ static class KnownObjects {
779869
private final Lock read = lock.readLock();
780870
private final Lock write = lock.writeLock();
781871

782-
private final Map<Long, Object> internalIdStore = new HashMap<>();
783-
private final Map<Long, Boolean> internalCurrentRecord = new HashMap<>();
784-
private final Set<Long> previousRecords = new HashSet<>();
785-
private final Set<Long> idsInCreation = new HashSet<>();
872+
private final Map<String, Object> internalIdStore = new HashMap<>();
873+
private final Map<String, Boolean> internalCurrentRecord = new HashMap<>();
874+
private final Set<String> previousRecords = new HashSet<>();
875+
private final Set<String> idsInCreation = new HashSet<>();
786876

787-
private final Map<Long, Integer> processedRelationships = new HashMap<>();
877+
private final Map<String, Integer> processedRelationships = new HashMap<>();
788878

789-
private void storeObject(@Nullable Long internalId, Object object) {
879+
private void storeObject(@Nullable String internalId, Object object) {
790880
if (internalId == null) {
791881
return;
792882
}
@@ -800,7 +890,7 @@ private void storeObject(@Nullable Long internalId, Object object) {
800890
}
801891
}
802892

803-
private void setInCreation(@Nullable Long internalId) {
893+
private void setInCreation(@Nullable String internalId) {
804894
if (internalId == null) {
805895
return;
806896
}
@@ -812,7 +902,7 @@ private void setInCreation(@Nullable Long internalId) {
812902
}
813903
}
814904

815-
private boolean isInCreation(@Nullable Long internalId) {
905+
private boolean isInCreation(@Nullable String internalId) {
816906
if (internalId == null) {
817907
return false;
818908
}
@@ -825,7 +915,7 @@ private boolean isInCreation(@Nullable Long internalId) {
825915
}
826916

827917
@Nullable
828-
private Object getObject(@Nullable Long internalId) {
918+
private Object getObject(@Nullable String internalId) {
829919
if (internalId == null) {
830920
return null;
831921
}
@@ -845,7 +935,7 @@ private Object getObject(@Nullable Long internalId) {
845935
return null;
846936
}
847937

848-
private void removeFromInCreation(@Nullable Long internalId) {
938+
private void removeFromInCreation(@Nullable String internalId) {
849939
if (internalId == null) {
850940
return;
851941
}
@@ -857,7 +947,7 @@ private void removeFromInCreation(@Nullable Long internalId) {
857947
}
858948
}
859949

860-
private boolean alreadyMappedInPreviousRecord(@Nullable Long internalId) {
950+
private boolean alreadyMappedInPreviousRecord(@Nullable String internalId) {
861951
if (internalId == null) {
862952
return false;
863953
}
@@ -877,7 +967,7 @@ private boolean alreadyMappedInPreviousRecord(@Nullable Long internalId) {
877967
* It increases the process count of relationships (mapped by their ids)
878968
* AND checks if it was already processed twice (INCOMING/OUTGOING).
879969
*/
880-
private boolean hasProcessedRelationshipCompletely(Long relationshipId) {
970+
private boolean hasProcessedRelationshipCompletely(String relationshipId) {
881971
try {
882972
write.lock();
883973

0 commit comments

Comments
 (0)