You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2018/11/28 22:25:09 UTC
[geode] branch develop updated: GEODE-5971: Refactor export/import commands to extend GfshCommand and… (#2894)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe 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 5820e9a GEODE-5971: Refactor export/import commands to extend GfshCommand and… (#2894)
5820e9a is described below
commit 5820e9aaabcca464400189f554c77a8addef3118
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Wed Nov 28 14:24:58 2018 -0800
GEODE-5971: Refactor export/import commands to extend GfshCommand and… (#2894)
---
.../cli/commands/ExportDataIntegrationTest.java | 5 ++-
.../cli/commands/ImportDataIntegrationTest.java | 5 ++-
.../internal/cli/commands/ExportDataCommand.java | 33 ++++++++--------
.../internal/cli/commands/ImportDataCommand.java | 33 ++++++++--------
.../internal/cli/functions/ExportDataFunction.java | 46 +++++++++++-----------
.../internal/cli/functions/ImportDataFunction.java | 11 ++++--
6 files changed, 71 insertions(+), 62 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java
index cb23852..33514a8 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java
@@ -100,8 +100,9 @@ public class ExportDataIntegrationTest {
.addOption(CliStrings.MEMBER, server.getName())
.addOption(CliStrings.EXPORT_DATA__REGION, "/nonExistentRegion")
.addOption(CliStrings.EXPORT_DATA__FILE, snapshotFile.toString()).getCommandString();
- gfsh.executeCommand(nonExistentRegionCommand);
- assertThat(gfsh.getGfshOutput()).contains("Could not process command due to error. Region");
+ gfsh.executeAndAssertThat(nonExistentRegionCommand).statusIsError()
+ .hasTableSection().hasColumn("Message")
+ .containsExactlyInAnyOrder("Region : /nonExistentRegion not found");
}
@Test
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java
index 4c828e4..fb241ac 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ImportDataIntegrationTest.java
@@ -134,8 +134,9 @@ public class ImportDataIntegrationTest {
.addOption(CliStrings.MEMBER, server.getName())
.addOption(CliStrings.IMPORT_DATA__REGION, "/nonExistentRegion")
.addOption(CliStrings.IMPORT_DATA__FILE, snapshotFile.toString()).getCommandString();
- gfsh.executeCommand(nonExistentRegionCommand);
- assertThat(gfsh.getGfshOutput()).contains("Could not process command due to error. Region");
+ gfsh.executeAndAssertThat(nonExistentRegionCommand).statusIsError()
+ .hasTableSection().hasColumn("Message")
+ .containsExactlyInAnyOrder("Region : /nonExistentRegion not found");
}
@Test
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java
index e77d1a9..52430bb 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportDataCommand.java
@@ -16,6 +16,7 @@
package org.apache.geode.management.internal.cli.commands;
import java.io.File;
+import java.util.List;
import java.util.Optional;
import org.springframework.shell.core.annotation.CliCommand;
@@ -28,20 +29,20 @@ import org.apache.geode.cache.snapshot.RegionSnapshotService;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
-import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.cli.GfshCommand;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import org.apache.geode.management.internal.cli.functions.ExportDataFunction;
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.security.ResourcePermission.Operation;
import org.apache.geode.security.ResourcePermission.Resource;
-public class ExportDataCommand extends InternalGfshCommand {
+public class ExportDataCommand extends GfshCommand {
private final ExportDataFunction exportDataFunction = new ExportDataFunction();
@CliCommand(value = CliStrings.EXPORT_DATA, help = CliStrings.EXPORT_DATA__HELP)
@CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DATA, CliStrings.TOPIC_GEODE_REGION})
- public Result exportData(
+ public ResultModel exportData(
@CliOption(key = CliStrings.EXPORT_DATA__REGION, mandatory = true,
optionContext = ConverterHint.REGION_PATH,
help = CliStrings.EXPORT_DATA__REGION__HELP) String regionName,
@@ -58,22 +59,22 @@ public class ExportDataCommand extends InternalGfshCommand {
authorize(Resource.DATA, Operation.READ, regionName);
final DistributedMember targetMember = getMember(memberNameOrId);
- Optional<Result> validationResult = validatePath(filePath, dirPath, parallel);
+ Optional<ResultModel> validationResult = validatePath(filePath, dirPath, parallel);
if (validationResult.isPresent()) {
return validationResult.get();
}
- Result result;
+ ResultModel result;
try {
String path = dirPath != null ? defaultFileName(dirPath, regionName) : filePath;
final String args[] = {regionName, path, Boolean.toString(parallel)};
ResultCollector<?, ?> rc = executeFunction(exportDataFunction, args, targetMember);
- result = CliUtil.getFunctionResult(rc, CliStrings.EXPORT_DATA);
+ result = ResultModel.createMemberStatusResult((List<CliFunctionResult>) rc.getResult());
} catch (CacheClosedException e) {
- result = ResultBuilder.createGemFireErrorResult(e.getMessage());
+ result = ResultModel.createError(e.getMessage());
} catch (FunctionInvocationTargetException e) {
- result = ResultBuilder.createGemFireErrorResult(
+ result = ResultModel.createError(
CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, CliStrings.IMPORT_DATA));
}
return result;
@@ -84,21 +85,21 @@ public class ExportDataCommand extends InternalGfshCommand {
.getAbsolutePath();
}
- private Optional<Result> validatePath(String filePath, String dirPath, boolean parallel) {
+ private Optional<ResultModel> validatePath(String filePath, String dirPath, boolean parallel) {
if (filePath == null && dirPath == null) {
return Optional
- .of(ResultBuilder.createUserErrorResult("Must specify a location to save snapshot"));
+ .of(ResultModel.createError("Must specify a location to save snapshot"));
} else if (filePath != null && dirPath != null) {
- return Optional.of(ResultBuilder.createUserErrorResult(
+ return Optional.of(ResultModel.createError(
"Options \"file\" and \"dir\" cannot be specified at the same time"));
} else if (parallel && dirPath == null) {
return Optional.of(
- ResultBuilder.createUserErrorResult("Must specify a directory to save snapshot files"));
+ ResultModel.createError("Must specify a directory to save snapshot files"));
}
if (dirPath == null && !filePath.endsWith(CliStrings.GEODE_DATA_FILE_EXTENSION)) {
- return Optional.of(ResultBuilder.createUserErrorResult(CliStrings
- .format(CliStrings.INVALID_FILE_EXTENSION, CliStrings.GEODE_DATA_FILE_EXTENSION)));
+ return Optional.of(ResultModel.createError(CliStrings.format(
+ CliStrings.INVALID_FILE_EXTENSION, CliStrings.GEODE_DATA_FILE_EXTENSION)));
}
return Optional.empty();
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportDataCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportDataCommand.java
index caece68..0b5a28f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportDataCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportDataCommand.java
@@ -15,6 +15,7 @@
package org.apache.geode.management.internal.cli.commands;
+import java.util.List;
import java.util.Optional;
import org.springframework.shell.core.annotation.CliCommand;
@@ -26,20 +27,20 @@ import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
-import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.cli.GfshCommand;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import org.apache.geode.management.internal.cli.functions.ImportDataFunction;
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.security.ResourcePermission.Operation;
import org.apache.geode.security.ResourcePermission.Resource;
-public class ImportDataCommand extends InternalGfshCommand {
+public class ImportDataCommand extends GfshCommand {
private final ImportDataFunction importDataFunction = new ImportDataFunction();
@CliCommand(value = CliStrings.IMPORT_DATA, help = CliStrings.IMPORT_DATA__HELP)
@CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DATA, CliStrings.TOPIC_GEODE_REGION})
- public Result importData(
+ public ResultModel importData(
@CliOption(key = CliStrings.IMPORT_DATA__REGION, optionContext = ConverterHint.REGION_PATH,
mandatory = true, help = CliStrings.IMPORT_DATA__REGION__HELP) String regionName,
@CliOption(key = CliStrings.IMPORT_DATA__FILE,
@@ -59,41 +60,41 @@ public class ImportDataCommand extends InternalGfshCommand {
final DistributedMember targetMember = getMember(memberNameOrId);
- Optional<Result> validationResult = validatePath(filePath, dirPath, parallel);
+ Optional<ResultModel> validationResult = validatePath(filePath, dirPath, parallel);
if (validationResult.isPresent()) {
return validationResult.get();
}
- Result result;
+ ResultModel result;
try {
String path = dirPath != null ? dirPath : filePath;
final Object args[] = {regionName, path, invokeCallbacks, parallel};
ResultCollector<?, ?> rc = executeFunction(importDataFunction, args, targetMember);
- result = CliUtil.getFunctionResult(rc, CliStrings.IMPORT_DATA);
+ result = ResultModel.createMemberStatusResult((List<CliFunctionResult>) rc.getResult());
} catch (CacheClosedException e) {
- result = ResultBuilder.createGemFireErrorResult(e.getMessage());
+ result = ResultModel.createError(e.getMessage());
} catch (FunctionInvocationTargetException e) {
- result = ResultBuilder.createGemFireErrorResult(
+ result = ResultModel.createError(
CliStrings.format(CliStrings.COMMAND_FAILURE_MESSAGE, CliStrings.IMPORT_DATA));
}
return result;
}
- private Optional<Result> validatePath(String filePath, String dirPath, boolean parallel) {
+ private Optional<ResultModel> validatePath(String filePath, String dirPath, boolean parallel) {
if (filePath == null && dirPath == null) {
return Optional
- .of(ResultBuilder.createUserErrorResult("Must specify a location to load snapshot from"));
+ .of(ResultModel.createError("Must specify a location to load snapshot from"));
} else if (filePath != null && dirPath != null) {
- return Optional.of(ResultBuilder.createUserErrorResult(
+ return Optional.of(ResultModel.createError(
"Options \"file\" and \"dir\" cannot be specified at the same time"));
} else if (parallel && dirPath == null) {
- return Optional.of(ResultBuilder
- .createUserErrorResult("Must specify a directory to load snapshot files from"));
+ return Optional
+ .of(ResultModel.createError("Must specify a directory to load snapshot files from"));
}
if (dirPath == null && !filePath.endsWith(CliStrings.GEODE_DATA_FILE_EXTENSION)) {
- return Optional.of(ResultBuilder.createUserErrorResult(CliStrings
+ return Optional.of(ResultModel.createError(CliStrings
.format(CliStrings.INVALID_FILE_EXTENSION, CliStrings.GEODE_DATA_FILE_EXTENSION)));
}
return Optional.empty();
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java
index 093130a..9c63352 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportDataFunction.java
@@ -23,8 +23,8 @@ import org.apache.geode.cache.snapshot.RegionSnapshotService;
import org.apache.geode.cache.snapshot.SnapshotOptions;
import org.apache.geode.cache.snapshot.SnapshotOptions.SnapshotFormat;
import org.apache.geode.internal.cache.InternalCache;
-import org.apache.geode.internal.cache.execute.InternalFunction;
import org.apache.geode.internal.cache.snapshot.SnapshotOptionsImpl;
+import org.apache.geode.management.cli.CliFunction;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
/***
@@ -33,10 +33,10 @@ import org.apache.geode.management.internal.cli.i18n.CliStrings;
*
*
*/
-public class ExportDataFunction implements InternalFunction {
+public class ExportDataFunction extends CliFunction {
private static final long serialVersionUID = 1L;
- public void execute(FunctionContext context) {
+ public CliFunctionResult executeFunction(FunctionContext context) throws Exception {
final String[] args = (String[]) context.getArguments();
if (args.length < 3) {
throw new IllegalStateException(
@@ -45,31 +45,31 @@ public class ExportDataFunction implements InternalFunction {
final String regionName = args[0];
final String fileName = args[1];
final boolean parallel = Boolean.parseBoolean(args[2]);
+ CliFunctionResult result;
- try {
- Cache cache = ((InternalCache) context.getCache()).getCacheForProcessingClientRequests();
- Region<?, ?> region = cache.getRegion(regionName);
- String hostName = cache.getDistributedSystem().getDistributedMember().getHost();
- if (region != null) {
- RegionSnapshotService<?, ?> snapshotService = region.getSnapshotService();
- final File exportFile = new File(fileName);
- if (parallel) {
- SnapshotOptions options = new SnapshotOptionsImpl<>().setParallelMode(true);
- snapshotService.save(exportFile, SnapshotFormat.GEMFIRE, options);
- } else {
- snapshotService.save(exportFile, SnapshotFormat.GEMFIRE);
- }
- String successMessage = CliStrings.format(CliStrings.EXPORT_DATA__SUCCESS__MESSAGE,
- regionName, exportFile.getCanonicalPath(), hostName);
- context.getResultSender().lastResult(successMessage);
+ Cache cache = ((InternalCache) context.getCache()).getCacheForProcessingClientRequests();
+ Region<?, ?> region = cache.getRegion(regionName);
+ String hostName = cache.getDistributedSystem().getDistributedMember().getHost();
+ if (region != null) {
+ RegionSnapshotService<?, ?> snapshotService = region.getSnapshotService();
+ final File exportFile = new File(fileName);
+ if (parallel) {
+ SnapshotOptions options = new SnapshotOptionsImpl<>().setParallelMode(true);
+ snapshotService.save(exportFile, SnapshotFormat.GEMFIRE, options);
} else {
- throw new IllegalArgumentException(
- CliStrings.format(CliStrings.REGION_NOT_FOUND, regionName));
+ snapshotService.save(exportFile, SnapshotFormat.GEMFIRE);
}
- } catch (Exception e) {
- context.getResultSender().sendException(e);
+ String successMessage = CliStrings.format(CliStrings.EXPORT_DATA__SUCCESS__MESSAGE,
+ regionName, exportFile.getCanonicalPath(), hostName);
+ result = new CliFunctionResult(context.getMemberName(), CliFunctionResult.StatusState.OK,
+ successMessage);
+ } else {
+ result = new CliFunctionResult(context.getMemberName(), CliFunctionResult.StatusState.ERROR,
+ CliStrings.format(CliStrings.REGION_NOT_FOUND, regionName));
}
+
+ return result;
}
public String getId() {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportDataFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportDataFunction.java
index 92dbd84..db48c7f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportDataFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportDataFunction.java
@@ -46,6 +46,7 @@ public class ImportDataFunction implements InternalFunction {
final boolean invokeCallbacks = (boolean) args[2];
final boolean parallel = (boolean) args[3];
+ CliFunctionResult result;
try {
final Cache cache =
((InternalCache) context.getCache()).getCacheForProcessingClientRequests();
@@ -60,15 +61,19 @@ public class ImportDataFunction implements InternalFunction {
snapshotService.load(new File(importFileName), SnapshotFormat.GEMFIRE, options);
String successMessage = CliStrings.format(CliStrings.IMPORT_DATA__SUCCESS__MESSAGE,
importFile.getCanonicalPath(), hostName, regionName);
- context.getResultSender().lastResult(successMessage);
+ result = new CliFunctionResult(context.getMemberName(), CliFunctionResult.StatusState.OK,
+ successMessage);
} else {
- throw new IllegalArgumentException(
+ result = new CliFunctionResult(context.getMemberName(), CliFunctionResult.StatusState.ERROR,
CliStrings.format(CliStrings.REGION_NOT_FOUND, regionName));
}
} catch (Exception e) {
- context.getResultSender().sendException(e);
+ result = new CliFunctionResult(context.getMemberName(), CliFunctionResult.StatusState.ERROR,
+ e.getMessage());
}
+
+ context.getResultSender().lastResult(result);
}
public String getId() {