Skip to content

Commit cbbf39d

Browse files
meistermeiermichael-simons
meistermeier
authored and
michael-simons
committed
GH-2289 - Improve relationship observe detection.
Closes #2289.
1 parent 89e6271 commit cbbf39d

File tree

9 files changed

+259
-20
lines changed

9 files changed

+259
-20
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,7 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Pers
607607
TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity);
608608
}
609609
stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean());
610+
stateMachine.markRelationshipAsProcessed(relatedInternalId, relationshipDescription.getRelationshipObverse());
610611

611612
if (processState != ProcessState.PROCESSED_ALL_VALUES) {
612613
processNestedRelations(targetEntity, targetPropertyAccessor, isEntityNew, inDatabase, stateMachine);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
698698
if (targetEntity.hasVersionProperty() && idAndVersion.getT2() != noVersion) {
699699
targetPropertyAccessor.setProperty(targetEntity.getVersionProperty(), idAndVersion.getT2());
700700
}
701+
stateMachine.markRelationshipAsProcessed(relatedInternalId, relationshipDescription.getRelationshipObverse());
701702

702703
Object idValue = idProperty != null
703704
? relationshipContext

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -125,32 +125,33 @@ protected Association<Neo4jPersistentProperty> createAssociation() {
125125
}
126126
}
127127

128-
Relationship outgoingRelationship = this.findAnnotation(Relationship.class);
128+
Relationship relationship = this.findAnnotation(Relationship.class);
129129

130130
String type;
131-
if (outgoingRelationship != null && StringUtils.hasText(outgoingRelationship.type())) {
132-
type = outgoingRelationship.type();
131+
if (relationship != null && StringUtils.hasText(relationship.type())) {
132+
type = relationship.type();
133133
} else {
134134
type = deriveRelationshipType(this.getName());
135135
}
136136

137-
Relationship.Direction direction = Relationship.Direction.OUTGOING;
138-
if (outgoingRelationship != null) {
139-
direction = outgoingRelationship.direction();
140-
}
137+
Relationship.Direction direction = relationship != null
138+
? relationship.direction()
139+
: Relationship.Direction.OUTGOING;
141140

142141
// Try to determine if there is a relationship definition that expresses logically the same relationship
143142
// on the other end.
144143
Optional<RelationshipDescription> obverseRelationshipDescription = obverseOwner.getRelationships().stream()
145-
.filter(rel -> rel.getType().equals(type) && rel.getTarget().equals(this.getOwner())).findFirst();
144+
.filter(rel -> rel.getType().equals(type)
145+
&& rel.getTarget().equals(this.getOwner())
146+
&& rel.getDirection() == direction.opposite()).findFirst();
146147

147148
DefaultRelationshipDescription relationshipDescription = new DefaultRelationshipDescription(this,
148149
obverseRelationshipDescription.orElse(null), type, dynamicAssociation, (NodeDescription<?>) getOwner(),
149150
this.getName(), obverseOwner, direction, relationshipPropertiesClass);
150151

151152
// Update the previous found, if any, relationship with the newly created one as its counterpart.
152153
obverseRelationshipDescription
153-
.ifPresent(relationship -> relationship.setRelationshipObverse(relationshipDescription));
154+
.ifPresent(observeRelationship -> observeRelationship.setRelationshipObverse(relationshipDescription));
154155

155156
return relationshipDescription;
156157
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,10 @@ public int hashCode() {
143143
*
144144
* @param relationshipDescription To be marked as processed
145145
*/
146-
public void markRelationshipAsProcessed(Object fromId, RelationshipDescription relationshipDescription) {
146+
public void markRelationshipAsProcessed(Object fromId, @Nullable RelationshipDescription relationshipDescription) {
147+
if (relationshipDescription == null) {
148+
return;
149+
}
147150

148151
try {
149152
write.lock();

‎src/main/java/org/springframework/data/neo4j/core/schema/Relationship.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ enum Direction {
5353
/**
5454
* Describes an incoming relationship.
5555
*/
56-
INCOMING
56+
INCOMING;
57+
58+
public Direction opposite() {
59+
return this == OUTGOING ? INCOMING : OUTGOING;
60+
}
5761
}
5862

5963
/**

‎src/test/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntityTest.java

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.junit.jupiter.api.Test;
3030
import org.junit.jupiter.params.ParameterizedTest;
3131
import org.junit.jupiter.params.provider.ValueSource;
32+
import org.springframework.data.mapping.AssociationHandler;
3233
import org.springframework.data.mapping.MappingException;
3334
import org.springframework.data.neo4j.core.schema.DynamicLabels;
3435
import org.springframework.data.neo4j.core.schema.GeneratedValue;
@@ -110,6 +111,71 @@ void doesFailOnRelationshipPropertiesWithMissingTargetNode() {
110111
.getPersistentEntity(EntityWithInCorrectRelationshipProperties.class))
111112
.withMessageContaining("Missing @TargetNode declaration in");
112113
}
114+
115+
@Test // GH-2289
116+
void correctlyFindRelationshipObverseSameEntity() {
117+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
118+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationship.class);
119+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
120+
RelationshipDescription rd = (RelationshipDescription) a;
121+
assertThat(rd.getRelationshipObverse()).isNotNull();
122+
});
123+
}
124+
125+
@Test // GH-2289
126+
void correctlyFindRelationshipObverse() {
127+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
128+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationshipToOtherEntity.class);
129+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
130+
RelationshipDescription rd = (RelationshipDescription) a;
131+
assertThat(rd.getRelationshipObverse()).isNotNull();
132+
});
133+
persistentEntity = neo4jMappingContext.getPersistentEntity(OtherEntityWithBidirectionalRelationship.class);
134+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
135+
RelationshipDescription rd = (RelationshipDescription) a;
136+
assertThat(rd.getRelationshipObverse()).isNotNull();
137+
});
138+
}
139+
140+
@Test // GH-2289
141+
void correctlyFindRelationshipObverseWithRelationshipProperties() {
142+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
143+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationshipToOtherEntityWithRelationshipProperties.class);
144+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
145+
RelationshipDescription rd = (RelationshipDescription) a;
146+
assertThat(rd.getRelationshipObverse()).isNotNull();
147+
});
148+
persistentEntity = neo4jMappingContext.getPersistentEntity(OtherEntityWithBidirectionalRelationship.class);
149+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
150+
RelationshipDescription rd = (RelationshipDescription) a;
151+
assertThat(rd.getRelationshipObverse()).isNotNull();
152+
});
153+
}
154+
155+
@Test // GH-2289
156+
void correctlyFindSameEntityRelationshipObverseWithRelationshipProperties() {
157+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
158+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationshipProperties.class);
159+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
160+
RelationshipDescription rd = (RelationshipDescription) a;
161+
assertThat(rd.getRelationshipObverse()).isNotNull();
162+
});
163+
}
164+
165+
@Test // GH-2289
166+
void correctlyDontFindRelationshipObverse() {
167+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
168+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityLooksLikeHasObserve.class);
169+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
170+
RelationshipDescription rd = (RelationshipDescription) a;
171+
assertThat(rd.getRelationshipObverse()).isNull();
172+
});
173+
persistentEntity = neo4jMappingContext.getPersistentEntity(OtherEntityLooksLikeHasObserve.class);
174+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
175+
RelationshipDescription rd = (RelationshipDescription) a;
176+
assertThat(rd.getRelationshipObverse()).isNull();
177+
});
178+
}
113179
}
114180

115181
@Nested
@@ -444,4 +510,123 @@ static class HasNoTargetNodeRelationshipProperties {
444510
@Id @GeneratedValue
445511
private Long id;
446512
}
513+
514+
@Node
515+
static class EntityWithBidirectionalRelationship {
516+
517+
@Id @GeneratedValue
518+
private Long id;
519+
520+
@Relationship("KNOWS")
521+
List<EntityWithBidirectionalRelationship> knows;
522+
523+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
524+
List<EntityWithBidirectionalRelationship> knownBy;
525+
526+
}
527+
528+
@Node
529+
static class EntityWithBidirectionalRelationshipToOtherEntity {
530+
531+
@Id @GeneratedValue
532+
private Long id;
533+
534+
@Relationship("KNOWS")
535+
List<OtherEntityWithBidirectionalRelationship> knows;
536+
537+
}
538+
539+
@Node
540+
static class OtherEntityWithBidirectionalRelationship {
541+
542+
@Id @GeneratedValue
543+
private Long id;
544+
545+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
546+
List<EntityWithBidirectionalRelationshipToOtherEntity> knownBy;
547+
548+
}
549+
550+
@Node
551+
static class EntityWithBidirectionalRelationshipToOtherEntityWithRelationshipProperties {
552+
553+
@Id @GeneratedValue
554+
private Long id;
555+
556+
@Relationship("KNOWS")
557+
List<OtherEntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties> knows;
558+
559+
}
560+
561+
@Node
562+
static class OtherEntityWithBidirectionalRelationshipWithRelationshipProperties {
563+
564+
@Id @GeneratedValue
565+
private Long id;
566+
567+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
568+
List<EntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties> knownBy;
569+
570+
}
571+
572+
@RelationshipProperties
573+
static class OtherEntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties {
574+
@Id @GeneratedValue
575+
private Long id;
576+
577+
@TargetNode
578+
OtherEntityWithBidirectionalRelationshipWithRelationshipProperties target;
579+
}
580+
581+
@RelationshipProperties
582+
static class EntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties {
583+
@Id @GeneratedValue
584+
private Long id;
585+
586+
@TargetNode
587+
EntityWithBidirectionalRelationshipToOtherEntityWithRelationshipProperties target;
588+
}
589+
590+
@Node
591+
static class EntityWithBidirectionalRelationshipProperties {
592+
593+
@Id @GeneratedValue
594+
private Long id;
595+
596+
@Relationship("KNOWS")
597+
List<BidirectionalRelationshipProperties> knows;
598+
599+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
600+
List<BidirectionalRelationshipProperties> knownBy;
601+
602+
}
603+
604+
@RelationshipProperties
605+
static class BidirectionalRelationshipProperties {
606+
607+
@Id @GeneratedValue
608+
private Long id;
609+
610+
@TargetNode
611+
EntityWithBidirectionalRelationshipProperties target;
612+
}
613+
614+
@Node
615+
static class EntityLooksLikeHasObserve {
616+
@Id @GeneratedValue
617+
private Long id;
618+
619+
@Relationship("KNOWS")
620+
private List<OtherEntityLooksLikeHasObserve> knows;
621+
}
622+
623+
@Node
624+
static class OtherEntityLooksLikeHasObserve {
625+
@Id @GeneratedValue
626+
private Long id;
627+
628+
@Relationship("KNOWS")
629+
private List<EntityLooksLikeHasObserve> knows;
630+
}
631+
447632
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2289/GH2289IT.java

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
*/
1616
package org.springframework.data.neo4j.integration.issues.gh2289;
1717

18-
import org.assertj.core.api.Assertions;
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
1920
import org.junit.jupiter.api.BeforeAll;
2021
import org.junit.jupiter.api.RepeatedTest;
2122
import org.neo4j.driver.Driver;
@@ -62,14 +63,26 @@ void testNewRelation(@Autowired SkuRepository skuRepo) {
6263
a.rangeRelationTo(d, 1, 1, RelationType.MULTIPLICATIVE);
6364
a = skuRepo.save(a);
6465

65-
Assertions.assertThat(a.getRangeRelationsOut()).hasSize(3);
66+
assertThat(a.getRangeRelationsOut()).hasSize(3);
6667
b = skuRepo.findById(b.getId()).get();
67-
Assertions.assertThat(b.getRangeRelationsIn()).hasSize(1);
68+
assertThat(b.getRangeRelationsIn()).hasSize(1);
69+
70+
assertThat(b.getRangeRelationsIn().stream().findFirst().get().getTargetSku().getRangeRelationsOut()).hasSize(3);
6871

6972
b.rangeRelationTo(c, 1, 1, RelationType.MULTIPLICATIVE);
7073
b = skuRepo.save(b);
71-
Assertions.assertThat(b.getRangeRelationsIn()).hasSize(1);
72-
Assertions.assertThat(b.getRangeRelationsOut()).hasSize(1);
74+
assertThat(b.getRangeRelationsIn()).hasSize(1);
75+
assertThat(b.getRangeRelationsOut()).hasSize(1);
76+
77+
a = skuRepo.findById(a.getId()).get();
78+
assertThat(a.getRangeRelationsOut()).hasSize(3);
79+
assertThat(a.getRangeRelationsOut()).allSatisfy(r -> {
80+
int expectedSize = 1;
81+
if ("C".equals(r.getTargetSku().getName())) {
82+
expectedSize = 2;
83+
}
84+
assertThat(r.getTargetSku().getRangeRelationsIn()).hasSize(expectedSize);
85+
});
7386
}
7487

7588
@Repository

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2289/ReactiveGH2289IT.java

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@
1515
*/
1616
package org.springframework.data.neo4j.integration.issues.gh2289;
1717

18+
import static org.assertj.core.api.Assertions.assertThat;
19+
1820
import reactor.test.StepVerifier;
1921

2022
import java.util.concurrent.atomic.AtomicLong;
2123
import java.util.concurrent.atomic.AtomicReference;
2224

23-
import org.assertj.core.api.Assertions;
2425
import org.junit.jupiter.api.BeforeAll;
2526
import org.junit.jupiter.api.RepeatedTest;
2627
import org.junit.jupiter.api.Tag;
@@ -60,6 +61,7 @@ protected static void setupData() {
6061
@RepeatedTest(23)
6162
void testNewRelation(@Autowired SkuRepository skuRepo) {
6263

64+
AtomicLong aId = new AtomicLong();
6365
AtomicLong bId = new AtomicLong();
6466
AtomicReference<Sku> cRef = new AtomicReference<>();
6567
skuRepo.save(new Sku(0L, "A"))
@@ -78,17 +80,37 @@ void testNewRelation(@Autowired SkuRepository skuRepo) {
7880
a.rangeRelationTo(d, 1, 1, RelationType.MULTIPLICATIVE);
7981
return skuRepo.save(a);
8082
}).as(StepVerifier::create)
81-
.expectNextMatches(a -> a.getRangeRelationsOut().size() == 3)
83+
.expectNextMatches(a -> {
84+
aId.set(a.getId()); // side-effects for the win
85+
return a.getRangeRelationsOut().size() == 3;
86+
})
8287
.verifyComplete();
8388

8489
skuRepo.findById(bId.get())
85-
.doOnNext(b -> Assertions.assertThat(b.getRangeRelationsIn()).hasSize(1))
90+
.doOnNext(b -> assertThat(b.getRangeRelationsIn()).hasSize(1))
8691
.flatMap(b -> {
8792
b.rangeRelationTo(cRef.get(), 1, 1, RelationType.MULTIPLICATIVE);
8893
return skuRepo.save(b);
8994
})
9095
.as(StepVerifier::create)
91-
.expectNextMatches(a -> a.getRangeRelationsIn().size() == 1 && a.getRangeRelationsOut().size() == 1)
96+
.assertNext(b -> {
97+
assertThat(b.getRangeRelationsIn()).hasSize(1);
98+
assertThat(b.getRangeRelationsOut()).hasSize(1);
99+
})
100+
.verifyComplete();
101+
102+
skuRepo.findById(aId.get())
103+
.as(StepVerifier::create)
104+
.assertNext(a -> {
105+
assertThat(a.getRangeRelationsOut()).hasSize(3);
106+
assertThat(a.getRangeRelationsOut()).allSatisfy(r -> {
107+
int expectedSize = 1;
108+
if ("C".equals(r.getTargetSku().getName())) {
109+
expectedSize = 2;
110+
}
111+
assertThat(r.getTargetSku().getRangeRelationsIn()).hasSize(expectedSize);
112+
});
113+
})
92114
.verifyComplete();
93115
}
94116

0 commit comments

Comments
 (0)