Move expunge logic into RealImapFolder.deleteMessages()

app/core/src/main/java/com/fsck/k9/controller/DraftOperations.kt

@@ -1,7 +1,6 @@
 package com.fsck.k9.controller
 
 import com.fsck.k9.Account
-import com.fsck.k9.Account.Expunge
 import com.fsck.k9.K9
 import com.fsck.k9.backend.api.Backend
 import com.fsck.k9.controller.MessagingControllerCommands.PendingAppend
@@ -115,7 +114,7 @@ internal class DraftOperations(
             uploadMessage(backend, account, localFolder, localMessage)
         }
 
-        deleteMessage(backend, account, localFolder, command.deleteMessageId)
+        deleteMessage(backend, localFolder, command.deleteMessageId)
     }
 
     private fun uploadMessage(
@@ -152,7 +151,7 @@ internal class DraftOperations(
         }
     }
 
-    private fun deleteMessage(backend: Backend, account: Account, localFolder: LocalFolder, messageId: Long) {
+    private fun deleteMessage(backend: Backend, localFolder: LocalFolder, messageId: Long) {
         val messageServerId = localFolder.getMessageUidById(messageId) ?: run {
             Timber.i("Couldn't find local copy of message [ID: %d] to be deleted. Skipping delete.", messageId)
             return
@@ -162,10 +161,6 @@ internal class DraftOperations(
         val folderServerId = localFolder.serverId
         backend.deleteMessages(folderServerId, messageServerIds)
 
-        if (backend.supportsExpunge && account.expungePolicy == Expunge.EXPUNGE_IMMEDIATELY) {
-            backend.expungeMessages(folderServerId, messageServerIds)
-        }
-
         messagingController.destroyPlaceholderMessages(localFolder, messageServerIds)
     }
 

app/core/src/main/java/com/fsck/k9/controller/MessagingController.java

@@ -29,7 +29,6 @@ import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
 import com.fsck.k9.Account;
 import com.fsck.k9.Account.DeletePolicy;
-import com.fsck.k9.Account.Expunge;
 import com.fsck.k9.DI;
 import com.fsck.k9.K9;
 import com.fsck.k9.Preferences;
@@ -992,10 +991,6 @@ public class MessagingController {
         String folderServerId = getFolderServerId(account, folderId);
         backend.deleteMessages(folderServerId, uids);
 
-        if (backend.getSupportsExpunge() && account.getExpungePolicy() == Expunge.EXPUNGE_IMMEDIATELY) {
-            backend.expungeMessages(folderServerId, uids);
-        }
-
         LocalStore localStore = localStoreProvider.getInstance(account);
         LocalFolder localFolder = localStore.getFolder(folderId);
         localFolder.open();

mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapFolder.kt

@@ -1089,7 +1089,14 @@ internal class RealImapFolder(
 
     @Throws(MessagingException::class)
     override fun deleteMessages(messages: List<ImapMessage>) {
+        checkOpenWithWriteAccess()
+
         setFlags(messages, setOf(Flag.DELETED), true)
+
+        if (internalImapStore.config.isExpungeImmediately()) {
+            val uids = messages.map { it.uid }
+            expungeUids(uids)
+        }
     }
 
     @Throws(MessagingException::class)

mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapFolderTest.kt

@@ -1150,6 +1150,67 @@ class RealImapFolderTest {
         assertThat(uid).isEqualTo("23")
     }
 
+    @Test
+    fun `deleteMessages() on closed folder should throw`() {
+        val folder = createFolder("Folder")
+        val messages = listOf(createImapMessage("1"))
+
+        assertFailure {
+            folder.deleteMessages(messages)
+        }.isInstanceOf<IllegalStateException>()
+            .hasMessage("Folder 'Folder' is not open.")
+
+        verifyNoMoreInteractions(imapConnection)
+    }
+
+    @Test
+    fun `deleteMessages() on folder opened as read-only should throw`() {
+        val folder = createFolder("Folder")
+        prepareImapFolderForOpen(OpenMode.READ_ONLY)
+        folder.open(OpenMode.READ_ONLY)
+        val messages = listOf(createImapMessage("1"))
+
+        assertFailure {
+            folder.deleteMessages(messages)
+        }.isInstanceOf<IllegalStateException>()
+            .hasMessage("Folder 'Folder' needs to be opened for read-write access.")
+    }
+
+    @Test
+    fun `deleteMessages() with expungeImmediately = true should set deleted flag and expunge message`() {
+        imapStoreConfig.expungeImmediately = true
+        val folder = createFolder("Folder")
+        prepareImapFolderForOpen(OpenMode.READ_WRITE)
+        imapConnection.stub {
+            on { isUidPlusCapable } doReturn true
+        }
+        folder.open(OpenMode.READ_WRITE)
+        val messages = listOf(createImapMessage("1"))
+
+        folder.deleteMessages(messages)
+
+        assertCommandWithIdsIssued("UID STORE 1 +FLAGS.SILENT (\\Deleted)")
+        assertCommandWithIdsIssued("UID EXPUNGE 1")
+    }
+
+    @Test
+    fun `deleteMessages() with expungeImmediately = false should set deleted flag but not expunge message`() {
+        imapStoreConfig.expungeImmediately = false
+        val folder = createFolder("Folder")
+        prepareImapFolderForOpen(OpenMode.READ_WRITE)
+        imapConnection.stub {
+            on { isUidPlusCapable } doReturn true
+        }
+        folder.open(OpenMode.READ_WRITE)
+        val messages = listOf(createImapMessage("1"))
+
+        folder.deleteMessages(messages)
+
+        assertCommandWithIdsIssued("UID STORE 1 +FLAGS.SILENT (\\Deleted)")
+        verify(imapConnection, never()).executeCommandWithIdSet("UID EXPUNGE", "", setOf(1L))
+        verify(imapConnection, never()).executeSimpleCommand("EXPUNGE")
+    }
+
     @Test
     fun `deleteAllMessages() on closed folder should throw`() {
         val folder = createFolder("Folder")