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 e252e6783..b6ed81279 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessor.java @@ -24,6 +24,7 @@ import java.util.List; import org.apache.lucene.queryparser.flexible.standard.QueryParserUtil; +import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.springframework.data.elasticsearch.annotations.FieldType; @@ -136,7 +137,7 @@ private QueryBuilder queryForEntries(Criteria criteria) { return null; String fieldName = field.getName(); - Assert.notNull(fieldName, "Unknown field"); + Assert.notNull(fieldName, "Unknown field " + fieldName); Iterator it = criteria.getQueryCriteriaEntries().iterator(); QueryBuilder query; @@ -152,6 +153,14 @@ 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); + } + return query; } 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 226779e3a..b544021d1 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 @@ -168,8 +168,15 @@ public void afterPropertiesSet() { @SuppressWarnings("unchecked") @Override public R read(Class type, Document source) { + TypeInformation typeHint = ClassTypeInformation.from((Class) ClassUtils.getUserClass(type)); - return read(typeHint, source); + R r = read(typeHint, source); + + if (r == null) { + throw new ConversionException("could not convert into object of class " + type); + } + + return r; } protected R readEntity(ElasticsearchPersistentEntity entity, Map source) { @@ -188,7 +195,7 @@ protected R readEntity(ElasticsearchPersistentEntity entity, Map getParameterProv ElasticsearchPropertyValueProvider provider = new ElasticsearchPropertyValueProvider(source, evaluator); // TODO: Support for non-static inner classes via ObjectPath + // noinspection ConstantConditions PersistentEntityParameterValueProvider parameterProvider = new PersistentEntityParameterValueProvider<>( entity, provider, null); @@ -281,7 +289,6 @@ protected R readProperties(ElasticsearchPersistentEntity entity, R instan return accessor.getBean(); } - @SuppressWarnings("unchecked") @Nullable protected R readValue(@Nullable Object value, ElasticsearchPersistentProperty property, TypeInformation type) { @@ -349,7 +356,7 @@ private R read(TypeInformation type, Map source) { } if (typeToUse.isMap()) { - return (R) readMap(typeToUse, source); + return readMap(typeToUse, source); } if (typeToUse.equals(ClassTypeInformation.OBJECT)) { @@ -549,7 +556,7 @@ public void write(Object source, Document sink) { } Class entityType = ClassUtils.getUserClass(source.getClass()); - TypeInformation typeInformation = ClassTypeInformation.from(entityType); + TypeInformation typeInformation = ClassTypeInformation.from(entityType); if (requiresTypeHint(entityType)) { typeMapper.writeType(typeInformation, sink); @@ -561,9 +568,9 @@ public void write(Object source, Document sink) { /** * Internal write conversion method which should be used for nested invocations. * - * @param source - * @param sink - * @param typeInformation + * @param source the object to write + * @param sink the write destination + * @param typeInformation type information for the source */ @SuppressWarnings("unchecked") protected void writeInternal(@Nullable Object source, Map sink, @@ -578,7 +585,10 @@ protected void writeInternal(@Nullable Object source, Map sink, if (customTarget.isPresent()) { Map result = conversionService.convert(source, Map.class); - sink.putAll(result); + + if (result != null) { + sink.putAll(result); + } return; } @@ -600,9 +610,9 @@ protected void writeInternal(@Nullable Object source, Map sink, /** * Internal write conversion method which should be used for nested invocations. * - * @param source - * @param sink - * @param entity + * @param source the object to write + * @param sink the write destination + * @param entity entity for the source */ protected void writeInternal(@Nullable Object source, Map sink, @Nullable ElasticsearchPersistentEntity entity) { @@ -734,7 +744,6 @@ protected void writeProperty(ElasticsearchPersistentProperty property, Object va * * @param collection must not be {@literal null}. * @param property must not be {@literal null}. - * @return */ protected List createCollection(Collection collection, ElasticsearchPersistentProperty property) { return writeCollectionInternal(collection, property.getTypeInformation(), new ArrayList<>(collection.size())); @@ -745,7 +754,6 @@ protected List createCollection(Collection collection, ElasticsearchP * * @param map must not {@literal null}. * @param property must not be {@literal null}. - * @return */ protected Map createMap(Map map, ElasticsearchPersistentProperty property) { @@ -761,7 +769,6 @@ protected Map createMap(Map map, ElasticsearchPersistentPr * @param source must not be {@literal null}. * @param sink must not be {@literal null}. * @param propertyType must not be {@literal null}. - * @return */ protected Map writeMapInternal(Map source, Map sink, TypeInformation propertyType) { @@ -801,7 +808,6 @@ protected Map writeMapInternal(Map source, Map writeCollectionInternal(Collection source, @Nullable TypeInformation type, @@ -837,8 +843,7 @@ private List writeCollectionInternal(Collection source, @Nullable Typ /** * Returns a {@link String} representation of the given {@link Map} key * - * @param key - * @return + * @param key the key to convert */ private String potentiallyConvertMapKey(Object key) { @@ -846,17 +851,22 @@ private String potentiallyConvertMapKey(Object key) { return (String) key; } - return conversions.hasCustomWriteTarget(key.getClass(), String.class) - ? (String) getPotentiallyConvertedSimpleWrite(key, Object.class) - : key.toString(); + if (conversions.hasCustomWriteTarget(key.getClass(), String.class)) { + Object potentiallyConvertedSimpleWrite = getPotentiallyConvertedSimpleWrite(key, Object.class); + + if (potentiallyConvertedSimpleWrite == null) { + return key.toString(); + } + return (String) potentiallyConvertedSimpleWrite; + } + return key.toString(); } /** * Checks whether we have a custom conversion registered for the given value into an arbitrary simple Elasticsearch * type. Returns the converted value if so. If not, we perform special enum handling or simply return the value as is. * - * @param value - * @return + * @param value value to convert */ @Nullable private Object getPotentiallyConvertedSimpleWrite(@Nullable Object value, @Nullable Class typeHint) { @@ -869,6 +879,10 @@ private Object getPotentiallyConvertedSimpleWrite(@Nullable Object value, @Nulla if (conversionService.canConvert(value.getClass(), typeHint)) { value = conversionService.convert(value, typeHint); + + if (value == null) { + return null; + } } } @@ -908,8 +922,8 @@ protected Object getWriteSimpleValue(Object value) { * @deprecated since 4.2, use {@link #writeInternal(Object, Map, TypeInformation)} instead. */ @Deprecated - protected Object getWriteComplexValue(ElasticsearchPersistentProperty property, TypeInformation typeHint, - Object value) { + protected Object getWriteComplexValue(ElasticsearchPersistentProperty property, + @SuppressWarnings("unused") TypeInformation typeHint, Object value) { Document document = Document.create(); writeInternal(value, document, property.getTypeInformation()); @@ -928,11 +942,19 @@ protected Object getWriteComplexValue(ElasticsearchPersistentProperty property, * * @param source must not be {@literal null}. * @param sink must not be {@literal null}. - * @param type + * @param type type to compare to */ - protected void addCustomTypeKeyIfNecessary(Object source, Map sink, @Nullable TypeInformation type) { + protected void addCustomTypeKeyIfNecessary(Object source, Map sink, + @Nullable TypeInformation type) { - Class reference = type != null ? type.getActualType().getType() : Object.class; + Class reference; + + if (type == null) { + reference = Object.class; + } else { + TypeInformation actualType = type.getActualType(); + reference = actualType == null ? Object.class : actualType.getType(); + } Class valueType = ClassUtils.getUserClass(source.getClass()); boolean notTheSameClass = !valueType.equals(reference); @@ -987,8 +1009,7 @@ private boolean isSimpleType(Class type) { * {@link Collection} already, will convert an array into a {@link Collection} or simply create a single element * collection for everything else. * - * @param source - * @return + * @param source object to convert */ private static Collection asCollection(Object source) { @@ -1019,21 +1040,42 @@ public void updateCriteriaQuery(CriteriaQuery criteriaQuery, Class domainClas } private void updateCriteria(Criteria criteria, ElasticsearchPersistentEntity persistentEntity) { + Field field = criteria.getField(); if (field == null) { return; } - String name = field.getName(); - ElasticsearchPersistentProperty property = persistentEntity.getPersistentProperty(name); + String[] fieldNames = field.getName().split("\\."); + ElasticsearchPersistentEntity currentEntity = persistentEntity; + ElasticsearchPersistentProperty persistentProperty = null; + for (int i = 0; i < fieldNames.length; i++) { + persistentProperty = currentEntity.getPersistentProperty(fieldNames[i]); + + if (persistentProperty != null) { + fieldNames[i] = persistentProperty.getFieldName(); + try { + currentEntity = mappingContext.getPersistentEntity(persistentProperty.getActualType()); + } catch (Exception e) { + // using system types like UUIDs will lead to java.lang.reflect.InaccessibleObjectException in JDK 16 + // so if we cannot get an entity here, bail out. + currentEntity = null; + } + } + + if (currentEntity == null) { + break; + } + } + + field.setName(String.join(".", fieldNames)); - if (property != null && property.getName().equals(name)) { - field.setName(property.getFieldName()); + if (persistentProperty != null) { - if (property.hasPropertyConverter()) { + if (persistentProperty.hasPropertyConverter()) { ElasticsearchPersistentPropertyConverter propertyConverter = Objects - .requireNonNull(property.getPropertyConverter()); + .requireNonNull(persistentProperty.getPropertyConverter()); criteria.getQueryCriteriaEntries().forEach(criteriaEntry -> { Object value = criteriaEntry.getValue(); if (value.getClass().isArray()) { @@ -1047,7 +1089,7 @@ private void updateCriteria(Criteria criteria, ElasticsearchPersistentEntity }); } - org.springframework.data.elasticsearch.annotations.Field fieldAnnotation = property + org.springframework.data.elasticsearch.annotations.Field fieldAnnotation = persistentProperty .findAnnotation(org.springframework.data.elasticsearch.annotations.Field.class); if (fieldAnnotation != null) { @@ -1055,6 +1097,7 @@ private void updateCriteria(Criteria criteria, ElasticsearchPersistentEntity } } } + // endregion static class MapValueAccessor { @@ -1148,7 +1191,6 @@ class ElasticsearchPropertyValueProvider implements PropertyValueProvider T getPropertyValue(ElasticsearchPersistentProperty property) { 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 c3e65e627..4cd01d1ef 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryMappingUnitTests.java @@ -15,12 +15,14 @@ */ package org.springframework.data.elasticsearch.core; +import static org.assertj.core.api.Assertions.*; import static org.skyscreamer.jsonassert.JSONAssert.*; import java.time.LocalDate; import java.util.Base64; import java.util.Collections; import java.util.Date; +import java.util.List; import java.util.Objects; import org.json.JSONException; @@ -68,8 +70,7 @@ void setUp() { // endregion // region tests - @Test - // DATAES-716 + @Test // DATAES-716 void shouldMapNamesAndConvertValuesInCriteriaQuery() throws JSONException { // use POJO properties and types in the query building @@ -111,8 +112,7 @@ void shouldMapNamesAndConvertValuesInCriteriaQuery() throws JSONException { assertEquals(expected, queryString, false); } - @Test - // #1668 + @Test // #1668 void shouldMapNamesAndConvertValuesInCriteriaQueryForSubCriteria() throws JSONException { // use POJO properties and types in the query building @@ -185,8 +185,7 @@ void shouldMapNamesAndConvertValuesInCriteriaQueryForSubCriteria() throws JSONEx assertEquals(expected, queryString, false); } - @Test - // #1668 + @Test // #1668 void shouldMapNamesAndConvertValuesInCriteriaQueryForSubCriteriaWithDate() throws JSONException { // use POJO properties and types in the query building CriteriaQuery criteriaQuery = new CriteriaQuery( // @@ -258,8 +257,7 @@ void shouldMapNamesAndConvertValuesInCriteriaQueryForSubCriteriaWithDate() throw assertEquals(expected, queryString, false); } - @Test - // DATAES-706 + @Test // DATAES-706 void shouldMapNamesAndValuesInSubCriteriaQuery() throws JSONException { CriteriaQuery criteriaQuery = new CriteriaQuery( // @@ -332,6 +330,41 @@ void shouldMapNamesInGeoJsonQuery() throws JSONException { assertEquals(expected, queryString, false); } + @Test // #1753 + @DisplayName("should map names and value in nested entities") + void shouldMapNamesAndValueInNestedEntities() throws JSONException { + + String expected = "{\n" + // + " \"bool\": {\n" + // + " \"must\": [\n" + // + " {\n" + // + " \"nested\": {\n" + // + " \"query\": {\n" + // + " \"query_string\": {\n" + // + " \"query\": \"03.10.1999\",\n" + // + " \"fields\": [\n" + // + " \"per-sons.birth-date^1.0\"\n" + // + " ]\n" + // + " }\n" + // + " },\n" + // + " \"path\": \"per-sons\"\n" + // + " }\n" + // + " }\n" + // + " ]\n" + // + " }\n" + // + "}\n"; // + + CriteriaQuery criteriaQuery = new CriteriaQuery( + new Criteria("persons.birthDate").is(LocalDate.of(1999, 10, 3)) + ); + mappingElasticsearchConverter.updateQuery(criteriaQuery, House.class); + String queryString = new CriteriaQueryProcessor().createQuery(criteriaQuery.getCriteria()).toString(); + + assertEquals(expected, queryString, false); + } + // endregion + + // region helper functions private String getBase64EncodedGeoShapeQuery(GeoJson geoJson, String elasticFieldName, String relation) { return Base64.getEncoder() .encodeToString(("{\"geo_shape\": {\"" @@ -347,7 +380,15 @@ static class Person { @Nullable @Field(name = "first-name") String firstName; @Nullable @Field(name = "last-name") String lastName; @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; + @Nullable @Field(name = "birth-date", type = FieldType.Date, format = {}, + pattern = "dd.MM.uuuu") LocalDate birthDate; + } + + static class House { + @Nullable @Id String id; + @Nullable + @Field(name = "per-sons", type = FieldType.Nested) + List persons; } static class GeoShapeEntity { 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 5708b762f..fdaa1b520 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessorUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/CriteriaQueryProcessorUnitTests.java @@ -18,6 +18,7 @@ import static org.skyscreamer.jsonassert.JSONAssert.*; import org.json.JSONException; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.data.elasticsearch.core.query.Criteria; @@ -338,4 +339,35 @@ void shouldBuildMatchAllQuery() throws JSONException { assertEquals(expected, query, false); } + + @Test // #1753 + @DisplayName("should build nested query") + void shouldBuildNestedQuery() throws JSONException { + + String expected = "{\n" + // + " \"bool\" : {\n" + // + " \"must\" : [\n" + // + " {\n" + // + " \"nested\" : {\n" + // + " \"query\" : {\n" + // + " \"query_string\" : {\n" + // + " \"query\" : \"murphy\",\n" + // + " \"fields\" : [\n" + // + " \"houses.inhabitants.lastName^1.0\"\n" + // + " ]\n" + // + " }\n" + // + " },\n" + // + " \"path\" : \"houses.inhabitants\"\n" + // + " }\n" + // + " }\n" + // + " ]\n" + // + " }\n" + // + "}"; // + + Criteria criteria = new Criteria("houses.inhabitants.lastName").is("murphy"); + + String query = queryProcessor.createQuery(criteria).toString(); + + assertEquals(expected, query, false); + } }