Added is_valid and post_dial_portion fields to DialerPhoneNumber.

These are frequently used attributes of numbers that we can compute once at parse-time.

Also did some general cleanup of DialerPhoneNumberUtil:

-Removed unused Future version of parse()
-Remove formatToValidE164 now that the new fields are available
-Inlined normalizeNumber()

Bug: 72563861
Test: existing
PiperOrigin-RevId: 183720128
Change-Id: I702dc265360e590439c5352c493ae8a858f36812

3 files changed, 48 insertions, 103 deletions

diff --git a/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java b/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
index 319467ccc..a5b9520fd 100644
--- a/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
+++ b/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
@@ -16,7 +16,6 @@
 
 package com.android.dialer.phonenumberproto;
 
-import android.support.annotation.AnyThread;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.WorkerThread;
@@ -25,9 +24,6 @@ import android.text.TextUtils;
 import com.android.dialer.DialerPhoneNumber;
 import com.android.dialer.common.Assert;
 import com.android.dialer.common.LogUtil;
-import com.google.common.base.Optional;
-import com.google.common.util.concurrent.ListenableFuture;
-import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.i18n.phonenumbers.NumberParseException;
 import com.google.i18n.phonenumbers.PhoneNumberUtil;
 import com.google.i18n.phonenumbers.PhoneNumberUtil.MatchType;
@@ -59,58 +55,30 @@ public class DialerPhoneNumberUtil {
     Assert.isWorkerThread();
 
     DialerPhoneNumber.Builder dialerPhoneNumber = DialerPhoneNumber.newBuilder();
-    // Numbers can be null or empty for incoming "unknown" calls.
-    if (numberToParse != null) {
-      dialerPhoneNumber.setNormalizedNumber(normalizeNumber(numberToParse, defaultRegion));
-    }
+
     if (defaultRegion != null) {
       dialerPhoneNumber.setCountryIso(defaultRegion);
     }
-    return dialerPhoneNumber.build();
-  }
-
-  /**
-   * Parses the provided raw phone number into a Future result of {@link DialerPhoneNumber}.
-   *
-   * <p>Work is run on the provided {@link ListeningExecutorService}.
-   *
-   * @see PhoneNumberUtil#parse(CharSequence, String)
-   */
-  @AnyThread
-  public ListenableFuture<DialerPhoneNumber> parse(
-      @Nullable String numberToParse,
-      @Nullable String defaultRegion,
-      @NonNull ListeningExecutorService service) {
-    return service.submit(() -> parse(numberToParse, defaultRegion));
-  }
 
-  /**
-   * 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 #formatToValidE164(DialerPhoneNumber)
-   * @see PhoneNumberUtils#normalizeNumber(String)
-   */
-  public String normalizeNumber(DialerPhoneNumber number) {
-    // TODO(zachh): Inline this method.
-    // TODO(zachh): This loses country info when number is not valid.
-    return number.getNormalizedNumber();
-  }
-
-  @WorkerThread
-  private String normalizeNumber(@NonNull String rawNumber, @Nullable String defaultRegion) {
-    Assert.isWorkerThread();
+    // Numbers can be null or empty for incoming "unknown" calls.
+    if (numberToParse == null) {
+      return dialerPhoneNumber.build();
+    }
 
     // If the number is a service number, just store the raw number and don't bother trying to parse
     // it. PhoneNumberUtil#parse ignores these characters which can lead to confusing behavior, such
     // as the numbers "#123" and "123" being considered the same. The "#" can appear in the middle
     // of a service number and the "*" can appear at the beginning (see a bug).
-    if (isServiceNumber(rawNumber)) {
-      return rawNumber;
+    if (isServiceNumber(numberToParse)) {
+      return dialerPhoneNumber.setNormalizedNumber(numberToParse).build();
     }
 
-    String postDialPortion = PhoneNumberUtils.extractPostDialPortion(rawNumber);
-    String networkPortion = PhoneNumberUtils.extractNetworkPortion(rawNumber);
+    String postDialPortion = PhoneNumberUtils.extractPostDialPortion(numberToParse);
+    if (!postDialPortion.isEmpty()) {
+      dialerPhoneNumber.setPostDialPortion(postDialPortion);
+    }
+
+    String networkPortion = PhoneNumberUtils.extractNetworkPortion(numberToParse);
 
     try {
       PhoneNumber phoneNumber = phoneNumberUtil.parse(networkPortion, defaultRegion);
@@ -118,18 +86,18 @@ public class DialerPhoneNumberUtil {
         String validNumber = phoneNumberUtil.format(phoneNumber, PhoneNumberFormat.E164);
         if (TextUtils.isEmpty(validNumber)) {
           throw new IllegalStateException(
-              "e164 number should not be empty: " + LogUtil.sanitizePii(rawNumber));
+              "e164 number should not be empty: " + LogUtil.sanitizePii(numberToParse));
         }
         // The E164 representation doesn't contain post-dial digits, but we need to preserve them.
-        if (postDialPortion != null) {
+        if (!postDialPortion.isEmpty()) {
           validNumber += postDialPortion;
         }
-        return validNumber;
+        return dialerPhoneNumber.setNormalizedNumber(validNumber).setIsValid(true).build();
       }
     } catch (NumberParseException e) {
       // fall through
     }
-    return networkPortion + postDialPortion;
+    return dialerPhoneNumber.setNormalizedNumber(networkPortion + postDialPortion).build();
   }
 
   /**
@@ -197,39 +165,7 @@ public class DialerPhoneNumberUtil {
     return (matchType == MatchType.SHORT_NSN_MATCH
             || matchType == MatchType.NSN_MATCH
             || matchType == MatchType.EXACT_MATCH)
-        && samePostDialPortion(firstNumberIn, secondNumberIn);
-  }
-
-  private static boolean samePostDialPortion(DialerPhoneNumber number1, DialerPhoneNumber number2) {
-    return PhoneNumberUtils.extractPostDialPortion(number1.getNormalizedNumber())
-        .equals(PhoneNumberUtils.extractPostDialPortion(number2.getNormalizedNumber()));
-  }
-
-  /**
-   * If the provided number is "valid" (see {@link PhoneNumberUtil#isValidNumber(PhoneNumber)}),
-   * formats it to E.164. Otherwise, returns {@link Optional#absent()}.
-   *
-   * <p>This method is analogous to {@link PhoneNumberUtils#formatNumberToE164(String, String)} (but
-   * works with an already parsed {@link DialerPhoneNumber} object).
-   *
-   * @see PhoneNumberUtil#isValidNumber(PhoneNumber)
-   * @see PhoneNumberUtil#format(PhoneNumber, PhoneNumberFormat)
-   * @see PhoneNumberUtils#formatNumberToE164(String, String)
-   */
-  @WorkerThread
-  public Optional<String> formatToValidE164(DialerPhoneNumber number) {
-    // TODO(zachh): We could do something like store a "valid" bit in DialerPhoneNumber?
-    Assert.isWorkerThread();
-    PhoneNumber phoneNumber;
-    try {
-      phoneNumber = phoneNumberUtil.parse(number.getNormalizedNumber(), number.getCountryIso());
-    } catch (NumberParseException e) {
-      return Optional.absent();
-    }
-    if (phoneNumberUtil.isValidNumber(phoneNumber)) {
-      return Optional.fromNullable(phoneNumberUtil.format(phoneNumber, PhoneNumberFormat.E164));
-    }
-    return Optional.absent();
+        && firstNumberIn.getPostDialPortion().equals(secondNumberIn.getPostDialPortion());
   }
 
   private boolean isServiceNumber(@NonNull String rawNumber) {
diff --git a/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java b/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
index dbf99365c..2c19c1232 100644
--- a/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
+++ b/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
@@ -20,19 +20,19 @@ import android.support.annotation.NonNull;
 import android.support.annotation.WorkerThread;
 import android.support.v4.util.ArrayMap;
 import android.support.v4.util.ArraySet;
-import android.telephony.PhoneNumberUtils;
 import com.android.dialer.DialerPhoneNumber;
 import com.android.dialer.common.Assert;
-import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.i18n.phonenumbers.PhoneNumberUtil;
 import java.util.Map;
 import java.util.Set;
 
 /**
  * Divides a set of {@link DialerPhoneNumber DialerPhoneNumbers} according to those that are valid
  * according to libphonenumber, and those that are not.
+ *
+ * <p>Numbers with post-dial portions are always considered invalid as most systems store E164
+ * numbers which do not support post-dial portions.
  */
 public final class PartitionedNumbers {
   private final ImmutableMap<String, ImmutableSet<DialerPhoneNumber>>
@@ -40,29 +40,27 @@ public final class PartitionedNumbers {
   private final ImmutableMap<String, ImmutableSet<DialerPhoneNumber>>
       invalidNumbersToDialerPhoneNumbers;
 
+  /**
+   * Divides a set of {@link DialerPhoneNumber DialerPhoneNumbers} according to those that are valid
+   * according to libphonenumber, and those that are not.
+   *
+   * <p>Numbers with post-dial portions are always considered invalid as most systems store E164
+   * numbers which do not support post-dial portions.
+   */
   @WorkerThread
   public PartitionedNumbers(@NonNull ImmutableSet<DialerPhoneNumber> dialerPhoneNumbers) {
     Assert.isWorkerThread();
-    DialerPhoneNumberUtil dialerPhoneNumberUtil =
-        new DialerPhoneNumberUtil(PhoneNumberUtil.getInstance());
     Map<String, Set<DialerPhoneNumber>> e164MapBuilder = new ArrayMap<>();
     Map<String, Set<DialerPhoneNumber>> invalidMapBuilder = new ArrayMap<>();
 
     for (DialerPhoneNumber dialerPhoneNumber : dialerPhoneNumbers) {
-      Optional<String> optValidE164 = dialerPhoneNumberUtil.formatToValidE164(dialerPhoneNumber);
       /*
        * Numbers with post-dial digits are considered valid and can be converted to E164, but their
-       * post dial digits are lost in the process. Similarly, if a contact's number has a post-dial
-       * digits, the normalized version of it stored in the contacts database does not include the
-       * post dial digits.
-       *
-       * A number with post-dial digits should not match a contact whose number does not have
-       * post-dial digits, which means that we cannot normalize such numbers for use in bulk lookup.
-       * Treat them as invalid which will cause them to be processed individually using
-       * ContactsContract.PHONE_LOOKUP.
+       * post dial digits are lost in the process. For example, the normalized version of a number
+       * with a post-dial portion in the contacts database is stored without the post-dial portion.
        */
-      if (optValidE164.isPresent() && !hasPostDialDigits(dialerPhoneNumber)) {
-        String validE164 = optValidE164.get();
+      if (dialerPhoneNumber.getIsValid() && dialerPhoneNumber.getPostDialPortion().isEmpty()) {
+        String validE164 = dialerPhoneNumber.getNormalizedNumber();
         Set<DialerPhoneNumber> currentNumbers = e164MapBuilder.get(validE164);
         if (currentNumbers == null) {
           currentNumbers = new ArraySet<>();
@@ -84,11 +82,6 @@ public final class PartitionedNumbers {
     invalidNumbersToDialerPhoneNumbers = makeImmutable(invalidMapBuilder);
   }
 
-  private boolean hasPostDialDigits(DialerPhoneNumber dialerPhoneNumber) {
-    return !PhoneNumberUtils.extractPostDialPortion(dialerPhoneNumber.getNormalizedNumber())
-        .isEmpty();
-  }
-
   /** Returns the set of invalid numbers from the original DialerPhoneNumbers */
   @NonNull
   public ImmutableSet<String> invalidNumbers() {
diff --git a/java/com/android/dialer/phonenumberproto/dialer_phone_number.proto b/java/com/android/dialer/phonenumberproto/dialer_phone_number.proto
index 941de0428..ad8a8f9fa 100644
--- a/java/com/android/dialer/phonenumberproto/dialer_phone_number.proto
+++ b/java/com/android/dialer/phonenumberproto/dialer_phone_number.proto
@@ -40,6 +40,11 @@ message DialerPhoneNumber {
   //  number is a 7-digit US number (missing an area code) like "456-7890" which
   //  would be stored as "4567890".
   //
+  //  Note: Using this field without country_iso effectively loses country info
+  //  when the number is not valid and no country prefix was prepended. This may
+  //  cause numbers like {"456-7890", "US"} to be treated equivalently to
+  //  {"456-7890", "DE"}, when they are not in fact equivalent.
+  //
   //  See DialerPhoneNumberUtil#parse.
   optional string normalized_number = 1;
 
@@ -47,4 +52,15 @@ message DialerPhoneNumber {
   // CallLog.Calls#COUNTRY: "The ISO 3166-1 two letters country code of the
   // country where the user received or made the call."
   optional string country_iso = 2;
+
+  // True if the number is valid according to libphonenumber.
+  optional bool is_valid = 3;
+
+  // The post dial portion of the number as described by
+  // PhoneNumberUtils#extractPostDialPortion. Note that this is also part of
+  // normalized_number, but this information is duplicated here for convenience.
+  //
+  // This includes pause and wait characters, but strips other characters, so
+  // for example would be ",123;456" given the raw input of "456-7890,123; 456".
+  optional string post_dial_portion = 4;
 }