This is an automated email from the ASF dual-hosted git repository. jjramos 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 0a628c9 GEODE-5342: Fix disk-store validation in commands (#2881) 0a628c9 is described below commit 0a628c99aac960b6b420d62c26e675ba68aba752 Author: Juan José Ramos <jujora...@users.noreply.github.com> AuthorDate: Fri Nov 30 10:52:26 2018 +0000 GEODE-5342: Fix disk-store validation in commands (#2881) - Fixed minor warnings. - Replaced the usage of `junit.Assert` by `assertj`. - Offline gfsh comamnds related to disk-stores avoid creating the disk-store's folder if it doesn't exist already. --- .../cli/commands/DiskStoreCommandsDUnitTest.java | 107 ++++---- ...t.java => DiskStoreFactoryIntegrationTest.java} | 304 +++++++++++---------- .../AlterDiskStoreCommandIntegrationTest.java} | 11 +- .../cli/commands/AlterOfflineDiskStoreCommand.java | 11 +- .../commands/CompactOfflineDiskStoreCommand.java | 19 +- .../commands/DescribeOfflineDiskStoreCommand.java | 10 +- .../commands/UpgradeOfflineDiskStoreCommand.java | 17 +- .../cli/commands/ValidateDiskStoreCommand.java | 10 +- 8 files changed, 261 insertions(+), 228 deletions(-) diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java index eaed28a..7a2e855 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java @@ -18,6 +18,9 @@ package org.apache.geode.management.internal.cli.commands; import static org.assertj.core.api.Assertions.assertThat; import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -25,10 +28,13 @@ import java.util.Properties; import java.util.stream.Collectors; import java.util.stream.IntStream; +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; import org.apache.geode.cache.Cache; import org.apache.geode.cache.Region; @@ -41,18 +47,19 @@ import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.cache.SnapshotTestUtil; import org.apache.geode.management.cli.Result; import org.apache.geode.management.internal.cli.result.CommandResult; +import org.apache.geode.test.dunit.SerializableRunnableIF; import org.apache.geode.test.dunit.rules.ClusterStartupRule; import org.apache.geode.test.dunit.rules.MemberVM; import org.apache.geode.test.junit.categories.PersistenceTest; import org.apache.geode.test.junit.rules.GfshCommandRule; import org.apache.geode.test.junit.rules.ServerStarterRule; -@Category({PersistenceTest.class}) +@Category(PersistenceTest.class) +@RunWith(JUnitParamsRunner.class) public class DiskStoreCommandsDUnitTest { - + private static final String GROUP = "GROUP1"; private static final String REGION_1 = "REGION1"; private static final String DISKSTORE = "DISKSTORE"; - private static final String GROUP = "GROUP1"; @Rule public ClusterStartupRule rule = new ClusterStartupRule(); @@ -80,6 +87,15 @@ public class DiskStoreCommandsDUnitTest { REGION_1, DISKSTORE, GROUP)).statusIsSuccess(); } + private static SerializableRunnableIF dataProducer() { + return () -> { + Cache cache = ClusterStartupRule.getCache(); + assertThat(cache).isNotNull(); + Region<String, String> r = cache.getRegion(REGION_1); + r.put("A", "B"); + }; + } + @Test public void testMissingDiskStore() throws Exception { Properties props = new Properties(); @@ -93,11 +109,7 @@ public class DiskStoreCommandsDUnitTest { createDiskStoreAndRegion(locator, 2); - server1.invoke(() -> { - Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); - r.put("A", "B"); - }); + server1.invoke(dataProducer()); gfsh.executeAndAssertThat("show missing-disk-stores") .containsOutput("No missing disk store found"); @@ -106,7 +118,8 @@ public class DiskStoreCommandsDUnitTest { server2.invoke(() -> { Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); + assertThat(cache).isNotNull(); + Region<String, String> r = cache.getRegion(REGION_1); r.put("A", "C"); }); @@ -136,6 +149,7 @@ public class DiskStoreCommandsDUnitTest { server1.invoke(() -> { Cache cache = ClusterStartupRule.getCache(); + assertThat(cache).isNotNull(); Region<String, String> r = cache.getRegion(REGION_1); assertThat(r.get("A")).isEqualTo("B"); }); @@ -149,11 +163,7 @@ public class DiskStoreCommandsDUnitTest { createDiskStoreAndRegion(server1, 1); - server1.invoke(() -> { - Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); - r.put("A", "B"); - }); + server1.invoke(dataProducer()); gfsh.executeAndAssertThat("show missing-disk-stores") .containsOutput("No missing disk store found"); @@ -175,11 +185,7 @@ public class DiskStoreCommandsDUnitTest { createDiskStoreAndRegion(server1, 1); - server1.invoke(() -> { - Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); - r.put("A", "B"); - }); + server1.invoke(dataProducer()); gfsh.executeAndAssertThat("show missing-disk-stores") .containsOutput("No missing disk store found"); @@ -198,15 +204,13 @@ public class DiskStoreCommandsDUnitTest { } private boolean diskStoreExistsInClusterConfig(MemberVM jmxManager) { - boolean result = jmxManager.invoke(() -> { + return jmxManager.invoke(() -> { InternalConfigurationPersistenceService sharedConfig = ((InternalLocator) Locator.getLocator()).getConfigurationPersistenceService(); List<DiskStoreType> diskStores = sharedConfig.getCacheConfig(GROUP).getDiskStores(); return diskStores.size() == 1 && DISKSTORE.equals(diskStores.get(0).getName()); }); - - return result; } @Test @@ -215,7 +219,7 @@ public class DiskStoreCommandsDUnitTest { props.setProperty("groups", GROUP); MemberVM locator = rule.startLocatorVM(0); - MemberVM server1 = rule.startServerVM(1, props, locator.getPort()); + rule.startServerVM(1, props, locator.getPort()); gfsh.connectAndVerify(locator); @@ -233,7 +237,7 @@ public class DiskStoreCommandsDUnitTest { props.setProperty("groups", GROUP); MemberVM locator = rule.startLocatorVM(0); - MemberVM server1 = rule.startServerVM(1, props, locator.getPort()); + rule.startServerVM(1, props, locator.getPort()); gfsh.connectAndVerify(locator.getJmxPort(), GfshCommandRule.PortType.jmxManager); @@ -257,11 +261,7 @@ public class DiskStoreCommandsDUnitTest { createDiskStoreAndRegion(locator, 1); - server1.invoke(() -> { - Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); - r.put("A", "B"); - }); + server1.invoke(dataProducer()); String backupDir = tempDir.newFolder().getCanonicalPath(); String diskDirs = new File(server1.getWorkingDir(), DISKSTORE).getAbsolutePath(); @@ -274,7 +274,7 @@ public class DiskStoreCommandsDUnitTest { @Test public void destroyDiskStoreIsIdempotent() throws Exception { MemberVM locator = rule.startLocatorVM(0); - MemberVM server1 = rule.startServerVM(1, locator.getPort()); + rule.startServerVM(1, locator.getPort()); gfsh.connectAndVerify(locator); @@ -286,8 +286,10 @@ public class DiskStoreCommandsDUnitTest { .statusIsSuccess(); locator.invoke(() -> { + InternalLocator internalLocator = ClusterStartupRule.getLocator(); + assertThat(internalLocator).isNotNull(); InternalConfigurationPersistenceService cc = - ClusterStartupRule.getLocator().getConfigurationPersistenceService(); + internalLocator.getConfigurationPersistenceService(); CacheConfig config = cc.getCacheConfig("cluster"); assertThat(config.getDiskStores().size()).isEqualTo(0); }); @@ -311,7 +313,8 @@ public class DiskStoreCommandsDUnitTest { server1.invoke(() -> { InternalCache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); + assertThat(cache).isNotNull(); + Region<Integer, String> r = cache.getRegion(REGION_1); // Make sure we have more than 1 log and there is something to compact for (int i = 0; i < 10000; i++) { r.put(i, "value_" + i); @@ -338,11 +341,7 @@ public class DiskStoreCommandsDUnitTest { createDiskStoreAndRegion(server1, 1); - server1.invoke(() -> { - Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); - r.put("A", "B"); - }); + server1.invoke(dataProducer()); CommandResult result = gfsh.executeCommand( String.format("describe disk-store --member=%s --name=%s", server1.getName(), DISKSTORE)); @@ -394,11 +393,7 @@ public class DiskStoreCommandsDUnitTest { createDiskStoreAndRegion(server1, 1); - server1.invoke(() -> { - Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); - r.put("A", "B"); - }); + server1.invoke(dataProducer()); // Should not be able to do this on a running system String diskDirs = new File(server1.getWorkingDir(), DISKSTORE).getAbsolutePath(); @@ -419,12 +414,11 @@ public class DiskStoreCommandsDUnitTest { props.setProperty("groups", GROUP); MemberVM locator = rule.startLocatorVM(0); - MemberVM server1 = rule.startServerVM(1, props, locator.getPort()); + rule.startServerVM(1, props, locator.getPort()); gfsh.connectAndVerify(locator); - - gfsh.executeAndAssertThat("revoke missing-disk-store --id=unknown-diskstore") - .statusIsError().containsOutput("Unable to find missing disk store to revoke"); + gfsh.executeAndAssertThat("revoke missing-disk-store --id=unknown-diskstore").statusIsError() + .containsOutput("Unable to find missing disk store to revoke"); } @Test @@ -435,11 +429,7 @@ public class DiskStoreCommandsDUnitTest { createDiskStoreAndRegion(server1, 1); - server1.invoke(() -> { - Cache cache = ClusterStartupRule.getCache(); - Region r = cache.getRegion(REGION_1); - r.put("A", "B"); - }); + server1.invoke(dataProducer()); String diskDirs = new File(server1.getWorkingDir(), DISKSTORE).getAbsolutePath(); gfsh.executeAndAssertThat( @@ -456,4 +446,19 @@ public class DiskStoreCommandsDUnitTest { DISKSTORE, diskDirs)) .statusIsError().containsOutput("The disk store does not contain a region named: /INVALID"); } + + @Test + @Parameters({"compact offline-disk-store", "describe offline-disk-store", + "upgrade offline-disk-store", "validate offline-disk-store", + "alter disk-store --region=testRegion --enable-statistics=true"}) + public void offlineDiskStoreCommandShouldNotCreateFolderIfDiskStoreDoesNotExist( + String baseCommand) { + Path nonExistingDiskStorePath = + Paths.get(tempDir.getRoot().getAbsolutePath() + File.separator + "nonExistingDiskStore"); + assertThat(Files.exists(nonExistingDiskStorePath)).isFalse(); + gfsh.executeAndAssertThat(baseCommand + " --name=" + DISKSTORE + " --disk-dirs=" + + nonExistingDiskStorePath.toAbsolutePath().toString()).statusIsError() + .containsOutput("Could not find disk-dirs:"); + assertThat(Files.exists(nonExistingDiskStorePath)).isFalse(); + } } diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/DiskStoreFactoryIntegrationTest.java similarity index 55% rename from geode-core/src/integrationTest/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java rename to geode-core/src/integrationTest/java/org/apache/geode/internal/cache/DiskStoreFactoryIntegrationTest.java index 2f15690..63748b0 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/DiskStoreFactoryJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/DiskStoreFactoryIntegrationTest.java @@ -14,29 +14,33 @@ */ package org.apache.geode.internal.cache; +import static org.apache.geode.cache.DiskStoreFactory.DEFAULT_DISK_DIR_SIZE; import static org.apache.geode.distributed.ConfigurationProperties.ENABLE_TIME_STATISTICS; import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS; import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL; import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; import static org.apache.geode.distributed.ConfigurationProperties.STATISTIC_ARCHIVE_FILE; import static org.apache.geode.distributed.ConfigurationProperties.STATISTIC_SAMPLING_ENABLED; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.fail; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import java.io.File; -import java.io.FilenameFilter; +import java.io.IOException; +import java.nio.file.Files; import java.util.Arrays; import java.util.Properties; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.apache.geode.cache.AttributesFactory; import org.apache.geode.cache.Cache; @@ -46,35 +50,36 @@ import org.apache.geode.cache.DiskStore; import org.apache.geode.cache.DiskStoreFactory; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionShortcut; -import org.apache.geode.distributed.DistributedSystem; /** * Tests DiskStoreFactory */ -public class DiskStoreFactoryJUnitTest { +public class DiskStoreFactoryIntegrationTest { private static Cache cache = null; - - private static DistributedSystem ds = null; private static Properties props = new Properties(); + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + static { props.setProperty(MCAST_PORT, "0"); props.setProperty(LOCATORS, ""); props.setProperty(LOG_LEVEL, "config"); // to keep diskPerf logs smaller props.setProperty(STATISTIC_SAMPLING_ENABLED, "true"); props.setProperty(ENABLE_TIME_STATISTICS, "true"); - props.setProperty(STATISTIC_ARCHIVE_FILE, "stats.gfs"); } @Before public void setUp() throws Exception { + props.setProperty(STATISTIC_ARCHIVE_FILE, + temporaryFolder.getRoot().getAbsolutePath() + File.separator + "stats.gfs"); createCache(); } private Cache createCache() { cache = new CacheFactory(props).create(); - ds = cache.getDistributedSystem(); + return cache; } @@ -90,25 +95,28 @@ public class DiskStoreFactoryJUnitTest { public void testGetDefaultInstance() { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testGetDefaultInstance"; - assertEquals(null, cache.findDiskStore(name)); + assertThat(cache.findDiskStore(name)).isNull(); DiskStore ds = dsf.create(name); - assertEquals(ds, cache.findDiskStore(name)); - assertEquals(name, ds.getName()); - assertEquals(DiskStoreFactory.DEFAULT_AUTO_COMPACT, ds.getAutoCompact()); - assertEquals(DiskStoreFactory.DEFAULT_COMPACTION_THRESHOLD, ds.getCompactionThreshold()); - assertEquals(DiskStoreFactory.DEFAULT_ALLOW_FORCE_COMPACTION, ds.getAllowForceCompaction()); - assertEquals(DiskStoreFactory.DEFAULT_MAX_OPLOG_SIZE, ds.getMaxOplogSize()); - assertEquals(DiskStoreFactory.DEFAULT_TIME_INTERVAL, ds.getTimeInterval()); - assertEquals(DiskStoreFactory.DEFAULT_WRITE_BUFFER_SIZE, ds.getWriteBufferSize()); - assertEquals(DiskStoreFactory.DEFAULT_QUEUE_SIZE, ds.getQueueSize()); - if (!Arrays.equals(DiskStoreFactory.DEFAULT_DISK_DIRS, ds.getDiskDirs())) { - fail("expected=" + Arrays.toString(DiskStoreFactory.DEFAULT_DISK_DIRS) + " had=" - + Arrays.toString(ds.getDiskDirs())); - } - if (!Arrays.equals(DiskStoreFactory.DEFAULT_DISK_DIR_SIZES, ds.getDiskDirSizes())) { - fail("expected=" + Arrays.toString(DiskStoreFactory.DEFAULT_DISK_DIR_SIZES) + " had=" - + Arrays.toString(ds.getDiskDirSizes())); - } + + assertThat(cache.findDiskStore(name)).isEqualTo(ds); + assertThat(ds.getName()).isEqualTo(name); + assertThat(ds.getAutoCompact()).isEqualTo(DiskStoreFactory.DEFAULT_AUTO_COMPACT); + assertThat(ds.getCompactionThreshold()) + .isEqualTo(DiskStoreFactory.DEFAULT_COMPACTION_THRESHOLD); + assertThat(ds.getAllowForceCompaction()) + .isEqualTo(DiskStoreFactory.DEFAULT_ALLOW_FORCE_COMPACTION); + assertThat(ds.getMaxOplogSize()).isEqualTo(DiskStoreFactory.DEFAULT_MAX_OPLOG_SIZE); + assertThat(ds.getTimeInterval()).isEqualTo(DiskStoreFactory.DEFAULT_TIME_INTERVAL); + assertThat(ds.getWriteBufferSize()).isEqualTo(DiskStoreFactory.DEFAULT_WRITE_BUFFER_SIZE); + assertThat(ds.getQueueSize()).isEqualTo(DiskStoreFactory.DEFAULT_QUEUE_SIZE); + assertThat(Arrays.equals(ds.getDiskDirs(), DiskStoreFactory.DEFAULT_DISK_DIRS)) + .as("expected=" + Arrays.toString(DiskStoreFactory.DEFAULT_DISK_DIRS) + " had=" + + Arrays.toString(ds.getDiskDirs())) + .isTrue(); + assertThat(Arrays.equals(ds.getDiskDirSizes(), DiskStoreFactory.DEFAULT_DISK_DIR_SIZES)) + .as("expected=" + Arrays.toString(DiskStoreFactory.DEFAULT_DISK_DIR_SIZES) + " had=" + + Arrays.toString(ds.getDiskDirSizes())) + .isTrue(); } @Test @@ -122,13 +130,16 @@ public class DiskStoreFactoryJUnitTest { .setTimeInterval(DiskStoreFactory.DEFAULT_TIME_INTERVAL + 1) .setWriteBufferSize(DiskStoreFactory.DEFAULT_WRITE_BUFFER_SIZE + 1) .setQueueSize(DiskStoreFactory.DEFAULT_QUEUE_SIZE + 1).create(name); - assertEquals(!DiskStoreFactory.DEFAULT_AUTO_COMPACT, ds.getAutoCompact()); - assertEquals(DiskStoreFactory.DEFAULT_COMPACTION_THRESHOLD / 2, ds.getCompactionThreshold()); - assertEquals(!DiskStoreFactory.DEFAULT_ALLOW_FORCE_COMPACTION, ds.getAllowForceCompaction()); - assertEquals(DiskStoreFactory.DEFAULT_MAX_OPLOG_SIZE + 1, ds.getMaxOplogSize()); - assertEquals(DiskStoreFactory.DEFAULT_TIME_INTERVAL + 1, ds.getTimeInterval()); - assertEquals(DiskStoreFactory.DEFAULT_WRITE_BUFFER_SIZE + 1, ds.getWriteBufferSize()); - assertEquals(DiskStoreFactory.DEFAULT_QUEUE_SIZE + 1, ds.getQueueSize()); + + assertThat(ds.getAutoCompact()).isEqualTo(!DiskStoreFactory.DEFAULT_AUTO_COMPACT); + assertThat(ds.getCompactionThreshold()) + .isEqualTo(DiskStoreFactory.DEFAULT_COMPACTION_THRESHOLD / 2); + assertThat(ds.getAllowForceCompaction()) + .isEqualTo(!DiskStoreFactory.DEFAULT_ALLOW_FORCE_COMPACTION); + assertThat(ds.getMaxOplogSize()).isEqualTo(DiskStoreFactory.DEFAULT_MAX_OPLOG_SIZE + 1); + assertThat(ds.getTimeInterval()).isEqualTo(DiskStoreFactory.DEFAULT_TIME_INTERVAL + 1); + assertThat(ds.getWriteBufferSize()).isEqualTo(DiskStoreFactory.DEFAULT_WRITE_BUFFER_SIZE + 1); + assertThat(ds.getQueueSize()).isEqualTo(DiskStoreFactory.DEFAULT_QUEUE_SIZE + 1); } @Test @@ -136,21 +147,16 @@ public class DiskStoreFactoryJUnitTest { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testCompactionThreshold1"; DiskStore ds = dsf.setCompactionThreshold(0).create(name); - assertEquals(0, ds.getCompactionThreshold()); + assertThat(ds.getCompactionThreshold()).isEqualTo(0); name = "testCompactionThreshold2"; ds = dsf.setCompactionThreshold(100).create(name); - assertEquals(100, ds.getCompactionThreshold()); + assertThat(ds.getCompactionThreshold()).isEqualTo(100); + // check illegal stuff - try { - dsf.setCompactionThreshold(-1); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } - try { - dsf.setCompactionThreshold(101); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + assertThatThrownBy(() -> dsf.setCompactionThreshold(-1)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> dsf.setCompactionThreshold(101)) + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -158,16 +164,13 @@ public class DiskStoreFactoryJUnitTest { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testQueueSize"; DiskStore ds = dsf.setQueueSize(0).create(name); - assertEquals(0, ds.getQueueSize()); + assertThat(ds.getQueueSize()).isEqualTo(0); name = "testQueueSize2"; ds = dsf.setQueueSize(Integer.MAX_VALUE).create(name); - assertEquals(Integer.MAX_VALUE, ds.getQueueSize()); + assertThat(ds.getQueueSize()).isEqualTo(Integer.MAX_VALUE); + // check illegal stuff - try { - dsf.setQueueSize(-1); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + assertThatThrownBy(() -> dsf.setQueueSize(-1)).isInstanceOf(IllegalArgumentException.class); } @Test @@ -175,16 +178,14 @@ public class DiskStoreFactoryJUnitTest { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testWriteBufferSize"; DiskStore ds = dsf.setWriteBufferSize(0).create(name); - assertEquals(0, ds.getWriteBufferSize()); + assertThat(ds.getWriteBufferSize()).isEqualTo(0); name = "testWriteBufferSize2"; ds = dsf.setWriteBufferSize(Integer.MAX_VALUE).create(name); - assertEquals(Integer.MAX_VALUE, ds.getWriteBufferSize()); + assertThat(ds.getWriteBufferSize()).isEqualTo(Integer.MAX_VALUE); + // check illegal stuff - try { - dsf.setWriteBufferSize(-1); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + assertThatThrownBy(() -> dsf.setWriteBufferSize(-1)) + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -192,16 +193,13 @@ public class DiskStoreFactoryJUnitTest { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testTimeInterval"; DiskStore ds = dsf.setTimeInterval(0).create(name); - assertEquals(0, ds.getTimeInterval()); + assertThat(ds.getTimeInterval()).isEqualTo(0); name = "testTimeInterval2"; ds = dsf.setTimeInterval(Long.MAX_VALUE).create(name); - assertEquals(Long.MAX_VALUE, ds.getTimeInterval()); + assertThat(ds.getTimeInterval()).isEqualTo(Long.MAX_VALUE); + // check illegal stuff - try { - dsf.setTimeInterval(-1); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + assertThatThrownBy(() -> dsf.setTimeInterval(-1)).isInstanceOf(IllegalArgumentException.class); } @Test @@ -209,22 +207,16 @@ public class DiskStoreFactoryJUnitTest { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testMaxOplogSize"; DiskStore ds = dsf.setMaxOplogSize(0).create(name); - assertEquals(0, ds.getMaxOplogSize()); + assertThat(ds.getMaxOplogSize()).isEqualTo(0); name = "testMaxOplogSize2"; long max = Long.MAX_VALUE / (1024 * 1024); ds = dsf.setMaxOplogSize(max).create(name); - assertEquals(max, ds.getMaxOplogSize()); + assertThat(ds.getMaxOplogSize()).isEqualTo(max); + // check illegal stuff - try { - dsf.setMaxOplogSize(-1); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } - try { - dsf.setMaxOplogSize(max + 1); - fail("expected IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + assertThatThrownBy(() -> dsf.setMaxOplogSize(-1)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> dsf.setMaxOplogSize(max + 1)) + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -248,21 +240,13 @@ public class DiskStoreFactoryJUnitTest { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testDestroy"; DiskStore ds = dsf.create(name); - Region region = cache.createRegionFactory(RegionShortcut.LOCAL_PERSISTENT) .setDiskStoreName("testDestroy").create("region"); - - try { - ds.destroy(); - fail("Should have thrown an exception"); - } catch (IllegalStateException expected) { - // expected - } - - region.destroyRegion(); + assertThatThrownBy(ds::destroy).isInstanceOf(IllegalStateException.class); // This should now work - ds.destroy(); + region.destroyRegion(); + assertThatCode(ds::destroy).doesNotThrowAnyException(); } @Test @@ -270,14 +254,12 @@ public class DiskStoreFactoryJUnitTest { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testDestroy"; DiskStore ds = dsf.create(name); - Region region = cache.createRegionFactory(RegionShortcut.LOCAL_PERSISTENT) .setDiskStoreName("testDestroy").create("region"); - region.close(); - // This should now work - ds.destroy(); + region.close(); + assertThatCode(ds::destroy).doesNotThrowAnyException(); } @Test @@ -288,18 +270,11 @@ public class DiskStoreFactoryJUnitTest { Region region = cache.createRegionFactory(RegionShortcut.LOCAL_OVERFLOW) .setDiskStoreName("testDestroy").create("region"); - - try { - ds.destroy(); - fail("Should have thrown an exception"); - } catch (IllegalStateException expected) { - System.err.println("Got expected :" + expected.getMessage()); - } - - region.close(); + assertThatThrownBy(ds::destroy).isInstanceOf(IllegalStateException.class); // The destroy should now work. - ds.destroy(); + region.close(); + assertThatCode(ds::destroy).doesNotThrowAnyException(); } @Test @@ -308,27 +283,29 @@ public class DiskStoreFactoryJUnitTest { dsf.setAllowForceCompaction(true); String name = "testForceCompaction"; DiskStore ds = dsf.create(name); - assertEquals(false, ds.forceCompaction()); + assertThat(ds.forceCompaction()).isFalse(); } @Test + @SuppressWarnings("deprecation") public void testMissingInitFile() { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testMissingInitFile"; DiskStore diskStore = dsf.create(name); File ifFile = new File(diskStore.getDiskDirs()[0], "BACKUP" + name + DiskInitFile.IF_FILE_EXT); - assertTrue(ifFile.exists()); - AttributesFactory af = new AttributesFactory(); + assertThat(ifFile.exists()).isTrue(); + AttributesFactory<String, String> af = new AttributesFactory<>(); af.setDiskStoreName(name); af.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE); cache.createRegion("r", af.create()); cache.close(); - assertTrue(ifFile.exists()); - assertTrue(ifFile.delete()); - assertFalse(ifFile.exists()); + assertThat(ifFile.exists()).isTrue(); + assertThat(ifFile.delete()).isTrue(); + assertThat(ifFile.exists()).isFalse(); cache = createCache(); dsf = cache.createDiskStoreFactory(); - assertEquals(null, ((GemFireCacheImpl) cache).findDiskStore(name)); + assertThat(cache.findDiskStore(name)).isNull(); + try { dsf.create(name); fail("expected IllegalStateException"); @@ -343,39 +320,33 @@ public class DiskStoreFactoryJUnitTest { File[] dirs = diskStore.getDiskDirs(); for (File dir : dirs) { - File[] files = dir.listFiles(new FilenameFilter() { - - @Override - public boolean accept(File dir, String name) { - return name.startsWith("BACKUP" + diskStoreName); - } - - }); - for (File file : files) { - file.delete(); - } + File[] files = dir.listFiles((file, name) -> name.startsWith("BACKUP" + diskStoreName)); + assertThat(files).isNotNull(); + Arrays.stream(files).forEach(file -> assertThat(file.delete()).isTrue()); } } @Test + @SuppressWarnings("deprecation") public void testMissingCrfFile() { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testMissingCrfFile"; DiskStore diskStore = dsf.create(name); File crfFile = new File(diskStore.getDiskDirs()[0], "BACKUP" + name + "_1.crf"); - AttributesFactory af = new AttributesFactory(); + AttributesFactory<String, String> af = new AttributesFactory<>(); af.setDiskStoreName(name); af.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE); - Region r = cache.createRegion("r", af.create()); + Region<String, String> r = cache.createRegion("r", af.create()); r.put("key", "value"); - assertTrue(crfFile.exists()); + assertThat(crfFile.exists()).isTrue(); cache.close(); - assertTrue(crfFile.exists()); - assertTrue(crfFile.delete()); - assertFalse(crfFile.exists()); + assertThat(crfFile.exists()).isTrue(); + assertThat(crfFile.delete()).isTrue(); + assertThat(crfFile.exists()).isFalse(); cache = createCache(); dsf = cache.createDiskStoreFactory(); - assertEquals(null, ((GemFireCacheImpl) cache).findDiskStore(name)); + assertThat(cache.findDiskStore(name)).isNull(); + try { dsf.create(name); fail("expected IllegalStateException"); @@ -386,24 +357,26 @@ public class DiskStoreFactoryJUnitTest { } @Test + @SuppressWarnings("deprecation") public void testMissingDrfFile() { DiskStoreFactory dsf = cache.createDiskStoreFactory(); String name = "testMissingDrfFile"; DiskStore diskStore = dsf.create(name); File drfFile = new File(diskStore.getDiskDirs()[0], "BACKUP" + name + "_1.drf"); - AttributesFactory af = new AttributesFactory(); + AttributesFactory<String, String> af = new AttributesFactory<>(); af.setDiskStoreName(name); af.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE); - Region r = cache.createRegion("r", af.create()); + Region<String, String> r = cache.createRegion("r", af.create()); r.put("key", "value"); - assertTrue(drfFile.exists()); + assertThat(drfFile.exists()).isTrue(); cache.close(); - assertTrue(drfFile.exists()); - assertTrue(drfFile.delete()); - assertFalse(drfFile.exists()); + assertThat(drfFile.exists()).isTrue(); + assertThat(drfFile.delete()).isTrue(); + assertThat(drfFile.exists()).isFalse(); cache = createCache(); dsf = cache.createDiskStoreFactory(); - assertEquals(null, ((GemFireCacheImpl) cache).findDiskStore(name)); + assertThat(cache.findDiskStore(name)).isNull(); + try { dsf.create(name); fail("expected IllegalStateException"); @@ -414,20 +387,20 @@ public class DiskStoreFactoryJUnitTest { } @Test + @SuppressWarnings("deprecation") public void testRedefiningDefaultDiskStore() { DiskStoreFactory dsf = cache.createDiskStoreFactory(); dsf.setAutoCompact(!DiskStoreFactory.DEFAULT_AUTO_COMPACT); - String name = "testMissingDrfFile"; - assertEquals(null, cache.findDiskStore(DiskStoreFactory.DEFAULT_DISK_STORE_NAME)); + assertThat(cache.findDiskStore(DiskStoreFactory.DEFAULT_DISK_STORE_NAME)).isNull(); DiskStore diskStore = dsf.create(DiskStoreFactory.DEFAULT_DISK_STORE_NAME); - AttributesFactory af = new AttributesFactory(); + AttributesFactory<String, String> af = new AttributesFactory<>(); af.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE); - Region r = cache.createRegion("r", af.create()); + Region<String, String> r = cache.createRegion("r", af.create()); r.put("key", "value"); DiskStore ds = ((LocalRegion) r).getDiskStore(); - assertEquals(ds, cache.findDiskStore(DiskStoreFactory.DEFAULT_DISK_STORE_NAME)); - assertEquals(DiskStoreFactory.DEFAULT_DISK_STORE_NAME, ds.getName()); - assertEquals(!DiskStoreFactory.DEFAULT_AUTO_COMPACT, ds.getAutoCompact()); + assertThat(cache.findDiskStore(DiskStoreFactory.DEFAULT_DISK_STORE_NAME)).isEqualTo(ds); + assertThat(ds.getName()).isEqualTo(DiskStoreFactory.DEFAULT_DISK_STORE_NAME); + assertThat(ds.getAutoCompact()).isEqualTo(!DiskStoreFactory.DEFAULT_AUTO_COMPACT); cache.close(); // if test passed clean up files removeFiles(diskStore); @@ -444,10 +417,45 @@ public class DiskStoreFactoryJUnitTest { } catch (RuntimeException e) { threwException = true; } - assertTrue(threwException); + assertThat(threwException).isTrue(); verify(diskStore, times(1)).close(); } - // setDiskDirs and setDiskDirsAndSizes are tested in DiskRegionIllegalArguementsJUnitTest - // also setDiskUsageWarningPercentage and setDiskUsageCriticalPercentage + @Test + public void checkIfDirectoriesExistShouldCreateDirectoriesWhenTheyDoNotExist() + throws IOException { + File[] diskDirs = new File[3]; + diskDirs[0] = new File( + temporaryFolder.getRoot().getAbsolutePath() + File.separator + "randomNonExistingDiskDir"); + diskDirs[1] = temporaryFolder.newFolder("existingDiskDir"); + diskDirs[2] = new File(temporaryFolder.getRoot().getAbsolutePath() + File.separator + + "anotherRandomNonExistingDiskDir"); + + assertThat(Files.exists(diskDirs[0].toPath())).isFalse(); + assertThat(Files.exists(diskDirs[1].toPath())).isTrue(); + assertThat(Files.exists(diskDirs[2].toPath())).isFalse(); + DiskStoreFactoryImpl.checkIfDirectoriesExist(diskDirs); + Arrays.stream(diskDirs).forEach(diskDir -> assertThat(Files.exists(diskDir.toPath())).isTrue()); + } + + @Test + public void setDiskDirsShouldInitializeInternalMetadataAndCreateDirectoriesWhenTheyDoNotExist() + throws IOException { + File[] diskDirs = new File[3]; + diskDirs[0] = + new File(temporaryFolder.getRoot().getAbsolutePath() + File.separator + "randomDiskDir"); + diskDirs[1] = temporaryFolder.newFolder("existingDiskDir"); + diskDirs[2] = new File( + temporaryFolder.getRoot().getAbsolutePath() + File.separator + "anotherRandomDiskDir"); + assertThat(Files.exists(diskDirs[0].toPath())).isFalse(); + assertThat(Files.exists(diskDirs[1].toPath())).isTrue(); + assertThat(Files.exists(diskDirs[2].toPath())).isFalse(); + + DiskStoreFactoryImpl factoryImpl = + (DiskStoreFactoryImpl) cache.createDiskStoreFactory().setDiskDirs(diskDirs); + Arrays.stream(diskDirs).forEach(diskDir -> assertThat(Files.exists(diskDir.toPath())).isTrue()); + assertThat(factoryImpl.getDiskStoreAttributes().diskDirs).isEqualTo(diskDirs); + assertThat(factoryImpl.getDiskStoreAttributes().diskDirSizes) + .isEqualTo(new int[] {DEFAULT_DISK_DIR_SIZE, DEFAULT_DISK_DIR_SIZE, DEFAULT_DISK_DIR_SIZE}); + } } diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java similarity index 87% rename from geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreJUnitTest.java rename to geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java index f2f0440..9981e61 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/AlterDiskStoreCommandIntegrationTest.java @@ -20,13 +20,18 @@ import static org.mockito.Mockito.spy; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.apache.geode.management.cli.GfshCommand; import org.apache.geode.management.internal.cli.i18n.CliStrings; import org.apache.geode.management.internal.cli.util.CommandStringBuilder; import org.apache.geode.test.junit.rules.GfshParserRule; -public class AlterDiskStoreJUnitTest { +public class AlterDiskStoreCommandIntegrationTest { + + @Rule + public TemporaryFolder tempDir = new TemporaryFolder(); + @Rule public GfshParserRule gfsh = new GfshParserRule(); @@ -38,11 +43,11 @@ public class AlterDiskStoreJUnitTest { } @Test - public void removeOptionMustBeUsedAlone() throws Exception { + public void removeOptionMustBeUsedAlone() { CommandStringBuilder csb = new CommandStringBuilder(CliStrings.ALTER_DISK_STORE); csb.addOption(CliStrings.ALTER_DISK_STORE__DISKSTORENAME, "diskStoreName"); csb.addOption(CliStrings.ALTER_DISK_STORE__REGIONNAME, "regionName"); - csb.addOption(CliStrings.ALTER_DISK_STORE__DISKDIRS, "./someDirectory"); + csb.addOption(CliStrings.ALTER_DISK_STORE__DISKDIRS, tempDir.getRoot().toString()); csb.addOption(CliStrings.ALTER_DISK_STORE__CONCURRENCY__LEVEL, "5"); csb.addOption(CliStrings.ALTER_DISK_STORE__REMOVE, "true"); String commandString = csb.toString(); diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java index 4ffcc63..7b5d204 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/AlterOfflineDiskStoreCommand.java @@ -12,7 +12,6 @@ * 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 java.io.File; @@ -30,7 +29,7 @@ import org.apache.geode.management.internal.cli.result.model.ResultModel; public class AlterOfflineDiskStoreCommand extends SingleGfshCommand { @CliCommand(value = CliStrings.ALTER_DISK_STORE, help = CliStrings.ALTER_DISK_STORE__HELP) - @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE}) + @CliMetaData(shellOnly = true, relatedTopic = CliStrings.TOPIC_GEODE_DISKSTORE) public ResultModel alterOfflineDiskStore( @CliOption(key = CliStrings.ALTER_DISK_STORE__DISKSTORENAME, mandatory = true, help = CliStrings.ALTER_DISK_STORE__DISKSTORENAME__HELP) String diskStoreName, @@ -60,6 +59,13 @@ public class AlterOfflineDiskStoreCommand extends SingleGfshCommand { help = CliStrings.ALTER_DISK_STORE__REMOVE__HELP, specifiedDefaultValue = "true", unspecifiedDefaultValue = "false") boolean remove) { + String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs); + if (validatedDirectories != null) { + throw new IllegalArgumentException( + "Could not find " + CliStrings.ALTER_DISK_STORE__DISKDIRS + ": \"" + + validatedDirectories + "\""); + } + try { File[] dirs = null; @@ -124,5 +130,4 @@ public class AlterOfflineDiskStoreCommand extends SingleGfshCommand { return ResultModel.createError(e.getMessage()); } } - } diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java index 3c1d974..f6a7a55 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactOfflineDiskStoreCommand.java @@ -12,7 +12,6 @@ * 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 java.io.BufferedReader; @@ -39,7 +38,7 @@ import org.apache.geode.management.internal.cli.util.DiskStoreCompacter; public class CompactOfflineDiskStoreCommand extends SingleGfshCommand { @CliCommand(value = CliStrings.COMPACT_OFFLINE_DISK_STORE, help = CliStrings.COMPACT_OFFLINE_DISK_STORE__HELP) - @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE}) + @CliMetaData(shellOnly = true, relatedTopic = CliStrings.TOPIC_GEODE_DISKSTORE) public ResultModel compactOfflineDiskStore( @CliOption(key = CliStrings.COMPACT_OFFLINE_DISK_STORE__NAME, mandatory = true, help = CliStrings.COMPACT_OFFLINE_DISK_STORE__NAME__HELP) String diskStoreName, @@ -50,20 +49,20 @@ public class CompactOfflineDiskStoreCommand extends SingleGfshCommand { help = CliStrings.COMPACT_OFFLINE_DISK_STORE__MAXOPLOGSIZE__HELP) long maxOplogSize, @CliOption(key = CliStrings.COMPACT_OFFLINE_DISK_STORE__J, help = CliStrings.COMPACT_OFFLINE_DISK_STORE__J__HELP) String[] jvmProps) { + + String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs); + if (validatedDirectories != null) { + throw new IllegalArgumentException( + "Could not find " + CliStrings.COMPACT_OFFLINE_DISK_STORE__DISKDIRS + ": \"" + + validatedDirectories + "\""); + } + ResultModel result = new ResultModel(); InfoResultModel infoResult = result.addInfo(); LogWrapper logWrapper = LogWrapper.getInstance(getCache()); - Process compactorProcess = null; try { - String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs); - if (validatedDirectories != null) { - throw new IllegalArgumentException( - "Could not find " + CliStrings.COMPACT_OFFLINE_DISK_STORE__DISKDIRS + ": \"" - + validatedDirectories + "\""); - } - List<String> commandList = new ArrayList<>(); commandList.add(System.getProperty("java.home") + File.separatorChar + "bin" + File.separatorChar + "java"); diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java index e73d919..2411b60 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeOfflineDiskStoreCommand.java @@ -12,7 +12,6 @@ * 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 java.io.ByteArrayOutputStream; @@ -33,7 +32,7 @@ import org.apache.geode.management.internal.cli.result.model.ResultModel; public class DescribeOfflineDiskStoreCommand extends SingleGfshCommand { @CliCommand(value = CliStrings.DESCRIBE_OFFLINE_DISK_STORE, help = CliStrings.DESCRIBE_OFFLINE_DISK_STORE__HELP) - @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE}) + @CliMetaData(shellOnly = true, relatedTopic = CliStrings.TOPIC_GEODE_DISKSTORE) public ResultModel describeOfflineDiskStore( @CliOption(key = CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKSTORENAME, mandatory = true, help = CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKSTORENAME__HELP) String diskStoreName, @@ -44,6 +43,13 @@ public class DescribeOfflineDiskStoreCommand extends SingleGfshCommand { @CliOption(key = CliStrings.DESCRIBE_OFFLINE_DISK_STORE__REGIONNAME, help = CliStrings.DESCRIBE_OFFLINE_DISK_STORE__REGIONNAME__HELP) String regionName) { + String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs); + if (validatedDirectories != null) { + throw new IllegalArgumentException( + "Could not find " + CliStrings.DESCRIBE_OFFLINE_DISK_STORE__DISKDIRS + ": \"" + + validatedDirectories + "\""); + } + try { final File[] dirs = new File[diskDirs.length]; for (int i = 0; i < diskDirs.length; i++) { diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java index 7e48c08..f1496ad 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UpgradeOfflineDiskStoreCommand.java @@ -12,7 +12,6 @@ * 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 java.io.BufferedReader; @@ -39,7 +38,7 @@ import org.apache.geode.management.internal.cli.util.DiskStoreUpgrader; public class UpgradeOfflineDiskStoreCommand extends SingleGfshCommand { @CliCommand(value = CliStrings.UPGRADE_OFFLINE_DISK_STORE, help = CliStrings.UPGRADE_OFFLINE_DISK_STORE__HELP) - @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE}) + @CliMetaData(shellOnly = true, relatedTopic = CliStrings.TOPIC_GEODE_DISKSTORE) public ResultModel upgradeOfflineDiskStore( @CliOption(key = CliStrings.UPGRADE_OFFLINE_DISK_STORE__NAME, mandatory = true, help = CliStrings.UPGRADE_OFFLINE_DISK_STORE__NAME__HELP) String diskStoreName, @@ -51,6 +50,13 @@ public class UpgradeOfflineDiskStoreCommand extends SingleGfshCommand { @CliOption(key = CliStrings.UPGRADE_OFFLINE_DISK_STORE__J, help = CliStrings.UPGRADE_OFFLINE_DISK_STORE__J__HELP) String[] jvmProps) { + String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs); + if (validatedDirectories != null) { + throw new IllegalArgumentException( + "Could not find " + CliStrings.UPGRADE_OFFLINE_DISK_STORE__DISKDIRS + ": \"" + + validatedDirectories + "\""); + } + ResultModel result = new ResultModel(); InfoResultModel infoResult = result.addInfo(); LogWrapper logWrapper = LogWrapper.getInstance(getCache()); @@ -58,13 +64,6 @@ public class UpgradeOfflineDiskStoreCommand extends SingleGfshCommand { Process upgraderProcess = null; try { - String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs); - if (validatedDirectories != null) { - throw new IllegalArgumentException( - "Could not find " + CliStrings.UPGRADE_OFFLINE_DISK_STORE__DISKDIRS + ": \"" - + validatedDirectories + "\""); - } - List<String> commandList = new ArrayList<>(); commandList.add(System.getProperty("java.home") + File.separatorChar + "bin" + File.separatorChar + "java"); diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java index 077b576..efbff96 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ValidateDiskStoreCommand.java @@ -12,7 +12,6 @@ * 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 java.io.BufferedReader; @@ -38,7 +37,7 @@ import org.apache.geode.management.internal.cli.util.DiskStoreValidater; public class ValidateDiskStoreCommand extends GfshCommand { @CliCommand(value = CliStrings.VALIDATE_DISK_STORE, help = CliStrings.VALIDATE_DISK_STORE__HELP) - @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE}) + @CliMetaData(shellOnly = true, relatedTopic = CliStrings.TOPIC_GEODE_DISKSTORE) public ResultModel validateDiskStore( @CliOption(key = CliStrings.VALIDATE_DISK_STORE__NAME, mandatory = true, help = CliStrings.VALIDATE_DISK_STORE__NAME__HELP) String diskStoreName, @@ -47,6 +46,13 @@ public class ValidateDiskStoreCommand extends GfshCommand { @CliOption(key = CliStrings.VALIDATE_DISK_STORE__J, help = CliStrings.VALIDATE_DISK_STORE__J__HELP) String[] jvmProps) { + String validatedDirectories = DiskStoreCommandsUtils.validatedDirectories(diskDirs); + if (validatedDirectories != null) { + throw new IllegalArgumentException( + "Could not find " + CliStrings.VALIDATE_DISK_STORE__DISKDIRS + ": \"" + + validatedDirectories + "\""); + } + ResultModel result = new ResultModel(); InfoResultModel infoResult = result.addInfo(); LogWrapper logWrapper = LogWrapper.getInstance(getCache());