From e1ec439563705d47f699a565c35eff4bc0dcbf61 Mon Sep 17 00:00:00 2001 From: zachh Date: Fri, 22 Dec 2017 16:59:32 -0800 Subject: Parameterized PhoneLookup with submessage type. This allows indvidual PhoneLookups to define and deal mostly with their own submessage type (with the exception of trivial setter and getter methods for converting from/to PhoneLookupInfo). This also simplifies the FakePhoneLookup and tests which use it a bit, I think. Bug: 34672501 Test: unit PiperOrigin-RevId: 179976215 Change-Id: I2db1fc85771621be2f2afcd6af114d82680e30d0 --- .../android/dialer/phonelookup/PhoneLookup.java | 40 ++++++----- .../dialer/phonelookup/PhoneLookupComponent.java | 2 +- .../dialer/phonelookup/PhoneLookupModule.java | 3 +- .../composite/CompositePhoneLookup.java | 57 +++++++++++----- .../dialer/phonelookup/cp2/Cp2PhoneLookup.java | 77 +++++++++++----------- 5 files changed, 103 insertions(+), 76 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 bb14c1ff6..859085e7b 100644 --- a/java/com/android/dialer/phonelookup/PhoneLookup.java +++ b/java/com/android/dialer/phonelookup/PhoneLookup.java @@ -27,19 +27,19 @@ import com.google.common.util.concurrent.ListenableFuture; * Provides operations related to retrieving information about phone numbers. * *

Some operations defined by this interface are generally targeted towards specific use cases; - * for example {@link #isDirty(ImmutableSet)}, {@link #getMostRecentPhoneLookupInfo(ImmutableMap)}, - * and {@link #onSuccessfulBulkUpdate()} are generally intended to be used by the call log. + * for example {@link #isDirty(ImmutableSet)}, {@link #getMostRecentInfo(ImmutableMap)}, and {@link + * #onSuccessfulBulkUpdate()} are generally intended to be used by the call log. */ -public interface PhoneLookup { +public interface PhoneLookup { /** - * Returns a future containing a new {@link PhoneLookupInfo} for the provided call. + * Returns a future containing a new info for the provided call. * *

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} with - * the {@link PhoneLookupInfo.Cp2Info} sub-message populated. + * {@link PhoneLookup}. For example, the CP2 implementation returns a {@link + * PhoneLookupInfo.Cp2Info} sub-message. */ - ListenableFuture lookup(@NonNull Call call); + ListenableFuture lookup(@NonNull Call call); /** * Returns a future which returns true if the information for any of the provided phone numbers @@ -56,24 +56,30 @@ public interface PhoneLookup { * {@code existingInfoMap}. * *

If there is no longer information associated with a number (for example, a local contact was - * deleted) the returned map should contain an empty {@link PhoneLookupInfo} for that number. + * deleted) the returned map should contain an empty info for that number. */ - ListenableFuture> getMostRecentPhoneLookupInfo( - ImmutableMap existingInfoMap); + ListenableFuture> getMostRecentInfo( + ImmutableMap existingInfoMap); /** - * Populates the sub-message that this {@link PhoneLookup} is responsible for by copying the - * sub-message value from {@code source} to {@code destination} + * Populates the sub-message that this {@link PhoneLookup} is responsible for by copying {@code + * subMessage} into the provided {@code phoneLookupInfo} builder. */ - void copySubMessage(PhoneLookupInfo.Builder destination, PhoneLookupInfo source); + void setSubMessage(PhoneLookupInfo.Builder phoneLookupInfo, T subMessage); /** - * Called when the results of the {@link #getMostRecentPhoneLookupInfo(ImmutableMap)} have been - * applied by the caller. + * Gets the sub-message that this {@link PhoneLookup} is responsible for from the provided {@code + * phoneLookupInfo}. + */ + T getSubMessage(PhoneLookupInfo phoneLookupInfo); + + /** + * Called when the results of the {@link #getMostRecentInfo(ImmutableMap)} have been applied by + * the caller. * *

Typically implementations will use this to store a "last processed" timestamp so that future - * invocations of {@link #isDirty(ImmutableSet)} and {@link - * #getMostRecentPhoneLookupInfo(ImmutableMap)} can be efficiently implemented. + * invocations of {@link #isDirty(ImmutableSet)} and {@link #getMostRecentInfo(ImmutableMap)} can + * be efficiently implemented. */ ListenableFuture onSuccessfulBulkUpdate(); } diff --git a/java/com/android/dialer/phonelookup/PhoneLookupComponent.java b/java/com/android/dialer/phonelookup/PhoneLookupComponent.java index 6d1f9a2fc..f59886bcd 100644 --- a/java/com/android/dialer/phonelookup/PhoneLookupComponent.java +++ b/java/com/android/dialer/phonelookup/PhoneLookupComponent.java @@ -23,7 +23,7 @@ import dagger.Subcomponent; @Subcomponent public abstract class PhoneLookupComponent { - public abstract PhoneLookup phoneLookup(); + public abstract PhoneLookup phoneLookup(); public static PhoneLookupComponent get(Context context) { return ((HasComponent) ((HasRootComponent) context.getApplicationContext()).component()) diff --git a/java/com/android/dialer/phonelookup/PhoneLookupModule.java b/java/com/android/dialer/phonelookup/PhoneLookupModule.java index 39b0a5083..8a590052d 100644 --- a/java/com/android/dialer/phonelookup/PhoneLookupModule.java +++ b/java/com/android/dialer/phonelookup/PhoneLookupModule.java @@ -32,7 +32,8 @@ public abstract class PhoneLookupModule { } @Provides - static PhoneLookup providePhoneLookup(CompositePhoneLookup compositePhoneLookup) { + static PhoneLookup providePhoneLookup( + CompositePhoneLookup compositePhoneLookup) { return compositePhoneLookup; } } diff --git a/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java b/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java index bb7856fff..da4378bb7 100644 --- a/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java +++ b/java/com/android/dialer/phonelookup/composite/CompositePhoneLookup.java @@ -28,18 +28,20 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import java.util.ArrayList; import java.util.List; +import java.util.Map; import javax.inject.Inject; /** * {@link PhoneLookup} which delegates to a configured set of {@link PhoneLookup PhoneLookups}, * iterating, prioritizing, and coalescing data as necessary. */ -public final class CompositePhoneLookup implements PhoneLookup { +public final class CompositePhoneLookup implements PhoneLookup { private final ImmutableList phoneLookups; private final ListeningExecutorService lightweightExecutorService; @@ -58,20 +60,22 @@ public final class CompositePhoneLookup implements PhoneLookup { *

Note: If any of the dependent lookups fails, the returned future will also fail. If any of * the dependent lookups does not complete, the returned future will also not complete. */ + @SuppressWarnings("unchecked") @Override public ListenableFuture lookup(@NonNull Call call) { // 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) { + List> futures = new ArrayList<>(); + for (PhoneLookup phoneLookup : phoneLookups) { futures.add(phoneLookup.lookup(call)); } return Futures.transform( Futures.allAsList(futures), infos -> { PhoneLookupInfo.Builder mergedInfo = PhoneLookupInfo.newBuilder(); - for (PhoneLookupInfo info : infos) { - mergedInfo.mergeFrom(info); + for (int i = 0; i < infos.size(); i++) { + PhoneLookup phoneLookup = phoneLookups.get(i); + phoneLookup.setSubMessage(mergedInfo, infos.get(i)); } return mergedInfo.build(); }, @@ -81,7 +85,7 @@ public final class CompositePhoneLookup implements PhoneLookup { @Override public ListenableFuture isDirty(ImmutableSet phoneNumbers) { List> futures = new ArrayList<>(); - for (PhoneLookup phoneLookup : phoneLookups) { + for (PhoneLookup phoneLookup : phoneLookups) { futures.add(phoneLookup.isDirty(phoneNumbers)); } // Executes all child lookups (possibly in parallel), completing when the first composite lookup @@ -96,14 +100,13 @@ public final class CompositePhoneLookup implements PhoneLookup { *

Note: If any of the dependent lookups fails, the returned future will also fail. If any of * the dependent lookups does not complete, the returned future will also not complete. */ + @SuppressWarnings("unchecked") @Override - public ListenableFuture> - getMostRecentPhoneLookupInfo( - ImmutableMap existingInfoMap) { - List>> futures = - new ArrayList<>(); + public ListenableFuture> getMostRecentInfo( + ImmutableMap existingInfoMap) { + List>> futures = new ArrayList<>(); for (PhoneLookup phoneLookup : phoneLookups) { - futures.add(phoneLookup.getMostRecentPhoneLookupInfo(existingInfoMap)); + futures.add(buildSubmapAndGetMostRecentInfo(existingInfoMap, phoneLookup)); } return Futures.transform( Futures.allAsList(futures), @@ -113,14 +116,14 @@ public final class CompositePhoneLookup implements PhoneLookup { for (DialerPhoneNumber dialerPhoneNumber : existingInfoMap.keySet()) { PhoneLookupInfo.Builder combinedInfo = PhoneLookupInfo.newBuilder(); for (int i = 0; i < allMaps.size(); i++) { - ImmutableMap map = allMaps.get(i); - PhoneLookupInfo subInfo = map.get(dialerPhoneNumber); + ImmutableMap map = allMaps.get(i); + Object subInfo = map.get(dialerPhoneNumber); if (subInfo == null) { throw new IllegalStateException( "A sublookup didn't return an info for number: " + LogUtil.sanitizePhoneNumber(dialerPhoneNumber.getRawInput().getNumber())); } - phoneLookups.get(i).copySubMessage(combinedInfo, subInfo); + phoneLookups.get(i).setSubMessage(combinedInfo, subInfo); } combinedMap.put(dialerPhoneNumber, combinedInfo.build()); } @@ -129,15 +132,33 @@ public final class CompositePhoneLookup implements PhoneLookup { lightweightExecutorService); } + private ListenableFuture> buildSubmapAndGetMostRecentInfo( + ImmutableMap existingInfoMap, + PhoneLookup phoneLookup) { + Map submap = + Maps.transformEntries( + existingInfoMap, + (dialerPhoneNumber, phoneLookupInfo) -> + phoneLookup.getSubMessage(existingInfoMap.get(dialerPhoneNumber))); + return phoneLookup.getMostRecentInfo(ImmutableMap.copyOf(submap)); + } + @Override - public void copySubMessage(PhoneLookupInfo.Builder destination, PhoneLookupInfo source) { - throw new UnsupportedOperationException(); + public void setSubMessage(PhoneLookupInfo.Builder destination, PhoneLookupInfo source) { + throw new UnsupportedOperationException( + "This method is only expected to be called by CompositePhoneLookup itself"); + } + + @Override + public PhoneLookupInfo getSubMessage(PhoneLookupInfo phoneLookupInfo) { + throw new UnsupportedOperationException( + "This method is only expected to be called by CompositePhoneLookup itself"); } @Override public ListenableFuture onSuccessfulBulkUpdate() { List> futures = new ArrayList<>(); - for (PhoneLookup phoneLookup : phoneLookups) { + for (PhoneLookup phoneLookup : phoneLookups) { futures.add(phoneLookup.onSuccessfulBulkUpdate()); } return Futures.transform( diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java index a477e035c..60c934ace 100644 --- a/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java +++ b/java/com/android/dialer/phonelookup/cp2/Cp2PhoneLookup.java @@ -53,7 +53,7 @@ import java.util.Set; import javax.inject.Inject; /** PhoneLookup implementation for local contacts. */ -public final class Cp2PhoneLookup implements PhoneLookup { +public final class Cp2PhoneLookup implements PhoneLookup { private static final String PREF_LAST_TIMESTAMP_PROCESSED = "cp2PhoneLookupLastTimestampProcessed"; @@ -98,14 +98,14 @@ public final class Cp2PhoneLookup implements PhoneLookup { } @Override - public ListenableFuture lookup(Call call) { + public ListenableFuture lookup(Call call) { return backgroundExecutorService.submit(() -> lookupInternal(call)); } - private PhoneLookupInfo lookupInternal(Call call) { + private Cp2Info lookupInternal(Call call) { String rawNumber = TelecomCallUtil.getNumber(call); if (TextUtils.isEmpty(rawNumber)) { - return PhoneLookupInfo.getDefaultInstance(); + return Cp2Info.getDefaultInstance(); } Optional e164 = TelecomCallUtil.getE164Number(appContext, call); Set cp2ContactInfos = new ArraySet<>(); @@ -115,15 +115,13 @@ public final class Cp2PhoneLookup implements PhoneLookup { : queryPhoneTableBasedOnRawNumber(CP2_INFO_PROJECTION, ImmutableSet.of(rawNumber))) { if (cursor == null) { LogUtil.w("Cp2PhoneLookup.lookupInternal", "null cursor"); - return PhoneLookupInfo.getDefaultInstance(); + return Cp2Info.getDefaultInstance(); } while (cursor.moveToNext()) { cp2ContactInfos.add(buildCp2ContactInfoFromPhoneCursor(appContext, cursor)); } } - return PhoneLookupInfo.newBuilder() - .setCp2Info(Cp2Info.newBuilder().addAllCp2ContactInfo(cp2ContactInfos)) - .build(); + return Cp2Info.newBuilder().addAllCp2ContactInfo(cp2ContactInfos).build(); } @Override @@ -323,20 +321,23 @@ public final class Cp2PhoneLookup implements PhoneLookup { } @Override - public ListenableFuture> - getMostRecentPhoneLookupInfo( - ImmutableMap existingInfoMap) { - return backgroundExecutorService.submit( - () -> getMostRecentPhoneLookupInfoInternal(existingInfoMap)); + public ListenableFuture> getMostRecentInfo( + ImmutableMap existingInfoMap) { + return backgroundExecutorService.submit(() -> getMostRecentInfoInternal(existingInfoMap)); + } + + @Override + public void setSubMessage(PhoneLookupInfo.Builder destination, Cp2Info subMessage) { + destination.setCp2Info(subMessage); } @Override - public void copySubMessage(PhoneLookupInfo.Builder destination, PhoneLookupInfo source) { - destination.setCp2Info(source.getCp2Info()); + public Cp2Info getSubMessage(PhoneLookupInfo phoneLookupInfo) { + return phoneLookupInfo.getCp2Info(); } - private ImmutableMap getMostRecentPhoneLookupInfoInternal( - ImmutableMap existingInfoMap) { + private ImmutableMap getMostRecentInfoInternal( + ImmutableMap existingInfoMap) { currentLastTimestampProcessed = null; long lastModified = sharedPreferences.getLong(PREF_LAST_TIMESTAMP_PROCESSED, 0L); @@ -352,25 +353,23 @@ public final class Cp2PhoneLookup implements PhoneLookup { buildMapForUpdatedOrAddedContacts(existingInfoMap, lastModified, deletedPhoneNumbers); // Start build a new map of updated info. This will replace existing info. - ImmutableMap.Builder newInfoMapBuilder = - ImmutableMap.builder(); + ImmutableMap.Builder newInfoMapBuilder = ImmutableMap.builder(); // For each DialerPhoneNumber in existing info... - for (Entry entry : existingInfoMap.entrySet()) { + for (Entry entry : existingInfoMap.entrySet()) { DialerPhoneNumber dialerPhoneNumber = entry.getKey(); - PhoneLookupInfo existingInfo = entry.getValue(); + Cp2Info existingInfo = entry.getValue(); // Build off the existing info - PhoneLookupInfo.Builder infoBuilder = PhoneLookupInfo.newBuilder(existingInfo); + Cp2Info.Builder infoBuilder = Cp2Info.newBuilder(existingInfo); // If the contact was updated, replace the Cp2ContactInfo list if (updatedContacts.containsKey(dialerPhoneNumber)) { - infoBuilder.setCp2Info( - Cp2Info.newBuilder().addAllCp2ContactInfo(updatedContacts.get(dialerPhoneNumber))); + infoBuilder.clear().addAllCp2ContactInfo(updatedContacts.get(dialerPhoneNumber)); // If it was deleted and not added to a new contact, clear all the CP2 information. } else if (deletedPhoneNumbers.contains(dialerPhoneNumber)) { - infoBuilder.clearCp2Info(); + infoBuilder.clear(); } // If the DialerPhoneNumber didn't change, add the unchanged existing info. @@ -399,11 +398,11 @@ public final class Cp2PhoneLookup implements PhoneLookup { * the smallest set of dialer phone numbers to query cp2 against. 4. build and return the map of * dialerphonenumbers to their new Cp2ContactInfo * - * @return Map of {@link DialerPhoneNumber} to {@link PhoneLookupInfo} with updated {@link + * @return Map of {@link DialerPhoneNumber} to {@link Cp2Info} with updated {@link * Cp2ContactInfo}. */ private Map> buildMapForUpdatedOrAddedContacts( - ImmutableMap existingInfoMap, + ImmutableMap existingInfoMap, long lastModified, Set deletedPhoneNumbers) { @@ -411,9 +410,9 @@ public final class Cp2PhoneLookup implements PhoneLookup { Set updatedNumbers = new ArraySet<>(); Set contactIds = new ArraySet<>(); - for (Entry entry : existingInfoMap.entrySet()) { + for (Entry entry : existingInfoMap.entrySet()) { DialerPhoneNumber dialerPhoneNumber = entry.getKey(); - PhoneLookupInfo existingInfo = entry.getValue(); + Cp2Info existingInfo = entry.getValue(); // If the number was deleted, we need to check if it was added to a new contact. if (deletedPhoneNumbers.contains(dialerPhoneNumber)) { @@ -423,13 +422,13 @@ public final class Cp2PhoneLookup implements PhoneLookup { /// When the PhoneLookupHistory contains no information for a number, because for example the // user just upgraded to the new UI, or cleared data, we need to check for updated info. - if (existingInfo.getCp2Info().getCp2ContactInfoCount() == 0) { + if (existingInfo.getCp2ContactInfoCount() == 0) { updatedNumbers.add(dialerPhoneNumber); } else { // 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 : existingInfo.getCp2Info().getCp2ContactInfoList()) { + for (Cp2ContactInfo cp2ContactInfo : existingInfo.getCp2ContactInfoList()) { long existingContactId = cp2ContactInfo.getContactId(); if (existingContactId == 0) { // If the number doesn't have a contact id, for various reasons, we need to look up the @@ -609,7 +608,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { /** Returns set of DialerPhoneNumbers that were associated with now deleted contacts. */ private Set getDeletedPhoneNumbers( - ImmutableMap existingInfoMap, long lastModified) { + ImmutableMap existingInfoMap, long lastModified) { // Build set of all contact IDs from our existing data. We're going to use this set to query // against the DeletedContacts table and see if any of them were deleted. Set contactIds = findContactIdsIn(existingInfoMap); @@ -621,10 +620,10 @@ public final class Cp2PhoneLookup implements PhoneLookup { } } - private Set findContactIdsIn(ImmutableMap map) { + private Set findContactIdsIn(ImmutableMap map) { Set contactIds = new ArraySet<>(); - for (PhoneLookupInfo info : map.values()) { - for (Cp2ContactInfo cp2ContactInfo : info.getCp2Info().getCp2ContactInfoList()) { + for (Cp2Info info : map.values()) { + for (Cp2ContactInfo cp2ContactInfo : info.getCp2ContactInfoList()) { contactIds.add(cp2ContactInfo.getContactId()); } } @@ -659,7 +658,7 @@ public final class Cp2PhoneLookup implements PhoneLookup { /** Returns set of DialerPhoneNumbers that are associated with deleted contact IDs. */ private Set findDeletedPhoneNumbersIn( - ImmutableMap existingInfoMap, Cursor cursor) { + ImmutableMap existingInfoMap, Cursor cursor) { int contactIdIndex = cursor.getColumnIndexOrThrow(DeletedContacts.CONTACT_ID); int deletedTimeIndex = cursor.getColumnIndexOrThrow(DeletedContacts.CONTACT_DELETED_TIMESTAMP); Set deletedPhoneNumbers = new ArraySet<>(); @@ -678,10 +677,10 @@ public final class Cp2PhoneLookup implements PhoneLookup { } private static Set findDialerPhoneNumbersContainingContactId( - ImmutableMap existingInfoMap, long contactId) { + ImmutableMap existingInfoMap, long contactId) { Set matches = new ArraySet<>(); - for (Entry entry : existingInfoMap.entrySet()) { - for (Cp2ContactInfo cp2ContactInfo : entry.getValue().getCp2Info().getCp2ContactInfoList()) { + for (Entry entry : existingInfoMap.entrySet()) { + for (Cp2ContactInfo cp2ContactInfo : entry.getValue().getCp2ContactInfoList()) { if (cp2ContactInfo.getContactId() == contactId) { matches.add(entry.getKey()); } -- cgit v1.2.3