diff options
Diffstat (limited to 'gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java')
| -rw-r--r-- | gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java | 237 |
1 files changed, 122 insertions, 115 deletions
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 47bae4f32e..640adbf70c 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,33 +14,29 @@ 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; -import com.google.gerrit.reviewdb.ApprovalCategoryValue; -import com.google.gerrit.reviewdb.Branch; -import com.google.gerrit.reviewdb.Change; -import com.google.gerrit.reviewdb.PatchSet; -import com.google.gerrit.reviewdb.PatchSetApproval; -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.MergeOp; -import com.google.gerrit.server.git.MergeQueue; -import com.google.gerrit.server.mail.AbandonedSender; +import com.google.gerrit.common.data.ReviewResult; +import com.google.gerrit.reviewdb.client.ApprovalCategory; +import com.google.gerrit.reviewdb.client.ApprovalCategoryValue; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.RevId; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.changedetail.AbandonChange; +import com.google.gerrit.server.changedetail.DeleteDraftPatchSet; +import com.google.gerrit.server.changedetail.PublishDraft; +import com.google.gerrit.server.changedetail.RestoreChange; +import com.google.gerrit.server.changedetail.Submit; 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; +import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectControl; -import com.google.gerrit.server.workflow.FunctionState; import com.google.gerrit.sshd.BaseCommand; import com.google.gerrit.util.cli.CmdLineParser; -import com.google.gwtorm.client.OrmException; -import com.google.gwtorm.client.ResultSet; +import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import org.apache.sshd.server.Environment; @@ -55,15 +51,14 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.concurrent.TimeUnit; public class ReviewCommand extends BaseCommand { private static final Logger log = LoggerFactory.getLogger(ReviewCommand.class); @Override - protected final CmdLineParser newCmdLineParser() { - final CmdLineParser parser = super.newCmdLineParser(); + protected final CmdLineParser newCmdLineParser(Object options) { + final CmdLineParser parser = super.newCmdLineParser(options); for (ApproveOption c : optionList) { parser.addOption(c, c); } @@ -98,39 +93,41 @@ public class ReviewCommand extends BaseCommand { @Option(name = "--submit", aliases = "-s", usage = "submit the patch set") private boolean submitChange; - @Inject - private ReviewDb db; + @Option(name = "--force-message", usage = "publish the message, " + + "even if the label score cannot be applied due to change being closed") + private boolean forceMessage = false; - @Inject - private IdentifiedUser currentUser; + @Option(name = "--publish", usage = "publish a draft patch set") + private boolean publishPatchSet; - @Inject - private MergeQueue merger; + @Option(name = "--delete", usage = "delete a draft patch set") + private boolean deleteDraftPatchSet; @Inject - private MergeOp.Factory opFactory; + private ReviewDb db; @Inject private ApprovalTypes approvalTypes; @Inject - private ChangeControl.Factory changeControlFactory; + private DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory; @Inject - private AbandonedSender.Factory abandonedSenderFactory; + private AbandonChange.Factory abandonChangeFactory; @Inject - private FunctionState.Factory functionStateFactory; + private PublishComments.Factory publishCommentsFactory; @Inject - private PublishComments.Factory publishCommentsFactory; + private PublishDraft.Factory publishDraftFactory; @Inject - private ChangeHookRunner hooks; + private RestoreChange.Factory restoreChangeFactory; - private List<ApproveOption> optionList; + @Inject + private Submit.Factory submitFactory; - private Set<PatchSet.Id> toSubmit = new HashSet<PatchSet.Id>(); + private List<ApproveOption> optionList; @Override public final void start(final Environment env) { @@ -146,6 +143,23 @@ public class ReviewCommand extends BaseCommand { if (submitChange) { throw error("abandon and submit actions are mutually exclusive"); } + if (publishPatchSet) { + throw error("abandon and publish actions are mutually exclusive"); + } + if (deleteDraftPatchSet) { + throw error("abandon and delete actions are mutually exclusive"); + } + } + if (publishPatchSet) { + if (restoreChange) { + throw error("publish and restore actions are mutually exclusive"); + } + if (submitChange) { + throw error("publish and submit actions are mutually exclusive"); + } + if (deleteDraftPatchSet) { + throw error("publish and delete actions are mutually exclusive"); + } } boolean ok = true; @@ -155,6 +169,9 @@ public class ReviewCommand extends BaseCommand { } catch (UnloggedFailure e) { ok = false; writeError("error: " + e.getMessage() + "\n"); + } catch (NoSuchChangeException e) { + ok = false; + writeError("no such change " + patchSetId.getParentKey().get()); } catch (Exception e) { ok = false; writeError("fatal: internal server error while approving " @@ -168,46 +185,12 @@ public class ReviewCommand extends BaseCommand { + " review output above"); } - if (!toSubmit.isEmpty()) { - final Set<Branch.NameKey> toMerge = new HashSet<Branch.NameKey>(); - try { - for (PatchSet.Id patchSetId : toSubmit) { - ChangeUtil.submit(patchSetId, currentUser, db, opFactory, - new MergeQueue() { - @Override - public void merge(MergeOp.Factory mof, Branch.NameKey branch) { - toMerge.add(branch); - } - - @Override - public void schedule(Branch.NameKey branch) { - toMerge.add(branch); - } - - @Override - public void recheckAfter(Branch.NameKey branch, long delay, - TimeUnit delayUnit) { - toMerge.add(branch); - } - }); - } - for (Branch.NameKey branch : toMerge) { - merger.merge(opFactory, branch); - } - } catch (OrmException updateError) { - throw new Failure(1, "one or more submits failed", updateError); - } - } } }); } - private void approveOne(final PatchSet.Id patchSetId) - throws NoSuchChangeException, UnloggedFailure, OrmException, - EmailException { - - final Change.Id changeId = patchSetId.getParentKey(); - ChangeControl changeControl = changeControlFactory.validateFor(changeId); + private void approveOne(final PatchSet.Id patchSetId) throws + NoSuchChangeException, OrmException, EmailException, Failure { if (changeComment == null) { changeComment = ""; @@ -217,43 +200,83 @@ public class ReviewCommand extends BaseCommand { for (ApproveOption ao : optionList) { Short v = ao.value(); if (v != null) { - assertScoreIsAllowed(patchSetId, changeControl, ao, v); aps.add(new ApprovalCategoryValue.Id(ao.getCategoryId(), v)); } } - 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"); + try { + publishCommentsFactory.create(patchSetId, changeComment, aps, forceMessage).call(); + + if (abandonChange) { + final ReviewResult result = abandonChangeFactory.create( + patchSetId, changeComment).call(); + handleReviewResultErrors(result); + } else if (restoreChange) { + final ReviewResult result = restoreChangeFactory.create( + patchSetId, changeComment).call(); + handleReviewResultErrors(result); } if (submitChange) { - changeControl = changeControlFactory.validateFor(changeId); + final ReviewResult result = submitFactory.create(patchSetId).call(); + handleReviewResultErrors(result); } + } catch (InvalidChangeOperationException e) { + throw error(e.getMessage()); + } catch (IllegalStateException e) { + throw error(e.getMessage()); } - if (submitChange) { - CanSubmitResult result = - changeControl.canSubmit(patchSetId, db, approvalTypes, - functionStateFactory); - if (result == CanSubmitResult.OK) { - toSubmit.add(patchSetId); - } else { - throw error(result.getMessage()); + if (publishPatchSet) { + final ReviewResult result = publishDraftFactory.create(patchSetId).call(); + handleReviewResultErrors(result); + } else if (deleteDraftPatchSet) { + final ReviewResult result = + deleteDraftPatchSetFactory.create(patchSetId).call(); + handleReviewResultErrors(result); + } + } + + private void handleReviewResultErrors(final ReviewResult result) { + for (ReviewResult.Error resultError : result.getErrors()) { + String errMsg = "error: (change " + result.getChangeId() + ") "; + switch (resultError.getType()) { + case ABANDON_NOT_PERMITTED: + errMsg += "not permitted to abandon change"; + break; + case RESTORE_NOT_PERMITTED: + errMsg += "not permitted to restore change"; + break; + case SUBMIT_NOT_PERMITTED: + errMsg += "not permitted to submit change"; + break; + case SUBMIT_NOT_READY: + errMsg += "approvals or dependencies lacking"; + break; + case CHANGE_IS_CLOSED: + errMsg += "change is closed"; + break; + case PUBLISH_NOT_PERMITTED: + errMsg += "not permitted to publish change"; + break; + case DELETE_NOT_PERMITTED: + errMsg += "not permitted to delete change/patch set"; + break; + case RULE_ERROR: + errMsg += "rule error"; + break; + case NOT_A_DRAFT: + errMsg += "change is not a draft"; + break; + case GIT_ERROR: + errMsg += "error writing change to git repository"; + break; + default: + errMsg += "failure in review"; } + if (resultError.getMessage() != null) { + errMsg += ": " + resultError.getMessage(); + } + writeError(errMsg); } } @@ -321,22 +344,6 @@ public class ReviewCommand extends BaseCommand { return projectControl.getProject().getNameKey().equals(change.getProject()); } - private void assertScoreIsAllowed(final PatchSet.Id patchSetId, - final ChangeControl changeControl, ApproveOption ao, Short v) - throws UnloggedFailure { - final PatchSetApproval psa = - new PatchSetApproval(new PatchSetApproval.Key(patchSetId, currentUser - .getAccountId(), ao.getCategoryId()), v); - final FunctionState fs = - functionStateFactory.create(changeControl.getChange(), patchSetId, - Collections.<PatchSetApproval> emptyList()); - psa.setValue(v); - fs.normalize(approvalTypes.getApprovalType(psa.getCategoryId()), psa); - if (v != psa.getValue()) { - throw error(ao.name() + "=" + ao.value() + " not permitted"); - } - } - private void initOptionList() { optionList = new ArrayList<ApproveOption>(); |