DATAES-142 - AbstractRepository.saveAll can process empty list.

Original PR: spring-projects#307

Author: sothawo

src/main/java/org/springframework/data/elasticsearch/repository/support/AbstractElasticsearchRepository.java

@@ -23,6 +23,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.index.query.QueryBuilder;
@@ -42,6 +43,7 @@
 import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder;
 import org.springframework.data.elasticsearch.core.query.SearchQuery;
 import org.springframework.data.elasticsearch.repository.ElasticsearchRepository;
+import org.springframework.data.util.Streamable;
 import org.springframework.util.Assert;
 
 /**
@@ -57,6 +59,7 @@
  * @author Michael Wirth
  * @author Sascha Woo
  * @author Murali Chevuri
+ * @author Peter-Josef Meisch
  */
 public abstract class AbstractElasticsearchRepository<T, ID> implements ElasticsearchRepository<T, ID> {
 
@@ -182,16 +185,8 @@ public <S extends T> S save(S entity) {
 	public <S extends T> List<S> save(List<S> entities) {
 
 		Assert.notNull(entities, "Cannot insert 'null' as a List.");
-		Assert.notEmpty(entities, "Cannot insert empty List.");
 
-		List<IndexQuery> queries = new ArrayList<>();
-		for (S s : entities) {
-			queries.add(createIndexQuery(s));
-		}
-		elasticsearchOperations.bulkIndex(queries);
-		elasticsearchOperations.refresh(entityInformation.getIndexName());
-
-		return entities;
+		return Streamable.of(saveAll(entities)).stream().collect(Collectors.toList());
 	}
 
 	@Override
@@ -215,12 +210,13 @@ public <S extends T> Iterable<S> saveAll(Iterable<S> entities) {
 
 		Assert.notNull(entities, "Cannot insert 'null' as a List.");
 
-		List<IndexQuery> queries = new ArrayList<>();
-		for (S s : entities) {
-			queries.add(createIndexQuery(s));
+		List<IndexQuery> queries = Streamable.of(entities).stream().map(this::createIndexQuery)
+				.collect(Collectors.toList());
+
+		if (!queries.isEmpty()) {
+			elasticsearchOperations.bulkIndex(queries);
+			elasticsearchOperations.refresh(entityInformation.getIndexName());
 		}
-		elasticsearchOperations.bulkIndex(queries);
-		elasticsearchOperations.refresh(entityInformation.getIndexName());
 
 		return entities;
 	}

src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryTests.java

@@ -40,10 +40,10 @@
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.RestHighLevelClient;
 import org.junit.Before;
+import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.reactivestreams.Publisher;
-
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.data.annotation.Id;
@@ -52,6 +52,7 @@
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.elasticsearch.TestNodeResource;
 import org.springframework.data.elasticsearch.TestUtils;
 import org.springframework.data.elasticsearch.annotations.Document;
 import org.springframework.data.elasticsearch.annotations.Field;
@@ -67,12 +68,14 @@
 
 /**
  * @author Christoph Strobl
- * @currentRead Fool's Fate - Robin Hobb
+ * @author Peter-Josef Meisch
  */
 @RunWith(SpringRunner.class)
 @ContextConfiguration
 public class SimpleReactiveElasticsearchRepositoryTests {
 
+	@ClassRule public static TestNodeResource testNodeResource = new TestNodeResource();
+
 	@Configuration
 	@EnableReactiveElasticsearchRepositories(considerNestedRepositories = true)
 	static class Config extends AbstractReactiveElasticsearchConfiguration {

src/test/java/org/springframework/data/elasticsearch/repository/support/simple/SimpleElasticsearchRepositoryTests.java

@@ -28,11 +28,13 @@
 import java.lang.Long;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.junit.Before;
+import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -42,12 +44,15 @@
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.elasticsearch.RestElasticsearchTestConfiguration;
+import org.springframework.data.elasticsearch.TestNodeResource;
 import org.springframework.data.elasticsearch.annotations.Document;
 import org.springframework.data.elasticsearch.annotations.Field;
-import org.springframework.data.elasticsearch.core.ElasticsearchTemplate;
+import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
 import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder;
 import org.springframework.data.elasticsearch.core.query.SearchQuery;
 import org.springframework.data.elasticsearch.repository.ElasticsearchRepository;
+import org.springframework.data.elasticsearch.repository.config.EnableElasticsearchRepositories;
 import org.springframework.data.elasticsearch.utils.IndexInitializer;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringRunner;
@@ -62,16 +67,19 @@
  * @author Murali Chevuri
  */
 @RunWith(SpringRunner.class)
-@ContextConfiguration("classpath:/simple-repository-test.xml")
+@ContextConfiguration(classes = { SimpleElasticsearchRepositoryTests.class, RestElasticsearchTestConfiguration.class })
+@EnableElasticsearchRepositories(considerNestedRepositories = true)
 public class SimpleElasticsearchRepositoryTests {
 
+	@ClassRule public static TestNodeResource testNodeResource = new TestNodeResource();
+
 	@Autowired private SampleElasticsearchRepository repository;
 
-	@Autowired private ElasticsearchTemplate elasticsearchTemplate;
+	@Autowired private ElasticsearchOperations elasticsearchOperations;
 
 	@Before
 	public void before() {
-		IndexInitializer.init(elasticsearchTemplate, SampleEntity.class);
+		IndexInitializer.init(elasticsearchOperations, SampleEntity.class);
 	}
 
 	@Test
@@ -636,6 +644,34 @@ public void shouldReturnSimilarEntities() {
 		assertThat(results.getTotalElements()).isGreaterThanOrEqualTo(1L);
 	}
 
+	@Test // DATAES-142
+	public void shouldIndexNotEmptyList() {
+		// given
+		List<SampleEntity> list = new ArrayList<>();
+		String documentId = randomNumeric(5);
+		SampleEntity sampleEntity1 = new SampleEntity();
+		sampleEntity1.setId(documentId);
+		sampleEntity1.setMessage("world");
+		list.add(sampleEntity1);
+
+		String documentId2 = randomNumeric(5);
+		SampleEntity sampleEntity2 = new SampleEntity();
+		sampleEntity2.setId(documentId2);
+		sampleEntity2.setMessage("hello");
+		list.add(sampleEntity2);
+
+		Iterable<SampleEntity> savedEntities = repository.saveAll(list);
+
+		assertThat(savedEntities).containsExactlyElementsOf(list);
+	}
+
+	@Test // DATAES-142
+	public void shouldNotFailOnIndexingEmptyList() {
+		Iterable<SampleEntity> savedEntities = repository.saveAll(Collections.emptyList());
+
+		assertThat(savedEntities).hasSize(0);
+	}
+
 	private static List<SampleEntity> createSampleEntitiesWithMessage(String message, int numberOfEntities) {
 		List<SampleEntity> sampleEntities = new ArrayList<>();
 		for (int i = 0; i < numberOfEntities; i++) {