From 75a8d5170399b37d33bced7825a9f6d88a5ec233 Mon Sep 17 00:00:00 2001 From: zachh Date: Mon, 29 Jan 2018 13:45:58 -0800 Subject: 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 --- .../phonenumberproto/DialerPhoneNumberUtil.java | 100 ++++----------------- .../phonenumberproto/PartitionedNumbers.java | 35 +++----- .../phonenumberproto/dialer_phone_number.proto | 16 ++++ 3 files changed, 48 insertions(+), 103 deletions(-) (limited to 'java/com/android/dialer/phonenumberproto') 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}. - * - *

Work is run on the provided {@link ListeningExecutorService}. - * - * @see PhoneNumberUtil#parse(CharSequence, String) - */ - @AnyThread - public ListenableFuture 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()}. - * - *

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 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. + * + *

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> @@ -40,29 +40,27 @@ public final class PartitionedNumbers { private final ImmutableMap> invalidNumbersToDialerPhoneNumbers; + /** + * Divides a set of {@link DialerPhoneNumber DialerPhoneNumbers} according to those that are valid + * according to libphonenumber, and those that are not. + * + *

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 dialerPhoneNumbers) { Assert.isWorkerThread(); - DialerPhoneNumberUtil dialerPhoneNumberUtil = - new DialerPhoneNumberUtil(PhoneNumberUtil.getInstance()); Map> e164MapBuilder = new ArrayMap<>(); Map> invalidMapBuilder = new ArrayMap<>(); for (DialerPhoneNumber dialerPhoneNumber : dialerPhoneNumbers) { - Optional 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 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 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; } -- cgit v1.2.3