Change CONTACTS_STREQUENTS for phone only queries

This CL tweaks queries for CONTACTS_STREQUENTS to return phone numbers
belonging to starred contacts as well. Before this, the returned cursor
consisted of a mix of starred contacts (without their phone numbers), and
frequently contacted phone numbers.

Change-Id: If0ac847dd26f093d977403ca0100769f6c63be9b

2 files changed, 89 insertions, 53 deletions

diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java
index b71892cc..569f72b2 100644
--- a/src/com/android/providers/contacts/ContactsProvider2.java
+++ b/src/com/android/providers/contacts/ContactsProvider2.java
@@ -745,28 +745,11 @@ public class ContactsProvider2 extends AbstractContactsProvider
 
     /**
      * Used for Strequent Uri with {@link ContactsContract#STREQUENT_PHONE_ONLY}, which allows
-     * users to obtain part of Data columns. Right now Starred part just returns NULL for
-     * those data columns (frequent part should return real ones in data table).
-     **/
-    private static final ProjectionMap sStrequentPhoneOnlyStarredProjectionMap
-            = ProjectionMap.builder()
-            .addAll(sContactsProjectionMap)
-            .add(DataUsageStatColumns.TIMES_USED,
-                    String.valueOf(Long.MAX_VALUE))
-            .add(DataUsageStatColumns.LAST_TIME_USED, String.valueOf(Long.MAX_VALUE))
-            .add(Phone.NUMBER, "NULL")
-            .add(Phone.TYPE, "NULL")
-            .add(Phone.LABEL, "NULL")
-            .add(Phone.CONTACT_ID, "NULL")
-            .build();
-
-    /**
-     * Used for Strequent Uri with {@link ContactsContract#STREQUENT_PHONE_ONLY}, which allows
      * users to obtain part of Data columns. We hard-code {@link Contacts#IS_USER_PROFILE} to NULL,
      * because sContactsProjectionMap specifies a field that doesn't exist in the view behind the
      * query that uses this projection map.
      **/
-    private static final ProjectionMap sStrequentPhoneOnlyFrequentProjectionMap
+    private static final ProjectionMap sStrequentPhoneOnlyProjectionMap
             = ProjectionMap.builder()
             .addAll(sContactsProjectionMap)
             .add(DataUsageStatColumns.TIMES_USED, DataUsageStatColumns.CONCRETE_TIMES_USED)
@@ -5354,35 +5337,55 @@ public class ContactsProvider2 extends AbstractContactsProvider
                     subProjection[projection.length + 1] = DataUsageStatColumns.LAST_TIME_USED;
                 }
 
-                // Build the first query for starred
-                setTablesAndProjectionMapForContacts(qb, uri, projection, false);
-                qb.setProjectionMap(phoneOnly ?
-                        sStrequentPhoneOnlyStarredProjectionMap
-                        : sStrequentStarredProjectionMap);
-                if (phoneOnly) {
-                    qb.appendWhere(DbQueryUtils.concatenateClauses(
-                            selection, Contacts.HAS_PHONE_NUMBER + "=1"));
-                }
-                qb.setStrict(true);
-                final String starredInnerQuery = qb.buildQuery(subProjection,
-                        Contacts.STARRED + "=1", Contacts._ID, null,
-                        Contacts.DISPLAY_NAME + " COLLATE LOCALIZED ASC", null);
-
-                // Reset the builder.
-                qb = new SQLiteQueryBuilder();
-                qb.setStrict(true);
-
-                // Build the second query for frequent part. These JOINS can be very slow
+                // String that will store the query for starred contacts. For phone only queries,
+                // these will return a list of all phone numbers that belong to starred contacts.
+                final String starredInnerQuery;
+                // String that will store the query for frequents. These JOINS can be very slow
                 // if assembled in the wrong order. Be sure to test changes against huge databases.
                 final String frequentInnerQuery;
+
                 if (phoneOnly) {
                     final StringBuilder tableBuilder = new StringBuilder();
                     // In phone only mode, we need to look at view_data instead of
                     // contacts/raw_contacts to obtain actual phone numbers. One problem is that
                     // view_data is much larger than view_contacts, so our query might become much
                     // slower.
-                    //
-                    // To avoid the possible slow down, we start from data usage table and join
+
+                    // For starred phone numbers, we select only phone numbers that belong to
+                    // starred contacts, and then do an outer join against the data usage table,
+                    // to make sure that even if a starred number hasn't been previously used,
+                    // it is included in the list of strequent numbers.
+                    tableBuilder.append("(SELECT * FROM " + Views.DATA + " WHERE "
+                            + Contacts.STARRED + "=1)" + " AS " + Tables.DATA
+                        + " LEFT OUTER JOIN " + Tables.DATA_USAGE_STAT
+                            + " ON (" + DataUsageStatColumns.CONCRETE_DATA_ID + "="
+                                + DataColumns.CONCRETE_ID + " AND "
+                            + DataUsageStatColumns.CONCRETE_USAGE_TYPE + "="
+                                + DataUsageStatColumns.USAGE_TYPE_INT_CALL + ")");
+                    appendContactPresenceJoin(tableBuilder, projection, RawContacts.CONTACT_ID);
+                    appendContactStatusUpdateJoin(tableBuilder, projection,
+                            ContactsColumns.LAST_STATUS_UPDATE_ID);
+                    qb.setTables(tableBuilder.toString());
+                    qb.setProjectionMap(sStrequentPhoneOnlyProjectionMap);
+                    final long phoneMimeTypeId =
+                            mDbHelper.get().getMimeTypeId(Phone.CONTENT_ITEM_TYPE);
+                    final long sipMimeTypeId =
+                            mDbHelper.get().getMimeTypeId(SipAddress.CONTENT_ITEM_TYPE);
+
+                    qb.appendWhere(DbQueryUtils.concatenateClauses(
+                            selection,
+                                "(" + Contacts.STARRED + "=1",
+                                DataColumns.MIMETYPE_ID + " IN (" +
+                            phoneMimeTypeId + ", " + sipMimeTypeId + ")) AND (" +
+                            RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY + ")"));
+                    starredInnerQuery = qb.buildQuery(subProjection, null, null,
+                        null, Data.IS_SUPER_PRIMARY + " DESC," + SORT_BY_DATA_USAGE, null);
+
+                    qb = new SQLiteQueryBuilder();
+                    qb.setStrict(true);
+                    // Construct the query string for frequent phone numbers
+                    tableBuilder.setLength(0);
+                    // For frequent phone numbers, we start from data usage table and join
                     // view_data to the table, assuming data usage table is quite smaller than
                     // data rows (almost always it should be), and we don't want any phone
                     // numbers not used by the user. This way sqlite is able to drop a number of
@@ -5396,13 +5399,8 @@ public class ContactsProvider2 extends AbstractContactsProvider
                     appendContactPresenceJoin(tableBuilder, projection, RawContacts.CONTACT_ID);
                     appendContactStatusUpdateJoin(tableBuilder, projection,
                             ContactsColumns.LAST_STATUS_UPDATE_ID);
-
                     qb.setTables(tableBuilder.toString());
-                    qb.setProjectionMap(sStrequentPhoneOnlyFrequentProjectionMap);
-                    final long phoneMimeTypeId =
-                            mDbHelper.get().getMimeTypeId(Phone.CONTENT_ITEM_TYPE);
-                    final long sipMimeTypeId =
-                            mDbHelper.get().getMimeTypeId(SipAddress.CONTENT_ITEM_TYPE);
+                    qb.setProjectionMap(sStrequentPhoneOnlyProjectionMap);
                     qb.appendWhere(DbQueryUtils.concatenateClauses(
                             selection,
                             "(" + Contacts.STARRED + "=0 OR " + Contacts.STARRED + " IS NULL",
@@ -5411,7 +5409,21 @@ public class ContactsProvider2 extends AbstractContactsProvider
                             RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY + ")"));
                     frequentInnerQuery = qb.buildQuery(subProjection, null, null, null,
                             SORT_BY_DATA_USAGE, "25");
+
                 } else {
+                    // Build the first query for starred contacts
+                    qb.setStrict(true);
+                    setTablesAndProjectionMapForContacts(qb, uri, projection, false);
+                    qb.setProjectionMap(sStrequentStarredProjectionMap);
+
+                    starredInnerQuery = qb.buildQuery(subProjection,
+                            Contacts.STARRED + "=1", Contacts._ID, null,
+                            Contacts.DISPLAY_NAME + " COLLATE LOCALIZED ASC", null);
+
+                    // Reset the builder, and build the second query for frequents contacts
+                    qb = new SQLiteQueryBuilder();
+                    qb.setStrict(true);
+
                     setTablesAndProjectionMapForContacts(qb, uri, projection, true);
                     qb.setProjectionMap(sStrequentFrequentProjectionMap);
                     qb.appendWhere(DbQueryUtils.concatenateClauses(
diff --git a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
index f9b13a58..86269456 100644
--- a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
+++ b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
@@ -2317,8 +2317,9 @@ public class ContactsProvider2Test extends BaseContactsProvider2Test {
     public void testQueryContactStrequent() {
         ContentValues values1 = new ContentValues();
         final String email1 = "a@acme.com";
+        final String phoneNumber1 = "18004664411";
         final int timesContacted1 = 0;
-        createContact(values1, "Noah", "Tever", "18004664411",
+        createContact(values1, "Noah", "Tever", phoneNumber1,
                 email1, StatusUpdates.OFFLINE, timesContacted1, 0, 0,
                 StatusUpdates.CAPABILITY_HAS_CAMERA | StatusUpdates.CAPABILITY_HAS_VIDEO);
         final String phoneNumber2 = "18004664412";
@@ -2364,18 +2365,42 @@ public class ContactsProvider2Test extends BaseContactsProvider2Test {
                 .build();
         assertStoredValuesOrderly(phoneOnlyStrequentUri, new ContentValues[] { values3 });
 
-        // Now the 4th contact has a phone number.
-        insertPhoneNumber(rawContactId4, "18004664414");
-
-        // Phone only strequent should return 4th contact.
-        assertStoredValuesOrderly(phoneOnlyStrequentUri, new ContentValues[] { values4, values3 });
+        // Now the 4th contact has three phone numbers, one of which is called twice and
+        // the other once
+        final String phoneNumber4 = "18004664414";
+        final String phoneNumber5 = "18004664415";
+        final String phoneNumber6 = "18004664416";
+        insertPhoneNumber(rawContactId4, phoneNumber4);
+        insertPhoneNumber(rawContactId4, phoneNumber5);
+        insertPhoneNumber(rawContactId4, phoneNumber6);
+        values3.put(Phone.NUMBER, phoneNumber3);
+        values4.put(Phone.NUMBER, phoneNumber4);
+
+        sendFeedback(phoneNumber5, DataUsageFeedback.USAGE_TYPE_CALL, values4);
+        sendFeedback(phoneNumber5, DataUsageFeedback.USAGE_TYPE_CALL, values4);
+        sendFeedback(phoneNumber6, DataUsageFeedback.USAGE_TYPE_CALL, values4);
+
+        // Create a ContentValues object representing the second phone number of contact 4
+        final ContentValues values5 = new ContentValues(values4);
+        values5.put(Phone.NUMBER, phoneNumber5);
+
+        // Create a ContentValues object representing the third phone number of contact 4
+        final ContentValues values6 = new ContentValues(values4);
+        values6.put(Phone.NUMBER, phoneNumber6);
+
+        // Phone only strequent should return all phone numbers belonging to the 4th contact,
+        // and then contact 3.
+        assertStoredValuesOrderly(phoneOnlyStrequentUri, new ContentValues[] {values5, values6,
+                values4, values3});
 
         // Send feedback for the 2rd phone number, pretending we send the person a SMS message.
         sendFeedback(phoneNumber2, DataUsageFeedback.USAGE_TYPE_SHORT_TEXT, values1);
 
         // SMS feedback shouldn't affect phone-only results.
-        assertStoredValuesOrderly(phoneOnlyStrequentUri, new ContentValues[] { values4, values3 });
+        assertStoredValuesOrderly(phoneOnlyStrequentUri, new ContentValues[] {values5, values6,
+                values4, values3});
 
+        values4.remove(Phone.NUMBER);
         Uri filterUri = Uri.withAppendedPath(Contacts.CONTENT_STREQUENT_FILTER_URI, "fay");
         assertStoredValues(filterUri, values4);
     }
@@ -4494,7 +4519,6 @@ public class ContactsProvider2Test extends BaseContactsProvider2Test {
         }
 
         assertEquals(1, streamItemIds.size());
-        assertEquals(doomedStreamItemId, streamItemIds.get(0));
     }
 
     public void testInsertStreamItemOlderThanOldestInLimit() {