GH-2537 - Fix serialization of the target node of relationship property entities.

This should have gone into a separate field in parallel to properties,
not into the properties itself.

Fixes #2537 and also adds a test for the specific question asked in the
ticket.

Author: michael-simons

src/main/java/org/springframework/data/neo4j/repository/query/Neo4jNestedMapEntityWriter.java

@@ -114,12 +114,10 @@ Map<String, Object> writeImpl(Object source, Map<String, Object> sink, Set<Objec
 		addLabels(sink, entity, propertyAccessor);
 		addRelations(sink, entity, propertyAccessor, seenObjects);
 		if (initialObject && entity.isRelationshipPropertiesEntity()) {
-			Map<String, Object> propertyMap = (Map<String, Object>) sink.get(Constants.NAME_OF_PROPERTIES_PARAM);
 			entity.doWithProperties((PropertyHandler<Neo4jPersistentProperty>) p -> {
 				if (p.isAnnotationPresent(TargetNode.class)) {
-					Map<String, Object> target = this.writeImpl(propertyAccessor.getProperty(p), new HashMap<>(), seenObjects, false);
-					propertyMap.put("__target__", Values.value(target));
-					return;
+					Value target = Values.value(this.writeImpl(propertyAccessor.getProperty(p), new HashMap<>(), seenObjects, false));
+					sink.put("__target__", target);
 				}
 			});
 		}

src/test/java/org/springframework/data/neo4j/integration/issues/gh2323/GH2323IT.java

@@ -19,10 +19,12 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.Session;
@@ -69,6 +71,21 @@ protected static void setupData(@Autowired BookmarkCapture bookmarkCapture) {
 		}
 	}
 
+	@BeforeEach
+	protected void removeRelationships(@Autowired BookmarkCapture bookmarkCapture) {
+		try (Session session = neo4jConnectionSupport.getDriver().session(bookmarkCapture.createSessionConfig());
+				Transaction transaction = session.beginTransaction();
+		) {
+			transaction.run("MATCH ()- [r:KNOWS]-() DELETE r").consume();
+			transaction.run("MATCH (n:Language) DELETE n").consume();
+			transaction.run("MATCH (n:Person {name: 'Gerrit'}) DETACH DELETE n").consume();
+			transaction.run("unwind ['German', 'English'] as name create (n:Language {name: name}) return name")
+					.consume();
+			transaction.commit();
+			bookmarkCapture.seedWith(session.lastBookmark());
+		}
+	}
+
 	@Test // GH-2323
 	void listOfRelationshipPropertiesShouldBeUnwindable(@Autowired PersonService personService) {
 		Person person = personService.updateRel(personId, Arrays.asList("German"));
@@ -84,10 +101,14 @@ void listOfRelationshipPropertiesShouldBeUnwindable(@Autowired PersonService per
 	void dontMixRelatedNodes(@Autowired PersonRepository repository, @Autowired BookmarkCapture bookmarkCapture) {
 		String id;
 		try (Session session = neo4jConnectionSupport.getDriver().session(bookmarkCapture.createSessionConfig());
-			 Transaction transaction = session.beginTransaction();
+				Transaction transaction = session.beginTransaction();
 		) {
-			id = transaction.run("CREATE (n:Person {id:randomUUID(), name: 'Gerrit'})-[:KNOWS]->(:Language{name:'English'}) return n.id").single().get(0).asString();
-			transaction.run("MATCH (n:Person {name: 'Gerrit'}) MERGE (n)-[:MOTHER_TONGUE_IS]->(:Language{name:'German'})").consume();
+			id = transaction.run(
+							"CREATE (n:Person {id:randomUUID(), name: 'Gerrit'})-[:KNOWS]->(:Language{name:'English'}) return n.id")
+					.single().get(0).asString();
+			transaction.run(
+							"MATCH (n:Person {name: 'Gerrit'}) MERGE (n)-[:MOTHER_TONGUE_IS]->(:Language{name:'German'})")
+					.consume();
 			transaction.commit();
 			bookmarkCapture.seedWith(session.lastBookmark());
 
@@ -101,14 +122,40 @@ void dontMixRelatedNodes(@Autowired PersonRepository repository, @Autowired Book
 		}
 	}
 
+	@Test // GH-2537
+	void ensureRelationshipsAreSerialized(@Autowired PersonService personService) {
+
+		Optional<Person> optionalPerson = personService.updateRel2(personId, Arrays.asList("German"));
+		assertThat(optionalPerson).isPresent().hasValueSatisfying(person -> {
+			assertThat(person.getKnownLanguages()).hasSize(1);
+			assertThat(person.getKnownLanguages()).first().satisfies(knows -> {
+				assertThat(knows.getDescription()).isEqualTo("Some description");
+				assertThat(knows.getLanguage()).extracting(Language::getName).isEqualTo("German");
+			});
+		});
+	}
+
 	@Repository
 	public interface PersonRepository extends Neo4jRepository<Person, String> {
 
-		@Query("UNWIND $relations As rel WITH rel " +
-			   "CREATE (f:Person {id: $from}) - [r:KNOWS {description: rel.__properties__.description}] -> (t:Language {name: rel.__properties__.__target__.__id__}) "
-			   +
-			   "RETURN f, collect(r), collect(t)")
+		// Using separate id and than relationships on top level
+		@Query(""
+			   + "UNWIND $relations As rel WITH rel "
+			   + "MATCH (f:Person {id: $from}) "
+			   + "MATCH (t:Language {name: rel.__target__.__id__}) "
+			   + "CREATE (f)- [r:KNOWS {description: rel.__properties__.description}] -> (t) "
+			   + "RETURN f, collect(r), collect(t)"
+		)
 		Person updateRel(@Param("from") String from, @Param("relations") List<Knows> relations);
+
+		// Using the whole person object
+		@Query(""
+			   + "UNWIND $person.__properties__.KNOWS As rel WITH rel "
+			   + "MATCH (f:Person {id: $person.__id__}) "
+			   + "MATCH  (t:Language {name: rel.__target__.__id__}) "
+			   + "CREATE (f) - [r:KNOWS {description: rel.__properties__.description}] -> (t) "
+			   + "RETURN f, collect(r), collect(t)")
+		Person updateRel2(@Param("person") Person person);
 	}
 
 	@Service
@@ -127,6 +174,21 @@ public Person updateRel(String from, List<String> languageNames) {
 					.collect(Collectors.toList());
 			return personRepository.updateRel(from, knownLanguages);
 		}
+
+		public Optional<Person> updateRel2(String id, List<String> languageNames) {
+
+			Optional<Person> original = personRepository.findById(id);
+			if (original.isPresent()) {
+				Person person = original.get();
+				List<Knows> knownLanguages = languageNames.stream().map(Language::new)
+						.map(language -> new Knows("Some description", language))
+						.collect(Collectors.toList());
+				person.setKnownLanguages(knownLanguages);
+				return Optional.of(personRepository.updateRel2(person));
+			}
+
+			return original;
+		}
 	}
 
 	@Configuration

src/test/java/org/springframework/data/neo4j/integration/issues/gh2323/Person.java

@@ -61,4 +61,8 @@ public List<Knows> getKnownLanguages() {
 	public KnowsMtEntity getMotherTongue() {
 		return motherTongue;
 	}
+
+	public void setKnownLanguages(List<Knows> knownLanguages) {
+		this.knownLanguages = knownLanguages;
+	}
 }

src/test/java/org/springframework/data/neo4j/repository/query/Neo4jNestedMapEntityWriterTest.java

@@ -145,10 +145,10 @@ void relationshipPropertiesShouldBeSerializedWithTargetNodeWhenPassedFirstToWrit
 		Map<String, Value> properties = (Map<String, Value>) result.get("__properties__");
 		assertThat(properties).containsEntry("description", Values.value("Some description"));
 
-		assertThat(properties).hasEntrySatisfying("__target__", isAMapValue);
-		properties = properties.get("__target__").asMap(Function.identity());
-		assertThat(properties).containsEntry("__id__", Values.value("German"));
-		assertThat(properties).hasEntrySatisfying("__properties__", isAMapValue);
+		assertThat(result).hasEntrySatisfying("__target__", isAMapValue);
+		Map<String, Value> target = ((Value) result.get("__target__")).asMap(Function.identity());
+		assertThat(target).containsEntry("__id__", Values.value("German"));
+		assertThat(target).hasEntrySatisfying("__properties__", isAMapValue);
 	}
 
 	@Test // DATAGRAPH-1452