MAILBOX-310 Mailbox query refactoring: MailboxQuery should not rely anymore on MailboxPath for its internal representation
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/1f37e3cf Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/1f37e3cf Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/1f37e3cf Branch: refs/heads/master Commit: 1f37e3cf60da280c7b94a6e9bf07d6ac8783804b Parents: 36333ba Author: benwa <btell...@linagora.com> Authored: Wed Oct 4 09:46:05 2017 +0700 Committer: Matthieu Baechler <matth...@apache.org> Committed: Thu Oct 5 20:00:38 2017 +0200 ---------------------------------------------------------------------- .../james/mailbox/model/MailboxQuery.java | 79 +++++++++----------- .../james/mailbox/MailboxManagerTest.java | 4 +- .../james/mailbox/model/MailboxQueryTest.java | 34 +++++---- .../mailbox/store/StoreMailboxManager.java | 17 ++++- .../mailbox/store/StoreMailboxManagerTest.java | 38 +++++++++- .../apache/james/modules/MailboxProbeImpl.java | 2 +- .../matchers/AbstractStorageQuota.java | 2 +- 7 files changed, 111 insertions(+), 65 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/1f37e3cf/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java index a812206..49db850 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxQuery.java @@ -26,6 +26,7 @@ import java.util.regex.Pattern; import org.apache.james.mailbox.MailboxSession; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; @@ -33,8 +34,6 @@ import com.google.common.base.Preconditions; * Expresses select criteria for mailboxes. */ public final class MailboxQuery { - public static final char SQL_WILDCARD_CHAR = '%'; - /** * Use this wildcard to match every char including the hierarchy delimiter */ @@ -44,15 +43,15 @@ public final class MailboxQuery { * Use this wildcard to match every char except the hierarchy delimiter */ public final static char LOCALWILDCARD = '%'; + private static final String EMPTY_PATH_NAME = ""; - private final MailboxPath base; private final String expression; private final char pathDelimiter; private final Pattern pattern; private final Optional<String> namespace; private final Optional<String> user; - private final Optional<String> name; + private final Optional<String> baseName; public static Builder builder() { return new Builder(); @@ -128,15 +127,11 @@ public final class MailboxQuery { * @param pathDelimiter * path delimiter to use */ - @VisibleForTesting MailboxQuery(Optional<String> namespace, Optional<String> user, Optional<String> name, + @VisibleForTesting MailboxQuery(Optional<String> namespace, Optional<String> user, Optional<String> baseName, String expression, MailboxSession session) { this.namespace = namespace; this.user = user; - this.name = name; - this.base = new MailboxPath( - namespace.orElse(MailboxConstants.USER_NAMESPACE), - user.orElse(session.getUser().getUserName()), - name.orElse(EMPTY_PATH_NAME)); + this.baseName = baseName; if (expression == null) { this.expression = ""; } else { @@ -146,20 +141,24 @@ public final class MailboxQuery { pattern = constructEscapedRegex(); } + public Optional<String> getNamespace() { + return namespace; + } + + public Optional<String> getUser() { + return user; + } + + public Optional<String> getBaseName() { + return baseName; + } + public boolean isPrivateMailboxes(MailboxSession session) { MailboxSession.User sessionUser = session.getUser(); return namespace.map(MailboxConstants.USER_NAMESPACE::equals).orElse(false) && user.map(sessionUser::isSameUser).orElse(false); } - public MailboxPath getPathLike() { - String combinedName = getCombinedName() - .replace(getFreeWildcard(), SQL_WILDCARD_CHAR) - .replace(getLocalWildcard(), SQL_WILDCARD_CHAR) - + SQL_WILDCARD_CHAR; - return new MailboxPath(getBase(), combinedName); - } - public boolean belongsToRequestedNamespaceAndUser(MailboxPath mailboxPath) { boolean belongsToRequestedNamespace = namespace .map(value -> value.equals(mailboxPath.getNamespace())) @@ -172,15 +171,6 @@ public final class MailboxQuery { } /** - * Gets the base reference for the search. - * - * @return the base - */ - public final MailboxPath getBase() { - return base; - } - - /** * Gets the name search expression. This may contain wildcards. * * @return the expression @@ -231,7 +221,7 @@ public final class MailboxQuery { } public boolean isPathMatch(MailboxPath mailboxPath) { - String baseName = name.orElse(EMPTY_PATH_NAME); + String baseName = this.baseName.orElse(EMPTY_PATH_NAME); int baseNameLength = baseName.length(); String mailboxName = mailboxPath.getName(); @@ -249,34 +239,33 @@ public final class MailboxQuery { * notnull */ public String getCombinedName() { - final String result; - if (base != null && base.getName() != null && base.getName().length() > 0) { - final int baseLength = base.getName().length(); - if (base.getName().charAt(baseLength - 1) == pathDelimiter) { + String baseName = this.baseName.orElse(null); + if (baseName != null && baseName.length() > 0) { + final int baseLength = baseName.length(); + if (baseName.charAt(baseLength - 1) == pathDelimiter) { if (expression != null && expression.length() > 0) { if (expression.charAt(0) == pathDelimiter) { - result = base.getName() + expression.substring(1); + return baseName + expression.substring(1); } else { - result = base.getName() + expression; + return baseName + expression; } } else { - result = base.getName(); + return baseName; } } else { if (expression != null && expression.length() > 0) { if (expression.charAt(0) == pathDelimiter) { - result = base.getName() + expression; + return baseName + expression; } else { - result = base.getName() + pathDelimiter + expression; + return baseName + pathDelimiter + expression; } } else { - result = base.getName(); + return baseName; } } } else { - result = expression; + return expression; } - return result; } /** @@ -294,8 +283,14 @@ public final class MailboxQuery { * @return a <code>String</code> representation of this object. */ public String toString() { - final String TAB = " "; - return "MailboxExpression [ " + "base = " + this.base + TAB + "expression = " + this.expression + TAB + "freeWildcard = " + this.getFreeWildcard() + TAB + "localWildcard = " + this.getLocalWildcard() + TAB + " ]"; + return MoreObjects.toStringHelper(this) + .add("expression", expression) + .add("pathDelimiter", pathDelimiter) + .add("pattern", pattern) + .add("namespace", namespace) + .add("user", user) + .add("baseName", baseName) + .toString(); } http://git-wip-us.apache.org/repos/asf/james-project/blob/1f37e3cf/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java index f33ffcf..2d2e438 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java @@ -480,7 +480,7 @@ public abstract class MailboxManagerTest { session1); MailboxQuery mailboxQuery = MailboxQuery.builder() - .matchesAllMailboxNames() + .matchesAll() .build(); assertThat(mailboxManager.search(mailboxQuery, session2)) @@ -510,7 +510,7 @@ public abstract class MailboxManagerTest { session1); MailboxQuery mailboxQuery = MailboxQuery.builder() - .matchesAllMailboxNames() + .matchesAll() .build(); assertThat(mailboxManager.search(mailboxQuery, session2)) http://git-wip-us.apache.org/repos/asf/james-project/blob/1f37e3cf/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java ---------------------------------------------------------------------- diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java index c714c95..1c3f697 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxQueryTest.java @@ -1681,7 +1681,9 @@ public class MailboxQueryTest { .mailboxSession(mailboxSession) .build(); //Then - assertThat(actual.getBase()).isEqualTo(expected); + assertThat(actual.getNamespace()).contains(expected.getNamespace()); + assertThat(actual.getUser()).contains(expected.getUser()); + assertThat(actual.getBaseName()).contains(expected.getName()); } @Test @@ -1722,18 +1724,6 @@ public class MailboxQueryTest { testee.build(); } - @Test - public void getPathLikeShouldReturnUserPathLikeWhenNoBaseDefined() throws Exception { - //Given - Builder testee = MailboxQuery.builder() - .expression("abc") - .mailboxSession(mailboxSession); - //When - MailboxQuery mailboxQuery = testee.build(); - - assertThat(mailboxQuery.getPathLike()).isEqualTo(MailboxPath.forUser("user", "abc%")); - } - @Test(expected=IllegalStateException.class) public void builderShouldThrowWhenBaseAndUsernameGiven() throws Exception { //Given @@ -1771,7 +1761,9 @@ public class MailboxQueryTest { .mailboxSession(mailboxSession) .build(); //Then - assertThat(actual.getBase()).isEqualTo(mailboxPath); + assertThat(actual.getNamespace()).contains(mailboxPath.getNamespace()); + assertThat(actual.getUser()).contains(mailboxPath.getUser()); + assertThat(actual.getBaseName()).contains(mailboxPath.getName()); } @Test @@ -1861,9 +1853,21 @@ public class MailboxQueryTest { .mailboxSession(mailboxSession) .build(); - assertThat(mailboxQuery.belongsToRequestedNamespaceAndUser(new MailboxPath("namespace", null + "2", "name"))) + assertThat(mailboxQuery.belongsToRequestedNamespaceAndUser(new MailboxPath("namespace", null, "name"))) .isFalse(); } + + @Test + public void belongsToNamespaceAndUserShouldReturnFalseWhenDifferentUser() { + MailboxQuery mailboxQuery = MailboxQuery.builder() + .base(new MailboxPath("namespace", CURRENT_USER, "name")) + .mailboxSession(mailboxSession) + .build(); + + assertThat(mailboxQuery.belongsToRequestedNamespaceAndUser(new MailboxPath("namespace", "other", "name"))) + .isFalse(); + } + @Test public void belongsToNamespaceAndUserShouldReturnFalseIfNamespaceAreDifferentWithNullUser() { MailboxQuery mailboxQuery = MailboxQuery.builder() http://git-wip-us.apache.org/repos/asf/james-project/blob/1f37e3cf/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java ---------------------------------------------------------------------- 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 446e216..91a5acd 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 @@ -88,6 +88,7 @@ import org.slf4j.LoggerFactory; import com.github.fge.lambdas.Throwing; import com.github.steveash.guavate.Guavate; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Iterables; /** @@ -100,6 +101,7 @@ import com.google.common.collect.Iterables; */ public class StoreMailboxManager implements MailboxManager { private static final Logger LOGGER = LoggerFactory.getLogger(StoreMailboxManager.class); + public static final char SQL_WILDCARD_CHAR = '%'; private MailboxEventDispatcher dispatcher; @@ -676,7 +678,7 @@ public class StoreMailboxManager implements MailboxManager { public List<MailboxMetaData> search(MailboxQuery mailboxExpression, MailboxSession session) throws MailboxException { MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(session); Stream<Mailbox> baseMailboxes = mailboxMapper - .findMailboxWithPathLike(mailboxExpression.getPathLike()) + .findMailboxWithPathLike(getPathLike(mailboxExpression, session)) .stream(); Stream<Mailbox> delegatedMailboxes = getDelegatedMailboxes(mailboxMapper, mailboxExpression, session); List<Mailbox> mailboxes = Stream.concat(baseMailboxes, @@ -692,6 +694,19 @@ public class StoreMailboxManager implements MailboxManager { .collect(Guavate.toImmutableList()); } + @VisibleForTesting + public static MailboxPath getPathLike(MailboxQuery mailboxQuery, MailboxSession mailboxSession) { + String combinedName = mailboxQuery.getCombinedName() + .replace(mailboxQuery.getFreeWildcard(), SQL_WILDCARD_CHAR) + .replace(mailboxQuery.getLocalWildcard(), SQL_WILDCARD_CHAR) + + SQL_WILDCARD_CHAR; + MailboxPath base = new MailboxPath( + mailboxQuery.getNamespace().orElse(MailboxConstants.USER_NAMESPACE), + mailboxQuery.getUser().orElse(mailboxSession.getUser().getUserName()), + mailboxQuery.getBaseName().orElse("")); + return new MailboxPath(base, combinedName); + } + private Stream<Mailbox> getDelegatedMailboxes(MailboxMapper mailboxMapper, MailboxQuery mailboxQuery, MailboxSession session) throws MailboxException { if (mailboxQuery.isPrivateMailboxes(session)) { return Stream.of(); http://git-wip-us.apache.org/repos/asf/james-project/blob/1f37e3cf/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java index 1777b98..0fad4e9 100644 --- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java +++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/StoreMailboxManagerTest.java @@ -36,6 +36,8 @@ import org.apache.james.mailbox.exception.UserDoesNotExistException; import org.apache.james.mailbox.mock.MockMailboxSession; import org.apache.james.mailbox.model.MailboxACL; import org.apache.james.mailbox.model.MailboxId; +import org.apache.james.mailbox.model.MailboxPath; +import org.apache.james.mailbox.model.MailboxQuery; import org.apache.james.mailbox.model.MessageId; import org.apache.james.mailbox.model.MessageId.Factory; import org.apache.james.mailbox.model.TestId; @@ -46,14 +48,15 @@ import org.junit.Before; import org.junit.Test; public class StoreMailboxManagerTest { - private static final String CURRENT_USER = "user"; private static final String CURRENT_USER_PASSWORD = "secret"; private static final String ADMIN = "admin"; private static final String ADMIN_PASSWORD = "adminsecret"; private static final MailboxId MAILBOX_ID = TestId.of(123); - public static final String UNKNOWN_USER = "otheruser"; - public static final String BAD_PASSWORD = "badpassword"; + private static final String UNKNOWN_USER = "otheruser"; + private static final String BAD_PASSWORD = "badpassword"; + private static final String EMPTY_PREFIX = ""; + private StoreMailboxManager storeMailboxManager; private MailboxMapper mockedMailboxMapper; private MailboxSession mockedMailboxSession; @@ -168,5 +171,34 @@ public class StoreMailboxManagerTest { assertThat(expected.getUser().getUserName()).isEqualTo(CURRENT_USER); } + + @Test + public void getPathLikeShouldReturnUserPathLikeWhenNoPrefixDefined() throws Exception { + //Given + MailboxSession session = new MockMailboxSession("user"); + MailboxQuery.Builder testee = MailboxQuery.builder() + .expression("abc") + .mailboxSession(session); + //When + MailboxQuery mailboxQuery = testee.build(); + + assertThat(StoreMailboxManager.getPathLike(mailboxQuery, session)) + .isEqualTo(MailboxPath.forUser("user", "abc%")); + } + + @Test + public void getPathLikeShouldReturnUserPathLikeWhenPrefixDefined() throws Exception { + //Given + MailboxSession session = new MockMailboxSession("user"); + MailboxQuery.Builder testee = MailboxQuery.builder() + .base(MailboxPath.forUser("user", "prefix.")) + .expression("abc") + .mailboxSession(session); + //When + MailboxQuery mailboxQuery = testee.build(); + + assertThat(StoreMailboxManager.getPathLike(mailboxQuery, session)) + .isEqualTo(MailboxPath.forUser("user", "prefix.abc%")); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/1f37e3cf/server/container/guice/mailbox/src/main/java/org/apache/james/modules/MailboxProbeImpl.java ---------------------------------------------------------------------- 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 4bebc40..7ad811a 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 @@ -126,7 +126,7 @@ public class MailboxProbeImpl implements GuiceProbe, MailboxProbe { MailboxQuery.builder() .base(MailboxPath.forUser(username, "")) .expression("*") - .pathDelimiter(session.getPathDelimiter()) + .mailboxSession(session) .build(), session); } http://git-wip-us.apache.org/repos/asf/james-project/blob/1f37e3cf/server/mailet/mailets/src/main/java/org/apache/james/transport/matchers/AbstractStorageQuota.java ---------------------------------------------------------------------- diff --git a/server/mailet/mailets/src/main/java/org/apache/james/transport/matchers/AbstractStorageQuota.java b/server/mailet/mailets/src/main/java/org/apache/james/transport/matchers/AbstractStorageQuota.java index 2bc8783..9034b9b 100755 --- a/server/mailet/mailets/src/main/java/org/apache/james/transport/matchers/AbstractStorageQuota.java +++ b/server/mailet/mailets/src/main/java/org/apache/james/transport/matchers/AbstractStorageQuota.java @@ -127,7 +127,7 @@ abstract public class AbstractStorageQuota extends AbstractQuotaMatcher { List<MailboxMetaData> mList = manager.search( MailboxQuery.builder() .base(MailboxPath.inbox(session)) - .pathDelimiter(session.getPathDelimiter()) + .mailboxSession(session) .build(), session); for (MailboxMetaData aMList : mList) { --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
