make KeyIdExtractor code more readable

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainExternalContract.java

@@ -25,6 +25,8 @@ import org.sufficientlysecure.keychain.Constants;
 
 
 public class KeychainExternalContract {
+    public static final int KEY_STATUS_UNVERIFIED = 1;
+    public static final int KEY_STATUS_VERIFIED = 2;
 
     // this is in KeychainExternalContract already, but we want to be double
     // sure this isn't mixed up with the internal one!

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/KeychainExternalProvider.java

@@ -170,10 +170,9 @@ public class KeychainExternalProvider extends ContentProvider implements SimpleC
                 // we take the minimum (>0) here, where "1" is "verified by known secret key", "2" is "self-certified"
                 projectionMap.put(EmailStatus.USER_ID_STATUS, "CASE ( MIN (" + Certs.VERIFIED + " ) ) "
                         // remap to keep this provider contract independent from our internal representation
-                        + " WHEN NULL THEN 1"
+                        + " WHEN " + Certs.VERIFIED_SELF + " THEN " + KeychainExternalContract.KEY_STATUS_UNVERIFIED
-                        + " WHEN " + Certs.VERIFIED_SELF + " THEN 1"
+                        + " WHEN " + Certs.VERIFIED_SECRET + " THEN " + KeychainExternalContract.KEY_STATUS_VERIFIED
-                        + " WHEN " + Certs.VERIFIED_SECRET + " THEN 2"
+                        + " WHEN NULL THEN " + KeychainExternalContract.KEY_STATUS_UNVERIFIED
-                        + " WHEN NULL THEN NULL"
                         + " END AS " + EmailStatus.USER_ID_STATUS);
                 projectionMap.put(EmailStatus.USER_ID, Tables.USER_PACKETS + "." + UserPackets.USER_ID + " AS " + EmailStatus.USER_ID);
                 projectionMap.put(EmailStatus.MASTER_KEY_ID,

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpServiceKeyIdExtractor.java

@@ -11,10 +11,12 @@ import android.content.ContentResolver;
 import android.content.Intent;
 import android.database.Cursor;
 import android.net.Uri;
+import android.support.annotation.NonNull;
 import android.support.annotation.VisibleForTesting;
 
 import org.openintents.openpgp.util.OpenPgpApi;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.provider.KeychainExternalContract;
 import org.sufficientlysecure.keychain.provider.KeychainExternalContract.EmailStatus;
 import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
 import org.sufficientlysecure.keychain.util.Log;
@@ -76,97 +78,108 @@ class OpenPgpServiceKeyIdExtractor {
         return result;
     }
 
-    private KeyIdResult returnKeyIdsFromEmails(Intent data, String[] encryptionUserIds, String callingPackageName) {
+    private KeyIdResult returnKeyIdsFromEmails(Intent data, String[] encryptionAddresses, String callingPackageName) {
-        boolean hasUserIds = (encryptionUserIds != null && encryptionUserIds.length > 0);
+        boolean hasAddresses = (encryptionAddresses != null && encryptionAddresses.length > 0);
 
-        boolean anyKeyNotVerified = false;
+        boolean allKeysConfirmed = false;
         HashSet<Long> keyIds = new HashSet<>();
         ArrayList<String> missingEmails = new ArrayList<>();
         ArrayList<String> duplicateEmails = new ArrayList<>();
-        HashMap<String,KeyRow> keyRows = new HashMap<>();
-        if (hasUserIds) {
-            Uri queryUri = EmailStatus.CONTENT_URI.buildUpon().appendPath(callingPackageName).build();
-            Cursor cursor = contentResolver.query(queryUri, PROJECTION_KEY_SEARCH, null, encryptionUserIds, null);
-            if (cursor == null) {
-                throw new IllegalStateException("Internal error, received null cursor!");
-            }
 
-            try {
+        if (hasAddresses) {
-                while (cursor.moveToNext()) {
+            HashMap<String, UserIdStatus> keyRows = getStatusMapForQueriedAddresses(encryptionAddresses, callingPackageName);
-                    String queryAddress = cursor.getString(INDEX_EMAIL_ADDRESS);
-                    Long masterKeyId = cursor.isNull(INDEX_MASTER_KEY_ID) ? null : cursor.getLong(INDEX_MASTER_KEY_ID);
-                    int verified = cursor.getInt(INDEX_EMAIL_STATUS);
 
-                    KeyRow row = new KeyRow(masterKeyId, verified == 2);
+            boolean anyKeyNotVerified = false;
-                    if (!keyRows.containsKey(queryAddress)) {
+            for (Entry<String, UserIdStatus> entry : keyRows.entrySet()) {
-                        keyRows.put(queryAddress, row);
-                        continue;
-                    }
-
-                    KeyRow previousRow = keyRows.get(queryAddress);
-                    if (previousRow.masterKeyId == null) {
-                        keyRows.put(queryAddress, row);
-                    } else if (!previousRow.verified && row.verified) {
-                        keyRows.put(queryAddress, row);
-                    } else if (previousRow.verified == row.verified) {
-                        previousRow.hasDuplicate = true;
-                    }
-                }
-            } finally {
-                cursor.close();
-            }
-
-            for (Entry<String, KeyRow> entry : keyRows.entrySet()) {
                 String queriedAddress = entry.getKey();
-                KeyRow keyRow = entry.getValue();
+                UserIdStatus userIdStatus = entry.getValue();
 
-                if (keyRow.masterKeyId == null) {
+                if (userIdStatus.masterKeyId == null) {
                     missingEmails.add(queriedAddress);
                     continue;
                 }
 
-                keyIds.add(keyRow.masterKeyId);
+                keyIds.add(userIdStatus.masterKeyId);
 
-                if (keyRow.hasDuplicate) {
+                if (userIdStatus.hasDuplicate) {
                     duplicateEmails.add(queriedAddress);
                 }
 
-                if (!keyRow.verified) {
+                if (!userIdStatus.verified) {
                     anyKeyNotVerified = true;
                 }
             }
 
-            if (keyRows.size() != encryptionUserIds.length) {
+            if (keyRows.size() != encryptionAddresses.length) {
                 Log.e(Constants.TAG, "Number of rows doesn't match number of retrieved rows! Probably a bug?");
             }
+
+            allKeysConfirmed = !anyKeyNotVerified;
         }
 
-        long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(keyIds);
-        PendingIntent pi = apiPendingIntentFactory.createSelectPublicKeyPendingIntent(data, keyIdsArray,
-                missingEmails, duplicateEmails, false);
-
         if (!missingEmails.isEmpty()) {
-            return createMissingKeysResult(pi);
+            return createMissingKeysResult(data, keyIds, missingEmails, duplicateEmails);
         }
 
         if (!duplicateEmails.isEmpty()) {
-            return createDuplicateKeysResult(pi);
+            return createDuplicateKeysResult(data, keyIds, missingEmails, duplicateEmails);
         }
 
         if (keyIds.isEmpty()) {
-            return createNoKeysResult(pi);
+            return createNoKeysResult(data, keyIds, missingEmails, duplicateEmails);
         }
 
-        boolean allKeysConfirmed = !anyKeyNotVerified;
         return createKeysOkResult(keyIds, allKeysConfirmed);
     }
 
-    private static class KeyRow {
+    /** This method queries the KeychainExternalProvider for all addresses given in encryptionUserIds.
+     * It returns a map with one UserIdStatus per queried address. If multiple key candidates exist,
+     * the one with the highest verification status is selected. If two candidates with the same
+     * verification status exist, the first one is returned and marked as having a duplicate.
+     */
+    @NonNull
+    private HashMap<String, UserIdStatus> getStatusMapForQueriedAddresses(String[] encryptionUserIds, String callingPackageName) {
+        HashMap<String,UserIdStatus> keyRows = new HashMap<>();
+        Uri queryUri = EmailStatus.CONTENT_URI.buildUpon().appendPath(callingPackageName).build();
+        Cursor cursor = contentResolver.query(queryUri, PROJECTION_KEY_SEARCH, null, encryptionUserIds, null);
+        if (cursor == null) {
+            throw new IllegalStateException("Internal error, received null cursor!");
+        }
+
+        try {
+            while (cursor.moveToNext()) {
+                String queryAddress = cursor.getString(INDEX_EMAIL_ADDRESS);
+                Long masterKeyId = cursor.isNull(INDEX_MASTER_KEY_ID) ? null : cursor.getLong(INDEX_MASTER_KEY_ID);
+                boolean isVerified = cursor.getInt(INDEX_EMAIL_STATUS) == KeychainExternalContract.KEY_STATUS_VERIFIED;
+                UserIdStatus userIdStatus = new UserIdStatus(masterKeyId, isVerified);
+
+                boolean seenBefore = keyRows.containsKey(queryAddress);
+                if (!seenBefore) {
+                    keyRows.put(queryAddress, userIdStatus);
+                    continue;
+                }
+
+                UserIdStatus previousUserIdStatus = keyRows.get(queryAddress);
+                if (previousUserIdStatus.masterKeyId == null) {
+                    keyRows.put(queryAddress, userIdStatus);
+                } else if (!previousUserIdStatus.verified && userIdStatus.verified) {
+                    keyRows.put(queryAddress, userIdStatus);
+                } else if (previousUserIdStatus.verified == userIdStatus.verified) {
+                    previousUserIdStatus.hasDuplicate = true;
+                }
+            }
+        } finally {
+            cursor.close();
+        }
+        return keyRows;
+    }
+
+    private static class UserIdStatus {
         private final Long masterKeyId;
         private final boolean verified;
         private boolean hasDuplicate;
 
-        KeyRow(Long masterKeyId, boolean verified) {
+        UserIdStatus(Long masterKeyId, boolean verified) {
             this.masterKeyId = masterKeyId;
             this.verified = verified;
         }
@@ -251,19 +264,34 @@ class OpenPgpServiceKeyIdExtractor {
         return new KeyIdResult(encryptKeyIds, allKeysConfirmed, KeyIdResultStatus.OK);
     }
 
-    private static KeyIdResult createNoKeysResult(PendingIntent pendingIntent) {
+    private KeyIdResult createNoKeysResult(Intent data,
-        return new KeyIdResult(pendingIntent, KeyIdResultStatus.NO_KEYS);
+            HashSet<Long> selectedKeyIds, ArrayList<String> missingEmails, ArrayList<String> duplicateEmails) {
+        long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(selectedKeyIds);
+        PendingIntent selectKeyPendingIntent = apiPendingIntentFactory.createSelectPublicKeyPendingIntent(
+                data, keyIdsArray, missingEmails, duplicateEmails, false);
+
+        return new KeyIdResult(selectKeyPendingIntent, KeyIdResultStatus.NO_KEYS);
     }
 
-    private static KeyIdResult createNoKeysResult() {
+    private KeyIdResult createDuplicateKeysResult(Intent data,
+            HashSet<Long> selectedKeyIds, ArrayList<String> missingEmails, ArrayList<String> duplicateEmails) {
+        long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(selectedKeyIds);
+        PendingIntent selectKeyPendingIntent = apiPendingIntentFactory.createSelectPublicKeyPendingIntent(
+                data, keyIdsArray, missingEmails, duplicateEmails, false);
+
+        return new KeyIdResult(selectKeyPendingIntent, KeyIdResultStatus.DUPLICATE);
+    }
+
+    private KeyIdResult createMissingKeysResult(Intent data,
+            HashSet<Long> selectedKeyIds, ArrayList<String> missingEmails, ArrayList<String> duplicateEmails) {
+        long[] keyIdsArray = KeyFormattingUtils.getUnboxedLongArray(selectedKeyIds);
+        PendingIntent selectKeyPendingIntent = apiPendingIntentFactory.createSelectPublicKeyPendingIntent(
+                data, keyIdsArray, missingEmails, duplicateEmails, false);
+
+        return new KeyIdResult(selectKeyPendingIntent, KeyIdResultStatus.MISSING);
+    }
+
+    private KeyIdResult createNoKeysResult() {
         return new KeyIdResult(null, KeyIdResultStatus.NO_KEYS_ERROR);
     }
-
-    private static KeyIdResult createDuplicateKeysResult(PendingIntent pendingIntent) {
-        return new KeyIdResult(pendingIntent, KeyIdResultStatus.DUPLICATE);
-    }
-
-    private static KeyIdResult createMissingKeysResult(PendingIntent pendingIntent) {
-        return new KeyIdResult(pendingIntent, KeyIdResultStatus.MISSING);
-    }
 }