From 6c21942ec45f561d711b3d74ecca8e62afb735c4 Mon Sep 17 00:00:00 2001 From: Andrew Stadler Date: Thu, 10 Sep 2009 11:52:36 -0700 Subject: [PATCH] Implement move-to-trash for IMAP and POP3. * Define new message-loaded state "FLAG_LOADED_DELETED" (used only for POP3, which needs to write sentinel messages that are not displayed.) * Also renamed the other flags to make the naming more consistent. * Tweak MessageList query generation to inhibit display of deleted message sentinels, and MessagingController won't try to resync them. * Clean up implementation of Controller.deleteMessage() * Add support for move to trash to MessagingController. This operates in three primary modes: * POP3 local delete (no server-side interaction) * POP3 server delete * IMAP server delete (and copy to IMAP trash mailbox) * Add missing implementation in provider to delete all of the attachments for a given message * Fix progress reporting in activities (the test for error vs. progress was inverted, which caused progress indicators to keep spinning after errors.) * Fix broken account settings UI (POP3 delete policy was not persisting) Addresses bug 2097409 TODO delete from trash / empty trash Change-Id: I00188e6dc2093823106e009f35b68c760227c9e6 --- src/com/android/email/Controller.java | 32 ++-- src/com/android/email/LegacyConversions.java | 6 +- .../android/email/MessagingController.java | 174 +++++++++++------- src/com/android/email/Utility.java | 4 +- .../email/activity/AccountFolderList.java | 21 ++- .../android/email/activity/MailboxList.java | 23 ++- .../email/activity/MessageCompose.java | 2 +- .../android/email/activity/MessageList.java | 32 ++-- .../android/email/activity/MessageView.java | 2 +- .../email/activity/setup/AccountSettings.java | 8 +- .../android/email/provider/EmailContent.java | 15 +- .../android/email/provider/EmailProvider.java | 34 +++- .../exchange/adapter/EmailSyncAdapter.java | 2 +- .../email/provider/ProviderTestUtils.java | 4 +- .../android/email/provider/ProviderTests.java | 112 ++++++++++- 15 files changed, 322 insertions(+), 149 deletions(-) diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index 14f9044f4..08d63a098 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -254,7 +254,7 @@ public class Controller { // and get out of here. Uri uri = ContentUris.withAppendedId(Message.CONTENT_URI, messageId); ContentValues cv = new ContentValues(); - cv.put(MessageColumns.FLAG_LOADED, Message.LOADED); + cv.put(MessageColumns.FLAG_LOADED, Message.FLAG_LOADED_COMPLETE); mContext.getContentResolver().update(uri, cv, null, null); Log.d(Email.LOG_TAG, "Unexpected loadMessageForView() for service-based message."); synchronized (mListeners) { @@ -565,24 +565,20 @@ public class Controller { Uri uri = ContentUris.withAppendedId(EmailContent.Message.SYNCED_CONTENT_URI, messageId); resolver.update(uri, cv, null, null); - // 5. Drop non-essential data for the message (e.g. attachments) - // TODO: find the actual files (if any, if loaded) & delete them - c = null; - try { - c = resolver.query(EmailContent.Attachment.CONTENT_URI, - EmailContent.Attachment.CONTENT_PROJECTION, - EmailContent.AttachmentColumns.MESSAGE_KEY + "=?", - new String[] { Long.toString(messageId) }, null); - while (c.moveToNext()) { - // delete any associated storage - // delete row? - } - } finally { - if (c != null) c.close(); - } + // 5. Drop non-essential data for the message (e.g. attachment files) + AttachmentProvider.deleteAllAttachmentFiles(mContext, accountId, messageId); - // 6. For IMAP/POP3 we may need to kick off an immediate delete (depends on acct settings) - // TODO write this + // 6. Service runs automatically, MessagingController needs a kick + final Message message = Message.restoreMessageWithId(mContext, messageId); + Account account = Account.restoreAccountWithId(mContext, message.mAccountKey); + if (isMessagingController(account)) { + new Thread() { + @Override + public void run() { + mLegacyController.processPendingCommands(message.mAccountKey); + } + }.start(); + } } /** diff --git a/src/com/android/email/LegacyConversions.java b/src/com/android/email/LegacyConversions.java index 424e0fae4..f8bb51a1f 100644 --- a/src/com/android/email/LegacyConversions.java +++ b/src/com/android/email/LegacyConversions.java @@ -74,11 +74,11 @@ public class LegacyConversions { // Keep the message in the "unloaded" state until it has (at least) a display name. // This prevents early flickering of empty messages in POP download. - if (localMessage.mFlagLoaded != EmailContent.Message.LOADED) { + if (localMessage.mFlagLoaded != EmailContent.Message.FLAG_LOADED_COMPLETE) { if (localMessage.mDisplayName == null || "".equals(localMessage.mDisplayName)) { - localMessage.mFlagLoaded = EmailContent.Message.NOT_LOADED; + localMessage.mFlagLoaded = EmailContent.Message.FLAG_LOADED_UNLOADED; } else { - localMessage.mFlagLoaded = EmailContent.Message.PARTIALLY_LOADED; + localMessage.mFlagLoaded = EmailContent.Message.FLAG_LOADED_PARTIAL; } } localMessage.mFlagFavorite = message.isSet(Flag.FLAGGED); diff --git a/src/com/android/email/MessagingController.java b/src/com/android/email/MessagingController.java index 4e6df5f70..1fc1bf94f 100644 --- a/src/com/android/email/MessagingController.java +++ b/src/com/android/email/MessagingController.java @@ -41,6 +41,7 @@ import com.android.email.provider.AttachmentProvider; import com.android.email.provider.EmailContent; import com.android.email.provider.EmailContent.Attachment; import com.android.email.provider.EmailContent.AttachmentColumns; +import com.android.email.provider.EmailContent.Body; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.MailboxColumns; import com.android.email.provider.EmailContent.MessageColumns; @@ -280,6 +281,7 @@ public class MessagingController implements Runnable { // Drops first, to make things smaller rather than larger HashSet localsToDrop = new HashSet(localFolderNames); localsToDrop.removeAll(remoteFolderNames); + // TODO drop all attachment files too for (String localNameToDrop : localsToDrop) { LocalMailboxInfo localInfo = localFolders.get(localNameToDrop); Uri uri = ContentUris.withAppendedId( @@ -563,6 +565,7 @@ public class MessagingController implements Runnable { throws MessagingException { Log.d(Email.LOG_TAG, "*** synchronizeMailboxGeneric ***"); + ContentResolver resolver = mContext.getContentResolver(); // 1. Get the message list from the local store and create an index of the uids @@ -570,7 +573,7 @@ public class MessagingController implements Runnable { HashMap localMessageMap = new HashMap(); try { - localUidCursor = mContext.getContentResolver().query( + localUidCursor = resolver.query( EmailContent.Message.CONTENT_URI, LocalMessageInfo.PROJECTION, EmailContent.MessageColumns.ACCOUNT_KEY + "=?" + @@ -660,14 +663,18 @@ public class MessagingController implements Runnable { * Get a list of the messages that are in the remote list but not on the * local store, or messages that are in the local store but failed to download * on the last sync. These are the new messages that we will download. + * Note, we also skip syncing messages which are flagged as "deleted message" sentinels, + * because they are locally deleted and we don't need or want the old message from + * the server. */ for (Message message : remoteMessages) { LocalMessageInfo localMessage = localMessageMap.get(message.getUid()); if (localMessage == null) { newMessageCount++; } - if (localMessage == null || - localMessage.mFlagLoaded != EmailContent.Message.LOADED) { + if (localMessage == null + || (localMessage.mFlagLoaded == EmailContent.Message.FLAG_LOADED_UNLOADED) + || (localMessage.mFlagLoaded == EmailContent.Message.FLAG_LOADED_PARTIAL)) { unsyncedMessages.add(message); } } @@ -770,7 +777,7 @@ public class MessagingController implements Runnable { ContentValues updateValues = new ContentValues(); updateValues.put(EmailContent.Message.FLAG_READ, remoteSeen); updateValues.put(EmailContent.Message.FLAG_FAVORITE, remoteFlagged); - mContext.getContentResolver().update(uri, updateValues, null, null); + resolver.update(uri, updateValues, null, null); } } } @@ -805,7 +812,7 @@ public class MessagingController implements Runnable { // Uri uri = ContentUris.withAppendedId(EmailContent.Mailbox.CONTENT_URI, folder.mId); // ContentValues updateValues = new ContentValues(); // updateValues.put(EmailContent.Mailbox.UNREAD_COUNT, remoteUnreadMessageCount); -// mContext.getContentResolver().update(uri, updateValues, null, null); +// resolver.update(uri, updateValues, null, null); // 11. Remove any messages that are in the local store but no longer on the remote store. @@ -814,9 +821,22 @@ public class MessagingController implements Runnable { for (String uidToDelete : localUidsToDelete) { LocalMessageInfo infoToDelete = localMessageMap.get(uidToDelete); + // Delete associated data (attachment files) + // Attachment & Body records are auto-deleted when we delete the Message record + AttachmentProvider.deleteAllAttachmentFiles(mContext, account.mId, infoToDelete.mId); + + // Delete the message itself Uri uriToDelete = ContentUris.withAppendedId( EmailContent.Message.CONTENT_URI, infoToDelete.mId); - mContext.getContentResolver().delete(uriToDelete, null, null); + resolver.delete(uriToDelete, null, null); + + // Delete extra rows (e.g. synced or deleted) + Uri syncRowToDelete = ContentUris.withAppendedId( + EmailContent.Message.UPDATED_CONTENT_URI, infoToDelete.mId); + resolver.delete(syncRowToDelete, null, null); + Uri deletERowToDelete = ContentUris.withAppendedId( + EmailContent.Message.UPDATED_CONTENT_URI, infoToDelete.mId); + resolver.delete(deletERowToDelete, null, null); } // 12. Divide the unsynced messages into small & large (by size) @@ -851,7 +871,7 @@ public class MessagingController implements Runnable { public void messageFinished(Message message, int number, int ofTotal) { // Store the updated message locally and mark it fully loaded copyOneMessageToProvider(message, account, folder, - EmailContent.Message.LOADED); + EmailContent.Message.FLAG_LOADED_COMPLETE); } public void messageStarted(String uid, int number, int ofTotal) { @@ -877,7 +897,7 @@ public class MessagingController implements Runnable { // Store the partially-loaded message and mark it partially loaded copyOneMessageToProvider(message, account, folder, - EmailContent.Message.PARTIALLY_LOADED); + EmailContent.Message.FLAG_LOADED_PARTIAL); } else { // We have a structure to deal with, from which // we can pull down the parts we want to actually store. @@ -895,7 +915,8 @@ public class MessagingController implements Runnable { remoteFolder.fetch(new Message[] { message }, fp, null); } // Store the updated message locally and mark it fully loaded - copyOneMessageToProvider(message, account, folder, EmailContent.Message.LOADED); + copyOneMessageToProvider(message, account, folder, + EmailContent.Message.FLAG_LOADED_COMPLETE); } } @@ -1135,14 +1156,15 @@ public class MessagingController implements Runnable { * Handles: * Read/Unread * Flagged + * Move To Trash * TODO: - * Delete + * Empty trash * Append * Move * * TODO: tighter projections * - * @param account the account to update + * @param account the account to scan for pending actions * @throws MessagingException */ private void processPendingCommandsSynchronous(EmailContent.Account account) @@ -1157,10 +1179,11 @@ public class MessagingController implements Runnable { try { // Defer setting up the store until we know we need to access it Store remoteStore = null; - // Demand load mailbox (note order-by to reduce thrashing here + // Demand load mailbox (note order-by to reduce thrashing here) Mailbox mailbox = null; // loop through messages marked as needing updates while (updates.moveToNext()) { + boolean changeMoveToTrash = false; boolean changeRead = false; boolean changeFlagged = false; @@ -1170,20 +1193,27 @@ public class MessagingController implements Runnable { EmailContent.Message newMessage = EmailContent.Message.restoreMessageWithId(mContext, oldMessage.mId); if (newMessage != null) { + if (mailbox == null || mailbox.mId != newMessage.mMailboxKey) { + mailbox = Mailbox.restoreMailboxWithId(mContext, newMessage.mMailboxKey); + } + changeMoveToTrash = (oldMessage.mMailboxKey != newMessage.mMailboxKey) + && (mailbox.mType == Mailbox.TYPE_TRASH); changeRead = oldMessage.mFlagRead != newMessage.mFlagRead; changeFlagged = oldMessage.mFlagFavorite != newMessage.mFlagFavorite; } - // Handle simple flag changes - if (changeRead || changeFlagged) { - if (remoteStore == null) { - remoteStore = Store.getInstance(account.getStoreUri(mContext), mContext, - null); - } - if (mailbox == null || mailbox.mId != newMessage.mMailboxKey) { - mailbox = - Mailbox.restoreMailboxWithId(mContext, newMessage.mMailboxKey); - } + // Load the remote store if it will be needed + if (remoteStore == null && (changeMoveToTrash || changeRead || changeFlagged)) { + remoteStore = Store.getInstance(account.getStoreUri(mContext), mContext, null); + } + + // Dispatch here for specific change types + if (changeMoveToTrash) { + // Move message to trash + processPendingMoveToTrash(remoteStore, account, mailbox, + oldMessage, newMessage); + } else if (changeRead || changeFlagged) { + // Upsync changes to read or flagged Folder remoteFolder = remoteStore.getFolder(mailbox.mDisplayName); if (remoteFolder.exists()) { remoteFolder.open(OpenMode.READ_WRITE, null); @@ -1331,44 +1361,68 @@ public class MessagingController implements Runnable { /** * Process a pending trash message command. * - * @param command arguments = (String folder, String uid) - * @param account - * @throws MessagingException + * @param remoteStore the remote store we're working in + * @param account The account in which we are working + * @param newMailbox The local trash mailbox + * @param oldMessage The message copy that was saved in the updates shadow table + * @param newMessage The message that was moved to the mailbox */ - private void processPendingTrash(PendingCommand command, final EmailContent.Account account) - throws MessagingException { - String folder = command.arguments[0]; - String uid = command.arguments[1]; + private void processPendingMoveToTrash(Store remoteStore, + EmailContent.Account account, Mailbox newMailbox, EmailContent.Message oldMessage, + final EmailContent.Message newMessage) throws MessagingException { - final LocalStore localStore = (LocalStore) Store.getInstance( - account.getLocalStoreUri(mContext), mContext, null); - LocalFolder localFolder = (LocalFolder) localStore.getFolder(folder); + // 1. Escape early if we can't find the local mailbox + // TODO smaller projection here + Mailbox oldMailbox = Mailbox.restoreMailboxWithId(mContext, oldMessage.mMailboxKey); + if (oldMailbox == null) { + // can't find old mailbox, it may have been deleted. just return. + return; + } + // 2. We don't support delete-from-trash here + if (oldMailbox.mType == Mailbox.TYPE_TRASH) { + return; + } - Store remoteStore = Store.getInstance(account.getStoreUri(mContext), mContext, - localStore.getPersistentCallbacks()); - Folder remoteFolder = remoteStore.getFolder(folder); + // 3. If DELETE_POLICY_NEVER, simply write back the deleted sentinel and return + // + // This sentinel takes the place of the server-side message, and locally "deletes" it + // by inhibiting future sync or display of the message. It will eventually go out of + // scope when it becomes old, or is deleted on the server, and the regular sync code + // will clean it up for us. + if (account.getDeletePolicy() == Account.DELETE_POLICY_NEVER) { + EmailContent.Message sentinel = new EmailContent.Message(); + sentinel.mAccountKey = oldMessage.mAccountKey; + sentinel.mMailboxKey = oldMessage.mMailboxKey; + sentinel.mFlagLoaded = EmailContent.Message.FLAG_LOADED_DELETED; + sentinel.mServerId = oldMessage.mServerId; + sentinel.save(mContext); + + return; + } + + // The rest of this method handles server-side deletion + + // 4. Find the remote mailbox (that we deleted from), and open it + Folder remoteFolder = remoteStore.getFolder(oldMailbox.mDisplayName); if (!remoteFolder.exists()) { return; } - remoteFolder.open(OpenMode.READ_WRITE, localFolder.getPersistentCallbacks()); + + remoteFolder.open(OpenMode.READ_WRITE, null); if (remoteFolder.getMode() != OpenMode.READ_WRITE) { remoteFolder.close(false); return; } - Message remoteMessage = null; - if (!uid.startsWith("Local")) { - remoteMessage = remoteFolder.getMessage(uid); - } + // 5. Find the remote original message + Message remoteMessage = remoteFolder.getMessage(oldMessage.mServerId); if (remoteMessage == null) { remoteFolder.close(false); return; } - Folder remoteTrashFolder = remoteStore.getFolder(account.getTrashFolderName(mContext)); - /* - * Attempt to copy the remote message to the remote trash folder. - */ + // 6. Find the remote trash folder, and create it if not found + Folder remoteTrashFolder = remoteStore.getFolder(newMailbox.mDisplayName); if (!remoteTrashFolder.exists()) { /* * If the remote trash folder doesn't exist we try to create it. @@ -1376,14 +1430,13 @@ public class MessagingController implements Runnable { remoteTrashFolder.create(FolderType.HOLDS_MESSAGES); } + // 7. Try to copy the message into the remote trash folder + // Note, this entire section will be skipped for POP3 because there's no remote trash if (remoteTrashFolder.exists()) { /* * Because remoteTrashFolder may be new, we need to explicitly open it - * and pass in the persistence callbacks. */ - final LocalFolder localTrashFolder = - (LocalFolder) localStore.getFolder(account.getTrashFolderName(mContext)); - remoteTrashFolder.open(OpenMode.READ_WRITE, localTrashFolder.getPersistentCallbacks()); + remoteTrashFolder.open(OpenMode.READ_WRITE, null); if (remoteTrashFolder.getMode() != OpenMode.READ_WRITE) { remoteFolder.close(false); remoteTrashFolder.close(false); @@ -1392,16 +1445,12 @@ public class MessagingController implements Runnable { remoteFolder.copyMessages(new Message[] { remoteMessage }, remoteTrashFolder, new Folder.MessageUpdateCallbacks() { - public void onMessageUidChange(Message message, String newUid) - throws MessagingException { + public void onMessageUidChange(Message message, String newUid) { // update the UID in the local trash folder, because some stores will // have to change it when copying to remoteTrashFolder - LocalMessage localMessage = - (LocalMessage) localTrashFolder.getMessage(message.getUid()); - if(localMessage != null) { - localMessage.setUid(newUid); - localTrashFolder.updateMessage(localMessage); - } + ContentValues cv = new ContentValues(); + cv.put(EmailContent.Message.SERVER_ID, newUid); + mContext.getContentResolver().update(newMessage.getUri(), cv, null, null); } /** @@ -1409,12 +1458,8 @@ public class MessagingController implements Runnable { * deleted (e.g. it was already deleted from the server.) In this case, * attempt to delete the local copy as well. */ - public void onMessageNotFound(Message message) throws MessagingException { - LocalMessage localMessage = - (LocalMessage) localTrashFolder.getMessage(message.getUid()); - if (localMessage != null) { - localMessage.setFlag(Flag.DELETED, true); - } + public void onMessageNotFound(Message message) { + mContext.getContentResolver().delete(newMessage.getUri(), null, null); } } @@ -1422,6 +1467,7 @@ public class MessagingController implements Runnable { remoteTrashFolder.close(false); } + // 8. Delete the message from the remote source folder remoteMessage.setFlag(Flag.DELETED, true); remoteFolder.expunge(); remoteFolder.close(false); @@ -1446,7 +1492,7 @@ public class MessagingController implements Runnable { mListeners.loadMessageForViewFailed(messageId, "Unknown message"); return; } - if (message.mFlagLoaded == EmailContent.Message.LOADED) { + if (message.mFlagLoaded == EmailContent.Message.FLAG_LOADED_COMPLETE) { mListeners.loadMessageForViewFinished(messageId); return; } @@ -1492,7 +1538,7 @@ public class MessagingController implements Runnable { // 5. Write to provider copyOneMessageToProvider(remoteMessage, account, mailbox, - EmailContent.Message.LOADED); + EmailContent.Message.FLAG_LOADED_COMPLETE); // 6. Notify UI mListeners.loadMessageForViewFinished(messageId); diff --git a/src/com/android/email/Utility.java b/src/com/android/email/Utility.java index b73bb3e5a..009ba57f4 100644 --- a/src/com/android/email/Utility.java +++ b/src/com/android/email/Utility.java @@ -238,7 +238,9 @@ public class Utility { public static String buildMailboxIdSelection(ContentResolver resolver, long mailboxId) { // Setup default selection & args, then add to it as necessary StringBuilder selection = new StringBuilder( - Message.FLAG_LOADED + "!=" + Message.NOT_LOADED + " AND "); + MessageColumns.FLAG_LOADED + " IN (" + + Message.FLAG_LOADED_PARTIAL + "," + Message.FLAG_LOADED_COMPLETE + + ") AND "); if (mailboxId == Mailbox.QUERY_ALL_INBOXES || mailboxId == Mailbox.QUERY_ALL_DRAFTS || mailboxId == Mailbox.QUERY_ALL_OUTBOX) { diff --git a/src/com/android/email/activity/AccountFolderList.java b/src/com/android/email/activity/AccountFolderList.java index 0faa45ce5..cd365f0fb 100644 --- a/src/com/android/email/activity/AccountFolderList.java +++ b/src/com/android/email/activity/AccountFolderList.java @@ -601,20 +601,12 @@ public class AccountFolderList extends ListActivity private class ControllerResults implements Controller.Result { public void updateMailboxListCallback(MessagingException result, long accountKey, int progress) { - if (progress == 0) { - mHandler.progress(true); - } else if (result != null || progress == 100) { - mHandler.progress(false); - } + updateProgress(result, progress); } public void updateMailboxCallback(MessagingException result, long accountKey, long mailboxKey, int progress, int numNewMessages) { - if (progress == 0) { - mHandler.progress(true); - } else if (result != null || progress == 100) { - mHandler.progress(false); - } + updateProgress(result, progress); } public void loadMessageForViewCallback(MessagingException result, long messageId, @@ -627,11 +619,20 @@ public class AccountFolderList extends ListActivity public void serviceCheckMailCallback(MessagingException result, long accountId, long mailboxId, int progress, long tag) { + updateProgress(result, progress); } public void sendMailCallback(MessagingException result, long accountId, long messageId, int progress) { } + + private void updateProgress(MessagingException result, int progress) { + if (result != null || progress == 100) { + mHandler.progress(false); + } else if (progress == 0) { + mHandler.progress(true); + } + } } /* package */ static class AccountsAdapter extends CursorAdapter { diff --git a/src/com/android/email/activity/MailboxList.java b/src/com/android/email/activity/MailboxList.java index 0fe5d07c4..951e6cb58 100644 --- a/src/com/android/email/activity/MailboxList.java +++ b/src/com/android/email/activity/MailboxList.java @@ -376,11 +376,7 @@ public class MailboxList extends ListActivity implements OnItemClickListener, On public void updateMailboxListCallback(MessagingException result, long accountKey, int progress) { if (accountKey == mAccountId) { - if (progress == 0) { - mHandler.progress(true); - } else if (result != null || progress == 100) { - mHandler.progress(false); - } + updateProgress(result, progress); } } @@ -388,11 +384,7 @@ public class MailboxList extends ListActivity implements OnItemClickListener, On public void updateMailboxCallback(MessagingException result, long accountKey, long mailboxKey, int progress, int numNewMessages) { if (accountKey == mAccountId) { - if (progress == 0) { - mHandler.progress(true); - } else if (result != null || progress == 100) { - mHandler.progress(false); - } + updateProgress(result, progress); } } @@ -410,6 +402,17 @@ public class MailboxList extends ListActivity implements OnItemClickListener, On public void sendMailCallback(MessagingException result, long accountId, long messageId, int progress) { + if (accountId == mAccountId) { + updateProgress(result, progress); + } + } + + private void updateProgress(MessagingException result, int progress) { + if (result != null || progress == 100) { + mHandler.progress(false); + } else if (progress == 0) { + mHandler.progress(true); + } } } diff --git a/src/com/android/email/activity/MessageCompose.java b/src/com/android/email/activity/MessageCompose.java index 462847cb4..4ee46763c 100644 --- a/src/com/android/email/activity/MessageCompose.java +++ b/src/com/android/email/activity/MessageCompose.java @@ -667,7 +667,7 @@ public class MessageCompose extends Activity implements OnClickListener, OnFocus message.mText = bodyText; message.mAccountKey = account.mId; message.mDisplayName = makeDisplayName(message.mTo, message.mCc, message.mBcc); - message.mFlagLoaded = Message.LOADED; + message.mFlagLoaded = Message.FLAG_LOADED_COMPLETE; message.mFlagAttachment = hasAttachments; String action = getIntent().getAction(); // Use the Intent to set flags saying this message is a reply or a forward and save the diff --git a/src/com/android/email/activity/MessageList.java b/src/com/android/email/activity/MessageList.java index 067659aa6..1aaceb3dd 100644 --- a/src/com/android/email/activity/MessageList.java +++ b/src/com/android/email/activity/MessageList.java @@ -1118,15 +1118,9 @@ public class MessageList extends ListActivity implements OnItemClickListener, On // TODO check accountKey and only react to relevant notifications public void updateMailboxListCallback(MessagingException result, long accountKey, int progress) { - if (progress == 0) { - mHandler.progress(true); - } else if (result != null || progress == 100) { - mHandler.progress(false); - if (mWaitForMailboxType != -1) { - if (result == null) { - mHandler.lookupMailboxType(accountKey, mWaitForMailboxType); - } - } + updateProgress(result, progress); + if (progress == 100) { + mHandler.lookupMailboxType(accountKey, mWaitForMailboxType); } } @@ -1134,11 +1128,7 @@ public class MessageList extends ListActivity implements OnItemClickListener, On // TODO check accountKey and only react to relevant notifications public void updateMailboxCallback(MessagingException result, long accountKey, long mailboxKey, int progress, int numNewMessages) { - if (progress == 0) { - mHandler.progress(true); - } else if (result != null || progress == 100) { - mHandler.progress(false); - } + updateProgress(result, progress); } public void loadMessageForViewCallback(MessagingException result, long messageId, @@ -1157,11 +1147,15 @@ public class MessageList extends ListActivity implements OnItemClickListener, On public void sendMailCallback(MessagingException result, long accountId, long messageId, int progress) { if (mListFooterMode == LIST_FOOTER_MODE_SEND) { - if (progress == 0) { - mHandler.progress(true); - } else if (result != null || progress == 100) { - mHandler.progress(false); - } + updateProgress(result, progress); + } + } + + private void updateProgress(MessagingException result, int progress) { + if (result != null || progress == 100) { + mHandler.progress(false); + } else if (progress == 0) { + mHandler.progress(true); } } } diff --git a/src/com/android/email/activity/MessageView.java b/src/com/android/email/activity/MessageView.java index 7ee8ef7af..ee70ca93f 100644 --- a/src/com/android/email/activity/MessageView.java +++ b/src/com/android/email/activity/MessageView.java @@ -1167,7 +1167,7 @@ public class MessageView extends Activity implements OnClickListener { // 2. If != LOADED, ask controller to load it // 3. Controller callback (after loaded) should trigger LoadBodyTask & LoadAttachmentsTask // 4. Else start the loader tasks right away (message already loaded) - if (okToFetch && message.mFlagLoaded != Message.LOADED) { + if (okToFetch && message.mFlagLoaded != Message.FLAG_LOADED_COMPLETE) { mWaitForLoadMessageId = message.mId; mController.loadMessageForView(message.mId, mControllerCallback); } else { diff --git a/src/com/android/email/activity/setup/AccountSettings.java b/src/com/android/email/activity/setup/AccountSettings.java index 32d802de0..41fa248e7 100644 --- a/src/com/android/email/activity/setup/AccountSettings.java +++ b/src/com/android/email/activity/setup/AccountSettings.java @@ -228,8 +228,12 @@ public class AccountSettings extends PreferenceActivity { this, mAccount.mHostAuthKeyRecv); mAccount.mHostAuthSend = HostAuth.restoreHostAuthWithId( this, mAccount.mHostAuthKeySend); - - // TODO write me. + + // Because "delete policy" UI is on edit incoming settings, we have + // to refresh that as well. + Account refreshedAccount = Account.restoreAccountWithId(this, mAccount.mId); + mAccount.setDeletePolicy(refreshedAccount.getDeletePolicy()); + mAccountDirty = false; } } diff --git a/src/com/android/email/provider/EmailContent.java b/src/com/android/email/provider/EmailContent.java index 8db282a70..b38d4d820 100644 --- a/src/com/android/email/provider/EmailContent.java +++ b/src/com/android/email/provider/EmailContent.java @@ -273,8 +273,7 @@ public abstract class EmailContent { /** * Returns the bodyId for the given messageId, or -1 if no body is found. */ - /* package */ - static long lookupBodyIdWithMessageId(ContentResolver resolver, long messageId) { + public static long lookupBodyIdWithMessageId(ContentResolver resolver, long messageId) { Cursor c = resolver.query(Body.CONTENT_URI, ID_PROJECTION, Body.MESSAGE_KEY + "=?", new String[] {Long.toString(messageId)}, null); @@ -356,7 +355,7 @@ public abstract class EmailContent { public static final String SUBJECT = "subject"; // Boolean, unread = 0, read = 1 [INDEX] public static final String FLAG_READ = "flagRead"; - // Three state, unloaded = 0, loaded = 1, partially loaded (optional) = 2 [INDEX] + // Load state, see constants below (unloaded, partial, complete, deleted) public static final String FLAG_LOADED = "flagLoaded"; // Boolean, unflagged = 0, flagged (favorite) = 1 public static final String FLAG_FAVORITE = "flagFavorite"; @@ -471,7 +470,7 @@ public abstract class EmailContent { public long mTimeStamp; public String mSubject; public boolean mFlagRead = false; - public int mFlagLoaded = 0; + public int mFlagLoaded = FLAG_LOADED_UNLOADED; public boolean mFlagFavorite = false; public boolean mFlagAttachment = false; public int mFlags = 0; @@ -502,12 +501,12 @@ public abstract class EmailContent { // Values used in mFlagRead public static final int UNREAD = 0; public static final int READ = 1; - public static final int DELETED = 2; // Values used in mFlagLoaded - public static final int NOT_LOADED = 0; - public static final int LOADED = 1; - public static final int PARTIALLY_LOADED = 2; + public static final int FLAG_LOADED_UNLOADED = 0; + public static final int FLAG_LOADED_COMPLETE = 1; + public static final int FLAG_LOADED_PARTIAL = 2; + public static final int FLAG_LOADED_DELETED = 3; // Bits used in mFlags // These three states are mutually exclusive, and indicate whether the message is an diff --git a/src/com/android/email/provider/EmailProvider.java b/src/com/android/email/provider/EmailProvider.java index 437abe407..2679aa346 100644 --- a/src/com/android/email/provider/EmailProvider.java +++ b/src/com/android/email/provider/EmailProvider.java @@ -617,6 +617,8 @@ public class EmailProvider extends ContentProvider { // 1) Begin a transaction, ensuring that both databases are affected atomically // 2) Do the requested deletion, with cascading deletions handled in triggers // 3) End the transaction, committing all changes atomically + // + // Bodies are auto-deleted here; Attachments are auto-deleted via trigger messageDeletion = true; if (!mInTransaction) { @@ -645,6 +647,13 @@ public class EmailProvider extends ContentProvider { result = db.delete(TABLE_NAMES[table], whereWithId(id, selection), selectionArgs); break; + case ATTACHMENTS_MESSAGE_ID: + // All attachments for the given message + id = uri.getPathSegments().get(2); + result = db.delete(TABLE_NAMES[table], + whereWith(Attachment.MESSAGE_KEY + "=" + id, selection), selectionArgs); + break; + case BODY: case MESSAGE: case DELETED_MESSAGE: @@ -655,6 +664,7 @@ public class EmailProvider extends ContentProvider { case HOSTAUTH: result = db.delete(TABLE_NAMES[table], selection, selectionArgs); break; + default: throw new IllegalArgumentException("Unknown URI " + uri); } @@ -846,18 +856,32 @@ public class EmailProvider extends ContentProvider { sb.append("_id="); sb.append(id); if (selection != null) { - sb.append(" AND "); + sb.append(" AND ("); sb.append(selection); + sb.append(')'); } return sb.toString(); } + /** + * Combine a locally-generated selection with a user-provided selection + * + * This introduces risk that the local selection might insert incorrect chars + * into the SQL, so use caution. + * + * @param where locally-generated selection, must not be null + * @param selection user-provided selection, may be null + * @return a single selection string + */ private String whereWith(String where, String selection) { - StringBuilder sb = new StringBuilder(where); - if (selection != null) { - sb.append(" AND "); - sb.append(selection); + if (selection == null) { + return where; } + StringBuilder sb = new StringBuilder(where); + sb.append(" AND ("); + sb.append(selection); + sb.append(')'); + return sb.toString(); } diff --git a/src/com/android/exchange/adapter/EmailSyncAdapter.java b/src/com/android/exchange/adapter/EmailSyncAdapter.java index 6f607e170..e715a975d 100644 --- a/src/com/android/exchange/adapter/EmailSyncAdapter.java +++ b/src/com/android/exchange/adapter/EmailSyncAdapter.java @@ -176,7 +176,7 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { Message msg = new Message(); msg.mAccountKey = mAccount.mId; msg.mMailboxKey = mMailbox.mId; - msg.mFlagLoaded = Message.LOADED; + msg.mFlagLoaded = Message.FLAG_LOADED_COMPLETE; while (nextTag(Tags.SYNC_ADD) != END) { switch (tag) { diff --git a/tests/src/com/android/email/provider/ProviderTestUtils.java b/tests/src/com/android/email/provider/ProviderTestUtils.java index 31c6dd355..6c9c80bcb 100644 --- a/tests/src/com/android/email/provider/ProviderTestUtils.java +++ b/tests/src/com/android/email/provider/ProviderTestUtils.java @@ -103,7 +103,7 @@ public class ProviderTestUtils extends Assert { message.mTimeStamp = 1; message.mSubject = "subject " + name; message.mFlagRead = true; - message.mFlagLoaded = Message.NOT_LOADED; + message.mFlagLoaded = Message.FLAG_LOADED_UNLOADED; message.mFlagFavorite = true; message.mFlagAttachment = true; message.mFlags = 2; @@ -127,7 +127,7 @@ public class ProviderTestUtils extends Assert { message.mHtml = "body html " + name; message.mTextReply = "reply text " + name; message.mHtmlReply = "reply html " + name; - message.mSourceKey = mailboxId; + message.mSourceKey = 1000; } if (saveIt) { diff --git a/tests/src/com/android/email/provider/ProviderTests.java b/tests/src/com/android/email/provider/ProviderTests.java index 2ad83c764..cc6dc3450 100644 --- a/tests/src/com/android/email/provider/ProviderTests.java +++ b/tests/src/com/android/email/provider/ProviderTests.java @@ -19,6 +19,7 @@ package com.android.email.provider; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.AccountColumns; import com.android.email.provider.EmailContent.Attachment; +import com.android.email.provider.EmailContent.AttachmentColumns; import com.android.email.provider.EmailContent.Body; import com.android.email.provider.EmailContent.BodyColumns; import com.android.email.provider.EmailContent.Mailbox; @@ -199,6 +200,7 @@ public class ProviderTests extends ProviderTestCase2 { String html2 = message2.mHtml; String textReply2 = message2.mTextReply; String htmlReply2 = message2.mHtmlReply; + long sourceKey2 = message2.mSourceKey; message2.mText = null; message2.mHtml = null; message2.mTextReply = null; @@ -213,7 +215,7 @@ public class ProviderTests extends ProviderTestCase2 { assertEquals("body html", html2, body2.mHtmlContent); assertEquals("reply text", textReply2, body2.mTextReply); assertEquals("reply html", htmlReply2, body2.mHtmlReply); - assertEquals("source key", message2.mMailboxKey, body2.mSourceKey); + assertEquals("source key", sourceKey2, body2.mSourceKey); // Message with attachments and body Message message3 = ProviderTestUtils.setupMessage("message3", account1Id, box1Id, true, @@ -816,10 +818,78 @@ public class ProviderTests extends ProviderTestCase2 { } /** - * TODO: Test cascaded delete message - * TODO: body - * TODO: attachments + * Test cascaded delete message + * Confirms that deleting a message will also delete its body & attachments */ + public void testCascadeMessageDelete() { + Account account1 = ProviderTestUtils.setupAccount("message-cascade", true, mMockContext); + long account1Id = account1.mId; + Mailbox box1 = ProviderTestUtils.setupMailbox("box1", account1Id, true, mMockContext); + long box1Id = box1.mId; + + // Each message has a body, and also give each 2 attachments + Message message1 = ProviderTestUtils.setupMessage("message1", account1Id, box1Id, true, + false, mMockContext); + ArrayList atts = new ArrayList(); + for (int i = 0; i < 2; i++) { + atts.add(ProviderTestUtils.setupAttachment( + -1, expectedAttachmentNames[i], expectedAttachmentSizes[i], + false, mMockContext)); + } + message1.mAttachments = atts; + message1.save(mMockContext); + long message1Id = message1.mId; + + Message message2 = ProviderTestUtils.setupMessage("message2", account1Id, box1Id, true, + false, mMockContext); + atts = new ArrayList(); + for (int i = 0; i < 2; i++) { + atts.add(ProviderTestUtils.setupAttachment( + -1, expectedAttachmentNames[i], expectedAttachmentSizes[i], + false, mMockContext)); + } + message2.mAttachments = atts; + message2.save(mMockContext); + long message2Id = message2.mId; + + // Set up to test total counts of bodies & attachments for our test messages + String bodySelection = BodyColumns.MESSAGE_KEY + " IN (?,?)"; + String attachmentSelection = AttachmentColumns.MESSAGE_KEY + " IN (?,?)"; + String[] selArgs = new String[] { String.valueOf(message1Id), String.valueOf(message2Id) }; + + // make sure there are two bodies + int numBodies = EmailContent.count(mMockContext, Body.CONTENT_URI, bodySelection, selArgs); + assertEquals(2, numBodies); + + // make sure there are four attachments + int numAttachments = EmailContent.count(mMockContext, Attachment.CONTENT_URI, + attachmentSelection, selArgs); + assertEquals(4, numAttachments); + + // now delete one of the messages + Uri uri = ContentUris.withAppendedId(Message.CONTENT_URI, message1Id); + mMockContext.getContentResolver().delete(uri, null, null); + + // there should be one body and two attachments + numBodies = EmailContent.count(mMockContext, Body.CONTENT_URI, bodySelection, selArgs); + assertEquals(1, numBodies); + + numAttachments = EmailContent.count(mMockContext, Attachment.CONTENT_URI, + attachmentSelection, selArgs); + assertEquals(2, numAttachments); + + // now delete the other message + uri = ContentUris.withAppendedId(Message.CONTENT_URI, message2Id); + mMockContext.getContentResolver().delete(uri, null, null); + + // make sure there are no bodies or attachments + numBodies = EmailContent.count(mMockContext, Body.CONTENT_URI, bodySelection, selArgs); + assertEquals(0, numBodies); + + numAttachments = EmailContent.count(mMockContext, Attachment.CONTENT_URI, + attachmentSelection, selArgs); + assertEquals(0, numAttachments); + } /** * Test that our unique file name algorithm works as expected. Since this test requires an @@ -895,6 +965,40 @@ public class ProviderTests extends ProviderTestCase2 { } } + /** + * Test deleting attachments by message ID (using EmailContent.Attachment.MESSAGE_ID_URI) + */ + public void testDeleteAttachmentByMessageIdUri() { + ContentResolver mockResolver = mMockContext.getContentResolver(); + + // Note, we don't strictly need accounts, mailboxes or messages to run this test. + ProviderTestUtils.setupAttachment(1, "a1", 100, true, mMockContext); + ProviderTestUtils.setupAttachment(1, "a2", 200, true, mMockContext); + Attachment a3 = ProviderTestUtils.setupAttachment(2, "a3", 300, true, mMockContext); + Attachment a4 = ProviderTestUtils.setupAttachment(2, "a4", 400, true, mMockContext); + + // Delete all attachments for message id=1 + Uri uri = ContentUris.withAppendedId(Attachment.MESSAGE_ID_URI, 1); + mockResolver.delete(uri, null, null); + + // Read back all attachments and confirm that we have the expected remaining attachments + // (the attachments that are set for message id=2). Note order-by size to simplify test. + Cursor c = mockResolver.query(Attachment.CONTENT_URI, Attachment.CONTENT_PROJECTION, + null, null, Attachment.SIZE); + assertEquals(2, c.getCount()); + + try { + c.moveToFirst(); + Attachment a3Get = EmailContent.getContent(c, Attachment.class); + ProviderTestUtils.assertAttachmentEqual("getAttachByUri-3", a3, a3Get); + c.moveToNext(); + Attachment a4Get = EmailContent.getContent(c, Attachment.class); + ProviderTestUtils.assertAttachmentEqual("getAttachByUri-4", a4, a4Get); + } finally { + c.close(); + } + } + /** * Tests of default account behavior *