From 486a11204606d6fba7784b49f15364a8f5095c2a Mon Sep 17 00:00:00 2001
From: rasmusfaber <r.faber@elsevier.com>
Date: Mon, 9 Sep 2019 20:37:57 +0200
Subject: [PATCH] DATAES-647: Use terms-query instead of multiple
 should-queries for In and NotIn.

Original PR: #315
---
 .../core/CriteriaQueryProcessor.java          | 24 +++----
 .../CustomMethodRepositoryBaseTests.java      | 67 +++++++++++++++++++
 2 files changed, 79 insertions(+), 12 deletions(-)

diff --git a/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java b/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java
index 78fe4d328..f3b19d241 100644
--- a/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java
+++ b/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java
@@ -19,6 +19,7 @@ import static org.elasticsearch.index.query.Operator.AND;
 import static org.elasticsearch.index.query.QueryBuilders.*;
 import static org.springframework.data.elasticsearch.core.query.Criteria.*;
 
+import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -36,6 +37,7 @@ import org.springframework.util.Assert;
  * @author Mohsin Husen
  * @author Franck Marchand
  * @author Artur Konczak
+ * @author Rasmus Faber-Espensen
  */
 class CriteriaQueryProcessor {
 
@@ -142,8 +144,6 @@ class CriteriaQueryProcessor {
 
 		String searchText = StringUtils.toString(value);
 
-		Iterable<Object> collection = null;
-
 		switch (key) {
 			case EQUALS:
 				query = queryStringQuery(searchText).field(fieldName).defaultOperator(AND);
@@ -180,23 +180,23 @@ class CriteriaQueryProcessor {
 				query = fuzzyQuery(fieldName, searchText);
 				break;
 			case IN:
-				query = boolQuery();
-				collection = (Iterable<Object>) value;
-				for (Object item : collection) {
-					((BoolQueryBuilder) query).should(queryStringQuery(item.toString()).field(fieldName));
-				}
+				query = boolQuery().must(termsQuery(fieldName, toStringList((Iterable<Object>) value)));
 				break;
 			case NOT_IN:
-				query = boolQuery();
-				collection = (Iterable<Object>) value;
-				for (Object item : collection) {
-					((BoolQueryBuilder) query).mustNot(queryStringQuery(item.toString()).field(fieldName));
-				}
+				query = boolQuery().mustNot(termsQuery(fieldName, toStringList((Iterable<Object>) value)));
 				break;
 		}
 		return query;
 	}
 
+	private static List<String> toStringList(Iterable<?> iterable){
+		List<String> list = new ArrayList<>();
+		for (Object item : iterable) {
+			list.add(StringUtils.toString(item));
+		}
+		return list;
+	}
+
 	private void addBoost(QueryBuilder query, float boost) {
 		if (Float.isNaN(boost)) {
 			return;
diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryBaseTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryBaseTests.java
index 2705908b1..db0523cce 100644
--- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryBaseTests.java
+++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryBaseTests.java
@@ -331,6 +331,68 @@ public abstract class CustomMethodRepositoryBaseTests {
 		assertThat(page.getContent().get(0).getId()).isEqualTo(documentId2);
 	}
 
+	@Test // DATAES-647
+	public void shouldHandleManyValuesQueryingIn() {
+
+		// given
+		String documentId1 = randomNumeric(32);
+		SampleEntity sampleEntity1 = new SampleEntity();
+		sampleEntity1.setId(documentId1);
+		sampleEntity1.setKeyword("foo");
+		repository.save(sampleEntity1);
+
+		String documentId2 = randomNumeric(32);
+		SampleEntity sampleEntity2 = new SampleEntity();
+		sampleEntity2.setId(documentId2);
+		sampleEntity2.setKeyword("bar");
+		repository.save(sampleEntity2);
+
+		List<String> keywords = new ArrayList<>();
+		keywords.add("foo");
+
+		for (int i = 0; i < 1025; i++) {
+			keywords.add(randomNumeric(32));
+		}
+
+		// when
+		List<SampleEntity> list = repository.findByKeywordIn(keywords);
+
+		// then
+		assertThat(list.size()).isEqualTo(1L);
+		assertThat(list.get(0).getId()).isEqualTo(documentId1);
+	}
+
+	@Test // DATAES-647
+	public void shouldHandleManyValuesQueryingNotIn() {
+
+		// given
+		String documentId1 = randomNumeric(32);
+		SampleEntity sampleEntity1 = new SampleEntity();
+		sampleEntity1.setId(documentId1);
+		sampleEntity1.setKeyword("foo");
+		repository.save(sampleEntity1);
+
+		String documentId2 = randomNumeric(32);
+		SampleEntity sampleEntity2 = new SampleEntity();
+		sampleEntity2.setId(documentId2);
+		sampleEntity2.setKeyword("bar");
+		repository.save(sampleEntity2);
+
+		List<String> keywords = new ArrayList<>();
+		keywords.add("foo");
+
+		for (int i = 0; i < 1025; i++) {
+			keywords.add(randomNumeric(32));
+		}
+
+		// when
+		List<SampleEntity> list = repository.findByKeywordNotIn(keywords);
+
+		// then
+		assertThat(list.size()).isEqualTo(1L);
+		assertThat(list.get(0).getId()).isEqualTo(documentId2);
+	}
+
 	@Test
 	public void shouldExecuteCustomMethodForTrue() {
 
@@ -1296,6 +1358,7 @@ public abstract class CustomMethodRepositoryBaseTests {
 		@Id private String id;
 		@Field(type = Text, store = true, fielddata = true) private String type;
 		@Field(type = Text, store = true, fielddata = true) private String message;
+		@Field(type = Keyword) private String keyword;
 		private int rate;
 		private boolean available;
 		private GeoPoint location;
@@ -1337,6 +1400,10 @@ public abstract class CustomMethodRepositoryBaseTests {
 
 		Page<SampleEntity> findByIdIn(List<String> ids, Pageable pageable);
 
+		List<SampleEntity> findByKeywordIn(List<String> keywords);
+
+		List<SampleEntity> findByKeywordNotIn(List<String> keywords);
+
 		Page<SampleEntity> findByIdNotIn(List<String> ids, Pageable pageable);
 
 		Page<SampleEntity> findByAvailableTrue(Pageable pageable);