Allow saving push certs on a subset of refs

Consider a BatchRefUpdate produced by Gerrit Code Review, where the
original command pushed over the wire might refer to
"refs/for/master", but that command is ignored and replaced with some
additional commands like creating "refs/changes/34/1234/1". We do not
want to store the cert in "refs/for/master@{cert}", since that may
lead someone looking to the ref to the incorrect conclusion that that
ref exists.

Add a separate put method that takes a collection of commands, and
only stores certs on those refs that have a matching command in the
cert.

Change-Id: I4661bfe2ead28a2883b33a4e3dfe579b3157d68a

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateStoreTest.java

@@ -55,6 +55,7 @@ import java.io.IOException;
 import java.io.InputStreamReader;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -284,15 +285,36 @@ public class PushCertificateStoreTest {
 		List<ReceiveCommand> commands = batch.getCommands();
 		assertEquals(1, commands.size());
 		ReceiveCommand cmd = commands.get(0);
-
-		try (RevWalk rw = new RevWalk(repo)) {
 		assertEquals("refs/meta/push-certs", cmd.getRefName());
 		assertEquals(ReceiveCommand.Result.NOT_ATTEMPTED, cmd.getResult());
+
+		try (RevWalk rw = new RevWalk(repo)) {
 			batch.execute(rw, NullProgressMonitor.INSTANCE);
 			assertEquals(ReceiveCommand.Result.OK, cmd.getResult());
 		}
 	}
 
+	@Test
+	public void putMatchingWithNoMatchingRefs() throws Exception {
+		PushCertificate addMaster = newCert(
+				command(zeroId(), ID1, "refs/heads/master"),
+				command(zeroId(), ID2, "refs/heads/branch"));
+		store.put(addMaster, newIdent(), Collections.<ReceiveCommand> emptyList());
+		assertEquals(NO_CHANGE, store.save());
+	}
+
+	@Test
+	public void putMatchingWithSomeMatchingRefs() throws Exception {
+		PushCertificate addMasterAndBranch = newCert(
+				command(zeroId(), ID1, "refs/heads/master"),
+				command(zeroId(), ID2, "refs/heads/branch"));
+		store.put(addMasterAndBranch, newIdent(),
+				Collections.singleton(addMasterAndBranch.getCommands().get(0)));
+		assertEquals(NEW, store.save());
+		assertCerts("refs/heads/master", addMasterAndBranch);
+		assertCerts("refs/heads/branch");
+	}
+
 	private PersonIdent newIdent() {
 		return new PersonIdent(
 				"A U. Thor", "author@example.com", ts.getAndIncrement(), 0);

org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateStore.java

@@ -55,10 +55,13 @@ import java.io.InputStreamReader;
 import java.io.Reader;
 import java.text.MessageFormat;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.NoSuchElementException;
 
 import org.eclipse.jgit.dircache.DirCache;
@@ -106,10 +109,13 @@ public class PushCertificateStore implements AutoCloseable {
 	private static class PendingCert {
 		private PushCertificate cert;
 		private PersonIdent ident;
+		private Collection<ReceiveCommand> matching;
 
-		private PendingCert(PushCertificate cert, PersonIdent ident) {
+		private PendingCert(PushCertificate cert, PersonIdent ident,
+				Collection<ReceiveCommand> matching) {
 			this.cert = cert;
 			this.ident = ident;
+			this.matching = matching;
 		}
 	}
 
@@ -308,7 +314,34 @@ public class PushCertificateStore implements AutoCloseable {
 	 *            #save()}.
 	 */
 	public void put(PushCertificate cert, PersonIdent ident) {
-		pending.add(new PendingCert(cert, ident));
+		put(cert, ident, null);
+	}
+
+	/**
+	 * Put a certificate to be saved to the store, matching a set of commands.
+	 * <p>
+	 * Like {@link #put(PushCertificate, PersonIdent)}, except a value is only
+	 * stored for a push certificate if there is a corresponding command in the
+	 * list that exactly matches the old/new values mentioned in the push
+	 * certificate.
+	 * <p>
+	 * Pending certificates added to this method are not returned by {@link
+	 * #get(String)} and {@link #getAll(String)} until after calling {@link
+	 * #save()}.
+	 *
+	 * @param cert
+	 *            certificate to store.
+	 * @param ident
+	 *            identity for the commit that stores this certificate. Pending
+	 *            certificates are sorted by identity timestamp during {@link
+	 *            #save()}.
+	 * @param matching
+	 *            only store certs for the refs listed in this list whose values
+	 *            match the commands in the cert.
+	 */
+	public void put(PushCertificate cert, PersonIdent ident,
+			Collection<ReceiveCommand> matching) {
+		pending.add(new PendingCert(cert, ident, matching));
 	}
 
 	/**
@@ -355,8 +388,9 @@ public class PushCertificateStore implements AutoCloseable {
 	 * #put(PushCertificate, PersonIdent)}, in order of identity timestamps, all
 	 * commits are flushed, and a single command is added to the batch.
 	 * <p>
-	 * The pending list is <em>not</em> cleared. If the ref update succeeds, the
-	 * caller is responsible for calling {@link #clear()}.
+	 * The cached ref value and pending list are <em>not</em> cleared. If the ref
+	 * update succeeds, the caller is responsible for calling {@link #close()}
+	 * and/or {@link #clear()}.
 	 *
 	 * @param batch
 	 *            update to save to.
@@ -423,10 +457,27 @@ public class PushCertificateStore implements AutoCloseable {
 
 	private ObjectId saveCert(ObjectInserter inserter, DirCache dc,
 			PendingCert pc, ObjectId curr) throws IOException {
+		Map<String, ReceiveCommand> byRef;
+		if (pc.matching != null) {
+			byRef = new HashMap<>();
+			for (ReceiveCommand cmd : pc.matching) {
+				if (byRef.put(cmd.getRefName(), cmd) != null) {
+					throw new IllegalStateException();
+				}
+			}
+		} else {
+			byRef = null;
+		}
+
 		DirCacheEditor editor = dc.editor();
 		String certText = pc.cert.toText() + pc.cert.getSignature();
 		final ObjectId certId = inserter.insert(OBJ_BLOB, certText.getBytes(UTF_8));
+		boolean any = false;
 		for (ReceiveCommand cmd : pc.cert.getCommands()) {
+			if (byRef != null && !commandsEqual(cmd, byRef.get(cmd.getRefName()))) {
+				continue;
+			}
+			any = true;
 			editor.add(new PathEdit(pathName(cmd.getRefName())) {
 				@Override
 				public void apply(DirCacheEntry ent) {
@@ -435,6 +486,9 @@ public class PushCertificateStore implements AutoCloseable {
 				}
 			});
 		}
+		if (!any) {
+			return curr;
+		}
 		editor.finish();
 		CommitBuilder cb = new CommitBuilder();
 		cb.setAuthor(pc.ident);
@@ -449,6 +503,15 @@ public class PushCertificateStore implements AutoCloseable {
 		return inserter.insert(OBJ_COMMIT, cb.build());
 	}
 
+	private static boolean commandsEqual(ReceiveCommand c1, ReceiveCommand c2) {
+		if (c1 == null || c2 == null) {
+			return c1 == c2;
+		}
+		return c1.getRefName().equals(c2.getRefName())
+				&& c1.getOldId().equals(c2.getOldId())
+				&& c1.getNewId().equals(c2.getNewId());
+	}
+
 	private RefUpdate.Result updateRef(ObjectId newId) throws IOException {
 		RefUpdate ru = db.updateRef(REF_NAME);
 		ru.setExpectedOldObjectId(commit != null ? commit : ObjectId.zeroId());