Merge changes I3dd5812f,Ib2c6e777,Ie6f32619,I5d2e4d03

* changes:
  Remove 'runsAt = MASTER' from @CommandMetaData annotations
  SetReviewersCommand: Fix metaVar for arguments
  BaseCommand: Add writeError utility method
  Consolidate error reporting in SSH commands
This commit is contained in:
David Pursehouse
2016-05-25 15:09:50 +00:00
committed by Gerrit Code Review
25 changed files with 85 additions and 121 deletions

View File

@@ -63,19 +63,19 @@ public class AliasCommand extends BaseCommand {
for (String name : chain(command)) {
CommandProvider p = map.get(name);
if (p == null) {
throw new UnloggedFailure(1, getName() + ": not found");
throw die(getName() + ": not found");
}
Command cmd = p.getProvider().get();
if (!(cmd instanceof DispatchCommand)) {
throw new UnloggedFailure(1, getName() + ": not found");
throw die(getName() + ": not found");
}
map = ((DispatchCommand) cmd).getMap();
}
CommandProvider p = map.get(command.value());
if (p == null) {
throw new UnloggedFailure(1, getName() + ": not found");
throw die(getName() + ": not found");
}
Command cmd = p.getProvider().get();

View File

@@ -377,6 +377,14 @@ public abstract class BaseCommand implements Command {
return new UnloggedFailure(1, "fatal: " + why.getMessage(), why);
}
protected void writeError(String type, String msg) {
try {
err.write((type + ": " + msg + "\n").getBytes(ENC));
} catch (IOException e) {
// Ignored
}
}
public void checkExclusivity(final Object arg1, final String arg1name,
final Object arg2, final String arg2name) throws UnloggedFailure {
if (arg1 != null && arg2 != null) {

View File

@@ -73,7 +73,7 @@ final class DispatchCommand extends BaseCommand {
if (Strings.isNullOrEmpty(commandName)) {
StringWriter msg = new StringWriter();
msg.write(usage());
throw new UnloggedFailure(1, msg.toString());
throw die(msg.toString());
}
final CommandProvider p = commands.get(commandName);
@@ -81,7 +81,7 @@ final class DispatchCommand extends BaseCommand {
String msg =
(getName().isEmpty() ? "Gerrit Code Review" : getName()) + ": "
+ commandName + ": not found";
throw new UnloggedFailure(1, msg);
throw die(msg);
}
final Command cmd = p.getProvider().get();
@@ -96,7 +96,7 @@ final class DispatchCommand extends BaseCommand {
bc.setArguments(args.toArray(new String[args.size()]));
} else if (!args.isEmpty()) {
throw new UnloggedFailure(1, commandName + " does not take arguments");
throw die(commandName + " does not take arguments");
}
provideStateTo(cmd);

View File

@@ -115,10 +115,9 @@ public final class SuExec extends BaseCommand {
if (caller instanceof PeerDaemonUser) {
// OK.
} else if (!enableRunAs) {
throw new UnloggedFailure(1,
"fatal: suexec disabled by auth.enableRunAs = false");
throw die("suexec disabled by auth.enableRunAs = false");
} else if (!caller.getCapabilities().canRunAs()) {
throw new UnloggedFailure(1, "fatal: suexec not permitted");
throw die("suexec not permitted");
}
}

View File

@@ -47,7 +47,7 @@ final class AdminQueryShell extends SshCommand {
try {
checkPermission();
} catch (PermissionDeniedException err) {
throw new UnloggedFailure("fatal: " + err.getMessage());
throw die(err.getMessage());
}
QueryShell shell = factory.create(in, out);

View File

@@ -84,12 +84,11 @@ final class AdminSetParent extends SshCommand {
@Override
protected void run() throws Failure {
if (oldParent == null && children.isEmpty()) {
throw new UnloggedFailure(1, "fatal: child projects have to be specified as " +
"arguments or the --children-of option has to be set");
throw die("child projects have to be specified as " +
"arguments or the --children-of option has to be set");
}
if (oldParent == null && !excludedChildren.isEmpty()) {
throw new UnloggedFailure(1, "fatal: --exclude can only be used together " +
"with --children-of");
throw die("--exclude can only be used together with --children-of");
}
final StringBuilder err = new StringBuilder();
@@ -164,7 +163,7 @@ final class AdminSetParent extends SshCommand {
while (err.charAt(err.length() - 1) == '\n') {
err.setLength(err.length() - 1);
}
throw new UnloggedFailure(1, err.toString());
throw die(err.toString());
}
}

View File

@@ -48,7 +48,7 @@ final class AproposCommand extends SshCommand {
docResult.url));
}
} catch (DocQueryException dqe) {
throw new UnloggedFailure(1, "fatal: " + dqe.getMessage());
throw die(dqe);
}
}
}

View File

@@ -76,7 +76,7 @@ abstract class BaseTestPrologCommand extends SshCommand {
OutputFormat.JSON.newGson().toJson(result, stdout);
stdout.print('\n');
} catch (Exception e) {
throw new UnloggedFailure("Processing of prolog script failed: " + e);
throw die("Processing of prolog script failed: " + e);
}
}
}

View File

@@ -48,7 +48,7 @@ public final class CreateBranchCommand extends SshCommand {
gApi.projects().name(project.getProject().getNameKey().get())
.branch(name).create(in);
} catch (RestApiException e) {
throw new UnloggedFailure(1, "fatal: " + e.getMessage(), e);
throw die(e);
}
}
}

View File

@@ -134,7 +134,7 @@ final class CreateProjectCommand extends SshCommand {
try {
if (!suggestParent) {
if (projectName == null) {
throw new UnloggedFailure(1, "fatal: Project name is required.");
throw die("Project name is required.");
}
ProjectInput input = new ProjectInput();
@@ -176,7 +176,7 @@ final class CreateProjectCommand extends SshCommand {
}
}
} catch (RestApiException | NoSuchProjectException err) {
throw new UnloggedFailure(1, "fatal: " + err.getMessage(), err);
throw die(err);
}
}
@@ -188,7 +188,7 @@ final class CreateProjectCommand extends SshCommand {
String[] s = pluginConfigValue.split("=");
String[] s2 = s[0].split("\\.");
if (s.length != 2 || s2.length != 2) {
throw new UnloggedFailure(1, "Invalid plugin config value '"
throw die("Invalid plugin config value '"
+ pluginConfigValue
+ "', expected format '<plugin-name>.<parameter-name>=<value>'"
+ " or '<plugin-name>.<parameter-name>=<value1,value2,...>'");

View File

@@ -60,14 +60,14 @@ final class FlushCaches extends SshCommand {
try {
if (list) {
if (all || caches.size() > 0) {
throw error("error: cannot use --list with --all or --cache");
throw die("cannot use --list with --all or --cache");
}
doList();
return;
}
if (all && caches.size() > 0) {
throw error("error: cannot combine --all and --cache");
throw die("cannot combine --all and --cache");
} else if (!all && caches.size() == 1 && caches.contains("all")) {
caches.clear();
all = true;
@@ -87,10 +87,6 @@ final class FlushCaches extends SshCommand {
}
}
private static UnloggedFailure error(String msg) {
return new UnloggedFailure(1, msg);
}
@SuppressWarnings("unchecked")
private void doList() {
for (String name : (List<String>) listCaches

View File

@@ -68,11 +68,10 @@ public class GarbageCollectionCommand extends SshCommand {
private void verifyCommandLine() throws UnloggedFailure {
if (!all && projects.isEmpty()) {
throw new UnloggedFailure(1,
"needs projects as command arguments or --all option");
throw die("needs projects as command arguments or --all option");
}
if (all && !projects.isEmpty()) {
throw new UnloggedFailure(1,
throw die(
"either specify projects as command arguments or use --all option");
}
}

View File

@@ -14,8 +14,6 @@
package com.google.gerrit.sshd.commands;
import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.lucene.LuceneVersionManager;
@@ -28,8 +26,7 @@ import org.kohsuke.args4j.Argument;
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@CommandMetaData(name = "activate",
description = "Activate the latest index version available",
runsAt = MASTER)
description = "Activate the latest index version available")
public class IndexActivateCommand extends SshCommand {
@Argument(index = 0, required = true, metaVar = "INDEX",
@@ -48,8 +45,7 @@ public class IndexActivateCommand extends SshCommand {
stdout.println("Not activating index, already using latest version");
}
} catch (ReindexerAlreadyRunningException e) {
throw new UnloggedFailure("Failed to activate latest index: "
+ e.getMessage());
throw die("Failed to activate latest index: " + e.getMessage());
}
}
}

View File

@@ -14,8 +14,6 @@
package com.google.gerrit.sshd.commands;
import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.lucene.LuceneVersionManager;
@@ -27,8 +25,7 @@ import com.google.inject.Inject;
import org.kohsuke.args4j.Argument;
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
@CommandMetaData(name = "start", description = "Start the online reindexer",
runsAt = MASTER)
@CommandMetaData(name = "start", description = "Start the online reindexer")
public class IndexStartCommand extends SshCommand {
@Argument(index = 0, required = true, metaVar = "INDEX",
@@ -47,7 +44,7 @@ public class IndexStartCommand extends SshCommand {
stdout.println("Nothing to reindex, index is already the latest version");
}
} catch (ReindexerAlreadyRunningException e) {
throw new UnloggedFailure("Failed to start reindexer: " + e.getMessage());
throw die("Failed to start reindexer: " + e.getMessage());
}
}
}

View File

@@ -49,7 +49,7 @@ public class ListGroupsCommand extends SshCommand {
@Override
public void run() throws Exception {
if (impl.getUser() != null && !impl.getProjects().isEmpty()) {
throw new UnloggedFailure(1, "fatal: --user and --project options are not compatible.");
throw die("--user and --project options are not compatible.");
}
impl.display(stdout);
}

View File

@@ -34,10 +34,10 @@ final class ListProjectsCommand extends SshCommand {
if (!impl.getFormat().isJson()) {
List<String> showBranch = impl.getShowBranch();
if (impl.isShowTree() && (showBranch != null) && !showBranch.isEmpty()) {
throw new UnloggedFailure(1, "fatal: --tree and --show-branch options are not compatible.");
throw die("--tree and --show-branch options are not compatible.");
}
if (impl.isShowTree() && impl.isShowDescription()) {
throw new UnloggedFailure(1, "fatal: --tree and --description options are not compatible.");
throw die("--tree and --description options are not compatible.");
}
}
impl.display(out);

View File

@@ -109,11 +109,10 @@ public class LsUserRefs extends SshCommand {
+ projectControl.getProject().getNameKey(), e);
}
} catch (RepositoryNotFoundException e) {
throw new UnloggedFailure("fatal: '"
+ projectControl.getProject().getNameKey() + "': not a git archive");
throw die("'" + projectControl.getProject().getNameKey()
+ "': not a git archive");
} catch (IOException e) {
throw new UnloggedFailure("fatal: Error opening: '"
+ projectControl.getProject().getNameKey());
throw die("Error opening: '" + projectControl.getProject().getNameKey());
}
}
}

View File

@@ -102,7 +102,7 @@ class Query extends SshCommand {
super.parseCommandLine();
if (processor.getIncludeFiles() &&
!(processor.getIncludePatchSets() || processor.getIncludeCurrentPatchSet())) {
throw new UnloggedFailure(1, "--files option needs --patch-sets or --current-patch-set");
throw die("--files option needs --patch-sets or --current-patch-set");
}
}

View File

@@ -83,7 +83,7 @@ final class Receive extends AbstractGitCommand {
Capable r = receive.canUpload();
if (r != Capable.OK) {
throw new UnloggedFailure(1, "\nfatal: " + r.getMessage());
throw die(r.getMessage());
}
verifyProjectVisible("reviewer", reviewerId);
@@ -165,7 +165,7 @@ final class Receive extends AbstractGitCommand {
for (final Account.Id id : who) {
final IdentifiedUser user = identifiedUserFactory.create(id);
if (!projectControl.forUser(user).isVisible()) {
throw new UnloggedFailure(1, type + " "
throw die(type + " "
+ user.getAccount().getFullName() + " cannot access the project");
}
}

View File

@@ -153,68 +153,68 @@ public class ReviewCommand extends SshCommand {
protected void run() throws UnloggedFailure {
if (abandonChange) {
if (restoreChange) {
throw error("abandon and restore actions are mutually exclusive");
throw die("abandon and restore actions are mutually exclusive");
}
if (submitChange) {
throw error("abandon and submit actions are mutually exclusive");
throw die("abandon and submit actions are mutually exclusive");
}
if (publishPatchSet) {
throw error("abandon and publish actions are mutually exclusive");
throw die("abandon and publish actions are mutually exclusive");
}
if (deleteDraftPatchSet) {
throw error("abandon and delete actions are mutually exclusive");
throw die("abandon and delete actions are mutually exclusive");
}
if (rebaseChange) {
throw error("abandon and rebase actions are mutually exclusive");
throw die("abandon and rebase actions are mutually exclusive");
}
}
if (publishPatchSet) {
if (restoreChange) {
throw error("publish and restore actions are mutually exclusive");
throw die("publish and restore actions are mutually exclusive");
}
if (submitChange) {
throw error("publish and submit actions are mutually exclusive");
throw die("publish and submit actions are mutually exclusive");
}
if (deleteDraftPatchSet) {
throw error("publish and delete actions are mutually exclusive");
throw die("publish and delete actions are mutually exclusive");
}
}
if (json) {
if (restoreChange) {
throw error("json and restore actions are mutually exclusive");
throw die("json and restore actions are mutually exclusive");
}
if (submitChange) {
throw error("json and submit actions are mutually exclusive");
throw die("json and submit actions are mutually exclusive");
}
if (deleteDraftPatchSet) {
throw error("json and delete actions are mutually exclusive");
throw die("json and delete actions are mutually exclusive");
}
if (publishPatchSet) {
throw error("json and publish actions are mutually exclusive");
throw die("json and publish actions are mutually exclusive");
}
if (abandonChange) {
throw error("json and abandon actions are mutually exclusive");
throw die("json and abandon actions are mutually exclusive");
}
if (changeComment != null) {
throw error("json and message are mutually exclusive");
throw die("json and message are mutually exclusive");
}
if (rebaseChange) {
throw error("json and rebase actions are mutually exclusive");
throw die("json and rebase actions are mutually exclusive");
}
if (changeTag != null) {
throw error("json and tag actions are mutually exclusive");
throw die("json and tag actions are mutually exclusive");
}
}
if (rebaseChange) {
if (deleteDraftPatchSet) {
throw error("rebase and delete actions are mutually exclusive");
throw die("rebase and delete actions are mutually exclusive");
}
if (submitChange) {
throw error("rebase and submit actions are mutually exclusive");
throw die("rebase and submit actions are mutually exclusive");
}
}
if (deleteDraftPatchSet && submitChange) {
throw error("delete and submit actions are mutually exclusive");
throw die("delete and submit actions are mutually exclusive");
}
boolean ok = true;
@@ -232,20 +232,21 @@ public class ReviewCommand extends SshCommand {
}
} catch (RestApiException | UnloggedFailure e) {
ok = false;
writeError("error: " + e.getMessage() + "\n");
writeError("error", e.getMessage() + "\n");
} catch (NoSuchChangeException e) {
ok = false;
writeError("no such change " + patchSet.getId().getParentKey().get());
writeError("error",
"no such change " + patchSet.getId().getParentKey().get());
} catch (Exception e) {
ok = false;
writeError("fatal: internal server error while reviewing "
writeError("fatal", "internal server error while reviewing "
+ patchSet.getId() + "\n");
log.error("internal error while reviewing " + patchSet.getId(), e);
}
}
if (!ok) {
throw error("one or more reviews failed; review output above");
throw die("one or more reviews failed; review output above");
}
}
@@ -262,8 +263,8 @@ public class ReviewCommand extends SshCommand {
return OutputFormat.JSON.newGson().
fromJson(CharStreams.toString(r), ReviewInput.class);
} catch (IOException | JsonSyntaxException e) {
writeError(e.getMessage() + '\n');
throw error("internal error while reading review input");
writeError("error", e.getMessage() + '\n');
throw die("internal error while reading review input");
}
}
@@ -321,7 +322,7 @@ public class ReviewCommand extends SshCommand {
revisionApi(patchSet).delete();
}
} catch (IllegalStateException | RestApiException e) {
throw error(e.getMessage());
throw die(e);
}
}
@@ -342,7 +343,7 @@ public class ReviewCommand extends SshCommand {
try {
allProjectsControl = projectControlFactory.controlFor(allProjects);
} catch (NoSuchProjectException e) {
throw new UnloggedFailure("missing " + allProjects.get());
throw die("missing " + allProjects.get());
}
for (LabelType type : allProjectsControl.getLabelTypes().getLabelTypes()) {
@@ -360,16 +361,4 @@ public class ReviewCommand extends SshCommand {
super.parseCommandLine();
}
private void writeError(final String msg) {
try {
err.write(msg.getBytes(ENC));
} catch (IOException e) {
// Ignored
}
}
private static UnloggedFailure error(final String msg) {
return new UnloggedFailure(1, msg);
}
}

View File

@@ -145,16 +145,14 @@ final class SetAccountCommand extends SshCommand {
private void validate() throws UnloggedFailure {
if (active && inactive) {
throw new UnloggedFailure(1,
"--active and --inactive options are mutually exclusive.");
throw die("--active and --inactive options are mutually exclusive.");
}
if (clearHttpPassword && !Strings.isNullOrEmpty(httpPassword)) {
throw new UnloggedFailure(1,
"--http-password and --clear-http-password options are mutually " +
"exclusive.");
throw die("--http-password and --clear-http-password options are "
+ "mutually exclusive.");
}
if (addSshKeys.contains("-") && deleteSshKeys.contains("-")) {
throw new UnloggedFailure(1, "Only one option may use the stdin");
throw die("Only one option may use the stdin");
}
if (deleteSshKeys.contains("ALL")) {
deleteSshKeys = Collections.singletonList("ALL");
@@ -163,8 +161,7 @@ final class SetAccountCommand extends SshCommand {
deleteEmails = Collections.singletonList("ALL");
}
if (deleteEmails.contains(preferredEmail)) {
throw new UnloggedFailure(1,
"--preferred-email and --delete-email options are mutually " +
throw die("--preferred-email and --delete-email options are mutually " +
"exclusive for the same email address.");
}
}

View File

@@ -49,7 +49,7 @@ public class SetHeadCommand extends SshCommand {
try {
setHead.apply(new ProjectResource(project), input);
} catch (UnprocessableEntityException e) {
throw new UnloggedFailure("fatal: " + e.getMessage());
throw die(e);
}
}
}

View File

@@ -155,7 +155,7 @@ final class SetProjectCommand extends SshCommand {
md.setMessage("Project settings updated");
config.commit(md);
} catch (RepositoryNotFoundException notFound) {
err.append("error: Project ").append(name).append(" not found\n");
err.append("Project ").append(name).append(" not found\n");
} catch (IOException | ConfigInvalidException e) {
final String msg = "Cannot update project " + name;
log.error(msg, e);
@@ -167,7 +167,7 @@ final class SetProjectCommand extends SshCommand {
while (err.charAt(err.length() - 1) == '\n') {
err.setLength(err.length() - 1);
}
throw new UnloggedFailure(1, err.toString());
throw die(err.toString());
}
}
}

View File

@@ -39,7 +39,6 @@ import org.kohsuke.args4j.Option;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.LinkedHashMap;
@@ -63,7 +62,7 @@ public class SetReviewersCommand extends SshCommand {
toRemove.add(who);
}
@Argument(index = 0, required = true, multiValued = true, metaVar = "COMMIT", usage = "changes to modify")
@Argument(index = 0, required = true, multiValued = true, metaVar = "CHANGE", usage = "changes to modify")
void addChange(String token) {
try {
addChangeImpl(token);
@@ -113,7 +112,7 @@ public class SetReviewersCommand extends SshCommand {
}
if (!ok) {
throw error("fatal: one or more updates failed; review output above");
throw die("one or more updates failed; review output above");
}
}
@@ -171,7 +170,7 @@ public class SetReviewersCommand extends SshCommand {
}
switch (toAdd.size()) {
case 0:
throw error("\"" + id + "\" no such change");
throw die("\"" + id + "\" no such change");
case 1:
ChangeControl ctl = toAdd.get(0);
@@ -179,7 +178,7 @@ public class SetReviewersCommand extends SshCommand {
break;
default:
throw error("\"" + id + "\" matches multiple changes");
throw die("\"" + id + "\" matches multiple changes");
}
}
@@ -191,16 +190,4 @@ public class SetReviewersCommand extends SshCommand {
return true;
}
}
private void writeError(String type, String msg) {
try {
err.write((type + ": " + msg + "\n").getBytes(ENC));
} catch (IOException e) {
// Ignored
}
}
private static UnloggedFailure error(String msg) {
return new UnloggedFailure(1, msg);
}
}

View File

@@ -14,7 +14,6 @@
package com.google.gerrit.sshd.commands;
import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Supplier;
@@ -48,8 +47,7 @@ import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingQueue;
@RequiresCapability(GlobalCapability.STREAM_EVENTS)
@CommandMetaData(name = "stream-events", description = "Monitor events occurring in real time",
runsAt = MASTER)
@CommandMetaData(name = "stream-events", description = "Monitor events occurring in real time")
final class StreamEvents extends BaseCommand {
/** Maximum number of events that may be queued up for each connection. */
private static final int MAX_EVENTS = 128;