From 77aabd951992057a543f1699150971bf1f2d4394 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Fri, 15 Oct 2010 11:45:00 -0700 Subject: [PATCH] Don't move drafts to trash Now deleted drafts are really deleted, rather than getting moved to trash. Also rewrote the test to avoid creating AsyncTasks on the test thread. In this case it seems to be running fine, but I've had problems doing this before. Bug 3099179 Change-Id: Ice5298bf94312ce764d90aa35c5a6c5262ec5b42 --- src/com/android/email/Controller.java | 77 ++++++++-------- .../email/ControllerProviderOpsTests.java | 87 +++++++++++++------ 2 files changed, 103 insertions(+), 61 deletions(-) diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index b200051b2..af19eb24a 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -677,53 +677,60 @@ public class Controller { } /** - * Delete a single message by moving it to the trash, or deleting it from the trash + * Delete a single message by moving it to the trash, or really delete it if it's already in + * trash or a draft message. * * 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 "delete". * @param accountId The id of the message's account, or -1 if not known by caller - * @return the AsyncTask used to execute the deletion */ - public AsyncTask deleteMessage(final long messageId, long accountId) { - return Utility.runAsync(new Runnable() { + public void deleteMessage(final long messageId, final long accountId) { + Utility.runAsync(new Runnable() { public void run() { - // 1. Get the message's account - Account account = Account.getAccountForMessageId(mProviderContext, messageId); - - // 2. Confirm that there is a trash mailbox available. If not, create one - long trashMailboxId = findOrCreateMailboxOfType(account.mId, Mailbox.TYPE_TRASH); - - // 3. Get the message's original mailbox - Mailbox mailbox = Mailbox.getMailboxForMessageId(mProviderContext, messageId); - - // 4. Drop non-essential data for the message (e.g. attachment files) - AttachmentProvider.deleteAllAttachmentFiles(mProviderContext, account.mId, - messageId); - - Uri uri = ContentUris.withAppendedId(EmailContent.Message.SYNCED_CONTENT_URI, - messageId); - ContentResolver resolver = mProviderContext.getContentResolver(); - - // 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); - } + deleteMessageSync(messageId, accountId); } }); } + /** + * Synchronous version of {@link #deleteMessage} for tests. + */ + /* package */ void deleteMessageSync(long messageId, long accountId) { + // 1. Get the message's account + Account account = Account.getAccountForMessageId(mProviderContext, messageId); + + // 2. Confirm that there is a trash mailbox available. If not, create one + long trashMailboxId = findOrCreateMailboxOfType(account.mId, Mailbox.TYPE_TRASH); + + // 3. Get the message's original mailbox + Mailbox mailbox = Mailbox.getMailboxForMessageId(mProviderContext, messageId); + + // 4. Drop non-essential data for the message (e.g. attachment files) + AttachmentProvider.deleteAllAttachmentFiles(mProviderContext, account.mId, + messageId); + + Uri uri = ContentUris.withAppendedId(EmailContent.Message.SYNCED_CONTENT_URI, + messageId); + ContentResolver resolver = mProviderContext.getContentResolver(); + + // 5. Perform "delete" as appropriate + if ((mailbox.mId == trashMailboxId) || (mailbox.mType == Mailbox.TYPE_DRAFTS)) { + // 5a. Really delete it + 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 messages to another folder * diff --git a/tests/src/com/android/email/ControllerProviderOpsTests.java b/tests/src/com/android/email/ControllerProviderOpsTests.java index 208baa8c1..d6d2c5f4d 100644 --- a/tests/src/com/android/email/ControllerProviderOpsTests.java +++ b/tests/src/com/android/email/ControllerProviderOpsTests.java @@ -199,36 +199,72 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 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); - long box1Id = box1.mId; - Mailbox box2 = ProviderTestUtils.setupMailbox("box2", account1Id, false, mProviderContext); - box2.mType = EmailContent.Mailbox.TYPE_TRASH; - box2.save(mProviderContext); - long box2Id = box2.mId; + Mailbox box = ProviderTestUtils.setupMailbox("box1", account1Id, true, mProviderContext); + long boxId = box.mId; - Message message1 = ProviderTestUtils.setupMessage("message1", account1Id, box1Id, false, - true, mProviderContext); - long message1Id = message1.mId; + Mailbox trashBox = ProviderTestUtils.setupMailbox("box2", account1Id, false, + mProviderContext); + trashBox.mType = EmailContent.Mailbox.TYPE_TRASH; + trashBox.save(mProviderContext); + long trashBoxId = trashBox.mId; - // Because deleteMessage now runs asynchronously, call get() to force it to complete - mTestController.deleteMessage(message1Id, account1Id).get(); + Mailbox draftBox = ProviderTestUtils.setupMailbox("box3", account1Id, false, + mProviderContext); + draftBox.mType = EmailContent.Mailbox.TYPE_DRAFTS; + draftBox.save(mProviderContext); + long draftBoxId = draftBox.mId; - // now read back a fresh copy and confirm it's in the trash - Message message1get = EmailContent.Message.restoreMessageWithId(mProviderContext, - message1Id); - assertEquals(box2Id, message1get.mMailboxKey); + { + // Case 1: Message in a regular mailbox, account known. + Message message = ProviderTestUtils.setupMessage("message1", account1Id, boxId, false, + true, mProviderContext); + long messageId = message.mId; - // Now repeat test with accountId "unknown" - Message message2 = ProviderTestUtils.setupMessage("message2", account1Id, box1Id, false, - true, mProviderContext); - long message2Id = message2.mId; + mTestController.deleteMessageSync(messageId, account1Id); - mTestController.deleteMessage(message2Id, -1).get(); + // now read back a fresh copy and confirm it's in the trash + Message restored = EmailContent.Message.restoreMessageWithId(mProviderContext, + messageId); + assertEquals(trashBoxId, restored.mMailboxKey); + } - // now read back a fresh copy and confirm it's in the trash - Message message2get = EmailContent.Message.restoreMessageWithId(mProviderContext, - message2Id); - assertEquals(box2Id, message2get.mMailboxKey); + { + // Case 2: Message in a regular mailbox, account *un*known. + Message message = ProviderTestUtils.setupMessage("message2", account1Id, boxId, false, + true, mProviderContext); + long messageId = message.mId; + + mTestController.deleteMessageSync(messageId, -1); + + // now read back a fresh copy and confirm it's in the trash + Message restored = EmailContent.Message.restoreMessageWithId(mProviderContext, + messageId); + assertEquals(trashBoxId, restored.mMailboxKey); + } + + { + // Case 3: Already in trash + Message message = ProviderTestUtils.setupMessage("message3", account1Id, trashBoxId, + false, true, mProviderContext); + long messageId = message.mId; + + mTestController.deleteMessageSync(messageId, account1Id); + + // Message should be deleted. + assertNull(EmailContent.Message.restoreMessageWithId(mProviderContext, messageId)); + } + + { + // Case 4: Draft + Message message = ProviderTestUtils.setupMessage("message3", account1Id, draftBoxId, + false, true, mProviderContext); + long messageId = message.mId; + + mTestController.deleteMessageSync(messageId, account1Id); + + // Message should be deleted. + assertNull(EmailContent.Message.restoreMessageWithId(mProviderContext, messageId)); + } } /** @@ -248,8 +284,7 @@ public class ControllerProviderOpsTests extends ProviderTestCase2 mProviderContext); long message1Id = message1.mId; - // Because deleteMessage now runs asynchronously, call get() to force it to complete - mTestController.deleteMessage(message1Id, account1Id).get(); + mTestController.deleteMessageSync(message1Id, account1Id); // now read back a fresh copy and confirm it's in the trash Message message1get =