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/09/27 23:01:23 UTC

[geode] branch develop updated: GEODE-3620: check for null argument to prevent NPE

This is an automated email from the ASF dual-hosted git repository.

khowe 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 dfd2a40  GEODE-3620: check for null argument to prevent NPE
dfd2a40 is described below

commit dfd2a406618b446a2efc5abc11c5ccba14cd7ea5
Author: Ken Howe <kh...@pivotal.io>
AuthorDate: Wed Sep 13 13:33:19 2017 -0700

    GEODE-3620: check for null argument to prevent NPE
    
    Added new tests
    
    Fixes problem in LocatorServerStartupRule where a faiulure due to
    suspect strings caused dunit VM cleanup to be skipped.
---
 .../io/MainWithChildrenRollingFileHandler.java     |   3 +
 .../test/dunit/rules/LocatorServerStartupRule.java |  16 ++-
 .../cli/commands/ConfigCommandDUnitTest.java       | 153 ++++++++++++++++++++-
 3 files changed, 164 insertions(+), 8 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/io/MainWithChildrenRollingFileHandler.java b/geode-core/src/main/java/org/apache/geode/internal/io/MainWithChildrenRollingFileHandler.java
index 225d1bf..8666799 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/io/MainWithChildrenRollingFileHandler.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/io/MainWithChildrenRollingFileHandler.java
@@ -228,6 +228,9 @@ public class MainWithChildrenRollingFileHandler implements RollingFileHandler {
 
   private File[] findChildrenExcept(final File dir, final Pattern pattern, final File exception) {
     final String exceptionName = (exception == null) ? null : exception.getName();
+    if (dir == null) {
+      return new File[] {};
+    }
     return dir
         .listFiles((dir1, name) -> !name.equals(exceptionName) && pattern.matcher(name).matches());
   }
diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
index 45a0a6a..3079373 100644
--- a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
+++ b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
@@ -107,15 +107,17 @@ public class LocatorServerStartupRule extends ExternalResource implements Serial
 
   @Override
   protected void after() {
-    DUnitLauncher.closeAndCheckForSuspects();
+    try {
+      DUnitLauncher.closeAndCheckForSuspects();
+    } finally {
+      MemberStarterRule.disconnectDSIfAny();
+      IntStream.range(0, DUnitLauncher.NUM_VMS).forEach(this::stopVM);
 
-    MemberStarterRule.disconnectDSIfAny();
-    IntStream.range(0, DUnitLauncher.NUM_VMS).forEach(this::stopVM);
-
-    if (useTempWorkingDir()) {
-      tempWorkingDir.delete();
+      if (useTempWorkingDir()) {
+        tempWorkingDir.delete();
+      }
+      restoreSystemProperties.after();
     }
-    restoreSystemProperties.after();
   }
 
   public MemberVM<Locator> startLocatorVM(int index) throws Exception {
diff --git a/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigCommandDUnitTest.java b/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigCommandDUnitTest.java
index e99a324..e5ee428 100644
--- a/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigCommandDUnitTest.java
+++ b/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/ConfigCommandDUnitTest.java
@@ -54,7 +54,8 @@ import org.apache.geode.test.junit.categories.DistributedTest;
 @RunWith(JUnitParamsRunner.class)
 public class ConfigCommandDUnitTest {
   @Rule
-  public LocatorServerStartupRule startupRule = new LocatorServerStartupRule();
+  public LocatorServerStartupRule startupRule =
+      new LocatorServerStartupRule().withTempWorkingDir().withLogFile();
 
   @Rule
   public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
@@ -215,6 +216,156 @@ public class ConfigCommandDUnitTest {
 
   @Test
   @Parameters({"true", "false"})
+  public void alterRuntimeConfig_logDiskSpaceLimitWithFileSizeLimitNotSet_OK(
+      final boolean connectOverHttp) throws Exception {
+
+    Properties props = new Properties();
+    props.setProperty(LOG_LEVEL, "error");
+    MemberVM locator = startupRule.startLocatorVM(0, props);
+    MemberVM server1 = startupRule.startServerVM(1, locator.getPort());
+    MemberVM server2 = startupRule.startServerVM(2, locator.getPort());
+
+    if (connectOverHttp) {
+      gfsh.connectAndVerify(locator.getHttpPort(), GfshShellConnectionRule.PortType.http);
+    } else {
+      gfsh.connectAndVerify(locator.getJmxPort(), GfshShellConnectionRule.PortType.jmxManger);
+    }
+
+    CommandStringBuilder csb = new CommandStringBuilder(CliStrings.ALTER_RUNTIME_CONFIG);
+    csb.addOption(CliStrings.ALTER_RUNTIME_CONFIG__LOG__DISK__SPACE__LIMIT, "10");
+
+    gfsh.executeAndVerifyCommand(csb.toString());
+    String resultStr = gfsh.getGfshOutput();
+
+    server1.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+      DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
+      assertThat(config.getLogFileSizeLimit()).isEqualTo(0);
+      assertThat(config.getArchiveDiskSpaceLimit()).isEqualTo(0);
+      assertThat(config.getStatisticSampleRate()).isEqualTo(1000);
+      assertThat(config.getStatisticArchiveFile().getName()).isEqualTo("");
+      assertThat(config.getStatisticSamplingEnabled()).isTrue();
+      assertThat(config.getLogDiskSpaceLimit()).isEqualTo(10);
+    });
+    server2.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+      DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
+      assertThat(config.getLogFileSizeLimit()).isEqualTo(0);
+      assertThat(config.getArchiveDiskSpaceLimit()).isEqualTo(0);
+      assertThat(config.getStatisticSampleRate()).isEqualTo(1000);
+      assertThat(config.getStatisticArchiveFile().getName()).isEqualTo("");
+      assertThat(config.getStatisticSamplingEnabled()).isTrue();
+      assertThat(config.getLogDiskSpaceLimit()).isEqualTo(10);
+    });
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void alterRuntimeConfig_logDiskSpaceLimitWithFileSizeLimitSet_OK(
+      final boolean connectOverHttp) throws Exception {
+
+    Properties props = new Properties();
+    props.setProperty(LOG_LEVEL, "error");
+    MemberVM locator = startupRule.startLocatorVM(0, props);
+    MemberVM server1 = startupRule.startServerVM(1, locator.getPort());
+    MemberVM server2 = startupRule.startServerVM(2, locator.getPort());
+
+    if (connectOverHttp) {
+      gfsh.connectAndVerify(locator.getHttpPort(), GfshShellConnectionRule.PortType.http);
+    } else {
+      gfsh.connectAndVerify(locator.getJmxPort(), GfshShellConnectionRule.PortType.jmxManger);
+    }
+
+    CommandStringBuilder csbSetFileSizeLimit =
+        new CommandStringBuilder(CliStrings.ALTER_RUNTIME_CONFIG);
+    csbSetFileSizeLimit.addOption(CliStrings.ALTER_RUNTIME_CONFIG__LOG__FILE__SIZE__LIMIT, "50");
+    gfsh.executeAndVerifyCommand(csbSetFileSizeLimit.toString());
+
+    server2.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+      DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
+      assertThat(config.getLogFileSizeLimit()).isEqualTo(50);
+      assertThat(config.getLogDiskSpaceLimit()).isEqualTo(0);
+    });
+
+    CommandStringBuilder csbSetDiskSpaceLimit =
+        new CommandStringBuilder(CliStrings.ALTER_RUNTIME_CONFIG);
+    csbSetDiskSpaceLimit.addOption(CliStrings.ALTER_RUNTIME_CONFIG__LOG__FILE__SIZE__LIMIT, "50");
+    csbSetDiskSpaceLimit.addOption(CliStrings.ALTER_RUNTIME_CONFIG__LOG__DISK__SPACE__LIMIT, "10");
+
+    gfsh.executeAndVerifyCommand(csbSetDiskSpaceLimit.toString());
+    String resultStr = gfsh.getGfshOutput();
+
+    server1.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+      DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
+      assertThat(config.getLogFileSizeLimit()).isEqualTo(50);
+      assertThat(config.getLogDiskSpaceLimit()).isEqualTo(10);
+      assertThat(config.getArchiveDiskSpaceLimit()).isEqualTo(0);
+      assertThat(config.getStatisticSampleRate()).isEqualTo(1000);
+      assertThat(config.getStatisticArchiveFile().getName()).isEqualTo("");
+      assertThat(config.getStatisticSamplingEnabled()).isTrue();
+    });
+    server2.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+      DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
+      assertThat(config.getLogFileSizeLimit()).isEqualTo(50);
+      assertThat(config.getLogDiskSpaceLimit()).isEqualTo(10);
+      assertThat(config.getArchiveDiskSpaceLimit()).isEqualTo(0);
+      assertThat(config.getStatisticSampleRate()).isEqualTo(1000);
+      assertThat(config.getStatisticArchiveFile().getName()).isEqualTo("");
+      assertThat(config.getStatisticSamplingEnabled()).isTrue();
+    });
+  }
+
+  @Test
+  @Parameters({"true", "false"})
+  public void alterRuntimeConfig_logDiskSpaceLimitOnMember_OK(final boolean connectOverHttp)
+      throws Exception {
+
+    Properties props = new Properties();
+    props.setProperty(LOG_LEVEL, "error");
+    MemberVM locator = startupRule.startLocatorVM(0, props);
+    MemberVM server1 = startupRule.startServerVM(1, locator.getPort());
+    MemberVM server2 = startupRule.startServerVM(2, locator.getPort());
+
+    if (connectOverHttp) {
+      gfsh.connectAndVerify(locator.getHttpPort(), GfshShellConnectionRule.PortType.http);
+    } else {
+      gfsh.connectAndVerify(locator.getJmxPort(), GfshShellConnectionRule.PortType.jmxManger);
+    }
+
+    CommandStringBuilder csb = new CommandStringBuilder(CliStrings.ALTER_RUNTIME_CONFIG);
+    csb.addOption(CliStrings.MEMBERS, server1.getName());
+    csb.addOption(CliStrings.ALTER_RUNTIME_CONFIG__LOG__DISK__SPACE__LIMIT, "10");
+
+    gfsh.executeAndVerifyCommand(csb.toString());
+    String resultStr = gfsh.getGfshOutput();
+
+    server1.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+      DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
+      assertThat(config.getLogFileSizeLimit()).isEqualTo(0);
+      assertThat(config.getLogDiskSpaceLimit()).isEqualTo(10);
+      assertThat(config.getArchiveDiskSpaceLimit()).isEqualTo(0);
+      assertThat(config.getStatisticSampleRate()).isEqualTo(1000);
+      assertThat(config.getStatisticArchiveFile().getName()).isEqualTo("");
+      assertThat(config.getStatisticSamplingEnabled()).isTrue();
+    });
+    server2.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
+      DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
+      assertThat(config.getLogFileSizeLimit()).isEqualTo(0);
+      assertThat(config.getLogDiskSpaceLimit()).isEqualTo(0);
+      assertThat(config.getArchiveDiskSpaceLimit()).isEqualTo(0);
+      assertThat(config.getStatisticSampleRate()).isEqualTo(1000);
+      assertThat(config.getStatisticArchiveFile().getName()).isEqualTo("");
+      assertThat(config.getStatisticSamplingEnabled()).isTrue();
+    });
+  }
+
+  @Test
+  @Parameters({"true", "false"})
   public void testAlterUpdatesSharedConfig(final boolean connectOverHttp) throws Exception {
     MemberVM locator = startupRule.startLocatorVM(0);
 

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].