Use 3 bucket sorting logic for frequent.

Bug 6343819

Change-Id: Id2a2827fca611e62a5a1406faa73026625feaede
diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java
index 19e81a9..8079420 100644
--- a/src/com/android/providers/contacts/ContactsProvider2.java
+++ b/src/com/android/providers/contacts/ContactsProvider2.java
@@ -232,12 +232,6 @@
     private static final ProfileAwareUriMatcher sUriMatcher =
             new ProfileAwareUriMatcher(UriMatcher.NO_MATCH);
 
-    /**
-     * Used to insert a column into strequent results, which enables SQL to sort the list using
-     * the total times contacted. See also {@link #sStrequentFrequentProjectionMap}.
-     */
-    private static final String TIMES_USED_SORT_COLUMN = "times_used_sort";
-
     private static final String FREQUENT_ORDER_BY = DataUsageStatColumns.TIMES_USED + " DESC,"
             + Contacts.DISPLAY_NAME + " COLLATE LOCALIZED ASC";
 
@@ -490,14 +484,22 @@
             " WHERE " + RawContacts._ID + " IN (";
 
     // Current contacts - those contacted within the last 3 days (in seconds)
-    private static final long EMAIL_FILTER_CURRENT = 3 * 24 * 60 * 60;
+    private static final long LAST_TIME_USED_CURRENT_SEC = 3 * 24 * 60 * 60;
 
     // Recent contacts - those contacted within the last 30 days (in seconds)
-    private static final long EMAIL_FILTER_RECENT = 30 * 24 * 60 * 60;
+    private static final long LAST_TIME_USED_RECENT_SEC = 30 * 24 * 60 * 60;
 
-    private static final String TIME_SINCE_LAST_USED =
+    private static final String TIME_SINCE_LAST_USED_SEC =
             "(strftime('%s', 'now') - " + DataUsageStatColumns.LAST_TIME_USED + "/1000)";
 
+    private static final String SORT_BY_DATA_USAGE =
+            "(CASE WHEN " + TIME_SINCE_LAST_USED_SEC + " < " + LAST_TIME_USED_CURRENT_SEC +
+            " THEN 0 " +
+                    " WHEN " + TIME_SINCE_LAST_USED_SEC + " < " + LAST_TIME_USED_RECENT_SEC +
+            " THEN 1 " +
+            " ELSE 2 END), " +
+            DataUsageStatColumns.TIMES_USED + " DESC";
+
     /*
      * Sorting order for email address suggestions: first starred, then the rest.
      * Within the two groups:
@@ -509,12 +511,7 @@
      */
     private static final String EMAIL_FILTER_SORT_ORDER =
         Contacts.STARRED + " DESC, "
-        + "(CASE WHEN " + TIME_SINCE_LAST_USED + " < " + EMAIL_FILTER_CURRENT
-        + " THEN 0 "
-                + " WHEN " + TIME_SINCE_LAST_USED + " < " + EMAIL_FILTER_RECENT
-        + " THEN 1 "
-        + " ELSE 2 END), "
-        + DataUsageStatColumns.TIMES_USED + " DESC, "
+        + SORT_BY_DATA_USAGE + ", "
         + Contacts.IN_VISIBLE_GROUP + " DESC, "
         + Contacts.DISPLAY_NAME + ", "
         + Data.CONTACT_ID + ", "
@@ -688,12 +685,16 @@
     /** Used for pushing starred contacts to the top of a times contacted list **/
     private static final ProjectionMap sStrequentStarredProjectionMap = ProjectionMap.builder()
             .addAll(sContactsProjectionMap)
-            .add(TIMES_USED_SORT_COLUMN, String.valueOf(Long.MAX_VALUE))
+            .add(DataUsageStatColumns.TIMES_USED, String.valueOf(Long.MAX_VALUE))
+            .add(DataUsageStatColumns.LAST_TIME_USED, String.valueOf(Long.MAX_VALUE))
             .build();
 
     private static final ProjectionMap sStrequentFrequentProjectionMap = ProjectionMap.builder()
             .addAll(sContactsProjectionMap)
-            .add(TIMES_USED_SORT_COLUMN, "SUM(" + DataUsageStatColumns.CONCRETE_TIMES_USED + ")")
+            .add(DataUsageStatColumns.TIMES_USED,
+                    "SUM(" + DataUsageStatColumns.CONCRETE_TIMES_USED + ")")
+            .add(DataUsageStatColumns.LAST_TIME_USED,
+                    "MAX(" + DataUsageStatColumns.CONCRETE_LAST_TIME_USED + ")")
             .build();
 
     /**
@@ -704,7 +705,9 @@
     private static final ProjectionMap sStrequentPhoneOnlyStarredProjectionMap
             = ProjectionMap.builder()
             .addAll(sContactsProjectionMap)
-            .add(TIMES_USED_SORT_COLUMN, String.valueOf(Long.MAX_VALUE))
+            .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")
@@ -719,7 +722,8 @@
     private static final ProjectionMap sStrequentPhoneOnlyFrequentProjectionMap
             = ProjectionMap.builder()
             .addAll(sContactsProjectionMap)
-            .add(TIMES_USED_SORT_COLUMN, DataUsageStatColumns.CONCRETE_TIMES_USED)
+            .add(DataUsageStatColumns.TIMES_USED, DataUsageStatColumns.CONCRETE_TIMES_USED)
+            .add(DataUsageStatColumns.LAST_TIME_USED, DataUsageStatColumns.CONCRETE_LAST_TIME_USED)
             .add(Phone.NUMBER)
             .add(Phone.TYPE)
             .add(Phone.LABEL)
@@ -5065,7 +5069,10 @@
 
                 String[] subProjection = null;
                 if (projection != null) {
-                    subProjection = appendProjectionArg(projection, TIMES_USED_SORT_COLUMN);
+                    subProjection = new String[projection.length + 2];
+                    System.arraycopy(projection, 0, subProjection, 0, projection.length);
+                    subProjection[projection.length + 0] = DataUsageStatColumns.TIMES_USED;
+                    subProjection[projection.length + 1] = DataUsageStatColumns.LAST_TIME_USED;
                 }
 
                 // Build the first query for starred
@@ -5123,9 +5130,8 @@
                             DataColumns.MIMETYPE_ID + " IN (" +
                             phoneMimeTypeId + ", " + sipMimeTypeId + ")) AND (" +
                             RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY + ")"));
-                    frequentInnerQuery =
-                            qb.buildQuery(subProjection, null, null, null,
-                            TIMES_USED_SORT_COLUMN + " DESC", "25");
+                    frequentInnerQuery = qb.buildQuery(subProjection, null, null, null,
+                            SORT_BY_DATA_USAGE, "25");
                 } else {
                     setTablesAndProjectionMapForContacts(qb, uri, projection, true);
                     qb.setProjectionMap(sStrequentFrequentProjectionMap);
@@ -5137,7 +5143,7 @@
                     final String HAVING =
                             RawContacts.CONTACT_ID + " IN " + Tables.DEFAULT_DIRECTORY;
                     frequentInnerQuery = qb.buildQuery(subProjection,
-                            null, Contacts._ID, HAVING, null, "25");
+                            null, Contacts._ID, HAVING, SORT_BY_DATA_USAGE, "25");
                 }
 
                 // We need to wrap the inner queries in an extra select, because they contain
@@ -6733,8 +6739,8 @@
 
         qb.setTables(sb.toString());
 
-        boolean useDistinct = distinct
-                || !mDbHelper.get().isInProjection(projection, DISTINCT_DATA_PROHIBITING_COLUMNS);
+        boolean useDistinct = distinct || !ContactsDatabaseHelper.isInProjection(
+                projection, DISTINCT_DATA_PROHIBITING_COLUMNS);
         qb.setDistinct(useDistinct);
 
         final ProjectionMap projectionMap;
@@ -6798,7 +6804,7 @@
 
     private void appendContactStatusUpdateJoin(StringBuilder sb, String[] projection,
             String lastStatusUpdateIdColumn) {
-        if (mDbHelper.get().isInProjection(projection,
+        if (ContactsDatabaseHelper.isInProjection(projection,
                 Contacts.CONTACT_STATUS,
                 Contacts.CONTACT_STATUS_RES_PACKAGE,
                 Contacts.CONTACT_STATUS_ICON,
@@ -6813,7 +6819,7 @@
 
     private void appendDataStatusUpdateJoin(StringBuilder sb, String[] projection,
             String dataIdColumn) {
-        if (mDbHelper.get().isInProjection(projection,
+        if (ContactsDatabaseHelper.isInProjection(projection,
                 StatusUpdates.STATUS,
                 StatusUpdates.STATUS_RES_PACKAGE,
                 StatusUpdates.STATUS_ICON,
@@ -6833,7 +6839,7 @@
 
     private void appendContactPresenceJoin(StringBuilder sb, String[] projection,
             String contactIdColumn) {
-        if (mDbHelper.get().isInProjection(projection,
+        if (ContactsDatabaseHelper.isInProjection(projection,
                 Contacts.CONTACT_PRESENCE, Contacts.CONTACT_CHAT_CAPABILITY)) {
             sb.append(" LEFT OUTER JOIN " + Tables.AGGREGATED_PRESENCE +
                     " ON (" + contactIdColumn + " = "
@@ -6843,7 +6849,8 @@
 
     private void appendDataPresenceJoin(StringBuilder sb, String[] projection,
             String dataIdColumn) {
-        if (mDbHelper.get().isInProjection(projection, Data.PRESENCE, Data.CHAT_CAPABILITY)) {
+        if (ContactsDatabaseHelper.isInProjection(
+                projection, Data.PRESENCE, Data.CHAT_CAPABILITY)) {
             sb.append(" LEFT OUTER JOIN " + Tables.PRESENCE +
                     " ON (" + StatusUpdates.DATA_ID + "=" + dataIdColumn + ")");
         }
@@ -7779,17 +7786,6 @@
         }
     }
 
-    private String[] appendProjectionArg(String[] projection, String arg) {
-        if (projection == null) {
-            return null;
-        }
-        final int length = projection.length;
-        String[] newProjection = new String[length + 1];
-        System.arraycopy(projection, 0, newProjection, 0, length);
-        newProjection[length] = arg;
-        return newProjection;
-    }
-
     protected Account getDefaultAccount() {
         AccountManager accountManager = AccountManager.get(getContext());
         try {
diff --git a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
index f6770de..965b60c 100644
--- a/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
+++ b/tests/src/com/android/providers/contacts/ContactsProvider2Test.java
@@ -135,6 +135,83 @@
         });
     }
 
+    public void testContactsStrequentProjection() {
+        assertProjection(Contacts.CONTENT_STREQUENT_URI, new String[]{
+                Contacts._ID,
+                Contacts.DISPLAY_NAME_PRIMARY,
+                Contacts.DISPLAY_NAME_ALTERNATIVE,
+                Contacts.DISPLAY_NAME_SOURCE,
+                Contacts.PHONETIC_NAME,
+                Contacts.PHONETIC_NAME_STYLE,
+                Contacts.SORT_KEY_PRIMARY,
+                Contacts.SORT_KEY_ALTERNATIVE,
+                Contacts.LAST_TIME_CONTACTED,
+                Contacts.TIMES_CONTACTED,
+                Contacts.STARRED,
+                Contacts.IN_VISIBLE_GROUP,
+                Contacts.PHOTO_ID,
+                Contacts.PHOTO_FILE_ID,
+                Contacts.PHOTO_URI,
+                Contacts.PHOTO_THUMBNAIL_URI,
+                Contacts.CUSTOM_RINGTONE,
+                Contacts.HAS_PHONE_NUMBER,
+                Contacts.SEND_TO_VOICEMAIL,
+                Contacts.IS_USER_PROFILE,
+                Contacts.LOOKUP_KEY,
+                Contacts.NAME_RAW_CONTACT_ID,
+                Contacts.CONTACT_PRESENCE,
+                Contacts.CONTACT_CHAT_CAPABILITY,
+                Contacts.CONTACT_STATUS,
+                Contacts.CONTACT_STATUS_TIMESTAMP,
+                Contacts.CONTACT_STATUS_RES_PACKAGE,
+                Contacts.CONTACT_STATUS_LABEL,
+                Contacts.CONTACT_STATUS_ICON,
+                DataUsageStatColumns.TIMES_USED,
+                DataUsageStatColumns.LAST_TIME_USED,
+        });
+    }
+
+    public void testContactsStrequentPhoneOnlyProjection() {
+        assertProjection(Contacts.CONTENT_STREQUENT_URI.buildUpon()
+                    .appendQueryParameter(ContactsContract.STREQUENT_PHONE_ONLY, "true").build(),
+                new String[] {
+                Contacts._ID,
+                Contacts.DISPLAY_NAME_PRIMARY,
+                Contacts.DISPLAY_NAME_ALTERNATIVE,
+                Contacts.DISPLAY_NAME_SOURCE,
+                Contacts.PHONETIC_NAME,
+                Contacts.PHONETIC_NAME_STYLE,
+                Contacts.SORT_KEY_PRIMARY,
+                Contacts.SORT_KEY_ALTERNATIVE,
+                Contacts.LAST_TIME_CONTACTED,
+                Contacts.TIMES_CONTACTED,
+                Contacts.STARRED,
+                Contacts.IN_VISIBLE_GROUP,
+                Contacts.PHOTO_ID,
+                Contacts.PHOTO_FILE_ID,
+                Contacts.PHOTO_URI,
+                Contacts.PHOTO_THUMBNAIL_URI,
+                Contacts.CUSTOM_RINGTONE,
+                Contacts.HAS_PHONE_NUMBER,
+                Contacts.SEND_TO_VOICEMAIL,
+                Contacts.IS_USER_PROFILE,
+                Contacts.LOOKUP_KEY,
+                Contacts.NAME_RAW_CONTACT_ID,
+                Contacts.CONTACT_PRESENCE,
+                Contacts.CONTACT_CHAT_CAPABILITY,
+                Contacts.CONTACT_STATUS,
+                Contacts.CONTACT_STATUS_TIMESTAMP,
+                Contacts.CONTACT_STATUS_RES_PACKAGE,
+                Contacts.CONTACT_STATUS_LABEL,
+                Contacts.CONTACT_STATUS_ICON,
+                DataUsageStatColumns.TIMES_USED,
+                DataUsageStatColumns.LAST_TIME_USED,
+                Phone.NUMBER,
+                Phone.TYPE,
+                Phone.LABEL,
+        });
+    }
+
     public void testContactsWithSnippetProjection() {
         assertProjection(Contacts.CONTENT_FILTER_URI.buildUpon().appendPath("nothing").build(),
             new String[]{
@@ -1855,6 +1932,93 @@
         assertStoredValues(filterUri, values4);
     }
 
+    public void testQueryContactStrequentFrequentOrder() {
+        // Prepare test data
+        final long rid1 = createRawContact();
+        final long did1 = ContentUris.parseId(insertPhoneNumber(rid1, "1"));
+        final long did1e = ContentUris.parseId(insertEmail(rid1, "1@email.com"));
+
+        final long rid2 = createRawContact();
+        final long did2 = ContentUris.parseId(insertPhoneNumber(rid2, "2"));
+
+        final long rid3 = createRawContact();
+        final long did3 = ContentUris.parseId(insertPhoneNumber(rid3, "3"));
+
+        final long rid4 = createRawContact();
+        final long did4 = ContentUris.parseId(insertPhoneNumber(rid4, "4"));
+
+        final long rid5 = createRawContact();
+        final long did5 = ContentUris.parseId(insertPhoneNumber(rid5, "5"));
+
+        final long rid6 = createRawContact();
+        final long did6 = ContentUris.parseId(insertPhoneNumber(rid6, "6"));
+
+        final long cid1 = queryContactId(rid1);
+        final long cid2 = queryContactId(rid2);
+        final long cid3 = queryContactId(rid3);
+        final long cid4 = queryContactId(rid4);
+        final long cid5 = queryContactId(rid5);
+        final long cid6 = queryContactId(rid6);
+
+        // Make sure they aren't aggregated.
+        EvenMoreAsserts.assertUnique(cid1, cid2, cid3, cid4, cid5, cid6);
+
+        // Prepare the clock
+        sMockClock.install();
+
+        // We check the timestamp in SQL, which doesn't know about the MockClock.  So we need to
+        // use the  actual (roughly) time.
+
+        final long nowInMillis = System.currentTimeMillis();
+        final long yesterdayInMillis = (nowInMillis - 24 * 60 * 60 * 1000);
+        final long sevenDaysAgoInMillis = (nowInMillis - 7 * 24 * 60 * 60 * 1000);
+        final long oneYearAgoInMillis = (nowInMillis - 365L * 24 * 60 * 60 * 1000);
+
+        // A year ago...
+        sMockClock.setCurrentTimeMillis(oneYearAgoInMillis);
+
+        updateDataUsageFeedback(DataUsageFeedback.USAGE_TYPE_CALL, did1, did2);
+        updateDataUsageFeedback(DataUsageFeedback.USAGE_TYPE_CALL, did1);
+
+        // 7 days ago...
+        sMockClock.setCurrentTimeMillis(sevenDaysAgoInMillis);
+
+        updateDataUsageFeedback(DataUsageFeedback.USAGE_TYPE_CALL, did3, did4);
+        updateDataUsageFeedback(DataUsageFeedback.USAGE_TYPE_CALL, did3);
+
+        // Yesterday...
+        sMockClock.setCurrentTimeMillis(yesterdayInMillis);
+
+        updateDataUsageFeedback(DataUsageFeedback.USAGE_TYPE_CALL, did5, did6);
+        updateDataUsageFeedback(DataUsageFeedback.USAGE_TYPE_CALL, did5);
+
+        // Contact cid1 again, but it's an email, not a phone call.
+        updateDataUsageFeedback(DataUsageFeedback.USAGE_TYPE_LONG_TEXT, did1e);
+
+        // Check the order -- The regular frequent, which is contact based.
+        // Note because we contacted cid1 yesterday, it's been contacted 3 times, so it comes
+        // first.
+        assertStoredValuesOrderly(Contacts.CONTENT_STREQUENT_URI,
+                cv(Contacts._ID, cid1),
+                cv(Contacts._ID, cid5),
+                cv(Contacts._ID, cid6),
+                cv(Contacts._ID, cid3),
+                cv(Contacts._ID, cid4),
+                cv(Contacts._ID, cid2));
+
+        // Check the order -- phone only frequent, which is data based.
+        // Note this is based on data, and only looks at phone numbers, so the order is different
+        // now.
+        assertStoredValuesOrderly(Contacts.CONTENT_STREQUENT_URI.buildUpon()
+                    .appendQueryParameter(ContactsContract.STREQUENT_PHONE_ONLY, "1").build(),
+                cv(Data._ID, did5),
+                cv(Data._ID, did6),
+                cv(Data._ID, did3),
+                cv(Data._ID, did4),
+                cv(Data._ID, did1),
+                cv(Data._ID, did2));
+    }
+
     /**
      * Checks ContactsProvider2 works well with frequent Uri. The provider should return frequently
      * contacted person ordered by number of times contacted.
diff --git a/tests/src/com/android/providers/contacts/EvenMoreAsserts.java b/tests/src/com/android/providers/contacts/EvenMoreAsserts.java
index 509a7fe..c73128a 100644
--- a/tests/src/com/android/providers/contacts/EvenMoreAsserts.java
+++ b/tests/src/com/android/providers/contacts/EvenMoreAsserts.java
@@ -16,6 +16,8 @@
 
 package com.android.providers.contacts;
 
+import com.google.android.collect.Sets;
+
 import android.content.Context;
 import android.graphics.BitmapFactory;
 
@@ -23,6 +25,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Arrays;
+import java.util.Set;
 
 import junit.framework.Assert;
 
@@ -102,4 +105,12 @@
 
         return "[" + o.outWidth + " x " + o.outHeight + "]";
     }
+
+    public static void assertUnique(Object... values) {
+        Set<Object> set = Sets.newHashSet();
+        for (Object o : values) {
+            Assert.assertFalse("Duplicate found: " + o, set.contains(o));
+            set.add(o);
+        }
+    }
 }
diff --git a/tests/src/com/android/providers/contacts/util/MockClock.java b/tests/src/com/android/providers/contacts/util/MockClock.java
index 3e84a09..dce06c9 100644
--- a/tests/src/com/android/providers/contacts/util/MockClock.java
+++ b/tests/src/com/android/providers/contacts/util/MockClock.java
@@ -18,12 +18,12 @@
 
 public class MockClock extends Clock {
     /** Current time.  Only updated with advance(). */
-    private long currentTimeMillis;
+    private long mCurrentTimeMillis;
 
     public void install() {
         Clock.injectInstance(this);
 
-        currentTimeMillis = 100000;
+        mCurrentTimeMillis = 100000;
     }
 
     public void uninstall() {
@@ -32,10 +32,14 @@
 
     @Override
     public long currentTimeMillis() {
-        return currentTimeMillis;
+        return mCurrentTimeMillis;
+    }
+
+    public void setCurrentTimeMillis(long time) {
+        mCurrentTimeMillis = time;
     }
 
     public void advance() {
-        currentTimeMillis++;
+        mCurrentTimeMillis++;
     }
 }