Relax phone number collapser
The current collapser basically compares two phone numbers for equality
using their normalzied numbers, but this can cause an issue when you have a
readonly contact from a different country who has a local version of their
phone numer in their profile. Even if you have an international version
of their phone number, it may be overwritten on the contacts by the local
vesion.
So let's relax it.
The new collapser merges two numbers only if the "unformatted"
(i.e. ignoring all formatting characters) version of two phone numbers
are the same.
The following numbers will no longer be merged. (All assume the user
is in the US.)
- +81-45-381-0000 and 011-81-45-381-0000
- 1-555-123-4567 and 555-123-4567
- +1-555-123-4567 and 1-555-123-4567
But the following will.
- 555-123-4567 and 5551234567
This affects:
- Contact card
- Phone number disambiguation on the phone app
- Quick contact
The list view on the phone app won't be affected, as it's done in SQL, group-by.
Bug 5779336
Change-Id: I2d1f2fcf8a8d85207383cb05a86f66c17af8acfe
diff --git a/src/com/android/contacts/ContactsUtils.java b/src/com/android/contacts/ContactsUtils.java
index ea567fb..9a014f4 100644
--- a/src/com/android/contacts/ContactsUtils.java
+++ b/src/com/android/contacts/ContactsUtils.java
@@ -22,7 +22,6 @@
import com.android.contacts.model.AccountWithDataSet;
import com.android.contacts.test.NeededForTesting;
import com.android.contacts.util.Constants;
-import com.android.i18n.phonenumbers.PhoneNumberUtil;
import android.content.Context;
import android.content.Intent;
@@ -129,36 +128,41 @@
// mimetypes could have more sophisticated matching is the future, e.g. addresses)
if (!TextUtils.equals(Phone.CONTENT_ITEM_TYPE, mimetype1)) return false;
- // Now do the full phone number thing. split into parts, seperated by waiting symbol
- // and compare them individually
- final String[] dataParts1 = data1.toString().split(WAIT_SYMBOL_AS_STRING);
- final String[] dataParts2 = data2.toString().split(WAIT_SYMBOL_AS_STRING);
- if (dataParts1.length != dataParts2.length) return false;
- final PhoneNumberUtil util = PhoneNumberUtil.getInstance();
- for (int i = 0; i < dataParts1.length; i++) {
- final String dataPart1 = dataParts1[i];
- final String dataPart2 = dataParts2[i];
+ return shouldCollapsePhoneNumbers(data1.toString(), data2.toString());
+ }
- // substrings equal? shortcut, don't parse
- if (TextUtils.equals(dataPart1, dataPart2)) continue;
+ private static final boolean shouldCollapsePhoneNumbers(
+ String number1WithLetters, String number2WithLetters) {
+ final String number1 = PhoneNumberUtils.convertKeypadLettersToDigits(number1WithLetters);
+ final String number2 = PhoneNumberUtils.convertKeypadLettersToDigits(number2WithLetters);
- // do a full parse of the numbers
- switch (util.isNumberMatch(dataPart1, dataPart2)) {
- case NOT_A_NUMBER:
- // don't understand the numbers? let's play it safe
- return false;
- case NO_MATCH:
- return false;
- case EXACT_MATCH:
- case SHORT_NSN_MATCH:
- case NSN_MATCH:
- break;
- default:
- throw new IllegalStateException("Unknown result value from phone number " +
- "library");
+ int index1 = 0;
+ int index2 = 0;
+ for (;;) {
+ // Skip formatting characters.
+ while (index1 < number1.length() &&
+ !PhoneNumberUtils.isNonSeparator(number1.charAt(index1))) {
+ index1++;
}
+ while (index2 < number2.length() &&
+ !PhoneNumberUtils.isNonSeparator(number2.charAt(index2))) {
+ index2++;
+ }
+ // If both have finished, match. If only one has finished, not match.
+ final boolean number1End = (index1 == number1.length());
+ final boolean number2End = (index2 == number2.length());
+ if (number1End) {
+ return number2End;
+ }
+ if (number2End) return false;
+
+ // If the non-formatting characters are different, not match.
+ if (number1.charAt(index1) != number2.charAt(index2)) return false;
+
+ // Go to the next characters.
+ index1++;
+ index2++;
}
- return true;
}
/**
diff --git a/src/com/android/contacts/detail/ContactDetailFragment.java b/src/com/android/contacts/detail/ContactDetailFragment.java
index ca6fa63..faeb0c5 100644
--- a/src/com/android/contacts/detail/ContactDetailFragment.java
+++ b/src/com/android/contacts/detail/ContactDetailFragment.java
@@ -601,7 +601,7 @@
} else if (Phone.CONTENT_ITEM_TYPE.equals(mimeType) && hasData) {
// Build phone entries
String phoneNumberE164 =
- entryValues.getAsString(PhoneLookup.NORMALIZED_NUMBER);
+ entryValues.getAsString(Phone.NORMALIZED_NUMBER);
entry.data = PhoneNumberUtils.formatNumber(
entry.data, phoneNumberE164, mDefaultCountryIso);
final Intent phoneIntent = mHasPhone ?
diff --git a/tests/src/com/android/contacts/ContactDetailTest.java b/tests/src/com/android/contacts/ContactDetailTest.java
index e8d1550..b2c19f4 100644
--- a/tests/src/com/android/contacts/ContactDetailTest.java
+++ b/tests/src/com/android/contacts/ContactDetailTest.java
@@ -21,7 +21,9 @@
import com.android.contacts.tests.mocks.MockContentProvider;
import android.test.ActivityUnitTestCase;
+import android.test.suitebuilder.annotation.SmallTest;
+@SmallTest
public class ContactDetailTest extends ActivityUnitTestCase<ContactDetailActivity> {
private ContactsMockContext mContext;
private MockContentProvider mContactsProvider;
diff --git a/tests/src/com/android/contacts/ContactsUtilsTests.java b/tests/src/com/android/contacts/ContactsUtilsTests.java
index 82d0cb0..d2d5bbb 100644
--- a/tests/src/com/android/contacts/ContactsUtilsTests.java
+++ b/tests/src/com/android/contacts/ContactsUtilsTests.java
@@ -126,7 +126,7 @@
assertCollapses("72", true,
Phone.CONTENT_ITEM_TYPE, "+49 (8092) 1234",
Phone.CONTENT_ITEM_TYPE, "+49 (8092)1234");
- assertCollapses("73", true,
+ assertCollapses("73", false,
Phone.CONTENT_ITEM_TYPE, "0049 (8092) 1234",
Phone.CONTENT_ITEM_TYPE, "+49/80921234");
assertCollapses("74", false,
@@ -147,14 +147,37 @@
Phone.CONTENT_ITEM_TYPE, "1234567;+49 (8092) 1234",
Phone.CONTENT_ITEM_TYPE, "1234567;+49/80921234");
- // this makes sure that if if two segments are identical, we don't even try to parse
- // (and therefore allow invalid phone numbers)
- assertCollapses("84", true,
- Phone.CONTENT_ITEM_TYPE, "+49/80921234;a89",
- Phone.CONTENT_ITEM_TYPE, "+49 (8092) 1234;a89");
- assertCollapses("85", false,
- Phone.CONTENT_ITEM_TYPE, "+49/80921234;a89",
- Phone.CONTENT_ITEM_TYPE, "+49/80921234;b89");
+ assertCollapses("86", true,
+ Phone.CONTENT_ITEM_TYPE, "",
+ Phone.CONTENT_ITEM_TYPE, "");
+
+ assertCollapses("87", false,
+ Phone.CONTENT_ITEM_TYPE, "1",
+ Phone.CONTENT_ITEM_TYPE, "");
+
+ assertCollapses("88", false,
+ Phone.CONTENT_ITEM_TYPE, "",
+ Phone.CONTENT_ITEM_TYPE, "1");
+
+ assertCollapses("89", true,
+ Phone.CONTENT_ITEM_TYPE, "---",
+ Phone.CONTENT_ITEM_TYPE, "---");
+
+ assertCollapses("90", true,
+ Phone.CONTENT_ITEM_TYPE, "1-/().",
+ Phone.CONTENT_ITEM_TYPE, "--$%1");
+
+ assertCollapses("91", true,
+ Phone.CONTENT_ITEM_TYPE, "abcdefghijklmnopqrstuvwxyz",
+ Phone.CONTENT_ITEM_TYPE, "22233344455566677778889999");
+
+ assertCollapses("92", false,
+ Phone.CONTENT_ITEM_TYPE, "1;2",
+ Phone.CONTENT_ITEM_TYPE, "12");
+
+ assertCollapses("93", false,
+ Phone.CONTENT_ITEM_TYPE, "1,2",
+ Phone.CONTENT_ITEM_TYPE, "12");
}
private void assertCollapses(String message, boolean expected, CharSequence mimetype1,
@@ -164,12 +187,17 @@
assertEquals(message, expected,
ContactsUtils.shouldCollapse(mimetype2, data2, mimetype1, data1));
+ // If data1 and data2 are the same instance, make sure the same test passes with different
+ // instances.
if (data1 == data2 && data1 != null) {
- // make sure we also do a test where object equality is not given
- final CharSequence data2_newref = data2 + "";
+ // Create a different instance
+ final CharSequence data2_newref = new StringBuilder(data2).append("").toString();
- // this just makes sure the test is working
- assertFalse(data1 == data2_newref);
+ if (data1 == data2_newref) {
+ // In some cases no matter what we do the runtime reuses the same instance, so
+ // we can't do the "different instance" test.
+ return;
+ }
// we have two different instances, now make sure we get the same result as before
assertEquals(message, expected,