From a1ef1b61271173a0b424bf4b9452172c2784224a Mon Sep 17 00:00:00 2001 From: Yorke Lee Date: Wed, 16 Sep 2015 17:56:00 -0700 Subject: Fix for inconsistent smart dial database The issue was caused by a contact's phone number being removed, but not the entire contact. Since we currently determine a list of contacts to be updated by querying for a list of all updated phone numbers, this would incorrectly exclude the aforementioned modified contact. * Use the Contact URI instead of the Phone URI when doing this query to fix the problem * Add tests for DialerDatabaseHelper update behavior * Refactor small portions of DialerDatabaseHelper to facilitate testing Bug: 24053247 Change-Id: I18a7706ebbfd39fd686dc84bdbb842cc9e9b5e20 --- .../dialer/database/DialerDatabaseHelper.java | 135 ++++++++++++++------- 1 file changed, 88 insertions(+), 47 deletions(-) (limited to 'src/com/android') diff --git a/src/com/android/dialer/database/DialerDatabaseHelper.java b/src/com/android/dialer/database/DialerDatabaseHelper.java index d36a0f6d8..413e8673f 100644 --- a/src/com/android/dialer/database/DialerDatabaseHelper.java +++ b/src/com/android/dialer/database/DialerDatabaseHelper.java @@ -184,6 +184,23 @@ public class DialerDatabaseHelper extends SQLiteOpenHelper { SELECT_IGNORE_LOOKUP_KEY_TOO_LONG_CLAUSE; } + /** + * Query for all contacts that have been updated since the last time the smart dial database + * was updated. + */ + public static interface UpdatedContactQuery { + static final Uri URI = ContactsContract.Contacts.CONTENT_URI; + + static final String[] PROJECTION = new String[] { + ContactsContract.Contacts._ID // 0 + }; + + static final int UPDATED_CONTACT_ID = 0; + + static final String SELECT_UPDATED_CLAUSE = + ContactsContract.Contacts.CONTACT_LAST_UPDATED_TIMESTAMP + " > ?"; + } + /** Query options for querying the deleted contact database.*/ public static interface DeleteContactQuery { static final Uri URI = ContactsContract.DeletedContacts.CONTENT_URI; @@ -563,15 +580,11 @@ public class DialerDatabaseHelper extends SQLiteOpenHelper { * Removes rows in the smartdial database that matches the contacts that have been deleted * by other apps since last update. * - * @param db Database pointer to the dialer database. - * @param last_update_time Time stamp of last update on the smartdial database + * @param db Database to operate on. + * @param deletedContactCursor Cursor containing rows of deleted contacts */ - private void removeDeletedContacts(SQLiteDatabase db, String last_update_time) { - final Cursor deletedContactCursor = mContext.getContentResolver().query( - DeleteContactQuery.URI, - DeleteContactQuery.PROJECTION, - DeleteContactQuery.SELECT_UPDATED_CLAUSE, - new String[] {last_update_time}, null); + @VisibleForTesting + void removeDeletedContacts(SQLiteDatabase db, Cursor deletedContactCursor) { if (deletedContactCursor == null) { return; } @@ -594,6 +607,15 @@ public class DialerDatabaseHelper extends SQLiteOpenHelper { } } + private Cursor getDeletedContactCursor(String lastUpdateMillis) { + return mContext.getContentResolver().query( + DeleteContactQuery.URI, + DeleteContactQuery.PROJECTION, + DeleteContactQuery.SELECT_UPDATED_CLAUSE, + new String[] {lastUpdateMillis}, + null); + } + /** * Removes potentially corrupted entries in the database. These contacts may be added before * the previous instance of the dialer was destroyed for some reason. For data integrity, we @@ -637,11 +659,14 @@ public class DialerDatabaseHelper extends SQLiteOpenHelper { * @param db Database pointer to the smartdial database * @param updatedContactCursor Cursor pointing to the list of recently updated contacts. */ - private void removeUpdatedContacts(SQLiteDatabase db, Cursor updatedContactCursor) { + @VisibleForTesting + void removeUpdatedContacts(SQLiteDatabase db, Cursor updatedContactCursor) { db.beginTransaction(); try { + updatedContactCursor.moveToPosition(-1); while (updatedContactCursor.moveToNext()) { - final Long contactId = updatedContactCursor.getLong(PhoneQuery.PHONE_CONTACT_ID); + final Long contactId = + updatedContactCursor.getLong(UpdatedContactQuery.UPDATED_CONTACT_ID); db.delete(Tables.SMARTDIAL_TABLE, SmartDialDbColumns.CONTACT_ID + "=" + contactId, null); @@ -814,59 +839,75 @@ public class DialerDatabaseHelper extends SQLiteOpenHelper { if (DEBUG) { Log.v(TAG, "Last updated at " + lastUpdateMillis); } - /** Queries the contact database to get contacts that have been updated since the last - * update time. - */ - final Cursor updatedContactCursor = mContext.getContentResolver().query(PhoneQuery.URI, - PhoneQuery.PROJECTION, PhoneQuery.SELECTION, - new String[]{lastUpdateMillis}, null); - if (updatedContactCursor == null) { - if (DEBUG) { - Log.e(TAG, "SmartDial query received null for cursor"); - } - return; - } /** Sets the time after querying the database as the current update time. */ final Long currentMillis = System.currentTimeMillis(); - try { - if (DEBUG) { - stopWatch.lap("Queried the Contacts database"); - } + if (DEBUG) { + stopWatch.lap("Queried the Contacts database"); + } - /** Prevents the app from reading the dialer database when updating. */ - sInUpdate.getAndSet(true); + /** Prevents the app from reading the dialer database when updating. */ + sInUpdate.getAndSet(true); - /** Removes contacts that have been deleted. */ - removeDeletedContacts(db, lastUpdateMillis); - removePotentiallyCorruptedContacts(db, lastUpdateMillis); + /** Removes contacts that have been deleted. */ + removeDeletedContacts(db, getDeletedContactCursor(lastUpdateMillis)); + removePotentiallyCorruptedContacts(db, lastUpdateMillis); - if (DEBUG) { - stopWatch.lap("Finished deleting deleted entries"); - } + if (DEBUG) { + stopWatch.lap("Finished deleting deleted entries"); + } - /** If the database did not exist before, jump through deletion as there is nothing - * to delete. + /** If the database did not exist before, jump through deletion as there is nothing + * to delete. + */ + if (!lastUpdateMillis.equals("0")) { + /** Removes contacts that have been updated. Updated contact information will be + * inserted later. Note that this has to use a separate result set from + * updatePhoneCursor, since it is possible for a contact to be updated (e.g. + * phone number deleted), but have no results show up in updatedPhoneCursor (since + * all of its phone numbers have been deleted). */ - if (!lastUpdateMillis.equals("0")) { - /** Removes contacts that have been updated. Updated contact information will be - * inserted later. - */ + final Cursor updatedContactCursor = mContext.getContentResolver().query( + UpdatedContactQuery.URI, + UpdatedContactQuery.PROJECTION, + UpdatedContactQuery.SELECT_UPDATED_CLAUSE, + new String[] {lastUpdateMillis}, + null + ); + if (updatedContactCursor == null) { + Log.e(TAG, "SmartDial query received null for cursor"); + return; + } + try { removeUpdatedContacts(db, updatedContactCursor); - if (DEBUG) { - stopWatch.lap("Finished deleting updated entries"); - } + } finally { + updatedContactCursor.close(); } + if (DEBUG) { + stopWatch.lap("Finished deleting entries belonging to updated contacts"); + } + } + + /** Queries the contact database to get all phone numbers that have been updated since the last + * update time. + */ + final Cursor updatedPhoneCursor = mContext.getContentResolver().query(PhoneQuery.URI, + PhoneQuery.PROJECTION, PhoneQuery.SELECTION, + new String[]{lastUpdateMillis}, null); + if (updatedPhoneCursor == null) { + Log.e(TAG, "SmartDial query received null for cursor"); + return; + } - /** Inserts recently updated contacts to the smartdial database.*/ - insertUpdatedContactsAndNumberPrefix(db, updatedContactCursor, currentMillis); + try { + /** Inserts recently updated phone numbers to the smartdial database.*/ + insertUpdatedContactsAndNumberPrefix(db, updatedPhoneCursor, currentMillis); if (DEBUG) { stopWatch.lap("Finished building the smart dial table"); } } finally { - /** Inserts prefixes of phone numbers into the prefix table.*/ - updatedContactCursor.close(); + updatedPhoneCursor.close(); } /** Gets a list of distinct contacts which have been updated, and adds the name prefixes -- cgit v1.2.3