Merge pull request #650 from open-keychain/v/import-log

Parcelable operation result with logging capabilities + import logging

5 files changed, 463 insertions, 110 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java
new file mode 100644
index 000000000..216e4b497
--- /dev/null
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/OperationResultParcel.java
@@ -0,0 +1,169 @@
+package org.sufficientlysecure.keychain.pgp;
+
+import android.os.Parcel;
+import android.os.Parcelable;
+
+import org.sufficientlysecure.keychain.R;
+
+import java.util.ArrayList;
+
+/** Represent the result of an operation.
+ *
+ * This class holds a result and the log of an operation. It can be subclassed
+ * to include typed additional information specific to the operation. To keep
+ * the class structure (somewhat) simple, this class contains an exhaustive
+ * list (ie, enum) of all possible log types, which should in all cases be tied
+ * to string resource ids.
+ *
+ */
+public class OperationResultParcel implements Parcelable {
+    /** Holds the overall result. A value of 0 is considered a success, all
+     * other values may represent failure or varying degrees of success. */
+    final int mResult;
+
+    /// A list of log entries tied to the operation result.
+    final ArrayList<LogEntryParcel> mLog;
+
+    public OperationResultParcel(int result, ArrayList<LogEntryParcel> log) {
+        mResult = result;
+        mLog = log;
+    }
+
+    public OperationResultParcel(Parcel source) {
+        mResult = source.readInt();
+        mLog = source.createTypedArrayList(LogEntryParcel.CREATOR);
+    }
+
+    public boolean isSuccessful() {
+        return mResult == 0;
+    }
+
+    /** One entry in the log. */
+    public static class LogEntryParcel implements Parcelable {
+        final LogLevel mLevel;
+        final LogType mType;
+        final String[] mParameters;
+        final int mIndent;
+
+        public LogEntryParcel(LogLevel level, LogType type, String[] parameters, int indent) {
+            mLevel = level;
+            mType = type;
+            mParameters = parameters;
+            mIndent = indent;
+        }
+        public LogEntryParcel(LogLevel level, LogType type, String[] parameters) {
+            this(level, type, parameters, 0);
+        }
+
+        public LogEntryParcel(Parcel source) {
+            mLevel = LogLevel.values()[source.readInt()];
+            mType = LogType.values()[source.readInt()];
+            mParameters = source.createStringArray();
+            mIndent = source.readInt();
+        }
+
+        @Override
+        public int describeContents() {
+            return 0;
+        }
+
+        @Override
+        public void writeToParcel(Parcel dest, int flags) {
+            dest.writeInt(mLevel.ordinal());
+            dest.writeInt(mType.ordinal());
+            dest.writeStringArray(mParameters);
+            dest.writeInt(mIndent);
+        }
+
+        public static final Creator<LogEntryParcel> CREATOR = new Creator<LogEntryParcel>() {
+            public LogEntryParcel createFromParcel(final Parcel source) {
+                return new LogEntryParcel(source);
+            }
+
+            public LogEntryParcel[] newArray(final int size) {
+                return new LogEntryParcel[size];
+            }
+        };
+
+    }
+
+    public static enum LogType {
+        MSG_IP_APPLY_BATCH (R.string.msg_ip_apply_batch),
+        MSG_IP_BAD_TYPE_SECRET (R.string.msg_ip_bad_type_secret),
+        MSG_IP_DELETE_OLD_FAIL (R.string.msg_ip_delete_old_fail),
+        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_REMOTE_EX (R.string.msg_ip_fail_remote_ex),
+        MSG_IP_IMPORTING (R.string.msg_ip_importing),
+        MSG_IP_INSERT_KEYRING (R.string.msg_ip_insert_keyring),
+        MSG_IP_INSERT_SUBKEY (R.string.msg_ip_insert_subkey),
+        MSG_IP_INSERT_SUBKEYS (R.string.msg_ip_insert_subkeys),
+        MSG_IP_PRESERVING_SECRET (R.string.msg_ip_preserving_secret),
+        MSG_IP_REINSERT_SECRET (R.string.msg_ip_reinsert_secret),
+        MSG_IP_SUCCESS (R.string.msg_ip_success),
+        MSG_IP_TRUST_RETRIEVE (R.string.msg_ip_trust_retrieve),
+        MSG_IP_TRUST_USING (R.string.msg_ip_trust_using),
+        MSG_IP_TRUST_USING_SEC (R.string.msg_ip_trust_using_sec),
+        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),
+        MSG_IP_UID_CERTS_UNKNOWN (R.string.msg_ip_uid_certs_unknown),
+        MSG_IP_UID_CLASSIFYING (R.string.msg_ip_uid_classifying),
+        MSG_IP_UID_INSERT (R.string.msg_ip_uid_insert),
+        MSG_IP_UID_PROCESSING (R.string.msg_ip_uid_processing),
+        MSG_IP_UID_SELF_BAD (R.string.msg_ip_uid_self_bad),
+        MSG_IP_UID_SELF_GOOD (R.string.msg_ip_uid_self_good),
+        MSG_IP_UID_SELF_IGNORING_OLD (R.string.msg_ip_uid_self_ignoring_old),
+        MSG_IP_UID_SELF_NEWER (R.string.msg_ip_uid_self_newer),
+        MSG_IS_BAD_TYPE_PUBLIC (R.string.msg_is_bad_type_public),
+        MSG_IS_IMPORTING (R.string.msg_is_importing),
+        MSG_IS_IMPORTING_SUBKEYS (R.string.msg_is_importing_subkeys),
+        MSG_IS_IO_EXCPTION (R.string.msg_is_io_excption),
+        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),
+        ;
+
+        private final int mMsgId;
+        LogType(int msgId) {
+            mMsgId = msgId;
+        }
+        public int getMsgId() {
+            return mMsgId;
+        }
+    }
+
+    /** Enumeration of possible log levels. */
+    public static enum LogLevel {
+        DEBUG,
+        INFO,
+        WARN,
+        /** If any ERROR log entry is included in the result, the overall operation should have failed. */
+        ERROR,
+    }
+
+    @Override
+    public int describeContents() {
+        return 0;
+    }
+
+    @Override
+    public void writeToParcel(Parcel dest, int flags) {
+        dest.writeInt(mResult);
+        dest.writeTypedList(mLog);
+    }
+
+    public static final Creator<OperationResultParcel> CREATOR = new Creator<OperationResultParcel>() {
+        public OperationResultParcel createFromParcel(final Parcel source) {
+            return new OperationResultParcel(source);
+        }
+
+        public OperationResultParcel[] newArray(final int size) {
+            return new OperationResultParcel[size];
+        }
+    };
+
+}
diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
index 14ec67e64..5ce0b11dd 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpImportExport.java
@@ -152,13 +152,14 @@ public class PgpImportExport {
                     }
                 }
 
-                mProviderHelper.savePublicKeyRing(key);
-                /*switch(status) {
-                    case RETURN_UPDATED: oldKeys++; break;
-                    case RETURN_OK: newKeys++; break;
-                    case RETURN_BAD: badKeys++; break;
-                }*/
-                // TODO proper import feedback
+                mProviderHelper.resetLog();
+                OperationResultParcel result = mProviderHelper.savePublicKeyRing(key);
+                for(OperationResultParcel.LogEntryParcel loge : result.mLog) {
+                    Log.d(Constants.TAG,
+                            loge.mIndent
+                            + new String(new char[loge.mIndent]).replace("\0", " ")
+                            + mContext.getString(loge.mType.getMsgId(), (Object[]) loge.mParameters));
+                }
                 newKeys += 1;
 
             } catch (PgpGeneralException e) {
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 02e5411ca..1264c8c36 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java
@@ -19,6 +19,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Vector;
@@ -149,13 +150,13 @@ public class UncachedKeyRing {
         aos.close();
     }
 
-    public ArrayList<Long> getAvailableSubkeys() {
+    public HashSet<Long> getAvailableSubkeys() {
         if(!isSecret()) {
             throw new RuntimeException("Tried to find available subkeys from non-secret keys. " +
                     "This is a programming error and should never happen!");
         }
 
-        ArrayList<Long> result = new ArrayList<Long>();
+        HashSet<Long> result = new HashSet<Long>();
         // then, mark exactly the keys we have available
         for (PGPSecretKey sub : new IterableIterator<PGPSecretKey>(
                 ((PGPSecretKeyRing) mRing).getSecretKeys())) {
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 ca7e622bb..5c8bf6752 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java
@@ -29,6 +29,9 @@ import android.support.v4.util.LongSparseArray;
 
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.pgp.KeyRing;
+import org.sufficientlysecure.keychain.pgp.OperationResultParcel;
+import org.sufficientlysecure.keychain.pgp.OperationResultParcel.LogType;
+import org.sufficientlysecure.keychain.pgp.OperationResultParcel.LogLevel;
 import org.sufficientlysecure.keychain.pgp.PgpHelper;
 import org.sufficientlysecure.keychain.pgp.PgpKeyHelper;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
@@ -59,12 +62,28 @@ import java.util.List;
 import java.util.Set;
 
 public class ProviderHelper {
-    private Context mContext;
-    private ContentResolver mContentResolver;
+    private final Context mContext;
+    private final ContentResolver mContentResolver;
+    private final ArrayList<OperationResultParcel.LogEntryParcel> mLog;
+    private int mIndent;
 
     public ProviderHelper(Context context) {
-        this.mContext = context;
-        this.mContentResolver = context.getContentResolver();
+        this(context, new ArrayList<OperationResultParcel.LogEntryParcel>(), 0);
+    }
+
+    public ProviderHelper(Context context, ArrayList<OperationResultParcel.LogEntryParcel> log,
+                          int indent) {
+        mContext = context;
+        mContentResolver = context.getContentResolver();
+        mLog = log;
+        mIndent = indent;
+    }
+
+    public void resetLog() {
+        if(mLog != null) {
+            mLog.clear();
+            mIndent = 0;
+        }
     }
 
     public static class NotFoundException extends Exception {
@@ -76,6 +95,17 @@ public class ProviderHelper {
         }
     }
 
+    public void log(LogLevel level, LogType type) {
+        if(mLog != null) {
+            mLog.add(new OperationResultParcel.LogEntryParcel(level, type, null, mIndent));
+        }
+    }
+    public void log(LogLevel level, LogType type, String[] parameters) {
+        if(mLog != null) {
+            mLog.add(new OperationResultParcel.LogEntryParcel(level, type, parameters, mIndent));
+        }
+    }
+
     // If we ever switch to api level 11, we can ditch this whole mess!
     public static final int FIELD_TYPE_NULL = 1;
     // this is called integer to stay coherent with the constants in Cursor (api level 11)
@@ -222,136 +252,219 @@ public class ProviderHelper {
      * Saves PGPPublicKeyRing with its keys and userIds in DB
      */
     @SuppressWarnings("unchecked")
-    public void savePublicKeyRing(UncachedKeyRing keyRing) throws IOException {
+    public OperationResultParcel savePublicKeyRing(UncachedKeyRing keyRing) {
         if (keyRing.isSecret()) {
-            throw new RuntimeException("Tried to save secret keyring as public! " +
-                    "This is a bug, please file a bug report.");
+            log(LogLevel.ERROR, LogType.MSG_IP_BAD_TYPE_SECRET);
+            return new OperationResultParcel(1, mLog);
         }
 
         UncachedPublicKey masterKey = keyRing.getPublicKey();
         long masterKeyId = masterKey.getKeyId();
+        log(LogLevel.INFO, LogType.MSG_IP_IMPORTING,
+                new String[]{Long.toString(masterKeyId)});
+        mIndent += 1;
 
         // IF there is a secret key, preserve it!
-        UncachedKeyRing secretRing = null;
+        UncachedKeyRing secretRing;
         try {
             secretRing = getWrappedSecretKeyRing(masterKeyId).getUncached();
+            log(LogLevel.DEBUG, LogType.MSG_IP_PRESERVING_SECRET);
         } catch (NotFoundException e) {
-            Log.e(Constants.TAG, "key not found!");
+            secretRing = null;
         }
 
         // delete old version of this keyRing, which also deletes all keys and userIds on cascade
         try {
             mContentResolver.delete(KeyRingData.buildPublicKeyRingUri(Long.toString(masterKeyId)), null, null);
+            log(LogLevel.DEBUG, LogType.MSG_IP_DELETE_OLD_OK);
         } catch (UnsupportedOperationException e) {
             Log.e(Constants.TAG, "Key could not be deleted! Maybe we are creating a new one!", e);
+            log(LogLevel.DEBUG, LogType.MSG_IP_DELETE_OLD_FAIL);
         }
 
         // insert new version of this keyRing
         ContentValues values = new ContentValues();
         values.put(KeyRingData.MASTER_KEY_ID, masterKeyId);
-        values.put(KeyRingData.KEY_RING_DATA, keyRing.getEncoded());
-        Uri uri = KeyRingData.buildPublicKeyRingUri(Long.toString(masterKeyId));
-        mContentResolver.insert(uri, values);
+        try {
+            values.put(KeyRingData.KEY_RING_DATA, keyRing.getEncoded());
+        } catch (IOException e) {
+            log(LogLevel.ERROR, LogType.MSG_IP_ENCODE_FAIL);
+            return new OperationResultParcel(1, mLog);
+        }
 
         // save all keys and userIds included in keyRing object in database
         ArrayList<ContentProviderOperation> operations = new ArrayList<ContentProviderOperation>();
 
-        int rank = 0;
-        for (UncachedPublicKey key : new IterableIterator<UncachedPublicKey>(keyRing.getPublicKeys())) {
-            operations.add(buildPublicKeyOperations(masterKeyId, key, rank));
-            ++rank;
-        }
+        try {
 
-        // get a list of owned secret keys, for verification filtering
-        LongSparseArray<UncachedPublicKey> allKeyRings =
-                getUncachedMasterKeys(KeyRingData.buildSecretKeyRingUri());
-        // special case: available secret keys verify themselves!
-        if (secretRing != null) {
-            allKeyRings.put(secretRing.getMasterKeyId(), secretRing.getPublicKey());
-        }
+            log(LogLevel.INFO, LogType.MSG_IP_INSERT_KEYRING);
+            Uri uri = KeyRingData.buildPublicKeyRingUri(Long.toString(masterKeyId));
+            operations.add(ContentProviderOperation.newInsert(uri).withValues(values).build());
 
-        // classify and order user ids. primary are moved to the front, revoked to the back,
-        // otherwise the order in the keyfile is preserved.
-        List<UserIdItem> uids = new ArrayList<UserIdItem>();
-
-        for (String userId : new IterableIterator<String>(
-                masterKey.getUnorderedUserIds().iterator())) {
-            UserIdItem item = new UserIdItem();
-            uids.add(item);
-            item.userId = userId;
-
-            // look through signatures for this specific key
-            for (WrappedSignature cert : new IterableIterator<WrappedSignature>(
-                    masterKey.getSignaturesForId(userId))) {
-                long certId = cert.getKeyId();
-                try {
-                    // self signature
-                    if (certId == masterKeyId) {
-                        cert.init(masterKey);
-                        if (!cert.verifySignature(masterKey, userId)) {
-                            // not verified?! dang! TODO notify user? this is kinda serious...
-                            Log.e(Constants.TAG, "Could not verify self signature for " + userId + "!");
-                            continue;
-                        }
-                        // is this the first, or a more recent certificate?
-                        if (item.selfCert == null ||
-                                item.selfCert.getCreationTime().before(cert.getCreationTime())) {
+            log(LogLevel.INFO, LogType.MSG_IP_INSERT_SUBKEYS);
+            mIndent += 1;
+            int rank = 0;
+            for (UncachedPublicKey key : new IterableIterator<UncachedPublicKey>(keyRing.getPublicKeys())) {
+                log(LogLevel.DEBUG, LogType.MSG_IP_INSERT_SUBKEY, new String[] {
+                    PgpKeyHelper.convertKeyIdToHex(key.getKeyId())
+                });
+                operations.add(buildPublicKeyOperations(masterKeyId, key, rank));
+                ++rank;
+            }
+            mIndent -= 1;
+
+            log(LogLevel.DEBUG, LogType.MSG_IP_TRUST_RETRIEVE);
+            // get a list of owned secret keys, for verification filtering
+            LongSparseArray<UncachedPublicKey> trustedKeys =
+                    getUncachedMasterKeys(KeyRingData.buildSecretKeyRingUri());
+            // special case: available secret keys verify themselves!
+            if (secretRing != null) {
+                trustedKeys.put(secretRing.getMasterKeyId(), secretRing.getPublicKey());
+                log(LogLevel.INFO, LogType.MSG_IP_TRUST_USING_SEC, new String[]{
+                        Integer.toString(trustedKeys.size())
+                });
+            } else {
+                log(LogLevel.INFO, LogType.MSG_IP_TRUST_USING, new String[] {
+                    Integer.toString(trustedKeys.size())
+                });
+            }
+
+            // classify and order user ids. primary are moved to the front, revoked to the back,
+            // otherwise the order in the keyfile is preserved.
+            log(LogLevel.DEBUG, LogType.MSG_IP_UID_CLASSIFYING);
+            mIndent += 1;
+            List<UserIdItem> uids = new ArrayList<UserIdItem>();
+            for (String userId : new IterableIterator<String>(
+                    masterKey.getUnorderedUserIds().iterator())) {
+                UserIdItem item = new UserIdItem();
+                uids.add(item);
+                item.userId = userId;
+
+                int unknownCerts = 0;
+
+                log(LogLevel.INFO, LogType.MSG_IP_UID_PROCESSING, new String[] { userId });
+                mIndent += 1;
+                // look through signatures for this specific key
+                for (WrappedSignature cert : new IterableIterator<WrappedSignature>(
+                        masterKey.getSignaturesForId(userId))) {
+                    long certId = cert.getKeyId();
+                    try {
+                        // self signature
+                        if (certId == masterKeyId) {
+                            cert.init(masterKey);
+                            if (!cert.verifySignature(masterKey, userId)) {
+                                // Bad self certification? That's kinda bad...
+                                log(LogLevel.ERROR, LogType.MSG_IP_UID_SELF_BAD);
+                                return new OperationResultParcel(1, mLog);
+                            }
+
+                            // if we already have a cert..
+                            if (item.selfCert != null) {
+                                // ..is this perchance a more recent one?
+                                if (item.selfCert.getCreationTime().before(cert.getCreationTime())) {
+                                    log(LogLevel.DEBUG, LogType.MSG_IP_UID_SELF_NEWER);
+                                } else {
+                                    log(LogLevel.DEBUG, LogType.MSG_IP_UID_SELF_IGNORING_OLD);
+                                    continue;
+                                }
+                            } else {
+                                log(LogLevel.DEBUG, LogType.MSG_IP_UID_SELF_GOOD);
+                            }
+
+                            // save certificate as primary self-cert
                             item.selfCert = cert;
                             item.isPrimary = cert.isPrimaryUserId();
                             item.isRevoked = cert.isRevocation();
+
                         }
-                    }
-                    // verify signatures from known private keys
-                    if (allKeyRings.indexOfKey(certId) >= 0) {
-                        cert.init(allKeyRings.get(certId));
-                        if (cert.verifySignature(masterKey, userId)) {
-                            item.trustedCerts.add(cert);
+
+                        // verify signatures from known private keys
+                        if (trustedKeys.indexOfKey(certId) >= 0) {
+                            UncachedPublicKey trustedKey = trustedKeys.get(certId);
+                            cert.init(trustedKey);
+                            if (cert.verifySignature(masterKey, userId)) {
+                                item.trustedCerts.add(cert);
+                                log(LogLevel.INFO, LogType.MSG_IP_UID_CERT_GOOD, new String[] {
+                                    PgpKeyHelper.convertKeyIdToHex(trustedKey.getKeyId())
+                                });
+                            } else {
+                                log(LogLevel.WARN, LogType.MSG_IP_UID_CERT_BAD);
+                            }
                         }
+
+                        unknownCerts += 1;
+
+                    } catch (PgpGeneralException e) {
+                        log(LogLevel.WARN, LogType.MSG_IP_UID_CERT_ERROR, new String[]{
+                                PgpKeyHelper.convertKeyIdToHex(cert.getKeyId())
+                        });
                     }
-                } catch (PgpGeneralException e) {
-                    Log.e(Constants.TAG, "Signature verification failed! "
-                            + PgpKeyHelper.convertKeyIdToHex(masterKey.getKeyId())
-                            + " from "
-                            + PgpKeyHelper.convertKeyIdToHex(cert.getKeyId()), e);
                 }
-            }
-        }
+                mIndent -= 1;
+
+                if (unknownCerts > 0) {
+                    log(LogLevel.DEBUG, LogType.MSG_IP_UID_CERTS_UNKNOWN, new String[] {
+                            Integer.toString(unknownCerts)
+                    });
+                }
 
-        // primary before regular before revoked (see UserIdItem.compareTo)
-        // this is a stable sort, so the order of keys is otherwise preserved.
-        Collections.sort(uids);
-        // iterate and put into db
-        for (int userIdRank = 0; userIdRank < uids.size(); userIdRank++) {
-            UserIdItem item = uids.get(userIdRank);
-            operations.add(buildUserIdOperations(masterKeyId, item, userIdRank));
-            // no self cert is bad, but allowed by the rfc...
-            if (item.selfCert != null) {
-                operations.add(buildCertOperations(
-                        masterKeyId, userIdRank, item.selfCert, Certs.VERIFIED_SELF));
-            }
-            // don't bother with trusted certs if the uid is revoked, anyways
-            if (item.isRevoked) {
-                continue;
             }
-            for (int i = 0; i < item.trustedCerts.size(); i++) {
-                operations.add(buildCertOperations(
-                        masterKeyId, userIdRank, item.trustedCerts.get(i), Certs.VERIFIED_SECRET));
+            mIndent -= 1;
+
+            log(LogLevel.INFO, LogType.MSG_IP_UID_INSERT);
+            // primary before regular before revoked (see UserIdItem.compareTo)
+            // this is a stable sort, so the order of keys is otherwise preserved.
+            Collections.sort(uids);
+            // iterate and put into db
+            for (int userIdRank = 0; userIdRank < uids.size(); userIdRank++) {
+                UserIdItem item = uids.get(userIdRank);
+                operations.add(buildUserIdOperations(masterKeyId, item, userIdRank));
+                // no self cert is bad, but allowed by the rfc...
+                if (item.selfCert != null) {
+                    operations.add(buildCertOperations(
+                            masterKeyId, userIdRank, item.selfCert, Certs.VERIFIED_SELF));
+                }
+                // don't bother with trusted certs if the uid is revoked, anyways
+                if (item.isRevoked) {
+                    continue;
+                }
+                for (int i = 0; i < item.trustedCerts.size(); i++) {
+                    operations.add(buildCertOperations(
+                            masterKeyId, userIdRank, item.trustedCerts.get(i), Certs.VERIFIED_SECRET));
+                }
             }
-        }
 
-        try {
+            log(LogLevel.DEBUG, LogType.MSG_IP_APPLY_BATCH);
             mContentResolver.applyBatch(KeychainContract.CONTENT_AUTHORITY, operations);
+        } catch (IOException e) {
+            log(LogLevel.ERROR, LogType.MSG_IP_FAIL_IO_EXC);
+            Log.e(Constants.TAG, "IOException during import", e);
+            mIndent -= 1;
+            return new OperationResultParcel(1, mLog);
         } catch (RemoteException e) {
-            Log.e(Constants.TAG, "applyBatch failed!", e);
+            log(LogLevel.ERROR, LogType.MSG_IP_FAIL_REMOTE_EX);
+            Log.e(Constants.TAG, "RemoteException during import", e);
+            mIndent -= 1;
+            return new OperationResultParcel(1, mLog);
         } catch (OperationApplicationException e) {
-            Log.e(Constants.TAG, "applyBatch failed!", e);
+            log(LogLevel.ERROR, LogType.MSG_IP_FAIL_OP_EX);
+            Log.e(Constants.TAG, "OperationApplicationException during import", e);
+            mIndent -= 1;
+            return new OperationResultParcel(1, mLog);
         }
 
         // Save the saved keyring (if any)
         if (secretRing != null) {
+            log(LogLevel.DEBUG, LogType.MSG_IP_REINSERT_SECRET);
+            mIndent += 1;
             saveSecretKeyRing(secretRing);
+            mIndent -= 1;
         }
 
+        log(LogLevel.INFO, LogType.MSG_IP_SUCCESS);
+        mIndent -= 1;
+        return new OperationResultParcel(0, mLog);
+
     }
 
     private static class UserIdItem implements Comparable<UserIdItem> {
@@ -379,13 +492,29 @@ public class ProviderHelper {
      * Saves a PGPSecretKeyRing in the DB. This will only work if a corresponding public keyring
      * is already in the database!
      */
-    public void saveSecretKeyRing(UncachedKeyRing keyRing) throws IOException {
+    public OperationResultParcel saveSecretKeyRing(UncachedKeyRing keyRing) {
         if (!keyRing.isSecret()) {
-            throw new RuntimeException("Tried to save publkc keyring as secret! " +
-                    "This is a bug, please file a bug report.");
+            log(LogLevel.ERROR, LogType.MSG_IS_BAD_TYPE_PUBLIC);
+            return new OperationResultParcel(1, mLog);
         }
 
         long masterKeyId = keyRing.getMasterKeyId();
+        log(LogLevel.INFO, LogType.MSG_IS_IMPORTING,
+                new String[]{ Long.toString(masterKeyId) });
+
+        // save secret keyring
+        try {
+            ContentValues values = new ContentValues();
+            values.put(KeyRingData.MASTER_KEY_ID, masterKeyId);
+            values.put(KeyRingData.KEY_RING_DATA, keyRing.getEncoded());
+            // insert new version of this keyRing
+            Uri uri = KeyRingData.buildSecretKeyRingUri(Long.toString(masterKeyId));
+            mContentResolver.insert(uri, values);
+        } catch (IOException e) {
+            Log.e(Constants.TAG, "Failed to encode key!", e);
+            log(LogLevel.ERROR, LogType.MSG_IS_IO_EXCPTION);
+            return new OperationResultParcel(1, mLog);
+        }
 
         {
             Uri uri = Keys.buildKeysUri(Long.toString(masterKeyId));
@@ -397,24 +526,38 @@ public class ProviderHelper {
 
             values.put(Keys.HAS_SECRET, 1);
             // then, mark exactly the keys we have available
-            for (Long sub : new IterableIterator<Long>(keyRing.getAvailableSubkeys().iterator())) {
-                mContentResolver.update(uri, values, Keys.KEY_ID + " = ?", new String[] {
-                    Long.toString(sub)
-                });
+            log(LogLevel.INFO, LogType.MSG_IS_IMPORTING_SUBKEYS);
+            mIndent += 1;
+            Set<Long> available = keyRing.getAvailableSubkeys();
+            for (UncachedPublicKey sub :
+                    new IterableIterator<UncachedPublicKey>(keyRing.getPublicKeys())) {
+                long id = sub.getKeyId();
+                if(available.contains(id)) {
+                    int upd = mContentResolver.update(uri, values, Keys.KEY_ID + " = ?",
+                            new String[] { Long.toString(id) });
+                    if (upd == 1) {
+                        log(LogLevel.DEBUG, LogType.MSG_IS_SUBKEY_OK, new String[]{
+                                PgpKeyHelper.convertKeyIdToHex(id)
+                        });
+                    } else {
+                        log(LogLevel.WARN, LogType.MSG_IS_SUBKEY_NONEXISTENT, new String[]{
+                                PgpKeyHelper.convertKeyIdToHex(id)
+                        });
+                    }
+                } else {
+                    log(LogLevel.INFO, LogType.MSG_IS_SUBKEY_STRIPPED, new String[]{
+                            PgpKeyHelper.convertKeyIdToHex(id)
+                    });
+                }
             }
+            mIndent -= 1;
+
             // this implicitly leaves all keys which were not in the secret key ring
             // with has_secret = 0
         }
 
-        // save secret keyring
-        {
-            ContentValues values = new ContentValues();
-            values.put(KeyRingData.MASTER_KEY_ID, masterKeyId);
-            values.put(KeyRingData.KEY_RING_DATA, keyRing.getEncoded());
-            // insert new version of this keyRing
-            Uri uri = KeyRingData.buildSecretKeyRingUri(Long.toString(masterKeyId));
-            mContentResolver.insert(uri, values);
-        }
+        log(LogLevel.INFO, LogType.MSG_IS_SUCCESS);
+        return new OperationResultParcel(0, mLog);
 
     }
 
diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml
index ad8abe78f..38f0519ac 100644
--- a/OpenKeychain/src/main/res/values/strings.xml
+++ b/OpenKeychain/src/main/res/values/strings.xml
@@ -523,4 +523,43 @@
     <string name="info_no_manual_account_creation">Do not create OpenKeychain-Accounts manually.\nFor more information, see Help.</string>
     <string name="contact_show_key">Show key (%s)</string>
 
+    <!-- Import log entries -->
+    <string name="msg_ip_apply_batch">Applying insert batch operation.</string>
+    <string name="msg_ip_bad_type_secret">Tried to import secret keyring as public. This is a bug, please file a report!</string>
+    <string name="msg_ip_delete_old_fail">No old key deleted (creating a new one?)</string>
+    <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_remote_ex">Operation failed due to internal error</string>
+    <string name="msg_ip_importing">Importing public keyring</string>
+    <string name="msg_ip_insert_keyring">Inserting keyring data</string>
+    <string name="msg_ip_insert_subkey">Inserting subkey %s</string>
+    <string name="msg_ip_insert_subkeys">Inserting subkeys</string>
+    <string name="msg_ip_preserving_secret">Preserving available secret key</string>
+    <string name="msg_ip_reinsert_secret">Re-inserting secret key</string>
+    <string name="msg_ip_success">Successfully inserted public keyring</string>
+    <string name="msg_ip_trust_retrieve">Retrieving trusted keys</string>
+    <string name="msg_ip_trust_using">Using %s trusted keys</string>
+    <string name="msg_ip_trust_using_sec">Secret key available, self certificates are trusted</string>
+    <string name="msg_ip_uid_cert_bad">Encountered bad certificate!</string>
+    <string name="msg_ip_uid_cert_error">Error processing certificate!</string>
+    <string name="msg_ip_uid_cert_good">Found good certificate from %s</string>
+    <string name="msg_ip_uid_certs_unknown">Ignored %s certificates from unknown pubkeys</string>
+    <string name="msg_ip_uid_classifying">Classifying user ids</string>
+    <string name="msg_ip_uid_insert">Inserting user ids</string>
+    <string name="msg_ip_uid_processing">Processing user id %s</string>
+    <string name="msg_ip_uid_self_bad">Bad self certificate encountered!</string>
+    <string name="msg_ip_uid_self_good">Found good self certificate</string>
+    <string name="msg_ip_uid_self_ignoring_old">Ignoring older self certificate</string>
+    <string name="msg_ip_uid_self_newer">Using more recent good self certificate</string>
+    <string name="msg_is_bad_type_public">Tried to import public keyring as secret. This is a bug, please file a report!</string>
+    <string name="msg_is_importing">Importing secret key %s</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_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 inserted secret keyring</string>
+
 </resources>