DATAJDBC-378 - Proper handling of null and empty collections in JdbcAggregateTemplate.

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-relational-parent</artifactId>
-	<version>1.1.0.BUILD-SNAPSHOT</version>
+	<version>1.1.0.DATAJDBC-378-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data Relational Parent</name>

spring-data-jdbc-distribution/pom.xml

@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>1.1.0.BUILD-SNAPSHOT</version>
+		<version>1.1.0.DATAJDBC-378-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-jdbc/pom.xml

@@ -5,7 +5,7 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-jdbc</artifactId>
-	<version>1.1.0.BUILD-SNAPSHOT</version>
+	<version>1.1.0.DATAJDBC-378-SNAPSHOT</version>
 
 	<name>Spring Data JDBC</name>
 	<description>Spring Data module for JDBC repositories.</description>
@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>1.1.0.BUILD-SNAPSHOT</version>
+		<version>1.1.0.DATAJDBC-378-SNAPSHOT</version>
 	</parent>
 
 	<properties>

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java

@@ -46,6 +46,7 @@
  * @author Jens Schauder
  * @author Mark Paluch
  * @author Thomas Lang
+ * @author Christoph Strobl
  */
 public class JdbcAggregateTemplate implements JdbcAggregateOperations {
 
@@ -152,6 +153,9 @@ public <T> T update(T instance) {
 	 */
 	@Override
 	public long count(Class<?> domainType) {
+
+		Assert.notNull(domainType, "Domain type must not be null");
+
 		return accessStrategy.count(domainType);
 	}
 
@@ -162,6 +166,9 @@ public long count(Class<?> domainType) {
 	@Override
 	public <T> T findById(Object id, Class<T> domainType) {
 
+		Assert.notNull(id, "Id must not be null!");
+		Assert.notNull(domainType, "Domain type must not be null!");
+
 		T entity = accessStrategy.findById(id, domainType);
 		if (entity != null) {
 			publishAfterLoad(id, entity);
@@ -175,6 +182,10 @@ public <T> T findById(Object id, Class<T> domainType) {
 	 */
 	@Override
 	public <T> boolean existsById(Object id, Class<T> domainType) {
+
+		Assert.notNull(id, "Id must not be null!");
+		Assert.notNull(domainType, "Domain type must not be null!");
+
 		return accessStrategy.existsById(id, domainType);
 	}
 
@@ -185,6 +196,8 @@ public <T> boolean existsById(Object id, Class<T> domainType) {
 	@Override
 	public <T> Iterable<T> findAll(Class<T> domainType) {
 
+		Assert.notNull(domainType, "Domain type must not be null!");
+
 		Iterable<T> all = accessStrategy.findAll(domainType);
 		publishAfterLoad(all);
 		return all;
@@ -197,6 +210,9 @@ public <T> Iterable<T> findAll(Class<T> domainType) {
 	@Override
 	public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 
+		Assert.notNull(ids, "Ids must not be null!");
+		Assert.notNull(domainType, "Domain type must not be null!");
+
 		Iterable<T> allById = accessStrategy.findAllById(ids, domainType);
 		publishAfterLoad(allById);
 		return allById;
@@ -209,6 +225,9 @@ public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 	@Override
 	public <S> void delete(S aggregateRoot, Class<S> domainType) {
 
+		Assert.notNull(aggregateRoot, "Aggregate root must not be null!");
+		Assert.notNull(domainType, "Domain type must not be null!");
+
 		IdentifierAccessor identifierAccessor = context.getRequiredPersistentEntity(domainType)
 				.getIdentifierAccessor(aggregateRoot);
 
@@ -221,6 +240,10 @@ public <S> void delete(S aggregateRoot, Class<S> domainType) {
 	 */
 	@Override
 	public <S> void deleteById(Object id, Class<S> domainType) {
+
+		Assert.notNull(id, "Id must not be null!");
+		Assert.notNull(domainType, "Domain type must not be null!");
+
 		deleteTree(id, null, domainType);
 	}
 
@@ -231,6 +254,8 @@ public <S> void deleteById(Object id, Class<S> domainType) {
 	@Override
 	public void deleteAll(Class<?> domainType) {
 
+		Assert.notNull(domainType, "Domain type must not be null!");
+
 		AggregateChange<?> change = createDeletingChange(domainType);
 		change.executeWith(interpreter, context, converter);
 	}
@@ -250,7 +275,7 @@ private <T> T store(T instance, IdentifierAccessor identifierAccessor, Aggregate
 
 		Object identifier = persistentEntity.getIdentifierAccessor(change.getEntity()).getIdentifier();
 
-		Assert.notNull(identifier, "After saving the identifier must not be null");
+		Assert.notNull(identifier, "After saving the identifier must not be null!");
 
 		publisher.publishEvent(new AfterSaveEvent( //
 				Identifier.of(identifier), //

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java

@@ -18,6 +18,7 @@
 import java.sql.JDBCType;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -241,6 +242,10 @@ public <T> Iterable<T> findAll(Class<T> domainType) {
 	@SuppressWarnings("unchecked")
 	public <T> Iterable<T> findAllById(Iterable<?> ids, Class<T> domainType) {
 
+		if (!ids.iterator().hasNext()) {
+			return Collections.emptyList();
+		}
+
 		RelationalPersistentProperty idProperty = getRequiredPersistentEntity(domainType).getRequiredIdProperty();
 		MapSqlParameterSource parameterSource = new MapSqlParameterSource();
 

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java

@@ -35,7 +35,6 @@
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
-
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.annotation.Bean;

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateUnitTests.java

@@ -0,0 +1,78 @@
+/*
+ * Copyright 2019 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.jdbc.core;
+
+import static java.util.Collections.*;
+import static org.assertj.core.api.Assertions.*;
+
+import javax.sql.DataSource;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.data.jdbc.core.JdbcAggregateTemplateIntegrationTests.LegoSet;
+import org.springframework.data.jdbc.core.convert.BasicJdbcConverter;
+import org.springframework.data.jdbc.core.convert.DataAccessStrategy;
+import org.springframework.data.jdbc.core.convert.DefaultDataAccessStrategy;
+import org.springframework.data.jdbc.core.convert.JdbcConverter;
+import org.springframework.data.jdbc.core.convert.RelationResolver;
+import org.springframework.data.jdbc.core.convert.SqlGeneratorSource;
+import org.springframework.data.relational.core.mapping.NamingStrategy;
+import org.springframework.data.relational.core.mapping.RelationalMappingContext;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
+import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
+
+/**
+ * @author Christoph Strobl
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class JdbcAggregateTemplateUnitTests {
+
+	JdbcAggregateOperations template;
+
+	@Mock ApplicationEventPublisher eventPublisher;
+	@Mock RelationResolver relationResolver;
+	@Mock DataSource dataSource;
+
+	@Before
+	public void setUp() {
+
+		RelationalMappingContext mappingContext = new RelationalMappingContext(NamingStrategy.INSTANCE);
+		JdbcConverter converter = new BasicJdbcConverter(mappingContext, relationResolver);
+		NamedParameterJdbcOperations namedParameterJdbcOperations = new NamedParameterJdbcTemplate(dataSource);
+		DataAccessStrategy dataAccessStrategy = new DefaultDataAccessStrategy(new SqlGeneratorSource(mappingContext),
+				mappingContext, converter, namedParameterJdbcOperations);
+		template = new JdbcAggregateTemplate(eventPublisher, mappingContext, converter, dataAccessStrategy);
+	}
+
+	@Test // DATAJDBC-378
+	public void findAllByIdMustNotAcceptNullArgumentForType() {
+		assertThatThrownBy(() -> template.findAllById(singleton(23L), null)).isInstanceOf(IllegalArgumentException.class);
+	}
+
+	@Test // DATAJDBC-378
+	public void findAllByIdMustNotAcceptNullArgumentForIds() {
+		assertThatThrownBy(() -> template.findAllById(null, LegoSet.class)).isInstanceOf(IllegalArgumentException.class);
+	}
+
+	@Test // DATAJDBC-378
+	public void findAllByIdWithEmpthListMustReturnEmptyResult() {
+		assertThat(template.findAllById(emptyList(), LegoSet.class)).isEmpty();
+	}
+}

spring-data-relational/pom.xml

@@ -5,15 +5,15 @@
 	<modelVersion>4.0.0</modelVersion>
 
 	<artifactId>spring-data-relational</artifactId>
-	<version>1.1.0.BUILD-SNAPSHOT</version>
+	<version>1.1.0.DATAJDBC-378-SNAPSHOT</version>
 
 	<name>Spring Data Relational</name>
 	<description>Spring Data Relational support</description>
 
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-relational-parent</artifactId>
-		<version>1.1.0.BUILD-SNAPSHOT</version>
+		<version>1.1.0.DATAJDBC-378-SNAPSHOT</version>
 	</parent>
 
 	<properties>