From 8c84ba35e5559b4b8e0df2923b3a2889b8a82bb1 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Thu, 10 Jun 2010 10:32:00 -0600 Subject: [PATCH] Add ssh command line feature to submit change set Rename the ssh gerrit approve command to review and enhance it with the ability to perform submits via --submit. Create an alias for 'review' as 'approve' to support legacy approve. Update docs accordingly. Bug: issue 310 Change-Id: I5b88c8818b227de7dfc9c8a879eb5e7c7821e3b5 --- Documentation/cmd-index.txt | 7 ++- .../{cmd-approve.txt => cmd-review.txt} | 22 ++++--- .../com/google/gerrit/server/ChangeUtil.java | 51 +++++++++++++++ .../server/project/CanSubmitResult.java | 43 +++++++++++++ .../gerrit/server/project/ChangeControl.java | 62 ++++++++++++++++++- .../gerrit/server/project/RefControl.java | 5 ++ .../sshd/commands/MasterCommandModule.java | 3 +- ...ApproveCommand.java => ReviewCommand.java} | 25 +++++++- .../sshd/commands/SlaveCommandModule.java | 1 + 9 files changed, 205 insertions(+), 14 deletions(-) rename Documentation/{cmd-approve.txt => cmd-review.txt} (74%) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/project/CanSubmitResult.java rename gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/{ApproveCommand.java => ReviewCommand.java} (93%) diff --git a/Documentation/cmd-index.txt b/Documentation/cmd-index.txt index a9107eb49f..7fbf31b402 100644 --- a/Documentation/cmd-index.txt +++ b/Documentation/cmd-index.txt @@ -60,12 +60,15 @@ link:cmd-receive-pack.html[git receive-pack]:: Also implements the magic associated with uploading commits for review. See link:user-upload.html#push_create[Creating Changes]. -link:cmd-approve.html[gerrit approve]:: - Approve a patch set from the command line. +link:cmd-review.html[gerrit approve]:: + Alias for 'gerrit review'. link:cmd-ls-projects.html[gerrit ls-projects]:: List projects visible to the caller. +link:cmd-review.html[gerrit review]:: + Verify, approve and/or submit a patch set from the command line. + link:cmd-stream-events.html[gerrit stream-events]:: Monitor events occuring in real time. diff --git a/Documentation/cmd-approve.txt b/Documentation/cmd-review.txt similarity index 74% rename from Documentation/cmd-approve.txt rename to Documentation/cmd-review.txt index a81ce827ff..d84a2ebc51 100644 --- a/Documentation/cmd-approve.txt +++ b/Documentation/cmd-review.txt @@ -1,19 +1,21 @@ -gerrit approve +gerrit review ============== NAME ---- -gerrit approve - Approve one or more patch sets +gerrit review - Verify, approve and/or submit one or more patch sets SYNOPSIS -------- [verse] -'ssh' -p 'gerrit approve' [\--project ] [\--message ] {COMMIT | CHANGEID,PATCHSET}... +'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}... DESCRIPTION ----------- Updates the current user's approval status of the specified patch -sets, sending out email notifications and updating the database. +sets and/or submits them for merging, sending out email +notifications and updating the database. Patch sets should be specified as complete or abbreviated commit SHA-1s. If the same commit is available in multiple projects the @@ -52,6 +54,10 @@ OPTIONS differs per site, check the output of \--help, or contact your site administrator for further details. +\--submit:: +-s:: + Submit the specified patch set(s) for merging. + ACCESS ------ Any user who has configured an SSH key. @@ -65,14 +71,16 @@ EXAMPLES Approve the change with commit c0ff33 as "Verified +1" ===== - $ ssh -p 29418 review.example.com gerrit approve --verified=+1 c0ff33 + $ ssh -p 29418 review.example.com gerrit review --verified=+1 c0ff33 ===== -Mark the unmerged commits both "Verified +1" and "Code Review +2": +Mark the unmerged commits both "Verified +1" and "Code Review +2" and +submit them for merging: ==== - $ ssh -p 29418 review.example.com gerrit approve \ + $ ssh -p 29418 review.example.com gerrit review \ --verified=+1 \ --code-review=+2 \ + --submit \ --project=this/project \ $(git rev-list origin/master..HEAD) ==== 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 4d9c177a8c..2f28f984ed 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 @@ -14,15 +14,23 @@ package com.google.gerrit.server; +import static com.google.gerrit.reviewdb.ApprovalCategory.SUBMIT; + 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.server.git.MergeQueue; +import com.google.gwtorm.client.AtomicUpdate; import com.google.gwtorm.client.OrmConcurrencyException; import com.google.gwtorm.client.OrmException; import org.eclipse.jgit.util.Base64; import org.eclipse.jgit.util.NB; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; public class ChangeUtil { private static int uuidPrefix; @@ -67,6 +75,49 @@ public class ChangeUtil { computeSortKey(c); } + public static void submit(PatchSet.Id patchSetId, IdentifiedUser user, ReviewDb db, MergeQueue merger) + throws OrmException { + final Change.Id changeId = patchSetId.getParentKey(); + final PatchSetApproval approval = createSubmitApproval(patchSetId, user, db); + + db.patchSetApprovals().upsert(Collections.singleton(approval)); + + final Change change = db.changes().atomicUpdate(changeId, new AtomicUpdate() { + @Override + public Change update(Change change) { + if (change.getStatus() == Change.Status.NEW) { + change.setStatus(Change.Status.SUBMITTED); + ChangeUtil.updated(change); + } + return change; + } + }); + + if (change.getStatus() == Change.Status.SUBMITTED) { + merger.merge(change.getDest()); + } + } + + public static PatchSetApproval createSubmitApproval(PatchSet.Id patchSetId, IdentifiedUser user, ReviewDb db) + throws OrmException { + final List allApprovals = + new ArrayList(db.patchSetApprovals().byPatchSet( + patchSetId).toList()); + + final PatchSetApproval.Key akey = + new PatchSetApproval.Key(patchSetId, user.getAccountId(), SUBMIT); + + for (final PatchSetApproval approval : allApprovals) { + if (akey.equals(approval.getKey())) { + approval.setValue((short) 1); + approval.setGranted(); + return approval; + } + } + return new PatchSetApproval(akey, (short) 1); + } + + public static void computeSortKey(final Change c) { // 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-server/src/main/java/com/google/gerrit/server/project/CanSubmitResult.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CanSubmitResult.java new file mode 100644 index 0000000000..0c14b80b35 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CanSubmitResult.java @@ -0,0 +1,43 @@ +// Copyright (C) 2010 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.project; + +/** + * Result from {@code ChangeControl.canSubmit()}. + * + * @see ChangeControl#canSubmit(com.google.gerrit.reviewdb.PatchSet.Id, + * com.google.gerrit.reviewdb.ReviewDb, + * com.google.gerrit.common.data.ApprovalTypes, + * com.google.gerrit.server.workflow.FunctionState.Factory) + */ +public class CanSubmitResult { + /** Magic constant meaning submitting is possible. */ + public static final CanSubmitResult OK = new CanSubmitResult("OK"); + + private final String errorMessage; + + CanSubmitResult(String error) { + this.errorMessage = error; + } + + public String getMessage() { + return errorMessage; + } + + @Override + public String toString() { + return "CanSubmitResult[" + getMessage() + "]"; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index 86afcbed77..1664178c53 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -14,17 +14,27 @@ package com.google.gerrit.server.project; -import com.google.gerrit.reviewdb.Account; +import com.google.gerrit.common.data.ApprovalType; +import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.reviewdb.Change; +import com.google.gerrit.reviewdb.PatchSet; import com.google.gerrit.reviewdb.PatchSetApproval; import com.google.gerrit.reviewdb.Project; import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; +import com.google.gerrit.server.workflow.CategoryFunction; +import com.google.gerrit.server.workflow.FunctionState; import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; +import java.util.ArrayList; +import java.util.List; + + /** Access control management for a user accessing a single change. */ public class ChangeControl { public static class Factory { @@ -173,4 +183,54 @@ public class ChangeControl { return false; } + + /** @return {@link CanSubmitResult#OK}, or a result with an error message. */ + public CanSubmitResult canSubmit(final PatchSet.Id patchSetId, final ReviewDb db, + final ApprovalTypes approvalTypes, + FunctionState.Factory functionStateFactory) + throws OrmException { + + if (change.getStatus().isClosed()) { + return new CanSubmitResult("Change " + change.getId() + " is closed"); + } + if (!patchSetId.equals(change.currentPatchSetId())) { + return new CanSubmitResult("Patch set " + patchSetId + " is not current"); + } + if (!getRefControl().canSubmit()) { + return new CanSubmitResult("User does not have permission to submit"); + } + if (!(getCurrentUser() instanceof IdentifiedUser)) { + return new CanSubmitResult("User is not signed-in"); + } + + final List allApprovals = + new ArrayList(db.patchSetApprovals().byPatchSet( + patchSetId).toList()); + final PatchSetApproval myAction = + ChangeUtil.createSubmitApproval(patchSetId, + (IdentifiedUser) getCurrentUser(), db); + + final ApprovalType actionType = + approvalTypes.getApprovalType(myAction.getCategoryId()); + if (actionType == null || !actionType.getCategory().isAction()) { + return new CanSubmitResult("Invalid action " + myAction.getCategoryId()); + } + + final FunctionState fs = + functionStateFactory.create(change, patchSetId, allApprovals); + for (ApprovalType c : approvalTypes.getApprovalTypes()) { + CategoryFunction.forCategory(c.getCategory()).run(c, fs); + } + if (!CategoryFunction.forCategory(actionType.getCategory()).isValid( + getCurrentUser(), actionType, fs)) { + return new CanSubmitResult(actionType.getCategory().getName() + + " not permitted"); + } + fs.normalize(actionType, myAction); + if (myAction.getValue() <= 0) { + return new CanSubmitResult(actionType.getCategory().getName() + + " not permitted"); + } + return CanSubmitResult.OK; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java index afeb422319..6d3d5cd124 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java @@ -118,6 +118,11 @@ public class RefControl { return canPerform(READ, (short) 2); } + /** @return true if this user can submit patch sets to this ref */ + public boolean canSubmit() { + return canPerform(ApprovalCategory.SUBMIT, (short) 1); + } + /** @return true if the user can update the reference as a fast-forward. */ public boolean canUpdate() { return canPerform(PUSH_HEAD, PUSH_HEAD_UPDATE); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/MasterCommandModule.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/MasterCommandModule.java index 886b4a8b2a..c83d604e34 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/MasterCommandModule.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/MasterCommandModule.java @@ -25,12 +25,13 @@ public class MasterCommandModule extends CommandModule { protected void configure() { final CommandName gerrit = Commands.named("gerrit"); - command(gerrit, "approve").to(ApproveCommand.class); + command(gerrit, "approve").to(ReviewCommand.class); command(gerrit, "create-account").to(AdminCreateAccount.class); command(gerrit, "create-project").to(CreateProject.class); command(gerrit, "gsql").to(AdminQueryShell.class); command(gerrit, "receive-pack").to(Receive.class); command(gerrit, "replicate").to(AdminReplicate.class); command(gerrit, "set-project-parent").to(AdminSetParent.class); + command(gerrit, "review").to(ReviewCommand.class); } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java similarity index 93% rename from gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveCommand.java rename to gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index 202ee7642a..448745167c 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -27,10 +27,12 @@ import com.google.gerrit.reviewdb.RevId; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.git.MergeQueue; import com.google.gerrit.server.mail.CommentSender; import com.google.gerrit.server.mail.EmailException; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; +import com.google.gerrit.server.project.CanSubmitResult; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectControl; @@ -56,9 +58,9 @@ import java.util.List; import java.util.Map; import java.util.Set; -public class ApproveCommand extends BaseCommand { +public class ReviewCommand extends BaseCommand { private static final Logger log = - LoggerFactory.getLogger(ApproveCommand.class); + LoggerFactory.getLogger(ReviewCommand.class); @Override protected final CmdLineParser newCmdLineParser() { @@ -71,7 +73,7 @@ public class ApproveCommand extends BaseCommand { private final Set patchSetIds = new HashSet(); - @Argument(index = 0, required = true, multiValued = true, metaVar = "{COMMIT | CHANGE,PATCHSET}", usage = "patch to approve") + @Argument(index = 0, required = true, multiValued = true, metaVar = "{COMMIT | CHANGE,PATCHSET}", usage = "patch to review") void addPatchSetId(final String token) { try { patchSetIds.addAll(parsePatchSetId(token)); @@ -88,12 +90,18 @@ public class ApproveCommand extends BaseCommand { @Option(name = "--message", aliases = "-m", usage = "cover message to publish on change", metaVar = "MESSAGE") private String changeComment; + @Option(name = "--submit", aliases = "-s", usage = "submit the patch set") + private boolean submitChange; + @Inject private ReviewDb db; @Inject private IdentifiedUser currentUser; + @Inject + private MergeQueue merger; + @Inject private CommentSender.Factory commentSenderFactory; @@ -209,6 +217,17 @@ public class ApproveCommand extends BaseCommand { hooks.doCommentAddedHook(change, currentUser.getAccount(), db.patchSets() .get(patchSetId), changeComment, approvalsMap); + + if (submitChange) { + CanSubmitResult result = + changeControl.canSubmit(patchSetId, db, approvalTypes, + functionStateFactory); + if (result == CanSubmitResult.OK) { + ChangeUtil.submit(patchSetId, currentUser, db, merger); + } else { + throw error(result.getMessage()); + } + } } private Set parsePatchSetId(final String patchIdentity) diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SlaveCommandModule.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SlaveCommandModule.java index 06bd80adbf..32ab2db09e 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SlaveCommandModule.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SlaveCommandModule.java @@ -31,6 +31,7 @@ public class SlaveCommandModule extends CommandModule { command(gerrit, "gsql").to(ErrorSlaveMode.class); command(gerrit, "receive-pack").to(ErrorSlaveMode.class); command(gerrit, "replicate").to(ErrorSlaveMode.class); + command(gerrit, "review").to(ErrorSlaveMode.class); command(gerrit, "set-project-parent").to(ErrorSlaveMode.class); } }