From 22d740565a2b8cbb598168522b4c8b551a8cefec Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Mon, 24 Mar 2025 21:40:40 +0100 Subject: [PATCH] Fix cutting of unknown properties in property paths for search. Closes #3081 Signed-off-by: Peter-Josef Meisch --- .../MappingElasticsearchConverter.java | 116 +++++++++--------- .../RepositoryPartQueryIntegrationTests.java | 42 +++++++ 2 files changed, 101 insertions(+), 57 deletions(-) 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 ff564f7b7..441a95c53 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 @@ -1372,53 +1372,21 @@ private void updatePropertiesInCriteria(Criteria criteria, ElasticsearchPersiste return; } - String[] fieldNames = field.getName().split("\\."); - - ElasticsearchPersistentEntity currentEntity = persistentEntity; - ElasticsearchPersistentProperty persistentProperty = null; - int propertyCount = 0; - boolean isNested = false; - - for (int i = 0; i < fieldNames.length; i++) { - persistentProperty = currentEntity.getPersistentProperty(fieldNames[i]); - - if (persistentProperty != null) { - propertyCount++; - fieldNames[i] = persistentProperty.getFieldName(); - - org.springframework.data.elasticsearch.annotations.Field fieldAnnotation = persistentProperty - .findAnnotation(org.springframework.data.elasticsearch.annotations.Field.class); - - if (fieldAnnotation != null && fieldAnnotation.type() == FieldType.Nested) { - isNested = true; - } - - 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; - } - } + var propertyNamesUpdate = updatePropertyNames(persistentEntity, field.getName()); + var fieldNames = propertyNamesUpdate.names(); field.setName(String.join(".", fieldNames)); - if (propertyCount > 1 && isNested) { + if (propertyNamesUpdate.propertyCount() > 1 && propertyNamesUpdate.nestedProperty()) { List propertyNames = Arrays.asList(fieldNames); - field.setPath(String.join(".", propertyNames.subList(0, propertyCount - 1))); + field.setPath(String.join(".", propertyNames.subList(0, propertyNamesUpdate.propertyCount - 1))); } - if (persistentProperty != null) { + if (propertyNamesUpdate.persistentProperty != null) { - if (persistentProperty.hasPropertyValueConverter()) { + if (propertyNamesUpdate.persistentProperty.hasPropertyValueConverter()) { PropertyValueConverter propertyValueConverter = Objects - .requireNonNull(persistentProperty.getPropertyValueConverter()); + .requireNonNull(propertyNamesUpdate.persistentProperty.getPropertyValueConverter()); criteria.getQueryCriteriaEntries().forEach(criteriaEntry -> { if (criteriaEntry.getKey().hasValue()) { @@ -1437,7 +1405,7 @@ private void updatePropertiesInCriteria(Criteria criteria, ElasticsearchPersiste }); } - org.springframework.data.elasticsearch.annotations.Field fieldAnnotation = persistentProperty + org.springframework.data.elasticsearch.annotations.Field fieldAnnotation = propertyNamesUpdate.persistentProperty .findAnnotation(org.springframework.data.elasticsearch.annotations.Field.class); if (fieldAnnotation != null) { @@ -1446,36 +1414,70 @@ private void updatePropertiesInCriteria(Criteria criteria, ElasticsearchPersiste } } + static record PropertyNamesUpdate( + String[] names, + Boolean nestedProperty, + Integer propertyCount, + ElasticsearchPersistentProperty persistentProperty) { + } + @Override public String updateFieldNames(String propertyPath, ElasticsearchPersistentEntity persistentEntity) { Assert.notNull(propertyPath, "propertyPath must not be null"); Assert.notNull(persistentEntity, "persistentEntity must not be null"); - var properties = propertyPath.split("\\.", 2); + var propertyNamesUpdate = updatePropertyNames(persistentEntity, propertyPath); + return String.join(".", propertyNamesUpdate.names()); + } - if (properties.length > 0) { - var propertyName = properties[0]; - var fieldName = propertyToFieldName(persistentEntity, propertyName); + /** + * Parse a propertyPath and replace the path values with the field names from a persistentEntity. path entries not + * found in the entity are kept as they are. + * + * @return the eventually modified names, a flag if a nested entity was encountered the number of processed + * propertiesand the last processed PersistentProperty. + */ + PropertyNamesUpdate updatePropertyNames(ElasticsearchPersistentEntity persistentEntity, String propertyPath) { - if (properties.length > 1) { - var persistentProperty = persistentEntity.getPersistentProperty(propertyName); + String[] propertyNames = propertyPath.split("\\."); + String[] fieldNames = Arrays.copyOf(propertyNames, propertyNames.length); - if (persistentProperty != null) { - ElasticsearchPersistentEntity nestedPersistentEntity = mappingContext - .getPersistentEntity(persistentProperty); - if (nestedPersistentEntity != null) { - return fieldName + '.' + updateFieldNames(properties[1], nestedPersistentEntity); - } else { - return fieldName; - } + ElasticsearchPersistentEntity currentEntity = persistentEntity; + ElasticsearchPersistentProperty persistentProperty = null; + + int propertyCount = 0; + boolean isNested = false; + + for (int i = 0; i < propertyNames.length; i++) { + persistentProperty = currentEntity.getPersistentProperty(propertyNames[i]); + + if (persistentProperty != null) { + propertyCount++; + fieldNames[i] = persistentProperty.getFieldName(); + + org.springframework.data.elasticsearch.annotations.Field fieldAnnotation = persistentProperty + .findAnnotation(org.springframework.data.elasticsearch.annotations.Field.class); + + if (fieldAnnotation != null && fieldAnnotation.type() == FieldType.Nested) { + isNested = true; + } + + 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; } } - return fieldName; - } else { - return propertyPath; + + if (currentEntity == null) { + break; + } } + return new PropertyNamesUpdate(fieldNames, isNested, propertyCount, persistentProperty); } // endregion diff --git a/src/test/java/org/springframework/data/elasticsearch/core/query/RepositoryPartQueryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/core/query/RepositoryPartQueryIntegrationTests.java index e73fe780c..0a5c963f5 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/query/RepositoryPartQueryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/query/RepositoryPartQueryIntegrationTests.java @@ -15,6 +15,7 @@ */ package org.springframework.data.elasticsearch.core.query; +import static org.assertj.core.api.Assertions.*; import static org.skyscreamer.jsonassert.JSONAssert.*; import java.lang.reflect.Method; @@ -27,6 +28,7 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.annotation.Id; +import org.springframework.data.domain.Sort; import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.annotations.FieldType; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; @@ -679,6 +681,45 @@ void shouldBuildSortObjectWithCorrectFieldNames() throws NoSuchMethodException, assertEquals(expected, query, false); } + @Test // #3081 + @DisplayName("should build sort object with unknown field names") + void shouldBuildSortObjectWithUnknownFieldNames() throws NoSuchMethodException, JSONException { + + String methodName = "findByName"; + Class[] parameterClasses = new Class[] { String.class, Sort.class }; + Object[] parameters = new Object[] { BOOK_TITLE, Sort.by("sortAuthor.sortName.raw") }; + + String query = getQueryString(methodName, parameterClasses, parameters); + + String expected = """ + + { + "query": { + "bool": { + "must": [ + { + "query_string": { + "query": "Title", + "fields": [ + "name" + ] + } + } + ] + } + }, + "sort": [ + { + "sort_author.sort_name.raw": { + "order": "asc" + } + } + ] + }"""; + + assertEquals(expected, query, false); + } + private String getQueryString(String methodName, Class[] parameterClasses, Object[] parameters) throws NoSuchMethodException { @@ -768,6 +809,7 @@ private interface SampleRepository extends ElasticsearchRepository List findByNameOrderBySortAuthor_SortName(String name); + List findByName(String name, Sort sort); } public static class Book {