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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
