Skip to content

Commit

Permalink
Make strequent-phone-only returns what we want
Browse files Browse the repository at this point in the history 
- in starred section, just starred persons should be listed.
  One person should not appear more than once.
- in freuqent section, we should be able to obtain phone numbers
  One person can appear multiple times with different numbers.

To achieve it, we need to tweak results further. Instead of
allowing users to have Contacts columns, we have to allow part of
Data columns (DATA1, DATA2, DATA3). Those Data-columns will be NULL
in the starred section right now.

In order to have those additional columns in Contacts Uri (this is
for Contacts.CONTENT_STREQUENT_URI, not Data), this change has
one restriction: with phone_only flag, users cannot have
IS_USER_PROFILE any more. That's not so critical as phone_only
is not part of public API.

Modify unit test for phone-only results (SMS feedback shouldn't
affect phone-only results). Also introduce comvenient function
for testing feedback stuff in general.

FUTURE TODO:
- We should also be able to obtain a phone number relevant to
  the starred person while this change doesn't support it.
- We may need to group numbers by contact instead of showing
  them apart  (bug: 5059874)

Bug: 5050181
Change-Id: I47f532e5b7bb3f8bfd77215c61abb31a09d7fd51
  • Loading branch information
Daisuke Miyakawa committed Jul 21, 2011
1 parent 72836d9 commit 4928b8c
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 70 deletions.
117 changes: 86 additions & 31 deletions src/com/android/providers/contacts/ContactsProvider2.java
View file
Expand Up @@ -110,6 +110,7 @@
import android.provider.ContactsContract.CommonDataKinds.Organization;
import android.provider.ContactsContract.CommonDataKinds.Phone;
import android.provider.ContactsContract.CommonDataKinds.Photo;
import android.provider.ContactsContract.CommonDataKinds.SipAddress;
import android.provider.ContactsContract.CommonDataKinds.StructuredName;
import android.provider.ContactsContract.CommonDataKinds.StructuredPostal;
import android.provider.ContactsContract.ContactCounts;
Expand Down Expand Up @@ -664,6 +665,36 @@ interface RawContactsQuery {
.add(TIMES_USED_SORT_COLUMN, "SUM(" + DataUsageStatColumns.CONCRETE_TIMES_USED + ")")
.build();

/**
* 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(TIMES_USED_SORT_COLUMN, String.valueOf(Long.MAX_VALUE))
.add(Phone.NUMBER, "NULL")
.add(Phone.TYPE, "NULL")
.add(Phone.LABEL, "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
= ProjectionMap.builder()
.addAll(sContactsProjectionMap)
.add(TIMES_USED_SORT_COLUMN, DataUsageStatColumns.CONCRETE_TIMES_USED)
.add(Phone.NUMBER)
.add(Phone.TYPE)
.add(Phone.LABEL)
.add(Contacts.IS_USER_PROFILE, "NULL")
.build();

/** Contains just the contacts vCard columns */
private static final ProjectionMap sContactsVCardProjectionMap = ProjectionMap.builder()
.add(Contacts._ID)
Expand Down Expand Up @@ -4487,9 +4518,10 @@ private Cursor queryLocal(Uri uri, String[] projection, String selection,
}

// Build the first query for starred
setTablesAndProjectionMapForContacts(qb, uri, projection,
false /* for frequent */, phoneOnly);
qb.setProjectionMap(sStrequentStarredProjectionMap);
setTablesAndProjectionMapForContacts(qb, uri, projection, false);
qb.setProjectionMap(phoneOnly ?
sStrequentPhoneOnlyStarredProjectionMap
: sStrequentStarredProjectionMap);
qb.appendWhere(DbQueryUtils.concatenateClauses(
selection, Contacts.IS_USER_PROFILE + "=0"));
qb.setStrict(true);
Expand All @@ -4498,17 +4530,51 @@ private Cursor queryLocal(Uri uri, String[] projection, String selection,

// Reset the builder.
qb = new SQLiteQueryBuilder();

// Build the second query for frequent
setTablesAndProjectionMapForContacts(qb, uri, projection,
true /* for frequent */, phoneOnly);
qb.setProjectionMap(sStrequentFrequentProjectionMap);
qb.appendWhere(DbQueryUtils.concatenateClauses(
selection, Contacts.IS_USER_PROFILE + "=0"));
qb.setStrict(true);
final String frequentQuery = qb.buildQuery(subProjection,
"(" + Contacts.STARRED + " =0 OR " + Contacts.STARRED + " IS NULL)",
Contacts._ID, null, null, null);

// Build the second query for frequent part.
final String frequentQuery;
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
// 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
// rows in view_data in the early stage of data lookup.
tableBuilder.append(Tables.DATA_USAGE_STAT
+ " INNER JOIN " + Views.DATA + " " + Tables.DATA
+ " 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(sStrequentPhoneOnlyFrequentProjectionMap);
qb.appendWhere(DbQueryUtils.concatenateClauses(
selection,
Contacts.STARRED + "=0 OR " + Contacts.STARRED + " IS NULL",
MimetypesColumns.MIMETYPE + " IN ("
+ "'" + Phone.CONTENT_ITEM_TYPE + "', "
+ "'" + SipAddress.CONTENT_ITEM_TYPE + "')"));
frequentQuery = qb.buildQuery(subProjection, null, null, null, null, null);
} else {
setTablesAndProjectionMapForContacts(qb, uri, projection, true);
qb.setProjectionMap(sStrequentFrequentProjectionMap);
qb.appendWhere(DbQueryUtils.concatenateClauses(
selection,
"(" + Contacts.STARRED + " =0 OR " + Contacts.STARRED + " IS NULL)",
Contacts.IS_USER_PROFILE + "=0"));
frequentQuery = qb.buildQuery(subProjection,
null, Contacts._ID, null, null, null);
}

// Put them together
final String unionQuery =
Expand Down Expand Up @@ -5619,38 +5685,27 @@ private long getMostReferencedContactId(ArrayList<LookupKeySegment> segments) {

private void setTablesAndProjectionMapForContacts(SQLiteQueryBuilder qb, Uri uri,
String[] projection) {
setTablesAndProjectionMapForContacts(qb, uri, projection, false, false);
setTablesAndProjectionMapForContacts(qb, uri, projection, false);
}

/**
* @param forStrequentFrequent Should be used only in strequent handling.
* true when this is for frequently contacted listing (not starred)
* @param strequentPhoneCallOnly Should be used only in strequent handling.
* true when this is for phone-only results. See also
* {@link ContactsContract#STREQUENT_PHONE_ONLY}.
* @param includeDataUsageStat true when the table should include DataUsageStat table.
* Note that this uses INNER JOIN instead of LEFT OUTER JOIN, so some of data in Contacts
* may be dropped.
*/
private void setTablesAndProjectionMapForContacts(SQLiteQueryBuilder qb, Uri uri,
String[] projection, boolean forStrequentFrequent, boolean strequentPhoneCallOnly) {
String[] projection, boolean includeDataUsageStat) {
StringBuilder sb = new StringBuilder();
sb.append(Views.CONTACTS);

// Just for frequently contacted contacts in Strequent Uri handling.
if (forStrequentFrequent) {
final String strequentPhoneCallOnlyClause =
(strequentPhoneCallOnly ? DbQueryUtils.concatenateClauses(
MimetypesColumns.MIMETYPE + "=\'" + Phone.CONTENT_ITEM_TYPE + "'",
DataUsageStatColumns.CONCRETE_USAGE_TYPE + "=" +
DataUsageStatColumns.USAGE_TYPE_INT_CALL)
: "");
// Use INNER JOIN for maximum performance, ommiting unnecessary rows as much as
// possible.
if (includeDataUsageStat) {
sb.append(" INNER JOIN " +
Views.DATA_USAGE_STAT + " AS " + Tables.DATA_USAGE_STAT +
" ON (" +
DbQueryUtils.concatenateClauses(
DataUsageStatColumns.CONCRETE_TIMES_USED + " > 0",
RawContacts.CONTACT_ID + "=" + Views.CONTACTS + "." + Contacts._ID,
strequentPhoneCallOnlyClause) +
RawContacts.CONTACT_ID + "=" + Views.CONTACTS + "." + Contacts._ID) +
")");
}

Expand Down
78 changes: 39 additions & 39 deletions tests/src/com/android/providers/contacts/ContactsProvider2Test.java
View file
Expand Up @@ -1202,10 +1202,13 @@ public void testEmailFilterPrimaryAccount() {
/** Tests {@link DataUsageFeedback} correctly promotes a data row instead of a raw contact. */
public void testEmailFilterSortOrderWithFeedback() {
long rawContactId1 = createRawContact();
insertEmail(rawContactId1, "address1@email.com");
String address1 = "address1@email.com";
insertEmail(rawContactId1, address1);
long rawContactId2 = createRawContact();
insertEmail(rawContactId2, "address2@email.com");
long dataId = ContentUris.parseId(insertEmail(rawContactId2, "address3@email.com"));
String address2 = "address2@email.com";
insertEmail(rawContactId2, address2);
String address3 = "address3@email.com";
ContentUris.parseId(insertEmail(rawContactId2, address3));

ContentValues v1 = new ContentValues();
v1.put(Email.ADDRESS, "address1@email.com");
Expand All @@ -1232,13 +1235,7 @@ public void testEmailFilterSortOrderWithFeedback() {
assertStoredValuesOrderly(filterUri3, new ContentValues[] { v1, v2, v3 });
assertStoredValuesOrderly(filterUri4, new ContentValues[] { v1, v2, v3 });

// Send feedback for address3 in the second account.
Uri feedbackUri = DataUsageFeedback.FEEDBACK_URI.buildUpon()
.appendPath(String.valueOf(dataId))
.appendQueryParameter(DataUsageFeedback.USAGE_TYPE,
DataUsageFeedback.USAGE_TYPE_LONG_TEXT)
.build();
assertNotSame(0, mResolver.update(feedbackUri, new ContentValues(), null, null));
sendFeedback(address3, DataUsageFeedback.USAGE_TYPE_LONG_TEXT, v3);

// account3@email.com should be the first. account2@email.com should also be promoted as
// it has same contact id.
Expand Down Expand Up @@ -1454,8 +1451,9 @@ public void testQueryContactStrequent() {
createContact(values1, "Noah", "Tever", "18004664411",
email1, StatusUpdates.OFFLINE, timesContacted1, 0, 0,
StatusUpdates.CAPABILITY_HAS_CAMERA | StatusUpdates.CAPABILITY_HAS_VIDEO);
final String phoneNumber2 = "18004664412";
ContentValues values2 = new ContentValues();
createContact(values2, "Sam", "Times", "18004664412",
createContact(values2, "Sam", "Times", phoneNumber2,
"b@acme.com", StatusUpdates.INVISIBLE, 3, 0, 0,
StatusUpdates.CAPABILITY_HAS_CAMERA);
ContentValues values3 = new ContentValues();
Expand All @@ -1473,20 +1471,8 @@ public void testQueryContactStrequent() {
// usage feedback should be used for "frequently contacted" listing.
assertStoredValues(Contacts.CONTENT_STREQUENT_URI, values4);

final long dataIdPhone3 = getStoredLongValue(Phone.CONTENT_URI,
Phone.NUMBER + "=?", new String[] { phoneNumber3 },
Data._ID);

// Send feedback for the 3rd phone number, pretending we called that person via phone.
Uri feedbackUri = DataUsageFeedback.FEEDBACK_URI.buildUpon()
.appendPath(String.valueOf(dataIdPhone3))
.appendQueryParameter(DataUsageFeedback.USAGE_TYPE,
DataUsageFeedback.USAGE_TYPE_CALL)
.build();
assertNotSame(0, mResolver.update(feedbackUri, new ContentValues(), null, null));

// After the feedback, times contacted should be incremented
values3.put(RawContacts.TIMES_CONTACTED, timesContacted3 + 1);
sendFeedback(phoneNumber3, DataUsageFeedback.USAGE_TYPE_CALL, values3);

// After the feedback, 3rd contact should be shown after starred one.
assertStoredValuesOrderly(Contacts.CONTENT_STREQUENT_URI,
Expand All @@ -1497,31 +1483,26 @@ public void testQueryContactStrequent() {
Email.ADDRESS + "=?", new String[] { email1},
Email._ID);

// Send feedback for the 1st email, pretending we sent the person an email twice.
// (we don't define the order for 1st and 3rd contacts with same times contacted)
feedbackUri = DataUsageFeedback.FEEDBACK_URI.buildUpon()
.appendPath(String.valueOf(dataIdEmail1))
.appendQueryParameter(DataUsageFeedback.USAGE_TYPE,
DataUsageFeedback.USAGE_TYPE_LONG_TEXT)
.build();
assertNotSame(0, mResolver.update(feedbackUri, new ContentValues(), null, null));
sendFeedback(email1, DataUsageFeedback.USAGE_TYPE_LONG_TEXT, values1);
// Twice.
assertNotSame(0, mResolver.update(feedbackUri, new ContentValues(), null, null));

// After the feedback, times contacted should be incremented
values1.put(RawContacts.TIMES_CONTACTED, timesContacted1 + 2);
sendFeedback(email1, DataUsageFeedback.USAGE_TYPE_LONG_TEXT, values1);

// After the feedback, 1st and 3rd contacts should be shown after starred one.
assertStoredValuesOrderly(Contacts.CONTENT_STREQUENT_URI,
new ContentValues[] { values4, values1, values3 });
new ContentValues[] { values4, values1, values3 });

// With phone-only parameter, the 1st contact shouldn't be returned, since it is only
// about email, not phone-call.
Uri phoneOnlyStrequentUri = Contacts.CONTENT_STREQUENT_URI.buildUpon()
.appendQueryParameter(ContactsContract.STREQUENT_PHONE_ONLY, "true")
.build();
assertStoredValuesOrderly(phoneOnlyStrequentUri,
new ContentValues[] { values4, values3 });
assertStoredValuesOrderly(phoneOnlyStrequentUri, new ContentValues[] { 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 });

Uri filterUri = Uri.withAppendedPath(Contacts.CONTENT_STREQUENT_FILTER_URI, "fay");
assertStoredValues(filterUri, values4);
Expand Down Expand Up @@ -5672,5 +5653,24 @@ private void putDataValues(ContentValues values, long rawContactId) {
values.put(Data.SYNC3, "sync3");
values.put(Data.SYNC4, "sync4");
}

/**
* @param data1 email address or phone number
* @param usageType One of {@link DataUsageFeedback#USAGE_TYPE}
* @param values ContentValues for this feedback. Useful for incrementing
* {Contacts#TIMES_CONTACTED} in the ContentValue. Can be null.
*/
private void sendFeedback(String data1, String usageType, ContentValues values) {
final long dataId = getStoredLongValue(Data.CONTENT_URI,
Data.DATA1 + "=?", new String[] { data1 }, Data._ID);
final Uri feedbackUri = DataUsageFeedback.FEEDBACK_URI.buildUpon()
.appendPath(String.valueOf(dataId))
.appendQueryParameter(DataUsageFeedback.USAGE_TYPE, usageType)
.build();
assertNotSame(0, mResolver.update(feedbackUri, new ContentValues(), null, null));
if (values != null && values.containsKey(Contacts.TIMES_CONTACTED)) {
values.put(Contacts.TIMES_CONTACTED, values.getAsInteger(Contacts.TIMES_CONTACTED) + 1);
}
}
}

0 comments on commit 4928b8c

Please sign in to comment.