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/23 22:35:17 UTC

[geode] branch develop updated: GEODE-5971: Remove FileResult (#3487)

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 323cd0d  GEODE-5971: Remove FileResult (#3487)
323cd0d is described below

commit 323cd0d52ffbf7e0d9837cd5cb3953f1a9e76b05
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Tue Apr 23 15:35:01 2019 -0700

    GEODE-5971: Remove FileResult (#3487)
---
 .../jdbc/internal/cli/CreateMappingCommand.java    |  10 +-
 .../cli/CreateMappingCommandInterceptorTest.java   |  13 +--
 .../cli/commands/ShowMetricsDUnitTest.java         |   2 +-
 .../result/model/ResultModelIntegrationTest.java   |   6 +-
 .../management/internal/cli/CommandRequest.java    |   2 +-
 .../internal/cli/commands/DeployCommand.java       |  18 ++--
 .../internal/cli/commands/ExportLogsCommand.java   |   3 +-
 .../ImportClusterConfigurationCommand.java         |   8 +-
 .../internal/cli/commands/ShowDeadlockCommand.java |  28 ++----
 .../cli/commands/ShowMetricsInterceptor.java       |  20 ++--
 .../management/internal/cli/result/FileResult.java | 101 ---------------------
 .../internal/cli/result/model/FileResultModel.java |  50 ++++++----
 .../internal/cli/result/model/ResultModel.java     |  85 ++++++++++++-----
 .../internal/cli/shell/GfshExecutionStrategy.java  |   7 +-
 .../internal/cli/result/FileResultTest.java        |  16 ++--
 .../internal/cli/result/model/ResultModelTest.java |  19 ++++
 16 files changed, 165 insertions(+), 223 deletions(-)

diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
index 3607a89..d761be7 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
@@ -58,7 +58,7 @@ import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.remote.CommandExecutionContext;
-import org.apache.geode.management.internal.cli.result.FileResult;
+import org.apache.geode.management.internal.cli.result.model.FileResultModel;
 import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission;
@@ -430,14 +430,14 @@ public class CreateMappingCommand extends SingleGfshCommand {
   public static class Interceptor extends AbstractCliAroundInterceptor {
 
     @Override
-    public Object preExecution(GfshParseResult parseResult) {
+    public ResultModel preExecution(GfshParseResult parseResult) {
       String pdxClassFileName = (String) parseResult.getParamValue(CREATE_MAPPING__PDX_CLASS_FILE);
 
       if (StringUtils.isBlank(pdxClassFileName)) {
         return ResultModel.createInfo("");
       }
 
-      FileResult fileResult = new FileResult();
+      ResultModel result = new ResultModel();
       File pdxClassFile = new File(pdxClassFileName);
       if (!pdxClassFile.exists()) {
         return ResultModel.createError(pdxClassFile + " not found.");
@@ -449,9 +449,9 @@ public class CreateMappingCommand extends SingleGfshCommand {
       if (!fileExtension.equalsIgnoreCase("jar") && !fileExtension.equalsIgnoreCase("class")) {
         return ResultModel.createError(pdxClassFile + " must end with \".jar\" or \".class\".");
       }
-      fileResult.addFile(pdxClassFile);
+      result.addFile(pdxClassFile, FileResultModel.FILE_TYPE_FILE);
 
-      return fileResult;
+      return result;
     }
   }
 
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandInterceptorTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandInterceptorTest.java
index 05c898a..c4361a6 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandInterceptorTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandInterceptorTest.java
@@ -29,7 +29,6 @@ import org.junit.rules.TemporaryFolder;
 import org.apache.geode.connectors.util.internal.MappingConstants;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.GfshParseResult;
-import org.apache.geode.management.internal.cli.result.FileResult;
 import org.apache.geode.management.internal.cli.result.model.ResultModel;
 
 public class CreateMappingCommandInterceptorTest {
@@ -86,11 +85,9 @@ public class CreateMappingCommandInterceptorTest {
     File tempFile = testFolder.newFile("tempFile.class");
     when(gfshParseResult.getParamValue(MappingConstants.PDX_CLASS_FILE))
         .thenReturn(tempFile.getAbsolutePath());
-    Result result = (Result) interceptor.preExecution(gfshParseResult);
+    ResultModel result = interceptor.preExecution(gfshParseResult);
     assertThat(result.getStatus()).isEqualTo(Result.Status.OK);
-    assertThat(result).isInstanceOf(FileResult.class);
-    FileResult fileResult = (FileResult) result;
-    assertThat(fileResult.getFiles()).containsExactly(tempFile);
+    assertThat(result.getFileList()).containsExactly(tempFile);
   }
 
   @Test
@@ -98,11 +95,9 @@ public class CreateMappingCommandInterceptorTest {
     File tempFile = testFolder.newFile("tempFile.jar");
     when(gfshParseResult.getParamValue(MappingConstants.PDX_CLASS_FILE))
         .thenReturn(tempFile.getAbsolutePath());
-    Result result = (Result) interceptor.preExecution(gfshParseResult);
+    ResultModel result = interceptor.preExecution(gfshParseResult);
     assertThat(result.getStatus()).isEqualTo(Result.Status.OK);
-    assertThat(result).isInstanceOf(FileResult.class);
-    FileResult fileResult = (FileResult) result;
-    assertThat(fileResult.getFiles()).containsExactly(tempFile);
+    assertThat(result.getFileList()).containsExactly(tempFile);
   }
 
 }
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ShowMetricsDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ShowMetricsDUnitTest.java
index 7db9a3d..56e90e9 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ShowMetricsDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/ShowMetricsDUnitTest.java
@@ -139,7 +139,7 @@ public class ShowMetricsDUnitTest {
     gfsh.executeAndAssertThat("show metrics --member=" + server.getName() + " --port="
         + server.getPort() + " --file=" + output.getAbsolutePath()).statusIsSuccess()
         .containsOutput("Member Metrics").containsOutput("cacheserver")
-        .containsOutput("Metrics saved to: " + output.getAbsolutePath());
+        .containsOutput("File saved to " + output.getAbsolutePath());
 
     assertThat(output).exists();
   }
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
index e298c7b..acaa763 100644
--- 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
@@ -16,6 +16,7 @@
 package org.apache.geode.management.internal.cli.result.model;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.io.File;
 import java.io.IOException;
@@ -50,9 +51,8 @@ public class ResultModelIntegrationTest {
   }
 
   @Test
-  public void savesToNoWhereDoesNothing() throws IOException {
-    result.saveFileTo(null);
-    assertThat(result.getInfoSections()).hasSize(0);
+  public void savesToNullThrowException() throws IOException {
+    assertThatThrownBy(() -> result.saveFileTo(null)).isInstanceOf(NullPointerException.class);
   }
 
   @Test
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandRequest.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandRequest.java
index 836fa7f..a4873ac 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandRequest.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandRequest.java
@@ -80,7 +80,7 @@ public class CommandRequest {
   }
 
   public boolean hasFileList() {
-    return (getFileList() != null);
+    return (getFileList() != null && getFileList().size() > 0);
   }
 
   protected GfshParseResult getParseResult() {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
index 62b0033..a9b4a1e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
@@ -52,7 +52,7 @@ import org.apache.geode.management.internal.cli.functions.DeployFunction;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.remote.CommandExecutionContext;
 import org.apache.geode.management.internal.cli.remote.CommandExecutor;
-import org.apache.geode.management.internal.cli.result.FileResult;
+import org.apache.geode.management.internal.cli.result.model.FileResultModel;
 import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.management.internal.cli.result.model.TabularResultModel;
 import org.apache.geode.management.internal.security.ResourceOperation;
@@ -168,7 +168,7 @@ public class DeployCommand extends GfshCommand {
      * @return FileResult object or ResultModel in case of errors
      */
     @Override
-    public Object preExecution(GfshParseResult parseResult) {
+    public ResultModel preExecution(GfshParseResult parseResult) {
       String[] jars = (String[]) parseResult.getParamValue("jar");
       String dir = (String) parseResult.getParamValue("dir");
 
@@ -181,14 +181,14 @@ public class DeployCommand extends GfshCommand {
         return ResultModel.createError("Parameters \"jar\" and \"dir\" can not both be specified.");
       }
 
-      FileResult fileResult = new FileResult();
+      ResultModel result = new ResultModel();
       if (jars != null) {
         for (String jar : jars) {
           File jarFile = new File(jar);
           if (!jarFile.exists()) {
             return ResultModel.createError(jar + " not found.");
           }
-          fileResult.addFile(jarFile);
+          result.addFile(jarFile, FileResultModel.FILE_TYPE_FILE);
         }
       } else {
         File fileDir = new File(dir);
@@ -197,22 +197,22 @@ public class DeployCommand extends GfshCommand {
         }
         File[] childJarFile = fileDir.listFiles(CliUtil.JAR_FILE_FILTER);
         for (File file : childJarFile) {
-          fileResult.addFile(file);
+          result.addFile(file, FileResultModel.FILE_TYPE_FILE);
         }
       }
 
       // check if user wants to upload with the computed file size
       String message =
-          "\nDeploying files: " + fileResult.getFormattedFileList() + "\nTotal file size is: "
-              + this.numFormatter.format((double) fileResult.computeFileSizeTotal() / ONE_MB)
+          "\nDeploying files: " + result.getFormattedFileList() + "\nTotal file size is: "
+              + this.numFormatter.format((double) result.computeFileSizeTotal() / ONE_MB)
               + "MB\n\nContinue? ";
 
       if (readYesNo(message, Response.YES) == Response.NO) {
         return ResultModel.createError(
-            "Aborted deploy of " + fileResult.getFormattedFileList() + ".");
+            "Aborted deploy of " + result.getFormattedFileList() + ".");
       }
 
-      return fileResult;
+      return result;
     }
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
index f0939f4..03c89b5 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
@@ -42,6 +42,7 @@ import org.apache.geode.management.cli.GfshCommand;
 import org.apache.geode.management.internal.cli.functions.ExportLogsFunction;
 import org.apache.geode.management.internal.cli.functions.SizeExportLogsFunction;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.model.FileResultModel;
 import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.management.internal.cli.util.ExportLogsCacheWriter;
 import org.apache.geode.management.internal.configuration.utils.ZipUtils;
@@ -195,7 +196,7 @@ public class ExportLogsCommand extends GfshCommand {
       FileUtils.deleteDirectory(tempDir.toFile());
 
       ResultModel result = new ResultModel();
-      result.setFileToDownload(exportedLogsZipFile);
+      result.addFile(exportedLogsZipFile.toFile(), FileResultModel.FILE_TYPE_FILE);
       return result;
     } finally {
       ExportLogsFunction.destroyExportLogsRegion(cache);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportClusterConfigurationCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportClusterConfigurationCommand.java
index 69c9a64..c75a7e3 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportClusterConfigurationCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ImportClusterConfigurationCommand.java
@@ -48,7 +48,7 @@ import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.remote.CommandExecutionContext;
-import org.apache.geode.management.internal.cli.result.FileResult;
+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;
@@ -201,7 +201,7 @@ public class ImportClusterConfigurationCommand extends GfshCommand {
 
   public static class ImportInterceptor extends AbstractCliAroundInterceptor {
     @Override
-    public Object preExecution(GfshParseResult parseResult) {
+    public ResultModel preExecution(GfshParseResult parseResult) {
       String zip = parseResult.getParamValueAsString(CliStrings.IMPORT_SHARED_CONFIG__ZIP);
       String xmlFile = parseResult.getParamValueAsString(XML_FILE);
       String group = parseResult.getParamValueAsString(CliStrings.GROUP);
@@ -260,8 +260,8 @@ public class ImportClusterConfigurationCommand extends GfshCommand {
         }
       }
 
-      FileResult result = new FileResult();
-      result.addFile(importedFile);
+      ResultModel result = new ResultModel();
+      result.addFile(importedFile, FileResultModel.FILE_TYPE_FILE);
       return result;
     }
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowDeadlockCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowDeadlockCommand.java
index fb38063..9155b75 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowDeadlockCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowDeadlockCommand.java
@@ -15,11 +15,10 @@
 
 package org.apache.geode.management.internal.cli.commands;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
-import java.text.MessageFormat;
 import java.util.Collection;
-import java.util.Map;
 import java.util.Set;
 
 import org.springframework.shell.core.annotation.CliCommand;
@@ -32,11 +31,9 @@ import org.apache.geode.distributed.internal.deadlock.DependencyGraph;
 import org.apache.geode.distributed.internal.deadlock.GemFireDeadlockDetector;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.GfshCommand;
-import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.CliAroundInterceptor;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 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;
@@ -94,25 +91,12 @@ public class ShowDeadlockCommand extends GfshCommand {
   public static class Interceptor implements CliAroundInterceptor {
     @Override
     public ResultModel postExecution(GfshParseResult parseResult, ResultModel resultModel,
-        Path tempFile) {
-
-      if (resultModel.getFiles().size() != 1) {
-        resultModel.addInfo("No filename found to save");
-        resultModel.setStatus(Result.Status.ERROR);
-        return resultModel;
-      }
-
-      try {
-        for (Map.Entry<String, FileResultModel> entry : resultModel.getFiles().entrySet()) {
-          entry.getValue().saveFile();
-          resultModel.addInfo().addLine(
-              MessageFormat.format(CliStrings.SHOW_DEADLOCK__DEPENDENCIES__REVIEW, entry.getKey()));
-        }
-      } catch (IOException e) {
-        resultModel.addInfo().addLine("Unable to save file: " + e.getMessage());
-        resultModel.setStatus(Result.Status.ERROR);
-      }
+        Path tempFile) throws IOException {
+      String saveAs =
+          parseResult.getParamValueAsString(CliStrings.SHOW_DEADLOCK__DEPENDENCIES__FILE);
 
+      File file = new File(saveAs).getAbsoluteFile();
+      resultModel.saveFileTo(file.getParentFile());
       return resultModel;
     }
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMetricsInterceptor.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMetricsInterceptor.java
index 0a1b44f..f6a0575 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMetricsInterceptor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShowMetricsInterceptor.java
@@ -14,21 +14,19 @@
  */
 package org.apache.geode.management.internal.cli.commands;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.AbstractCliAroundInterceptor;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.commands.ShowMetricsCommand.Category;
 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.ResultModel;
 
 public class ShowMetricsInterceptor extends AbstractCliAroundInterceptor {
@@ -106,17 +104,15 @@ public class ShowMetricsInterceptor extends AbstractCliAroundInterceptor {
 
   @Override
   public ResultModel postExecution(GfshParseResult parseResult, ResultModel resultModel,
-      Path tempFile) {
-    try {
-      for (Map.Entry<String, FileResultModel> entry : resultModel.getFiles().entrySet()) {
-        entry.getValue().saveFile();
-        resultModel.addInfo().addLine("Metrics saved to: " + entry.getKey());
-      }
-    } catch (IOException e) {
-      resultModel.addInfo().addLine("Unable to save file: " + e.getMessage());
-      resultModel.setStatus(Result.Status.ERROR);
+      Path tempFile) throws IOException {
+    String saveAs = parseResult.getParamValueAsString(CliStrings.SHOW_METRICS__FILE);
+
+    if (saveAs == null) {
+      return resultModel;
     }
 
+    File file = new File(saveAs).getAbsoluteFile();
+    resultModel.saveFileTo(file.getParentFile());
     return resultModel;
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/FileResult.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/FileResult.java
deleted file mode 100644
index 3d5db9b..0000000
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/result/FileResult.java
+++ /dev/null
@@ -1,101 +0,0 @@
-/*
- * 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;
-
-import java.io.File;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.stream.Collectors;
-
-import org.apache.geode.management.cli.Result;
-
-/**
- *
- *
- * @since GemFire 7.0
- */
-public class FileResult implements Result {
-  private List<File> files = new ArrayList<>();
-
-  public void addFile(File file) {
-    files.add(file);
-  }
-
-  @Override
-  public Status getStatus() {
-    return Status.OK;
-  }
-
-  @Override
-  public void resetToFirstLine() {}
-
-  @Override
-  public boolean hasNextLine() {
-    return false;
-  }
-
-  @Override
-  public String nextLine() {
-    return "";
-  }
-
-  public List<File> getFiles() {
-    return files;
-  }
-
-  /**
-   * Calculates the total file size of all files associated with this result.
-   *
-   * @return Total file size.
-   */
-  public long computeFileSizeTotal() {
-    long byteCount = 0;
-    for (File file : files) {
-      byteCount += file.length();
-    }
-    return byteCount;
-  }
-
-  /**
-   * Get a comma separated list of all files associated with this result.
-   *
-   * @return Comma separated list of files.
-   */
-  public String getFormattedFileList() {
-    return files.stream().map(File::getName).collect(Collectors.joining(", "));
-  }
-
-  @Override
-  public boolean hasIncomingFiles() {
-    return true;
-  }
-
-  @Override
-  public void saveIncomingFiles(String directory)
-      throws UnsupportedOperationException, IOException {
-    throw new UnsupportedOperationException("not supported");
-  }
-
-  @Override
-  public boolean failedToPersist() {
-    return false;
-  }
-
-  @Override
-  public void setCommandPersisted(boolean commandPersisted) {
-    throw new UnsupportedOperationException("not supported");
-  }
-}
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 2f1613c..a158d24 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
@@ -21,7 +21,9 @@ import java.io.FileOutputStream;
 import java.io.FileWriter;
 import java.io.IOException;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
 
 import org.apache.geode.management.internal.cli.result.ResultData;
 import org.apache.geode.management.internal.cli.shell.Gfsh;
@@ -33,38 +35,46 @@ import org.apache.geode.management.internal.cli.shell.Gfsh;
 public class FileResultModel {
   public static final int FILE_TYPE_BINARY = 0;
   public static final int FILE_TYPE_TEXT = 1;
+  public static final int FILE_TYPE_FILE = 2;
 
+  // this saves only the name part of the file, no directory info is saved.
   private String filename;
   private int type;
+
+  // some commands saves file data in bytes like ExportConfigCommand
   private byte[] data;
-  private int length;
+
+  // some commands has files to download over http, like ExportLogCommand
+  // it's also used for uploading files by the preExecutor (like DeployCommand)
+  private File file;
 
   /**
    * This unused constructor is actually used for deserialization
    */
   public FileResultModel() {}
 
-  /**
-   * @param fileName only the name of the file, should not include directory information
-   */
   public FileResultModel(String fileName, String content) {
-    this.filename = fileName;
+    this.filename = FilenameUtils.getName(fileName);
     this.data = content.getBytes();
     this.type = FILE_TYPE_TEXT;
   }
 
   public FileResultModel(File file, int fileType) {
-    if (fileType != FILE_TYPE_BINARY && fileType != FILE_TYPE_TEXT) {
+    if (fileType != FILE_TYPE_BINARY && fileType != FILE_TYPE_FILE) {
       throw new IllegalArgumentException("Unsupported file type is specified.");
     }
 
-    this.filename = file.getName();
+    this.filename = FilenameUtils.getName(file.getName());
     this.type = fileType;
 
-    try {
-      this.data = FileUtils.readFileToByteArray(file);
-    } catch (IOException e) {
-      throw new RuntimeException("Unable to read file: " + file.getAbsolutePath(), e);
+    if (fileType == FILE_TYPE_BINARY) {
+      try {
+        this.data = FileUtils.readFileToByteArray(file);
+      } catch (IOException e) {
+        throw new RuntimeException("Unable to read file: " + file.getAbsolutePath(), e);
+      }
+    } else {
+      this.file = file;
     }
   }
 
@@ -92,17 +102,17 @@ public class FileResultModel {
     this.data = data;
   }
 
-  public int getLength() {
-    return this.data.length;
+  @JsonIgnore
+  public long getLength() {
+    if (type == FILE_TYPE_BINARY || type == FILE_TYPE_TEXT) {
+      return this.data.length;
+    } else {
+      return this.file.length();
+    }
   }
 
-  /**
-   * Save the file to the current working directory
-   *
-   * @return the message you would like to return to the user.
-   */
-  public String saveFile() throws IOException {
-    return saveFile(null);
+  public File getFile() {
+    return file;
   }
 
   /**
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 4988346..5a7b77c 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
@@ -76,13 +76,7 @@ public class ResultModel {
   private Map<String, AbstractResultModel> sections = new LinkedHashMap<>();
   private Result.Status status = Result.Status.OK;
   private Object configObject;
-  // this is used by commands (e.g. ExportConfigCommand) saving the file content in the memory and
-  // transfer the byte[] in json string back to the client
-  private Map<String, FileResultModel> files = new LinkedHashMap<>();
-
-  // this is used by commands (e.g ExportlogsCommand) that can download file to the client when in
-  // http connection
-  private Path fileToDownload;
+  private List<FileResultModel> files = new ArrayList<>();
 
   @JsonIgnore
   public Object getConfigObject() {
@@ -145,29 +139,25 @@ public class ResultModel {
     this.sections = content;
   }
 
-  public Map<String, FileResultModel> getFiles() {
+  public List<FileResultModel> getFiles() {
     return files;
   }
 
-  public void setFiles(Map<String, FileResultModel> files) {
+  public void setFiles(List<FileResultModel> files) {
     this.files = files;
   }
 
+  /**
+   * @param fileName: only the name part of the file, no directory infomation
+   * @param content: the content to be saved to the file
+   */
   public void addFile(String fileName, String content) {
     FileResultModel fileModel = new FileResultModel(fileName, content);
-    files.put(fileName, fileModel);
+    files.add(fileModel);
   }
 
   public void addFile(File file, int fileType) {
-    files.put(file.getName(), new FileResultModel(file, fileType));
-  }
-
-  public Path getFileToDownload() {
-    return fileToDownload;
-  }
-
-  public void setFileToDownload(Path fileToDownload) {
-    this.fileToDownload = fileToDownload;
+    files.add(new FileResultModel(file, fileType));
   }
 
   /**
@@ -362,8 +352,6 @@ public class ResultModel {
     return result;
   }
 
-
-
   public static ResultModel createMemberStatusResult(List<CliFunctionResult> functionResults,
       boolean ignoreIgnorable, boolean ignorePartialFailure) {
     return createMemberStatusResult(functionResults, null, null, ignoreIgnorable,
@@ -403,13 +391,18 @@ public class ResultModel {
   /**
    * 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.
+   * this only applies to the commands that saves the file content in byte[], not download the
+   * files over http channel
    *
    */
   public void saveFileTo(File dir) throws IOException {
-    if (getFiles().size() == 0 || dir == null) {
+    InfoResultModel info = addInfo("fileSave");
+    if (files.size() == 0) {
+      info.addLine("No file found to be saved.");
+      setStatus(Result.Status.ERROR);
       return;
     }
-    InfoResultModel info = addInfo("fileSave");
+
     if (!dir.exists() && !dir.mkdirs()) {
       info.addLine(dir.getAbsolutePath() + " can not be created.");
       setStatus(Result.Status.ERROR);
@@ -426,8 +419,52 @@ public class ResultModel {
       return;
     }
 
-    for (FileResultModel fileResult : files.values()) {
+    for (FileResultModel fileResult : files) {
       info.addLine(fileResult.saveFile(dir));
     }
+
+  }
+
+  @JsonIgnore
+  public Path getFileToDownload() {
+    if (files.size() != 1) {
+      return null;
+    }
+    File file = files.get(0).getFile();
+    if (file == null) {
+      return null;
+    }
+    return file.toPath();
+  }
+
+  @JsonIgnore
+  public List<File> getFileList() {
+    return files.stream().filter(f -> f.getFile() != null).map(FileResultModel::getFile)
+        .collect(
+            Collectors.toList());
+  }
+
+  /**
+   * Calculates the total file size of all files associated with this result.
+   *
+   * @return Total file size.
+   */
+  public long computeFileSizeTotal() {
+    long byteCount = 0;
+    for (FileResultModel file : files) {
+      byteCount += file.getLength();
+    }
+    return byteCount;
+  }
+
+  /**
+   * Get a comma separated list of all files associated with this result.
+   *
+   * @return Comma separated list of files.
+   */
+  @JsonIgnore
+  public String getFormattedFileList() {
+    return files.stream().map(FileResultModel::getFilename)
+        .collect(Collectors.joining(", "));
   }
 }
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 b33b099..36d7cb9 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
@@ -39,7 +39,6 @@ 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.FileResult;
 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;
@@ -188,9 +187,9 @@ public class GfshExecutionStrategy implements ExecutionStrategy {
       }
 
       // when the preExecution yields a FileResult, we will get the fileData out of it
-      if (preExecResult instanceof FileResult) {
-        FileResult fileResult = (FileResult) preExecResult;
-        fileData = fileResult.getFiles();
+      if (preExecResult instanceof ResultModel) {
+        ResultModel fileResult = (ResultModel) preExecResult;
+        fileData = fileResult.getFileList();
       }
     }
 
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/FileResultTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/FileResultTest.java
index b4fee14..903232a 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/FileResultTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/result/FileResultTest.java
@@ -22,21 +22,23 @@ import java.io.File;
 import org.junit.Before;
 import org.junit.Test;
 
+import org.apache.geode.management.internal.cli.result.model.FileResultModel;
+import org.apache.geode.management.internal.cli.result.model.ResultModel;
 
 
 public class FileResultTest {
 
-  private FileResult fileResult;
+  private ResultModel fileResult;
 
   @Before
   public void before() {
-    fileResult = new FileResult();
+    fileResult = new ResultModel();
   }
 
   @Test
   public void getFormattedFileList() {
-    fileResult.addFile(new File("file1.txt"));
-    fileResult.addFile(new File("file2.txt"));
+    fileResult.addFile(new File("file1.txt"), FileResultModel.FILE_TYPE_FILE);
+    fileResult.addFile(new File("file2.txt"), FileResultModel.FILE_TYPE_FILE);
     assertThat(fileResult.getFormattedFileList()).isEqualTo("file1.txt, file2.txt");
   }
 
@@ -46,8 +48,8 @@ public class FileResultTest {
 
     File file1 = new File("file1.txt");
     File file2 = new File("file2.txt");
-    fileResult.addFile(file1);
-    fileResult.addFile(file2);
-    assertThat(fileResult.getFiles()).containsExactlyInAnyOrder(file1, file2);
+    fileResult.addFile(file1, FileResultModel.FILE_TYPE_FILE);
+    fileResult.addFile(file2, FileResultModel.FILE_TYPE_FILE);
+    assertThat(fileResult.getFileList()).containsExactlyInAnyOrder(file1, file2);
   }
 }
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 9f11da4..b5e29c4 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
@@ -20,14 +20,19 @@ import static org.apache.geode.management.internal.cli.functions.CliFunctionResu
 import static org.apache.geode.management.internal.cli.functions.CliFunctionResult.StatusState.OK;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.File;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.util.internal.GeodeJsonMapper;
 
 
 public class ResultModelTest {
@@ -35,6 +40,9 @@ public class ResultModelTest {
   private ResultModel result;
   private TabularResultModel table;
 
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
   @Before
   public void setUp() throws Exception {
     result = new ResultModel();
@@ -162,4 +170,15 @@ public class ResultModelTest {
     List<String> sectionNames = result.getSectionNames();
     assertThat(sectionNames).containsExactly("Section1", "Section2", "Section3", "Section4");
   }
+
+  @Test
+  public void serializeFileToDownload() throws Exception {
+    File file = temporaryFolder.newFile("test.log");
+    result.addFile(file, FileResultModel.FILE_TYPE_FILE);
+    ObjectMapper mapper = GeodeJsonMapper.getMapper();
+    String json = mapper.writeValueAsString(result);
+    System.out.println(json);
+    ResultModel resultModel = mapper.readValue(json, ResultModel.class);
+    assertThat(resultModel.getFileToDownload()).isEqualTo(file.toPath());
+  }
 }