diff --git a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java index 58682ab80..e736b1cd9 100644 --- a/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java +++ b/src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java @@ -1195,7 +1195,6 @@ public final class Neo4jTemplate implements return NodesAndRelationshipsByIdStatementProvider.EMPTY; } // load first level relationships -// final Set relationshipIds = new HashSet<>(); final Map> relationshipsToRelatedNodeIds = new HashMap<>(); for (RelationshipDescription relationshipDescription : entityMetaData.getRelationshipsInHierarchy(queryFragments::includeField)) { diff --git a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java index e45a6245c..4ca845a95 100644 --- a/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java +++ b/src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java @@ -25,6 +25,7 @@ import java.util.List; 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 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { // save final state of the bean knownObjects.storeObject(internalId, bean); + knownObjects.mappedWithQueryResult(internalId, queryResult); return bean; }; @@ -342,14 +344,15 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { 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) genericTargetNodeDescription, nodeDescription, internalId, mappedObject, lastMappedEntity, relationshipsFromResult, nodesFromResult, true); } @@ -357,6 +360,20 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { return getMostCurrentInstance(internalId, mappedObject); } + private boolean hasMoreFields(Map currentQueryResult, Set> savedQueryResults) { + if (savedQueryResults.isEmpty()) { + return true; + } + Set currentFields = new HashSet<>(currentQueryResult.keySet()); + Set alreadyProcessedFields = new HashSet<>(); + + for (Map savedQueryResult : savedQueryResults) { + alreadyProcessedFields.addAll(savedQueryResult.keySet()); + } + currentFields.removeAll(alreadyProcessedFields); + return !currentFields.isEmpty(); + } + @Nullable private ET getMostCurrentInstance(String internalId, ET fallbackInstance) { return (ET) (knownObjects.getObject(internalId) != null ? knownObjects.getObject(internalId) : fallbackInstance); @@ -383,21 +400,19 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { Predicate 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 handler = populateFrom(queryResult, propertyAccessor, - isConstructorParameter, nodeDescriptionAndLabels.getDynamicLabels(), lastMappedEntity, isKotlinType); - PropertyHandlerSupport.of(concreteNodeDescription).doWithProperties(handler); - } + boolean isKotlinType = KotlinDetector.isKotlinType(concreteNodeDescription.getType()); + // Fill simple properties + PropertyHandler 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,7 +514,7 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { private PropertyHandler populateFrom(MapAccessor queryResult, PersistentPropertyAccessor propertyAccessor, Predicate isConstructorParameter, - Collection surplusLabels, @Nullable Object targetNode, boolean ownerIsKotlinType) { + Collection surplusLabels, @Nullable Object targetNode, boolean ownerIsKotlinType, boolean objectAlreadyMapped) { return property -> { if (isConstructorParameter.test(property)) { @@ -507,14 +522,17 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { } 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 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { private AssociationHandler populateFrom(MapAccessor queryResult, NodeDescription baseDescription, PersistentPropertyAccessor propertyAccessor, Predicate isConstructorParameter, - boolean objectAlreadyMapped, Collection relationshipsFromResult, Collection nodesFromResult) { + boolean objectAlreadyMapped, Collection relationshipsFromResult, Collection nodesFromResult, String internalId) { return association -> { @@ -555,15 +573,15 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { 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 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { createInstanceOfRelationships(persistentProperty, queryResult, (RelationshipDescription) association, baseDescription, relationshipsFromResult, nodesFromResult) .ifPresent(value -> propertyAccessor.setProperty(persistentProperty, value)); + }; } @@ -903,6 +922,7 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { private final Set idsInCreation = new HashSet<>(); private final Map processedRelationships = new HashMap<>(); + private final Map>> mappedQueryResults = new HashMap<>(); private void storeObject(@Nullable String internalId, Object object) { if (internalId == null) { @@ -1029,5 +1049,24 @@ final class DefaultNeo4jEntityConverter implements Neo4jEntityConverter { 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> getQueryResultsFor(String internalId) { + try { + read.lock(); + return mappedQueryResults.get(internalId); + } finally { + read.unlock(); + } + } } } diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java b/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java index 50aac5f6f..38af06af2 100644 --- a/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java +++ b/src/test/java/org/springframework/data/neo4j/integration/issues/IssuesIT.java @@ -145,6 +145,8 @@ import org.springframework.data.neo4j.integration.issues.gh2639.ProgrammingLangu 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 @@ class IssuesIT extends TestBase { } + @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") diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2858/GH2858.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2858/GH2858.java new file mode 100644 index 000000000..688d703d7 --- /dev/null +++ b/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 friends; + + @Relationship("RELATED_TO") + public List relatives; + + /** + * Projection of GH2858 entity + */ + public interface GH2858Projection { + String getName(); + List getFriends(); + List 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 getFriends(); + } + } + +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/issues/gh2858/GH2858Repository.java b/src/test/java/org/springframework/data/neo4j/integration/issues/gh2858/GH2858Repository.java new file mode 100644 index 000000000..9c0d8a0db --- /dev/null +++ b/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.GH2858Projection findOneByName(String name); +}