diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index 8987acf40..0bd455d69 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -42,6 +42,7 @@ import android.content.Context; import android.content.Intent; import android.database.Cursor; import android.net.Uri; +import android.os.AsyncTask; import android.os.Bundle; import android.os.IBinder; import android.os.RemoteCallbackList; @@ -675,67 +676,75 @@ public class Controller { * * @param messageId The id of the message to "delete". * @param accountId The id of the message's account, or -1 if not known by caller - * - * TODO: Move out of UI thread - * TODO: "get account a for message m" should be a utility - * TODO: "get mailbox of type n for account a" should be a utility + * @return the AsyncTask used to execute the deletion */ - public void deleteMessage(long messageId, long accountId) { - ContentResolver resolver = mProviderContext.getContentResolver(); + public AsyncTask deleteMessage(final long messageId, long accountId) { + return Utility.runAsync(new Runnable() { + public void run() { + // 1. Get the message's account + Account account = Account.getAccountForMessageId(mProviderContext, messageId); - // 1. Look up acct# for message we're deleting - if (accountId == -1) { - accountId = lookupAccountForMessage(messageId); - } - if (accountId == -1) { - return; - } + // 2. Confirm that there is a trash mailbox available. If not, create one + long trashMailboxId = findOrCreateMailboxOfType(account.mId, Mailbox.TYPE_TRASH); - // 2. Confirm that there is a trash mailbox available. If not, create one - long trashMailboxId = findOrCreateMailboxOfType(accountId, Mailbox.TYPE_TRASH); + // 3. Get the message's original mailbox + Mailbox mailbox = Mailbox.getMailboxForMessageId(mProviderContext, messageId); - // 3. Are we moving to trash or deleting? It depends on where the message currently sits. - long sourceMailboxId = -1; - Cursor c = resolver.query(EmailContent.Message.CONTENT_URI, - MESSAGEID_TO_MAILBOXID_PROJECTION, EmailContent.RECORD_ID + "=?", - new String[] { Long.toString(messageId) }, null); - try { - sourceMailboxId = c.moveToFirst() - ? c.getLong(MESSAGEID_TO_MAILBOXID_COLUMN_MAILBOXID) - : -1; - } finally { - c.close(); - } + // 4. Drop non-essential data for the message (e.g. attachment files) + AttachmentProvider.deleteAllAttachmentFiles(mProviderContext, account.mId, + messageId); - // 4. Drop non-essential data for the message (e.g. attachment files) - AttachmentProvider.deleteAllAttachmentFiles(mProviderContext, accountId, messageId); + Uri uri = ContentUris.withAppendedId(EmailContent.Message.SYNCED_CONTENT_URI, + messageId); + ContentResolver resolver = mProviderContext.getContentResolver(); - Uri uri = ContentUris.withAppendedId(EmailContent.Message.SYNCED_CONTENT_URI, messageId); - - // 5. Perform "delete" as appropriate - if (sourceMailboxId == trashMailboxId) { - // 5a. Delete from trash - resolver.delete(uri, null, null); - } else { - // 5b. Move to trash - ContentValues cv = new ContentValues(); - cv.put(EmailContent.MessageColumns.MAILBOX_KEY, trashMailboxId); - resolver.update(uri, cv, null, null); - } - - // 6. Service runs automatically, MessagingController needs a kick - Account account = Account.restoreAccountWithId(mProviderContext, accountId); - if (account == null) { - return; // isMessagingController returns false for null, but let's make it clear. - } - if (isMessagingController(account)) { - final long syncAccountId = accountId; - Utility.runAsync(new Runnable() { - public void run() { - mLegacyController.processPendingActions(syncAccountId); + // 5. Perform "delete" as appropriate + if (mailbox.mId == trashMailboxId) { + // 5a. Delete from trash + resolver.delete(uri, null, null); + } else { + // 5b. Move to trash + ContentValues cv = new ContentValues(); + cv.put(EmailContent.MessageColumns.MAILBOX_KEY, trashMailboxId); + resolver.update(uri, cv, null, null); } - }); - } + + if (isMessagingController(account)) { + mLegacyController.processPendingActions(account.mId); + } + } + }); + } + + /** + * Moving a message to another folder + * + * This function has no callback, no result reporting, because the desired outcome + * is reflected entirely by changes to one or more cursors. + * + * @param messageId The id of the message to move + * @param mailboxId The id of the folder we're supposed to move the folder to + * @return the AsyncTask that will execute the move + */ + public AsyncTask moveMessage(final long messageId, final long mailboxId) { + return Utility.runAsync(new Runnable() { + public void run() { + Account account = Account.getAccountForMessageId(mProviderContext, messageId); + if (account != null) { + Uri uri = ContentUris.withAppendedId(EmailContent.Message.SYNCED_CONTENT_URI, + messageId); + ContentValues cv = new ContentValues(); + cv.put(EmailContent.MessageColumns.MAILBOX_KEY, mailboxId); + // Set the serverId to 0, since we don't know what the new server id will be + // TODO: Check if this could be cv.setNull(EmailContent.Message.SERVER_ID) + cv.put(EmailContent.Message.SERVER_ID, "0"); + mProviderContext.getContentResolver().update(uri, cv, null, null); + if (isMessagingController(account)) { + mLegacyController.processPendingActions(account.mId); + } + } + } + }); } /** diff --git a/src/com/android/email/MessagingController.java b/src/com/android/email/MessagingController.java index b37db9e28..334777290 100644 --- a/src/com/android/email/MessagingController.java +++ b/src/com/android/email/MessagingController.java @@ -1278,6 +1278,7 @@ public class MessagingController implements Runnable { boolean changeMoveToTrash = false; boolean changeRead = false; boolean changeFlagged = false; + boolean changeMailbox = false; EmailContent.Message oldMessage = EmailContent.getContent(updates, EmailContent.Message.class); @@ -1291,14 +1292,20 @@ public class MessagingController implements Runnable { continue; // Mailbox removed. Move to the next message. } } - changeMoveToTrash = (oldMessage.mMailboxKey != newMessage.mMailboxKey) - && (mailbox.mType == Mailbox.TYPE_TRASH); + if (oldMessage.mMailboxKey != newMessage.mMailboxKey) { + if (mailbox.mType == Mailbox.TYPE_TRASH) { + changeMoveToTrash = true; + } else { + changeMailbox = true; + } + } changeRead = oldMessage.mFlagRead != newMessage.mFlagRead; changeFlagged = oldMessage.mFlagFavorite != newMessage.mFlagFavorite; - } + } // Load the remote store if it will be needed - if (remoteStore == null && (changeMoveToTrash || changeRead || changeFlagged)) { + if (remoteStore == null && + (changeMoveToTrash || changeRead || changeFlagged || changeMailbox)) { remoteStore = Store.getInstance(account.getStoreUri(mContext), mContext, null); } @@ -1307,9 +1314,9 @@ public class MessagingController implements Runnable { // Move message to trash processPendingMoveToTrash(remoteStore, account, mailbox, oldMessage, newMessage); - } else if (changeRead || changeFlagged) { - processPendingFlagChange(remoteStore, mailbox, changeRead, changeFlagged, - newMessage); + } else if (changeRead || changeFlagged || changeMailbox) { + processPendingDataChange(remoteStore, mailbox, changeRead, changeFlagged, + changeMailbox, oldMessage, newMessage); } // Finally, delete the update @@ -1378,23 +1385,35 @@ public class MessagingController implements Runnable { } /** - * Upsync changes to read or flagged + * Upsync changes to read, flagged, or mailbox * - * @param remoteStore - * @param mailbox - * @param changeRead - * @param changeFlagged - * @param newMessage + * @param remoteStore the remote store for this mailbox + * @param mailbox the mailbox the message is stored in + * @param changeRead whether the message's read state has changed + * @param changeFlagged whether the message's flagged state has changed + * @param changeMailbox whether the message's mailbox has changed + * @param oldMessage the message in it's pre-change state + * @param newMessage the current version of the message */ - private void processPendingFlagChange(Store remoteStore, Mailbox mailbox, boolean changeRead, - boolean changeFlagged, EmailContent.Message newMessage) throws MessagingException { + private void processPendingDataChange(Store remoteStore, Mailbox mailbox, boolean changeRead, + boolean changeFlagged, boolean changeMailbox, EmailContent.Message oldMessage, + EmailContent.Message newMessage) throws MessagingException { + Mailbox newMailbox = null;; // 0. No remote update if the message is local-only if (newMessage.mServerId == null || newMessage.mServerId.equals("") - || newMessage.mServerId.startsWith(LOCAL_SERVERID_PREFIX)) { + || newMessage.mServerId.startsWith(LOCAL_SERVERID_PREFIX) || (mailbox == null)) { return; } + // 0.5 If the mailbox has changed, use the original mailbox for operations + // After any flag changes (which we execute in the original mailbox), we then + // copy the message to the new mailbox + if (changeMailbox) { + newMailbox = mailbox; + mailbox = Mailbox.restoreMailboxWithId(mContext, oldMessage.mMailboxKey); + } + // 1. No remote update for DRAFTS or OUTBOX if (mailbox.mType == Mailbox.TYPE_DRAFTS || mailbox.mType == Mailbox.TYPE_OUTBOX) { return; @@ -1417,9 +1436,10 @@ public class MessagingController implements Runnable { } if (Email.DEBUG) { Log.d(Email.LOG_TAG, - "Update flags for msg id=" + newMessage.mId + "Update for msg id=" + newMessage.mId + " read=" + newMessage.mFlagRead - + " flagged=" + newMessage.mFlagFavorite); + + " flagged=" + newMessage.mFlagFavorite + + " new mailbox=" + newMessage.mMailboxKey); } Message[] messages = new Message[] { remoteMessage }; if (changeRead) { @@ -1428,6 +1448,18 @@ public class MessagingController implements Runnable { if (changeFlagged) { remoteFolder.setFlags(messages, FLAG_LIST_FLAGGED, newMessage.mFlagFavorite); } + if (changeMailbox) { + Folder toFolder = remoteStore.getFolder(newMailbox.mDisplayName); + if (!remoteFolder.exists()) { + return; + } + // Copy the message to its new folder + remoteFolder.copyMessages(messages, toFolder, null); + // Delete the message from the remote source folder + remoteMessage.setFlag(Flag.DELETED, true); + remoteFolder.expunge(); + } + remoteFolder.close(false); } /** @@ -1540,9 +1572,7 @@ public class MessagingController implements Runnable { public void onMessageNotFound(Message message) { mContext.getContentResolver().delete(newMessage.getUri(), null, null); } - - } - ); + }); remoteTrashFolder.close(false); } diff --git a/src/com/android/email/Utility.java b/src/com/android/email/Utility.java index 5ba0f35c7..540ef3dab 100644 --- a/src/com/android/email/Utility.java +++ b/src/com/android/email/Utility.java @@ -699,10 +699,12 @@ public class Utility { } /** - * Run {@code r} on a worker thread. + * Run {@code r} on a worker thread, returning the AsyncTask + * @return the AsyncTask; this is primarily for use by unit tests, which require the + * result of the task */ - public static void runAsync(final Runnable r) { - new AsyncTask() { + public static AsyncTask runAsync(final Runnable r) { + return new AsyncTask() { @Override protected Void doInBackground(Void... params) { r.run(); return null; diff --git a/tests/src/com/android/email/ControllerProviderOpsTests.java b/tests/src/com/android/email/ControllerProviderOpsTests.java index 7b0994bdb..29f3cb617 100644 --- a/tests/src/com/android/email/ControllerProviderOpsTests.java +++ b/tests/src/com/android/email/ControllerProviderOpsTests.java @@ -17,19 +17,20 @@ package com.android.email; import com.android.email.provider.EmailContent; +import com.android.email.provider.EmailProvider; +import com.android.email.provider.ProviderTestUtils; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.Body; import com.android.email.provider.EmailContent.HostAuth; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.Message; -import com.android.email.provider.EmailProvider; -import com.android.email.provider.ProviderTestUtils; import android.content.Context; import android.net.Uri; import android.test.ProviderTestCase2; import java.util.Locale; +import java.util.concurrent.ExecutionException; /** * Tests of the Controller class that depend on the underlying provider. @@ -155,12 +156,40 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 assertEquals(Mailbox.NO_MAILBOX, mTestController.findOrCreateMailboxOfType(accountId, -1)); } + /** + * Test the "move message" function. + * @throws ExecutionException + * @throws InterruptedException + */ + public void testMoveMessage() throws InterruptedException, ExecutionException { + Account account1 = ProviderTestUtils.setupAccount("message-move", true, mProviderContext); + long account1Id = account1.mId; + Mailbox box1 = ProviderTestUtils.setupMailbox("box1", account1Id, true, mProviderContext); + long box1Id = box1.mId; + Mailbox box2 = ProviderTestUtils.setupMailbox("box2", account1Id, true, mProviderContext); + long box2Id = box2.mId; + + Message message1 = ProviderTestUtils.setupMessage("message1", account1Id, box1Id, false, + true, mProviderContext); + long message1Id = message1.mId; + + // Because moveMessage runs asynchronously, call get() to force it to complete + mTestController.moveMessage(message1Id, box2Id).get(); + + // now read back a fresh copy and confirm it's in the trash + Message message1get = EmailContent.Message.restoreMessageWithId(mProviderContext, + message1Id); + assertEquals(box2Id, message1get.mMailboxKey); + } + /** * Test the "delete message" function. Sunny day: * - message/mailbox/account all exist * - trash mailbox exists + * @throws ExecutionException + * @throws InterruptedException */ - public void testDeleteMessage() { + public void testDeleteMessage() throws InterruptedException, ExecutionException { Account account1 = ProviderTestUtils.setupAccount("message-delete", true, mProviderContext); long account1Id = account1.mId; Mailbox box1 = ProviderTestUtils.setupMailbox("box1", account1Id, true, mProviderContext); @@ -174,7 +203,8 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 true, mProviderContext); long message1Id = message1.mId; - mTestController.deleteMessage(message1Id, account1Id); + // Because deleteMessage now runs asynchronously, call get() to force it to complete + mTestController.deleteMessage(message1Id, account1Id).get(); // now read back a fresh copy and confirm it's in the trash Message message1get = EmailContent.Message.restoreMessageWithId(mProviderContext, @@ -186,7 +216,7 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 true, mProviderContext); long message2Id = message2.mId; - mTestController.deleteMessage(message2Id, -1); + mTestController.deleteMessage(message2Id, -1).get(); // now read back a fresh copy and confirm it's in the trash Message message2get = EmailContent.Message.restoreMessageWithId(mProviderContext, @@ -196,8 +226,10 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 /** * Test deleting message when there is no trash mailbox + * @throws ExecutionException + * @throws InterruptedException */ - public void testDeleteMessageNoTrash() { + public void testDeleteMessageNoTrash() throws InterruptedException, ExecutionException { Account account1 = ProviderTestUtils.setupAccount("message-delete-notrash", true, mProviderContext); long account1Id = account1.mId; @@ -209,7 +241,8 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 mProviderContext); long message1Id = message1.mId; - mTestController.deleteMessage(message1Id, account1Id); + // Because deleteMessage now runs asynchronously, call get() to force it to complete + mTestController.deleteMessage(message1Id, account1Id).get(); // now read back a fresh copy and confirm it's in the trash Message message1get =