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
diff --git a/java/com/android/dialer/speeddial/database/SpeedDialEntry.java b/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
index 13ef4e3..89aed8f 100644
--- a/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
+++ b/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
@@ -92,16 +92,14 @@
/**
* 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 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 7c823bd..137933f 100644
--- a/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java
+++ b/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java
@@ -37,7 +37,7 @@
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 @@
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 @@
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 @@
+ (CONTACT_ID + " integer, ")
+ (LOOKUP_KEY + " text, ")
+ (PHONE_NUMBER + " text, ")
+ + (PHONE_TYPE + " integer, ")
+ (PHONE_LABEL + " text, ")
+ (PHONE_TECHNOLOGY + " integer ")
+ ");";
@@ -109,6 +112,7 @@
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 @@
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 3381bf8..28c2274 100644
--- a/java/com/android/dialer/speeddial/loader/SpeedDialUiItem.java
+++ b/java/com/android/dialer/speeddial/loader/SpeedDialUiItem.java
@@ -18,6 +18,7 @@
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 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 @@
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}.
+ *
+ * <p>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.
+ *
+ * <p>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 @@
// contact's phone numbers.
List<Channel> 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 c80a062..3149480 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.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 @@
@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 @@
* list is composed of starred contacts from {@link SpeedDialEntryDatabaseHelper} and suggestions
* from {@link Contacts#STREQUENT_PHONE_ONLY}.
*/
- @Override
public ListenableFuture<ImmutableList<SpeedDialUiItem>> loadSpeedDialUiItems() {
return dialerFutureSerializer.submitAsync(
() -> backgroundExecutor.submit(this::doInBackground), backgroundExecutor);
@@ -104,10 +108,19 @@
List<SpeedDialEntry> entriesToUpdate = new ArrayList<>();
List<Long> 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<SpeedDialEntry> entries = db.getAllEntries();
+ entries = updateContactIdsAndLookupKeys(entries);
+
+ // Build SpeedDialUiItems from our updated entries.
+ Map<SpeedDialEntry, SpeedDialUiItem> 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 @@
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.
+ *
+ * <p>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 SpeedDialUiItem getSpeedDialContact(SpeedDialEntry entry) {
+ private List<SpeedDialEntry> updateContactIdsAndLookupKeys(List<SpeedDialEntry> 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;
+ List<SpeedDialEntry> 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 Map<SpeedDialEntry, SpeedDialUiItem> getSpeedDialUiItemsFromEntries(
+ List<SpeedDialEntry> entries) {
+ Assert.isWorkerThread();
+ // Fetch the contact ids from the SpeedDialEntries
+ Set<String> 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<SpeedDialEntry, SpeedDialUiItem> 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<SpeedDialUiItem> getStrequentContacts() {
Assert.isWorkerThread();
- Uri uri =
+ Set<String> 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 @@
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<SpeedDialUiItem> 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 4b9a731..0000000
--- 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<ImmutableList<SpeedDialUiItem>> loadSpeedDialUiItems();
-}