Work around failed database updates during patch set replace

If the database doesn't update when adding a new patch set to a change
the next attempt will fail because the refs/changes/ reference already
exists. Increment to the next patch set id.

This works around the error seen by Bruce at the hackathon, where he
couldn't upload a new patch set to a change on gerrit-review without
intervention from a Googler mucking with the backend database.  The
bug is also possible in other Gerrit installations, but less likely as
the database is usually more reliable.

Change-Id: I8546524d3fdcac827b3dcab0338077f4f4a30205

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchSet.java

@@ -61,6 +61,22 @@ public final class PatchSet {
       patchSetId = newValue;
     }
 
+    public String toRefName() {
+      StringBuilder r = new StringBuilder();
+      r.append(REFS_CHANGES);
+      int change = changeId.get();
+      int m = change % 100;
+      if (m < 10) {
+        r.append('0');
+      }
+      r.append(m);
+      r.append('/');
+      r.append(change);
+      r.append('/');
+      r.append(patchSetId);
+      return r.toString();
+    }
+
     /** Parse a PatchSet.Id out of a string representation. */
     public static Id parse(final String str) {
       final Id r = new Id();
@@ -148,19 +164,7 @@ public final class PatchSet {
   }
 
   public String getRefName() {
-    final StringBuilder r = new StringBuilder();
-    r.append(REFS_CHANGES);
-    final int changeId = id.getParentKey().get();
-    final int m = changeId % 100;
-    if (m < 10) {
-      r.append('0');
-    }
-    r.append(m);
-    r.append('/');
-    r.append(changeId);
-    r.append('/');
-    r.append(id.get());
-    return r.toString();
+    return id.toRefName();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -107,9 +107,13 @@ import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.revwalk.filter.RevFilter;
 import org.eclipse.jgit.transport.AdvertiseRefsHook;
 import org.eclipse.jgit.transport.AdvertiseRefsHookChain;
+import org.eclipse.jgit.transport.BaseReceivePack;
 import org.eclipse.jgit.transport.ReceiveCommand;
 import org.eclipse.jgit.transport.ReceiveCommand.Result;
 import org.eclipse.jgit.transport.ReceivePack;
+import org.eclipse.jgit.transport.ServiceMayNotContinueException;
+import org.eclipse.jgit.transport.UploadPack;
+import org.eclipse.jgit.util.RefList;
 import org.eclipse.jgit.util.SystemReader;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -272,6 +276,7 @@ public class ReceiveCommits {
 
   private Collection<ObjectId> existingObjects;
   private Map<ObjectId, Ref> refsById;
+  private Map<String, Ref> allRefs;
 
   private String destTopicName;
 
@@ -358,7 +363,23 @@ public class ReceiveCommits {
       rp.setCheckReferencedObjectsAreReachable(true);
       rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo, projectControl, db, false));
     }
-    List<AdvertiseRefsHook> advHooks = new ArrayList<AdvertiseRefsHook>(2);
+    List<AdvertiseRefsHook> advHooks = new ArrayList<AdvertiseRefsHook>(3);
+    advHooks.add(new AdvertiseRefsHook() {
+      @Override
+      public void advertiseRefs(BaseReceivePack rp)
+          throws ServiceMayNotContinueException {
+        allRefs = rp.getAdvertisedRefs();
+        if (allRefs == null) {
+          allRefs = rp.getRepository().getAllRefs();
+        }
+        rp.setAdvertisedRefs(allRefs, rp.getAdvertisedObjects());
+      }
+
+      @Override
+      public void advertiseRefs(UploadPack uploadPack)
+          throws ServiceMayNotContinueException {
+      }
+    });
     advHooks.add(rp.getAdvertiseRefsHook());
     advHooks.add(new ReceiveCommitsAdvertiseRefsHook());
     rp.setAdvertiseRefsHook(AdvertiseRefsHookChain.newChain(advHooks));
@@ -1132,7 +1153,7 @@ public class ReceiveCommits {
     try {
       Set<ObjectId> existing = Sets.newHashSet();
       walk.markStart(walk.parseCommit(newChange.getNewId()));
-      for (Ref ref : repo.getAllRefs().values()) {
+      for (Ref ref : allRefs.values()) {
         if (ref.getObjectId() == null) {
           continue;
         } else if (ref.getName().startsWith("refs/changes/")) {
@@ -1398,7 +1419,6 @@ public class ReceiveCommits {
     try {
       readChangesForReplace();
       readPatchSetsForReplace();
-
       for (ReplaceRequest req : replaceByChange.values()) {
         if (req.inputCommand.getResult() == NOT_ATTEMPTED) {
           req.validate(false);
@@ -1591,7 +1611,13 @@ public class ReceiveCommits {
       }
 
       change.nextPatchSetId();
-      newPatchSet = new PatchSet(change.currPatchSetId());
+      PatchSet.Id id = change.currPatchSetId();
+      while (allRefs.containsKey(id.toRefName())) {
+        change.nextPatchSetId();
+        id = change.currPatchSetId();
+      }
+
+      newPatchSet = new PatchSet(id);
       newPatchSet.setCreatedOn(new Timestamp(System.currentTimeMillis()));
       newPatchSet.setUploader(currentUser.getAccountId());
       newPatchSet.setRevision(toRevId(newCommit));
@@ -1904,9 +1930,8 @@ public class ReceiveCommits {
 
   private Collection<ObjectId> existingObjects() {
     if (existingObjects == null) {
-      Map<String, Ref> refs = repo.getAllRefs();
-      existingObjects = new ArrayList<ObjectId>(refs.size());
-      for (Ref r : refs.values()) {
+      existingObjects = new ArrayList<ObjectId>(allRefs.size());
+      for (Ref r : allRefs.values()) {
         existingObjects.add(r.getObjectId());
       }
     }