Remove nbrPatchSets from Change

The nbrPatchSets counter was used to generate the next PatchSetId.
Instead, find the first reference after the currentPatchSetId() that
does not exist by examining the references themselves.

Also in a few locations, the use of currPatchSetId() was corrected
to be currentPatchSetId().

Change-Id: I06bcf52798f1dad1ea602f914784dfcbc7d97086

gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeInfo.java

@@ -50,7 +50,7 @@ public class ChangeInfo {
     lastUpdatedOn = c.getLastUpdatedOn();
     sortKey = c.getSortKey();
     patchSetId = patchId;
-    latest = patchSetId == null || c.currPatchSetId().equals(patchSetId);
+    latest = patchSetId == null || patchSetId.equals(c.currentPatchSetId());
   }
 
   public ChangeInfo(final Change c) {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java

@@ -83,7 +83,8 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
   protected Change.Id updateProjectConfig(ProjectConfig config, MetaDataUpdate md)
       throws IOException, OrmException {
     Change.Id changeId = new Change.Id(db.nextChangeId());
-    PatchSet ps = new PatchSet(new PatchSet.Id(changeId, 1));
+    PatchSet ps =
+        new PatchSet(new PatchSet.Id(changeId, Change.INITIAL_PATCH_SET_ID));
     RevCommit commit = config.commitToNewRef(md, ps.getRefName());
     if (commit.getId().equals(base)) {
       return null;
@@ -96,7 +97,6 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
         new Branch.NameKey(
             config.getProject().getNameKey(),
             GitRepositoryManager.REF_CONFIG));
-    change.nextPatchSetId();
 
     ps.setCreatedOn(change.getCreatedOn());
     ps.setUploader(change.getOwner());

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

@@ -193,6 +193,9 @@ public final class Change {
   /** Database constant for {@link Status#MERGED}. */
   public static final char STATUS_MERGED = 'M';
 
+  /** ID number of the first patch set in a change. */
+  public static final int INITIAL_PATCH_SET_ID = 1;
+
   /**
    * Current state within the basic workflow of the change.
    *
@@ -359,10 +362,6 @@ public final class Change {
   @Column(id = 10)
   protected char status;
 
-  /** The total number of {@link PatchSet} children in this Change. */
-  @Column(id = 11)
-  protected int nbrPatchSets;
-
   /** The current patch set. */
   @Column(id = 12)
   protected int currentPatchSetId;
@@ -473,23 +472,6 @@ public final class Change {
     subject = ps.getSubject();
   }
 
-  /**
-   * Allocate a new PatchSet id within this change.
-   * <p>
-   * <b>Note: This makes the change dirty. Call update() after.</b>
-   */
-  public void nextPatchSetId() {
-    ++nbrPatchSets;
-  }
-
-  public void updateNumberOfPatchSets(int max) {
-    nbrPatchSets = Math.max(nbrPatchSets, max);
-  }
-
-  public PatchSet.Id currPatchSetId() {
-    return new PatchSet.Id(changeId, nbrPatchSets);
-  }
-
   public Status getStatus() {
     return Status.forCode(status);
   }

gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java

@@ -47,6 +47,7 @@ import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.FooterLine;
@@ -63,6 +64,7 @@ import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.regex.Matcher;
 
@@ -243,10 +245,10 @@ public class ChangeUtil {
           new Change.Id(db.nextChangeId()),
           user.getAccountId(),
           changeToRevert.getDest());
-      change.nextPatchSetId();
       change.setTopic(changeToRevert.getTopic());
 
-      final PatchSet ps = new PatchSet(change.currPatchSetId());
+      PatchSet.Id id = nextPatchSetId(git, change.currentPatchSetId());
+      final PatchSet ps = new PatchSet(id);
       ps.setCreatedOn(change.getCreatedOn());
       ps.setUploader(change.getOwner());
       ps.setRevision(new RevId(revertCommit.name()));
@@ -356,10 +358,9 @@ public class ChangeUtil {
           oi.release();
         }
 
-        change.nextPatchSetId();
-
         final PatchSet originalPS = db.patchSets().get(patchSetId);
-        final PatchSet newPatchSet = new PatchSet(change.currPatchSetId());
+        PatchSet.Id id = nextPatchSetId(git, change.currentPatchSetId());
+        final PatchSet newPatchSet = new PatchSet(id);
         newPatchSet.setCreatedOn(new Timestamp(now.getTime()));
         newPatchSet.setUploader(user.getAccountId());
         newPatchSet.setRevision(new RevId(newCommit.name()));
@@ -381,19 +382,8 @@ public class ChangeUtil {
 
         db.changes().beginTransaction(change.getId());
         try {
-          Change updatedChange =
+          Change updatedChange = db.changes().get(change.getId());
-              db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
+          if (updatedChange != null && updatedChange.getStatus().isOpen()) {
-                @Override
-                public Change update(Change change) {
-                  if (change.getStatus().isOpen()) {
-                    change.updateNumberOfPatchSets(newPatchSet.getPatchSetId());
-                    return change;
-                  } else {
-                    return null;
-                  }
-                }
-              });
-          if (updatedChange != null) {
             change = updatedChange;
           } else {
             throw new InvalidChangeOperationException(String.format(
@@ -528,6 +518,23 @@ public class ChangeUtil {
     c.setSortKey(sortKey(lastUpdated, id));
   }
 
+  public static PatchSet.Id nextPatchSetId(Map<String, Ref> allRefs,
+      PatchSet.Id id) {
+    PatchSet.Id next = nextPatchSetId(id);
+    while (allRefs.containsKey(next.toRefName())) {
+      next = nextPatchSetId(next);
+    }
+    return next;
+  }
+
+  public static PatchSet.Id nextPatchSetId(Repository git, PatchSet.Id id) {
+    return nextPatchSetId(git.getAllRefs(), id);
+  }
+
+  private static PatchSet.Id nextPatchSetId(PatchSet.Id id) {
+    return new PatchSet.Id(id.getParentKey(), id.get() + 1);
+  }
+
   private static final char[] hexchar =
       {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', //
           'a', 'b', 'c', 'd', 'e', 'f'};

gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java

@@ -320,8 +320,8 @@ public class RebaseChange {
 
     rebasedCommit = revWalk.parseCommit(newId);
 
-    change.nextPatchSetId();
+    PatchSet.Id id = ChangeUtil.nextPatchSetId(git, change.currentPatchSetId());
-    final PatchSet newPatchSet = new PatchSet(change.currPatchSetId());
+    final PatchSet newPatchSet = new PatchSet(id);
     newPatchSet.setCreatedOn(new Timestamp(System.currentTimeMillis()));
     newPatchSet.setUploader(uploader);
     newPatchSet.setRevision(new RevId(rebasedCommit.name()));
@@ -343,21 +343,8 @@ public class RebaseChange {
 
     db.changes().beginTransaction(change.getId());
     try {
-      Change updatedChange;
+      Change updatedChange = db.changes().get(change.getId());
-
+      if (updatedChange != null && change.getStatus().isOpen()) {
-      updatedChange =
-          db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
-            @Override
-            public Change update(Change change) {
-              if (change.getStatus().isOpen()) {
-                change.updateNumberOfPatchSets(newPatchSet.getPatchSetId());
-                return change;
-              } else {
-                return null;
-              }
-            }
-          });
-      if (updatedChange != null) {
         change = updatedChange;
       } else {
         throw new InvalidChangeOperationException(String.format(

gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java

@@ -214,7 +214,7 @@ public class EventFactory {
 
   private DependencyAttribute newDependsOn(Change c, PatchSet ps) {
     DependencyAttribute d = newDependencyAttribute(c, ps);
-    d.isCurrentPatchSet = c.currPatchSetId().equals(ps.getId());
+    d.isCurrentPatchSet = ps.getId().equals(c.currentPatchSetId());
     return d;
   }
 

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

@@ -30,6 +30,7 @@ import com.google.gerrit.reviewdb.client.PatchSetAncestor;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gwtorm.server.OrmException;
@@ -37,6 +38,7 @@ import com.google.inject.Provider;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.revwalk.RevCommit;
 
@@ -148,7 +150,7 @@ public class CherryPick extends SubmitStrategy {
     args.rw.parseBody(n);
 
     final PatchSetApproval submitAudit =
-        getSubmitter(args.db, n.change.currPatchSetId());
+        getSubmitter(args.db, n.change.currentPatchSetId());
 
     PersonIdent cherryPickCommitterIdent = null;
     if (submitAudit != null) {
@@ -172,9 +174,9 @@ public class CherryPick extends SubmitStrategy {
         return null;
     }
 
-    n.change.nextPatchSetId();
+    PatchSet.Id id =
-
+        ChangeUtil.nextPatchSetId(args.repo, n.change.currentPatchSetId());
-    final PatchSet ps = new PatchSet(n.change.currPatchSetId());
+    final PatchSet ps = new PatchSet(id);
     ps.setCreatedOn(new Timestamp(System.currentTimeMillis()));
     ps.setUploader(submitAudit.getAccountId());
     ps.setRevision(new RevId(newCommit.getId().getName()));

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.git;
 
+import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
 import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
 import static org.eclipse.jgit.lib.Constants.R_HEADS;
 import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
@@ -1304,9 +1305,8 @@ public class ReceiveCommits {
           currentUser.getAccountId(),
           destBranch);
       change.setTopic(destTopicName);
-      change.nextPatchSetId();
 
-      ps = new PatchSet(change.currPatchSetId());
+      ps = new PatchSet(new PatchSet.Id(change.getId(), INITIAL_PATCH_SET_ID));
       ps.setCreatedOn(change.getCreatedOn());
       ps.setUploader(change.getOwner());
       ps.setRevision(toRevId(c));
@@ -1616,13 +1616,8 @@ public class ReceiveCommits {
         }
       }
 
-      change.nextPatchSetId();
+      PatchSet.Id id =
-      PatchSet.Id id = change.currPatchSetId();
+          ChangeUtil.nextPatchSetId(allRefs, change.currentPatchSetId());
-      while (allRefs.containsKey(id.toRefName())) {
-        change.nextPatchSetId();
-        id = change.currPatchSetId();
-      }
-
       newPatchSet = new PatchSet(id);
       newPatchSet.setCreatedOn(new Timestamp(System.currentTimeMillis()));
       newPatchSet.setUploader(currentUser.getAccountId());
@@ -1694,19 +1689,8 @@ public class ReceiveCommits {
 
       db.changes().beginTransaction(change.getId());
       try {
-        change =
+        change = db.changes().get(change.getId());
-          db.changes().atomicUpdate(change.getId(), new AtomicUpdate<Change>() {
+        if (change == null || change.getStatus().isClosed()) {
-            @Override
-            public Change update(Change change) {
-              if (change.getStatus().isClosed()) {
-                return null;
-              }
-
-              change.updateNumberOfPatchSets(newPatchSet.getPatchSetId());
-              return change;
-            }
-          });
-        if (change == null) {
           reject(inputCommand, "change is closed");
           return null;
         }