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/24 17:54:29 UTC
[geode] branch develop updated: GEODE-5971: have command pipeline
send ResultModel json across the wire (#3495)
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 d3154bc GEODE-5971: have command pipeline send ResultModel json across the wire (#3495)
d3154bc is described below
commit d3154bc8d3e950e40219d3a422d9dd927dbe620c
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Wed Apr 24 10:54:16 2019 -0700
GEODE-5971: have command pipeline send ResultModel json across the wire (#3495)
Co-authored-by: Owen Nichols <on...@pivotal.io>
---
.../extension/mock/MockExtensionCommands.java | 41 +++-----
.../internal/beans/MemberMBeanBridge.java | 10 +-
.../internal/cli/CliAroundInterceptor.java | 13 +--
.../internal/cli/remote/CommandExecutor.java | 22 ++++-
.../cli/remote/OnlineCommandProcessor.java | 15 +--
.../internal/cli/result/model/ResultModel.java | 1 +
.../internal/cli/shell/GfshExecutionStrategy.java | 106 ++++++---------------
.../internal/cli/shell/JmxOperationInvoker.java | 5 +-
.../internal/web/http/support/HttpRequester.java | 6 ++
.../internal/web/shell/HttpOperationInvoker.java | 2 +-
.../cli/remote/OnlineCommandProcessorTest.java | 13 ++-
.../internal/cli/result/model/ResultModelTest.java | 11 +++
.../cli/shell/GfshExecutionStrategyTest.java | 29 +++---
.../lucene/LuceneCommandsSecurityDUnitTest.java | 6 +-
.../ShellCommandsControllerProcessCommandTest.java | 50 +++++-----
.../web/controllers/ShellCommandsController.java | 10 +-
16 files changed, 141 insertions(+), 199 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java
index cb3b8ef..70a5c2a 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/extension/mock/MockExtensionCommands.java
@@ -17,8 +17,8 @@ package org.apache.geode.internal.cache.extension.mock;
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.InternalConfigurationPersistenceSer
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 @@ import org.apache.geode.security.ResourcePermission.Resource;
*
* @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 @@ public class MockExtensionCommands extends GfshCommand {
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);
}
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
index 902e811..311611f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
+++ b/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.StatsKey;
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 class MemberMBeanBridge {
+ 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() {
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 61e2a84..7710f29 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
@@ -16,7 +16,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.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 {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
index 5ed084a..d65fb50 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
@@ -25,6 +25,7 @@ import org.apache.logging.log4j.Logger;
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 class CommandExecutor {
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);
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
index 6309665..7e9a622 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
@@ -32,7 +32,7 @@ import org.apache.geode.management.cli.CommandProcessingException;
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 class OnlineCommandProcessor {
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 class OnlineCommandProcessor {
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 class OnlineCommandProcessor {
// 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);
}
}
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 5a7b77c..e6f20a6 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
@@ -293,6 +293,7 @@ public class ResultModel {
return (DataResultModel) getSection(name);
}
+ @JsonIgnore
public List<String> getSectionNames() {
List<String> sectionNames = new ArrayList<>();
sections.forEach((k, v) -> sectionNames.add(k));
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
index 36d7cb9..f662b38 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java
@@ -20,7 +20,6 @@ import java.nio.file.Path;
import java.util.List;
import java.util.Map;
-import org.apache.commons.lang3.StringUtils;
import org.springframework.shell.core.ExecutionStrategy;
import org.springframework.shell.core.Shell;
import org.springframework.shell.event.ParseResult;
@@ -28,19 +27,13 @@ import org.springframework.util.Assert;
import org.apache.geode.internal.ClassPathLoader;
import org.apache.geode.management.cli.CliMetaData;
-import org.apache.geode.management.cli.Result;
import org.apache.geode.management.cli.Result.Status;
import org.apache.geode.management.internal.cli.CliAroundInterceptor;
import org.apache.geode.management.internal.cli.CommandRequest;
-import org.apache.geode.management.internal.cli.CommandResponse;
-import org.apache.geode.management.internal.cli.CommandResponseBuilder;
import org.apache.geode.management.internal.cli.GfshParseResult;
import org.apache.geode.management.internal.cli.LogWrapper;
-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.CommandResult;
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.ResultModel;
import org.apache.geode.security.NotAuthorizedException;
@@ -67,11 +60,12 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
*
* @param parseResult that should be executed (never presented as null)
* @return an object which will be rendered by the {@link Shell} implementation (may return null)
+ * this returns a ModelCommandResult for all the online commands. For offline-commands,
+ * this can return either a ModelCommandResult or ExitShellRequest
* @throws RuntimeException which is handled by the {@link Shell} implementation
*/
@Override
public Object execute(ParseResult parseResult) {
- Result result;
Method method = parseResult.getMethod();
// check if it's a shell only command
@@ -94,8 +88,12 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
throw new IllegalStateException("Configuration error!");
}
- result = executeOnRemote((GfshParseResult) parseResult);
- return result;
+ ResultModel resultModel = executeOnRemote((GfshParseResult) parseResult);
+
+ if (resultModel == null) {
+ return null;
+ }
+ return new ModelCommandResult(resultModel);
}
/**
@@ -143,7 +141,7 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
* @return result of execution/processing of the command
* @throws IllegalStateException if gfsh doesn't have an active connection.
*/
- private Result executeOnRemote(GfshParseResult parseResult) {
+ private ResultModel executeOnRemote(GfshParseResult parseResult) {
Path tempFile = null;
if (!shell.isConnectedAndReady()) {
@@ -159,7 +157,6 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
CliAroundInterceptor interceptor = null;
String interceptorClass = getInterceptor(parseResult.getMethod());
- boolean useResultModel = false;
// 1. Pre Remote Execution
if (!CliMetaData.ANNOTATION_NULL_VALUE.equals(interceptorClass)) {
@@ -171,26 +168,14 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
}
if (interceptor == null) {
- return ResultBuilder.createBadConfigurationErrorResult("Interceptor Configuration Error");
- }
-
- Object preExecResult = interceptor.preExecution(parseResult);
- if (preExecResult instanceof ResultModel) {
- useResultModel = true;
- if (((ResultModel) preExecResult).getStatus() != Status.OK) {
- return new ModelCommandResult((ResultModel) preExecResult);
- }
- } else { // Must be Result
- if (Status.ERROR.equals(((Result) preExecResult).getStatus())) {
- return (Result) preExecResult;
- }
+ return ResultModel.createError("Interceptor Configuration Error");
}
- // when the preExecution yields a FileResult, we will get the fileData out of it
- if (preExecResult instanceof ResultModel) {
- ResultModel fileResult = (ResultModel) preExecResult;
- fileData = fileResult.getFileList();
+ ResultModel preExecResult = interceptor.preExecution(parseResult);
+ if (preExecResult.getStatus() != Status.OK) {
+ return preExecResult;
}
+ fileData = preExecResult.getFileList();
}
// 2. Remote Execution
@@ -201,51 +186,25 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
.processCommand(new CommandRequest(parseResult, env, fileData));
if (response == null) {
- return ResultBuilder
- .createBadResponseErrorResult("Response was null for: " + parseResult.getUserInput());
+ return ResultModel.createError("Response was null for: " + parseResult.getUserInput());
}
} catch (NotAuthorizedException e) {
- return ResultBuilder
- .createGemFireUnAuthorizedErrorResult("Unauthorized. Reason : " + e.getMessage());
+ return ResultModel.createError("Unauthorized. Reason : " + e.getMessage());
} catch (Exception e) {
shell.logSevere(e.getMessage(), e);
- e.printStackTrace();
- return ResultBuilder.createBadResponseErrorResult(
+ return ResultModel.createError(
"Error occurred while executing \"" + parseResult.getUserInput() + "\" on manager.");
} finally {
env.clear();
}
// the response could be a string which is a json representation of the
- // CommandResult/ResultModel object
+ // ResultModel object
// it can also be a Path to a temp file downloaded from the rest http request
- Object commandResult = null;
+ ResultModel commandResult = null;
if (response instanceof String) {
- try {
- // if it's ResultModel
- commandResult = ResultModel.fromJson((String) response);
- useResultModel = true;
- } catch (Exception ex) {
- // if it's a CommandResult
- CommandResponse commandResponse =
- CommandResponseBuilder.prepareCommandResponseFromJson((String) response);
+ commandResult = ResultModel.fromJson((String) response);
- if (commandResponse.isFailedToPersist()) {
- shell.printAsSevere(CliStrings.SHARED_CONFIGURATION_FAILED_TO_PERSIST_COMMAND_CHANGES);
- logWrapper.severe(CliStrings.SHARED_CONFIGURATION_FAILED_TO_PERSIST_COMMAND_CHANGES);
- }
-
- String debugInfo = commandResponse.getDebugInfo();
- if (StringUtils.isNotBlank(debugInfo)) {
- debugInfo = debugInfo.replaceAll("\n\n\n", "\n");
- debugInfo = debugInfo.replaceAll("\n\n", "\n");
- debugInfo =
- debugInfo.replaceAll("\n", "\n[From Manager : " + commandResponse.getSender() + "]");
- debugInfo = "[From Manager : " + commandResponse.getSender() + "]" + debugInfo;
- this.logWrapper.info(debugInfo);
- }
- commandResult = ResultBuilder.fromJson((String) response);
- }
} else if (response instanceof Path) {
tempFile = (Path) response;
}
@@ -253,31 +212,20 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
// 3. Post Remote Execution
if (interceptor != null) {
try {
- if (useResultModel) {
- commandResult =
- interceptor.postExecution(parseResult, (ResultModel) commandResult, tempFile);
- } else {
- commandResult =
- interceptor.postExecution(parseResult, (CommandResult) commandResult, tempFile);
- }
+ commandResult =
+ interceptor.postExecution(parseResult, commandResult, tempFile);
+
} catch (Exception e) {
logWrapper.severe("error running post interceptor", e);
- commandResult = ResultBuilder.createGemFireErrorResult(e.getMessage());
+ commandResult = ResultModel.createError(e.getMessage());
}
}
if (commandResult == null) {
- commandResult = ResultBuilder
- .createGemFireErrorResult("Unable to build commandResult using the remote response.");
- }
-
- CommandResult gfshResult;
- if (commandResult instanceof ResultModel) {
- gfshResult = new ModelCommandResult((ResultModel) commandResult);
- } else {
- gfshResult = (CommandResult) commandResult;
+ commandResult =
+ ResultModel.createError("Unable to build ResultModel using the remote response.");
}
- return gfshResult;
+ return commandResult;
}
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
index 2eba0b5..38c20b0 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
@@ -263,7 +263,10 @@ public class JmxOperationInvoker implements OperationInvoker {
}
@Override
- public Object processCommand(final CommandRequest commandRequest) {
+ /**
+ * this should only returns a json representation of ResultModel
+ */
+ public String processCommand(final CommandRequest commandRequest) {
List<String> stagedFilePaths = null;
try {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/http/support/HttpRequester.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/http/support/HttpRequester.java
index d7ac175..3b93ad1 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/http/support/HttpRequester.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/http/support/HttpRequester.java
@@ -145,6 +145,9 @@ public class HttpRequester {
return response.getBody();
}
+ /**
+ * @return either a json representation of ResultModel or a Path
+ */
public Object executeWithResponseExtractor(URI url) {
return restTemplate.execute(url, HttpMethod.POST, this::addHeaderValues, this::extractResponse);
}
@@ -153,6 +156,9 @@ public class HttpRequester {
addHeaderValues(request.getHeaders());
}
+ /**
+ * @return either a json representation of ResultModel or a Path
+ */
Object extractResponse(ClientHttpResponse response) throws IOException {
MediaType mediaType = response.getHeaders().getContentType();
if (mediaType.equals(MediaType.APPLICATION_JSON)) {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
index d5485c4..a925e06 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
@@ -421,7 +421,7 @@ public class HttpOperationInvoker implements OperationInvoker {
* (execution).
*
* @param command the command requested/entered by the user to be processed.
- * @return the result of the command execution.
+ * @return the result of the command execution, either a json string of ResultModel or a Path
*/
@Override
public Object processCommand(final CommandRequest command) {
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
index 4c3d27d..b2f3193 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
@@ -29,8 +29,7 @@ import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.apache.geode.internal.security.SecurityService;
-import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
import org.apache.geode.security.NotAuthorizedException;
public class OnlineCommandProcessorTest {
@@ -39,7 +38,7 @@ public class OnlineCommandProcessorTest {
SecurityService securityService;
CommandExecutor executor;
OnlineCommandProcessor onlineCommandProcessor;
- Result result;
+ ResultModel result;
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -49,7 +48,7 @@ public class OnlineCommandProcessorTest {
properties = new Properties();
securityService = mock(SecurityService.class);
executor = mock(CommandExecutor.class);
- result = mock(Result.class);
+ result = mock(ResultModel.class);
when(executor.execute(any())).thenReturn(result);
onlineCommandProcessor =
@@ -75,7 +74,7 @@ public class OnlineCommandProcessorTest {
@Test
public void executeReturnsExecutorResult() throws Exception {
- Object commandResult = onlineCommandProcessor.executeCommand("start locator");
+ ResultModel commandResult = onlineCommandProcessor.executeCommand("start locator");
assertThat(commandResult).isSameAs(result);
}
@@ -88,8 +87,8 @@ public class OnlineCommandProcessorTest {
@Test
public void handlesParsingError() throws Exception {
- Object commandResult = onlineCommandProcessor.executeCommand("foo --bar");
- assertThat(commandResult).isInstanceOf(CommandResult.class);
+ ResultModel commandResult = onlineCommandProcessor.executeCommand("foo --bar");
+ assertThat(commandResult).isInstanceOf(ResultModel.class);
assertThat(commandResult.toString()).contains("Could not parse command string. foo --bar");
}
}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/model/ResultModelTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/model/ResultModelTest.java
index b5e29c4..51c97df 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/model/ResultModelTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/model/ResultModelTest.java
@@ -181,4 +181,15 @@ public class ResultModelTest {
ResultModel resultModel = mapper.readValue(json, ResultModel.class);
assertThat(resultModel.getFileToDownload()).isEqualTo(file.toPath());
}
+
+ @Test
+ public void serializeInfoResult() throws Exception {
+ InfoResultModel info = result.addInfo();
+ info.addLine("line2");
+ ObjectMapper mapper = GeodeJsonMapper.getMapper();
+ String json = mapper.writeValueAsString(result);
+ System.out.println(json);
+ ResultModel resultModel = mapper.readValue(json, ResultModel.class);
+ assertThat(resultModel.getSectionNames()).containsExactly("info");
+ }
}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java
index 40dfdd2..dc2e8b7 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyTest.java
@@ -29,11 +29,8 @@ import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.Result;
import org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
import org.apache.geode.management.internal.cli.CommandRequest;
-import org.apache.geode.management.internal.cli.CommandResponseBuilder;
import org.apache.geode.management.internal.cli.GfshParseResult;
import org.apache.geode.management.internal.cli.LogWrapper;
-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;
/**
@@ -97,9 +94,8 @@ public class GfshExecutionStrategyTest {
when(gfsh.isConnectedAndReady()).thenReturn(true);
OperationInvoker invoker = mock(OperationInvoker.class);
- Result offLineResult = new Commands().onlineCommand();
- String jsonResult = CommandResponseBuilder.createCommandResponseJson("memberName",
- (CommandResult) offLineResult);
+ ResultModel offLineResult = new Commands().onlineCommand();
+ String jsonResult = offLineResult.toJson();
when(invoker.processCommand(any(CommandRequest.class))).thenReturn(jsonResult);
when(gfsh.getOperationInvoker()).thenReturn(invoker);
Result result = (Result) gfshExecutionStrategy.execute(parsedCommand);
@@ -114,9 +110,8 @@ public class GfshExecutionStrategyTest {
when(gfsh.isConnectedAndReady()).thenReturn(true);
OperationInvoker invoker = mock(OperationInvoker.class);
- Result interceptedResult = new Commands().interceptedCommand();
- String jsonResult = CommandResponseBuilder.createCommandResponseJson("memberName",
- (CommandResult) interceptedResult);
+ ResultModel interceptedResult = new Commands().interceptedCommand();
+ String jsonResult = interceptedResult.toJson();
when(invoker.processCommand(any(CommandRequest.class))).thenReturn(jsonResult);
when(gfsh.getOperationInvoker()).thenReturn(invoker);
Result result = (Result) gfshExecutionStrategy.execute(parsedCommand);
@@ -129,8 +124,8 @@ public class GfshExecutionStrategyTest {
*/
public static class Commands implements CommandMarker {
@CliMetaData(shellOnly = true)
- public Result offlineCommand() {
- return ResultBuilder.createInfoResult(COMMAND1_SUCCESS);
+ public ResultModel offlineCommand() {
+ return ResultModel.createInfo(COMMAND1_SUCCESS);
}
@CliMetaData(shellOnly = true)
@@ -139,14 +134,14 @@ public class GfshExecutionStrategyTest {
}
@CliMetaData(shellOnly = false)
- public Result onlineCommand() {
- return ResultBuilder.createInfoResult(COMMAND2_SUCCESS);
+ public ResultModel onlineCommand() {
+ return ResultModel.createInfo(COMMAND2_SUCCESS);
}
@CliMetaData(shellOnly = false,
interceptor = "org.apache.geode.management.internal.cli.shell.GfshExecutionStrategyTest$TestInterceptor")
- public Result interceptedCommand() {
- return ResultBuilder.createInfoResult(COMMAND4_SUCCESS);
+ public ResultModel interceptedCommand() {
+ return ResultModel.createInfo(COMMAND4_SUCCESS);
}
}
@@ -156,10 +151,10 @@ public class GfshExecutionStrategyTest {
public static class TestInterceptor extends AbstractCliAroundInterceptor {
@Override
- public Result preExecution(GfshParseResult parseResult) {
+ public ResultModel preExecution(GfshParseResult parseResult) {
parseResult.setUserInput(AFTER_INTERCEPTION_MESSAGE);
- return ResultBuilder.createInfoResult("Interceptor Result");
+ return ResultModel.createInfo("Interceptor Result");
}
}
}
diff --git a/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java b/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java
index aa75f04..76087b2 100644
--- a/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java
+++ b/geode-lucene/src/distributedTest/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java
@@ -37,8 +37,6 @@ import org.apache.geode.examples.SimpleSecurityManager;
import org.apache.geode.management.cli.Result;
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.ErrorResultData;
-import org.apache.geode.management.internal.cli.result.ResultBuilder;
import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
@@ -208,9 +206,7 @@ public class LuceneCommandsSecurityDUnitTest {
private void verifyResult(UserNameAndExpectedResponse user, CommandResult result) {
if (user.getExpectAuthorizationError()) {
- assertTrue(result.getResultData() instanceof ErrorResultData);
- assertEquals(ResultBuilder.ERRORCODE_UNAUTHORIZED,
- ((ErrorResultData) result.getResultData()).getErrorCode());
+ assertEquals(Result.Status.ERROR, result.getStatus());
} else {
assertEquals(Result.Status.OK, result.getStatus());
}
diff --git a/geode-web/src/integrationTest/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerProcessCommandTest.java b/geode-web/src/integrationTest/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerProcessCommandTest.java
index 1247e4c..8cb0dc9 100644
--- a/geode-web/src/integrationTest/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerProcessCommandTest.java
+++ b/geode-web/src/integrationTest/java/org/apache/geode/management/internal/web/controllers/ShellCommandsControllerProcessCommandTest.java
@@ -15,10 +15,13 @@
package org.apache.geode.management.internal.web.controllers;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
import java.io.File;
import java.io.IOException;
-import java.util.Map;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
@@ -30,59 +33,48 @@ import org.springframework.core.io.InputStreamResource;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
-import org.springframework.web.multipart.MultipartFile;
-import org.apache.geode.management.internal.cli.CommandResponseBuilder;
-import org.apache.geode.management.internal.cli.result.CommandResult;
-import org.apache.geode.management.internal.cli.result.ErrorResultData;
-import org.apache.geode.management.internal.cli.result.InfoResultData;
-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.FileResultModel;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
public class ShellCommandsControllerProcessCommandTest {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
private ShellCommandsController controller;
- private CommandResult fakeResult;
+ private ResultModel fakeResult;
@Before
- public void setup() {
-
- controller = new ShellCommandsController() {
- @Override
- protected String processCommand(String command, final Map<String, String> environment,
- MultipartFile[] fileData) {
- return CommandResponseBuilder.createCommandResponseJson("someMember", fakeResult);
- }
- };
+ public void setup() throws IOException {
+ controller = spy(ShellCommandsController.class);
}
@Test
public void infoOkResult() throws IOException {
- fakeResult = new LegacyCommandResult(new InfoResultData("Some info message"));
-
+ fakeResult = ResultModel.createInfo("Some info message");
+ doReturn(fakeResult.toJson()).when(controller).processCommand(anyString(), any(), any());
ResponseEntity<InputStreamResource> responseJsonStream = controller.command("xyz", null);
assertThatContentTypeEquals(responseJsonStream, MediaType.APPLICATION_JSON);
String responseJson = toString(responseJsonStream);
- CommandResult result = ResultBuilder.fromJson(responseJson);
+ ResultModel result = ResultModel.fromJson(responseJson);
- assertThat(result.nextLine()).isEqualTo(fakeResult.nextLine());
+ assertThat(result.getInfoSection("info").getContent().get(0))
+ .isEqualTo(fakeResult.getInfoSection("info").getContent().get(0));
}
@Test
public void errorResult() throws IOException {
- ErrorResultData errorResultData = new ErrorResultData("Some error message");
- fakeResult = new LegacyCommandResult(errorResultData);
-
+ fakeResult = ResultModel.createError("Some error message");
+ doReturn(fakeResult.toJson()).when(controller).processCommand(anyString(), any(), any());
ResponseEntity<InputStreamResource> responseJsonStream = controller.command("xyz", null);
assertThatContentTypeEquals(responseJsonStream, MediaType.APPLICATION_JSON);
String responseJson = toString(responseJsonStream);
- CommandResult result = ResultBuilder.fromJson(responseJson);
+ ResultModel result = ResultModel.fromJson(responseJson);
- assertThat(result.nextLine()).isEqualTo(fakeResult.nextLine());
+ assertThat(result.getInfoSection("info").getContent().get(0))
+ .isEqualTo(fakeResult.getInfoSection("info").getContent().get(0));
}
@Test
@@ -90,8 +82,10 @@ public class ShellCommandsControllerProcessCommandTest {
File tempFile = temporaryFolder.newFile();
FileUtils.writeStringToFile(tempFile, "some file contents", "UTF-8");
- fakeResult = new LegacyCommandResult(tempFile.toPath());
+ fakeResult = new ResultModel();
+ fakeResult.addFile(tempFile, FileResultModel.FILE_TYPE_FILE);
+ doReturn(fakeResult.toJson()).when(controller).processCommand(anyString(), any(), any());
ResponseEntity<InputStreamResource> responseFileStream = controller.command("xyz", null);
assertThatContentTypeEquals(responseFileStream, MediaType.APPLICATION_OCTET_STREAM);
diff --git a/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java b/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
index 16e53e5..55dd04e 100644
--- a/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
+++ b/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
@@ -52,8 +52,7 @@ import org.apache.geode.internal.GemFireVersion;
import org.apache.geode.internal.util.IOUtils;
import org.apache.geode.management.cli.Result;
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.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
import org.apache.geode.management.internal.web.domain.QueryParameterSource;
@@ -189,8 +188,9 @@ public class ShellCommandsController extends AbstractCommandsController {
private ResponseEntity<InputStreamResource> getResponse(String result) {
- CommandResult commandResult = ResultBuilder.fromJson(result);
- if (commandResult.getStatus().equals(Result.Status.OK) && commandResult.hasFileToDownload()) {
+ ResultModel commandResult = ResultModel.fromJson(result);
+ if (commandResult.getStatus().equals(Result.Status.OK)
+ && commandResult.getFileToDownload() != null) {
return getFileDownloadResponse(commandResult);
} else {
return getJsonResponse(result);
@@ -208,7 +208,7 @@ public class ShellCommandsController extends AbstractCommandsController {
}
}
- private ResponseEntity<InputStreamResource> getFileDownloadResponse(CommandResult commandResult) {
+ private ResponseEntity<InputStreamResource> getFileDownloadResponse(ResultModel commandResult) {
HttpHeaders respHeaders = new HttpHeaders();
Path filePath = commandResult.getFileToDownload();
try {