You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2019/04/05 15:05:46 UTC

[geode] branch develop updated: GEODE-5971: refactor various commands to use ResultModel (#3399)

This is an automated email from the ASF dual-hosted git repository.

jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 291a2d3  GEODE-5971: refactor various commands to use ResultModel (#3399)
291a2d3 is described below

commit 291a2d3dedb5f12faf4e8e714438a7bff277c45b
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Fri Apr 5 08:05:32 2019 -0700

    GEODE-5971: refactor various commands to use ResultModel (#3399)
---
 .../internal/cli/commands/SetVariableCommand.java  |  20 +---
 .../internal/cli/commands/ShCommand.java           |  34 +++---
 .../internal/cli/commands/ShutdownCommand.java     | 119 ++++++++++-----------
 3 files changed, 75 insertions(+), 98 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/SetVariableCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/SetVariableCommand.java
index cb022a8..735a759 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/SetVariableCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/SetVariableCommand.java
@@ -19,29 +19,19 @@ import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
 import org.apache.geode.management.cli.CliMetaData;
-import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.management.internal.cli.result.ErrorResultData;
-import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 
 public class SetVariableCommand extends OfflineGfshCommand {
   @CliCommand(value = {CliStrings.SET_VARIABLE}, help = CliStrings.SET_VARIABLE__HELP)
   @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GFSH})
-  public Result setVariable(
+  public ResultModel setVariable(
       @CliOption(key = CliStrings.SET_VARIABLE__VAR, mandatory = true,
           help = CliStrings.SET_VARIABLE__VAR__HELP) String var,
       @CliOption(key = CliStrings.SET_VARIABLE__VALUE, mandatory = true,
           help = CliStrings.SET_VARIABLE__VALUE__HELP) String value) {
-    Result result;
-    try {
-      getGfsh().setEnvProperty(var, String.valueOf(value));
-      result =
-          ResultBuilder.createInfoResult("Value for variable " + var + " is now: " + value + ".");
-    } catch (IllegalArgumentException e) {
-      ErrorResultData errorResultData = ResultBuilder.createErrorResultData();
-      errorResultData.addLine(e.getMessage());
-      result = ResultBuilder.buildResult(errorResultData);
-    }
-    return result;
+
+    getGfsh().setEnvProperty(var, String.valueOf(value));
+    return ResultModel.createInfo("Value for variable " + var + " is now: " + value + ".");
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShCommand.java
index aece9dd..172cba6 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShCommand.java
@@ -24,37 +24,32 @@ import org.springframework.shell.core.annotation.CliOption;
 
 import org.apache.geode.internal.lang.SystemUtils;
 import org.apache.geode.management.cli.CliMetaData;
-import org.apache.geode.management.cli.Result;
-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.InfoResultData;
-import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.model.InfoResultModel;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.management.internal.cli.shell.Gfsh;
 
-public class ShCommand extends InternalGfshCommand {
+public class ShCommand extends OfflineGfshCommand {
   @CliCommand(value = {CliStrings.SH}, help = CliStrings.SH__HELP)
   @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GFSH})
-  public Result sh(
+  public ResultModel sh(
       @CliOption(key = {"", CliStrings.SH__COMMAND}, mandatory = true,
           help = CliStrings.SH__COMMAND__HELP) String command,
       @CliOption(key = CliStrings.SH__USE_CONSOLE, specifiedDefaultValue = "true",
           unspecifiedDefaultValue = "false",
-          help = CliStrings.SH__USE_CONSOLE__HELP) boolean useConsole) {
-    Result result;
-    try {
-      result =
-          ResultBuilder.buildResult(executeCommand(Gfsh.getCurrentInstance(), command, useConsole));
-    } catch (IllegalStateException | IOException e) {
-      result = ResultBuilder.createUserErrorResult(e.getMessage());
-      LogWrapper.getInstance(getCache())
-          .warning("Unable to execute command \"" + command + "\". Reason:" + e.getMessage() + ".");
-    }
+          help = CliStrings.SH__USE_CONSOLE__HELP) boolean useConsole)
+      throws IOException {
+    ResultModel result = new ResultModel();
+    InfoResultModel info = result.getInfoSection("info");
+
+    executeCommand(info, Gfsh.getCurrentInstance(), command, useConsole);
+
     return result;
   }
 
-  private static InfoResultData executeCommand(Gfsh gfsh, String userCommand, boolean useConsole)
+  private void executeCommand(InfoResultModel result, Gfsh gfsh, String userCommand,
+      boolean useConsole)
       throws IOException {
-    InfoResultData infoResultData = ResultBuilder.createInfoResultData();
 
     String cmdToExecute = userCommand;
     String cmdExecutor = "/bin/sh";
@@ -77,7 +72,7 @@ public class ShCommand extends InternalGfshCommand {
 
     String lineRead;
     while ((lineRead = input.readLine()) != null) {
-      infoResultData.addLine(lineRead);
+      result.addLine(lineRead);
     }
 
     proc.getOutputStream().close();
@@ -90,6 +85,5 @@ public class ShCommand extends InternalGfshCommand {
       Thread.currentThread().interrupt();
       throw new IllegalStateException(e.getMessage(), e);
     }
-    return infoResultData;
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShutdownCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShutdownCommand.java
index 717d378..f574371 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShutdownCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShutdownCommand.java
@@ -37,16 +37,16 @@ import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.logging.LoggingExecutors;
 import org.apache.geode.management.cli.CliMetaData;
-import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.cli.GfshCommand;
 import org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.functions.ShutDownFunction;
 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.model.ResultModel;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission;
 
-public class ShutdownCommand extends InternalGfshCommand {
+public class ShutdownCommand extends GfshCommand {
   private static final String DEFAULT_TIME_OUT = "10";
   private static final Logger logger = LogService.getLogger();
 
@@ -55,78 +55,72 @@ public class ShutdownCommand extends InternalGfshCommand {
       interceptor = "org.apache.geode.management.internal.cli.commands.ShutdownCommand$ShutdownCommandInterceptor")
   @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
       operation = ResourcePermission.Operation.MANAGE)
-  public Result shutdown(
+  public ResultModel shutdown(
       @CliOption(key = CliStrings.SHUTDOWN__TIMEOUT, unspecifiedDefaultValue = DEFAULT_TIME_OUT,
           help = CliStrings.SHUTDOWN__TIMEOUT__HELP) int userSpecifiedTimeout,
       @CliOption(key = CliStrings.INCLUDE_LOCATORS, unspecifiedDefaultValue = "false",
           specifiedDefaultValue = "true",
-          help = CliStrings.INCLUDE_LOCATORS_HELP) boolean shutdownLocators) {
-    try {
-      if (userSpecifiedTimeout < Integer.parseInt(DEFAULT_TIME_OUT)) {
-        return ResultBuilder.createInfoResult(CliStrings.SHUTDOWN__MSG__IMPROPER_TIMEOUT);
-      }
+          help = CliStrings.INCLUDE_LOCATORS_HELP) boolean shutdownLocators)
+      throws InterruptedException, ExecutionException, TimeoutException {
 
-      // convert to milliseconds
-      long timeout = userSpecifiedTimeout * 1000L;
-      InternalCache cache = (InternalCache) getCache();
-      int numDataNodes = getAllNormalMembers().size();
-      Set<DistributedMember> locators = getAllMembers();
-      Set<DistributedMember> dataNodes = getAllNormalMembers();
-      locators.removeAll(dataNodes);
+    if (userSpecifiedTimeout < Integer.parseInt(DEFAULT_TIME_OUT)) {
+      return ResultModel.createInfo(CliStrings.SHUTDOWN__MSG__IMPROPER_TIMEOUT);
+    }
 
-      if (!shutdownLocators && numDataNodes == 0) {
-        return ResultBuilder.createInfoResult(CliStrings.SHUTDOWN__MSG__NO_DATA_NODE_FOUND);
-      }
+    // convert to milliseconds
+    long timeout = userSpecifiedTimeout * 1000L;
+    InternalCache cache = (InternalCache) getCache();
+    int numDataNodes = getAllNormalMembers().size();
+    Set<DistributedMember> locators = getAllMembers();
+    Set<DistributedMember> dataNodes = getAllNormalMembers();
+    locators.removeAll(dataNodes);
 
-      String managerName = cache.getJmxManagerAdvisor().getDistributionManager().getId().getId();
+    if (!shutdownLocators && numDataNodes == 0) {
+      return ResultModel.createInfo(CliStrings.SHUTDOWN__MSG__NO_DATA_NODE_FOUND);
+    }
 
-      final DistributedMember manager = getMember(managerName);
+    String managerName = cache.getJmxManagerAdvisor().getDistributionManager().getId().getId();
 
-      dataNodes.remove(manager);
+    final DistributedMember manager = getMember(managerName);
 
-      // shut down all data members excluding this manager if manager is a data node
-      long timeElapsed = shutDownNodeWithTimeOut(timeout, dataNodes);
-      timeout = timeout - timeElapsed;
+    dataNodes.remove(manager);
 
-      // shut down locators one by one
-      if (shutdownLocators) {
-        if (manager == null) {
-          return ResultBuilder.createUserErrorResult(CliStrings.SHUTDOWN__MSG__MANAGER_NOT_FOUND);
-        }
+    // shut down all data members excluding this manager if manager is a data node
+    long timeElapsed = shutDownNodeWithTimeOut(timeout, dataNodes);
+    timeout = timeout - timeElapsed;
 
-        // remove current locator as that would get shutdown last
-        if (locators.contains(manager)) {
-          locators.remove(manager);
-        }
+    // shut down locators one by one
+    if (shutdownLocators) {
+      if (manager == null) {
+        return ResultModel.createError(CliStrings.SHUTDOWN__MSG__MANAGER_NOT_FOUND);
+      }
 
-        for (DistributedMember locator : locators) {
-          Set<DistributedMember> lsSet = new HashSet<>();
-          lsSet.add(locator);
-          long elapsedTime = shutDownNodeWithTimeOut(timeout, lsSet);
-          timeout = timeout - elapsedTime;
-        }
+      // remove current locator as that would get shutdown last
+      if (locators.contains(manager)) {
+        locators.remove(manager);
       }
 
-      if (locators.contains(manager) && !shutdownLocators) { // This means manager is a locator and
-        // shutdownLocators is false. Hence we
-        // should not stop the manager
-        return ResultBuilder.createInfoResult("Shutdown is triggered");
+      for (DistributedMember locator : locators) {
+        Set<DistributedMember> lsSet = new HashSet<>();
+        lsSet.add(locator);
+        long elapsedTime = shutDownNodeWithTimeOut(timeout, lsSet);
+        timeout = timeout - elapsedTime;
       }
-      // now shut down this manager
-      Set<DistributedMember> mgrSet = new HashSet<>();
-      mgrSet.add(manager);
-      // No need to check further timeout as this is the last node we will be
-      // shutting down
-      shutDownNodeWithTimeOut(timeout, mgrSet);
-
-    } catch (TimeoutException tex) {
-      return ResultBuilder.createInfoResult(CliStrings.SHUTDOWN_TIMEDOUT);
-    } catch (Exception ex) {
-      logger.error(ex.getMessage(), ex);
-      return ResultBuilder.createUserErrorResult(ex.getMessage());
     }
-    // @TODO. List all the nodes which could be successfully shutdown
-    return ResultBuilder.createInfoResult("Shutdown is triggered");
+
+    if (locators.contains(manager) && !shutdownLocators) { // This means manager is a locator and
+      // shutdownLocators is false. Hence we
+      // should not stop the manager
+      return ResultModel.createInfo("Shutdown is triggered");
+    }
+    // now shut down this manager
+    Set<DistributedMember> mgrSet = new HashSet<>();
+    mgrSet.add(manager);
+    // No need to check further timeout as this is the last node we will be
+    // shutting down
+    shutDownNodeWithTimeOut(timeout, mgrSet);
+
+    return ResultModel.createInfo("Shutdown is triggered");
   }
 
   /**
@@ -185,19 +179,18 @@ public class ShutdownCommand extends InternalGfshCommand {
 
   public static class ShutdownCommandInterceptor extends AbstractCliAroundInterceptor {
     @Override
-    public Result preExecution(GfshParseResult parseResult) {
+    public ResultModel preExecution(GfshParseResult parseResult) {
 
       // This hook is for testing purpose only.
       if (Boolean.getBoolean(CliStrings.IGNORE_INTERCEPTORS)) {
-        return ResultBuilder.createInfoResult(CliStrings.SHUTDOWN__MSG__SHUTDOWN_ENTIRE_DS);
+        return ResultModel.createInfo(CliStrings.SHUTDOWN__MSG__SHUTDOWN_ENTIRE_DS);
       }
 
       Response response = readYesNo(CliStrings.SHUTDOWN__MSG__WARN_USER, Response.YES);
       if (response == Response.NO) {
-        return ResultBuilder
-            .createShellClientAbortOperationResult(CliStrings.SHUTDOWN__MSG__ABORTING_SHUTDOWN);
+        return ResultModel.createInfo(CliStrings.SHUTDOWN__MSG__ABORTING_SHUTDOWN);
       } else {
-        return ResultBuilder.createInfoResult(CliStrings.SHUTDOWN__MSG__SHUTDOWN_ENTIRE_DS);
+        return ResultModel.createInfo(CliStrings.SHUTDOWN__MSG__SHUTDOWN_ENTIRE_DS);
       }
     }
   }