Merge "Push: Ensure ref updates are processed in input order"

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

@@ -51,12 +51,16 @@ import static org.junit.Assert.fail;
 
 import java.io.IOException;
 import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectId;
@@ -76,6 +80,7 @@ public class PushConnectionTest {
 	private Object ctx = new Object();
 	private InMemoryRepository server;
 	private InMemoryRepository client;
+	private List<String> processedRefs;
 	private ObjectId obj1;
 	private ObjectId obj2;
 	private ObjectId obj3;
@@ -85,6 +90,7 @@ public class PushConnectionTest {
 	public void setUp() throws Exception {
 		server = newRepo("server");
 		client = newRepo("client");
+		processedRefs = new ArrayList<>();
 		testProtocol = new TestProtocol<>(
 				null,
 				new ReceivePackFactory<Object>() {
@@ -92,7 +98,18 @@ public class PushConnectionTest {
 					public ReceivePack create(Object req, Repository db)
 							throws ServiceNotEnabledException,
 							ServiceNotAuthorizedException {
-						return new ReceivePack(db);
+						ReceivePack rp = new ReceivePack(db);
+						rp.setPreReceiveHook(
+								new PreReceiveHook() {
+									@Override
+									public void onPreReceive(ReceivePack receivePack,
+											Collection<ReceiveCommand> cmds) {
+										for (ReceiveCommand cmd : cmds) {
+											processedRefs.add(cmd.getRefName());
+										}
+									}
+								});
+						return rp;
 					}
 				});
 		uri = testProtocol.register(ctx, server);
@@ -196,4 +213,45 @@ public class PushConnectionTest {
 			}
 		}
 	}
+
+	@Test
+	public void commandOrder() throws Exception {
+		TestRepository<?> tr = new TestRepository<>(client);
+		List<RemoteRefUpdate> updates = new ArrayList<>();
+		// Arbitrary non-sorted order.
+		for (int i = 9; i >= 0; i--) {
+			String name = "refs/heads/b" + i;
+			tr.branch(name).commit().create();
+			RemoteRefUpdate rru = new RemoteRefUpdate(client, name, name, false, null,
+					ObjectId.zeroId());
+			updates.add(rru);
+		}
+
+		PushResult result;
+		try (Transport tn = testProtocol.open(uri, client, "server")) {
+			result = tn.push(NullProgressMonitor.INSTANCE, updates);
+		}
+
+		for (RemoteRefUpdate remoteUpdate : result.getRemoteUpdates()) {
+			assertEquals(
+					"update should succeed on " + remoteUpdate.getRemoteName(),
+					RemoteRefUpdate.Status.OK, remoteUpdate.getStatus());
+		}
+
+		List<String> expected = remoteRefNames(updates);
+		assertEquals(
+				"ref names processed by ReceivePack should match input ref names in order",
+				expected, processedRefs);
+		assertEquals(
+				"remote ref names should match input ref names in order",
+				expected, remoteRefNames(result.getRemoteUpdates()));
+	}
+
+	private static List<String> remoteRefNames(Collection<RemoteRefUpdate> updates) {
+		List<String> result = new ArrayList<>();
+		for (RemoteRefUpdate u : updates) {
+			result.add(u.getRemoteName());
+		}
+		return result;
+	}
 }

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

@@ -49,6 +49,7 @@ import java.text.MessageFormat;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -124,7 +125,7 @@ class PushProcess {
 			throws TransportException {
 		this.walker = new RevWalk(transport.local);
 		this.transport = transport;
-		this.toPush = new HashMap<>();
+		this.toPush = new LinkedHashMap<>();
 		this.out = out;
 		this.pushOptions = transport.getPushOptions();
 		for (final RemoteRefUpdate rru : toPush) {
@@ -190,7 +191,7 @@ class PushProcess {
 	private Map<String, RemoteRefUpdate> prepareRemoteUpdates()
 			throws TransportException {
 		boolean atomic = transport.isPushAtomic();
-		final Map<String, RemoteRefUpdate> result = new HashMap<>();
+		final Map<String, RemoteRefUpdate> result = new LinkedHashMap<>();
 		for (final RemoteRefUpdate rru : toPush.values()) {
 			final Ref advertisedRef = connection.getRef(rru.getRemoteName());
 			ObjectId advertisedOld = null;

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

@@ -64,7 +64,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -690,7 +690,7 @@ public abstract class Transport implements AutoCloseable {
 			final Repository db, final Collection<RefSpec> specs)
 			throws IOException {
 		final Map<String, Ref> localRefs = db.getRefDatabase().getRefs(ALL);
-		final Collection<RefSpec> procRefs = new HashSet<>();
+		final Collection<RefSpec> procRefs = new LinkedHashSet<>();
 
 		for (final RefSpec spec : specs) {
 			if (spec.isWildcard()) {