GH-2858 - Add re-population of properties and relationships during mapping.

Prior to this, an incomplete loaded entity, due to projection, was never
touched again to add missing properties loaded via a different relationship
and projection definition.

Closes #2858

Author: meistermeier

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

@@ -1202,7 +1202,6 @@ private NodesAndRelationshipsByIdStatementProvider createNodesAndRelationshipsBy
 				return NodesAndRelationshipsByIdStatementProvider.EMPTY;
 			}
 			// load first level relationships
-//			final Set<String> relationshipIds = new HashSet<>();
 			final Map<String, Set<String>> relationshipsToRelatedNodeIds = new HashMap<>();
 
 			for (RelationshipDescription relationshipDescription : entityMetaData.getRelationshipsInHierarchy(queryFragments::includeField)) {

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

@@ -25,6 +25,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.BiConsumer;
@@ -334,6 +335,7 @@ private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescr
 
 			// save final state of the bean
 			knownObjects.storeObject(internalId, bean);
+			knownObjects.mappedWithQueryResult(internalId, queryResult);
 			return bean;
 		};
 
@@ -342,21 +344,36 @@ private <ET> ET map(MapAccessor queryResult, Neo4jPersistentEntity<ET> nodeDescr
 		if (mappedObject == null) {
 			mappedObject = mappedObjectSupplier.get();
 			knownObjects.storeObject(internalId, mappedObject);
-		} else if (knownObjects.alreadyMappedInPreviousRecord(internalId)) {
-			// If the object were created in a run before, it _could_ have missing relationships
-			// (e.g. due to incomplete fetching by a custom query)
-			// in such cases we will add the additional data from the next record.
+			knownObjects.mappedWithQueryResult(internalId, queryResult);
+		} else if (knownObjects.alreadyMappedInPreviousRecord(internalId) || hasMoreFields(queryResult.asMap(), knownObjects.getQueryResultsFor(internalId))) {
+			// If the object were created in a run before or from a different path that represents another projection,
+			// it _could_ have missing relationships and properties.
+			// In such cases, we will add the additional data from the next record.
 			// This can and should only work for
-			// 1. mutable owning types
+			// 1. Mutable owning types
 			// AND (!!!)
-			// 2. mutable target types
+			// 2. Mutable target types
 			// because we cannot just create new instances
 			populateProperties(queryResult, (Neo4jPersistentEntity<ET>) genericTargetNodeDescription, nodeDescription, internalId, mappedObject, lastMappedEntity, relationshipsFromResult, nodesFromResult, true);
 		}
 		// due to a needed side effect in `populateProperties`, the entity might have been changed
 		return getMostCurrentInstance(internalId, mappedObject);
 	}
 
+	private boolean hasMoreFields(Map<String, Object> currentQueryResult, Set<Map<String, Object>> savedQueryResults) {
+		if (savedQueryResults.isEmpty()) {
+			return true;
+		}
+		Set<String> currentFields = new HashSet<>(currentQueryResult.keySet());
+		Set<String> alreadyProcessedFields = new HashSet<>();
+
+		for (Map<String, Object> savedQueryResult : savedQueryResults) {
+			alreadyProcessedFields.addAll(savedQueryResult.keySet());
+		}
+		currentFields.removeAll(alreadyProcessedFields);
+		return !currentFields.isEmpty();
+	}
+
 	@Nullable
 	private <ET> ET getMostCurrentInstance(String internalId, ET fallbackInstance) {
 		return (ET) (knownObjects.getObject(internalId) != null ? knownObjects.getObject(internalId) : fallbackInstance);
@@ -383,21 +400,19 @@ private <ET> void populateProperties(MapAccessor queryResult, Neo4jPersistentEnt
 		Predicate<Neo4jPersistentProperty> isConstructorParameter = concreteNodeDescription
 				.getInstanceCreatorMetadata()::isCreatorParameter;
 
-		// if the object were mapped before, we assume that at least all properties are populated
-		if (!objectAlreadyMapped) {
-			boolean isKotlinType = KotlinDetector.isKotlinType(concreteNodeDescription.getType());
-			// Fill simple properties
-			PropertyHandler<Neo4jPersistentProperty> handler = populateFrom(queryResult, propertyAccessor,
-					isConstructorParameter, nodeDescriptionAndLabels.getDynamicLabels(), lastMappedEntity, isKotlinType);
-			PropertyHandlerSupport.of(concreteNodeDescription).doWithProperties(handler);
-		}
+		boolean isKotlinType = KotlinDetector.isKotlinType(concreteNodeDescription.getType());
+		// Fill simple properties
+		PropertyHandler<Neo4jPersistentProperty> handler = populateFrom(queryResult, propertyAccessor,
+				isConstructorParameter, nodeDescriptionAndLabels.getDynamicLabels(), lastMappedEntity, isKotlinType, objectAlreadyMapped);
+		PropertyHandlerSupport.of(concreteNodeDescription).doWithProperties(handler);
 		// in a cyclic graph / with bidirectional relationships, we could end up in a state in which we
 		// reference the start again. Because it is getting still constructed, it won't be in the knownObjects
 		// store unless we temporarily put it there.
 		knownObjects.storeObject(internalId, propertyAccessor.getBean());
+		knownObjects.mappedWithQueryResult(internalId, queryResult);
 
 		AssociationHandlerSupport.of(concreteNodeDescription).doWithAssociations(
-				populateFrom(queryResult, baseNodeDescription, propertyAccessor, isConstructorParameter, objectAlreadyMapped, relationshipsFromResult, nodesFromResult));
+				populateFrom(queryResult, baseNodeDescription, propertyAccessor, isConstructorParameter, objectAlreadyMapped, relationshipsFromResult, nodesFromResult, internalId));
 	}
 
 	@NonNull
@@ -499,22 +514,25 @@ public <T> T getParameterValue(Parameter<T, Neo4jPersistentProperty> parameter)
 
 	private PropertyHandler<Neo4jPersistentProperty> populateFrom(MapAccessor queryResult,
 			  PersistentPropertyAccessor<?> propertyAccessor, Predicate<Neo4jPersistentProperty> isConstructorParameter,
-			  Collection<String> surplusLabels, @Nullable Object targetNode, boolean ownerIsKotlinType) {
+			  Collection<String> surplusLabels, @Nullable Object targetNode, boolean ownerIsKotlinType, boolean objectAlreadyMapped) {
 
 		return property -> {
 			if (isConstructorParameter.test(property)) {
 				return;
 			}
 
 			TypeInformation<?> typeInformation = property.getTypeInformation();
-			if (property.isDynamicLabels()) {
-				propertyAccessor.setProperty(property,
-						createDynamicLabelsProperty(typeInformation, surplusLabels));
-			} else if (property.isAnnotationPresent(TargetNode.class)) {
-				if (queryResult instanceof Relationship) {
-					propertyAccessor.setProperty(property, targetNode);
+			if (!objectAlreadyMapped) {
+				if (property.isDynamicLabels()) {
+					propertyAccessor.setProperty(property,
+							createDynamicLabelsProperty(typeInformation, surplusLabels));
+				} else if (property.isAnnotationPresent(TargetNode.class)) {
+					if (queryResult instanceof Relationship) {
+						propertyAccessor.setProperty(property, targetNode);
+					}
 				}
-			} else {
+			}
+			if (!property.isDynamicLabels() && !property.isAnnotationPresent(TargetNode.class)) {
 				Object value = conversionService.readValue(extractValueOf(property, queryResult), typeInformation, property.getOptionalConverter());
 				if (value != null) {
 					Class<?> rawType = typeInformation.getType();
@@ -532,7 +550,7 @@ private static Object getValueOrDefault(boolean ownerIsKotlinType, Class<?> rawT
 
 	private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor queryResult, NodeDescription<?> baseDescription,
 			PersistentPropertyAccessor<?> propertyAccessor, Predicate<Neo4jPersistentProperty> isConstructorParameter,
-		    boolean objectAlreadyMapped, Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult) {
+		    boolean objectAlreadyMapped, Collection<Relationship> relationshipsFromResult, Collection<Node> nodesFromResult, String internalId) {
 
 		return association -> {
 
@@ -555,15 +573,15 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
 
 			boolean propertyValueNotNull = propertyValue != null;
 
-			boolean populatedCollection = persistentProperty.isCollectionLike()
+			boolean populatedCollection = objectAlreadyMapped && persistentProperty.isCollectionLike()
 					&& propertyValueNotNull
 					&& !((Collection<?>) propertyValue).isEmpty();
 
-			boolean populatedMap = persistentProperty.isMap()
+			boolean populatedMap = objectAlreadyMapped && persistentProperty.isMap()
 					&& propertyValueNotNull
 					&& !((Map<?, ?>) propertyValue).isEmpty();
 
-			boolean populatedScalarValue = !persistentProperty.isCollectionLike() && !persistentProperty.isMap()
+			boolean populatedScalarValue = objectAlreadyMapped && !persistentProperty.isCollectionLike() && !persistentProperty.isMap()
 					&& propertyValueNotNull;
 
 			if (populatedCollection) {
@@ -592,6 +610,7 @@ private AssociationHandler<Neo4jPersistentProperty> populateFrom(MapAccessor que
 
 			createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, relationshipsFromResult, nodesFromResult)
 					.ifPresent(value -> propertyAccessor.setProperty(persistentProperty, value));
+
 		};
 	}
 
@@ -903,6 +922,7 @@ static class KnownObjects {
 		private final Set<String> idsInCreation = new HashSet<>();
 
 		private final Map<String, Integer> processedRelationships = new HashMap<>();
+		private final Map<String, Set<Map<String, Object>>> mappedQueryResults = new HashMap<>();
 
 		private void storeObject(@Nullable String internalId, Object object) {
 			if (internalId == null) {
@@ -1029,5 +1049,24 @@ private void nextRecord() {
 			previousRecords.addAll(internalCurrentRecord.keySet());
 			internalCurrentRecord.clear();
 		}
+
+		private void mappedWithQueryResult(String internalId, MapAccessor queryResult) {
+			try {
+				write.lock();
+				mappedQueryResults.computeIfAbsent(internalId, id -> ConcurrentHashMap.newKeySet())
+						.add(queryResult.asMap());
+			} finally {
+				write.unlock();
+			}
+		}
+
+		private Set<Map<String, Object>> getQueryResultsFor(String internalId) {
+			try {
+				read.lock();
+				return mappedQueryResults.get(internalId);
+			} finally {
+				read.unlock();
+			}
+		}
 	}
 }

src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java

@@ -145,6 +145,8 @@
 import org.springframework.data.neo4j.integration.issues.gh2639.Sales;
 import org.springframework.data.neo4j.integration.issues.gh2819.GH2819Model;
 import org.springframework.data.neo4j.integration.issues.gh2819.GH2819Repository;
+import org.springframework.data.neo4j.integration.issues.gh2858.GH2858;
+import org.springframework.data.neo4j.integration.issues.gh2858.GH2858Repository;
 import org.springframework.data.neo4j.integration.issues.qbe.A;
 import org.springframework.data.neo4j.integration.issues.qbe.ARepository;
 import org.springframework.data.neo4j.integration.issues.qbe.B;
@@ -1124,6 +1126,42 @@ void inheritanceAndProjectionShouldMapRelatedNodesCorrectly(@Autowired GH2819Rep
 
 	}
 
+	@Test
+	@Tag("GH-2858")
+	void hydrateProjectionReachableViaMultiplePaths(@Autowired GH2858Repository repository) {
+		GH2858 entity = new GH2858();
+		entity.name = "rootEntity";
+
+		GH2858 friend1 = new GH2858();
+		friend1.name = "friend1";
+
+		GH2858 friendAndRelative = new GH2858();
+		friendAndRelative.name = "friendAndRelative";
+
+		// root -> friend1 -> friendAndRelative
+		//   \                   /|
+		//    -------------------
+		friend1.friends = List.of(friendAndRelative);
+		entity.relatives = List.of(friendAndRelative);
+		entity.friends = List.of(friend1);
+
+		GH2858 savedEntity = repository.save(entity);
+
+		GH2858.GH2858Projection projection = repository.findOneByName(savedEntity.name);
+
+		assertThat(projection.getFriends()).hasSize(1);
+		assertThat(projection.getRelatives()).hasSize(1);
+
+		GH2858.GH2858Projection.Friend loadedFriend = projection.getFriends().get(0);
+		assertThat(loadedFriend.getName()).isEqualTo("friend1");
+		assertThat(loadedFriend.getFriends()).hasSize(1);
+		GH2858.GH2858Projection.KnownPerson friendOfFriend = loadedFriend.getFriends().get(0);
+		assertThat(friendOfFriend.getName()).isEqualTo("friendAndRelative");
+
+		assertThat(projection.getRelatives().get(0).getName()).isEqualTo(friendOfFriend.getName());
+
+	}
+
 	@Configuration
 	@EnableTransactionManagement
 	@EnableNeo4jRepositories(namedQueriesLocation = "more-custom-queries.properties")

src/test/java/org/springframework/data/neo4j/integration/issues/gh2858/GH2858.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright 2011-2024 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.issues.gh2858;
+
+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;
+
+import java.util.List;
+
+/**
+ * @author Gerrit Meier
+ */
+@Node
+public class GH2858 {
+
+	@Id
+	@GeneratedValue
+	public String id;
+
+	public String name;
+
+	@Relationship("FRIEND_WITH")
+	public List<GH2858> friends;
+
+	@Relationship("RELATED_TO")
+	public List<GH2858> relatives;
+
+	/**
+	 * Projection of GH2858 entity
+	 */
+	public interface GH2858Projection {
+		String getName();
+		List<Friend> getFriends();
+		List<KnownPerson> getRelatives();
+
+		/**
+		 * Additional projection with just the name field.
+		 */
+		interface KnownPerson {
+			String getName();
+		}
+
+		/**
+		 * Additional projection with name field and friends relationship.
+		 */
+		interface Friend {
+			String getName();
+			List<KnownPerson> getFriends();
+		}
+	}
+
+}

src/test/java/org/springframework/data/neo4j/integration/issues/gh2858/GH2858Repository.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2011-2024 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.issues.gh2858;
+
+import org.springframework.data.neo4j.repository.Neo4jRepository;
+
+/**
+ * @author Gerrit Meier
+ */
+public interface GH2858Repository extends Neo4jRepository<GH2858, String> {
+
+	GH2858.GH2858Projection findOneByName(String name);
+}