Skip to content

Commit b3dfbba

Browse files
michael-simonsmeistermeier
michael-simons
authored and
meistermeier
committed
GH-2236 - Create correct collection type for RelationshipProperties.
This fixes #2236.
1 parent 75c7f12 commit b3dfbba

File tree

2 files changed

+208
-29
lines changed

2 files changed

+208
-29
lines changed

‎src/main/java/org/springframework/data/neo4j/core/RelationshipHandler.java‎

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@
1515
*/
1616
package org.springframework.data.neo4j.core;
1717

18-
import java.util.Collection;
19-
import java.util.Collections;
20-
import java.util.List;
21-
import java.util.Map;
22-
2318
import org.apiguardian.api.API;
2419
import org.springframework.core.CollectionFactory;
2520
import org.springframework.data.mapping.PersistentPropertyAccessor;
2621
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
2722

23+
import java.util.Collection;
24+
import java.util.Collections;
25+
import java.util.Map;
26+
import java.util.Optional;
27+
2828
/**
2929
* Internal helper class that takes care of tracking whether a related object or a collection of related objects was recreated
3030
* due to changing immutable properties
@@ -45,19 +45,19 @@ enum Cardinality {
4545
static RelationshipHandler forProperty(Neo4jPersistentProperty property, Object rawValue) {
4646

4747
Cardinality cardinality;
48-
Collection<Object> newRelationshipObjectCollection = null;
49-
Map<Object, Object> newRelationshipObjectCollectionMap = null;
48+
Collection<Object> newRelationshipObjectCollection = Collections.emptyList();
49+
Map<Object, Object> newRelationshipObjectCollectionMap = Collections.emptyMap();
5050

5151
// Order is important here, all map based associations are dynamic, but not all dynamic associations are one to many
5252
if (property.isCollectionLike()) {
5353
cardinality = Cardinality.ONE_TO_MANY;
54-
newRelationshipObjectCollection = CollectionFactory.createApproximateCollection(rawValue, ((Collection<?>) rawValue).size());
54+
newRelationshipObjectCollection = CollectionFactory.createCollection(property.getType(), ((Collection<?>) rawValue).size());
5555
} else if (property.isDynamicOneToManyAssociation()) {
5656
cardinality = Cardinality.DYNAMIC_ONE_TO_MANY;
57-
newRelationshipObjectCollectionMap = CollectionFactory.createApproximateMap(rawValue, ((Map<?, ?>) rawValue).size());
57+
newRelationshipObjectCollectionMap = CollectionFactory.createMap(property.getType(), ((Map<?, ?>) rawValue).size());
5858
} else if (property.isDynamicAssociation()) {
5959
cardinality = Cardinality.DYNAMIC_ONE_TO_ONE;
60-
newRelationshipObjectCollectionMap = CollectionFactory.createApproximateMap(rawValue, ((Map<?, ?>) rawValue).size());
60+
newRelationshipObjectCollectionMap = CollectionFactory.createMap(property.getType(), ((Map<?, ?>) rawValue).size());
6161
} else {
6262
cardinality = Cardinality.ONE_TO_ONE;
6363
}
@@ -76,9 +76,9 @@ static RelationshipHandler forProperty(Neo4jPersistentProperty property, Object
7676
private final Map<Object, Object> newRelatedObjectsByType;
7777

7878
RelationshipHandler(Neo4jPersistentProperty property,
79-
Object rawValue, Cardinality cardinality,
80-
Collection<Object> newRelatedObjects,
81-
Map<Object, Object> newRelatedObjectsByType) {
79+
Object rawValue, Cardinality cardinality,
80+
Collection<Object> newRelatedObjects,
81+
Map<Object, Object> newRelatedObjectsByType) {
8282
this.property = property;
8383
this.rawValue = rawValue;
8484
this.cardinality = cardinality;
@@ -87,31 +87,33 @@ static RelationshipHandler forProperty(Neo4jPersistentProperty property, Object
8787
}
8888

8989
void handle(Object relatedValueToStore, Object newRelatedObject, Object potentiallyRecreatedRelatedObject) {
90-
if (potentiallyRecreatedRelatedObject == newRelatedObject) {
91-
return;
92-
} else if (cardinality == Cardinality.ONE_TO_ONE) {
93-
this.newRelatedObjects = Collections.singletonList(potentiallyRecreatedRelatedObject);
94-
} else if (cardinality == Cardinality.ONE_TO_MANY) {
95-
newRelatedObjects.add(potentiallyRecreatedRelatedObject);
96-
} else {
97-
Object key = ((Map.Entry<Object, Object>) relatedValueToStore).getKey();
98-
if (cardinality == Cardinality.DYNAMIC_ONE_TO_ONE) {
99-
newRelatedObjectsByType.put(key, potentiallyRecreatedRelatedObject);
90+
91+
if (potentiallyRecreatedRelatedObject != newRelatedObject) {
92+
if (cardinality == Cardinality.ONE_TO_ONE) {
93+
this.newRelatedObjects = Collections.singletonList(potentiallyRecreatedRelatedObject);
94+
} else if (cardinality == Cardinality.ONE_TO_MANY) {
95+
newRelatedObjects.add(potentiallyRecreatedRelatedObject);
10096
} else {
101-
Collection<Object> newCollection = (Collection<Object>) newRelatedObjectsByType
102-
.computeIfAbsent(key, k -> CollectionFactory.createCollection(
103-
property.getTypeInformation().getRequiredActualType().getType(),
104-
((Collection) ((Map) rawValue).get(key)).size()));
105-
newCollection.add(potentiallyRecreatedRelatedObject);
97+
Object key = ((Map.Entry<Object, Object>) relatedValueToStore).getKey();
98+
if (cardinality == Cardinality.DYNAMIC_ONE_TO_ONE) {
99+
newRelatedObjectsByType.put(key, potentiallyRecreatedRelatedObject);
100+
} else {
101+
Collection<Object> newCollection = (Collection<Object>) newRelatedObjectsByType
102+
.computeIfAbsent(key, k -> CollectionFactory.createCollection(
103+
property.getTypeInformation().getRequiredActualType().getType(),
104+
((Collection) ((Map) rawValue).get(key)).size()));
105+
newCollection.add(potentiallyRecreatedRelatedObject);
106+
}
106107
}
107108
}
108109
}
109110

110111
void applyFinalResultToOwner(PersistentPropertyAccessor<?> parentPropertyAccessor) {
112+
111113
Object finalRelation = null;
112114
switch (cardinality) {
113115
case ONE_TO_ONE:
114-
finalRelation = newRelatedObjects == null ? null : ((List) newRelatedObjects).get(0);
116+
finalRelation = Optional.ofNullable(newRelatedObjects).flatMap(v -> v.stream().findFirst()).orElse(null);
115117
break;
116118
case ONE_TO_MANY:
117119
if (!newRelatedObjects.isEmpty()) {

‎src/test/java/org/springframework/data/neo4j/integration/imperative/CollectionsIT.java‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.integration.imperative;
17+
18+
import org.junit.jupiter.api.Test;
19+
import org.neo4j.driver.Driver;
20+
import org.neo4j.driver.Session;
21+
import org.springframework.beans.factory.annotation.Autowired;
22+
import org.springframework.context.annotation.Bean;
23+
import org.springframework.context.annotation.Configuration;
24+
import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
25+
import org.springframework.data.neo4j.core.Neo4jTemplate;
26+
import org.springframework.data.neo4j.core.convert.Neo4jConversions;
27+
import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
28+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
29+
import org.springframework.data.neo4j.core.schema.Id;
30+
import org.springframework.data.neo4j.core.schema.Node;
31+
import org.springframework.data.neo4j.core.schema.RelationshipProperties;
32+
import org.springframework.data.neo4j.core.schema.TargetNode;
33+
import org.springframework.data.neo4j.test.Neo4jExtension;
34+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
35+
import org.springframework.transaction.annotation.EnableTransactionManagement;
36+
37+
import java.util.Arrays;
38+
import java.util.Collections;
39+
import java.util.HashSet;
40+
import java.util.Optional;
41+
import java.util.Set;
42+
43+
import static org.assertj.core.api.Assertions.assertThat;
44+
45+
/**
46+
* @author Michael J. Simons
47+
*/
48+
@Neo4jIntegrationTest
49+
public class CollectionsIT {
50+
51+
private static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
52+
53+
private final Driver driver;
54+
55+
@Autowired
56+
CollectionsIT(Driver driver) {
57+
this.driver = driver;
58+
}
59+
60+
@Test // GH-2236
61+
void loadingOfRelPropertiesInSetsShouldWork(@Autowired Neo4jTemplate repository) {
62+
63+
Long id;
64+
try (Session session = driver.session()) {
65+
id = session.run(
66+
"CREATE (c:CollectionChildNodeA {name: 'The Child'}) <- [:CHILDREN_WITH_PROPERTIES {prop: 'The Property'}] - (p:CollectionParentNode {name: 'The Parent'}) RETURN id(p)"
67+
).single().get(0).asLong();
68+
}
69+
70+
Optional<CollectionParentNode> optionalParent = repository.findById(id, CollectionParentNode.class);
71+
72+
assertThat(optionalParent).hasValueSatisfying(parent -> {
73+
assertThat(parent.id).isNotNull();
74+
assertThat(parent.name).isEqualTo("The Parent");
75+
assertThat(parent.childrenWithProperties).isNotNull();
76+
assertThat(parent.childrenWithProperties).first().satisfies(p -> {
77+
assertThat(p.target.id).isNotNull();
78+
assertThat(p.target.name).isEqualTo("The Child");
79+
assertThat(p.prop).isEqualTo("The Property");
80+
});
81+
});
82+
}
83+
84+
@Test // GH-2236
85+
void storingOfRelPropertiesInSetsShouldWork(@Autowired Neo4jTemplate template) {
86+
87+
CollectionParentNode parent = new CollectionParentNode("parent");
88+
parent.childrenWithProperties.add(new RelProperties(new CollectionChildNodeA("child"), "a property"));
89+
90+
parent = template.save(parent);
91+
assertThat(parent.id).isNotNull();
92+
assertThat(parent.childrenWithProperties).isNotNull();
93+
assertThat(parent.childrenWithProperties).first().satisfies(p -> {
94+
assertThat(p.target.id).isNotNull();
95+
assertThat(p.target.name).isEqualTo("child");
96+
assertThat(p.prop).isEqualTo("a property");
97+
});
98+
99+
100+
try (Session session = driver.session()) {
101+
long cnt = session.run(
102+
"MATCH (c:CollectionChildNodeA) <- [:CHILDREN_WITH_PROPERTIES] - (p:CollectionParentNode) WHERE id(p) = $id RETURN count(c) ",
103+
Collections.singletonMap("id", parent.id)
104+
).single().get(0).asLong();
105+
assertThat(cnt).isEqualTo(1L);
106+
}
107+
}
108+
109+
@Node
110+
static class CollectionParentNode {
111+
112+
@Id
113+
@GeneratedValue
114+
Long id;
115+
116+
final String name;
117+
118+
Set<RelProperties> childrenWithProperties = new HashSet<>();
119+
120+
CollectionParentNode(String name) {
121+
this.name = name;
122+
}
123+
}
124+
125+
@Node
126+
static class CollectionChildNodeA {
127+
128+
@Id
129+
@GeneratedValue
130+
Long id;
131+
132+
final String name;
133+
134+
CollectionChildNodeA(String name) {
135+
this.name = name;
136+
}
137+
}
138+
139+
@RelationshipProperties
140+
static class RelProperties {
141+
142+
@Id
143+
@GeneratedValue
144+
Long id;
145+
146+
@TargetNode
147+
final CollectionChildNodeA target;
148+
149+
final String prop;
150+
151+
RelProperties(CollectionChildNodeA target, String prop) {
152+
this.target = target;
153+
this.prop = prop;
154+
}
155+
}
156+
157+
@Configuration
158+
@EnableTransactionManagement
159+
static class Config extends AbstractNeo4jConfig {
160+
161+
@Bean
162+
public Driver driver() {
163+
return neo4jConnectionSupport.getDriver();
164+
}
165+
166+
@Override
167+
public Neo4jMappingContext neo4jMappingContext(Neo4jConversions neo4JConversions) throws ClassNotFoundException {
168+
169+
// Don't create repositories for the entities, otherwise they must be moved
170+
// to a public reachable place. I didn't want that as the mapping context is polluted already
171+
// enough with the shared package of nodes.
172+
Neo4jMappingContext ctx = new Neo4jMappingContext(neo4JConversions);
173+
ctx.setInitialEntitySet(new HashSet<>(Arrays.asList(CollectionParentNode.class, CollectionChildNodeA.class, RelProperties.class)));
174+
return ctx;
175+
}
176+
}
177+
}

0 commit comments

Comments
 (0)