You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2019/05/23 03:38:01 UTC

[james-project] 13/14: JAMES-2675 Rely on ScrollIterable when no limit is specified

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 496f4194c92f618add6ee9527a7c792996d91ded
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Tue May 21 18:31:21 2019 +0700

    JAMES-2675 Rely on ScrollIterable when no limit is specified
    
    Otherwise ES default limit of 10 will be applied. A more optimized "search query"
    can be used when the limit is specified, avoiding opening a scroll-session.
---
 .../elasticsearch/ElasticSearchQuotaSearcher.java  | 55 +++++++++++++++-----
 .../ElasticSearchQuotaSearcherTest.java            | 60 ++++++++++++++++++++++
 .../java/org/apache/james/quota/search/Limit.java  |  4 ++
 .../org/apache/james/quota/search/LimitTest.java   | 24 ++++++---
 4 files changed, 123 insertions(+), 20 deletions(-)

diff --git a/mailbox/plugin/quota-search-elasticsearch-v6/src/main/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcher.java b/mailbox/plugin/quota-search-elasticsearch-v6/src/main/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcher.java
index b08c4d8..e3f6ab5 100644
--- a/mailbox/plugin/quota-search-elasticsearch-v6/src/main/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcher.java
+++ b/mailbox/plugin/quota-search-elasticsearch-v6/src/main/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcher.java
@@ -24,16 +24,19 @@ import static org.apache.james.quota.search.elasticsearch.json.JsonMessageConsta
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
+import java.util.stream.Stream;
 
 import org.apache.james.backends.es.v6.AliasName;
 import org.apache.james.backends.es.v6.NodeMappingFactory;
 import org.apache.james.backends.es.v6.ReadAliasName;
+import org.apache.james.backends.es.v6.search.ScrollIterable;
 import org.apache.james.core.User;
 import org.apache.james.quota.search.QuotaQuery;
 import org.apache.james.quota.search.QuotaSearcher;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.RestHighLevelClient;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.sort.SortBuilders;
@@ -42,6 +45,8 @@ import org.elasticsearch.search.sort.SortOrder;
 import com.github.steveash.guavate.Guavate;
 
 public class ElasticSearchQuotaSearcher implements QuotaSearcher {
+    private static final TimeValue TIMEOUT = TimeValue.timeValueMinutes(1);
+
     private final RestHighLevelClient client;
     private final AliasName readAlias;
     private final QuotaQueryConverter quotaQueryConverter;
@@ -55,9 +60,7 @@ public class ElasticSearchQuotaSearcher implements QuotaSearcher {
     @Override
     public List<User> search(QuotaQuery query) {
         try {
-            return Arrays.stream(client.search(prepareSearch(query), RequestOptions.DEFAULT)
-                .getHits()
-                .getHits())
+            return searchHits(query)
                 .map(SearchHit::getId)
                 .map(User::fromUsername)
                 .collect(Guavate.toImmutableList());
@@ -66,19 +69,43 @@ public class ElasticSearchQuotaSearcher implements QuotaSearcher {
         }
     }
 
-    private SearchRequest prepareSearch(QuotaQuery query) {
-        SearchSourceBuilder sourceBuilder = new SearchSourceBuilder()
-            .query(quotaQueryConverter.from(query))
-            .sort(SortBuilders.fieldSort(USER)
-                .order(SortOrder.ASC))
-            .from(query.getOffset().getValue());
+    private Stream<SearchHit> searchHits(QuotaQuery query) throws IOException {
+        if (query.getLimit().isLimited()) {
+            return executeSingleSearch(query);
+        } else {
+            return executeScrolledSearch(query);
+        }
+    }
 
-        query.getLimit()
-            .getValue()
-            .ifPresent(sourceBuilder::size);
+    private Stream<SearchHit> executeSingleSearch(QuotaQuery query) throws IOException {
+        SearchSourceBuilder searchSourceBuilder = searchSourceBuilder(query)
+            .from(query.getOffset().getValue());
+        query.getLimit().getValue()
+            .ifPresent(searchSourceBuilder::size);
 
-        return new SearchRequest(readAlias.getValue())
+        SearchRequest searchRequest = new SearchRequest(readAlias.getValue())
             .types(NodeMappingFactory.DEFAULT_MAPPING_NAME)
-            .source(sourceBuilder);
+            .source(searchSourceBuilder);
+
+        return Arrays.stream(client.search(searchRequest, RequestOptions.DEFAULT)
+            .getHits()
+            .getHits());
+    }
+
+    private Stream<SearchHit> executeScrolledSearch(QuotaQuery query) {
+        return new ScrollIterable(client,
+            new SearchRequest(readAlias.getValue())
+                .types(NodeMappingFactory.DEFAULT_MAPPING_NAME)
+                .source(searchSourceBuilder(query))
+            .scroll(TIMEOUT))
+            .stream()
+            .flatMap(searchResponse -> Arrays.stream(searchResponse.getHits().getHits()))
+            .skip(query.getOffset().getValue());
+    }
+
+    private SearchSourceBuilder searchSourceBuilder(QuotaQuery query) {
+        return new SearchSourceBuilder()
+            .query(quotaQueryConverter.from(query))
+            .sort(SortBuilders.fieldSort(USER).order(SortOrder.ASC));
     }
 }
\ No newline at end of file
diff --git a/mailbox/plugin/quota-search-elasticsearch-v6/src/test/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcherTest.java b/mailbox/plugin/quota-search-elasticsearch-v6/src/test/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcherTest.java
index 2d4b79e..8c41989 100644
--- a/mailbox/plugin/quota-search-elasticsearch-v6/src/test/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcherTest.java
+++ b/mailbox/plugin/quota-search-elasticsearch-v6/src/test/java/org/apache/james/quota/search/elasticsearch/ElasticSearchQuotaSearcherTest.java
@@ -19,10 +19,70 @@
 
 package org.apache.james.quota.search.elasticsearch;
 
+import static org.apache.james.core.CoreFixture.Domains.SIMPSON_COM;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.stream.IntStream;
+
+import org.apache.james.core.User;
+import org.apache.james.core.quota.QuotaSize;
+import org.apache.james.quota.search.Limit;
+import org.apache.james.quota.search.Offset;
+import org.apache.james.quota.search.QuotaQuery;
+import org.apache.james.quota.search.QuotaSearchTestSystem;
 import org.apache.james.quota.search.QuotaSearcherContract;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 @ExtendWith(ElasticSearchQuotaSearchTestSystemExtension.class)
 class ElasticSearchQuotaSearcherTest implements QuotaSearcherContract {
+    @Test
+    void searchShouldNotBeLimitedByElasticSearchDefaultSearchLimit(QuotaSearchTestSystem testSystem) throws Exception {
+        int userCount = 11;
+        testSystem.getDomainList().addDomain(SIMPSON_COM);
+        testSystem.getMaxQuotaManager().setGlobalMaxStorage(QuotaSize.size(100));
+
+        IntStream.range(0, userCount)
+            .boxed()
+            .map(i -> User.fromLocalPartWithDomain("user" + i, SIMPSON_COM))
+            .forEach(user -> provisionUser(testSystem, user));
+        testSystem.await();
+
+        assertThat(
+            testSystem.getQuotaSearcher()
+                .search(QuotaQuery.builder()
+                    .withLimit(Limit.unlimited())
+                    .build()))
+            .hasSize(userCount);
+    }
+
+    @Test
+    void searchShouldNotBeLimitedByElasticSearchDefaultSearchLimitWhenUsingOffset(QuotaSearchTestSystem testSystem) throws Exception {
+        int userCount = 12;
+        testSystem.getDomainList().addDomain(SIMPSON_COM);
+        testSystem.getMaxQuotaManager().setGlobalMaxStorage(QuotaSize.size(100));
+
+        IntStream.range(0, userCount)
+            .boxed()
+            .map(i -> User.fromLocalPartWithDomain("user" + i, SIMPSON_COM))
+            .forEach(user -> provisionUser(testSystem, user));
+        testSystem.await();
+
+        assertThat(
+            testSystem.getQuotaSearcher()
+                .search(QuotaQuery.builder()
+                    .withLimit(Limit.unlimited())
+                    .withOffset(Offset.of(1))
+                    .build()))
+            .hasSize(userCount - 1);
+    }
 
+    private void provisionUser(QuotaSearchTestSystem testSystem, User user) {
+        try {
+            testSystem.getUsersRepository().addUser(user.asString(), PASSWORD);
+            appendMessage(testSystem, user, withSize(49));
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
 }
diff --git a/mailbox/plugin/quota-search/src/main/java/org/apache/james/quota/search/Limit.java b/mailbox/plugin/quota-search/src/main/java/org/apache/james/quota/search/Limit.java
index b62dd5e..0ba698b 100644
--- a/mailbox/plugin/quota-search/src/main/java/org/apache/james/quota/search/Limit.java
+++ b/mailbox/plugin/quota-search/src/main/java/org/apache/james/quota/search/Limit.java
@@ -45,6 +45,10 @@ public class Limit {
         return value;
     }
 
+    public boolean isLimited() {
+        return value.isPresent();
+    }
+
     @Override
     public final boolean equals(Object o) {
         if (o instanceof Limit) {
diff --git a/mailbox/plugin/quota-search/src/test/java/org/apache/james/quota/search/LimitTest.java b/mailbox/plugin/quota-search/src/test/java/org/apache/james/quota/search/LimitTest.java
index 9bd6e8e..4f565cb 100644
--- a/mailbox/plugin/quota-search/src/test/java/org/apache/james/quota/search/LimitTest.java
+++ b/mailbox/plugin/quota-search/src/test/java/org/apache/james/quota/search/LimitTest.java
@@ -26,36 +26,48 @@ import org.junit.jupiter.api.Test;
 
 import nl.jqno.equalsverifier.EqualsVerifier;
 
-public class LimitTest {
+class LimitTest {
     @Test
-    public void shouldMatchBeanContract() {
+    void shouldMatchBeanContract() {
         EqualsVerifier.forClass(Limit.class)
             .verify();
     }
 
     @Test
-    public void getValueShouldReturnEmptyWhenUnlimited() {
+    void getValueShouldReturnEmptyWhenUnlimited() {
         assertThat(Limit.unlimited()
             .getValue())
             .isEmpty();
     }
 
     @Test
-    public void getValueShouldReturnZeroWhenZero() {
+    void getValueShouldReturnZeroWhenZero() {
         assertThat(Limit.of(0)
             .getValue())
             .contains(0);
     }
 
     @Test
-    public void getValueShouldReturnSuppliedValue() {
+    void getValueShouldReturnSuppliedValue() {
         assertThat(Limit.of(3)
             .getValue())
             .contains(3);
     }
 
     @Test
-    public void ofShouldThrowOnNegativeValue() {
+    void isLimitedShouldBeTrueWhenAValueIsSpecified() {
+        assertThat(Limit.of(3).isLimited())
+            .isTrue();
+    }
+
+    @Test
+    void isLimitedShouldBeFalseWhenUnlimited() {
+        assertThat(Limit.unlimited().isLimited())
+            .isFalse();
+    }
+
+    @Test
+    void ofShouldThrowOnNegativeValue() {
         assertThatThrownBy(() -> Limit.of(-1))
             .isInstanceOf(IllegalArgumentException.class);
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org