geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jinmeil...@apache.org
Subject [geode] branch develop updated: GEODE-5971: refactor ExportLogsCommand and ExecuteScriptCommand to us… (#3285)
Date Wed, 13 Mar 2019 13:57:12 GMT
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 a8241d7  GEODE-5971: refactor ExportLogsCommand and ExecuteScriptCommand to us…
(#3285)
a8241d7 is described below

commit a8241d7676cab6c258cac4f4cc3c5705152f49ae
Author: jinmeiliao <jiliao@pivotal.io>
AuthorDate: Wed Mar 13 06:56:42 2019 -0700

    GEODE-5971: refactor ExportLogsCommand and ExecuteScriptCommand to us… (#3285)
---
 .../internal/cli/CliAroundInterceptor.java         |  3 +-
 .../cli/commands/AlterRuntimeConfigCommand.java    |  8 ++---
 .../cli/commands/ChangeLogLevelCommand.java        |  7 ++--
 .../cli/commands/ExecuteScriptCommand.java         | 16 +++------
 .../internal/cli/commands/ExportLogsCommand.java   | 26 +++++++--------
 .../cli/commands/ExportLogsInterceptor.java        | 38 ++++++++++++----------
 .../internal/cli/result/ModelCommandResult.java    |  8 ++---
 .../internal/cli/result/model/FileResultModel.java |  4 +++
 .../internal/cli/result/model/ResultModel.java     | 15 +++++++++
 .../geode/management/internal/cli/shell/Gfsh.java  |  8 ++---
 .../internal/cli/shell/ScriptExecutionDetails.java | 28 ++++++++--------
 .../cli/commands/ExportLogsCommandTest.java        |  8 ++---
 .../cli/commands/ExportLogsInterceptorTest.java    | 17 ++++++----
 .../cli/commands/LogLevelInterceptorTest.java      | 17 +++++-----
 14 files changed, 108 insertions(+), 95 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliAroundInterceptor.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliAroundInterceptor.java
index 5b3224d..61e2a84 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliAroundInterceptor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliAroundInterceptor.java
@@ -17,7 +17,6 @@ package org.apache.geode.management.internal.cli;
 import java.nio.file.Path;
 
 import org.apache.geode.management.internal.cli.result.CommandResult;
-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.cli.shell.GfshExecutionStrategy;
 
@@ -34,7 +33,7 @@ public interface CliAroundInterceptor {
    * called by the OperationInvoker before the command is executed
    */
   default Object preExecution(GfshParseResult parseResult) {
-    return ResultBuilder.createInfoResult("");
+    return ResultModel.createInfo("");
   }
 
   /**
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterRuntimeConfigCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterRuntimeConfigCommand.java
index a6117c8..aef0104 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterRuntimeConfigCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterRuntimeConfigCommand.java
@@ -36,7 +36,6 @@ import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.logging.log4j.LogLevel;
 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.AbstractCliAroundInterceptor;
 import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.cli.GfshParseResult;
@@ -44,7 +43,6 @@ import org.apache.geode.management.internal.cli.functions.AlterRuntimeConfigFunc
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.remote.CommandExecutor;
-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.configuration.domain.XmlEntity;
@@ -243,13 +241,13 @@ public class AlterRuntimeConfigCommand extends InternalGfshCommand {
 
   public static class AlterRuntimeInterceptor extends AbstractCliAroundInterceptor {
     @Override
-    public Result preExecution(GfshParseResult parseResult) {
+    public ResultModel preExecution(GfshParseResult parseResult) {
       // validate log level
       String logLevel = parseResult.getParamValueAsString("log-level");
       if (StringUtils.isNotBlank(logLevel) && (LogLevel.getLevel(logLevel) == null))
{
-        return ResultBuilder.createUserErrorResult("Invalid log level: " + logLevel);
+        return ResultModel.createError("Invalid log level: " + logLevel);
       }
-      return ResultBuilder.createInfoResult("");
+      return ResultModel.createInfo("");
     }
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ChangeLogLevelCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ChangeLogLevelCommand.java
index 1e0a9bc..3c7edb7 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ChangeLogLevelCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ChangeLogLevelCommand.java
@@ -34,7 +34,6 @@ import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.logging.log4j.LogLevel;
 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.AbstractCliAroundInterceptor;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.functions.ChangeLogLevelFunction;
@@ -148,14 +147,14 @@ public class ChangeLogLevelCommand extends InternalGfshCommand {
 
   public static class ChangeLogLevelCommandInterceptor extends AbstractCliAroundInterceptor
{
     @Override
-    public Result preExecution(GfshParseResult parseResult) {
+    public ResultModel preExecution(GfshParseResult parseResult) {
       // validate log level
       String logLevel = parseResult.getParamValueAsString("loglevel");
       if (StringUtils.isBlank(logLevel) || LogLevel.getLevel(logLevel) == null) {
-        return ResultBuilder.createUserErrorResult("Invalid log level: " + logLevel);
+        return ResultModel.createError("Invalid log level: " + logLevel);
       }
 
-      return ResultBuilder.createInfoResult("");
+      return ResultModel.createInfo("");
     }
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteScriptCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteScriptCommand.java
index 4d79934..df6ca22 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteScriptCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExecuteScriptCommand.java
@@ -22,15 +22,14 @@ import org.springframework.shell.core.annotation.CliOption;
 
 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.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.cli.shell.Gfsh;
 
-public class ExecuteScriptCommand extends InternalGfshCommand {
+public class ExecuteScriptCommand extends OfflineGfshCommand {
   @CliCommand(value = {CliStrings.RUN}, help = CliStrings.RUN__HELP)
   @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GFSH})
-  public Result executeScript(
+  public ResultModel executeScript(
       @CliOption(key = CliStrings.RUN__FILE, optionContext = ConverterHint.FILE, mandatory
= true,
           help = CliStrings.RUN__FILE__HELP) File file,
       @CliOption(key = {CliStrings.RUN__QUIET}, specifiedDefaultValue = "true",
@@ -38,15 +37,8 @@ public class ExecuteScriptCommand extends InternalGfshCommand {
       @CliOption(key = {CliStrings.RUN__CONTINUEONERROR}, specifiedDefaultValue = "true",
           unspecifiedDefaultValue = "false",
           help = CliStrings.RUN__CONTINUEONERROR__HELP) boolean continueOnError) {
-    Result result;
-
     Gfsh gfsh = Gfsh.getCurrentInstance();
-    try {
-      result = gfsh.executeScript(file, quiet, continueOnError);
-    } catch (IllegalArgumentException e) {
-      result = ResultBuilder.createShellClientErrorResult(e.getMessage());
-    } // let CommandProcessingException go to the caller
-    return result;
+    return gfsh.executeScript(file, quiet, continueOnError);
   }
 
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
index a01df08..f0939f4 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
@@ -38,18 +38,17 @@ import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.ManagementException;
 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.cli.GfshCommand;
 import org.apache.geode.management.internal.cli.functions.ExportLogsFunction;
 import org.apache.geode.management.internal.cli.functions.SizeExportLogsFunction;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.management.internal.cli.result.LegacyCommandResult;
-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.cli.util.ExportLogsCacheWriter;
 import org.apache.geode.management.internal.configuration.utils.ZipUtils;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission;
 
-public class ExportLogsCommand extends InternalGfshCommand {
+public class ExportLogsCommand extends GfshCommand {
 
   private static final Logger logger = LogService.getLogger();
 
@@ -66,7 +65,7 @@ public class ExportLogsCommand extends InternalGfshCommand {
       relatedTopic = {CliStrings.TOPIC_GEODE_SERVER, CliStrings.TOPIC_GEODE_DEBUG_UTIL})
   @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
       operation = ResourcePermission.Operation.READ)
-  public Result exportLogs(
+  public ResultModel exportLogs(
       @CliOption(key = CliStrings.EXPORT_LOGS__DIR,
           help = CliStrings.EXPORT_LOGS__DIR__HELP) String dirName,
       @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
@@ -100,7 +99,6 @@ public class ExportLogsCommand extends InternalGfshCommand {
       throws Exception {
 
     long totalEstimatedExportSize = 0;
-    Result result;
     InternalCache cache = (InternalCache) getCache();
     try {
       Set<DistributedMember> targetMembers = getMembersIncludingLocators(groups, memberIds);
@@ -121,14 +119,14 @@ public class ExportLogsCommand extends InternalGfshCommand {
               totalEstimatedExportSize += estimatedSize;
             } else if (results.get(0) instanceof ManagementException) {
               ManagementException exception = (ManagementException) results.get(0);
-              return ResultBuilder.createUserErrorResult(exception.getMessage());
+              return ResultModel.createError(exception.getMessage());
             }
           }
         }
 
         // first check if totalEstimate file size exceeds available disk space on locator
         if (totalEstimatedExportSize > getLocalDiskAvailable()) {
-          return ResultBuilder.createUserErrorResult(
+          return ResultModel.createError(
               "Estimated logs size will exceed the available disk space on the locator.");
         }
         // then check if total estimated file size exceeds user specified value
@@ -139,7 +137,7 @@ public class ExportLogsCommand extends InternalGfshCommand {
               .append(CliStrings.EXPORT_LOGS__FILESIZELIMIT).append(" = ")
               .append(userSpecifiedLimit).append(
                   ". To disable exported logs file size check use option \"--file-size-limit=0\".");
-          return ResultBuilder.createUserErrorResult(sb.toString());
+          return ResultModel.createError(sb.toString());
         }
       }
 
@@ -167,7 +165,7 @@ public class ExportLogsCommand extends InternalGfshCommand {
       }
 
       if (zipFilesFromMembers.isEmpty()) {
-        return ResultBuilder.createUserErrorResult("No files to be exported.");
+        return ResultModel.createError("No files to be exported.");
       }
 
       Path tempDir = Files.createTempDirectory("exportedLogs");
@@ -196,14 +194,12 @@ public class ExportLogsCommand extends InternalGfshCommand {
       ZipUtils.zipDirectory(exportedLogsDir, exportedLogsZipFile);
       FileUtils.deleteDirectory(tempDir.toFile());
 
-      result = new LegacyCommandResult(exportedLogsZipFile);
+      ResultModel result = new ResultModel();
+      result.setFileToDownload(exportedLogsZipFile);
+      return result;
     } finally {
       ExportLogsFunction.destroyExportLogsRegion(cache);
     }
-    if (logger.isDebugEnabled()) {
-      logger.debug("Exporting logs returning = {}", result);
-    }
-    return result;
   }
 
   /**
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
index 99d348a..d2218c3 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
@@ -30,8 +30,7 @@ import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.functions.ExportLogsFunction;
-import org.apache.geode.management.internal.cli.result.CommandResult;
-import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 
 /**
  * after the export logs, will need to copy the tempFile to the desired location and delete
the temp
@@ -41,18 +40,18 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor
{
   private static final Logger logger = LogService.getLogger();
 
   @Override
-  public Result preExecution(GfshParseResult parseResult) {
+  public ResultModel preExecution(GfshParseResult parseResult) {
 
     // validates groupId and memberIds not both set
     if (parseResult.getParamValueAsString("group") != null
         && parseResult.getParamValueAsString("member") != null) {
-      return ResultBuilder.createUserErrorResult("Can't specify both group and member.");
+      return ResultModel.createError("Can't specify both group and member.");
     }
 
     // validate log level
     String logLevel = parseResult.getParamValueAsString("log-level");
     if (StringUtils.isBlank(logLevel) || LogLevel.getLevel(logLevel) == null) {
-      return ResultBuilder.createUserErrorResult("Invalid log level: " + logLevel);
+      return ResultModel.createError("Invalid log level: " + logLevel);
     }
 
     // validate start date and end date
@@ -63,7 +62,7 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor
{
       LocalDateTime startTime = ExportLogsFunction.parseTime(start);
       LocalDateTime endTime = ExportLogsFunction.parseTime(end);
       if (startTime.isAfter(endTime)) {
-        return ResultBuilder.createUserErrorResult("start-time has to be earlier than end-time.");
+        return ResultModel.createError("start-time has to be earlier than end-time.");
       }
     }
 
@@ -71,17 +70,17 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor
{
     boolean onlyLogs = (boolean) parseResult.getParamValue("logs-only");
     boolean onlyStats = (boolean) parseResult.getParamValue("stats-only");
     if (onlyLogs && onlyStats) {
-      return ResultBuilder.createUserErrorResult("logs-only and stats-only can't both be
true");
+      return ResultModel.createError("logs-only and stats-only can't both be true");
     }
 
-    return ResultBuilder.createInfoResult("");
+    return ResultModel.createInfo("");
   }
 
   @Override
-  public CommandResult postExecution(GfshParseResult parseResult, CommandResult commandResult,
+  public ResultModel postExecution(GfshParseResult parseResult, ResultModel commandResult,
       Path tempFile) {
-    // in the command over http case, the command result is in the downloaded temp file
     if (tempFile != null) {
+      // in the command over http case, the command result is in the downloaded temp file
       Path dirPath;
       String dirName = parseResult.getParamValueAsString("dir");
       if (StringUtils.isBlank(dirName)) {
@@ -94,16 +93,21 @@ public class ExportLogsInterceptor extends AbstractCliAroundInterceptor
{
       try {
         FileUtils.copyFile(tempFile.toFile(), exportedLogFile);
         FileUtils.deleteQuietly(tempFile.toFile());
-        commandResult = (CommandResult) ResultBuilder
-            .createInfoResult("Logs exported to: " + exportedLogFile.getAbsolutePath());
+        return ResultModel.createInfo("Logs exported to: " + exportedLogFile.getAbsolutePath());
       } catch (IOException e) {
         logger.error(e.getMessage(), e);
-        commandResult = ResultBuilder.createGemFireErrorResult(e.getMessage());
+        return ResultModel.createError(e.getMessage());
       }
-    } else if (commandResult.getStatus() == Result.Status.OK) {
-      commandResult = (CommandResult) ResultBuilder.createInfoResult(
-          "Logs exported to the connected member's file system: " + commandResult.nextLine());
     }
-    return commandResult;
+
+    // otherwise if result status is false, return it
+    if (commandResult.getStatus() != Result.Status.OK) {
+      return commandResult;
+    }
+
+    // if there is no downloaded file. File is saved on the locator/manager.
+    return ResultModel.createInfo(
+        "Logs exported to the connected member's file system: "
+            + commandResult.getFileToDownload().toString());
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ModelCommandResult.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ModelCommandResult.java
index 3143d61..ffe5832 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ModelCommandResult.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/ModelCommandResult.java
@@ -62,12 +62,12 @@ public class ModelCommandResult implements CommandResult {
 
   @Override
   public Path getFileToDownload() {
-    return null;
+    return result.getFileToDownload();
   }
 
   @Override
   public boolean hasFileToDownload() {
-    return false;
+    return getFileToDownload() != null;
   }
 
   @Override
@@ -88,8 +88,8 @@ public class ModelCommandResult implements CommandResult {
     commandOutputIndex = 0;
   }
 
-  // ModelCommandResult should not handle saving files. File saving should be done by each
-  // command's postExecutor in the ResultModel
+  // ModelCommandResult should not handle saving files. File contents saved in FileResultModel's
+  // byte[] should be saved by each command's postExecutor in the ResultModel
   @Override
   public boolean hasIncomingFiles() {
     return false;
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/FileResultModel.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/FileResultModel.java
index 3b1c3d8..0999655 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/FileResultModel.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/FileResultModel.java
@@ -26,6 +26,10 @@ import org.apache.commons.io.FileUtils;
 import org.apache.geode.management.internal.cli.result.ResultData;
 import org.apache.geode.management.internal.cli.shell.Gfsh;
 
+/**
+ * this is used to save the content of the file in the byte[] and send the results
+ * back to the gfsh client.
+ */
 public class FileResultModel {
   public static final int FILE_TYPE_BINARY = 0;
   public static final int FILE_TYPE_TEXT = 1;
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/ResultModel.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/ResultModel.java
index 9f12d80..4cdb002 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/ResultModel.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/ResultModel.java
@@ -18,6 +18,7 @@ package org.apache.geode.management.internal.cli.result.model;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -75,8 +76,14 @@ public class ResultModel {
   private Map<String, AbstractResultModel> sections = new LinkedHashMap<>();
   private Result.Status status = Result.Status.OK;
   private Object configObject;
+  // this is used by commands (e.g. ExportConfigCommand) saving the file content in the memory
and
+  // transfer the byte[] in json string back to the client
   private Map<String, FileResultModel> files = new LinkedHashMap<>();
 
+  // this is used by commands (e.g ExportlogsCommand) that can download file to the client
when in
+  // http connection
+  private Path fileToDownload;
+
   @JsonIgnore
   public Object getConfigObject() {
     return configObject;
@@ -155,6 +162,14 @@ public class ResultModel {
     files.put(file.getName(), new FileResultModel(file, fileType));
   }
 
+  public Path getFileToDownload() {
+    return fileToDownload;
+  }
+
+  public void setFileToDownload(Path fileToDownload) {
+    this.fileToDownload = fileToDownload;
+  }
+
   /**
    * Overloaded method to create an {@code InfoResultModel} section called "info".
    */
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
index ac38304..6e6e2d8 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java
@@ -62,7 +62,7 @@ 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.CommandResult;
 import org.apache.geode.management.internal.cli.result.LegacyCommandResult;
-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.cli.shell.jline.ANSIHandler;
 import org.apache.geode.management.internal.cli.shell.jline.ANSIHandler.ANSIStyle;
 import org.apache.geode.management.internal.cli.shell.jline.GfshHistory;
@@ -895,8 +895,8 @@ public class Gfsh extends JLineShell {
     return loggedMessage;
   }
 
-  public Result executeScript(File scriptFile, boolean quiet, boolean continueOnError) {
-    Result result;
+  public ResultModel executeScript(File scriptFile, boolean quiet, boolean continueOnError)
{
+    ResultModel result = new ResultModel();
     String initialIsQuiet = getEnvProperty(ENV_APP_QUIET_EXECUTION);
     try {
       this.isScriptRunning = true;
@@ -973,7 +973,7 @@ public class Gfsh extends JLineShell {
       scriptInfo.logScriptExecutionInfo(gfshFileLogger, result);
       if (quiet) {
         // Create empty result when in quiet mode
-        result = ResultBuilder.createInfoResult("");
+        result = ResultModel.createInfo("");
       }
     } catch (IOException e) {
       throw new CommandProcessingException("Error while reading file " + scriptFile,
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/ScriptExecutionDetails.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/ScriptExecutionDetails.java
index c267d49..22ca6c8 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/ScriptExecutionDetails.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/ScriptExecutionDetails.java
@@ -19,8 +19,10 @@ import java.util.List;
 
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.LogWrapper;
-import org.apache.geode.management.internal.cli.result.CompositeResultData;
+import org.apache.geode.management.internal.cli.result.ModelCommandResult;
 import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.model.DataResultModel;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 
 class ScriptExecutionDetails {
   private final String filePath;
@@ -28,38 +30,38 @@ class ScriptExecutionDetails {
 
   ScriptExecutionDetails(String filePath) {
     this.filePath = filePath;
-    this.commandAndStatusList = new ArrayList<CommandAndStatus>();
+    this.commandAndStatusList = new ArrayList<>();
   }
 
   void addCommandAndStatus(String command, String status) {
     this.commandAndStatusList.add(new CommandAndStatus(command, status));
   }
 
-  Result getResult() {
-    CompositeResultData compositeResultData = ResultBuilder.createCompositeResultData();
-    compositeResultData.setHeader(
+  ResultModel getResult() {
+    ResultModel result = new ResultModel();
+    result.setHeader(
         "************************* Execution Summary ***********************\nScript file:
"
             + filePath);
 
     for (int i = 0; i < this.commandAndStatusList.size(); i++) {
       int commandSrNo = i + 1;
-      CompositeResultData.SectionResultData section = compositeResultData.addSection("" +
(i + 1));
+      DataResultModel commandDetail = result.addData("command" + commandSrNo);
       CommandAndStatus commandAndStatus = commandAndStatusList.get(i);
-      section.addData("Command-" + String.valueOf(commandSrNo), commandAndStatus.command);
-      section.addData("Status", commandAndStatus.status);
+      commandDetail.addData("Command-" + String.valueOf(commandSrNo), commandAndStatus.command);
+      commandDetail.addData("Status", commandAndStatus.status);
       if (commandAndStatus.status.equals("FAILED")) {
-        compositeResultData.setStatus(Result.Status.ERROR);
+        result.setStatus(Result.Status.ERROR);
       }
       if (i != this.commandAndStatusList.size()) {
-        section.setFooter(Gfsh.LINE_SEPARATOR);
+        result.setFooter(Gfsh.LINE_SEPARATOR);
       }
     }
 
-    return ResultBuilder.buildResult(compositeResultData);
+    return result;
   }
 
-  void logScriptExecutionInfo(LogWrapper logWrapper, Result result) {
-    logWrapper.info(ResultBuilder.resultAsString(result));
+  void logScriptExecutionInfo(LogWrapper logWrapper, ResultModel result) {
+    logWrapper.info(ResultBuilder.resultAsString(new ModelCommandResult(result)));
   }
 
   static class CommandAndStatus {
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
index 5a340dc..77f2de2 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
@@ -45,7 +45,7 @@ import org.apache.geode.internal.cache.InternalCacheForClientAccess;
 import org.apache.geode.management.ManagementException;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.functions.SizeExportLogsFunction;
-import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.management.internal.cli.util.BytesToString;
 import org.apache.geode.test.junit.categories.GfshTest;
 import org.apache.geode.test.junit.categories.LoggingTest;
@@ -192,7 +192,7 @@ public class ExportLogsCommandTest {
         .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member2));
     doReturn(10 * MEGABYTE).when(spyCmd).getLocalDiskAvailable();
 
-    CommandResult res = (CommandResult) spyCmd.exportLogs("working dir", null, null, logLevel,
+    ResultModel res = spyCmd.exportLogs("working dir", null, null, logLevel,
         onlyLogLevel, false, start, end, logsOnly, statsOnly, "125m");
     assertThat(res.getStatus()).isEqualTo(Result.Status.ERROR);
     assertThat(res.toJson())
@@ -235,7 +235,7 @@ public class ExportLogsCommandTest {
         .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member2));
     doReturn(GIGABYTE).when(spyCmd).getLocalDiskAvailable();
 
-    CommandResult res = (CommandResult) spyCmd.exportLogs("working dir", null, null, logLevel,
+    ResultModel res = spyCmd.exportLogs("working dir", null, null, logLevel,
         onlyLogLevel, false, start, end, logsOnly, statsOnly, "125m");
     assertThat(res.getStatus()).isEqualTo(Result.Status.ERROR);
     assertThat(res.toJson()).contains(
@@ -275,7 +275,7 @@ public class ExportLogsCommandTest {
     doReturn(testResults1).when(spyCmd)
         .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member1));
 
-    CommandResult res = (CommandResult) spyCmd.exportLogs("working dir", null, null, logLevel,
+    ResultModel res = spyCmd.exportLogs("working dir", null, null, logLevel,
         onlyLogLevel, false, start, end, logsOnly, statsOnly, "125m");
     assertThat(res.getStatus()).isEqualTo(Result.Status.ERROR);
     assertThat(res.toJson()).contains(
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
index b54e32d..969f40b 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorTest.java
@@ -22,8 +22,8 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.mockito.Mockito;
 
-import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.GfshParseResult;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.test.junit.categories.GfshTest;
 import org.apache.geode.test.junit.categories.LoggingTest;
 
@@ -32,7 +32,7 @@ public class ExportLogsInterceptorTest {
 
   private ExportLogsInterceptor interceptor;
   private GfshParseResult parseResult;
-  private Result result;
+  private ResultModel result;
 
   @Before
   public void before() {
@@ -48,7 +48,8 @@ public class ExportLogsInterceptorTest {
     when(parseResult.getParamValueAsString("group")).thenReturn("group");
     when(parseResult.getParamValueAsString("member")).thenReturn("group");
     result = interceptor.preExecution(parseResult);
-    assertThat(result.nextLine()).contains("Can't specify both group and member");
+    assertThat(result.getInfoSection("info").getContent())
+        .containsOnly("Can't specify both group and member.");
   }
 
   @Test
@@ -56,12 +57,13 @@ public class ExportLogsInterceptorTest {
     when(parseResult.getParamValueAsString("start-time")).thenReturn("2000/01/01");
     when(parseResult.getParamValueAsString("end-time")).thenReturn("2000/01/02");
     result = interceptor.preExecution(parseResult);
-    assertThat(result.nextLine()).isEmpty();
+    assertThat(result.getInfoSection("info").getContent()).containsOnly("");
 
     when(parseResult.getParamValueAsString("start-time")).thenReturn("2000/01/02");
     when(parseResult.getParamValueAsString("end-time")).thenReturn("2000/01/01");
     result = interceptor.preExecution(parseResult);
-    assertThat(result.nextLine()).contains("start-time has to be earlier than end-time");
+    assertThat(result.getInfoSection("info").getContent())
+        .containsOnly("start-time has to be earlier than end-time.");
   }
 
   @Test
@@ -69,11 +71,12 @@ public class ExportLogsInterceptorTest {
     when(parseResult.getParamValue("logs-only")).thenReturn(true);
     when(parseResult.getParamValue("stats-only")).thenReturn(false);
     result = interceptor.preExecution(parseResult);
-    assertThat(result.nextLine()).isEmpty();
+    assertThat(result.getInfoSection("info").getContent()).containsOnly("");
 
     when(parseResult.getParamValue("logs-only")).thenReturn(true);
     when(parseResult.getParamValue("stats-only")).thenReturn(true);
     result = interceptor.preExecution(parseResult);
-    assertThat(result.nextLine()).contains("logs-only and stats-only can't both be true");
+    assertThat(result.getInfoSection("info").getContent())
+        .containsOnly("logs-only and stats-only can't both be true");
   }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LogLevelInterceptorTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LogLevelInterceptorTest.java
index 2e583f2..be026ee 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LogLevelInterceptorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LogLevelInterceptorTest.java
@@ -25,9 +25,9 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.mockito.Mockito;
 
-import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
 import org.apache.geode.management.internal.cli.GfshParseResult;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.test.junit.categories.GfshTest;
 import org.apache.geode.test.junit.categories.LoggingTest;
 
@@ -36,7 +36,7 @@ public class LogLevelInterceptorTest {
 
   private final List<AbstractCliAroundInterceptor> interceptors = new ArrayList<>();
   private GfshParseResult parseResult;
-  private Result result;
+  private ResultModel result;
 
   @Before
   public void before() {
@@ -53,8 +53,9 @@ public class LogLevelInterceptorTest {
     when(parseResult.getParamValueAsString("log-level")).thenReturn("test");
     when(parseResult.getParamValueAsString("loglevel")).thenReturn("test");
     for (AbstractCliAroundInterceptor interceptor : interceptors) {
-      result = (Result) interceptor.preExecution(parseResult);
-      assertThat(result.nextLine()).contains("Invalid log level: test");
+      result = (ResultModel) interceptor.preExecution(parseResult);
+      assertThat(result.getInfoSection("info").getContent())
+          .containsOnly("Invalid log level: test");
     }
   }
 
@@ -63,8 +64,8 @@ public class LogLevelInterceptorTest {
     when(parseResult.getParamValueAsString("log-level")).thenReturn("fine");
     when(parseResult.getParamValueAsString("loglevel")).thenReturn("fine");
     for (AbstractCliAroundInterceptor interceptor : interceptors) {
-      result = (Result) interceptor.preExecution(parseResult);
-      assertThat(result.nextLine()).isEmpty();
+      result = (ResultModel) interceptor.preExecution(parseResult);
+      assertThat(result.getInfoSection("info").getContent()).containsOnly("");
     }
   }
 
@@ -73,8 +74,8 @@ public class LogLevelInterceptorTest {
     when(parseResult.getParamValueAsString("log-level")).thenReturn("trace");
     when(parseResult.getParamValueAsString("loglevel")).thenReturn("trace");
     for (AbstractCliAroundInterceptor interceptor : interceptors) {
-      result = (Result) interceptor.preExecution(parseResult);
-      assertThat(result.nextLine()).isEmpty();
+      result = (ResultModel) interceptor.preExecution(parseResult);
+      assertThat(result.getInfoSection("info").getContent()).containsOnly("");
     }
   }
 }


Mime
View raw message