opendev/gerrit
Code Issues Proposed changes

Merge changes Ic8052b4d,I41829d35,Ia2d112eb,Ieb95a0ed,I38b15303

Browse Source 
* changes:
  Refactor ls-projects code to gerrit-server
  Support parsing command line options from a map
  Allow command line parsing to happen on a different object
  Refactor display of --help/-h to be common
  Move parsing of --help/-h to CmdLineParser
This commit is contained in:
Shawn O. Pearce
2012-04-07 13:01:09 -07:00
committed by gerrit code review
parent cb64a4b208 1bc77620bd
commit 4b6f8c1ef2
13 changed files with 304 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/AbstractProgram.java
View File

@@ -34,9 +34,6 @@ public abstract class AbstractProgram {
@Option(name = "--show-stack-trace", usage = "display stack trace on failure")
protected boolean showStackTrace;
@Option(name = "--help", usage = "display this help text", aliases = {"-h"})
private boolean help;
private String getName() {
String n = getClass().getName();
int dot = n.lastIndexOf('.');
@@ -52,21 +49,15 @@ public abstract class AbstractProgram {
try {
clp.parseArgument(argv);
} catch (CmdLineException err) {
if (!help) {
if (!clp.wasHelpRequestedByOption()) {
System.err.println("fatal: " + err.getMessage());
return 1;
}
}
if (help) {
final StringWriter msg = new StringWriter();
msg.write(getName());
clp.printSingleLineUsage(msg, null);
msg.write('\n');
msg.write('\n');
clp.printUsage(msg, null);
msg.write('\n');
if (clp.wasHelpRequestedByOption()) {
StringWriter msg = new StringWriter();
clp.printDetailedUsage(getName(), msg);
System.err.println(msg.toString());
return 1;
}

6
gerrit-server/pom.xml
View File

@@ -121,6 +121,12 @@ limitations under the License.
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-util-cli</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-util-ssl</artifactId>

2
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@@ -57,6 +57,7 @@ import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.PermissionCollection;
import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectNode;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SectionSortCache;
import com.google.gerrit.server.tools.ToolsCatalog;
@@ -118,6 +119,7 @@ public class GerritGlobalModule extends FactoryModule {
factory(AccountInfoCacheFactory.Factory.class);
factory(CapabilityControl.Factory.class);
factory(GroupInfoCacheFactory.Factory.class);
factory(ProjectNode.Factory.class);
factory(ProjectState.Factory.class);
factory(MaterializedGroupMembership.Factory.class);
bind(PermissionCollection.Factory.class);

2
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java
View File

@@ -53,6 +53,7 @@ import com.google.gerrit.server.patch.PublishComments;
import com.google.gerrit.server.patch.RemoveReviewer;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.CreateProject;
import com.google.gerrit.server.project.ListProjects;
import com.google.gerrit.server.project.PerRequestProjectControlCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.SuggestParentCandidates;
@@ -71,6 +72,7 @@ public class GerritRequestModule extends FactoryModule {
bind(MetaDataUpdate.User.class).in(RequestScoped.class);
bind(AccountResolver.class);
bind(ChangeQueryRewriter.class);
bind(ListProjects.class);
bind(AnonymousUser.class).in(RequestScoped.class);
bind(PerRequestProjectControlCache.class).in(RequestScoped.class);

83
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjects.java → gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
View File

@@ -12,18 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.sshd.commands;
package com.google.gerrit.server.project;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.sshd.BaseCommand;
import com.google.gerrit.server.util.TreeFormatter;
import com.google.inject.Inject;
import org.apache.sshd.server.Environment;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
@@ -32,18 +28,23 @@ import org.kohsuke.args4j.Option;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.BufferedWriter;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.UnsupportedEncodingException;
import java.util.Arrays;
import java.util.List;
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
final class ListProjects extends BaseCommand {
/** List projects visible to the calling user. */
public class ListProjects {
private static final Logger log = LoggerFactory.getLogger(ListProjects.class);
static enum FilterType {
public static enum FilterType {
CODE {
@Override
boolean matches(Repository git) throws IOException {
@@ -69,24 +70,18 @@ final class ListProjects extends BaseCommand {
abstract boolean matches(Repository git) throws IOException;
}
@Inject
private IdentifiedUser currentUser;
@Inject
private ProjectCache projectCache;
@Inject
private GitRepositoryManager repoManager;
@Inject
private ProjectNode.Factory projectNodeFactory;
private final CurrentUser currentUser;
private final ProjectCache projectCache;
private final GitRepositoryManager repoManager;
private final ProjectNode.Factory projectNodeFactory;
@Option(name = "--show-branch", aliases = {"-b"}, multiValued = true,
usage = "displays the sha of each project in the specified branch")
private List<String> showBranch;
@Option(name = "--tree", aliases = {"-t"}, usage = "displays project inheritance in a tree-like format\n" +
"this option does not work together with the show-branch option")
@Option(name = "--tree", aliases = {"-t"}, usage =
"displays project inheritance in a tree-like format\n"
+ "this option does not work together with the show-branch option")
private boolean showTree;
@Option(name = "--type", usage = "type of project")
@@ -98,27 +93,37 @@ final class ListProjects extends BaseCommand {
@Option(name = "--all", usage = "display all projects that are accessible by the calling user")
private boolean all;
@Override
public void start(final Environment env) {
startThread(new CommandRunnable() {
@Override
public void run() throws Exception {
parseCommandLine();
ListProjects.this.display();
}
});
@Inject
protected ListProjects(CurrentUser currentUser, ProjectCache projectCache,
GitRepositoryManager repoManager,
ProjectNode.Factory projectNodeFactory) {
this.currentUser = currentUser;
this.projectCache = projectCache;
this.repoManager = repoManager;
this.projectNodeFactory = projectNodeFactory;
}
private void display() throws Failure {
if (showTree && (showBranch != null)) {
throw new UnloggedFailure(1, "fatal: --tree and --show-branch options are not compatible.");
public List<String> getShowBranch() {
return showBranch;
}
public boolean isShowTree() {
return showTree;
}
public boolean isShowDescription() {
return showDescription;
}
public void display(OutputStream out) {
final PrintWriter stdout;
try {
stdout = new PrintWriter(new BufferedWriter(new OutputStreamWriter(out, "UTF-8")));
} catch (UnsupportedEncodingException e) {
// Our encoding is required by the specifications for the runtime.
throw new RuntimeException("JVM lacks UTF-8 encoding", e);
}
if (showTree && showDescription) {
throw new UnloggedFailure(1, "fatal: --tree and --description options are not compatible.");
}
final PrintWriter stdout = toPrintWriter(out);
final TreeMap<Project.NameKey, ProjectNode> treeMap =
new TreeMap<Project.NameKey, ProjectNode>();
try {

9
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ProjectNode.java → gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectNode.java
View File

@@ -12,32 +12,31 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.sshd.commands;
package com.google.gerrit.server.project;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.sshd.commands.TreeFormatter.TreeNode;
import com.google.gerrit.server.util.TreeFormatter.TreeNode;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.SortedSet;
import java.util.TreeSet;
/** Node of a Project in a tree formatted by {@link ListProjects}. */
public class ProjectNode implements TreeNode, Comparable<ProjectNode> {
public interface Factory {
ProjectNode create(final Project project, final boolean isVisible);
}
private final AllProjectsName allProjectsName;
private final Project project;
private final boolean isVisible;
private final SortedSet<ProjectNode> children = new TreeSet<ProjectNode>();
@Inject
public ProjectNode(final AllProjectsName allProjectsName,
protected ProjectNode(final AllProjectsName allProjectsName,
@Assisted final Project project, @Assisted final boolean isVisible) {
this.allProjectsName = allProjectsName;
this.project = project;

2
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/TreeFormatter.java → gerrit-server/src/main/java/com/google/gerrit/server/util/TreeFormatter.java
View File

@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.sshd.commands;
package com.google.gerrit.server.util;
import java.io.PrintWriter;
import java.util.SortedSet;

40
gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java
View File

@@ -60,9 +60,6 @@ public abstract class BaseCommand implements Command {
static final int STATUS_NOT_FOUND = PRIVATE_STATUS | 2;
public static final int STATUS_NOT_ADMIN = PRIVATE_STATUS | 3;
@Option(name = "--help", usage = "display this help text", aliases = {"-h"})
private boolean help;
@SuppressWarnings("unused")
@Option(name = "--", usage = "end of options", handler = EndOfOptionsHandler.class)
private boolean endOfOptions;
@@ -155,28 +152,37 @@ public abstract class BaseCommand implements Command {
* @see Argument
*/
protected void parseCommandLine() throws UnloggedFailure {
final CmdLineParser clp = newCmdLineParser();
parseCommandLine(this);
}
/**
* Parses the command line argument, injecting parsed values into fields.
* <p>
* This method must be explicitly invoked to cause a parse.
*
* @param options object whose fields declare Option and Argument annotations
* to describe the parameters of the command. Usually {@code this}.
* @throws UnloggedFailure if the command line arguments were invalid.
* @see Option
* @see Argument
*/
protected void parseCommandLine(Object options) throws UnloggedFailure {
final CmdLineParser clp = newCmdLineParser(options);
try {
clp.parseArgument(argv);
} catch (IllegalArgumentException err) {
if (!help) {
if (!clp.wasHelpRequestedByOption()) {
throw new UnloggedFailure(1, "fatal: " + err.getMessage());
}
} catch (CmdLineException err) {
if (!help) {
if (!clp.wasHelpRequestedByOption()) {
throw new UnloggedFailure(1, "fatal: " + err.getMessage());
}
}
if (help) {
final StringWriter msg = new StringWriter();
msg.write(commandName);
clp.printSingleLineUsage(msg, null);
msg.write('\n');
msg.write('\n');
clp.printUsage(msg, null);
msg.write('\n');
if (clp.wasHelpRequestedByOption()) {
StringWriter msg = new StringWriter();
clp.printDetailedUsage(commandName, msg);
msg.write(usage());
throw new UnloggedFailure(1, msg.toString());
}
@@ -187,8 +193,8 @@ public abstract class BaseCommand implements Command {
}
/** Construct a new parser for this command's received command line. */
protected CmdLineParser newCmdLineParser() {
return cmdLineParserFactory.create(this);
protected CmdLineParser newCmdLineParser(Object options) {
return cmdLineParserFactory.create(options);
}
/**

2
gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshModule.java
View File

@@ -40,7 +40,6 @@ import com.google.gerrit.sshd.args4j.PatchSetIdHandler;
import com.google.gerrit.sshd.args4j.ProjectControlHandler;
import com.google.gerrit.sshd.args4j.SocketAddressHandler;
import com.google.gerrit.sshd.commands.DefaultCommandModule;
import com.google.gerrit.sshd.commands.ProjectNode;
import com.google.gerrit.sshd.commands.QueryShell;
import com.google.gerrit.util.cli.CmdLineParser;
import com.google.gerrit.util.cli.OptionHandlerUtil;
@@ -79,7 +78,6 @@ public class SshModule extends FactoryModule {
bind(QueueProvider.class).to(CommandExecutorQueueProvider.class).in(SINGLETON);
bind(AccountManager.class);
factory(ChangeUserName.Factory.class);
factory(ProjectNode.Factory.class);
bind(PublickeyAuthenticator.class).to(DatabasePubKeyAuth.class);
bind(KeyPairProvider.class).toProvider(HostKeyProvider.class).in(SINGLETON);

2
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/DefaultCommandModule.java
View File

@@ -36,7 +36,7 @@ public class DefaultCommandModule extends CommandModule {
command(gerrit).toProvider(new DispatchCommandProvider(gerrit));
command(gerrit, "flush-caches").to(FlushCaches.class);
command(gerrit, "ls-projects").to(ListProjects.class);
command(gerrit, "ls-projects").to(ListProjectsCommand.class);
command(gerrit, "ls-groups").to(ListGroupsCommand.class);
command(gerrit, "query").to(Query.class);
command(gerrit, "show-caches").to(ShowCaches.class);

43
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListProjectsCommand.java Normal file
View File

@@ -0,0 +1,43 @@
// Copyright (C) 2009 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.sshd.commands;
import com.google.gerrit.server.project.ListProjects;
import com.google.gerrit.sshd.BaseCommand;
import com.google.inject.Inject;
import org.apache.sshd.server.Environment;
final class ListProjectsCommand extends BaseCommand {
@Inject
private ListProjects impl;
@Override
public void start(final Environment env) {
startThread(new CommandRunnable() {
@Override
public void run() throws Exception {
parseCommandLine(impl);
if (impl.isShowTree() && (impl.getShowBranch() != null)) {
throw new UnloggedFailure(1, "fatal: --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.");
}
impl.display(out);
}
});
}
}

4
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
View File

@@ -57,8 +57,8 @@ public class ReviewCommand extends BaseCommand {
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);
}

176
gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java
View File

@@ -42,13 +42,19 @@ import com.google.inject.assistedinject.Assisted;
import org.kohsuke.args4j.Argument;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.IllegalAnnotationError;
import org.kohsuke.args4j.NamedOptionDef;
import org.kohsuke.args4j.Option;
import org.kohsuke.args4j.OptionDef;
import org.kohsuke.args4j.spi.BooleanOptionHandler;
import org.kohsuke.args4j.spi.OptionHandler;
import org.kohsuke.args4j.spi.Setter;
import java.io.StringWriter;
import java.io.Writer;
import java.lang.annotation.Annotation;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.ResourceBundle;
/**
@@ -60,6 +66,7 @@ import java.util.ResourceBundle;
* args4j style format prior to invoking args4j for parsing.
*/
public class CmdLineParser {
public interface Factory {
CmdLineParser create(Object bean);
}
@@ -102,6 +109,19 @@ public class CmdLineParser {
parser.printUsage(out, rb);
}
public void printDetailedUsage(String name, StringWriter out) {
out.write(name);
printSingleLineUsage(out, null);
out.write('\n');
out.write('\n');
printUsage(out, null);
out.write('\n');
}
public boolean wasHelpRequestedByOption() {
return parser.help.value;
}
public void parseArgument(final String... args) throws CmdLineException {
final ArrayList<String> tmp = new ArrayList<String>(args.length);
for (int argi = 0; argi < args.length; argi++) {
@@ -123,13 +143,86 @@ public class CmdLineParser {
tmp.add(str);
}
parser.parseArgument(tmp.toArray(new String[tmp.size()]));
}
public void parseOptionMap(Map<String, String[]> parameters)
throws CmdLineException {
ArrayList<String> tmp = new ArrayList<String>();
for (Map.Entry<String, String[]> ent : parameters.entrySet()) {
String name = ent.getKey();
if (!name.startsWith("-")) {
if (name.length() == 1) {
name = "-" + name;
} else {
name = "--" + name;
}
}
if (findHandler(name) instanceof BooleanOptionHandler) {
boolean on = false;
for (String value : ent.getValue()) {
on = toBoolean(ent.getKey(), value);
}
if (on) {
tmp.add(name);
}
} else {
for (String value : ent.getValue()) {
tmp.add(name);
tmp.add(value);
}
}
}
parser.parseArgument(tmp.toArray(new String[tmp.size()]));
}
@SuppressWarnings("rawtypes")
private OptionHandler findHandler(String name) {
for (OptionHandler handler : parser.options) {
if (handler.option instanceof NamedOptionDef) {
NamedOptionDef def = (NamedOptionDef) handler.option;
if (name.equals(def.name())) {
return handler;
}
for (String alias : def.aliases()) {
if (name.equals(alias)) {
return handler;
}
}
}
}
return null;
}
private boolean toBoolean(String name, String value) throws CmdLineException {
if ("true".equals(value) || "t".equals(value)
|| "yes".equals(value) || "y".equals(value)
|| "on".equals(value)
|| "1".equals(value)
|| value == null || "".equals(value)) {
return true;
}
if ("false".equals(value) || "f".equals(value)
|| "no".equals(value) || "n".equals(value)
|| "off".equals(value)
|| "0".equals(value)) {
return false;
}
throw new CmdLineException(parser, String.format(
"invalid boolean \"%s=%s\"", name, value));
}
private class MyParser extends org.kohsuke.args4j.CmdLineParser {
@SuppressWarnings("rawtypes")
private List<OptionHandler> options;
private HelpOption help;
MyParser(final Object bean) {
super(bean);
ensureOptionsInitialized();
}
@SuppressWarnings({"unchecked", "rawtypes"})
@@ -137,7 +230,7 @@ public class CmdLineParser {
protected OptionHandler createOptionHandler(final OptionDef option,
final Setter setter) {
if (isHandlerSpecified(option) || isEnum(setter) || isPrimitive(setter)) {
return super.createOptionHandler(option, setter);
return add(super.createOptionHandler(option, setter));
}
final Key<OptionHandlerFactory<?>> key =
@@ -145,12 +238,28 @@ public class CmdLineParser {
Injector i = injector;
while (i != null) {
if (i.getBindings().containsKey(key)) {
return i.getInstance(key).create(this, option, setter);
return add(i.getInstance(key).create(this, option, setter));
}
i = i.getParent();
}
return super.createOptionHandler(option, setter);
return add(super.createOptionHandler(option, setter));
}
@SuppressWarnings("rawtypes")
private OptionHandler add(OptionHandler handler) {
ensureOptionsInitialized();
options.add(handler);
return handler;
}
@SuppressWarnings("rawtypes")
private void ensureOptionsInitialized() {
if (options == null) {
help = new HelpOption();
options = new ArrayList<OptionHandler>();
addOption(help, help);
}
}
private boolean isHandlerSpecified(final OptionDef option) {
@@ -165,4 +274,63 @@ public class CmdLineParser {
return setter.getType().isPrimitive();
}
}
private static class HelpOption implements Option, Setter<Boolean> {
private boolean value;
@Override
public String name() {
return "--help";
}
@Override
public String[] aliases() {
return new String[] {"-h"};
}
@Override
public String usage() {
return "display this help text";
}
@Override
public void addValue(Boolean val) {
value = val;
}
@Override
public Class<? extends OptionHandler<Boolean>> handler() {
return BooleanOptionHandler.class;
}
@Override
public String metaVar() {
return "";
}
@Override
public boolean multiValued() {
return false;
}
@Override
public boolean required() {
return false;
}
@Override
public Class<? extends Annotation> annotationType() {
return Option.class;
}
@Override
public Class<Boolean> getType() {
return Boolean.class;
}
@Override
public boolean isMultiValued() {
return multiValued();
}
}
}