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 7659698f8..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(); @@ -2094,10 +2089,6 @@ public class MessagingController { Backend backend = getBackend(account); backend.deleteAllMessages(trashFolderServerId); - if (account.getExpungePolicy() == Expunge.EXPUNGE_IMMEDIATELY && backend.getSupportsExpunge()) { - backend.expunge(trashFolderServerId); - } - // Remove all messages marked as deleted folder.destroyDeletedMessages(); diff --git a/backend/api/src/main/java/com/fsck/k9/backend/api/Backend.kt b/backend/api/src/main/java/com/fsck/k9/backend/api/Backend.kt index 7dcfdce08..3c3bdcda0 100644 --- a/backend/api/src/main/java/com/fsck/k9/backend/api/Backend.kt +++ b/backend/api/src/main/java/com/fsck/k9/backend/api/Backend.kt @@ -41,9 +41,6 @@ interface Backend { @Throws(MessagingException::class) fun expunge(folderServerId: String) - @Throws(MessagingException::class) - fun expungeMessages(folderServerId: String, messageServerIds: List) - @Throws(MessagingException::class) fun deleteMessages(folderServerId: String, messageServerIds: List) diff --git a/backend/demo/src/main/java/app/k9mail/backend/demo/DemoBackend.kt b/backend/demo/src/main/java/app/k9mail/backend/demo/DemoBackend.kt index 00305fac7..ae148141f 100644 --- a/backend/demo/src/main/java/app/k9mail/backend/demo/DemoBackend.kt +++ b/backend/demo/src/main/java/app/k9mail/backend/demo/DemoBackend.kt @@ -104,10 +104,6 @@ class DemoBackend(private val backendStorage: BackendStorage) : Backend { throw UnsupportedOperationException("not implemented") } - override fun expungeMessages(folderServerId: String, messageServerIds: List) { - throw UnsupportedOperationException("not implemented") - } - override fun deleteMessages(folderServerId: String, messageServerIds: List) = Unit override fun deleteAllMessages(folderServerId: String) = Unit diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandDelete.kt b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandDelete.kt new file mode 100644 index 000000000..0b641737c --- /dev/null +++ b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandDelete.kt @@ -0,0 +1,22 @@ +package com.fsck.k9.backend.imap + +import com.fsck.k9.mail.MessagingException +import com.fsck.k9.mail.store.imap.ImapStore +import com.fsck.k9.mail.store.imap.OpenMode + +internal class CommandDelete(private val imapStore: ImapStore) { + + @Throws(MessagingException::class) + fun deleteMessages(folderServerId: String, messageServerIds: List) { + val remoteFolder = imapStore.getFolder(folderServerId) + try { + remoteFolder.open(OpenMode.READ_WRITE) + + val messages = messageServerIds.map { uid -> remoteFolder.getMessage(uid) } + + remoteFolder.deleteMessages(messages) + } finally { + remoteFolder.close() + } + } +} diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandDeleteAll.kt b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandDeleteAll.kt index 23a307217..8e67e95c7 100644 --- a/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandDeleteAll.kt +++ b/backend/imap/src/main/java/com/fsck/k9/backend/imap/CommandDeleteAll.kt @@ -1,6 +1,5 @@ package com.fsck.k9.backend.imap -import com.fsck.k9.mail.Flag import com.fsck.k9.mail.MessagingException import com.fsck.k9.mail.store.imap.ImapStore import com.fsck.k9.mail.store.imap.OpenMode @@ -13,7 +12,7 @@ internal class CommandDeleteAll(private val imapStore: ImapStore) { try { remoteFolder.open(OpenMode.READ_WRITE) - remoteFolder.setFlagsForAllMessages(setOf(Flag.DELETED), true) + remoteFolder.deleteAllMessages() } finally { remoteFolder.close() } diff --git a/backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapBackend.kt b/backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapBackend.kt index 4ed792bec..be6cbac03 100644 --- a/backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapBackend.kt +++ b/backend/imap/src/main/java/com/fsck/k9/backend/imap/ImapBackend.kt @@ -30,6 +30,7 @@ class ImapBackend( private val commandMarkAllAsRead = CommandMarkAllAsRead(imapStore) private val commandExpunge = CommandExpunge(imapStore) private val commandMoveOrCopyMessages = CommandMoveOrCopyMessages(imapStore) + private val commandDelete = CommandDelete(imapStore) private val commandDeleteAll = CommandDeleteAll(imapStore) private val commandSearch = CommandSearch(imapStore) private val commandDownloadMessage = CommandDownloadMessage(backendStorage, imapStore) @@ -79,12 +80,8 @@ class ImapBackend( commandExpunge.expunge(folderServerId) } - override fun expungeMessages(folderServerId: String, messageServerIds: List) { - commandExpunge.expungeMessages(folderServerId, messageServerIds) - } - override fun deleteMessages(folderServerId: String, messageServerIds: List) { - commandSetFlag.setFlag(folderServerId, messageServerIds, Flag.DELETED, true) + commandDelete.deleteMessages(folderServerId, messageServerIds) } override fun deleteAllMessages(folderServerId: String) { diff --git a/backend/imap/src/test/java/com/fsck/k9/backend/imap/TestImapFolder.kt b/backend/imap/src/test/java/com/fsck/k9/backend/imap/TestImapFolder.kt index 13fefe9f4..6ee8ca374 100644 --- a/backend/imap/src/test/java/com/fsck/k9/backend/imap/TestImapFolder.kt +++ b/backend/imap/src/test/java/com/fsck/k9/backend/imap/TestImapFolder.kt @@ -171,6 +171,14 @@ open class TestImapFolder(override val serverId: String) : ImapFolder { throw UnsupportedOperationException("not implemented") } + override fun deleteMessages(messages: List) { + setFlags(messages, setOf(Flag.DELETED), true) + } + + override fun deleteAllMessages() { + setFlagsForAllMessages(setOf(Flag.DELETED), true) + } + override fun expunge() { mode = OpenMode.READ_WRITE wasExpunged = true diff --git a/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt b/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt index d916eadc7..a2f3c42c7 100644 --- a/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt +++ b/backend/jmap/src/main/java/com/fsck/k9/backend/jmap/JmapBackend.kt @@ -72,10 +72,6 @@ class JmapBackend( throw UnsupportedOperationException("not implemented") } - override fun expungeMessages(folderServerId: String, messageServerIds: List) { - throw UnsupportedOperationException("not implemented") - } - override fun deleteMessages(folderServerId: String, messageServerIds: List) { commandDelete.deleteMessages(messageServerIds) } diff --git a/backend/pop3/src/main/java/com/fsck/k9/backend/pop3/Pop3Backend.kt b/backend/pop3/src/main/java/com/fsck/k9/backend/pop3/Pop3Backend.kt index 804ec655f..94ad0b1d9 100644 --- a/backend/pop3/src/main/java/com/fsck/k9/backend/pop3/Pop3Backend.kt +++ b/backend/pop3/src/main/java/com/fsck/k9/backend/pop3/Pop3Backend.kt @@ -66,10 +66,6 @@ class Pop3Backend( throw UnsupportedOperationException("not supported") } - override fun expungeMessages(folderServerId: String, messageServerIds: List) { - throw UnsupportedOperationException("not supported") - } - override fun deleteMessages(folderServerId: String, messageServerIds: List) { commandSetFlag.setFlag(folderServerId, messageServerIds, Flag.DELETED, true) } diff --git a/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.kt b/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.kt index 2706c16d8..71be040c1 100644 --- a/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.kt +++ b/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.kt @@ -81,6 +81,12 @@ interface ImapFolder { @Throws(MessagingException::class) fun moveMessages(messages: List, folder: ImapFolder): Map? + @Throws(MessagingException::class) + fun deleteMessages(messages: List) + + @Throws(MessagingException::class) + fun deleteAllMessages() + @Throws(MessagingException::class) fun expunge() 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 d5843204c..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 @@ -1087,6 +1087,29 @@ internal class RealImapFolder( return searchResponse.numbers.firstOrNull()?.toString() } + @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) + override fun deleteAllMessages() { + checkOpenWithWriteAccess() + + setFlagsForAllMessages(setOf(Flag.DELETED), true) + + if (internalImapStore.config.isExpungeImmediately()) { + expunge() + } + } + @Throws(MessagingException::class) override fun expunge() { checkOpenWithWriteAccess() 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 56eab41a5..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,117 @@ 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") + + assertFailure { + folder.deleteAllMessages() + }.isInstanceOf() + .hasMessage("Folder 'Folder' is not open.") + + verifyNoMoreInteractions(imapConnection) + } + + @Test + fun `deleteAllMessages() on folder opened as read-only should throw`() { + val folder = createFolder("Folder") + prepareImapFolderForOpen(OpenMode.READ_ONLY) + folder.open(OpenMode.READ_ONLY) + + assertFailure { + folder.deleteAllMessages() + }.isInstanceOf() + .hasMessage("Folder 'Folder' needs to be opened for read-write access.") + } + + @Test + fun `deleteAllMessages() with expungeImmediately = true should set deleted flag and expunge`() { + imapStoreConfig.expungeImmediately = true + val folder = createFolder("Folder") + prepareImapFolderForOpen(OpenMode.READ_WRITE) + folder.open(OpenMode.READ_WRITE) + + folder.deleteAllMessages() + + assertCommandIssued("UID STORE 1:* +FLAGS.SILENT (\\Deleted)") + assertCommandIssued("EXPUNGE") + } + + @Test + fun `deleteAllMessages() with expungeImmediately = false should set deleted flag but not expunge`() { + imapStoreConfig.expungeImmediately = false + val folder = createFolder("Folder") + prepareImapFolderForOpen(OpenMode.READ_WRITE) + folder.open(OpenMode.READ_WRITE) + + folder.deleteAllMessages() + + assertCommandIssued("UID STORE 1:* +FLAGS.SILENT (\\Deleted)") + verify(imapConnection, never()).executeSimpleCommand("EXPUNGE") + } + @Test fun `expunge() on closed folder should throw`() { val folder = createFolder("Folder") diff --git a/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/TestImapFolder.kt b/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/TestImapFolder.kt index f922d8ae9..af291ea4e 100644 --- a/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/TestImapFolder.kt +++ b/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/TestImapFolder.kt @@ -112,6 +112,14 @@ internal open class TestImapFolder( throw UnsupportedOperationException("not implemented") } + override fun deleteMessages(messages: List) { + throw UnsupportedOperationException("not implemented") + } + + override fun deleteAllMessages() { + throw UnsupportedOperationException("not implemented") + } + override fun expunge() { throw UnsupportedOperationException("not implemented") }