PROTOCOLS-117 General fixes: LSUB should not be used for hierarchy delimiter finding
RFC-3501 Have no mentions about it. Most probably code was originally copy and pasted from LIST. We should also take this occasion to refactor LSUB a bit Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/4e74c7c1 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/4e74c7c1 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/4e74c7c1 Branch: refs/heads/master Commit: 4e74c7c1bd07d398c58aa799386ba2101665e6a2 Parents: 8ffa76d Author: benwa <[email protected]> Authored: Wed Nov 1 14:57:09 2017 +0700 Committer: benwa <[email protected]> Committed: Fri Nov 3 11:11:44 2017 +0700 ---------------------------------------------------------------------- .../james/imap/processor/LSubProcessor.java | 105 ++++++------------- .../james/imap/processor/LSubProcessorTest.java | 20 ---- 2 files changed, 34 insertions(+), 91 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/4e74c7c1/protocols/imap/src/main/java/org/apache/james/imap/processor/LSubProcessor.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/LSubProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/LSubProcessor.java index 62b7d29..f3a2dd8 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/LSubProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/LSubProcessor.java @@ -30,7 +30,6 @@ import org.apache.james.imap.api.display.HumanReadableText; import org.apache.james.imap.api.message.response.StatusResponseFactory; import org.apache.james.imap.api.process.ImapProcessor; import org.apache.james.imap.api.process.ImapSession; -import org.apache.james.imap.main.PathConverter; import org.apache.james.imap.message.request.LsubRequest; import org.apache.james.imap.message.response.LSubResponse; import org.apache.james.mailbox.MailboxManager; @@ -38,9 +37,7 @@ import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.SubscriptionManager; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.exception.SubscriptionException; -import org.apache.james.mailbox.model.MailboxConstants; -import org.apache.james.mailbox.model.MailboxPath; -import org.apache.james.mailbox.model.search.MailboxQuery; +import org.apache.james.mailbox.model.search.MailboxNameExpression; import org.apache.james.mailbox.model.search.PrefixedRegex; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.util.MDCBuilder; @@ -55,49 +52,54 @@ public class LSubProcessor extends AbstractSubscriptionProcessor<LsubRequest> { super(LsubRequest.class, next, mailboxManager, subscriptionManager, factory, metricFactory); } - private void listSubscriptions(ImapSession session, Responder responder, String referenceName, String mailboxName) throws SubscriptionException, MailboxException { - final MailboxSession mailboxSession = ImapSessionUtils.getMailboxSession(session); - final Collection<String> mailboxes = getSubscriptionManager().subscriptions(mailboxSession); - // If the mailboxName is fully qualified, ignore the reference name. - String finalReferencename = referenceName; + /** + * @see org.apache.james.imap.processor.AbstractSubscriptionProcessor + * #doProcessRequest(org.apache.james.imap.api.message.request.ImapRequest, + * org.apache.james.imap.api.process.ImapSession, java.lang.String, + * org.apache.james.imap.api.ImapCommand, + * org.apache.james.imap.api.process.ImapProcessor.Responder) + */ + protected void doProcessRequest(LsubRequest request, ImapSession session, String tag, ImapCommand command, Responder responder) { + String referenceName = request.getBaseReferenceName(); + String mailboxPattern = request.getMailboxPattern(); - if (mailboxName.charAt(0) == MailboxConstants.NAMESPACE_PREFIX_CHAR) { - finalReferencename = ""; - } + try { + listSubscriptions(session, responder, referenceName, mailboxPattern); - // Is the interpreted (combined) pattern relative? - boolean isRelative = ((finalReferencename + mailboxName).charAt(0) != MailboxConstants.NAMESPACE_PREFIX_CHAR); - MailboxPath basePath = null; - if (isRelative) { - basePath = MailboxPath.forUser(mailboxSession.getUser().getUserName(), CharsetUtil.decodeModifiedUTF7(finalReferencename)); - } else { - basePath = PathConverter.forSession(session).buildFullPath(CharsetUtil.decodeModifiedUTF7(finalReferencename)); + okComplete(command, tag, responder); + } catch (MailboxException e) { + LOGGER.error("LSub failed for reference " + referenceName + " and pattern " + mailboxPattern, e); + no(command, tag, responder, HumanReadableText.GENERIC_LSUB_FAILURE); } + } + + private void listSubscriptions(ImapSession session, Responder responder, String referenceName, String mailboxName) throws SubscriptionException, MailboxException { + MailboxSession mailboxSession = ImapSessionUtils.getMailboxSession(session); + Collection<String> mailboxes = getSubscriptionManager().subscriptions(mailboxSession); + + String decodedMailName = CharsetUtil.decodeModifiedUTF7(referenceName); + + MailboxNameExpression expression = new PrefixedRegex( + decodedMailName, + CharsetUtil.decodeModifiedUTF7(mailboxName), + mailboxSession.getPathDelimiter()); + Collection<String> mailboxResponses = new ArrayList<>(); - final MailboxQuery expression = MailboxQuery.builder() - .userAndNamespaceFrom(basePath) - .expression(new PrefixedRegex( - basePath.getName(), - CharsetUtil.decodeModifiedUTF7(mailboxName), - mailboxSession.getPathDelimiter())) - .build(); - final Collection<String> mailboxResponses = new ArrayList<>(); for (String mailbox : mailboxes) { respond(responder, expression, mailbox, true, mailboxes, mailboxResponses, mailboxSession.getPathDelimiter()); } } - private void respond(Responder responder, MailboxQuery expression, String mailboxName, boolean originalSubscription, Collection<String> mailboxes, Collection<String> mailboxResponses, char delimiter) { + private void respond(Responder responder, MailboxNameExpression expression, String mailboxName, boolean originalSubscription, Collection<String> mailboxes, Collection<String> mailboxResponses, char delimiter) { if (expression.isExpressionMatch(mailboxName)) { if (!mailboxResponses.contains(mailboxName)) { - final LSubResponse response = new LSubResponse(mailboxName, !originalSubscription, delimiter); - responder.respond(response); + responder.respond(new LSubResponse(mailboxName, !originalSubscription, delimiter)); mailboxResponses.add(mailboxName); } } else { - final int lastDelimiter = mailboxName.lastIndexOf(delimiter); + int lastDelimiter = mailboxName.lastIndexOf(delimiter); if (lastDelimiter > 0) { - final String parentMailbox = mailboxName.substring(0, lastDelimiter); + String parentMailbox = mailboxName.substring(0, lastDelimiter); if (!mailboxes.contains(parentMailbox)) { respond(responder, expression, parentMailbox, false, mailboxes, mailboxResponses, delimiter); } @@ -105,45 +107,6 @@ public class LSubProcessor extends AbstractSubscriptionProcessor<LsubRequest> { } } - /** - * An empty mailboxPattern signifies a request for the hierarchy delimiter - * and root name of the referenceName argument - * - * @param referenceName - * IMAP reference name, possibly null - */ - private void respondWithHierarchyDelimiter(Responder responder, char delimiter) { - final LSubResponse response = new LSubResponse("", true, delimiter); - responder.respond(response); - } - - /** - * @see org.apache.james.imap.processor.AbstractSubscriptionProcessor - * #doProcessRequest(org.apache.james.imap.api.message.request.ImapRequest, - * org.apache.james.imap.api.process.ImapSession, java.lang.String, - * org.apache.james.imap.api.ImapCommand, - * org.apache.james.imap.api.process.ImapProcessor.Responder) - */ - protected void doProcessRequest(LsubRequest request, ImapSession session, String tag, ImapCommand command, Responder responder) { - final String referenceName = request.getBaseReferenceName(); - final String mailboxPattern = request.getMailboxPattern(); - final MailboxSession mailboxSession = ImapSessionUtils.getMailboxSession(session); - - try { - if (mailboxPattern.length() == 0) { - respondWithHierarchyDelimiter(responder, mailboxSession.getPathDelimiter()); - } else { - listSubscriptions(session, responder, referenceName, mailboxPattern); - } - - okComplete(command, tag, responder); - } catch (MailboxException e) { - LOGGER.error("LSub failed for reference " + referenceName + " and pattern " + mailboxPattern, e); - final HumanReadableText displayTextKey = HumanReadableText.GENERIC_LSUB_FAILURE; - no(command, tag, responder, displayTextKey); - } - } - @Override protected Closeable addContextToMDC(LsubRequest message) { return MDCBuilder.create() http://git-wip-us.apache.org/repos/asf/james-project/blob/4e74c7c1/protocols/imap/src/test/java/org/apache/james/imap/processor/LSubProcessorTest.java ---------------------------------------------------------------------- diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/LSubProcessorTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/LSubProcessorTest.java index 0f62d3a..058103f 100644 --- a/protocols/imap/src/test/java/org/apache/james/imap/processor/LSubProcessorTest.java +++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/LSubProcessorTest.java @@ -139,26 +139,6 @@ public class LSubProcessorTest { } @Test - public void testHierarchy() throws Exception { - subscriptions.add(MAILBOX_A); - subscriptions.add(MAILBOX_B); - subscriptions.add(MAILBOX_C); - - mockery.checking(new Expectations() {{ - allowing(session).getAttribute(ImapSessionUtils.MAILBOX_SESSION_ATTRIBUTE_SESSION_KEY); will(returnValue(mailboxSession)); - allowing(mailboxSession).getPathDelimiter(); will(returnValue(HIERARCHY_DELIMITER)); - oneOf(responder).respond(with( - equal(new LSubResponse("", true, HIERARCHY_DELIMITER)))); - }}); - - expectOk(); - - LsubRequest request = new LsubRequest(command, "", "", TAG); - processor.doProcessRequest(request, session, TAG, command, responderImpl); - - } - - @Test public void testShouldRespondToRegexWithSubscribedMailboxes() throws Exception { subscriptions.add(MAILBOX_A); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
