diff options
3 files changed, 176 insertions, 67 deletions
diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java index b55aeb7..e594eb0 100644 --- a/src/com/android/providers/contacts/ContactsProvider2.java +++ b/src/com/android/providers/contacts/ContactsProvider2.java @@ -1502,6 +1502,8 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun private HashSet<Long> mUpdatedRawContacts = Sets.newHashSet(); private HashMap<Long, Object> mUpdatedSyncStates = Maps.newHashMap(); + private boolean mVisibleTouched = false; + private boolean mSyncToNetwork; public ContactsProvider2() { @@ -1814,6 +1816,10 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun super.beforeTransactionCommit(); flushTransactionalChanges(); mContactAggregator.aggregateInTransaction(mDb); + if (mVisibleTouched) { + mVisibleTouched = false; + mOpenHelper.updateAllVisible(); + } } private void flushTransactionalChanges() { @@ -2219,31 +2225,6 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun } /** - * Determine if the given {@link Uri} and {@link ContentValues} should - * trigger a call to {@link OpenHelper#updateAllVisible()}, usually based on - * query parameters like {@link Contacts#DELAY_STARRED_UPDATE} and columns - * like {@link Groups#GROUP_VISIBLE}. - */ - private boolean shouldUpdateAllVisible(Uri uri, ContentValues values, String visibleColumn) { - final boolean delayUpdate = readBooleanQueryParameter(uri, - Contacts.DELAY_STARRED_UPDATE, false); - final boolean forceUpdate = readBooleanQueryParameter(uri, - Contacts.FORCE_STARRED_UPDATE, false); - final boolean touchedVisible = (values == null) ? true : values.containsKey(visibleColumn); - - return forceUpdate || (!delayUpdate && touchedVisible); - } - - /** - * Determine if the given {@link Uri} and {@link ContentValues} should - * trigger a call to {@link OpenHelper#updateAllVisible()}, usually based on - * query parameters like {@link Contacts#DELAY_STARRED_UPDATE}. - */ - private boolean shouldUpdateAllVisible(Uri uri) { - return shouldUpdateAllVisible(uri, null, null); - } - - /** * Inserts an item in the groups table */ private long insertGroup(Uri uri, ContentValues values, Account account, boolean callerIsSyncAdapter) { @@ -2265,8 +2246,8 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun long result = mDb.insert(Tables.GROUPS, Groups.TITLE, overriddenValues); - if (shouldUpdateAllVisible(uri, overriddenValues, Groups.GROUP_VISIBLE)) { - mOpenHelper.updateAllVisible(); + if (overriddenValues.containsKey(Groups.GROUP_VISIBLE)) { + mVisibleTouched = true; } return result; @@ -2275,9 +2256,10 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun private long insertSettings(Uri uri, ContentValues values) { final long id = mDb.insert(Tables.SETTINGS, null, values); - if (shouldUpdateAllVisible(uri, values, Settings.UNGROUPED_VISIBLE)) { - mOpenHelper.updateAllVisible(); + if (values.containsKey(Settings.UNGROUPED_VISIBLE)) { + mVisibleTouched = true; } + return id; } @@ -2573,17 +2555,13 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun return mDb.update(Tables.GROUPS, mValues, Groups._ID + "=" + groupId, null); } } finally { - if (shouldUpdateAllVisible(uri)) { - mOpenHelper.updateAllVisible(); - } + mVisibleTouched = true; } } private int deleteSettings(Uri uri, String selection, String[] selectionArgs) { final int count = mDb.delete(Tables.SETTINGS, selection, selectionArgs); - if (count > 0 && shouldUpdateAllVisible(uri)) { - mOpenHelper.updateAllVisible(); - } + mVisibleTouched = true; return count; } @@ -2782,18 +2760,16 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun } int count = mDb.update(Tables.GROUPS, updatedValues, selectionWithId, selectionArgs); - - // If changing visibility, then update contacts - if (shouldUpdateAllVisible(uri, updatedValues, Groups.GROUP_VISIBLE)) { - mOpenHelper.updateAllVisible(); + if (updatedValues.containsKey(Groups.GROUP_VISIBLE)) { + mVisibleTouched = true; } return count; } private int updateSettings(Uri uri, ContentValues values, String selection, String[] selectionArgs) { final int count = mDb.update(Tables.SETTINGS, values, selection, selectionArgs); - if (shouldUpdateAllVisible(uri, values, Settings.UNGROUPED_VISIBLE)) { - mOpenHelper.updateAllVisible(); + if (values.containsKey(Settings.UNGROUPED_VISIBLE)) { + mVisibleTouched = true; } return count; } @@ -4198,7 +4174,8 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun Groups.SYNC4, Groups.SYSTEM_ID, Groups.NOTES, - Groups.DELETED}; + Groups.DELETED, + Groups.SHOULD_SYNC}; private static final int COLUMN_ID = 0; private static final int COLUMN_ACCOUNT_NAME = 1; @@ -4217,6 +4194,7 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun private static final int COLUMN_SYSTEM_ID = 14; private static final int COLUMN_NOTES = 15; private static final int COLUMN_DELETED = 16; + private static final int COLUMN_SHOULD_SYNC = 17; public GroupsEntityIterator(ContactsProvider2 provider, String groupIdString, Uri uri, String selection, String[] selectionArgs, String sortOrder) { @@ -4300,6 +4278,7 @@ public class ContactsProvider2 extends SQLiteContentProvider implements OnAccoun groupValues.put(Groups.SYSTEM_ID, c.getString(COLUMN_SYSTEM_ID)); groupValues.put(Groups.DELETED, c.getLong(COLUMN_DELETED)); groupValues.put(Groups.NOTES, c.getString(COLUMN_NOTES)); + groupValues.put(Groups.SHOULD_SYNC, c.getString(COLUMN_SHOULD_SYNC)); Entity group = new Entity(groupValues); mEntityCursor.moveToNext(); diff --git a/src/com/android/providers/contacts/OpenHelper.java b/src/com/android/providers/contacts/OpenHelper.java index 93d8d0e..b39a365 100644 --- a/src/com/android/providers/contacts/OpenHelper.java +++ b/src/com/android/providers/contacts/OpenHelper.java @@ -217,9 +217,12 @@ import java.util.HashMap; final String ZERO_GROUP_MEMBERSHIPS = "COUNT(" + GroupsColumns.CONCRETE_ID + ")=0"; + final String OUTER_RAW_CONTACTS = "outer_raw_contacts"; + final String OUTER_RAW_CONTACTS_ID = OUTER_RAW_CONTACTS + "." + RawContacts._ID; + final String CONTACT_IS_VISIBLE = "SELECT " + - "(CASE WHEN " + + "MAX((SELECT (CASE WHEN " + "(CASE" + " WHEN " + RAW_CONTACT_IS_LOCAL + " THEN 1 " + @@ -228,6 +231,8 @@ import java.util.HashMap; " ELSE MAX(" + Groups.GROUP_VISIBLE + ")" + "END)=1 THEN 1 ELSE 0 END)" + " FROM " + Tables.RAW_CONTACTS_JOIN_SETTINGS_DATA_GROUPS + + " WHERE " + RawContactsColumns.CONCRETE_ID + "=" + OUTER_RAW_CONTACTS_ID + "))" + + " FROM " + Tables.RAW_CONTACTS + " AS " + OUTER_RAW_CONTACTS + " WHERE " + RawContacts.CONTACT_ID + "=" + ContactsColumns.CONCRETE_ID + " GROUP BY " + RawContacts.CONTACT_ID; diff --git a/tests/src/com/android/providers/contacts/GroupsTest.java b/tests/src/com/android/providers/contacts/GroupsTest.java index a4e5188..13f3a19 100644 --- a/tests/src/com/android/providers/contacts/GroupsTest.java +++ b/tests/src/com/android/providers/contacts/GroupsTest.java @@ -16,18 +16,26 @@ package com.android.providers.contacts; +import com.google.android.collect.Lists; + import android.accounts.Account; +import android.content.ContentProviderOperation; import android.content.ContentUris; import android.content.ContentValues; +import android.content.OperationApplicationException; import android.database.Cursor; import android.net.Uri; +import android.os.RemoteException; +import android.provider.ContactsContract; +import android.provider.ContactsContract.AggregationExceptions; import android.provider.ContactsContract.Contacts; import android.provider.ContactsContract.Groups; -import android.provider.ContactsContract.RawContacts; +import android.provider.ContactsContract.Settings; import android.provider.ContactsContract.CommonDataKinds.GroupMembership; -import android.provider.ContactsContract; import android.test.suitebuilder.annotation.LargeTest; +import java.util.ArrayList; + /** * Unit tests for {@link Groups} and {@link GroupMembership}. * @@ -216,49 +224,166 @@ public class GroupsTest extends BaseContactsProvider2Test { } private static final Account sTestAccount = new Account("user@example.com", "com.example"); + private static final Account sSecondAccount = new Account("other@example.net", "net.example"); private static final String GROUP_ID = "testgroup"; - public boolean queryContactVisible(Uri contactUri) { - final Cursor cursor = mResolver.query(contactUri, new String[] { + public void assertRawContactVisible(long rawContactId, boolean expected) { + final long contactId = this.queryContactId(rawContactId); + assertContactVisible(contactId, expected); + } + + public void assertContactVisible(long contactId, boolean expected) { + final Cursor cursor = mResolver.query(Contacts.CONTENT_URI, new String[] { Contacts.IN_VISIBLE_GROUP - }, null, null, null); + }, Contacts._ID + "=" + contactId, null, null); assertTrue("Contact not found", cursor.moveToFirst()); - final boolean visible = (cursor.getInt(0) != 0); + final boolean actual = (cursor.getInt(0) != 0); cursor.close(); - return visible; + assertEquals("Unexpected visibility", expected, actual); + } + + public ContentProviderOperation buildVisibleAssert(long contactId, boolean visible) { + return ContentProviderOperation.newAssertQuery(Contacts.CONTENT_URI).withSelection( + Contacts._ID + "=" + contactId + " AND " + Contacts.IN_VISIBLE_GROUP + "=" + + (visible ? 1 : 0), null).withExpectedCount(1).build(); } - public void testDelayStarredUpdate() { + public void testDelayVisibleTransaction() throws RemoteException, OperationApplicationException { final ContentValues values = new ContentValues(); final long groupId = this.createGroup(sTestAccount, GROUP_ID, GROUP_ID, 1); final Uri groupUri = ContentUris.withAppendedId(Groups.CONTENT_URI, groupId); - final Uri groupUriDelay = groupUri.buildUpon().appendQueryParameter( - Contacts.DELAY_STARRED_UPDATE, "1").build(); - final Uri groupUriForce = Groups.CONTENT_URI.buildUpon().appendQueryParameter( - Contacts.FORCE_STARRED_UPDATE, "1").build(); - // Create contact with specific membership in visible group + // Create contact with specific membership final long rawContactId = this.createRawContact(sTestAccount); final long contactId = this.queryContactId(rawContactId); final Uri contactUri = ContentUris.withAppendedId(Contacts.CONTENT_URI, contactId); this.insertGroupMembership(rawContactId, groupId); - // Update the group visibility normally - assertTrue(queryContactVisible(contactUri)); - values.put(Groups.GROUP_VISIBLE, 0); - mResolver.update(groupUri, values, null, null); - assertFalse(queryContactVisible(contactUri)); + final ArrayList<ContentProviderOperation> oper = Lists.newArrayList(); + + // Update visibility inside a transaction and assert that inside the + // transaction it hasn't been updated yet. + oper.add(buildVisibleAssert(contactId, true)); + oper.add(ContentProviderOperation.newUpdate(groupUri).withValue(Groups.GROUP_VISIBLE, 0) + .build()); + oper.add(buildVisibleAssert(contactId, true)); + mResolver.applyBatch(ContactsContract.AUTHORITY, oper); + + // After previous transaction finished, visibility should be updated + oper.clear(); + oper.add(buildVisibleAssert(contactId, false)); + mResolver.applyBatch(ContactsContract.AUTHORITY, oper); + } + + public void testLocalSingleVisible() { + final long rawContactId = this.createRawContact(); + + // Single, local contacts should always be visible + assertRawContactVisible(rawContactId, true); + } + + public void testLocalMixedVisible() { + // Aggregate, when mixed with local, should become visible + final long rawContactId1 = this.createRawContact(); + final long rawContactId2 = this.createRawContact(sTestAccount); + + final long groupId = this.createGroup(sTestAccount, GROUP_ID, GROUP_ID, 0); + this.insertGroupMembership(rawContactId2, groupId); + + // Make sure they are still apart + assertNotAggregated(rawContactId1, rawContactId2); + assertRawContactVisible(rawContactId1, true); + assertRawContactVisible(rawContactId2, false); + + // Force together and see what happens + final ContentValues values = new ContentValues(); + values.put(AggregationExceptions.TYPE, AggregationExceptions.TYPE_KEEP_TOGETHER); + values.put(AggregationExceptions.RAW_CONTACT_ID1, rawContactId1); + values.put(AggregationExceptions.RAW_CONTACT_ID2, rawContactId2); + mResolver.update(AggregationExceptions.CONTENT_URI, values, null, null); + + assertRawContactVisible(rawContactId1, true); + assertRawContactVisible(rawContactId2, true); + } + + public void testUngroupedVisible() { + final long rawContactId = this.createRawContact(sTestAccount); + + final ContentValues values = new ContentValues(); + values.put(Settings.ACCOUNT_NAME, sTestAccount.name); + values.put(Settings.ACCOUNT_TYPE, sTestAccount.type); + values.put(Settings.UNGROUPED_VISIBLE, 0); + mResolver.insert(Settings.CONTENT_URI, values); + + assertRawContactVisible(rawContactId, false); + + values.clear(); + values.put(Settings.UNGROUPED_VISIBLE, 1); + mResolver.update(Settings.CONTENT_URI, values, Settings.ACCOUNT_NAME + "=? AND " + + Settings.ACCOUNT_TYPE + "=?", new String[] { + sTestAccount.name, sTestAccount.type + }); - // Update visibility using delayed approach + assertRawContactVisible(rawContactId, true); + } + + public void testMultipleSourcesVisible() { + final long rawContactId1 = this.createRawContact(sTestAccount); + final long rawContactId2 = this.createRawContact(sSecondAccount); + + final long groupId = this.createGroup(sTestAccount, GROUP_ID, GROUP_ID, 0); + this.insertGroupMembership(rawContactId1, groupId); + + // Make sure still invisible + assertRawContactVisible(rawContactId1, false); + assertRawContactVisible(rawContactId2, false); + + // Make group visible + final ContentValues values = new ContentValues(); values.put(Groups.GROUP_VISIBLE, 1); - mResolver.update(groupUriDelay, values, null, null); - assertFalse(queryContactVisible(contactUri)); + mResolver.update(Groups.CONTENT_URI, values, Groups._ID + "=" + groupId, null); - // Force update and verify results + assertRawContactVisible(rawContactId1, true); + assertRawContactVisible(rawContactId2, false); + + // Force them together + values.clear(); + values.put(AggregationExceptions.TYPE, AggregationExceptions.TYPE_KEEP_TOGETHER); + values.put(AggregationExceptions.RAW_CONTACT_ID1, rawContactId1); + values.put(AggregationExceptions.RAW_CONTACT_ID2, rawContactId2); + mResolver.update(AggregationExceptions.CONTENT_URI, values, null, null); + + assertRawContactVisible(rawContactId1, true); + assertRawContactVisible(rawContactId2, true); + + // Make group invisible + values.clear(); + values.put(Groups.GROUP_VISIBLE, 0); + mResolver.update(Groups.CONTENT_URI, values, Groups._ID + "=" + groupId, null); + + assertRawContactVisible(rawContactId1, false); + assertRawContactVisible(rawContactId2, false); + + // Turn on ungrouped for first + values.clear(); + values.put(Settings.ACCOUNT_NAME, sTestAccount.name); + values.put(Settings.ACCOUNT_TYPE, sTestAccount.type); + values.put(Settings.UNGROUPED_VISIBLE, 1); + mResolver.insert(Settings.CONTENT_URI, values); + + assertRawContactVisible(rawContactId1, false); + assertRawContactVisible(rawContactId2, false); + + // Turn on ungrouped for second account values.clear(); - mResolver.update(groupUriForce, values, null, null); - assertTrue(queryContactVisible(contactUri)); + values.put(Settings.ACCOUNT_NAME, sSecondAccount.name); + values.put(Settings.ACCOUNT_TYPE, sSecondAccount.type); + values.put(Settings.UNGROUPED_VISIBLE, 1); + mResolver.insert(Settings.CONTENT_URI, values); + + assertRawContactVisible(rawContactId1, true); + assertRawContactVisible(rawContactId2, true); } } |