Notify only account cursors when resetting new msg count

- Resetting the new message count is now correctly done on a BG thread.
- Added special content provider URI to reset the count.
  (/resetNewMessageCount)
- This URI only supports update, which will notify only account
  cursors.
- Fixed a problem that an insert with MAILBOX_ID/MESSAGE_ID/ACCOUNT_ID
  triggers two notifications.

- This CL changes how we use notification URIs, but unfortunately
  no tests for this part.  It turned out MockContentResolver doesn't
  support the notification mechanism, which made it very hard
  to write tests.

Bug 2911646

Change-Id: I35b30a7e6bf2d57510486c7ed19b9f263d8c9b58
This commit is contained in:
Makoto Onuki 2010-09-14 16:28:50 -07:00
parent cc91619b6a
commit 261d6c3f0c
5 changed files with 118 additions and 39 deletions

View File

@ -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());
}
}

View File

@ -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;

View File

@ -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;
}

View File

@ -112,15 +112,6 @@ public class MailService extends Service {
/*package*/ static HashMap<Long,AccountSyncReport> mSyncReports =
new HashMap<Long,AccountSyncReport>();
/**
* 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);
}
});
}
/**

View File

@ -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<EmailProvider> {
@ -2083,4 +2090,64 @@ public class ProviderTests extends ProviderTestCase2<EmailProvider> {
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);
}
}