geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aba...@apache.org
Subject [3/3] incubator-geode git commit: Addressing Review comments.
Date Sat, 12 Nov 2016 17:44:29 GMT
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/develop
Commit: e9c571692c9875f1ec5113c06703703b7206099f
Parents: 86f4755
Author: adongre <avinash@ampool.io>
Authored: Sun Nov 6 17:27:55 2016 +0530
Committer: Anthony Baker <abaker@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


Mime
View raw message