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 2018/09/13 14:30:53 UTC
[geode] branch develop updated: GEODE-5727: rework how ResultModel
deal with file contents. (#2460)
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 4fbe655 GEODE-5727: rework how ResultModel deal with file contents. (#2460)
4fbe655 is described below
commit 4fbe65515c82bbdea715c488ab82baa5fda05bd9
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Thu Sep 13 07:30:43 2018 -0700
GEODE-5727: rework how ResultModel deal with file contents. (#2460)
* GEODE-5727: rework how ResultModel deal with file contents.
* file saving should be handled by the command's postExecutor to save to appropriate places
* after saving file content to a directory, the ResultModel turned the FileResultModel into InfoResultModel.
* gfsh should not be the place to save files in ModelCommandResult.
---
...ExportClusterConfigurationCommandDUnitTest.java | 68 +++++++++++++++-
.../ClusterConfigImportDUnitTest.java | 60 +-------------
.../result/model/ResultModelIntegrationTest.java | 92 ++++++++++++++++++++++
.../ExportClusterConfigurationCommand.java | 5 +-
.../internal/cli/commands/ExportConfigCommand.java | 17 ++--
.../internal/cli/result/ModelCommandResult.java | 11 +--
.../internal/cli/result/model/FileResultModel.java | 86 +++++---------------
.../internal/cli/result/model/ResultModel.java | 34 +++++++-
.../geode/management/internal/cli/shell/Gfsh.java | 6 +-
.../test/junit/assertions/ResultModelAssert.java | 50 ++++++++++++
.../cli/commands/ExportConfigCommandDUnitTest.java | 4 +-
.../internal/cli/commands/CommandOverHttpTest.java | 5 +-
12 files changed, 283 insertions(+), 155 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommandDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommandDUnitTest.java
index 9c14c43..75ffc25 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommandDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommandDUnitTest.java
@@ -21,7 +21,13 @@ import static org.assertj.core.api.Assertions.assertThat;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
+import java.nio.file.Path;
+import java.util.HashSet;
import java.util.Properties;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
import org.apache.commons.io.FileUtils;
import org.junit.BeforeClass;
@@ -29,6 +35,8 @@ import org.junit.ClassRule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
+import org.apache.geode.management.internal.configuration.ClusterConfig;
+import org.apache.geode.management.internal.configuration.ConfigGroup;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.rules.GfshCommandRule;
@@ -45,7 +53,7 @@ public class ExportClusterConfigurationCommandDUnitTest {
private static File xmlFile;
- private static MemberVM locator;
+ private static MemberVM locator, server;
@BeforeClass
public static void beforeClass() throws Exception {
@@ -53,7 +61,7 @@ public class ExportClusterConfigurationCommandDUnitTest {
locator = cluster.startLocatorVM(0);
Properties properties = new Properties();
properties.setProperty(GROUPS_NAME, "groupB");
- cluster.startServerVM(1, properties, locator.getPort());
+ server = cluster.startServerVM(1, properties, locator.getPort());
gfsh.connectAndVerify(locator);
gfsh.executeAndAssertThat("create region --name=regionA --type=REPLICATE").statusIsSuccess();
gfsh.executeAndAssertThat("create region --name=regionB --type=REPLICATE --group=groupB")
@@ -86,4 +94,60 @@ public class ExportClusterConfigurationCommandDUnitTest {
assertThat(content).startsWith("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>")
.contains("<region name=\"regionA\">");
}
+
+ @Test
+ public void testExportWithAbsolutePath() throws Exception {
+ Path exportedZipPath =
+ tempFolder.getRoot().toPath().resolve("exportedCC.zip").toAbsolutePath();
+
+ testExportClusterConfig(exportedZipPath.toString());
+ }
+
+ @Test
+ public void testExportWithRelativePath() throws Exception {
+ testExportClusterConfig("mytemp/exportedCC.zip");
+ FileUtils.deleteQuietly(new File("mytemp"));
+ }
+
+ @Test
+ public void testExportWithOnlyFileName() throws Exception {
+ testExportClusterConfig("exportedCC.zip");
+ FileUtils.deleteQuietly(new File("exportedCC.zip"));
+ }
+
+ public void testExportClusterConfig(String zipFilePath) throws Exception {
+ ConfigGroup cluster = new ConfigGroup("cluster").regions("regionA");
+ ConfigGroup groupB = new ConfigGroup("groupB").regions("regionB");
+ ClusterConfig expectedClusterConfig = new ClusterConfig(cluster, groupB);
+ expectedClusterConfig.verify(locator);
+ expectedClusterConfig.verify(server);
+
+ String expectedFilePath = new File(zipFilePath).getAbsolutePath();
+ gfsh.executeAndAssertThat("export cluster-configuration --zip-file-name=" + zipFilePath)
+ .statusIsSuccess()
+ .containsOutput("File saved to " + expectedFilePath);
+
+ File exportedZip = new File(zipFilePath);
+ assertThat(exportedZip).exists();
+
+ Set<String> actualZipEnries =
+ new ZipFile(exportedZip).stream().map(ZipEntry::getName).collect(Collectors.toSet());
+
+ ConfigGroup exportedClusterGroup = cluster.configFiles("cluster.xml", "cluster.properties");
+ ConfigGroup exportedGroupB = groupB.configFiles("groupB.xml", "groupB.properties");
+ ClusterConfig expectedExportedClusterConfig =
+ new ClusterConfig(exportedClusterGroup, exportedGroupB);
+
+ Set<String> expectedZipEntries = new HashSet<>();
+ for (ConfigGroup group : expectedExportedClusterConfig.getGroups()) {
+ String groupDir = group.getName() + File.separator;
+
+ for (String jarOrXmlOrPropFile : group.getAllFiles()) {
+ expectedZipEntries.add(groupDir + jarOrXmlOrPropFile);
+ }
+ }
+
+ assertThat(actualZipEnries).isEqualTo(expectedZipEntries);
+ }
+
}
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
index 180d0d4..b8d495e 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
@@ -20,14 +20,7 @@ import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
import static org.assertj.core.api.Assertions.assertThat;
import java.io.File;
-import java.nio.file.Path;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.stream.Collectors;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipFile;
-
-import org.apache.commons.io.FileUtils;
+
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -171,55 +164,4 @@ public class ClusterConfigImportDUnitTest extends ClusterConfigTestBase {
REPLICATED_CONFIG_FROM_ZIP.verify(locator1);
REPLICATED_CONFIG_FROM_ZIP.verify(locator2);
}
-
- @Test
- public void testExportWithAbsolutePath() throws Exception {
- Path exportedZipPath =
- temporaryFolder.getRoot().toPath().resolve("exportedCC.zip").toAbsolutePath();
-
- testExportClusterConfig(exportedZipPath.toString());
- }
-
- @Test
- public void testExportWithRelativePath() throws Exception {
- testExportClusterConfig("mytemp/exportedCC.zip");
- FileUtils.deleteQuietly(new File("mytemp"));
- }
-
- public void testExportClusterConfig(String zipFilePath) throws Exception {
- MemberVM server1 = lsRule.startServerVM(1, serverProps, locatorVM.getPort());
-
- gfshConnector.executeAndAssertThat("create region --name=myRegion --type=REPLICATE")
- .statusIsSuccess();
-
- ConfigGroup cluster = new ConfigGroup("cluster").regions("myRegion");
- ClusterConfig expectedClusterConfig = new ClusterConfig(cluster);
- expectedClusterConfig.verify(server1);
- expectedClusterConfig.verify(locatorVM);
-
- gfshConnector
- .executeAndAssertThat("export cluster-configuration --zip-file-name=" + zipFilePath)
- .statusIsSuccess();
-
- File exportedZip = new File(zipFilePath);
- assertThat(exportedZip).exists();
-
- Set<String> actualZipEnries =
- new ZipFile(exportedZip).stream().map(ZipEntry::getName).collect(Collectors.toSet());
-
- ConfigGroup exportedClusterGroup = cluster.configFiles("cluster.xml", "cluster.properties");
- ClusterConfig expectedExportedClusterConfig = new ClusterConfig(exportedClusterGroup);
-
- Set<String> expectedZipEntries = new HashSet<>();
- for (ConfigGroup group : expectedExportedClusterConfig.getGroups()) {
- String groupDir = group.getName() + File.separator;
-
- for (String jarOrXmlOrPropFile : group.getAllFiles()) {
- expectedZipEntries.add(groupDir + jarOrXmlOrPropFile);
- }
- }
-
- assertThat(actualZipEnries).isEqualTo(expectedZipEntries);
- }
-
}
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/result/model/ResultModelIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/result/model/ResultModelIntegrationTest.java
new file mode 100644
index 0000000..e298c7b
--- /dev/null
+++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/result/model/ResultModelIntegrationTest.java
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.result.model;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.management.internal.cli.result.ModelCommandResult;
+import org.apache.geode.test.junit.assertions.ResultModelAssert;
+
+public class ResultModelIntegrationTest {
+
+ private ResultModel result;
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+ @Before
+ public void setUp() throws Exception {
+ result = new ResultModel();
+ result.addFile("test1.txt", "hello");
+ result.addFile("test2.txt", "hello again");
+ }
+
+ @Test
+ public void emptyFileSizeDoesNothing() throws IOException {
+ ResultModel emptyFileResult = new ResultModel();
+ result.saveFileTo(temporaryFolder.newFolder());
+ assertThat(emptyFileResult.getInfoSections()).hasSize(0);
+ }
+
+ @Test
+ public void savesToNoWhereDoesNothing() throws IOException {
+ result.saveFileTo(null);
+ assertThat(result.getInfoSections()).hasSize(0);
+ }
+
+ @Test
+ public void notADirectory() throws IOException {
+ result.saveFileTo(temporaryFolder.newFile());
+ assertThis(result).hasInfoResultModel("fileSave").hasOutput().contains("is not a directory");
+ }
+
+ @Test
+ public void dirNotExistBefore() throws IOException {
+ File dir = temporaryFolder.newFolder("test");
+ dir.delete();
+
+ result.saveFileTo(dir);
+ assertThat(dir).exists();
+ File file1 = new File(dir, "test1.txt");
+ File file2 = new File(dir, "test2.txt");
+ assertThat(dir.listFiles()).contains(file1, file2);
+
+ assertThis(result).hasInfoResultModel("fileSave").hasLines().hasSize(2)
+ .containsExactlyInAnyOrder(
+ "File saved to " + file1.getAbsolutePath(),
+ "File saved to " + file2.getAbsolutePath());
+ }
+
+ @Test
+ public void modelCommandResultShouldNotDealWithFiles() throws IOException {
+ result.saveFileTo(temporaryFolder.newFolder("test"));
+ ModelCommandResult commandResult = new ModelCommandResult(result);
+ assertThat(commandResult.hasIncomingFiles()).isFalse();
+ assertThat(commandResult.getNumTimesSaved()).isEqualTo(0);
+ }
+
+ public static ResultModelAssert assertThis(ResultModel model) {
+ return new ResultModelAssert(model);
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommand.java
index aa55092..90d2106 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportClusterConfigurationCommand.java
@@ -195,12 +195,11 @@ public class ExportClusterConfigurationCommand extends InternalGfshCommand {
}
} else if (zipFile != null) {
// delete the existing file since at this point, user is OK to replace the old zip.
- File file = new File(zipFile);
+ File file = new File(zipFile).getAbsoluteFile();
if (file.exists()) {
FileUtils.deleteQuietly(file);
}
- FileResultModel fileResultModel = result.getFiles().values().iterator().next();
- fileResultModel.writeFile(file.getParentFile().getAbsolutePath());
+ result.saveFileTo(file.getParentFile());
}
return result;
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommand.java
index 6e80a6b..bf19148 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommand.java
@@ -34,7 +34,6 @@ import org.apache.geode.management.internal.cli.GfshParseResult;
import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import org.apache.geode.management.internal.cli.functions.ExportConfigFunction;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.management.internal.cli.result.model.FileResultModel;
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.security.ResourceOperation;
@@ -87,8 +86,8 @@ public class ExportConfigCommand extends InternalGfshCommand {
String cacheFileName = result.getMemberIdOrName() + "-cache.xml";
String propsFileName = result.getMemberIdOrName() + "-gf.properties";
String[] fileContent = (String[]) result.getSerializables();
- crm.addFile(cacheFileName, fileContent[0], "Downloading Cache XML file: " + cacheFileName);
- crm.addFile(propsFileName, fileContent[1], "Downloading properties file: " + propsFileName);
+ crm.addFile(cacheFileName, fileContent[0]);
+ crm.addFile(propsFileName, fileContent[1]);
}
}
@@ -99,17 +98,17 @@ public class ExportConfigCommand extends InternalGfshCommand {
* Interceptor used by gfsh to intercept execution of export config command at "shell".
*/
public static class Interceptor extends AbstractCliAroundInterceptor {
- private String saveDirString;
+ private File saveDirFile;
@Override
public ResultModel preExecution(GfshParseResult parseResult) {
String dir = parseResult.getParamValueAsString("dir");
if (StringUtils.isBlank(dir)) {
- saveDirString = new File(".").getAbsolutePath();
+ saveDirFile = new File(".").getAbsoluteFile();
return new ResultModel();
}
- File saveDirFile = new File(dir.trim());
+ saveDirFile = new File(dir.trim()).getAbsoluteFile();
if (!saveDirFile.exists() && !saveDirFile.mkdirs()) {
return ResultModel
@@ -130,17 +129,13 @@ public class ExportConfigCommand extends InternalGfshCommand {
return ResultModel.createError(
CliStrings.format(CliStrings.EXPORT_CONFIG__MSG__NOT_WRITEABLE, saveDirFile.getName()));
}
-
- saveDirString = saveDirFile.getAbsolutePath();
return new ResultModel();
}
@Override
public ResultModel postExecution(GfshParseResult parseResult, ResultModel commandResult,
Path tempFile) throws Exception {
- for (FileResultModel file : commandResult.getFiles().values()) {
- file.writeFile(saveDirString);
- }
+ commandResult.saveFileTo(saveDirFile);
return commandResult;
}
}
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 106401e..65e3d56 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
@@ -31,7 +31,6 @@ import org.apache.geode.management.internal.cli.GfshParser;
import org.apache.geode.management.internal.cli.json.GfJsonObject;
import org.apache.geode.management.internal.cli.result.model.AbstractResultModel;
import org.apache.geode.management.internal.cli.result.model.DataResultModel;
-import org.apache.geode.management.internal.cli.result.model.FileResultModel;
import org.apache.geode.management.internal.cli.result.model.InfoResultModel;
import org.apache.geode.management.internal.cli.result.model.ResultModel;
import org.apache.geode.management.internal.cli.result.model.TabularResultModel;
@@ -83,9 +82,11 @@ 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
@Override
public boolean hasIncomingFiles() {
- return result.getFiles().size() > 0;
+ return false;
}
@Override
@@ -94,11 +95,7 @@ public class ModelCommandResult implements CommandResult {
}
@Override
- public void saveIncomingFiles(String directory) throws IOException {
- for (FileResultModel file : result.getFiles().values()) {
- file.writeFile(directory);
- }
- }
+ public void saveIncomingFiles(String directory) throws IOException {}
@Override
public boolean hasNextLine() {
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 ca3474f..6726fcf 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
@@ -20,11 +20,9 @@ import java.io.File;
import java.io.FileOutputStream;
import java.io.FileWriter;
import java.io.IOException;
-import java.text.MessageFormat;
import org.apache.commons.io.FileUtils;
-import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.result.ResultData;
import org.apache.geode.management.internal.cli.shell.Gfsh;
@@ -34,18 +32,16 @@ public class FileResultModel {
private String filename;
private int type;
- private String message;
private byte[] data;
private int length;
public FileResultModel() {}
- public FileResultModel(String fileName, String content, String message) {
+ public FileResultModel(String fileName, String content) {
this.filename = fileName;
this.data = content.getBytes();
this.length = data.length;
this.type = FILE_TYPE_TEXT;
- this.message = message;
}
public FileResultModel(File file, int fileType) {
@@ -79,14 +75,6 @@ public class FileResultModel {
this.type = type;
}
- public String getMessage() {
- return message;
- }
-
- public void setMessage(String message) {
- this.message = message;
- }
-
public byte[] getData() {
return data;
}
@@ -103,79 +91,45 @@ public class FileResultModel {
this.length = length;
}
- public void writeFile(String directory) throws IOException {
- String options = "(y/N)";
-
- File fileToDumpData = new File(filename);
- if (!fileToDumpData.isAbsolute()) {
- if (directory == null || directory.isEmpty()) {
- directory = System.getProperty("user.dir", ".");
- }
- fileToDumpData = new File(directory, filename);
- }
+ /**
+ * at this point, the dir should already exist and is confirmed as a directory
+ * filename in this instance should be file name only. no path in the file name
+ *
+ * @param directory the directory where to write the content of byte[] to with the filename
+ * @return the message you would like to return to the user.
+ */
+ public String saveFile(File directory) throws IOException {
+ String options = "(Y/N)";
+ File file = new File(directory, filename).getAbsoluteFile();
- File parentDirectory = fileToDumpData.getParentFile();
- if (parentDirectory != null) {
- parentDirectory.mkdirs();
- }
Gfsh gfsh = Gfsh.getCurrentInstance();
- if (fileToDumpData.exists()) {
- String fileExistsMessage =
- CliStrings.format(CliStrings.ABSTRACTRESULTDATA__MSG__FILE_WITH_NAME_0_EXISTS_IN_1,
- filename, fileToDumpData.getParent(), options);
+ if (file.exists()) {
+ String fileExistsMessage = String.format("File with name \"$s\" already exists in \"$s\".",
+ filename, directory.getAbsolutePath());
if (gfsh != null && !gfsh.isQuietMode()) {
- fileExistsMessage = fileExistsMessage + " Overwrite? " + options + " : ";
+ fileExistsMessage += " Overwrite? " + options + " : ";
String interaction = gfsh.interact(fileExistsMessage);
if (!"y".equalsIgnoreCase(interaction.trim())) {
// do not save file & continue
- return;
+ return "User aborted. Did not overwrite " + file.getAbsolutePath();
}
} else {
- throw new IOException(fileExistsMessage);
+ return fileExistsMessage;
}
- } else if (!parentDirectory.exists()) {
- handleCondition(CliStrings.format(
- CliStrings.ABSTRACTRESULTDATA__MSG__PARENT_DIRECTORY_OF_0_DOES_NOT_EXIST,
- fileToDumpData.getAbsolutePath()));
- return;
- } else if (!parentDirectory.canWrite()) {
- handleCondition(CliStrings.format(
- CliStrings.ABSTRACTRESULTDATA__MSG__PARENT_DIRECTORY_OF_0_IS_NOT_WRITABLE,
- fileToDumpData.getAbsolutePath()));
- return;
- } else if (!parentDirectory.isDirectory()) {
- handleCondition(
- CliStrings.format(CliStrings.ABSTRACTRESULTDATA__MSG__PARENT_OF_0_IS_NOT_DIRECTORY,
- fileToDumpData.getAbsolutePath()));
- return;
}
if (type == ResultData.FILE_TYPE_TEXT) {
- FileWriter fw = new FileWriter(fileToDumpData);
+ FileWriter fw = new FileWriter(file);
BufferedWriter bw = new BufferedWriter(fw);
bw.write(new String(data));
bw.flush();
fw.flush();
fw.close();
} else if (type == ResultData.FILE_TYPE_BINARY) {
- FileOutputStream fos = new FileOutputStream(fileToDumpData);
+ FileOutputStream fos = new FileOutputStream(file);
fos.write(data);
fos.flush();
fos.close();
}
- if (message != null && !message.isEmpty()) {
- if (gfsh != null) {
- Gfsh.println(MessageFormat.format(message, fileToDumpData.getAbsolutePath()));
- }
- }
- }
-
- private void handleCondition(String message) throws IOException {
- Gfsh gfsh = Gfsh.getCurrentInstance();
- // null check required in GfshVM too to avoid test issues
- if (gfsh != null && !gfsh.isQuietMode()) {
- gfsh.logWarning(message, null);
- } else {
- throw new IOException(message);
- }
+ return "File saved to " + file.getAbsolutePath();
}
}
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 9a20b62..d416c32 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
@@ -146,8 +146,8 @@ public class ResultModel {
this.files = files;
}
- public void addFile(String fileName, String content, String message) {
- FileResultModel fileModel = new FileResultModel(fileName, content, message);
+ public void addFile(String fileName, String content) {
+ FileResultModel fileModel = new FileResultModel(fileName, content);
files.put(fileName, fileModel);
}
@@ -384,4 +384,34 @@ public class ResultModel {
return result;
}
+ /**
+ * this saves the file data in this result model to the specified directory, and add appropriate
+ * information to the result model to indicate the result of the file save.
+ *
+ */
+ public void saveFileTo(File dir) throws IOException {
+ if (getFiles().size() == 0 || dir == null) {
+ return;
+ }
+ InfoResultModel info = addInfo("fileSave");
+ if (!dir.exists() && !dir.mkdirs()) {
+ info.addLine(dir.getAbsolutePath() + " can not be created.");
+ setStatus(Result.Status.ERROR);
+ return;
+ }
+ if (!dir.isDirectory()) {
+ info.addLine(dir.getAbsolutePath() + " is not a directory.");
+ setStatus(Result.Status.ERROR);
+ return;
+ }
+ if (!dir.canWrite()) {
+ info.addLine("Can not write to " + dir.getAbsolutePath());
+ setStatus(Result.Status.ERROR);
+ return;
+ }
+
+ for (FileResultModel fileResult : files.values()) {
+ info.addLine(fileResult.saveFile(dir));
+ }
+ }
}
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 cb3a369..ca9aeb9 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
@@ -58,6 +58,7 @@ import org.apache.geode.management.internal.cli.GfshParser;
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.shell.jline.ANSIHandler;
import org.apache.geode.management.internal.cli.shell.jline.ANSIHandler.ANSIStyle;
@@ -708,7 +709,10 @@ public class Gfsh extends JLineShell {
resultTypeTL.set(null);
- if (result instanceof CommandResult) {
+ // this only saves the incoming file to the user.dir. We should not rely on this
+ // to save the exported file at this point. All file saving should be done in the
+ // specific command's postExecutor
+ if (result instanceof LegacyCommandResult) {
CommandResult cmdResult = (CommandResult) result;
if (cmdResult.hasIncomingFiles()) {
boolean isAlreadySaved = cmdResult.getNumTimesSaved() > 0;
diff --git a/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/ResultModelAssert.java b/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/ResultModelAssert.java
new file mode 100644
index 0000000..99b8b8d
--- /dev/null
+++ b/geode-junit/src/main/java/org/apache/geode/test/junit/assertions/ResultModelAssert.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.test.junit.assertions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.assertj.core.api.AbstractAssert;
+
+import org.apache.geode.management.internal.cli.result.model.DataResultModel;
+import org.apache.geode.management.internal.cli.result.model.InfoResultModel;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
+import org.apache.geode.management.internal.cli.result.model.TabularResultModel;
+
+public class ResultModelAssert extends AbstractAssert<ResultModelAssert, ResultModel> {
+
+ public ResultModelAssert(ResultModel infoResultModel) {
+ super(infoResultModel, ResultModelAssert.class);
+ }
+
+ public InfoResultModelAssert hasInfoResultModel(String name) {
+ InfoResultModel infoSection = actual.getInfoSection(name);
+ assertThat(infoSection).isNotNull();
+ return new InfoResultModelAssert(infoSection);
+ }
+
+ public DataResultModelAssert hasDateResultModel(String name) {
+ DataResultModel dataSection = actual.getDataSection(name);
+ assertThat(dataSection).isNotNull();
+ return new DataResultModelAssert(dataSection);
+ }
+
+ public TabularResultModelAssert hasTabularResultModel(String name) {
+ TabularResultModel dataSection = actual.getTableSection(name);
+ assertThat(dataSection).isNotNull();
+ return new TabularResultModelAssert(dataSection);
+ }
+}
diff --git a/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommandDUnitTest.java b/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommandDUnitTest.java
index ba5df56..c5090c3 100644
--- a/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommandDUnitTest.java
+++ b/geode-web/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ExportConfigCommandDUnitTest.java
@@ -85,7 +85,7 @@ public class ExportConfigCommandDUnitTest {
// export just one member's config
tempDir = temporaryFolder.newFolder("member0");
gfsh.executeAndAssertThat("export config --member=server-0 --dir=" + tempDir.getAbsolutePath())
- .statusIsSuccess();
+ .statusIsSuccess().containsOutput(tempDir.getAbsolutePath());
expectedFiles = Arrays.asList("server-0-cache.xml", "server-0-gf.properties");
@@ -97,7 +97,7 @@ public class ExportConfigCommandDUnitTest {
// export group2 config into a folder
tempDir = temporaryFolder.newFolder("group2");
gfsh.executeAndAssertThat("export config --group=Group2 --dir=" + tempDir.getAbsolutePath())
- .statusIsSuccess();
+ .statusIsSuccess().containsOutput(tempDir.getAbsolutePath());
expectedFiles = Arrays.asList("server-1-cache.xml", "server-2-cache.xml",
"server-1-gf.properties", "server-2-gf.properties");
diff --git a/geode-web/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpTest.java b/geode-web/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpTest.java
index 6d57377..0285b1e 100644
--- a/geode-web/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpTest.java
+++ b/geode-web/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/CommandOverHttpTest.java
@@ -18,6 +18,7 @@ package org.apache.geode.management.internal.cli.commands;
import static org.assertj.core.api.Assertions.assertThat;
import java.io.File;
+import java.nio.file.Paths;
import org.junit.Before;
import org.junit.ClassRule;
@@ -86,7 +87,7 @@ public class CommandOverHttpTest {
public void exportConfig() throws Exception {
String dir = temporaryFolder.getRoot().getAbsolutePath();
gfshRule.executeAndAssertThat("export config --dir=" + dir).statusIsSuccess()
- .containsOutput("Downloading Cache XML file: server-cache.xml")
- .containsOutput("Downloading properties file: server-gf.properties");
+ .containsOutput("File saved to " + Paths.get(dir, "server-cache.xml").toString())
+ .containsOutput("File saved to " + Paths.get(dir, "server-gf.properties").toString());
}
}