diff --git a/src/com/android/email/activity/MessageListFragment.java b/src/com/android/email/activity/MessageListFragment.java index b1879b153..455e0a321 100644 --- a/src/com/android/email/activity/MessageListFragment.java +++ b/src/com/android/email/activity/MessageListFragment.java @@ -889,15 +889,6 @@ public class MessageListFragment extends ListFragment "MessageListFragment onCreateLoader(messages) mailboxId=" + mMailboxId); } - // Reset new message count. - // TODO Do it in onLoadFinished(). Unfortunately - // resetNewMessageCount() ends up a - // db operation, which causes a onContentChanged notification, which - // makes cursor - // loaders to requery. Until we fix ContentProvider (don't notify - // unrelated cursors) - // we need to do it here. - resetNewMessageCount(mActivity, mMailboxId, getAccountId()); return MessagesAdapter.createLoader(getActivity(), mMailboxId); } @@ -933,6 +924,8 @@ public class MessageListFragment extends ListFragment // Restore the state -- it has to be the last. // (Some of the "post processing" resets the state.) lss.restore(lv); + + resetNewMessageCount(mActivity, mMailboxId, getAccountId()); } } diff --git a/src/com/android/email/provider/EmailContent.java b/src/com/android/email/provider/EmailContent.java index aa8b8451b..911eef380 100644 --- a/src/com/android/email/provider/EmailContent.java +++ b/src/com/android/email/provider/EmailContent.java @@ -869,6 +869,8 @@ public abstract class EmailContent { public static final Uri CONTENT_URI = Uri.parse(EmailContent.CONTENT_URI + "/account"); public static final Uri ADD_TO_FIELD_URI = Uri.parse(EmailContent.CONTENT_URI + "/accountIdAddToField"); + public static final Uri RESET_NEW_MESSAGE_COUNT_URI = + Uri.parse(EmailContent.CONTENT_URI + "/resetNewMessageCount"); public final static int FLAGS_NOTIFY_NEW_MAIL = 1; public final static int FLAGS_VIBRATE_ALWAYS = 2; diff --git a/src/com/android/email/provider/EmailProvider.java b/src/com/android/email/provider/EmailProvider.java index 64f175e81..2e64a2300 100644 --- a/src/com/android/email/provider/EmailProvider.java +++ b/src/com/android/email/provider/EmailProvider.java @@ -108,6 +108,8 @@ public class EmailProvider extends ContentProvider { private static final int ACCOUNT = ACCOUNT_BASE; private static final int ACCOUNT_ID = ACCOUNT_BASE + 1; private static final int ACCOUNT_ID_ADD_TO_FIELD = ACCOUNT_BASE + 2; + private static final int ACCOUNT_RESET_NEW_COUNT = ACCOUNT_BASE + 3; + private static final int ACCOUNT_RESET_NEW_COUNT_ID = ACCOUNT_BASE + 4; private static final int MAILBOX_BASE = 0x1000; private static final int MAILBOX = MAILBOX_BASE; @@ -144,7 +146,6 @@ public class EmailProvider extends ContentProvider { private static final int BODY = BODY_BASE; private static final int BODY_ID = BODY_BASE + 1; - private static final int BASE_SHIFT = 12; // 12 bits to the base type: 0, 0x1000, 0x2000, etc. private static final String[] TABLE_NAMES = { @@ -196,6 +197,8 @@ public class EmailProvider extends ContentProvider { " where " + MessageColumns.MAILBOX_KEY + "=old." + EmailContent.RECORD_ID + "; end"; + private static final ContentValues CONTENT_VALUES_RESET_NEW_MESSAGE_COUNT; + static { // Email URI matching table UriMatcher matcher = sURIMatcher; @@ -206,6 +209,11 @@ public class EmailProvider extends ContentProvider { // insert into this URI causes a mailbox to be added to the account matcher.addURI(EMAIL_AUTHORITY, "account/#", ACCOUNT_ID); + // Special URI to reset the new message count. Only update works, and content values + // will be ignored. + matcher.addURI(EMAIL_AUTHORITY, "resetNewMessageCount", ACCOUNT_RESET_NEW_COUNT); + matcher.addURI(EMAIL_AUTHORITY, "resetNewMessageCount/#", ACCOUNT_RESET_NEW_COUNT_ID); + // All mailboxes matcher.addURI(EMAIL_AUTHORITY, "mailbox", MAILBOX); // A specific mailbox @@ -261,6 +269,9 @@ public class EmailProvider extends ContentProvider { matcher.addURI(EMAIL_AUTHORITY, "updatedMessage", UPDATED_MESSAGE); // A specific updated message matcher.addURI(EMAIL_AUTHORITY, "updatedMessage/#", UPDATED_MESSAGE_ID); + + CONTENT_VALUES_RESET_NEW_MESSAGE_COUNT = new ContentValues(); + CONTENT_VALUES_RESET_NEW_MESSAGE_COUNT.put(Account.NEW_MESSAGE_COUNT, 0); } /* @@ -962,7 +973,9 @@ public class EmailProvider extends ContentProvider { db.endTransaction(); } } - getContext().getContentResolver().notifyChange(uri, null); + + // Notify all existing cursors. + getContext().getContentResolver().notifyChange(EmailContent.CONTENT_URI, null); return result; } @@ -1058,20 +1071,17 @@ public class EmailProvider extends ContentProvider { // already in the values... id = Long.parseLong(uri.getPathSegments().get(1)); values.put(MessageColumns.MAILBOX_KEY, id); - resultUri = insert(Message.CONTENT_URI, values); - break; + return insert(Message.CONTENT_URI, values); // Recurse case MESSAGE_ID: // This implies adding an attachment to a message. id = Long.parseLong(uri.getPathSegments().get(1)); values.put(AttachmentColumns.MESSAGE_KEY, id); - resultUri = insert(Attachment.CONTENT_URI, values); - break; + return insert(Attachment.CONTENT_URI, values); // Recurse case ACCOUNT_ID: // This implies adding a mailbox to an account. id = Long.parseLong(uri.getPathSegments().get(1)); values.put(MailboxColumns.ACCOUNT_KEY, id); - resultUri = insert(Mailbox.CONTENT_URI, values); - break; + return insert(Mailbox.CONTENT_URI, values); // Recurse case ATTACHMENTS_MESSAGE_ID: id = db.insert(TABLE_NAMES[table], "foo", values); resultUri = ContentUris.withAppendedId(Attachment.CONTENT_URI, id); @@ -1084,8 +1094,8 @@ public class EmailProvider extends ContentProvider { throw e; } - // Notify with the base uri, not the new uri (nobody is watching a new record) - getContext().getContentResolver().notifyChange(uri, null); + // Notify all existing cursors. + getContext().getContentResolver().notifyChange(EmailContent.CONTENT_URI, null); return resultUri; } @@ -1128,7 +1138,6 @@ public class EmailProvider extends ContentProvider { String sortOrder) { if (Email.DEBUG_THREAD_CHECK) Email.warnIfUiThread(); Cursor c = null; - Uri notificationUri = EmailContent.CONTENT_URI; int match = sURIMatcher.match(uri); Context context = getContext(); // See the comment at delete(), above @@ -1181,7 +1190,7 @@ public class EmailProvider extends ContentProvider { } if ((c != null) && !isTemporary()) { - c.setNotificationUri(getContext().getContentResolver(), notificationUri); + c.setNotificationUri(getContext().getContentResolver(), uri); } return c; } @@ -1230,6 +1239,9 @@ public class EmailProvider extends ContentProvider { return 0; } + // Notify all existing cursors, except for ACCOUNT_RESET_NEW_COUNT(_ID) + Uri notificationUri = EmailContent.CONTENT_URI; + int match = sURIMatcher.match(uri); Context context = getContext(); // See the comment at delete(), above @@ -1317,6 +1329,17 @@ public class EmailProvider extends ContentProvider { case HOSTAUTH: result = db.update(TABLE_NAMES[table], values, selection, selectionArgs); break; + case ACCOUNT_RESET_NEW_COUNT_ID: + id = uri.getPathSegments().get(1); + result = db.update(TABLE_NAMES[table], CONTENT_VALUES_RESET_NEW_MESSAGE_COUNT, + whereWithId(id, selection), selectionArgs); + notificationUri = Account.CONTENT_URI; // Only notify account cursors. + break; + case ACCOUNT_RESET_NEW_COUNT: + result = db.update(TABLE_NAMES[table], CONTENT_VALUES_RESET_NEW_MESSAGE_COUNT, + selection, selectionArgs); + notificationUri = Account.CONTENT_URI; // Only notify account cursors. + break; default: throw new IllegalArgumentException("Unknown URI " + uri); } @@ -1325,7 +1348,7 @@ public class EmailProvider extends ContentProvider { throw e; } - context.getContentResolver().notifyChange(uri, null); + context.getContentResolver().notifyChange(notificationUri, null); return result; } diff --git a/src/com/android/email/service/MailService.java b/src/com/android/email/service/MailService.java index 82e6d9dfd..55dffb67c 100644 --- a/src/com/android/email/service/MailService.java +++ b/src/com/android/email/service/MailService.java @@ -112,15 +112,6 @@ public class MailService extends Service { /*package*/ static HashMap mSyncReports = new HashMap(); - /** - * Simple template used for clearing new message count in accounts - */ - private static final ContentValues CLEAR_NEW_MESSAGES; - static { - CLEAR_NEW_MESSAGES = new ContentValues(); - CLEAR_NEW_MESSAGES.put(Account.NEW_MESSAGE_COUNT, 0); - } - public static void actionReschedule(Context context) { Intent i = new Intent(); i.setClass(context, MailService.class); @@ -154,7 +145,7 @@ public class MailService extends Service { * * @param accountId account to clear, or -1 for all accounts */ - public static void resetNewMessageCount(Context context, long accountId) { + public static void resetNewMessageCount(final Context context, final long accountId) { synchronized (mSyncReports) { for (AccountSyncReport report : mSyncReports.values()) { if (accountId == -1 || accountId == report.accountId) { @@ -163,13 +154,16 @@ public class MailService extends Service { } } // now do the database - all accounts, or just one of them - Uri uri; - if (accountId == -1) { - uri = Account.CONTENT_URI; - } else { - uri = ContentUris.withAppendedId(Account.CONTENT_URI, accountId); - } - context.getContentResolver().update(uri, CLEAR_NEW_MESSAGES, null, null); + Utility.runAsync(new Runnable() { + @Override + public void run() { + Uri uri = Account.RESET_NEW_MESSAGE_COUNT_URI; + if (accountId != -1) { + uri = ContentUris.withAppendedId(uri, accountId); + } + context.getContentResolver().update(uri, null, null, null); + } + }); } /** diff --git a/tests/src/com/android/email/provider/ProviderTests.java b/tests/src/com/android/email/provider/ProviderTests.java index 4704afc8b..9d93e8421 100644 --- a/tests/src/com/android/email/provider/ProviderTests.java +++ b/tests/src/com/android/email/provider/ProviderTests.java @@ -51,6 +51,13 @@ import java.util.ArrayList; * * You can run this entire test case with: * runtest -c com.android.email.provider.ProviderTests email + * + * TODO: Add tests for cursor notification mechanism. (setNotificationUri and notifyChange) + * We can't test the entire notification mechanism with a mock content resolver, because which URI + * to notify when notifyChange() is called is in the actual content resolver. + * Implementing the same mechanism in a mock one is pointless. Instead what we could do is check + * what notification URI each cursor has, and with which URI is notified when + * inserting/updating/deleting. (The former require a new method from AbstractCursor) */ public class ProviderTests extends ProviderTestCase2 { @@ -2083,4 +2090,64 @@ public class ProviderTests extends ProviderTestCase2 { assertFalse(Mailbox.canMoveFrom(c, bd.mId)); assertFalse(Mailbox.canMoveFrom(c, bo.mId)); } + + /** + * Check if update to {@link Account#RESET_NEW_MESSAGE_COUNT_URI} resets the new message count. + */ + public void testResetNewMessageCount() { + final Context c = mMockContext; + final ContentResolver cr = c.getContentResolver(); + + // Prepare test data + Account a1 = ProviderTestUtils.setupAccount("acct1", false, c); + a1.mNewMessageCount = 1; + a1.save(c); + Account a2 = ProviderTestUtils.setupAccount("acct2", false, c); + a2.mNewMessageCount = 2; + a2.save(c); + Account a3 = ProviderTestUtils.setupAccount("acct3", false, c); + a3.mNewMessageCount = 3; + a3.save(c); + Account a4 = ProviderTestUtils.setupAccount("acct4", false, c); + a4.mNewMessageCount = 4; + a4.save(c); + Account a5 = ProviderTestUtils.setupAccount("acct5", false, c); + a5.mNewMessageCount = 5; + a5.save(c); + + // With ID in URI, no selection + cr.update(ContentUris.withAppendedId(Account.RESET_NEW_MESSAGE_COUNT_URI, a1.mId), + null, null, null); + assertEquals(0, Account.restoreAccountWithId(c, a1.mId).mNewMessageCount); + assertEquals(2, Account.restoreAccountWithId(c, a2.mId).mNewMessageCount); + assertEquals(3, Account.restoreAccountWithId(c, a3.mId).mNewMessageCount); + assertEquals(4, Account.restoreAccountWithId(c, a4.mId).mNewMessageCount); + assertEquals(5, Account.restoreAccountWithId(c, a5.mId).mNewMessageCount); + + // No ID in URI, with selection + cr.update(Account.RESET_NEW_MESSAGE_COUNT_URI, null, + EmailContent.ID_SELECTION, new String[] {Long.toString(a2.mId)}); + assertEquals(0, Account.restoreAccountWithId(c, a1.mId).mNewMessageCount); + assertEquals(0, Account.restoreAccountWithId(c, a2.mId).mNewMessageCount); + assertEquals(3, Account.restoreAccountWithId(c, a3.mId).mNewMessageCount); + assertEquals(4, Account.restoreAccountWithId(c, a4.mId).mNewMessageCount); + assertEquals(5, Account.restoreAccountWithId(c, a5.mId).mNewMessageCount); + + // With ID, with selection + cr.update(ContentUris.withAppendedId(Account.RESET_NEW_MESSAGE_COUNT_URI, a3.mId), null, + EmailContent.ID_SELECTION, new String[] {Long.toString(a3.mId)}); + assertEquals(0, Account.restoreAccountWithId(c, a1.mId).mNewMessageCount); + assertEquals(0, Account.restoreAccountWithId(c, a2.mId).mNewMessageCount); + assertEquals(0, Account.restoreAccountWithId(c, a3.mId).mNewMessageCount); + assertEquals(4, Account.restoreAccountWithId(c, a4.mId).mNewMessageCount); + assertEquals(5, Account.restoreAccountWithId(c, a5.mId).mNewMessageCount); + + // No ID in URI, no selection + cr.update(Account.RESET_NEW_MESSAGE_COUNT_URI, null, null, null); + assertEquals(0, Account.restoreAccountWithId(c, a1.mId).mNewMessageCount); + assertEquals(0, Account.restoreAccountWithId(c, a2.mId).mNewMessageCount); + assertEquals(0, Account.restoreAccountWithId(c, a3.mId).mNewMessageCount); + assertEquals(0, Account.restoreAccountWithId(c, a4.mId).mNewMessageCount); + assertEquals(0, Account.restoreAccountWithId(c, a5.mId).mNewMessageCount); + } }