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/11/24 01:37:25 UTC
[GitHub] [james-project] vttranlina opened a new pull request, #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
vttranlina opened a new pull request, #1329:
URL: https://github.com/apache/james-project/pull/1329
resolve: https://github.com/linagora/james-project/issues/4675
--
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 merged pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
Arsnael merged PR #1329:
URL: https://github.com/apache/james-project/pull/1329
--
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 #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1329:
URL: https://github.com/apache/james-project/pull/1329#issuecomment-1328839487
```
Checked exception is invalid for this method!
Invalid: org.apache.james.mailbox.exception.MailboxException
at org.apache.james.jmap.draft.methods.SetMessagesCreationProcessorTest.assertIsUserOwnerOfMailboxesShouldThrowWhenRetrievingMailboxPathFails(SetMessagesCreationProcessorTest.java:385)
```
=> Remove this test ?
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
vttranlina commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1031011780
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -140,9 +153,18 @@ private Mono<Void> respondMailboxList(String referenceName, String mailboxName,
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
- return getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath())))
- .then();
+ if (selectSubscribed) {
+ return Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get())
+ .collectList()
+ .flatMapMany(litSubscribed -> getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
+ .filter(metaData -> litSubscribed.contains(metaData.getPath()))
+ .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath()), true)))
+ .then();
Review Comment:
> Are we supposed to also return subscribed items that do not correspond to an existing mailbox?
Yes, then we have an attribute `NonExistent` in response
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1033052325
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -170,26 +174,90 @@ private Mono<Void> respondMailboxList(T request, ImapSession session,
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
if (selectSubscribed) {
- return getMailboxManager().search(mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession), Minimal, mailboxSession).collectList()
- .map(searchedResultList -> searchedResultList.stream().collect(Collectors.toMap(MailboxMetaData::getPath, Function.identity())))
- .flatMapMany(searchedResultMap -> Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get())
- .doOnNext(subscribed -> {
- if (searchedResultMap.containsKey(subscribed)) {
- processResult(responder, isRelative, searchedResultMap.get(subscribed), getMailboxType(session, subscribed), RETURN_SUBSCRIBED);
- } else {
- processNonExistenceMailboxResult(responder, getMailboxType(session, subscribed), subscribed);
- }
- })
- .flatMap(subscribed -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(subscribed, statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty())))
+ MailboxQuery mailboxQuery = mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession);
+
+ return Mono.zip(getMailboxManager().search(mailboxQuery, Minimal, mailboxSession).collectList()
+ .map(searchedResultList -> searchedResultList.stream().collect(Collectors.toMap(MailboxMetaData::getPath, Function.identity()))),
+ Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get()).collectList())
+ .map(tuple -> getListResponseForSelectSubscribed(tuple.getT1(), tuple.getT2(), request, mailboxSession, isRelative, mailboxQuery))
+ .flatMapIterable(list -> list)
+ .doOnNext(pathAndResponse -> responder.respond(pathAndResponse.getRight()))
+ .flatMap(pathAndResponse -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(pathAndResponse.getLeft(), statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty()))
.then();
+
} else {
return getMailboxManager().search(mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath()), !RETURN_SUBSCRIBED))
+ .doOnNext(metaData -> responder.respond(
+ createResponse(metaData.inferiors(),
+ metaData.getSelectability(),
+ mailboxName(isRelative, metaData.getPath(), metaData.getHierarchyDelimiter()),
+ metaData.getHierarchyDelimiter(),
+ getMailboxType(session, metaData.getPath()))))
.flatMap(metaData -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(metaData.getPath(), statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty()))
.then();
}
}
+ private List<Pair<MailboxPath, ListResponse>> getListResponseForSelectSubscribed(Map<MailboxPath, MailboxMetaData> searchedResultMap, List<MailboxPath> allSubscribedSearch,
Review Comment:
This method is way too long and deserve to be extracted in more submethods.
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1030988565
##########
protocols/imap/src/main/java/org/apache/james/imap/message/request/ListRequest.java:
##########
@@ -46,11 +68,25 @@ public final String getMailboxPattern() {
return mailboxPattern;
}
+ public EnumSet<ListSelectOption> getSelectOptions() {
+ return selectOptions;
+ }
+
+ public EnumSet<ListReturnOption> getReturnOptions() {
+ return returnOptions;
+ }
+
+ public final boolean selectSubscribed() {
Review Comment:
```suggestion
public boolean isSelectSubscribed() {
```
-> No need "final"
-> Boolean getters starts by "is..." ?
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1031654372
##########
protocols/imap/src/main/java/org/apache/james/imap/decode/parser/ListCommandParser.java:
##########
@@ -126,17 +129,29 @@ private ListSelectOption parseListSelectOption(ImapRequestLineReader request) th
"Unknown select option: '" + request.consumeWord(ImapRequestLineReader.NoopCharValidator.INSTANCE) + "'");
}
+ private Pair<ListReturnOption, Optional<StatusDataItems>> readS(ImapRequestLineReader request) throws DecodingException {
+ request.consume();
+ char c = request.nextWordChar();
+ if (c == 'T' || c == 't') {
+ return readStatus(request);
+ } else {
+ return Pair.of(readReturnSubscribed(request), Optional.empty());
+ }
+ }
+
+ private Pair<ListReturnOption, Optional<StatusDataItems>> readStatus(ImapRequestLineReader request) throws DecodingException {
+ // 'S' is already consummed
+ assertChar(request, 'T', 't');
+ assertChar(request, 'A', 'a');
+ assertChar(request, 'T','t');
Review Comment:
```suggestion
assertChar(request, 'T', 't');
```
--
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 pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
vttranlina commented on PR #1329:
URL: https://github.com/apache/james-project/pull/1329#issuecomment-1334696292
Squash & rebase master
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1030987755
##########
mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/ListMailboxes.test:
##########
@@ -26,6 +26,15 @@ S: 12 OK \[MAILBOXID \(.+\)\] CREATE completed.
C: 13 CREATE listtest1.subfolder1
S: 13 OK \[MAILBOXID \(.+\)\] CREATE completed.
+# List select option: SUBSCRIBE
+C: a02 SUBSCRIBE listtest
+S: a02 OK SUBSCRIBE completed.
+C: A01 LIST (SUBSCRIBED) "" "*"
+SUB {
+S: \* LIST \(\\HasChildren \\Subscribed\) \"\.\" "listtest"
+}
+S: A01 OK LIST completed.
+
Review Comment:
Please have a dedicates `ListSubscribed.test` that duplicated `Lsub.test`.
--
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 #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1329:
URL: https://github.com/apache/james-project/pull/1329#issuecomment-1334758913
Done in https://github.com/apache/james-project/pull/1331/commits/060358aaf10d8f96447284cff953238973335059
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1030991796
##########
protocols/imap/src/main/java/org/apache/james/imap/message/request/ListRequest.java:
##########
@@ -46,11 +68,25 @@ public final String getMailboxPattern() {
return mailboxPattern;
}
+ public EnumSet<ListSelectOption> getSelectOptions() {
+ return selectOptions;
+ }
+
+ public EnumSet<ListReturnOption> getReturnOptions() {
+ return returnOptions;
+ }
+
+ public final boolean selectSubscribed() {
+ return getSelectOptions().contains(ListSelectOption.SUBSCRIBED);
+ }
+
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("baseReferenceName", baseReferenceName)
.add("mailboxPattern", mailboxPattern)
+ .add("selectOptions", selectOptions.stream().map(Enum::toString).collect(Collectors.joining(",")))
+ .add("returnOptions", returnOptions.stream().map(Enum::toString).collect(Collectors.joining(",")))
Review Comment:
```suggestion
.add("selectOptions", selectOptions)
.add("returnOptions", returnOptions)
```
EnumSet default toString is displaying good enough and is more efficient.
--
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 pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #1329:
URL: https://github.com/apache/james-project/pull/1329#issuecomment-1333175454
Can you squash your fixups? As I see two distinct kind of commits here :)
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1031654153
##########
protocols/imap/src/main/java/org/apache/james/imap/decode/parser/ListCommandParser.java:
##########
@@ -157,30 +172,30 @@ private ListSelectOption readR(ImapRequestLineReader request) throws DecodingExc
}
private ListSelectOption readRemote(ImapRequestLineReader request) throws DecodingException {
- assertChar(request, 'M');
- assertChar(request, 'O');
- assertChar(request, 'T');
- assertChar(request, 'E');
+ assertChar(request, 'M', 'm');
+ assertChar(request, 'O', 'o');
+ assertChar(request, 'T', 'y');
Review Comment:
```suggestion
assertChar(request, 'T', 't');
```
--
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 diff in pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1033236424
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -258,6 +236,39 @@ private List<Pair<MailboxPath, ListResponse>> getListResponseForSelectSubscribed
return responseBuilders.build();
}
+ private List<Pair<MailboxPath, ListResponse>> listRecursiveMatch(Map<MailboxPath, MailboxMetaData> searchedResultMap,
+ List<MailboxPath> allSubscribedSearch, MailboxSession mailboxSession,
+ boolean relative, ListRequest listRequest) {
+ if (!listRequest.getSelectOptions().contains(RECURSIVEMATCH)) {
+ return List.of();
+ }
+
+ Set<MailboxPath> allSubscribedSearchParent = allSubscribedSearch.stream()
+ .flatMap(mailboxPath -> mailboxPath.getParents(mailboxSession.getPathDelimiter()).stream())
+ .collect(Collectors.toSet());
+
+ return searchedResultMap.entrySet().stream()
+ .filter(pair -> allSubscribedSearchParent.contains(pair.getKey()))
+ .map(pair -> {
+ MailboxMetaData metaData = pair.getValue();
+ ListResponse.Builder builder = ListResponse.builder()
+ .children(metaData.inferiors())
+ .selectability(metaData.getSelectability())
+ .hierarchyDelimiter(metaData.getHierarchyDelimiter())
+ .returnNonExistent(!RETURN_NON_EXISTENT)
+ .name(mailboxName(relative, pair.getValue().getPath(), pair.getValue().getHierarchyDelimiter()))
+ .childInfos(ListResponse.ChildInfo.SUBSCRIBED);
+
+ if (allSubscribedSearch.contains(pair.getKey())) {
+ builder.returnSubscribed(RETURN_SUBSCRIBED);
+ } else {
+ builder.returnSubscribed(!RETURN_SUBSCRIBED);
+ }
Review Comment:
```suggestion
builder.returnSubscribed(allSubscribedSearch.contains(pair.getKey()));
```
--
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 diff in pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1034241221
##########
protocols/imap/src/main/java/org/apache/james/imap/decode/parser/ListCommandParser.java:
##########
@@ -74,13 +83,190 @@ public String listMailbox(ImapRequestLineReader request) throws DecodingExceptio
@Override
protected ImapMessage decode(ImapRequestLineReader request, Tag tag, boolean useUids, ImapSession session) throws DecodingException {
+ EnumSet<ListSelectOption> listOptions = parseSelectOptions(request);
String referenceName = request.mailbox();
String mailboxPattern = listMailbox(request);
+
+ Pair<EnumSet<ListReturnOption>, Optional<StatusDataItems>> listReturnOptions = parseReturnOptions(request);
request.eol();
- return createMessage(referenceName, mailboxPattern, tag);
+
+ if (listOptions.isEmpty() && listReturnOptions.getLeft().isEmpty()) {
+ return createMessage(referenceName, mailboxPattern, tag);
+ }
+ return new ListRequest(ImapConstants.LIST_COMMAND, referenceName, mailboxPattern, tag, listOptions, listReturnOptions.getLeft(), listReturnOptions.getRight());
}
protected ImapMessage createMessage(String referenceName, String mailboxPattern, Tag tag) {
return new ListRequest(referenceName, mailboxPattern, tag);
}
+
+ private EnumSet<ListSelectOption> parseSelectOptions(ImapRequestLineReader request) throws DecodingException {
+ EnumSet<ListSelectOption> listOptions = EnumSet.noneOf(ListSelectOption.class);
+ if (request.nextWordChar() != '(') {
+ return listOptions;
+ }
+
+ request.consumeChar('(');
+ request.nextWordChar();
+
+ while (request.nextChar() != ')') {
+ listOptions.add(parseListSelectOption(request));
+ request.nextWordChar();
+ }
+ request.consumeChar(')');
+ return listOptions;
+ }
+
+ private ListSelectOption parseListSelectOption(ImapRequestLineReader request) throws DecodingException {
+ char c = request.nextWordChar();
+ if (c == 'r' || c == 'R') {
+ return readR(request);
+ }
+ if (c == 's' || c == 'S') {
+ return readSubscribed(request);
+ }
+ throw new DecodingException(HumanReadableText.ILLEGAL_ARGUMENTS,
+ "Unknown select option: '" + request.consumeWord(ImapRequestLineReader.NoopCharValidator.INSTANCE) + "'");
+ }
+
+ private Pair<ListReturnOption, Optional<StatusDataItems>> readS(ImapRequestLineReader request) throws DecodingException {
+ request.consume();
+ char c = request.nextWordChar();
+ if (c == 'T' || c == 't') {
+ return readStatus(request);
+ } else {
+ return Pair.of(readReturnSubscribed(request), Optional.empty());
+ }
+ }
+
+ private Pair<ListReturnOption, Optional<StatusDataItems>> readStatus(ImapRequestLineReader request) throws DecodingException {
+ // 'S' is already consummed
+ assertChar(request, 'T', 't');
+ assertChar(request, 'A', 'a');
+ assertChar(request, 'T', 't');
+ assertChar(request, 'U', 'u');
+ assertChar(request, 'S', 's');
+ return Pair.of(ListReturnOption.STATUS, Optional.empty());
+ }
+
+ private ListSelectOption readSubscribed(ImapRequestLineReader request) throws DecodingException {
+ consumeSubscribed(request);
+ return ListSelectOption.SUBSCRIBED;
+ }
+
+ private ListSelectOption readR(ImapRequestLineReader request) throws DecodingException {
+ request.consume();
+ char c2 = request.nextChar();
+ if (c2 == 'e' || c2 == 'E') {
+ request.consume();
+ char c3 = request.nextChar();
+ if (c3 == 'm' || c3 == 'M') {
+ return readRemote(request);
+ } else {
+ return readRecursivematch(request);
+ }
+ }
+ throw new DecodingException(HumanReadableText.ILLEGAL_ARGUMENTS,
+ "Unknown select option: '" + request.consumeWord(ImapRequestLineReader.NoopCharValidator.INSTANCE) + "'");
+ }
+
+ private ListSelectOption readRemote(ImapRequestLineReader request) throws DecodingException {
+ assertChar(request, 'M', 'm');
+ assertChar(request, 'O', 'o');
+ assertChar(request, 'T', 'y');
Review Comment:
OUPS still
--
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 pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
vttranlina commented on PR #1329:
URL: https://github.com/apache/james-project/pull/1329#issuecomment-1326164618
Right now, James already return `HasChildren`/`HasNoChildren` for the client (by default).
Should we care to `RETURN (CHILDREN)` in RFC-5258?
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1033051825
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -170,26 +174,90 @@ private Mono<Void> respondMailboxList(T request, ImapSession session,
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
if (selectSubscribed) {
- return getMailboxManager().search(mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession), Minimal, mailboxSession).collectList()
- .map(searchedResultList -> searchedResultList.stream().collect(Collectors.toMap(MailboxMetaData::getPath, Function.identity())))
- .flatMapMany(searchedResultMap -> Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get())
- .doOnNext(subscribed -> {
- if (searchedResultMap.containsKey(subscribed)) {
- processResult(responder, isRelative, searchedResultMap.get(subscribed), getMailboxType(session, subscribed), RETURN_SUBSCRIBED);
- } else {
- processNonExistenceMailboxResult(responder, getMailboxType(session, subscribed), subscribed);
- }
- })
- .flatMap(subscribed -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(subscribed, statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty())))
+ MailboxQuery mailboxQuery = mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession);
+
+ return Mono.zip(getMailboxManager().search(mailboxQuery, Minimal, mailboxSession).collectList()
+ .map(searchedResultList -> searchedResultList.stream().collect(Collectors.toMap(MailboxMetaData::getPath, Function.identity()))),
+ Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get()).collectList())
+ .map(tuple -> getListResponseForSelectSubscribed(tuple.getT1(), tuple.getT2(), request, mailboxSession, isRelative, mailboxQuery))
+ .flatMapIterable(list -> list)
+ .doOnNext(pathAndResponse -> responder.respond(pathAndResponse.getRight()))
+ .flatMap(pathAndResponse -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(pathAndResponse.getLeft(), statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty()))
.then();
+
} else {
return getMailboxManager().search(mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath()), !RETURN_SUBSCRIBED))
+ .doOnNext(metaData -> responder.respond(
+ createResponse(metaData.inferiors(),
+ metaData.getSelectability(),
+ mailboxName(isRelative, metaData.getPath(), metaData.getHierarchyDelimiter()),
+ metaData.getHierarchyDelimiter(),
+ getMailboxType(session, metaData.getPath()))))
.flatMap(metaData -> request.getStatusDataItems().map(statusDataItems -> statusProcessor.sendStatus(metaData.getPath(), statusDataItems, responder, session, mailboxSession)).orElse(Mono.empty()))
.then();
}
}
+ private List<Pair<MailboxPath, ListResponse>> getListResponseForSelectSubscribed(Map<MailboxPath, MailboxMetaData> searchedResultMap, List<MailboxPath> allSubscribedSearch,
+ ListRequest listRequest, MailboxSession mailboxSession, boolean relative, MailboxQuery mailboxQuery) {
+
+ ImmutableList.Builder<Pair<MailboxPath, ListResponse>> responseBuilders = ImmutableList.builder();
+ List<MailboxPath> allSubscribedSearchRemain = new ArrayList<>(allSubscribedSearch);
+
+ if (listRequest.getSelectOptions().contains(RECURSIVEMATCH)) {
+ var allSubscribedSearchParent = allSubscribedSearch.stream()
Review Comment:
Please avoid `var`
--
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 diff in pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1034241980
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -127,22 +155,109 @@ private String computeReferenceRoot(String referenceName, MailboxSession mailbox
}
}
- private Mono<Void> respondMailboxList(String referenceName, String mailboxName, ImapSession session, Responder responder, MailboxSession mailboxSession) {
+ private Mono<Void> respondMailboxList(T request, ImapSession session,
+ Responder responder, MailboxSession mailboxSession) {
+ if (request.selectRemote()) {
+ // https://www.rfc-editor.org/rfc/rfc5258.html. NOT YET SUPPORT `REMOTE`
+ return Mono.empty();
+ }
+
// If the mailboxPattern is fully qualified, ignore the
// reference name.
- String finalReferencename = referenceName;
- if (mailboxName.charAt(0) == MailboxConstants.NAMESPACE_PREFIX_CHAR) {
+ String finalReferencename = request.getBaseReferenceName();
+ if (request.getMailboxPattern().charAt(0) == MailboxConstants.NAMESPACE_PREFIX_CHAR) {
finalReferencename = "";
}
// Is the interpreted (combined) pattern relative?
// Should the namespace section be returned or not?
- boolean isRelative = ((finalReferencename + mailboxName).charAt(0) != MailboxConstants.NAMESPACE_PREFIX_CHAR);
+ boolean isRelative = ((finalReferencename + request.getMailboxPattern()).charAt(0) != MailboxConstants.NAMESPACE_PREFIX_CHAR);
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
- return getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath())))
- .then();
+ if (request.selectSubscribed()) {
+ MailboxQuery mailboxQuery = mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession);
+
+ return Mono.zip(getMailboxManager().search(mailboxQuery, Minimal, mailboxSession).collectList()
+ .map(searchedResultList -> searchedResultList.stream().collect(Collectors.toMap(MailboxMetaData::getPath, Function.identity()))),
+ Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get()).collectList())
+ .map(tuple -> getListResponseForSelectSubscribed(tuple.getT1(), tuple.getT2(), request, mailboxSession, isRelative, mailboxQuery))
+ .flatMapIterable(list -> list)
+ .doOnNext(pathAndResponse -> responder.respond(pathAndResponse.getRight()))
+ .then();
+
+ } else {
+ return getMailboxManager().search(mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession), Minimal, mailboxSession)
+ .doOnNext(metaData -> responder.respond(
+ createResponse(metaData.inferiors(),
+ metaData.getSelectability(),
+ mailboxName(isRelative, metaData.getPath(), metaData.getHierarchyDelimiter()),
+ metaData.getHierarchyDelimiter(),
+ getMailboxType(session, metaData.getPath()))))
+ .then();
+ }
+ }
+
+ private List<Pair<MailboxPath, ListResponse>> getListResponseForSelectSubscribed(Map<MailboxPath, MailboxMetaData> searchedResultMap, List<MailboxPath> allSubscribedSearch,
+ ListRequest listRequest, MailboxSession mailboxSession, boolean relative, MailboxQuery mailboxQuery) {
+ ImmutableList.Builder<Pair<MailboxPath, ListResponse>> responseBuilders = ImmutableList.builder();
+ List<Pair<MailboxPath, ListResponse>> listRecursiveMatch = listRecursiveMatch(searchedResultMap, allSubscribedSearch, mailboxSession, relative, listRequest);
+ responseBuilders.addAll(listRecursiveMatch);
+ Set<MailboxPath> listRecursiveMatchPath = listRecursiveMatch.stream().map(Pair::getKey).collect(Collectors.toUnmodifiableSet());
+
+ allSubscribedSearch.stream()
+ .filter(subscribed -> !listRecursiveMatchPath.contains(subscribed))
+ .filter(mailboxQuery::isPathMatch)
+ .forEach(subscribed -> {
+ ListResponse.Builder builder = ListResponse.builder()
+ .returnSubscribed(RETURN_SUBSCRIBED);
+ if (searchedResultMap.containsKey(subscribed)) {
+ MailboxMetaData metaData = searchedResultMap.get(subscribed);
+
+ builder.name(mailboxName(relative, subscribed, metaData.getHierarchyDelimiter()))
+ .children(metaData.inferiors())
+ .selectability(metaData.getSelectability())
+ .hierarchyDelimiter(metaData.getHierarchyDelimiter())
+ .returnNonExistent(!RETURN_NON_EXISTENT);
+ } else {
+ builder.name(subscribed.getName())
+ .children(MailboxMetaData.Children.CHILDREN_ALLOWED_BUT_UNKNOWN)
+ .selectability(MailboxMetaData.Selectability.NONE)
+ .hierarchyDelimiter(mailboxSession.getPathDelimiter())
+ .returnNonExistent(RETURN_NON_EXISTENT);
+ }
+ responseBuilders.add(Pair.of(subscribed, builder.build()));
+ });
Review Comment:
Extact the content of the foreach in a dedicated method?
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1030989992
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -140,9 +153,18 @@ private Mono<Void> respondMailboxList(String referenceName, String mailboxName,
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
- return getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath())))
- .then();
+ if (selectSubscribed) {
+ return Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get())
+ .collectList()
+ .flatMapMany(litSubscribed -> getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
+ .filter(metaData -> litSubscribed.contains(metaData.getPath()))
+ .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath()), true)))
+ .then();
+ } else {
+ return getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
+ .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath()), false))
Review Comment:
What does `false` mean?
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -140,9 +153,18 @@ private Mono<Void> respondMailboxList(String referenceName, String mailboxName,
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
- return getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath())))
- .then();
+ if (selectSubscribed) {
+ return Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get())
+ .collectList()
+ .flatMapMany(litSubscribed -> getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
+ .filter(metaData -> litSubscribed.contains(metaData.getPath()))
+ .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath()), true)))
Review Comment:
What does `"true"` means?
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/XListProcessor.java:
##########
@@ -48,7 +48,7 @@ public class XListProcessor extends ListProcessor<XListRequest> implements Capab
public XListProcessor(MailboxManager mailboxManager, StatusResponseFactory factory, MailboxTyper mailboxTyper,
MetricFactory metricFactory) {
- super(XListRequest.class, mailboxManager, factory, metricFactory);
+ super(XListRequest.class, mailboxManager, factory, metricFactory, null);
Review Comment:
What is `null` ?
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -140,9 +153,18 @@ private Mono<Void> respondMailboxList(String referenceName, String mailboxName,
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
- return getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath())))
- .then();
+ if (selectSubscribed) {
+ return Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get())
+ .collectList()
+ .flatMapMany(litSubscribed -> getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
+ .filter(metaData -> litSubscribed.contains(metaData.getPath()))
+ .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath()), true)))
+ .then();
Review Comment:
Are we supposed to also return subscribed items that do not correspond to an existing mailbox?
CF the RFC:
```
SUBSCRIBED - causes the LIST command to list subscribed names,
rather than the existing mailboxes. This will often be a subset
of the actual mailboxes. It's also possible for this list to
contain the names of mailboxes that don't exist. In any case, the
list MUST include exactly those mailbox names that match the
canonical list pattern and are subscribed to. This option is
intended to supplement the LSUB command.
```
--
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 diff in pull request #1329: [WIP] JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
vttranlina commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1031012176
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/XListProcessor.java:
##########
@@ -48,7 +48,7 @@ public class XListProcessor extends ListProcessor<XListRequest> implements Capab
public XListProcessor(MailboxManager mailboxManager, StatusResponseFactory factory, MailboxTyper mailboxTyper,
MetricFactory metricFactory) {
- super(XListRequest.class, mailboxManager, factory, metricFactory);
+ super(XListRequest.class, mailboxManager, factory, metricFactory, null);
Review Comment:
`XListProcessor` do not use SubscriptionManager, so can we set `null` here?
--
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 diff in pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1033235618
##########
protocols/imap/src/main/java/org/apache/james/imap/message/request/ListRequest.java:
##########
@@ -86,6 +85,10 @@ public final boolean selectSubscribed() {
return getSelectOptions().contains(ListSelectOption.SUBSCRIBED);
}
+ public final boolean selectRemote() {
Review Comment:
final is not needed here
--
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 diff in pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1034378772
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -207,30 +213,33 @@ private List<Pair<MailboxPath, ListResponse>> getListResponseForSelectSubscribed
allSubscribedSearch.stream()
.filter(subscribed -> !listRecursiveMatchPath.contains(subscribed))
.filter(mailboxQuery::isPathMatch)
- .forEach(subscribed -> {
- ListResponse.Builder builder = ListResponse.builder()
- .returnSubscribed(RETURN_SUBSCRIBED);
- if (searchedResultMap.containsKey(subscribed)) {
- MailboxMetaData metaData = searchedResultMap.get(subscribed);
-
- builder.name(mailboxName(relative, subscribed, metaData.getHierarchyDelimiter()))
- .children(metaData.inferiors())
- .selectability(metaData.getSelectability())
- .hierarchyDelimiter(metaData.getHierarchyDelimiter())
- .returnNonExistent(!RETURN_NON_EXISTENT);
- } else {
- builder.name(subscribed.getName())
- .children(MailboxMetaData.Children.CHILDREN_ALLOWED_BUT_UNKNOWN)
- .selectability(MailboxMetaData.Selectability.NONE)
- .hierarchyDelimiter(mailboxSession.getPathDelimiter())
- .returnNonExistent(RETURN_NON_EXISTENT);
- }
- responseBuilders.add(Pair.of(subscribed, builder.build()));
- });
+ .map(subscribed -> buildListResponse(searchedResultMap, mailboxSession, relative, subscribed))
+ .forEach(responseBuilders::add);
return responseBuilders.build();
}
+ private Pair<MailboxPath, ListResponse> buildListResponse(Map<MailboxPath, MailboxMetaData> searchedResultMap, MailboxSession mailboxSession, boolean relative, MailboxPath subscribed) {
+ ListResponse.Builder builder = ListResponse.builder()
+ .returnSubscribed(RETURN_SUBSCRIBED);
+ if (searchedResultMap.containsKey(subscribed)) {
+ MailboxMetaData metaData = searchedResultMap.get(subscribed);
+
+ builder.name(mailboxName(relative, subscribed, metaData.getHierarchyDelimiter()))
+ .children(metaData.inferiors())
+ .selectability(metaData.getSelectability())
+ .hierarchyDelimiter(metaData.getHierarchyDelimiter())
+ .returnNonExistent(!RETURN_NON_EXISTENT);
+ } else {
+ builder.name(subscribed.getName())
+ .children(MailboxMetaData.Children.CHILDREN_ALLOWED_BUT_UNKNOWN)
+ .selectability(MailboxMetaData.Selectability.NONE)
+ .hierarchyDelimiter(mailboxSession.getPathDelimiter())
+ .returnNonExistent(RETURN_NON_EXISTENT);
+ }
+ return Pair.of(subscribed, builder.build());
Review Comment:
We could easily do this with an Optional.
Also duplicating the first line and the last line would allow to avoid mutability.
```
return Pair.of(subscribed,
Optional.ofNullable(searchedResultMap.get(subscribed))
.map(metaData -> ListResponse.builder()
.returnSubscribed(RETURN_SUBSCRIBED)
.name(mailboxName(relative, subscribed, metaData.getHierarchyDelimiter()))
.children(metaData.inferiors())
.selectability(metaData.getSelectability())
.hierarchyDelimiter(metaData.getHierarchyDelimiter())
.returnNonExistent(!RETURN_NON_EXISTENT))
.orElseGet(() -> ListResponse.builder()
.returnSubscribed(RETURN_SUBSCRIBED)
.name(subscribed.getName())
.children(MailboxMetaData.Children.CHILDREN_ALLOWED_BUT_UNKNOWN)
.selectability(MailboxMetaData.Selectability.NONE)
.hierarchyDelimiter(mailboxSession.getPathDelimiter())
.returnNonExistent(RETURN_NON_EXISTENT))
.build());
```
In my bucket list, from there, I would try to look how factory methods looks like:
```
ListResponse.builder()
.nonExitingSubscribedMailbox(subscribed)
```
For instance.
Or...
```
ListResponse.builder()
.forPath(subscribed)
.forMetadata(metaData)
.returnSubscribed(RETURN_SUBSCRIBED)
```
My feeling is also that by putting a bit more convenience methods in the ListResponse.Builder we could kill a lot of boiler-plate and code duplication, and make this more readable.
--
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 diff in pull request #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1329:
URL: https://github.com/apache/james-project/pull/1329#discussion_r1034241729
##########
protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java:
##########
@@ -127,22 +155,109 @@ private String computeReferenceRoot(String referenceName, MailboxSession mailbox
}
}
- private Mono<Void> respondMailboxList(String referenceName, String mailboxName, ImapSession session, Responder responder, MailboxSession mailboxSession) {
+ private Mono<Void> respondMailboxList(T request, ImapSession session,
+ Responder responder, MailboxSession mailboxSession) {
+ if (request.selectRemote()) {
+ // https://www.rfc-editor.org/rfc/rfc5258.html. NOT YET SUPPORT `REMOTE`
+ return Mono.empty();
+ }
+
// If the mailboxPattern is fully qualified, ignore the
// reference name.
- String finalReferencename = referenceName;
- if (mailboxName.charAt(0) == MailboxConstants.NAMESPACE_PREFIX_CHAR) {
+ String finalReferencename = request.getBaseReferenceName();
+ if (request.getMailboxPattern().charAt(0) == MailboxConstants.NAMESPACE_PREFIX_CHAR) {
finalReferencename = "";
}
// Is the interpreted (combined) pattern relative?
// Should the namespace section be returned or not?
- boolean isRelative = ((finalReferencename + mailboxName).charAt(0) != MailboxConstants.NAMESPACE_PREFIX_CHAR);
+ boolean isRelative = ((finalReferencename + request.getMailboxPattern()).charAt(0) != MailboxConstants.NAMESPACE_PREFIX_CHAR);
MailboxPath basePath = computeBasePath(session, finalReferencename, isRelative);
- return getMailboxManager().search(mailboxQuery(basePath, mailboxName, mailboxSession), Minimal, mailboxSession)
- .doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath())))
- .then();
+ if (request.selectSubscribed()) {
+ MailboxQuery mailboxQuery = mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession);
+
+ return Mono.zip(getMailboxManager().search(mailboxQuery, Minimal, mailboxSession).collectList()
+ .map(searchedResultList -> searchedResultList.stream().collect(Collectors.toMap(MailboxMetaData::getPath, Function.identity()))),
+ Flux.from(Throwing.supplier(() -> subscriptionManager.subscriptionsReactive(mailboxSession)).get()).collectList())
+ .map(tuple -> getListResponseForSelectSubscribed(tuple.getT1(), tuple.getT2(), request, mailboxSession, isRelative, mailboxQuery))
+ .flatMapIterable(list -> list)
+ .doOnNext(pathAndResponse -> responder.respond(pathAndResponse.getRight()))
+ .then();
+
+ } else {
+ return getMailboxManager().search(mailboxQuery(basePath, request.getMailboxPattern(), mailboxSession), Minimal, mailboxSession)
+ .doOnNext(metaData -> responder.respond(
+ createResponse(metaData.inferiors(),
+ metaData.getSelectability(),
+ mailboxName(isRelative, metaData.getPath(), metaData.getHierarchyDelimiter()),
+ metaData.getHierarchyDelimiter(),
+ getMailboxType(session, metaData.getPath()))))
+ .then();
+ }
Review Comment:
Extract the two side of this `if` in dedicated methods?
--
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 #1329: JAMES-3754 - IMAP support List command extension RFC-5258
Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1329:
URL: https://github.com/apache/james-project/pull/1329#issuecomment-1333870122
@vttranlina ?
--
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