diff --git a/src/main/java/org/springframework/data/neo4j/core/RelationshipHandler.java b/src/main/java/org/springframework/data/neo4j/core/RelationshipHandler.java index 9d57313c5..0d62512c0 100644 --- a/src/main/java/org/springframework/data/neo4j/core/RelationshipHandler.java +++ b/src/main/java/org/springframework/data/neo4j/core/RelationshipHandler.java @@ -15,16 +15,16 @@ */ package org.springframework.data.neo4j.core; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; - import org.apiguardian.api.API; import org.springframework.core.CollectionFactory; import org.springframework.data.mapping.PersistentPropertyAccessor; import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.Optional; + /** * Internal helper class that takes care of tracking whether a related object or a collection of related objects was recreated * due to changing immutable properties @@ -45,19 +45,19 @@ final class RelationshipHandler { static RelationshipHandler forProperty(Neo4jPersistentProperty property, Object rawValue) { Cardinality cardinality; - Collection newRelationshipObjectCollection = null; - Map newRelationshipObjectCollectionMap = null; + Collection newRelationshipObjectCollection = Collections.emptyList(); + Map newRelationshipObjectCollectionMap = Collections.emptyMap(); // Order is important here, all map based associations are dynamic, but not all dynamic associations are one to many if (property.isCollectionLike()) { cardinality = Cardinality.ONE_TO_MANY; - newRelationshipObjectCollection = CollectionFactory.createApproximateCollection(rawValue, ((Collection) rawValue).size()); + newRelationshipObjectCollection = CollectionFactory.createCollection(property.getType(), ((Collection) rawValue).size()); } else if (property.isDynamicOneToManyAssociation()) { cardinality = Cardinality.DYNAMIC_ONE_TO_MANY; - newRelationshipObjectCollectionMap = CollectionFactory.createApproximateMap(rawValue, ((Map) rawValue).size()); + newRelationshipObjectCollectionMap = CollectionFactory.createMap(property.getType(), ((Map) rawValue).size()); } else if (property.isDynamicAssociation()) { cardinality = Cardinality.DYNAMIC_ONE_TO_ONE; - newRelationshipObjectCollectionMap = CollectionFactory.createApproximateMap(rawValue, ((Map) rawValue).size()); + newRelationshipObjectCollectionMap = CollectionFactory.createMap(property.getType(), ((Map) rawValue).size()); } else { cardinality = Cardinality.ONE_TO_ONE; } @@ -76,9 +76,9 @@ final class RelationshipHandler { private final Map newRelatedObjectsByType; RelationshipHandler(Neo4jPersistentProperty property, - Object rawValue, Cardinality cardinality, - Collection newRelatedObjects, - Map newRelatedObjectsByType) { + Object rawValue, Cardinality cardinality, + Collection newRelatedObjects, + Map newRelatedObjectsByType) { this.property = property; this.rawValue = rawValue; this.cardinality = cardinality; @@ -87,31 +87,33 @@ final class RelationshipHandler { } void handle(Object relatedValueToStore, Object newRelatedObject, Object potentiallyRecreatedRelatedObject) { - if (potentiallyRecreatedRelatedObject == newRelatedObject) { - return; - } else if (cardinality == Cardinality.ONE_TO_ONE) { - this.newRelatedObjects = Collections.singletonList(potentiallyRecreatedRelatedObject); - } else if (cardinality == Cardinality.ONE_TO_MANY) { - newRelatedObjects.add(potentiallyRecreatedRelatedObject); - } else { - Object key = ((Map.Entry) relatedValueToStore).getKey(); - if (cardinality == Cardinality.DYNAMIC_ONE_TO_ONE) { - newRelatedObjectsByType.put(key, potentiallyRecreatedRelatedObject); + + if (potentiallyRecreatedRelatedObject != newRelatedObject) { + if (cardinality == Cardinality.ONE_TO_ONE) { + this.newRelatedObjects = Collections.singletonList(potentiallyRecreatedRelatedObject); + } else if (cardinality == Cardinality.ONE_TO_MANY) { + newRelatedObjects.add(potentiallyRecreatedRelatedObject); } else { - Collection newCollection = (Collection) newRelatedObjectsByType - .computeIfAbsent(key, k -> CollectionFactory.createCollection( - property.getTypeInformation().getRequiredActualType().getType(), - ((Collection) ((Map) rawValue).get(key)).size())); - newCollection.add(potentiallyRecreatedRelatedObject); + Object key = ((Map.Entry) relatedValueToStore).getKey(); + if (cardinality == Cardinality.DYNAMIC_ONE_TO_ONE) { + newRelatedObjectsByType.put(key, potentiallyRecreatedRelatedObject); + } else { + Collection newCollection = (Collection) newRelatedObjectsByType + .computeIfAbsent(key, k -> CollectionFactory.createCollection( + property.getTypeInformation().getRequiredActualType().getType(), + ((Collection) ((Map) rawValue).get(key)).size())); + newCollection.add(potentiallyRecreatedRelatedObject); + } } } } void applyFinalResultToOwner(PersistentPropertyAccessor parentPropertyAccessor) { + Object finalRelation = null; switch (cardinality) { case ONE_TO_ONE: - finalRelation = newRelatedObjects == null ? null : ((List) newRelatedObjects).get(0); + finalRelation = Optional.ofNullable(newRelatedObjects).flatMap(v -> v.stream().findFirst()).orElse(null); break; case ONE_TO_MANY: if (!newRelatedObjects.isEmpty()) { diff --git a/src/test/java/org/springframework/data/neo4j/integration/imperative/CollectionsIT.java b/src/test/java/org/springframework/data/neo4j/integration/imperative/CollectionsIT.java new file mode 100644 index 000000000..23ce93534 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/imperative/CollectionsIT.java @@ -0,0 +1,177 @@ +/* + * Copyright 2011-2021 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.imperative; + +import org.junit.jupiter.api.Test; +import org.neo4j.driver.Driver; +import org.neo4j.driver.Session; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.neo4j.config.AbstractNeo4jConfig; +import org.springframework.data.neo4j.core.Neo4jTemplate; +import org.springframework.data.neo4j.core.convert.Neo4jConversions; +import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext; +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.RelationshipProperties; +import org.springframework.data.neo4j.core.schema.TargetNode; +import org.springframework.data.neo4j.test.Neo4jExtension; +import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.transaction.annotation.EnableTransactionManagement; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Michael J. Simons + */ +@Neo4jIntegrationTest +public class CollectionsIT { + + private static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport; + + private final Driver driver; + + @Autowired + CollectionsIT(Driver driver) { + this.driver = driver; + } + + @Test // GH-2236 + void loadingOfRelPropertiesInSetsShouldWork(@Autowired Neo4jTemplate repository) { + + Long id; + try (Session session = driver.session()) { + id = session.run( + "CREATE (c:CollectionChildNodeA {name: 'The Child'}) <- [:CHILDREN_WITH_PROPERTIES {prop: 'The Property'}] - (p:CollectionParentNode {name: 'The Parent'}) RETURN id(p)" + ).single().get(0).asLong(); + } + + Optional optionalParent = repository.findById(id, CollectionParentNode.class); + + assertThat(optionalParent).hasValueSatisfying(parent -> { + assertThat(parent.id).isNotNull(); + assertThat(parent.name).isEqualTo("The Parent"); + assertThat(parent.childrenWithProperties).isNotNull(); + assertThat(parent.childrenWithProperties).first().satisfies(p -> { + assertThat(p.target.id).isNotNull(); + assertThat(p.target.name).isEqualTo("The Child"); + assertThat(p.prop).isEqualTo("The Property"); + }); + }); + } + + @Test // GH-2236 + void storingOfRelPropertiesInSetsShouldWork(@Autowired Neo4jTemplate template) { + + CollectionParentNode parent = new CollectionParentNode("parent"); + parent.childrenWithProperties.add(new RelProperties(new CollectionChildNodeA("child"), "a property")); + + parent = template.save(parent); + assertThat(parent.id).isNotNull(); + assertThat(parent.childrenWithProperties).isNotNull(); + assertThat(parent.childrenWithProperties).first().satisfies(p -> { + assertThat(p.target.id).isNotNull(); + assertThat(p.target.name).isEqualTo("child"); + assertThat(p.prop).isEqualTo("a property"); + }); + + + try (Session session = driver.session()) { + long cnt = session.run( + "MATCH (c:CollectionChildNodeA) <- [:CHILDREN_WITH_PROPERTIES] - (p:CollectionParentNode) WHERE id(p) = $id RETURN count(c) ", + Collections.singletonMap("id", parent.id) + ).single().get(0).asLong(); + assertThat(cnt).isEqualTo(1L); + } + } + + @Node + static class CollectionParentNode { + + @Id + @GeneratedValue + Long id; + + final String name; + + Set childrenWithProperties = new HashSet<>(); + + CollectionParentNode(String name) { + this.name = name; + } + } + + @Node + static class CollectionChildNodeA { + + @Id + @GeneratedValue + Long id; + + final String name; + + CollectionChildNodeA(String name) { + this.name = name; + } + } + + @RelationshipProperties + static class RelProperties { + + @Id + @GeneratedValue + Long id; + + @TargetNode + final CollectionChildNodeA target; + + final String prop; + + RelProperties(CollectionChildNodeA target, String prop) { + this.target = target; + this.prop = prop; + } + } + + @Configuration + @EnableTransactionManagement + static class Config extends AbstractNeo4jConfig { + + @Bean + public Driver driver() { + return neo4jConnectionSupport.getDriver(); + } + + @Override + public Neo4jMappingContext neo4jMappingContext(Neo4jConversions neo4JConversions) throws ClassNotFoundException { + + // Don't create repositories for the entities, otherwise they must be moved + // to a public reachable place. I didn't want that as the mapping context is polluted already + // enough with the shared package of nodes. + Neo4jMappingContext ctx = new Neo4jMappingContext(neo4JConversions); + ctx.setInitialEntitySet(new HashSet<>(Arrays.asList(CollectionParentNode.class, CollectionChildNodeA.class, RelProperties.class))); + return ctx; + } + } +}