Skip to content

Commit 71e2325

Browse files
meistermeiermichael-simons
meistermeier
authored and
michael-simons
committed
GH-2289 - Improve relationship observe detection.
Closes #2289.
1 parent 70174ec commit 71e2325

File tree

9 files changed

+251
-14
lines changed

9 files changed

+251
-14
lines changed

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

+1
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,7 @@ private <T> T processNestedRelations(Neo4jPersistentEntity<?> sourceEntity, Pers
750750
TemplateSupport.updateVersionPropertyIfPossible(targetEntity, targetPropertyAccessor, savedEntity);
751751
}
752752
stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean());
753+
stateMachine.markRelationshipAsProcessed(relatedInternalId, relationshipDescription.getRelationshipObverse());
753754

754755
Object idValue = idProperty != null
755756
? relationshipContext

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

+1
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,7 @@ private <T> Mono<T> processNestedRelations(Neo4jPersistentEntity<?> sourceEntity
842842
targetPropertyAccessor.setProperty(targetEntity.getVersionProperty(), idAndVersion.getT2()[0]);
843843
}
844844
stateMachine.markValueAsProcessedAs(relatedObjectBeforeCallbacksApplied, targetPropertyAccessor.getBean());
845+
stateMachine.markRelationshipAsProcessed(relatedInternalId, relationshipDescription.getRelationshipObverse());
845846

846847
Object idValue = idProperty != null
847848
? relationshipContext

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

+10-9
Original file line numberDiff line numberDiff line change
@@ -143,32 +143,33 @@ protected Association<Neo4jPersistentProperty> createAssociation() {
143143
}
144144
}
145145

146-
Relationship outgoingRelationship = this.findAnnotation(Relationship.class);
146+
Relationship relationship = this.findAnnotation(Relationship.class);
147147

148148
String type;
149-
if (outgoingRelationship != null && StringUtils.hasText(outgoingRelationship.type())) {
150-
type = outgoingRelationship.type();
149+
if (relationship != null && StringUtils.hasText(relationship.type())) {
150+
type = relationship.type();
151151
} else {
152152
type = deriveRelationshipType(this.getName());
153153
}
154154

155-
Relationship.Direction direction = Relationship.Direction.OUTGOING;
156-
if (outgoingRelationship != null) {
157-
direction = outgoingRelationship.direction();
158-
}
155+
Relationship.Direction direction = relationship != null
156+
? relationship.direction()
157+
: Relationship.Direction.OUTGOING;
159158

160159
// Try to determine if there is a relationship definition that expresses logically the same relationship
161160
// on the other end.
162161
Optional<RelationshipDescription> obverseRelationshipDescription = obverseOwner.getRelationships().stream()
163-
.filter(rel -> rel.getType().equals(type) && rel.getTarget().equals(this.getOwner())).findFirst();
162+
.filter(rel -> rel.getType().equals(type)
163+
&& rel.getTarget().equals(this.getOwner())
164+
&& rel.getDirection() == direction.opposite()).findFirst();
164165

165166
DefaultRelationshipDescription relationshipDescription = new DefaultRelationshipDescription(this,
166167
obverseRelationshipDescription.orElse(null), type, dynamicAssociation, (NodeDescription<?>) getOwner(),
167168
this.getName(), obverseOwner, direction, relationshipPropertiesClass);
168169

169170
// Update the previous found, if any, relationship with the newly created one as its counterpart.
170171
obverseRelationshipDescription
171-
.ifPresent(relationship -> relationship.setRelationshipObverse(relationshipDescription));
172+
.ifPresent(observeRelationship -> observeRelationship.setRelationshipObverse(relationshipDescription));
172173

173174
return relationshipDescription;
174175
}

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

+4-1
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

+5-1
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

+185
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.junit.jupiter.params.ParameterizedTest;
3131
import org.junit.jupiter.params.provider.ValueSource;
3232
import org.springframework.data.annotation.Transient;
33+
import org.springframework.data.mapping.AssociationHandler;
3334
import org.springframework.data.mapping.MappingException;
3435
import org.springframework.data.neo4j.core.schema.DynamicLabels;
3536
import org.springframework.data.neo4j.core.schema.GeneratedValue;
@@ -129,6 +130,71 @@ void doesFailOnRelationshipPropertiesWithMissingTargetNode() {
129130
.getPersistentEntity(EntityWithInCorrectRelationshipProperties.class))
130131
.withMessageContaining("Missing @TargetNode declaration in");
131132
}
133+
134+
@Test // GH-2289
135+
void correctlyFindRelationshipObverseSameEntity() {
136+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
137+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationship.class);
138+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
139+
RelationshipDescription rd = (RelationshipDescription) a;
140+
assertThat(rd.getRelationshipObverse()).isNotNull();
141+
});
142+
}
143+
144+
@Test // GH-2289
145+
void correctlyFindRelationshipObverse() {
146+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
147+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationshipToOtherEntity.class);
148+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
149+
RelationshipDescription rd = (RelationshipDescription) a;
150+
assertThat(rd.getRelationshipObverse()).isNotNull();
151+
});
152+
persistentEntity = neo4jMappingContext.getPersistentEntity(OtherEntityWithBidirectionalRelationship.class);
153+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
154+
RelationshipDescription rd = (RelationshipDescription) a;
155+
assertThat(rd.getRelationshipObverse()).isNotNull();
156+
});
157+
}
158+
159+
@Test // GH-2289
160+
void correctlyFindRelationshipObverseWithRelationshipProperties() {
161+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
162+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationshipToOtherEntityWithRelationshipProperties.class);
163+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
164+
RelationshipDescription rd = (RelationshipDescription) a;
165+
assertThat(rd.getRelationshipObverse()).isNotNull();
166+
});
167+
persistentEntity = neo4jMappingContext.getPersistentEntity(OtherEntityWithBidirectionalRelationship.class);
168+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
169+
RelationshipDescription rd = (RelationshipDescription) a;
170+
assertThat(rd.getRelationshipObverse()).isNotNull();
171+
});
172+
}
173+
174+
@Test // GH-2289
175+
void correctlyFindSameEntityRelationshipObverseWithRelationshipProperties() {
176+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
177+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityWithBidirectionalRelationshipProperties.class);
178+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
179+
RelationshipDescription rd = (RelationshipDescription) a;
180+
assertThat(rd.getRelationshipObverse()).isNotNull();
181+
});
182+
}
183+
184+
@Test // GH-2289
185+
void correctlyDontFindRelationshipObverse() {
186+
Neo4jMappingContext neo4jMappingContext = new Neo4jMappingContext();
187+
Neo4jPersistentEntity<?> persistentEntity = neo4jMappingContext.getPersistentEntity(EntityLooksLikeHasObserve.class);
188+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
189+
RelationshipDescription rd = (RelationshipDescription) a;
190+
assertThat(rd.getRelationshipObverse()).isNull();
191+
});
192+
persistentEntity = neo4jMappingContext.getPersistentEntity(OtherEntityLooksLikeHasObserve.class);
193+
persistentEntity.doWithAssociations((AssociationHandler<Neo4jPersistentProperty>) a -> {
194+
RelationshipDescription rd = (RelationshipDescription) a;
195+
assertThat(rd.getRelationshipObverse()).isNull();
196+
});
197+
}
132198
}
133199

134200
@Nested
@@ -491,4 +557,123 @@ static class HasNoTargetNodeRelationshipProperties {
491557
@Id @GeneratedValue
492558
private Long id;
493559
}
560+
561+
@Node
562+
static class EntityWithBidirectionalRelationship {
563+
564+
@Id @GeneratedValue
565+
private Long id;
566+
567+
@Relationship("KNOWS")
568+
List<EntityWithBidirectionalRelationship> knows;
569+
570+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
571+
List<EntityWithBidirectionalRelationship> knownBy;
572+
573+
}
574+
575+
@Node
576+
static class EntityWithBidirectionalRelationshipToOtherEntity {
577+
578+
@Id @GeneratedValue
579+
private Long id;
580+
581+
@Relationship("KNOWS")
582+
List<OtherEntityWithBidirectionalRelationship> knows;
583+
584+
}
585+
586+
@Node
587+
static class OtherEntityWithBidirectionalRelationship {
588+
589+
@Id @GeneratedValue
590+
private Long id;
591+
592+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
593+
List<EntityWithBidirectionalRelationshipToOtherEntity> knownBy;
594+
595+
}
596+
597+
@Node
598+
static class EntityWithBidirectionalRelationshipToOtherEntityWithRelationshipProperties {
599+
600+
@Id @GeneratedValue
601+
private Long id;
602+
603+
@Relationship("KNOWS")
604+
List<OtherEntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties> knows;
605+
606+
}
607+
608+
@Node
609+
static class OtherEntityWithBidirectionalRelationshipWithRelationshipProperties {
610+
611+
@Id @GeneratedValue
612+
private Long id;
613+
614+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
615+
List<EntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties> knownBy;
616+
617+
}
618+
619+
@RelationshipProperties
620+
static class OtherEntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties {
621+
@Id @GeneratedValue
622+
private Long id;
623+
624+
@TargetNode
625+
OtherEntityWithBidirectionalRelationshipWithRelationshipProperties target;
626+
}
627+
628+
@RelationshipProperties
629+
static class EntityWithBidirectionalRelationshipWithRelationshipPropertiesProperties {
630+
@Id @GeneratedValue
631+
private Long id;
632+
633+
@TargetNode
634+
EntityWithBidirectionalRelationshipToOtherEntityWithRelationshipProperties target;
635+
}
636+
637+
@Node
638+
static class EntityWithBidirectionalRelationshipProperties {
639+
640+
@Id @GeneratedValue
641+
private Long id;
642+
643+
@Relationship("KNOWS")
644+
List<BidirectionalRelationshipProperties> knows;
645+
646+
@Relationship(type = "KNOWS", direction = Relationship.Direction.INCOMING)
647+
List<BidirectionalRelationshipProperties> knownBy;
648+
649+
}
650+
651+
@RelationshipProperties
652+
static class BidirectionalRelationshipProperties {
653+
654+
@Id @GeneratedValue
655+
private Long id;
656+
657+
@TargetNode
658+
EntityWithBidirectionalRelationshipProperties target;
659+
}
660+
661+
@Node
662+
static class EntityLooksLikeHasObserve {
663+
@Id @GeneratedValue
664+
private Long id;
665+
666+
@Relationship("KNOWS")
667+
private List<OtherEntityLooksLikeHasObserve> knows;
668+
}
669+
670+
@Node
671+
static class OtherEntityLooksLikeHasObserve {
672+
@Id @GeneratedValue
673+
private Long id;
674+
675+
@Relationship("KNOWS")
676+
private List<EntityLooksLikeHasObserve> knows;
677+
}
678+
494679
}

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

+13-1
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,25 @@ void testNewRelation(@Autowired SkuRepository skuRepo) {
7474
b = skuRepo.findById(b.getId()).get();
7575
assertThat(b.getRangeRelationsIn()).hasSize(1);
7676

77+
assertThat(b.getRangeRelationsIn().stream().findFirst().get().getTargetSku().getRangeRelationsOut()).hasSize(3);
78+
7779
b.rangeRelationTo(c, 1, 1, RelationType.MULTIPLICATIVE);
7880
b = skuRepo.save(b);
7981
assertThat(b.getRangeRelationsIn()).hasSize(1);
8082
assertThat(b.getRangeRelationsOut()).hasSize(1);
83+
84+
a = skuRepo.findById(a.getId()).get();
85+
assertThat(a.getRangeRelationsOut()).hasSize(3);
86+
assertThat(a.getRangeRelationsOut()).allSatisfy(r -> {
87+
int expectedSize = 1;
88+
if ("C".equals(r.getTargetSku().getName())) {
89+
expectedSize = 2;
90+
}
91+
assertThat(r.getTargetSku().getRangeRelationsIn()).hasSize(expectedSize);
92+
});
8193
}
8294

83-
@RepeatedTest(5) // GH-2294
95+
@RepeatedTest(5) // GH-2294
8496
void testNewRelationRo(@Autowired SkuRORepository skuRepo) {
8597
SkuRO a = skuRepo.findOneByName("A");
8698
SkuRO b = skuRepo.findOneByName("B");

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

+23-2
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ protected static void setupData(@Autowired BookmarkCapture bookmarkCapture) {
6969
@RepeatedTest(23)
7070
void testNewRelation(@Autowired SkuRepository skuRepo) {
7171

72+
AtomicLong aId = new AtomicLong();
7273
AtomicLong bId = new AtomicLong();
7374
AtomicReference<Sku> cRef = new AtomicReference<>();
7475
skuRepo.save(new Sku(0L, "A"))
@@ -87,7 +88,10 @@ void testNewRelation(@Autowired SkuRepository skuRepo) {
8788
a.rangeRelationTo(d, 1, 1, RelationType.MULTIPLICATIVE);
8889
return skuRepo.save(a);
8990
}).as(StepVerifier::create)
90-
.expectNextMatches(a -> a.getRangeRelationsOut().size() == 3)
91+
.expectNextMatches(a -> {
92+
aId.set(a.getId()); // side-effects for the win
93+
return a.getRangeRelationsOut().size() == 3;
94+
})
9195
.verifyComplete();
9296

9397
skuRepo.findById(bId.get())
@@ -97,7 +101,24 @@ void testNewRelation(@Autowired SkuRepository skuRepo) {
97101
return skuRepo.save(b);
98102
})
99103
.as(StepVerifier::create)
100-
.expectNextMatches(a -> a.getRangeRelationsIn().size() == 1 && a.getRangeRelationsOut().size() == 1)
104+
.assertNext(b -> {
105+
assertThat(b.getRangeRelationsIn()).hasSize(1);
106+
assertThat(b.getRangeRelationsOut()).hasSize(1);
107+
})
108+
.verifyComplete();
109+
110+
skuRepo.findById(aId.get())
111+
.as(StepVerifier::create)
112+
.assertNext(a -> {
113+
assertThat(a.getRangeRelationsOut()).hasSize(3);
114+
assertThat(a.getRangeRelationsOut()).allSatisfy(r -> {
115+
int expectedSize = 1;
116+
if ("C".equals(r.getTargetSku().getName())) {
117+
expectedSize = 2;
118+
}
119+
assertThat(r.getTargetSku().getRangeRelationsIn()).hasSize(expectedSize);
120+
});
121+
})
101122
.verifyComplete();
102123
}
103124

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

+9
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,13 @@ public RangeRelation rangeRelationTo(Sku sku, double minDelta, double maxDelta,
6262
sku.rangeRelationsIn.add(relationIn);
6363
return relationOut;
6464
}
65+
66+
@Override
67+
public String toString() {
68+
return "Sku{" +
69+
"id=" + id +
70+
", number=" + number +
71+
", name='" + name +
72+
'}';
73+
}
6574
}

0 commit comments

Comments
 (0)