From ab8573b754a0b5a102a68f5783743ee86ef24614 Mon Sep 17 00:00:00 2001 From: calderwoodra Date: Tue, 17 Apr 2018 12:09:01 -0700 Subject: Migrate SpeedDialUiLoader to use contact lookup URIs. The changes in this CL improve the loader in the follow ways: - Speed: by reducing the number of queries - Long term accuracy: by using lookup URIs This change also adds ViLTE presence and contact labels to the results returned by the Loader. Bug: 36841782,77724710,77725860 Test: SpeedDialUiLoaderTest PiperOrigin-RevId: 193235046 Change-Id: If5e6aa821c8bad88dc77af81827abbd8e0f1b28e --- .../dialer/speeddial/database/SpeedDialEntry.java | 14 +- .../database/SpeedDialEntryDatabaseHelper.java | 11 +- .../dialer/speeddial/loader/SpeedDialUiItem.java | 37 ++++- .../speeddial/loader/SpeedDialUiItemLoader.java | 167 ++++++++++++++++----- .../dialer/speeddial/loader/UiItemLoader.java | 32 ---- 5 files changed, 177 insertions(+), 84 deletions(-) delete mode 100644 java/com/android/dialer/speeddial/loader/UiItemLoader.java diff --git a/java/com/android/dialer/speeddial/database/SpeedDialEntry.java b/java/com/android/dialer/speeddial/database/SpeedDialEntry.java index 13ef4e306..89aed8f37 100644 --- a/java/com/android/dialer/speeddial/database/SpeedDialEntry.java +++ b/java/com/android/dialer/speeddial/database/SpeedDialEntry.java @@ -92,16 +92,14 @@ public abstract class SpeedDialEntry { /** * Raw phone number as the user entered it. * - * @see {@link Phone#NUMBER} + * @see Phone#NUMBER */ public abstract String number(); - /** - * Label that the user associated with this number like {@link Phone#TYPE_WORK}, {@link - * Phone#TYPE_HOME}, ect. - * - * @see {@link Phone#LABEL} - */ + /** @see Phone#TYPE */ + public abstract int phoneType(); + + /** @see Phone#LABEL */ public abstract String label(); public abstract @Technology int technology(); @@ -118,6 +116,8 @@ public abstract class SpeedDialEntry { public abstract Builder setNumber(String number); + public abstract Builder setPhoneType(int phoneType); + public abstract Builder setLabel(String label); public abstract Builder setTechnology(@Technology int technology); diff --git a/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java b/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java index 7c823bd63..137933fbe 100644 --- a/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java +++ b/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java @@ -37,7 +37,7 @@ import java.util.List; public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper implements SpeedDialEntryDao { - private static final int DATABASE_VERSION = 1; + private static final int DATABASE_VERSION = 2; private static final String DATABASE_NAME = "CPSpeedDialEntry"; // Column names @@ -46,6 +46,7 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper private static final String CONTACT_ID = "contact_id"; private static final String LOOKUP_KEY = "lookup_key"; private static final String PHONE_NUMBER = "phone_number"; + private static final String PHONE_TYPE = "phone_type"; private static final String PHONE_LABEL = "phone_label"; private static final String PHONE_TECHNOLOGY = "phone_technology"; @@ -54,8 +55,9 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper private static final int POSITION_CONTACT_ID = 1; private static final int POSITION_LOOKUP_KEY = 2; private static final int POSITION_PHONE_NUMBER = 3; - private static final int POSITION_PHONE_LABEL = 4; - private static final int POSITION_PHONE_TECHNOLOGY = 5; + private static final int POSITION_PHONE_TYPE = 4; + private static final int POSITION_PHONE_LABEL = 5; + private static final int POSITION_PHONE_TECHNOLOGY = 6; // Create Table Query private static final String CREATE_TABLE_SQL = @@ -66,6 +68,7 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper + (CONTACT_ID + " integer, ") + (LOOKUP_KEY + " text, ") + (PHONE_NUMBER + " text, ") + + (PHONE_TYPE + " integer, ") + (PHONE_LABEL + " text, ") + (PHONE_TECHNOLOGY + " integer ") + ");"; @@ -109,6 +112,7 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper channel = Channel.builder() .setNumber(number) + .setPhoneType(cursor.getInt(POSITION_PHONE_TYPE)) .setLabel(cursor.getString(POSITION_PHONE_LABEL)) .setTechnology(cursor.getInt(POSITION_PHONE_TECHNOLOGY)) .build(); @@ -216,6 +220,7 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper values.put(LOOKUP_KEY, entry.lookupKey()); if (entry.defaultChannel() != null) { values.put(PHONE_NUMBER, entry.defaultChannel().number()); + values.put(PHONE_TYPE, entry.defaultChannel().phoneType()); values.put(PHONE_LABEL, entry.defaultChannel().label()); values.put(PHONE_TECHNOLOGY, entry.defaultChannel().technology()); } diff --git a/java/com/android/dialer/speeddial/loader/SpeedDialUiItem.java b/java/com/android/dialer/speeddial/loader/SpeedDialUiItem.java index 3381bf8c0..28c22747e 100644 --- a/java/com/android/dialer/speeddial/loader/SpeedDialUiItem.java +++ b/java/com/android/dialer/speeddial/loader/SpeedDialUiItem.java @@ -18,6 +18,7 @@ package com.android.dialer.speeddial.loader; import android.database.Cursor; import android.provider.ContactsContract.CommonDataKinds.Phone; +import android.provider.ContactsContract.Data; import android.support.annotation.Nullable; import android.text.TextUtils; import com.android.dialer.common.Assert; @@ -45,9 +46,11 @@ public abstract class SpeedDialUiItem { public static final int DISPLAY_NAME = 2; public static final int STARRED = 3; public static final int NUMBER = 4; - public static final int LABEL = 5; - public static final int PHOTO_ID = 6; - public static final int PHOTO_URI = 7; + public static final int TYPE = 5; + public static final int LABEL = 6; + public static final int PHOTO_ID = 7; + public static final int PHOTO_URI = 8; + public static final int CARRIER_PRESENCE = 9; public static final String[] PHONE_PROJECTION = { Phone.LOOKUP_KEY, @@ -55,16 +58,27 @@ public abstract class SpeedDialUiItem { Phone.DISPLAY_NAME, Phone.STARRED, Phone.NUMBER, + Phone.TYPE, Phone.LABEL, Phone.PHOTO_ID, - Phone.PHOTO_URI + Phone.PHOTO_URI, + Phone.CARRIER_PRESENCE }; public static Builder builder() { return new AutoValue_SpeedDialUiItem.Builder().setChannels(ImmutableList.of()); } - /** Convert a cursor with projection {@link #PHONE_PROJECTION} into a {@link SpeedDialUiItem}. */ + /** + * Convert a cursor with projection {@link #PHONE_PROJECTION} into a {@link SpeedDialUiItem}. + * + *

This cursor is structured such that contacts are grouped by contact id and lookup key and + * each row that shares the same contact id and lookup key represents a phone number that belongs + * to a single contact. + * + *

If the cursor started at row X, this method will advance to row Y s.t. rows X, X + 1, ... Y + * - 1 all belong to the same contact (that is, share the same contact id and lookup key). + */ public static SpeedDialUiItem fromCursor(Cursor cursor) { Assert.checkArgument(cursor != null); Assert.checkArgument(cursor.getCount() != 0); @@ -84,13 +98,20 @@ public abstract class SpeedDialUiItem { // contact's phone numbers. List channels = new ArrayList<>(); do { - channels.add( + Channel channel = Channel.builder() .setNumber(cursor.getString(NUMBER)) + .setPhoneType(cursor.getInt(TYPE)) .setLabel(TextUtils.isEmpty(cursor.getString(LABEL)) ? "" : cursor.getString(LABEL)) - // TODO(a bug): add another channel for each technology (Duo, ViLTE, ect.) .setTechnology(Channel.VOICE) - .build()); + .build(); + channels.add(channel); + + if ((cursor.getInt(CARRIER_PRESENCE) & Data.CARRIER_PRESENCE_VT_CAPABLE) == 1) { + // Add another channel if the number is ViLTE reachable + channels.add(channel.toBuilder().setTechnology(Channel.IMS_VIDEO).build()); + } + // TODO(a bug): add another channel for Duo (needs to happen on main thread) } while (cursor.moveToNext() && Objects.equals(lookupKey, cursor.getString(LOOKUP_KEY))); builder.setChannels(ImmutableList.copyOf(channels)); diff --git a/java/com/android/dialer/speeddial/loader/SpeedDialUiItemLoader.java b/java/com/android/dialer/speeddial/loader/SpeedDialUiItemLoader.java index c80a06245..31494805f 100644 --- a/java/com/android/dialer/speeddial/loader/SpeedDialUiItemLoader.java +++ b/java/com/android/dialer/speeddial/loader/SpeedDialUiItemLoader.java @@ -25,21 +25,26 @@ import android.provider.ContactsContract; import android.provider.ContactsContract.CommonDataKinds.Phone; import android.provider.ContactsContract.Contacts; import android.support.annotation.WorkerThread; +import android.util.ArrayMap; +import android.util.ArraySet; import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; import com.android.dialer.common.concurrent.Annotations.BackgroundExecutor; import com.android.dialer.common.concurrent.DialerExecutor.SuccessListener; import com.android.dialer.common.concurrent.DialerFutureSerializer; +import com.android.dialer.common.database.Selection; import com.android.dialer.inject.ApplicationContext; import com.android.dialer.speeddial.database.SpeedDialEntry; -import com.android.dialer.speeddial.database.SpeedDialEntry.Channel; import com.android.dialer.speeddial.database.SpeedDialEntryDao; import com.android.dialer.speeddial.database.SpeedDialEntryDatabaseHelper; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import javax.inject.Inject; import javax.inject.Singleton; @@ -65,7 +70,7 @@ import javax.inject.Singleton; @SuppressWarnings("AndroidApiChecker") @TargetApi(VERSION_CODES.N) @Singleton -public final class SpeedDialUiItemLoader implements UiItemLoader { +public final class SpeedDialUiItemLoader { private final Context appContext; private final ListeningExecutorService backgroundExecutor; @@ -85,7 +90,6 @@ public final class SpeedDialUiItemLoader implements UiItemLoader { * list is composed of starred contacts from {@link SpeedDialEntryDatabaseHelper} and suggestions * from {@link Contacts#STREQUENT_PHONE_ONLY}. */ - @Override public ListenableFuture> loadSpeedDialUiItems() { return dialerFutureSerializer.submitAsync( () -> backgroundExecutor.submit(this::doInBackground), backgroundExecutor); @@ -104,10 +108,19 @@ public final class SpeedDialUiItemLoader implements UiItemLoader { List entriesToUpdate = new ArrayList<>(); List entriesToDelete = new ArrayList<>(); - // Get all SpeedDialEntries and mark them to be updated or deleted + // Get all SpeedDialEntries and update their contact ids and lookupkeys. List entries = db.getAllEntries(); + entries = updateContactIdsAndLookupKeys(entries); + + // Build SpeedDialUiItems from our updated entries. + Map entriesToUiItems = getSpeedDialUiItemsFromEntries(entries); + Assert.checkArgument( + entries.size() == entriesToUiItems.size(), + "Updated entries are incomplete: " + entries.size() + " != " + entriesToUiItems.size()); + + // Mark the SpeedDialEntries to be updated or deleted for (SpeedDialEntry entry : entries) { - SpeedDialUiItem contact = getSpeedDialContact(entry); + SpeedDialUiItem contact = entriesToUiItems.get(entry); // Remove contacts that no longer exist or are no longer starred if (contact == null || !contact.isStarred()) { entriesToDelete.add(entry.id()); @@ -159,48 +172,108 @@ public final class SpeedDialUiItemLoader implements UiItemLoader { return ImmutableList.copyOf(speedDialUiItems); } + /** + * Returns the same list of SpeedDialEntries that are passed in except their contact ids and + * lookup keys are updated to current values. + * + *

Unfortunately, we need to look up each contact individually to update the contact id and + * lookup key. Luckily though, this query is highly optimized on the framework side and very + * quick. + */ + @WorkerThread + private List updateContactIdsAndLookupKeys(List entries) { + Assert.isWorkerThread(); + List updatedEntries = new ArrayList<>(); + for (SpeedDialEntry entry : entries) { + try (Cursor cursor = + appContext + .getContentResolver() + .query( + Contacts.getLookupUri(entry.contactId(), entry.lookupKey()), + new String[] {Contacts._ID, Contacts.LOOKUP_KEY}, + null, + null, + null)) { + if (cursor == null) { + LogUtil.e("SpeedDialUiItemLoader.updateContactIdsAndLookupKeys", "null cursor"); + return new ArrayList<>(); + } + if (cursor.getCount() == 0) { + // No need to update this entry, the contact was deleted. We'll clear it up later. + updatedEntries.add(entry); + continue; + } + // Since all cursor rows will be have the same contact id and lookup key, just grab the + // first one. + cursor.moveToFirst(); + updatedEntries.add( + entry + .toBuilder() + .setContactId(cursor.getLong(0)) + .setLookupKey(cursor.getString(1)) + .build()); + } + } + return updatedEntries; + } + + /** + * Returns a map of SpeedDialEntries to their corresponding SpeedDialUiItems. Mappings to null + * elements imply that the contact was deleted. + */ @WorkerThread - private SpeedDialUiItem getSpeedDialContact(SpeedDialEntry entry) { + private Map getSpeedDialUiItemsFromEntries( + List entries) { Assert.isWorkerThread(); - // TODO(b77725860): Might need to use the lookup uri to get the contact id first, then query - // based on that. - SpeedDialUiItem contact; + // Fetch the contact ids from the SpeedDialEntries + Set contactIds = new HashSet<>(); + entries.forEach(entry -> contactIds.add(Long.toString(entry.contactId()))); + if (contactIds.isEmpty()) { + return new ArrayMap<>(); + } + + // Build SpeedDialUiItems from those contact ids and map them to their entries + Selection selection = + Selection.builder().and(Selection.column(Phone.CONTACT_ID).in(contactIds)).build(); try (Cursor cursor = appContext .getContentResolver() .query( Phone.CONTENT_URI, SpeedDialUiItem.PHONE_PROJECTION, - Phone.NUMBER + " IS NOT NULL AND " + Phone.LOOKUP_KEY + "=?", - new String[] {entry.lookupKey()}, + selection.getSelection(), + selection.getSelectionArgs(), null)) { - - if (cursor == null || cursor.getCount() == 0) { - // Contact not found, potentially deleted - LogUtil.e("SpeedDialUiItemLoader.getSpeedDialContact", "Contact not found."); - return null; + Map map = new ArrayMap<>(); + for (cursor.moveToFirst(); !cursor.isAfterLast(); /* Iterate in the loop */ ) { + SpeedDialUiItem item = SpeedDialUiItem.fromCursor(cursor); + for (SpeedDialEntry entry : entries) { + if (entry.contactId() == item.contactId()) { + // It's impossible for two contacts to exist with the same contact id, so if this entry + // was previously matched to a SpeedDialUiItem and is being matched again, something + // went horribly wrong. + Assert.checkArgument( + map.put(entry, item) == null, + "Each SpeedDialEntry only has one correct SpeedDialUiItem"); + } + } } - cursor.moveToFirst(); - contact = SpeedDialUiItem.fromCursor(cursor); - } - - // Preserve the default channel if it didn't change/still exists - Channel defaultChannel = entry.defaultChannel(); - if (defaultChannel != null) { - if (contact.channels().contains(defaultChannel)) { - contact = contact.toBuilder().setDefaultChannel(defaultChannel).build(); + // Contact must have been deleted + for (SpeedDialEntry entry : entries) { + map.putIfAbsent(entry, null); } + return map; } - - // TODO(calderwoodra): Consider setting the default channel if there is only one channel - return contact; } @WorkerThread private List getStrequentContacts() { Assert.isWorkerThread(); - Uri uri = + Set contactIds = new ArraySet<>(); + + // Fetch the contact ids of all strequent contacts + Uri strequentUri = Contacts.CONTENT_STREQUENT_URI .buildUpon() .appendQueryParameter(ContactsContract.STREQUENT_PHONE_ONLY, "true") @@ -208,14 +281,40 @@ public final class SpeedDialUiItemLoader implements UiItemLoader { try (Cursor cursor = appContext .getContentResolver() - .query(uri, SpeedDialUiItem.PHONE_PROJECTION, null, null, null)) { + .query(strequentUri, new String[] {Phone.CONTACT_ID}, null, null, null)) { + if (cursor == null) { + LogUtil.e("SpeedDialUiItemLoader.getStrequentContacts", "null cursor"); + return new ArrayList<>(); + } + if (cursor.getCount() == 0) { + return new ArrayList<>(); + } + for (cursor.moveToFirst(); !cursor.isAfterLast(); cursor.moveToNext()) { + contactIds.add(Long.toString(cursor.getLong(0))); + } + } + + // Build SpeedDialUiItems from those contact ids + Selection selection = + Selection.builder().and(Selection.column(Phone.CONTACT_ID).in(contactIds)).build(); + try (Cursor cursor = + appContext + .getContentResolver() + .query( + Phone.CONTENT_URI, + SpeedDialUiItem.PHONE_PROJECTION, + selection.getSelection(), + selection.getSelectionArgs(), + null)) { List contacts = new ArrayList<>(); - if (cursor == null || cursor.getCount() == 0) { + if (cursor == null) { + LogUtil.e("SpeedDialUiItemLoader.getStrequentContacts", "null cursor"); + return new ArrayList<>(); + } + if (cursor.getCount() == 0) { return contacts; } - - cursor.moveToPosition(-1); - while (cursor.moveToNext()) { + for (cursor.moveToFirst(); !cursor.isAfterLast(); /* Iterate in the loop */ ) { contacts.add(SpeedDialUiItem.fromCursor(cursor)); } return contacts; diff --git a/java/com/android/dialer/speeddial/loader/UiItemLoader.java b/java/com/android/dialer/speeddial/loader/UiItemLoader.java deleted file mode 100644 index 4b9a7319f..000000000 --- a/java/com/android/dialer/speeddial/loader/UiItemLoader.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (C) 2017 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License - */ - -package com.android.dialer.speeddial.loader; - -import com.google.common.collect.ImmutableList; -import com.google.common.util.concurrent.ListenableFuture; - -/** Provides operation for loading {@link SpeedDialUiItem SpeedDialUiItems} */ -public interface UiItemLoader { - - /** - * Returns a {@link ListenableFuture} for a list of {@link SpeedDialUiItem SpeedDialUiItems}. This - * list is composed of starred contacts from {@link - * com.android.dialer.speeddial.database.SpeedDialEntryDatabaseHelper} and suggestions from {@link - * android.provider.ContactsContract.Contacts#STREQUENT_PHONE_ONLY}. - */ - ListenableFuture> loadSpeedDialUiItems(); -} -- cgit v1.2.3