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
diff --git a/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java b/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java
index 54df399..e6c15e8 100644
--- a/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java
+++ b/java/com/android/dialer/phonelookup/blockednumber/DialerBlockedNumberPhoneLookup.java
@@ -134,7 +134,8 @@
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 @@
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 @@
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 8879fee..eede0f1 100644
--- a/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java
+++ b/java/com/android/dialer/phonelookup/cp2/Cp2LocalPhoneLookup.java
@@ -102,14 +102,14 @@
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 @@
@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 @@
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 @@
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 @@
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 @@
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 @@
}
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 8cb4557..8969737 100644
--- a/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
+++ b/java/com/android/dialer/phonenumberproto/DialerPhoneNumberUtil.java
@@ -132,13 +132,13 @@
* 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 @@
* @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 372f21e..4c8ac2f 100644
--- a/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
+++ b/java/com/android/dialer/phonenumberproto/PartitionedNumbers.java
@@ -30,14 +30,14 @@
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 @@
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 @@
}
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 7d71b4b..3ae9523 100644
--- a/java/com/android/dialer/telecom/TelecomCallUtil.java
+++ b/java/com/android/dialer/telecom/TelecomCallUtil.java
@@ -82,9 +82,9 @@
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 @@
}
/**
- * 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 @@
}
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()));