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

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 <[email protected]>

Author: cberes

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

@@ -98,6 +98,7 @@
  * @author Michael J. Simons
  * @author Philipp Tölle
  * @author Gerrit Meier
+ * @author Corey Beres
  * @soundtrack Motörhead - We Are Motörhead
  * @since 6.0
  */
@@ -549,6 +550,14 @@ public <T, R> List<R> saveAllAs(Iterable<T> instances, Class<R> resultType) {
 
 		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);

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

@@ -491,6 +491,15 @@ public <T, R> Flux<R> saveAllAs(Iterable<T> instances, Class<R> resultType) {
 
 		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);
 		}

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

@@ -64,6 +64,14 @@
 @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 static Class<?> findCommonElementType(Iterable<?> collection) {
 				.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) {

src/test/java/org/springframework/data/neo4j/core/TemplateSupportTest.java

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

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

@@ -17,9 +17,11 @@
 
 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 @@
  * @author Gerrit Meier
  * @author Michael J. Simons
  * @author Rosetta Roberts
+ * @author Corey Beres
  */
 @Neo4jIntegrationTest
 class Neo4jTemplateIT {
@@ -795,6 +798,24 @@ void saveAllAsWithClosedProjectionShouldWork() {
 		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<>();

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

@@ -22,6 +22,7 @@
 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;
@@ -688,6 +689,26 @@ void saveAsWithClosedProjectionOnThreeLevelShouldWork(@Autowired ReactiveNeo4jTe
 				.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) {