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
This commit is contained in:
Makoto Onuki 2010-09-08 17:38:34 -07:00
parent 6d51b7eb63
commit 7183724276
4 changed files with 112 additions and 14 deletions

View File

@ -81,6 +81,12 @@ public class Utility {
private static final Pattern DATE_CLEANUP_PATTERN_WRONG_TIMEZONE = private static final Pattern DATE_CLEANUP_PATTERN_WRONG_TIMEZONE =
Pattern.compile("GMT([-+]\\d{4})$"); 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 { public final static String readInputStream(InputStream in, String encoding) throws IOException {
InputStreamReader reader = new InputStreamReader(in, encoding); InputStreamReader reader = new InputStreamReader(in, encoding);
StringBuffer sb = new StringBuffer(); StringBuffer sb = new StringBuffer();
@ -284,13 +290,18 @@ public class Utility {
// } // }
} }
// TODO: unit test this /**
public static String buildMailboxIdSelection(ContentResolver resolver, long mailboxId) { * Returns the where clause for a message list selection.
// Setup default selection & args, then add to it as necessary *
StringBuilder selection = new StringBuilder( * MUST NOT be called on the UI thread.
MessageColumns.FLAG_LOADED + " IN (" */
+ Message.FLAG_LOADED_PARTIAL + "," + Message.FLAG_LOADED_COMPLETE public static String buildMailboxIdSelection(Context context, long mailboxId) {
+ ") AND "); 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 if (mailboxId == Mailbox.QUERY_ALL_INBOXES
|| mailboxId == Mailbox.QUERY_ALL_DRAFTS || mailboxId == Mailbox.QUERY_ALL_DRAFTS
|| mailboxId == Mailbox.QUERY_ALL_OUTBOX) { || mailboxId == Mailbox.QUERY_ALL_OUTBOX) {
@ -302,6 +313,7 @@ public class Utility {
type = Mailbox.TYPE_DRAFTS; type = Mailbox.TYPE_DRAFTS;
} else { } else {
type = Mailbox.TYPE_OUTBOX; type = Mailbox.TYPE_OUTBOX;
testFlagLoaded = false;
} }
StringBuilder inboxes = new StringBuilder(); StringBuilder inboxes = new StringBuilder();
Cursor c = resolver.query(Mailbox.CONTENT_URI, Cursor c = resolver.query(Mailbox.CONTENT_URI,
@ -309,7 +321,6 @@ public class Utility {
MailboxColumns.TYPE + "=? AND " + MailboxColumns.FLAG_VISIBLE + "=1", MailboxColumns.TYPE + "=? AND " + MailboxColumns.FLAG_VISIBLE + "=1",
new String[] { Integer.toString(type) }, null); new String[] { Integer.toString(type) }, null);
// build an IN (mailboxId, ...) list // build an IN (mailboxId, ...) list
// TODO do this directly in the provider
while (c.moveToNext()) { while (c.moveToNext()) {
if (inboxes.length() != 0) { if (inboxes.length() != 0) {
inboxes.append(","); inboxes.append(",");
@ -325,7 +336,19 @@ public class Utility {
selection.append(Message.FLAG_FAVORITE + "=1"); selection.append(Message.FLAG_FAVORITE + "=1");
} else { } else {
selection.append(MessageColumns.MAILBOX_KEY + "=" + mailboxId); 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(); return selection.toString();
} }

View File

@ -265,7 +265,7 @@ public class MessageOrderManager {
} }
/* package */ String getQuerySelection() { // Extracted for testing /* package */ String getQuerySelection() { // Extracted for testing
return Utility.buildMailboxIdSelection(mContentResolver, mMailboxId); return Utility.buildMailboxIdSelection(mContext, mMailboxId);
} }
/** /**

View File

@ -33,6 +33,7 @@ import android.content.res.Resources.Theme;
import android.database.Cursor; import android.database.Cursor;
import android.graphics.Typeface; import android.graphics.Typeface;
import android.graphics.drawable.Drawable; import android.graphics.drawable.Drawable;
import android.net.Uri;
import android.os.Bundle; import android.os.Bundle;
import android.text.TextUtils; import android.text.TextUtils;
import android.util.Log; import android.util.Log;
@ -301,11 +302,30 @@ import java.util.Set;
if (Email.DEBUG_LIFECYCLE && Email.DEBUG) { if (Email.DEBUG_LIFECYCLE && Email.DEBUG) {
Log.d(Email.LOG_TAG, "MessagesAdapter createLoader mailboxId=" + mailboxId); Log.d(Email.LOG_TAG, "MessagesAdapter createLoader mailboxId=" + mailboxId);
} }
String selection = return new MessagesCursor(context, mailboxId);
Utility.buildMailboxIdSelection(context.getContentResolver(), mailboxId);
return new ThrottlingCursorLoader(context, EmailContent.Message.CONTENT_URI,
MESSAGE_PROJECTION, selection, null,
EmailContent.MessageColumns.TIMESTAMP + " DESC", REFRESH_INTERVAL_MS);
} }
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();
}
}
} }

View File

@ -21,6 +21,7 @@ import com.android.email.provider.EmailContent.Account;
import com.android.email.provider.EmailContent.Attachment; import com.android.email.provider.EmailContent.Attachment;
import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailContent.Mailbox;
import com.android.email.provider.EmailContent.Message; 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.EmailProvider;
import com.android.email.provider.ProviderTestUtils; import com.android.email.provider.ProviderTestUtils;
@ -185,4 +186,58 @@ public class UtilityMediumTests extends ProviderTestCase2<EmailProvider> {
null, null,
EmailContent.ID_PROJECTION_COLUMN, -1)); 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));
}
} }