Account refactoring follow-up

- Check if there's really a new or removed account in
updateAccountsInBackground() before doing everything else.
If there's none, we'll skip re-scanning directories too.

- Preheat the account cache when the provider starts.

- Use account_id for the account filter query parameters.  The contacts URI
now supports the parameters too.

Change-Id: I12e1116890df4c20b354618acfaee9dc009dc68e
diff --git a/src/com/android/providers/contacts/AccountWithDataSet.java b/src/com/android/providers/contacts/AccountWithDataSet.java
index 410e1cf..3fea8a6 100644
--- a/src/com/android/providers/contacts/AccountWithDataSet.java
+++ b/src/com/android/providers/contacts/AccountWithDataSet.java
@@ -19,6 +19,7 @@
 import com.android.internal.util.Objects;
 
 import android.accounts.Account;
+import android.text.TextUtils;
 
 /**
  * Account information that includes the data set, if any.
@@ -31,9 +32,13 @@
     private final String mDataSet;
 
     public AccountWithDataSet(String accountName, String accountType, String dataSet) {
-        mAccountName = accountName;
-        mAccountType = accountType;
-        mDataSet = dataSet;
+        mAccountName = emptyToNull(accountName);
+        mAccountType = emptyToNull(accountType);
+        mDataSet = emptyToNull(dataSet);
+    }
+
+    private static final String emptyToNull(String text) {
+        return TextUtils.isEmpty(text) ? null : text;
     }
 
     public static AccountWithDataSet get(String accountName, String accountType, String dataSet) {
diff --git a/src/com/android/providers/contacts/ContactDirectoryManager.java b/src/com/android/providers/contacts/ContactDirectoryManager.java
index 1473679..25494f8 100644
--- a/src/com/android/providers/contacts/ContactDirectoryManager.java
+++ b/src/com/android/providers/contacts/ContactDirectoryManager.java
@@ -16,6 +16,7 @@
 
 package com.android.providers.contacts;
 
+import com.android.providers.contacts.ContactsDatabaseHelper.DbProperties;
 import com.android.providers.contacts.ContactsDatabaseHelper.DirectoryColumns;
 import com.android.providers.contacts.ContactsDatabaseHelper.Tables;
 import com.google.android.collect.Lists;
@@ -53,7 +54,6 @@
     private static final String TAG = "ContactDirectoryManager";
     private static final boolean DEBUG = false; // DON'T SUBMIT WITH TRUE
 
-    public static final String PROPERTY_DIRECTORY_SCAN_COMPLETE = "directoryScanComplete";
     public static final String CONTACT_DIRECTORY_META_DATA = "android.content.ContactDirectory";
 
     public class DirectoryInfo {
@@ -176,21 +176,21 @@
      */
     public void scanAllPackages(boolean rescan) {
         if (rescan || !areTypeResourceIdsValid()) {
-            getDbHelper().setProperty(PROPERTY_DIRECTORY_SCAN_COMPLETE, "0");
+            getDbHelper().setProperty(DbProperties.DIRECTORY_SCAN_COMPLETE, "0");
         }
 
         scanAllPackagesIfNeeded();
     }
 
     private void scanAllPackagesIfNeeded() {
-        String scanComplete = getDbHelper().getProperty(PROPERTY_DIRECTORY_SCAN_COMPLETE, "0");
+        String scanComplete = getDbHelper().getProperty(DbProperties.DIRECTORY_SCAN_COMPLETE, "0");
         if (!"0".equals(scanComplete)) {
             return;
         }
 
         long start = SystemClock.currentThreadTimeMillis();
         int count = scanAllPackages();
-        getDbHelper().setProperty(PROPERTY_DIRECTORY_SCAN_COMPLETE, "1");
+        getDbHelper().setProperty(DbProperties.DIRECTORY_SCAN_COMPLETE, "1");
         long end = SystemClock.currentThreadTimeMillis();
         Log.i(TAG, "Discovered " + count + " contact directories in " + (end - start) + "ms");
 
diff --git a/src/com/android/providers/contacts/ContactsDatabaseHelper.java b/src/com/android/providers/contacts/ContactsDatabaseHelper.java
index 3d83a1b..ccebf05 100644
--- a/src/com/android/providers/contacts/ContactsDatabaseHelper.java
+++ b/src/com/android/providers/contacts/ContactsDatabaseHelper.java
@@ -80,7 +80,6 @@
 import android.util.Log;
 
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -689,6 +688,16 @@
         String[] LITERAL_ONE = new String[] {"1"};
     }
 
+    /**
+     * Property names for {@link ContactsDatabaseHelper#getProperty} and
+     * {@link ContactsDatabaseHelper#setProperty}.
+     */
+    public interface DbProperties {
+        String DIRECTORY_SCAN_COMPLETE = "directoryScanComplete";
+        String AGGREGATION_ALGORITHM = "aggregation_v2";
+        String KNOWN_ACCOUNTS = "known_accounts";
+    }
+
     /** In-memory cache of previously found MIME-type mappings */
     // TODO Use ConcurrentHashMap?
     private final HashMap<String, Long> mMimetypeCache = new HashMap<String, Long>();
@@ -803,7 +812,7 @@
     private void initializeCache(SQLiteDatabase db) {
         mMimetypeCache.clear();
         mPackageCache.clear();
-        mAccountCache.clear();
+        refreshAccountCache(db);
 
         // TODO: This could be optimized into one query instead of 7
         //        Also: We shouldn't have those fields in the first place. This should just be
@@ -1361,7 +1370,7 @@
         ");");
 
         // Trigger a full scan of directories in the system
-        setProperty(db, ContactDirectoryManager.PROPERTY_DIRECTORY_SCAN_COMPLETE, "0");
+        setProperty(db, DbProperties.DIRECTORY_SCAN_COMPLETE, "0");
     }
 
     public void createSearchIndexTable(SQLiteDatabase db) {
@@ -4118,76 +4127,51 @@
         }
     }
 
-    /**
-     * Gets all accounts in the accounts table.
-     */
-    public Set<AccountWithDataSet> getAllAccountsWithDataSets() {
-        Set<AccountWithDataSet> accountsWithDataSets = new HashSet<AccountWithDataSet>();
-        Cursor c = getWritableDatabase().rawQuery(
-                "SELECT DISTINCT " + AccountsColumns.ACCOUNT_NAME +
+    /** Refresh {@link #mAccountCache}. */
+    public void refreshAccountCache() {
+        refreshAccountCache(getReadableDatabase());
+    }
+
+    /** Refresh {@link #mAccountCache}. */
+    private void refreshAccountCache(SQLiteDatabase db) {
+        mAccountCache.clear();
+        Cursor c = db.rawQuery(
+                "SELECT DISTINCT " +  AccountsColumns._ID + "," + AccountsColumns.ACCOUNT_NAME +
                 "," + AccountsColumns.ACCOUNT_TYPE + "," + AccountsColumns.DATA_SET +
                 " FROM " + Tables.ACCOUNTS, null);
         try {
             while (c.moveToNext()) {
-                accountsWithDataSets.add(
-                        AccountWithDataSet.get(c.getString(0), c.getString(1), c.getString(2)));
+                mAccountCache.put(
+                        AccountWithDataSet.get(c.getString(1), c.getString(2), c.getString(3)),
+                        c.getLong(0));
             }
         } finally {
             c.close();
         }
-        return accountsWithDataSets;
     }
 
     /**
-     * @return ID of the specified account, looking up on the Account2 table.  Unlike
-     * {@link #getPackageId(String)} and {@link #getMimeTypeId(String)} it won't create a record
-     * even if it doesn't exist; just returns null instead.
-     *
-     * Intended to be used in read operations, as opposed to
-     * {@link #getOrCreateAccountIdInTransaction} and {@link #invalidateAccountCacheInTransaction},
-     * which should only be used in a transaction, so there's no need for synchronization.
+     * Gets all accounts in the accounts table.
+     */
+    public Set<AccountWithDataSet> getAllAccountsWithDataSets() {
+        return mAccountCache.keySet();
+    }
+
+    /**
+     * @return ID of the specified account, or null if the account doesn't exist.
      */
     public Long getAccountIdOrNull(AccountWithDataSet accountWithDataSet) {
         if (accountWithDataSet == null) {
             accountWithDataSet = AccountWithDataSet.LOCAL;
         }
-        Long id = mAccountCache.get(accountWithDataSet);
-
-        if (id != null) return id;
-
-        final SQLiteStatement select = getWritableDatabase().compileStatement(
-              "SELECT " + AccountsColumns._ID +
-              " FROM " + Tables.ACCOUNTS +
-              " WHERE " +
-              "((?1 IS NULL AND " + AccountsColumns.ACCOUNT_NAME + " IS NULL) OR " +
-                      "(" + AccountsColumns.ACCOUNT_NAME + "=?1)) AND " +
-              "((?2 IS NULL AND " + AccountsColumns.ACCOUNT_TYPE + " IS NULL) OR " +
-                      "(" + AccountsColumns.ACCOUNT_TYPE + "=?2)) AND " +
-              "((?3 IS NULL AND " + AccountsColumns.DATA_SET + " IS NULL) OR " +
-                      "(" + AccountsColumns.DATA_SET + "=?3))");
-        try {
-            DatabaseUtils.bindObjectToProgram(select, 1, accountWithDataSet.getAccountName());
-            DatabaseUtils.bindObjectToProgram(select, 2, accountWithDataSet.getAccountType());
-            DatabaseUtils.bindObjectToProgram(select, 3, accountWithDataSet.getDataSet());
-            try {
-                id = select.simpleQueryForLong();
-                mAccountCache.put(accountWithDataSet, id);
-                return id;
-            } catch (SQLiteDoneException notFound) {
-                return null;
-            }
-        } finally {
-            select.close();
-        }
+        return mAccountCache.get(accountWithDataSet);
     }
 
     /**
-     * @return ID of the specified account.  This method will create a record if the account doesn't
-     *     exist in the accounts table.
+     * @return ID of the specified account.  This method will create a record in the accounts table
+     *     if the account doesn't exist in the accounts table.
      *
      * This must be used in a transaction, so there's no need for synchronization.
-     *
-     * TODO Pre-init the cache in {@link #initializeCache} and remove the lazy-lookup?
      */
     public long getOrCreateAccountIdInTransaction(AccountWithDataSet accountWithDataSet) {
         if (accountWithDataSet == null) {
@@ -4217,15 +4201,6 @@
     }
 
     /**
-     * Clear the account cache.
-     *
-     * This must be used in a transaction, so there's no need for synchronization.
-     */
-    public void invalidateAccountCacheInTransaction() {
-        mAccountCache.clear();
-    }
-
-    /**
      * Update {@link Contacts#IN_VISIBLE_GROUP} for all contacts.
      */
     public void updateAllVisible() {
diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java
index 66fed9c..4dbfbcb 100644
--- a/src/com/android/providers/contacts/ContactsProvider2.java
+++ b/src/com/android/providers/contacts/ContactsProvider2.java
@@ -28,6 +28,7 @@
 import com.android.providers.contacts.ContactsDatabaseHelper.ContactsStatusUpdatesColumns;
 import com.android.providers.contacts.ContactsDatabaseHelper.DataColumns;
 import com.android.providers.contacts.ContactsDatabaseHelper.DataUsageStatColumns;
+import com.android.providers.contacts.ContactsDatabaseHelper.DbProperties;
 import com.android.providers.contacts.ContactsDatabaseHelper.GroupsColumns;
 import com.android.providers.contacts.ContactsDatabaseHelper.Joins;
 import com.android.providers.contacts.ContactsDatabaseHelper.NameLookupColumns;
@@ -209,7 +210,6 @@
 
     private static final String PREF_LOCALE = "locale";
 
-    private static final String PROPERTY_AGGREGATION_ALGORITHM = "aggregation_v2";
     private static final int PROPERTY_AGGREGATION_ALGORITHM_VERSION = 2;
 
     private static final String AGGREGATE_CONTACTS = "sync.contacts.aggregate";
@@ -2436,7 +2436,7 @@
             } else {
                 values.put(RawContacts.DATA_SET, dataSet);
             }
-            accountWithDataSet = new AccountWithDataSet(account.name, account.type, dataSet);
+            accountWithDataSet = AccountWithDataSet.get(account.name, account.type, dataSet);
         }
         return accountWithDataSet;
     }
@@ -4362,11 +4362,75 @@
         scheduleBackgroundTask(BACKGROUND_TASK_UPDATE_ACCOUNTS);
     }
 
+    private static final String ACCOUNT_STRING_SEPARATOR_OUTER = "\u0001";
+    private static final String ACCOUNT_STRING_SEPARATOR_INNER = "\u0002";
+
+    /** return serialized version of {@code accounts} */
+    @VisibleForTesting
+    static String accountsToString(Set<Account> accounts) {
+        final StringBuilder sb = new StringBuilder();
+        for (Account account : accounts) {
+            if (sb.length() > 0) {
+                sb.append(ACCOUNT_STRING_SEPARATOR_OUTER);
+            }
+            sb.append(account.name);
+            sb.append(ACCOUNT_STRING_SEPARATOR_INNER);
+            sb.append(account.type);
+        }
+        return sb.toString();
+    }
+
+    /**
+     * de-serialize string returned by {@link #accountsToString} and return it.
+     * If {@code accountsString} is malformed it'll throw {@link IllegalArgumentException}.
+     */
+    @VisibleForTesting
+    static Set<Account> stringToAccounts(String accountsString) {
+        final Set<Account> ret = Sets.newHashSet();
+        if (accountsString.length() == 0) return ret; // no accounts
+        try {
+            for (String accountString : accountsString.split(ACCOUNT_STRING_SEPARATOR_OUTER)) {
+                String[] nameAndType = accountString.split(ACCOUNT_STRING_SEPARATOR_INNER);
+                ret.add(new Account(nameAndType[0], nameAndType[1]));
+            }
+            return ret;
+        } catch (RuntimeException ex) {
+            throw new IllegalArgumentException("Malformed string", ex);
+        }
+    }
+
+    /**
+     * @return {@code true} if the given {@code currentSystemAccounts} are different from the
+     *    accounts we know, which are stored in the {@link DbProperties#KNOWN_ACCOUNTS} property.
+     */
+    @VisibleForTesting
+    boolean haveAccountsChanged(Account[] currentSystemAccounts) {
+        final ContactsDatabaseHelper dbHelper = mDbHelper.get();
+        final Set<Account> knownAccountSet;
+        try {
+            knownAccountSet = stringToAccounts(
+                    dbHelper.getProperty(DbProperties.KNOWN_ACCOUNTS, ""));
+        } catch (IllegalArgumentException e) {
+            // Failed to get the last known accounts for an unknown reason.  Let's just
+            // treat as if accounts have changed.
+            return true;
+        }
+        final Set<Account> currentAccounts = Sets.newHashSet(currentSystemAccounts);
+        return !knownAccountSet.equals(currentAccounts);
+    }
+
+    @VisibleForTesting
+    void saveAccounts(Account[] systemAccounts) {
+        final ContactsDatabaseHelper dbHelper = mDbHelper.get();
+        dbHelper.setProperty(DbProperties.KNOWN_ACCOUNTS,
+                accountsToString(Sets.newHashSet(systemAccounts)));
+    }
+
     private boolean updateAccountsInBackground(Account[] systemAccounts) {
-
-        // TODO Really detect account changes here, by storing the last known accounts in the
-        // DB property.  If there's no accounts added or removed, just return with false here.
-
+        if (!haveAccountsChanged(systemAccounts)) {
+            return false;
+        }
+        Log.i(TAG, "Accounts changed");
         final ContactsDatabaseHelper dbHelper = mDbHelper.get();
         final SQLiteDatabase db = dbHelper.getWritableDatabase();
         mActiveDb.set(db);
@@ -4471,6 +4535,12 @@
                 // search index for the profile DB, and updating it for the contacts DB in this case
                 // makes no sense and risks a deadlock.
                 if (!inProfileMode()) {
+                    // TODO Fix it.  It only updates index for contacts/raw_contacts that the
+                    // current transaction context knows updated, but here in this method we don't
+                    // update that information, so effectively it's no-op.
+                    // We can probably just schedule BACKGROUND_TASK_UPDATE_SEARCH_INDEX.
+                    // (But make sure it's not scheduled yet. We schedule this task in initialize()
+                    // too.)
                     updateSearchIndexInTransaction();
                 }
             }
@@ -4484,7 +4554,8 @@
             // Third, remaining tasks that must be done in a transaction.
             // TODO: Should sync state take data set into consideration?
             dbHelper.getSyncState().onAccountsChanged(db, systemAccounts);
-            dbHelper.invalidateAccountCacheInTransaction();
+
+            saveAccounts(systemAccounts);
 
             db.setTransactionSuccessful();
         } finally {
@@ -4492,6 +4563,7 @@
         }
         mAccountWritability.clear();
 
+        dbHelper.refreshAccountCache();
         updateContactsAccountCount(systemAccounts);
         updateProviderStatus();
         return true;
@@ -4742,7 +4814,7 @@
 
             case CONTACTS: {
                 setTablesAndProjectionMapForContacts(qb, uri, projection);
-                appendLocalDirectorySelectionIfNeeded(qb, directoryId);
+                appendLocalDirectoryAndAccountSelectionIfNeeded(qb, directoryId, uri);
                 break;
             }
 
@@ -5589,7 +5661,7 @@
             case GROUPS: {
                 qb.setTables(Views.GROUPS);
                 qb.setProjectionMap(sGroupsProjectionMap);
-                appendAccountFromParameter(qb, uri);
+                appendAccountIdFromParameter(qb, uri);
                 break;
             }
 
@@ -5614,7 +5686,7 @@
                 }
                 qb.setTables(tables);
                 qb.setProjectionMap(sGroupsSummaryProjectionMap);
-                appendAccountFromParameter(qb, uri);
+                appendAccountIdFromParameter(qb, uri);
                 groupBy = GroupsColumns.CONCRETE_ID;
                 break;
             }
@@ -6487,13 +6559,13 @@
         sb.append(Views.RAW_CONTACTS);
         qb.setTables(sb.toString());
         qb.setProjectionMap(sRawContactsProjectionMap);
-        appendAccountFromParameter(qb, uri);
+        appendAccountIdFromParameter(qb, uri);
     }
 
     private void setTablesAndProjectionMapForRawEntities(SQLiteQueryBuilder qb, Uri uri) {
         qb.setTables(Views.RAW_ENTITIES);
         qb.setProjectionMap(sRawEntityProjectionMap);
-        appendAccountFromParameter(qb, uri);
+        appendAccountIdFromParameter(qb, uri);
     }
 
     private void setTablesAndProjectionMapForData(SQLiteQueryBuilder qb, Uri uri,
@@ -6545,7 +6617,7 @@
         }
 
         qb.setProjectionMap(projectionMap);
-        appendAccountFromParameter(qb, uri);
+        appendAccountIdFromParameter(qb, uri);
     }
 
     private void setTableAndProjectionMapForStatusUpdates(SQLiteQueryBuilder qb,
@@ -6592,7 +6664,7 @@
 
         qb.setTables(sb.toString());
         qb.setProjectionMap(sEntityProjectionMap);
-        appendAccountFromParameter(qb, uri);
+        appendAccountIdFromParameter(qb, uri);
     }
 
     private void appendContactStatusUpdateJoin(StringBuilder sb, String[] projection,
@@ -6648,49 +6720,82 @@
         }
     }
 
-    private boolean appendLocalDirectorySelectionIfNeeded(SQLiteQueryBuilder qb, long directoryId) {
+    private void appendLocalDirectoryAndAccountSelectionIfNeeded(SQLiteQueryBuilder qb,
+            long directoryId, Uri uri) {
+        final StringBuilder sb = new StringBuilder();
         if (directoryId == Directory.DEFAULT) {
-            qb.appendWhere(Contacts._ID + " IN " + Tables.DEFAULT_DIRECTORY);
-            return true;
+            sb.append("(" + Contacts._ID + " IN " + Tables.DEFAULT_DIRECTORY + ")");
         } else if (directoryId == Directory.LOCAL_INVISIBLE){
-            qb.appendWhere(Contacts._ID + " NOT IN " + Tables.DEFAULT_DIRECTORY);
-            return true;
+            sb.append("(" + Contacts._ID + " NOT IN " + Tables.DEFAULT_DIRECTORY + ")");
+        } else {
+            sb.append("(1)");
         }
-        return false;
+
+        final AccountWithDataSet accountWithDataSet = getAccountWithDataSetFromUri(uri);
+        // Accounts are valid by only checking one parameter, since we've
+        // already ruled out partial accounts.
+        final boolean validAccount = !TextUtils.isEmpty(accountWithDataSet.getAccountName());
+        if (validAccount) {
+            final Long accountId = mDbHelper.get().getAccountIdOrNull(accountWithDataSet);
+            if (accountId == null) {
+                // No such account.
+                sb.setLength(0);
+                sb.append("(1=2)");
+            } else {
+                sb.append(
+                        " AND (" + Contacts._ID + " IN (" +
+                        "SELECT " + RawContacts.CONTACT_ID + " FROM " + Tables.RAW_CONTACTS +
+                        " WHERE " + RawContactsColumns.ACCOUNT_ID + "=" + accountId.toString() +
+                        "))");
+            }
+        }
+        qb.appendWhere(sb.toString());
     }
 
     private void appendAccountFromParameter(SQLiteQueryBuilder qb, Uri uri) {
-        final String accountName = getQueryParameter(uri, RawContacts.ACCOUNT_NAME);
-        final String accountType = getQueryParameter(uri, RawContacts.ACCOUNT_TYPE);
-        final String dataSet = getQueryParameter(uri, RawContacts.DATA_SET);
-
-        final boolean partialUri = TextUtils.isEmpty(accountName) ^ TextUtils.isEmpty(accountType);
-        if (partialUri) {
-            // Throw when either account is incomplete
-            throw new IllegalArgumentException(mDbHelper.get().exceptionMessage(
-                    "Must specify both or neither of ACCOUNT_NAME and ACCOUNT_TYPE", uri));
-        }
+        final AccountWithDataSet accountWithDataSet = getAccountWithDataSetFromUri(uri);
 
         // Accounts are valid by only checking one parameter, since we've
         // already ruled out partial accounts.
-        final boolean validAccount = !TextUtils.isEmpty(accountName);
+        final boolean validAccount = !TextUtils.isEmpty(accountWithDataSet.getAccountName());
         if (validAccount) {
-            String toAppend = RawContacts.ACCOUNT_NAME + "="
-                    + DatabaseUtils.sqlEscapeString(accountName) + " AND "
+            String toAppend = "(" + RawContacts.ACCOUNT_NAME + "="
+                    + DatabaseUtils.sqlEscapeString(accountWithDataSet.getAccountName()) + " AND "
                     + RawContacts.ACCOUNT_TYPE + "="
-                    + DatabaseUtils.sqlEscapeString(accountType);
-            if (dataSet == null) {
+                    + DatabaseUtils.sqlEscapeString(accountWithDataSet.getAccountType());
+            if (accountWithDataSet.getDataSet() == null) {
                 toAppend += " AND " + RawContacts.DATA_SET + " IS NULL";
             } else {
                 toAppend += " AND " + RawContacts.DATA_SET + "=" +
-                        DatabaseUtils.sqlEscapeString(dataSet);
+                        DatabaseUtils.sqlEscapeString(accountWithDataSet.getDataSet());
             }
+            toAppend += ")";
             qb.appendWhere(toAppend);
         } else {
             qb.appendWhere("1");
         }
     }
 
+    private void appendAccountIdFromParameter(SQLiteQueryBuilder qb, Uri uri) {
+        final AccountWithDataSet accountWithDataSet = getAccountWithDataSetFromUri(uri);
+
+        // Accounts are valid by only checking one parameter, since we've
+        // already ruled out partial accounts.
+        final boolean validAccount = !TextUtils.isEmpty(accountWithDataSet.getAccountName());
+        if (validAccount) {
+            final Long accountId = mDbHelper.get().getAccountIdOrNull(accountWithDataSet);
+            if (accountId == null) {
+                // No such account.
+                qb.appendWhere("(1=2)");
+            } else {
+                qb.appendWhere(
+                        "(" + RawContactsColumns.ACCOUNT_ID + "=" + accountId.toString() + ")");
+            }
+        } else {
+            qb.appendWhere("1");
+        }
+    }
+
     private AccountWithDataSet getAccountWithDataSetFromUri(Uri uri) {
         final String accountName = getQueryParameter(uri, RawContacts.ACCOUNT_NAME);
         final String accountType = getQueryParameter(uri, RawContacts.ACCOUNT_TYPE);
@@ -7676,7 +7781,7 @@
         }
 
         int version = Integer.parseInt(mContactsHelper.getProperty(
-                PROPERTY_AGGREGATION_ALGORITHM, "1"));
+                DbProperties.AGGREGATION_ALGORITHM, "1"));
         return version < PROPERTY_AGGREGATION_ALGORITHM_VERSION;
     }
 
@@ -7711,7 +7816,7 @@
             mContactAggregator.aggregateInTransaction(mTransactionContext.get(), db);
             updateSearchIndexInTransaction();
             db.setTransactionSuccessful();
-            mContactsHelper.setProperty(PROPERTY_AGGREGATION_ALGORITHM,
+            mContactsHelper.setProperty(DbProperties.AGGREGATION_ALGORITHM,
                     String.valueOf(PROPERTY_AGGREGATION_ALGORITHM_VERSION));
         } finally {
             if (db != null) {
@@ -7895,4 +8000,11 @@
     private boolean snippetNeeded(String [] projection) {
         return ContactsDatabaseHelper.isInProjection(projection, SearchSnippetColumns.SNIPPET);
     }
+
+    /**
+     * @return the currently active {@link ContactsDatabaseHelper} for the current thread.
+     */
+    public ContactsDatabaseHelper getThreadActiveDatabaseHelperForTest() {
+        return mDbHelper.get();
+    }
 }
diff --git a/tests/src/com/android/providers/contacts/ContactsDatabaseHelperTest.java b/tests/src/com/android/providers/contacts/ContactsDatabaseHelperTest.java
index 6babf83..bbfba1e 100644
--- a/tests/src/com/android/providers/contacts/ContactsDatabaseHelperTest.java
+++ b/tests/src/com/android/providers/contacts/ContactsDatabaseHelperTest.java
@@ -87,13 +87,14 @@
         assertEquals(a1id, mDbHelper.getOrCreateAccountIdInTransaction(AccountWithDataSet.LOCAL));
         assertEquals((Long) a1id, mDbHelper.getAccountIdOrNull(AccountWithDataSet.LOCAL));
 
-        // Clear the table, but until we invalidate the cache getAccountIdOrNull() keeps returning
+        // Clear the table, but until we refresh the cache getAccountIdOrNull() keeps returning
         // cached values.
         mDbHelper.getWritableDatabase().execSQL("delete from " + Tables.ACCOUNTS);
         assertEquals((Long) a1id, mDbHelper.getAccountIdOrNull(AccountWithDataSet.LOCAL));
 
-        // invalidateAccountCacheInTransaction() clears the cache.
-        mDbHelper.invalidateAccountCacheInTransaction();
+        // refreshAccountCache() clears the cache.
+        mDbHelper.refreshAccountCache();
+
         assertNull(mDbHelper.getAccountIdOrNull(AccountWithDataSet.LOCAL));
         assertNull(mDbHelper.getAccountIdOrNull(a1));
         assertNull(mDbHelper.getAccountIdOrNull(a2));
diff --git a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
index 5839467..046dec8 100644
--- a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
+++ b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
@@ -19,9 +19,11 @@
 import com.android.internal.util.ArrayUtils;
 import com.android.providers.contacts.ContactsDatabaseHelper.AggregationExceptionColumns;
 import com.android.providers.contacts.ContactsDatabaseHelper.DataUsageStatColumns;
+import com.android.providers.contacts.ContactsDatabaseHelper.DbProperties;
 import com.android.providers.contacts.ContactsDatabaseHelper.PresenceColumns;
 import com.android.providers.contacts.tests.R;
 import com.google.android.collect.Lists;
+import com.google.android.collect.Sets;
 
 import android.accounts.Account;
 import android.content.ContentProviderOperation;
@@ -80,6 +82,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
+import java.util.Set;
 
 /**
  * Unit tests for {@link ContactsProvider2}.
@@ -4523,6 +4526,103 @@
         assertStoredValue(uri, RawContacts.DELETED, "1");
     }
 
+    /**
+     * Test for {@link ContactsProvider2#stringToAccounts} and
+     * {@link ContactsProvider2#accountsToString}.
+     */
+    public void testAccountsToString() {
+        final Set<Account> EXPECTED_0 = Sets.newHashSet();
+        final Set<Account> EXPECTED_1 = Sets.newHashSet(ACCOUNT_1);
+        final Set<Account> EXPECTED_2 = Sets.newHashSet(ACCOUNT_2);
+        final Set<Account> EXPECTED_1_2 = Sets.newHashSet(ACCOUNT_1, ACCOUNT_2);
+
+        final Set<Account> ACTUAL_0 = Sets.newHashSet();
+        final Set<Account> ACTUAL_1 = Sets.newHashSet(ACCOUNT_1);
+        final Set<Account> ACTUAL_2 = Sets.newHashSet(ACCOUNT_2);
+        final Set<Account> ACTUAL_1_2 = Sets.newHashSet(ACCOUNT_2, ACCOUNT_1);
+
+        assertTrue(EXPECTED_0.equals(accountsToStringToAccounts(ACTUAL_0)));
+        assertFalse(EXPECTED_0.equals(accountsToStringToAccounts(ACTUAL_1)));
+        assertFalse(EXPECTED_0.equals(accountsToStringToAccounts(ACTUAL_2)));
+        assertFalse(EXPECTED_0.equals(accountsToStringToAccounts(ACTUAL_1_2)));
+
+        assertFalse(EXPECTED_1.equals(accountsToStringToAccounts(ACTUAL_0)));
+        assertTrue(EXPECTED_1.equals(accountsToStringToAccounts(ACTUAL_1)));
+        assertFalse(EXPECTED_1.equals(accountsToStringToAccounts(ACTUAL_2)));
+        assertFalse(EXPECTED_1.equals(accountsToStringToAccounts(ACTUAL_1_2)));
+
+        assertFalse(EXPECTED_2.equals(accountsToStringToAccounts(ACTUAL_0)));
+        assertFalse(EXPECTED_2.equals(accountsToStringToAccounts(ACTUAL_1)));
+        assertTrue(EXPECTED_2.equals(accountsToStringToAccounts(ACTUAL_2)));
+        assertFalse(EXPECTED_2.equals(accountsToStringToAccounts(ACTUAL_1_2)));
+
+        assertFalse(EXPECTED_1_2.equals(accountsToStringToAccounts(ACTUAL_0)));
+        assertFalse(EXPECTED_1_2.equals(accountsToStringToAccounts(ACTUAL_1)));
+        assertFalse(EXPECTED_1_2.equals(accountsToStringToAccounts(ACTUAL_2)));
+        assertTrue(EXPECTED_1_2.equals(accountsToStringToAccounts(ACTUAL_1_2)));
+
+        try {
+            ContactsProvider2.stringToAccounts("x");
+            fail("Didn't throw for malformed input");
+        } catch (IllegalArgumentException expected) {
+        }
+    }
+
+    private static final Set<Account> accountsToStringToAccounts(Set<Account> accounts) {
+        return ContactsProvider2.stringToAccounts(ContactsProvider2.accountsToString(accounts));
+    }
+
+    /**
+     * Test for {@link ContactsProvider2#haveAccountsChanged} and
+     * {@link ContactsProvider2#saveAccounts}.
+     */
+    public void testHaveAccountsChanged() {
+        final ContactsProvider2 cp = (ContactsProvider2) getProvider();
+
+        final Account[] ACCOUNTS_0 = new Account[] {};
+        final Account[] ACCOUNTS_1 = new Account[] {ACCOUNT_1};
+        final Account[] ACCOUNTS_2 = new Account[] {ACCOUNT_2};
+        final Account[] ACCOUNTS_1_2 = new Account[] {ACCOUNT_1, ACCOUNT_2};
+        final Account[] ACCOUNTS_2_1 = new Account[] {ACCOUNT_2, ACCOUNT_1};
+
+        // Add ACCOUNT_1
+
+        assertTrue(cp.haveAccountsChanged(ACCOUNTS_1));
+        cp.saveAccounts(ACCOUNTS_1);
+        assertFalse(cp.haveAccountsChanged(ACCOUNTS_1));
+
+        // Add ACCOUNT_2
+
+        assertTrue(cp.haveAccountsChanged(ACCOUNTS_1_2));
+        // (try with reverse order)
+        assertTrue(cp.haveAccountsChanged(ACCOUNTS_2_1));
+        cp.saveAccounts(ACCOUNTS_1_2);
+        assertFalse(cp.haveAccountsChanged(ACCOUNTS_1_2));
+        // (try with reverse order)
+        assertFalse(cp.haveAccountsChanged(ACCOUNTS_2_1));
+
+        // Remove ACCOUNT_1
+
+        assertTrue(cp.haveAccountsChanged(ACCOUNTS_2));
+        cp.saveAccounts(ACCOUNTS_2);
+        assertFalse(cp.haveAccountsChanged(ACCOUNTS_2));
+
+        // Remove ACCOUNT_2
+
+        assertTrue(cp.haveAccountsChanged(ACCOUNTS_0));
+        cp.saveAccounts(ACCOUNTS_0);
+        assertFalse(cp.haveAccountsChanged(ACCOUNTS_0));
+
+        // Test with malformed DB property.
+
+        final ContactsDatabaseHelper dbHelper = cp.getThreadActiveDatabaseHelperForTest();
+        dbHelper.setProperty(DbProperties.KNOWN_ACCOUNTS, "x");
+
+        // With malformed property the method always return true.
+        assertTrue(cp.haveAccountsChanged(ACCOUNTS_0));
+        assertTrue(cp.haveAccountsChanged(ACCOUNTS_1));
+    }
+
     public void testAccountsUpdated() {
         // This is to ensure we do not delete contacts with null, null (account name, type)
         // accidentally.