You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2020/10/12 07:40:10 UTC
[james-project] 03/12: JAMES-3408 Allow searching mailboxes without
reading counters
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 1491cc7c5bfb1bd3741de0ed683107d2b2011cc1
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Oct 9 09:50:03 2020 +0700
JAMES-3408 Allow searching mailboxes without reading counters
I first tried an alternative: adding an API letting retrieve the counters from a mailbox.
However for safety we want to enforce the right access on the mailbox before disclosing its
counters. But for efficiency, we should not refetch this information.
A pattern would then be "protocol layer first retrieves the mailbox" then "using that mailbox requests
the counters". This would be unsafe as the mailbox object could be crafted in the protocol layers to
cheat the right resolution system.
I came to the conclusion we need two "search mailboxes" methods, one that fetch counters and one that don't.
The smoother way to do that is to follow the already classic FetchType pattern in the StoreMailboxManager.
IMAP LIST and many other methods do not need the mailbox counters. Reading them is however a major performance hit,
and accounts for most of Cassandra request time upon IMAP LIST.
This is even more important if (potentially expenssive) read repairs
are performed upon counter reads.
---
.../org/apache/james/mailbox/MailboxManager.java | 11 ++++++-
.../james/mailbox/backup/DefaultMailboxBackup.java | 4 ++-
.../james/mailbox/store/StoreMailboxManager.java | 37 ++++++++++++++++++----
.../mailbox/store/SystemMailboxesProviderImpl.java | 4 ++-
.../store/SystemMailboxesProviderImplTest.java | 2 +-
.../apache/james/imap/processor/ListProcessor.java | 4 ++-
.../org/apache/james/modules/MailboxProbeImpl.java | 2 ++
.../adapter/mailbox/MailboxManagerManagement.java | 3 ++
.../james/transport/mailets/RandomStoring.java | 4 ++-
.../jmap/MessageFastViewProjectionCorrector.java | 4 ++-
.../webadmin/service/UserMailboxesService.java | 4 ++-
.../webadmin/routes/UserMailboxesRoutesTest.java | 4 +--
12 files changed, 67 insertions(+), 16 deletions(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
index 1b235bd..01740b8 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/MailboxManager.java
@@ -289,6 +289,11 @@ public interface MailboxManager extends RequestAware, RightManager, MailboxAnnot
*/
List<MessageRange> moveMessages(MessageRange set, MailboxPath from, MailboxPath to, MailboxSession session) throws MailboxException;
+ enum MailboxSearchFetchType {
+ Minimal,
+ Counters
+ }
+
/**
* Searches for mailboxes matching the given query.
*
@@ -297,7 +302,11 @@ public interface MailboxManager extends RequestAware, RightManager, MailboxAnnot
* @param session
* the context for this call, not null
*/
- Flux<MailboxMetaData> search(MailboxQuery expression, MailboxSession session);
+ default Flux<MailboxMetaData> search(MailboxQuery expression, MailboxSession session) {
+ return search(expression, MailboxSearchFetchType.Counters, session);
+ }
+
+ Flux<MailboxMetaData> search(MailboxQuery expression, MailboxSearchFetchType fetchType, MailboxSession session);
/**
* Searches for messages matching the given query.
diff --git a/mailbox/backup/src/main/java/org/apache/james/mailbox/backup/DefaultMailboxBackup.java b/mailbox/backup/src/main/java/org/apache/james/mailbox/backup/DefaultMailboxBackup.java
index 9f19a89..a72b8b7 100644
--- a/mailbox/backup/src/main/java/org/apache/james/mailbox/backup/DefaultMailboxBackup.java
+++ b/mailbox/backup/src/main/java/org/apache/james/mailbox/backup/DefaultMailboxBackup.java
@@ -18,6 +18,8 @@
****************************************************************/
package org.apache.james.mailbox.backup;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
+
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -145,7 +147,7 @@ public class DefaultMailboxBackup implements MailboxBackup {
.privateNamespace()
.user(session.getUser())
.build();
- Stream<MailboxPath> paths = mailboxManager.search(queryUser, session)
+ Stream<MailboxPath> paths = mailboxManager.search(queryUser, Minimal, session)
.toStream()
.map(MailboxMetaData::getPath);
List<MailAccountContent> mailboxes = paths
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
index e90651d..2ca6eb0 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
@@ -29,6 +29,7 @@ import java.util.EnumSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Function;
import java.util.stream.Stream;
import javax.inject.Inject;
@@ -622,20 +623,44 @@ public class StoreMailboxManager implements MailboxManager {
}
@Override
- public Flux<MailboxMetaData> search(MailboxQuery expression, MailboxSession session) {
+ public Flux<MailboxMetaData> search(MailboxQuery expression, MailboxSearchFetchType fetchType, MailboxSession session) {
Mono<List<Mailbox>> mailboxesMono = searchMailboxes(expression, session, Right.Lookup).collectList();
- MessageMapper messageMapper = mailboxSessionMapperFactory.getMessageMapper(session);
return mailboxesMono
.flatMapMany(mailboxes -> Flux.fromIterable(mailboxes)
.filter(expression::matches)
- .flatMap(mailbox -> retrieveCounters(messageMapper, mailbox, session)
- .map(Throwing.<MailboxCounters, MailboxMetaData>function(
- counters -> toMailboxMetadata(session, mailboxes, mailbox, counters))
- .sneakyThrow())))
+ .transform(metadataTransformation(fetchType, session, mailboxes)))
.sort(MailboxMetaData.COMPARATOR);
}
+ private Function<Flux<Mailbox>, Flux<MailboxMetaData>> metadataTransformation(MailboxSearchFetchType fetchType, MailboxSession session, List<Mailbox> mailboxes) {
+ if (fetchType == MailboxSearchFetchType.Counters) {
+ return withCounters(session, mailboxes);
+ }
+ return withoutCounters(session, mailboxes);
+ }
+
+ private Function<Flux<Mailbox>, Flux<MailboxMetaData>> withCounters(MailboxSession session, List<Mailbox> mailboxes) {
+ MessageMapper messageMapper = mailboxSessionMapperFactory.getMessageMapper(session);
+ return mailboxFlux -> mailboxFlux
+ .flatMap(mailbox -> retrieveCounters(messageMapper, mailbox, session)
+ .map(Throwing.<MailboxCounters, MailboxMetaData>function(
+ counters -> toMailboxMetadata(session, mailboxes, mailbox, counters))
+ .sneakyThrow()));
+ }
+
+ private Function<Flux<Mailbox>, Flux<MailboxMetaData>> withoutCounters(MailboxSession session, List<Mailbox> mailboxes) {
+ return mailboxFlux -> mailboxFlux
+ .map(Throwing.<Mailbox, MailboxMetaData>function(
+ mailbox -> toMailboxMetadata(session, mailboxes, mailbox, MailboxCounters
+ .builder()
+ .mailboxId(mailbox.getMailboxId())
+ .count(0)
+ .unseen(0)
+ .build()))
+ .sneakyThrow());
+ }
+
private Mono<MailboxCounters> retrieveCounters(MessageMapper messageMapper, Mailbox mailbox, MailboxSession session) {
return messageMapper.getMailboxCountersReactive(mailbox)
.filter(Throwing.<MailboxCounters>predicate(counter -> storeRightManager.hasRight(mailbox, Right.Read, session)).sneakyThrow())
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/SystemMailboxesProviderImpl.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/SystemMailboxesProviderImpl.java
index e8a6eed..8b81a4b 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/SystemMailboxesProviderImpl.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/SystemMailboxesProviderImpl.java
@@ -19,6 +19,8 @@
package org.apache.james.mailbox.store;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
+
import javax.inject.Inject;
import org.apache.james.core.Username;
@@ -73,7 +75,7 @@ public class SystemMailboxesProviderImpl implements SystemMailboxesProvider {
MailboxQuery mailboxQuery = MailboxQuery.privateMailboxesBuilder(session)
.expression(new PrefixedWildcard(aRole.getDefaultMailbox()))
.build();
- return mailboxManager.search(mailboxQuery, session)
+ return mailboxManager.search(mailboxQuery, Minimal, session)
.map(MailboxMetaData::getPath)
.filter(path -> hasRole(aRole, path))
.map(Throwing.function(loadMailbox).sneakyThrow());
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/SystemMailboxesProviderImplTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/SystemMailboxesProviderImplTest.java
index 5bdbf56..35b7870 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/SystemMailboxesProviderImplTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/SystemMailboxesProviderImplTest.java
@@ -58,7 +58,7 @@ class SystemMailboxesProviderImplTest {
void getMailboxByRoleShouldReturnEmptyWhenNoMailbox() throws Exception {
when(mailboxManager.createSystemSession(MailboxFixture.ALICE)).thenReturn(mailboxSession);
when(mailboxManager.getMailbox(eq(MailboxFixture.INBOX_ALICE), eq(mailboxSession))).thenThrow(MailboxNotFoundException.class);
- when(mailboxManager.search(any(), any())).thenReturn(Flux.empty());
+ when(mailboxManager.search(any(), any(), any())).thenReturn(Flux.empty());
assertThat(Flux.from(systemMailboxProvider.getMailboxByRole(Role.INBOX, mailboxSession.getUser())).toStream())
.isEmpty();
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java
index b489a68..3155fa2 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/ListProcessor.java
@@ -19,6 +19,8 @@
package org.apache.james.imap.processor;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
+
import java.io.Closeable;
import org.apache.james.imap.api.ImapMessage;
@@ -137,7 +139,7 @@ public class ListProcessor extends AbstractMailboxProcessor<ListRequest> {
basePath.getName(),
ModifiedUtf7.decodeModifiedUTF7(mailboxName),
mailboxSession.getPathDelimiter()))
- .build(), mailboxSession)
+ .build(), Minimal, mailboxSession)
.doOnNext(metaData -> processResult(responder, isRelative, metaData, getMailboxType(session, metaData.getPath())))
.then()
.block();
diff --git a/server/container/guice/mailbox/src/main/java/org/apache/james/modules/MailboxProbeImpl.java b/server/container/guice/mailbox/src/main/java/org/apache/james/modules/MailboxProbeImpl.java
index 1f5a540..4910ab1 100644
--- a/server/container/guice/mailbox/src/main/java/org/apache/james/modules/MailboxProbeImpl.java
+++ b/server/container/guice/mailbox/src/main/java/org/apache/james/modules/MailboxProbeImpl.java
@@ -19,6 +19,7 @@
package org.apache.james.modules;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
import static org.apache.james.mailbox.store.MailboxReactorUtils.block;
import java.io.InputStream;
@@ -121,6 +122,7 @@ public class MailboxProbeImpl implements GuiceProbe, MailboxProbe {
MailboxQuery.privateMailboxesBuilder(session)
.expression(Wildcard.INSTANCE)
.build(),
+ Minimal,
session);
}
diff --git a/server/container/mailbox-jmx/src/main/java/org/apache/james/adapter/mailbox/MailboxManagerManagement.java b/server/container/mailbox-jmx/src/main/java/org/apache/james/adapter/mailbox/MailboxManagerManagement.java
index a482d5f..66c830d 100644
--- a/server/container/mailbox-jmx/src/main/java/org/apache/james/adapter/mailbox/MailboxManagerManagement.java
+++ b/server/container/mailbox-jmx/src/main/java/org/apache/james/adapter/mailbox/MailboxManagerManagement.java
@@ -18,6 +18,8 @@
****************************************************************/
package org.apache.james.adapter.mailbox;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
+
import java.io.Closeable;
import java.io.FileInputStream;
import java.io.IOException;
@@ -205,6 +207,7 @@ public class MailboxManagerManagement extends StandardMBean implements MailboxMa
MailboxQuery.privateMailboxesBuilder(session)
.matchesAllMailboxNames()
.build(),
+ Minimal,
session)
.collect(Guavate.toImmutableList())
.block();
diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RandomStoring.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RandomStoring.java
index 683e853..20ef0aa 100644
--- a/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RandomStoring.java
+++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/mailets/RandomStoring.java
@@ -19,6 +19,8 @@
package org.apache.james.transport.mailets;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
+
import java.time.Duration;
import java.util.Collection;
import java.util.List;
@@ -116,7 +118,7 @@ public class RandomStoring extends GenericMailet {
try {
MailboxSession session = mailboxManager.createSystemSession(username);
return mailboxManager
- .search(MailboxQuery.privateMailboxesBuilder(session).build(), session)
+ .search(MailboxQuery.privateMailboxesBuilder(session).build(), Minimal, session)
.toStream()
.map(metaData -> new ReroutingInfos(metaData.getPath().getName(), username));
} catch (Exception e) {
diff --git a/server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/MessageFastViewProjectionCorrector.java b/server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/MessageFastViewProjectionCorrector.java
index 7d37d04..612c6ca 100644
--- a/server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/MessageFastViewProjectionCorrector.java
+++ b/server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/MessageFastViewProjectionCorrector.java
@@ -19,6 +19,8 @@
package org.apache.james.webadmin.data.jmap;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
+
import java.io.IOException;
import java.time.Duration;
import java.util.concurrent.atomic.AtomicLong;
@@ -225,7 +227,7 @@ public class MessageFastViewProjectionCorrector {
}
private Flux<MailboxMetaData> listUsersMailboxes(MailboxSession session) {
- return mailboxManager.search(MailboxQuery.privateMailboxesBuilder(session).build(), session);
+ return mailboxManager.search(MailboxQuery.privateMailboxesBuilder(session).build(), Minimal, session);
}
private Mono<MessageManager> retrieveMailbox(MailboxSession session, MailboxMetaData mailboxMetadata) {
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
index 6938f5e..aa64e2e 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/UserMailboxesService.java
@@ -19,6 +19,8 @@
package org.apache.james.webadmin.service;
+import static org.apache.james.mailbox.MailboxManager.MailboxSearchFetchType.Minimal;
+
import java.util.List;
import java.util.stream.Stream;
@@ -127,7 +129,7 @@ public class UserMailboxesService {
private Stream<MailboxMetaData> listUserMailboxes(MailboxSession mailboxSession) throws MailboxException {
return mailboxManager.search(
MailboxQuery.privateMailboxesBuilder(mailboxSession).build(),
- mailboxSession)
+ Minimal, mailboxSession)
.toStream();
}
diff --git a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
index ffb3705..03daea8 100644
--- a/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
+++ b/server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/UserMailboxesRoutesTest.java
@@ -952,7 +952,7 @@ class UserMailboxesRoutesTest {
MailboxMetaData metaData = mock(MailboxMetaData.class);
when(metaData.getPath()).thenReturn(MailboxPath.forUser(USERNAME, MAILBOX_NAME));
doReturn(Flux.just(metaData))
- .when(mailboxManager).search(any(MailboxQuery.class), any(MailboxSession.class));
+ .when(mailboxManager).search(any(MailboxQuery.class), any(), any(MailboxSession.class));
doThrow(new MailboxNotFoundException(MAILBOX_NAME)).when(mailboxManager).deleteMailbox(any(MailboxPath.class), any());
when()
@@ -1000,7 +1000,7 @@ class UserMailboxesRoutesTest {
@Test
void deleteShouldReturnOkOnMailboxNotFoundExceptionWhenRemovingMailboxes() throws Exception {
MailboxId mailboxId = InMemoryId.of(12);
- when(mailboxManager.search(any(MailboxQuery.class), any()))
+ when(mailboxManager.search(any(MailboxQuery.class), any(), any()))
.thenReturn(
Flux.just(MailboxMetaData.unselectableMailbox(MailboxPath.forUser(USERNAME, "any"), mailboxId, '.')));
doThrow(new MailboxNotFoundException("any")).when(mailboxManager).deleteMailbox(any(MailboxPath.class), any());
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org