You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kh...@apache.org on 2017/03/20 20:05:53 UTC

geode git commit: Refacotring for testability. Corrected size unit multiplier. Added TestSuite to consolidate testing of ExportLogs command.

Repository: geode
Updated Branches:
  refs/heads/feature/GEODE-2420 7c6531e88 -> 81bb0e5c1


Refacotring for testability.
Corrected size unit multiplier.
Added TestSuite to consolidate testing of ExportLogs command.


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/81bb0e5c
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/81bb0e5c
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/81bb0e5c

Branch: refs/heads/feature/GEODE-2420
Commit: 81bb0e5c153f5cf98240322aa14f4f8786d30dd6
Parents: 7c6531e
Author: Ken Howe <kh...@pivotal.io>
Authored: Mon Mar 20 13:02:56 2017 -0700
Committer: Ken Howe <kh...@pivotal.io>
Committed: Mon Mar 20 13:02:56 2017 -0700

----------------------------------------------------------------------
 .../cli/commands/ExportLogsCommand.java         | 39 +++++++++++++-------
 .../internal/cli/i18n/CliStrings.java           |  3 ++
 .../commands/ExportLogsFileSizeLimitTest.java   | 14 ++++++-
 .../cli/commands/ExportLogsTestSuite.java       | 26 +++++++++++++
 4 files changed, 67 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/81bb0e5c/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
----------------------------------------------------------------------
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 9f5f9fd..81c8a6c 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
@@ -93,8 +93,9 @@ public class ExportLogsCommand implements CommandMarker {
       @CliOption(key = CliStrings.EXPORT_LOGS__STATSONLY, unspecifiedDefaultValue = "false",
           specifiedDefaultValue = "true",
           help = CliStrings.EXPORT_LOGS__STATSONLY__HELP) boolean statsOnly,
-      @CliOption(key = CliStrings.EXPORT_LOGS__FILESIZELIMIT, unspecifiedDefaultValue = "false",
-          specifiedDefaultValue = "true",
+      @CliOption(key = CliStrings.EXPORT_LOGS__FILESIZELIMIT,
+          unspecifiedDefaultValue = CliStrings.EXPORT_LOGS__FILESIZELIMIT__UNSPECIFIED_DEFAULT,
+          specifiedDefaultValue = CliStrings.EXPORT_LOGS__FILESIZELIMIT__SPECIFIED_DEFAULT,
           help = CliStrings.EXPORT_LOGS__FILESIZELIMIT__HELP) String fileSizeLimit) {
     Result result = null;
     GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
@@ -154,11 +155,16 @@ public class ExportLogsCommand implements CommandMarker {
       ZipUtils.zipDirectory(exportedLogsDir, exportedLogsZipFile);
       FileUtils.deleteDirectory(tempDir.toFile());
 
-      checkOverDiskSpaceThreshold(parseFileSizeLimit(fileSizeLimit), exportedLogsZipFile.toFile());
+//      try {
+        isFileSizeCheckEnabledAndWithinLimit(parseFileSizeLimit(fileSizeLimit),
+            exportedLogsZipFile.toFile());
+//      } catch (IllegalArgumentException e) {
+//        return ResultBuilder.createUserErrorResult("TOO BIG: fileSizeLimit = " + fileSizeLimit);
+//      }
 
       result = ResultBuilder.createInfoResult(exportedLogsZipFile.toString());
     } catch (Exception ex) {
-      logger.error(ex, ex);
+      logger.error(ex.getMessage(), ex);
       result = ResultBuilder.createGemFireErrorResult(ex.getMessage());
     } finally {
       ExportLogsFunction.destroyExportLogsRegion(cache);
@@ -183,12 +189,19 @@ public class ExportLogsCommand implements CommandMarker {
 
   /**
    * Throws IllegalArgumentException if file size is over fileSizeLimitBytes
+   * false == limit is zero
+   * true == file size is less than limit
+   * exception == file size is over limit
    */
-  void checkOverDiskSpaceThreshold(int fileSizeLimitBytes, File file) {
+  boolean isFileSizeCheckEnabledAndWithinLimit(int fileSizeLimitBytes, File file) {
     // TODO:GEODE-2420: warn user if exportedLogsZipFile size > threshold
-    if (FileUtils.sizeOf(file) > fileSizeLimitBytes) {
-      throw new IllegalArgumentException("TOO BIG"); // FileTooBigException
+    if (fileSizeLimitBytes < 1) {
+      return false;
+    }
+    if (FileUtils.sizeOf(file) < fileSizeLimitBytes) {
+      return true;
     }
+    throw new IllegalArgumentException("TOO BIG: fileSizeLimit = " + fileSizeLimitBytes + ", fileSize = " + FileUtils.sizeOf(file)); // FileTooBigException
   }
 
   static int parseSize(String diskSpaceLimit) {
@@ -206,15 +219,15 @@ public class ExportLogsCommand implements CommandMarker {
       throw new IllegalArgumentException();
     }
     switch (matcher.group(2).toLowerCase()) {
-      case "t": return 1024 ^4;
-      case "g": return 1024 ^3;
+      case "t": return (int)Math.pow(1024, 4);
+      case "g": return (int)Math.pow(1024, 3);
       case "m":
-      default: return 1024 ^2;
+      default: return (int)Math.pow(1024, 2);
     }
   }
 
-  static final int MEGABYTE = 1024 ^2;
-  static final int GIGABYTE = 1024 ^3;
-  static final int TERABYTE = 1024 ^4;
+  static final int MEGABYTE = (int)Math.pow(1024, 2);
+  static final int GIGABYTE = (int)Math.pow(1024, 3);
+  static final int TERABYTE = (int)Math.pow(1024, 4);
 
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/81bb0e5c/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
index 271fe06..f5bc67f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
@@ -1437,6 +1437,9 @@ public class CliStrings {
   public static final String EXPORT_LOGS__FILESIZELIMIT = "file-size-limit";
   public static final String
       EXPORT_LOGS__FILESIZELIMIT__HELP = "Limits size of the file that can be exported. Specify zero for no limit. Value is in megabytes by default or [m|g|t] may be specified.";
+  public static final String EXPORT_LOGS__FILESIZELIMIT__SPECIFIED_DEFAULT = "0";
+  public static final String EXPORT_LOGS__FILESIZELIMIT__UNSPECIFIED_DEFAULT = "100m";
+
 
   /* export stack-trace command */
   public static final String EXPORT_STACKTRACE = "export stack-traces";

http://git-wip-us.apache.org/repos/asf/geode/blob/81bb0e5c/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
index d893efa..8a2685e 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
@@ -53,7 +53,7 @@ public class ExportLogsFileSizeLimitTest {
 
     ExportLogsCommand exportLogsCommand = new ExportLogsCommand();
 
-    assertThatThrownBy(() -> exportLogsCommand.checkOverDiskSpaceThreshold(MEGABYTE, file));
+    assertThatThrownBy(() -> exportLogsCommand.isFileSizeCheckEnabledAndWithinLimit(MEGABYTE, file));
   }
 
   @Test
@@ -63,7 +63,17 @@ public class ExportLogsFileSizeLimitTest {
 
     ExportLogsCommand exportLogsCommand = new ExportLogsCommand();
 
-    assertThatThrownBy(() -> exportLogsCommand.checkOverDiskSpaceThreshold(MEGABYTE, file));
+    assertThat(exportLogsCommand.isFileSizeCheckEnabledAndWithinLimit(MEGABYTE, file)).isTrue();
+  }
+
+  @Test
+  public void sizeZeroIsUnlimitedSize() throws Exception {
+    File file = new File(this.dir, this.testName.getMethodName());
+    fillUpFile(file, MEGABYTE * 2);
+
+    ExportLogsCommand exportLogsCommand = new ExportLogsCommand();
+
+    assertThat(exportLogsCommand.isFileSizeCheckEnabledAndWithinLimit(0, file)).isFalse();
   }
 
   private void fillUpFile(File file, int sizeInBytes) throws IOException {

http://git-wip-us.apache.org/repos/asf/geode/blob/81bb0e5c/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
new file mode 100644
index 0000000..8d46c3b
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
@@ -0,0 +1,26 @@
+/*
+ * 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.commands;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+
+/**
+ * All of the JUnit, DUnit and Integration tests for gfsh command export logs.
+ */
+@Suite.SuiteClasses({ExportLogsCommandTest.class, ExportLogsFileSizeLimitTest.class, ExportLogsIntegrationTest.class, ExportLogsDUnitTest.class,})
+@RunWith(Suite.class)
+public class ExportLogsTestSuite {
+}