From 745f9e9d796258c24a606d533de5c97c08bd1a68 Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Fri, 20 Mar 2020 18:27:50 +0100 Subject: [PATCH] DATAES-765 - Pageable.unpaged() is not used to build a query returning all documents. Original PR: #408 --- .../elasticsearch/core/RequestFactory.java | 10 +++++ .../core/RequestFactoryTests.java | 37 ++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java b/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java index e54d5aa23..32192d32f 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java @@ -84,6 +84,10 @@ import org.springframework.util.StringUtils; * @since 4.0 */ class RequestFactory { + + // the default max result window size of Elasticsearch + static final Integer INDEX_MAX_RESULT_WINDOW = 10_000; + private final ElasticsearchConverter elasticsearchConverter; public RequestFactory(ElasticsearchConverter elasticsearchConverter) { @@ -557,6 +561,9 @@ class RequestFactory { if (query.getPageable().isPaged()) { sourceBuilder.from((int) query.getPageable().getOffset()); sourceBuilder.size(query.getPageable().getPageSize()); + } else { + sourceBuilder.from(0); + sourceBuilder.size(INDEX_MAX_RESULT_WINDOW); } if (!query.getFields().isEmpty()) { @@ -720,6 +727,9 @@ class RequestFactory { if (query.getPageable().isPaged()) { searchRequestBuilder.setFrom((int) query.getPageable().getOffset()); searchRequestBuilder.setSize(query.getPageable().getPageSize()); + } else { + searchRequestBuilder.setFrom(0); + searchRequestBuilder.setSize(INDEX_MAX_RESULT_WINDOW); } if (!query.getFields().isEmpty()) { diff --git a/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java b/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java index b2055ef8b..f7d0bb6da 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/RequestFactoryTests.java @@ -16,16 +16,25 @@ package org.springframework.data.elasticsearch.core; import static org.assertj.core.api.Assertions.*; +import static org.elasticsearch.index.query.QueryBuilders.*; +import static org.mockito.Mockito.*; import static org.skyscreamer.jsonassert.JSONAssert.*; import java.util.Collections; +import org.elasticsearch.action.search.SearchAction; import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.action.search.SearchRequestBuilder; +import org.elasticsearch.client.Client; import org.json.JSONException; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.annotation.Id; +import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter; @@ -35,18 +44,22 @@ import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMa import org.springframework.data.elasticsearch.core.query.Criteria; import org.springframework.data.elasticsearch.core.query.CriteriaQuery; import org.springframework.data.elasticsearch.core.query.GeoDistanceOrder; +import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder; +import org.springframework.data.elasticsearch.core.query.Query; import org.springframework.lang.Nullable; /** * @author Peter-Josef Meisch */ +@ExtendWith(MockitoExtension.class) class RequestFactoryTests { @Nullable private static RequestFactory requestFactory; @Nullable private static MappingElasticsearchConverter converter; - @BeforeAll + @Mock private Client client; + @BeforeAll static void setUpAll() { SimpleElasticsearchMappingContext mappingContext = new SimpleElasticsearchMappingContext(); mappingContext.setInitialEntitySet(Collections.singleton(Person.class)); @@ -118,6 +131,28 @@ class RequestFactoryTests { assertThat(searchRequest.routing()).isEqualTo(route); } + @Test // DATAES-765 + void shouldAddMaxQueryWindowForUnpagedToRequest() { + Query query = new NativeSearchQueryBuilder().withQuery(matchAllQuery()).withPageable(Pageable.unpaged()).build(); + + SearchRequest searchRequest = requestFactory.searchRequest(query, Person.class, IndexCoordinates.of("persons")); + + assertThat(searchRequest.source().from()).isEqualTo(0); + assertThat(searchRequest.source().size()).isEqualTo(RequestFactory.INDEX_MAX_RESULT_WINDOW); + } + + @Test // DATAES-765 + void shouldAddMaxQueryWindowForUnpagedToRequestBuilder() { + when(client.prepareSearch(any())).thenReturn(new SearchRequestBuilder(client, SearchAction.INSTANCE)); + Query query = new NativeSearchQueryBuilder().withQuery(matchAllQuery()).withPageable(Pageable.unpaged()).build(); + + SearchRequestBuilder searchRequestBuilder = requestFactory.searchRequestBuilder(client, query, Person.class, + IndexCoordinates.of("persons")); + + assertThat(searchRequestBuilder.request().source().from()).isEqualTo(0); + assertThat(searchRequestBuilder.request().source().size()).isEqualTo(RequestFactory.INDEX_MAX_RESULT_WINDOW); + } + static class Person { @Nullable @Id String id; @Nullable @Field(name = "last-name") String lastName;