From 8fa8fa50c88b6892594db0651faa04510f56c0fc Mon Sep 17 00:00:00 2001 From: cketti Date: Mon, 6 May 2024 19:06:15 +0200 Subject: [PATCH] Move expunge logic into `RealImapFolder.deleteMessages()` --- .../com/fsck/k9/controller/DraftOperations.kt | 9 +-- .../k9/controller/MessagingController.java | 5 -- .../fsck/k9/mail/store/imap/RealImapFolder.kt | 7 +++ .../k9/mail/store/imap/RealImapFolderTest.kt | 61 +++++++++++++++++++ 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/app/core/src/main/java/com/fsck/k9/controller/DraftOperations.kt b/app/core/src/main/java/com/fsck/k9/controller/DraftOperations.kt index 1baf8531a..c712f8486 100644 --- a/app/core/src/main/java/com/fsck/k9/controller/DraftOperations.kt +++ b/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) } diff --git a/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java b/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java index f1a6de3f7..fa11b0442 100644 --- a/app/core/src/main/java/com/fsck/k9/controller/MessagingController.java +++ b/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(); diff --git a/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapFolder.kt b/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapFolder.kt index 4cef7e66f..7fc2ffb0d 100644 --- a/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapFolder.kt +++ b/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) { + checkOpenWithWriteAccess() + setFlags(messages, setOf(Flag.DELETED), true) + + if (internalImapStore.config.isExpungeImmediately()) { + val uids = messages.map { it.uid } + expungeUids(uids) + } } @Throws(MessagingException::class) diff --git a/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapFolderTest.kt b/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapFolderTest.kt index 6c911d135..a164fa16e 100644 --- a/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapFolderTest.kt +++ b/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() + .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() + .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")