GEODE-5971: have command pipeline send ResultModel json across the wi…

…re (apache#3495)

Co-authored-by: Owen Nichols <[email protected]>

geode-core/src/distributedTest/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java

@@ -17,8 +17,8 @@
 
 import java.util.List;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
 
+import org.springframework.shell.core.CommandMarker;
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
@@ -30,13 +30,12 @@
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.InternalCache;
-import org.apache.geode.management.cli.GfshCommand;
 import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.cli.Result.Status;
 import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
-import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.ModelCommandResult;
 import org.apache.geode.management.internal.cli.result.TabularResultData;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.management.internal.configuration.domain.XmlEntity;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission.Operation;
@@ -47,7 +46,7 @@
  *
  * @since GemFire 8.1
  */
-public class MockExtensionCommands extends GfshCommand {
+public class MockExtensionCommands implements CommandMarker {
 
   public static final String OPTION_VALUE = "value";
 
@@ -182,38 +181,26 @@ protected Result executeFunctionOnAllMembersTabulateResultPersist(final Function
     final ResultCollector<CliFunctionResult, List<CliFunctionResult>> resultCollector =
         (ResultCollector<CliFunctionResult, List<CliFunctionResult>>) CliUtil
             .executeFunction(function, args, members);
-    final List<CliFunctionResult> functionResults =
-        (List<CliFunctionResult>) resultCollector.getResult();
-
-    AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
-    final TabularResultData tabularResultData = ResultBuilder.createTabularResultData();
-    final String errorPrefix = "ERROR: ";
-    for (CliFunctionResult functionResult : functionResults) {
-      boolean success = functionResult.isSuccessful();
-      tabularResultData.accumulate("Member", functionResult.getMemberIdOrName());
-      if (success) {
-        tabularResultData.accumulate("Status", functionResult.getMessage());
-        xmlEntity.set(functionResult.getXmlEntity());
-      } else {
-        tabularResultData.accumulate("Status", errorPrefix + functionResult.getMessage());
-        tabularResultData.setStatus(Status.ERROR);
-      }
-    }
+    final List<CliFunctionResult> functionResults = resultCollector.getResult();
+
+    ResultModel resultModel = ResultModel.createMemberStatusResult(functionResults);
 
-    final Result result = ResultBuilder.buildResult(tabularResultData);
+    XmlEntity xmlEntity = functionResults.stream().filter(CliFunctionResult::isSuccessful)
+        .map(CliFunctionResult::getXmlEntity)
+        .findFirst().orElse(null);
 
     InternalConfigurationPersistenceService ccService =
         InternalLocator.getLocator().getConfigurationPersistenceService();
     System.out.println("MockExtensionCommands: persisting xmlEntity=" + xmlEntity);
-    if (null != xmlEntity.get()) {
+    if (xmlEntity != null) {
       if (addXmlElement) {
-        ccService.addXmlEntity(xmlEntity.get(), null);
+        ccService.addXmlEntity(xmlEntity, null);
       } else {
-        ccService.deleteXmlEntity(xmlEntity.get(), null);
+        ccService.deleteXmlEntity(xmlEntity, null);
       }
     }
 
-    return result;
+    return new ModelCommandResult(resultModel);
   }
 
 }

geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java

@@ -108,9 +108,7 @@
 import org.apache.geode.management.internal.beans.stats.StatsLatency;
 import org.apache.geode.management.internal.beans.stats.StatsRate;
 import org.apache.geode.management.internal.beans.stats.VMStatsMonitor;
-import org.apache.geode.management.internal.cli.CommandResponseBuilder;
 import org.apache.geode.management.internal.cli.remote.OnlineCommandProcessor;
-import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.management.internal.cli.result.model.ResultModel;
 
 /**
@@ -1504,13 +1502,9 @@ public String processCommand(String commandString, Map<String, String> env,
               + commandServiceInitError);
     }
 
-    Object result = commandProcessor.executeCommand(commandString, env, stagedFilePaths);
+    ResultModel result = commandProcessor.executeCommand(commandString, env, stagedFilePaths);
 
-    if (result instanceof CommandResult) {
-      return CommandResponseBuilder.createCommandResponseJson(getMember(), (CommandResult) result);
-    } else {
-      return CommandResponseBuilder.createCommandResponseJson(getMember(), (ResultModel) result);
-    }
+    return result.toJson();
   }
 
   public long getTotalDiskUsage() {

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

@@ -16,7 +16,6 @@
 
 import java.nio.file.Path;
 
-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.shell.GfshExecutionStrategy;
 
@@ -32,20 +31,10 @@ public interface CliAroundInterceptor {
   /**
    * called by the OperationInvoker before the command is executed
    */
-  default Object preExecution(GfshParseResult parseResult) {
+  default ResultModel preExecution(GfshParseResult parseResult) {
     return ResultModel.createInfo("");
   }
 
-  /**
-   * called by the OperationInvoker after the command is executed
-   *
-   * @param tempFile if the command's isFileDownloadOverHttp is true, the is the File downloaded
-   *        after the http response is processed.
-   */
-  default CommandResult postExecution(GfshParseResult parseResult, CommandResult commandResult,
-      Path tempFile) throws Exception {
-    return commandResult;
-  }
 
   default ResultModel postExecution(GfshParseResult parseResult, ResultModel resultModel,
       Path tempFile) throws Exception {

geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java

@@ -25,6 +25,7 @@
 import org.springframework.util.ReflectionUtils;
 
 import org.apache.geode.SystemFailure;
+import org.apache.geode.annotations.VisibleForTesting;
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.util.ArgumentRedactor;
@@ -53,12 +54,20 @@ public class CommandExecutor {
 
   private Logger logger = LogService.getLogger();
 
-  // used by the product
+  /**
+   *
+   * @return always return ResultModel for online command, for offline command, return either
+   *         ResultModel or ExitShellRequest
+   */
   public Object execute(GfshParseResult parseResult) {
     return execute(null, parseResult);
   }
 
-  // used by the GfshParserRule to pass in a mock command
+  /**
+   * @return always return ResultModel for online command, for offline command, return either
+   *         ResultModel or ExitShellRequest
+   */
+  @VisibleForTesting
   public Object execute(Object command, GfshParseResult parseResult) {
     String userInput = parseResult.getUserInput();
     if (userInput != null) {
@@ -68,6 +77,15 @@ public Object execute(Object command, GfshParseResult parseResult) {
     try {
       Object result = invokeCommand(command, parseResult);
 
+      if (result instanceof Result) {
+        Result customResult = (Result) result;
+        result = new ResultModel();
+        InfoResultModel info = ((ResultModel) result).addInfo();
+        while (customResult.hasNextLine()) {
+          info.addLine(customResult.nextLine());
+        }
+      }
+
       if (result == null) {
         return ResultModel.createError("Command returned null: " + parseResult);
       }

geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java

@@ -32,7 +32,7 @@
 import org.apache.geode.management.internal.cli.CommandManager;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.GfshParser;
-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.CommentSkipHelper;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission;
@@ -82,11 +82,11 @@ public ParseResult parseCommand(String commentLessLine)
     throw new IllegalStateException("Command String should not be null.");
   }
 
-  public Object executeCommand(String command) {
+  public ResultModel executeCommand(String command) {
     return executeCommand(command, Collections.emptyMap(), null);
   }
 
-  public Object executeCommand(String command, Map<String, String> env,
+  public ResultModel executeCommand(String command, Map<String, String> env,
       List<String> stagedFilePaths) {
     CommentSkipHelper commentSkipper = new CommentSkipHelper();
     String commentLessLine = commentSkipper.skipComments(command);
@@ -101,7 +101,7 @@ public Object executeCommand(String command, Map<String, String> env,
     ParseResult parseResult = parseCommand(commentLessLine);
 
     if (parseResult == null) {
-      return ResultBuilder.createParsingErrorResult(command);
+      return ResultModel.createError("Could not parse command string. " + command);
     }
 
     Method method = parseResult.getMethod();
@@ -116,10 +116,11 @@ public Object executeCommand(String command, Map<String, String> env,
     // this command processor does not execute commands that need fileData passed from client
     CliMetaData metaData = method.getAnnotation(CliMetaData.class);
     if (metaData != null && metaData.isFileUploaded() && stagedFilePaths == null) {
-      return ResultBuilder
-          .createUserErrorResult(command + " can not be executed only from server side");
+      return ResultModel
+          .createError(command + " can not be executed only from server side");
     }
 
-    return commandExecutor.execute((GfshParseResult) parseResult);
+    // we can do a direct cast because this only process online commands
+    return (ResultModel) commandExecutor.execute((GfshParseResult) parseResult);
   }
 }

geode-core/src/main/java/org/apache/geode/management/internal/cli/result/model/ResultModel.java

@@ -293,6 +293,7 @@ public DataResultModel getDataSection(String name) {
     return (DataResultModel) getSection(name);
   }
 
+  @JsonIgnore
   public List<String> getSectionNames() {
     List<String> sectionNames = new ArrayList<>();
     sections.forEach((k, v) -> sectionNames.add(k));