Skip to content

Fix mapping of property names in sort parameters. #3074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.springframework.core.env.EnvironmentCapable;
import org.springframework.core.env.StandardEnvironment;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.domain.Sort;
import org.springframework.data.elasticsearch.annotations.FieldType;
import org.springframework.data.elasticsearch.annotations.ScriptedField;
import org.springframework.data.elasticsearch.core.document.Document;
Expand All @@ -50,6 +51,7 @@
import org.springframework.data.elasticsearch.core.query.CriteriaQuery;
import org.springframework.data.elasticsearch.core.query.FetchSourceFilter;
import org.springframework.data.elasticsearch.core.query.Field;
import org.springframework.data.elasticsearch.core.query.Order;
import org.springframework.data.elasticsearch.core.query.Query;
import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm;
import org.springframework.data.elasticsearch.core.query.SourceFilter;
Expand Down Expand Up @@ -1231,7 +1233,7 @@ public void updateQuery(Query query, @Nullable Class<?> domainClass) {
return;
}

updatePropertiesInFieldsAndSourceFilter(query, domainClass);
updatePropertiesInFieldsSortAndSourceFilter(query, domainClass);

if (query instanceof CriteriaQuery criteriaQuery) {
updatePropertiesInCriteriaQuery(criteriaQuery, domainClass);
Expand All @@ -1242,20 +1244,27 @@ public void updateQuery(Query query, @Nullable Class<?> domainClass) {
}
}

private void updatePropertiesInFieldsAndSourceFilter(Query query, Class<?> domainClass) {
/**
* replaces the names of fields in the query, the sort or soucre filters with the field names used in Elasticsearch
* when they are defined on the ElasticsearchProperties
*
* @param query the query to process
* @param domainClass the domain class (persistent entity)
*/
private void updatePropertiesInFieldsSortAndSourceFilter(Query query, Class<?> domainClass) {

ElasticsearchPersistentEntity<?> persistentEntity = mappingContext.getPersistentEntity(domainClass);

if (persistentEntity != null) {
List<String> fields = query.getFields();

if (!fields.isEmpty()) {
query.setFields(updateFieldNames(fields, persistentEntity));
query.setFields(propertyToFieldNames(fields, persistentEntity));
}

List<String> storedFields = query.getStoredFields();
if (!CollectionUtils.isEmpty(storedFields)) {
query.setStoredFields(updateFieldNames(storedFields, persistentEntity));
query.setStoredFields(propertyToFieldNames(storedFields, persistentEntity));
}

SourceFilter sourceFilter = query.getSourceFilter();
Expand All @@ -1266,37 +1275,60 @@ private void updatePropertiesInFieldsAndSourceFilter(Query query, Class<?> domai
String[] excludes = null;

if (sourceFilter.getIncludes() != null) {
includes = updateFieldNames(Arrays.asList(sourceFilter.getIncludes()), persistentEntity)
includes = propertyToFieldNames(Arrays.asList(sourceFilter.getIncludes()), persistentEntity)
.toArray(new String[] {});
}

if (sourceFilter.getExcludes() != null) {
excludes = updateFieldNames(Arrays.asList(sourceFilter.getExcludes()), persistentEntity)
excludes = propertyToFieldNames(Arrays.asList(sourceFilter.getExcludes()), persistentEntity)
.toArray(new String[] {});
}

query.addSourceFilter(new FetchSourceFilter(sourceFilter.fetchSource(), includes, excludes));
}

if (query.getSort() != null) {
var sort = query.getSort();
// stream the orders and map them to a new order with the changed names,
// then replace the existing sort with a new sort containing the new orders.
var newOrders = sort.stream().map(order -> {
var fieldNames = updateFieldNames(order.getProperty(), persistentEntity);

if (order instanceof Order springDataElasticsearchOrder) {
return springDataElasticsearchOrder.withProperty(fieldNames);
} else {
return new Sort.Order(order.getDirection(),
fieldNames,
order.isIgnoreCase(),
order.getNullHandling());
}
}).toList();

if (query instanceof BaseQuery baseQuery) {
baseQuery.setSort(Sort.by(newOrders));
}
}
}
}

/**
* relaces the fieldName with the property name of a property of the persistentEntity with the corresponding
* fieldname. If no such property exists, the original fieldName is kept.
* replaces property name of a property of the persistentEntity with the corresponding fieldname. If no such property
* exists, the original fieldName is kept.
*
* @param fieldNames list of fieldnames
* @param propertyNames list of fieldnames
* @param persistentEntity the persistent entity to check
* @return an updated list of field names
*/
private List<String> updateFieldNames(List<String> fieldNames, ElasticsearchPersistentEntity<?> persistentEntity) {
return fieldNames.stream().map(fieldName -> updateFieldName(persistentEntity, fieldName))
private List<String> propertyToFieldNames(List<String> propertyNames,
ElasticsearchPersistentEntity<?> persistentEntity) {
return propertyNames.stream().map(propertyName -> propertyToFieldName(persistentEntity, propertyName))
.collect(Collectors.toList());
}

@NotNull
private String updateFieldName(ElasticsearchPersistentEntity<?> persistentEntity, String fieldName) {
ElasticsearchPersistentProperty persistentProperty = persistentEntity.getPersistentProperty(fieldName);
return persistentProperty != null ? persistentProperty.getFieldName() : fieldName;
private String propertyToFieldName(ElasticsearchPersistentEntity<?> persistentEntity, String propertyName) {
ElasticsearchPersistentProperty persistentProperty = persistentEntity.getPersistentProperty(propertyName);
return persistentProperty != null ? persistentProperty.getFieldName() : propertyName;
}

private void updatePropertiesInCriteriaQuery(CriteriaQuery criteriaQuery, Class<?> domainClass) {
Expand Down Expand Up @@ -1410,7 +1442,7 @@ public String updateFieldNames(String propertyPath, ElasticsearchPersistentEntit

if (properties.length > 0) {
var propertyName = properties[0];
var fieldName = updateFieldName(persistentEntity, propertyName);
var fieldName = propertyToFieldName(persistentEntity, propertyName);

if (properties.length > 1) {
var persistentProperty = persistentEntity.getPersistentProperty(propertyName);
Expand All @@ -1431,7 +1463,6 @@ public String updateFieldNames(String propertyPath, ElasticsearchPersistentEntit
}

}

// endregion

@SuppressWarnings("ClassCanBeRecord")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.List;

import org.json.JSONException;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.annotation.Id;
Expand All @@ -35,7 +36,6 @@
import org.springframework.data.elasticsearch.repository.query.RepositoryPartQuery;
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
import org.springframework.data.repository.query.ValueExpressionDelegate;
import org.springframework.lang.Nullable;

Expand Down Expand Up @@ -640,6 +640,45 @@ void findByAvailableTrueOrderByNameDesc() throws NoSuchMethodException, JSONExce
assertEquals(expected, query, false);
}

@Test // #3072
@DisplayName("should build sort object with correct field names")
void shouldBuildSortObjectWithCorrectFieldNames() throws NoSuchMethodException, JSONException {

String methodName = "findByNameOrderBySortAuthor_SortName";
Class<?>[] parameterClasses = new Class[] { String.class };
Object[] parameters = new Object[] { BOOK_TITLE };

String query = getQueryString(methodName, parameterClasses, parameters);

String expected = """

{
"query": {
"bool": {
"must": [
{
"query_string": {
"query": "Title",
"fields": [
"name"
]
}
}
]
}
},
"sort": [
{
"sort_author.sort_name": {
"order": "asc"
}
}
]
}""";

assertEquals(expected, query, false);
}

private String getQueryString(String methodName, Class<?>[] parameterClasses, Object[] parameters)
throws NoSuchMethodException {

Expand Down Expand Up @@ -727,6 +766,8 @@ private interface SampleRepository extends ElasticsearchRepository<Book, String>

List<Book> findByAvailableTrueOrderByNameDesc();

List<Book> findByNameOrderBySortAuthor_SortName(String name);

}

public static class Book {
Expand All @@ -736,6 +777,10 @@ public static class Book {
@Nullable private Integer price;
@Field(type = FieldType.Boolean) private boolean available;

// this is needed for the #3072 test
@Nullable
@Field(name = "sort_author", type = FieldType.Object) private Author sortAuthor;

@Nullable
public String getId() {
return id;
Expand Down Expand Up @@ -767,8 +812,32 @@ public Boolean getAvailable() {
return available;
}

@Nullable
public Author getSortAuthor() {
return sortAuthor;
}

public void setSortAuthor(@Nullable Author sortAuthor) {
this.sortAuthor = sortAuthor;
}

public void setAvailable(Boolean available) {
this.available = available;

}
}

public static class Author {
@Nullable
@Field(name = "sort_name", type = FieldType.Keyword) private String sortName;

@Nullable
public String getSortName() {
return sortName;
}

public void setSortName(@Nullable String sortName) {
this.sortName = sortName;
}
}
}