geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ladyva...@apache.org
Subject [13/25] geode git commit: GEODE-1597: use Spring shell's parser and delete our own parsing code
Date Tue, 02 May 2017 23:29:26 GMT
GEODE-1597: use Spring shell's parser and delete our own parsing code

* Use Spring's SimpleParser as a basis for command parsing
* reworked help/hint
* removing singleton CommandManager


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/1fc0f0ca
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/1fc0f0ca
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/1fc0f0ca

Branch: refs/heads/feature/GEODE-2852
Commit: 1fc0f0ca72705f5ea84ea67f0507bfcad515aacf
Parents: bee0b7d
Author: Jinmei Liao <jiliao@pivotal.io>
Authored: Tue Apr 25 13:51:42 2017 -0700
Committer: Jinmei Liao <jiliao@pivotal.io>
Committed: Thu Apr 27 13:16:24 2017 -0700

----------------------------------------------------------------------
 .../geode/management/cli/CliMetaData.java       |   22 +-
 .../geode/management/cli/ConverterHint.java     |    6 +-
 .../management/internal/cli/CommandManager.java |  514 +-----
 .../internal/cli/CommandResponseBuilder.java    |    5 +-
 .../internal/cli/GfshParseResult.java           |   79 +-
 .../management/internal/cli/GfshParser.java     | 1613 +++---------------
 .../geode/management/internal/cli/Launcher.java |   67 +-
 .../internal/cli/annotation/CliArgument.java    |   81 -
 .../internal/cli/commands/ClientCommands.java   |   25 +-
 .../internal/cli/commands/ConfigCommands.java   |    6 +-
 .../CreateAlterDestroyRegionCommands.java       |   24 +-
 .../internal/cli/commands/DataCommands.java     |   48 +-
 .../internal/cli/commands/DeployCommands.java   |   12 +-
 .../cli/commands/DiskStoreCommands.java         |   43 +-
 .../internal/cli/commands/FunctionCommands.java |    6 +-
 .../internal/cli/commands/GfshHelpCommands.java |   72 +-
 .../cli/commands/LauncherLifecycleCommands.java |   23 +-
 .../cli/commands/MiscellaneousCommands.java     |   16 +-
 .../internal/cli/commands/PDXCommands.java      |   21 +-
 .../internal/cli/commands/QueueCommands.java    |    7 +-
 .../internal/cli/commands/ShellCommands.java    |  332 ++--
 .../internal/cli/commands/WanCommands.java      |   82 +-
 .../cli/converters/BooleanConverter.java        |   54 -
 .../internal/cli/converters/DirConverter.java   |  158 --
 .../internal/cli/converters/EnumConverter.java  |   64 -
 .../internal/cli/converters/HelpConverter.java  |   68 -
 .../cli/converters/HintTopicConverter.java      |   71 -
 .../cli/converters/LogLevelConverter.java       |    2 +-
 .../cli/converters/StringArrayConverter.java    |   53 -
 .../cli/converters/StringListConverter.java     |   56 -
 .../cli/exceptions/CliCommandException.java     |   66 -
 .../exceptions/CliCommandInvalidException.java  |   39 -
 .../CliCommandMultiModeOptionException.java     |   49 -
 .../CliCommandNotAvailableException.java        |   36 -
 .../exceptions/CliCommandOptionException.java   |   65 -
 ...CommandOptionHasMultipleValuesException.java |   47 -
 .../CliCommandOptionInvalidException.java       |   37 -
 .../CliCommandOptionMissingException.java       |   45 -
 .../CliCommandOptionNotApplicableException.java |   46 -
 ...liCommandOptionValueConversionException.java |   38 -
 .../CliCommandOptionValueException.java         |   49 -
 .../CliCommandOptionValueMissingException.java  |   46 -
 .../cli/exceptions/ExceptionGenerator.java      |   48 -
 .../cli/exceptions/ExceptionHandler.java        |   92 -
 .../management/internal/cli/help/CliTopic.java  |  132 --
 .../management/internal/cli/help/HelpBlock.java |   86 +
 .../management/internal/cli/help/Helper.java    |  345 ++++
 .../management/internal/cli/help/Topic.java     |   58 +
 .../internal/cli/help/format/Block.java         |   42 -
 .../internal/cli/help/format/DataNode.java      |   48 -
 .../internal/cli/help/format/Help.java          |   44 -
 .../internal/cli/help/format/NewHelp.java       |   52 -
 .../internal/cli/help/format/Row.java           |   28 -
 .../internal/cli/help/utils/FormatOutput.java   |   33 -
 .../internal/cli/help/utils/HelpUtils.java      |  401 -----
 .../internal/cli/i18n/CliStrings.java           |    7 +-
 .../cli/multistep/CLIMultiStepHelper.java       |   11 +-
 .../internal/cli/parser/Argument.java           |   71 -
 .../internal/cli/parser/AvailabilityTarget.java |  106 --
 .../internal/cli/parser/CommandTarget.java      |  176 --
 .../internal/cli/parser/GfshMethodTarget.java   |  121 --
 .../internal/cli/parser/GfshOptionParser.java   |   37 -
 .../internal/cli/parser/MethodParameter.java    |   39 -
 .../management/internal/cli/parser/Option.java  |  217 ---
 .../internal/cli/parser/OptionSet.java          |  128 --
 .../internal/cli/parser/Parameter.java          |  116 --
 .../internal/cli/parser/ParserUtils.java        |  186 --
 .../internal/cli/parser/SyntaxConstants.java    |   34 -
 .../cli/parser/jopt/JoptOptionParser.java       |  302 ----
 .../preprocessor/EnclosingCharacters.java       |   32 -
 .../cli/parser/preprocessor/Preprocessor.java   |  151 --
 .../parser/preprocessor/PreprocessorUtils.java  |  327 ----
 .../internal/cli/parser/preprocessor/Stack.java |   52 -
 .../cli/parser/preprocessor/TrimmedInput.java   |   44 -
 .../internal/cli/remote/CommandProcessor.java   |   23 +-
 .../management/internal/cli/shell/Gfsh.java     |  516 +++---
 .../cli/shell/GfshExecutionStrategy.java        |    5 +-
 .../internal/cli/shell/MultiCommandHelper.java  |   10 +-
 .../internal/cli/shell/jline/GfshHistory.java   |   21 +-
 .../internal/cli/util/CommandStringBuilder.java |   32 +-
 .../web/controllers/DataCommandsController.java |    6 +-
 .../web/controllers/ExportLogController.java    |    2 +-
 .../internal/cli/CommandManagerJUnitTest.java   |  163 +-
 .../internal/cli/GfshParserIntegrationTest.java |  421 ++++-
 .../internal/cli/GfshParserJUnitTest.java       |  881 +---------
 .../internal/cli/JoptOptionParserTest.java      |  527 ------
 .../cli/annotations/CliArgumentJUnitTest.java   |  154 --
 .../cli/commands/CliCommandTestBase.java        |   85 +-
 ...eateAlterDestroyRegionCommandsDUnitTest.java |    6 +-
 .../cli/commands/DeployCommandsDUnitTest.java   |   13 +-
 .../commands/DiskStoreCommandsDUnitTest.java    |    4 +-
 .../commands/GemfireDataCommandsDUnitTest.java  |   55 +-
 .../commands/HelpCommandsIntegrationTest.java   |  141 --
 .../cli/commands/QueueCommandsDUnitTest.java    |    4 +-
 .../internal/cli/help/HelpBlockUnitTest.java    |   76 +
 .../internal/cli/help/HelperUnitTest.java       |  171 ++
 .../cli/parser/ParserUtilsJUnitTest.java        |   81 -
 .../preprocessor/PreprocessorJUnitTest.java     |  296 ----
 .../PreprocessorUtilsJUnitTest.java             |  121 --
 .../shell/GfshExecutionStrategyJUnitTest.java   |  129 +-
 .../cli/shell/GfshHistoryJUnitTest.java         |   27 +-
 .../internal/cli/shell/GfshJunitTest.java       |   45 +
 .../ClusterConfigDistributionDUnitTest.java     |    3 +-
 .../internal/security/TestCommand.java          |   10 +-
 .../internal/cli/LuceneIndexCommands.java       |   18 +-
 .../lucene/LuceneCommandsSecurityDUnitTest.java |   24 +-
 .../cli/LuceneIndexCommandsDUnitTest.java       |   40 +-
 .../LuceneClusterConfigurationDUnitTest.java    |    6 -
 108 files changed, 2354 insertions(+), 9521 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/1fc0f0ca/geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java b/geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java
index e69d78a..2e6dc39 100644
--- a/geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java
+++ b/geode-core/src/main/java/org/apache/geode/management/cli/CliMetaData.java
@@ -68,29 +68,19 @@ public @interface CliMetaData {
 
   /**
    * String used as a separator when multiple values for a command are specified
+   * 
+   * @deprecated since 1.2, Command methods may override both the delimiter and the escape
through
+   *             spring shell's {@code splittingRegex} option context
    */
   String valueSeparator() default org.apache.geode.management.cli.CliMetaData.ANNOTATION_NULL_VALUE;
 
-
-  // TODO - Abhishek - refactor to group this
-  // /**
-  // *
-  // * @since GemFire 8.0
-  // */
-  // @Retention(RetentionPolicy.RUNTIME)
-  // @Target({ ElementType.PARAMETER })
-  // public @interface ParameterMetadata {
-  // /**
-  // * String used as a separator when multiple values for a command are specified
-  // */
-  // String valueSeparator() default CliMetaData.ANNOTATION_NULL_VALUE;
-  // }
-
   /**
    * An annotation to define additional meta-data for availability of commands.
-   * 
+   *
    *
    * @since GemFire 8.0
+   *
+   * @deprecated since Geode1.2, not used at all
    */
   @Retention(RetentionPolicy.RUNTIME)
   @Target({ElementType.METHOD})

http://git-wip-us.apache.org/repos/asf/geode/blob/1fc0f0ca/geode-core/src/main/java/org/apache/geode/management/cli/ConverterHint.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/cli/ConverterHint.java b/geode-core/src/main/java/org/apache/geode/management/cli/ConverterHint.java
index f45abc4..a4b30be 100644
--- a/geode-core/src/main/java/org/apache/geode/management/cli/ConverterHint.java
+++ b/geode-core/src/main/java/org/apache/geode/management/cli/ConverterHint.java
@@ -23,7 +23,6 @@ import org.springframework.shell.core.annotation.CliOption;
  * @since GemFire 8.0
  */
 public interface ConverterHint {
-  public static final String DIRS = "converter.hint.dirs";
   public static final String DIR_PATHSTRING = "converter.hint.dir.path.string";
   public static final String DISKSTORE_ALL = "converter.hint.cluster.diskstore";
   public static final String FILE = "converter.hint.file";
@@ -40,10 +39,7 @@ public interface ConverterHint {
   public static final String LOCATOR_DISCOVERY_CONFIG = "converter.hint.locators.discovery.config";
   public static final String REGIONPATH = "converter.hint.region.path";
   public static final String INDEX_TYPE = "converter.hint.index.type";
-  public static final String STRING_LIST = "converter.hint.list.string";
   public static final String GATEWAY_SENDER_ID = "converter.hint.gateway.senderid";
   public static final String GATEWAY_RECEIVER_ID = "converter.hint.gateway.receiverid";
-  public static final String LOG_LEVEL = "converter.hint.log.levels";
-
-  public static final String STRING_DISABLER = "converter.hint.disable-string-converter";
+  public static final String LOG_LEVEL = "converter.hint.log.levels:disable-string-converter";
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/1fc0f0ca/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java
----------------------------------------------------------------------
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 4400445..24b07fa 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,68 +14,81 @@
  */
 package org.apache.geode.management.internal.cli;
 
+import static org.apache.geode.distributed.ConfigurationProperties.USER_COMMAND_PACKAGES;
+
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.ClassPathLoader;
-import org.apache.geode.management.cli.CliMetaData;
-import org.apache.geode.management.internal.cli.annotation.CliArgument;
-import org.apache.geode.management.internal.cli.help.CliTopic;
-import org.apache.geode.management.internal.cli.parser.*;
-import org.apache.geode.management.internal.cli.parser.jopt.JoptOptionParser;
+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;
 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.springframework.shell.core.annotation.CliOption;
 
 import java.io.IOException;
-import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
-import java.util.*;
-import java.util.Map.Entry;
-
-import static org.apache.geode.distributed.ConfigurationProperties.*;
+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 java.util.Set;
+import java.util.StringTokenizer;
 
 /**
+ *
+ * this only takes care of loading all available command markers and converters from the
application
  * 
  * @since GemFire 7.0
  */
 public class CommandManager {
-  // 1. Load Commands, availability indicators - Take from GfshParser
-  // 2. Load Converters - Take from GfshParser
-  // 3. Load Result Converters - Add
-
-  private static final Object INSTANCE_LOCK = new Object();
-  private static CommandManager INSTANCE = null;
   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 Properties cacheProperties;
+  private final Helper helper = new Helper();
+
+  private final List<Converter<?>> converters = new ArrayList<Converter<?>>();
+  private final List<CommandMarker> commandMarkers = new ArrayList<>();
 
+  private Properties cacheProperties;
   private LogWrapper logWrapper;
 
-  private CommandManager(final boolean loadDefaultCommands, final Properties cacheProperties)
-      throws ClassNotFoundException, IOException {
+  /**
+   * this constructor is used from Gfsh VM. We are getting the user-command-package from
system
+   * environment. used by Gfsh.
+   */
+  public CommandManager() {
+    this(null);
+  }
+
+  /**
+   * this is used when getting the instance in a cache server. We are getting the
+   * user-command-package from distribution properties. used by CommandProcessor.
+   */
+  public CommandManager(final Properties cacheProperties) {
     if (cacheProperties != null) {
       this.cacheProperties = cacheProperties;
     }
-
     logWrapper = LogWrapper.getInstance();
-    if (loadDefaultCommands) {
-      loadCommands();
-
-      if (logWrapper.fineEnabled()) {
-        logWrapper.fine("Commands Loaded: " + commands.keySet());
-        logWrapper
-            .fine("Command Availability Indicators Loaded: " + availabilityIndicators.keySet());
-        logWrapper.fine("Converters Loaded: " + converters);
-      }
+    loadCommands();
+  }
+
+  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.");
     }
   }
 
-  private void loadUserCommands() throws ClassNotFoundException, IOException {
+  private void loadUserCommands() {
     final Set<String> userCommandPackages = new HashSet<String>();
 
     // Find by packages specified by the system property
@@ -122,12 +135,8 @@ public class CommandManager {
           }
         }
         raiseExceptionIfEmpty(foundClasses, "User Command");
-      } catch (ClassNotFoundException e) {
+      } catch (ClassNotFoundException | IOException e) {
         logWrapper.warning("Could not load User Commands due to " + e.getLocalizedMessage());
-        throw e;
-      } catch (IOException e) {
-        logWrapper.warning("Could not load User Commands due to " + e.getLocalizedMessage());
-        throw e;
       } catch (IllegalStateException e) {
         logWrapper.warning(e.getMessage(), e);
         throw e;
@@ -137,7 +146,7 @@ public class CommandManager {
 
   /**
    * Loads commands via {@link ServiceLoader} from {@link ClassPathLoader}.
-   * 
+   *
    * @since GemFire 8.1
    */
   private void loadPluginCommands() {
@@ -158,7 +167,7 @@ public class CommandManager {
     }
   }
 
-  private void loadCommands() throws ClassNotFoundException, IOException {
+  private void loadCommands() {
     loadUserCommands();
 
     loadPluginCommands();
@@ -166,6 +175,7 @@ public class CommandManager {
     // CommandMarkers
     Set<Class<?>> foundClasses = null;
     try {
+      // geode's commands
       foundClasses = ClasspathScanLoadHelper.loadAndGet(
           "org.apache.geode.management.internal.cli.commands", CommandMarker.class, true);
       for (Class<?> klass : foundClasses) {
@@ -177,12 +187,13 @@ public class CommandManager {
         }
       }
       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 (ClassNotFoundException e) {
       logWrapper.warning("Could not load Commands due to " + e.getLocalizedMessage());
-      throw e;
     } catch (IOException e) {
       logWrapper.warning("Could not load Commands due to " + e.getLocalizedMessage());
-      throw e;
     } catch (IllegalStateException e) {
       logWrapper.warning(e.getMessage(), e);
       throw e;
@@ -201,26 +212,13 @@ public class CommandManager {
         }
       }
       raiseExceptionIfEmpty(foundClasses, "Converters");
-    } catch (ClassNotFoundException e) {
-      logWrapper.warning("Could not load Converters due to " + e.getLocalizedMessage());
-      throw e;
-    } catch (IOException e) {
-      logWrapper.warning("Could not load Converters due to " + e.getLocalizedMessage());
-      throw e;
-    } catch (IllegalStateException e) {
-      logWrapper.warning(e.getMessage(), e);
-      throw e;
-    }
 
-    // Roo's Converters
-    try {
+      // Spring shell's converters
       foundClasses = ClasspathScanLoadHelper.loadAndGet("org.springframework.shell.converters",
           Converter.class, true);
       for (Class<?> klass : foundClasses) {
         try {
-          if (!SHL_CONVERTERS_TOSKIP.contains(klass.getName())) {
-            add((Converter<?>) klass.newInstance());
-          }
+          add((Converter<?>) klass.newInstance());
         } catch (Exception e) {
           logWrapper.warning(
               "Could not load Converter from: " + klass + " due to " + e.getLocalizedMessage());
// continue
@@ -228,417 +226,71 @@ public class CommandManager {
       }
       raiseExceptionIfEmpty(foundClasses, "Basic Converters");
     } catch (ClassNotFoundException e) {
-      logWrapper.warning("Could not load Default Converters due to " + e.getLocalizedMessage());//
TODO
-                                                                                        
       // -
-                                                                                        
       // Abhishek:
-                                                                                        
       // Should
-                                                                                        
       // these
-                                                                                        
       // converters
-                                                                                        
       // be
-                                                                                        
       // moved
-                                                                                        
       // in
-                                                                                        
       // GemFire?
-      throw e;
+      logWrapper.warning("Could not load Converters due to " + e.getLocalizedMessage());
     } catch (IOException e) {
-      logWrapper.warning("Could not load Default Converters due to " + e.getLocalizedMessage());//
TODO
-                                                                                        
       // -
-                                                                                        
       // Abhishek:
-                                                                                        
       // Should
-                                                                                        
       // these
-                                                                                        
       // converters
-                                                                                        
       // be
-                                                                                        
       // moved
-                                                                                        
       // in
-                                                                                        
       // GemFire?
-      throw e;
+      logWrapper.warning("Could not load Converters due to " + e.getLocalizedMessage());
     } catch (IllegalStateException e) {
       logWrapper.warning(e.getMessage(), e);
       throw e;
     }
   }
 
-  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.");
-    }
-  }
-
-  public static CommandManager getInstance() throws ClassNotFoundException, IOException {
-    return getInstance(true);
-  }
-
-  public static CommandManager getInstance(Properties cacheProperties)
-      throws ClassNotFoundException, IOException {
-    return getInstance(true, cacheProperties);
-  }
-
-  // For testing.
-  public static void clearInstance() {
-    synchronized (INSTANCE_LOCK) {
-      INSTANCE = null;
-    }
-  }
-
-  // This method exists for test code use only ...
-  /* package */static CommandManager getInstance(boolean loadDefaultCommands)
-      throws ClassNotFoundException, IOException {
-    return getInstance(loadDefaultCommands, null);
-  }
-
-  private static CommandManager getInstance(boolean loadDefaultCommands, Properties cacheProperties)
-      throws ClassNotFoundException, IOException {
-    synchronized (INSTANCE_LOCK) {
-      if (INSTANCE == null) {
-        INSTANCE = new CommandManager(loadDefaultCommands, cacheProperties);
-      }
-      return INSTANCE;
-    }
+  public List<Converter<?>> getConverters() {
+    return converters;
   }
 
-  public static CommandManager getExisting() {
-    // if (INSTANCE == null) {
-    // throw new IllegalStateException("CommandManager doesn't exist.");
-    // }
-    return INSTANCE;
+  public List<CommandMarker> getCommandMarkers() {
+    return commandMarkers;
   }
 
-  /** Skip some of the Converters from Spring Shell for our customization */
-  private static List<String> SHL_CONVERTERS_TOSKIP = new ArrayList<String>();
-  static {
-    // Over-ridden by cggm.internal.cli.converters.BooleanConverter
-    SHL_CONVERTERS_TOSKIP.add("org.springframework.shell.converters.BooleanConverter");
-    // Over-ridden by cggm.internal.cli.converters.EnumConverter
-    SHL_CONVERTERS_TOSKIP.add("org.springframework.shell.converters.EnumConverter");
-  }
-
-  /**
-   * List of converters which should be populated first before any command can be added
-   */
-  private final List<Converter<?>> converters = new ArrayList<Converter<?>>();
-
-  /**
-   * Map of command string and actual CommandTarget object
-   * 
-   * This map can also be implemented as a trie to support command abbreviation
-   */
-  private final Map<String, CommandTarget> commands = new TreeMap<String, CommandTarget>();
-
-  /**
-   * This method will store the all the availabilityIndicators
-   */
-  private final Map<String, AvailabilityTarget> availabilityIndicators =
-      new HashMap<String, AvailabilityTarget>();
-
-  /**
-   */
-  private final Map<String, CliTopic> topics = new TreeMap<String, CliTopic>();
-
   /**
    * Method to add new Converter
-   * 
+   *
    * @param converter
    */
-  public void add(Converter<?> converter) {
+  void add(Converter<?> converter) {
     converters.add(converter);
   }
 
   /**
    * Method to add new Commands to the parser
-   * 
+   *
    * @param commandMarker
    */
-  public void add(CommandMarker commandMarker) {
-    // First we need to find out all the methods marked with
-    // Command annotation
-    Method[] methods = commandMarker.getClass().getMethods();
-    for (Method method : methods) {
-      if (method.getAnnotation(CliCommand.class) != null) {
-
-        //
-        // First Build the option parser
-        //
-
-        // Create the empty LinkedLists for storing the argument and
-        // options
-        LinkedList<Argument> arguments = new LinkedList<Argument>();
-        LinkedList<Option> options = new LinkedList<Option>();
-        // Also we need to create the OptionParser for each command
-        GfshOptionParser optionParser = getOptionParser();
-        // Now get all the parameters annotations of the method
-        Annotation[][] parameterAnnotations = method.getParameterAnnotations();
-        // Also get the parameter Types
-        Class<?>[] parameterTypes = method.getParameterTypes();
-
-        int parameterNo = 0;
-
-        for (int i = 0; i < parameterAnnotations.length; i++) {
-          // Get all the annotations for this specific parameter
-          Annotation[] annotations = parameterAnnotations[i];
-          // Also get the parameter type for this parameter
-          Class<?> parameterType = parameterTypes[i];
-
-          boolean paramFound = false;
-          String valueSeparator = CliMetaData.ANNOTATION_NULL_VALUE;
-          for (Annotation annotation : annotations) {
-            if (annotation instanceof CliArgument) {
-              // Here we need to create the argument Object
-              Argument argumentToAdd =
-                  createArgument((CliArgument) annotation, parameterType, parameterNo);
-              arguments.add(argumentToAdd);
-              parameterNo++;
-            } else if (annotation instanceof CliOption) {
-              Option createdOption =
-                  createOption((CliOption) annotation, parameterType, parameterNo);
-              if (!CliMetaData.ANNOTATION_NULL_VALUE.equals(valueSeparator)) { // CliMetaData
was
-                                                                               // found earlier
-                createdOption.setValueSeparator(valueSeparator);
-
-                // reset valueSeparator back to null
-                valueSeparator = CliMetaData.ANNOTATION_NULL_VALUE;
-              } else { // CliMetaData is yet to be found
-                paramFound = true;
-              }
-              options.add(createdOption);
-              parameterNo++;
-            } else if (annotation instanceof CliMetaData) {
-              valueSeparator = ((CliMetaData) annotation).valueSeparator();
-              if (!CliMetaData.ANNOTATION_NULL_VALUE.equals(valueSeparator)) {
-                if (paramFound) { // CliOption was detected earlier
-                  Option lastAddedOption = options.getLast();
-                  lastAddedOption.setValueSeparator(valueSeparator);
-                  // reset valueSeparator back to null
-                  valueSeparator = CliMetaData.ANNOTATION_NULL_VALUE;
-                } // param not found yet, store valueSeparator value
-              } else {
-                // reset valueSeparator back to null
-                valueSeparator = CliMetaData.ANNOTATION_NULL_VALUE;
-              }
-            }
-          }
-        }
-        optionParser.setArguments(arguments);
-        optionParser.setOptions(options);
-
-        //
-        // Now build the commandTarget
-        //
-
-        // First build the MethodTarget for the command Method
-        GfshMethodTarget gfshMethodTarget = new GfshMethodTarget(method, commandMarker);
-
-        // Fetch the value array from the cliCommand annotation
-        CliCommand cliCommand = method.getAnnotation(CliCommand.class);
-        String[] values = cliCommand.value();
-
-        // First string will point to the command
-        // rest of them will act as synonyms
-        String commandName = null;
-        String[] synonyms = null;
-        if (values.length > 1) {
-          synonyms = new String[values.length - 1];
-        }
-
-        commandName = values[0];
-
-        for (int j = 1; j < values.length; j++) {
-          synonyms[j - 1] = values[j];
-        }
-
-        // Create the commandTarget object
-        CommandTarget commandTarget = new CommandTarget(commandName, synonyms, gfshMethodTarget,
-            optionParser, null, cliCommand.help());
-
-        // Now for each string in values put an entry in the commands
-        // map
-        for (String string : values) {
-          if (commands.get(string) == null) {
-            commands.put(string, commandTarget);
-          } else {
-            // TODO Handle collision
-            logWrapper.info("Multiple commands configured with the same name: " + string);
-          }
-        }
-
-        if (CliUtil.isGfshVM()) {
-          CliMetaData commandMetaData = method.getAnnotation(CliMetaData.class);
-          if (commandMetaData != null) {
-            String[] relatedTopics = commandMetaData.relatedTopic();
-            // System.out.println("relatedTopic :: "+Arrays.toString(relatedTopics));
-            for (String topicName : relatedTopics) {
-              CliTopic topic = topics.get(topicName);
-              if (topic == null) {
-                topic = new CliTopic(topicName);
-                topics.put(topicName, topic);
-              }
-              topic.addCommandTarget(commandTarget);
-            }
-          }
-        }
-
-      } else if (method.getAnnotation(CliAvailabilityIndicator.class) != null) {
-        // Now add this availability Indicator to the list of
-        // availability Indicators
-        CliAvailabilityIndicator cliAvailabilityIndicator =
-            method.getAnnotation(CliAvailabilityIndicator.class);
-
-        // Create a AvailabilityTarget for this availability Indicator
-        AvailabilityTarget availabilityIndicator = new AvailabilityTarget(commandMarker,
method);
-
-        String[] value = cliAvailabilityIndicator.value();
-        for (String string : value) {
-          availabilityIndicators.put(string, availabilityIndicator);
-        }
-
+  void add(CommandMarker commandMarker) {
+    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;
       }
-    }
-    // Now we must update all the existing CommandTargets to add
-    // this availability Indicator if it applies to them
-    updateAvailabilityIndicators();
-  }
-
 
-  /**
-   * Will update all the references to availability Indicators for commands
-   * 
-   */
-  public void updateAvailabilityIndicators() {
-    for (String string : availabilityIndicators.keySet()) {
-      CommandTarget commandTarget = commands.get(string);
-      if (commandTarget != null) {
-        commandTarget.setAvailabilityIndicator(availabilityIndicators.get(string));
+      if (cliCommand != null) {
+        helper.addCommand(cliCommand, method);
       }
-    }
-  }
 
-  /**
-   * Creates a new {@link Option} instance
-   * 
-   * @param cliOption
-   * @param parameterType
-   * @param parameterNo
-   * @return Option
-   */
-  public Option createOption(CliOption cliOption, Class<?> parameterType, int parameterNo)
{
-    Option option = new Option();
-
-    // First set the Option identifiers
-    List<String> synonyms = new ArrayList<String>();
-    for (String string : cliOption.key()) {
-      if (!option.setLongOption(string)) {
-        synonyms.add(string);
+      if (availability != null) {
+        helper.addAvailabilityIndicator(availability, new MethodTarget(method, commandMarker));
       }
     }
-    option.setSynonyms(synonyms);
-    if (!(option.getAggregate().size() > 0)) {
-      logWrapper.warning("Option should have a name");
-    }
-    // Set the option Help
-    option.setHelp(cliOption.help());
-
-    // Set whether the option is required or not
-    option.setRequired(cliOption.mandatory());
-
-    // Set the fields related to option value
-    option.setSystemProvided(cliOption.systemProvided());
-    option.setSpecifiedDefaultValue(cliOption.specifiedDefaultValue());
-    option.setUnspecifiedDefaultValue(cliOption.unspecifiedDefaultValue());
-
-    // Set the things which are useful for value conversion and
-    // auto-completion
-    option.setContext(cliOption.optionContext());
-    // Find the matching Converter<?> for this option
-    option.setConverter(getConverter(parameterType, option.getContext()));
-
-    option.setDataType(parameterType);
-    option.setParameterNo(parameterNo);
-    return option;
   }
 
-  /**
-   * Creates a new {@link Argument} instance
-   * 
-   * @param cliArgument
-   * @param parameterType
-   * @param parameterNo
-   * @return Argument
-   */
-  public Argument createArgument(CliArgument cliArgument, Class<?> parameterType, int
parameterNo) {
-    Argument argument = new Argument();
-    argument.setArgumentName(cliArgument.name());
-    argument.setContext(cliArgument.argumentContext());
-    argument.setConverter(getConverter(parameterType, argument.getContext()));
-    argument.setHelp(cliArgument.help());
-    argument.setRequired(cliArgument.mandatory());
-    argument.setDataType(parameterType);
-    argument.setParameterNo(parameterNo);
-    argument.setUnspecifiedDefaultValue(cliArgument.unspecifiedDefaultValue());
-    argument.setSystemProvided(cliArgument.systemProvided());
-    return argument;
+  public Helper getHelper() {
+    return helper;
   }
 
-  /**
-   * Looks for a matching {@link Converter}
-   * 
-   * @param parameterType
-   * @param context
-   * @return {@link Converter}
-   */
-  public Converter<?> getConverter(Class<?> parameterType, String context) {
-    for (Converter<?> converter : converters) {
-      if (converter.supports(parameterType, context)) {
-        return converter;
-      }
+  public String obtainHelp(String buffer) {
+    int terminalWidth = -1;
+    Gfsh gfsh = Gfsh.getCurrentInstance();
+    if (gfsh != null) {
+      terminalWidth = gfsh.getTerminalWidth();
     }
-    return null;
+    return helper.getHelp(buffer, terminalWidth);
   }
 
-  /**
-   * For the time being this method returns a {@link JoptOptionParser} object but in the
future we
-   * can change which optionParser should be returned.
-   * 
-   * @return {@link GfshOptionParser}
-   */
-  private GfshOptionParser getOptionParser() {
-    return new JoptOptionParser();
+  public String obtainHint(String topic) {
+    return helper.getHint(topic);
   }
 
-  /**
-   * @return the commands
-   */
-  public Map<String, CommandTarget> getCommands() {
-    return Collections.unmodifiableMap(commands);
-  }
-
-  AvailabilityTarget getAvailabilityIndicator(Object key) {
-    return availabilityIndicators.get(key);
-  }
-
-  public Set<String> getTopicNames() {
-    Set<String> topicsNames = topics.keySet();
-    return Collections.unmodifiableSet(topicsNames);
-  }
-
-  public List<CliTopic> getTopics() {
-    List<CliTopic> topicsList = new ArrayList<CliTopic>(topics.values());
-    return Collections.unmodifiableList(topicsList);
-  }
-
-  public CliTopic getTopic(String topicName) {
-    CliTopic foundTopic = topics.get(topicName);
-
-    if (foundTopic == null) {
-      Set<Entry<String, CliTopic>> entries = topics.entrySet();
-
-      for (Entry<String, CliTopic> entry : entries) {
-        if (entry.getKey().equalsIgnoreCase(topicName)) {
-          foundTopic = entry.getValue();
-          break;
-        }
-      }
-    }
-
-    return foundTopic;
-  }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/1fc0f0ca/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java
index bda030d..3f8f20d 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandResponseBuilder.java
@@ -14,6 +14,7 @@
  */
 package org.apache.geode.management.internal.cli;
 
+import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.internal.cli.json.GfJsonException;
 import org.apache.geode.management.internal.cli.json.GfJsonObject;
 import org.apache.geode.management.internal.cli.remote.CommandExecutionContext;
@@ -24,8 +25,6 @@ import org.apache.geode.management.internal.cli.result.CommandResult;
  * @since GemFire 7.0
  */
 public class CommandResponseBuilder {
-  // Command Response Constants
-  private static final String NO_TOKEN_ACCESSOR = "__NULL__";
 
   public static CommandResponse prepareCommandResponse(String memberName, CommandResult result)
{
     GfJsonObject content = null;
@@ -42,7 +41,7 @@ public class CommandResponseBuilder {
         getType(result), // contentType
         result.getStatus().getCode(), // status code
         "1/1", // page --- TODO - Abhishek - define a scrollable ResultData
-        NO_TOKEN_ACCESSOR, // tokenAccessor for next results
+        CliMetaData.ANNOTATION_NULL_VALUE, // tokenAccessor for next results
         getDebugInfo(result), // debugData
         result.getHeader(), // header
         content, // content

http://git-wip-us.apache.org/repos/asf/geode/blob/1fc0f0ca/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
index d879e2d..e91e365 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java
@@ -14,17 +14,20 @@
  */
 package org.apache.geode.management.internal.cli;
 
+import org.apache.commons.lang.StringUtils;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.internal.cli.shell.GfshExecutionStrategy;
+import org.apache.geode.management.internal.cli.shell.OperationInvoker;
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+import org.springframework.shell.event.ParseResult;
+
+import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
-import org.springframework.shell.event.ParseResult;
-
-import org.apache.geode.management.cli.CliMetaData;
-import org.apache.geode.management.internal.cli.shell.GfshExecutionStrategy;
-import org.apache.geode.management.internal.cli.shell.OperationInvoker;
-
 /**
  * Immutable representation of the outcome of parsing a given shell line. * Extends
  * {@link ParseResult} to add a field to specify the command string that was input by the
user.
@@ -41,7 +44,7 @@ import org.apache.geode.management.internal.cli.shell.OperationInvoker;
 public class GfshParseResult extends ParseResult {
   private String userInput;
   private String commandName;
-  private Map<String, String> paramValueStringMap;
+  private Map<String, String> paramValueStringMap = new HashMap<>();
 
   /**
    * Creates a GfshParseResult instance to represent parsing outcome.
@@ -52,12 +55,40 @@ public class GfshParseResult extends ParseResult {
    * @param userInput user specified commands string
    */
   protected GfshParseResult(final Method method, final Object instance, final Object[] arguments,
-      final String userInput, final String commandName,
-      final Map<String, String> parametersAsString) {
+      final String userInput) {
     super(method, instance, arguments);
-    this.userInput = userInput;
-    this.commandName = commandName;
-    this.paramValueStringMap = new HashMap<String, String>(parametersAsString);
+    this.userInput = userInput.trim();
+
+    CliCommand cliCommand = method.getAnnotation(CliCommand.class);
+    commandName = cliCommand.value()[0];
+
+    Annotation[][] parameterAnnotations = method.getParameterAnnotations();
+    if (arguments == null) {
+      return;
+    }
+
+    for (int i = 0; i < arguments.length; i++) {
+      Object argument = arguments[i];
+      if (argument == null) {
+        continue;
+      }
+
+      CliOption cliOption = getCliOption(parameterAnnotations, i);
+
+      String argumentAsString;
+      if (argument instanceof Object[]) {
+        argumentAsString = StringUtils.join((Object[]) argument, ",");
+      } else {
+        argumentAsString = argument.toString();
+      }
+      // need to quote the argument with single quote if it contains white space.
+      // these will be used for the http request parameters, when turned into the
+      // commands again, the options will be quoted.
+      if (argumentAsString.contains(" ")) {
+        argumentAsString = "'" + argumentAsString + "'";
+      }
+      paramValueStringMap.put(cliOption.key()[0], argumentAsString);
+    }
   }
 
   /**
@@ -67,6 +98,9 @@ public class GfshParseResult extends ParseResult {
     return userInput;
   }
 
+  public String getParamValue(String param) {
+    return paramValueStringMap.get(param);
+  }
 
   /**
    * @return the unmodifiable paramValueStringMap
@@ -75,18 +109,17 @@ public class GfshParseResult extends ParseResult {
     return Collections.unmodifiableMap(paramValueStringMap);
   }
 
-  @Override
-  public String toString() {
-    StringBuilder builder = new StringBuilder();
-    builder.append(GfshParseResult.class.getSimpleName());
-    builder.append(" [method=").append(getMethod());
-    builder.append(", instance=").append(getInstance());
-    builder.append(", arguments=").append(CliUtil.arrayToString(getArguments()));
-    builder.append("]");
-    return builder.toString();
-  }
-
   public String getCommandName() {
     return commandName;
   }
+
+  private CliOption getCliOption(Annotation[][] parameterAnnotations, int index) {
+    Annotation[] annotations = parameterAnnotations[index];
+    for (Annotation annotation : annotations) {
+      if (annotation instanceof CliOption) {
+        return (CliOption) annotation;
+      }
+    }
+    return null;
+  }
 }


Mime
View raw message