consolidate: almost sane logging

4 files changed, 63 insertions, 49 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
index 0f0d7cca3..a1c6b158b 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -634,7 +634,7 @@ public class UncachedKeyRing {
      */
     public UncachedKeyRing merge(UncachedKeyRing other, OperationLog log, int indent) {
 
-        log.add(LogLevel.START, isSecret() ? LogType.MSG_MG_SECRET : LogType.MSG_MG_PUBLIC,
+        log.add(LogLevel.DEBUG, isSecret() ? LogType.MSG_MG_SECRET : LogType.MSG_MG_PUBLIC,
                 new String[]{ PgpKeyHelper.convertKeyIdToHex(getMasterKeyId()) }, indent);
         indent += 1;
 
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
index 36b4b4ac7..79bd5777c 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
@@ -532,7 +532,7 @@ public class ProviderHelper {
             mIndent -= 1;
             return SaveKeyringResult.RESULT_ERROR;
         } catch (OperationApplicationException e) {
-            log(LogLevel.ERROR, LogType.MSG_IP_FAIL_OP_EX);
+            log(LogLevel.ERROR, LogType.MSG_IP_FAIL_OP_EXC);
             Log.e(Constants.TAG, "OperationApplicationException during import", e);
             mIndent -= 1;
             return SaveKeyringResult.RESULT_ERROR;
@@ -603,7 +603,7 @@ public class ProviderHelper {
             }
         } catch (IOException e) {
             Log.e(Constants.TAG, "Failed to encode key!", e);
-            log(LogLevel.ERROR, LogType.MSG_IS_IO_EXCPTION);
+            log(LogLevel.ERROR, LogType.MSG_IS_FAIL_IO_EXC);
             return SaveKeyringResult.RESULT_ERROR;
         }
 
@@ -681,16 +681,22 @@ public class ProviderHelper {
             long masterKeyId = publicRing.getMasterKeyId();
             log(LogLevel.START, LogType.MSG_IP,
                     new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) });
-
             mIndent += 1;
 
-            // IF there is a secret key, preserve it!
+            // If there is an old keyring, merge it
             try {
                 UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncached();
 
                 // Merge data from new public ring into the old one
                 publicRing = oldPublicRing.merge(publicRing, mLog, mIndent);
 
+                // If this is null, there is an error in the log so we can just return
+                if (publicRing == null) {
+                    return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+                }
+
+                // Canonicalize this keyring, to assert a number of assumptions made about it.
+                publicRing = publicRing.canonicalize(mLog, mIndent);
                 if (publicRing == null) {
                     return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
                 }
@@ -698,37 +704,37 @@ public class ProviderHelper {
                 // Early breakout if nothing changed
                 if (Arrays.hashCode(publicRing.getEncoded())
                         == Arrays.hashCode(oldPublicRing.getEncoded())) {
-                    log(LogLevel.OK, LogType.MSG_IP,
-                            new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) });
+                    log(LogLevel.OK, LogType.MSG_IP_SUCCESS_IDENTICAL, null);
                     return new SaveKeyringResult(SaveKeyringResult.RESULT_OK, mLog);
                 }
             } catch (NotFoundException e) {
-                // not an issue, just means we are dealing with a new keyring
-            }
+                // Not an issue, just means we are dealing with a new keyring.
+
+                // Canonicalize this keyring, to assert a number of assumptions made about it.
+                publicRing = publicRing.canonicalize(mLog, mIndent);
+                if (publicRing == null) {
+                    return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+                }
 
-            // Canonicalize this keyring, to assert a number of assumptions made about it.
-            publicRing = publicRing.canonicalize(mLog, mIndent);
-            if (publicRing == null) {
-                return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
             }
 
-            // IF there is a secret key, preserve it!
+            // If there is a secret key, merge new data (if any) and save the key for later
             UncachedKeyRing secretRing;
             try {
                 secretRing = getWrappedSecretKeyRing(publicRing.getMasterKeyId()).getUncached();
-                log(LogLevel.DEBUG, LogType.MSG_IP_PRESERVING_SECRET);
-                progress.setProgress(LogType.MSG_IP_PRESERVING_SECRET.getMsgId(), 10, 100);
-                mIndent += 1;
 
                 // Merge data from new public ring into secret one
                 secretRing = secretRing.merge(publicRing, mLog, mIndent);
                 if (secretRing == null) {
                     return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
                 }
-
-                mIndent -= 1;
+                secretRing = secretRing.canonicalize(mLog, mIndent);
+                if (secretRing == null) {
+                    return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+                }
 
             } catch (NotFoundException e) {
+                // No secret key available (this is what happens most of the time)
                 secretRing = null;
             }
 
@@ -736,19 +742,19 @@ public class ProviderHelper {
 
             // Save the saved keyring (if any)
             if (secretRing != null) {
-                log(LogLevel.DEBUG, LogType.MSG_IP_REINSERT_SECRET);
                 progress.setProgress(LogType.MSG_IP_REINSERT_SECRET.getMsgId(), 90, 100);
-                mIndent += 1;
-                secretRing = secretRing.canonicalize(mLog, mIndent);
-                internalSaveSecretKeyRing(secretRing);
-                result |= SaveKeyringResult.SAVED_SECRET;
-                mIndent -= 1;
+                int secretResult = internalSaveSecretKeyRing(secretRing);
+                if ((secretResult & SaveKeyringResult.RESULT_ERROR) != SaveKeyringResult.RESULT_ERROR) {
+                    result |= SaveKeyringResult.SAVED_SECRET;
+                }
             }
 
+            mIndent -= 1;
             return new SaveKeyringResult(result, mLog);
 
         } catch (IOException e) {
-            return null;
+            log(LogLevel.ERROR, LogType.MSG_IP_FAIL_IO_EXC);
+            return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
         }
 
     }
@@ -759,16 +765,22 @@ public class ProviderHelper {
             long masterKeyId = secretRing.getMasterKeyId();
             log(LogLevel.START, LogType.MSG_IS,
                     new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) });
-
             mIndent += 1;
 
-            // If there is a secret key, merge it.
+            // If there is an old secret key, merge it.
             try {
                 UncachedKeyRing oldSecretRing = getWrappedSecretKeyRing(masterKeyId).getUncached();
 
-                // Merge data from new public ring into the old one
+                // Merge data from new secret ring into old one
                 secretRing = oldSecretRing.merge(secretRing, mLog, mIndent);
 
+                // If this is null, there is an error in the log so we can just return
+                if (secretRing == null) {
+                    return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+                }
+
+                // Canonicalize this keyring, to assert a number of assumptions made about it.
+                secretRing = secretRing.canonicalize(mLog, mIndent);
                 if (secretRing == null) {
                     return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
                 }
@@ -776,26 +788,24 @@ public class ProviderHelper {
                 // Early breakout if nothing changed
                 if (Arrays.hashCode(secretRing.getEncoded())
                         == Arrays.hashCode(oldSecretRing.getEncoded())) {
-                    log(LogLevel.OK, LogType.MSG_IS,
+                    log(LogLevel.OK, LogType.MSG_IS_SUCCESS_IDENTICAL,
                             new String[]{ PgpKeyHelper.convertKeyIdToHex(masterKeyId) });
                     return new SaveKeyringResult(SaveKeyringResult.RESULT_OK, mLog);
                 }
             } catch (NotFoundException e) {
-                // not an issue, just means we are dealing with a new keyring
-            }
+                // Not an issue, just means we are dealing with a new keyring
+
+                // Canonicalize this keyring, to assert a number of assumptions made about it.
+                secretRing = secretRing.canonicalize(mLog, mIndent);
+                if (secretRing == null) {
+                    return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+                }
 
-            // Canonicalize this keyring, to assert a number of assumptions made about it.
-            secretRing = secretRing.canonicalize(mLog, mIndent);
-            if (secretRing == null) {
-                return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
             }
 
             // Merge new data into public keyring as well, if there is any
             try {
                 UncachedKeyRing oldPublicRing = getWrappedPublicKeyRing(masterKeyId).getUncached();
-                log(LogLevel.DEBUG, LogType.MSG_IP_PRESERVING_SECRET);
-                progress.setProgress(LogType.MSG_IP_PRESERVING_SECRET.getMsgId(), 10, 100);
-                mIndent += 1;
 
                 // Merge data from new public ring into secret one
                 UncachedKeyRing publicRing = oldPublicRing.merge(secretRing, mLog, mIndent);
@@ -803,7 +813,7 @@ public class ProviderHelper {
                     return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
                 }
 
-                // Early breakout if nothing changed
+                // If anything changed, reinsert
                 if (Arrays.hashCode(publicRing.getEncoded())
                         != Arrays.hashCode(oldPublicRing.getEncoded())) {
 
@@ -816,11 +826,12 @@ public class ProviderHelper {
                     }
 
                     int result = internalSavePublicKeyRing(publicRing, progress, true);
+                    if ((result & SaveKeyringResult.RESULT_ERROR) == SaveKeyringResult.RESULT_ERROR) {
+                        return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
+                    }
 
                 }
 
-                mIndent -= 1;
-
             } catch (NotFoundException e) {
                 // TODO, this WILL error out later because secret rings cannot be inserted without
                 // public ones
@@ -831,7 +842,8 @@ public class ProviderHelper {
             return new SaveKeyringResult(result, mLog);
 
         } catch (IOException e) {
-            return null;
+            log(LogLevel.ERROR, LogType.MSG_IS_FAIL_IO_EXC, null);
+            return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog);
         }
 
     }
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
index 4010ab108..48eb39a39 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java
@@ -131,12 +131,11 @@ public class OperationResultParcel implements Parcelable {
         MSG_IP_DELETE_OLD_OK (R.string.msg_ip_delete_old_ok),
         MSG_IP_ENCODE_FAIL (R.string.msg_ip_encode_fail),
         MSG_IP_FAIL_IO_EXC (R.string.msg_ip_fail_io_exc),
-        MSG_IP_FAIL_OP_EX (R.string.msg_ip_fail_op_ex),
+        MSG_IP_FAIL_OP_EXC (R.string.msg_ip_fail_op_exc),
         MSG_IP_FAIL_REMOTE_EX (R.string.msg_ip_fail_remote_ex),
         MSG_IP_INSERT_KEYRING (R.string.msg_ip_insert_keyring),
         MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_keys),
         MSG_IP_PREPARE (R.string.msg_ip_prepare),
-        MSG_IP_PRESERVING_SECRET (R.string.msg_ip_preserving_secret),
         MSG_IP_REINSERT_SECRET (R.string.msg_ip_reinsert_secret),
         MSG_IP_MASTER (R.string.msg_ip_master),
         MSG_IP_MASTER_EXPIRED (R.string.msg_ip_master_expired),
@@ -161,6 +160,7 @@ public class OperationResultParcel implements Parcelable {
         MSG_IP_SUBKEY_FLAGS_XXS (R.string.msg_ip_subkey_flags_xxs),
         MSG_IP_SUBKEY_FLAGS_XXX (R.string.msg_ip_subkey_flags_xxx),
         MSG_IP_SUCCESS (R.string.msg_ip_success),
+        MSG_IP_SUCCESS_IDENTICAL (R.string.msg_ip_success_identical),
         MSG_IP_UID_CERT_BAD (R.string.msg_ip_uid_cert_bad),
         MSG_IP_UID_CERT_ERROR (R.string.msg_ip_uid_cert_error),
         MSG_IP_UID_CERT_GOOD (R.string.msg_ip_uid_cert_good),
@@ -175,11 +175,12 @@ public class OperationResultParcel implements Parcelable {
         MSG_IS_BAD_TYPE_PUBLIC (R.string.msg_is_bad_type_public),
         MSG_IS_DB_EXCEPTION (R.string.msg_is_db_exception),
         MSG_IS_IMPORTING_SUBKEYS (R.string.msg_is_importing_subkeys),
-        MSG_IS_IO_EXCPTION (R.string.msg_is_io_excption),
+        MSG_IS_FAIL_IO_EXC (R.string.msg_is_io_exc),
         MSG_IS_SUBKEY_NONEXISTENT (R.string.msg_is_subkey_nonexistent),
         MSG_IS_SUBKEY_OK (R.string.msg_is_subkey_ok),
         MSG_IS_SUBKEY_STRIPPED (R.string.msg_is_subkey_stripped),
         MSG_IS_SUCCESS (R.string.msg_is_success),
+        MSG_IS_SUCCESS_IDENTICAL (R.string.msg_is_success_identical),
 
         // keyring canonicalization
         MSG_KC_PUBLIC (R.string.msg_kc_public),
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index d9c033a96..ed9093194 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -507,13 +507,12 @@
     <string name="msg_ip_delete_old_ok">Deleted old key from database</string>
     <string name="msg_ip_encode_fail">Operation failed due to encoding error</string>
     <string name="msg_ip_fail_io_exc">Operation failed due to i/o error</string>
-    <string name="msg_ip_fail_op_ex">Operation failed due to database error</string>
+    <string name="msg_ip_fail_op_exc">Operation failed due to database error</string>
     <string name="msg_ip_fail_remote_ex">Operation failed due to internal error</string>
     <string name="msg_ip">Importing public keyring %s</string>
     <string name="msg_ip_insert_keyring">Encoding keyring data</string>
     <string name="msg_ip_insert_keys">Parsing keys</string>
     <string name="msg_ip_prepare">Preparing database operations</string>
-    <string name="msg_ip_preserving_secret">Preserving available secret key</string>
     <string name="msg_ip_master">Processing master key %s</string>
     <string name="msg_ip_master_expired">Keyring expired on %s</string>
     <string name="msg_ip_master_expires">Keyring expires on %s</string>
@@ -537,6 +536,7 @@
     <string name="msg_ip_subkey_flags_xxs">Subkey flags: sign</string>
     <string name="msg_ip_subkey_flags_xxx">Subkey flags: none</string>
     <string name="msg_ip_success">Successfully imported public keyring</string>
+    <string name="msg_ip_success_identical">Keyring contains no new data, nothing to do</string>
     <string name="msg_ip_reinsert_secret">Re-inserting secret key</string>
     <string name="msg_ip_uid_cert_bad">Encountered bad certificate!</string>
     <string name="msg_ip_uid_cert_error">Error processing certificate!</string>
@@ -552,11 +552,12 @@
     <string name="msg_is">Importing secret key %s</string>
     <string name="msg_is_db_exception">Database error!</string>
     <string name="msg_is_importing_subkeys">Processing secret subkeys</string>
-    <string name="msg_is_io_excption">Error encoding keyring</string>
+    <string name="msg_is_io_exc">Error encoding keyring</string>
     <string name="msg_is_subkey_nonexistent">Subkey %s unavailable in public key</string>
     <string name="msg_is_subkey_ok">Marked %s as available</string>
     <string name="msg_is_subkey_stripped">Marked %s as stripped</string>
     <string name="msg_is_success">Successfully imported secret keyring</string>
+    <string name="msg_is_success_identical">Keyring contains no new data, nothing to do</string>
 
     <!-- Keyring Canonicalization log entries -->
     <string name="msg_kc_public">Canonicalizing public keyring %s</string>