Allow project-specific LabelTypes in ReviewCommand

Preserve the old site-global options by using label names from
All-Projects. Allow custom/project-specific label names with
--label=My-Label=N, which override the old-style options, if present.

When we have project-specific labels, this means the help text will
include e.g. --verified even if that is removed or has different value
in the relevant projects; the command will then fail when the server
actually tries to apply the review score.

Change-Id: I915399556e9e977b36e996071ff0346f76bc0ff5
This commit is contained in:
Dave Borowitz
2013-02-15 14:16:07 -08:00
parent 6cae753a4c
commit 55b290a92c
2 changed files with 51 additions and 14 deletions

View File

@@ -17,6 +17,7 @@ SYNOPSIS
[--publish] [--publish]
[--delete] [--delete]
[--verified <N>] [--code-review <N>] [--verified <N>] [--code-review <N>]
[--label Label-Name=<N>]
{COMMIT | CHANGEID,PATCHSET}... {COMMIT | CHANGEID,PATCHSET}...
DESCRIPTION DESCRIPTION
@@ -94,10 +95,14 @@ fails because the user is not permitted to change the label.
--code-review:: --code-review::
--verified:: --verified::
Set the approval category to the value 'N'. The exact Set the label to the value 'N'. The exact option names
option names supported and the range of values permitted supported and the range of values permitted differs per site,
differs per site, check the output of --help, or contact check the output of --help, or contact your site administrator
your site administrator for further details. for further details. These options are only available for these
built-in labels; for other labels, see --label.
--label::
Set a label by name to the value 'N'.
ACCESS ACCESS
------ ------

View File

@@ -14,10 +14,11 @@
package com.google.gerrit.sshd.commands; package com.google.gerrit.sshd.commands;
import com.google.common.base.Splitter;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.LabelValue; import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.ReviewResult; import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.common.data.ReviewResult.Error.Type; import com.google.gerrit.common.data.ReviewResult.Error.Type;
@@ -36,9 +37,11 @@ import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Submit; import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.changedetail.DeleteDraftPatchSet; import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.changedetail.PublishDraft;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; 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.server.project.ProjectControl;
import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand; import com.google.gerrit.sshd.SshCommand;
@@ -57,6 +60,7 @@ import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;
@CommandMetaData(name = "review", descr = "Verify, approve and/or submit one or more patch sets") @CommandMetaData(name = "review", descr = "Verify, approve and/or submit one or more patch sets")
@@ -75,8 +79,7 @@ public class ReviewCommand extends SshCommand {
private final Set<PatchSet> patchSets = new HashSet<PatchSet>(); private final Set<PatchSet> patchSets = new HashSet<PatchSet>();
@Argument(index = 0, required = true, multiValued = true, metaVar = "{COMMIT | CHANGE,PATCHSET}", @Argument(index = 0, required = true, multiValued = true, metaVar = "{COMMIT | CHANGE,PATCHSET}", usage = "list of commits or patch sets to review")
usage = "list of commits or patch sets to review")
void addPatchSetId(final String token) { void addPatchSetId(final String token) {
try { try {
patchSets.add(parsePatchSet(token)); patchSets.add(parsePatchSet(token));
@@ -112,14 +115,33 @@ public class ReviewCommand extends SshCommand {
@Option(name = "--delete", usage = "delete the specified draft patch set(s)") @Option(name = "--delete", usage = "delete the specified draft patch set(s)")
private boolean deleteDraftPatchSet; 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 @Inject
private ReviewDb db; private ReviewDb db;
@Inject @Inject
private LabelTypes labelTypes; private DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory;
@Inject @Inject
private DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory; private ProjectControl.Factory projectControlFactory;
@Inject
private AllProjectsName allProjects;
@Inject @Inject
private ChangeControl.Factory changeControlFactory; private ChangeControl.Factory changeControlFactory;
@@ -140,6 +162,7 @@ public class ReviewCommand extends SshCommand {
private Provider<Submit> submitProvider; private Provider<Submit> submitProvider;
private List<ApproveOption> optionList; private List<ApproveOption> optionList;
private Map<String, Short> customLabels;
@Override @Override
protected void run() throws UnloggedFailure { protected void run() throws UnloggedFailure {
@@ -218,6 +241,7 @@ public class ReviewCommand extends SshCommand {
review.labels.put(ao.getLabelName(), v); review.labels.put(ao.getLabelName(), v);
} }
} }
review.labels.putAll(customLabels);
// If review labels are being applied, the comment will be included // 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 // on the review note. We don't need to add it again on the abandon
@@ -237,9 +261,9 @@ public class ReviewCommand extends SshCommand {
applyReview(ctl, patchSet, review); applyReview(ctl, patchSet, review);
try { try {
abandon.apply(new ChangeResource(ctl), input); abandon.apply(new ChangeResource(ctl), input);
} catch(AuthException e) { } catch (AuthException e) {
writeError("error: " + parseError(Type.ABANDON_NOT_PERMITTED) + "\n"); writeError("error: " + parseError(Type.ABANDON_NOT_PERMITTED) + "\n");
} catch(ResourceConflictException e) { } catch (ResourceConflictException e) {
writeError("error: " + parseError(Type.CHANGE_IS_CLOSED) + "\n"); writeError("error: " + parseError(Type.CHANGE_IS_CLOSED) + "\n");
} }
} else if (restoreChange) { } else if (restoreChange) {
@@ -249,9 +273,9 @@ public class ReviewCommand extends SshCommand {
try { try {
restore.apply(new ChangeResource(ctl), input); restore.apply(new ChangeResource(ctl), input);
applyReview(ctl, patchSet, review); applyReview(ctl, patchSet, review);
} catch(AuthException e) { } catch (AuthException e) {
writeError("error: " + parseError(Type.RESTORE_NOT_PERMITTED) + "\n"); writeError("error: " + parseError(Type.RESTORE_NOT_PERMITTED) + "\n");
} catch(ResourceConflictException e) { } catch (ResourceConflictException e) {
writeError("error: " + parseError(Type.CHANGE_NOT_ABANDONED) + "\n"); writeError("error: " + parseError(Type.CHANGE_NOT_ABANDONED) + "\n");
} }
} else { } else {
@@ -399,8 +423,16 @@ public class ReviewCommand extends SshCommand {
@Override @Override
protected void parseCommandLine() throws UnloggedFailure { protected void parseCommandLine() throws UnloggedFailure {
optionList = new ArrayList<ApproveOption>(); optionList = new ArrayList<ApproveOption>();
customLabels = Maps.newHashMap();
for (LabelType type : labelTypes.getLabelTypes()) { ProjectControl allProjectsControl;
try {
allProjectsControl = projectControlFactory.controlFor(allProjects);
} catch (NoSuchProjectException e) {
throw new UnloggedFailure("missing " + allProjects.get());
}
for (LabelType type : allProjectsControl.getLabelTypes().getLabelTypes()) {
String usage = ""; String usage = "";
usage = "score for " + type.getName() + "\n"; usage = "score for " + type.getName() + "\n";