From ab73c68ca982e981cf810e7befefb10e37ff86c1 Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Mon, 5 Apr 2021 15:42:02 +0200 Subject: [PATCH] Nested Criteria queries must consider sub-fields. Original Pull Request #1760 Closes #1758 --- .../core/CriteriaQueryProcessor.java | 8 ++--- .../core/convert/ElasticsearchConverter.java | 19 +++++----- .../MappingElasticsearchConverter.java | 11 ++++++ .../data/elasticsearch/core/query/Field.java | 13 +++++++ .../elasticsearch/core/query/SimpleField.java | 12 +++++++ .../core/CriteriaQueryMappingUnitTests.java | 36 +++++++++++++++++++ .../core/CriteriaQueryProcessorUnitTests.java | 1 + 7 files changed, 84 insertions(+), 16 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 b6ed81279..02988ced4 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java @@ -18,6 +18,7 @@ package org.springframework.data.elasticsearch.core; 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 @@ class CriteriaQueryProcessor { 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 --git a/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java b/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java index ebdd2bfa4..a4a40a15e 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java @@ -86,21 +86,18 @@ public interface ElasticsearchConverter // 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 @@ public interface ElasticsearchConverter * 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 --git a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java index b544021d1..1f3dc30d1 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java @@ -1025,6 +1025,9 @@ public class MappingElasticsearchConverter @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 @@ public class MappingElasticsearchConverter } 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 @@ public class MappingElasticsearchConverter field.setName(String.join(".", fieldNames)); + if (propertyCount > 1) { + List propertyNames = Arrays.asList(fieldNames); + field.setPath(String.join(".", propertyNames.subList(0, propertyCount - 1))); + } + if (persistentProperty != null) { if (persistentProperty.hasPropertyConverter()) { diff --git a/src/main/java/org/springframework/data/elasticsearch/core/query/Field.java b/src/main/java/org/springframework/data/elasticsearch/core/query/Field.java index 760664e0e..a2567fea5 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/Field.java +++ b/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 --git a/src/main/java/org/springframework/data/elasticsearch/core/query/SimpleField.java b/src/main/java/org/springframework/data/elasticsearch/core/query/SimpleField.java index ca1d77342..67866728d 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/query/SimpleField.java +++ b/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 class SimpleField implements Field { 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 --git a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java index 4cd01d1ef..10f2344b6 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java @@ -34,6 +34,8 @@ import org.springframework.data.annotation.Id; 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 @@ public class CriteriaQueryMappingUnitTests { 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 @@ public class CriteriaQueryMappingUnitTests { @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 --git a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessorUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessorUnitTests.java index fdaa1b520..222b18376 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessorUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessorUnitTests.java @@ -365,6 +365,7 @@ class CriteriaQueryProcessorUnitTests { "}"; // Criteria criteria = new Criteria("houses.inhabitants.lastName").is("murphy"); + criteria.getField().setPath("houses.inhabitants"); String query = queryProcessor.createQuery(criteria).toString();