Merge pull request #661 from mar-v-in/improve-contacts

Improve contact sync

2 files changed, 198 insertions, 53 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ContactHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ContactHelper.java
index f50ccf6f8..d8a7e8427 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ContactHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/helper/ContactHelper.java
@@ -22,7 +22,6 @@ import android.accounts.AccountManager;
 import android.content.*;
 import android.database.Cursor;
 import android.net.Uri;
-import android.os.RemoteException;
 import android.provider.ContactsContract;
 import android.util.Patterns;
 import org.sufficientlysecure.keychain.Constants;
@@ -32,10 +31,7 @@ import org.sufficientlysecure.keychain.pgp.PgpKeyHelper;
 import org.sufficientlysecure.keychain.provider.KeychainContract;
 import org.sufficientlysecure.keychain.util.Log;
 
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import java.util.*;
 
 public class ContactHelper {
 
@@ -43,12 +39,26 @@ public class ContactHelper {
             KeychainContract.KeyRings.USER_ID,
             KeychainContract.KeyRings.FINGERPRINT,
             KeychainContract.KeyRings.KEY_ID,
-            KeychainContract.KeyRings.MASTER_KEY_ID};
-    public static final String[] RAW_CONTACT_ID_PROJECTION = new String[]{ContactsContract.RawContacts._ID};
-    public static final String FIND_RAW_CONTACT_SELECTION =
+            KeychainContract.KeyRings.MASTER_KEY_ID,
+            KeychainContract.KeyRings.EXPIRY,
+            KeychainContract.KeyRings.IS_REVOKED};
+    public static final String[] USER_IDS_PROJECTION = new String[]{
+            KeychainContract.UserIds.USER_ID
+    };
+
+    public static final String NON_REVOKED_SELECTION = KeychainContract.UserIds.IS_REVOKED + "=0";
+
+    public static final String[] ID_PROJECTION = new String[]{ContactsContract.RawContacts._ID};
+    public static final String[] SOURCE_ID_PROJECTION = new String[]{ContactsContract.RawContacts.SOURCE_ID};
+
+    public static final String ACCOUNT_TYPE_AND_SOURCE_ID_SELECTION =
             ContactsContract.RawContacts.ACCOUNT_TYPE + "=? AND " + ContactsContract.RawContacts.SOURCE_ID + "=?";
+    public static final String ACCOUNT_TYPE_SELECTION = ContactsContract.RawContacts.ACCOUNT_TYPE + "=?";
+    public static final String RAW_CONTACT_AND_MIMETYPE_SELECTION =
+            ContactsContract.Data.RAW_CONTACT_ID + "=? AND " + ContactsContract.Data.MIMETYPE + "=?";
+    public static final String ID_SELECTION = ContactsContract.RawContacts._ID + "=?";
 
-    public static final List<String> getMailAccounts(Context context) {
+    public static List<String> getMailAccounts(Context context) {
         final Account[] accounts = AccountManager.get(context).getAccounts();
         final Set<String> emailSet = new HashSet<String>();
         for (Account account : accounts) {
@@ -78,7 +88,8 @@ public class ContactHelper {
     }
 
     public static Uri dataUriFromContactUri(Context context, Uri contactUri) {
-        Cursor contactMasterKey = context.getContentResolver().query(contactUri, new String[]{ContactsContract.Data.DATA2}, null, null, null, null);
+        Cursor contactMasterKey = context.getContentResolver().query(contactUri,
+                new String[]{ContactsContract.Data.DATA2}, null, null, null, null);
         if (contactMasterKey != null) {
             if (contactMasterKey.moveToNext()) {
                 return KeychainContract.KeyRings.buildGenericKeyRingUri(contactMasterKey.getLong(0));
@@ -88,62 +99,178 @@ public class ContactHelper {
         return null;
     }
 
+    /**
+     * Write the current Keychain to the contact db
+     */
     public static void writeKeysToContacts(Context context) {
         ContentResolver resolver = context.getContentResolver();
+        Set<String> contactFingerprints = getRawContactFingerprints(resolver);
+
+        // Load all Keys from OK
         Cursor cursor = resolver.query(KeychainContract.KeyRings.buildUnifiedKeyRingsUri(), KEYS_TO_CONTACT_PROJECTION,
                 null, null, null);
         if (cursor != null) {
             while (cursor.moveToNext()) {
-                String[] userId = KeyRing.splitUserId(cursor.getString(0));
+                String[] primaryUserId = KeyRing.splitUserId(cursor.getString(0));
                 String fingerprint = PgpKeyHelper.convertFingerprintToHex(cursor.getBlob(1));
+                contactFingerprints.remove(fingerprint);
                 String keyIdShort = PgpKeyHelper.convertKeyIdToHexShort(cursor.getLong(2));
                 long masterKeyId = cursor.getLong(3);
-                int rawContactId = -1;
-                Cursor raw = resolver.query(ContactsContract.RawContacts.CONTENT_URI, RAW_CONTACT_ID_PROJECTION,
-                        FIND_RAW_CONTACT_SELECTION, new String[]{Constants.PACKAGE_NAME, fingerprint}, null, null);
-                if (raw != null) {
-                    if (raw.moveToNext()) {
-                        rawContactId = raw.getInt(0);
-                    }
-                    raw.close();
-                }
+                boolean isExpired = !cursor.isNull(4) && new Date(cursor.getLong(4) * 1000).before(new Date());
+                boolean isRevoked = cursor.getInt(5) > 0;
+                int rawContactId = findRawContactId(resolver, fingerprint);
                 ArrayList<ContentProviderOperation> ops = new ArrayList<ContentProviderOperation>();
-                if (rawContactId == -1) {
-                    ops.add(ContentProviderOperation.newInsert(ContactsContract.RawContacts.CONTENT_URI)
-                            .withValue(ContactsContract.RawContacts.ACCOUNT_NAME, context.getString(R.string.app_name))
-                            .withValue(ContactsContract.RawContacts.ACCOUNT_TYPE, Constants.PACKAGE_NAME)
-                            .withValue(ContactsContract.RawContacts.SOURCE_ID, fingerprint)
-                            .build());
-                    if (userId[0] != null) {
-                        ops.add(ContentProviderOperation.newInsert(ContactsContract.Data.CONTENT_URI)
-                                .withValueBackReference(ContactsContract.Data.RAW_CONTACT_ID, 0)
-                                .withValue(ContactsContract.Data.MIMETYPE, ContactsContract.CommonDataKinds.StructuredName.CONTENT_ITEM_TYPE)
-                                .withValue(ContactsContract.CommonDataKinds.StructuredName.DISPLAY_NAME, userId[0])
-                                .build());
+
+                // Do not store expired or revoked keys in contact db - and remove them if they already exist
+                if (isExpired || isRevoked) {
+                    if (rawContactId != -1) {
+                        resolver.delete(ContactsContract.RawContacts.CONTENT_URI, ID_SELECTION,
+                                new String[]{Integer.toString(rawContactId)});
                     }
-                    if (userId[1] != null) {
-                        ops.add(ContentProviderOperation.newInsert(ContactsContract.Data.CONTENT_URI)
-                                .withValueBackReference(ContactsContract.Data.RAW_CONTACT_ID, 0)
-                                .withValue(ContactsContract.Data.MIMETYPE, ContactsContract.CommonDataKinds.Email.CONTENT_ITEM_TYPE)
-                                .withValue(ContactsContract.CommonDataKinds.Email.DATA, userId[1])
-                                .build());
+                } else {
+
+                    // Create a new rawcontact with corresponding key if it does not exist yet
+                    if (rawContactId == -1) {
+                        insertContact(ops, context, fingerprint);
+                        writeContactKey(ops, context, rawContactId, masterKeyId, keyIdShort);
+                    }
+
+                    // We always update the display name (which is derived from primary user id)
+                    // and email addresses from user id
+                    writeContactDisplayName(ops, rawContactId, primaryUserId[0]);
+                    writeContactEmail(ops, resolver, rawContactId, masterKeyId);
+                    try {
+                        resolver.applyBatch(ContactsContract.AUTHORITY, ops);
+                    } catch (Exception e) {
+                        Log.w(Constants.TAG, e);
                     }
-                    ops.add(ContentProviderOperation.newInsert(ContactsContract.Data.CONTENT_URI)
-                            .withValueBackReference(ContactsContract.Data.RAW_CONTACT_ID, 0)
-                            .withValue(ContactsContract.Data.MIMETYPE, Constants.CUSTOM_CONTACT_DATA_MIME_TYPE)
-                            .withValue(ContactsContract.Data.DATA1, String.format(context.getString(R.string.contact_show_key), keyIdShort))
-                            .withValue(ContactsContract.Data.DATA2, masterKeyId)
-                            .build());
-                }
-                try {
-                    resolver.applyBatch(ContactsContract.AUTHORITY, ops);
-                } catch (RemoteException e) {
-                    e.printStackTrace();
-                } catch (OperationApplicationException e) {
-                    e.printStackTrace();
                 }
             }
             cursor.close();
         }
+
+        // Delete fingerprints that are no longer present in OK
+        for (String fingerprint : contactFingerprints) {
+            resolver.delete(ContactsContract.RawContacts.CONTENT_URI, ACCOUNT_TYPE_AND_SOURCE_ID_SELECTION,
+                    new String[]{Constants.PACKAGE_NAME, fingerprint});
+        }
+
+    }
+
+    /**
+     * @return a set of all key fingerprints currently present in the contact db
+     */
+    private static Set<String> getRawContactFingerprints(ContentResolver resolver) {
+        HashSet<String> result = new HashSet<String>();
+        Cursor fingerprints = resolver.query(ContactsContract.RawContacts.CONTENT_URI, SOURCE_ID_PROJECTION,
+                ACCOUNT_TYPE_SELECTION, new String[]{Constants.PACKAGE_NAME}, null);
+        if (fingerprints != null) {
+            while (fingerprints.moveToNext()) {
+                result.add(fingerprints.getString(0));
+            }
+            fingerprints.close();
+        }
+        return result;
+    }
+
+    /**
+     * This will search the contact db for a raw contact with a given fingerprint
+     *
+     * @return raw contact id or -1 if not found
+     */
+    private static int findRawContactId(ContentResolver resolver, String fingerprint) {
+        int rawContactId = -1;
+        Cursor raw = resolver.query(ContactsContract.RawContacts.CONTENT_URI, ID_PROJECTION,
+                ACCOUNT_TYPE_AND_SOURCE_ID_SELECTION, new String[]{Constants.PACKAGE_NAME, fingerprint}, null, null);
+        if (raw != null) {
+            if (raw.moveToNext()) {
+                rawContactId = raw.getInt(0);
+            }
+            raw.close();
+        }
+        return rawContactId;
+    }
+
+    /**
+     * Creates a empty raw contact with a given fingerprint
+     */
+    private static void insertContact(ArrayList<ContentProviderOperation> ops, Context context, String fingerprint) {
+        ops.add(ContentProviderOperation.newInsert(ContactsContract.RawContacts.CONTENT_URI)
+                .withValue(ContactsContract.RawContacts.ACCOUNT_NAME, context.getString(R.string.app_name))
+                .withValue(ContactsContract.RawContacts.ACCOUNT_TYPE, Constants.PACKAGE_NAME)
+                .withValue(ContactsContract.RawContacts.SOURCE_ID, fingerprint)
+                .build());
+    }
+
+    /**
+     * Adds a key id to the given raw contact.
+     * <p/>
+     * This creates the link to OK in contact details
+     */
+    private static void writeContactKey(ArrayList<ContentProviderOperation> ops, Context context, int rawContactId,
+                                        long masterKeyId, String keyIdShort) {
+        ops.add(referenceRawContact(ContentProviderOperation.newInsert(ContactsContract.Data.CONTENT_URI), rawContactId)
+                .withValue(ContactsContract.Data.MIMETYPE, Constants.CUSTOM_CONTACT_DATA_MIME_TYPE)
+                .withValue(ContactsContract.Data.DATA1, context.getString(R.string.contact_show_key, keyIdShort))
+                .withValue(ContactsContract.Data.DATA2, masterKeyId)
+                .build());
+    }
+
+    /**
+     * Write all known email addresses of a key (derived from user ids) to a given raw contact
+     */
+    private static void writeContactEmail(ArrayList<ContentProviderOperation> ops, ContentResolver resolver,
+                                          int rawContactId, long masterKeyId) {
+        ops.add(selectByRawContactAndItemType(ContentProviderOperation.newDelete(ContactsContract.Data.CONTENT_URI),
+                rawContactId, ContactsContract.CommonDataKinds.Email.CONTENT_ITEM_TYPE).build());
+        Cursor ids = resolver.query(KeychainContract.UserIds.buildUserIdsUri(Long.toString(masterKeyId)),
+                USER_IDS_PROJECTION, NON_REVOKED_SELECTION, null, null);
+        if (ids != null) {
+            while (ids.moveToNext()) {
+                String[] userId = KeyRing.splitUserId(ids.getString(0));
+                if (userId[1] != null) {
+                    ops.add(referenceRawContact(ContentProviderOperation.newInsert(ContactsContract.Data.CONTENT_URI),
+                            rawContactId)
+                            .withValue(ContactsContract.Data.MIMETYPE,
+                                    ContactsContract.CommonDataKinds.Email.CONTENT_ITEM_TYPE)
+                            .withValue(ContactsContract.CommonDataKinds.Email.DATA, userId[1])
+                            .build());
+                }
+            }
+            ids.close();
+        }
+    }
+
+    private static void writeContactDisplayName(ArrayList<ContentProviderOperation> ops, int rawContactId,
+                                                String displayName) {
+        if (displayName != null) {
+            ops.add(insertOrUpdateForRawContact(ContactsContract.Data.CONTENT_URI, rawContactId,
+                    ContactsContract.CommonDataKinds.StructuredName.CONTENT_ITEM_TYPE)
+                    .withValue(ContactsContract.CommonDataKinds.StructuredName.DISPLAY_NAME, displayName)
+                    .build());
+        }
+    }
+
+    private static ContentProviderOperation.Builder referenceRawContact(ContentProviderOperation.Builder builder,
+                                                                        int rawContactId) {
+        return rawContactId == -1 ?
+                builder.withValueBackReference(ContactsContract.Data.RAW_CONTACT_ID, 0) :
+                builder.withValue(ContactsContract.Data.RAW_CONTACT_ID, rawContactId);
+    }
+
+    private static ContentProviderOperation.Builder insertOrUpdateForRawContact(Uri uri, int rawContactId,
+                                                                                String itemType) {
+        if (rawContactId == -1) {
+            return referenceRawContact(ContentProviderOperation.newInsert(uri), rawContactId).withValue(
+                    ContactsContract.Data.MIMETYPE, itemType);
+        } else {
+            return selectByRawContactAndItemType(ContentProviderOperation.newUpdate(uri), rawContactId, itemType);
+        }
+    }
+
+    private static ContentProviderOperation.Builder selectByRawContactAndItemType(
+            ContentProviderOperation.Builder builder, int rawContactId, String itemType) {
+        return builder.withSelection(RAW_CONTACT_AND_MIMETYPE_SELECTION,
+                new String[]{Integer.toString(rawContactId), itemType});
     }
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ContactSyncAdapterService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ContactSyncAdapterService.java
index 8db9294df..6c4d59a77 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ContactSyncAdapterService.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/ContactSyncAdapterService.java
@@ -30,10 +30,14 @@ import org.sufficientlysecure.keychain.helper.ContactHelper;
 import org.sufficientlysecure.keychain.helper.EmailKeyHelper;
 import org.sufficientlysecure.keychain.util.Log;
 
+import java.util.concurrent.atomic.AtomicBoolean;
+
 public class ContactSyncAdapterService extends Service {
 
     private class ContactSyncAdapter extends AbstractThreadedSyncAdapter {
 
+        private final AtomicBoolean importDone = new AtomicBoolean(false);
+
         public ContactSyncAdapter() {
             super(ContactSyncAdapterService.this, true);
         }
@@ -41,6 +45,8 @@ public class ContactSyncAdapterService extends Service {
         @Override
         public void onPerformSync(Account account, Bundle extras, String authority, ContentProviderClient provider,
                                   final SyncResult syncResult) {
+            importDone.set(false);
+            KeychainApplication.setupAccountAsNeeded(ContactSyncAdapterService.this);
             EmailKeyHelper.importContacts(getContext(), new Messenger(new Handler(Looper.getMainLooper(),
                     new Handler.Callback() {
                         @Override
@@ -48,11 +54,16 @@ public class ContactSyncAdapterService extends Service {
                             Bundle data = msg.getData();
                             switch (msg.arg1) {
                                 case KeychainIntentServiceHandler.MESSAGE_OKAY:
+                                    Log.d(Constants.TAG, "Syncing... Done.");
+                                    synchronized (importDone) {
+                                        importDone.set(true);
+                                        importDone.notifyAll();
+                                    }
                                     return true;
                                 case KeychainIntentServiceHandler.MESSAGE_UPDATE_PROGRESS:
                                     if (data.containsKey(KeychainIntentServiceHandler.DATA_PROGRESS) &&
                                             data.containsKey(KeychainIntentServiceHandler.DATA_PROGRESS_MAX)) {
-                                        Log.d(Constants.TAG, "Progress: " +
+                                        Log.d(Constants.TAG, "Syncing... Progress: " +
                                                 data.getInt(KeychainIntentServiceHandler.DATA_PROGRESS) + "/" +
                                                 data.getInt(KeychainIntentServiceHandler.DATA_PROGRESS_MAX));
                                         return false;
@@ -63,7 +74,14 @@ public class ContactSyncAdapterService extends Service {
                             }
                         }
                     })));
-            KeychainApplication.setupAccountAsNeeded(ContactSyncAdapterService.this);
+            synchronized (importDone) {
+                try {
+                    if (!importDone.get()) importDone.wait();
+                } catch (InterruptedException e) {
+                    Log.w(Constants.TAG, e);
+                    return;
+                }
+            }
             ContactHelper.writeKeysToContacts(ContactSyncAdapterService.this);
         }
     }