Correctly match SIP addresses.
When grouping items in the call log, we were using the function to
compare phone numbers. However, this strips all non-numeric characters,
which means that all SIP addresses without a number in them will match.
Instead, SIP addresses are defined to match only if they are identical.
Bug: 5483719
Bug: 5390325
Change-Id: Ic6f1d55ccbd433cc6062ca803fcfd88ae4f68a8a
diff --git a/src/com/android/contacts/calllog/CallLogGroupBuilder.java b/src/com/android/contacts/calllog/CallLogGroupBuilder.java
index a7f8fa3..b636842 100644
--- a/src/com/android/contacts/calllog/CallLogGroupBuilder.java
+++ b/src/com/android/contacts/calllog/CallLogGroupBuilder.java
@@ -17,6 +17,7 @@
package com.android.contacts.calllog;
import com.android.common.widget.GroupingListAdapter;
+import com.google.common.annotations.VisibleForTesting;
import android.database.CharArrayBuffer;
import android.database.Cursor;
@@ -74,7 +75,7 @@
while (cursor.moveToNext()) {
cursor.copyStringToBuffer(CallLogQuery.NUMBER, currentNumber);
final int callType = cursor.getInt(CallLogQuery.CALL_TYPE);
- final boolean sameNumber = equalPhoneNumbers(firstNumber, currentNumber);
+ final boolean sameNumber = equalNumbers(firstNumber, currentNumber);
final boolean shouldGroup;
if (CallLogQuery.isSectionHeader(cursor)) {
@@ -129,10 +130,45 @@
mGroupCreator.addGroup(cursorPosition, size, false);
}
- private boolean equalPhoneNumbers(CharArrayBuffer buffer1, CharArrayBuffer buffer2) {
+ @VisibleForTesting
+ boolean equalNumbers(CharArrayBuffer buffer1, CharArrayBuffer buffer2) {
// TODO add PhoneNumberUtils.compare(CharSequence, CharSequence) to avoid
// string allocation
- return PhoneNumberUtils.compare(new String(buffer1.data, 0, buffer1.sizeCopied),
- new String(buffer2.data, 0, buffer2.sizeCopied));
+ String number1 = buffer1 == null ? null : new String(buffer1.data, 0, buffer1.sizeCopied);
+ String number2 = buffer2 == null ? null : new String(buffer2.data, 0, buffer2.sizeCopied);
+ if (PhoneNumberUtils.isUriNumber(number1) || PhoneNumberUtils.isUriNumber(number2)) {
+ return compareSipAddresses(number1, number2);
+ } else {
+ return PhoneNumberUtils.compare(number1, number2);
+ }
+ }
+
+ @VisibleForTesting
+ boolean compareSipAddresses(String number1, String number2) {
+ if (number1 == null || number2 == null) return number1 == number2;
+
+ int index1 = number1.indexOf('@');
+ final String userinfo1;
+ final String rest1;
+ if (index1 != -1) {
+ userinfo1 = number1.substring(0, index1);
+ rest1 = number1.substring(index1);
+ } else {
+ userinfo1 = number1;
+ rest1 = "";
+ }
+
+ int index2 = number2.indexOf('@');
+ final String userinfo2;
+ final String rest2;
+ if (index2 != -1) {
+ userinfo2 = number2.substring(0, index2);
+ rest2 = number2.substring(index2);
+ } else {
+ userinfo2 = number2;
+ rest2 = "";
+ }
+
+ return userinfo1.equals(userinfo2) && rest1.equalsIgnoreCase(rest2);
}
}
diff --git a/src/com/android/contacts/calllog/ContactInfoHelper.java b/src/com/android/contacts/calllog/ContactInfoHelper.java
index c837c9a..7d39bb1 100644
--- a/src/com/android/contacts/calllog/ContactInfoHelper.java
+++ b/src/com/android/contacts/calllog/ContactInfoHelper.java
@@ -114,6 +114,10 @@
// uppercase the incoming SIP address, in order to do a
// case-insensitive match.
//
+ // TODO: SIP URIs are defined as being case sensitive for the user part (before the '@')
+ // and case insensitive everywhere else. We should change the code to handle this
+ // accordingly.
+ //
// TODO: May also need to normalize by adding "sip:" as a
// prefix, if we start storing SIP addresses that way in the
// database.
diff --git a/src/com/android/contacts/format/FormatUtils.java b/src/com/android/contacts/format/FormatUtils.java
index cb4dc2d..8e2bb63 100644
--- a/src/com/android/contacts/format/FormatUtils.java
+++ b/src/com/android/contacts/format/FormatUtils.java
@@ -16,7 +16,6 @@
package com.android.contacts.format;
import com.android.contacts.test.NeededForTesting;
-import com.google.common.annotations.VisibleForTesting;
import android.database.CharArrayBuffer;
import android.graphics.Typeface;
@@ -110,8 +109,8 @@
return text;
}
- @VisibleForTesting
- /*package*/ static void copyToCharArrayBuffer(String text, CharArrayBuffer buffer) {
+ @NeededForTesting
+ public static void copyToCharArrayBuffer(String text, CharArrayBuffer buffer) {
if (text != null) {
char[] data = buffer.data;
if (data == null || data.length < text.length()) {
diff --git a/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java b/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java
index 31ad548..2e029cc 100644
--- a/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java
+++ b/tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java
@@ -18,6 +18,9 @@
import static com.google.android.collect.Lists.newArrayList;
+import com.android.contacts.format.FormatUtils;
+
+import android.database.CharArrayBuffer;
import android.database.MatrixCursor;
import android.provider.CallLog.Calls;
import android.test.AndroidTestCase;
@@ -167,6 +170,81 @@
assertGroupIs(7, 3, false, mFakeGroupCreator.groups.get(2));
}
+ public void testEqualPhoneNumbers() {
+ // Identical.
+ assertTrue(checkEqualNumbers("6505555555", "6505555555"));
+ assertTrue(checkEqualNumbers("650 555 5555", "650 555 5555"));
+ // Formatting.
+ assertTrue(checkEqualNumbers("6505555555", "650 555 5555"));
+ assertTrue(checkEqualNumbers("6505555555", "(650) 555-5555"));
+ assertTrue(checkEqualNumbers("650 555 5555", "(650) 555-5555"));
+ // Short codes.
+ assertTrue(checkEqualNumbers("55555", "55555"));
+ assertTrue(checkEqualNumbers("55555", "555 55"));
+ // Different numbers.
+ assertFalse(checkEqualNumbers("6505555555", "650555555"));
+ assertFalse(checkEqualNumbers("6505555555", "6505555551"));
+ assertFalse(checkEqualNumbers("650 555 5555", "650 555 555"));
+ assertFalse(checkEqualNumbers("650 555 5555", "650 555 5551"));
+ assertFalse(checkEqualNumbers("55555", "5555"));
+ assertFalse(checkEqualNumbers("55555", "55551"));
+ // SIP addresses.
+ assertTrue(checkEqualNumbers("6505555555@host.com", "6505555555@host.com"));
+ assertTrue(checkEqualNumbers("6505555555@host.com", "6505555555@HOST.COM"));
+ assertTrue(checkEqualNumbers("user@host.com", "user@host.com"));
+ assertTrue(checkEqualNumbers("user@host.com", "user@HOST.COM"));
+ assertFalse(checkEqualNumbers("USER@host.com", "user@host.com"));
+ assertFalse(checkEqualNumbers("user@host.com", "user@host1.com"));
+ // SIP address vs phone number.
+ assertFalse(checkEqualNumbers("6505555555@host.com", "6505555555"));
+ assertFalse(checkEqualNumbers("6505555555", "6505555555@host.com"));
+ assertFalse(checkEqualNumbers("user@host.com", "6505555555"));
+ assertFalse(checkEqualNumbers("6505555555", "user@host.com"));
+ // Nulls.
+ assertTrue(checkEqualNumbers(null, null));
+ assertFalse(checkEqualNumbers(null, "6505555555"));
+ assertFalse(checkEqualNumbers("6505555555", null));
+ assertFalse(checkEqualNumbers(null, "6505555555@host.com"));
+ assertFalse(checkEqualNumbers("6505555555@host.com", null));
+ }
+
+ public void testCompareSipAddresses() {
+ // Identical.
+ assertTrue(mBuilder.compareSipAddresses("6505555555@host.com", "6505555555@host.com"));
+ assertTrue(mBuilder.compareSipAddresses("user@host.com", "user@host.com"));
+ // Host is case insensitive.
+ assertTrue(mBuilder.compareSipAddresses("6505555555@host.com", "6505555555@HOST.COM"));
+ assertTrue(mBuilder.compareSipAddresses("user@host.com", "user@HOST.COM"));
+ // Userinfo is case sensitive.
+ assertFalse(mBuilder.compareSipAddresses("USER@host.com", "user@host.com"));
+ // Different hosts.
+ assertFalse(mBuilder.compareSipAddresses("user@host.com", "user@host1.com"));
+ // Different users.
+ assertFalse(mBuilder.compareSipAddresses("user1@host.com", "user@host.com"));
+ // Nulls.
+ assertTrue(mBuilder.compareSipAddresses(null, null));
+ assertFalse(mBuilder.compareSipAddresses(null, "6505555555@host.com"));
+ assertFalse(mBuilder.compareSipAddresses("6505555555@host.com", null));
+ }
+
+ /** Calls {@link CallLogGroupBuilder#equalNumbers(CharArrayBuffer, CharArrayBuffer)}. */
+ private boolean checkEqualNumbers(String number1, String number2) {
+ final CharArrayBuffer buffer1 = stringToCharArrayBuffer(number1);
+ final CharArrayBuffer buffer2 = stringToCharArrayBuffer(number2);
+ return mBuilder.equalNumbers(buffer1, buffer2);
+ }
+
+ /** Converts a string into a {@link CharArrayBuffer}, preserving null values. */
+ private CharArrayBuffer stringToCharArrayBuffer(String value) {
+ if (value == null) {
+ return null;
+ }
+
+ final CharArrayBuffer buffer = new CharArrayBuffer(value.length());
+ FormatUtils.copyToCharArrayBuffer(value, buffer);
+ return buffer;
+ }
+
/** Creates (or recreates) the cursor used to store the call log content for the tests. */
private void createCursor() {
mCursor = new MatrixCursor(CallLogQuery.EXTENDED_PROJECTION);