Skip to content

Fix reactive native sort. #2746

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
Oct 23, 2023
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 @@ -1362,14 +1362,6 @@ private <T> void prepareSearchRequest(Query query, @Nullable String routing, @Nu
builder.minScore((double) query.getMinScore());
}

if (query.getSort() != null) {
List<SortOptions> sortOptions = getSortOptions(query.getSort(), persistentEntity);

if (!sortOptions.isEmpty()) {
builder.sort(sortOptions);
}
}

addHighlight(query, builder);

query.getScriptedFields().forEach(scriptedField -> builder.scriptFields(scriptedField.getFieldName(),
Expand All @@ -1378,6 +1370,15 @@ private <T> void prepareSearchRequest(Query query, @Nullable String routing, @Nu
if (query instanceof NativeQuery nativeQuery) {
prepareNativeSearch(nativeQuery, builder);
}
// query.getSort() must be checked after prepareNativeSearch as this already might hav a sort set that must have
// higher priority
if (query.getSort() != null) {
List<SortOptions> sortOptions = getSortOptions(query.getSort(), persistentEntity);

if (!sortOptions.isEmpty()) {
builder.sort(sortOptions);
}
}

if (query.getTrackTotalHits() != null) {
// logic from the RHLC, choose between -1 and Integer.MAX_VALUE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.assertj.core.api.Assertions.*;
import static org.springframework.data.elasticsearch.client.elc.Queries.*;

import co.elastic.clients.elasticsearch._types.SortOrder;
import co.elastic.clients.elasticsearch._types.aggregations.Aggregate;
import co.elastic.clients.elasticsearch._types.aggregations.Buckets;
import co.elastic.clients.elasticsearch._types.aggregations.StringTermsAggregate;
Expand All @@ -27,9 +28,12 @@
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.data.domain.Pageable;
import org.springframework.data.elasticsearch.client.elc.Aggregation;
import org.springframework.data.elasticsearch.client.elc.ElasticsearchAggregation;
import org.springframework.data.elasticsearch.client.elc.NativeQuery;
Expand All @@ -48,6 +52,51 @@
@ContextConfiguration(classes = ReactiveElasticsearchELCIntegrationTests.Config.class)
public class ReactiveElasticsearchELCIntegrationTests extends ReactiveElasticsearchIntegrationTests {

@Test // #2745
@DisplayName("should use sort defined in native unbounded query")
void shouldUseSortDefinedInNativeUnboundedQuery() {
var entity1 = randomEntity(null);
entity1.setRate(7);
var entity2 = randomEntity(null);
entity2.setRate(5);
var entity3 = randomEntity(null);
entity3.setRate(11);

operations.saveAll(List.of(entity1, entity2, entity3), SampleEntity.class).blockLast();

var query = NativeQuery.builder()
.withQuery(qb -> qb
.matchAll(m -> m))
.withSort(sob -> sob
.field(f -> f
.field("rate")
.order(SortOrder.Asc)))
.withPageable(Pageable.unpaged())
.build();

var rates = operations.search(query, SampleEntity.class)
.map(SearchHit::getContent)
.map(SampleEntity::getRate)
.collectList().block();
assertThat(rates).containsExactly(5, 7, 11);

query = NativeQuery.builder()
.withQuery(qb -> qb
.matchAll(m -> m))
.withSort(sob -> sob
.field(f -> f
.field("rate")
.order(SortOrder.Desc)))
.withPageable(Pageable.unpaged())
.build();

rates = operations.search(query, SampleEntity.class)
.map(SearchHit::getContent)
.map(SampleEntity::getRate)
.collectList().block();
assertThat(rates).containsExactly(11, 7, 5);
}

@Configuration
@Import({ ReactiveElasticsearchTemplateConfiguration.class })
static class Config {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
@SpringIntegrationTest
public abstract class ReactiveElasticsearchIntegrationTests {

@Autowired private ReactiveElasticsearchOperations operations;
@Autowired protected ReactiveElasticsearchOperations operations;
@Autowired private IndexNameProvider indexNameProvider;

// region Setup
Expand Down Expand Up @@ -1224,7 +1224,7 @@ void shouldFailWithConflictOnAttemptToSaveWithSameVersion() {
// endregion

// region Helper functions
private SampleEntity randomEntity(String message) {
protected SampleEntity randomEntity(@Nullable String message) {

SampleEntity entity = new SampleEntity();
entity.setId(UUID.randomUUID().toString());
Expand Down