james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From matth...@apache.org
Subject [16/27] james-project git commit: MAILBOX-310 Mailbox query refactoring: MailboxQuery should not rely anymore on MailboxPath for its internal representation
Date Thu, 05 Oct 2017 18:02:28 GMT
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 <btellier@linagora.com>
Authored: Wed Oct 4 09:46:05 2017 +0700
Committer: Matthieu Baechler <matthieu@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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message