Move code to move/copy messages to Backend implementations

backend/api/src/main/java/com/fsck/k9/backend/api/Backend.kt

@@ -21,4 +21,21 @@ interface Backend {
 
     @Throws(MessagingException::class)
     fun expunge(folderServerId: String)
+
+    @Throws(MessagingException::class)
+    fun expungeMessages(folderServerId: String, messageServerIds: List<String>)
+
+    @Throws(MessagingException::class)
+    fun moveMessages(
+            sourceFolderServerId: String,
+            targetFolderServerId: String,
+            messageServerIds: List<String>
+    ): Map<String, String>?
+
+    @Throws(MessagingException::class)
+    fun copyMessages(
+            sourceFolderServerId: String,
+            targetFolderServerId: String,
+            messageServerIds: List<String>
+    ): Map<String, String>?
 }

backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandExpunge.java

@@ -1,6 +1,8 @@
 package com.fsck.k9.backend.imap;
 
 
+import java.util.List;
+
 import com.fsck.k9.mail.Folder;
 import com.fsck.k9.mail.MessagingException;
 import com.fsck.k9.mail.store.imap.ImapStore;
@@ -35,4 +37,21 @@ class CommandExpunge {
             remoteFolder.close();
         }
     }
+
+    void expungeMessages(@NotNull String folderServerId, @NotNull List<String> messageServerIds)
+            throws MessagingException {
+        Folder remoteFolder = imapStore.getFolder(folderServerId);
+        try {
+            if (!remoteFolder.exists()) {
+                return;
+            }
+            remoteFolder.open(Folder.OPEN_MODE_RW);
+            if (remoteFolder.getMode() != Folder.OPEN_MODE_RW) {
+                return;
+            }
+            remoteFolder.expungeUids(messageServerIds);
+        } finally {
+            remoteFolder.close();
+        }
+    }
 }

backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandMoveOrCopyMessages.java

@@ -0,0 +1,92 @@
+package com.fsck.k9.backend.imap;
+
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import com.fsck.k9.backend.api.BackendFolder;
+import com.fsck.k9.mail.Folder;
+import com.fsck.k9.mail.Message;
+import com.fsck.k9.mail.MessagingException;
+import com.fsck.k9.mail.store.imap.ImapFolder;
+import com.fsck.k9.mail.store.imap.ImapStore;
+import org.jetbrains.annotations.NotNull;
+import timber.log.Timber;
+
+
+class CommandMoveOrCopyMessages {
+    private final ImapStore imapStore;
+
+
+    CommandMoveOrCopyMessages(ImapStore imapStore) {
+        this.imapStore = imapStore;
+    }
+
+    Map<String, String> moveMessages(@NotNull String sourceFolderServerId, @NotNull String targetFolderServerId,
+            @NotNull List<String> messageServerIds) throws MessagingException {
+        return moveOrCopyMessages(sourceFolderServerId, targetFolderServerId, messageServerIds, false);
+    }
+
+    Map<String, String> copyMessages(@NotNull String sourceFolderServerId, @NotNull String targetFolderServerId,
+            @NotNull List<String> messageServerIds) throws MessagingException {
+        return moveOrCopyMessages(sourceFolderServerId, targetFolderServerId, messageServerIds, true);
+    }
+
+    private Map<String, String> moveOrCopyMessages(String srcFolder, String destFolder, Collection<String> uids,
+            boolean isCopy) throws MessagingException {
+        ImapFolder remoteSrcFolder = null;
+        ImapFolder remoteDestFolder = null;
+
+        try {
+            remoteSrcFolder = imapStore.getFolder(srcFolder);
+
+            List<Message> messages = new ArrayList<>();
+
+            for (String uid : uids) {
+                // TODO: Local messages should never end up here. Throw in debug builds.
+                if (!uid.startsWith(BackendFolder.LOCAL_UID_PREFIX)) {
+                    messages.add(remoteSrcFolder.getMessage(uid));
+                }
+            }
+
+            if (messages.isEmpty()) {
+                Timber.i("processingPendingMoveOrCopy: no remote messages to move, skipping");
+                return null;
+            }
+
+            if (!remoteSrcFolder.exists()) {
+                throw new MessagingException("processingPendingMoveOrCopy: remoteFolder " + srcFolder +
+                        " does not exist", true);
+            }
+
+            remoteSrcFolder.open(Folder.OPEN_MODE_RW);
+            if (remoteSrcFolder.getMode() != Folder.OPEN_MODE_RW) {
+                throw new MessagingException("processingPendingMoveOrCopy: could not open remoteSrcFolder " +
+                        srcFolder + " read/write", true);
+            }
+
+            Timber.d("processingPendingMoveOrCopy: source folder = %s, %d messages, " +
+                    "destination folder = %s, isCopy = %s", srcFolder, messages.size(), destFolder, isCopy);
+
+
+            remoteDestFolder = imapStore.getFolder(destFolder);
+
+            if (isCopy) {
+                return remoteSrcFolder.copyMessages(messages, remoteDestFolder);
+            } else {
+                return remoteSrcFolder.moveMessages(messages, remoteDestFolder);
+            }
+        } finally {
+            closeFolder(remoteSrcFolder);
+            closeFolder(remoteDestFolder);
+        }
+    }
+
+    private static void closeFolder(ImapFolder folder) {
+        if (folder != null) {
+            folder.close();
+        }
+    }
+}

backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapBackend.java

@@ -2,6 +2,7 @@ package com.fsck.k9.backend.imap;
 
 
 import java.util.List;
+import java.util.Map;
 
 import com.fsck.k9.backend.api.Backend;
 import com.fsck.k9.backend.api.BackendStorage;
@@ -12,6 +13,7 @@ import com.fsck.k9.mail.Folder;
 import com.fsck.k9.mail.MessagingException;
 import com.fsck.k9.mail.store.imap.ImapStore;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 
 public class ImapBackend implements Backend {
@@ -19,6 +21,7 @@ public class ImapBackend implements Backend {
     private final CommandSetFlag commandSetFlag;
     private final CommandMarkAllAsRead commandMarkAllAsRead;
     private final CommandExpunge commandExpunge;
+    private final CommandMoveOrCopyMessages commandMoveOrCopyMessages;
 
 
     public ImapBackend(String accountName, BackendStorage backendStorage, ImapStore imapStore) {
@@ -26,6 +29,7 @@ public class ImapBackend implements Backend {
         commandSetFlag = new CommandSetFlag(imapStore);
         commandMarkAllAsRead = new CommandMarkAllAsRead(imapStore);
         commandExpunge = new CommandExpunge(imapStore);
+        commandMoveOrCopyMessages = new CommandMoveOrCopyMessages(imapStore);
     }
 
     @Override
@@ -59,4 +63,24 @@ public class ImapBackend implements Backend {
     public void expunge(@NotNull String folderServerId) throws MessagingException {
         commandExpunge.expunge(folderServerId);
     }
+
+    @Override
+    public void expungeMessages(@NotNull String folderServerId, @NotNull List<String> messageServerIds)
+            throws MessagingException {
+        commandExpunge.expungeMessages(folderServerId, messageServerIds);
+    }
+
+    @Nullable
+    @Override
+    public Map<String, String> moveMessages(@NotNull String sourceFolderServerId, @NotNull String targetFolderServerId,
+            @NotNull List<String> messageServerIds) throws MessagingException {
+        return commandMoveOrCopyMessages.moveMessages(sourceFolderServerId, targetFolderServerId, messageServerIds);
+    }
+
+    @Nullable
+    @Override
+    public Map<String, String> copyMessages(@NotNull String sourceFolderServerId, @NotNull String targetFolderServerId,
+            @NotNull List<String> messageServerIds) throws MessagingException {
+        return commandMoveOrCopyMessages.copyMessages(sourceFolderServerId, targetFolderServerId, messageServerIds);
+    }
 }

backend/pop3/src/main/java/com/fsck/k9/backend/pop3/Pop3Backend.kt

@@ -30,4 +30,24 @@ class Pop3Backend(accountName: String, backendStorage: BackendStorage, pop3Store
     override fun expunge(folderServerId: String) {
         throw UnsupportedOperationException("not supported")
     }
+
+    override fun expungeMessages(folderServerId: String, messageServerIds: List<String>) {
+        throw UnsupportedOperationException("not supported")
+    }
+
+    override fun moveMessages(
+            sourceFolderServerId: String,
+            targetFolderServerId: String,
+            messageServerIds: List<String>
+    ): Map<String, String>? {
+        throw UnsupportedOperationException("not supported")
+    }
+
+    override fun copyMessages(
+            sourceFolderServerId: String,
+            targetFolderServerId: String,
+            messageServerIds: List<String>
+    ): Map<String, String>? {
+        throw UnsupportedOperationException("not supported")
+    }
 }

backend/webdav/src/main/java/com/fsck/k9/backend/webdav/CommandMoveOrCopyMessages.java

@@ -0,0 +1,92 @@
+package com.fsck.k9.backend.webdav;
+
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+import com.fsck.k9.backend.api.BackendFolder;
+import com.fsck.k9.mail.Folder;
+import com.fsck.k9.mail.Message;
+import com.fsck.k9.mail.MessagingException;
+import com.fsck.k9.mail.store.webdav.WebDavFolder;
+import com.fsck.k9.mail.store.webdav.WebDavStore;
+import org.jetbrains.annotations.NotNull;
+import timber.log.Timber;
+
+
+class CommandMoveOrCopyMessages {
+    private final WebDavStore webDavStore;
+
+
+    CommandMoveOrCopyMessages(WebDavStore webDavStore) {
+        this.webDavStore = webDavStore;
+    }
+
+    Map<String, String> moveMessages(@NotNull String sourceFolderServerId, @NotNull String targetFolderServerId,
+            @NotNull List<String> messageServerIds) throws MessagingException {
+        return moveOrCopyMessages(sourceFolderServerId, targetFolderServerId, messageServerIds, false);
+    }
+
+    Map<String, String> copyMessages(@NotNull String sourceFolderServerId, @NotNull String targetFolderServerId,
+            @NotNull List<String> messageServerIds) throws MessagingException {
+        return moveOrCopyMessages(sourceFolderServerId, targetFolderServerId, messageServerIds, true);
+    }
+
+    private Map<String, String> moveOrCopyMessages(String srcFolder, String destFolder, Collection<String> uids,
+            boolean isCopy) throws MessagingException {
+        WebDavFolder remoteSrcFolder = null;
+        WebDavFolder remoteDestFolder = null;
+
+        try {
+            remoteSrcFolder = webDavStore.getFolder(srcFolder);
+
+            List<Message> messages = new ArrayList<>();
+
+            for (String uid : uids) {
+                // TODO: Local messages should never end up here. Throw in debug builds.
+                if (!uid.startsWith(BackendFolder.LOCAL_UID_PREFIX)) {
+                    messages.add(remoteSrcFolder.getMessage(uid));
+                }
+            }
+
+            if (messages.isEmpty()) {
+                Timber.i("processingPendingMoveOrCopy: no remote messages to move, skipping");
+                return null;
+            }
+
+            if (!remoteSrcFolder.exists()) {
+                throw new MessagingException("processingPendingMoveOrCopy: remoteFolder " + srcFolder +
+                        " does not exist", true);
+            }
+
+            remoteSrcFolder.open(Folder.OPEN_MODE_RW);
+            if (remoteSrcFolder.getMode() != Folder.OPEN_MODE_RW) {
+                throw new MessagingException("processingPendingMoveOrCopy: could not open remoteSrcFolder " +
+                        srcFolder + " read/write", true);
+            }
+
+            Timber.d("processingPendingMoveOrCopy: source folder = %s, %d messages, " +
+                    "destination folder = %s, isCopy = %s", srcFolder, messages.size(), destFolder, isCopy);
+
+
+            remoteDestFolder = webDavStore.getFolder(destFolder);
+
+            if (isCopy) {
+                return remoteSrcFolder.copyMessages(messages, remoteDestFolder);
+            } else {
+                return remoteSrcFolder.moveMessages(messages, remoteDestFolder);
+            }
+        } finally {
+            closeFolder(remoteSrcFolder);
+            closeFolder(remoteDestFolder);
+        }
+    }
+
+    private static void closeFolder(WebDavFolder folder) {
+        if (folder != null) {
+            folder.close();
+        }
+    }
+}

backend/webdav/src/main/java/com/fsck/k9/backend/webdav/WebDavBackend.kt

@@ -13,6 +13,7 @@ class WebDavBackend(accountName: String, backendStorage: BackendStorage, webDavS
     private val webDavSync: WebDavSync = WebDavSync(accountName, backendStorage, webDavStore)
     private val commandSetFlag = CommandSetFlag(webDavStore)
     private val commandMarkAllAsRead = CommandMarkAllAsRead(webDavStore)
+    private val commandMoveOrCopyMessages = CommandMoveOrCopyMessages(webDavStore)
 
     override val supportsSeenFlag: Boolean = true
     override val supportsExpunge: Boolean = true
@@ -34,4 +35,24 @@ class WebDavBackend(accountName: String, backendStorage: BackendStorage, webDavS
     override fun expunge(folderServerId: String) {
         throw UnsupportedOperationException("not supported")
     }
+
+    override fun expungeMessages(folderServerId: String, messageServerIds: List<String>) {
+        throw UnsupportedOperationException("not supported")
+    }
+
+    override fun moveMessages(
+            sourceFolderServerId: String,
+            targetFolderServerId: String,
+            messageServerIds: List<String>
+    ): Map<String, String>? {
+        return commandMoveOrCopyMessages.moveMessages(sourceFolderServerId, targetFolderServerId, messageServerIds)
+    }
+
+    override fun copyMessages(
+            sourceFolderServerId: String,
+            targetFolderServerId: String,
+            messageServerIds: List<String>
+    ): Map<String, String>? {
+        return commandMoveOrCopyMessages.copyMessages(sourceFolderServerId, targetFolderServerId, messageServerIds)
+    }
 }

k9mail/src/main/java/com/fsck/k9/controller/MessagingController.java

@@ -1589,105 +1589,56 @@ public class MessagingController {
         boolean isCopy = command.isCopy;
 
         Map<String, String> newUidMap = command.newUidMap;
-        Collection<String> uids = newUidMap != null ? newUidMap.keySet() : command.uids;
+        List<String> uids = newUidMap != null ? new ArrayList<>(newUidMap.keySet()) : command.uids;
 
         processPendingMoveOrCopy(account, srcFolder, destFolder, uids, isCopy, newUidMap);
     }
 
     @VisibleForTesting
-    void processPendingMoveOrCopy(Account account, String srcFolder, String destFolder, Collection<String> uids,
+    void processPendingMoveOrCopy(Account account, String srcFolder, String destFolder, List<String> uids,
             boolean isCopy, Map<String, String> newUidMap) throws MessagingException {
-        Folder remoteSrcFolder = null;
-        Folder remoteDestFolder = null;
         LocalFolder localDestFolder;
 
-        try {
+        LocalStore localStore = account.getLocalStore();
-            RemoteStore remoteStore = account.getRemoteStore();
+        localDestFolder = localStore.getFolder(destFolder);
-            remoteSrcFolder = remoteStore.getFolder(srcFolder);
 
-            LocalStore localStore = account.getLocalStore();
+        Backend backend = getBackend(account);
-            localDestFolder = localStore.getFolder(destFolder);
-            List<Message> messages = new ArrayList<>();
 
-            for (String uid : uids) {
+        Map<String, String> remoteUidMap;
-                if (!uid.startsWith(K9.LOCAL_UID_PREFIX)) {
+        if (isCopy) {
-                    messages.add(remoteSrcFolder.getMessage(uid));
+            remoteUidMap = backend.copyMessages(srcFolder, destFolder, uids);
+        } else {
+            remoteUidMap = backend.moveMessages(srcFolder, destFolder, uids);
+        }
+
+        if (!isCopy && Expunge.EXPUNGE_IMMEDIATELY == account.getExpungePolicy()) {
+            Timber.i("processingPendingMoveOrCopy expunging folder %s:%s", account.getDescription(), srcFolder);
+            backend.expungeMessages(srcFolder, uids);
+        }
+
+        /*
+         * This next part is used to bring the local UIDs of the local destination folder
+         * upto speed with the remote UIDs of remote destination folder.
+         */
+        if (newUidMap != null && remoteUidMap != null && !remoteUidMap.isEmpty()) {
+            Timber.i("processingPendingMoveOrCopy: changing local uids of %d messages", remoteUidMap.size());
+            for (Entry<String, String> entry : remoteUidMap.entrySet()) {
+                String remoteSrcUid = entry.getKey();
+                String newUid = entry.getValue();
+                String localDestUid = newUidMap.get(remoteSrcUid);
+                if (localDestUid == null) {
+                    continue;
                 }
-            }
 
-            if (messages.isEmpty()) {
+                Message localDestMessage = localDestFolder.getMessage(localDestUid);
-                Timber.i("processingPendingMoveOrCopy: no remote messages to move, skipping");
+                if (localDestMessage != null) {
-                return;
+                    localDestMessage.setUid(newUid);
-            }
+                    localDestFolder.changeUid((LocalMessage) localDestMessage);
-
+                    for (MessagingListener l : getListeners()) {
-            if (!remoteSrcFolder.exists()) {
+                        l.messageUidChanged(account, destFolder, localDestUid, newUid);
-                throw new MessagingException(
-                        "processingPendingMoveOrCopy: remoteFolder " + srcFolder + " does not exist", true);
-            }
-            remoteSrcFolder.open(Folder.OPEN_MODE_RW);
-            if (remoteSrcFolder.getMode() != Folder.OPEN_MODE_RW) {
-                throw new MessagingException("processingPendingMoveOrCopy: could not open remoteSrcFolder "
-                        + srcFolder + " read/write", true);
-            }
-
-            Timber.d("processingPendingMoveOrCopy: source folder = %s, %d messages, destination folder = %s, " +
-                    "isCopy = %s", srcFolder, messages.size(), destFolder, isCopy);
-
-            Map<String, String> remoteUidMap = null;
-
-            if (!isCopy && destFolder.equals(account.getTrashFolder())) {
-                Timber.d("processingPendingMoveOrCopy doing special case for deleting message");
-
-                String trashFolder = destFolder;
-                if (K9.FOLDER_NONE.equals(trashFolder)) {
-                    trashFolder = null;
-                }
-                remoteSrcFolder.delete(messages, trashFolder);
-            } else {
-                remoteDestFolder = remoteStore.getFolder(destFolder);
-
-                if (isCopy) {
-                    remoteUidMap = remoteSrcFolder.copyMessages(messages, remoteDestFolder);
-                } else {
-                    remoteUidMap = remoteSrcFolder.moveMessages(messages, remoteDestFolder);
-                }
-            }
-            if (!isCopy && Expunge.EXPUNGE_IMMEDIATELY == account.getExpungePolicy()) {
-                Timber.i("processingPendingMoveOrCopy expunging folder %s:%s", account.getDescription(), srcFolder);
-                List<String> movedUids = new ArrayList<>(messages.size());
-                for (Message message : messages) {
-                    movedUids.add(message.getUid());
-                }
-                remoteSrcFolder.expungeUids(movedUids);
-            }
-
-            /*
-             * This next part is used to bring the local UIDs of the local destination folder
-             * upto speed with the remote UIDs of remote destination folder.
-             */
-            if (newUidMap != null && remoteUidMap != null && !remoteUidMap.isEmpty()) {
-                Timber.i("processingPendingMoveOrCopy: changing local uids of %d messages", remoteUidMap.size());
-                for (Entry<String, String> entry : remoteUidMap.entrySet()) {
-                    String remoteSrcUid = entry.getKey();
-                    String newUid = entry.getValue();
-                    String localDestUid = newUidMap.get(remoteSrcUid);
-                    if (localDestUid == null) {
-                        continue;
-                    }
-
-                    Message localDestMessage = localDestFolder.getMessage(localDestUid);
-                    if (localDestMessage != null) {
-                        localDestMessage.setUid(newUid);
-                        localDestFolder.changeUid((LocalMessage) localDestMessage);
-                        for (MessagingListener l : getListeners()) {
-                            l.messageUidChanged(account, destFolder, localDestUid, newUid);
-                        }
                     }
                 }
             }
-        } finally {
-            closeFolder(remoteSrcFolder);
-            closeFolder(remoteDestFolder);
         }
     }