You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/03/21 07:10:37 UTC

[GitHub] [james-project] vttranlina commented on a change in pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC

vttranlina commented on a change in pull request #909:
URL: https://github.com/apache/james-project/pull/909#discussion_r830795564



##########
File path: server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
##########
@@ -1276,12 +1258,250 @@ void selectShouldReturnDeletedMessagesWhenNoSubsequentModification() throws Exce
             server.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
             readBytes(server);
             server.write(ByteBuffer.wrap(("a1 ENABLE QRESYNC\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a1 OK ENABLE completed."));
+            server.write(ByteBuffer.wrap(String.format("I00104 SELECT INBOX (QRESYNC (%d %d 2:37 (1,10,28 2,11,29)))\r\n", uidValidity.asLong(), highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));
+
+            assertThat(readStringUntil(server, s -> s.contains("I00104 OK [READ-WRITE] SELECT completed.")))
+                .filteredOn(s -> s.contains("* VANISHED (EARLIER) 10"))
+                .hasSize(1);
+        }
+
+        @Test
+        void selectShouldCombineIntoRangesWhenRespondingVanished() throws Exception {
+            inbox.delete(ImmutableList.of(MessageUid.MIN_VALUE), mailboxSession);
+
+            ModSeq highestModSeq = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMetaData(false, mailboxSession, MessageManager.MailboxMetaData.FetchGroup.NO_COUNT)
+                .getHighestModSeq();
+
+            UidValidity uidValidity = memoryIntegrationResources.getMailboxManager()
+                .getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMailboxEntity().getUidValidity();
+
+            inbox.delete(ImmutableList.of(MessageUid.of(10), MessageUid.of(11), MessageUid.of(12),
+                    MessageUid.of(25), MessageUid.of(26),
+                    MessageUid.of(32)), mailboxSession);
+
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, port));
             readBytes(server);
+
+            server.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
             readBytes(server);
+            server.write(ByteBuffer.wrap(("a1 ENABLE QRESYNC\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a1 OK ENABLE completed."));
             server.write(ByteBuffer.wrap(String.format("I00104 SELECT INBOX (QRESYNC (%d %d 2:37 (1,10,28 2,11,29)))\r\n", uidValidity.asLong(), highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));
 
             assertThat(readStringUntil(server, s -> s.contains("I00104 OK [READ-WRITE] SELECT completed.")))
-                .filteredOn(s -> s.contains("* VANISHED (EARLIER) 10"))
+                .filteredOn(s -> s.contains("* VANISHED (EARLIER) 10:12,25:26,32"))
+                .hasSize(1);
+        }
+
+        private void setUpTestingData() {
+            IntStream.range(0, 37)
+                .forEach(Throwing.intConsumer(i -> inbox.appendMessage(MessageManager.AppendCommand.builder()
+                    .build("MIME-Version: 1.0\r\n\r\nCONTENT\r\n"), mailboxSession)));
+        }
+
+        @Test
+        void fetchShouldAllowChangedSinceModifier() throws Exception {
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, port));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("a1 ENABLE QRESYNC\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a1 OK ENABLE completed."));
+            server.write(ByteBuffer.wrap(("a2 SELECT INBOX\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .setFlags(new Flags(Flags.Flag.ANSWERED), MessageManager.FlagsUpdateMode.REPLACE, MessageRange.one(MessageUid.of(10)), mailboxSession);
+
+            ModSeq highestModSeq = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMetaData(false, mailboxSession, MessageManager.MailboxMetaData.FetchGroup.NO_COUNT)
+                .getHighestModSeq();
+            server.write(ByteBuffer.wrap(String.format("I00104 UID FETCH 1:37 (FLAGS) (CHANGEDSINCE %d)\r\n", highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));
+
+            assertThat(readStringUntil(server, s -> s.contains("I00104 OK FETCH completed.")))
+                .filteredOn(s -> s.contains("* 10 FETCH (MODSEQ (39) FLAGS (\\Answered \\Recent) UID 10)"))
+                .hasSize(1);
+        }
+
+        @Test
+        void fetchShouldNotReturnChangedItemsOutOfRange() throws Exception {
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, port));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("a1 ENABLE QRESYNC\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a1 OK ENABLE completed."));
+            server.write(ByteBuffer.wrap(("a2 SELECT INBOX\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            inbox.setFlags(new Flags(Flags.Flag.ANSWERED), MessageManager.FlagsUpdateMode.REPLACE, MessageRange.one(MessageUid.of(10)), mailboxSession);
+
+            ModSeq highestModSeq = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMetaData(false, mailboxSession, MessageManager.MailboxMetaData.FetchGroup.NO_COUNT)
+                .getHighestModSeq();
+            server.write(ByteBuffer.wrap(String.format("I00104 UID FETCH 12:37 (FLAGS) (CHANGEDSINCE %d)\r\n", highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));
+
+            assertThat(readStringUntil(server, s -> s.contains("I00104 OK FETCH completed.")))
+                .filteredOn(s -> s.contains("FLAGS")) // No FLAGS FETCH responses
+                .hasSize(1);
+        }
+
+        @Disabled("JAMES-3722 IMAP stack failed to parse FETCH command with two modifiers and thus do" +
+            "not conform to the example of the RFC-5162")
+        @Test
+        void fetchShouldSupportVanishedModifiedWithEarlierTag() throws Exception {
+            inbox.delete(ImmutableList.of(MessageUid.of(14)), mailboxSession);
+
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, port));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("a1 ENABLE QRESYNC\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a1 OK ENABLE completed."));
+            server.write(ByteBuffer.wrap(("a2 SELECT INBOX\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .setFlags(new Flags(Flags.Flag.ANSWERED), MessageManager.FlagsUpdateMode.REPLACE, MessageRange.one(MessageUid.of(10)), mailboxSession);
+            memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .setFlags(new Flags(Flags.Flag.ANSWERED), MessageManager.FlagsUpdateMode.REPLACE, MessageRange.one(MessageUid.of(25)), mailboxSession);
+
+            ModSeq highestModSeq = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMetaData(false, mailboxSession, MessageManager.MailboxMetaData.FetchGroup.NO_COUNT)
+                .getHighestModSeq();
+            server.write(ByteBuffer.wrap(String.format("I00104 UID FETCH 12:37 (FLAGS) (CHANGEDSINCE %d VANISHED)\r\n", highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));
+
+            assertThat(readStringUntil(server, s -> s.contains("I00104 OK FETCH completed.")))
+                .filteredOn(s -> s.contains("* VANISHED (EARLIER) 14"))
+                .hasSize(1);
+        }
+
+        @Test
+        void unsolicitedNotificationsShouldBeSent() throws Exception {
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, port));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("a1 ENABLE QRESYNC\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a1 OK ENABLE completed."));
+            server.write(ByteBuffer.wrap(("a2 SELECT INBOX\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a2 OK [READ-WRITE] SELECT completed."));
+
+            memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .setFlags(new Flags(Flags.Flag.ANSWERED), MessageManager.FlagsUpdateMode.REPLACE, MessageRange.one(MessageUid.of(10)), mailboxSession);
+            memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .setFlags(new Flags(Flags.Flag.ANSWERED), MessageManager.FlagsUpdateMode.REPLACE, MessageRange.one(MessageUid.of(25)), mailboxSession);
+
+            inbox.delete(ImmutableList.of(MessageUid.of(14)), mailboxSession);
+
+            ModSeq highestModSeq = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMetaData(false, mailboxSession, MessageManager.MailboxMetaData.FetchGroup.NO_COUNT)
+                .getHighestModSeq();
+            server.write(ByteBuffer.wrap(String.format("I00104 NOOP\r\n", highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));

Review comment:
       missing `%d` in String.format

##########
File path: protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractSelectionProcessor.java
##########
@@ -278,21 +280,28 @@ private void respondVanished(ImapSession session, Responder responder, IdRange[]
             .reduce((t3_1, t3_2) -> t3_2)
             .orElse(MessageUid.MIN_VALUE);
 
-        // Ok now its time to filter out the IdRanges which we are not interested in
-        List<UidRange> filteredUidSet = new ArrayList<>();
-        for (UidRange r : uidSet) {
-            if (r.getLowVal().compareTo(firstKnownUid) < 0) {
-                if (r.getHighVal().compareTo(firstKnownUid) > 0) {
-                    filteredUidSet.add(new UidRange(firstKnownUid, r.getHighVal()));
-                }
+        return filter(uidSet, firstKnownUid);
+    }
+
+    private UidRange[] filter(UidRange[] uidSet, MessageUid lowerBound) {
+        return Arrays.stream(uidSet)
+            .flatMap(range -> filter(range, lowerBound))
+            .collect(ImmutableList.toImmutableList())

Review comment:
       Why do we need to convert it to ImmutableList in middle?
   
   ```
   return Arrays.stream(uidSet)
               .flatMap(range -> filter(range, lowerBound))
               .toArray(UidRange[]::new);
   ``` 
   is better?
   

##########
File path: server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
##########
@@ -1323,7 +1324,83 @@ void selectShouldReturnDeletedMessagesWhenKnownUidSet() throws Exception {
             server.write(ByteBuffer.wrap(String.format("I00104 SELECT INBOX (QRESYNC (%d %d 5:11,28:36 (1,10,28 2,11,29)))\r\n", uidValidity.asLong(), highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));
 
             assertThat(readStringUntil(server, s -> s.contains("I00104 OK [READ-WRITE] SELECT completed.")))
-                .filteredOn(s -> s.contains("* VANISHED (EARLIER) 10"))
+                .filteredOn(s -> s.contains("* VANISHED (EARLIER) 10:11,32"))
+                .hasSize(1);
+        }
+
+        @Disabled("JAMES-3722 Known sequence sets are buggy and never restrict vanished replies." +
+            "Known sequence sets are a way to restrict the scope of vanished responses for servers " +
+            "not storing deletion sequences.")
+        @Test
+        void knownUidSetShouldBeUsedToRestrictVanishedResponses() throws Exception {
+            inbox.delete(ImmutableList.of(MessageUid.MIN_VALUE), mailboxSession);
+
+            ModSeq highestModSeq = memoryIntegrationResources.getMailboxManager().getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMetaData(false, mailboxSession, MessageManager.MailboxMetaData.FetchGroup.NO_COUNT)
+                .getHighestModSeq();
+
+            UidValidity uidValidity = memoryIntegrationResources.getMailboxManager()
+                .getMailbox(MailboxPath.inbox(USER), mailboxSession)
+                .getMailboxEntity().getUidValidity();
+
+            inbox.delete(ImmutableList.of(MessageUid.of(10), MessageUid.of(11), MessageUid.of(12),
+                MessageUid.of(25), MessageUid.of(26),
+                MessageUid.of(32)), mailboxSession);
+
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, port));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(String.format("a0 LOGIN %s %s\r\n", USER.asString(), USER_PASS).getBytes(StandardCharsets.UTF_8)));
+            readBytes(server);
+            server.write(ByteBuffer.wrap(("a1 ENABLE QRESYNC\r\n").getBytes(StandardCharsets.UTF_8)));
+            readStringUntil(server, s -> s.contains("a1 OK ENABLE completed."));
+            // MSN 1 => UID 2 MATCH
+            // MSN 13 => UID 17 MATCH
+            // MSN 28 => UID 30 MISMATCH stored value is 34
+            // Thus we know we can skip resynchronisation for UIDs up to 17
+            server.write(ByteBuffer.wrap(String.format("I00104 SELECT INBOX (QRESYNC (%d %d 1:37 (1,13,28 2,17,30)))\r\n", uidValidity.asLong(), highestModSeq.asLong()).getBytes(StandardCharsets.UTF_8)));
+
+            assertThat(readStringUntil(server, s -> s.contains("I00104 OK [READ-WRITE] SELECT completed.")))
+                .filteredOn(s -> s.contains("* VANISHED (EARLIER) 25:26,32"))
+                .hasSize(1);
+        }
+
+        @Disabled("JAMES-3722 Known sequence sets are buggy and never restrict vanished replies." +
+            "Known sequence sets are a way to restrict the scope of vanished responses for servers " +
+            "not storing deletion sequences.")
+        @Test
+        void knownUidSetShouldTorelateDeletedMessages() throws Exception {

Review comment:
       Torelate -> Tolerate ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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