export: more cleanup and splitting into subroutines

4 files changed, 54 insertions, 58 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ExportOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ExportOperation.java
index 115003da4..2eefd869b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ExportOperation.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ExportOperation.java
@@ -70,7 +70,7 @@ public class ExportOperation extends BaseOperation<ExportKeyringParcel> {
     };
     private static final int INDEX_MASTER_KEY_ID = 0;
     private static final int INDEX_PUBKEY_DATA = 1;
-    private static final int INDEX_PRIVKEY_DATA = 2;
+    private static final int INDEX_SECKEY_DATA = 2;
     private static final int INDEX_HAS_ANY_SECRET = 3;
 
     public ExportOperation(Context context, ProviderHelper providerHelper, Progressable
@@ -93,16 +93,10 @@ public class ExportOperation extends BaseOperation<ExportKeyringParcel> {
             log.add(LogType.MSG_EXPORT_ALL, 0);
         }
 
-        // do we have a file name?
-        if (exportInput.mOutputUri == null) {
-            log.add(LogType.MSG_EXPORT_ERROR_NO_URI, 1);
-            return new ExportResult(ExportResult.RESULT_ERROR, log);
-        }
-
         try {
             OutputStream outStream = mProviderHelper.getContentResolver().openOutputStream(exportInput.mOutputUri);
             outStream = new BufferedOutputStream(outStream);
-            return exportKeyRings(log, exportInput.mMasterKeyIds, exportInput.mExportSecret, outStream);
+            return exportKeysToStream(log, exportInput.mMasterKeyIds, exportInput.mExportSecret, outStream);
         } catch (FileNotFoundException e) {
             log.add(LogType.MSG_EXPORT_ERROR_URI_OPEN, 1);
             return new ExportResult(ExportResult.RESULT_ERROR, log);
@@ -110,8 +104,7 @@ public class ExportOperation extends BaseOperation<ExportKeyringParcel> {
 
     }
 
-    ExportResult exportKeyRings(OperationLog log, long[] masterKeyIds, boolean exportSecret,
-                                OutputStream outStream) {
+    ExportResult exportKeysToStream(OperationLog log, long[] masterKeyIds, boolean exportSecret, OutputStream outStream) {
 
         int okSecret = 0, okPublic = 0, progress = 0;
 
@@ -133,50 +126,21 @@ public class ExportOperation extends BaseOperation<ExportKeyringParcel> {
             while (!cursor.isAfterLast()) {
 
                 long keyId = cursor.getLong(INDEX_MASTER_KEY_ID);
-                ArmoredOutputStream arOutStream = null;
-
-                // Create an output stream
-                try {
-                    arOutStream = new ArmoredOutputStream(outStream);
-
-                    log.add(LogType.MSG_EXPORT_PUBLIC, 1, KeyFormattingUtils.beautifyKeyId(keyId));
-
-                    byte[] data = cursor.getBlob(INDEX_PUBKEY_DATA);
-                    CanonicalizedKeyRing ring = UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true);
-                    ring.encode(arOutStream);
+                log.add(LogType.MSG_EXPORT_PUBLIC, 1, KeyFormattingUtils.beautifyKeyId(keyId));
 
+                if (writePublicKeyToStream(log, outStream, cursor)) {
                     okPublic += 1;
-                } catch (PgpGeneralException e) {
-                    log.add(LogType.MSG_EXPORT_ERROR_KEY, 2);
-                } finally {
-                    if (arOutStream != null) {
-                        arOutStream.close();
-                    }
-                    arOutStream = null;
-                }
 
-                if (exportSecret && cursor.getInt(INDEX_HAS_ANY_SECRET) > 0) {
-                    try {
-                        arOutStream = new ArmoredOutputStream(outStream);
-
-                        // export secret key part
+                    boolean hasSecret = cursor.getInt(INDEX_HAS_ANY_SECRET) > 0;
+                    if (exportSecret && hasSecret) {
                         log.add(LogType.MSG_EXPORT_SECRET, 2, KeyFormattingUtils.beautifyKeyId(keyId));
-                        byte[] data = cursor.getBlob(INDEX_PRIVKEY_DATA);
-                        CanonicalizedKeyRing ring = UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true);
-                        ring.encode(arOutStream);
-
-                        okSecret += 1;
-                    } catch (PgpGeneralException e) {
-                        log.add(LogType.MSG_EXPORT_ERROR_KEY, 2);
-                    } finally {
-                        if (arOutStream != null) {
-                            arOutStream.close();
+                        if (writeSecretKeyToStream(log, outStream, cursor)) {
+                            okSecret += 1;
                         }
                     }
                 }
 
                 updateProgress(progress++, numKeys);
-
                 cursor.moveToNext();
             }
 
@@ -192,9 +156,7 @@ public class ExportOperation extends BaseOperation<ExportKeyringParcel> {
             } catch (Exception e) {
                 Log.e(Constants.TAG, "error closing stream", e);
             }
-            if (cursor != null) {
-                cursor.close();
-            }
+            cursor.close();
         }
 
         log.add(LogType.MSG_EXPORT_SUCCESS, 1);
@@ -202,6 +164,48 @@ public class ExportOperation extends BaseOperation<ExportKeyringParcel> {
 
     }
 
+    private boolean writePublicKeyToStream(OperationLog log, OutputStream outStream, Cursor cursor)
+            throws IOException {
+
+        ArmoredOutputStream arOutStream = null;
+
+        try {
+            arOutStream = new ArmoredOutputStream(outStream);
+            byte[] data = cursor.getBlob(INDEX_PUBKEY_DATA);
+            CanonicalizedKeyRing ring = UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true);
+            ring.encode(arOutStream);
+
+        } catch (PgpGeneralException e) {
+            log.add(LogType.MSG_EXPORT_ERROR_KEY, 2);
+        } finally {
+            if (arOutStream != null) {
+                arOutStream.close();
+            }
+        }
+        return true;
+    }
+
+    private boolean writeSecretKeyToStream(OperationLog log, OutputStream outStream, Cursor cursor)
+            throws IOException {
+
+        ArmoredOutputStream arOutStream = null;
+
+        try {
+            arOutStream = new ArmoredOutputStream(outStream);
+            byte[] data = cursor.getBlob(INDEX_SECKEY_DATA);
+            CanonicalizedKeyRing ring = UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true);
+            ring.encode(arOutStream);
+
+        } catch (PgpGeneralException e) {
+            log.add(LogType.MSG_EXPORT_ERROR_KEY, 2);
+        } finally {
+            if (arOutStream != null) {
+                arOutStream.close();
+            }
+        }
+        return true;
+    }
+
     private Cursor queryForKeys(long[] masterKeyIds) {
 
         String selection = null, selectionArgs[] = null;
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
index a22346f4f..6ac34faa2 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java
@@ -767,7 +767,6 @@ public abstract class OperationResult implements Parcelable {
         MSG_EXPORT_PUBLIC (LogLevel.DEBUG, R.string.msg_export_public),
         MSG_EXPORT_SECRET (LogLevel.DEBUG, R.string.msg_export_secret),
         MSG_EXPORT_ALL (LogLevel.START, R.string.msg_export_all),
-        MSG_EXPORT_ERROR_NO_URI (LogLevel.ERROR, R.string.msg_export_error_no_uri),
         MSG_EXPORT_ERROR_URI_OPEN (LogLevel.ERROR, R.string.msg_export_error_uri_open),
         MSG_EXPORT_ERROR_DB (LogLevel.ERROR, R.string.msg_export_error_db),
         MSG_EXPORT_ERROR_IO (LogLevel.ERROR, R.string.msg_export_error_io),
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index 81743213f..aa25b2aa7 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -1293,16 +1293,11 @@
         <item quantity="one">"Exporting one key"</item>
         <item quantity="other">"Exporting %d keys"</item>
     </plurals>
-    <string name="msg_export_file_name">"Filename: %s"</string>
     <string name="msg_export_all">"Exporting all keys"</string>
     <string name="msg_export_public">"Exporting public key %s"</string>
     <string name="msg_export_upload_public">"Uploading public key %s"</string>
     <string name="msg_export_secret">"Exporting secret key %s"</string>
-    <string name="msg_export_error_no_file">"No filename specified!"</string>
-    <string name="msg_export_error_fopen">"Error opening file!"</string>
-    <string name="msg_export_error_no_uri">"No URI specified!"</string>
     <string name="msg_export_error_uri_open">"Error opening URI stream!"</string>
-    <string name="msg_export_error_storage">"Storage is not ready for writing!"</string>
     <string name="msg_export_error_db">"Database error!"</string>
     <string name="msg_export_error_io">"Input/output error!"</string>
     <string name="msg_export_error_key">"Error preprocessing key data!"</string>
diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
index a659dc7da..305f7a7f8 100644
--- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
+++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/ExportTest.java
@@ -22,9 +22,7 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.robolectric.Robolectric;
 import org.robolectric.RobolectricGradleTestRunner;
-import org.robolectric.RobolectricTestRunner;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
 import org.robolectric.shadows.ShadowLog;
@@ -133,7 +131,7 @@ public class ExportTest {
         Assert.assertTrue("second keyring has local certification", checkForLocal(mStaticRing2));
 
         ByteArrayOutputStream out = new ByteArrayOutputStream();
-        ExportResult result = op.exportKeyRings(new OperationLog(), null, false, out);
+        ExportResult result = op.exportKeysToStream(new OperationLog(), null, false, out);
 
         Assert.assertTrue("export must be a success", result.success());
 
@@ -170,7 +168,7 @@ public class ExportTest {
         }
 
         out = new ByteArrayOutputStream();
-        result = op.exportKeyRings(new OperationLog(), null, true, out);
+        result = op.exportKeysToStream(new OperationLog(), null, true, out);
 
         Assert.assertTrue("export must be a success", result.success());