You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by rc...@apache.org on 2020/02/21 02:27:17 UTC

[james-project] 04/13: JAMES-3057 Fix list mailboxes method for maildir implementation

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 0723f4111ec2526f519af6e5f2d6a255468b2a80
Author: Rene Cordier <rc...@linagora.com>
AuthorDate: Wed Feb 19 16:13:32 2020 +0700

    JAMES-3057 Fix list mailboxes method for maildir implementation
    
    The tests were crashing for rename mailbox on maildir because the listing was implemented wrongly.
    If the user didn't have INBOX, list() would crash with a MailboxNotFoundException.
    Oddly enough, in previous implementation where create and rename were shared, that would
    lead the flow to end up in the catch and create a mailbox instead of renaming one.
    
    With proper listing now, the tests with rename on FullUserMaildirMailboxManagerTest work again.
    
    However, it is not the case for DomainUserMaildirMailboxManagerTest. I suspect it is because
    we create a path hierarchy with domains, while our tested users don't have domains,
    creating some issues with assertIsOwner function. This would deserve maybe its own tests with users having domain...
---
 .../mailbox/maildir/mail/MaildirMailboxMapper.java | 22 ++++------
 .../maildir/FullUserMaildirMailboxManagerTest.java | 47 ----------------------
 2 files changed, 7 insertions(+), 62 deletions(-)

diff --git a/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java b/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java
index db822e5..f3c2bbc 100644
--- a/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java
+++ b/mailbox/maildir/src/main/java/org/apache/james/mailbox/maildir/mail/MaildirMailboxMapper.java
@@ -71,7 +71,6 @@ public class MaildirMailboxMapper extends NonTransactionalMapper implements Mail
 
     @Override
     public void delete(Mailbox mailbox) throws MailboxException {
-        
         String folderName = maildirStore.getFolderName(mailbox);
         File folder = new File(folderName);
         if (folder.isDirectory()) {
@@ -257,11 +256,8 @@ public class MaildirMailboxMapper extends NonTransactionalMapper implements Mail
 
     @Override
     public List<Mailbox> list() throws MailboxException {
-        
        File maildirRoot = maildirStore.getMaildirRoot();
        List<Mailbox> mailboxList = new ArrayList<>();
-        
-
 
        if (maildirStore.getMaildirLocation().endsWith("/" + MaildirStore.PATH_DOMAIN + "/" + MaildirStore.PATH_USER)) {
            File[] domains = maildirRoot.listFiles();
@@ -275,7 +271,6 @@ public class MaildirMailboxMapper extends NonTransactionalMapper implements Mail
         File[] users = maildirRoot.listFiles();
         visitUsersForMailboxList(null, users, mailboxList);
         return mailboxList;
-        
     }
 
     @Override
@@ -284,12 +279,9 @@ public class MaildirMailboxMapper extends NonTransactionalMapper implements Mail
     }
 
     private void visitUsersForMailboxList(File domain, File[] users, List<Mailbox> mailboxList) throws MailboxException {
-        
         String userName = null;
         
         for (File user: users) {
-            
-            
             if (domain == null) {
                 userName = user.getName();
             } else {
@@ -298,24 +290,24 @@ public class MaildirMailboxMapper extends NonTransactionalMapper implements Mail
             
             // Special case for INBOX: Let's use the user's folder.
             MailboxPath inboxMailboxPath = MailboxPath.forUser(Username.of(userName), MailboxConstants.INBOX);
-            mailboxList.add(maildirStore.loadMailbox(session, inboxMailboxPath));
+
+            try {
+                mailboxList.add(maildirStore.loadMailbox(session, inboxMailboxPath));
+            } catch (MailboxException e) {
+                //do nothing, we should still be able to list the mailboxes even if INBOX does not exist
+            }
+
             
             // List all INBOX sub folders.
-            
             File[] mailboxes = user.listFiles(pathname -> pathname.getName().startsWith("."));
             
             for (File mailbox: mailboxes) {
-               
-                
                 MailboxPath mailboxPath = new MailboxPath(MailboxConstants.USER_NAMESPACE, 
                         Username.of(userName),
                         mailbox.getName().substring(1));
                 mailboxList.add(maildirStore.loadMailbox(session, mailboxPath));
-
             }
-
         }
-        
     }
 
     @Override
diff --git a/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java b/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java
index 080cc37..cb5d08f 100644
--- a/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java
+++ b/mailbox/maildir/src/test/java/org/apache/james/mailbox/maildir/FullUserMaildirMailboxManagerTest.java
@@ -24,7 +24,6 @@ import org.apache.james.mailbox.events.EventBus;
 import org.apache.james.mailbox.store.StoreMailboxManager;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Nested;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 class FullUserMaildirMailboxManagerTest extends MailboxManagerTest<StoreMailboxManager> {
@@ -35,52 +34,6 @@ class FullUserMaildirMailboxManagerTest extends MailboxManagerTest<StoreMailboxM
     class HookTests {
     }
 
-    @Nested
-    class BasicFeaturesTests extends MailboxManagerTest<StoreMailboxManager>.BasicFeaturesTests {
-        @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
-        @Test
-        protected void renameMailboxShouldChangeTheMailboxPathOfAMailbox() {
-        }
-
-        @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
-        @Test
-        protected void renameMailboxByIdShouldChangeTheMailboxPathOfAMailbox() {
-        }
-
-        @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
-        @Test
-        protected void user1ShouldBeAbleToDeleteSubmailboxByid() {
-        }
-
-        @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
-        @Test
-        protected void user1ShouldBeAbleToDeleteInboxById() {
-        }
-
-        @Disabled("JAMES-2993 mailboxId support for Maildir is partial")
-        @Test
-        protected void getMailboxByIdShouldReturnMailboxWhenBelongingToUser() {
-        }
-    }
-
-    @Nested
-    class MailboxNameLimitTests extends MailboxManagerTest<StoreMailboxManager>.MailboxNameLimitTests {
-        @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
-        @Test
-        protected void renamingMailboxByIdShouldNotThrowWhenNameWithoutEmptyHierarchicalLevel() {
-        }
-
-        @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
-        @Test
-        protected void renamingMailboxByIdShouldNotFailWhenLimitNameLength() {
-        }
-
-        @Disabled("MAILBOX-389 Mailbox rename fails with Maildir")
-        @Test
-        protected void renamingMailboxByIdShouldNotThrowWhenNameWithASingleToBeNormalizedTrailingDelimiter() {
-        }
-    }
-
     @RegisterExtension
     TemporaryFolderExtension temporaryFolder = new TemporaryFolderExtension();
     


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org