Nested Criteria queries must consider sub-fields.

Original Pull Request #1760 
Closes #1758

Author: sothawo

Diff for: src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java

@@ -18,6 +18,7 @@
 import static org.elasticsearch.index.query.Operator.*;
 import static org.elasticsearch.index.query.QueryBuilders.*;
 import static org.springframework.data.elasticsearch.core.query.Criteria.*;
+import static org.springframework.util.StringUtils.*;
 
 import java.util.ArrayList;
 import java.util.Iterator;
@@ -154,11 +155,8 @@ private QueryBuilder queryForEntries(Criteria criteria) {
 
 		addBoost(query, criteria.getBoost());
 
-		int dotPosition = fieldName.lastIndexOf('.');
-
-		if (dotPosition > 0) {
-			String nestedPath = fieldName.substring(0, dotPosition);
-			query = nestedQuery(nestedPath, query, ScoreMode.Avg);
+		if (hasText(field.getPath())) {
+			query = nestedQuery(field.getPath(), query, ScoreMode.Avg);
 		}
 
 		return query;

Diff for: src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java

@@ -86,21 +86,18 @@ default Document mapObject(@Nullable Object source) {
 
 	// region query
 	/**
-	 * Updates a query by renaming the property names in the query to the correct mapped field names and the values to the
-	 * converted values if the {@link ElasticsearchPersistentProperty} for a property has a
+	 * Updates a {@link CriteriaQuery} by renaming the property names in the query to the correct mapped field names and
+	 * the values to the converted values if the {@link ElasticsearchPersistentProperty} for a property has a
 	 * {@link org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentPropertyConverter}. If
-	 * domainClass is null, it's a noop; handling null here eliminates null checks in the caller.
-	 * 
+	 * domainClass is null or query is not a {@link CriteriaQuery}, it's a noop.
+	 *
 	 * @param query the query that is internally updated
 	 * @param domainClass the class of the object that is searched with the query
 	 */
 	default void updateQuery(Query query, @Nullable Class<?> domainClass) {
 
-		if (domainClass != null) {
-
-			if (query instanceof CriteriaQuery) {
-				updateCriteriaQuery((CriteriaQuery) query, domainClass);
-			}
+		if (domainClass != null && query instanceof CriteriaQuery) {
+			updateCriteriaQuery((CriteriaQuery) query, domainClass);
 		}
 	}
 
@@ -109,8 +106,8 @@ default void updateQuery(Query query, @Nullable Class<?> domainClass) {
 	 * the values to the converted values if the {@link ElasticsearchPersistentProperty} for a property has a
 	 * {@link org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentPropertyConverter}.
 	 *
-	 * @param criteriaQuery the query that is internally updated
-	 * @param domainClass the class of the object that is searched with the query
+	 * @param criteriaQuery the query that is internally updated, must not be {@literal null}
+	 * @param domainClass the class of the object that is searched with the query, must not be {@literal null}
 	 */
 	void updateCriteriaQuery(CriteriaQuery criteriaQuery, Class<?> domainClass);
 	// endregion

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

@@ -1025,6 +1025,9 @@ private static Collection<?> asCollection(Object source) {
 	@Override
 	public void updateCriteriaQuery(CriteriaQuery criteriaQuery, Class<?> domainClass) {
 
+		Assert.notNull(criteriaQuery, "criteriaQuery must not be null");
+		Assert.notNull(domainClass, "domainClass must not be null");
+
 		ElasticsearchPersistentEntity<?> persistentEntity = mappingContext.getPersistentEntity(domainClass);
 
 		if (persistentEntity != null) {
@@ -1048,12 +1051,15 @@ private void updateCriteria(Criteria criteria, ElasticsearchPersistentEntity<?>
 		}
 
 		String[] fieldNames = field.getName().split("\\.");
+
 		ElasticsearchPersistentEntity<?> currentEntity = persistentEntity;
 		ElasticsearchPersistentProperty persistentProperty = null;
+		int propertyCount = 0;
 		for (int i = 0; i < fieldNames.length; i++) {
 			persistentProperty = currentEntity.getPersistentProperty(fieldNames[i]);
 
 			if (persistentProperty != null) {
+				propertyCount++;
 				fieldNames[i] = persistentProperty.getFieldName();
 				try {
 					currentEntity = mappingContext.getPersistentEntity(persistentProperty.getActualType());
@@ -1071,6 +1077,11 @@ private void updateCriteria(Criteria criteria, ElasticsearchPersistentEntity<?>
 
 		field.setName(String.join(".", fieldNames));
 
+		if (propertyCount > 1) {
+			List<String> propertyNames = Arrays.asList(fieldNames);
+			field.setPath(String.join(".", propertyNames.subList(0, propertyCount - 1)));
+		}
+
 		if (persistentProperty != null) {
 
 			if (persistentProperty.hasPropertyConverter()) {

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

@@ -41,4 +41,17 @@ public interface Field {
 	 */
 	@Nullable
 	FieldType getFieldType();
+
+	/**
+	 * Sets the path if this field has a multi-part name that should be used in a nested query.
+	 * @param path the value to set
+	 * @since 4.2
+	 */
+	void setPath(@Nullable String path);
+
+	/**
+	 * @return the path if this is a field for a nested query
+	 * @since 4.2
+	 */
+	@Nullable String getPath();
 }

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

@@ -31,6 +31,7 @@ public class SimpleField implements Field {
 
 	private String name;
 	@Nullable private FieldType fieldType;
+	@Nullable private String path;
 
 	public SimpleField(String name) {
 
@@ -63,6 +64,17 @@ public FieldType getFieldType() {
 		return fieldType;
 	}
 
+	@Override
+	public void setPath(@Nullable String path) {
+		this.path = path;
+	}
+
+	@Override
+	@Nullable
+	public String getPath() {
+		return path;
+	}
+
 	@Override
 	public String toString() {
 		return getName();

Diff for: src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java

@@ -34,6 +34,8 @@
 import org.springframework.data.elasticsearch.annotations.DateFormat;
 import org.springframework.data.elasticsearch.annotations.Field;
 import org.springframework.data.elasticsearch.annotations.FieldType;
+import org.springframework.data.elasticsearch.annotations.InnerField;
+import org.springframework.data.elasticsearch.annotations.MultiField;
 import org.springframework.data.elasticsearch.core.convert.GeoConverters;
 import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
 import org.springframework.data.elasticsearch.core.document.Document;
@@ -362,6 +364,39 @@ void shouldMapNamesAndValueInNestedEntities() throws JSONException {
 
 		assertEquals(expected, queryString, false);
 	}
+
+	@Test // #1753
+	@DisplayName("should map names and value in nested entities with sub-fields")
+	void shouldMapNamesAndValueInNestedEntitiesWithSubfields() throws JSONException {
+
+		String expected = "{\n" + //
+				"  \"bool\": {\n" + //
+				"    \"must\": [\n" + //
+				"      {\n" + //
+				"        \"nested\": {\n" + //
+				"          \"query\": {\n" + //
+				"            \"query_string\": {\n" + //
+				"              \"query\": \"Foobar\",\n" + //
+				"              \"fields\": [\n" + //
+				"                \"per-sons.nick-name.keyword^1.0\"\n" + //
+				"              ]\n" + //
+				"            }\n" + //
+				"          },\n" + //
+				"          \"path\": \"per-sons\"\n" + //
+				"        }\n" + //
+				"      }\n" + //
+				"    ]\n" + //
+				"  }\n" + //
+				"}\n"; //
+
+		CriteriaQuery criteriaQuery = new CriteriaQuery(
+				new Criteria("persons.nickName.keyword").is("Foobar")
+		);
+		mappingElasticsearchConverter.updateQuery(criteriaQuery, House.class);
+		String queryString = new CriteriaQueryProcessor().createQuery(criteriaQuery.getCriteria()).toString();
+
+		assertEquals(expected, queryString, false);
+	}
 	// endregion
 
 	// region helper functions
@@ -379,6 +414,7 @@ static class Person {
 		@Nullable @Id String id;
 		@Nullable @Field(name = "first-name") String firstName;
 		@Nullable @Field(name = "last-name") String lastName;
+		@Nullable @MultiField(mainField = @Field(name="nick-name"), otherFields = {@InnerField(suffix = "keyword", type = FieldType.Keyword)}) String nickName;
 		@Nullable @Field(name = "created-date", type = FieldType.Date, format = DateFormat.epoch_millis) Date createdDate;
 		@Nullable @Field(name = "birth-date", type = FieldType.Date, format = {},
 				pattern = "dd.MM.uuuu") LocalDate birthDate;

Diff for: src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessorUnitTests.java

@@ -365,6 +365,7 @@ void shouldBuildNestedQuery() throws JSONException {
 				"}"; //
 
 		Criteria criteria = new Criteria("houses.inhabitants.lastName").is("murphy");
+		criteria.getField().setPath("houses.inhabitants");
 
 		String query = queryProcessor.createQuery(criteria).toString();