From 7183724276e0d829fd01a5bc1f2f6d0f6b6a8818 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Wed, 8 Sep 2010 17:38:34 -0700 Subject: [PATCH] Don't check flagLoaded for outbox message list We found a case where messages in outbox have flagLoaded=FLAG_LOADED_UNLOADED, where it should be FLAG_LOADED_COMPLETE. Haven't found the root problem, but remove the flagLoaded test to mitigate the problem. Also moved the buildMailboxIdSelection call to a worker thread. (this method issues some queries.) Bug 2984285 Change-Id: I2a5be300fb3da703698512262cc38bea75d0f7ba --- src/com/android/email/Utility.java | 39 ++++++++++--- .../email/activity/MessageOrderManager.java | 2 +- .../email/activity/MessagesAdapter.java | 30 ++++++++-- .../com/android/email/UtilityMediumTests.java | 55 +++++++++++++++++++ 4 files changed, 112 insertions(+), 14 deletions(-) diff --git a/src/com/android/email/Utility.java b/src/com/android/email/Utility.java index 8c2bfa63b..dadc5914f 100644 --- a/src/com/android/email/Utility.java +++ b/src/com/android/email/Utility.java @@ -81,6 +81,12 @@ public class Utility { private static final Pattern DATE_CLEANUP_PATTERN_WRONG_TIMEZONE = Pattern.compile("GMT([-+]\\d{4})$"); + private static final String SELECTION_FLAG_LOADED_FOR_VISIBLE_MESSAGE = + " AND (" + + MessageColumns.FLAG_LOADED + " IN (" + + Message.FLAG_LOADED_PARTIAL + "," + Message.FLAG_LOADED_COMPLETE + + "))"; + public final static String readInputStream(InputStream in, String encoding) throws IOException { InputStreamReader reader = new InputStreamReader(in, encoding); StringBuffer sb = new StringBuffer(); @@ -284,13 +290,18 @@ public class Utility { // } } - // TODO: unit test this - public static String buildMailboxIdSelection(ContentResolver resolver, long mailboxId) { - // Setup default selection & args, then add to it as necessary - StringBuilder selection = new StringBuilder( - MessageColumns.FLAG_LOADED + " IN (" - + Message.FLAG_LOADED_PARTIAL + "," + Message.FLAG_LOADED_COMPLETE - + ") AND "); + /** + * Returns the where clause for a message list selection. + * + * MUST NOT be called on the UI thread. + */ + public static String buildMailboxIdSelection(Context context, long mailboxId) { + final ContentResolver resolver = context.getContentResolver(); + final StringBuilder selection = new StringBuilder(); + + // We don't check "flagLoaded" for messages in Outbox. + boolean testFlagLoaded = true; + if (mailboxId == Mailbox.QUERY_ALL_INBOXES || mailboxId == Mailbox.QUERY_ALL_DRAFTS || mailboxId == Mailbox.QUERY_ALL_OUTBOX) { @@ -302,6 +313,7 @@ public class Utility { type = Mailbox.TYPE_DRAFTS; } else { type = Mailbox.TYPE_OUTBOX; + testFlagLoaded = false; } StringBuilder inboxes = new StringBuilder(); Cursor c = resolver.query(Mailbox.CONTENT_URI, @@ -309,7 +321,6 @@ public class Utility { MailboxColumns.TYPE + "=? AND " + MailboxColumns.FLAG_VISIBLE + "=1", new String[] { Integer.toString(type) }, null); // build an IN (mailboxId, ...) list - // TODO do this directly in the provider while (c.moveToNext()) { if (inboxes.length() != 0) { inboxes.append(","); @@ -325,7 +336,19 @@ public class Utility { selection.append(Message.FLAG_FAVORITE + "=1"); } else { selection.append(MessageColumns.MAILBOX_KEY + "=" + mailboxId); + if (Mailbox.getMailboxType(context, mailboxId) == Mailbox.TYPE_OUTBOX) { + testFlagLoaded = false; + } } + + if (testFlagLoaded) { + // POP messages at the initial stage have very little information. (Server UID only) + // This makes sure they're not visible in the message list. + // This means unread counts on the mailbox list can be different from the + // number of messages in the message list, but it should be transient... + selection.append(SELECTION_FLAG_LOADED_FOR_VISIBLE_MESSAGE); + } + return selection.toString(); } diff --git a/src/com/android/email/activity/MessageOrderManager.java b/src/com/android/email/activity/MessageOrderManager.java index 15d359306..571aaf9cd 100644 --- a/src/com/android/email/activity/MessageOrderManager.java +++ b/src/com/android/email/activity/MessageOrderManager.java @@ -265,7 +265,7 @@ public class MessageOrderManager { } /* package */ String getQuerySelection() { // Extracted for testing - return Utility.buildMailboxIdSelection(mContentResolver, mMailboxId); + return Utility.buildMailboxIdSelection(mContext, mMailboxId); } /** diff --git a/src/com/android/email/activity/MessagesAdapter.java b/src/com/android/email/activity/MessagesAdapter.java index 1cc965414..dcb87e75e 100644 --- a/src/com/android/email/activity/MessagesAdapter.java +++ b/src/com/android/email/activity/MessagesAdapter.java @@ -33,6 +33,7 @@ import android.content.res.Resources.Theme; import android.database.Cursor; import android.graphics.Typeface; import android.graphics.drawable.Drawable; +import android.net.Uri; import android.os.Bundle; import android.text.TextUtils; import android.util.Log; @@ -301,11 +302,30 @@ import java.util.Set; if (Email.DEBUG_LIFECYCLE && Email.DEBUG) { Log.d(Email.LOG_TAG, "MessagesAdapter createLoader mailboxId=" + mailboxId); } - String selection = - Utility.buildMailboxIdSelection(context.getContentResolver(), mailboxId); - return new ThrottlingCursorLoader(context, EmailContent.Message.CONTENT_URI, - MESSAGE_PROJECTION, selection, null, - EmailContent.MessageColumns.TIMESTAMP + " DESC", REFRESH_INTERVAL_MS); + return new MessagesCursor(context, mailboxId); } + + private static class MessagesCursor extends ThrottlingCursorLoader { + private final Context mContext; + private final long mMailboxId; + + public MessagesCursor(Context context, long mailboxId) { + // Initialize with no where clause. We'll set it later. + super(context, EmailContent.Message.CONTENT_URI, + MESSAGE_PROJECTION, null, null, + EmailContent.MessageColumns.TIMESTAMP + " DESC", REFRESH_INTERVAL_MS); + mContext = context; + mMailboxId = mailboxId; + } + + @Override + public Cursor loadInBackground() { + // Determine the where clause. (Can't do this on the UI thread.) + setSelection(Utility.buildMailboxIdSelection(mContext, mMailboxId)); + + // Then do a query. + return super.loadInBackground(); + } + } } diff --git a/tests/src/com/android/email/UtilityMediumTests.java b/tests/src/com/android/email/UtilityMediumTests.java index 223d49c2b..39f9dc731 100644 --- a/tests/src/com/android/email/UtilityMediumTests.java +++ b/tests/src/com/android/email/UtilityMediumTests.java @@ -21,6 +21,7 @@ import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.Attachment; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.Message; +import com.android.email.provider.EmailContent.MessageColumns; import com.android.email.provider.EmailProvider; import com.android.email.provider.ProviderTestUtils; @@ -185,4 +186,58 @@ public class UtilityMediumTests extends ProviderTestCase2 { null, EmailContent.ID_PROJECTION_COLUMN, -1)); } + + public void testBuildMailboxIdSelection() { + // Create dummy data... + Context c = mMockContext; + Account account1 = ProviderTestUtils.setupAccount("1", true, mMockContext); + Account account2 = ProviderTestUtils.setupAccount("X1", true, mMockContext); + + Mailbox box1in = ProviderTestUtils.setupMailbox("m", account1.mId, true, c, + Mailbox.TYPE_INBOX); + Mailbox box1out = ProviderTestUtils.setupMailbox("m", account1.mId, true, c, + Mailbox.TYPE_OUTBOX); + Mailbox box1d = ProviderTestUtils.setupMailbox("m", account1.mId, true, c, + Mailbox.TYPE_DRAFTS); + + Mailbox box2in = ProviderTestUtils.setupMailbox("m", account2.mId, true, c, + Mailbox.TYPE_INBOX); + Mailbox box2out = ProviderTestUtils.setupMailbox("m", account2.mId, true, c, + Mailbox.TYPE_OUTBOX); + Mailbox box2d = ProviderTestUtils.setupMailbox("m", account2.mId, true, c, + Mailbox.TYPE_DRAFTS); + + final String FLAG_LOADED_TEST = + " AND (" + + MessageColumns.FLAG_LOADED + " IN (" + + Message.FLAG_LOADED_PARTIAL + "," + Message.FLAG_LOADED_COMPLETE + + "))"; + + // Test! + + // Normal mailbox + assertEquals(MessageColumns.MAILBOX_KEY + "=" + box1in.mId + FLAG_LOADED_TEST, + Utility.buildMailboxIdSelection(mMockContext, box1in.mId)); + + // Outbox query doesn't have FLAG_LOADED_TEST + assertEquals(MessageColumns.MAILBOX_KEY + "=" + box1out.mId, + Utility.buildMailboxIdSelection(mMockContext, box1out.mId)); + + // Combined mailboxes + assertEquals(Message.FLAG_READ + "=0" + FLAG_LOADED_TEST, + Utility.buildMailboxIdSelection(mMockContext, Mailbox.QUERY_ALL_UNREAD)); + assertEquals(Message.FLAG_FAVORITE + "=1" + FLAG_LOADED_TEST, + Utility.buildMailboxIdSelection(mMockContext, Mailbox.QUERY_ALL_FAVORITES)); + + assertEquals(MessageColumns.MAILBOX_KEY + " IN (" + box1in.mId + "," + box2in.mId + ")" + + FLAG_LOADED_TEST, + Utility.buildMailboxIdSelection(mMockContext, Mailbox.QUERY_ALL_INBOXES)); + + assertEquals(MessageColumns.MAILBOX_KEY + " IN (" + box1d.mId + "," + box2d.mId + ")" + + FLAG_LOADED_TEST, + Utility.buildMailboxIdSelection(mMockContext, Mailbox.QUERY_ALL_DRAFTS)); + + assertEquals(MessageColumns.MAILBOX_KEY + " IN (" + box1out.mId + "," + box2out.mId + ")", + Utility.buildMailboxIdSelection(mMockContext, Mailbox.QUERY_ALL_OUTBOX)); + } }