Repository: geode
Updated Branches:
  refs/heads/develop fd47ed660 -> cf9142669


GEODE-3261: Refactoring GfshHelpCommands

This closes #685


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

Branch: refs/heads/develop
Commit: cf91426692349d0c81ce77394935576d9cc336e8
Parents: fd47ed6
Author: YehEmily <emilyyeh1...@gmail.com>
Authored: Fri Aug 4 11:12:50 2017 -0700
Committer: Kirk Lund <kl...@apache.org>
Committed: Fri Aug 11 14:43:53 2017 -0700

----------------------------------------------------------------------
 .../internal/cli/commands/GfshHelpCommand.java  | 51 ++++++++++++++
 .../internal/cli/commands/GfshHelpCommands.java | 53 --------------
 .../internal/cli/commands/GfshHintCommand.java  | 42 +++++++++++
 .../internal/cli/converters/HelpConverter.java  | 25 +++----
 .../cli/help/HelperIntegrationTest.java         | 73 +++++++++++++++++---
 .../internal/security/TestCommand.java          |  2 +-
 6 files changed, 166 insertions(+), 80 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/cf914266/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommand.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommand.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommand.java
new file mode 100644
index 0000000..4cb6194
--- /dev/null
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommand.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import 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.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CommandManager;
+import org.apache.geode.management.internal.cli.CommandManagerAware;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+
+public class GfshHelpCommand implements GfshCommand, CommandManagerAware {
+  private CommandManager commandManager = null;
+
+  public void setCommandManager(CommandManager commandManager) {
+    this.commandManager = commandManager;
+  }
+
+  @CliCommand(value = CliStrings.HELP, help = CliStrings.HELP__HELP)
+  @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_HELP})
+  public Result obtainHelp(
+      @CliOption(key = {"", CliStrings.SH__COMMAND}, optionContext = 
ConverterHint.HELP,
+          help = "Command name to provide help for") String buffer) {
+    return ResultBuilder.createInfoResult(commandManager.obtainHelp(buffer));
+  }
+
+  @CliCommand(value = CliStrings.HINT, help = CliStrings.HINT__HELP)
+  @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_HELP})
+  public Result hint(
+      @CliOption(key = {"", CliStrings.HINT__TOPICNAME}, optionContext = 
ConverterHint.HINT,
+          help = CliStrings.HINT__TOPICNAME__HELP) String topicName) {
+    return 
ResultBuilder.createInfoResult(commandManager.obtainHint(topicName));
+  }
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/cf914266/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommands.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommands.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommands.java
deleted file mode 100644
index 847c107..0000000
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommands.java
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
- * agreements. See the NOTICE file distributed with this work for additional 
information regarding
- * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the 
License. You may obtain a
- * copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software 
distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
- * or implied. See the License for the specific language governing permissions 
and limitations under
- * the License.
- */
-package org.apache.geode.management.internal.cli.commands;
-
-import 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.ConverterHint;
-import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.CommandManager;
-import org.apache.geode.management.internal.cli.CommandManagerAware;
-import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.management.internal.cli.result.ResultBuilder;
-
-/**
- * @since GemFire 7.0
- */
-public class GfshHelpCommands implements GfshCommand, CommandManagerAware {
-  private CommandManager commandManager = null;
-
-  public void setCommandManager(CommandManager commandManager) {
-    this.commandManager = commandManager;
-  }
-
-  @CliCommand(value = CliStrings.HELP, help = CliStrings.HELP__HELP)
-  @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_HELP})
-  public Result obtainHelp(
-      @CliOption(key = {"", CliStrings.SH__COMMAND}, optionContext = 
ConverterHint.HELP,
-          help = "Command name to provide help for") String buffer) {
-    return ResultBuilder.createInfoResult(commandManager.obtainHelp(buffer));
-  }
-
-  @CliCommand(value = CliStrings.HINT, help = CliStrings.HINT__HELP)
-  @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_HELP})
-  public Result hint(
-      @CliOption(key = {"", CliStrings.HINT__TOPICNAME}, optionContext = 
ConverterHint.HINT,
-          help = CliStrings.HINT__TOPICNAME__HELP) String topicName) {
-    return 
ResultBuilder.createInfoResult(commandManager.obtainHint(topicName));
-  }
-}

http://git-wip-us.apache.org/repos/asf/geode/blob/cf914266/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHintCommand.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHintCommand.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHintCommand.java
new file mode 100644
index 0000000..ccc1900
--- /dev/null
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHintCommand.java
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import 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.ConverterHint;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CommandManager;
+import org.apache.geode.management.internal.cli.CommandManagerAware;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+
+public class GfshHintCommand implements GfshCommand, CommandManagerAware {
+  private CommandManager commandManager = null;
+
+  public void setCommandManager(CommandManager commandManager) {
+    this.commandManager = commandManager;
+  }
+
+  @CliCommand(value = CliStrings.HINT, help = CliStrings.HINT__HELP)
+  @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_HELP})
+  public Result hint(@CliOption(key = {"", CliStrings.HINT__TOPICNAME},
+      optionContext = ConverterHint.HINT, help = CliStrings.HINT__TOPICNAME) 
String topicName) {
+    return 
ResultBuilder.createInfoResult(commandManager.obtainHint(topicName));
+  }
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/cf914266/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/HelpConverter.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/HelpConverter.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/HelpConverter.java
index 88fd758..502eddd 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/HelpConverter.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/HelpConverter.java
@@ -14,19 +14,20 @@
  */
 package org.apache.geode.management.internal.cli.converters;
 
-import org.apache.geode.management.cli.ConverterHint;
-import org.apache.geode.management.internal.cli.CommandManager;
-import org.apache.geode.management.internal.cli.CommandManagerAware;
-import org.apache.geode.management.internal.cli.commands.GfshHelpCommands;
+import java.util.List;
+import java.util.Set;
+
 import org.springframework.shell.core.Completion;
 import org.springframework.shell.core.Converter;
 import org.springframework.shell.core.MethodTarget;
 
-import java.util.List;
-import java.util.Set;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.management.internal.cli.CommandManager;
+import org.apache.geode.management.internal.cli.CommandManagerAware;
+import org.apache.geode.management.internal.cli.commands.GfshHelpCommand;
 
 /**
- * {@link Converter} for {@link GfshHelpCommands#obtainHelp(String)}
+ * {@link Converter} for {@link GfshHelpCommand#obtainHelp(String)}
  * 
  *
  * @since GemFire 7.0
@@ -48,18 +49,12 @@ public class HelpConverter implements Converter<String>, 
CommandManagerAware {
       completionCandidates.add(new Completion(string));
     }
 
-    if (completionCandidates.size() > 0) {
-      return true;
-    }
-    return false;
+    return completionCandidates.size() > 0;
   }
 
   @Override
   public boolean supports(Class<?> arg0, String optionContext) {
-    if (String.class.isAssignableFrom(arg0) && 
optionContext.contains(ConverterHint.HELP)) {
-      return true;
-    }
-    return false;
+    return String.class.isAssignableFrom(arg0) && 
optionContext.contains(ConverterHint.HELP);
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/geode/blob/cf914266/geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperIntegrationTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperIntegrationTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperIntegrationTest.java
index 48ad499..e44465f 100644
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperIntegrationTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperIntegrationTest.java
@@ -15,16 +15,22 @@
 
 package org.apache.geode.management.internal.cli.help;
 
+import static 
org.apache.geode.management.internal.cli.i18n.CliStrings.HINT__MSG__TOPICS_AVAILABLE;
+import static 
org.apache.geode.management.internal.cli.i18n.CliStrings.HINT__MSG__UNKNOWN_TOPIC;
+import static 
org.apache.geode.management.internal.cli.i18n.CliStrings.TOPIC_CLIENT__DESC;
 import static org.assertj.core.api.Assertions.assertThat;
 
-import org.apache.geode.management.internal.cli.commands.GfshHelpCommands;
-import org.apache.geode.test.junit.categories.IntegrationTest;
+import java.lang.reflect.Method;
+
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.springframework.shell.core.annotation.CliCommand;
 
-import java.lang.reflect.Method;
+import org.apache.geode.management.internal.cli.commands.GfshHelpCommand;
+import org.apache.geode.management.internal.cli.commands.GfshHintCommand;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.test.junit.categories.IntegrationTest;
 
 @Category(IntegrationTest.class)
 public class HelperIntegrationTest {
@@ -33,8 +39,20 @@ public class HelperIntegrationTest {
   @BeforeClass
   public static void beforeClass() {
     helper = new Helper();
-    // use GfshHelpCommand for testing
-    Method[] methods = GfshHelpCommands.class.getMethods();
+  }
+
+  private void getHelpCommand() {
+    Method[] methods = GfshHelpCommand.class.getMethods();
+    for (Method method : methods) {
+      CliCommand cliCommand = method.getDeclaredAnnotation(CliCommand.class);
+      if (cliCommand != null) {
+        helper.addCommand(cliCommand, method);
+      }
+    }
+  }
+
+  private void getHintCommand() {
+    Method[] methods = GfshHintCommand.class.getMethods();
     for (Method method : methods) {
       CliCommand cliCommand = method.getDeclaredAnnotation(CliCommand.class);
       if (cliCommand != null) {
@@ -45,19 +63,52 @@ public class HelperIntegrationTest {
 
   @Test
   public void testHelpWithNoInput() {
-    String test = helper.getHelp(null, -1);
-    String[] helpLines = test.split("\n");
-    assertThat(helpLines).hasSize(4);
+    getHelpCommand();
+    String testNoInput = helper.getHelp(null, -1);
+    String[] helpLines = testNoInput.split("\n");
+    assertThat(helpLines).hasSize(2);
     assertThat(helpLines[0]).isEqualTo("help (Available)");
-    assertThat(helpLines[2]).isEqualTo("hint (Available)");
+    assertThat(helpLines[1]).isEqualTo(CliStrings.HELP__HELP);
   }
 
   @Test
   public void testHelpWithInput() {
-    String test = helper.getHelp("help", -1);
-    String[] helpLines = test.split("\n");
+    getHelpCommand();
+    String testInput = helper.getHelp("help", -1);
+    String[] helpLines = testInput.split("\n");
     assertThat(helpLines).hasSize(12);
     assertThat(helpLines[0]).isEqualTo("NAME");
     assertThat(helpLines[1]).isEqualTo("help");
   }
+
+  @Test
+  public void testHelpWithInvalidInput() {
+    getHelpCommand();
+    String testInvalidInput = helper.getHelp("InvalidTopic", -1);
+    assertThat(testInvalidInput).isEqualTo("no help exists for this command.");
+  }
+
+  @Test
+  public void testHintWithNoInput() {
+    getHintCommand();
+    String testNoInput = helper.getHint(null);
+    String[] hintLines = testNoInput.split("\n");
+    assertThat(hintLines).hasSize(21);
+    assertThat(hintLines[0]).isEqualTo(HINT__MSG__TOPICS_AVAILABLE);
+  }
+
+  @Test
+  public void testHintWithInput() {
+    getHintCommand();
+    String testInput = helper.getHint("Client");
+    assertThat(testInput).contains(TOPIC_CLIENT__DESC);
+  }
+
+  @Test
+  public void testHintWithInvalidInput() {
+    getHintCommand();
+    String testInvalidInput = helper.getHint("InvalidTopic");
+    assertThat(testInvalidInput)
+        .isEqualTo(CliStrings.format(HINT__MSG__UNKNOWN_TOPIC, 
"InvalidTopic"));
+  }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/cf914266/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
index 195b04a..1a9c683 100644
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java
@@ -188,7 +188,7 @@ public class TestCommand {
     createTestCommand("execute function --id=InterestCalculations 
--groups=Group1", dataWrite);
     createTestCommand("list functions", clusterRead);
 
-    // GfshHelpCommands
+    // GfshHelpCommand, GfshHintCommand
     createTestCommand("hint");
     createTestCommand("help");
 

Reply via email to