tests: add testSubkeyAdd

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

@@ -30,6 +30,7 @@ import java.io.InputStream;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.SortedSet;
 
 /**
  * Helper for tests of the Keyring import in ProviderHelper.
@@ -66,10 +67,15 @@ public class KeyringTestingHelper {
         return saveSuccess;
     }
 
-    public static class Packet {
+    public static class Packet implements Comparable<Packet> {
-        int tag;
+        public int position;
-        int length;
+        public int tag;
-        byte[] buf;
+        public int length;
+        public byte[] buf;
+
+        public int compareTo(Packet other) {
+            return Integer.compare(position, other.position);
+        }
 
         public boolean equals(Object other) {
             return other instanceof Packet && Arrays.areEqual(this.buf, ((Packet) other).buf);
@@ -81,7 +87,8 @@ public class KeyringTestingHelper {
         }
     }
 
-    public static boolean diffKeyrings(byte[] ringA, byte[] ringB, Set<Packet> onlyA, Set<Packet> onlyB)
+    public static boolean diffKeyrings(byte[] ringA, byte[] ringB,
+                                       SortedSet<Packet> onlyA, SortedSet<Packet> onlyB)
             throws IOException {
         InputStream streamA = new ByteArrayInputStream(ringA);
         InputStream streamB = new ByteArrayInputStream(ringB);
@@ -89,18 +96,22 @@ public class KeyringTestingHelper {
         HashSet<Packet> a = new HashSet<Packet>(), b = new HashSet<Packet>();
 
         Packet p;
+        int pos = 0;
         while(true) {
             p = readPacket(streamA);
             if (p == null) {
                 break;
             }
+            p.position = pos++;
             a.add(p);
         }
+        pos = 0;
         while(true) {
             p = readPacket(streamB);
             if (p == null) {
                 break;
             }
+            p.position = pos++;
             b.add(p);
         }
 
@@ -109,7 +120,7 @@ public class KeyringTestingHelper {
         onlyB.addAll(b);
         onlyB.removeAll(a);
 
-        return onlyA.isEmpty() && onlyB.isEmpty();
+        return !onlyA.isEmpty() || !onlyB.isEmpty();
     }
 
     private static Packet readPacket(InputStream in) throws IOException {

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

@@ -7,24 +7,31 @@ import org.junit.BeforeClass;
 import org.junit.runner.RunWith;
 import org.robolectric.*;
 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.Constants.choice.algorithm;
 import org.sufficientlysecure.keychain.pgp.PgpKeyOperation;
 import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
+import org.sufficientlysecure.keychain.pgp.WrappedSecretKeyRing;
 import org.sufficientlysecure.keychain.service.OperationResultParcel;
 import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
+import org.sufficientlysecure.keychain.service.SaveKeyringParcel.SubkeyAdd;
 import org.sufficientlysecure.keychain.support.KeyringBuilder;
 import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
+import org.sufficientlysecure.keychain.support.KeyringTestingHelper.Packet;
 import org.sufficientlysecure.keychain.support.TestDataUtil;
 
-import java.util.HashSet;
+import java.util.Iterator;
+import java.util.TreeSet;
 
 @RunWith(RobolectricTestRunner.class)
 @org.robolectric.annotation.Config(emulateSdk = 18) // Robolectric doesn't yet support 19
-public class UncachedKeyringTest {
+public class PgpKeyOperationTest {
 
-    static UncachedKeyRing staticRing;
+    static WrappedSecretKeyRing staticRing;
-    UncachedKeyRing ring;
+    WrappedSecretKeyRing ring;
+    PgpKeyOperation op;
 
     @BeforeClass public static void setUpOnce() throws Exception {
         SaveKeyringParcel parcel = new SaveKeyringParcel();
@@ -36,27 +43,56 @@ public class UncachedKeyringTest {
         PgpKeyOperation op = new PgpKeyOperation(null);
 
         OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
-        staticRing = op.createSecretKeyRing(parcel, log, 0);
+        UncachedKeyRing ring = op.createSecretKeyRing(parcel, log, 0);
+        staticRing = new WrappedSecretKeyRing(ring.getEncoded(), false, 0);
     }
 
     @Before public void setUp() throws Exception {
         // show Log.x messages in system.out
         ShadowLog.stream = System.out;
         ring = staticRing;
+
+        op = new PgpKeyOperation(null);
     }
 
     @Test
-    public void testCreateKey() throws Exception {
+    public void testCreatedKey() throws Exception {
 
         // parcel.addSubKeys.add(new SubkeyAdd(algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null));
 
         Assert.assertNotNull("key creation failed", ring);
 
         Assert.assertEquals("incorrect primary user id",
-                "swagerinho", ring.getPublicKey().getPrimaryUserId());
+                "swagerinho", ring.getPrimaryUserId());
 
         Assert.assertEquals("wrong number of subkeys",
-                1, ring.getAvailableSubkeys().size());
+                1, ring.getUncachedKeyRing().getAvailableSubkeys().size());
+
+    }
+
+    @Test
+    public void testSubkeyAdd() throws Exception {
+
+        SaveKeyringParcel parcel = new SaveKeyringParcel();
+        parcel.mMasterKeyId = ring.getMasterKeyId();
+        parcel.mFingerprint = ring.getUncached().getFingerprint();
+        parcel.addSubKeys.add(new SubkeyAdd(algorithm.rsa, 1024, KeyFlags.SIGN_DATA, null));
+
+        OperationResultParcel.OperationLog log = new OperationResultParcel.OperationLog();
+        UncachedKeyRing modified = op.modifySecretKeyRing(ring, parcel, "swag", log, 0);
+
+        Assert.assertNotNull("key modification failed", modified);
+
+        TreeSet<Packet> onlyA = new TreeSet<Packet>();
+        TreeSet<Packet> onlyB = new TreeSet<Packet>();
+        Assert.assertTrue("keyrings do not differ", KeyringTestingHelper.diffKeyrings(
+                ring.getUncached().getEncoded(), modified.getEncoded(), onlyA, onlyB));
+
+        Assert.assertEquals("no extra packets in original", onlyA.size(), 0);
+        Assert.assertEquals("two extra packets in modified", onlyB.size(), 2);
+        Iterator<Packet> it = onlyB.iterator();
+        Assert.assertEquals("first new packet must be secret subkey", it.next().tag, PacketTags.SECRET_SUBKEY);
+        Assert.assertEquals("second new packet must be signature", it.next().tag, PacketTags.SIGNATURE);
 
     }
 
@@ -73,9 +109,9 @@ public class UncachedKeyringTest {
             throw new AssertionError("Canonicalization failed; messages: [" + log + "]");
         }
 
-        HashSet onlyA = new HashSet<KeyringTestingHelper.Packet>();
+        TreeSet onlyA = new TreeSet<KeyringTestingHelper.Packet>();
-        HashSet onlyB = new HashSet<KeyringTestingHelper.Packet>();
+        TreeSet onlyB = new TreeSet<KeyringTestingHelper.Packet>();
-        Assert.assertTrue(KeyringTestingHelper.diffKeyrings(
+        Assert.assertTrue("keyrings differ", !KeyringTestingHelper.diffKeyrings(
                 expectedKeyRing.getEncoded(), expectedKeyRing.getEncoded(), onlyA, onlyB));
 
     }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedKeyRing.java

@@ -101,6 +101,10 @@ public abstract class WrappedKeyRing extends KeyRing {
 
     abstract public IterableIterator<WrappedPublicKey> publicKeyIterator();
 
+    public byte[] getEncoded() throws IOException {
+        return getRing().getEncoded();
+    }
+
     public UncachedKeyRing getUncached() {
         return new UncachedKeyRing(getRing());
     }

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/OperationResultParcel.java

@@ -325,6 +325,7 @@ public class OperationResultParcel implements Parcelable {
         }
 
         public void add(LogLevel level, LogType type, int indent) {
+            Log.d(Constants.TAG, type.toString());
             parcels.add(new OperationResultParcel.LogEntryParcel(level, type, indent, (Object[]) null));
         }