Addressing Review comments. This closes #278
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/e9c57169 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/e9c57169 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/e9c57169 Branch: refs/heads/feature/GEODE-1930 Commit: e9c571692c9875f1ec5113c06703703b7206099f Parents: 86f4755 Author: adongre <avin...@ampool.io> Authored: Sun Nov 6 17:27:55 2016 +0530 Committer: Anthony Baker <aba...@apache.org> Committed: Sat Nov 12 07:56:16 2016 -0800 ---------------------------------------------------------------------- .../CreateAlterDestroyRegionCommands.java | 22 ++++++-------------- .../cli/functions/RegionCreateFunction.java | 18 +++++++++------- ...eateAlterDestroyRegionCommandsDUnitTest.java | 6 +++--- 3 files changed, 19 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9c57169/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java index 58af6cb..358cdc1 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java @@ -31,7 +31,6 @@ import javax.management.MBeanServer; import javax.management.MalformedObjectNameException; import javax.management.ObjectName; -import joptsimple.internal.Strings; import org.apache.geode.cache.*; import org.springframework.shell.core.annotation.CliAvailabilityIndicator; import org.springframework.shell.core.annotation.CliCommand; @@ -905,26 +904,17 @@ public class CreateAlterDestroyRegionCommands extends AbstractCommandsSupport { } if (regionFunctionArgs.hasPartitionAttributes()) { - boolean partitionResolverFailure = false; if (regionFunctionArgs.isPartitionResolverSet()) { String partitionResolverClassName = regionFunctionArgs.getPartitionResolver(); - Object partitionResolver = null; - try { - Class<?> compressorClass = - (Class<?>) ClassPathLoader.getLatest().forName(partitionResolverClassName); - partitionResolver = compressorClass.newInstance(); - } catch (InstantiationException e) { - partitionResolverFailure = true; - } catch (IllegalAccessException e) { - partitionResolverFailure = true; - } catch (ClassNotFoundException e) { - partitionResolverFailure = true; - } - if (partitionResolverFailure || !(partitionResolver instanceof PartitionResolver)) { + Class<PartitionResolver> resolverClass = (Class<PartitionResolver>) ClassPathLoader + .getLatest().forName(partitionResolverClassName); + PartitionResolver partitionResolver = resolverClass.newInstance(); + } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) { throw new IllegalArgumentException( CliStrings.format(CliStrings.CREATE_REGION__MSG__INVALID_PARTITION_RESOLVER, - new Object[] {regionFunctionArgs.getCompressor()})); + new Object[] {regionFunctionArgs.getCompressor()}), + e); } } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9c57169/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java index b925936..c99f84a 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java @@ -16,8 +16,8 @@ package org.apache.geode.management.internal.cli.functions; import java.util.Set; -import joptsimple.internal.Strings; import org.apache.geode.internal.ClassPathLoader; +import org.apache.geode.internal.lang.StringUtils; import org.apache.logging.log4j.Logger; import org.apache.geode.cache.Cache; @@ -409,8 +409,8 @@ public class RegionCreateFunction extends FunctionAdapter implements InternalEnt } if (regionCreateArgs.isPartitionResolverSet()) { - Class<?> partitionResolverClass = forName(regionCreateArgs.getPartitionResolver(), - CliStrings.CREATE_REGION__PARTITION_RESOLVER); + Class<PartitionResolver> partitionResolverClass = forName( + regionCreateArgs.getPartitionResolver(), CliStrings.CREATE_REGION__PARTITION_RESOLVER); prAttrFactory .setPartitionResolver((PartitionResolver<K, V>) newInstance(partitionResolverClass, CliStrings.CREATE_REGION__PARTITION_RESOLVER)); @@ -419,12 +419,14 @@ public class RegionCreateFunction extends FunctionAdapter implements InternalEnt } - private static Class<?> forName(String className, String neededFor) { - if (Strings.isNullOrEmpty(className)) { - return null; + private static Class<PartitionResolver> forName(String className, String neededFor) { + if (StringUtils.isBlank(className)) { + throw new IllegalArgumentException( + CliStrings.format(CliStrings.CREATE_REGION__MSG__INVALID_PARTITION_RESOLVER, + new Object[] {className, neededFor})); } try { - return ClassPathLoader.getLatest().forName(className); + return (Class<PartitionResolver>) ClassPathLoader.getLatest().forName(className); } catch (ClassNotFoundException e) { throw new RuntimeException(CliStrings.format( CliStrings.CREATE_REGION_PARTITION_RESOLVER__MSG__COULDNOT_FIND_CLASS_0_SPECIFIED_FOR_1, @@ -436,7 +438,7 @@ public class RegionCreateFunction extends FunctionAdapter implements InternalEnt } } - private static Object newInstance(Class<?> klass, String neededFor) { + private static PartitionResolver newInstance(Class<PartitionResolver> klass, String neededFor) { try { return klass.newInstance(); } catch (InstantiationException e) { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e9c57169/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java index 4896ead..7a75cfa 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommandsDUnitTest.java @@ -1112,13 +1112,14 @@ public class CreateAlterDestroyRegionCommandsDUnitTest extends CliCommandTestBas ClassBuilder classBuilder = new ClassBuilder(); // classBuilder.addToClassPath("."); - final File prJarFile = new File(new File(".").getAbsolutePath(), "myPartitionResolver.jar"); + final File prJarFile = new File(temporaryFolder.getRoot().getCanonicalPath() + File.separator, + "myPartitionResolver.jar"); this.filesToBeDeleted.add(prJarFile.getAbsolutePath()); byte[] jarBytes = classBuilder.createJarFromClassContent("com/cadrdunit/TestPartitionResolver", PR_STRING); writeJarBytesToFile(prJarFile, jarBytes); - CommandResult cmdResult = executeCommand("deploy --jar=myPartitionResolver.jar"); + CommandResult cmdResult = executeCommand("deploy --jar=" + prJarFile.getAbsolutePath()); assertEquals(Result.Status.OK, cmdResult.getStatus()); @@ -1194,7 +1195,6 @@ public class CreateAlterDestroyRegionCommandsDUnitTest extends CliCommandTestBas commandStringBuilder.addOption(CliStrings.CREATE_REGION__REGIONSHORTCUT, "REPLICATE"); commandStringBuilder.addOption(CliStrings.CREATE_REGION__PARTITION_RESOLVER, "a.b.c.d"); CommandResult cmdResult = executeCommand(commandStringBuilder.toString()); - System.out.println("Result --> " + cmdResult); assertEquals(Result.Status.ERROR, cmdResult.getStatus()); // Assert that our region was not created
