diff --git a/Documentation/cmd-review.txt b/Documentation/cmd-review.txt index d84a2ebc51..ec16bcce1a 100644 --- 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 'gerrit approve' [\--project ] [\--message ] [\--verified ] [\--code-review ] [\--submit] {COMMIT | CHANGEID,PATCHSET}... -'ssh' -p 'gerrit review' [\--project ] [\--message ] [\--verified ] [\--code-review ] [\--submit] {COMMIT | CHANGEID,PATCHSET}... +'ssh' -p 'gerrit approve' [\--project ] [\--message ] [\--verified ] [\--code-review ] [\--abandon] [\--restore] [\--submit] {COMMIT | CHANGEID,PATCHSET}... +'ssh' -p 'gerrit review' [\--project ] [\--message ] [\--verified ] [\--code-review ] [\--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 ------ diff --git 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 index 4a4d9d13f4..d2f59f5d79 100644 --- 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 { @@ -78,60 +72,15 @@ class AbandonChange extends Handler { @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() { - @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 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(); } diff --git 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 index 9639f1a2bb..4a67260e34 100644 --- 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 { @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() { - @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 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(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 36fc2ff80a..591dee83b2 100644 --- 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() { + @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 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() { + @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 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. diff --git 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 index cd87498fad..4e6607e178 100644 --- 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 optionList; + @Inject + private FunctionState.Factory functionStateFactory; @Inject private PublishComments.Factory publishCommentsFactory; + @Inject + private ChangeHookRunner hooks; + + private List optionList; + private Set toSubmit = new HashSet(); @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,