DATAES-937 - Repository queries with IN filters fail with empty input list.

Original PR: #525

(cherry picked from commit 7117e5d70d7b52aa689787b2201a3f26639bbcf8)

src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java

@@ -15,7 +15,7 @@
  */
 package org.springframework.data.elasticsearch.core;
 
-import static org.elasticsearch.index.query.Operator.AND;
+import static org.elasticsearch.index.query.Operator.*;
 import static org.elasticsearch.index.query.QueryBuilders.*;
 import static org.springframework.data.elasticsearch.core.query.Criteria.*;
 
@@ -25,7 +25,9 @@ import java.util.List;
 import java.util.ListIterator;
 
 import org.apache.lucene.queryparser.flexible.core.util.StringUtils;
-import org.elasticsearch.index.query.*;
+import org.apache.lucene.queryparser.flexible.standard.QueryParserUtil;
+import org.elasticsearch.index.query.BoolQueryBuilder;
+import org.elasticsearch.index.query.QueryBuilder;
 import org.springframework.data.elasticsearch.core.query.Criteria;
 import org.springframework.util.Assert;
 
@@ -39,7 +41,6 @@ import org.springframework.util.Assert;
  */
 class CriteriaQueryProcessor {
 
-
 	QueryBuilder createQueryFromCriteria(Criteria criteria) {
 		if (criteria == null)
 			return null;
@@ -104,7 +105,6 @@ class CriteriaQueryProcessor {
 		return query;
 	}
 
-
 	private QueryBuilder createQueryFragmentForCriteria(Criteria chainedCriteria) {
 		if (chainedCriteria.getQueryCriteriaEntries().isEmpty())
 			return null;
@@ -131,8 +131,8 @@ class CriteriaQueryProcessor {
 		return query;
 	}
 
-
+	private QueryBuilder processCriteriaEntry(Criteria.CriteriaEntry entry,
-	private QueryBuilder processCriteriaEntry(Criteria.CriteriaEntry entry,/* OperationKey key, Object value,*/ String fieldName) {
+			/* OperationKey key, Object value,*/ String fieldName) {
 		Object value = entry.getValue();
 		if (value == null) {
 			return null;
@@ -180,17 +180,15 @@ class CriteriaQueryProcessor {
 				query = fuzzyQuery(fieldName, searchText);
 				break;
 			case IN:
-				query = boolQuery();
+				if (value instanceof Iterable) {
-				collection = (Iterable<Object>) value;
+					Iterable<?> iterable = (Iterable<?>) value;
-				for (Object item : collection) {
+					query = queryStringQuery(orQueryString(iterable)).field(fieldName);
-					((BoolQueryBuilder) query).should(queryStringQuery(item.toString()).field(fieldName));
 				}
 				break;
 			case NOT_IN:
-				query = boolQuery();
+				if (value instanceof Iterable) {
-				collection = (Iterable<Object>) value;
+					Iterable<?> iterable = (Iterable<?>) value;
-				for (Object item : collection) {
+					query = queryStringQuery("NOT(" + orQueryString(iterable) + ')').field(fieldName);
-					((BoolQueryBuilder) query).mustNot(queryStringQuery(item.toString()).field(fieldName));
 				}
 				break;
 		}
@@ -203,4 +201,23 @@ class CriteriaQueryProcessor {
 		}
 		query.boost(boost);
 	}
+
+	private static String orQueryString(Iterable<?> iterable) {
+		StringBuilder sb = new StringBuilder();
+
+		for (Object item : iterable) {
+
+			if (item != null) {
+
+				if (sb.length() > 0) {
+					sb.append(' ');
+				}
+				sb.append('"');
+				sb.append(QueryParserUtil.escape(item.toString()));
+				sb.append('"');
+			}
+		}
+
+		return sb.toString();
+	}
 }

src/main/java/org/springframework/data/elasticsearch/core/query/Criteria.java

@@ -15,10 +15,13 @@
  */
 package org.springframework.data.elasticsearch.core.query;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
 
-import org.springframework.util.ObjectUtils;
-import org.springframework.util.StringUtils;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.elasticsearch.core.geo.GeoBox;
 import org.springframework.data.elasticsearch.core.geo.GeoPoint;
@@ -26,6 +29,8 @@ import org.springframework.data.geo.Box;
 import org.springframework.data.geo.Distance;
 import org.springframework.data.geo.Point;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
+import org.springframework.util.StringUtils;
 
 /**
  * Criteria is the central class when constructing queries. It follows more or less a fluent API style, which allows to
@@ -39,13 +44,9 @@ public class Criteria {
 
 	@Override
 	public String toString() {
-		return "Criteria{" +
+		return "Criteria{" + "field=" + field.getName() + ", boost=" + boost + ", negating=" + negating + ", queryCriteria="
-				"field=" + field.getName() +
+				+ ObjectUtils.nullSafeToString(queryCriteria) + ", filterCriteria="
-				", boost=" + boost +
+				+ ObjectUtils.nullSafeToString(filterCriteria) + '}';
-				", negating=" + negating +
-				", queryCriteria=" + ObjectUtils.nullSafeToString(queryCriteria) +
-				", filterCriteria=" + ObjectUtils.nullSafeToString(filterCriteria) +
-				'}';
 	}
 
 	public static final String WILDCARD = "*";
@@ -64,8 +65,7 @@ public class Criteria {
 
 	private Set<CriteriaEntry> filterCriteria = new LinkedHashSet<>();
 
-	public Criteria() {
+	public Criteria() {}
-	}
 
 	/**
 	 * Creates a new Criteria with provided field name
@@ -305,7 +305,7 @@ public class Criteria {
 			throw new InvalidDataAccessApiUsageException("Range [* TO *] is not allowed");
 		}
 
-		queryCriteria.add(new CriteriaEntry(OperationKey.BETWEEN, new Object[]{lowerBound, upperBound}));
+		queryCriteria.add(new CriteriaEntry(OperationKey.BETWEEN, new Object[] { lowerBound, upperBound }));
 		return this;
 	}
 
@@ -376,11 +376,6 @@ public class Criteria {
 	}
 
 	private List<Object> toCollection(Object... values) {
-		if (values.length == 0 || (values.length > 1 && values[1] instanceof Collection)) {
-			throw new InvalidDataAccessApiUsageException("At least one element "
-					+ (values.length > 0 ? ("of argument of type " + values[1].getClass().getName()) : "")
-					+ " has to be present.");
-		}
 		return Arrays.asList(values);
 	}
 
@@ -398,15 +393,14 @@ public class Criteria {
 	 * Creates new CriteriaEntry for {@code location WITHIN distance}
 	 *
 	 * @param location {@link org.springframework.data.elasticsearch.core.geo.GeoPoint} center coordinates
-	 * @param distance {@link String} radius as a string (e.g. : '100km').
+	 * @param distance {@link String} radius as a string (e.g. : '100km'). Distance unit : either mi/miles or km can be
-	 * Distance unit :
+	 *          set
-	 * either mi/miles or km can be set
 	 * @return Criteria the chaind criteria with the new 'within' criteria included.
 	 */
 	public Criteria within(GeoPoint location, String distance) {
 		Assert.notNull(location, "Location value for near criteria must not be null");
 		Assert.notNull(location, "Distance value for near criteria must not be null");
-		filterCriteria.add(new CriteriaEntry(OperationKey.WITHIN, new Object[]{location, distance}));
+		filterCriteria.add(new CriteriaEntry(OperationKey.WITHIN, new Object[] { location, distance }));
 		return this;
 	}
 
@@ -414,56 +408,54 @@ public class Criteria {
 	 * Creates new CriteriaEntry for {@code location WITHIN distance}
 	 *
 	 * @param location {@link org.springframework.data.geo.Point} center coordinates
-	 * @param distance {@link org.springframework.data.geo.Distance} radius
+	 * @param distance {@link org.springframework.data.geo.Distance} radius .
-	 * .
 	 * @return Criteria the chaind criteria with the new 'within' criteria included.
 	 */
 	public Criteria within(Point location, Distance distance) {
 		Assert.notNull(location, "Location value for near criteria must not be null");
 		Assert.notNull(location, "Distance value for near criteria must not be null");
-		filterCriteria.add(new CriteriaEntry(OperationKey.WITHIN, new Object[]{location, distance}));
+		filterCriteria.add(new CriteriaEntry(OperationKey.WITHIN, new Object[] { location, distance }));
 		return this;
 	}
 
 	/**
 	 * Creates new CriteriaEntry for {@code geoLocation WITHIN distance}
 	 *
-	 * @param geoLocation {@link String} center point
+	 * @param geoLocation {@link String} center point supported formats: lat on = > "41.2,45.1", geohash = > "asd9as0d"
-	 * supported formats:
+	 * @param distance {@link String} radius as a string (e.g. : '100km'). Distance unit : either mi/miles or km can be
-	 * lat on = > "41.2,45.1",
+	 *          set
-	 * geohash = > "asd9as0d"
-	 * @param distance {@link String} radius as a string (e.g. : '100km').
-	 * Distance unit :
-	 * either mi/miles or km can be set
 	 * @return
 	 */
 	public Criteria within(String geoLocation, String distance) {
 		Assert.isTrue(!StringUtils.isEmpty(geoLocation), "geoLocation value must not be null");
-		filterCriteria.add(new CriteriaEntry(OperationKey.WITHIN, new Object[]{geoLocation, distance}));
+		filterCriteria.add(new CriteriaEntry(OperationKey.WITHIN, new Object[] { geoLocation, distance }));
 		return this;
 	}
 
 	/**
 	 * Creates new CriteriaEntry for {@code location GeoBox bounding box}
 	 *
-	 * @param boundingBox {@link org.springframework.data.elasticsearch.core.geo.GeoBox} bounding box(left top corner + right bottom corner)
+	 * @param boundingBox {@link org.springframework.data.elasticsearch.core.geo.GeoBox} bounding box(left top corner +
+	 *          right bottom corner)
 	 * @return Criteria the chaind criteria with the new 'boundingBox' criteria included.
 	 */
 	public Criteria boundedBy(GeoBox boundingBox) {
 		Assert.notNull(boundingBox, "boundingBox value for boundedBy criteria must not be null");
-		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[]{boundingBox}));
+		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[] { boundingBox }));
 		return this;
 	}
 
 	/**
 	 * Creates new CriteriaEntry for {@code location Box bounding box}
 	 *
-	 * @param boundingBox {@link org.springframework.data.elasticsearch.core.geo.GeoBox} bounding box(left top corner + right bottom corner)
+	 * @param boundingBox {@link org.springframework.data.elasticsearch.core.geo.GeoBox} bounding box(left top corner +
+	 *          right bottom corner)
 	 * @return Criteria the chaind criteria with the new 'boundingBox' criteria included.
 	 */
 	public Criteria boundedBy(Box boundingBox) {
 		Assert.notNull(boundingBox, "boundingBox value for boundedBy criteria must not be null");
-		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[]{boundingBox.getFirst(), boundingBox.getSecond()}));
+		filterCriteria
+				.add(new CriteriaEntry(OperationKey.BBOX, new Object[] { boundingBox.getFirst(), boundingBox.getSecond() }));
 		return this;
 	}
 
@@ -477,7 +469,7 @@ public class Criteria {
 	public Criteria boundedBy(String topLeftGeohash, String bottomRightGeohash) {
 		Assert.isTrue(!StringUtils.isEmpty(topLeftGeohash), "topLeftGeohash must not be empty");
 		Assert.isTrue(!StringUtils.isEmpty(bottomRightGeohash), "bottomRightGeohash must not be empty");
-		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[]{topLeftGeohash, bottomRightGeohash}));
+		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[] { topLeftGeohash, bottomRightGeohash }));
 		return this;
 	}
 
@@ -491,14 +483,15 @@ public class Criteria {
 	public Criteria boundedBy(GeoPoint topLeftPoint, GeoPoint bottomRightPoint) {
 		Assert.notNull(topLeftPoint, "topLeftPoint must not be null");
 		Assert.notNull(bottomRightPoint, "bottomRightPoint must not be null");
-		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[]{topLeftPoint, bottomRightPoint}));
+		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[] { topLeftPoint, bottomRightPoint }));
 		return this;
 	}
 
 	public Criteria boundedBy(Point topLeftPoint, Point bottomRightPoint) {
 		Assert.notNull(topLeftPoint, "topLeftPoint must not be null");
 		Assert.notNull(bottomRightPoint, "bottomRightPoint must not be null");
-		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX, new Object[]{GeoPoint.fromPoint(topLeftPoint), GeoPoint.fromPoint(bottomRightPoint)}));
+		filterCriteria.add(new CriteriaEntry(OperationKey.BBOX,
+				new Object[] { GeoPoint.fromPoint(topLeftPoint), GeoPoint.fromPoint(bottomRightPoint) }));
 		return this;
 	}
 
@@ -611,10 +604,7 @@ public class Criteria {
 
 		@Override
 		public String toString() {
-			return "CriteriaEntry{" +
+			return "CriteriaEntry{" + "key=" + key + ", value=" + value + '}';
-					"key=" + key +
-					", value=" + value +
-					'}';
 		}
 	}
 }

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

@@ -155,7 +155,13 @@ public abstract class AbstractElasticsearchRepository<T, ID> implements Elastics
 
 		Assert.notNull(ids, "ids can't be null.");
 
-		SearchQuery query = new NativeSearchQueryBuilder().withIds(stringIdsRepresentation(ids)).build();
+		List<String> stringIds = stringIdsRepresentation(ids);
+
+		if (stringIds.isEmpty()) {
+			return Collections.emptyList();
+		}
+
+		SearchQuery query = new NativeSearchQueryBuilder().withIds(stringIds).build();
 
 		return elasticsearchOperations.multiGet(query, getEntityClass());
 	}

src/test/java/org/springframework/data/elasticsearch/repository/query/keywords/QueryKeywordsTests.java

@@ -23,6 +23,7 @@ import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.List;
@@ -61,15 +62,15 @@ abstract class QueryKeywordsTests {
 
 		IndexInitializer.init(elasticsearchTemplate, Product.class);
 
-		 Product product1 = Product.builder().id("1").name("Sugar").text("Cane sugar").price(1.0f).available(false)
+		Product product1 = Product.builder().id("1").name("Sugar").text("Cane sugar").price(1.0f).available(false)
 				.sortName("sort5").build();
-		 Product product2 = Product.builder().id("2").name("Sugar").text("Cane sugar").price(1.2f).available(true)
+		Product product2 = Product.builder().id("2").name("Sugar").text("Cane sugar").price(1.2f).available(true)
 				.sortName("sort4").build();
-		 Product product3 = Product.builder().id("3").name("Sugar").text("Beet sugar").price(1.1f).available(true)
+		Product product3 = Product.builder().id("3").name("Sugar").text("Beet sugar").price(1.1f).available(true)
 				.sortName("sort3").build();
-		 Product product4 = Product.builder().id("4").name("Salt").text("Rock salt").price(1.9f).available(true)
+		Product product4 = Product.builder().id("4").name("Salt").text("Rock salt").price(1.9f).available(true)
 				.sortName("sort2").build();
-		 Product product5 = Product.builder().id("5").name("Salt").text("Sea salt").price(2.1f).available(false)
+		Product product5 = Product.builder().id("5").name("Salt").text("Sea salt").price(2.1f).available(false)
 				.sortName("sort1").build();
 
 		repository.saveAll(Arrays.asList(product1, product2, product3, product4, product5));
@@ -165,7 +166,7 @@ abstract class QueryKeywordsTests {
 
 	@Test // DATAES-615
 	public void shouldSupportSortOnStandardFieldWithCriteria() {
-		 List<String> sortedIds = repository.findAllByNameOrderByText("Salt").stream() //
+		List<String> sortedIds = repository.findAllByNameOrderByText("Salt").stream() //
 				.map(it -> it.id).collect(Collectors.toList());
 
 		assertThat(sortedIds).containsExactly("4", "5");
@@ -174,7 +175,7 @@ abstract class QueryKeywordsTests {
 	@Test // DATAES-615
 	public void shouldSupportSortOnFieldWithCustomFieldNameWithCriteria() {
 
-		 List<String> sortedIds = repository.findAllByNameOrderBySortName("Sugar").stream() //
+		List<String> sortedIds = repository.findAllByNameOrderBySortName("Sugar").stream() //
 				.map(it -> it.id).collect(Collectors.toList());
 
 		assertThat(sortedIds).containsExactly("3", "2", "1");
@@ -182,7 +183,7 @@ abstract class QueryKeywordsTests {
 
 	@Test // DATAES-615
 	public void shouldSupportSortOnStandardFieldWithoutCriteria() {
-		 List<String> sortedIds = repository.findAllByOrderByText().stream() //
+		List<String> sortedIds = repository.findAllByOrderByText().stream() //
 				.map(it -> it.text).collect(Collectors.toList());
 
 		assertThat(sortedIds).containsExactly("Beet sugar", "Cane sugar", "Cane sugar", "Rock salt", "Sea salt");
@@ -191,12 +192,28 @@ abstract class QueryKeywordsTests {
 	@Test // DATAES-615
 	public void shouldSupportSortOnFieldWithCustomFieldNameWithoutCriteria() {
 
-		 List<String> sortedIds = repository.findAllByOrderBySortName().stream() //
+		List<String> sortedIds = repository.findAllByOrderBySortName().stream() //
 				.map(it -> it.id).collect(Collectors.toList());
 
 		assertThat(sortedIds).containsExactly("5", "4", "3", "2", "1");
 	}
 
+	@Test // DATAES-937
+	public void shouldReturnEmptyListOnFindByIdWithEmptyInputList() {
+
+		Iterable<Product> products = repository.findAllById(new ArrayList<>());
+
+		assertThat(products).isEmpty();
+	}
+
+	@Test // DATAES-937
+	public void shouldReturnEmptyListOnDerivedMethodWithEmptyInputList() {
+
+		Iterable<Product> products = repository.findAllByNameIn(new ArrayList<>());
+
+		assertThat(products).isEmpty();
+	}
+
 	/**
 	 * @author Mohsin Husen
 	 * @author Artur Konczak
@@ -279,6 +296,8 @@ abstract class QueryKeywordsTests {
 		List<Product> findAllByOrderByText();
 
 		List<Product> findAllByOrderBySortName();
+
+		List<Product> findAllByNameIn(List<String> names);
 	}
 
 }