You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2022/03/01 02:07:56 UTC

[james-project] 02/02: JAMES-3715 PartialFetchBodyElement: use Optional to represent absence

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 c30874891307f077ea5060dac86988caafa30271
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu Feb 24 09:25:05 2022 +0700

    JAMES-3715 PartialFetchBodyElement: use Optional to represent absence
    
    This makes the behaviour explicit and prevents mis-calculations down the
    line, and solves partial fetchs with only the offset (commons-net IMAP
    stack is happy...)
---
 .../imap/processor/fetch/FetchResponseBuilder.java |  3 +--
 .../processor/fetch/PartialFetchBodyElement.java   | 15 +++++++------
 .../fetch/PartialFetchBodyElementTest.java         | 25 +++++++++++-----------
 .../james/imapserver/netty/IMAPServerTest.java     |  2 --
 4 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchResponseBuilder.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchResponseBuilder.java
index fc26dd6..8182f5a 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchResponseBuilder.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchResponseBuilder.java
@@ -28,7 +28,6 @@ import java.util.Collection;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 
 import javax.mail.Flags;
@@ -275,7 +274,7 @@ public final class FetchResponseBuilder {
         if (firstOctet == null) {
             return fullResult;
         }
-        final long numberOfOctetsAsLong = Objects.requireNonNullElse(numberOfOctets, Long.MAX_VALUE);
+        final Optional<Long> numberOfOctetsAsLong = Optional.ofNullable(numberOfOctets);
         final long firstOctetAsLong = firstOctet;
         return new PartialFetchBodyElement(fullResult, firstOctetAsLong, numberOfOctetsAsLong);
     }
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElement.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElement.java
index f2546cc..9d33e1f 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElement.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElement.java
@@ -22,6 +22,7 @@ package org.apache.james.imap.processor.fetch;
 import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Optional;
 
 import org.apache.james.imap.message.response.FetchResponse.BodyElement;
 
@@ -31,10 +32,10 @@ import org.apache.james.imap.message.response.FetchResponse.BodyElement;
 final class PartialFetchBodyElement implements BodyElement {
     private final BodyElement delegate;
     private final long firstOctet;
-    private final long numberOfOctets;
+    private final Optional<Long> numberOfOctets;
     private final String name;
 
-    public PartialFetchBodyElement(BodyElement delegate, long firstOctet, long numberOfOctets) {
+    public PartialFetchBodyElement(BodyElement delegate, long firstOctet, Optional<Long> numberOfOctets) {
         this.delegate = delegate;
         this.firstOctet = firstOctet;
         this.numberOfOctets = numberOfOctets;
@@ -49,14 +50,14 @@ final class PartialFetchBodyElement implements BodyElement {
     @Override
     public long size() throws IOException {
         final long size = delegate.size();
-        final long lastOctet = this.numberOfOctets + firstOctet;
+
         if (firstOctet > size) {
             return  0;
-        } else if (size > lastOctet) {
-            return numberOfOctets;
-        } else {
-            return size - firstOctet;
         }
+
+        return numberOfOctets
+            .map(requestedSize -> Math.min(requestedSize, size - firstOctet))
+            .orElse(size - firstOctet);
     }
 
     @Override
diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElementTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElementTest.java
index a35226d..188d80b 100644
--- a/protocols/imap/src/test/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElementTest.java
+++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/fetch/PartialFetchBodyElementTest.java
@@ -23,9 +23,10 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.util.Optional;
+
 import org.apache.james.imap.message.response.FetchResponse.BodyElement;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 class PartialFetchBodyElementTest {
@@ -34,7 +35,7 @@ class PartialFetchBodyElementTest {
     BodyElement mockBodyElement;
 
     @BeforeEach
-    void setUp() throws Exception {
+    void setUp() {
         mockBodyElement = mock(BodyElement.class);
         when(mockBodyElement.getName()).thenReturn("Name");
     }
@@ -42,7 +43,7 @@ class PartialFetchBodyElementTest {
     @Test
     void testSizeShouldBeNumberOfOctetsWhenSizeMoreWhenStartIsZero() throws Exception {
         final long moreThanNumberOfOctets = NUMBER_OF_OCTETS + 1;
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 0, NUMBER_OF_OCTETS);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 0, Optional.of(NUMBER_OF_OCTETS));
         when(mockBodyElement.size()).thenReturn(moreThanNumberOfOctets);
 
         assertThat(element.size()).describedAs("Size is more than number of octets so should be number of octets").isEqualTo(NUMBER_OF_OCTETS);
@@ -51,7 +52,7 @@ class PartialFetchBodyElementTest {
     @Test
     void testSizeShouldBeSizeWhenNumberOfOctetsMoreWhenStartIsZero() throws Exception {
         final long lessThanNumberOfOctets = NUMBER_OF_OCTETS - 1;
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 0, NUMBER_OF_OCTETS);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 0, Optional.of(NUMBER_OF_OCTETS));
         when(mockBodyElement.size()).thenReturn(lessThanNumberOfOctets);
 
         assertThat(element.size()).describedAs("Size is less than number of octets so should be size").isEqualTo(lessThanNumberOfOctets);
@@ -60,7 +61,7 @@ class PartialFetchBodyElementTest {
     @Test
     void testWhenStartPlusNumberOfOctetsIsMoreThanSizeSizeShouldBeSizeMinusStart() throws Exception {
         final long size = 60;
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 10, NUMBER_OF_OCTETS);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 10, Optional.of(NUMBER_OF_OCTETS));
         when(mockBodyElement.size()).thenReturn(size);
 
         assertThat(element.size()).describedAs("Size is less than number of octets so should be size").isEqualTo(50);
@@ -70,7 +71,7 @@ class PartialFetchBodyElementTest {
     void testWhenStartPlusNumberOfOctetsIsLessThanSizeSizeShouldBeNumberOfOctetsMinusStart()
         throws Exception {
         final long size = 100;
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 10, NUMBER_OF_OCTETS);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 10, Optional.of(NUMBER_OF_OCTETS));
         when(mockBodyElement.size()).thenReturn(size);
 
         assertThat(element.size()).describedAs("Size is less than number of octets so should be size").isEqualTo(90);
@@ -79,16 +80,16 @@ class PartialFetchBodyElementTest {
     @Test
     void testSizeShouldBeZeroWhenStartIsMoreThanSize() throws Exception {
         final long size = 100;
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 1000, NUMBER_OF_OCTETS);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 1000, Optional.of(NUMBER_OF_OCTETS));
         when(mockBodyElement.size()).thenReturn(size);
 
-        assertThat(element.size()).describedAs("Size is less than number of octets so should be size").isEqualTo(0);
+        assertThat(element.size()).describedAs("Size is less than number of octets so should be size").isZero();
     }
 
     @Test
     void testSizeShouldBeNumberOfOctetsWhenStartMoreThanOctets() throws Exception {
         final long size = 2000;
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 1000, NUMBER_OF_OCTETS);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 1000, Optional.of(NUMBER_OF_OCTETS));
         when(mockBodyElement.size()).thenReturn(size);
 
         assertThat(element.size()).describedAs("Content size is less than start. Size should be zero.").isEqualTo(NUMBER_OF_OCTETS);
@@ -96,19 +97,17 @@ class PartialFetchBodyElementTest {
 
     @Test
     void testSizeShouldBeNumberOfOctetsWhenSizeMoreWhenStartIsZeroAndNoLimitSpecified() throws Exception {
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 0, Long.MAX_VALUE);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 0, Optional.empty());
         when(mockBodyElement.size()).thenReturn(NUMBER_OF_OCTETS);
 
         assertThat(element.size()).describedAs("Size is more than number of octets so should be number of octets")
             .isEqualTo(NUMBER_OF_OCTETS);
     }
 
-    @Disabled("JAMES-3715 A bug due to usage of Long.MAX to represent a value that is absent prevents this" +
-        "from working decently. Returns 9223372036854775807L which cannot be parsed nor handled by IMAP clients.")
     @Test
     void testWhenStartPlusNumberOfOctetsIsMoreThanSizeSizeShouldBeSizeMinusStartAndNoLimitSpecified() throws Exception {
         final long size = 60;
-        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 10, Long.MAX_VALUE);
+        PartialFetchBodyElement element = new PartialFetchBodyElement(mockBodyElement, 10, Optional.empty());
         when(mockBodyElement.size()).thenReturn(size);
 
         assertThat(element.size()).describedAs("Size is less than number of octets so should be size").isEqualTo(50);
diff --git a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
index 4094f8d..07f3bfa 100644
--- a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
+++ b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
@@ -204,8 +204,6 @@ class IMAPServerTest {
                 .contains("* 1 FETCH (FLAGS (\\Recent \\Seen) BODY[]<8> {12}\r\nvalue\r\n\r\nBOD)\r\n");
         }
 
-        @Disabled("JAMES-3715 A bug due to usage of Long.MAX to represent a value that is absent prevents this" +
-            "from working decently.")
         @Test
         void fetchShouldRetrieveMessageWhenOffsetAndNoLimitSpecified() throws Exception {
             testIMAPClient.connect("127.0.0.1", port)

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