Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8b66b046 authored by cketti's avatar cketti
Browse files

IMAP move operation should ignore expunge policy 

Because the original IMAP specification doesn't include a move operation, we implement it as copy, followed by deleting the source message. Deleting messages in IMAP is a two stage process. First a message is marked as deleted, then the EXPUNGE command is issued. However, the EXPUNGE command will remove all messages in a folder marked as deleted. For a move operation, we don't want to remove other messages, and therefore won't issue the EXPUNGE command. However, if the server supports the UIDPLUS extension, we can specify which messages exactly should be expunged. So if that extension is available, we will use the UID EXPUNGE command on the source message of a move operation.

Since the EXPUNGE command removes all messages marked as deleted, K-9 Mail has a setting that controls when the command is issued (when deleting a message, when polling, manually via a menu option). Previously this setting was also used for move operations. However, that probably should have never been the case.
parent 9fe2c68c

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

+0 −5
Original line number Diff line number Diff line
@@ -893,11 +893,6 @@ public class MessagingController { 
        }



        if (operation != MoveOrCopyFlavor.COPY) {

            if (backend.getSupportsExpunge() && account.getExpungePolicy() == Expunge.EXPUNGE_IMMEDIATELY) {

                Timber.i("processingPendingMoveOrCopy expunging folder %s:%s", account, srcFolderServerId);

                backend.expungeMessages(srcFolderServerId, uids);

            }



            destroyPlaceholderMessages(localSourceFolder, uids);

        }

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

+14 −2
Original line number Diff line number Diff line
@@ -320,8 +320,11 @@ internal class RealImapFolder( 
            return null

        }



        val uids = messages.map { it.uid }



        val uidMapping = copyMessages(messages, folder)

        setFlags(messages, setOf(Flag.DELETED), true)

        expungeUidsOnly(uids)



        return uidMapping

    }
@@ -1080,8 +1083,15 @@ internal class RealImapFolder( 
        }

    }



    @Throws(MessagingException::class)

    override fun expungeUids(uids: List<String>) {

        expungeUids(uids, fullExpungeFallback = true)

    }



    private fun expungeUidsOnly(uids: List<String>) {

        expungeUids(uids, fullExpungeFallback = false)

    }



    private fun expungeUids(uids: List<String>, fullExpungeFallback: Boolean) {

        require(uids.isNotEmpty()) { "expungeUids() must be called with a non-empty set of UIDs" }



        open(OpenMode.READ_WRITE)
@@ -1091,8 +1101,10 @@ internal class RealImapFolder( 
            if (connection!!.isUidPlusCapable) {

                val longUids = uids.map { it.toLong() }.toSet()

                connection!!.executeCommandWithIdSet(Commands.UID_EXPUNGE, "", longUids)

            } else {

            } else if (fullExpungeFallback) {

                executeSimpleCommand("EXPUNGE")

            } else {

                Timber.v("Server doesn't support expunging individual messages: %s", uids)

            }

        } catch (ioe: IOException) {

            throw ioExceptionHandler(connection, ioe)

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

+24 −1
Original line number Diff line number Diff line
@@ -48,6 +48,8 @@ import org.mockito.kotlin.doReturn 
import org.mockito.kotlin.doThrow

import org.mockito.kotlin.eq

import org.mockito.kotlin.mock

import org.mockito.kotlin.never

import org.mockito.kotlin.stub

import org.mockito.kotlin.whenever



class RealImapFolderTest {
@@ -312,9 +314,12 @@ class RealImapFolderTest { 
    }



    @Test

    fun moveMessages_shouldDeleteMessagesFromSourceFolder() {

    fun `moveMessages() should delete messages from source folder but not issue EXPUNGE command`() {

        val sourceFolder = createFolder("Folder")

        prepareImapFolderForOpen(OpenMode.READ_WRITE)

        imapConnection.stub {

            on { isUidPlusCapable } doReturn false

        }

        val destinationFolder = createFolder("Destination")

        val messages = listOf(createImapMessage("1"))

        sourceFolder.open(OpenMode.READ_WRITE)
@@ -322,6 +327,24 @@ class RealImapFolderTest { 
        sourceFolder.moveMessages(messages, destinationFolder)



        assertCommandWithIdsIssued("UID STORE 1 +FLAGS.SILENT (\\Deleted)")

        verify(imapConnection, never()).executeSimpleCommand("EXPUNGE")

    }



    @Test

    fun `moveMessages() should delete messages from source folder and issue UID EXPUNGE command when available`() {

        val sourceFolder = createFolder("Folder")

        prepareImapFolderForOpen(OpenMode.READ_WRITE)

        imapConnection.stub {

            on { isUidPlusCapable } doReturn true

        }

        val destinationFolder = createFolder("Destination")

        val messages = listOf(createImapMessage("1"))

        sourceFolder.open(OpenMode.READ_WRITE)



        sourceFolder.moveMessages(messages, destinationFolder)



        assertCommandWithIdsIssued("UID STORE 1 +FLAGS.SILENT (\\Deleted)")

        assertCommandWithIdsIssued("UID EXPUNGE 1")

    }



    @Test