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 <edwin.kempin@sap.com>

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;
+  public String id;
-  String name;
+  public String name;
-  String url;
+  public String url;
-  Boolean visibleToAll;
+  public Boolean visibleToAll;
 
   // These fields are only supplied for internal groups.
-  String description;
+  public String description;
-  Integer groupId;
+  public Integer groupId;
-  String ownerId;
+  public String ownerId;
 
   public GroupInfo(GroupDescription.Basic group) {
     id = Url.encode(group.getGroupUUID().get());

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<TopLevelResource> {
 
-  private final GroupCache groupCache;
   private final VisibleGroups.Factory visibleGroupsFactory;
   private final IdentifiedUser.GenericFactory userFactory;
   private final Provider<GetGroups> accountGetGroups;
@@ -74,21 +64,13 @@ public class ListGroups implements RestReadView<TopLevelResource> {
       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,
+  protected ListGroups(final VisibleGroups.Factory visibleGroupsFactory,
-      final VisibleGroups.Factory visibleGroupsFactory,
       final IdentifiedUser.GenericFactory userFactory,
       Provider<GetGroups> accountGetGroups) {
-    this.groupCache = groupCache;
     this.visibleGroupsFactory = visibleGroupsFactory;
     this.userFactory = userFactory;
     this.accountGetGroups = accountGetGroups;
@@ -105,78 +87,38 @@ public class ListGroups implements RestReadView<TopLevelResource> {
   @Override
   public Object apply(TopLevelResource resource) throws AuthException,
       BadRequestException, ResourceConflictException, Exception {
-    return display(null);
+    final Map<String, GroupInfo> 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<Map<String, GroupInfo>>() {}.getType());
   }
 
-  public JsonElement display(OutputStream displayOutputStream)
+  public List<GroupInfo> get() throws NoSuchGroupException {
-      throws NoSuchGroupException {
+    List<GroupInfo> groups;
-    PrintWriter stdout = null;
+    if (user != null) {
-    if (displayOutputStream != null) {
+      groups = accountGetGroups.get().apply(
-      try {
+          new AccountResource(userFactory.create(user)));
-        stdout = new PrintWriter(new BufferedWriter(
+    } else {
-            new OutputStreamWriter(displayOutputStream, "UTF-8")));
+      VisibleGroups visibleGroups = visibleGroupsFactory.create();
-      } catch (UnsupportedEncodingException e) {
+      visibleGroups.setOnlyVisibleToAll(visibleToAll);
-        throw new RuntimeException("JVM lacks UTF-8 encoding", e);
+      visibleGroups.setGroupType(groupType);
-      }
+      visibleGroups.setMatch(matchSubstring);
-    }
+      List<AccountGroup> groupList;
-
+      if (!projects.isEmpty()) {
-    try {
+        groupList = visibleGroups.get(projects);
-      List<GroupInfo> groups;
+      } else {
-      if (user != null) {
+        groupList = visibleGroups.get();
-        groups = accountGetGroups.get().apply(
+      }
-            new AccountResource(userFactory.create(user)));
+      groups = Lists.newArrayListWithCapacity(groupList.size());
-      } else {
+      for (AccountGroup group : groupList) {
-        VisibleGroups visibleGroups = visibleGroupsFactory.create();
+        groups.add(new GroupInfo(GroupDescriptions.forAccountGroup(group)));
-        visibleGroups.setOnlyVisibleToAll(visibleToAll);
-        visibleGroups.setGroupType(groupType);
-        visibleGroups.setMatch(matchSubstring);
-        List<AccountGroup> 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<String, GroupInfo> 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<Map<String, GroupInfo>>() {}.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();
       }
     }
+    return groups;
   }
 }

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<GetGroups> 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();
+    }
+  }
 }