GH-2398 - Fix flaky RelationshipProperties field determination.

Closes #2398

Author: meistermeier

src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentEntity.java

@@ -255,22 +255,14 @@ private void verifyDynamicAssociations() {
 
 	private void verifyAssociationsWithProperties() {
 
-		this.doWithAssociations((Association<Neo4jPersistentProperty> association) -> {
-
-			if (association instanceof RelationshipDescription) {
-				RelationshipDescription relationship = (RelationshipDescription) association;
-				if (relationship.hasRelationshipProperties()) {
-					NodeDescription<?> relationshipPropertiesEntity = relationship.getRelationshipPropertiesEntity();
-					Supplier<String> messageSupplier = () -> String.format(
-							"The target class `%s` for the properties of the relationship `%s` "
+		if (this.isRelationshipPropertiesEntity()) {
+			Supplier<String> messageSupplier = () -> String.format(
+					"The class `%s` for the properties of a relationship "
 							+ "is missing a property for the generated, internal ID (`@Id @GeneratedValue Long id`) "
 							+ "which is needed for safely updating properties.",
-							relationshipPropertiesEntity.getUnderlyingClass().getName(),
-							relationship.getType());
-					Assert.state(relationshipPropertiesEntity.getIdDescription() != null && relationshipPropertiesEntity.getIdDescription().isInternallyGeneratedId(), messageSupplier);
-				}
-			}
-		});
+					this.getUnderlyingClass().getName());
+			Assert.state(this.getIdDescription() != null && this.getIdDescription().isInternallyGeneratedId(), messageSupplier);
+		}
 	}
 
 	private void verifyDynamicLabels() {

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

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.neo4j.core.mapping;
 
+import java.lang.reflect.Field;
 import java.util.Optional;
 import java.util.function.Function;
 
@@ -23,7 +24,6 @@
 import org.springframework.data.mapping.Association;
 import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mapping.PersistentEntity;
-import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.model.AnnotationBasedPersistentProperty;
 import org.springframework.data.mapping.model.Property;
 import org.springframework.data.mapping.model.SimpleTypeHolder;
@@ -35,6 +35,7 @@
 import org.springframework.data.neo4j.core.schema.TargetNode;
 import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.data.util.Lazy;
+import org.springframework.data.util.ReflectionUtils;
 import org.springframework.data.util.TypeInformation;
 import org.springframework.lang.NonNull;
 import org.springframework.lang.Nullable;
@@ -114,9 +115,9 @@ protected Association<Neo4jPersistentProperty> createAssociation() {
 		Neo4jPersistentEntity<?> relationshipPropertiesClass = null;
 
 		if (this.hasActualTypeAnnotation(RelationshipProperties.class)) {
-			TypeInformation<?> type = getRelationshipPropertiesTargetType(getActualType());
-			obverseOwner = this.mappingContext.getPersistentEntity(type);
-			relationshipPropertiesClass = this.mappingContext.getPersistentEntity(getActualType());
+			Class<?> type = getRelationshipPropertiesTargetType(getActualType());
+			obverseOwner = this.mappingContext.addPersistentEntity(ClassTypeInformation.from(type)).get();
+			relationshipPropertiesClass = this.mappingContext.addPersistentEntity(ClassTypeInformation.from(getActualType())).get();
 		} else {
 			Class<?> associationTargetType = this.getAssociationTargetType();
 			obverseOwner = this.mappingContext.addPersistentEntity(ClassTypeInformation.from(associationTargetType)).get();
@@ -132,13 +133,13 @@ protected Association<Neo4jPersistentProperty> createAssociation() {
 						mapValueType.getType().isAnnotationPresent(RelationshipProperties.class);
 
 				if (relationshipPropertiesCollection) {
-					TypeInformation<?> type = getRelationshipPropertiesTargetType(mapValueType.getActualType().getType());
-					obverseOwner = this.mappingContext.getPersistentEntity(type);
+					Class<?> type = getRelationshipPropertiesTargetType(mapValueType.getActualType().getType());
+					obverseOwner = this.mappingContext.addPersistentEntity(ClassTypeInformation.from(type)).get();
 					relationshipPropertiesClass = this.mappingContext
-							.getPersistentEntity(mapValueType.getComponentType().getType());
+							.addPersistentEntity(mapValueType.getComponentType()).get();
 
 				} else if (relationshipPropertiesScalar) {
-					relationshipPropertiesClass = this.mappingContext.getPersistentEntity(mapValueType.getType());
+					relationshipPropertiesClass = this.mappingContext.addPersistentEntity(mapValueType.getComponentType()).get();
 				}
 			}
 		}
@@ -175,12 +176,16 @@ protected Association<Neo4jPersistentProperty> createAssociation() {
 	}
 
 	@NonNull
-	private TypeInformation<?> getRelationshipPropertiesTargetType(Class<?> relationshipPropertiesType) {
-		return this.mappingContext.addPersistentEntity(ClassTypeInformation.from(relationshipPropertiesType))
-				.map(entity -> entity.getPersistentProperty(TargetNode.class))
-				.map(PersistentProperty::getTypeInformation)
-				.orElseThrow(
-						() -> new MappingException("Missing @TargetNode declaration in " + relationshipPropertiesType));
+	private Class<?> getRelationshipPropertiesTargetType(Class<?> relationshipPropertiesType) {
+
+		Field targetNodeField = ReflectionUtils.findField(relationshipPropertiesType,
+				field -> field.isAnnotationPresent(TargetNode.class));
+
+		if (targetNodeField == null) {
+			throw new MappingException("Missing @TargetNode declaration in " + relationshipPropertiesType);
+		}
+		ClassTypeInformation<?> relationshipPropertiesTypeInformation = ClassTypeInformation.from(relationshipPropertiesType);
+		return relationshipPropertiesTypeInformation.getProperty(targetNodeField.getName()).getType();
 	}
 
 	@Override

src/test/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContextTest.java

@@ -87,7 +87,7 @@ void startupWithoutInternallyGeneratedIDShouldFail() {
 								Arrays.asList(IrrelevantSourceContainer.class, InvalidRelationshipPropertyContainer.class,
 										IrrelevantTargetContainer.class)));
 						schema.initialize();
-					}).withMessage("The target class `org.springframework.data.neo4j.core.mapping.Neo4jMappingContextTest$InvalidRelationshipPropertyContainer` for the properties of the relationship `RELATIONSHIP_PROPERTY_CONTAINER` is missing a property for the generated, internal ID (`@Id @GeneratedValue Long id`) which is needed for safely updating properties.");
+					}).withMessage("The class `org.springframework.data.neo4j.core.mapping.Neo4jMappingContextTest$InvalidRelationshipPropertyContainer` for the properties of a relationship is missing a property for the generated, internal ID (`@Id @GeneratedValue Long id`) which is needed for safely updating properties.");
 		}
 
 		@Test // GH-2214
@@ -99,7 +99,7 @@ void startupWithWrongKindOfGeneratedIDShouldFail() {
 						Arrays.asList(IrrelevantSourceContainer3.class, InvalidRelationshipPropertyContainer2.class,
 								IrrelevantTargetContainer.class)));
 				schema.initialize();
-			}).withMessage("The target class `org.springframework.data.neo4j.core.mapping.Neo4jMappingContextTest$InvalidRelationshipPropertyContainer2` for the properties of the relationship `RELATIONSHIP_PROPERTY_CONTAINER` is missing a property for the generated, internal ID (`@Id @GeneratedValue Long id`) which is needed for safely updating properties.");
+			}).withMessage("The class `org.springframework.data.neo4j.core.mapping.Neo4jMappingContextTest$InvalidRelationshipPropertyContainer2` for the properties of a relationship is missing a property for the generated, internal ID (`@Id @GeneratedValue Long id`) which is needed for safely updating properties.");
 		}
 
 		@Test // GH-2118
@@ -523,6 +523,14 @@ void shouldNotCreateEntityForConvertedSimpleTypes() {
 		assertThat(schema.hasPersistentEntityFor(ThingWithCustomTypes.CustomType.class)).isFalse();
 	}
 
+	@Test
+	void dontFailOnCyclicRelationshipProperties() { // GH-2398
+		Neo4jMappingContext context = new Neo4jMappingContext();
+		Neo4jPersistentEntity<?> persistentEntity = context.getPersistentEntity(ARelationship.class);
+		assertThat(persistentEntity.isRelationshipPropertiesEntity()).isTrue();
+		assertThat(persistentEntity.getGraphProperties()).hasSize(3);
+	}
+
 	static class DummyIdGenerator implements IdGenerator<Void> {
 
 		@Override
@@ -865,6 +873,58 @@ static class SomeInterfaceImpl3b implements SomeInterface3 {
 		SomeInterface3 related;
 	}
 
+	@Node
+	public static class SomeBaseEntity {
+		@Id
+		@GeneratedValue
+		public Long internalId;
+
+		public String id;
+	}
+
+	@Node
+	public static class ConcreteEntity extends SomeBaseEntity {
+
+		@Relationship("A")
+		public Set<ARelationship> as;
+		@Relationship("B")
+		public Set<BRelationship> bs;
+		@Relationship("C")
+		public Set<CRelationship> cs;
+
+
+	}
+
+	public static class RelationshipPropertiesBaseClass<T extends SomeBaseEntity> {
+		@Id
+		@GeneratedValue
+		public Long internalId;
+
+		public String id;
+
+		@TargetNode
+		public T target;
+	}
+
+	public static abstract class RelationshipPropertiesAbstractClass extends RelationshipPropertiesBaseClass<ConcreteEntity> {
+
+	}
+
+	@RelationshipProperties
+	public static class ARelationship extends RelationshipPropertiesAbstractClass {
+
+	}
+
+	@RelationshipProperties
+	public static class BRelationship extends RelationshipPropertiesAbstractClass {
+
+	}
+
+	@RelationshipProperties
+	public static class CRelationship extends RelationshipPropertiesAbstractClass {
+
+	}
+
 	static class MissingIdToMapConverter implements Neo4jPersistentPropertyToMapConverter<String, MissingId> {
 
 		@Override public Map<String, Value> decompose(MissingId property, Neo4jConversionService conversionService) {