MAILBOX-355 SelectedMailboxImpl should not track MailboxPath change We should rather read it from the MailboxManager.
Note that SelectedMailbox::getPath now throws. Then error handling can not rely on it. Logging mailboxId is a good solution for this. Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/e7e5ba68 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/e7e5ba68 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/e7e5ba68 Branch: refs/heads/master Commit: e7e5ba68ad459120e9ebb5ea4a8705ac4a705103 Parents: 6fe8891 Author: Benoit Tellier <btell...@linagora.com> Authored: Tue Dec 4 09:58:50 2018 +0700 Committer: Benoit Tellier <btell...@linagora.com> Committed: Wed Dec 5 16:34:25 2018 +0700 ---------------------------------------------------------------------- .../james/imap/api/process/SelectedMailbox.java | 3 ++- .../processor/AbstractMessageRangeProcessor.java | 4 ++-- .../james/imap/processor/CloseProcessor.java | 2 +- .../james/imap/processor/ExpungeProcessor.java | 2 +- .../james/imap/processor/IdleProcessor.java | 4 ++-- .../james/imap/processor/SearchProcessor.java | 5 +++-- .../james/imap/processor/StoreProcessor.java | 4 ++-- .../imap/processor/base/SelectedMailboxImpl.java | 18 +++++++----------- .../imap/processor/fetch/FetchProcessor.java | 4 ++-- .../processor/base/MailboxEventAnalyserTest.java | 6 ++++-- .../processor/base/SelectedMailboxImplTest.java | 4 ++-- .../james/imapserver/netty/IMAPMDCContext.java | 2 +- 12 files changed, 29 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/api/process/SelectedMailbox.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/api/process/SelectedMailbox.java b/protocols/imap/src/main/java/org/apache/james/imap/api/process/SelectedMailbox.java index 4157de8..a0a1425 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/api/process/SelectedMailbox.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/api/process/SelectedMailbox.java @@ -25,6 +25,7 @@ import java.util.Optional; import javax.mail.Flags; import org.apache.james.mailbox.MessageUid; +import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.model.MailboxId; import org.apache.james.mailbox.model.MailboxPath; @@ -89,7 +90,7 @@ public interface SelectedMailbox { * * @return path */ - MailboxPath getPath(); + MailboxPath getPath() throws MailboxException; /** * Return the mailboxId of the selected Mailbox. http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractMessageRangeProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractMessageRangeProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractMessageRangeProcessor.java index d76c76e..b3ec4d7 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractMessageRangeProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractMessageRangeProcessor.java @@ -107,11 +107,11 @@ public abstract class AbstractMessageRangeProcessor<M extends AbstractMessageRan } } catch (MessageRangeException e) { LOGGER.debug("{} failed from mailbox {} to {} for invalid sequence-set {}", - getOperationName(), currentMailbox.getPath(), targetMailbox, idSet, e); + getOperationName(), currentMailbox.getMailboxId(), targetMailbox, idSet, e); taggedBad(command, tag, responder, HumanReadableText.INVALID_MESSAGESET); } catch (MailboxException e) { LOGGER.error("{} failed from mailbox {} to {} for sequence-set {}", - getOperationName(), currentMailbox.getPath(), targetMailbox, idSet, e); + getOperationName(), currentMailbox.getMailboxId(), targetMailbox, idSet, e); no(command, tag, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/CloseProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/CloseProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/CloseProcessor.java index 2bb518c..19fa829 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/CloseProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/CloseProcessor.java @@ -64,7 +64,7 @@ public class CloseProcessor extends AbstractMailboxProcessor<CloseRequest> { } } catch (MailboxException e) { - LOGGER.error("Close failed for mailbox {}", session.getSelected().getPath(), e); + LOGGER.error("Close failed for mailbox {}", session.getSelected().getMailboxId(), e); no(command, tag, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/ExpungeProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/ExpungeProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/ExpungeProcessor.java index aa4d577..a0ab2fa 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/ExpungeProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/ExpungeProcessor.java @@ -103,7 +103,7 @@ public class ExpungeProcessor extends AbstractMailboxProcessor<ExpungeRequest> i LOGGER.debug("Expunge failed", e); taggedBad(command, tag, responder, HumanReadableText.INVALID_MESSAGESET); } catch (MailboxException e) { - LOGGER.error("Expunge failed for mailbox {}", session.getSelected().getPath(), e); + LOGGER.error("Expunge failed for mailbox {}", session.getSelected().getMailboxId(), e); no(command, tag, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/IdleProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/IdleProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/IdleProcessor.java index 684c9e9..0018458 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/IdleProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/IdleProcessor.java @@ -114,7 +114,7 @@ public class IdleProcessor extends AbstractMailboxProcessor<IdleRequest> impleme try { mailboxManager.removeListener(sm.getMailboxId(), idleListener, mailboxSession); } catch (MailboxException e) { - LOGGER.error("Unable to remove idle listener for mailbox {}", sm.getPath(), e); + LOGGER.error("Unable to remove idle listener for mailbox {}", sm.getMailboxId(), e); } } session.popLineHandler(); @@ -163,7 +163,7 @@ public class IdleProcessor extends AbstractMailboxProcessor<IdleRequest> impleme } catch (MailboxException e) { - LOGGER.error("Enable idle for {} failed", session.getSelected().getPath(), e); + LOGGER.error("Enable idle for {} failed", session.getSelected().getMailboxId(), e); no(command, tag, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/SearchProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/SearchProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/SearchProcessor.java index c7e9301..f074eec 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/SearchProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/SearchProcessor.java @@ -27,6 +27,7 @@ import java.util.Iterator; import java.util.List; import java.util.Optional; import java.util.TreeSet; + import javax.mail.Flags.Flag; import org.apache.james.imap.api.ImapCommand; @@ -203,10 +204,10 @@ public class SearchProcessor extends AbstractMailboxProcessor<SearchRequest> imp unsolicitedResponses(session, responder, omitExpunged, useUids); okComplete(command, tag, responder); } catch (MessageRangeException e) { - LOGGER.debug("Search failed in mailbox {} because of an invalid sequence-set ", session.getSelected().getPath(), e); + LOGGER.debug("Search failed in mailbox {} because of an invalid sequence-set ", session.getSelected().getMailboxId(), e); taggedBad(command, tag, responder, HumanReadableText.INVALID_MESSAGESET); } catch (MailboxException e) { - LOGGER.error("Search failed in mailbox {}", session.getSelected().getPath(), e); + LOGGER.error("Search failed in mailbox {}", session.getSelected().getMailboxId(), e); no(command, tag, responder, HumanReadableText.SEARCH_FAILED); if (resultOptions.contains(SearchResultOption.SAVE)) { http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/StoreProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/StoreProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/StoreProcessor.java index c15fa1f..27e86d5 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/StoreProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/StoreProcessor.java @@ -209,10 +209,10 @@ public class StoreProcessor extends AbstractMailboxProcessor<StoreRequest> { } } } catch (MessageRangeException e) { - LOGGER.debug("Store failed for mailbox {} because of an invalid sequence-set {}", session.getSelected().getPath(), idSet, e); + LOGGER.debug("Store failed for mailbox {} because of an invalid sequence-set {}", session.getSelected().getMailboxId(), idSet, e); taggedBad(imapCommand, tag, responder, HumanReadableText.INVALID_MESSAGESET); } catch (MailboxException e) { - LOGGER.error("Store failed for mailbox {} and sequence-set {}", session.getSelected().getPath(), idSet, e); + LOGGER.error("Store failed for mailbox {} and sequence-set {}", session.getSelected().getMailboxId(), idSet, e); no(imapCommand, tag, responder, HumanReadableText.SAVE_FAILED); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java index f0eb68e..d2aabf1 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/SelectedMailboxImpl.java @@ -63,7 +63,6 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener { private final MailboxManager mailboxManager; private final MailboxId mailboxId; - private MailboxPath path; private final ImapSession session; @@ -79,6 +78,7 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener { private final Flags applicableFlags; private boolean applicableFlagsChanged; + private final MailboxSession mailboxSession; public SelectedMailboxImpl(MailboxManager mailboxManager, ImapSession session, MailboxPath path) throws MailboxException { this.session = session; @@ -87,9 +87,8 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener { // Ignore events from our session setSilentFlagChanges(true); - this.path = path; - MailboxSession mailboxSession = ImapSessionUtils.getMailboxSession(session); + mailboxSession = ImapSessionUtils.getMailboxSession(session); uidMsnConverter = new UidMsnConverter(); @@ -162,8 +161,8 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener { } @Override - public synchronized MailboxPath getPath() { - return path; + public MailboxPath getPath() throws MailboxException { + return mailboxManager.getMailbox(mailboxId, mailboxSession).getMailboxPath(); } @Override @@ -329,7 +328,7 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener { private void mailboxEvent(MailboxEvent mailboxEvent) { // Check if the event was for the mailbox we are observing - if (mailboxEvent.getMailboxPath().equals(getPath())) { + if (mailboxEvent.getMailboxId().equals(getMailboxId())) { final long eventSessionId = mailboxEvent.getSession().getSessionId(); if (mailboxEvent instanceof MessageEvent) { final MessageEvent messageEvent = (MessageEvent) mailboxEvent; @@ -367,8 +366,8 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener { while (flags.hasNext()) { if (Flag.RECENT.equals(flags.next())) { - MailboxPath path = sm.getPath(); - if (path != null && path.equals(mailboxEvent.getMailboxPath())) { + MailboxId id = sm.getMailboxId(); + if (id != null && id.equals(mailboxEvent.getMailboxId())) { sm.addRecent(u.getUid()); } } @@ -404,9 +403,6 @@ public class SelectedMailboxImpl implements SelectedMailbox, MailboxListener { if (eventSessionId != sessionId) { isDeletedByOtherSession = true; } - } else if (mailboxEvent instanceof MailboxRenamed) { - final MailboxRenamed mailboxRenamed = (MailboxRenamed) mailboxEvent; - path = mailboxRenamed.getNewPath(); } } } http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java index 611f2f1..0a32dc3 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/fetch/FetchProcessor.java @@ -125,10 +125,10 @@ public class FetchProcessor extends AbstractMailboxProcessor<FetchRequest> { unsolicitedResponses(session, responder, omitExpunged, useUids); okComplete(command, tag, responder); } catch (MessageRangeException e) { - LOGGER.debug("Fetch failed for mailbox {} because of invalid sequence-set {}", session.getSelected().getPath(), idSet, e); + LOGGER.debug("Fetch failed for mailbox {} because of invalid sequence-set {}", session.getSelected().getMailboxId(), idSet, e); taggedBad(command, tag, responder, HumanReadableText.INVALID_MESSAGESET); } catch (MailboxException e) { - LOGGER.error("Fetch failed for mailbox {} and sequence-set {}", session.getSelected().getPath(), idSet, e); + LOGGER.error("Fetch failed for mailbox {} and sequence-set {}", session.getSelected().getMailboxId(), idSet, e); no(command, tag, responder, HumanReadableText.SEARCH_FAILED); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/test/java/org/apache/james/imap/processor/base/MailboxEventAnalyserTest.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/MailboxEventAnalyserTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/MailboxEventAnalyserTest.java index 48c6fd8..5cd3449 100644 --- a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/MailboxEventAnalyserTest.java +++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/MailboxEventAnalyserTest.java @@ -105,13 +105,15 @@ public class MailboxEventAnalyserTest { .thenReturn(messageManager); when(mailboxManager.getMailbox(any(MailboxPath.class), any(MailboxSession.class))) .thenReturn(messageManager); + when(mailboxManager.getMailbox(any(MailboxId.class), any(MailboxSession.class))) + .thenReturn(messageManager); MessageResult messageResult = mock(MessageResult.class); when(messageResult.getMailboxId()).thenReturn(MAILBOX_ID); when(messageResult.getUid()).thenReturn(MESSAGE_UID); - when(messageManager.getApplicableFlags(any())) - .thenReturn(new Flags()); + when(messageManager.getApplicableFlags(any())).thenReturn(new Flags()); + when(messageManager.getId()).thenReturn(MAILBOX_ID); when(messageManager.search(any(), any())) .thenReturn(ImmutableList.of(MESSAGE_UID).iterator()); when(messageManager.getMessages(any(), any(), any())) http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java index 12daae0..e923b2a 100644 --- a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java +++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/SelectedMailboxImplTest.java @@ -97,8 +97,8 @@ public class SelectedMailboxImplTest { when(imapSession.getAttribute(ImapSessionUtils.MAILBOX_SESSION_ATTRIBUTE_SESSION_KEY)).thenReturn(mock(MailboxSession.class)); - when(mailbox.generateAssociatedPath()) - .thenReturn(mailboxPath); + when(mailbox.generateAssociatedPath()).thenReturn(mailboxPath); + when(mailbox.getMailboxId()).thenReturn(mailboxId); } @After http://git-wip-us.apache.org/repos/asf/james-project/blob/e7e5ba68/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java ---------------------------------------------------------------------- diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java index 35ccf32..a0d5710 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPMDCContext.java @@ -72,7 +72,7 @@ public class IMAPMDCContext { private static MDCBuilder from(Optional<SelectedMailbox> selectedMailbox) { return selectedMailbox .map(value -> MDCBuilder.create() - .addContext("selectedMailbox", value.getPath().asString())) + .addContext("selectedMailbox", value.getMailboxId().serialize())) .orElse(MDCBuilder.create()); } } --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
