ImportKeys: Refactoring list entries

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/FacebookKeyserver.java

@@ -23,10 +23,6 @@ import android.net.Uri;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 
-
-import okhttp3.OkHttpClient;
-import okhttp3.Request;
-import okhttp3.Response;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.pgp.PgpHelper;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
@@ -39,12 +35,15 @@ import org.sufficientlysecure.keychain.util.TlsHelper;
 
 import java.io.IOException;
 import java.net.Proxy;
-
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.regex.Matcher;
 
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+
 public class FacebookKeyserver extends Keyserver {
 
     private static final String FB_KEY_URL_FORMAT
@@ -157,10 +156,7 @@ public class FacebookKeyserver extends Keyserver {
 
         UncachedPublicKey key = ring.getPublicKey();
 
-        entry.setPrimaryUserId(key.getPrimaryUserIdWithFallback());
         entry.setUserIds(key.getUnorderedUserIds());
-        entry.updateMergedUserIds();
-
         entry.setPrimaryUserId(key.getPrimaryUserIdWithFallback());
 
         entry.setKeyId(key.getKeyId());

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ImportKeysList.java

@@ -91,23 +91,15 @@ public class ImportKeysList extends ArrayList<ImportKeysListEntry> {
             }
         }
 
-        ArrayList<String> incomingIDs = incoming.getUserIds();
+        if (incomingFromHkpServer) {
-        ArrayList<String> existingIDs = existing.getUserIds();
+            // Mail addresses returned by HKP servers are preferred over keybase.io IDs
-        for (String incomingID : incomingIDs) {
+            existing.setPrimaryUserId(incoming.getPrimaryUserId());
-            if (!existingIDs.contains(incomingID)) {
+            modified = true;
-                // prepend  HKP server results to the start of the list,
-                // so that the UI (for cloud key search, which is picking the first list item)
-                // shows the right main email address, as mail addresses returned by HKP servers
-                // are preferred over keybase.io IDs
-                if (incomingFromHkpServer) {
-                    existingIDs.add(0, incomingID);
-                } else {
-                    existingIDs.add(incomingID);
-                }
-                modified = true;
-            }
         }
-        existing.updateMergedUserIds();
+
+        if (existing.addUserIds(incoming.getUserIds()))
+            modified = true;
+
         return modified;
     }
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/ImportKeysListEntry.java

@@ -22,7 +22,6 @@ import android.os.Parcel;
 import android.os.Parcelable;
 
 import org.openintents.openpgp.util.OpenPgpUtils;
-import org.sufficientlysecure.keychain.R;
 import org.sufficientlysecure.keychain.pgp.KeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
@@ -30,9 +29,13 @@ import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
 
 public class ImportKeysListEntry implements Serializable, Parcelable {
     private static final long serialVersionUID = -7797972103284992662L;
@@ -40,6 +43,8 @@ public class ImportKeysListEntry implements Serializable, Parcelable {
     private byte[] mEncodedRing;
     private ArrayList<String> mUserIds;
     private HashMap<String, HashSet<String>> mMergedUserIds;
+    private ArrayList<Map.Entry<String, HashSet<String>>> mSortedUserIds;
+
     private long mKeyId;
     private String mKeyIdHex;
     private boolean mRevoked;
@@ -209,15 +214,6 @@ public class ImportKeysListEntry implements Serializable, Parcelable {
         mSecretKey = secretKey;
     }
 
-    public ArrayList<String> getUserIds() {
-        return mUserIds;
-    }
-
-    public void setUserIds(ArrayList<String> userIds) {
-        mUserIds = userIds;
-        updateMergedUserIds();
-    }
-
     public String getPrimaryUserId() {
         return mPrimaryUserId;
     }
@@ -258,8 +254,45 @@ public class ImportKeysListEntry implements Serializable, Parcelable {
         mOrigins.add(origin);
     }
 
-    public HashMap<String, HashSet<String>> getMergedUserIds() {
+    public List<String> getUserIds() {
-        return mMergedUserIds;
+        // To ensure choerency, use methods of this class to edit the list
+        return Collections.unmodifiableList(mUserIds);
+    }
+
+    public ArrayList<Map.Entry<String, HashSet<String>>> getSortedUserIds() {
+        if (mSortedUserIds == null)
+            sortMergedUserIds();
+
+        return mSortedUserIds;
+    }
+
+    public void setUserIds(ArrayList<String> userIds) {
+        mUserIds = userIds;
+        updateMergedUserIds();
+    }
+
+    public boolean addUserIds(List<String> userIds) {
+        boolean modified = false;
+        for (String uid : userIds) {
+            if (!mUserIds.contains(uid)) {
+                mUserIds.add(uid);
+                modified = true;
+            }
+        }
+
+        if (modified)
+            updateMergedUserIds();
+
+        return modified;
+    }
+
+    public ArrayList<String> getKeybaseUserIds() {
+        ArrayList<String> keybaseUserIds = new ArrayList<>();
+        for (String s : mUserIds) {
+            if (s.contains(":"))
+                keybaseUserIds.add(s);
+        }
+        return keybaseUserIds;
     }
 
     /**
@@ -288,11 +321,6 @@ public class ImportKeysListEntry implements Serializable, Parcelable {
         mUserIds = key.getUnorderedUserIds();
         updateMergedUserIds();
 
-        // if there was no user id flagged as primary, use the first one
-        if (mPrimaryUserId == null) {
-            mPrimaryUserId = context.getString(R.string.user_id_none);
-        }
-
         mKeyId = key.getKeyId();
         mKeyIdHex = KeyFormattingUtils.convertKeyIdToHex(mKeyId);
 
@@ -306,7 +334,7 @@ public class ImportKeysListEntry implements Serializable, Parcelable {
         mAlgorithm = KeyFormattingUtils.getAlgorithmInfo(context, algorithm, mBitStrength, mCurveOid);
     }
 
-    public void updateMergedUserIds() {
+    private void updateMergedUserIds() {
         mMergedUserIds = new HashMap<>();
         for (String userId : mUserIds) {
             OpenPgpUtils.UserId userIdSplit = KeyRing.splitUserId(userId);
@@ -331,15 +359,27 @@ public class ImportKeysListEntry implements Serializable, Parcelable {
                 mMergedUserIds.put(userId, new HashSet<String>());
             }
         }
+
+        mSortedUserIds = null;
     }
 
-    public ArrayList<String> getKeybaseUserIds() {
+    private void sortMergedUserIds() {
-        ArrayList<String> keybaseUserIds = new ArrayList<>();
+        mSortedUserIds = new ArrayList<>(mMergedUserIds.entrySet());
-        for (String s : mUserIds) {
+
-            if (s.contains(":"))
+        Collections.sort(mSortedUserIds, new Comparator<Map.Entry<String, HashSet<String>>>() {
-                keybaseUserIds.add(s);
+            @Override
-        }
+            public int compare(Map.Entry<String, HashSet<String>> entry1,
-        return keybaseUserIds;
+                               Map.Entry<String, HashSet<String>> entry2) {
+
+                // sort keybase UserIds after non-Keybase
+                boolean e1IsKeybase = entry1.getKey().contains(":");
+                boolean e2IsKeybase = entry2.getKey().contains(":");
+                if (e1IsKeybase != e2IsKeybase) {
+                    return (e1IsKeybase) ? 1 : -1;
+                }
+                return entry1.getKey().compareTo(entry2.getKey());
+            }
+        });
     }
 
 }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysAdapter.java

@@ -55,8 +55,6 @@ import org.sufficientlysecure.keychain.util.ParcelableFileCache;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -152,7 +150,7 @@ public class ImportKeysAdapter extends RecyclerView.Adapter<ImportKeysAdapter.Vi
         b.setExpired(entry.isExpired());
         b.setRevoked(entry.isRevoked());
 
-        String userId = entry.getUserIds().get(0); // main user id
+        String userId = entry.getPrimaryUserId(); // main user id
         OpenPgpUtils.UserId userIdSplit = KeyRing.splitUserId(userId);
 
         b.setAlgorithm(entry.getAlgorithm());
@@ -213,21 +211,7 @@ public class ImportKeysAdapter extends RecyclerView.Adapter<ImportKeysAdapter.Vi
             realUserIdsPlusKeybase.addAll(keyRing.getUnorderedUserIds());
             entry.setUserIds(realUserIdsPlusKeybase);
 
-            HashMap<String, HashSet<String>> mergedUserIds = entry.getMergedUserIds();
+            ArrayList<Map.Entry<String, HashSet<String>>> sortedIds = entry.getSortedUserIds();
-            ArrayList<Map.Entry<String, HashSet<String>>> sortedIds = new ArrayList<>(mergedUserIds.entrySet());
-            Collections.sort(sortedIds, new Comparator<Map.Entry<String, HashSet<String>>>() {
-                @Override
-                public int compare(Map.Entry<String, HashSet<String>> entry1, Map.Entry<String, HashSet<String>> entry2) {
-                    // sort keybase UserIds after non-Keybase
-                    boolean e1IsKeybase = entry1.getKey().contains(":");
-                    boolean e2IsKeybase = entry2.getKey().contains(":");
-                    if (e1IsKeybase != e2IsKeybase) {
-                        return (e1IsKeybase) ? 1 : -1;
-                    }
-                    return entry1.getKey().compareTo(entry2.getKey());
-                }
-            });
-
             for (Map.Entry<String, HashSet<String>> pair : sortedIds) {
                 String cUserId = pair.getKey();
                 HashSet<String> cEmails = pair.getValue();