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 | 271 |
1 files changed, 173 insertions, 98 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 5ebb6c7fbd..5769a22bdb 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,25 +14,36 @@ package com.google.gerrit.sshd.commands; -import com.google.gerrit.common.data.ApprovalType; -import com.google.gerrit.common.data.ApprovalTypes; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Maps; +import com.google.gerrit.common.data.LabelType; +import com.google.gerrit.common.data.LabelValue; 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.common.data.ReviewResult.Error.Type; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.ResourceConflictException; 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.change.Abandon; +import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.PostReview; +import com.google.gerrit.server.change.Restore; +import com.google.gerrit.server.change.RevisionResource; +import com.google.gerrit.server.change.Submit; 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.config.AllProjectsName; +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.NoSuchProjectException; import com.google.gerrit.server.project.ProjectControl; +import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.SshCommand; import com.google.gerrit.util.cli.CmdLineParser; import com.google.gwtorm.server.OrmException; @@ -40,7 +51,6 @@ import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import com.google.inject.Provider; -import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; import org.slf4j.Logger; @@ -48,11 +58,12 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; +@CommandMetaData(name = "review", descr = "Verify, approve and/or submit one or more patch sets") public class ReviewCommand extends SshCommand { private static final Logger log = LoggerFactory.getLogger(ReviewCommand.class); @@ -66,13 +77,12 @@ public class ReviewCommand extends SshCommand { return parser; } - private final Set<PatchSet.Id> patchSetIds = new HashSet<PatchSet.Id>(); + private final Set<PatchSet> patchSets = new HashSet<PatchSet>(); - @Argument(index = 0, required = true, multiValued = true, metaVar = "{COMMIT | CHANGE,PATCHSET}", - usage = "list of commits or patch sets to review") + @Argument(index = 0, required = true, multiValued = true, metaVar = "{COMMIT | CHANGE,PATCHSET}", usage = "list of commits or patch sets to review") void addPatchSetId(final String token) { try { - patchSetIds.addAll(parsePatchSetId(token)); + patchSets.add(parsePatchSet(token)); } catch (UnloggedFailure e) { throw new IllegalArgumentException(e.getMessage(), e); } catch (OrmException e) { @@ -105,31 +115,54 @@ public class ReviewCommand extends SshCommand { @Option(name = "--delete", usage = "delete the specified draft patch set(s)") private boolean deleteDraftPatchSet; + @Option(name = "--label", aliases = "-l", usage = "custom label(s) to assign", metaVar = "LABEL=VALUE") + void addLabel(final String token) { + List<String> parts = ImmutableList.copyOf(Splitter.on('=').split(token)); + if (parts.size() != 2) { + throw new IllegalArgumentException("invalid custom label " + token); + } + short value; + try { + value = Short.parseShort(parts.get(1)); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("invalid custom label value " + + parts.get(1)); + } + customLabels.put(parts.get(0), value); + } + @Inject private ReviewDb db; @Inject - private ApprovalTypes approvalTypes; + private DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory; @Inject - private DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory; + private ProjectControl.Factory projectControlFactory; + + @Inject + private AllProjectsName allProjects; + + @Inject + private ChangeControl.Factory changeControlFactory; @Inject - private Provider<AbandonChange> abandonChangeProvider; + private Provider<Abandon> abandonProvider; @Inject - private PublishComments.Factory publishCommentsFactory; + private Provider<PostReview> reviewProvider; @Inject private PublishDraft.Factory publishDraftFactory; @Inject - private Provider<RestoreChange> restoreChangeProvider; + private Provider<Restore> restoreProvider; @Inject - private Submit.Factory submitFactory; + private Provider<Submit> submitProvider; private List<ApproveOption> optionList; + private Map<String, Short> customLabels; @Override protected void run() throws UnloggedFailure { @@ -160,20 +193,20 @@ public class ReviewCommand extends SshCommand { } boolean ok = true; - for (final PatchSet.Id patchSetId : patchSetIds) { + for (final PatchSet patchSet : patchSets) { try { - approveOne(patchSetId); + approveOne(patchSet); } catch (UnloggedFailure e) { ok = false; writeError("error: " + e.getMessage() + "\n"); } catch (NoSuchChangeException e) { ok = false; - writeError("no such change " + patchSetId.getParentKey().get()); + writeError("no such change " + patchSet.getId().getParentKey().get()); } catch (Exception e) { ok = false; writeError("fatal: internal server error while approving " - + patchSetId + "\n"); - log.error("internal error while approving " + patchSetId, e); + + patchSet.getId() + "\n"); + log.error("internal error while approving " + patchSet.getId(), e); } } @@ -183,54 +216,97 @@ public class ReviewCommand extends SshCommand { } } - private void approveOne(final PatchSet.Id patchSetId) - throws NoSuchChangeException, OrmException, EmailException, Failure, - RepositoryNotFoundException, IOException { + private void applyReview(final ChangeControl ctl, final PatchSet patchSet, + final PostReview.Input review) throws Exception { + reviewProvider.get().apply(new RevisionResource( + new ChangeResource(ctl), patchSet), review); + } + + private void approveOne(final PatchSet patchSet) throws Exception { if (changeComment == null) { changeComment = ""; } - Set<ApprovalCategoryValue.Id> aps = new HashSet<ApprovalCategoryValue.Id>(); + PostReview.Input review = new PostReview.Input(); + review.message = Strings.emptyToNull(changeComment); + review.labels = Maps.newTreeMap(); + review.drafts = PostReview.DraftHandling.PUBLISH; + review.strictLabels = false; for (ApproveOption ao : optionList) { Short v = ao.value(); if (v != null) { - aps.add(new ApprovalCategoryValue.Id(ao.getCategoryId(), v)); + review.labels.put(ao.getLabelName(), v); } } + review.labels.putAll(customLabels); + + // If review labels are being applied, the comment will be included + // on the review note. We don't need to add it again on the abandon + // or restore comment. + if (!review.labels.isEmpty() && (abandonChange || restoreChange)) { + changeComment = null; + } try { - publishCommentsFactory.create(patchSetId, changeComment, aps, forceMessage).call(); + ChangeControl ctl = + changeControlFactory.controlFor(patchSet.getId().getParentKey()); if (abandonChange) { - final AbandonChange abandonChange = abandonChangeProvider.get(); - abandonChange.setChangeId(patchSetId.getParentKey()); - abandonChange.setMessage(changeComment); - final ReviewResult result = abandonChange.call(); - handleReviewResultErrors(result); + final Abandon abandon = abandonProvider.get(); + final Abandon.Input input = new Abandon.Input(); + input.message = changeComment; + applyReview(ctl, patchSet, review); + try { + abandon.apply(new ChangeResource(ctl), input); + } catch (AuthException e) { + writeError("error: " + parseError(Type.ABANDON_NOT_PERMITTED) + "\n"); + } catch (ResourceConflictException e) { + writeError("error: " + parseError(Type.CHANGE_IS_CLOSED) + "\n"); + } } else if (restoreChange) { - final RestoreChange restoreChange = restoreChangeProvider.get(); - restoreChange.setChangeId(patchSetId.getParentKey()); - restoreChange.setMessage(changeComment); - final ReviewResult result = restoreChange.call(); - handleReviewResultErrors(result); + final Restore restore = restoreProvider.get(); + final Restore.Input input = new Restore.Input(); + input.message = changeComment; + try { + restore.apply(new ChangeResource(ctl), input); + applyReview(ctl, patchSet, review); + } catch (AuthException e) { + writeError("error: " + parseError(Type.RESTORE_NOT_PERMITTED) + "\n"); + } catch (ResourceConflictException e) { + writeError("error: " + parseError(Type.CHANGE_NOT_ABANDONED) + "\n"); + } + } else { + applyReview(ctl, patchSet, review); } + if (submitChange) { - final ReviewResult result = submitFactory.create(patchSetId).call(); - handleReviewResultErrors(result); + Submit submit = submitProvider.get(); + Submit.Input input = new Submit.Input(); + input.waitForMerge = true; + submit.apply(new RevisionResource( + new ChangeResource(ctl), patchSet), + input); } } catch (InvalidChangeOperationException e) { throw error(e.getMessage()); } catch (IllegalStateException e) { throw error(e.getMessage()); + } catch (AuthException e) { + throw error(e.getMessage()); + } catch (BadRequestException e) { + throw error(e.getMessage()); + } catch (ResourceConflictException e) { + throw error(e.getMessage()); } if (publishPatchSet) { - final ReviewResult result = publishDraftFactory.create(patchSetId).call(); + final ReviewResult result = + publishDraftFactory.create(patchSet.getId()).call(); handleReviewResultErrors(result); } else if (deleteDraftPatchSet) { final ReviewResult result = - deleteDraftPatchSetFactory.create(patchSetId).call(); + deleteDraftPatchSetFactory.create(patchSet.getId()).call(); handleReviewResultErrors(result); } } @@ -238,46 +314,7 @@ public class ReviewCommand extends SshCommand { 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 CHANGE_NOT_ABANDONED: - errMsg += "change is not abandoned"; - 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/patch set is not a draft"; - break; - case GIT_ERROR: - errMsg += "error writing change to git repository"; - break; - case DEST_BRANCH_NOT_FOUND: - errMsg += "destination branch not found"; - break; - default: - errMsg += "failure in review"; - } + errMsg += parseError(resultError.getType()); if (resultError.getMessage() != null) { errMsg += ": " + resultError.getMessage(); } @@ -285,7 +322,38 @@ public class ReviewCommand extends SshCommand { } } - private Set<PatchSet.Id> parsePatchSetId(final String patchIdentity) + private String parseError(Type type) { + switch (type) { + case ABANDON_NOT_PERMITTED: + return "not permitted to abandon change"; + case RESTORE_NOT_PERMITTED: + return "not permitted to restore change"; + case SUBMIT_NOT_PERMITTED: + return "not permitted to submit change"; + case SUBMIT_NOT_READY: + return "approvals or dependencies lacking"; + case CHANGE_IS_CLOSED: + return "change is closed"; + case CHANGE_NOT_ABANDONED: + return "change is not abandoned"; + case PUBLISH_NOT_PERMITTED: + return "not permitted to publish change"; + case DELETE_NOT_PERMITTED: + return "not permitted to delete change/patch set"; + case RULE_ERROR: + return "rule error"; + case NOT_A_DRAFT: + return "change/patch set is not a draft"; + case GIT_ERROR: + return "error writing change to git repository"; + case DEST_BRANCH_NOT_FOUND: + return "destination branch not found"; + default: + return "failure in review"; + } + } + + private PatchSet parsePatchSet(final String patchIdentity) throws UnloggedFailure, OrmException { // By commit? // @@ -298,17 +366,17 @@ public class ReviewCommand extends SshCommand { patches = db.patchSets().byRevisionRange(id, id.max()); } - final Set<PatchSet.Id> matches = new HashSet<PatchSet.Id>(); + final Set<PatchSet> matches = new HashSet<PatchSet>(); for (final PatchSet ps : patches) { final Change change = db.changes().get(ps.getId().getParentKey()); if (inProject(change)) { - matches.add(ps.getId()); + matches.add(ps); } } switch (matches.size()) { case 1: - return matches; + return matches.iterator().next(); case 0: throw error("\"" + patchIdentity + "\" no such patch set"); default: @@ -325,7 +393,8 @@ public class ReviewCommand extends SshCommand { } catch (IllegalArgumentException e) { throw error("\"" + patchIdentity + "\" is not a valid patch set"); } - if (db.patchSets().get(patchSetId) == null) { + final PatchSet patchSet = db.patchSets().get(patchSetId); + if (patchSet == null) { throw error("\"" + patchIdentity + "\" no such patch set"); } if (projectControl != null) { @@ -335,7 +404,7 @@ public class ReviewCommand extends SshCommand { + projectControl.getProject().getName()); } } - return Collections.singleton(patchSetId); + return patchSet; } throw error("\"" + patchIdentity + "\" is not a valid patch set"); @@ -352,18 +421,24 @@ public class ReviewCommand extends SshCommand { @Override protected void parseCommandLine() throws UnloggedFailure { optionList = new ArrayList<ApproveOption>(); + customLabels = Maps.newHashMap(); + + ProjectControl allProjectsControl; + try { + allProjectsControl = projectControlFactory.controlFor(allProjects); + } catch (NoSuchProjectException e) { + throw new UnloggedFailure("missing " + allProjects.get()); + } - for (ApprovalType type : approvalTypes.getApprovalTypes()) { + for (LabelType type : allProjectsControl.getLabelTypes().getLabelTypes()) { String usage = ""; - final ApprovalCategory category = type.getCategory(); - usage = "score for " + category.getName() + "\n"; + usage = "score for " + type.getName() + "\n"; - for (ApprovalCategoryValue v : type.getValues()) { + for (LabelValue v : type.getValues()) { usage += v.format() + "\n"; } - final String name = - "--" + category.getName().toLowerCase().replace(' ', '-'); + final String name = "--" + type.getName().toLowerCase(); optionList.add(new ApproveOption(name, usage, type)); } |