james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From btell...@apache.org
Subject [08/15] james-project git commit: JAMES-2255 getMessagesList.position range is long instead of int type
Date Fri, 05 Jan 2018 09:17:34 GMT
JAMES-2255 getMessagesList.position range is long instead of int type


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/260dfceb
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/260dfceb
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/260dfceb

Branch: refs/heads/master
Commit: 260dfceba38658dcaff9492e2e6b92fa912a2d43
Parents: d6760a7
Author: quynhn <qnguyen@linagora.com>
Authored: Fri Dec 8 14:59:10 2017 +0700
Committer: benwa <btellier@linagora.com>
Committed: Fri Jan 5 16:10:35 2018 +0700

----------------------------------------------------------------------
 .../integration/GetMessageListMethodTest.java   | 27 ++++++++++++++++++++
 .../jmap/methods/GetMessageListMethod.java      |  5 ++--
 .../james/jmap/model/GetMessageListRequest.java | 18 ++++++++-----
 .../jmap/model/GetMessageListRequestTest.java   |  6 ++---
 4 files changed, 44 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/260dfceb/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
index 52b33de..5ba9bd4 100644
--- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
+++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/GetMessageListMethodTest.java
@@ -25,6 +25,7 @@ import static com.jayway.restassured.config.RestAssuredConfig.newConfig;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
@@ -1898,4 +1899,30 @@ public abstract class GetMessageListMethodTest {
     private Date convertToDate(LocalDate localDate) {
         return Date.from(localDate.atStartOfDay(ZONE_ID).toInstant());
     }
+
+    @Test
+    public void getMessageListShouldAcceptLessThan2Pow53NumberForPosition() throws Exception
{
+        given()
+            .header("Authorization", aliceAccessToken.serialize())
+            .body("[[\"getMessageList\", {\"position\":" + Math.pow(2, 53) + "}, \"#0\"]]")
+        .when()
+            .post("/jmap")
+        .then()
+            .statusCode(200)
+            .body(NAME, equalTo("messageList"));
+    }
+
+    @Test
+    public void getMessageListShouldErrorWhenPositionOver2Pow53() throws Exception {
+        given()
+            .header("Authorization", aliceAccessToken.serialize())
+            .body("[[\"getMessageList\", {\"position\":" + Math.pow(2, 54) + "}, \"#0\"]]")
+        .when()
+            .post("/jmap")
+        .then()
+            .statusCode(200)
+            .body(NAME, equalTo("error"))
+            .body(ARGUMENTS + ".type", equalTo("invalidArguments"))
+            .body(ARGUMENTS + ".description", containsString("'position' should be positive
and less than 2^53 or empty"));
+    }
 }

http://git-wip-us.apache.org/repos/asf/james-project/blob/260dfceb/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
index fcdefa4..749c104 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/methods/GetMessageListMethod.java
@@ -53,6 +53,7 @@ import com.google.common.base.Throwables;
 
 public class GetMessageListMethod implements Method {
 
+    private static final long DEFAULT_POSITION = 0;
     public static final String MAXIMUM_LIMIT = "maximumLimit";
     public static final int DEFAULT_MAXIMUM_LIMIT = 256;
 
@@ -125,9 +126,9 @@ public class GetMessageListMethod implements Method {
             MultimailboxesSearchQuery searchQuery = convertToSearchQuery(messageListRequest);
             mailboxManager.search(searchQuery,
                 mailboxSession,
-                messageListRequest.getLimit().orElse(maximumLimit) + messageListRequest.getPosition())
+                messageListRequest.getLimit().orElse(maximumLimit) + messageListRequest.getPosition().orElse(DEFAULT_POSITION))
                 .stream()
-                .skip(messageListRequest.getPosition())
+                .skip(messageListRequest.getPosition().orElse(DEFAULT_POSITION))
                 .forEach(builder::messageId);
             return builder.build();
         } catch (MailboxException e) {

http://git-wip-us.apache.org/repos/asf/james-project/blob/260dfceb/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListRequest.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListRequest.java
b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListRequest.java
index 7f67006..3531042 100644
--- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListRequest.java
+++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListRequest.java
@@ -40,12 +40,14 @@ public class GetMessageListRequest implements JmapRequest {
 
     @JsonPOJOBuilder(withPrefix = "")
     public static class Builder {
+        private static final int MIN_POSITION = 0;
+        private static final double MAX_POSITION = Math.pow(2, 53);
 
         private String accountId;
         private Filter filter;
         private final ImmutableList.Builder<String> sort;
         private Boolean collapseThreads;
-        private int position;
+        private Optional<Long> position;
         private String anchor;
         private Integer anchorOffset;
         private Integer limit;
@@ -55,6 +57,7 @@ public class GetMessageListRequest implements JmapRequest {
         private Boolean fetchSearchSnippets;
 
         private Builder() {
+            position = Optional.empty();
             sort = ImmutableList.builder();
             fetchMessageProperties = ImmutableList.builder();
         }
@@ -78,8 +81,8 @@ public class GetMessageListRequest implements JmapRequest {
             return this;
         }
 
-        public Builder position(int position) {
-            this.position = position;
+        public Builder position(long position) {
+            this.position = Optional.of(position);
             return this;
         }
 
@@ -115,7 +118,8 @@ public class GetMessageListRequest implements JmapRequest {
         }
 
         public GetMessageListRequest build() {
-            Preconditions.checkState(position >= 0, "'position' should be positive or
null");
+            Preconditions.checkState(position.map(position -> position >= MIN_POSITION
&& position <= MAX_POSITION).orElse(true),
+                "'position' should be positive and less than 2^53 or empty");
             checkLimit();
             return new GetMessageListRequest(Optional.ofNullable(accountId), Optional.ofNullable(filter),
sort.build(), Optional.ofNullable(collapseThreads),
                     position, Optional.ofNullable(anchor), Optional.ofNullable(anchorOffset),
Optional.ofNullable(limit), Optional.ofNullable(fetchThreads),
@@ -133,7 +137,7 @@ public class GetMessageListRequest implements JmapRequest {
     private final Optional<Filter> filter;
     private final List<String> sort;
     private final Optional<Boolean> collapseThreads;
-    private final int position;
+    private final Optional<Long> position;
     private final Optional<String> anchor;
     private final Optional<Integer> anchorOffset;
     private final Optional<Integer> limit;
@@ -143,7 +147,7 @@ public class GetMessageListRequest implements JmapRequest {
     private final Optional<Boolean> fetchSearchSnippets;
 
     @VisibleForTesting GetMessageListRequest(Optional<String> accountId, Optional<Filter>
filter, List<String> sort, Optional<Boolean> collapseThreads,
-            int position, Optional<String> anchor, Optional<Integer> anchorOffset,
Optional<Integer> limit, Optional<Boolean> fetchThreads,
+            Optional<Long> position, Optional<String> anchor, Optional<Integer>
anchorOffset, Optional<Integer> limit, Optional<Boolean> fetchThreads,
             Optional<Boolean> fetchMessages, List<String> fetchMessageProperties,
Optional<Boolean> fetchSearchSnippets) {
 
         this.accountId = accountId;
@@ -176,7 +180,7 @@ public class GetMessageListRequest implements JmapRequest {
         return collapseThreads;
     }
 
-    public int getPosition() {
+    public Optional<Long> getPosition() {
         return position;
     }
 

http://git-wip-us.apache.org/repos/asf/james-project/blob/260dfceb/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/GetMessageListRequestTest.java
----------------------------------------------------------------------
diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/GetMessageListRequestTest.java
b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/GetMessageListRequestTest.java
index 1adcdd1..77e7247 100644
--- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/GetMessageListRequestTest.java
+++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/model/GetMessageListRequestTest.java
@@ -33,7 +33,7 @@ public class GetMessageListRequestTest {
 
     @Test(expected=IllegalStateException.class)
     public void builderShouldThrowWhenPositionIsNegative() {
-        GetMessageListRequest.builder().position(-1).build();
+        GetMessageListRequest.builder().position(-1L).build();
     }
 
     @Test(expected=IllegalStateException.class)
@@ -73,14 +73,14 @@ public class GetMessageListRequestTest {
                 .build();
         List<String> sort = ImmutableList.of("date desc");
         List<String> fetchMessageProperties = ImmutableList.of("id", "blobId");
-        GetMessageListRequest expectedGetMessageListRequest = new GetMessageListRequest(Optional.empty(),
Optional.of(filterCondition), sort, Optional.of(true), 1, Optional.empty(), Optional.empty(),
Optional.of(2),
+        GetMessageListRequest expectedGetMessageListRequest = new GetMessageListRequest(Optional.empty(),
Optional.of(filterCondition), sort, Optional.of(true), Optional.of(1L), Optional.empty(),
Optional.empty(), Optional.of(2),
                 Optional.empty(), Optional.of(true), fetchMessageProperties, Optional.empty());
 
         GetMessageListRequest getMessageListRequest = GetMessageListRequest.builder()
             .filter(filterCondition)
             .sort(sort)
             .collapseThreads(true)
-            .position(1)
+            .position(1L)
             .limit(2)
             .fetchMessages(true)
             .fetchMessageProperties(fetchMessageProperties)


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


Mime
View raw message