Spring/spring-data-neo4j
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

GH-2544 - Check for empty collections in saveAllAs methods.

Browse Source 
When an empty collection is passed to imperative and reactive `saveAllAs` variants a common element type can't be computed. We did not check for this scenario correctly and failed with a NPE. The introduction of a marker class fixes #2544 

Co-authored-by: Michael Simons <michael@simons.ac>
This commit is contained in:
Corey Beres
2022-05-30 04:06:57 -04:00
committed by GitHub
parent 376ad7006b
commit 4aa58754e3
6 changed files with 79 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java
View File

@@ -99,6 +99,7 @@ import org.springframework.util.Assert;
* @author Michael J. Simons
* @author Philipp Tölle
* @author Gerrit Meier
* @author Corey Beres
* @soundtrack Motörhead - We Are Motörhead
* @since 6.0
*/
@@ -522,6 +523,14 @@ public final class Neo4jTemplate implements
Class<?> commonElementType = TemplateSupport.findCommonElementType(instances);
if (commonElementType == null) {
throw new IllegalArgumentException("Could not determine a common element of an heterogeneous collection.");
}
if (commonElementType == TemplateSupport.EmptyIterable.class) {
return Collections.emptyList();
}
if (resultType.isAssignableFrom(commonElementType)) {
@SuppressWarnings("unchecked") // Nicer to live with this than streaming, mapping and collecting to avoid the cast. It's easier on the reactive side.
List<R> saveElements = (List<R>) saveAll(instances);

19
src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java
View File

@@ -19,11 +19,6 @@ import static org.neo4j.cypherdsl.core.Cypher.anyNode;
import static org.neo4j.cypherdsl.core.Cypher.asterisk;
import static org.neo4j.cypherdsl.core.Cypher.parameter;
import org.neo4j.cypherdsl.core.renderer.Configuration;
import org.springframework.data.mapping.Association;
import org.springframework.data.neo4j.core.TemplateSupport.FilteredBinderFunction;
import org.springframework.data.neo4j.core.mapping.AssociationHandlerSupport;
import org.springframework.data.neo4j.core.schema.TargetNode;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.util.function.Tuple2;
@@ -54,6 +49,7 @@ import org.neo4j.cypherdsl.core.Cypher;
import org.neo4j.cypherdsl.core.Functions;
import org.neo4j.cypherdsl.core.Node;
import org.neo4j.cypherdsl.core.Statement;
import org.neo4j.cypherdsl.core.renderer.Configuration;
import org.neo4j.cypherdsl.core.renderer.Renderer;
import org.neo4j.driver.Value;
import org.neo4j.driver.types.Entity;
@@ -67,10 +63,13 @@ import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.core.log.LogAccessor;
import org.springframework.dao.IncorrectResultSizeDataAccessException;
import org.springframework.dao.OptimisticLockingFailureException;
import org.springframework.data.mapping.Association;
import org.springframework.data.mapping.PersistentPropertyAccessor;
import org.springframework.data.mapping.PropertyPath;
import org.springframework.data.mapping.callback.ReactiveEntityCallbacks;
import org.springframework.data.neo4j.core.TemplateSupport.FilteredBinderFunction;
import org.springframework.data.neo4j.core.TemplateSupport.NodesAndRelationshipsByIdStatementProvider;
import org.springframework.data.neo4j.core.mapping.AssociationHandlerSupport;
import org.springframework.data.neo4j.core.mapping.Constants;
import org.springframework.data.neo4j.core.mapping.CreateRelationshipStatementHolder;
import org.springframework.data.neo4j.core.mapping.CypherGenerator;
@@ -88,6 +87,7 @@ import org.springframework.data.neo4j.core.mapping.NodeDescription;
import org.springframework.data.neo4j.core.mapping.PropertyFilter;
import org.springframework.data.neo4j.core.mapping.RelationshipDescription;
import org.springframework.data.neo4j.core.mapping.callback.ReactiveEventSupport;
import org.springframework.data.neo4j.core.schema.TargetNode;
import org.springframework.data.neo4j.repository.query.QueryFragments;
import org.springframework.data.neo4j.repository.query.QueryFragmentsAndParameters;
import org.springframework.data.projection.ProjectionFactory;
@@ -478,6 +478,15 @@ public final class ReactiveNeo4jTemplate implements
Class<?> commonElementType = TemplateSupport.findCommonElementType(instances);
if (commonElementType == null) {
return Flux.error(() -> new IllegalArgumentException(
"Could not determine a common element of an heterogeneous collection."));
}
if (commonElementType == TemplateSupport.EmptyIterable.class) {
return Flux.empty();
}
if (resultType.isAssignableFrom(commonElementType)) {
return saveAll(instances).map(resultType::cast);
}

12
src/main/java/org/springframework/data/neo4j/core/TemplateSupport.java
View File

@@ -64,6 +64,14 @@ import org.springframework.util.Assert;
@API(status = API.Status.INTERNAL, since = "6.0.9")
public final class TemplateSupport {
/**
* Indicator for an empty collection
*/
public static final class EmptyIterable {
private EmptyIterable() {
}
}
enum FetchType {
ONE,
@@ -81,6 +89,10 @@ public final class TemplateSupport {
.filter(o -> o != null)
.map(Object::getClass).collect(Collectors.toSet());
if (allClasses.isEmpty()) {
return EmptyIterable.class;
}
Class<?> candidate = null;
for (Class<?> type : allClasses) {
if (candidate == null) {

2
src/test/java/org/springframework/data/neo4j/core/TemplateSupportTest.java
View File

@@ -88,7 +88,7 @@ class TemplateSupportTest {
void shouldNotFailWithEmptyInput() {
Class<?> type = TemplateSupport.findCommonElementType(Collections.emptyList());
assertThat(type).isNull();
assertThat(type).isEqualTo(TemplateSupport.EmptyIterable.class);
}
@Test

21
src/test/java/org/springframework/data/neo4j/integration/imperative/Neo4jTemplateIT.java
View File

@@ -17,9 +17,11 @@ package org.springframework.data.neo4j.integration.imperative;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import lombok.Data;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -76,6 +78,7 @@ import org.springframework.transaction.annotation.EnableTransactionManagement;
* @author Gerrit Meier
* @author Michael J. Simons
* @author Rosetta Roberts
* @author Corey Beres
*/
@Neo4jIntegrationTest
class Neo4jTemplateIT {
@@ -795,6 +798,24 @@ class Neo4jTemplateIT {
assertThat(people).allMatch(p -> p.getAddress() != null);
}
@Test // GH-2544
void saveAllAsWithEmptyList() {
List<ClosedProjection> projections = neo4jTemplate.saveAllAs(Collections.emptyList(), ClosedProjection.class);
assertThat(projections).isEmpty();
}
@Test // GH-2544
void saveWeirdHierarchy() {
List<Object> things = new ArrayList<>();
things.add(1);
things.add("eins");
assertThatIllegalArgumentException().isThrownBy(() -> neo4jTemplate.saveAllAs(things, ClosedProjection.class))
.withMessage("Could not determine a common element of an heterogeneous collection.");
}
@Test
void updatingFindShouldWork() {
Map<String, Object> params = new HashMap<>();

23
src/test/java/org/springframework/data/neo4j/integration/reactive/ReactiveNeo4jTemplateIT.java
View File

@@ -19,10 +19,10 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.neo4j.cypherdsl.core.Cypher.parameter;
import lombok.Data;
import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration;
import reactor.core.publisher.Flux;
import reactor.test.StepVerifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -71,6 +71,7 @@ import org.springframework.data.neo4j.test.BookmarkCapture;
import org.springframework.data.neo4j.test.Neo4jExtension;
import org.springframework.data.neo4j.test.Neo4jExtension.Neo4jConnectionSupport;
import org.springframework.data.neo4j.test.Neo4jIntegrationTest;
import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration;
import org.springframework.transaction.ReactiveTransactionManager;
import org.springframework.transaction.annotation.EnableTransactionManagement;
@@ -688,6 +689,26 @@ class ReactiveNeo4jTemplateIT {
.verifyComplete();
}
@Test // GH-2544
void saveAllAsWithEmptyList(@Autowired ReactiveNeo4jTemplate template) {
template.saveAllAs(Collections.emptyList(), ClosedProjection.class)
.as(StepVerifier::create)
.verifyComplete();
}
@Test // GH-2544
void saveWeirdHierarchy(@Autowired ReactiveNeo4jTemplate template) {
List<Object> things = new ArrayList<>();
things.add(1);
things.add("eins");
template.saveAllAs(things, ClosedProjection.class)
.as(StepVerifier::create)
.verifyErrorMatches(t -> t instanceof IllegalArgumentException && t.getMessage().equals("Could not determine a common element of an heterogeneous collection."));
}
@Test
void saveAllAsWithClosedProjectionShouldWork(@Autowired ReactiveNeo4jTemplate template) {