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>
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 8bb8baa..6c297aa 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 @@
 
 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 c2d3026..fb369b3 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.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 @@
 /** 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 @@
       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<GetGroups> accountGetGroups) {
-    this.groupCache = groupCache;
     this.visibleGroupsFactory = visibleGroupsFactory;
     this.userFactory = userFactory;
     this.accountGetGroups = accountGetGroups;
@@ -105,78 +87,38 @@
   @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)
-      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);
+  public List<GroupInfo> get() throws NoSuchGroupException {
+    List<GroupInfo> 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<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)));
       }
     }
-
-    try {
-      List<GroupInfo> 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<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;
   }
 }
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 412ea70..015e0af 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 @@
         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();
+    }
+  }
 }