Cleaned up wording around "valid" and "formattable".

We don't actually parition by "formattable", we parition by "valid". An invalid number like 456-7890 can be formatted to E164 ("+14567890") but what ParitionedNumbers actually does is parition by valid/invalid (and then converts the valid numbers to E164).

Also added a new sharded test suite for phonenumberproto tests which had occasionally been timing out on TAP.

Test: existing
PiperOrigin-RevId: 181800443
Change-Id: Id64fc32c893025b0115dd350dd87e3277607f21c

5 files changed, 80 insertions, 78 deletions

diff --git a/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java b/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java
index 54df3995c..e6c15e8d9 100644
--- a/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java
+++ b/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java
@@ -134,7 +134,8 @@ public final class DialerBlockedNumberPhoneLookup implements PhoneLookup<DialerB
                 null)) {
       while (cursor != null && cursor.moveToNext()) {
         if (cursor.getInt(1) == FilteredNumberTypes.BLOCKED_NUMBER) {
-          blockedNumbers.addAll(partitionedNumbers.dialerPhoneNumbersForE164(cursor.getString(0)));
+          blockedNumbers.addAll(
+              partitionedNumbers.dialerPhoneNumbersForValidE164(cursor.getString(0)));
         }
       }
     }
@@ -143,8 +144,8 @@ public final class DialerBlockedNumberPhoneLookup implements PhoneLookup<DialerB
         Selection.column(FilteredNumberColumns.NUMBER)
             .in(
                 partitionedNumbers
-                    .unformattableNumbers()
-                    .toArray(new String[partitionedNumbers.unformattableNumbers().size()]));
+                    .invalidNumbers()
+                    .toArray(new String[partitionedNumbers.invalidNumbers().size()]));
     try (Cursor cursor =
         appContext
             .getContentResolver()
@@ -157,7 +158,7 @@ public final class DialerBlockedNumberPhoneLookup implements PhoneLookup<DialerB
       while (cursor != null && cursor.moveToNext()) {
         if (cursor.getInt(1) == FilteredNumberTypes.BLOCKED_NUMBER) {
           blockedNumbers.addAll(
-              partitionedNumbers.dialerPhoneNumbersForUnformattable(cursor.getString(0)));
+              partitionedNumbers.dialerPhoneNumbersForInvalid(cursor.getString(0)));
         }
       }
     }
diff --git a/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java b/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java
index 8879fee06..eede0f1b0 100644
--- a/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java
+++ b/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java
@@ -102,14 +102,14 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup<Cp2Info> {
     if (TextUtils.isEmpty(rawNumber)) {
       return Cp2Info.getDefaultInstance();
     }
-    Optional<String> e164 = TelecomCallUtil.getE164Number(appContext, call);
+    Optional<String> validE164 = TelecomCallUtil.getValidE164Number(appContext, call);
     Set<Cp2ContactInfo> 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.
     try (Cursor cursor =
-        e164.isPresent()
+        validE164.isPresent()
             ? queryPhoneTableBasedOnE164(
-                Cp2Projections.getProjectionForPhoneTable(), ImmutableSet.of(e164.get()))
+                Cp2Projections.getProjectionForPhoneTable(), ImmutableSet.of(validE164.get()))
             : queryPhoneLookup(Cp2Projections.getProjectionForPhoneLookupTable(), rawNumber)) {
       if (cursor == null) {
         LogUtil.w("Cp2LocalPhoneLookup.lookupInternal", "null cursor");
@@ -154,7 +154,7 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup<Cp2Info> {
   @Override
   public ListenableFuture<Boolean> isDirty(ImmutableSet<DialerPhoneNumber> phoneNumbers) {
     PartitionedNumbers partitionedNumbers = new PartitionedNumbers(phoneNumbers);
-    if (partitionedNumbers.unformattableNumbers().size() > MAX_SUPPORTED_INVALID_NUMBERS) {
+    if (partitionedNumbers.invalidNumbers().size() > MAX_SUPPORTED_INVALID_NUMBERS) {
       // If there are N invalid numbers, we can't determine determine dirtiness without running N
       // queries; since running this many queries is not feasible for the (lightweight) isDirty
       // check, simply return true. The expectation is that this should rarely be the case as the
@@ -239,15 +239,14 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup<Cp2Info> {
 
     List<ListenableFuture<Set<Long>>> queryFutures = new ArrayList<>();
 
-    // First use the E164 numbers to query the NORMALIZED_NUMBER column.
+    // First use the valid E164 numbers to query the NORMALIZED_NUMBER column.
     queryFutures.add(
         queryPhoneTableForContactIdsBasedOnE164(partitionedNumbers.validE164Numbers()));
 
     // Then run a separate query for each invalid number. Separate queries are done to accomplish
     // loose matching which couldn't be accomplished with a batch query.
-    Assert.checkState(
-        partitionedNumbers.unformattableNumbers().size() <= MAX_SUPPORTED_INVALID_NUMBERS);
-    for (String invalidNumber : partitionedNumbers.unformattableNumbers()) {
+    Assert.checkState(partitionedNumbers.invalidNumbers().size() <= MAX_SUPPORTED_INVALID_NUMBERS);
+    for (String invalidNumber : partitionedNumbers.invalidNumbers()) {
       queryFutures.add(queryPhoneLookupTableForContactIdsBasedOnRawNumber(invalidNumber));
     }
     return Futures.transform(
@@ -527,10 +526,9 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup<Cp2Info> {
       ImmutableMap<DialerPhoneNumber, Cp2Info> existingInfoMap) {
     ArraySet<DialerPhoneNumber> unprocessableNumbers = new ArraySet<>();
     PartitionedNumbers partitionedNumbers = new PartitionedNumbers(existingInfoMap.keySet());
-    if (partitionedNumbers.unformattableNumbers().size() > MAX_SUPPORTED_INVALID_NUMBERS) {
-      for (String invalidNumber : partitionedNumbers.unformattableNumbers()) {
-        unprocessableNumbers.addAll(
-            partitionedNumbers.dialerPhoneNumbersForUnformattable(invalidNumber));
+    if (partitionedNumbers.invalidNumbers().size() > MAX_SUPPORTED_INVALID_NUMBERS) {
+      for (String invalidNumber : partitionedNumbers.invalidNumbers()) {
+        unprocessableNumbers.addAll(partitionedNumbers.dialerPhoneNumbersForInvalid(invalidNumber));
       }
     }
     return unprocessableNumbers;
@@ -652,39 +650,39 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup<Cp2Info> {
             return Futures.immediateFuture(new ArrayMap<>());
           }
 
-          // Divide the numbers into those we can format to E164 and those we can't. Issue a single
-          // batch query for the E164 numbers against the PHONE table, and in parallel issue
-          // individual queries against PHONE_LOOKUP for each non-E164 number.
+          // Divide the numbers into those that are valid and those that are not. Issue a single
+          // batch query for the valid numbers against the PHONE table, and in parallel issue
+          // individual queries against PHONE_LOOKUP for each invalid number.
           // TODO(zachh): These queries are inefficient without a lastModified column to filter on.
           PartitionedNumbers partitionedNumbers =
               new PartitionedNumbers(ImmutableSet.copyOf(updatedNumbers));
 
-          ListenableFuture<Map<String, Set<Cp2ContactInfo>>> e164Future =
+          ListenableFuture<Map<String, Set<Cp2ContactInfo>>> validNumbersFuture =
               batchQueryForValidNumbers(partitionedNumbers.validE164Numbers());
 
-          List<ListenableFuture<Set<Cp2ContactInfo>>> nonE164FuturesList = new ArrayList<>();
-          for (String invalidNumber : partitionedNumbers.unformattableNumbers()) {
-            nonE164FuturesList.add(individualQueryForInvalidNumber(invalidNumber));
+          List<ListenableFuture<Set<Cp2ContactInfo>>> invalidNumbersFuturesList = new ArrayList<>();
+          for (String invalidNumber : partitionedNumbers.invalidNumbers()) {
+            invalidNumbersFuturesList.add(individualQueryForInvalidNumber(invalidNumber));
           }
 
-          ListenableFuture<List<Set<Cp2ContactInfo>>> nonE164Future =
-              Futures.allAsList(nonE164FuturesList);
+          ListenableFuture<List<Set<Cp2ContactInfo>>> invalidNumbersFuture =
+              Futures.allAsList(invalidNumbersFuturesList);
 
           Callable<Map<DialerPhoneNumber, Set<Cp2ContactInfo>>> computeMap =
               () -> {
                 // These get() calls are safe because we are using whenAllSucceed below.
-                Map<String, Set<Cp2ContactInfo>> e164Result = e164Future.get();
-                List<Set<Cp2ContactInfo>> non164Results = nonE164Future.get();
+                Map<String, Set<Cp2ContactInfo>> validNumbersResult = validNumbersFuture.get();
+                List<Set<Cp2ContactInfo>> invalidNumbersResult = invalidNumbersFuture.get();
 
                 Map<DialerPhoneNumber, Set<Cp2ContactInfo>> map = new ArrayMap<>();
 
-                // First update the map with the E164 results.
-                for (Entry<String, Set<Cp2ContactInfo>> entry : e164Result.entrySet()) {
-                  String e164Number = entry.getKey();
+                // First update the map with the valid number results.
+                for (Entry<String, Set<Cp2ContactInfo>> entry : validNumbersResult.entrySet()) {
+                  String validNumber = entry.getKey();
                   Set<Cp2ContactInfo> cp2ContactInfos = entry.getValue();
 
                   Set<DialerPhoneNumber> dialerPhoneNumbers =
-                      partitionedNumbers.dialerPhoneNumbersForE164(e164Number);
+                      partitionedNumbers.dialerPhoneNumbersForValidE164(validNumber);
 
                   addInfo(map, dialerPhoneNumbers, cp2ContactInfos);
 
@@ -694,12 +692,12 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup<Cp2Info> {
                   updatedNumbers.removeAll(dialerPhoneNumbers);
                 }
 
-                // Next update the map with the non-E164 results.
+                // Next update the map with the invalid results.
                 int i = 0;
-                for (String unformattableNumber : partitionedNumbers.unformattableNumbers()) {
-                  Set<Cp2ContactInfo> cp2Infos = non164Results.get(i++);
+                for (String invalidNumber : partitionedNumbers.invalidNumbers()) {
+                  Set<Cp2ContactInfo> cp2Infos = invalidNumbersResult.get(i++);
                   Set<DialerPhoneNumber> dialerPhoneNumbers =
-                      partitionedNumbers.dialerPhoneNumbersForUnformattable(unformattableNumber);
+                      partitionedNumbers.dialerPhoneNumbersForInvalid(invalidNumber);
 
                   addInfo(map, dialerPhoneNumbers, cp2Infos);
 
@@ -717,32 +715,32 @@ public final class Cp2LocalPhoneLookup implements PhoneLookup<Cp2Info> {
                 }
                 return map;
               };
-          return Futures.whenAllSucceed(e164Future, nonE164Future)
+          return Futures.whenAllSucceed(validNumbersFuture, invalidNumbersFuture)
               .call(computeMap, lightweightExecutorService);
         },
         lightweightExecutorService);
   }
 
   private ListenableFuture<Map<String, Set<Cp2ContactInfo>>> batchQueryForValidNumbers(
-      Set<String> e164Numbers) {
+      Set<String> validE164Numbers) {
     return backgroundExecutorService.submit(
         () -> {
           Map<String, Set<Cp2ContactInfo>> cp2ContactInfosByNumber = new ArrayMap<>();
-          if (e164Numbers.isEmpty()) {
+          if (validE164Numbers.isEmpty()) {
             return cp2ContactInfosByNumber;
           }
           try (Cursor cursor =
               queryPhoneTableBasedOnE164(
-                  Cp2Projections.getProjectionForPhoneTable(), e164Numbers)) {
+                  Cp2Projections.getProjectionForPhoneTable(), validE164Numbers)) {
             if (cursor == null) {
               LogUtil.w("Cp2LocalPhoneLookup.batchQueryForValidNumbers", "null cursor");
             } else {
               while (cursor.moveToNext()) {
-                String e164Number = Cp2Projections.getNormalizedNumberFromCursor(cursor);
-                Set<Cp2ContactInfo> cp2ContactInfos = cp2ContactInfosByNumber.get(e164Number);
+                String validE164Number = Cp2Projections.getNormalizedNumberFromCursor(cursor);
+                Set<Cp2ContactInfo> cp2ContactInfos = cp2ContactInfosByNumber.get(validE164Number);
                 if (cp2ContactInfos == null) {
                   cp2ContactInfos = new ArraySet<>();
-                  cp2ContactInfosByNumber.put(e164Number, cp2ContactInfos);
+                  cp2ContactInfosByNumber.put(validE164Number, cp2ContactInfos);
                 }
                 cp2ContactInfos.add(
                     Cp2Projections.buildCp2ContactInfoFromCursor(appContext, cursor));
diff --git a/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java b/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
index 8cb4557cb..8969737d4 100644
--- a/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
+++ b/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
@@ -132,13 +132,13 @@ public class DialerPhoneNumberUtil {
    * Formats the provided number to E164 format or return a normalized version of the raw number if
    * the number is not valid according to {@link PhoneNumberUtil#isValidNumber(PhoneNumber)}.
    *
-   * @see #formatToE164(DialerPhoneNumber)
+   * @see #formatToValidE164(DialerPhoneNumber)
    * @see PhoneNumberUtils#normalizeNumber(String)
    */
   @WorkerThread
   public String normalizeNumber(DialerPhoneNumber number) {
     Assert.isWorkerThread();
-    return formatToE164(number)
+    return formatToValidE164(number)
         .or(PhoneNumberUtils.normalizeNumber(number.getRawInput().getNumber()));
   }
 
@@ -154,7 +154,7 @@ public class DialerPhoneNumberUtil {
    * @see PhoneNumberUtils#formatNumberToE164(String, String)
    */
   @WorkerThread
-  public Optional<String> formatToE164(DialerPhoneNumber number) {
+  public Optional<String> formatToValidE164(DialerPhoneNumber number) {
     Assert.isWorkerThread();
     if (number.hasDialerInternalPhoneNumber()) {
       PhoneNumber phoneNumber = Converter.protoToPojo(number.getDialerInternalPhoneNumber());
diff --git a/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java b/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
index 372f21ee8..4c8ac2f21 100644
--- a/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
+++ b/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
@@ -30,14 +30,14 @@ import java.util.Map;
 import java.util.Set;
 
 /**
- * Divides a set of {@link DialerPhoneNumber DialerPhoneNumbers} by those that can be formatted to
- * E164 and those that cannot.
+ * Divides a set of {@link DialerPhoneNumber DialerPhoneNumbers} according to those that are valid
+ * according to libphonenumber, and those that are not.
  */
 public final class PartitionedNumbers {
   private final ImmutableMap<String, ImmutableSet<DialerPhoneNumber>>
       e164NumbersToDialerPhoneNumbers;
   private final ImmutableMap<String, ImmutableSet<DialerPhoneNumber>>
-      unformattableNumbersToDialerPhoneNumbers;
+      invalidNumbersToDialerPhoneNumbers;
 
   @WorkerThread
   public PartitionedNumbers(@NonNull ImmutableSet<DialerPhoneNumber> dialerPhoneNumbers) {
@@ -45,12 +45,12 @@ public final class PartitionedNumbers {
     DialerPhoneNumberUtil dialerPhoneNumberUtil =
         new DialerPhoneNumberUtil(PhoneNumberUtil.getInstance());
     Map<String, Set<DialerPhoneNumber>> e164MapBuilder = new ArrayMap<>();
-    Map<String, Set<DialerPhoneNumber>> unformattableMapBuilder = new ArrayMap<>();
+    Map<String, Set<DialerPhoneNumber>> invalidMapBuilder = new ArrayMap<>();
 
     for (DialerPhoneNumber dialerPhoneNumber : dialerPhoneNumbers) {
-      Optional<String> e164 = dialerPhoneNumberUtil.formatToE164(dialerPhoneNumber);
-      if (e164.isPresent()) {
-        String validE164 = e164.get();
+      Optional<String> optValidE164 = dialerPhoneNumberUtil.formatToValidE164(dialerPhoneNumber);
+      if (optValidE164.isPresent()) {
+        String validE164 = optValidE164.get();
         Set<DialerPhoneNumber> currentNumbers = e164MapBuilder.get(validE164);
         if (currentNumbers == null) {
           currentNumbers = new ArraySet<>();
@@ -58,49 +58,52 @@ public final class PartitionedNumbers {
         }
         currentNumbers.add(dialerPhoneNumber);
       } else {
-        String unformattableNumber = dialerPhoneNumber.getRawInput().getNumber();
-        Set<DialerPhoneNumber> currentNumbers = unformattableMapBuilder.get(unformattableNumber);
+        String invalidNumber = dialerPhoneNumber.getRawInput().getNumber();
+        Set<DialerPhoneNumber> currentNumbers = invalidMapBuilder.get(invalidNumber);
         if (currentNumbers == null) {
           currentNumbers = new ArraySet<>();
-          unformattableMapBuilder.put(unformattableNumber, currentNumbers);
+          invalidMapBuilder.put(invalidNumber, currentNumbers);
         }
         currentNumbers.add(dialerPhoneNumber);
       }
     }
 
     e164NumbersToDialerPhoneNumbers = makeImmutable(e164MapBuilder);
-    unformattableNumbersToDialerPhoneNumbers = makeImmutable(unformattableMapBuilder);
+    invalidNumbersToDialerPhoneNumbers = makeImmutable(invalidMapBuilder);
   }
 
-  /** Returns the set of formatted number from the original DialerPhoneNumbers */
+  /** Returns the set of invalid numbers from the original DialerPhoneNumbers */
   @NonNull
-  public ImmutableSet<String> unformattableNumbers() {
-    return unformattableNumbersToDialerPhoneNumbers.keySet();
+  public ImmutableSet<String> invalidNumbers() {
+    return invalidNumbersToDialerPhoneNumbers.keySet();
   }
 
-  /** Returns the set of raw number that is unformattable from the original DialerPhoneNumbers */
+  /** Returns the set of valid, E164 formatted numbers from the original DialerPhoneNumbers */
   @NonNull
   public ImmutableSet<String> validE164Numbers() {
     return e164NumbersToDialerPhoneNumbers.keySet();
   }
 
   /**
-   * Returns the corresponding set of original DialerPhoneNumber that maps to the e.164 number, or
-   * an empty set if the number is not found.
+   * Returns the corresponding set of original DialerPhoneNumbers that map to the valid E164 number
+   * from {@link #validE164Numbers()}.
+   *
+   * @throws NullPointerException if there are no numbers found
    */
   @NonNull
-  public ImmutableSet<DialerPhoneNumber> dialerPhoneNumbersForE164(String e164) {
-    return Assert.isNotNull(e164NumbersToDialerPhoneNumbers.get(e164));
+  public ImmutableSet<DialerPhoneNumber> dialerPhoneNumbersForValidE164(String validE164) {
+    return Assert.isNotNull(e164NumbersToDialerPhoneNumbers.get(validE164));
   }
 
   /**
-   * Returns the corresponding set of original DialerPhoneNumber that maps to the unformattable
-   * number returned by {@link #unformattableNumbers()}, or an empty set if the number is not found.
+   * Returns the corresponding set of original DialerPhoneNumbers that map to the invalid number
+   * from {@link #invalidNumbers()}.
+   *
+   * @throws NullPointerException if there are no numbers found
    */
   @NonNull
-  public ImmutableSet<DialerPhoneNumber> dialerPhoneNumbersForUnformattable(
-      String unformattableNumber) {
-    return Assert.isNotNull(unformattableNumbersToDialerPhoneNumbers.get(unformattableNumber));
+  public ImmutableSet<DialerPhoneNumber> dialerPhoneNumbersForInvalid(String invalidNumber) {
+    return Assert.isNotNull(invalidNumbersToDialerPhoneNumbers.get(invalidNumber));
   }
 
   private static <K, V> ImmutableMap<K, ImmutableSet<V>> makeImmutable(
diff --git a/java/com/android/dialer/telecom/TelecomCallUtil.java b/java/com/android/dialer/telecom/TelecomCallUtil.java
index 7d71b4b90..3ae952357 100644
--- a/java/com/android/dialer/telecom/TelecomCallUtil.java
+++ b/java/com/android/dialer/telecom/TelecomCallUtil.java
@@ -82,9 +82,9 @@ public class TelecomCallUtil {
   public static Optional<String> getNormalizedNumber(Context appContext, Call call) {
     Assert.isWorkerThread();
 
-    Optional<String> e164 = getE164Number(appContext, call);
-    if (e164.isPresent()) {
-      return e164;
+    Optional<String> validE164 = getValidE164Number(appContext, call);
+    if (validE164.isPresent()) {
+      return validE164;
     }
     String rawNumber = getNumber(call);
     if (TextUtils.isEmpty(rawNumber)) {
@@ -94,14 +94,14 @@ public class TelecomCallUtil {
   }
 
   /**
-   * Formats the number of the {@code call} to E.164. The country of the SIM associated with the
-   * call is used to determine the country.
+   * Formats the number of the {@code call} to E.164 if it is valid. The country of the SIM
+   * associated with the call is used to determine the country.
    *
-   * <p>If the number cannot be formatted (because for example the country cannot be determined),
-   * returns {@link Optional#absent()}.
+   * <p>If the number cannot be formatted (because for example it is invalid or the country cannot
+   * be determined), returns {@link Optional#absent()}.
    */
   @WorkerThread
-  public static Optional<String> getE164Number(Context appContext, Call call) {
+  public static Optional<String> getValidE164Number(Context appContext, Call call) {
     Assert.isWorkerThread();
     String rawNumber = getNumber(call);
     if (TextUtils.isEmpty(rawNumber)) {
@@ -109,7 +109,7 @@ public class TelecomCallUtil {
     }
     Optional<String> countryCode = getCountryCode(appContext, call);
     if (!countryCode.isPresent()) {
-      LogUtil.w("TelecomCallUtil.getE164Number", "couldn't find a country code for call");
+      LogUtil.w("TelecomCallUtil.getValidE164Number", "couldn't find a country code for call");
       return Optional.absent();
     }
     return Optional.fromNullable(PhoneNumberUtils.formatNumberToE164(rawNumber, countryCode.get()));