Skip to content

Commit db9c3b7

Browse files
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 454647f commit db9c3b7

File tree

7 files changed

+338
-26
lines changed

7 files changed

+338
-26
lines changed

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

+12-13
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@
7474
import org.springframework.data.neo4j.core.mapping.NestedRelationshipProcessingStateMachine;
7575
import org.springframework.data.neo4j.core.mapping.NestedRelationshipProcessingStateMachine.ProcessState;
7676
import org.springframework.data.neo4j.core.mapping.NodeDescription;
77+
import org.springframework.data.neo4j.core.mapping.PropertyFilter;
7778
import org.springframework.data.neo4j.core.mapping.RelationshipDescription;
7879
import org.springframework.data.neo4j.core.mapping.callback.EventSupport;
7980
import org.springframework.data.neo4j.repository.NoResultException;
80-
import org.springframework.data.neo4j.core.mapping.PropertyFilter;
8181
import org.springframework.data.neo4j.repository.query.QueryFragments;
8282
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
8383
import org.springframework.data.projection.ProjectionFactory;
@@ -425,22 +425,21 @@ public <T> List<T> saveAll(Iterable<T> instances) {
425425

426426
private <T> List<T> saveAllImpl(Iterable<T> instances, List<PropertyPath> includedProperties) {
427427

428-
List<T> entities;
429-
if (instances instanceof Collection) {
430-
entities = new ArrayList<>((Collection<T>) instances);
431-
} else {
432-
entities = new ArrayList<>();
433-
instances.forEach(entities::add);
434-
}
428+
Set<Class<?>> types = new HashSet<>();
429+
List<T> entities = new ArrayList<>();
430+
instances.forEach(instance -> {
431+
entities.add(instance);
432+
types.add(instance.getClass());
433+
});
435434

436435
if (entities.isEmpty()) {
437436
return Collections.emptyList();
438437
}
439438

440-
Class<T> domainClass = (Class<T>) TemplateSupport.findCommonElementType(entities);
441-
Assert.notNull(domainClass, "Could not determine common domain class to save.");
442-
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(domainClass);
443-
if (entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
439+
boolean heterogeneousCollection = types.size() > 1;
440+
Class<T> domainClass = (Class<T>) types.iterator().next();
441+
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getRequiredPersistentEntity(domainClass);
442+
if (heterogeneousCollection || entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
444443
|| entityMetaData.getDynamicLabelsProperty().isPresent()) {
445444
log.debug("Saving entities using single statements.");
446445

@@ -460,7 +459,7 @@ class Tuple3<T> {
460459
}
461460

462461
List<Tuple3<T>> entitiesToBeSaved = entities.stream()
463-
.map(e -> new Tuple3<>(e, neo4jMappingContext.getPersistentEntity(e.getClass()).isNew(e), eventSupport.maybeCallBeforeBind(e)))
462+
.map(e -> new Tuple3<>(e, entityMetaData.isNew(e), eventSupport.maybeCallBeforeBind(e)))
464463
.collect(Collectors.toList());
465464

466465
// Save roots

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

+12-13
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Collection;
3232
import java.util.Collections;
3333
import java.util.HashMap;
34+
import java.util.HashSet;
3435
import java.util.LinkedHashSet;
3536
import java.util.List;
3637
import java.util.Map;
@@ -461,24 +462,22 @@ public <T, R> Flux<R> saveAllAs(Iterable<T> instances, Class<R> resultType) {
461462

462463
private <T> Flux<T> saveAllImpl(Iterable<T> instances, @Nullable List<PropertyPath> includedProperties) {
463464

464-
List<T> entities;
465-
if (instances instanceof Collection) {
466-
entities = new ArrayList<>((Collection<T>) instances);
467-
} else {
468-
entities = new ArrayList<>();
469-
instances.forEach(entities::add);
470-
}
465+
Set<Class<?>> types = new HashSet<>();
466+
List<T> entities = new ArrayList<>();
467+
instances.forEach(instance -> {
468+
entities.add(instance);
469+
types.add(instance.getClass());
470+
});
471471

472472
if (entities.isEmpty()) {
473473
return Flux.empty();
474474
}
475475

476-
Class<T> domainClass = (Class<T>) TemplateSupport.findCommonElementType(entities);
477-
Assert.notNull(domainClass, "Could not determine common domain class to save.");
478-
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getPersistentEntity(domainClass);
479-
480-
if (entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
481-
|| entityMetaData.getDynamicLabelsProperty().isPresent()) {
476+
boolean heterogeneousCollection = types.size() > 1;
477+
Class<T> domainClass = (Class<T>) types.iterator().next();
478+
Neo4jPersistentEntity<?> entityMetaData = neo4jMappingContext.getRequiredPersistentEntity(domainClass);
479+
if (heterogeneousCollection || entityMetaData.isUsingInternalIds() || entityMetaData.hasVersionProperty()
480+
|| entityMetaData.getDynamicLabelsProperty().isPresent()) {
482481
log.debug("Saving entities using single statements.");
483482

484483
return Flux.fromIterable(entities).flatMap(e -> this.saveImpl(e, includedProperties));
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+
}
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+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
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.BookmarkCapture;
31+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
32+
import org.springframework.transaction.annotation.EnableTransactionManagement;
33+
34+
/**
35+
* @author Michael J. Simons
36+
*/
37+
@Neo4jIntegrationTest
38+
class GH2326IT extends TestBase {
39+
40+
@Test // GH-2326
41+
void saveShouldAddAllLabels(@Autowired AnimalRepository animalRepository, @Autowired BookmarkCapture bookmarkCapture) {
42+
43+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
44+
List<String> ids = animals.stream().map(animalRepository::save).map(BaseEntity::getId)
45+
.collect(Collectors.toList());
46+
47+
assertLabels(bookmarkCapture, ids);
48+
}
49+
50+
@Test // GH-2326
51+
void saveAllShouldAddAllLabels(@Autowired AnimalRepository animalRepository, @Autowired BookmarkCapture bookmarkCapture) {
52+
53+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
54+
List<String> ids = animalRepository.saveAll(animals).stream().map(BaseEntity::getId)
55+
.collect(Collectors.toList());
56+
57+
assertLabels(bookmarkCapture, ids);
58+
}
59+
60+
public interface AnimalRepository extends Neo4jRepository<Animal, String> {
61+
}
62+
63+
@Configuration
64+
@EnableTransactionManagement
65+
@EnableNeo4jRepositories(considerNestedRepositories = true)
66+
static class Config extends AbstractNeo4jConfig {
67+
68+
@Bean
69+
public BookmarkCapture bookmarkCapture() {
70+
return new BookmarkCapture();
71+
}
72+
73+
@Bean
74+
public Driver driver() {
75+
76+
return neo4jConnectionSupport.getDriver();
77+
}
78+
}
79+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
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.BookmarkCapture;
34+
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
35+
import org.springframework.transaction.annotation.EnableTransactionManagement;
36+
37+
/**
38+
* @author Michael J. Simons
39+
*/
40+
@Neo4jIntegrationTest
41+
class ReactiveGH2326IT extends TestBase {
42+
43+
@Test // GH-2326
44+
void saveShouldAddAllLabels(@Autowired AnimalRepository animalRepository, @Autowired BookmarkCapture bookmarkCapture) {
45+
46+
List<String> ids = new ArrayList<>();
47+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
48+
Flux.fromIterable(animals).flatMap(animalRepository::save)
49+
.map(BaseEntity::getId)
50+
.as(StepVerifier::create)
51+
.recordWith(() -> ids)
52+
.expectNextCount(2)
53+
.verifyComplete();
54+
55+
assertLabels(bookmarkCapture, ids);
56+
}
57+
58+
@Test // GH-2326
59+
void saveAllShouldAddAllLabels(@Autowired AnimalRepository animalRepository, @Autowired BookmarkCapture bookmarkCapture) {
60+
61+
List<String> ids = new ArrayList<>();
62+
List<Animal> animals = Arrays.asList(new Animal.Pet.Cat(), new Animal.Pet.Dog());
63+
animalRepository.saveAll(animals)
64+
.map(BaseEntity::getId)
65+
.as(StepVerifier::create)
66+
.recordWith(() -> ids)
67+
.expectNextCount(2)
68+
.verifyComplete();
69+
70+
assertLabels(bookmarkCapture, ids);
71+
}
72+
73+
public interface AnimalRepository extends ReactiveNeo4jRepository<Animal, String> {
74+
}
75+
76+
@Configuration
77+
@EnableTransactionManagement
78+
@EnableReactiveNeo4jRepositories(considerNestedRepositories = true)
79+
static class Config extends AbstractReactiveNeo4jConfig {
80+
81+
@Bean
82+
public BookmarkCapture bookmarkCapture() {
83+
return new BookmarkCapture();
84+
}
85+
86+
@Bean
87+
public Driver driver() {
88+
89+
return neo4jConnectionSupport.getDriver();
90+
}
91+
}
92+
}

0 commit comments

Comments
 (0)