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/08 03:18:47 UTC
[GitHub] [james-project] chibenwa opened a new pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
chibenwa opened a new pull request #909:
URL: https://github.com/apache/james-project/pull/909
## ISSUES
- James sends empty VANISHED response that don't comply with formal syntax
- We confused highest modseq and highest modseq for which deletion occured which caused under some circumpstances vanished response not to be returned
- Closed response was never sent upon implicit mailbox closure
- FETCH was not allowing to combine several modifiers
- SELECT was not strictly obeying QRESYNC formal syntax
- Math.abs related bug upon UIDVALIDITY generation
## What to do from here
We need to test this changeset manually seriously with Evolution client with QRESYNC enabled to see if we solved our issues.
--
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
[GitHub] [james-project] chibenwa commented on pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #909:
URL: https://github.com/apache/james-project/pull/909#issuecomment-1068664093
Rebased to solve conlict!
--
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
[GitHub] [james-project] vttranlina commented on a change in pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [james-project] chibenwa merged pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #909:
URL: https://github.com/apache/james-project/pull/909
--
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
[GitHub] [james-project] chibenwa commented on a change in pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #909:
URL: https://github.com/apache/james-project/pull/909#discussion_r830773910
##########
File path: protocols/imap/src/main/java/org/apache/james/imap/decode/parser/FetchCommandParser.java
##########
@@ -81,39 +81,40 @@ private FetchData fetchRequest(ImapRequestLineReader request, boolean useUid) th
}
request.consumeChar(')');
-
next = nextNonSpaceChar(request);
if (next == '(') {
request.consumeChar('(');
-
- next = request.nextChar();
- switch (next) {
- case 'C':
- // Now check for the CHANGEDSINCE option which is part of CONDSTORE
- request.consumeWord(StringMatcherCharacterValidator.ignoreCase(CHANGEDSINCE));
- fetch.changedSince(request.number(true));
- break;
- case 'V':
- // Check for the VANISHED option which is part of QRESYNC
- request.consumeWord(StringMatcherCharacterValidator.ignoreCase(VANISHED));
- fetch.vanished(true);
- break;
- default:
- break;
- }
-
-
+ parseModifier(request, fetch);
+ nextNonSpaceChar(request);
+ parseModifier(request, fetch);
+ nextNonSpaceChar(request);
Review comment:
Done in https://github.com/apache/james-project/pull/909/commits/b2a8404c605192adb8b788d3f6941b68c949bab9
--
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
[GitHub] [james-project] chibenwa commented on pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #909:
URL: https://github.com/apache/james-project/pull/909#issuecomment-1073506582
Reviews here?
I would like to merge those tests / fixes...
--
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
[GitHub] [james-project] chibenwa commented on pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #909:
URL: https://github.com/apache/james-project/pull/909#issuecomment-1066586930
I confirms this solves the QRESYNC issues with evolution...
--
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
[GitHub] [james-project] Arsnael commented on a change in pull request #909: JAMES-3722 Tests and fixes for IMAP QRESYNC
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #909:
URL: https://github.com/apache/james-project/pull/909#discussion_r827783530
##########
File path: protocols/imap/src/main/java/org/apache/james/imap/decode/parser/FetchCommandParser.java
##########
@@ -81,39 +81,40 @@ private FetchData fetchRequest(ImapRequestLineReader request, boolean useUid) th
}
request.consumeChar(')');
-
next = nextNonSpaceChar(request);
if (next == '(') {
request.consumeChar('(');
-
- next = request.nextChar();
- switch (next) {
- case 'C':
- // Now check for the CHANGEDSINCE option which is part of CONDSTORE
- request.consumeWord(StringMatcherCharacterValidator.ignoreCase(CHANGEDSINCE));
- fetch.changedSince(request.number(true));
- break;
- case 'V':
- // Check for the VANISHED option which is part of QRESYNC
- request.consumeWord(StringMatcherCharacterValidator.ignoreCase(VANISHED));
- fetch.vanished(true);
- break;
- default:
- break;
- }
-
-
+ parseModifier(request, fetch);
+ nextNonSpaceChar(request);
+ parseModifier(request, fetch);
+ nextNonSpaceChar(request);
Review comment:
I get there is only maybe 2 options but that seems very arbitrary code to me.
How about sth like:
```
while (parseModifier(request, fetch)) {
nextNonSpaceChar(request);
}
```
with `parseModifier` return a boolean, true as long as it finds an option and false if nothing? If no options, you would not parse twice either?
I'm not sure though, let me know what you think
--
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