geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jensde...@apache.org
Subject [geode] branch develop updated: GEODE-3007: Simplify support for custom GFSH commands (#1042)
Date Thu, 16 Nov 2017 02:31:27 GMT
This is an automated email from the ASF dual-hosted git repository.

jensdeppe 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 0e5dd6b  GEODE-3007: Simplify support for custom GFSH commands (#1042)
0e5dd6b is described below

commit 0e5dd6ba45519463147337c8265db15e8f1840fc
Author: Jens Deppe <jdeppe@pivotal.io>
AuthorDate: Wed Nov 15 18:31:25 2017 -0800

    GEODE-3007: Simplify support for custom GFSH commands (#1042)
    
    * GEODE-3007: Simplify support for custom GFSH commands
---
 geode-assembly/build.gradle                        |   2 -
 .../cli/commands/StatusLocatorRealGfshTest.java    |   1 -
 .../geode/distributed/ConfigurationProperties.java |   3 +
 .../internal/AbstractDistributionConfig.java       |   3 -
 .../distributed/internal/DistributionConfig.java   |  28 --
 .../internal/DistributionConfigImpl.java           |  21 +-
 .../internal/beans/MemberMBeanBridge.java          |   3 +-
 .../management/internal/cli/CommandManager.java    | 307 ++++-----------------
 .../geode/management/internal/cli/GfshParser.java  |   8 +-
 .../geode/management/internal/cli/help/Helper.java |  63 +++--
 .../internal/cli/remote/MemberCommandService.java  |   3 +-
 .../cli/remote/OnlineCommandProcessor.java         |  16 +-
 .../internal/cli/util/ClasspathScanLoadHelper.java |  23 +-
 .../internal/DistributionConfigJUnitTest.java      |   4 +-
 .../cli/ClasspathScanLoadHelperJUnitTest.java      |  27 +-
 .../internal/cli/CommandManagerJUnitTest.java      | 156 +----------
 .../internal/cli/commands/CliCommandTestBase.java  |   4 +-
 .../cli/remote/OnlineCommandProcessorTest.java     |   6 +-
 .../org.springframework.shell.core.CommandMarker   |   8 -
 .../org.springframework.shell.core.CommandMarker   |   2 -
 20 files changed, 151 insertions(+), 537 deletions(-)

diff --git a/geode-assembly/build.gradle b/geode-assembly/build.gradle
index 1f139a5..4615a6b 100755
--- a/geode-assembly/build.gradle
+++ b/geode-assembly/build.gradle
@@ -198,14 +198,12 @@ def cp = {
         it.contains('snappy') ||
         it.contains('jgroups') ||
         it.contains('netty') ||
-
         // dependencies from geode-lucene
         it.contains('lucene-analyzers-common') ||
         it.contains('lucene-core') ||
         it.contains('lucene-queries') ||
         it.contains('lucene-queryparser') ||
         it.contains('lucene-analyzers-phonetic') ||
-
         // dependencies from geode-protobuf
         it.contains('protobuf-java')
       }
diff --git a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
index 7e7b2bd..94ddc8f 100644
--- a/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
+++ b/geode-assembly/src/test/java/org/apache/geode/management/internal/cli/commands/StatusLocatorRealGfshTest.java
@@ -30,7 +30,6 @@ public class StatusLocatorRealGfshTest {
   @Test
   public void statusLocatorSucceedsWhenConnected() throws Exception {
     GfshScript.of("start locator --name=locator1").execute(gfshRule);
-
     GfshScript.of("connect", "status locator --name=locator1").execute(gfshRule);
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
index 649bfad..4944e98 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
@@ -1860,7 +1860,10 @@ public interface ConfigurationProperties {
    * <U>Default</U>: <code>""</code>
    * </p>
    * <U>Since</U>: GemFire 8.0
+   *
+   * @deprecated Since Geode 1.4 (See GEODE-3007)
    */
+  @Deprecated()
   String USER_COMMAND_PACKAGES = "user-command-packages";
   /**
    * The static String definition of the <i>"off-heap-memory-size"</i> property <a
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
index 43c5cb4..7c5929c 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
@@ -1018,9 +1018,6 @@ public abstract class AbstractDistributionConfig extends AbstractConfig
     m.put(GROUPS,
         "A comma separated list of all the groups this member belongs to." + " Defaults to \"\".");
 
-    m.put(USER_COMMAND_PACKAGES,
-        "A comma separated list of the names of the packages containing classes that implement user commands.");
-
     m.put(JMX_MANAGER,
         "If true then this member is willing to be a jmx manager. Defaults to false except on a locator.");
     m.put(JMX_MANAGER_START,
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
index eabe6cc..3aeb19f 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java
@@ -387,34 +387,6 @@ public interface DistributionConfig extends Config, LogConfig {
   File DEFAULT_DEPLOY_WORKING_DIR = new File(System.getProperty("user.dir"));
 
   /**
-   * Returns the value of the {@link ConfigurationProperties#USER_COMMAND_PACKAGES} property
-   */
-  @ConfigAttributeGetter(name = USER_COMMAND_PACKAGES)
-  String getUserCommandPackages();
-
-  /**
-   * Sets the system's user command path.
-   *
-   * @throws IllegalArgumentException if the specified value is not acceptable.
-   * @throws org.apache.geode.UnmodifiableException if this attribute can not be modified.
-   * @throws org.apache.geode.GemFireIOException if the set failure is caused by an error when
-   *         writing to the system's configuration file.
-   */
-  @ConfigAttributeSetter(name = USER_COMMAND_PACKAGES)
-  void setUserCommandPackages(String value);
-
-  /**
-   * The name of the {@link ConfigurationProperties#USER_COMMAND_PACKAGES} property.
-   */
-  @ConfigAttribute(type = String.class)
-  String USER_COMMAND_PACKAGES_NAME = USER_COMMAND_PACKAGES;
-
-  /**
-   * The default value of the {@link ConfigurationProperties#USER_COMMAND_PACKAGES} property
-   */
-  String DEFAULT_USER_COMMAND_PACKAGES = "";
-
-  /**
    * Returns the value of the {@link ConfigurationProperties#LOG_FILE} property
    *
    * @return <code>null</code> if logging information goes to standard out
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
index cceeaf9..0c13480 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java
@@ -625,8 +625,6 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement
   private Map<String, ConfigSource> sourceMap =
       Collections.synchronizedMap(new HashMap<String, ConfigSource>());
 
-  protected String userCommandPackages = DEFAULT_USER_COMMAND_PACKAGES;
-
   /**
    * "off-heap-memory-size" with value of "" or "<size>[g|m]"
    */
@@ -759,7 +757,6 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement
     this.redisPort = other.getRedisPort();
     this.redisBindAddress = other.getRedisBindAddress();
     this.redisPassword = other.getRedisPassword();
-    this.userCommandPackages = other.getUserCommandPackages();
 
     // following added for 8.0
     this.enableSharedConfiguration = other.getEnableClusterConfiguration();
@@ -1834,10 +1831,6 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement
     return this.asyncMaxQueueSize;
   }
 
-  public String getUserCommandPackages() {
-    return this.userCommandPackages;
-  }
-
   public int getHttpServicePort() {
     return this.httpServicePort;
   }
@@ -1862,10 +1855,6 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement
     this.startDevRestApi = value;
   }
 
-  public void setUserCommandPackages(String value) {
-    this.userCommandPackages = value;
-  }
-
   public boolean getDeltaPropagation() {
     return this.deltaPropagation;
   }
@@ -3013,9 +3002,8 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement
         .append(sslTrustStore, that.sslTrustStore)
         .append(sslTrustStorePassword, that.sslTrustStorePassword)
         .append(locatorSSLAlias, that.locatorSSLAlias).append(sslDefaultAlias, that.sslDefaultAlias)
-        .append(sourceMap, that.sourceMap).append(userCommandPackages, that.userCommandPackages)
-        .append(offHeapMemorySize, that.offHeapMemorySize).append(shiroInit, that.shiroInit)
-        .isEquals();
+        .append(sourceMap, that.sourceMap).append(offHeapMemorySize, that.offHeapMemorySize)
+        .append(shiroInit, that.shiroInit).isEquals();
   }
 
   /**
@@ -3083,9 +3071,8 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement
         .append(sslCiphers).append(sslRequireAuthentication).append(sslKeyStore)
         .append(sslKeyStoreType).append(sslKeyStorePassword).append(sslTrustStore)
         .append(sslTrustStorePassword).append(sslWebServiceRequireAuthentication)
-        .append(locatorSSLAlias).append(sslDefaultAlias).append(sourceMap)
-        .append(userCommandPackages).append(offHeapMemorySize).append(lockMemory).append(shiroInit)
-        .append(modifiable).toHashCode();
+        .append(locatorSSLAlias).append(sslDefaultAlias).append(sourceMap).append(offHeapMemorySize)
+        .append(lockMemory).append(shiroInit).append(modifiable).toHashCode();
   }
 
   /**
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
index 0afdfd1..de466af 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
@@ -338,8 +338,7 @@ public class MemberMBeanBridge {
 
     this.config = system.getConfig();
     try {
-      this.commandProcessor =
-          new OnlineCommandProcessor(system.getProperties(), cache.getSecurityService());
+      this.commandProcessor = new OnlineCommandProcessor(cache.getSecurityService());
     } catch (Exception e) {
       commandServiceInitError = e.getMessage();
       logger.info(LogMarker.CONFIG, "Command processor could not be initialized. {}",
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
index 44617d3..0b13d62 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
@@ -14,293 +14,93 @@
  */
 package org.apache.geode.management.internal.cli;
 
-import static org.apache.geode.distributed.ConfigurationProperties.USER_COMMAND_PACKAGES;
 
-import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Properties;
-import java.util.ServiceConfigurationError;
-import java.util.ServiceLoader;
+import static java.util.stream.Collectors.toSet;
+
+import java.util.Objects;
 import java.util.Set;
-import java.util.StringTokenizer;
+import java.util.stream.Stream;
 
-import org.springframework.shell.converters.EnumConverter;
-import org.springframework.shell.converters.SimpleFileConverter;
 import org.springframework.shell.core.CommandMarker;
 import org.springframework.shell.core.Converter;
-import org.springframework.shell.core.MethodTarget;
-import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
-import org.springframework.shell.core.annotation.CliCommand;
 
-import org.apache.geode.distributed.ConfigurationProperties;
-import org.apache.geode.distributed.internal.DistributionConfig;
-import org.apache.geode.internal.ClassPathLoader;
-import org.apache.geode.management.internal.cli.commands.GfshCommand;
 import org.apache.geode.management.internal.cli.help.Helper;
 import org.apache.geode.management.internal.cli.shell.Gfsh;
 import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
 
 /**
- *
  * this only takes care of loading all available command markers and converters from the application
  *
  * @since GemFire 7.0
  */
 public class CommandManager {
-  public static final String USER_CMD_PACKAGES_PROPERTY =
-      DistributionConfig.GEMFIRE_PREFIX + USER_COMMAND_PACKAGES;
-  public static final String USER_CMD_PACKAGES_ENV_VARIABLE = "GEMFIRE_USER_COMMAND_PACKAGES";
-  private static final Object INSTANCE_LOCK = new Object();
-
-  private final Helper helper = new Helper();
-
-  private final List<Converter<?>> converters = new ArrayList<Converter<?>>();
-  private final List<CommandMarker> commandMarkers = new ArrayList<>();
+  // Skip some of the Converters from Spring Shell for our customization
+  private static final Set<String> EXCLUDED_CLASSES =
+      Stream.of("-org.springframework.shell.converters.SimpleFileConverter",
+          "-org.springframework.shell.converters.FileConverter",
+          "-org.springframework.shell.converters.EnumConverter",
+          "-org.springframework.shell.commands.ExitCommands",
+          "-org.springframework.shell.commands.HelpCommands",
+          "-org.springframework.shell.commands.VersionCommands",
+          "-org.springframework.shell.commands.ConsoleCommands").collect(toSet());
+
+  private final LogWrapper logWrapper = LogWrapper.getInstance();
+  private final Set<CommandMarker> commandMarkers;
+  private final Set<Converter> converters;
+  private final Helper helper;
 
-  private Properties cacheProperties;
-  private LogWrapper logWrapper;
-
-  /**
-   * this constructor is used from Gfsh VM. We are getting the user-command-package from system
-   * environment. used by Gfsh.
-   */
   public CommandManager() {
-    this(null);
+    helper = new Helper();
+    converters = loadConverters();
+    commandMarkers = loadCommandMarkers();
   }
 
-  /**
-   * this is used when getting the instance in a cache server. We are getting the
-   * user-command-package from distribution properties. used by OnlineCommandProcessor.
-   */
-  public CommandManager(final Properties cacheProperties) {
-    if (cacheProperties != null) {
-      this.cacheProperties = cacheProperties;
-    }
-    logWrapper = LogWrapper.getInstance();
-    loadCommands();
-  }
+  private Set<Converter> loadConverters() {
+    Set<Converter> converters = instantiateAllClassesImplementing(Converter.class);
+    raiseExceptionIfEmpty(converters, "converters");
 
-  private static void raiseExceptionIfEmpty(Set<Class<?>> foundClasses, String errorFor)
-      throws IllegalStateException {
-    if (foundClasses == null || foundClasses.isEmpty()) {
-      throw new IllegalStateException(
-          "Required " + errorFor + " classes were not loaded. Check logs for errors.");
-    }
+    converters.forEach(this::setContextIfCommandManagerAware);
+    return converters;
   }
 
-  private void loadUserCommands() {
-    final Set<String> userCommandPackages = new HashSet<String>();
-
-    // Find by packages specified by the system property
-    if (System.getProperty(USER_CMD_PACKAGES_PROPERTY) != null) {
-      StringTokenizer tokenizer =
-          new StringTokenizer(System.getProperty(USER_CMD_PACKAGES_PROPERTY), ",");
-      while (tokenizer.hasMoreTokens()) {
-        userCommandPackages.add(tokenizer.nextToken());
-      }
-    }
-
-    // Find by packages specified by the environment variable
-    if (System.getenv().containsKey(USER_CMD_PACKAGES_ENV_VARIABLE)) {
-      StringTokenizer tokenizer =
-          new StringTokenizer(System.getenv().get(USER_CMD_PACKAGES_ENV_VARIABLE), ",");
-      while (tokenizer.hasMoreTokens()) {
-        userCommandPackages.add(tokenizer.nextToken());
-      }
-    }
+  private Set<CommandMarker> loadCommandMarkers() {
+    Set<CommandMarker> commandMarkers = instantiateAllClassesImplementing(CommandMarker.class);
+    raiseExceptionIfEmpty(commandMarkers, "commandMarkers");
 
-    // Find by packages specified in the distribution config
-    if (this.cacheProperties != null) {
-      String cacheUserCmdPackages =
-          this.cacheProperties.getProperty(ConfigurationProperties.USER_COMMAND_PACKAGES);
-      if (cacheUserCmdPackages != null && !cacheUserCmdPackages.isEmpty()) {
-        StringTokenizer tokenizer = new StringTokenizer(cacheUserCmdPackages, ",");
-        while (tokenizer.hasMoreTokens()) {
-          userCommandPackages.add(tokenizer.nextToken());
-        }
-      }
-    }
-
-    // Load commands found in all of the packages
-    for (String userCommandPackage : userCommandPackages) {
-      try {
-        Set<Class<?>> foundClasses = ClasspathScanLoadHelper
-            .scanPackageForClassesImplementing(userCommandPackage, CommandMarker.class);
-        for (Class<?> klass : foundClasses) {
-          try {
-            add((CommandMarker) klass.newInstance());
-          } catch (Exception e) {
-            logWrapper.warning("Could not load User Commands from: " + klass + " due to "
-                + e.getLocalizedMessage()); // continue
-          }
-        }
-        raiseExceptionIfEmpty(foundClasses, "User Command");
-      } catch (IllegalStateException e) {
-        logWrapper.warning(e.getMessage(), e);
-        throw e;
-      }
-    }
-  }
-
-  /**
-   * Loads commands via {@link ServiceLoader} from {@link ClassPathLoader}.
-   *
-   * @since GemFire 8.1
-   */
-  private void loadPluginCommands() {
-    final Iterator<CommandMarker> iterator = ServiceLoader
-        .load(CommandMarker.class, ClassPathLoader.getLatest().asClassLoader()).iterator();
-    while (iterator.hasNext()) {
-      try {
-        final CommandMarker commandMarker = iterator.next();
-        try {
-          add(commandMarker);
-        } catch (Exception e) {
-          logWrapper.warning("Could not load Command from: " + commandMarker.getClass() + " due to "
-              + e.getLocalizedMessage(), e); // continue
-        }
-      } catch (ServiceConfigurationError e) {
-        logWrapper.severe("Could not load Command: " + e.getLocalizedMessage(), e); // continue
-      }
-    }
+    commandMarkers.forEach(this::setContextIfCommandManagerAware);
+    commandMarkers.forEach(helper::registerCommand);
+    return commandMarkers;
   }
 
+  private <T> Set<T> instantiateAllClassesImplementing(Class<T> implementedInterface) {
+    Set<Class<? extends T>> classes = ClasspathScanLoadHelper.scanClasspathForClassesImplementing(
+        implementedInterface, EXCLUDED_CLASSES.toArray(new String[0]));
 
-  private void loadCommands() {
-    loadUserCommands();
-
-    loadPluginCommands();
-    loadGeodeCommands();
-    loadConverters();
+    return classes.stream().map(this::instantiateClass).filter(Objects::nonNull).collect(toSet());
   }
 
-  private void loadConverters() {
-    Set<Class<?>> foundClasses;
-    // Converters
+  private <T> T instantiateClass(Class<T> classToInstantiate) {
     try {
-      foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
-          "org.apache.geode.management.internal.cli.converters", Converter.class);
-      for (Class<?> klass : foundClasses) {
-        try {
-          Converter<?> object = (Converter<?>) klass.newInstance();
-          add(object);
-
-        } catch (Exception e) {
-          logWrapper.warning(
-              "Could not load Converter from: " + klass + " due to " + e.getLocalizedMessage()); // continue
-        }
-      }
-      raiseExceptionIfEmpty(foundClasses, "Converters");
-
-      // Spring shell's converters
-      foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
-          "org.springframework.shell.converters", Converter.class);
-      for (Class<?> klass : foundClasses) {
-        if (!SHL_CONVERTERS_TOSKIP.contains(klass)) {
-          try {
-            add((Converter<?>) klass.newInstance());
-          } catch (Exception e) {
-            logWrapper.warning(
-                "Could not load Converter from: " + klass + " due to " + e.getLocalizedMessage()); // continue
-          }
-        }
-      }
-      raiseExceptionIfEmpty(foundClasses, "Basic Converters");
-    } catch (IllegalStateException e) {
-      logWrapper.warning(e.getMessage(), e);
-      throw e;
+      return classToInstantiate.newInstance();
+    } catch (Exception e) {
+      logWrapper.warning("Could not load command or converter from: " + classToInstantiate, e);
     }
+    return null;
   }
 
-  private void loadGeodeCommands() {
-    // CommandMarkers
-    Set<Class<?>> foundClasses;
-    try {
-      // geode's commands
-      foundClasses = ClasspathScanLoadHelper.scanPackageForClassesImplementing(
-          GfshCommand.class.getPackage().getName(), CommandMarker.class);
-
-      for (Class<?> klass : foundClasses) {
-        try {
-          add((CommandMarker) klass.newInstance());
-        } catch (Exception e) {
-          logWrapper.warning(
-              "Could not load Command from: " + klass + " due to " + e.getLocalizedMessage()); // continue
-        }
-      }
-      raiseExceptionIfEmpty(foundClasses, "Commands");
-
-      // do not add Spring shell's commands for now. When we add it, we need to tell the parser that
-      // these are offline commands.
-    } catch (IllegalStateException e) {
-      logWrapper.warning(e.getMessage(), e);
-      throw e;
+  private void setContextIfCommandManagerAware(Object commandOrConverter) {
+    if (CommandManagerAware.class.isAssignableFrom(commandOrConverter.getClass())) {
+      ((CommandManagerAware) commandOrConverter).setCommandManager(this);
     }
   }
 
-  /** Skip some of the Converters from Spring Shell for our customization */
-  private static List<Class> SHL_CONVERTERS_TOSKIP = new ArrayList();
-  static {
-    // skip springs SimpleFileConverter to use our own FilePathConverter
-    SHL_CONVERTERS_TOSKIP.add(SimpleFileConverter.class);
-    // skip spring's EnumConverter to use our own EnumConverter
-    SHL_CONVERTERS_TOSKIP.add(EnumConverter.class);
-  }
-
-  public List<Converter<?>> getConverters() {
-    return converters;
-  }
-
-  public List<CommandMarker> getCommandMarkers() {
-    return commandMarkers;
-  }
-
-  /**
-   * Method to add new Converter
-   *
-   * @param converter
-   */
-  void add(Converter<?> converter) {
-    if (CommandManagerAware.class.isAssignableFrom(converter.getClass())) {
-      ((CommandManagerAware) converter).setCommandManager(this);
-    }
-    converters.add(converter);
-  }
-
-  /**
-   * Method to add new Commands to the parser
-   *
-   * @param commandMarker
-   */
-  void add(CommandMarker commandMarker) {
-    if (CommandManagerAware.class.isAssignableFrom(commandMarker.getClass())) {
-      ((CommandManagerAware) commandMarker).setCommandManager(this);
-    }
-    commandMarkers.add(commandMarker);
-    for (Method method : commandMarker.getClass().getMethods()) {
-      CliCommand cliCommand = method.getAnnotation(CliCommand.class);
-      CliAvailabilityIndicator availability = method.getAnnotation(CliAvailabilityIndicator.class);
-      if (cliCommand == null && availability == null) {
-        continue;
-      }
-
-      if (cliCommand != null) {
-        helper.addCommand(cliCommand, method);
-      }
-
-      if (availability != null) {
-        helper.addAvailabilityIndicator(availability, new MethodTarget(method, commandMarker));
-      }
+  private static void raiseExceptionIfEmpty(Set<?> foundClasses, String classType)
+      throws IllegalStateException {
+    if (foundClasses == null || foundClasses.isEmpty()) {
+      throw new IllegalStateException("No " + classType + " were loaded. Check logs for errors.");
     }
   }
 
-  public Helper getHelper() {
-    return helper;
-  }
-
   public String obtainHelp(String buffer) {
     int terminalWidth = -1;
     Gfsh gfsh = Gfsh.getCurrentInstance();
@@ -314,4 +114,15 @@ public class CommandManager {
     return helper.getHint(topic);
   }
 
+  public Set<Converter> getConverters() {
+    return converters;
+  }
+
+  public Set<CommandMarker> getCommandMarkers() {
+    return commandMarkers;
+  }
+
+  public Helper getHelper() {
+    return helper;
+  }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
index 4fc7d4e..4d3b546 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
@@ -57,10 +57,14 @@ public class GfshParser extends SimpleParser {
       add(command);
     }
 
-    for (Converter<?> converter : commandManager.getConverters()) {
+    for (Converter converter : commandManager.getConverters()) {
       if (converter.getClass().isAssignableFrom(ArrayConverter.class)) {
         ArrayConverter arrayConverter = (ArrayConverter) converter;
-        arrayConverter.setConverters(new HashSet<>(commandManager.getConverters()));
+        HashSet<Converter<?>> converters = new HashSet<>();
+        for (Converter c : commandManager.getConverters()) {
+          converters.add(c);
+        }
+        arrayConverter.setConverters(converters);
       }
       add(converter);
     }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
index 917f3e4..ec6f4e3 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java
@@ -14,9 +14,10 @@
  */
 package org.apache.geode.management.internal.cli.help;
 
+import static java.util.stream.Collectors.joining;
+
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -27,6 +28,7 @@ import java.util.Set;
 import java.util.TreeMap;
 
 import org.apache.commons.lang.StringUtils;
+import org.springframework.shell.core.CommandMarker;
 import org.springframework.shell.core.MethodTarget;
 import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
 import org.springframework.shell.core.annotation.CliCommand;
@@ -65,8 +67,7 @@ public class Helper {
 
   private final Map<String, Topic> topics = new HashMap<>();
   private final Map<String, Method> commands = new TreeMap<String, Method>();
-  private final Map<String, MethodTarget> availabilityIndicators =
-      new HashMap<String, MethodTarget>();
+  private final Map<String, MethodTarget> availabilityIndicators = new HashMap<>();
 
   public Helper() {
     initTopic(CliStrings.DEFAULT_TOPIC_GEODE, CliStrings.DEFAULT_TOPIC_GEODE__DESC);
@@ -94,7 +95,25 @@ public class Helper {
     topics.put(topic, new Topic(topic, desc));
   }
 
-  public void addCommand(CliCommand command, Method commandMethod) {
+  public void registerCommand(CommandMarker commandMarker) {
+    for (Method method : commandMarker.getClass().getMethods()) {
+      CliCommand cliCommand = method.getAnnotation(CliCommand.class);
+      CliAvailabilityIndicator availability = method.getAnnotation(CliAvailabilityIndicator.class);
+      if (cliCommand == null && availability == null) {
+        continue;
+      }
+
+      if (cliCommand != null) {
+        addCommand(cliCommand, method);
+      }
+
+      if (availability != null) {
+        addAvailabilityIndicator(availability, new MethodTarget(method, commandMarker));
+      }
+    }
+  }
+
+  protected void addCommand(CliCommand command, Method commandMethod) {
     // put all the command synonyms in the command map
     Arrays.stream(command.value()).forEach(cmd -> {
       commands.put(cmd, commandMethod);
@@ -120,7 +139,8 @@ public class Helper {
     });
   }
 
-  public void addAvailabilityIndicator(CliAvailabilityIndicator availability, MethodTarget target) {
+  private void addAvailabilityIndicator(CliAvailabilityIndicator availability,
+      MethodTarget target) {
     Arrays.stream(availability.value()).forEach(command -> {
       availabilityIndicators.put(command, target);
     });
@@ -152,9 +172,7 @@ public class Helper {
       builder.append(CliStrings.HINT__MSG__TOPICS_AVAILABLE).append(GfshParser.LINE_SEPARATOR)
           .append(GfshParser.LINE_SEPARATOR);
 
-      List<String> sortedTopics = new ArrayList<>(topics.keySet());
-      Collections.sort(sortedTopics);
-      sortedTopics.stream()
+      topics.keySet().stream().sorted()
           .forEachOrdered(topic -> builder.append(topic).append(GfshParser.LINE_SEPARATOR));
       return builder.toString();
     }
@@ -165,8 +183,7 @@ public class Helper {
     }
 
     builder.append(topic.desc).append(GfshParser.LINE_SEPARATOR).append(GfshParser.LINE_SEPARATOR);
-    Collections.sort(topic.relatedCommands);
-    topic.relatedCommands.stream().forEachOrdered(command -> builder.append(command.command)
+    topic.relatedCommands.stream().sorted().forEach(command -> builder.append(command.command)
         .append(": ").append(command.desc).append(GfshParser.LINE_SEPARATOR));
     return builder.toString();
   }
@@ -175,7 +192,7 @@ public class Helper {
     return topics.keySet();
   }
 
-  boolean isAvailable(String command) {
+  private boolean isAvailable(String command) {
     MethodTarget target = availabilityIndicators.get(command);
     if (target == null) {
       return true;
@@ -261,16 +278,12 @@ public class Helper {
     HelpBlock optionNode = new HelpBlock(getPrimaryKey(cliOption));
     String help = cliOption.help();
     optionNode.addChild(new HelpBlock((StringUtils.isNotBlank(help) ? help : "")));
-    if (getSynonyms(cliOption).size() > 0) {
-      StringBuilder builder = new StringBuilder();
-      for (String string : getSynonyms(cliOption)) {
-        if (builder.length() > 0) {
-          builder.append(",");
-        }
-        builder.append(string);
-      }
-      optionNode.addChild(new HelpBlock(SYNONYMS_SUB_NAME + builder.toString()));
+
+    String synonyms = getSynonyms(cliOption).stream().collect(joining(","));
+    if (StringUtils.isNotEmpty(synonyms)) {
+      optionNode.addChild(new HelpBlock(SYNONYMS_SUB_NAME + synonyms));
     }
+
     optionNode.addChild(
         new HelpBlock(REQUIRED_SUB_NAME + ((cliOption.mandatory()) ? TRUE_TOKEN : FALSE_TOKEN)));
     if (!isNullOrBlank(cliOption.specifiedDefaultValue())) {
@@ -359,18 +372,14 @@ public class Helper {
   }
 
   private static List<String> getSynonyms(CliOption option) {
-    List<String> synonyms = new ArrayList<>();
     String[] keys = option.key();
     if (keys.length < 2)
-      return synonyms;
+      return Collections.emptyList();
     // if the primary key is empty (like sh and help command), then there should be no synonyms.
     if ("".equals(keys[0]))
-      return synonyms;
+      return Collections.emptyList();
 
-    for (int i = 1; i < keys.length; i++) {
-      synonyms.add(keys[i]);
-    }
-    return synonyms;
+    return Arrays.asList(keys).subList(1, keys.length);
   }
 
   private static boolean isNullOrBlank(String value) {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
index 6130117..a4c6f59 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/MemberCommandService.java
@@ -35,8 +35,7 @@ public class MemberCommandService extends CommandService {
   public MemberCommandService(InternalCache cache) throws CommandServiceException {
     this.cache = cache;
     try {
-      this.onlineCommandProcessor = new OnlineCommandProcessor(
-          cache.getDistributedSystem().getProperties(), cache.getSecurityService());
+      this.onlineCommandProcessor = new OnlineCommandProcessor(cache.getSecurityService());
     } catch (Exception e) {
       throw new CommandServiceException("Could not load commands.", e);
     }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
index 7fa4acb..38ab364 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java
@@ -18,7 +18,6 @@ import java.io.IOException;
 import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.Map;
-import java.util.Properties;
 
 import org.springframework.shell.core.Parser;
 import org.springframework.shell.event.ParseResult;
@@ -26,7 +25,6 @@ import org.springframework.util.StringUtils;
 
 import org.apache.geode.annotations.TestingOnly;
 import org.apache.geode.internal.security.SecurityService;
-import org.apache.geode.internal.security.SecurityServiceFactory;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.CommandProcessingException;
 import org.apache.geode.management.cli.Result;
@@ -49,20 +47,14 @@ public class OnlineCommandProcessor {
 
   private final SecurityService securityService;
 
-  @TestingOnly
-  public OnlineCommandProcessor() throws ClassNotFoundException, IOException {
-    this(new Properties(), SecurityServiceFactory.create());
-  }
-
-  public OnlineCommandProcessor(Properties cacheProperties, SecurityService securityService)
+  public OnlineCommandProcessor(SecurityService securityService)
       throws ClassNotFoundException, IOException {
-    this(cacheProperties, securityService, new CommandExecutor());
+    this(securityService, new CommandExecutor());
   }
 
   @TestingOnly
-  public OnlineCommandProcessor(Properties cacheProperties, SecurityService securityService,
-      CommandExecutor commandExecutor) {
-    this.gfshParser = new GfshParser(new CommandManager(cacheProperties));
+  public OnlineCommandProcessor(SecurityService securityService, CommandExecutor commandExecutor) {
+    this.gfshParser = new GfshParser(new CommandManager());
     this.executor = commandExecutor;
     this.securityService = securityService;
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
index 046d890..11da4b2 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/util/ClasspathScanLoadHelper.java
@@ -21,6 +21,7 @@ import java.util.HashSet;
 import java.util.Set;
 
 import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
+import io.github.lukehutch.fastclasspathscanner.matchprocessor.ImplementingClassMatchProcessor;
 
 /**
  * Utility class to scan class-path & load classes.
@@ -28,18 +29,26 @@ import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
  * @since GemFire 7.0
  */
 public class ClasspathScanLoadHelper {
-  public static Set<Class<?>> scanPackageForClassesImplementing(String packageToScan,
-      Class<?> implementedInterface) {
-    Set<Class<?>> classesImplementing = new HashSet<>();
-    new FastClasspathScanner(packageToScan)
-        .matchClassesImplementing(implementedInterface, classesImplementing::add).scan();
+
+  public static <T> Set<Class<? extends T>> scanClasspathForClassesImplementing(
+      Class<T> implementedInterface, String... packageSpec) {
+    return scanPackageForClassesImplementing(implementedInterface, packageSpec);
+  }
+
+  public static <T> Set<Class<? extends T>> scanPackageForClassesImplementing(
+      Class<T> implementedInterface, String... packageSpec) {
+    Set<Class<? extends T>> classesImplementing = new HashSet<>();
+    ImplementingClassMatchProcessor<T> matchProcessor = classesImplementing::add;
+
+    new FastClasspathScanner(packageSpec)
+        .matchClassesImplementing(implementedInterface, matchProcessor).scan();
 
     return classesImplementing.stream().filter(ClasspathScanLoadHelper::isInstantiable)
         .collect(toSet());
   }
 
-  private static boolean isInstantiable(Class<?> klass) {
-    int modifiers = klass.getModifiers();
+  private static <T> boolean isInstantiable(Class<T> classToInstantiate) {
+    int modifiers = classToInstantiate.getModifiers();
 
     return !Modifier.isAbstract(modifiers) && !Modifier.isInterface(modifiers)
         && Modifier.isPublic(modifiers);
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
index 7e5f83f..e874898 100644
--- a/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
@@ -99,7 +99,7 @@ public class DistributionConfigJUnitTest {
   @Test
   public void testGetAttributeNames() {
     String[] attNames = AbstractDistributionConfig._getAttNames();
-    assertEquals(attNames.length, 157);
+    assertEquals(156, attNames.length);
 
     List boolList = new ArrayList();
     List intList = new ArrayList();
@@ -135,7 +135,7 @@ public class DistributionConfigJUnitTest {
     // are.
     assertEquals(29, boolList.size());
     assertEquals(33, intList.size());
-    assertEquals(86, stringList.size());
+    assertEquals(85, stringList.size());
     assertEquals(5, fileList.size());
     assertEquals(4, otherList.size());
   }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
index 1dfd61d..e48c8d2 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/ClasspathScanLoadHelperJUnitTest.java
@@ -14,7 +14,8 @@
  */
 package org.apache.geode.management.internal.cli;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.util.Set;
 
@@ -35,36 +36,36 @@ public class ClasspathScanLoadHelperJUnitTest {
 
   private final String PACKAGE_NAME = "org.apache.geode.management.internal.cli.domain";
   private final String WRONG_PACKAGE_NAME = "org.apache.geode.management.internal.cli.domain1";
-  private final Class<?> INTERFACE1 = Interface1.class;
-  private final Class<?> NO_IMPL_INTERFACE = Versionable.class;
-  private final Class<?> INTERFACE2 = Interface2.class;
-  private final Class<?> IMPL1 = Impl1.class;
-  private final Class<?> IMPL2 = Impl12.class;
-  private final Class<?> ABSTRACT_IMPL = AbstractImpl.class;
+  private final Class INTERFACE1 = Interface1.class;
+  private final Class NO_IMPL_INTERFACE = Versionable.class;
+  private final Class INTERFACE2 = Interface2.class;
+  private final Class IMPL1 = Impl1.class;
+  private final Class IMPL2 = Impl12.class;
+  private final Class ABSTRACT_IMPL = AbstractImpl.class;
 
   @Test
   public void testLoadAndGet() throws Exception {
     Set<Class<?>> classLoaded =
-        ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, INTERFACE1);
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(INTERFACE1, PACKAGE_NAME);
     assertEquals(2, classLoaded.size());
     assertTrue(classLoaded.contains(IMPL1));
     assertTrue(classLoaded.contains(IMPL2));
 
     classLoaded =
-        ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, INTERFACE2);
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(INTERFACE2, PACKAGE_NAME);
     assertEquals(1, classLoaded.size());
     assertTrue(classLoaded.contains(IMPL2));
 
     classLoaded =
-        ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME, INTERFACE2);
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(INTERFACE2, WRONG_PACKAGE_NAME);
     assertEquals(0, classLoaded.size());
 
     classLoaded =
-        ClasspathScanLoadHelper.scanPackageForClassesImplementing(PACKAGE_NAME, NO_IMPL_INTERFACE);
+        ClasspathScanLoadHelper.scanPackageForClassesImplementing(NO_IMPL_INTERFACE, PACKAGE_NAME);
     assertEquals(0, classLoaded.size());
 
-    classLoaded = ClasspathScanLoadHelper.scanPackageForClassesImplementing(WRONG_PACKAGE_NAME,
-        NO_IMPL_INTERFACE);
+    classLoaded = ClasspathScanLoadHelper.scanPackageForClassesImplementing(NO_IMPL_INTERFACE,
+        WRONG_PACKAGE_NAME);
     assertEquals(0, classLoaded.size());
   }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
index 8133471..c65ff04 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java
@@ -18,20 +18,12 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
-import java.util.List;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.springframework.shell.core.CommandMarker;
-import org.springframework.shell.core.Completion;
-import org.springframework.shell.core.Converter;
-import org.springframework.shell.core.MethodTarget;
-import org.springframework.shell.core.annotation.CliAvailabilityIndicator;
 import org.springframework.shell.core.annotation.CliCommand;
-import org.springframework.shell.core.annotation.CliOption;
 
-import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.ResourcePermission.Operation;
@@ -43,49 +35,6 @@ import org.apache.geode.test.junit.categories.UnitTest;
  */
 @Category(UnitTest.class)
 public class CommandManagerJUnitTest {
-
-  private static final String COMMAND1_NAME = "command1";
-  private static final String COMMAND1_NAME_ALIAS = "command1_alias";
-  private static final String COMMAND2_NAME = "c2";
-
-  private static final String COMMAND1_HELP = "help for " + COMMAND1_NAME;
-  // ARGUMENTS
-  private static final String ARGUMENT1_NAME = "argument1";
-  private static final String ARGUMENT1_HELP = "help for argument1";
-  private static final String ARGUMENT1_CONTEXT = "context for argument 1";
-  private static final Completion[] ARGUMENT1_COMPLETIONS =
-      {new Completion("arg1"), new Completion("arg1alt")};
-  private static final String ARGUMENT2_NAME = "argument2";
-  private static final String ARGUMENT2_CONTEXT = "context for argument 2";
-  private static final String ARGUMENT2_HELP = "help for argument2";
-  private static final String ARGUMENT2_UNSPECIFIED_DEFAULT_VALUE =
-      "{unspecified default value for argument2}";
-  private static final Completion[] ARGUMENT2_COMPLETIONS =
-      {new Completion("arg2"), new Completion("arg2alt")};
-
-  // OPTIONS
-  private static final String OPTION1_NAME = "option1";
-  private static final String OPTION1_SYNONYM = "opt1";
-  private static final String OPTION1_HELP = "help for option1";
-  private static final String OPTION1_CONTEXT = "context for option1";
-  private static final String OPTION1_SPECIFIED_DEFAULT_VALUE =
-      "{specified default value for option1}";
-  private static final Completion[] OPTION1_COMPLETIONS =
-      {new Completion("option1"), new Completion("option1Alternate")};
-  private static final String OPTION2_NAME = "option2";
-  private static final String OPTION2_HELP = "help for option2";
-  private static final String OPTION2_CONTEXT = "context for option2";
-  private static final String OPTION2_SPECIFIED_DEFAULT_VALUE =
-      "{specified default value for option2}";
-  private static final String OPTION3_NAME = "option3";
-  private static final String OPTION3_SYNONYM = "opt3";
-  private static final String OPTION3_HELP = "help for option3";
-  private static final String OPTION3_CONTEXT = "context for option3";
-  private static final String OPTION3_SPECIFIED_DEFAULT_VALUE =
-      "{specified default value for option3}";
-  private static final String OPTION3_UNSPECIFIED_DEFAULT_VALUE =
-      "{unspecified default value for option3}";
-
   private CommandManager commandManager;
 
   @Before
@@ -111,11 +60,6 @@ public class CommandManagerJUnitTest {
     assertNotNull(commandManager);
   }
 
-  /**
-   * Tests {@link CommandManager#loadPluginCommands()}.
-   *
-   * @since GemFire 8.1
-   */
   @Test
   public void testCommandManagerLoadPluginCommands() throws Exception {
     assertNotNull(commandManager);
@@ -123,97 +67,10 @@ public class CommandManagerJUnitTest {
     // see META-INF/services/org.springframework.shell.core.CommandMarker service loader file.
     assertTrue("Should find listed plugin.",
         commandManager.getHelper().getCommands().contains("mock plugin command"));
-    assertTrue("Should not find unlisted plugin.",
-        !commandManager.getHelper().getCommands().contains("mock plugin command unlisted"));
-  }
-
-  /**
-   * class that represents dummy commands
-   */
-  public static class Commands implements CommandMarker {
-
-    @CliCommand(value = {COMMAND1_NAME, COMMAND1_NAME_ALIAS}, help = COMMAND1_HELP)
-    @CliMetaData(shellOnly = true, relatedTopic = {"relatedTopicOfCommand1"})
-    @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
-    public static String command1(
-        @CliOption(key = ARGUMENT1_NAME, optionContext = ARGUMENT1_CONTEXT, help = ARGUMENT1_HELP,
-            mandatory = true) String argument1,
-        @CliOption(key = ARGUMENT2_NAME, optionContext = ARGUMENT2_CONTEXT, help = ARGUMENT2_HELP,
-            unspecifiedDefaultValue = ARGUMENT2_UNSPECIFIED_DEFAULT_VALUE) String argument2,
-        @CliOption(key = {OPTION1_NAME, OPTION1_SYNONYM}, help = OPTION1_HELP, mandatory = true,
-            optionContext = OPTION1_CONTEXT,
-            specifiedDefaultValue = OPTION1_SPECIFIED_DEFAULT_VALUE) String option1,
-        @CliOption(key = {OPTION2_NAME}, help = OPTION2_HELP, optionContext = OPTION2_CONTEXT,
-            specifiedDefaultValue = OPTION2_SPECIFIED_DEFAULT_VALUE) String option2,
-        @CliOption(key = {OPTION3_NAME, OPTION3_SYNONYM}, help = OPTION3_HELP,
-            optionContext = OPTION3_CONTEXT,
-            unspecifiedDefaultValue = OPTION3_UNSPECIFIED_DEFAULT_VALUE,
-            specifiedDefaultValue = OPTION3_SPECIFIED_DEFAULT_VALUE) String option3) {
-      return null;
-    }
-
-    @CliCommand(value = {COMMAND2_NAME})
-    @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
-    public static String command2() {
-      return null;
-    }
-
-    @CliCommand(value = {"testParamConcat"})
-    @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
-    public static Result testParamConcat(@CliOption(key = {"string"}) String string,
-        @CliOption(key = {"stringArray"}) String[] stringArray,
-        @CliOption(key = {"integer"}) Integer integer,
-        @CliOption(key = {"colonArray"}) String[] colonArray) {
-      return null;
-    }
-
-    @CliCommand(value = {"testMultiWordArg"})
-    @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
-    public static Result testMultiWordArg(@CliOption(key = "arg1") String arg1,
-        @CliOption(key = "arg2") String arg2) {
-      return null;
-    }
-
-    @CliAvailabilityIndicator({COMMAND1_NAME})
-    public boolean isAvailable() {
-      return true; // always available on server
-    }
+    assertThat(
+        commandManager.getCommandMarkers().stream().anyMatch(c -> c instanceof MockPluginCommand));
   }
 
-  /**
-   * Used by testCommandManagerLoadPluginCommands
-   */
-  private static class SimpleConverter implements Converter<String> {
-
-    @Override
-    public boolean supports(Class<?> type, String optionContext) {
-      return type.isAssignableFrom(String.class);
-    }
-
-    @Override
-    public String convertFromText(String value, Class<?> targetType, String optionContext) {
-      return value;
-    }
-
-    @Override
-    public boolean getAllPossibleValues(List<Completion> completions, Class<?> targetType,
-        String existingData, String context, MethodTarget target) {
-      if (context.equals(ARGUMENT1_CONTEXT)) {
-        for (Completion completion : ARGUMENT1_COMPLETIONS) {
-          completions.add(completion);
-        }
-      } else if (context.equals(ARGUMENT2_CONTEXT)) {
-        for (Completion completion : ARGUMENT2_COMPLETIONS) {
-          completions.add(completion);
-        }
-      } else if (context.equals(OPTION1_CONTEXT)) {
-        for (Completion completion : OPTION1_COMPLETIONS) {
-          completions.add(completion);
-        }
-      }
-      return true;
-    }
-  }
 
   public static class MockPluginCommand implements CommandMarker {
     @CliCommand(value = "mock plugin command")
@@ -222,13 +79,4 @@ public class CommandManagerJUnitTest {
       return null;
     }
   }
-
-  public static class MockPluginCommandUnlisted implements CommandMarker {
-    @CliCommand(value = "mock plugin command unlisted")
-    @ResourceOperation(resource = Resource.CLUSTER, operation = Operation.READ)
-    public Result mockPluginCommandUnlisted() {
-      return null;
-    }
-  }
-
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
index 5714829..d799932 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java
@@ -27,9 +27,9 @@ import java.io.IOException;
 import java.io.PrintStream;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -80,7 +80,7 @@ public abstract class CliCommandTestBase extends JUnit4CacheTestCase {
 
   public static boolean checkIfCommandsAreLoadedOrNot() {
     CommandManager manager = new CommandManager();
-    List<CommandMarker> commands = manager.getCommandMarkers();
+    Set<CommandMarker> commands = manager.getCommandMarkers();
     return commands.size() >= 1;
 
   }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
index df00cf1..db7176b 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java
@@ -21,8 +21,6 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import java.util.Properties;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -36,7 +34,6 @@ import org.apache.geode.test.junit.categories.UnitTest;
 @Category(UnitTest.class)
 public class OnlineCommandProcessorTest {
 
-  Properties properties;
   SecurityService securityService;
   CommandExecutor executor;
   OnlineCommandProcessor onlineCommandProcessor;
@@ -44,13 +41,12 @@ public class OnlineCommandProcessorTest {
 
   @Before
   public void before() {
-    properties = new Properties();
     securityService = mock(SecurityService.class);
     executor = mock(CommandExecutor.class);
     result = mock(Result.class);
     when(executor.execute(any())).thenReturn(result);
 
-    onlineCommandProcessor = new OnlineCommandProcessor(properties, securityService, executor);
+    onlineCommandProcessor = new OnlineCommandProcessor(securityService, executor);
   }
 
 
diff --git a/geode-core/src/test/resources/META-INF/services/org.springframework.shell.core.CommandMarker b/geode-core/src/test/resources/META-INF/services/org.springframework.shell.core.CommandMarker
deleted file mode 100644
index 1bb7c6d..0000000
--- a/geode-core/src/test/resources/META-INF/services/org.springframework.shell.core.CommandMarker
+++ /dev/null
@@ -1,8 +0,0 @@
-# Mock command for org.apache.geode.management.internal.cli.CommandManagerJUnitTest.testLoadPluginCommands
-org.apache.geode.management.internal.cli.CommandManagerJUnitTest$MockPluginCommand
-# Should cause ServiceConfigurationException while iterating.
-#TODO jbarrett - causes tests to be marked as failed because of exception in log org.apache.geode.management.internal.cli.CommandManagerJUnitTest$MockPluginCommandNotFound
-
-
-# Mock Extension commands
-org.apache.geode.internal.cache.extension.mock.MockExtensionCommands
\ No newline at end of file
diff --git a/geode-lucene/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker b/geode-lucene/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
deleted file mode 100644
index 0879e15..0000000
--- a/geode-lucene/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
+++ /dev/null
@@ -1,2 +0,0 @@
-# Lucene Extensions command
-org.apache.geode.cache.lucene.internal.cli.LuceneIndexCommands
\ No newline at end of file

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

Mime
View raw message