Deprioritizing twitter pictures in contact aggregation
Discussed the solution with Tim Sullivan, got a Dr.No approval
for fixing the issue in FroYo and for the solution itself.
In two separate CLs we will set picture priorities for
Facebook (5) and Twitter (3)
Bug: 2535693
Change-Id: I3d90474d92b58db2845a43658845176260c24812
diff --git a/src/com/android/providers/contacts/ContactAggregator.java b/src/com/android/providers/contacts/ContactAggregator.java
index de55f93..b5d6b27 100644
--- a/src/com/android/providers/contacts/ContactAggregator.java
+++ b/src/com/android/providers/contacts/ContactAggregator.java
@@ -92,6 +92,7 @@
private final ContactsProvider2 mContactsProvider;
private final ContactsDatabaseHelper mDbHelper;
+ private PhotoPriorityResolver mPhotoPriorityResolver;
private boolean mEnabled = true;
/** Precompiled sql statement for setting an aggregated presence */
@@ -198,9 +199,11 @@
* Constructor.
*/
public ContactAggregator(ContactsProvider2 contactsProvider,
- ContactsDatabaseHelper contactsDatabaseHelper) {
+ ContactsDatabaseHelper contactsDatabaseHelper,
+ PhotoPriorityResolver photoPriorityResolver) {
mContactsProvider = contactsProvider;
mDbHelper = contactsDatabaseHelper;
+ mPhotoPriorityResolver = photoPriorityResolver;
SQLiteDatabase db = mDbHelper.getReadableDatabase();
@@ -1119,7 +1122,7 @@
long currentRawContactId = -1;
long bestPhotoId = -1;
boolean foundSuperPrimaryPhoto = false;
- String photoAccount = null;
+ int photoPriority = -1;
int totalRowCount = 0;
int contactSendToVoicemail = 0;
String contactCustomRingtone = null;
@@ -1201,24 +1204,21 @@
if (!c.isNull(RawContactsQuery.DATA_ID)) {
long dataId = c.getLong(RawContactsQuery.DATA_ID);
int mimetypeId = c.getInt(RawContactsQuery.MIMETYPE_ID);
- boolean superprimary = c.getInt(RawContactsQuery.IS_SUPER_PRIMARY) != 0;
+ boolean superPrimary = c.getInt(RawContactsQuery.IS_SUPER_PRIMARY) != 0;
if (mimetypeId == mMimeTypeIdPhoto) {
-
- // For now, just choose the first photo in a list sorted by account name.
- String account = c.getString(RawContactsQuery.ACCOUNT_NAME);
- if (!foundSuperPrimaryPhoto && (
- superprimary || photoAccount == null ||
- (account != null &&
- photoAccount.compareToIgnoreCase(account) >= 0))) {
- photoAccount = account;
- bestPhotoId = dataId;
- foundSuperPrimaryPhoto |= superprimary;
+ if (!foundSuperPrimaryPhoto) {
+ String accountType = c.getString(RawContactsQuery.ACCOUNT_TYPE);
+ int priority = mPhotoPriorityResolver.getPhotoPriority(accountType);
+ if (superPrimary || priority > photoPriority) {
+ photoPriority = priority;
+ bestPhotoId = dataId;
+ foundSuperPrimaryPhoto |= superPrimary;
+ }
}
} else if (mimetypeId == mMimeTypeIdPhone) {
hasPhoneNumber = 1;
}
}
-
}
} finally {
c.close();
@@ -1296,12 +1296,12 @@
private interface PhotoIdQuery {
String[] COLUMNS = new String[] {
- RawContacts.ACCOUNT_NAME,
+ RawContacts.ACCOUNT_TYPE,
DataColumns.CONCRETE_ID,
Data.IS_SUPER_PRIMARY,
};
- int ACCOUNT_NAME = 0;
+ int ACCOUNT_TYPE = 0;
int DATA_ID = 1;
int IS_SUPER_PRIMARY = 2;
}
@@ -1314,7 +1314,7 @@
}
long bestPhotoId = -1;
- String photoAccount = null;
+ int photoPriority = -1;
long photoMimeType = mDbHelper.getMimeTypeId(Photo.CONTENT_ITEM_TYPE);
@@ -1335,16 +1335,11 @@
break;
}
- // For now, just choose the first photo in a list sorted by account name.
- String account = c.getString(PhotoIdQuery.ACCOUNT_NAME);
- if (photoAccount == null) {
- photoAccount = account;
+ String accountType = c.getString(PhotoIdQuery.ACCOUNT_TYPE);
+ int priority = mPhotoPriorityResolver.getPhotoPriority(accountType);
+ if (priority > photoPriority) {
+ photoPriority = priority;
bestPhotoId = dataId;
- } else {
- if (account.compareToIgnoreCase(photoAccount) < 0) {
- photoAccount = account;
- bestPhotoId = dataId;
- }
}
}
} finally {
diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java
index 62c2709..a87d7d5 100644
--- a/src/com/android/providers/contacts/ContactsProvider2.java
+++ b/src/com/android/providers/contacts/ContactsProvider2.java
@@ -1854,7 +1854,8 @@
mDbHelper = (ContactsDatabaseHelper)getDatabaseHelper();
mGlobalSearchSupport = new GlobalSearchSupport(this);
mLegacyApiSupport = new LegacyApiSupport(context, mDbHelper, this, mGlobalSearchSupport);
- mContactAggregator = new ContactAggregator(this, mDbHelper);
+ mContactAggregator = new ContactAggregator(this, mDbHelper,
+ createPhotoPriorityResolver(context));
mContactAggregator.setEnabled(SystemProperties.getBoolean(AGGREGATE_CONTACTS, true));
mDb = mDbHelper.getWritableDatabase();
@@ -1998,6 +1999,13 @@
}
/**
+ * Visible for testing.
+ */
+ /* package */ PhotoPriorityResolver createPhotoPriorityResolver(Context context) {
+ return new PhotoPriorityResolver(context);
+ }
+
+ /**
* (Re)allocates all locale-sensitive structures.
*/
private void initForDefaultLocale() {
diff --git a/src/com/android/providers/contacts/PhotoPriorityResolver.java b/src/com/android/providers/contacts/PhotoPriorityResolver.java
new file mode 100644
index 0000000..c0dc4d9
--- /dev/null
+++ b/src/com/android/providers/contacts/PhotoPriorityResolver.java
@@ -0,0 +1,165 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.providers.contacts;
+
+import com.android.internal.util.XmlUtils;
+import com.google.android.collect.Maps;
+
+import org.xmlpull.v1.XmlPullParser;
+import org.xmlpull.v1.XmlPullParserException;
+
+import android.accounts.AccountManager;
+import android.accounts.AuthenticatorDescription;
+import android.content.Context;
+import android.content.pm.PackageInfo;
+import android.content.pm.PackageManager;
+import android.content.pm.ServiceInfo;
+import android.content.pm.PackageManager.NameNotFoundException;
+import android.content.res.XmlResourceParser;
+import android.util.Log;
+
+import java.io.IOException;
+import java.util.HashMap;
+
+/**
+ * Maintains a cache of photo priority per account type. During contact aggregation
+ * photo with a higher priority is chosen for the the entire contact, barring an
+ * explicit override by the user, which is captured as the is_superprimary flag
+ * on the photo itself.
+ */
+public class PhotoPriorityResolver {
+ private static final String TAG = "PhotoPriorityResolver";
+
+ public static final int DEFAULT_PRIORITY = 7;
+
+ /**
+ * Meta-data key for the contacts configuration associated with a sync service.
+ */
+ private static final String METADATA_CONTACTS = "android.provider.CONTACTS_STRUCTURE";
+
+ /**
+ * The XML tag capturing the picture priority. The syntax is:
+ * <code><Picture android:priority="6"/></code>
+ */
+ private static final String PICTURE_TAG = "Picture";
+
+ /**
+ * Name of the attribute of the Picture tag capturing the priority itself.
+ */
+ private static final String PRIORITY_ATTR = "priority";
+
+ private Context mContext;
+ private HashMap<String, Integer> mPhotoPriorities = Maps.newHashMap();
+
+ public PhotoPriorityResolver(Context context) {
+ mContext = context;
+ }
+
+ /**
+ * Returns the photo priority for the specified account type. Maintains cache
+ * of photo priorities.
+ */
+ public synchronized int getPhotoPriority(String accountType) {
+ if (accountType == null) {
+ return DEFAULT_PRIORITY;
+ }
+
+ Integer priority = mPhotoPriorities.get(accountType);
+ if (priority == null) {
+ priority = resolvePhotoPriority(accountType);
+ mPhotoPriorities.put(accountType, priority);
+ }
+ return priority;
+ }
+
+ /**
+ * Finds photo priority for the specified account type.
+ */
+ private int resolvePhotoPriority(String accountType) {
+ final AccountManager am = AccountManager.get(mContext);
+
+ for (AuthenticatorDescription auth : am.getAuthenticatorTypes()) {
+ if (accountType.equals(auth.type)) {
+ return resolvePhotoPriorityFromMetaData(auth.packageName);
+ }
+ }
+
+ return DEFAULT_PRIORITY;
+ }
+
+ /**
+ * Finds the meta-data XML containing the contacts configuration and
+ * reads the picture priority from that file.
+ */
+ /* package */ int resolvePhotoPriorityFromMetaData(String packageName) {
+ final PackageManager pm = mContext.getPackageManager();
+ try {
+ PackageInfo pi = pm.getPackageInfo(packageName, PackageManager.GET_SERVICES
+ | PackageManager.GET_META_DATA);
+ if (pi != null && pi.services != null) {
+ for (ServiceInfo si : pi.services) {
+ final XmlResourceParser parser = si.loadXmlMetaData(pm, METADATA_CONTACTS);
+ if (parser != null) {
+ return loadPhotoPriorityFromXml(mContext, parser);
+ }
+ }
+ }
+ } catch (NameNotFoundException e) {
+ Log.w(TAG, "Problem loading photo priorities: " + e.toString());
+ }
+ return DEFAULT_PRIORITY;
+ }
+
+ private int loadPhotoPriorityFromXml(Context context, XmlPullParser parser) {
+ int priority = DEFAULT_PRIORITY;
+ try {
+ int type;
+ while ((type = parser.next()) != XmlPullParser.START_TAG
+ && type != XmlPullParser.END_DOCUMENT) {
+ // Drain comments and whitespace
+ }
+
+ if (type != XmlPullParser.START_TAG) {
+ throw new IllegalStateException("No start tag found");
+ }
+
+ final int depth = parser.getDepth();
+ while (((type = parser.next()) != XmlPullParser.END_TAG || parser.getDepth() > depth)
+ && type != XmlPullParser.END_DOCUMENT) {
+ String name = parser.getName();
+ if (type == XmlPullParser.START_TAG && PICTURE_TAG.equals(name)) {
+ int attributeCount = parser.getAttributeCount();
+ for (int i = 0; i < attributeCount; i++) {
+ String attr = parser.getAttributeName(i);
+ if (PRIORITY_ATTR.equals(attr)) {
+ priority = XmlUtils.convertValueToInt(parser.getAttributeValue(i),
+ DEFAULT_PRIORITY);
+ } else {
+ throw new IllegalStateException("Unsupported attribute " + attr);
+ }
+ }
+ }
+ }
+ } catch (XmlPullParserException e) {
+ throw new IllegalStateException("Problem reading XML", e);
+ } catch (IOException e) {
+ throw new IllegalStateException("Problem reading XML", e);
+ }
+
+ return priority;
+ }
+}
diff --git a/tests/AndroidManifest.xml b/tests/AndroidManifest.xml
index 5063a7d..d0e3810 100644
--- a/tests/AndroidManifest.xml
+++ b/tests/AndroidManifest.xml
@@ -23,10 +23,16 @@
<application>
<uses-library android:name="android.test.runner" />
+
+ <!-- Mock contacts sync adapter -->
+ <service android:name=".MockSyncAdapter" android:exported="true">
+ <meta-data android:name="android.provider.CONTACTS_STRUCTURE"
+ android:resource="@xml/contacts" />
+ </service>
</application>
<!--
- The test delcared in this instrumentation will be run along with tests declared by
+ The test declared in this instrumentation will be run along with tests declared by
all other applications via the command: "adb shell itr".
The "itr" command will find all tests declared by all applications. If you want to run just these
tests on their own then use the command:
diff --git a/tests/res/xml/contacts.xml b/tests/res/xml/contacts.xml
new file mode 100644
index 0000000..863574a
--- /dev/null
+++ b/tests/res/xml/contacts.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2010 The Android Open Source Project
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+
+<ContactsSource xmlns:android="http://schemas.android.com/apk/res/android">
+
+ <Picture android:priority="42"/>
+
+</ContactsSource>
diff --git a/tests/src/com/android/providers/contacts/BaseContactsProvider2Test.java b/tests/src/com/android/providers/contacts/BaseContactsProvider2Test.java
index ca68856..de410a9 100644
--- a/tests/src/com/android/providers/contacts/BaseContactsProvider2Test.java
+++ b/tests/src/com/android/providers/contacts/BaseContactsProvider2Test.java
@@ -371,8 +371,9 @@
return resultUri;
}
- protected void setContactAccountName(long rawContactId, String accountName) {
+ protected void setContactAccount(long rawContactId, String accountType, String accountName) {
ContentValues values = new ContentValues();
+ values.put(RawContacts.ACCOUNT_TYPE, accountType);
values.put(RawContacts.ACCOUNT_NAME, accountName);
mResolver.update(ContentUris.withAppendedId(
diff --git a/tests/src/com/android/providers/contacts/ContactAggregatorTest.java b/tests/src/com/android/providers/contacts/ContactAggregatorTest.java
index cd36590..cc43b61 100644
--- a/tests/src/com/android/providers/contacts/ContactAggregatorTest.java
+++ b/tests/src/com/android/providers/contacts/ContactAggregatorTest.java
@@ -641,21 +641,41 @@
assertSuggestions(contactId1, "eddie");
}
- public void testChoosePhoto() {
+ public void testChoosePhotoSetBeforeAggregation() {
long rawContactId1 = createRawContact();
- setContactAccountName(rawContactId1, "donut");
+ setContactAccount(rawContactId1, "donut", "donut_act");
+ insertPhoto(rawContactId1);
+
+ long rawContactId2 = createRawContact();
+ setContactAccount(rawContactId2, "cupcake", "cupcake_act");
+ long cupcakeId = ContentUris.parseId(insertPhoto(rawContactId2));
+
+ long rawContactId3 = createRawContact();
+ setContactAccount(rawContactId3, "froyo", "froyo_act");
+ insertPhoto(rawContactId3);
+
+ setAggregationException(AggregationExceptions.TYPE_KEEP_TOGETHER,
+ rawContactId1, rawContactId2);
+ setAggregationException(AggregationExceptions.TYPE_KEEP_TOGETHER,
+ rawContactId1, rawContactId3);
+ assertEquals(cupcakeId, queryPhotoId(queryContactId(rawContactId2)));
+ }
+
+ public void testChoosePhotoSetAfterAggregation() {
+ long rawContactId1 = createRawContact();
+ setContactAccount(rawContactId1, "donut", "donut_act");
insertPhoto(rawContactId1);
long rawContactId2 = createRawContact();
setAggregationException(AggregationExceptions.TYPE_KEEP_TOGETHER,
rawContactId1, rawContactId2);
- setContactAccountName(rawContactId2, "cupcake");
+ setContactAccount(rawContactId2, "cupcake", "cupcake_act");
long cupcakeId = ContentUris.parseId(insertPhoto(rawContactId2));
long rawContactId3 = createRawContact();
setAggregationException(AggregationExceptions.TYPE_KEEP_TOGETHER,
rawContactId1, rawContactId3);
- setContactAccountName(rawContactId3, "flan");
+ setContactAccount(rawContactId3, "froyo", "froyo_act");
insertPhoto(rawContactId3);
assertEquals(cupcakeId, queryPhotoId(queryContactId(rawContactId2)));
diff --git a/tests/src/com/android/providers/contacts/MockSyncAdapter.java b/tests/src/com/android/providers/contacts/MockSyncAdapter.java
new file mode 100644
index 0000000..9255d28
--- /dev/null
+++ b/tests/src/com/android/providers/contacts/MockSyncAdapter.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.contacts;
+
+import android.app.Service;
+import android.content.Intent;
+import android.os.IBinder;
+
+/**
+ * A mock sync adapter.
+ */
+public class MockSyncAdapter extends Service {
+
+ @Override
+ public IBinder onBind(Intent intent) {
+ return null;
+ }
+}
diff --git a/tests/src/com/android/providers/contacts/PhotoPriorityResolverTest.java b/tests/src/com/android/providers/contacts/PhotoPriorityResolverTest.java
new file mode 100644
index 0000000..b105595
--- /dev/null
+++ b/tests/src/com/android/providers/contacts/PhotoPriorityResolverTest.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.providers.contacts;
+
+import android.test.AndroidTestCase;
+import android.test.suitebuilder.annotation.MediumTest;
+
+/**
+ * Unit tests for {@link PhotoPriorityResolver}.
+ *
+ * Run the test like this:
+ * <code>
+ * adb shell am instrument -e class com.android.providers.contacts.PhotoPriorityResolverTest -w \
+ * com.android.providers.contacts.tests/android.test.InstrumentationTestRunner
+ * </code>
+ */
+@MediumTest
+public class PhotoPriorityResolverTest extends AndroidTestCase {
+
+ public void testLoadPicturePriorityFromXml() {
+ PhotoPriorityResolver resolver = new PhotoPriorityResolver(getContext());
+
+ // See the res/xml/contacts.xml file where this priority is specified.
+ assertEquals(42,
+ resolver.resolvePhotoPriorityFromMetaData("com.android.providers.contacts.tests"));
+
+ assertEquals(PhotoPriorityResolver.DEFAULT_PRIORITY,
+ resolver.resolvePhotoPriorityFromMetaData("no.such.package"));
+ }
+}
+
diff --git a/tests/src/com/android/providers/contacts/SynchronousContactsProvider2.java b/tests/src/com/android/providers/contacts/SynchronousContactsProvider2.java
index 622a61e..0248094 100644
--- a/tests/src/com/android/providers/contacts/SynchronousContactsProvider2.java
+++ b/tests/src/com/android/providers/contacts/SynchronousContactsProvider2.java
@@ -96,6 +96,28 @@
return mAccount;
}
+ /**
+ * Creates a mock PhotoPriorityResolver
+ */
+ @Override
+ PhotoPriorityResolver createPhotoPriorityResolver(Context context) {
+ return new PhotoPriorityResolver(context) {
+ @Override
+ public synchronized int getPhotoPriority(String accountType) {
+ if ("cupcake".equals(accountType)) {
+ return 3;
+ }
+ if ("donut".equals(accountType)) {
+ return 2;
+ }
+ if ("froyo".equals(accountType)) {
+ return 1;
+ }
+ return 0;
+ }
+ };
+ }
+
@Override
protected Locale getLocale() {
return Locale.US;