DATAES-912 - Derived Query with "In" Keyword does not work on Text field.

Original PR: #510

src/main/asciidoc/reference/elasticsearch-repository-queries.adoc

@@ -48,7 +48,9 @@ A list of supported keywords for Elasticsearch is shown below.
 |===
 | Keyword
 | Sample
-| Elasticsearch Query String| `And`
+| Elasticsearch Query String
+
+| `And`
 | `findByNameAndPrice`
 | `{ "query" : {
 "bool" : {
@@ -201,7 +203,7 @@ A list of supported keywords for Elasticsearch is shown below.
   }
 }}`
 
-| `In`
+| `In` (when annotated as FieldType.Keyword)
 | `findByNameIn(Collection<String>names)`
 | `{ "query" : {
 "bool" : {
@@ -215,7 +217,12 @@ A list of supported keywords for Elasticsearch is shown below.
   }
 }}`
 
-| `NotIn`
+
+| `In`
+| `findByNameIn(Collection<String>names)`
+| `{ "query": {"bool": {"must": [{"query_string":{"query": "\"?\" \"?\"", "fields": ["name"]}}]}}}`
+
+| `NotIn`  (when annotated as FieldType.Keyword)
 | `findByNameNotIn(Collection<String>names)`
 | `{ "query" : {
 "bool" : {
@@ -229,6 +236,10 @@ A list of supported keywords for Elasticsearch is shown below.
   }
 }}`
 
+| `NotIn`
+| `findByNameNotIn(Collection<String>names)`
+| `{"query": {"bool": {"must": [{"query_string": {"query": "NOT(\"?\" \"?\")", "fields": ["name"]}}]}}}`
+
 | `Near`
 | `findByStoreNear`
 | `Not Supported Yet !`

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

@@ -26,7 +26,9 @@ import java.util.List;
 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.annotations.FieldType;
 import org.springframework.data.elasticsearch.core.query.Criteria;
+import org.springframework.data.elasticsearch.core.query.Field;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
@@ -128,23 +130,24 @@ class CriteriaQueryProcessor {
 	@Nullable
 	private QueryBuilder queryForEntries(Criteria criteria) {
 
-		if (criteria.getField() == null || criteria.getQueryCriteriaEntries().isEmpty())
+		Field field = criteria.getField();
+
+		if (field == null || criteria.getQueryCriteriaEntries().isEmpty())
 			return null;
 
-		String fieldName = criteria.getField().getName();
+		String fieldName = field.getName();
-
 		Assert.notNull(fieldName, "Unknown field");
 
 		Iterator<Criteria.CriteriaEntry> it = criteria.getQueryCriteriaEntries().iterator();
 		QueryBuilder query;
 
 		if (criteria.getQueryCriteriaEntries().size() == 1) {
-			query = queryFor(it.next(), fieldName);
+			query = queryFor(it.next(), field);
 		} else {
 			query = boolQuery();
 			while (it.hasNext()) {
 				Criteria.CriteriaEntry entry = it.next();
-				((BoolQueryBuilder) query).must(queryFor(entry, fieldName));
+				((BoolQueryBuilder) query).must(queryFor(entry, field));
 			}
 		}
 
@@ -153,7 +156,11 @@ class CriteriaQueryProcessor {
 	}
 
 	@Nullable
-	private QueryBuilder queryFor(Criteria.CriteriaEntry entry, String fieldName) {
+	private QueryBuilder queryFor(Criteria.CriteriaEntry entry, Field field) {
+
+		String fieldName = field.getName();
+		boolean isKeywordField = FieldType.Keyword == field.getFieldType();
+
 		OperationKey key = entry.getKey();
 
 		if (key == OperationKey.EXISTS) {
@@ -209,13 +216,21 @@ class CriteriaQueryProcessor {
 			case IN:
 				if (value instanceof Iterable) {
 					Iterable<?> iterable = (Iterable<?>) value;
-					query = boolQuery().must(termsQuery(fieldName, toStringList(iterable)));
+					if (isKeywordField) {
+						query = boolQuery().must(termsQuery(fieldName, toStringList(iterable)));
+					} else {
+						query = queryStringQuery(orQueryString(iterable)).field(fieldName);
+					}
 				}
 				break;
 			case NOT_IN:
 				if (value instanceof Iterable) {
 					Iterable<?> iterable = (Iterable<?>) value;
-					query = boolQuery().mustNot(termsQuery(fieldName, toStringList(iterable)));
+					if (isKeywordField) {
+						query = boolQuery().mustNot(termsQuery(fieldName, toStringList(iterable)));
+					} else {
+						query = queryStringQuery("NOT(" + orQueryString(iterable) + ')').field(fieldName);
+					}
 				}
 				break;
 		}
@@ -230,6 +245,25 @@ class CriteriaQueryProcessor {
 		return list;
 	}
 
+	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();
+	}
+
 	private void addBoost(@Nullable QueryBuilder query, float boost) {
 
 		if (query == null || Float.isNaN(boost)) {

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

@@ -92,8 +92,8 @@ public class ElasticsearchRestTemplate extends AbstractElasticsearchTemplate {
 
 	private static final Logger LOGGER = LoggerFactory.getLogger(ElasticsearchRestTemplate.class);
 
-	private RestHighLevelClient client;
+	private final RestHighLevelClient client;
-	private ElasticsearchExceptionTranslator exceptionTranslator;
+	private final ElasticsearchExceptionTranslator exceptionTranslator;
 
 	// region Initialization
 	public ElasticsearchRestTemplate(RestHighLevelClient client) {
@@ -241,11 +241,11 @@ public class ElasticsearchRestTemplate extends AbstractElasticsearchTemplate {
 			IndexCoordinates index) {
 		maybeCallbackBeforeConvertWithQueries(queries, index);
 		BulkRequest bulkRequest = requestFactory.bulkRequest(queries, bulkOptions, index);
-		List<IndexedObjectInformation> indexedObjectInformations = checkForBulkOperationFailure(
+		List<IndexedObjectInformation> indexedObjectInformationList = checkForBulkOperationFailure(
 				execute(client -> client.bulk(bulkRequest, RequestOptions.DEFAULT)));
-		updateIndexedObjectsWithQueries(queries, indexedObjectInformations);
+		updateIndexedObjectsWithQueries(queries, indexedObjectInformationList);
 		maybeCallbackAfterSaveWithQueries(queries, index);
-		return indexedObjectInformations;
+		return indexedObjectInformationList;
 	}
 	// endregion
 

src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java

@@ -50,6 +50,7 @@ import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersiste
 import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentPropertyConverter;
 import org.springframework.data.elasticsearch.core.query.Criteria;
 import org.springframework.data.elasticsearch.core.query.CriteriaQuery;
+import org.springframework.data.elasticsearch.core.query.Field;
 import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.mapping.context.MappingContext;
@@ -94,7 +95,7 @@ public class MappingElasticsearchConverter
 
 	private final ElasticsearchTypeMapper typeMapper;
 
-	private ConcurrentHashMap<String, Integer> propertyWarnings = new ConcurrentHashMap<>();
+	private final ConcurrentHashMap<String, Integer> propertyWarnings = new ConcurrentHashMap<>();
 
 	public MappingElasticsearchConverter(
 			MappingContext<? extends ElasticsearchPersistentEntity<?>, ElasticsearchPersistentProperty> mappingContext) {
@@ -579,13 +580,13 @@ public class MappingElasticsearchConverter
 		}
 
 		if (property.isEntity() || !isSimpleType(value)) {
-			return writeEntity(value, property, typeHint);
+			return writeEntity(value, property);
 		}
 
 		return value;
 	}
 
-	private Object writeEntity(Object value, ElasticsearchPersistentProperty property, TypeInformation<?> typeHint) {
+	private Object writeEntity(Object value, ElasticsearchPersistentProperty property) {
 
 		Document target = Document.create();
 		writeEntity(mappingContext.getRequiredPersistentEntity(value.getClass()), value, target,
@@ -769,11 +770,17 @@ public class MappingElasticsearchConverter
 	}
 
 	private void updateCriteria(Criteria criteria, ElasticsearchPersistentEntity<?> persistentEntity) {
-		String name = criteria.getField().getName();
+		Field field = criteria.getField();
+
+		if (field == null) {
+			return;
+		}
+
+		String name = field.getName();
 		ElasticsearchPersistentProperty property = persistentEntity.getPersistentProperty(name);
 
 		if (property != null && property.getName().equals(name)) {
-			criteria.getField().setName(property.getFieldName());
+			field.setName(property.getFieldName());
 
 			if (property.hasPropertyConverter()) {
 				ElasticsearchPersistentPropertyConverter propertyConverter = property.getPropertyConverter();
@@ -789,6 +796,12 @@ public class MappingElasticsearchConverter
 					}
 				});
 			}
+
+			org.springframework.data.elasticsearch.annotations.Field fieldAnnotation = property.findAnnotation(org.springframework.data.elasticsearch.annotations.Field.class);
+
+			if (fieldAnnotation != null) {
+				field.setFieldType(fieldAnnotation.type());
+			}
 		}
 
 		for (Criteria subCriteria : criteria.getSubCriteria()) {

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

@@ -15,6 +15,9 @@
  */
 package org.springframework.data.elasticsearch.core.query;
 
+import org.springframework.data.elasticsearch.annotations.FieldType;
+import org.springframework.lang.Nullable;
+
 /**
  * Defines a Field that can be used within a Criteria.
  *
@@ -27,4 +30,15 @@ public interface Field {
 	void setName(String name);
 
 	String getName();
+
+	/**
+	 * @param fieldType sets the field's type
+	 */
+	void setFieldType(FieldType fieldType);
+
+	/**
+	 * @return The annotated FieldType of the field
+	 */
+	@Nullable
+	FieldType getFieldType();
 }

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

@@ -15,10 +15,12 @@
  */
 package org.springframework.data.elasticsearch.core.query;
 
+import org.springframework.data.elasticsearch.annotations.FieldType;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
- * The most trivial implementation of a Field. The {@link #name} is updateable, so it may be changed during query
+ * The most trivial implementation of a Field. The {@link #name} is updatable, so it may be changed during query
  * preparation by the {@link org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter}.
  *
  * @author Rizwan Idrees
@@ -28,6 +30,7 @@ import org.springframework.util.Assert;
 public class SimpleField implements Field {
 
 	private String name;
+	@Nullable private FieldType fieldType;
 
 	public SimpleField(String name) {
 
@@ -49,6 +52,17 @@ public class SimpleField implements Field {
 		return name;
 	}
 
+	@Override
+	public void setFieldType(FieldType fieldType) {
+		this.fieldType = fieldType;
+	}
+
+	@Nullable
+	@Override
+	public FieldType getFieldType() {
+		return fieldType;
+	}
+
 	@Override
 	public String toString() {
 		return getName();

src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchPartQueryTests.java

@@ -197,14 +197,22 @@ class ElasticsearchPartQueryTests {
 
 		String query = getQueryBuilder(methodName, parameterClasses, parameters);
 
-		String expected = "{\"query\": {" + //
+		String expected = "{\n" + //
-				"  \"bool\" : {" + //
+				"  \"query\": {\n" + //
-				"    \"must\" : [" + //
+				"    \"bool\": {\n" + //
-				"      {\"bool\" : {\"must\" : [{\"terms\" : {\"name\" : [\"" + names.get(0) + "\", \"" + names.get(1)
+				"      \"must\": [\n" + //
-				+ "\"]}}]}}" + //
+				"        {\n" + //
-				"    ]" + //
+				"          \"query_string\": {\n" + //
-				"  }" + //
+				"            \"query\": \"\\\"Title\\\" \\\"Title2\\\"\",\n" + //
-				"}}"; //
+				"            \"fields\": [\n" + //
+				"              \"name^1.0\"\n" + //
+				"            ]\n" + //
+				"          }\n" + //
+				"        }\n" + //
+				"      ]\n" + //
+				"    }\n" + //
+				"  }\n" + //
+				"}\n"; //
 
 		assertEquals(expected, query, false);
 	}
@@ -220,14 +228,22 @@ class ElasticsearchPartQueryTests {
 
 		String query = getQueryBuilder(methodName, parameterClasses, parameters);
 
-		String expected = "{\"query\": {" + //
+		String expected = "{\n" + //
-				"  \"bool\" : {" + //
+				"  \"query\": {\n" + //
-				"    \"must\" : [" + //
+				"    \"bool\": {\n" + //
-				"      {\"bool\" : {\"must_not\" : [{\"terms\" : {\"name\" : [\"" + names.get(0) + "\", \"" + names.get(1)
+				"      \"must\": [\n" + //
-				+ "\"]}}]}}" + //
+				"        {\n" + //
-				"    ]" + //
+				"          \"query_string\": {\n" + //
-				"  }" + //
+				"            \"query\": \"NOT(\\\"Title\\\" \\\"Title2\\\")\",\n" + //
-				"}}"; //
+				"            \"fields\": [\n" + //
+				"              \"name^1.0\"\n" + //
+				"            ]\n" + //
+				"          }\n" + //
+				"        }\n" + //
+				"      ]\n" + //
+				"    }\n" + //
+				"  }\n" + //
+				"}\n"; //
 
 		assertEquals(expected, query, false);
 	}

src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryBaseTests.java

@@ -360,7 +360,7 @@ public abstract class CustomMethodRepositoryBaseTests {
 	}
 
 	@Test // DATAES-647
-	public void shouldHandleManyValuesQueryingIn() {
+	public void shouldHandleManyKeywordValuesQueryingIn() {
 
 		// given
 		String documentId1 = nextIdAsString();
@@ -378,7 +378,8 @@ public abstract class CustomMethodRepositoryBaseTests {
 		List<String> keywords = new ArrayList<>();
 		keywords.add("foo");
 
-		for (int i = 0; i < 1025; i++) {
+		// limit for normal query clauses is 1024, for keywords we change to terms queries
+		for (int i = 0; i < 1200; i++) {
 			keywords.add(nextIdAsString());
 		}
 
@@ -391,7 +392,7 @@ public abstract class CustomMethodRepositoryBaseTests {
 	}
 
 	@Test // DATAES-647
-	public void shouldHandleManyValuesQueryingNotIn() {
+	public void shouldHandleManyKeywordValuesQueryingNotIn() {
 
 		// given
 		String documentId1 = nextIdAsString();
@@ -409,7 +410,8 @@ public abstract class CustomMethodRepositoryBaseTests {
 		List<String> keywords = new ArrayList<>();
 		keywords.add("foo");
 
-		for (int i = 0; i < 1025; i++) {
+		// limit for normal query clauses is 1024, for keywords we change to terms queries
+		for (int i = 0; i < 1200; i++) {
 			keywords.add(nextIdAsString());
 		}
 
@@ -421,6 +423,46 @@ public abstract class CustomMethodRepositoryBaseTests {
 		assertThat(list.get(0).getId()).isEqualTo(documentId2);
 	}
 
+	@Test // DATAES-912
+	void shouldHandleTextFieldQueryingIn() {
+		String documentId1 = nextIdAsString();
+		SampleEntity sampleEntity1 = new SampleEntity();
+		sampleEntity1.setId(documentId1);
+		sampleEntity1.setMessage("foo");
+		repository.save(sampleEntity1);
+
+		String documentId2 = nextIdAsString();
+		SampleEntity sampleEntity2 = new SampleEntity();
+		sampleEntity2.setId(documentId2);
+		sampleEntity2.setMessage("bar");
+		repository.save(sampleEntity2);
+
+		List<SampleEntity> list = repository.findByMessageIn(Arrays.asList("Foo", "Bar"));
+
+		assertThat(list).hasSize(2);
+		assertThat(list.stream().map(SampleEntity::getId)).containsExactlyInAnyOrder(documentId1, documentId2);
+	}
+
+	@Test // DATAES-912
+	void shouldHandleTextFieldQueryingNotIn() {
+		String documentId1 = nextIdAsString();
+		SampleEntity sampleEntity1 = new SampleEntity();
+		sampleEntity1.setId(documentId1);
+		sampleEntity1.setMessage("foo");
+		repository.save(sampleEntity1);
+
+		String documentId2 = nextIdAsString();
+		SampleEntity sampleEntity2 = new SampleEntity();
+		sampleEntity2.setId(documentId2);
+		sampleEntity2.setMessage("bar");
+		repository.save(sampleEntity2);
+
+		List<SampleEntity> list = repository.findByMessageNotIn(Arrays.asList("Boo", "Bar"));
+
+		assertThat(list).hasSize(1);
+		assertThat(list.get(0).getId()).isEqualTo(documentId1);
+	}
+
 	@Test
 	public void shouldExecuteCustomMethodForTrue() {
 
@@ -1622,6 +1664,10 @@ public abstract class CustomMethodRepositoryBaseTests {
 
 		List<SampleEntity> findByKeywordNotIn(List<String> keywords);
 
+		List<SampleEntity> findByMessageIn(List<String> keywords);
+
+		List<SampleEntity> findByMessageNotIn(List<String> keywords);
+
 		Page<SampleEntity> findByIdNotIn(List<String> ids, Pageable pageable);
 
 		Page<SampleEntity> findByAvailableTrue(Pageable pageable);