From 75812adca7fdebc90eb150035e399625f5c36a8b Mon Sep 17 00:00:00 2001 From: zachh Date: Tue, 12 Dec 2017 11:22:57 -0800 Subject: Updated writing of PhoneLookup columns in annotated call log. We're not going to use the "cached" columns from the system call log any longer, and instead will write them using PhoneLookup. Bug: 34672501 Test: unit PiperOrigin-RevId: 178788155 Change-Id: I9255dd3cb727eef3b45bc05aeb3c6fd6fd513d63 --- .../contract/AnnotatedCallLogContract.java | 16 ++--- .../phonelookup/PhoneLookupDataSource.java | 31 ++++++--- .../systemcalllog/SystemCallLogDataSource.java | 57 +++------------- .../dialer/phonelookup/PhoneLookupSelector.java | 79 ++++++++++++++++++++-- .../dialer/phonelookup/cp2/Cp2PhoneLookup.java | 77 ++++++++++++++------- .../dialer/phonelookup/phone_lookup_info.proto | 6 +- 6 files changed, 172 insertions(+), 94 deletions(-) diff --git a/java/com/android/dialer/calllog/database/contract/AnnotatedCallLogContract.java b/java/com/android/dialer/calllog/database/contract/AnnotatedCallLogContract.java index c9c053cea..7071ab5c1 100644 --- a/java/com/android/dialer/calllog/database/contract/AnnotatedCallLogContract.java +++ b/java/com/android/dialer/calllog/database/contract/AnnotatedCallLogContract.java @@ -42,10 +42,9 @@ public class AnnotatedCallLogContract { String TIMESTAMP = "timestamp"; /** - * Copied from {@link android.provider.CallLog.Calls#CACHED_NAME}. - * - *

This is exactly how it should appear to the user. If the user's locale or name display - * preferences change, this column should be rewritten. + * The name (which may be a person's name or business name, but not a number) formatted exactly + * as it should appear to the user. If the user's locale or name display preferences change, + * this column should be rewritten. * *

Type: TEXT */ @@ -61,28 +60,29 @@ public class AnnotatedCallLogContract { String NUMBER = "number"; /** - * Copied from {@link android.provider.CallLog.Calls#CACHED_FORMATTED_NUMBER}. + * The number formatted as it should be displayed to the user. Note that it may not always be + * displayed, for example if the number has a corresponding person or business name. * *

Type: TEXT */ String FORMATTED_NUMBER = "formatted_number"; /** - * Copied from {@link android.provider.CallLog.Calls#CACHED_PHOTO_URI}. + * A photo URI for the contact to display in the call log list view. * *

TYPE: TEXT */ String PHOTO_URI = "photo_uri"; /** - * Copied from {@link android.provider.CallLog.Calls#CACHED_PHOTO_ID}. + * A photo ID (from the contacts provider) for the contact to display in the call log list view. * *

Type: INTEGER (long) */ String PHOTO_ID = "photo_id"; /** - * Copied from {@link android.provider.CallLog.Calls#CACHED_LOOKUP_URI}. + * The contacts provider lookup URI for the contact associated with the call. * *

TYPE: TEXT */ diff --git a/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java b/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java index fa7d3be16..93e841409 100644 --- a/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java +++ b/java/com/android/dialer/calllog/datasources/phonelookup/PhoneLookupDataSource.java @@ -32,6 +32,7 @@ import com.android.dialer.DialerPhoneNumber; import com.android.dialer.calllog.database.contract.AnnotatedCallLogContract.AnnotatedCallLog; import com.android.dialer.calllog.datasources.CallLogDataSource; import com.android.dialer.calllog.datasources.CallLogMutations; +import com.android.dialer.calllog.datasources.util.RowCombiner; import com.android.dialer.common.LogUtil; import com.android.dialer.common.concurrent.Annotations.BackgroundExecutor; import com.android.dialer.common.concurrent.Annotations.LightweightExecutor; @@ -260,8 +261,13 @@ public final class PhoneLookupDataSource implements CallLogDataSource { @WorkerThread @Override public ContentValues coalesce(List individualRowsSortedByTimestampDesc) { - // TODO(zachh): Implementation. - return new ContentValues(); + return new RowCombiner(individualRowsSortedByTimestampDesc) + .useMostRecentString(AnnotatedCallLog.NAME) + .useMostRecentString(AnnotatedCallLog.NUMBER_TYPE_LABEL) + .useMostRecentString(AnnotatedCallLog.PHOTO_URI) + .useMostRecentLong(AnnotatedCallLog.PHOTO_ID) + .useMostRecentString(AnnotatedCallLog.LOOKUP_URI) + .combine(); } @MainThread @@ -434,7 +440,7 @@ public final class PhoneLookupDataSource implements CallLogDataSource { PhoneLookupInfo phoneLookupInfo = existingInfo.get(id); // Existing info might be missing if data was cleared or for other reasons. if (phoneLookupInfo != null) { - contentValues.put(AnnotatedCallLog.NAME, selectName(phoneLookupInfo)); + updateContentValues(contentValues, phoneLookupInfo); } } } @@ -474,17 +480,17 @@ public final class PhoneLookupDataSource implements CallLogDataSource { * mutations from PhoneLookupHistory; in this case "John" would be copied during * populateInserts() and there wouldn't be further updates needed here. */ - contentValuesToInsert.put(AnnotatedCallLog.NAME, selectName(phoneLookupInfo)); + updateContentValues(contentValuesToInsert, phoneLookupInfo); continue; } ContentValues contentValuesToUpdate = mutations.getUpdates().get(id); if (contentValuesToUpdate != null) { - contentValuesToUpdate.put(AnnotatedCallLog.NAME, selectName(phoneLookupInfo)); + updateContentValues(contentValuesToUpdate, phoneLookupInfo); continue; } // Else this row is not already scheduled for insert or update and we need to schedule it. ContentValues contentValues = new ContentValues(); - contentValues.put(AnnotatedCallLog.NAME, selectName(phoneLookupInfo)); + updateContentValues(contentValues, phoneLookupInfo); mutations.getUpdates().put(id, contentValues); } } @@ -525,8 +531,17 @@ public final class PhoneLookupDataSource implements CallLogDataSource { return normalizedNumbersToDelete; } - private static String selectName(PhoneLookupInfo phoneLookupInfo) { - return PhoneLookupSelector.selectName(phoneLookupInfo); + private static void updateContentValues( + ContentValues contentValues, PhoneLookupInfo phoneLookupInfo) { + contentValues.put(AnnotatedCallLog.NAME, PhoneLookupSelector.selectName(phoneLookupInfo)); + contentValues.put( + AnnotatedCallLog.PHOTO_URI, PhoneLookupSelector.selectPhotoUri(phoneLookupInfo)); + contentValues.put( + AnnotatedCallLog.PHOTO_ID, PhoneLookupSelector.selectPhotoId(phoneLookupInfo)); + contentValues.put( + AnnotatedCallLog.LOOKUP_URI, PhoneLookupSelector.selectLookupUri(phoneLookupInfo)); + contentValues.put( + AnnotatedCallLog.NUMBER_TYPE_LABEL, PhoneLookupSelector.selectNumberLabel(phoneLookupInfo)); } private static Uri numberUri(String number) { diff --git a/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java b/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java index 91db915ef..0ed185966 100644 --- a/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java +++ b/java/com/android/dialer/calllog/datasources/systemcalllog/SystemCallLogDataSource.java @@ -27,7 +27,6 @@ import android.os.Build; import android.os.Handler; import android.provider.CallLog; import android.provider.CallLog.Calls; -import android.provider.ContactsContract.CommonDataKinds.Phone; import android.support.annotation.ColorInt; import android.support.annotation.MainThread; import android.support.annotation.Nullable; @@ -35,6 +34,7 @@ import android.support.annotation.VisibleForTesting; import android.support.annotation.WorkerThread; import android.telecom.PhoneAccount; import android.telecom.PhoneAccountHandle; +import android.telephony.PhoneNumberUtils; import android.text.TextUtils; import android.util.ArraySet; import com.android.dialer.DialerPhoneNumber; @@ -174,23 +174,16 @@ public class SystemCallLogDataSource implements CallLogDataSource { @Override public ContentValues coalesce(List individualRowsSortedByTimestampDesc) { - // TODO(zachh): Complete implementation. - assertNoVoicemailsInRows(individualRowsSortedByTimestampDesc); return new RowCombiner(individualRowsSortedByTimestampDesc) .useMostRecentLong(AnnotatedCallLog.TIMESTAMP) .useMostRecentLong(AnnotatedCallLog.NEW) - .useMostRecentString(AnnotatedCallLog.NUMBER_TYPE_LABEL) - .useMostRecentString(AnnotatedCallLog.NAME) // Two different DialerPhoneNumbers could be combined if they are different but considered // to be an "exact match" by libphonenumber; in this case we arbitrarily select the most // recent one. .useMostRecentBlob(AnnotatedCallLog.NUMBER) .useMostRecentString(AnnotatedCallLog.FORMATTED_NUMBER) - .useMostRecentString(AnnotatedCallLog.PHOTO_URI) - .useMostRecentLong(AnnotatedCallLog.PHOTO_ID) - .useMostRecentString(AnnotatedCallLog.LOOKUP_URI) .useMostRecentString(AnnotatedCallLog.GEOCODED_LOCATION) .useSingleValueString(AnnotatedCallLog.PHONE_ACCOUNT_COMPONENT_NAME) .useSingleValueString(AnnotatedCallLog.PHONE_ACCOUNT_ID) @@ -233,13 +226,6 @@ public class SystemCallLogDataSource implements CallLogDataSource { Calls.NUMBER, Calls.TYPE, Calls.COUNTRY_ISO, - Calls.CACHED_NAME, - Calls.CACHED_FORMATTED_NUMBER, - Calls.CACHED_PHOTO_URI, - Calls.CACHED_PHOTO_ID, - Calls.CACHED_LOOKUP_URI, - Calls.CACHED_NUMBER_TYPE, - Calls.CACHED_NUMBER_LABEL, Calls.DURATION, Calls.DATA_USAGE, Calls.TRANSCRIPTION, @@ -272,14 +258,6 @@ public class SystemCallLogDataSource implements CallLogDataSource { int numberColumn = cursor.getColumnIndexOrThrow(Calls.NUMBER); int typeColumn = cursor.getColumnIndexOrThrow(Calls.TYPE); int countryIsoColumn = cursor.getColumnIndexOrThrow(Calls.COUNTRY_ISO); - int cachedNameColumn = cursor.getColumnIndexOrThrow(Calls.CACHED_NAME); - int cachedFormattedNumberColumn = - cursor.getColumnIndexOrThrow(Calls.CACHED_FORMATTED_NUMBER); - int cachedPhotoUriColumn = cursor.getColumnIndexOrThrow(Calls.CACHED_PHOTO_URI); - int cachedPhotoIdColumn = cursor.getColumnIndexOrThrow(Calls.CACHED_PHOTO_ID); - int cachedLookupUriColumn = cursor.getColumnIndexOrThrow(Calls.CACHED_LOOKUP_URI); - int cachedNumberTypeColumn = cursor.getColumnIndexOrThrow(Calls.CACHED_NUMBER_TYPE); - int cachedNumberLabelColumn = cursor.getColumnIndexOrThrow(Calls.CACHED_NUMBER_LABEL); int durationsColumn = cursor.getColumnIndexOrThrow(Calls.DURATION); int dataUsageColumn = cursor.getColumnIndexOrThrow(Calls.DATA_USAGE); int transcriptionColumn = cursor.getColumnIndexOrThrow(Calls.TRANSCRIPTION); @@ -301,13 +279,6 @@ public class SystemCallLogDataSource implements CallLogDataSource { String numberAsStr = cursor.getString(numberColumn); long type = cursor.getInt(typeColumn); String countryIso = cursor.getString(countryIsoColumn); - String cachedName = cursor.getString(cachedNameColumn); - String formattedNumber = cursor.getString(cachedFormattedNumberColumn); - String cachedPhotoUri = cursor.getString(cachedPhotoUriColumn); - long cachedPhotoId = cursor.getLong(cachedPhotoIdColumn); - String cachedLookupUri = cursor.getString(cachedLookupUriColumn); - int cachedNumberType = cursor.getInt(cachedNumberTypeColumn); - String cachedNumberLabel = cursor.getString(cachedNumberLabelColumn); int duration = cursor.getInt(durationsColumn); int dataUsage = cursor.getInt(dataUsageColumn); String transcription = cursor.getString(transcriptionColumn); @@ -323,31 +294,19 @@ public class SystemCallLogDataSource implements CallLogDataSource { contentValues.put(AnnotatedCallLog.TIMESTAMP, date); if (!TextUtils.isEmpty(numberAsStr)) { - byte[] numberAsProtoBytes = - dialerPhoneNumberUtil.parse(numberAsStr, countryIso).toByteArray(); + DialerPhoneNumber dialerPhoneNumber = + dialerPhoneNumberUtil.parse(numberAsStr, countryIso); + + contentValues.put(AnnotatedCallLog.NUMBER, dialerPhoneNumber.toByteArray()); + contentValues.put( + AnnotatedCallLog.FORMATTED_NUMBER, + PhoneNumberUtils.formatNumber(numberAsStr, countryIso)); // TODO(zachh): Need to handle post-dial digits; different on N and M. - contentValues.put(AnnotatedCallLog.NUMBER, numberAsProtoBytes); } else { contentValues.put( AnnotatedCallLog.NUMBER, DialerPhoneNumber.getDefaultInstance().toByteArray()); } - contentValues.put(AnnotatedCallLog.CALL_TYPE, type); - contentValues.put(AnnotatedCallLog.NAME, cachedName); - // TODO(zachh): Format the number using DialerPhoneNumberUtil here. - contentValues.put(AnnotatedCallLog.FORMATTED_NUMBER, formattedNumber); - contentValues.put(AnnotatedCallLog.PHOTO_URI, cachedPhotoUri); - contentValues.put(AnnotatedCallLog.PHOTO_ID, cachedPhotoId); - contentValues.put(AnnotatedCallLog.LOOKUP_URI, cachedLookupUri); - - // Phone.getTypeLabel returns "Custom" if given (0, null) which is not of any use. Just - // omit setting the label if there's no information for it. - if (cachedNumberType != 0 || cachedNumberLabel != null) { - contentValues.put( - AnnotatedCallLog.NUMBER_TYPE_LABEL, - Phone.getTypeLabel(appContext.getResources(), cachedNumberType, cachedNumberLabel) - .toString()); - } contentValues.put(AnnotatedCallLog.IS_READ, isRead); contentValues.put(AnnotatedCallLog.NEW, isNew); contentValues.put(AnnotatedCallLog.GEOCODED_LOCATION, geocodedLocation); diff --git a/java/com/android/dialer/phonelookup/PhoneLookupSelector.java b/java/com/android/dialer/phonelookup/PhoneLookupSelector.java index a746ea44f..af8b849e5 100644 --- a/java/com/android/dialer/phonelookup/PhoneLookupSelector.java +++ b/java/com/android/dialer/phonelookup/PhoneLookupSelector.java @@ -16,6 +16,8 @@ package com.android.dialer.phonelookup; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; +import com.android.dialer.phonelookup.PhoneLookupInfo.Cp2Info.Cp2ContactInfo; /** * Prioritizes information from a {@link PhoneLookupInfo}. @@ -37,11 +39,80 @@ public final class PhoneLookupSelector { */ @NonNull public static String selectName(PhoneLookupInfo phoneLookupInfo) { - if (phoneLookupInfo.getCp2Info().getCp2ContactInfoCount() > 0) { - // Arbitrarily select the first contact's name. In the future, it may make sense to join the - // names such as "Mom, Dad" in the case that multiple contacts share the same number. - return phoneLookupInfo.getCp2Info().getCp2ContactInfo(0).getName(); + Cp2ContactInfo firstLocalContact = firstLocalContact(phoneLookupInfo); + if (firstLocalContact != null) { + String name = firstLocalContact.getName(); + if (!name.isEmpty()) { + return firstLocalContact.getName(); + } + } + return ""; + } + + /** Select the photo URI associated with this number. */ + @NonNull + public static String selectPhotoUri(PhoneLookupInfo phoneLookupInfo) { + Cp2ContactInfo firstLocalContact = firstLocalContact(phoneLookupInfo); + if (firstLocalContact != null) { + String photoUri = firstLocalContact.getPhotoUri(); + if (!photoUri.isEmpty()) { + return photoUri; + } + } + return ""; + } + + /** Select the photo ID associated with this number, or 0 if there is none. */ + public static long selectPhotoId(PhoneLookupInfo phoneLookupInfo) { + Cp2ContactInfo firstLocalContact = firstLocalContact(phoneLookupInfo); + if (firstLocalContact != null) { + long photoId = firstLocalContact.getPhotoId(); + if (photoId > 0) { + return photoId; + } + } + return 0; + } + + /** Select the lookup URI associated with this number. */ + @NonNull + public static String selectLookupUri(PhoneLookupInfo phoneLookupInfo) { + Cp2ContactInfo firstLocalContact = firstLocalContact(phoneLookupInfo); + if (firstLocalContact != null) { + String lookupUri = firstLocalContact.getLookupUri(); + if (!lookupUri.isEmpty()) { + return lookupUri; + } } return ""; } + + /** + * A localized string representing the number type such as "Home" or "Mobile", or a custom value + * set by the user. + */ + @NonNull + public static String selectNumberLabel(PhoneLookupInfo phoneLookupInfo) { + Cp2ContactInfo firstLocalContact = firstLocalContact(phoneLookupInfo); + if (firstLocalContact != null) { + String label = firstLocalContact.getLabel(); + if (!label.isEmpty()) { + return label; + } + } + return ""; + } + + /** + * Arbitrarily select the first contact. In the future, it may make sense to display contact + * information from all contacts with the same number (for example show the name as "Mom, Dad" or + * show a synthesized photo containing photos of both "Mom" and "Dad"). + */ + @Nullable + private static Cp2ContactInfo firstLocalContact(PhoneLookupInfo phoneLookupInfo) { + if (phoneLookupInfo.getCp2Info().getCp2ContactInfoCount() > 0) { + return phoneLookupInfo.getCp2Info().getCp2ContactInfo(0); + } + return null; + } } diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java index 03e05b563..3829a8df1 100644 --- a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java +++ b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java @@ -58,17 +58,21 @@ public final class Cp2PhoneLookup implements PhoneLookup { Phone.DISPLAY_NAME_PRIMARY, // 0 Phone.PHOTO_THUMBNAIL_URI, // 1 Phone.PHOTO_ID, // 2 - Phone.LABEL, // 3 - Phone.NORMALIZED_NUMBER, // 4 - Phone.CONTACT_ID, // 5 + Phone.TYPE, // 3 + Phone.LABEL, // 4 + Phone.NORMALIZED_NUMBER, // 5 + Phone.CONTACT_ID, // 6 + Phone.LOOKUP_KEY // 7 }; private static final int CP2_INFO_NAME_INDEX = 0; private static final int CP2_INFO_PHOTO_URI_INDEX = 1; private static final int CP2_INFO_PHOTO_ID_INDEX = 2; - private static final int CP2_INFO_LABEL_INDEX = 3; - private static final int CP2_INFO_NUMBER_INDEX = 4; - private static final int CP2_INFO_CONTACT_ID_INDEX = 5; + private static final int CP2_INFO_TYPE_INDEX = 3; + private static final int CP2_INFO_LABEL_INDEX = 4; + private static final int CP2_INFO_NUMBER_INDEX = 5; + private static final int CP2_INFO_CONTACT_ID_INDEX = 6; + private static final int CP2_INFO_LOOKUP_KEY_INDEX = 7; private final Context appContext; private final SharedPreferences sharedPreferences; @@ -89,6 +93,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { @Override public ListenableFuture lookup(@NonNull Call call) { // TODO(zachh): Implementation. + // TODO(zachh): Note: Should write empty Cp2Info even when no contact found. return backgroundExecutorService.submit(PhoneLookupInfo::getDefaultInstance); } @@ -207,7 +212,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { // For each DialerPhoneNumber that was associated with a contact or added to a contact, // build a map of those DialerPhoneNumbers to a set Cp2ContactInfos, where each Cp2ContactInfo // represents a contact. - ImmutableMap> updatedContacts = + Map> updatedContacts = buildMapForUpdatedOrAddedContacts(existingInfoMap, lastModified, deletedPhoneNumbers); // Start build a new map of updated info. This will replace existing info. @@ -216,23 +221,26 @@ public final class Cp2PhoneLookup implements PhoneLookup { // For each DialerPhoneNumber in existing info... for (Entry entry : existingInfoMap.entrySet()) { + DialerPhoneNumber dialerPhoneNumber = entry.getKey(); + PhoneLookupInfo existingInfo = entry.getValue(); + // Build off the existing info - PhoneLookupInfo.Builder infoBuilder = PhoneLookupInfo.newBuilder(entry.getValue()); + PhoneLookupInfo.Builder infoBuilder = PhoneLookupInfo.newBuilder(existingInfo); // If the contact was updated, replace the Cp2ContactInfo list - if (updatedContacts.containsKey(entry.getKey())) { + if (updatedContacts.containsKey(dialerPhoneNumber)) { infoBuilder.setCp2Info( - Cp2Info.newBuilder().addAllCp2ContactInfo(updatedContacts.get(entry.getKey()))); + Cp2Info.newBuilder().addAllCp2ContactInfo(updatedContacts.get(dialerPhoneNumber))); // If it was deleted and not added to a new contact, replace the Cp2ContactInfo list with // the default instance of Cp2ContactInfo - } else if (deletedPhoneNumbers.contains(entry.getKey())) { + } else if (deletedPhoneNumbers.contains(dialerPhoneNumber)) { infoBuilder.setCp2Info( Cp2Info.newBuilder().addCp2ContactInfo(Cp2ContactInfo.getDefaultInstance())); } // If the DialerPhoneNumber didn't change, add the unchanged existing info. - newInfoMapBuilder.put(entry.getKey(), infoBuilder.build()); + newInfoMapBuilder.put(dialerPhoneNumber, infoBuilder.build()); } return newInfoMapBuilder.build(); } @@ -260,7 +268,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { * @return Map of {@link DialerPhoneNumber} to {@link PhoneLookupInfo} with updated {@link * Cp2ContactInfo}. */ - private ImmutableMap> buildMapForUpdatedOrAddedContacts( + private Map> buildMapForUpdatedOrAddedContacts( ImmutableMap existingInfoMap, long lastModified, Set deletedPhoneNumbers) { @@ -270,16 +278,24 @@ public final class Cp2PhoneLookup implements PhoneLookup { Set contactIds = new ArraySet<>(); for (Entry entry : existingInfoMap.entrySet()) { + DialerPhoneNumber dialerPhoneNumber = entry.getKey(); + PhoneLookupInfo existingInfo = entry.getValue(); + // If the number was deleted, we need to check if it was added to a new contact. - if (deletedPhoneNumbers.contains(entry.getKey())) { - updatedNumbers.add(entry.getKey()); + if (deletedPhoneNumbers.contains(dialerPhoneNumber)) { + updatedNumbers.add(dialerPhoneNumber); continue; } + // Note: Methods in this class must always set at least one Cp2Info, setting it to + // getDefaultInstance() if there is no information for the contact. + Assert.checkState( + existingInfo.getCp2Info().getCp2ContactInfoCount() > 0, "existing info has no cp2 infos"); + // For each Cp2ContactInfo for each existing DialerPhoneNumber... // Store the contact id if it exist, else automatically add the DialerPhoneNumber to our // set of DialerPhoneNumbers we want to update. - for (Cp2ContactInfo cp2ContactInfo : entry.getValue().getCp2Info().getCp2ContactInfoList()) { + for (Cp2ContactInfo cp2ContactInfo : existingInfo.getCp2Info().getCp2ContactInfoList()) { if (Objects.equals(cp2ContactInfo, Cp2ContactInfo.getDefaultInstance())) { // If the number doesn't have any Cp2ContactInfo set to it, for various reasons, we need // to look up the number to check if any exists. @@ -287,7 +303,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { // - An existing contact that wasn't in the call log is now in the call log. // - A number was in the call log before but has now been added to a contact. // - A number is in the call log, but isn't associated with any contact. - updatedNumbers.add(entry.getKey()); + updatedNumbers.add(dialerPhoneNumber); } else { contactIds.add(cp2ContactInfo.getContactId()); } @@ -322,7 +338,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { // Map each dialer phone number to it's new cp2 info Set phoneNumbers = getDialerPhoneNumbers(updatedNumbers, cursor.getString(CP2_INFO_NUMBER_INDEX)); - Cp2ContactInfo info = buildCp2ContactInfoFromUpdatedContactsCursor(cursor); + Cp2ContactInfo info = buildCp2ContactInfoFromUpdatedContactsCursor(appContext, cursor); for (DialerPhoneNumber phoneNumber : phoneNumbers) { if (map.containsKey(phoneNumber)) { map.get(phoneNumber).add(info); @@ -334,7 +350,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { } } } - return ImmutableMap.copyOf(map); + return map; } /** @@ -358,10 +374,15 @@ public final class Cp2PhoneLookup implements PhoneLookup { * @param cursor with projection {@link #CP2_INFO_PROJECTION}. * @return new {@link Cp2ContactInfo} based on current row of {@code cursor}. */ - private static Cp2ContactInfo buildCp2ContactInfoFromUpdatedContactsCursor(Cursor cursor) { + private static Cp2ContactInfo buildCp2ContactInfoFromUpdatedContactsCursor( + Context appContext, Cursor cursor) { String displayName = cursor.getString(CP2_INFO_NAME_INDEX); String photoUri = cursor.getString(CP2_INFO_PHOTO_URI_INDEX); + int photoId = cursor.getInt(CP2_INFO_PHOTO_ID_INDEX); + int type = cursor.getInt(CP2_INFO_TYPE_INDEX); String label = cursor.getString(CP2_INFO_LABEL_INDEX); + int contactId = cursor.getInt(CP2_INFO_CONTACT_ID_INDEX); + String lookupKey = cursor.getString(CP2_INFO_LOOKUP_KEY_INDEX); Cp2ContactInfo.Builder infoBuilder = Cp2ContactInfo.newBuilder(); if (!TextUtils.isEmpty(displayName)) { @@ -370,11 +391,19 @@ public final class Cp2PhoneLookup implements PhoneLookup { if (!TextUtils.isEmpty(photoUri)) { infoBuilder.setPhotoUri(photoUri); } - if (!TextUtils.isEmpty(label)) { - infoBuilder.setLabel(label); + if (photoId > 0) { + infoBuilder.setPhotoId(photoId); + } + + // Phone.getTypeLabel returns "Custom" if given (0, null) which is not of any use. Just + // omit setting the label if there's no information for it. + if (type != 0 || !TextUtils.isEmpty(label)) { + infoBuilder.setLabel(Phone.getTypeLabel(appContext.getResources(), type, label).toString()); + } + infoBuilder.setContactId(contactId); + if (!TextUtils.isEmpty(lookupKey)) { + infoBuilder.setLookupUri(Contacts.getLookupUri(contactId, lookupKey).toString()); } - infoBuilder.setPhotoId(cursor.getLong(CP2_INFO_PHOTO_ID_INDEX)); - infoBuilder.setContactId(cursor.getLong(CP2_INFO_CONTACT_ID_INDEX)); return infoBuilder.build(); } diff --git a/java/com/android/dialer/phonelookup/phone_lookup_info.proto b/java/com/android/dialer/phonelookup/phone_lookup_info.proto index 93dd01e86..36596c1a2 100644 --- a/java/com/android/dialer/phonelookup/phone_lookup_info.proto +++ b/java/com/android/dialer/phonelookup/phone_lookup_info.proto @@ -34,9 +34,13 @@ message PhoneLookupInfo { // android.provider.ContactsContract.CommonDataKinds.Phone.CONTACT_ID optional fixed64 contact_id = 5; + + // android.provider.ContactsContract.CONTENT_LOOKUP_URI + optional string lookup_uri = 6; } // Repeated because one phone number can be associated with multiple CP2 - // contacts. + // contacts. If no contact is found for a number, this will contain exactly + // one element which is the default Cp2ContactInfo instance. repeated Cp2ContactInfo cp2_contact_info = 1; } optional Cp2Info cp2_info = 1; -- cgit v1.2.3 From 4b43c451a7215ef05622fb9bdb6cb7d84e70efec Mon Sep 17 00:00:00 2001 From: uabdullah Date: Tue, 12 Dec 2017 11:41:57 -0800 Subject: Download and play voicemails from server when not locally available. Test: Unit tests PiperOrigin-RevId: 178791213 Change-Id: I9e68c561285988cc1def894f5c7ecf9715ecf6b6 --- .../voicemail/listui/NewVoicemailAdapter.java | 39 ++++++- .../voicemail/listui/NewVoicemailFragment.java | 24 ++++- .../voicemail/listui/NewVoicemailMediaPlayer.java | 20 ++++ .../listui/NewVoicemailMediaPlayerView.java | 119 +++++++++++++++++++-- .../voicemail/listui/NewVoicemailViewHolder.java | 19 ++++ .../layout/new_voicemail_media_player_layout.xml | 1 - 6 files changed, 205 insertions(+), 17 deletions(-) diff --git a/java/com/android/dialer/voicemail/listui/NewVoicemailAdapter.java b/java/com/android/dialer/voicemail/listui/NewVoicemailAdapter.java index 955c7daee..671a39a67 100644 --- a/java/com/android/dialer/voicemail/listui/NewVoicemailAdapter.java +++ b/java/com/android/dialer/voicemail/listui/NewVoicemailAdapter.java @@ -56,7 +56,7 @@ final class NewVoicemailAdapter extends RecyclerView.Adapter int VOICEMAIL_ENTRY = 2; } - private final Cursor cursor; + private Cursor cursor; private final Clock clock; /** {@link Integer#MAX_VALUE} when the "Today" header should not be displayed. */ @@ -129,6 +129,11 @@ final class NewVoicemailAdapter extends RecyclerView.Adapter mediaPlayer.setOnErrorListener(onErrorListener); } + public void updateCursor(Cursor updatedCursor) { + this.cursor = updatedCursor; + notifyDataSetChanged(); + } + @Override public ViewHolder onCreateViewHolder(ViewGroup viewGroup, @RowType int viewType) { LogUtil.enterBlock("NewVoicemailAdapter.onCreateViewHolder"); @@ -714,7 +719,7 @@ final class NewVoicemailAdapter extends RecyclerView.Adapter // returned when currentlyExpandedViewHolderId = -1 (viewholder was collapsed) LogUtil.i( "NewVoicemailAdapter.getCurrentlyExpandedViewHolder", - "no view holder found in newVoicemailViewHolderArrayMap size:%d for %d", + "no view holder found in hashmap size:%d for %d", newVoicemailViewHolderArrayMap.size(), currentlyExpandedViewHolderId); // TODO(uabdullah): a bug Remove logging, temporarily here for debugging. @@ -749,4 +754,34 @@ final class NewVoicemailAdapter extends RecyclerView.Adapter } return RowType.VOICEMAIL_ENTRY; } + + /** + * This will be called once the voicemail that was attempted to be played (and was not locally + * available) was downloaded from the server. However it is possible that by the time the download + * was completed, the view holder was collapsed. In that case we shouldn't play the voicemail. + */ + public void checkAndPlayVoicemail() { + LogUtil.i( + "NewVoicemailAdapter.checkAndPlayVoicemail", + "expandedViewHolder:%d, inViewHolderSet:%b, MPRequestToDownload:%s", + currentlyExpandedViewHolderId, + isCurrentlyExpandedViewHolderInViewHolderSet(), + String.valueOf(mediaPlayer.getVoicemailRequestedToDownload())); + + NewVoicemailViewHolder currentlyExpandedViewHolder = getCurrentlyExpandedViewHolder(); + if (currentlyExpandedViewHolderId != -1 + && isCurrentlyExpandedViewHolderInViewHolderSet() + && currentlyExpandedViewHolder != null + // Used to differentiate underlying table changes from voicemail downloads and other changes + // (e.g delete) + && mediaPlayer.getVoicemailRequestedToDownload() != null + && (mediaPlayer + .getVoicemailRequestedToDownload() + .equals(currentlyExpandedViewHolder.getViewHolderVoicemailUri()))) { + currentlyExpandedViewHolder.clickPlayButtonOfViewHoldersMediaPlayerView( + currentlyExpandedViewHolder); + } else { + LogUtil.i("NewVoicemailAdapter.checkAndPlayVoicemail", "not playing downloaded voicemail"); + } + } } diff --git a/java/com/android/dialer/voicemail/listui/NewVoicemailFragment.java b/java/com/android/dialer/voicemail/listui/NewVoicemailFragment.java index 9a89dbe3e..82e704d39 100644 --- a/java/com/android/dialer/voicemail/listui/NewVoicemailFragment.java +++ b/java/com/android/dialer/voicemail/listui/NewVoicemailFragment.java @@ -38,6 +38,7 @@ public final class NewVoicemailFragment extends Fragment implements LoaderCallba @Override public View onCreateView( LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { + LogUtil.enterBlock("NewVoicemailFragment.onCreateView"); View view = inflater.inflate(R.layout.new_voicemail_call_log_fragment, container, false); recyclerView = view.findViewById(R.id.new_voicemail_call_log_recycler_view); getLoaderManager().restartLoader(0, null, this); @@ -52,11 +53,24 @@ public final class NewVoicemailFragment extends Fragment implements LoaderCallba @Override public void onLoadFinished(Loader loader, Cursor data) { - LogUtil.i("NewVoicemailFragment.onCreateLoader", "cursor size is %d", data.getCount()); - recyclerView.setLayoutManager(new LinearLayoutManager(getContext())); - recyclerView.setAdapter( - new NewVoicemailAdapter( - data, System::currentTimeMillis, getActivity().getFragmentManager())); + LogUtil.i("NewVoicemailFragment.onLoadFinished", "cursor size is %d", data.getCount()); + if (recyclerView.getAdapter() == null) { + recyclerView.setLayoutManager(new LinearLayoutManager(getContext())); + recyclerView.setAdapter( + new NewVoicemailAdapter( + data, System::currentTimeMillis, getActivity().getFragmentManager())); + } else { + // This would only be called in cases such as when voicemail has been fetched from the server + // or a changed occurred in the annotated table changed (e.g deletes). To check if the change + // was due to a voicemail download, + // NewVoicemailAdapter.mediaPlayer.getVoicemailRequestedToDownload() is called. + LogUtil.i( + "NewVoicemailFragment.onLoadFinished", + "adapter: %s was not null, checking and playing the voicemail if conditions met", + recyclerView.getAdapter()); + ((NewVoicemailAdapter) recyclerView.getAdapter()).updateCursor(data); + ((NewVoicemailAdapter) recyclerView.getAdapter()).checkAndPlayVoicemail(); + } } @Override diff --git a/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayer.java b/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayer.java index 2d59b241b..48062a87d 100644 --- a/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayer.java +++ b/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayer.java @@ -23,6 +23,7 @@ import android.media.MediaPlayer.OnErrorListener; import android.media.MediaPlayer.OnPreparedListener; import android.net.Uri; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; import java.io.IOException; @@ -38,6 +39,7 @@ public class NewVoicemailMediaPlayer { private OnPreparedListener newVoicemailMediaPlayerOnPreparedListener; private OnCompletionListener newVoicemailMediaPlayerOnCompletionListener; private Uri pausedUri; + @Nullable private Uri voicemailRequestedToDownload; public NewVoicemailMediaPlayer(@NonNull MediaPlayer player) { mediaPlayer = Assert.isNotNull(player); @@ -94,6 +96,7 @@ public class NewVoicemailMediaPlayer { mediaPlayer.start(); voicemailLastPlayedOrPlayingUri = startPlayingVoicemailUri; pausedUri = null; + voicemailRequestedToDownload = null; } public void reset() { @@ -102,6 +105,7 @@ public class NewVoicemailMediaPlayer { voicemailLastPlayedOrPlayingUri = null; voicemailUriLastPreparedOrPreparingToPlay = null; pausedUri = null; + voicemailRequestedToDownload = null; } public void pauseMediaPlayer(Uri voicemailUri) { @@ -134,6 +138,11 @@ public class NewVoicemailMediaPlayer { newVoicemailMediaPlayerOnCompletionListener = onCompletionListener; } + public void setVoicemailRequestedToDownload(@NonNull Uri uri) { + Assert.isNotNull(uri, "cannot download a null voicemail"); + voicemailRequestedToDownload = uri; + } + /** * Note: In some cases it's possible mediaPlayer.isPlaying() can return true, but * mediaPlayer.getCurrentPosition() can be greater than mediaPlayer.getDuration(), after which @@ -182,6 +191,17 @@ public class NewVoicemailMediaPlayer { return mediaPlayer.getDuration(); } + /** + * A null v/s non-value is important for the {@link NewVoicemailAdapter} to differentiate between + * a underlying table change due to a voicemail being downloaded or something else (e.g delete). + * + * @return if there was a Uri that was requested to be downloaded from the server, null otherwise. + */ + @Nullable + public Uri getVoicemailRequestedToDownload() { + return voicemailRequestedToDownload; + } + public boolean isPaused() { return pausedUri != null; } diff --git a/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayerView.java b/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayerView.java index 77dd9cc4b..3f2de7d00 100644 --- a/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayerView.java +++ b/java/com/android/dialer/voicemail/listui/NewVoicemailMediaPlayerView.java @@ -17,12 +17,15 @@ package com.android.dialer.voicemail.listui; import android.app.FragmentManager; +import android.content.ContentValues; import android.content.Context; +import android.content.Intent; import android.database.Cursor; import android.net.Uri; import android.provider.VoicemailContract; +import android.provider.VoicemailContract.Voicemails; import android.support.annotation.NonNull; -import android.support.annotation.VisibleForTesting; +import android.support.annotation.Nullable; import android.support.v4.util.Pair; import android.util.AttributeSet; import android.view.LayoutInflater; @@ -45,7 +48,7 @@ import java.util.Locale; /** * The view of the media player that is visible when a {@link NewVoicemailViewHolder} is expanded. */ -public class NewVoicemailMediaPlayerView extends LinearLayout { +public final class NewVoicemailMediaPlayerView extends LinearLayout { private ImageButton playButton; private ImageButton pauseButton; @@ -55,6 +58,7 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { private TextView currentSeekBarPosition; private SeekBar seekBarView; private TextView totalDurationView; + private TextView voicemailLoadingStatusView; private Uri voicemailUri; private FragmentManager fragmentManager; private NewVoicemailViewHolder newVoicemailViewHolder; @@ -86,6 +90,7 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { phoneButton = findViewById(R.id.phoneButton); deleteButton = findViewById(R.id.deleteButton); totalDurationView = findViewById(R.id.playback_seek_total_duration); + voicemailLoadingStatusView = findViewById(R.id.playback_state_text); } private void setupListenersForMediaPlayerButtons() { @@ -100,6 +105,7 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { public void reset() { LogUtil.i("NewVoicemailMediaPlayer.reset", "the uri for this is " + voicemailUri); voicemailUri = null; + voicemailLoadingStatusView.setVisibility(GONE); } /** @@ -261,6 +267,16 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { } }; + /** + * Attempts to imitate clicking the play button. This is useful for when we the user attempted to + * play a voicemail, but the media player didn't start playing till the voicemail was downloaded + * from the server. However once we have the voicemail downloaded, we want to start playing, so as + * to make it seem like that this is a continuation of the users initial play button click. + */ + public final void clickPlayButton() { + playButtonListener.onClick(null); + } + private final View.OnClickListener playButtonListener = new View.OnClickListener() { @Override @@ -268,7 +284,8 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { LogUtil.i( "NewVoicemailMediaPlayer.playButtonListener", "play button for voicemailUri: %s", - voicemailUri.toString()); + String.valueOf(voicemailUri)); + if (mediaPlayer.getLastPausedVoicemailUri() != null && mediaPlayer .getLastPausedVoicemailUri() @@ -350,10 +367,83 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { + getContext()); } } else { - // TODO(a bug): Add logic for downloading voicemail content from the server. LogUtil.i( "NewVoicemailMediaPlayer.prepareVoicemailForMediaPlayer", "need to download content"); + // Important to set since it allows the adapter to differentiate when to start playing the + // voicemail, after it's downloaded. + mediaPlayer.setVoicemailRequestedToDownload(uri); + voicemailLoadingStatusView.setVisibility(VISIBLE); + sendIntentToDownloadVoicemail(uri); + } + } + + private void sendIntentToDownloadVoicemail(Uri uri) { + LogUtil.i("NewVoicemailMediaPlayer.sendIntentToDownloadVoicemail", "uri:%s", uri.toString()); + // Send voicemail fetch request. + Intent intent = new Intent(VoicemailContract.ACTION_FETCH_VOICEMAIL, uri); + + Worker, Pair> getVoicemailSourcePackage = + this::queryVoicemailSourcePackage; + SuccessListener> checkVoicemailHasSourcePackageCallBack = this::sendIntent; + + DialerExecutorComponent.get(getContext()) + .dialerExecutorFactory() + .createUiTaskBuilder(fragmentManager, "lookup_voicemail_pkg", getVoicemailSourcePackage) + .onSuccess(checkVoicemailHasSourcePackageCallBack) + .build() + .executeSerial(new Pair<>(getContext(), voicemailUri)); + } + + private void sendIntent(Pair booleanUriPair) { + String sourcePackage = booleanUriPair.first; + Uri uri = booleanUriPair.second; + LogUtil.i( + "NewVoicemailMediaPlayer.sendIntent", + "srcPkg:%s, uri:%%s", + sourcePackage, + String.valueOf(uri)); + Intent intent = new Intent(VoicemailContract.ACTION_FETCH_VOICEMAIL, uri); + intent.setPackage(sourcePackage); + voicemailLoadingStatusView.setVisibility(VISIBLE); + getContext().sendBroadcast(intent); + } + + @Nullable + private Pair queryVoicemailSourcePackage(Pair contextUriPair) { + LogUtil.enterBlock("NewVoicemailMediaPlayer.queryVoicemailSourcePackage"); + Context context = contextUriPair.first; + Uri uri = contextUriPair.second; + String sourcePackage; + try (Cursor cursor = + context + .getContentResolver() + .query(uri, new String[] {Voicemails.SOURCE_PACKAGE}, null, null, null)) { + + if (!hasContent(cursor)) { + LogUtil.e( + "NewVoicemailMediaPlayer.queryVoicemailSourcePackage", + "uri: %s does not return a SOURCE_PACKAGE", + uri.toString()); + sourcePackage = null; + } else { + sourcePackage = cursor.getString(0); + LogUtil.i( + "NewVoicemailMediaPlayer.queryVoicemailSourcePackage", + "uri: %s has a SOURCE_PACKAGE: %s", + uri.toString(), + sourcePackage); + } + LogUtil.i( + "NewVoicemailMediaPlayer.queryVoicemailSourcePackage", + "uri: %s has a SOURCE_PACKAGE: %s", + uri.toString(), + sourcePackage); } + return new Pair<>(sourcePackage, uri); + } + + private boolean hasContent(Cursor cursor) { + return cursor != null && cursor.moveToFirst(); } private final View.OnClickListener speakerButtonListener = @@ -386,6 +476,21 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { "NewVoicemailMediaPlayer.deleteButtonListener", "delete voicemailUri %s", voicemailUri.toString()); + // TODO(uabdullah): This will be removed in cl/177404259. It only sets the has_content to + // 0, to allow the annotated call log to change, and refresh the fragment. This is used to + // demo and test the downloading of voicemails from the server. + ContentValues contentValues = new ContentValues(); + contentValues.put("has_content", 0); + + try { + getContext().getContentResolver().update(voicemailUri, contentValues, "type = 4", null); + } catch (Exception e) { + LogUtil.i( + "NewVoicemailMediaPlayer.deleteButtonListener", + "update has content of voicemailUri %s caused an error: %s", + voicemailUri.toString(), + e.toString()); + } } }; @@ -401,6 +506,7 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { playButton.setVisibility(GONE); pauseButton.setVisibility(VISIBLE); + voicemailLoadingStatusView.setVisibility(GONE); Assert.checkArgument( mp.equals(mediaPlayer), "there should only be one instance of a media player"); @@ -510,9 +616,4 @@ public class NewVoicemailMediaPlayerView extends LinearLayout { } return String.format(Locale.US, "%02d:%02d", minutes, seconds); } - - @VisibleForTesting(otherwise = VisibleForTesting.NONE) - void setFragmentManager(FragmentManager fragmentManager) { - this.fragmentManager = fragmentManager; - } } diff --git a/java/com/android/dialer/voicemail/listui/NewVoicemailViewHolder.java b/java/com/android/dialer/voicemail/listui/NewVoicemailViewHolder.java index d5b17a19d..072546552 100644 --- a/java/com/android/dialer/voicemail/listui/NewVoicemailViewHolder.java +++ b/java/com/android/dialer/voicemail/listui/NewVoicemailViewHolder.java @@ -187,6 +187,8 @@ final class NewVoicemailViewHolder extends RecyclerView.ViewHolder implements On String.valueOf(viewHolderVoicemailUri)); transcriptionTextView.setMaxLines(1); isViewHolderExpanded = false; + + mediaPlayerView.reset(); mediaPlayerView.setVisibility(GONE); } @@ -333,6 +335,23 @@ final class NewVoicemailViewHolder extends RecyclerView.ViewHolder implements On return viewHolderVoicemailUri; } + public void clickPlayButtonOfViewHoldersMediaPlayerView( + NewVoicemailViewHolder expandedViewHolder) { + LogUtil.i( + "NewVoicemailViewHolder.clickPlayButtonOfViewHoldersMediaPlayerView", + "expandedViewHolderID:%d", + expandedViewHolder.getViewHolderId()); + + Assert.checkArgument( + mediaPlayerView.getVoicemailUri().equals(expandedViewHolder.getViewHolderVoicemailUri())); + Assert.checkArgument( + expandedViewHolder.getViewHolderVoicemailUri().equals(getViewHolderVoicemailUri())); + Assert.checkArgument( + mediaPlayerView.getVisibility() == View.VISIBLE, + "the media player must be visible for viewholder id:%d, before we attempt to play"); + mediaPlayerView.clickPlayButton(); + } + interface NewVoicemailViewHolderListener { void expandViewHolderFirstTimeAndCollapseAllOtherVisibleViewHolders( NewVoicemailViewHolder expandedViewHolder, diff --git a/java/com/android/dialer/voicemail/listui/res/layout/new_voicemail_media_player_layout.xml b/java/com/android/dialer/voicemail/listui/res/layout/new_voicemail_media_player_layout.xml index 32726a9e5..3efcea543 100644 --- a/java/com/android/dialer/voicemail/listui/res/layout/new_voicemail_media_player_layout.xml +++ b/java/com/android/dialer/voicemail/listui/res/layout/new_voicemail_media_player_layout.xml @@ -22,7 +22,6 @@ android:paddingTop="@dimen/voicemail_media_player_padding_top" android:orientation="vertical"> - Date: Tue, 12 Dec 2017 13:24:16 -0800 Subject: BEGIN_PUBLIC Automated rollback of changelist 172956409 Bug: 30225112 Test: rollback PiperOrigin-RevId: 178805314 Change-Id: Ia85b10081948b64d4cfc9c9d10d35679df8296ef --- .../dialer/phonenumberutil/PhoneNumberHelper.java | 124 +++------------------ 1 file changed, 14 insertions(+), 110 deletions(-) diff --git a/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java b/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java index cdc06dead..783e04d7b 100644 --- a/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java +++ b/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java @@ -29,7 +29,6 @@ import android.telephony.TelephonyManager; import android.text.BidiFormatter; import android.text.TextDirectionHeuristics; import android.text.TextUtils; -import android.util.SparseIntArray; import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; import com.android.dialer.compat.CompatUtils; @@ -47,71 +46,6 @@ public class PhoneNumberHelper { private static final Set LEGACY_UNKNOWN_NUMBERS = new HashSet<>(Arrays.asList("-1", "-2", "-3")); - /** The phone keypad letter mapping (see ITU E.161 or ISO/IEC 9995-8.) */ - private static final SparseIntArray KEYPAD_MAP = new SparseIntArray(); - - static { - KEYPAD_MAP.put('a', '2'); - KEYPAD_MAP.put('b', '2'); - KEYPAD_MAP.put('c', '2'); - KEYPAD_MAP.put('A', '2'); - KEYPAD_MAP.put('B', '2'); - KEYPAD_MAP.put('C', '2'); - - KEYPAD_MAP.put('d', '3'); - KEYPAD_MAP.put('e', '3'); - KEYPAD_MAP.put('f', '3'); - KEYPAD_MAP.put('D', '3'); - KEYPAD_MAP.put('E', '3'); - KEYPAD_MAP.put('F', '3'); - - KEYPAD_MAP.put('g', '4'); - KEYPAD_MAP.put('h', '4'); - KEYPAD_MAP.put('i', '4'); - KEYPAD_MAP.put('G', '4'); - KEYPAD_MAP.put('H', '4'); - KEYPAD_MAP.put('I', '4'); - - KEYPAD_MAP.put('j', '5'); - KEYPAD_MAP.put('k', '5'); - KEYPAD_MAP.put('l', '5'); - KEYPAD_MAP.put('J', '5'); - KEYPAD_MAP.put('K', '5'); - KEYPAD_MAP.put('L', '5'); - - KEYPAD_MAP.put('m', '6'); - KEYPAD_MAP.put('n', '6'); - KEYPAD_MAP.put('o', '6'); - KEYPAD_MAP.put('M', '6'); - KEYPAD_MAP.put('N', '6'); - KEYPAD_MAP.put('O', '6'); - - KEYPAD_MAP.put('p', '7'); - KEYPAD_MAP.put('q', '7'); - KEYPAD_MAP.put('r', '7'); - KEYPAD_MAP.put('s', '7'); - KEYPAD_MAP.put('P', '7'); - KEYPAD_MAP.put('Q', '7'); - KEYPAD_MAP.put('R', '7'); - KEYPAD_MAP.put('S', '7'); - - KEYPAD_MAP.put('t', '8'); - KEYPAD_MAP.put('u', '8'); - KEYPAD_MAP.put('v', '8'); - KEYPAD_MAP.put('T', '8'); - KEYPAD_MAP.put('U', '8'); - KEYPAD_MAP.put('V', '8'); - - KEYPAD_MAP.put('w', '9'); - KEYPAD_MAP.put('x', '9'); - KEYPAD_MAP.put('y', '9'); - KEYPAD_MAP.put('z', '9'); - KEYPAD_MAP.put('W', '9'); - KEYPAD_MAP.put('X', '9'); - KEYPAD_MAP.put('Y', '9'); - KEYPAD_MAP.put('Z', '9'); - } - /** Returns true if it is possible to place a call to the given number. */ public static boolean canPlaceCallsTo(CharSequence number, int presentation) { return presentation == CallLog.Calls.PRESENTATION_ALLOWED @@ -129,59 +63,29 @@ public class PhoneNumberHelper { *

  • their corresponding raw numbers are both global phone numbers (i.e., they can be accepted * by {@link PhoneNumberUtils#isGlobalPhoneNumber(String)}) and {@link * PhoneNumberUtils#compare(String, String)} deems them as "identical enough"; OR - *
  • neither of the raw numbers is a global phone number and they are identical. + *
  • at least one of the raw numbers is not a global phone number and the two raw numbers are + * exactly the same. * * - * See {@link #convertAndStrip(String)} for how a raw number is obtained. + * The raw number of a phone number is obtained by first translating any alphabetic letters + * ([A-Za-z]) into the equivalent numeric digits and then removing all separators. See {@link + * PhoneNumberUtils#convertKeypadLettersToDigits(String)} and {@link + * PhoneNumberUtils#stripSeparators(String)}. */ public static boolean compare(@Nullable String number1, @Nullable String number2) { if (number1 == null || number2 == null) { return Objects.equals(number1, number2); } - String rawNumber1 = convertAndStrip(number1); - String rawNumber2 = convertAndStrip(number2); - - boolean isGlobalPhoneNumber1 = PhoneNumberUtils.isGlobalPhoneNumber(rawNumber1); - boolean isGlobalPhoneNumber2 = PhoneNumberUtils.isGlobalPhoneNumber(rawNumber2); - - if (isGlobalPhoneNumber1 && isGlobalPhoneNumber2) { - return PhoneNumberUtils.compare(rawNumber1, rawNumber2); - } - if (!isGlobalPhoneNumber1 && !isGlobalPhoneNumber2) { - return rawNumber1.equals(rawNumber2); - } - return false; - } - - /** - * Translating any alphabetic letters ([A-Za-z]) in the given phone number into the equivalent - * numeric digits and then removing all separators. The caller should ensure the number passed to - * this method is not null. - */ - private static String convertAndStrip(@NonNull String number) { - int len = number.length(); - if (len == 0) { - return number; - } - - StringBuilder ret = new StringBuilder(len); - for (int i = 0; i < len; i++) { - char c = number.charAt(i); - - // If the char isn't in KEYPAD_MAP, leave it alone for now. - c = (char) KEYPAD_MAP.get(c, c); - - // Append the char to the result if it's a digit or non-separator. - int digit = Character.digit(c, 10); - if (digit != -1) { - ret.append(digit); - } else if (PhoneNumberUtils.isNonSeparator(c)) { - ret.append(c); - } - } + String rawNumber1 = + PhoneNumberUtils.stripSeparators(PhoneNumberUtils.convertKeypadLettersToDigits(number1)); + String rawNumber2 = + PhoneNumberUtils.stripSeparators(PhoneNumberUtils.convertKeypadLettersToDigits(number2)); - return ret.toString(); + return PhoneNumberUtils.isGlobalPhoneNumber(rawNumber1) + && PhoneNumberUtils.isGlobalPhoneNumber(rawNumber2) + ? PhoneNumberUtils.compare(rawNumber1, rawNumber2) + : rawNumber1.equals(rawNumber2); } /** -- cgit v1.2.3 From 75d67f5d83bdb28c6254cf064d09ac24fc9ae928 Mon Sep 17 00:00:00 2001 From: linyuh Date: Tue, 12 Dec 2017 13:43:26 -0800 Subject: BEGIN_PUBLIC Automated rollback of changelist 172683494 Bug: 30225112 Test: None PiperOrigin-RevId: 178807986 Change-Id: I45978ea4daab71985ac93c3c3a0439fdd8e873d5 --- .../dialer/app/calllog/CallLogGroupBuilder.java | 3 +- .../dialer/phonenumberutil/PhoneNumberHelper.java | 76 ++++++++-------------- 2 files changed, 30 insertions(+), 49 deletions(-) diff --git a/java/com/android/dialer/app/calllog/CallLogGroupBuilder.java b/java/com/android/dialer/app/calllog/CallLogGroupBuilder.java index f689571bd..513c8aa59 100644 --- a/java/com/android/dialer/app/calllog/CallLogGroupBuilder.java +++ b/java/com/android/dialer/app/calllog/CallLogGroupBuilder.java @@ -21,6 +21,7 @@ import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; +import android.telephony.PhoneNumberUtils; import android.text.TextUtils; import android.text.format.Time; import com.android.contacts.common.util.DateUtils; @@ -193,7 +194,7 @@ public class CallLogGroupBuilder { if (PhoneNumberHelper.isUriNumber(number1) || PhoneNumberHelper.isUriNumber(number2)) { return compareSipAddresses(number1, number2); } else { - return PhoneNumberHelper.compare(number1, number2); + return PhoneNumberUtils.compare(number1, number2); } } diff --git a/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java b/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java index 783e04d7b..e32ace59e 100644 --- a/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java +++ b/java/com/android/dialer/phonenumberutil/PhoneNumberHelper.java @@ -37,7 +37,6 @@ import com.android.dialer.phonenumbergeoutil.PhoneNumberGeoUtilComponent; import com.android.dialer.telecom.TelecomUtil; import java.util.Arrays; import java.util.HashSet; -import java.util.Objects; import java.util.Set; public class PhoneNumberHelper { @@ -54,46 +53,7 @@ public class PhoneNumberHelper { } /** - * Compare two phone numbers, return true if they're identical enough for caller ID purposes. This - * is an enhanced version of {@link PhoneNumberUtils#compare(String, String)}. - * - *

    The two phone numbers are considered "identical enough" if - * - *

      - *
    • their corresponding raw numbers are both global phone numbers (i.e., they can be accepted - * by {@link PhoneNumberUtils#isGlobalPhoneNumber(String)}) and {@link - * PhoneNumberUtils#compare(String, String)} deems them as "identical enough"; OR - *
    • at least one of the raw numbers is not a global phone number and the two raw numbers are - * exactly the same. - *
    - * - * The raw number of a phone number is obtained by first translating any alphabetic letters - * ([A-Za-z]) into the equivalent numeric digits and then removing all separators. See {@link - * PhoneNumberUtils#convertKeypadLettersToDigits(String)} and {@link - * PhoneNumberUtils#stripSeparators(String)}. - */ - public static boolean compare(@Nullable String number1, @Nullable String number2) { - if (number1 == null || number2 == null) { - return Objects.equals(number1, number2); - } - - String rawNumber1 = - PhoneNumberUtils.stripSeparators(PhoneNumberUtils.convertKeypadLettersToDigits(number1)); - String rawNumber2 = - PhoneNumberUtils.stripSeparators(PhoneNumberUtils.convertKeypadLettersToDigits(number2)); - - return PhoneNumberUtils.isGlobalPhoneNumber(rawNumber1) - && PhoneNumberUtils.isGlobalPhoneNumber(rawNumber2) - ? PhoneNumberUtils.compare(rawNumber1, rawNumber2) - : rawNumber1.equals(rawNumber2); - } - - /** - * Find the cursor pointing to the row in which a number is identical enough to the number in a - * contact lookup URI. - * - *

    See the description of {@link PhoneNumberHelper#compare(String, String)} for the definition - * of "identical enough". + * Find the cursor pointing to a number that matches the number in a contact lookup URI. * *

    When determining whether two phone numbers are identical enough for caller ID purposes, the * Contacts Provider uses {@link PhoneNumberUtils#compare(String, String)}, which ignores special @@ -101,17 +61,25 @@ public class PhoneNumberHelper { * by the Contacts Provider to have multiple rows even when the URI asks for a specific number. * *

    For example, suppose the user has two contacts whose numbers are "#123" and "123", - * respectively. When the URI asks for number "123", both numbers will be returned. Therefore, - * {@link PhoneNumberHelper#compare(String, String)}, which is an enhanced version of {@link - * PhoneNumberUtils#compare(String, String)}, is employed to find a match. + * respectively. When the URI asks for number "123", both numbers will be returned. Therefore, the + * following strategy is employed to find a match. + * + *

    If the cursor points to a global phone number (i.e., a number that can be accepted by {@link + * PhoneNumberUtils#isGlobalPhoneNumber(String)}) and the lookup number in the URI is a PARTIAL + * match, return the cursor. + * + *

    If the cursor points to a number that is not a global phone number, return the cursor iff + * the lookup number in the URI is an EXACT match. + * + *

    Return null in all other circumstances. * * @param cursor A cursor returned by the Contacts Provider. * @param columnIndexForNumber The index of the column where phone numbers are stored. It is the * caller's responsibility to pass the correct column index. * @param contactLookupUri A URI used to retrieve a contact via the Contacts Provider. It is the * caller's responsibility to ensure the URI is one that asks for a specific phone number. - * @return The cursor pointing to the row in which the number is considered a match by the - * description above or null if no such cursor can be found. + * @return The cursor considered as a match by the description above or null if no such cursor can + * be found. */ public static Cursor getCursorMatchForContactLookupUri( Cursor cursor, int columnIndexForNumber, Uri contactLookupUri) { @@ -131,10 +99,22 @@ public class PhoneNumberHelper { return null; } + boolean isMatchFound; do { - String existingContactNumber = cursor.getString(columnIndexForNumber); + // All undialable characters should be converted/removed before comparing the lookup number + // and the existing contact number. + String rawExistingContactNumber = + PhoneNumberUtils.stripSeparators( + PhoneNumberUtils.convertKeypadLettersToDigits( + cursor.getString(columnIndexForNumber))); + String rawQueryNumber = + PhoneNumberUtils.stripSeparators( + PhoneNumberUtils.convertKeypadLettersToDigits(lookupNumber)); - boolean isMatchFound = compare(existingContactNumber, lookupNumber); + isMatchFound = + PhoneNumberUtils.isGlobalPhoneNumber(rawExistingContactNumber) + ? rawExistingContactNumber.contains(rawQueryNumber) + : rawExistingContactNumber.equals(rawQueryNumber); if (isMatchFound) { return cursor; -- cgit v1.2.3