tests: adapt to new EditKeyResult return value

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/KeyringTestingHelper.java

@@ -29,6 +29,7 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -351,4 +352,12 @@ public class KeyringTestingHelper {
         }
     }
 
+    public static <E> List<E> itToList(Iterator<E> it) {
+        List<E> result = new ArrayList<E>();
+        while(it.hasNext()) {
+            result.add(it.next());
+        }
+        return result;
+    }
+
 }

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/ProviderHelperStub.java

@@ -20,7 +20,7 @@ package org.sufficientlysecure.keychain.support;
 import android.content.Context;
 import android.net.Uri;
 
-import org.sufficientlysecure.keychain.pgp.WrappedPublicKeyRing;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
 import org.sufficientlysecure.keychain.provider.ProviderHelper;
 
 /**
@@ -32,8 +32,8 @@ class ProviderHelperStub extends ProviderHelper {
     }
 
     @Override
-    public WrappedPublicKeyRing getCanonicalizedPublicKeyRing(Uri id) throws NotFoundException {
+    public CanonicalizedPublicKeyRing getCanonicalizedPublicKeyRing(Uri id) throws NotFoundException {
         byte[] data = TestDataUtil.readFully(getClass().getResourceAsStream("/public-key-for-sample.blob"));
-        return new WrappedPublicKeyRing(data, false, 0);
+        return new CanonicalizedPublicKeyRing(data, 0);
     }
 }

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/support/UncachedKeyringTestingHelper.java

@@ -1,297 +0,0 @@
-/*
- * Copyright (C) Art O Cathain
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-package org.sufficientlysecure.keychain.support;
-
-import org.spongycastle.bcpg.BCPGKey;
-import org.spongycastle.bcpg.PublicKeyPacket;
-import org.spongycastle.bcpg.SignatureSubpacket;
-import org.spongycastle.openpgp.PGPException;
-import org.spongycastle.openpgp.PGPPublicKey;
-import org.spongycastle.openpgp.PGPSignature;
-import org.spongycastle.openpgp.PGPSignatureSubpacketVector;
-import org.spongycastle.openpgp.PGPUserAttributeSubpacketVector;
-import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
-import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
-import org.sufficientlysecure.keychain.service.OperationResultParcel;
-
-import java.util.Arrays;
-
-/**
- * Created by art on 28/06/14.
- */
-public class UncachedKeyringTestingHelper {
-
-    public static boolean compareRing(UncachedKeyRing keyRing1, UncachedKeyRing keyRing2) {
-        OperationResultParcel.OperationLog operationLog = new OperationResultParcel.OperationLog();
-        UncachedKeyRing canonicalized = keyRing1.canonicalize(operationLog, 0);
-
-        if (canonicalized == null) {
-            throw new AssertionError("Canonicalization failed; messages: [" + operationLog.toList() + "]");
-        }
-
-        return TestDataUtil.iterEquals(canonicalized.getPublicKeys(), keyRing2.getPublicKeys(), new
-                TestDataUtil.EqualityChecker<UncachedPublicKey>() {
-                    @Override
-                    public boolean areEquals(UncachedPublicKey lhs, UncachedPublicKey rhs) {
-                        return comparePublicKey(lhs, rhs);
-                    }
-                });
-    }
-
-    public static boolean comparePublicKey(UncachedPublicKey key1, UncachedPublicKey key2) {
-        boolean equal = true;
-
-        if (key1.canAuthenticate() != key2.canAuthenticate()) {
-            return false;
-        }
-        if (key1.canCertify() != key2.canCertify()) {
-            return false;
-        }
-        if (key1.canEncrypt() != key2.canEncrypt()) {
-            return false;
-        }
-        if (key1.canSign() != key2.canSign()) {
-            return false;
-        }
-        if (key1.getAlgorithm() != key2.getAlgorithm()) {
-            return false;
-        }
-        if (key1.getBitStrength() != key2.getBitStrength()) {
-            return false;
-        }
-        if (!TestDataUtil.equals(key1.getCreationTime(), key2.getCreationTime())) {
-            return false;
-        }
-        if (!TestDataUtil.equals(key1.getExpiryTime(), key2.getExpiryTime())) {
-            return false;
-        }
-        if (!Arrays.equals(key1.getFingerprint(), key2.getFingerprint())) {
-            return false;
-        }
-        if (key1.getKeyId() != key2.getKeyId()) {
-            return false;
-        }
-        if (key1.getKeyUsage() != key2.getKeyUsage()) {
-            return false;
-        }
-        if (!TestDataUtil.equals(key1.getPrimaryUserId(), key2.getPrimaryUserId())) {
-            return false;
-        }
-
-        // Ooops, getPublicKey is due to disappear. But then how to compare?
-        if (!keysAreEqual(key1.getPublicKey(), key2.getPublicKey())) {
-            return false;
-        }
-
-        return equal;
-    }
-
-    public static boolean keysAreEqual(PGPPublicKey a, PGPPublicKey b) {
-
-        if (a.getAlgorithm() != b.getAlgorithm()) {
-            return false;
-        }
-
-        if (a.getBitStrength() != b.getBitStrength()) {
-            return false;
-        }
-
-        if (!TestDataUtil.equals(a.getCreationTime(), b.getCreationTime())) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getFingerprint(), b.getFingerprint())) {
-            return false;
-        }
-
-        if (a.getKeyID() != b.getKeyID()) {
-            return false;
-        }
-
-        if (!pubKeyPacketsAreEqual(a.getPublicKeyPacket(), b.getPublicKeyPacket())) {
-            return false;
-        }
-
-        if (a.getVersion() != b.getVersion()) {
-            return false;
-        }
-
-        if (a.getValidDays() != b.getValidDays()) {
-            return false;
-        }
-
-        if (a.getValidSeconds() != b.getValidSeconds()) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getTrustData(), b.getTrustData())) {
-            return false;
-        }
-
-        if (!TestDataUtil.iterEquals(a.getUserIDs(), b.getUserIDs())) {
-            return false;
-        }
-
-        if (!TestDataUtil.iterEquals(a.getUserAttributes(), b.getUserAttributes(),
-                new TestDataUtil.EqualityChecker<PGPUserAttributeSubpacketVector>() {
-                    public boolean areEquals(PGPUserAttributeSubpacketVector lhs, PGPUserAttributeSubpacketVector rhs) {
-                        // For once, BC defines equals, so we use it implicitly.
-                        return TestDataUtil.equals(lhs, rhs);
-                    }
-                }
-        )) {
-            return false;
-        }
-
-
-        if (!TestDataUtil.iterEquals(a.getSignatures(), b.getSignatures(),
-                new TestDataUtil.EqualityChecker<PGPSignature>() {
-                    public boolean areEquals(PGPSignature lhs, PGPSignature rhs) {
-                        return signaturesAreEqual(lhs, rhs);
-                    }
-                }
-        )) {
-            return false;
-        }
-
-        return true;
-    }
-
-    public static boolean signaturesAreEqual(PGPSignature a, PGPSignature b) {
-
-        if (a.getVersion() != b.getVersion()) {
-            return false;
-        }
-
-        if (a.getKeyAlgorithm() != b.getKeyAlgorithm()) {
-            return false;
-        }
-
-        if (a.getHashAlgorithm() != b.getHashAlgorithm()) {
-            return false;
-        }
-
-        if (a.getSignatureType() != b.getSignatureType()) {
-            return false;
-        }
-
-        try {
-            if (!Arrays.equals(a.getSignature(), b.getSignature())) {
-                return false;
-            }
-        } catch (PGPException ex) {
-            throw new RuntimeException(ex);
-        }
-
-        if (a.getKeyID() != b.getKeyID()) {
-            return false;
-        }
-
-        if (!TestDataUtil.equals(a.getCreationTime(), b.getCreationTime())) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getSignatureTrailer(), b.getSignatureTrailer())) {
-            return false;
-        }
-
-        if (!subPacketVectorsAreEqual(a.getHashedSubPackets(), b.getHashedSubPackets())) {
-            return false;
-        }
-
-        if (!subPacketVectorsAreEqual(a.getUnhashedSubPackets(), b.getUnhashedSubPackets())) {
-            return false;
-        }
-
-        return true;
-    }
-
-    private static boolean subPacketVectorsAreEqual(PGPSignatureSubpacketVector aHashedSubPackets, PGPSignatureSubpacketVector bHashedSubPackets) {
-        for (int i = 0; i < Byte.MAX_VALUE; i++) {
-            if (!TestDataUtil.iterEquals(Arrays.asList(aHashedSubPackets.getSubpackets(i)).iterator(),
-                    Arrays.asList(bHashedSubPackets.getSubpackets(i)).iterator(),
-                    new TestDataUtil.EqualityChecker<SignatureSubpacket>() {
-                        @Override
-                        public boolean areEquals(SignatureSubpacket lhs, SignatureSubpacket rhs) {
-                            return signatureSubpacketsAreEqual(lhs, rhs);
-                        }
-                    }
-            )) {
-                return false;
-            }
-
-        }
-        return true;
-    }
-
-    private static boolean signatureSubpacketsAreEqual(SignatureSubpacket lhs, SignatureSubpacket rhs) {
-        if (lhs.getType() != rhs.getType()) {
-            return false;
-        }
-        if (!Arrays.equals(lhs.getData(), rhs.getData())) {
-            return false;
-        }
-        return true;
-    }
-
-    public static boolean pubKeyPacketsAreEqual(PublicKeyPacket a, PublicKeyPacket b) {
-
-        if (a.getAlgorithm() != b.getAlgorithm()) {
-            return false;
-        }
-
-        if (!bcpgKeysAreEqual(a.getKey(), b.getKey())) {
-            return false;
-        }
-
-        if (!TestDataUtil.equals(a.getTime(), b.getTime())) {
-            return false;
-        }
-
-        if (a.getValidDays() != b.getValidDays()) {
-            return false;
-        }
-
-        if (a.getVersion() != b.getVersion()) {
-            return false;
-        }
-
-        return true;
-    }
-
-    public static boolean bcpgKeysAreEqual(BCPGKey a, BCPGKey b) {
-
-        if (!TestDataUtil.equals(a.getFormat(), b.getFormat())) {
-            return false;
-        }
-
-        if (!Arrays.equals(a.getEncoded(), b.getEncoded())) {
-            return false;
-        }
-
-        return true;
-    }
-
-
-    public void doTestCanonicalize(UncachedKeyRing inputKeyRing, UncachedKeyRing expectedKeyRing) {
-        if (!compareRing(inputKeyRing, expectedKeyRing)) {
-            throw new AssertionError("Expected [" + inputKeyRing + "] to match [" + expectedKeyRing + "]");
-        }
-    }
-
-}

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/PgpKeyOperationTest.java

@@ -19,12 +19,13 @@ import org.spongycastle.bcpg.sig.KeyFlags;
 import org.spongycastle.openpgp.PGPSignature;
 import org.sufficientlysecure.keychain.Constants;
 import org.sufficientlysecure.keychain.Constants.choice.algorithm;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
 import org.sufficientlysecure.keychain.pgp.WrappedSignature;
-import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResultParcel.OperationLog;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyChange;
@@ -82,8 +83,7 @@ public class PgpKeyOperationTest {
         parcel.mNewPassphrase = passphrase;
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+        staticRing = op.createSecretKeyRing(parcel).getRing();
-        staticRing = op.createSecretKeyRing(parcel, log, 0);
 
         Assert.assertNotNull("initial test key creation must succeed", staticRing);
 
@@ -109,8 +109,6 @@ public class PgpKeyOperationTest {
     @Test
     public void createSecretKeyRingTests() {
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-
         {
             parcel.reset();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
@@ -118,7 +116,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
 
             Assert.assertNull("creating ring with < 512 bytes keysize should fail", ring);
         }
@@ -130,7 +128,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
 
             Assert.assertNull("creating ring with ElGamal master key should fail", ring);
         }
@@ -142,7 +140,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring with bad algorithm choice should fail", ring);
         }
 
@@ -153,7 +151,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring with non-certifying master key should fail", ring);
         }
 
@@ -163,7 +161,7 @@ public class PgpKeyOperationTest {
                     Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER, null));
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring without user ids should fail", ring);
         }
 
@@ -172,7 +170,7 @@ public class PgpKeyOperationTest {
             parcel.mAddUserIds.add("shy");
             parcel.mNewPassphrase = passphrase;
 
-            UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+            UncachedKeyRing ring = op.createSecretKeyRing(parcel).getRing();
             Assert.assertNull("creating ring without subkeys should fail", ring);
         }
 
@@ -186,11 +184,10 @@ public class PgpKeyOperationTest {
         parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                 Constants.choice.algorithm.rsa, 1024, KeyFlags.CERTIFY_OTHER | KeyFlags.SIGN_DATA, null));
         parcel.mAddUserIds.add("luna");
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+        ring = op.createSecretKeyRing(parcel).getRing();
-        ring = op.createSecretKeyRing(parcel, log, 0);
 
         Assert.assertEquals("the keyring should contain only the master key",
-                1, ring.getAvailableSubkeys().size());
+                1, KeyringTestingHelper.itToList(ring.getPublicKeys()).size());
         Assert.assertEquals("first (master) key must have both flags",
                 KeyFlags.CERTIFY_OTHER | KeyFlags.SIGN_DATA, ring.getPublicKey().getKeyUsage());
 
@@ -212,7 +209,7 @@ public class PgpKeyOperationTest {
                 2, ring.getPublicKey().getUnorderedUserIds().size());
 
         Assert.assertEquals("number of subkeys must be three",
-                3, ring.getAvailableSubkeys().size());
+                3, KeyringTestingHelper.itToList(ring.getPublicKeys()).size());
 
         Assert.assertTrue("key ring should have been created in the last 120 seconds",
                 ring.getPublicKey().getCreationTime().after(new Date(new Date().getTime()-1000*120)));
@@ -251,8 +248,7 @@ public class PgpKeyOperationTest {
             parcel.mFingerprint = ring.getFingerprint();
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("keyring modification with bad master key id should fail", modified);
         }
@@ -264,8 +260,7 @@ public class PgpKeyOperationTest {
             parcel.mFingerprint = ring.getFingerprint();
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("keyring modification with null master key id should fail", modified);
         }
@@ -278,8 +273,7 @@ public class PgpKeyOperationTest {
             parcel.mFingerprint[5] += 1;
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("keyring modification with bad fingerprint should fail", modified);
         }
@@ -290,16 +284,18 @@ public class PgpKeyOperationTest {
             parcel.mFingerprint = null;
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("keyring modification with null fingerprint should fail", modified);
         }
 
         {
+            String badphrase = "";
+            if (badphrase.equals(passphrase)) {
+                badphrase = "a";
+            }
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, badphrase).getRing();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, "bad passphrase", log, 0);
 
             Assert.assertNull("keyring modification with bad passphrase should fail", modified);
         }
@@ -356,8 +352,7 @@ public class PgpKeyOperationTest {
                     algorithm.rsa, new Random().nextInt(512), KeyFlags.SIGN_DATA, null));
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("creating a subkey with keysize < 512 should fail", modified);
         }
@@ -368,8 +363,7 @@ public class PgpKeyOperationTest {
                     new Date().getTime()/1000-10));
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("creating subkey with past expiry date should fail", modified);
         }
@@ -437,8 +431,7 @@ public class PgpKeyOperationTest {
             parcel.mChangeSubKeys.add(new SubkeyChange(keyId, null, new Date().getTime()/1000-10));
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("setting subkey expiry to a past date should fail", modified);
         }
@@ -448,8 +441,7 @@ public class PgpKeyOperationTest {
             parcel.mChangeSubKeys.add(new SubkeyChange(123, null, null));
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("modifying non-existent subkey should fail", modified);
         }
@@ -470,8 +462,7 @@ public class PgpKeyOperationTest {
             parcel.mRevokeSubKeys.add(123L);
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("revoking a nonexistent subkey should fail", otherModified);
 
@@ -573,8 +564,7 @@ public class PgpKeyOperationTest {
             parcel.mChangePrimaryUserId = uid;
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(modified.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing otherModified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("setting primary user id to a revoked user id should fail", otherModified);
 
@@ -622,8 +612,7 @@ public class PgpKeyOperationTest {
         {
             parcel.mAddUserIds.add("");
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
             Assert.assertNull("adding an empty user id should fail", modified);
         }
 
@@ -693,8 +682,7 @@ public class PgpKeyOperationTest {
             }
 
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            modified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
 
             Assert.assertNull("changing primary user id to a non-existent one should fail", modified);
         }
@@ -718,14 +706,14 @@ public class PgpKeyOperationTest {
                                                                ArrayList<RawPacket> onlyB,
                                                                boolean canonicalize,
                                                                boolean constantCanonicalize) {
+
         try {
 
             Assert.assertTrue("modified keyring must be secret", ring.isSecret());
             CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ring.getEncoded(), false, 0);
 
             PgpKeyOperation op = new PgpKeyOperation(null);
-            OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+            UncachedKeyRing rawModified = op.modifySecretKeyRing(secretRing, parcel, passphrase).getRing();
-            UncachedKeyRing rawModified = op.modifySecretKeyRing(secretRing, parcel, passphrase, log, 0);
             Assert.assertNotNull("key modification failed", rawModified);
 
             if (!canonicalize) {
@@ -734,7 +722,7 @@ public class PgpKeyOperationTest {
                 return rawModified;
             }
 
-            UncachedKeyRing modified = rawModified.canonicalize(log, 0);
+            CanonicalizedKeyRing modified = rawModified.canonicalize(new OperationLog(), 0);
             if (constantCanonicalize) {
                 Assert.assertTrue("key must be constant through canonicalization",
                         !KeyringTestingHelper.diffKeyrings(
@@ -743,7 +731,8 @@ public class PgpKeyOperationTest {
             }
             Assert.assertTrue("keyring must differ from original", KeyringTestingHelper.diffKeyrings(
                     ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
-            return modified;
+
+            return modified.getUncachedKeyRing();
 
         } catch (IOException e) {
             throw new AssertionFailedError("error during encoding!");
@@ -756,12 +745,8 @@ public class PgpKeyOperationTest {
         UncachedKeyRing expectedKeyRing = KeyringBuilder.correctRing();
         UncachedKeyRing inputKeyRing = KeyringBuilder.ringWithExtraIncorrectSignature();
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+        CanonicalizedKeyRing canonicalized = inputKeyRing.canonicalize(new OperationLog(), 0);
-        UncachedKeyRing canonicalizedRing = inputKeyRing.canonicalize(log, 0);
+        Assert.assertNotNull("canonicalization must succeed", canonicalized);
-
-        if (canonicalizedRing == null) {
-            throw new AssertionError("Canonicalization failed; messages: [" + log + "]");
-        }
 
         ArrayList onlyA = new ArrayList<RawPacket>();
         ArrayList onlyB = new ArrayList<RawPacket>();

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringCanonicalizeTest.java

@@ -26,11 +26,13 @@ import org.spongycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator;
 import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder;
 import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
 import org.sufficientlysecure.keychain.pgp.WrappedSignature;
 import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
@@ -78,9 +80,9 @@ public class UncachedKeyringCanonicalizeTest {
         parcel.mNewPassphrase = "";
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+        EditKeyResult result = op.createSecretKeyRing(parcel);
-        staticRing = op.createSecretKeyRing(parcel, log, 0);
+        Assert.assertTrue("initial test key creation must succeed", result.success());
-
+        staticRing = result.getRing();
         Assert.assertNotNull("initial test key creation must succeed", staticRing);
 
         // just for later reference
@@ -147,18 +149,18 @@ public class UncachedKeyringCanonicalizeTest {
 
         { // bad certificates get stripped
             UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, brokenSig.getEncoded(), 3);
-            modified = modified.canonicalize(log, 0);
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
 
             Assert.assertTrue("canonicalized keyring with invalid extra sig must be same as original one",
                     !KeyringTestingHelper.diffKeyrings(
-                        ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
+                        ring.getEncoded(), canonicalized.getEncoded(), onlyA, onlyB));
         }
 
         // remove user id certificate for one user
         final UncachedKeyRing base = KeyringTestingHelper.removePacket(ring, 2);
 
         { // user id without certificate should be removed
-            UncachedKeyRing modified = base.canonicalize(log, 0);
+            CanonicalizedKeyRing modified = base.canonicalize(log, 0);
             Assert.assertTrue("canonicalized keyring must differ", KeyringTestingHelper.diffKeyrings(
                     ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
 
@@ -178,10 +180,10 @@ public class UncachedKeyringCanonicalizeTest {
         { // add error to signature
 
             UncachedKeyRing modified = KeyringTestingHelper.injectPacket(base, brokenSig.getEncoded(), 3);
-            modified = modified.canonicalize(log, 0);
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
 
             Assert.assertTrue("canonicalized keyring must differ", KeyringTestingHelper.diffKeyrings(
-                    ring.getEncoded(), modified.getEncoded(), onlyA, onlyB));
+                    ring.getEncoded(), canonicalized.getEncoded(), onlyA, onlyB));
 
             Assert.assertEquals("two packets should be missing after canonicalization", 2, onlyA.size());
             Assert.assertEquals("no new packets after canonicalization", 0, onlyB.size());
@@ -205,7 +207,7 @@ public class UncachedKeyringCanonicalizeTest {
         ring = KeyringTestingHelper.removePacket(ring, 3);
 
         // canonicalization should fail, because there are no valid uids left
-        UncachedKeyRing canonicalized = ring.canonicalize(log, 0);
+        CanonicalizedKeyRing canonicalized = ring.canonicalize(log, 0);
         Assert.assertNull("canonicalization of keyring with no valid uids should fail", canonicalized);
 
     }
@@ -284,7 +286,7 @@ public class UncachedKeyringCanonicalizeTest {
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-        UncachedKeyRing foreign = op.createSecretKeyRing(parcel, log, 0);
+        UncachedKeyRing foreign = op.createSecretKeyRing(parcel).getRing();
 
         Assert.assertNotNull("initial test key creation must succeed", foreign);
         PGPSecretKey foreignSecretKey =
@@ -321,7 +323,7 @@ public class UncachedKeyringCanonicalizeTest {
         UncachedKeyRing modified = KeyringTestingHelper.removePacket(ring, 6);
 
         // canonicalization should fail, because there are no valid uids left
-        UncachedKeyRing canonicalized = modified.canonicalize(log, 0);
+        CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
         Assert.assertTrue("keyring with missing subkey binding sig should differ from intact one after canonicalization",
                 KeyringTestingHelper.diffKeyrings(ring.getEncoded(), canonicalized.getEncoded(),
                         onlyA, onlyB)
@@ -367,7 +369,7 @@ public class UncachedKeyringCanonicalizeTest {
             UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, sig.getEncoded(), 6);
 
             // canonicalize, and check if we lose the bad signature
-            UncachedKeyRing canonicalized = modified.canonicalize(log, 0);
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
             Assert.assertFalse("subkey binding signature should be gone after canonicalization",
                     KeyringTestingHelper.diffKeyrings(ring.getEncoded(), canonicalized.getEncoded(),
                             onlyA, onlyB)
@@ -392,7 +394,7 @@ public class UncachedKeyringCanonicalizeTest {
             UncachedKeyRing modified = KeyringTestingHelper.injectPacket(ring, sig.getEncoded(), 6);
 
             // canonicalize, and check if we lose the bad signature
-            UncachedKeyRing canonicalized = modified.canonicalize(log, 0);
+            CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
             Assert.assertFalse("subkey binding signature should be gone after canonicalization",
                     KeyringTestingHelper.diffKeyrings(ring.getEncoded(), canonicalized.getEncoded(),
                             onlyA, onlyB)
@@ -427,7 +429,7 @@ public class UncachedKeyringCanonicalizeTest {
         modified = KeyringTestingHelper.injectPacket(modified, sig3.getEncoded(), 11);
 
         // canonicalize, and check if we lose the bad signature
-        UncachedKeyRing canonicalized = modified.canonicalize(log, 0);
+        CanonicalizedKeyRing canonicalized = modified.canonicalize(log, 0);
         Assert.assertTrue("subkey binding signature should be gone after canonicalization",
                 KeyringTestingHelper.diffKeyrings(modified.getEncoded(), canonicalized.getEncoded(),
                         onlyA, onlyB)
@@ -524,14 +526,14 @@ public class UncachedKeyringCanonicalizeTest {
 
                 UncachedKeyRing brokenRing = UncachedKeyRing.decodeFromData(brokenEncoded);
 
-                brokenRing = brokenRing.canonicalize(log, 0);
+                CanonicalizedKeyRing canonicalized = brokenRing.canonicalize(log, 0);
-                if (brokenRing == null) {
+                if (canonicalized == null) {
                     System.out.println("ok, canonicalization failed.");
                     continue;
                 }
 
                 Assert.assertArrayEquals("injected bad signature must be gone after canonicalization",
-                        ring.getEncoded(), brokenRing.getEncoded());
+                        ring.getEncoded(), canonicalized.getEncoded());
 
             } catch (Exception e) {
                 System.out.println("ok, rejected with: " + e.getMessage());

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringMergeTest.java

@@ -10,13 +10,15 @@ import org.robolectric.shadows.ShadowLog;
 import org.spongycastle.bcpg.PacketTags;
 import org.spongycastle.bcpg.sig.KeyFlags;
 import org.sufficientlysecure.keychain.Constants;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
 import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
-import org.sufficientlysecure.keychain.pgp.WrappedPublicKeyRing;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing;
 import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey;
-import org.sufficientlysecure.keychain.pgp.WrappedSecretKeyRing;
+import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKeyRing;
 import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
@@ -79,7 +81,9 @@ public class UncachedKeyringMergeTest {
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            staticRingA = op.createSecretKeyRing(parcel, log, 0);
+
+            EditKeyResult result = op.createSecretKeyRing(parcel);
+            staticRingA = result.getRing();
         }
 
         {
@@ -93,7 +97,8 @@ public class UncachedKeyringMergeTest {
             PgpKeyOperation op = new PgpKeyOperation(null);
 
             OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-            staticRingB = op.createSecretKeyRing(parcel, log, 0);
+            EditKeyResult result = op.createSecretKeyRing(parcel);
+            staticRingB = result.getRing();
         }
 
         Assert.assertNotNull("initial test key creation must succeed", staticRingA);
@@ -145,15 +150,16 @@ public class UncachedKeyringMergeTest {
     public void testAddedUserId() throws Exception {
 
         UncachedKeyRing modifiedA, modifiedB; {
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ringA.getEncoded(), false, 0);
+            CanonicalizedSecretKeyRing secretRing =
+                    new CanonicalizedSecretKeyRing(ringA.getEncoded(), false, 0);
 
             parcel.reset();
             parcel.mAddUserIds.add("flim");
-            modifiedA = op.modifySecretKeyRing(secretRing, parcel, "", log, 0);
+            modifiedA = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
 
             parcel.reset();
             parcel.mAddUserIds.add("flam");
-            modifiedB = op.modifySecretKeyRing(secretRing, parcel, "", log, 0);
+            modifiedB = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
         }
 
         { // merge A into base
@@ -185,13 +191,13 @@ public class UncachedKeyringMergeTest {
         UncachedKeyRing modifiedA, modifiedB;
         long subKeyIdA, subKeyIdB;
         {
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(ringA.getEncoded(), false, 0);
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(ringA.getEncoded(), false, 0);
 
             parcel.reset();
             parcel.mAddSubKeys.add(new SaveKeyringParcel.SubkeyAdd(
                     Constants.choice.algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null));
-            modifiedA = op.modifySecretKeyRing(secretRing, parcel, "", log, 0);
+            modifiedA = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
-            modifiedB = op.modifySecretKeyRing(secretRing, parcel, "", log, 0);
+            modifiedB = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
 
             subKeyIdA = KeyringTestingHelper.getSubkeyId(modifiedA, 2);
             subKeyIdB = KeyringTestingHelper.getSubkeyId(modifiedB, 2);
@@ -230,9 +236,9 @@ public class UncachedKeyringMergeTest {
         final UncachedKeyRing modified; {
             parcel.reset();
             parcel.mRevokeSubKeys.add(KeyringTestingHelper.getSubkeyId(ringA, 1));
-            WrappedSecretKeyRing secretRing = new WrappedSecretKeyRing(
+            CanonicalizedSecretKeyRing secretRing = new CanonicalizedSecretKeyRing(
                     ringA.getEncoded(), false, 0);
-            modified = op.modifySecretKeyRing(secretRing, parcel, "", log, 0);
+            modified = op.modifySecretKeyRing(secretRing, parcel, "").getRing();
         }
 
         {
@@ -252,10 +258,10 @@ public class UncachedKeyringMergeTest {
         final UncachedKeyRing pubRing = ringA.extractPublicKeyRing();
 
         final UncachedKeyRing modified; {
-            WrappedPublicKeyRing publicRing = new WrappedPublicKeyRing(
+            CanonicalizedPublicKeyRing publicRing = new CanonicalizedPublicKeyRing(
-                    pubRing.getEncoded(), false, 0);
+                    pubRing.getEncoded(), 0);
 
-            CanonicalizedSecretKey secretKey = new WrappedSecretKeyRing(
+            CanonicalizedSecretKey secretKey = new CanonicalizedSecretKeyRing(
                     ringB.getEncoded(), false, 0).getSecretKey();
             secretKey.unlock("");
             // sign all user ids

OpenKeychain-Test/src/test/java/org/sufficientlysecure/keychain/tests/UncachedKeyringTest.java

@@ -14,6 +14,7 @@ import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
 import org.sufficientlysecure.keychain.pgp.UncachedPublicKey;
 import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
 import org.sufficientlysecure.keychain.service.OperationResultParcel;
+import org.sufficientlysecure.keychain.service.OperationResults.EditKeyResult;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
 import org.sufficientlysecure.keychain.util.ProgressScaler;
@@ -32,7 +33,6 @@ public class UncachedKeyringTest {
     UncachedKeyRing ring, pubRing;
     ArrayList<RawPacket> onlyA = new ArrayList<RawPacket>();
     ArrayList<RawPacket> onlyB = new ArrayList<RawPacket>();
-    OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
     PgpKeyOperation op;
     SaveKeyringParcel parcel;
 
@@ -54,8 +54,8 @@ public class UncachedKeyringTest {
         parcel.mNewPassphrase = "";
         PgpKeyOperation op = new PgpKeyOperation(null);
 
-        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+        EditKeyResult result = op.createSecretKeyRing(parcel);
-        staticRing = op.createSecretKeyRing(parcel, log, 0);
+        staticRing = result.getRing();
         staticPubRing = staticRing.extractPublicKeyRing();
 
         Assert.assertNotNull("initial test key creation must succeed", staticRing);
@@ -102,24 +102,20 @@ public class UncachedKeyringTest {
         ring.encodeArmored(out, "OpenKeychain");
         pubRing.encodeArmored(out, "OpenKeychain");
 
-        List<UncachedKeyRing> rings =
+        Iterator<UncachedKeyRing> it =
                 UncachedKeyRing.fromStream(new ByteArrayInputStream(out.toByteArray()));
-        Assert.assertEquals("there should be two rings in the stream", 2, rings.size());
+        Assert.assertTrue("there should be two rings in the stream", it.hasNext());
         Assert.assertArrayEquals("first ring should be the first we put in",
-                ring.getEncoded(), rings.get(0).getEncoded());
+                ring.getEncoded(), it.next().getEncoded());
+        Assert.assertTrue("there should be two rings in the stream", it.hasNext());
         Assert.assertArrayEquals("second ring should be the second we put in",
-                pubRing.getEncoded(), rings.get(1).getEncoded());
+                pubRing.getEncoded(), it.next().getEncoded());
+        Assert.assertFalse("there should be two rings in the stream", it.hasNext());
 
         // this should fail with PgpGeneralException, since it expects exactly one ring
         UncachedKeyRing.decodeFromData(out.toByteArray());
     }
 
-    @Test(expected = RuntimeException.class)
-    public void testPublicAvailableSubkeys() throws Exception {
-        // can't do this!
-        pubRing.getAvailableSubkeys();
-    }
-
     @Test(expected = RuntimeException.class)
     public void testPublicExtractPublic() throws Exception {
         // can't do this, either!