Merge pull request #632 from timbray/master

Refactor server query exceptions again

7 files changed, 54 insertions, 63 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java
index f14978b39..5969455bd 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/HkpKeyserver.java
@@ -166,12 +166,12 @@ public class HkpKeyserver extends Keyserver {
         mPort = port;
     }
 
-    private String query(String request) throws QueryException, HttpError {
+    private String query(String request) throws QueryFailedException, HttpError {
         InetAddress ips[];
         try {
             ips = InetAddress.getAllByName(mHost);
         } catch (UnknownHostException e) {
-            throw new QueryException(e.toString());
+            throw new QueryFailedException(e.toString());
         }
         for (int i = 0; i < ips.length; ++i) {
             try {
@@ -196,16 +196,16 @@ public class HkpKeyserver extends Keyserver {
             }
         }
 
-        throw new QueryException("querying server(s) for '" + mHost + "' failed");
+        throw new QueryFailedException("querying server(s) for '" + mHost + "' failed");
     }
 
     @Override
-    public ArrayList<ImportKeysListEntry> search(String query) throws QueryException, TooManyResponses,
-            InsufficientQuery {
+    public ArrayList<ImportKeysListEntry> search(String query) throws QueryFailedException,
+            QueryNeedsRepairException {
         ArrayList<ImportKeysListEntry> results = new ArrayList<ImportKeysListEntry>();
 
         if (query.length() < 3) {
-            throw new InsufficientQuery();
+            throw new QueryTooShortException();
         }
 
         String encodedQuery;
@@ -226,12 +226,12 @@ public class HkpKeyserver extends Keyserver {
                 if (e.getData().toLowerCase(Locale.US).contains("no keys found")) {
                     return results;
                 } else if (e.getData().toLowerCase(Locale.US).contains("too many")) {
-                    throw new TooManyResponses();
+                    throw new TooManyResponsesException();
                 } else if (e.getData().toLowerCase(Locale.US).contains("insufficient")) {
-                    throw new InsufficientQuery();
+                    throw new QueryTooShortException();
                 }
             }
-            throw new QueryException("querying server(s) for '" + mHost + "' failed");
+            throw new QueryFailedException("querying server(s) for '" + mHost + "' failed");
         }
 
         final Matcher matcher = PUB_KEY_LINE.matcher(data);
@@ -287,7 +287,7 @@ public class HkpKeyserver extends Keyserver {
     }
 
     @Override
-    public String get(String keyIdHex) throws QueryException {
+    public String get(String keyIdHex) throws QueryFailedException {
         HttpClient client = new DefaultHttpClient();
         try {
             String query = "http://" + mHost + ":" + mPort +
@@ -296,7 +296,7 @@ public class HkpKeyserver extends Keyserver {
             HttpGet get = new HttpGet(query);
             HttpResponse response = client.execute(get);
             if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
-                throw new QueryException("not found");
+                throw new QueryFailedException("not found");
             }
 
             HttpEntity entity = response.getEntity();
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/KeybaseKeyserver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/KeybaseKeyserver.java
index 5b66b50c5..f9b6abf18 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/KeybaseKeyserver.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/KeybaseKeyserver.java
@@ -34,8 +34,8 @@ public class KeybaseKeyserver extends Keyserver {
     private String mQuery;
 
     @Override
-    public ArrayList<ImportKeysListEntry> search(String query) throws QueryException, TooManyResponses,
-            InsufficientQuery {
+    public ArrayList<ImportKeysListEntry> search(String query) throws QueryFailedException,
+            QueryNeedsRepairException {
         ArrayList<ImportKeysListEntry> results = new ArrayList<ImportKeysListEntry>();
 
         if (query.startsWith("0x")) {
@@ -65,13 +65,13 @@ public class KeybaseKeyserver extends Keyserver {
             }
         } catch (Exception e) {
             Log.e(Constants.TAG, "keybase result parsing error", e);
-            throw new QueryException("Unexpected structure in keybase search result: " + e.getMessage());
+            throw new QueryFailedException("Unexpected structure in keybase search result: " + e.getMessage());
         }
 
         return results;
     }
 
-    private JSONObject getUser(String keybaseId) throws QueryException {
+    private JSONObject getUser(String keybaseId) throws QueryFailedException {
         try {
             return getFromKeybase("_/api/1.0/user/lookup.json?username=", keybaseId);
         } catch (Exception e) {
@@ -79,11 +79,12 @@ public class KeybaseKeyserver extends Keyserver {
             if (keybaseId != null) {
                 detail = ". Query was for user '" + keybaseId + "'";
             }
-            throw new QueryException(e.getMessage() + detail);
+            throw new QueryFailedException(e.getMessage() + detail);
         }
     }
 
-    private ImportKeysListEntry makeEntry(JSONObject match) throws QueryException, JSONException {
+    private ImportKeysListEntry makeEntry(JSONObject match) throws QueryFailedException, JSONException {
+
         final ImportKeysListEntry entry = new ImportKeysListEntry();
         entry.setQuery(mQuery);
 
@@ -127,7 +128,7 @@ public class KeybaseKeyserver extends Keyserver {
         return entry;
     }
 
-    private JSONObject getFromKeybase(String path, String query) throws QueryException {
+    private JSONObject getFromKeybase(String path, String query) throws QueryFailedException {
         try {
             String url = "https://keybase.io/" + path + URLEncoder.encode(query, "utf8");
             Log.d(Constants.TAG, "keybase query: " + url);
@@ -143,29 +144,29 @@ public class KeybaseKeyserver extends Keyserver {
                 try {
                     JSONObject json = new JSONObject(text);
                     if (JWalk.getInt(json, "status", "code") != 0) {
-                        throw new QueryException("Keybase autocomplete search failed");
+                        throw new QueryFailedException("Keybase autocomplete search failed");
                     }
                     return json;
                 } catch (JSONException e) {
-                    throw new QueryException("Keybase.io query returned broken JSON");
+                    throw new QueryFailedException("Keybase.io query returned broken JSON");
                 }
             } else {
                 String message = readAll(conn.getErrorStream(), conn.getContentEncoding());
-                throw new QueryException("Keybase.io query error (status=" + response +
+                throw new QueryFailedException("Keybase.io query error (status=" + response +
                         "): " + message);
             }
         } catch (Exception e) {
-            throw new QueryException("Keybase.io query error");
+            throw new QueryFailedException("Keybase.io query error");
         }
     }
 
     @Override
-    public String get(String id) throws QueryException {
+    public String get(String id) throws QueryFailedException {
         try {
             JSONObject user = getUser(id);
             return JWalk.getString(user, "them", "public_keys", "primary", "bundle");
         } catch (Exception e) {
-            throw new QueryException(e.getMessage());
+            throw new QueryFailedException(e.getMessage());
         }
     }
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java
index 19591eda8..868f543f0 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/Keyserver.java
@@ -24,19 +24,23 @@ import java.io.InputStream;
 import java.util.List;
 
 public abstract class Keyserver {
-    public static class QueryException extends Exception {
+    public static class QueryFailedException extends Exception {
         private static final long serialVersionUID = 2703768928624654512L;
 
-        public QueryException(String message) {
+        public QueryFailedException(String message) {
             super(message);
         }
     }
 
-    public static class TooManyResponses extends Exception {
+    public static class QueryNeedsRepairException extends Exception {
+        private static final long serialVersionUID = 2693768928624654512L;
+    }
+
+    public static class TooManyResponsesException extends QueryNeedsRepairException {
         private static final long serialVersionUID = 2703768928624654513L;
     }
 
-    public static class InsufficientQuery extends Exception {
+    public static class QueryTooShortException extends QueryNeedsRepairException {
         private static final long serialVersionUID = 2703768928624654514L;
     }
 
@@ -44,10 +48,10 @@ public abstract class Keyserver {
         private static final long serialVersionUID = -507574859137295530L;
     }
 
-    abstract List<ImportKeysListEntry> search(String query) throws QueryException, TooManyResponses,
-            InsufficientQuery;
+    abstract List<ImportKeysListEntry> search(String query) throws QueryFailedException,
+            QueryNeedsRepairException;
 
-    abstract String get(String keyIdHex) throws QueryException;
+    abstract String get(String keyIdHex) throws QueryFailedException;
 
     abstract void add(String armoredKey) throws AddKeyException;
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java
index e93d717e1..d9bd9b782 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java
@@ -211,7 +211,7 @@ public class ImportKeysListFragment extends ListFragment implements
 
     @Override
     public Loader<AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>>
-                                        onCreateLoader(int id, Bundle args) {
+    onCreateLoader(int id, Bundle args) {
         switch (id) {
             case LOADER_ID_BYTES: {
                 InputData inputData = getInputData(mKeyBytes, mDataUri);
@@ -273,38 +273,30 @@ public class ImportKeysListFragment extends ListFragment implements
                 break;
 
             case LOADER_ID_SERVER_QUERY:
+            case LOADER_ID_KEYBASE:
 
+                // TODO: possibly fine-tune message building for these two cases
                 if (error == null) {
                     AppMsg.makeText(
                             getActivity(), getResources().getQuantityString(R.plurals.keys_found,
                                     mAdapter.getCount(), mAdapter.getCount()),
                             AppMsg.STYLE_INFO
                     ).show();
-                } else if (error instanceof Keyserver.InsufficientQuery) {
+                } else if (error instanceof Keyserver.QueryTooShortException) {
                     AppMsg.makeText(getActivity(), R.string.error_keyserver_insufficient_query,
                             AppMsg.STYLE_ALERT).show();
-                } else if (error instanceof Keyserver.QueryException) {
-                    AppMsg.makeText(getActivity(), R.string.error_keyserver_query,
-                            AppMsg.STYLE_ALERT).show();
-                } else if (error instanceof Keyserver.TooManyResponses) {
+                } else if (error instanceof Keyserver.TooManyResponsesException) {
                     AppMsg.makeText(getActivity(), R.string.error_keyserver_too_many_responses,
                             AppMsg.STYLE_ALERT).show();
+                } else if (error instanceof Keyserver.QueryFailedException) {
+                    Log.d(Constants.TAG,
+                            "Unrecoverable keyserver query error: " + error.getLocalizedMessage());
+                    String alert = getActivity().getString(R.string.error_searching_keys);
+                    alert = alert + " (" + error.getLocalizedMessage() + ")";
+                    AppMsg.makeText(getActivity(), alert, AppMsg.STYLE_ALERT).show();
                 }
                 break;
 
-            case LOADER_ID_KEYBASE:
-
-                if (error == null) {
-                    AppMsg.makeText(
-                            getActivity(), getResources().getQuantityString(R.plurals.keys_found,
-                                    mAdapter.getCount(), mAdapter.getCount()),
-                            AppMsg.STYLE_INFO
-                    ).show();
-                }  else if (error instanceof Keyserver.QueryException) {
-                    AppMsg.makeText(getActivity(), R.string.error_keyserver_query,
-                            AppMsg.STYLE_ALERT).show();
-                }
-
             default:
                 break;
         }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListKeybaseLoader.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListKeybaseLoader.java
index 0fdc019d0..e8c5da5a7 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListKeybaseLoader.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListKeybaseLoader.java
@@ -94,14 +94,10 @@ public class ImportKeysListKeybaseLoader
 
             mEntryList.addAll(searchResult);
             mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, null);
-        } catch (Keyserver.InsufficientQuery e) {
+        } catch (Keyserver.QueryFailedException e) {
             mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, e);
-        } catch (Keyserver.QueryException e) {
-            mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, e);
-        } catch (Keyserver.TooManyResponses e) {
+        } catch (Keyserver.QueryNeedsRepairException e) {
             mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, e);
         }
-
     }
-
 }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListServerLoader.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListServerLoader.java
index 1b8d7d30e..4eb6f158b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListServerLoader.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysListServerLoader.java
@@ -116,11 +116,9 @@ public class ImportKeysListServerLoader
                 mEntryList.addAll(searchResult);
             }
             mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, null);
-        } catch (Keyserver.InsufficientQuery e) {
+        } catch (Keyserver.QueryFailedException e) {
             mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, e);
-        } catch (Keyserver.QueryException e) {
-            mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, e);
-        } catch (Keyserver.TooManyResponses e) {
+        } catch (Keyserver.QueryNeedsRepairException e) {
             mEntryListWrapper = new AsyncTaskResultWrapper<ArrayList<ImportKeysListEntry>>(mEntryList, e);
         }
     }
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 4d39b6273..1ba8a6d2d 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -295,9 +295,9 @@
     <string name="error_jelly_bean_needed">You need Android 4.1 to use Android\'s NFC Beam feature!</string>
     <string name="error_nfc_needed">NFC is not available on your device!</string>
     <string name="error_nothing_import">Nothing to import!</string>
-    <string name="error_keyserver_insufficient_query">Insufficient server query</string>
-    <string name="error_keyserver_query">Querying keyserver failed</string>
-    <string name="error_keyserver_too_many_responses">Too many possible keys. Please refine your query!</string>
+    <string name="error_keyserver_insufficient_query">Key search query too short</string>
+    <string name="error_searching_keys">Unrecoverable error searching for keys at server</string>
+    <string name="error_keyserver_too_many_responses">Key search query returned too many candidates; Please refine query</string>
     <string name="error_import_file_no_content">File has no content</string>
     <string name="error_generic_report_bug">A generic error occurred, please create a new bug report for OpenKeychain.</string>
     <plurals name="error_import_non_pgp_part">