DATAGRAPH-1397 - Create correct target collections for relationships.

The target type of the collection for relationships with properties has not been taken into account and this commit fixes it.

The commit also addresses a possible issue in the Cypher generator for relationships when several mapping to different nodes with the same type exists. The loop needs to continue in case one has already been found, not left.

 Co-authored-by: Gerrit Meier <[email protected]>

Author: michael-simons

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

@@ -401,11 +401,11 @@ private String[] collectAllRelationshipTypes(NodeDescription<?> nodeDescription)
 		for (RelationshipDescription relationshipDescription : nodeDescription.getRelationships()) {
 			String relationshipType = relationshipDescription.getType();
 			if (relationshipTypes.contains(relationshipType)) {
-				break;
+				continue;
 			}
 			if (relationshipDescription.isDynamic()) {
 				relationshipTypes.clear();
-				break;
+				continue;
 			}
 			relationshipTypes.add(relationshipType);
 			collectAllRelationshipTypes(relationshipDescription.getTarget(), relationshipTypes);

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

@@ -507,13 +507,10 @@ private Optional<Object> createInstanceOfRelationships(Neo4jPersistentProperty p
 		}
 
 		if (persistentProperty.getTypeInformation().isCollectionLike()) {
-			if (relationshipDescription.hasRelationshipProperties()) {
-				return Optional.of(relationshipsAndProperties);
-			} else if (persistentProperty.getType().equals(Set.class)) {
-				return Optional.of(new HashSet(value));
-			} else {
-				return Optional.of(value);
-			}
+			List<Object> returnedValues = relationshipDescription.hasRelationshipProperties() ?  relationshipsAndProperties : value;
+			Collection<Object> target = CollectionFactory.createCollection(persistentProperty.getRawType(), persistentProperty.getComponentType(), returnedValues.size());
+			target.addAll(returnedValues);
+			return Optional.of(target);
 		} else {
 			if (relationshipDescription.isDynamic()) {
 				return Optional.ofNullable(dynamicValue.isEmpty() ? null : dynamicValue);

src/test/java/org/springframework/data/neo4j/integration/imperative/RepositoryIT.java

@@ -76,6 +76,7 @@
 import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
 import org.springframework.data.neo4j.core.DatabaseSelection;
 import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
+import org.springframework.data.neo4j.core.Neo4jTemplate;
 import org.springframework.data.neo4j.core.convert.Neo4jConversions;
 import org.springframework.data.neo4j.integration.imperative.repositories.PersonRepository;
 import org.springframework.data.neo4j.integration.imperative.repositories.ThingRepository;
@@ -86,6 +87,7 @@
 import org.springframework.data.neo4j.integration.shared.BidirectionalEnd;
 import org.springframework.data.neo4j.integration.shared.BidirectionalStart;
 import org.springframework.data.neo4j.integration.shared.Club;
+import org.springframework.data.neo4j.integration.shared.ClubRelationship;
 import org.springframework.data.neo4j.integration.shared.DeepRelationships;
 import org.springframework.data.neo4j.integration.shared.EntityWithConvertedId;
 import org.springframework.data.neo4j.integration.shared.Friend;
@@ -100,6 +102,7 @@
 import org.springframework.data.neo4j.integration.shared.PersonWithNoConstructor;
 import org.springframework.data.neo4j.integration.shared.PersonWithRelationship;
 import org.springframework.data.neo4j.integration.shared.PersonWithRelationshipWithProperties;
+import org.springframework.data.neo4j.integration.shared.PersonWithRelationshipWithProperties2;
 import org.springframework.data.neo4j.integration.shared.PersonWithWither;
 import org.springframework.data.neo4j.integration.shared.Pet;
 import org.springframework.data.neo4j.integration.shared.SimilarThing;
@@ -903,6 +906,33 @@ void findEntityWithRelationshipWithAssignedId(@Autowired PetRepository repositor
 	@Nested
 	class RelationshipProperties extends IntegrationTestBase {
 
+		@Test // DATAGRAPH-1397
+		void shouldBeStorableOnSets(
+				@Autowired Neo4jTemplate template) {
+
+			long personId;
+
+			try (Session session = createSession()) {
+				Record record = session.run("CREATE (n:PersonWithRelationshipWithProperties2{name:'Freddie'}),"
+						+ " (n)-[l1:LIKES "
+						+ "{since: 1995, active: true, localDate: date('1995-02-26'), myEnum: 'SOMETHING', point: point({x: 0, y: 1})}"
+						+ "]->(h1:Hobby{name:'Music'}), "
+						+ "(n)-[l2:LIKES "
+						+ "{since: 2000, active: false, localDate: date('2000-06-28'), myEnum: 'SOMETHING_DIFFERENT', point: point({x: 2, y: 3})}"
+						+ "]->(h2:Hobby{name:'Something else'})"
+						+ "RETURN n, h1, h2").single();
+
+				Node personNode = record.get("n").asNode();
+				personId = personNode.id();
+			}
+
+			Optional<PersonWithRelationshipWithProperties2> optionalPerson = template.findById(personId, PersonWithRelationshipWithProperties2.class);
+			assertThat(optionalPerson).hasValueSatisfying(person -> {
+				assertThat(person.getName()).isEqualTo("Freddie");
+				assertThat(person.getHobbies()).hasSize(2).extracting(LikesHobbyRelationship::getSince).containsExactlyInAnyOrder(1995, 2000);
+			});
+		}
+
 		@Test
 		void findEntityWithRelationshipWithProperties(
 				@Autowired PersonWithRelationshipWithPropertiesRepository repository) {
@@ -913,11 +943,16 @@ void findEntityWithRelationshipWithProperties(
 
 			try (Session session = createSession()) {
 				Record record = session.run("CREATE (n:PersonWithRelationshipWithProperties{name:'Freddie'}),"
-						+ " (n)-[l1:LIKES"
+						+ " (n)-[l1:LIKES "
 						+ "{since: 1995, active: true, localDate: date('1995-02-26'), myEnum: 'SOMETHING', point: point({x: 0, y: 1})}"
-						+ "]->(h1:Hobby{name:'Music'})," + " (n)-[l2:LIKES"
+						+ "]->(h1:Hobby{name:'Music'}), "
+						+ "(n)-[l2:LIKES "
 						+ "{since: 2000, active: false, localDate: date('2000-06-28'), myEnum: 'SOMETHING_DIFFERENT', point: point({x: 2, y: 3})}"
-						+ "]->(h2:Hobby{name:'Something else'})" + "RETURN n, h1, h2").single();
+						+ "]->(h2:Hobby{name:'Something else'}), "
+						+ "(n) - [:OWNS] -> (p:Pet {name: 'A Pet'}), "
+						+ "(n) - [:OWNS {place: 'The place to be'}] -> (c1:Club {name: 'Berlin Mitte'}), "
+						+ "(n) - [:OWNS {place: 'Whatever'}] -> (c2:Club {name: 'Schachklub'}) "
+						+ "RETURN n, h1, h2").single();
 
 				Node personNode = record.get("n").asNode();
 				Node hobbyNode1 = record.get("h1").asNode();
@@ -932,6 +967,7 @@ void findEntityWithRelationshipWithProperties(
 			assertThat(optionalPerson).isPresent();
 			PersonWithRelationshipWithProperties person = optionalPerson.get();
 			assertThat(person.getName()).isEqualTo("Freddie");
+			assertThat(person.getPets()).hasSize(1).first().extracting(Pet::getName).isEqualTo("A Pet");
 
 			Hobby hobby1 = new Hobby();
 			hobby1.setName("Music");
@@ -955,17 +991,22 @@ void findEntityWithRelationshipWithProperties(
 			assertThat(hobbies).containsExactlyInAnyOrder(rel1, rel2);
 			assertThat(hobbies.get(hobbies.indexOf(rel1)).getHobby()).isEqualTo(hobby1);
 			assertThat(hobbies.get(hobbies.indexOf(rel2)).getHobby()).isEqualTo(hobby2);
+
+			assertThat(person.getClubs()).hasSize(2)
+					.extracting(ClubRelationship::getPlace)
+					.containsExactlyInAnyOrder("The place to be", "Whatever");
 		}
 
 		@Test
-		void findEntityWithRelationshipWithPropertiesScalar(
-				@Autowired PersonWithRelationshipWithPropertiesRepository repository) {
+		void findEntityWithRelationshipWithPropertiesScalar(@Autowired PersonWithRelationshipWithPropertiesRepository repository) {
 
 			long personId;
 
 			try (Session session = createSession()) {
 				Record record = session.run("CREATE (n:PersonWithRelationshipWithProperties{name:'Freddie'}),"
-						+ " (n)-[:WORKS_IN{since: 1995}]->(:Club{name:'Blubb'})"
+						+ " (n)-[:WORKS_IN{since: 1995}]->(:Club{name:'Blubb'}),"
+						+ "(n) - [:OWNS {place: 'The place to be'}] -> (c1:Club {name: 'Berlin Mitte'}), "
+						+ "(n) - [:OWNS {place: 'Whatever'}] -> (c2:Club {name: 'Schachklub'}) "
 						+ "RETURN n").single();
 
 				Node personNode = record.get("n").asNode();

src/test/java/org/springframework/data/neo4j/integration/shared/LikesHobbyRelationship.java

@@ -91,6 +91,10 @@ public void setHobby(Hobby hobby) {
 		this.hobby = hobby;
 	}
 
+	public Integer getSince() {
+		return since;
+	}
+
 	/**
 	 * The missing javadoc
 	 */

src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithRelationshipWithProperties.java

@@ -40,6 +40,8 @@ public class PersonWithRelationshipWithProperties {
 
 	@Relationship("OWNS") private Set<Pet> pets;
 
+	@Relationship("OWNS") private List<ClubRelationship> clubs;
+
 	public PersonWithRelationshipWithProperties(String name) {
 		this.name = name;
 	}
@@ -64,4 +66,11 @@ public WorksInClubRelationship getClub() {
 		return club;
 	}
 
+	public Set<Pet> getPets() {
+		return pets;
+	}
+
+	public List<ClubRelationship> getClubs() {
+		return clubs;
+	}
 }

src/test/java/org/springframework/data/neo4j/integration/shared/PersonWithRelationshipWithProperties2.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2011-2020 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.neo4j.integration.shared;
+
+import java.util.Set;
+
+import org.springframework.data.neo4j.core.schema.GeneratedValue;
+import org.springframework.data.neo4j.core.schema.Id;
+import org.springframework.data.neo4j.core.schema.Node;
+import org.springframework.data.neo4j.core.schema.Relationship;
+
+/**
+ * @author Michael J. Simons
+ */
+@Node
+public class PersonWithRelationshipWithProperties2 {
+
+	@Id @GeneratedValue private Long id;
+
+	private final String name;
+
+	@Relationship("LIKES") private Set<LikesHobbyRelationship> hobbies;
+
+	public PersonWithRelationshipWithProperties2(String name) {
+		this.name = name;
+	}
+
+	public Long getId() {
+		return id;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public Set<LikesHobbyRelationship> getHobbies() {
+		return hobbies;
+	}
+}