Skip to content

Commit b3b0505

Browse files
michael-simonsmichael-simons
committed
GH-2326 - Fall back to individual statements when a heterogeneous
collection is persisted. In case a heterogeneous is persisted via `saveAll` on a repository or any of the templates, we do fall back to individual statements to persist its content the same way we handle dynamic labels, versioned entities or entities with generated ids. The reason for this is simple: We need the exact entity to determine labels in an inheritance hierachy, otherwise only the information given by the lowest common denominator will be considered, effectively leading to `saveAll` behaving different than `save`. This fixes #2326.
1 parent 865946a commit b3b0505

File tree

7 files changed

+322
-25
lines changed

7 files changed

+322
-25
lines changed

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

+11-12
Original file line numberDiff line numberDiff line change
@@ -297,22 +297,21 @@ public <T> List<T> saveAll(Iterable<T> instances) {
297297

298298
String databaseName = getDatabaseName();
299299

300-
List<T> entities;
301-
if (instances instanceof Collection) {
302-
entities = new ArrayList<>((Collection<T>) instances);
303-
} else {
304-
entities = new ArrayList<>();
305-
instances.forEach(entities::add);
306-
}
300+
Set<Class<?>> types = new HashSet<>();
301+
List<T> entities = new ArrayList<>();
302+
instances.forEach(instance -> {
303+
entities.add(instance);
304+
types.add(instance.getClass());
305+
});
307306

308307
if (entities.isEmpty()) {
309308
return Collections.emptyList();
310309
}
311310

312-
Class<T> domainClass = (Class<T>) TemplateSupport.findCommonElementType(entities);
313-
Assert.notNull(domainClass, "Could not determine common domain class to save.");
314-
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(domainClass);
315-
if (entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
311+
boolean heterogeneousCollection = types.size() > 1;
312+
Class<T> domainClass = (Class<T>) types.iterator().next();
313+
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getRequiredPersistentEntity(domainClass);
314+
if (heterogeneousCollection || entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
316315
|| entityMetaData.getDynamicLabelsProperty().isPresent()) {
317316
log.debug("Saving entities using single statements.");
318317

@@ -332,7 +331,7 @@ class Tuple3<T> {
332331
}
333332

334333
List<Tuple3<T>> entitiesToBeSaved = entities.stream()
335-
.map(e -> new Tuple3<>(e, neo4jMappingContext.getPersistentEntity(e.getClass()).isNew(e), eventSupport.maybeCallBeforeBind(e)))
334+
.map(e -> new Tuple3<>(e, entityMetaData.isNew(e), eventSupport.maybeCallBeforeBind(e)))
336335
.collect(Collectors.toList());
337336

338337
// Save roots

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

+12-13
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Collection;
2929
import java.util.Collections;
3030
import java.util.HashMap;
31+
import java.util.HashSet;
3132
import java.util.LinkedHashSet;
3233
import java.util.List;
3334
import java.util.Map;
@@ -293,24 +294,22 @@ private <T> Mono<Tuple2<T, DynamicLabels>> determineDynamicLabels(T entityToBeSa
293294
@Override
294295
public <T> Flux<T> saveAll(Iterable<T> instances) {
295296

296-
List<T> entities;
297-
if (instances instanceof Collection) {
298-
entities = new ArrayList<>((Collection<T>) instances);
299-
} else {
300-
entities = new ArrayList<>();
301-
instances.forEach(entities::add);
302-
}
297+
Set<Class<?>> types = new HashSet<>();
298+
List<T> entities = new ArrayList<>();
299+
instances.forEach(instance -> {
300+
entities.add(instance);
301+
types.add(instance.getClass());
302+
});
303303

304304
if (entities.isEmpty()) {
305305
return Flux.empty();
306306
}
307307

308-
Class<T> domainClass = (Class<T>) TemplateSupport.findCommonElementType(entities);
309-
Assert.notNull(domainClass, "Could not determine common domain class to save.");
310-
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(domainClass);
311-
312-
if (entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
313-
|| entityMetaData.getDynamicLabelsProperty().isPresent()) {
308+
boolean heterogeneousCollection = types.size() > 1;
309+
Class<T> domainClass = (Class<T>) types.iterator().next();
310+
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getRequiredPersistentEntity(domainClass);
311+
if (heterogeneousCollection || entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
312+
|| entityMetaData.getDynamicLabelsProperty().isPresent()) {
314313
log.debug("Saving entities using single statements.");
315314

316315
return getDatabaseName().flatMapMany(

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2326/Animal.java

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
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.issues.gh2326;
17+
18+
import org.springframework.data.neo4j.core.schema.Node;
19+
20+
/**
21+
* @author Michael J. Simons
22+
*/
23+
@Node
24+
public abstract class Animal extends BaseEntity {
25+
26+
/**
27+
* Provides label `Pet`
28+
*/
29+
@Node
30+
public static abstract class Pet extends Animal {
31+
32+
/**
33+
* Provides label `Cat`
34+
*/
35+
@Node
36+
public static class Cat extends Pet {
37+
}
38+
39+
/**
40+
* Provides label `Dog`
41+
*/
42+
@Node
43+
public static class Dog extends Pet {
44+
}
45+
}
46+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2326/BaseEntity.java

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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.issues.gh2326;
17+
18+
import org.springframework.data.neo4j.core.schema.GeneratedValue;
19+
import org.springframework.data.neo4j.core.schema.Id;
20+
import org.springframework.data.neo4j.core.support.UUIDStringGenerator;
21+
22+
/**
23+
* @author Michael J. Simons
24+
*/
25+
public abstract class BaseEntity {
26+
27+
@Id @GeneratedValue(UUIDStringGenerator.class)
28+
private String id;
29+
30+
public String getId() {
31+
return id;
32+
}
33+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2326/GH2326IT.java

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
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.issues.gh2326;
17+
18+
import java.util.Arrays;
19+
import java.util.List;
20+
import java.util.stream.Collectors;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.neo4j.driver.Driver;
24+
import org.springframework.beans.factory.annotation.Autowired;
25+
import org.springframework.context.annotation.Bean;
26+
import org.springframework.context.annotation.Configuration;
27+
import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
28+
import org.springframework.data.neo4j.repository.Neo4jRepository;
29+
import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories;
30+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
31+
import org.springframework.transaction.annotation.EnableTransactionManagement;
32+
33+
/**
34+
* @author Michael J. Simons
35+
*/
36+
@Neo4jIntegrationTest
37+
class GH2326IT extends TestBase {
38+
39+
@Test // GH-2326
40+
void saveShouldAddAllLabels(@Autowired AnimalRepository animalRepository) {
41+
42+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
43+
List<String> ids = animals.stream().map(animalRepository::save).map(BaseEntity::getId)
44+
.collect(Collectors.toList());
45+
46+
assertLabels(ids);
47+
}
48+
49+
@Test // GH-2326
50+
void saveAllShouldAddAllLabels(@Autowired AnimalRepository animalRepository) {
51+
52+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
53+
List<String> ids = animalRepository.saveAll(animals).stream().map(BaseEntity::getId)
54+
.collect(Collectors.toList());
55+
56+
assertLabels(ids);
57+
}
58+
59+
public interface AnimalRepository extends Neo4jRepository<Animal, String> {
60+
}
61+
62+
@Configuration
63+
@EnableTransactionManagement
64+
@EnableNeo4jRepositories(considerNestedRepositories = true)
65+
static class Config extends AbstractNeo4jConfig {
66+
67+
@Bean
68+
public Driver driver() {
69+
70+
return neo4jConnectionSupport.getDriver();
71+
}
72+
}
73+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2326/ReactiveGH2326IT.java

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
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.issues.gh2326;
17+
18+
import reactor.core.publisher.Flux;
19+
import reactor.test.StepVerifier;
20+
21+
import java.util.ArrayList;
22+
import java.util.Arrays;
23+
import java.util.List;
24+
25+
import org.junit.jupiter.api.Test;
26+
import org.neo4j.driver.Driver;
27+
import org.springframework.beans.factory.annotation.Autowired;
28+
import org.springframework.context.annotation.Bean;
29+
import org.springframework.context.annotation.Configuration;
30+
import org.springframework.data.neo4j.config.AbstractReactiveNeo4jConfig;
31+
import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository;
32+
import org.springframework.data.neo4j.repository.config.EnableReactiveNeo4jRepositories;
33+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
34+
import org.springframework.transaction.annotation.EnableTransactionManagement;
35+
36+
/**
37+
* @author Michael J. Simons
38+
*/
39+
@Neo4jIntegrationTest
40+
class ReactiveGH2326IT extends TestBase {
41+
42+
@Test // GH-2326
43+
void saveShouldAddAllLabels(@Autowired AnimalRepository animalRepository) {
44+
45+
List<String> ids = new ArrayList<>();
46+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
47+
Flux.fromIterable(animals).flatMap(animalRepository::save)
48+
.map(BaseEntity::getId)
49+
.as(StepVerifier::create)
50+
.recordWith(() -> ids)
51+
.expectNextCount(2)
52+
.verifyComplete();
53+
54+
assertLabels(ids);
55+
}
56+
57+
@Test // GH-2326
58+
void saveAllShouldAddAllLabels(@Autowired AnimalRepository animalRepository) {
59+
60+
List<String> ids = new ArrayList<>();
61+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
62+
animalRepository.saveAll(animals)
63+
.map(BaseEntity::getId)
64+
.as(StepVerifier::create)
65+
.recordWith(() -> ids)
66+
.expectNextCount(2)
67+
.verifyComplete();
68+
69+
assertLabels(ids);
70+
}
71+
72+
public interface AnimalRepository extends ReactiveNeo4jRepository<Animal, String> {
73+
}
74+
75+
@Configuration
76+
@EnableTransactionManagement
77+
@EnableReactiveNeo4jRepositories(considerNestedRepositories = true)
78+
static class Config extends AbstractReactiveNeo4jConfig {
79+
80+
@Bean
81+
public Driver driver() {
82+
83+
return neo4jConnectionSupport.getDriver();
84+
}
85+
}
86+
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2326/TestBase.java

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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.issues.gh2326;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
20+
import java.util.Collections;
21+
import java.util.List;
22+
23+
import org.junit.jupiter.api.BeforeAll;
24+
import org.neo4j.driver.Session;
25+
import org.neo4j.driver.Transaction;
26+
import org.neo4j.driver.Value;
27+
import org.springframework.data.neo4j.test.Neo4jExtension;
28+
29+
/**
30+
* @author Michael J. Simons
31+
*/
32+
abstract class TestBase {
33+
34+
protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport;
35+
36+
@BeforeAll
37+
protected static void setupData() {
38+
try (Session session = neo4jConnectionSupport.getDriver().session();
39+
Transaction transaction = session.beginTransaction();
40+
) {
41+
transaction.run("MATCH (n) detach delete n");
42+
transaction.commit();
43+
}
44+
}
45+
46+
protected static void assertLabels(List<String> ids) {
47+
try (Session session = neo4jConnectionSupport.getDriver().session()) {
48+
for (String id : ids) {
49+
List<String> labels = session.readTransaction(
50+
tx -> tx.run("MATCH (n) WHERE n.id = $id RETURN labels(n)", Collections.singletonMap("id", id))
51+
.single().get(0).asList(
52+
Value::asString));
53+
assertThat(labels)
54+
.hasSize(3)
55+
.contains("Animal", "Pet")
56+
.containsAnyOf("Dog", "Cat");
57+
58+
}
59+
}
60+
}
61+
}

0 commit comments

Comments
 (0)