Merge "Enhance review ssh command to abandon/restore patchsets."

2011-03-28 10:04:49 -07:00 · 2011-03-28 10:04:49 -07:00 · d3bb8dc1cc
commit d3bb8dc1cc
parent f324eb6c9f 30f823c1f8
5 changed files with 188 additions and 108 deletions
--- a/Documentation/cmd-review.txt
+++ b/Documentation/cmd-review.txt
@ -8,8 +8,8 @@ gerrit review - Verify, approve and/or submit one or more patch sets
 SYNOPSIS
 --------
 [verse]
-'ssh' -p <port> <host> 'gerrit approve' [\--project <PROJECT>] [\--message <MESSAGE>] [\--verified <N>] [\--code-review <N>] [\--submit] {COMMIT | CHANGEID,PATCHSET}...
-'ssh' -p <port> <host> 'gerrit review' [\--project <PROJECT>] [\--message <MESSAGE>] [\--verified <N>] [\--code-review <N>] [\--submit] {COMMIT | CHANGEID,PATCHSET}...
+'ssh' -p <port> <host> 'gerrit approve' [\--project <PROJECT>] [\--message <MESSAGE>] [\--verified <N>] [\--code-review <N>] [\--abandon] [\--restore] [\--submit]  {COMMIT | CHANGEID,PATCHSET}...
+'ssh' -p <port> <host> 'gerrit review' [\--project <PROJECT>] [\--message <MESSAGE>] [\--verified <N>] [\--code-review <N>] [\--abandon] [\--restore] [\--submit]  {COMMIT | CHANGEID,PATCHSET}...

 DESCRIPTION
 -----------
@ -54,9 +54,18 @@ OPTIONS
 	differs per site, check the output of \--help, or contact
 	your site administrator for further details.

+\--abandon::
+	Abandon the specified patch set(s).
+	(option is mutually exclusive with --submit and --restore)
+
+\--restore::
+	Restore the specified abandonned patch set(s).
+	(option is mutually exclusive with --abandon)
+
 \--submit::
 -s::
 	Submit the specified patch set(s) for merging.
+	(option is mutually exclusive with --abandon)

 ACCESS
 ------
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChange.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AbandonChange.java
@ -19,9 +19,7 @@ import com.google.gerrit.common.data.ChangeDetail;
 import com.google.gerrit.common.errors.NoSuchEntityException;
 import com.google.gerrit.httpd.rpc.Handler;
 import com.google.gerrit.reviewdb.Change;
-import com.google.gerrit.reviewdb.ChangeMessage;
 import com.google.gerrit.reviewdb.PatchSet;
-import com.google.gerrit.reviewdb.PatchSetApproval;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
@ -30,14 +28,10 @@ import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gwtorm.client.AtomicUpdate;
 import com.google.gwtorm.client.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;

-import java.util.Collections;
-import java.util.List;
-
 import javax.annotation.Nullable;

 class AbandonChange extends Handler<ChangeDetail> {
@ -78,60 +72,15 @@ class AbandonChange extends Handler<ChangeDetail> {
  @Override
  public ChangeDetail call() throws NoSuchChangeException, OrmException,
      EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException {
+
    final Change.Id changeId = patchSetId.getParentKey();
    final ChangeControl control = changeControlFactory.validateFor(changeId);
    if (!control.canAbandon()) {
      throw new NoSuchChangeException(changeId);
    }
-    Change change = control.getChange();
-    final PatchSet patch = db.patchSets().get(patchSetId);
-    if (patch == null) {
-      throw new NoSuchChangeException(changeId);
-    }

-    final ChangeMessage cmsg =
-        new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil
-            .messageUUID(db)), currentUser.getAccountId());
-    final StringBuilder msgBuf =
-        new StringBuilder("Patch Set " + patchSetId.get() + ": Abandoned");
-    if (message != null && message.length() > 0) {
-      msgBuf.append("\n\n");
-      msgBuf.append(message);
-    }
-    cmsg.setMessage(msgBuf.toString());
-
-    change = db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() {
-      @Override
-      public Change update(Change change) {
-        if (change.getStatus().isOpen()
-            && change.currentPatchSetId().equals(patchSetId)) {
-          change.setStatus(Change.Status.ABANDONED);
-          ChangeUtil.updated(change);
-          return change;
-        } else {
-          return null;
-        }
-      }
-    });
-
-    if (change != null) {
-      db.changeMessages().insert(Collections.singleton(cmsg));
-
-      final List<PatchSetApproval> approvals =
-          db.patchSetApprovals().byChange(changeId).toList();
-      for (PatchSetApproval a : approvals) {
-        a.cache(change);
-      }
-      db.patchSetApprovals().update(approvals);
-
-      // Email the reviewers
-      final AbandonedSender cm = abandonedSenderFactory.create(change);
-      cm.setFrom(currentUser.getAccountId());
-      cm.setChangeMessage(cmsg);
-      cm.send();
-    }
-
-    hooks.doChangeAbandonedHook(change, currentUser.getAccount(), message);
+    ChangeUtil.abandon(patchSetId, currentUser, message, db,
+       abandonedSenderFactory, hooks);

    return changeDetailFactory.create(changeId).call();
  }
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChange.java
@ -73,59 +73,15 @@ class RestoreChange extends Handler<ChangeDetail> {
  @Override
  public ChangeDetail call() throws NoSuchChangeException, OrmException,
      EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException {
+
    final Change.Id changeId = patchSetId.getParentKey();
    final ChangeControl control = changeControlFactory.validateFor(changeId);
    if (!control.canRestore()) {
      throw new NoSuchChangeException(changeId);
    }
-    final PatchSet patch = db.patchSets().get(patchSetId);
-    if (patch == null) {
-      throw new NoSuchChangeException(changeId);
-    }

-    final ChangeMessage cmsg =
-        new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil
-            .messageUUID(db)), currentUser.getAccountId());
-    final StringBuilder msgBuf =
-        new StringBuilder("Patch Set " + patchSetId.get() + ": Restored");
-    if (message != null && message.length() > 0) {
-      msgBuf.append("\n\n");
-      msgBuf.append(message);
-    }
-    cmsg.setMessage(msgBuf.toString());
-
-    Change change = db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() {
-      @Override
-      public Change update(Change change) {
-        if (change.getStatus() == Change.Status.ABANDONED
-            && change.currentPatchSetId().equals(patchSetId)) {
-          change.setStatus(Change.Status.NEW);
-          ChangeUtil.updated(change);
-          return change;
-        } else {
-          return null;
-        }
-      }
-    });
-
-    if (change != null) {
-      db.changeMessages().insert(Collections.singleton(cmsg));
-
-      final List<PatchSetApproval> approvals =
-          db.patchSetApprovals().byChange(changeId).toList();
-      for (PatchSetApproval a : approvals) {
-        a.cache(change);
-      }
-      db.patchSetApprovals().update(approvals);
-
-      // Email the reviewers
-      final AbandonedSender cm = abandonedSenderFactory.create(change);
-      cm.setFrom(currentUser.getAccountId());
-      cm.setChangeMessage(cmsg);
-      cm.send();
-    }
-
-    hooks.doChangeRestoreHook(change, currentUser.getAccount(), message);
+    ChangeUtil.restore(patchSetId, currentUser, message, db,
+       abandonedSenderFactory, hooks);

    return changeDetailFactory.create(changeId).call();
  }
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
@ -16,19 +16,28 @@ package com.google.gerrit.server;

 import static com.google.gerrit.reviewdb.ApprovalCategory.SUBMIT;

+import com.google.gerrit.common.ChangeHookRunner;
+import com.google.gerrit.common.data.ChangeDetail;
+import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.ChangeMessage;
 import com.google.gerrit.reviewdb.Change;
 import com.google.gerrit.reviewdb.PatchSet;
 import com.google.gerrit.reviewdb.PatchSetApproval;
 import com.google.gerrit.reviewdb.ReviewDb;
 import com.google.gerrit.reviewdb.TrackingId;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.TrackingFooter;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.MergeQueue;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.mail.AbandonedSender;
+import com.google.gerrit.server.mail.EmailException;
 import com.google.gwtorm.client.AtomicUpdate;
 import com.google.gwtorm.client.OrmConcurrencyException;
 import com.google.gwtorm.client.OrmException;

+
 import org.eclipse.jgit.revwalk.FooterLine;
 import org.eclipse.jgit.util.Base64;
 import org.eclipse.jgit.util.NB;
@ -178,6 +187,119 @@ public class ChangeUtil {
    return new PatchSetApproval(akey, (short) 1);
  }

+  public static void abandon(final PatchSet.Id patchSetId,
+      final IdentifiedUser user, final String message, final ReviewDb db,
+      final AbandonedSender.Factory abandonedSenderFactory,
+      final ChangeHookRunner hooks) throws NoSuchChangeException,
+      EmailException, OrmException {
+    final Change.Id changeId = patchSetId.getParentKey();
+    final PatchSet patch = db.patchSets().get(patchSetId);
+    if (patch == null) {
+      throw new NoSuchChangeException(changeId);
+    }
+
+    final ChangeMessage cmsg =
+        new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil
+            .messageUUID(db)), user.getAccountId());
+    final StringBuilder msgBuf =
+        new StringBuilder("Patch Set " + patchSetId.get() + ": Abandoned");
+    if (message != null && message.length() > 0) {
+      msgBuf.append("\n\n");
+      msgBuf.append(message);
+    }
+    cmsg.setMessage(msgBuf.toString());
+
+    final Change change = db.changes().atomicUpdate(changeId,
+        new AtomicUpdate<Change>() {
+      @Override
+      public Change update(Change change) {
+        if (change.getStatus().isOpen()
+            && change.currentPatchSetId().equals(patchSetId)) {
+          change.setStatus(Change.Status.ABANDONED);
+          ChangeUtil.updated(change);
+          return change;
+        } else {
+          return null;
+        }
+      }
+    });
+
+    if (change != null) {
+      db.changeMessages().insert(Collections.singleton(cmsg));
+
+      final List<PatchSetApproval> approvals =
+          db.patchSetApprovals().byChange(changeId).toList();
+      for (PatchSetApproval a : approvals) {
+        a.cache(change);
+      }
+      db.patchSetApprovals().update(approvals);
+
+      // Email the reviewers
+      final AbandonedSender cm = abandonedSenderFactory.create(change);
+      cm.setFrom(user.getAccountId());
+      cm.setChangeMessage(cmsg);
+      cm.send();
+    }
+
+    hooks.doChangeAbandonedHook(change, user.getAccount(), message);
+  }
+
+  public static void restore(final PatchSet.Id patchSetId,
+      final IdentifiedUser user, final String message, final ReviewDb db,
+      final AbandonedSender.Factory abandonedSenderFactory,
+      final ChangeHookRunner hooks) throws NoSuchChangeException,
+      EmailException, OrmException {
+    final Change.Id changeId = patchSetId.getParentKey();
+    final PatchSet patch = db.patchSets().get(patchSetId);
+    if (patch == null) {
+      throw new NoSuchChangeException(changeId);
+    }
+
+    final ChangeMessage cmsg =
+        new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil
+            .messageUUID(db)), user.getAccountId());
+    final StringBuilder msgBuf =
+        new StringBuilder("Patch Set " + patchSetId.get() + ": Restored");
+    if (message != null && message.length() > 0) {
+      msgBuf.append("\n\n");
+      msgBuf.append(message);
+    }
+    cmsg.setMessage(msgBuf.toString());
+
+    Change change = db.changes().atomicUpdate(changeId, new AtomicUpdate<Change>() {
+      @Override
+      public Change update(Change change) {
+        if (change.getStatus() == Change.Status.ABANDONED
+            && change.currentPatchSetId().equals(patchSetId)) {
+          change.setStatus(Change.Status.NEW);
+          ChangeUtil.updated(change);
+          return change;
+        } else {
+          return null;
+        }
+      }
+    });
+
+    if (change != null) {
+      db.changeMessages().insert(Collections.singleton(cmsg));
+
+      final List<PatchSetApproval> approvals =
+          db.patchSetApprovals().byChange(changeId).toList();
+      for (PatchSetApproval a : approvals) {
+        a.cache(change);
+      }
+      db.patchSetApprovals().update(approvals);
+
+      // Email the reviewers
+      final AbandonedSender cm = abandonedSenderFactory.create(change);
+      cm.setFrom(user.getAccountId());
+      cm.setChangeMessage(cmsg);
+      cm.send();
+    }
+
+    hooks.doChangeRestoreHook(change, user.getAccount(), message);
+  }
+
  public static String sortKey(long lastUpdated, int id){
    // The encoding uses minutes since Wed Oct 1 00:00:00 2008 UTC.
    // We overrun approximately 4,085 years later, so ~6093.
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
@ -14,6 +14,7 @@

 package com.google.gerrit.sshd.commands;

+import com.google.gerrit.common.ChangeHookRunner;
 import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.reviewdb.ApprovalCategory;
@ -30,6 +31,8 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.MergeOp.Factory;
 import com.google.gerrit.server.git.MergeQueue;
+import com.google.gerrit.server.mail.AbandonedSender;
+import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.patch.PublishComments;
 import com.google.gerrit.server.project.CanSubmitResult;
 import com.google.gerrit.server.project.ChangeControl;
@ -88,6 +91,12 @@ public class ReviewCommand extends BaseCommand {
  @Option(name = "--message", aliases = "-m", usage = "cover message to publish on change", metaVar = "MESSAGE")
  private String changeComment;

+  @Option(name = "--abandon", usage = "abandon the patch set")
+  private boolean abandonChange;
+
+  @Option(name = "--restore", usage = "restore an abandoned the patch set")
+  private boolean restoreChange;
+
  @Option(name = "--submit", aliases = "-s", usage = "submit the patch set")
  private boolean submitChange;

@ -110,13 +119,19 @@ public class ReviewCommand extends BaseCommand {
  private ChangeControl.Factory changeControlFactory;

  @Inject
-  private FunctionState.Factory functionStateFactory;
+  private AbandonedSender.Factory abandonedSenderFactory;

-  private List<ApproveOption> optionList;
+  @Inject
+  private FunctionState.Factory functionStateFactory;

  @Inject
  private PublishComments.Factory publishCommentsFactory;

+  @Inject
+  private ChangeHookRunner hooks;
+
+  private List<ApproveOption> optionList;
+
  private Set<PatchSet.Id> toSubmit = new HashSet<PatchSet.Id>();

  @Override
@ -126,6 +141,14 @@ public class ReviewCommand extends BaseCommand {
      public void run() throws Failure {
        initOptionList();
        parseCommandLine();
+        if (abandonChange) {
+          if (restoreChange) {
+            throw error("abandon and restore actions are mutually exclusive");
+          }
+          if (submitChange) {
+            throw error("abandon and submit actions are mutually exclusive");
+          }
+        }

        boolean ok = true;
        for (final PatchSet.Id patchSetId : patchSetIds) {
@ -182,11 +205,11 @@ public class ReviewCommand extends BaseCommand {
  }

  private void approveOne(final PatchSet.Id patchSetId)
-      throws NoSuchChangeException, UnloggedFailure, OrmException {
+      throws NoSuchChangeException, UnloggedFailure, OrmException,
+             EmailException {

    final Change.Id changeId = patchSetId.getParentKey();
-    final ChangeControl changeControl =
-        changeControlFactory.validateFor(changeId);
+    ChangeControl changeControl = changeControlFactory.validateFor(changeId);

    if (changeComment == null) {
      changeComment = "";
@ -203,6 +226,27 @@ public class ReviewCommand extends BaseCommand {

    publishCommentsFactory.create(patchSetId, changeComment, aps).call();

+    if (abandonChange) {
+      if (changeControl.canAbandon()) {
+        ChangeUtil.abandon(patchSetId, currentUser, changeComment, db,
+          abandonedSenderFactory, hooks);
+      } else {
+        throw error("Not permitted to abandon change");
+      }
+    }
+
+    if (restoreChange) {
+      if (changeControl.canRestore()) {
+        ChangeUtil.restore(patchSetId, currentUser, changeComment, db,
+          abandonedSenderFactory, hooks);
+      } else {
+        throw error("Not permitted to restore change");
+      }
+      if (submitChange) {
+        changeControl = changeControlFactory.validateFor(changeId);
+      }
+    }
+
    if (submitChange) {
      CanSubmitResult result =
          changeControl.canSubmit(patchSetId, db, approvalTypes,