From 0026dfe142f77e41e32e9c1add1944678295e5c7 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 22 Jan 2013 15:42:49 +0100 Subject: [PATCH] Refactor ListGroups: Move code to format output into command class Move the code to format the output for the SSH command into the SSH command class. This makes the ListGroups class which is also used to serve REST calls much cleaner. Change-Id: I876eb4f9d472dff2c44649c75a37535c67e90760 Signed-off-by: Edwin Kempin --- .../google/gerrit/server/group/GroupInfo.java | 14 +-- .../gerrit/server/group/ListGroups.java | 118 +++++------------- .../sshd/commands/ListGroupsCommand.java | 64 +++++++++- 3 files changed, 99 insertions(+), 97 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java index 8bb8baa6cb..6c297aa28e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupInfo.java @@ -21,15 +21,15 @@ import com.google.gerrit.server.util.Url; public class GroupInfo { final String kind = "gerritcodereview#group"; - String id; - String name; - String url; - Boolean visibleToAll; + public String id; + public String name; + public String url; + public Boolean visibleToAll; // These fields are only supplied for internal groups. - String description; - Integer groupId; - String ownerId; + public String description; + public Integer groupId; + public String ownerId; public GroupInfo(GroupDescription.Basic group) { id = Url.encode(group.getGroupUUID().get()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java index c2d30268d3..fb369b3957 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.group; import com.google.common.base.Objects; -import com.google.common.base.Strings; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gerrit.common.data.GroupDescriptions; @@ -31,23 +30,15 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.OutputFormat; import com.google.gerrit.server.account.AccountResource; import com.google.gerrit.server.account.GetGroups; -import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.VisibleGroups; -import com.google.gerrit.server.ioutil.ColumnFormatter; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.util.Url; -import com.google.gson.JsonElement; import com.google.gson.reflect.TypeToken; import com.google.inject.Inject; import com.google.inject.Provider; import org.kohsuke.args4j.Option; -import java.io.BufferedWriter; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.PrintWriter; -import java.io.UnsupportedEncodingException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -55,7 +46,6 @@ import java.util.Map; /** List groups visible to the calling user. */ public class ListGroups implements RestReadView { - private final GroupCache groupCache; private final VisibleGroups.Factory visibleGroupsFactory; private final IdentifiedUser.GenericFactory userFactory; private final Provider accountGetGroups; @@ -74,21 +64,13 @@ public class ListGroups implements RestReadView { usage = "user for which the groups should be listed") private Account.Id user; - @Option(name = "--verbose", aliases = {"-v"}, - usage = "verbose output format with tab-separated columns for the " + - "group name, UUID, description, owner group name, " + - "owner group UUID, and whether the group is visible to all") - private boolean verboseOutput; - @Option(name = "-m", metaVar = "MATCH", usage = "match group substring") private String matchSubstring; @Inject - protected ListGroups(final GroupCache groupCache, - final VisibleGroups.Factory visibleGroupsFactory, + protected ListGroups(final VisibleGroups.Factory visibleGroupsFactory, final IdentifiedUser.GenericFactory userFactory, Provider accountGetGroups) { - this.groupCache = groupCache; this.visibleGroupsFactory = visibleGroupsFactory; this.userFactory = userFactory; this.accountGetGroups = accountGetGroups; @@ -105,78 +87,38 @@ public class ListGroups implements RestReadView { @Override public Object apply(TopLevelResource resource) throws AuthException, BadRequestException, ResourceConflictException, Exception { - return display(null); + final Map output = Maps.newTreeMap(); + for (GroupInfo info : get()) { + output.put(Objects.firstNonNull( + info.name, + "Group " + Url.decode(info.id)), info); + info.name = null; + } + return OutputFormat.JSON.newGson().toJsonTree(output, + new TypeToken>() {}.getType()); } - public JsonElement display(OutputStream displayOutputStream) - throws NoSuchGroupException { - PrintWriter stdout = null; - if (displayOutputStream != null) { - try { - stdout = new PrintWriter(new BufferedWriter( - new OutputStreamWriter(displayOutputStream, "UTF-8"))); - } catch (UnsupportedEncodingException e) { - throw new RuntimeException("JVM lacks UTF-8 encoding", e); - } - } - - try { - List groups; - if (user != null) { - groups = accountGetGroups.get().apply( - new AccountResource(userFactory.create(user))); - } else { - VisibleGroups visibleGroups = visibleGroupsFactory.create(); - visibleGroups.setOnlyVisibleToAll(visibleToAll); - visibleGroups.setGroupType(groupType); - visibleGroups.setMatch(matchSubstring); - List groupList; - if (!projects.isEmpty()) { - groupList = visibleGroups.get(projects); - } else { - groupList = visibleGroups.get(); - } - groups = Lists.newArrayListWithCapacity(groupList.size()); - for (AccountGroup group : groupList) { - groups.add(new GroupInfo(GroupDescriptions.forAccountGroup(group))); - } - } - - if (stdout == null) { - final Map output = Maps.newTreeMap(); - for (GroupInfo info : groups) { - output.put(Objects.firstNonNull( - info.name, - "Group " + Url.decode(info.id)), info); - info.name = null; - } - return OutputFormat.JSON.newGson().toJsonTree(output, - new TypeToken>() {}.getType()); - } else { - final ColumnFormatter formatter = new ColumnFormatter(stdout, '\t'); - for (GroupInfo info : groups) { - formatter.addColumn(info.name); - if (verboseOutput) { - AccountGroup o = info.ownerId != null - ? groupCache.get(new AccountGroup.UUID(Url.decode(info.ownerId))) - : null; - - formatter.addColumn(Url.decode(info.id)); - formatter.addColumn(Strings.nullToEmpty(info.description)); - formatter.addColumn(o != null ? o.getName() : "n/a"); - formatter.addColumn(o != null ? o.getGroupUUID().get() : ""); - formatter.addColumn(Boolean.toString( - Objects.firstNonNull(info.visibleToAll, Boolean.FALSE))); - } - formatter.nextLine(); - } - formatter.finish(); - return null; - } - } finally { - if (stdout != null) { - stdout.flush(); + public List get() throws NoSuchGroupException { + List groups; + if (user != null) { + groups = accountGetGroups.get().apply( + new AccountResource(userFactory.create(user))); + } else { + VisibleGroups visibleGroups = visibleGroupsFactory.create(); + visibleGroups.setOnlyVisibleToAll(visibleToAll); + visibleGroups.setGroupType(groupType); + visibleGroups.setMatch(matchSubstring); + List groupList; + if (!projects.isEmpty()) { + groupList = visibleGroups.get(projects); + } else { + groupList = visibleGroups.get(); + } + groups = Lists.newArrayListWithCapacity(groupList.size()); + for (AccountGroup group : groupList) { + groups.add(new GroupInfo(GroupDescriptions.forAccountGroup(group))); } } + return groups; } } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java index 412ea7035e..015e0afc66 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java @@ -14,15 +14,30 @@ package com.google.gerrit.sshd.commands; +import com.google.common.base.Objects; +import com.google.common.base.Strings; +import com.google.gerrit.common.errors.NoSuchGroupException; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.GetGroups; +import com.google.gerrit.server.account.GroupCache; +import com.google.gerrit.server.account.VisibleGroups; +import com.google.gerrit.server.group.GroupInfo; import com.google.gerrit.server.group.ListGroups; +import com.google.gerrit.server.ioutil.ColumnFormatter; +import com.google.gerrit.server.util.Url; import com.google.gerrit.sshd.BaseCommand; import com.google.inject.Inject; +import com.google.inject.Provider; import org.apache.sshd.server.Environment; +import org.kohsuke.args4j.Option; + +import java.io.PrintWriter; public class ListGroupsCommand extends BaseCommand { @Inject - private ListGroups impl; + private MyListGroups impl; @Override public void start(final Environment env) { @@ -33,8 +48,53 @@ public class ListGroupsCommand extends BaseCommand { if (impl.getUser() != null && !impl.getProjects().isEmpty()) { throw new UnloggedFailure(1, "fatal: --user and --project options are not compatible."); } - impl.display(out); + final PrintWriter stdout = toPrintWriter(out); + try { + impl.display(stdout); + } finally { + stdout.flush(); + } } }); } + + private static class MyListGroups extends ListGroups { + @Option(name = "--verbose", aliases = {"-v"}, + usage = "verbose output format with tab-separated columns for the " + + "group name, UUID, description, owner group name, " + + "owner group UUID, and whether the group is visible to all") + private boolean verboseOutput; + + private final GroupCache groupCache; + + @Inject + MyListGroups(final VisibleGroups.Factory visibleGroupsFactory, + final IdentifiedUser.GenericFactory userFactory, + final Provider accountGetGroups, + final GroupCache groupCache) { + super(visibleGroupsFactory, userFactory, accountGetGroups); + this.groupCache = groupCache; + } + + void display(final PrintWriter out) throws NoSuchGroupException { + final ColumnFormatter formatter = new ColumnFormatter(out, '\t'); + for (final GroupInfo info : get()) { + formatter.addColumn(info.name); + if (verboseOutput) { + AccountGroup o = info.ownerId != null + ? groupCache.get(new AccountGroup.UUID(Url.decode(info.ownerId))) + : null; + + formatter.addColumn(Url.decode(info.id)); + formatter.addColumn(Strings.nullToEmpty(info.description)); + formatter.addColumn(o != null ? o.getName() : "n/a"); + formatter.addColumn(o != null ? o.getGroupUUID().get() : ""); + formatter.addColumn(Boolean.toString( + Objects.firstNonNull(info.visibleToAll, Boolean.FALSE))); + } + formatter.nextLine(); + } + formatter.finish(); + } + } }