GEODE-3264: Refactoring MemberCommands

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MemberCommands.java → geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeMemberCommand.java

@@ -12,93 +12,40 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
+
 package org.apache.geode.management.internal.cli.commands;
 
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Set;
-import java.util.TreeSet;
 
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
 import org.apache.geode.cache.CacheClosedException;
-import org.apache.geode.cache.execute.FunctionInvocationTargetException;
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
-import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.CliUtil;
-import org.apache.geode.management.internal.cli.LogWrapper;
 import org.apache.geode.management.internal.cli.domain.CacheServerInfo;
 import org.apache.geode.management.internal.cli.domain.MemberInformation;
 import org.apache.geode.management.internal.cli.functions.GetMemberInformationFunction;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.result.CompositeResultData;
-import org.apache.geode.management.internal.cli.result.CompositeResultData.SectionResultData;
 import org.apache.geode.management.internal.cli.result.ResultBuilder;
-import org.apache.geode.management.internal.cli.result.TabularResultData;
 import org.apache.geode.management.internal.security.ResourceOperation;
-import org.apache.geode.security.ResourcePermission.Operation;
-import org.apache.geode.security.ResourcePermission.Resource;
-
-/**
- * @since GemFire 7.0
- */
-public class MemberCommands implements GfshCommand {
+import org.apache.geode.security.ResourcePermission;
 
+public class DescribeMemberCommand implements GfshCommand {
   private static final GetMemberInformationFunction getMemberInformation =
       new GetMemberInformationFunction();
 
-  @CliCommand(value = {CliStrings.LIST_MEMBER}, help = CliStrings.LIST_MEMBER__HELP)
-  @CliMetaData(shellOnly = false, relatedTopic = CliStrings.TOPIC_GEODE_SERVER)
-  @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
-  public Result listMember(@CliOption(key = {CliStrings.GROUP}, unspecifiedDefaultValue = "",
-      optionContext = ConverterHint.MEMBERGROUP,
-      help = CliStrings.LIST_MEMBER__GROUP__HELP) String group) {
-    Result result = null;
-
-    // TODO: Add the code for identifying the system services
-    try {
-      Set<DistributedMember> memberSet = new TreeSet<DistributedMember>();
-      InternalCache cache = getCache();
-
-      // default get all the members in the DS
-      if (group.isEmpty()) {
-        memberSet.addAll(CliUtil.getAllMembers(cache));
-      } else {
-        memberSet.addAll(cache.getDistributedSystem().getGroupMembers(group));
-      }
-
-      if (memberSet.isEmpty()) {
-        result = ResultBuilder.createInfoResult(CliStrings.LIST_MEMBER__MSG__NO_MEMBER_FOUND);
-      } else {
-        TabularResultData resultData = ResultBuilder.createTabularResultData();
-        Iterator<DistributedMember> memberIters = memberSet.iterator();
-        while (memberIters.hasNext()) {
-          DistributedMember member = memberIters.next();
-          resultData.accumulate("Name", member.getName());
-          resultData.accumulate("Id", member.getId());
-        }
-
-        result = ResultBuilder.buildResult(resultData);
-      }
-    } catch (Exception e) {
-
-      result = ResultBuilder
-          .createGemFireErrorResult("Could not fetch the list of members. " + e.getMessage());
-      LogWrapper.getInstance().warning(e.getMessage(), e);
-    }
-
-    return result;
-  }
-
   @CliCommand(value = {CliStrings.DESCRIBE_MEMBER}, help = CliStrings.DESCRIBE_MEMBER__HELP)
-  @CliMetaData(shellOnly = false, relatedTopic = CliStrings.TOPIC_GEODE_SERVER)
-  @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
+  @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_SERVER)
+  @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+      operation = ResourcePermission.Operation.READ)
   public Result describeMember(@CliOption(key = CliStrings.DESCRIBE_MEMBER__IDENTIFIER,
       optionContext = ConverterHint.ALL_MEMBER_IDNAME,
       help = CliStrings.DESCRIBE_MEMBER__IDENTIFIER__HELP,
@@ -110,12 +57,6 @@ public Result describeMember(@CliOption(key = CliStrings.DESCRIBE_MEMBER__IDENTI
           CliUtil.getDistributedMemberByNameOrId(memberNameOrId);
 
       if (memberToBeDescribed != null) {
-        // This information should be available through the MBeans too. We might not need
-        // the function.
-
-        // Yes, but then the command is subject to Mbean availability, which would be
-        // affected once MBean filters are used.
-
         ResultCollector<?, ?> rc =
             CliUtil.executeFunction(getMemberInformation, null, memberToBeDescribed);
 
@@ -132,7 +73,7 @@ public Result describeMember(@CliOption(key = CliStrings.DESCRIBE_MEMBER__IDENTI
           memberInformation.setHost(memberToBeDescribed.getHost());
           memberInformation.setProcessId("" + memberToBeDescribed.getProcessId());
 
-          SectionResultData section = crd.addSection();
+          CompositeResultData.SectionResultData section = crd.addSection();
           section.addData("Name", memberInformation.getName());
           section.addData("Id", memberInformation.getId());
           section.addData("Host", memberInformation.getHost());
@@ -154,7 +95,7 @@ public Result describeMember(@CliOption(key = CliStrings.DESCRIBE_MEMBER__IDENTI
           section.addData("Locators", memberInformation.getLocators());
 
           if (memberInformation.isServer()) {
-            SectionResultData clientServiceSection = crd.addSection();
+            CompositeResultData.SectionResultData clientServiceSection = crd.addSection();
             List<CacheServerInfo> csList = memberInformation.getCacheServeInfo();
 
             if (csList != null) {
@@ -183,14 +124,10 @@ public Result describeMember(@CliOption(key = CliStrings.DESCRIBE_MEMBER__IDENTI
         result = ResultBuilder.createInfoResult(CliStrings
             .format(CliStrings.DESCRIBE_MEMBER__MSG__NOT_FOUND, new Object[] {memberNameOrId}));
       }
-    } catch (CacheClosedException e) {
-
-    } catch (FunctionInvocationTargetException e) {
-      result = ResultBuilder.createGemFireErrorResult(e.getMessage());
+    } catch (CacheClosedException ignored) {
     } catch (Exception e) {
       result = ResultBuilder.createGemFireErrorResult(e.getMessage());
     }
     return result;
   }
-
 }

geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ListMemberCommand.java

@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You 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 org.apache.geode.management.internal.cli.commands;
+
+import java.util.Set;
+import java.util.TreeSet;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.LogWrapper;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.TabularResultData;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class ListMemberCommand implements GfshCommand {
+  @CliCommand(value = {CliStrings.LIST_MEMBER}, help = CliStrings.LIST_MEMBER__HELP)
+  @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_SERVER)
+  @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+      operation = ResourcePermission.Operation.READ)
+  public Result listMember(@CliOption(key = {CliStrings.GROUP}, unspecifiedDefaultValue = "",
+      optionContext = ConverterHint.MEMBERGROUP,
+      help = CliStrings.LIST_MEMBER__GROUP__HELP) String group) {
+    Result result;
+
+    // TODO: Add the code for identifying the system services
+    try {
+      Set<DistributedMember> memberSet = new TreeSet<>();
+      InternalCache cache = getCache();
+
+      // default get all the members in the DS
+      if (group.isEmpty()) {
+        memberSet.addAll(CliUtil.getAllMembers(cache));
+      } else {
+        memberSet.addAll(cache.getDistributedSystem().getGroupMembers(group));
+      }
+
+      if (memberSet.isEmpty()) {
+        result = ResultBuilder.createInfoResult(CliStrings.LIST_MEMBER__MSG__NO_MEMBER_FOUND);
+      } else {
+        TabularResultData resultData = ResultBuilder.createTabularResultData();
+        for (DistributedMember member : memberSet) {
+          resultData.accumulate("Name", member.getName());
+          resultData.accumulate("Id", member.getId());
+        }
+
+        result = ResultBuilder.buildResult(resultData);
+      }
+    } catch (Exception e) {
+      result = ResultBuilder
+          .createGemFireErrorResult("Could not fetch the list of members. " + e.getMessage());
+      LogWrapper.getInstance().warning(e.getMessage(), e);
+    }
+    return result;
+  }
+}

geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/MemberCommandsController.java

@@ -14,22 +14,23 @@
  */
 package org.apache.geode.management.internal.web.controllers;
 
-import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
-
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.ResponseBody;
 
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+
 /**
  * The MemberCommandsController class implements GemFire Management REST API web service endpoints
  * for the Gfsh Member Commands.
  * <p/>
  * 
- * @see org.apache.geode.management.internal.cli.commands.MemberCommands
+ * @see org.apache.geode.management.internal.cli.commands.ListMemberCommand
+ * @see org.apache.geode.management.internal.cli.commands.DescribeMemberCommand
  * @see org.apache.geode.management.internal.web.controllers.AbstractCommandsController
  * @see org.springframework.stereotype.Controller
  * @see org.springframework.web.bind.annotation.PathVariable
@@ -46,25 +47,12 @@ public class MemberCommandsController extends AbstractCommandsController {
 
   @RequestMapping(method = RequestMethod.GET, value = "/members")
   @ResponseBody
-  // public String listMembers(@RequestBody MultiValueMap<String, String> requestParameters) {
-  // public String listMembers(@RequestParam(value = "group", required = false) final String
-  // groupName,
-  // @RequestParam(value = "group", required = false) final String[] groupNames) {
   public String listMembers(
       @RequestParam(value = CliStrings.GROUP, required = false) final String groupName) {
     CommandStringBuilder command = new CommandStringBuilder(CliStrings.LIST_MEMBER);
-
-    // logger.info(String.format("Request Body: %1$s", requestParameters));
-    // logger.info(String.format("Request Parameter (group): %1$s", groupName));
-    // logger.info(String.format("Request Parameter (group) as array: %1$s",
-    // ArrayUtils.toString(groupNames)));
-
-    // final String groupName = requestParameters.getFirst("group");
-
     if (hasValue(groupName)) {
       command.addOption(CliStrings.GROUP, groupName);
     }
-
     return processCommand(command.toString());
   }
 
@@ -75,5 +63,4 @@ public String describeMember(@PathVariable("name") final String memberNameId) {
     command.addOption(CliStrings.DESCRIBE_MEMBER__IDENTIFIER, decode(memberNameId));
     return processCommand(command.toString());
   }
-
 }

geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java

@@ -216,7 +216,7 @@ private static void init() {
     // createTestCommand("stop locator --name=locator1", clusterManage);
     // createTestCommand("stop server --name=server1", clusterManage);
 
-    // MemberCommands
+    // DescribeMemberCommand, ListMemberCommand
     createTestCommand("describe member --name=server1", clusterRead);
     createTestCommand("list members", clusterRead);