Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ffa8963f authored by Flavio Lerda's avatar Flavio Lerda
Browse files

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
parent cd7550b2

src/com/android/contacts/calllog/CallLogGroupBuilder.java

+40 −4
Original line number Diff line number Diff line
@@ -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 @@ public class CallLogGroupBuilder { 
        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 @@ public class CallLogGroupBuilder { 
        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);

    }

}

src/com/android/contacts/calllog/ContactInfoHelper.java

+4 −0
Original line number Diff line number Diff line
@@ -114,6 +114,10 @@ public class ContactInfoHelper { 
        // 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.

src/com/android/contacts/format/FormatUtils.java

+2 −3
Original line number Diff line number Diff line
@@ -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 @@ public class FormatUtils { 
        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()) {

tests/src/com/android/contacts/calllog/CallLogGroupBuilderTest.java

+78 −0
Original line number Diff line number Diff line
@@ -18,6 +18,9 @@ package com.android.contacts.calllog; 


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 @@ public class CallLogGroupBuilderTest extends AndroidTestCase { 
        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);