From 05838b54c2ba81e74e205141c436ad8e9900b933 Mon Sep 17 00:00:00 2001 From: zachh Date: Mon, 22 Jan 2018 14:43:03 -0800 Subject: Changed PhoneLookup#lookup to accept a DialerPhoneNumber. There's a problem with the existing implementation of RealtimeRowProcessor; when CP2 information is not present, data from other sources can potentially be erased. This CL fixes the problem by fetching the latest data from all sources, instead of just CP2. This requires being able to fetch PhoneLookup info without a Call, using only a number, so I changed PhoneLookup#lookup to accept a DialerPhoneNumber rather than a Call. (The reason that it accepted a Call was to support CNAP so we'll need a revised solution for that later.) There is a potential concern with performance in RealtimeRowProcessor due to performing a full [Composite]PhoneLookup vs. a CP2 lookup, because the full lookup includes network requests. However, it's anticipated that the real time lookups will very rarely have changes to apply so this might be OK as-is. If not, a mitigation strategy could be improving the performance of CompositePhoneLookup#lookup by short-circutiing slower sources when faster, higher priority sources have already completed. A follow-up CL will write the result of RealtimeRowProcessor queries to PhoneLookupHistory to further reduce how frequently real time updates need to be applied. Bug: 72229553 Test: existing unit PiperOrigin-RevId: 182839130 Change-Id: I8cb26827b4f4dc4702fb416234c8938179cd5ac5 --- .../android/dialer/phonelookup/PhoneLookup.java | 6 +-- .../DialerBlockedNumberPhoneLookup.java | 18 +-------- .../composite/CompositePhoneLookup.java | 6 +-- .../phonelookup/cp2/Cp2LocalPhoneLookup.java | 47 +++++----------------- .../phonelookup/cp2/Cp2RemotePhoneLookup.java | 19 +++++---- 5 files changed, 24 insertions(+), 72 deletions(-) (limited to 'java/com/android/dialer/phonelookup') diff --git a/java/com/android/dialer/phonelookup/PhoneLookup.java b/java/com/android/dialer/phonelookup/PhoneLookup.java index 118ae603e..76ff98e7c 100644 --- a/java/com/android/dialer/phonelookup/PhoneLookup.java +++ b/java/com/android/dialer/phonelookup/PhoneLookup.java @@ -18,8 +18,6 @@ package com.android.dialer.phonelookup; import android.content.Context; import android.support.annotation.MainThread; -import android.support.annotation.NonNull; -import android.telecom.Call; import com.android.dialer.DialerPhoneNumber; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -35,13 +33,13 @@ import com.google.common.util.concurrent.ListenableFuture; public interface PhoneLookup { /** - * Returns a future containing a new info for the provided call. + * Returns a future containing a new info for the provided number. * *

The returned message should contain populated data for the sub-message corresponding to this * {@link PhoneLookup}. For example, the CP2 implementation returns a {@link * PhoneLookupInfo.Cp2Info} sub-message. */ - ListenableFuture lookup(@NonNull Call call); + ListenableFuture lookup(DialerPhoneNumber dialerPhoneNumber); /** * Returns a future which returns true if the information for any of the provided phone numbers diff --git a/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java b/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java index fa67feec8..ce1217751 100644 --- a/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java +++ b/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java @@ -21,9 +21,7 @@ import android.database.ContentObserver; import android.database.Cursor; import android.net.Uri; import android.support.annotation.MainThread; -import android.support.annotation.NonNull; import android.support.annotation.WorkerThread; -import android.telecom.Call; import android.util.ArraySet; import com.android.dialer.DialerPhoneNumber; import com.android.dialer.common.Assert; @@ -39,15 +37,12 @@ import com.android.dialer.phonelookup.PhoneLookup; import com.android.dialer.phonelookup.PhoneLookupInfo; import com.android.dialer.phonelookup.PhoneLookupInfo.BlockedState; import com.android.dialer.phonelookup.PhoneLookupInfo.DialerBlockedNumberInfo; -import com.android.dialer.phonenumberproto.DialerPhoneNumberUtil; import com.android.dialer.phonenumberproto.PartitionedNumbers; -import com.android.dialer.telecom.TelecomCallUtil; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; -import com.google.i18n.phonenumbers.PhoneNumberUtil; import java.util.Set; import javax.inject.Inject; @@ -69,18 +64,9 @@ public final class DialerBlockedNumberPhoneLookup implements PhoneLookup lookup(@NonNull Call call) { + public ListenableFuture lookup(DialerPhoneNumber dialerPhoneNumber) { return executorService.submit( - () -> { - DialerPhoneNumberUtil dialerPhoneNumberUtil = - new DialerPhoneNumberUtil(PhoneNumberUtil.getInstance()); - - DialerPhoneNumber number = - dialerPhoneNumberUtil.parse( - TelecomCallUtil.getNumber(call), - TelecomCallUtil.getCountryCode(appContext, call).orNull()); - return queryNumbers(ImmutableSet.of(number)).get(number); - }); + () -> queryNumbers(ImmutableSet.of(dialerPhoneNumber)).get(dialerPhoneNumber)); } @Override diff --git a/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java b/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java index 68695b7e9..622b4db10 100644 --- a/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java +++ b/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java @@ -18,8 +18,6 @@ package com.android.dialer.phonelookup.composite; import android.content.Context; import android.support.annotation.MainThread; -import android.support.annotation.NonNull; -import android.telecom.Call; import com.android.dialer.DialerPhoneNumber; import com.android.dialer.common.LogUtil; import com.android.dialer.common.concurrent.Annotations.LightweightExecutor; @@ -65,12 +63,12 @@ public final class CompositePhoneLookup implements PhoneLookup */ @SuppressWarnings({"unchecked", "rawtype"}) @Override - public ListenableFuture lookup(@NonNull Call call) { + public ListenableFuture lookup(DialerPhoneNumber dialerPhoneNumber) { // TODO(zachh): Add short-circuiting logic so that this call is not blocked on low-priority // lookups finishing when a higher-priority one has already finished. List> futures = new ArrayList<>(); for (PhoneLookup phoneLookup : phoneLookups) { - futures.add(phoneLookup.lookup(call)); + futures.add(phoneLookup.lookup(dialerPhoneNumber)); } return Futures.transform( Futures.allAsList(futures), diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java index 995950d0e..127569b9f 100644 --- a/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java +++ b/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java @@ -27,7 +27,6 @@ import android.provider.ContactsContract.DeletedContacts; import android.support.annotation.Nullable; import android.support.v4.util.ArrayMap; import android.support.v4.util.ArraySet; -import android.telecom.Call; import android.text.TextUtils; import com.android.dialer.DialerPhoneNumber; import com.android.dialer.common.Assert; @@ -43,7 +42,6 @@ import com.android.dialer.phonelookup.database.contract.PhoneLookupHistoryContra import com.android.dialer.phonenumberproto.DialerPhoneNumberUtil; import com.android.dialer.phonenumberproto.PartitionedNumbers; import com.android.dialer.storage.Unencrypted; -import com.android.dialer.telecom.TelecomCallUtil; import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -93,16 +91,18 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup { } @Override - public ListenableFuture lookup(Call call) { - return backgroundExecutorService.submit(() -> lookupInternal(call)); + public ListenableFuture lookup(DialerPhoneNumber dialerPhoneNumber) { + return backgroundExecutorService.submit(() -> lookupInternal(dialerPhoneNumber)); } - private Cp2Info lookupInternal(Call call) { - String rawNumber = TelecomCallUtil.getNumber(call); - if (TextUtils.isEmpty(rawNumber)) { + private Cp2Info lookupInternal(DialerPhoneNumber dialerPhoneNumber) { + DialerPhoneNumberUtil dialerPhoneNumberUtil = + new DialerPhoneNumberUtil(PhoneNumberUtil.getInstance()); + String number = dialerPhoneNumberUtil.normalizeNumber(dialerPhoneNumber); + if (TextUtils.isEmpty(number)) { return Cp2Info.getDefaultInstance(); } - Optional validE164 = TelecomCallUtil.getValidE164Number(appContext, call); + Optional validE164 = dialerPhoneNumberUtil.formatToValidE164(dialerPhoneNumber); Set cp2ContactInfos = new ArraySet<>(); // Note: It would make sense to use PHONE_LOOKUP for E164 numbers as well, but we use PHONE to // ensure consistency when the batch methods are used to update data. @@ -110,7 +110,7 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup { validE164.isPresent() ? queryPhoneTableBasedOnE164( Cp2Projections.getProjectionForPhoneTable(), ImmutableSet.of(validE164.get())) - : queryPhoneLookup(Cp2Projections.getProjectionForPhoneLookupTable(), rawNumber)) { + : queryPhoneLookup(Cp2Projections.getProjectionForPhoneLookupTable(), number)) { if (cursor == null) { LogUtil.w("Cp2LocalPhoneLookup.lookupInternal", "null cursor"); return Cp2Info.getDefaultInstance(); @@ -122,35 +122,6 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup { return Cp2Info.newBuilder().addAllCp2ContactInfo(cp2ContactInfos).build(); } - /** - * Queries ContactsContract.PhoneLookup for the {@link Cp2Info} associated with the provided - * {@link DialerPhoneNumber}. Returns {@link Cp2Info#getDefaultInstance()} if there is no - * information. - */ - public ListenableFuture lookupByNumber(DialerPhoneNumber dialerPhoneNumber) { - return backgroundExecutorService.submit( - () -> { - DialerPhoneNumberUtil dialerPhoneNumberUtil = - new DialerPhoneNumberUtil(PhoneNumberUtil.getInstance()); - String rawNumber = dialerPhoneNumberUtil.normalizeNumber(dialerPhoneNumber); - if (rawNumber.isEmpty()) { - return Cp2Info.getDefaultInstance(); - } - Set cp2ContactInfos = new ArraySet<>(); - try (Cursor cursor = - queryPhoneLookup(Cp2Projections.getProjectionForPhoneLookupTable(), rawNumber)) { - if (cursor == null) { - LogUtil.w("Cp2LocalPhoneLookup.lookupByNumber", "null cursor"); - return Cp2Info.getDefaultInstance(); - } - while (cursor.moveToNext()) { - cp2ContactInfos.add(Cp2Projections.buildCp2ContactInfoFromCursor(appContext, cursor)); - } - } - return Cp2Info.newBuilder().addAllCp2ContactInfo(cp2ContactInfos).build(); - }); - } - @Override public ListenableFuture isDirty(ImmutableSet phoneNumbers) { PartitionedNumbers partitionedNumbers = new PartitionedNumbers(phoneNumbers); diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2RemotePhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2RemotePhoneLookup.java index 6a4682958..9596f15f7 100644 --- a/java/com/android/dialer/phonelookup/cp2/Cp2RemotePhoneLookup.java +++ b/java/com/android/dialer/phonelookup/cp2/Cp2RemotePhoneLookup.java @@ -24,7 +24,6 @@ import android.os.Build.VERSION_CODES; import android.provider.ContactsContract; import android.provider.ContactsContract.Directory; import android.support.annotation.VisibleForTesting; -import android.telecom.Call; import com.android.dialer.DialerPhoneNumber; import com.android.dialer.common.LogUtil; import com.android.dialer.common.concurrent.Annotations.BackgroundExecutor; @@ -33,13 +32,14 @@ import com.android.dialer.inject.ApplicationContext; import com.android.dialer.phonelookup.PhoneLookup; import com.android.dialer.phonelookup.PhoneLookupInfo; import com.android.dialer.phonelookup.PhoneLookupInfo.Cp2Info; +import com.android.dialer.phonenumberproto.DialerPhoneNumberUtil; import com.android.dialer.phonenumberutil.PhoneNumberHelper; -import com.android.dialer.telecom.TelecomCallUtil; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.i18n.phonenumbers.PhoneNumberUtil; import java.util.ArrayList; import java.util.List; import javax.inject.Inject; @@ -62,15 +62,10 @@ public final class Cp2RemotePhoneLookup implements PhoneLookup { } @Override - public ListenableFuture lookup(Call call) { - String number = TelecomCallUtil.getNumber(call); - if (number == null) { - return Futures.immediateFuture(Cp2Info.getDefaultInstance()); - } - + public ListenableFuture lookup(DialerPhoneNumber dialerPhoneNumber) { return Futures.transformAsync( queryCp2ForRemoteDirectoryIds(), - remoteDirectoryIds -> queryCp2ForRemoteContact(number, remoteDirectoryIds), + remoteDirectoryIds -> queryCp2ForRemoteContact(dialerPhoneNumber, remoteDirectoryIds), lightweightExecutorService); } @@ -114,11 +109,15 @@ public final class Cp2RemotePhoneLookup implements PhoneLookup { } private ListenableFuture queryCp2ForRemoteContact( - String number, List remoteDirectoryIds) { + DialerPhoneNumber dialerPhoneNumber, List remoteDirectoryIds) { if (remoteDirectoryIds.isEmpty()) { return Futures.immediateFuture(Cp2Info.getDefaultInstance()); } + DialerPhoneNumberUtil dialerPhoneNumberUtil = + new DialerPhoneNumberUtil(PhoneNumberUtil.getInstance()); + String number = dialerPhoneNumberUtil.normalizeNumber(dialerPhoneNumber); + List> cp2InfoFutures = new ArrayList<>(); for (long remoteDirectoryId : remoteDirectoryIds) { cp2InfoFutures.add(queryCp2ForRemoteContact(number, remoteDirectoryId)); -- cgit v1.2.3