From c29e435eb3a1b42b7bd8ddfe2d9d72b140c69928 Mon Sep 17 00:00:00 2001 From: Marc Blank Date: Fri, 4 Dec 2009 12:49:28 -0800 Subject: [PATCH] Don't delete referenced messages from the Exchange server * Addresses #2287439 incompletely * The most likely reason for a reply/forward to get stuck in the Outbox is that the referenced message has been deleted from the client, with the deletion occuring BEFORE the message gets sent (currently, the two are completely independent) * This change causes deletes NOT to be sent to the server if the message to be deleted is referenced by an outgoing message Change-Id: I146f63ab345c07e684790e1d7d1fc08870468bbf --- .../android/email/provider/EmailContent.java | 6 +- .../exchange/adapter/EmailSyncAdapter.java | 111 ++++++++++++------ .../EmailSyncAdapterTests.java} | 100 +++++++++++++++- 3 files changed, 177 insertions(+), 40 deletions(-) rename tests/src/com/android/exchange/{EasEmailSyncAdapterTests.java => adapter/EmailSyncAdapterTests.java} (51%) diff --git a/src/com/android/email/provider/EmailContent.java b/src/com/android/email/provider/EmailContent.java index 62c7c537e..955ea0d27 100644 --- a/src/com/android/email/provider/EmailContent.java +++ b/src/com/android/email/provider/EmailContent.java @@ -185,7 +185,9 @@ public abstract class EmailContent { public static final String HTML_REPLY = "htmlReply"; // Replied-to or forwarded body (in text form) public static final String TEXT_REPLY = "textReply"; - // Message id of the source (if this is a reply/forward) + // A reference to a message's unique id used in reply/forward. + // Protocol code can be expected to use this column in determining whether a message can be + // deleted safely (i.e. isn't referenced by other messages) public static final String SOURCE_MESSAGE_KEY = "sourceMessageKey"; // The text to be placed between a reply/forward response and the original message public static final String INTRO_TEXT = "introText"; @@ -519,7 +521,7 @@ public abstract class EmailContent { public String mBcc; public String mReplyTo; - // The following transient members may be used while building and manipulating messages, + // The following transient members may be used while building and manipulating messages, // but they are NOT persisted directly by EmailProvider transient public String mText; transient public String mHtml; diff --git a/src/com/android/exchange/adapter/EmailSyncAdapter.java b/src/com/android/exchange/adapter/EmailSyncAdapter.java index 6838b52aa..a75b293f6 100644 --- a/src/com/android/exchange/adapter/EmailSyncAdapter.java +++ b/src/com/android/exchange/adapter/EmailSyncAdapter.java @@ -24,6 +24,7 @@ import com.android.email.provider.EmailProvider; 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.Body; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.Message; import com.android.email.provider.EmailContent.MessageColumns; @@ -68,8 +69,10 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { private static final String[] MESSAGE_ID_SUBJECT_PROJECTION = new String[] { Message.RECORD_ID, MessageColumns.SUBJECT }; + private static final String WHERE_BODY_SOURCE_MESSAGE_KEY = Body.SOURCE_MESSAGE_KEY + "=?"; - String[] bindArguments = new String[2]; + String[] mBindArguments = new String[2]; + String[] mBindArgument = new String[1]; ArrayList mDeletedIdList = new ArrayList(); ArrayList mUpdatedIdList = new ArrayList(); @@ -311,10 +314,10 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { } private Cursor getServerIdCursor(String serverId, String[] projection) { - bindArguments[0] = serverId; - bindArguments[1] = mMailboxIdAsString; + mBindArguments[0] = serverId; + mBindArguments[1] = mMailboxIdAsString; return mContentResolver.query(Message.CONTENT_URI, projection, - WHERE_SERVER_ID_AND_MAILBOX_KEY, bindArguments, null); + WHERE_SERVER_ID_AND_MAILBOX_KEY, mBindArguments, null); } private void deleteParser(ArrayList deletes, int entryTag) throws IOException { @@ -566,6 +569,68 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { return sb.toString(); } + /** + * Note that messages in the deleted database preserve the message's unique id; therefore, we + * can utilize this id to find references to the message. The only reference situation at this + * point is in the Body table; it is when sending messages via SmartForward and SmartReply + */ + private boolean messageReferenced(ContentResolver cr, long id) { + mBindArgument[0] = Long.toString(id); + // See if this id is referenced in a body + Cursor c = cr.query(Body.CONTENT_URI, Body.ID_PROJECTION, WHERE_BODY_SOURCE_MESSAGE_KEY, + mBindArgument, null); + try { + return c.moveToFirst(); + } finally { + c.close(); + } + } + + /*private*/ /** + * Serialize commands to delete items from the server; as we find items to delete, add their + * id's to the deletedId's array + * + * @param s the Serializer we're using to create post data + * @param deletedIds ids whose deletions are being sent to the server + * @param first whether or not this is the first command being sent + * @return true if SYNC_COMMANDS hasn't been sent (false otherwise) + * @throws IOException + */ + boolean sendDeletedItems(Serializer s, ArrayList deletedIds, boolean first) + throws IOException { + ContentResolver cr = mContext.getContentResolver(); + + // Find any of our deleted items + Cursor c = cr.query(Message.DELETED_CONTENT_URI, Message.LIST_PROJECTION, + MessageColumns.MAILBOX_KEY + '=' + mMailbox.mId, null, null); + // We keep track of the list of deleted item id's so that we can remove them from the + // deleted table after the server receives our command + deletedIds.clear(); + try { + while (c.moveToNext()) { + String serverId = c.getString(Message.LIST_SERVER_ID_COLUMN); + // Keep going if there's no serverId + if (serverId == null) { + continue; + // Also check if this message is referenced elsewhere + } else if (messageReferenced(cr, c.getLong(Message.CONTENT_ID_COLUMN))) { + userLog("Postponing deletion of referenced message: ", serverId); + continue; + } else if (first) { + s.start(Tags.SYNC_COMMANDS); + first = false; + } + // Send the command to delete this message + s.start(Tags.SYNC_DELETE).data(Tags.SYNC_SERVER_ID, serverId).end(); + deletedIds.add(c.getLong(Message.LIST_ID_COLUMN)); + } + } finally { + c.close(); + } + + return first; + } + @Override public boolean sendLocalChanges(Serializer s) throws IOException { ContentResolver cr = mContext.getContentResolver(); @@ -575,37 +640,15 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { return false; } - // Find any of our deleted items - Cursor c = cr.query(Message.DELETED_CONTENT_URI, Message.LIST_PROJECTION, - MessageColumns.MAILBOX_KEY + '=' + mMailbox.mId, null, null); - boolean first = true; - // We keep track of the list of deleted item id's so that we can remove them from the - // deleted table after the server receives our command - mDeletedIdList.clear(); - try { - while (c.moveToNext()) { - String serverId = c.getString(Message.LIST_SERVER_ID_COLUMN); - // Keep going if there's no serverId - if (serverId == null) { - continue; - } else if (first) { - s.start(Tags.SYNC_COMMANDS); - first = false; - } - // Send the command to delete this message - s.start(Tags.SYNC_DELETE).data(Tags.SYNC_SERVER_ID, serverId).end(); - mDeletedIdList.add(c.getLong(Message.LIST_ID_COLUMN)); - } - } finally { - c.close(); - } + // This code is split out for unit testing purposes + boolean firstCommand = sendDeletedItems(s, mDeletedIdList, true); // Find our trash mailbox, since deletions will have been moved there... long trashMailboxId = Mailbox.findMailboxOfType(mContext, mMailbox.mAccountKey, Mailbox.TYPE_TRASH); // Do the same now for updated items - c = cr.query(Message.UPDATED_CONTENT_URI, Message.LIST_PROJECTION, + Cursor c = cr.query(Message.UPDATED_CONTENT_URI, Message.LIST_PROJECTION, MessageColumns.MAILBOX_KEY + '=' + mMailbox.mId, null, null); // We keep track of the list of updated item id's as we did above with deleted items @@ -631,9 +674,9 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { } // If the message is now in the trash folder, it has been deleted by the user if (currentCursor.getLong(UPDATES_MAILBOX_KEY_COLUMN) == trashMailboxId) { - if (first) { + if (firstCommand) { s.start(Tags.SYNC_COMMANDS); - first = false; + firstCommand = false; } // Send the command to delete this message s.start(Tags.SYNC_DELETE).data(Tags.SYNC_SERVER_ID, serverId).end(); @@ -663,9 +706,9 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { continue; } - if (first) { + if (firstCommand) { s.start(Tags.SYNC_COMMANDS); - first = false; + firstCommand = false; } // Send the change to "read" and "favorite" (flagged) s.start(Tags.SYNC_CHANGE) @@ -712,7 +755,7 @@ public class EmailSyncAdapter extends AbstractSyncAdapter { c.close(); } - if (!first) { + if (!firstCommand) { s.end(); // SYNC_COMMANDS } return false; diff --git a/tests/src/com/android/exchange/EasEmailSyncAdapterTests.java b/tests/src/com/android/exchange/adapter/EmailSyncAdapterTests.java similarity index 51% rename from tests/src/com/android/exchange/EasEmailSyncAdapterTests.java rename to tests/src/com/android/exchange/adapter/EmailSyncAdapterTests.java index a3e7daeec..a702428f7 100644 --- a/tests/src/com/android/exchange/EasEmailSyncAdapterTests.java +++ b/tests/src/com/android/exchange/adapter/EmailSyncAdapterTests.java @@ -14,22 +14,50 @@ * limitations under the License. */ -package com.android.exchange; +package com.android.exchange.adapter; +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.Mailbox; -import com.android.exchange.adapter.EmailSyncAdapter; +import com.android.email.provider.EmailContent.Message; +import com.android.exchange.EasSyncService; import com.android.exchange.adapter.EmailSyncAdapter.EasEmailSyncParser; -import android.test.AndroidTestCase; +import android.content.ContentResolver; +import android.content.ContentUris; +import android.content.ContentValues; +import android.content.Context; +import android.test.ProviderTestCase2; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.util.ArrayList; import java.util.GregorianCalendar; import java.util.TimeZone; -public class EasEmailSyncAdapterTests extends AndroidTestCase { +public class EmailSyncAdapterTests extends ProviderTestCase2 { + + EmailProvider mProvider; + Context mMockContext; + + public EmailSyncAdapterTests() { + super(EmailProvider.class, EmailProvider.EMAIL_AUTHORITY); + } + + @Override + public void setUp() throws Exception { + super.setUp(); + mMockContext = getMockContext(); + } + + @Override + public void tearDown() throws Exception { + super.tearDown(); + } /** * Create and return a short, simple InputStream that has at least four bytes, which is all @@ -100,4 +128,68 @@ public class EasEmailSyncAdapterTests extends AndroidTestCase { date = adapter.formatDateTime(calendar); assertEquals("2012-01-02T23:00:01.000Z", date); } + + public void testSendDeletedItems() throws IOException { + EasSyncService service = getTestService(); + EmailSyncAdapter adapter = new EmailSyncAdapter(service.mMailbox, service); + Serializer s = new Serializer(); + ArrayList ids = new ArrayList(); + ArrayList deletedIds = new ArrayList(); + + Context context = mMockContext; + adapter.mContext = context; + final ContentResolver resolver = context.getContentResolver(); + + // Create account and two mailboxes + Account acct = ProviderTestUtils.setupAccount("account", true, context); + adapter.mAccount = acct; + Mailbox box1 = ProviderTestUtils.setupMailbox("box1", acct.mId, true, context); + adapter.mMailbox = box1; + + // Create 3 messages + Message msg1 = + ProviderTestUtils.setupMessage("message1", acct.mId, box1.mId, true, true, context); + ids.add(msg1.mId); + Message msg2 = + ProviderTestUtils.setupMessage("message2", acct.mId, box1.mId, true, true, context); + ids.add(msg2.mId); + Message msg3 = + ProviderTestUtils.setupMessage("message3", acct.mId, box1.mId, true, true, context); + ids.add(msg3.mId); + assertEquals(3, EmailContent.count(context, Message.CONTENT_URI, null, null)); + + // Delete them + for (long id: ids) { + resolver.delete(ContentUris.withAppendedId(Message.SYNCED_CONTENT_URI, id), null, null); + } + + // Confirm that the messages are in the proper table + assertEquals(0, EmailContent.count(context, Message.CONTENT_URI, null, null)); + assertEquals(3, EmailContent.count(context, Message.DELETED_CONTENT_URI, null, null)); + + // Call code to send deletions; the id's of the ones actually deleted will be in the + // deletedIds list + adapter.sendDeletedItems(s, deletedIds, true); + assertEquals(3, deletedIds.size()); + + // Clear this out for the next test + deletedIds.clear(); + + // Create a new message + Message msg4 = + ProviderTestUtils.setupMessage("message3", acct.mId, box1.mId, true, true, context); + assertEquals(1, EmailContent.count(context, Message.CONTENT_URI, null, null)); + // Find the body for this message + Body body = Body.restoreBodyWithMessageId(context, msg4.mId); + // Set its source message to msg2's id + ContentValues values = new ContentValues(); + values.put(Body.SOURCE_MESSAGE_KEY, msg2.mId); + body.update(context, values); + + // Now send deletions again; this time only two should get deleted; msg2 should NOT be + // deleted as it's referenced by msg4 + adapter.sendDeletedItems(s, deletedIds, true); + assertEquals(2, deletedIds.size()); + assertFalse(deletedIds.contains(msg2.mId)); + } }