JAMES-2255 GetMessageList should use Number for number data
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/a763b173 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/a763b173 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/a763b173 Branch: refs/heads/master Commit: a763b17376a29cc7cd1cf9a45d111e881da01890 Parents: 260dfce Author: quynhn <[email protected]> Authored: Tue Dec 12 13:53:54 2017 +0700 Committer: benwa <[email protected]> Committed: Fri Jan 5 16:10:36 2018 +0700 ---------------------------------------------------------------------- .../integration/GetMessageListMethodTest.java | 7 ++-- .../jmap/methods/GetMessageListMethod.java | 6 ++- .../james/jmap/model/GetMessageListRequest.java | 39 +++++++------------- .../jmap/model/GetMessageListResponse.java | 19 ++++++---- .../jmap/model/GetMessageListRequestTest.java | 2 +- 5 files changed, 33 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/a763b173/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 5ba9bd4..119eea7 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 @@ -44,6 +44,7 @@ import org.apache.http.client.utils.URIBuilder; import org.apache.james.GuiceJamesServer; import org.apache.james.jmap.HttpJmapAuthentication; import org.apache.james.jmap.api.access.AccessToken; +import org.apache.james.jmap.model.Number; import org.apache.james.mailbox.FlagsBuilder; import org.apache.james.mailbox.model.ComposedMessageId; import org.apache.james.mailbox.model.MailboxACL.Rfc4314Rights; @@ -1904,7 +1905,7 @@ public abstract class GetMessageListMethodTest { public void getMessageListShouldAcceptLessThan2Pow53NumberForPosition() throws Exception { given() .header("Authorization", aliceAccessToken.serialize()) - .body("[[\"getMessageList\", {\"position\":" + Math.pow(2, 53) + "}, \"#0\"]]") + .body("[[\"getMessageList\", {\"position\":" + Number.MAX_VALUE + "}, \"#0\"]]") .when() .post("/jmap") .then() @@ -1916,13 +1917,13 @@ public abstract class GetMessageListMethodTest { public void getMessageListShouldErrorWhenPositionOver2Pow53() throws Exception { given() .header("Authorization", aliceAccessToken.serialize()) - .body("[[\"getMessageList\", {\"position\":" + Math.pow(2, 54) + "}, \"#0\"]]") + .body("[[\"getMessageList\", {\"position\":" + Number.MAX_VALUE + 1 + "}, \"#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")); + .body(ARGUMENTS + ".description", containsString("value should be positive and less than 2^53 or empty")); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/a763b173/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 749c104..c618126 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 @@ -34,6 +34,7 @@ import org.apache.james.jmap.model.FilterCondition; import org.apache.james.jmap.model.GetMessageListRequest; import org.apache.james.jmap.model.GetMessageListResponse; import org.apache.james.jmap.model.GetMessagesRequest; +import org.apache.james.jmap.model.Number; import org.apache.james.jmap.utils.FilterToSearchQuery; import org.apache.james.jmap.utils.SortConverter; import org.apache.james.mailbox.MailboxManager; @@ -124,11 +125,12 @@ public class GetMessageListMethod implements Method { GetMessageListResponse.Builder builder = GetMessageListResponse.builder(); try { MultimailboxesSearchQuery searchQuery = convertToSearchQuery(messageListRequest); + Long postionValue = messageListRequest.getPosition().map(Number::asLong).orElse(DEFAULT_POSITION); mailboxManager.search(searchQuery, mailboxSession, - messageListRequest.getLimit().orElse(maximumLimit) + messageListRequest.getPosition().orElse(DEFAULT_POSITION)) + postionValue + messageListRequest.getLimit().map(Number::asInt).orElse(maximumLimit)) .stream() - .skip(messageListRequest.getPosition().orElse(DEFAULT_POSITION)) + .skip(postionValue) .forEach(builder::messageId); return builder.build(); } catch (MailboxException e) { http://git-wip-us.apache.org/repos/asf/james-project/blob/a763b173/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 3531042..b675537 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 @@ -28,7 +28,6 @@ import org.apache.james.jmap.methods.JmapRequest; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @JsonDeserialize(builder = GetMessageListRequest.Builder.class) @@ -40,17 +39,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 Optional<Long> position; + private Optional<Number> position; private String anchor; - private Integer anchorOffset; - private Integer limit; + private Number anchorOffset; + private Number limit; private Boolean fetchThreads; private Boolean fetchMessages; private final ImmutableList.Builder<String> fetchMessageProperties; @@ -82,7 +78,7 @@ public class GetMessageListRequest implements JmapRequest { } public Builder position(long position) { - this.position = Optional.of(position); + this.position = Optional.of(Number.fromLong(position)); return this; } @@ -94,8 +90,8 @@ public class GetMessageListRequest implements JmapRequest { throw new NotImplementedException(); } - public Builder limit(int limit) { - this.limit = limit; + public Builder limit(long limit) { + this.limit = Number.fromLong(limit); return this; } @@ -118,36 +114,27 @@ public class GetMessageListRequest implements JmapRequest { } public GetMessageListRequest build() { - 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), Optional.ofNullable(fetchMessages), fetchMessageProperties.build(), Optional.ofNullable(fetchSearchSnippets)); } - - private void checkLimit() { - if (limit != null) { - Preconditions.checkState(limit >= 0, "'limit' should be positive or null"); - } - } } private final Optional<String> accountId; private final Optional<Filter> filter; private final List<String> sort; private final Optional<Boolean> collapseThreads; - private final Optional<Long> position; + private final Optional<Number> position; private final Optional<String> anchor; - private final Optional<Integer> anchorOffset; - private final Optional<Integer> limit; + private final Optional<Number> anchorOffset; + private final Optional<Number> limit; private final Optional<Boolean> fetchThreads; private final Optional<Boolean> fetchMessages; private final List<String> fetchMessageProperties; private final Optional<Boolean> fetchSearchSnippets; @VisibleForTesting GetMessageListRequest(Optional<String> accountId, Optional<Filter> filter, List<String> sort, Optional<Boolean> collapseThreads, - Optional<Long> position, Optional<String> anchor, Optional<Integer> anchorOffset, Optional<Integer> limit, Optional<Boolean> fetchThreads, + Optional<Number> position, Optional<String> anchor, Optional<Number> anchorOffset, Optional<Number> limit, Optional<Boolean> fetchThreads, Optional<Boolean> fetchMessages, List<String> fetchMessageProperties, Optional<Boolean> fetchSearchSnippets) { this.accountId = accountId; @@ -180,7 +167,7 @@ public class GetMessageListRequest implements JmapRequest { return collapseThreads; } - public Optional<Long> getPosition() { + public Optional<Number> getPosition() { return position; } @@ -188,11 +175,11 @@ public class GetMessageListRequest implements JmapRequest { return anchor; } - public Optional<Integer> getAnchorOffset() { + public Optional<Number> getAnchorOffset() { return anchorOffset; } - public Optional<Integer> getLimit() { + public Optional<Number> getLimit() { return limit; } http://git-wip-us.apache.org/repos/asf/james-project/blob/a763b173/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListResponse.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListResponse.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListResponse.java index 8c3c9a2..490892f 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListResponse.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/model/GetMessageListResponse.java @@ -20,6 +20,7 @@ package org.apache.james.jmap.model; import java.util.List; +import java.util.Optional; import org.apache.commons.lang.NotImplementedException; import org.apache.james.jmap.methods.Method; @@ -42,8 +43,8 @@ public class GetMessageListResponse implements Method.Response { private boolean collapseThreads; private String state; private boolean canCalculateUpdates; - private int position; - private int total; + private Optional<Number> position; + private Optional<Number> total; private final ImmutableList.Builder<String> threadIds; private final ImmutableList.Builder<MessageId> messageIds; @@ -51,6 +52,8 @@ public class GetMessageListResponse implements Method.Response { sort = ImmutableList.builder(); threadIds = ImmutableList.builder(); messageIds = ImmutableList.builder(); + position = Optional.empty(); + total = Optional.empty(); } public Builder accountId(String accountId) { @@ -103,7 +106,7 @@ public class GetMessageListResponse implements Method.Response { public GetMessageListResponse build() { return new GetMessageListResponse(accountId, filter, sort.build(), collapseThreads, state, - canCalculateUpdates, position, total, threadIds.build(), messageIds.build()); + canCalculateUpdates, position.orElse(Number.ZERO), total.orElse(Number.ZERO), threadIds.build(), messageIds.build()); } } @@ -113,13 +116,13 @@ public class GetMessageListResponse implements Method.Response { private final boolean collapseThreads; private final String state; private final boolean canCalculateUpdates; - private final int position; - private final int total; + private final Number position; + private final Number total; private final List<String> threadIds; private final List<MessageId> messageIds; @VisibleForTesting GetMessageListResponse(String accountId, Filter filter, List<String> sort, boolean collapseThreads, String state, - boolean canCalculateUpdates, int position, int total, List<String> threadIds, List<MessageId> messageIds) { + boolean canCalculateUpdates, Number position, Number total, List<String> threadIds, List<MessageId> messageIds) { this.accountId = accountId; this.filter = filter; @@ -157,11 +160,11 @@ public class GetMessageListResponse implements Method.Response { return canCalculateUpdates; } - public int getPosition() { + public Number getPosition() { return position; } - public int getTotal() { + public Number getTotal() { return total; } http://git-wip-us.apache.org/repos/asf/james-project/blob/a763b173/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 77e7247..3e674e9 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 @@ -73,7 +73,7 @@ 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), Optional.of(1L), Optional.empty(), Optional.empty(), Optional.of(2), + GetMessageListRequest expectedGetMessageListRequest = new GetMessageListRequest(Optional.empty(), Optional.of(filterCondition), sort, Optional.of(true), Optional.of(Number.fromLong(1L)), Optional.empty(), Optional.empty(), Optional.of(Number.fromInt(2)), Optional.empty(), Optional.of(true), fetchMessageProperties, Optional.empty()); GetMessageListRequest getMessageListRequest = GetMessageListRequest.builder() --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
