From 45e9e28ad9daad3c64a16685bd9c07ed18d2ab50 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Wed, 2 Sep 2015 15:04:33 -0400
Subject: [PATCH] PushCertificateStore: Don't add no-op command to batch

If no refs match the input list and we are writing to a batch,
the returned new commit from write() will match the current commit.
Adding a command to the batch for this case is harmless as it will
succeed, but it's more straightforward to just skip adding a command
in this case.

Add tests or the combination of saving matching refs and saving to a
batch.

Change-Id: I6837389b08e6c80bc2d4c9e9c506d07293ea5fb2
---
 .../transport/PushCertificateStoreTest.java   | 28 +++++++++++++++++++
 .../jgit/transport/PushCertificateStore.java  |  2 +-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateStoreTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateStoreTest.java
index 459ca876d..68e012952 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateStoreTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateStoreTest.java
@@ -307,6 +307,34 @@ public class PushCertificateStoreTest {
 		assertEquals(NO_CHANGE, store.save());
 	}
 
+	@Test
+	public void putMatchingWithNoMatchingRefsInBatchOnEmptyRef()
+			throws Exception {
+		PushCertificate addMaster = newCert(
+				command(zeroId(), ID1, "refs/heads/master"),
+				command(zeroId(), ID2, "refs/heads/branch"));
+		store.put(addMaster, newIdent(), Collections.<ReceiveCommand> emptyList());
+		BatchRefUpdate batch = repo.getRefDatabase().newBatchUpdate();
+		assertFalse(store.save(batch));
+		assertEquals(0, batch.getCommands().size());
+	}
+
+	@Test
+	public void putMatchingWithNoMatchingRefsInBatchOnNonEmptyRef()
+			throws Exception {
+		PushCertificate addMaster = newCert(
+				command(zeroId(), ID1, "refs/heads/master"));
+		store.put(addMaster, newIdent());
+		assertEquals(NEW, store.save());
+
+		PushCertificate addBranch = newCert(
+				command(zeroId(), ID2, "refs/heads/branch"));
+		store.put(addBranch, newIdent(), Collections.<ReceiveCommand> emptyList());
+		BatchRefUpdate batch = repo.getRefDatabase().newBatchUpdate();
+		assertFalse(store.save(batch));
+		assertEquals(0, batch.getCommands().size());
+	}
+
 	@Test
 	public void putMatchingWithSomeMatchingRefs() throws Exception {
 		PushCertificate addMasterAndBranch = newCert(
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateStore.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateStore.java
index 389bd689c..d8672d5a2 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateStore.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateStore.java
@@ -401,7 +401,7 @@ public class PushCertificateStore implements AutoCloseable {
 	 */
 	public boolean save(BatchRefUpdate batch) throws IOException {
 		ObjectId newId = write();
-		if (newId == null) {
+		if (newId == null || newId.equals(commit)) {
 			return false;
 		}
 		batch.addCommand(new ReceiveCommand(