geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jinmeil...@apache.org
Subject [geode] branch develop updated: GEODE-3539: refactor CreateIndexCommand and add more tests (#915)
Date Thu, 12 Oct 2017 15:46:06 GMT
This is an automated email from the ASF dual-hosted git repository.

jinmeiliao 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 9952a1e  GEODE-3539: refactor CreateIndexCommand and add more tests (#915)
9952a1e is described below

commit 9952a1ea95bfa80fd96d94a83b93c389d431fa1f
Author: jinmeiliao <jiliao@pivotal.io>
AuthorDate: Thu Oct 12 08:46:03 2017 -0700

    GEODE-3539: refactor CreateIndexCommand and add more tests (#915)
    
    * refactor CreateIndexCommand and add more tests
    * fix EnumConverter
---
 .../apache/geode/management/cli/ConverterHint.java |   3 +-
 .../management/internal/cli/CommandManager.java    |   3 +
 .../internal/cli/commands/CreateIndexCommand.java  | 157 ++++++++---------
 .../internal/cli/commands/DefineIndexCommand.java  |  31 +---
 .../internal/cli/commands/DestroyIndexCommand.java |   1 -
 .../internal/cli/commands/GfshCommand.java         |   5 +
 ...{IndexTypeConverter.java => EnumConverter.java} |  34 ++--
 .../cli/converters/IndexTypeConverter.java         |  30 ++--
 .../internal/cli/GfshParserAutoCompletionTest.java |  18 +-
 .../internal/cli/GfshParserParsingTest.java        |  10 +-
 .../cli/commands/CreateIndexCommandTest.java       | 134 +++++++++++++++
 ...Test.java => IndexCommandsIntegrationTest.java} | 187 ++++++++++-----------
 .../cli/converters/IndexTypeConverterTest.java     |  64 +++++++
 .../geode/test/junit/rules/GfshParserRule.java     |  17 +-
 .../cli/commands/IndexCommandOverHttpTest.java     |  12 +-
 15 files changed, 438 insertions(+), 268 deletions(-)

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 9c32559..18e296c 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
@@ -24,6 +24,7 @@ import org.springframework.shell.core.annotation.CliOption;
  */
 public interface ConverterHint {
   public static final String DISABLE_STRING_CONVERTER = ":disable-string-converter";
+  public static final String DISABLE_ENUM_CONVERTER = ":disable-enum-converter";
   public static final String DISKSTORE =
       "geode.converter.cluster.diskstore" + DISABLE_STRING_CONVERTER;
   public static final String FILE = "geode.converter.file";
@@ -40,7 +41,7 @@ public interface ConverterHint {
   /** Hint to be used for configured locators for discovery */
   public static final String LOCATOR_DISCOVERY_CONFIG = "geode.converter.locators.discovery.config";
   public static final String REGION_PATH = "geode.converter.region.path" + DISABLE_STRING_CONVERTER;
-  public static final String INDEX_TYPE = "geode.converter.index.type";
+  public static final String INDEX_TYPE = "geode.converter.index.type" + DISABLE_ENUM_CONVERTER;
   public static final String GATEWAY_SENDER_ID = "geode.converter.gateway.senderid";
   public static final String GATEWAY_RECEIVER_ID = "geode.converter.gateway.receiverid";
   public static final String LOG_LEVEL = "geode.converter.log.levels" + DISABLE_STRING_CONVERTER;
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 b686396..0160598 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
@@ -27,6 +27,7 @@ import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.StringTokenizer;
 
+import org.springframework.shell.converters.EnumConverter;
 import org.springframework.shell.converters.SimpleFileConverter;
 import org.springframework.shell.core.CommandMarker;
 import org.springframework.shell.core.Converter;
@@ -245,6 +246,8 @@ public class CommandManager {
   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() {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommand.java
index 6124151..91b3d28 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommand.java
@@ -29,7 +29,6 @@ import org.apache.geode.cache.Region;
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.cache.query.IndexType;
 import org.apache.geode.distributed.DistributedMember;
-import org.apache.geode.internal.lang.StringUtils;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
@@ -69,7 +68,7 @@ public class CreateIndexCommand implements GfshCommand {
 
       @CliOption(key = CliStrings.CREATE_INDEX__TYPE, unspecifiedDefaultValue = "range",
           optionContext = ConverterHint.INDEX_TYPE,
-          help = CliStrings.CREATE_INDEX__TYPE__HELP) final String indexType,
+          help = CliStrings.CREATE_INDEX__TYPE__HELP) final IndexType indexType,
 
       @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS},
           optionContext = ConverterHint.MEMBERGROUP,
@@ -78,118 +77,94 @@ public class CreateIndexCommand implements GfshCommand {
     Result result;
     AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
 
-    try {
-      IndexType idxType;
-
-      // Index type check
-      if ("range".equalsIgnoreCase(indexType)) {
-        idxType = IndexType.FUNCTIONAL;
-      } else if ("hash".equalsIgnoreCase(indexType)) {
-        idxType = IndexType.HASH;
-      } else if ("key".equalsIgnoreCase(indexType)) {
-        idxType = IndexType.PRIMARY_KEY;
-      } else {
-        return ResultBuilder
-            .createUserErrorResult(CliStrings.CREATE_INDEX__INVALID__INDEX__TYPE__MESSAGE);
-      }
-
-      if (indexName == null || indexName.isEmpty()) {
-        return ResultBuilder.createUserErrorResult(CliStrings.CREATE_INDEX__INVALID__INDEX__NAME);
-      }
-
-      if (indexedExpression == null || indexedExpression.isEmpty()) {
-        return ResultBuilder.createUserErrorResult(CliStrings.CREATE_INDEX__INVALID__EXPRESSION);
-      }
-
-      if (StringUtils.isBlank(regionPath) || regionPath.equals(Region.SEPARATOR)) {
-        return ResultBuilder.createUserErrorResult(CliStrings.CREATE_INDEX__INVALID__REGIONPATH);
-      }
-
-      if (!regionPath.startsWith(Region.SEPARATOR)) {
-        regionPath = Region.SEPARATOR + regionPath;
-      }
+    if (!regionPath.startsWith(Region.SEPARATOR)) {
+      regionPath = Region.SEPARATOR + regionPath;
+    }
 
-      IndexInfo indexInfo = new IndexInfo(indexName, indexedExpression, regionPath, idxType);
-      final Set<DistributedMember> targetMembers = CliUtil.findMembers(group, memberNameOrID);
+    final Set<DistributedMember> targetMembers = findMembers(group, memberNameOrID);
 
-      if (targetMembers.isEmpty()) {
-        return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
-      }
+    if (targetMembers.isEmpty()) {
+      return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
+    }
 
-      final ResultCollector<?, ?> rc =
-          CliUtil.executeFunction(createIndexFunction, indexInfo, targetMembers);
+    final ResultCollector<?, ?> rc =
+        createIndexOnMember(indexName, indexedExpression, regionPath, indexType, targetMembers);
 
-      final List<Object> funcResults = (List<Object>) rc.getResult();
-      final Set<String> successfulMembers = new TreeSet<>();
-      final Map<String, Set<String>> indexOpFailMap = new HashMap<>();
+    final List<Object> funcResults = (List<Object>) rc.getResult();
+    final Set<String> successfulMembers = new TreeSet<>();
+    final Map<String, Set<String>> indexOpFailMap = new HashMap<>();
 
-      for (final Object funcResult : funcResults) {
-        if (funcResult instanceof CliFunctionResult) {
-          final CliFunctionResult cliFunctionResult = (CliFunctionResult) funcResult;
+    for (final Object funcResult : funcResults) {
+      if (funcResult instanceof CliFunctionResult) {
+        final CliFunctionResult cliFunctionResult = (CliFunctionResult) funcResult;
 
-          if (cliFunctionResult.isSuccessful()) {
-            successfulMembers.add(cliFunctionResult.getMemberIdOrName());
+        if (cliFunctionResult.isSuccessful()) {
+          successfulMembers.add(cliFunctionResult.getMemberIdOrName());
 
-            if (xmlEntity.get() == null) {
-              xmlEntity.set(cliFunctionResult.getXmlEntity());
-            }
-          } else {
-            final String exceptionMessage = cliFunctionResult.getMessage();
-            Set<String> failedMembers = indexOpFailMap.get(exceptionMessage);
+          if (xmlEntity.get() == null) {
+            xmlEntity.set(cliFunctionResult.getXmlEntity());
+          }
+        } else {
+          final String exceptionMessage = cliFunctionResult.getMessage();
+          Set<String> failedMembers = indexOpFailMap.get(exceptionMessage);
 
-            if (failedMembers == null) {
-              failedMembers = new TreeSet<>();
-            }
-            failedMembers.add(cliFunctionResult.getMemberIdOrName());
-            indexOpFailMap.put(exceptionMessage, failedMembers);
+          if (failedMembers == null) {
+            failedMembers = new TreeSet<>();
           }
+          failedMembers.add(cliFunctionResult.getMemberIdOrName());
+          indexOpFailMap.put(exceptionMessage, failedMembers);
         }
       }
+    }
 
-      if (!successfulMembers.isEmpty()) {
-        final InfoResultData infoResult = ResultBuilder.createInfoResultData();
-        infoResult.addLine(CliStrings.CREATE_INDEX__SUCCESS__MSG);
-        infoResult.addLine(CliStrings.format(CliStrings.CREATE_INDEX__NAME__MSG, indexName));
-        infoResult.addLine(
-            CliStrings.format(CliStrings.CREATE_INDEX__EXPRESSION__MSG, indexedExpression));
-        infoResult.addLine(CliStrings.format(CliStrings.CREATE_INDEX__REGIONPATH__MSG, regionPath));
-        infoResult.addLine(CliStrings.CREATE_INDEX__MEMBER__MSG);
+    if (!successfulMembers.isEmpty()) {
+      final InfoResultData infoResult = ResultBuilder.createInfoResultData();
+      infoResult.addLine(CliStrings.CREATE_INDEX__SUCCESS__MSG);
+      infoResult.addLine(CliStrings.format(CliStrings.CREATE_INDEX__NAME__MSG, indexName));
+      infoResult
+          .addLine(CliStrings.format(CliStrings.CREATE_INDEX__EXPRESSION__MSG, indexedExpression));
+      infoResult.addLine(CliStrings.format(CliStrings.CREATE_INDEX__REGIONPATH__MSG, regionPath));
+      infoResult.addLine(CliStrings.CREATE_INDEX__MEMBER__MSG);
 
-        int num = 0;
+      int num = 0;
 
-        for (final String memberId : successfulMembers) {
+      for (final String memberId : successfulMembers) {
+        ++num;
+        infoResult.addLine(
+            CliStrings.format(CliStrings.CREATE_INDEX__NUMBER__AND__MEMBER, num, memberId));
+      }
+      result = ResultBuilder.buildResult(infoResult);
+
+    } else {
+      // Group members by the exception thrown.
+      final ErrorResultData erd = ResultBuilder.createErrorResultData();
+      erd.addLine(CliStrings.format(CliStrings.CREATE_INDEX__FAILURE__MSG, indexName));
+      final Set<String> exceptionMessages = indexOpFailMap.keySet();
+
+      for (final String exceptionMessage : exceptionMessages) {
+        erd.addLine(exceptionMessage);
+        erd.addLine(CliStrings.CREATE_INDEX__EXCEPTION__OCCURRED__ON);
+        final Set<String> memberIds = indexOpFailMap.get(exceptionMessage);
+        int num = 0;
+        for (final String memberId : memberIds) {
           ++num;
-          infoResult.addLine(
+          erd.addLine(
               CliStrings.format(CliStrings.CREATE_INDEX__NUMBER__AND__MEMBER, num, memberId));
         }
-        result = ResultBuilder.buildResult(infoResult);
-
-      } else {
-        // Group members by the exception thrown.
-        final ErrorResultData erd = ResultBuilder.createErrorResultData();
-        erd.addLine(CliStrings.format(CliStrings.CREATE_INDEX__FAILURE__MSG, indexName));
-        final Set<String> exceptionMessages = indexOpFailMap.keySet();
-
-        for (final String exceptionMessage : exceptionMessages) {
-          erd.addLine(exceptionMessage);
-          erd.addLine(CliStrings.CREATE_INDEX__EXCEPTION__OCCURRED__ON);
-          final Set<String> memberIds = indexOpFailMap.get(exceptionMessage);
-          int num = 0;
-          for (final String memberId : memberIds) {
-            ++num;
-            erd.addLine(
-                CliStrings.format(CliStrings.CREATE_INDEX__NUMBER__AND__MEMBER, num, memberId));
-          }
-        }
-        result = ResultBuilder.buildResult(erd);
       }
-    } catch (Exception e) {
-      result = ResultBuilder.createGemFireErrorResult(e.getMessage());
+      result = ResultBuilder.buildResult(erd);
     }
+
     if (xmlEntity.get() != null) {
       persistClusterConfiguration(result,
           () -> getSharedConfiguration().addXmlEntity(xmlEntity.get(), group));
     }
     return result;
   }
+
+  ResultCollector<?, ?> createIndexOnMember(String indexName, String indexedExpression,
+      String regionPath, IndexType indexType, Set<DistributedMember> targetMembers) {
+    IndexInfo indexInfo = new IndexInfo(indexName, indexedExpression, regionPath, indexType);
+    return CliUtil.executeFunction(createIndexFunction, indexInfo, targetMembers);
+  }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DefineIndexCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DefineIndexCommand.java
index 00a12de..692125c 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DefineIndexCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DefineIndexCommand.java
@@ -20,7 +20,6 @@ import org.springframework.shell.core.annotation.CliOption;
 
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.query.IndexType;
-import org.apache.geode.internal.lang.StringUtils;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
@@ -47,40 +46,14 @@ public class DefineIndexCommand implements GfshCommand {
           help = CliStrings.DEFINE_INDEX__REGION__HELP) String regionPath,
       @CliOption(key = CliStrings.DEFINE_INDEX__TYPE, unspecifiedDefaultValue = "range",
           optionContext = ConverterHint.INDEX_TYPE,
-          help = CliStrings.DEFINE_INDEX__TYPE__HELP) final String indexType) {
+          help = CliStrings.DEFINE_INDEX__TYPE__HELP) final IndexType indexType) {
 
     Result result;
-    IndexType idxType;
-
-    // Index type check
-    if ("range".equalsIgnoreCase(indexType)) {
-      idxType = IndexType.FUNCTIONAL;
-    } else if ("hash".equalsIgnoreCase(indexType)) {
-      idxType = IndexType.HASH;
-    } else if ("key".equalsIgnoreCase(indexType)) {
-      idxType = IndexType.PRIMARY_KEY;
-    } else {
-      return ResultBuilder
-          .createUserErrorResult(CliStrings.DEFINE_INDEX__INVALID__INDEX__TYPE__MESSAGE);
-    }
-
-    if (indexName == null || indexName.isEmpty()) {
-      return ResultBuilder.createUserErrorResult(CliStrings.DEFINE_INDEX__INVALID__INDEX__NAME);
-    }
-
-    if (indexedExpression == null || indexedExpression.isEmpty()) {
-      return ResultBuilder.createUserErrorResult(CliStrings.DEFINE_INDEX__INVALID__EXPRESSION);
-    }
-
-    if (StringUtils.isBlank(regionPath) || regionPath.equals(Region.SEPARATOR)) {
-      return ResultBuilder.createUserErrorResult(CliStrings.DEFINE_INDEX__INVALID__REGIONPATH);
-    }
-
     if (!regionPath.startsWith(Region.SEPARATOR)) {
       regionPath = Region.SEPARATOR + regionPath;
     }
 
-    IndexInfo indexInfo = new IndexInfo(indexName, indexedExpression, regionPath, idxType);
+    IndexInfo indexInfo = new IndexInfo(indexName, indexedExpression, regionPath, indexType);
     IndexDefinition.indexDefinitions.add(indexInfo);
 
     final InfoResultData infoResult = ResultBuilder.createInfoResultData();
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyIndexCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyIndexCommand.java
index c9f2b64..413dbdd 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyIndexCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyIndexCommand.java
@@ -76,7 +76,6 @@ public class DestroyIndexCommand implements GfshCommand {
           CliStrings.format(CliStrings.PROVIDE_ATLEAST_ONE_OPTION, CliStrings.DESTROY_INDEX));
     }
 
-    final Cache cache = CacheFactory.getAnyInstance();
     String regionName = null;
     if (regionPath != null) {
       regionName = regionPath.startsWith("/") ? regionPath.substring(1) : regionPath;
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
index 5455c97..87452da 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
@@ -32,6 +32,7 @@ import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.security.SecurityService;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.shell.Gfsh;
 import org.apache.geode.management.internal.cli.util.MemberNotFoundException;
@@ -145,4 +146,8 @@ public interface GfshCommand extends CommandMarker {
     return FunctionService.onMembers(members);
   }
 
+  default Set<DistributedMember> findMembers(String[] groups, String[] members) {
+    return CliUtil.findMembers(groups, members);
+  }
+
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverter.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/EnumConverter.java
similarity index 52%
copy from geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverter.java
copy to geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/EnumConverter.java
index 85c8b66..68b2923 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverter.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/EnumConverter.java
@@ -12,6 +12,7 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
+
 package org.apache.geode.management.internal.cli.converters;
 
 import java.util.List;
@@ -22,31 +23,36 @@ import org.springframework.shell.core.MethodTarget;
 
 import org.apache.geode.management.cli.ConverterHint;
 
-/***
- * Added converter to enable auto-completion for index-type
+/**
+ * we can not directly use Spring's EnumConverter because we have some Enum types that does not
+ * directly translate from name to the enum type, e.g IndexType. IndexType will use a special
+ * converter to convert to enum type, so we need a way to disable this converter
  *
+ * This needs to implement the interface, instead of extend the EnumConverter directly because the
+ * FastPathScanner can only find classes directly implement an interface
  */
-public class IndexTypeConverter implements Converter<String> {
+public class EnumConverter implements Converter<Enum<?>> {
+  private org.springframework.shell.converters.EnumConverter delegate;
+
+  public EnumConverter() {
+    delegate = new org.springframework.shell.converters.EnumConverter();
+  }
 
   @Override
-  public boolean supports(Class<?> type, String optionContext) {
-    return String.class.equals(type) && ConverterHint.INDEX_TYPE.equals(optionContext);
+  public boolean supports(final Class<?> requiredType, final String optionContext) {
+    return Enum.class.isAssignableFrom(requiredType)
+        && !optionContext.contains(ConverterHint.DISABLE_ENUM_CONVERTER);
   }
 
   @Override
-  public String convertFromText(String value, Class<?> targetType, String optionContext) {
-    return value;
+  public Enum<?> convertFromText(String value, Class<?> targetType, String optionContext) {
+    return delegate.convertFromText(value, targetType, optionContext);
   }
 
   @Override
   public boolean getAllPossibleValues(List<Completion> completions, Class<?> targetType,
       String existingData, String optionContext, MethodTarget target) {
-
-    if (String.class.equals(targetType) && ConverterHint.INDEX_TYPE.equals(optionContext)) {
-      completions.add(new Completion("range"));
-      completions.add(new Completion("key"));
-      completions.add(new Completion("hash"));
-    }
-    return !completions.isEmpty();
+    return delegate.getAllPossibleValues(completions, targetType, existingData, optionContext,
+        target);
   }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverter.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverter.java
index 85c8b66..1b10f11 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverter.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverter.java
@@ -20,33 +20,41 @@ import org.springframework.shell.core.Completion;
 import org.springframework.shell.core.Converter;
 import org.springframework.shell.core.MethodTarget;
 
+import org.apache.geode.cache.query.IndexType;
 import org.apache.geode.management.cli.ConverterHint;
 
 /***
  * Added converter to enable auto-completion for index-type
  *
  */
-public class IndexTypeConverter implements Converter<String> {
+public class IndexTypeConverter implements Converter<IndexType> {
 
   @Override
   public boolean supports(Class<?> type, String optionContext) {
-    return String.class.equals(type) && ConverterHint.INDEX_TYPE.equals(optionContext);
+    return IndexType.class.isAssignableFrom(type)
+        && optionContext.contains(ConverterHint.INDEX_TYPE);
   }
 
   @Override
-  public String convertFromText(String value, Class<?> targetType, String optionContext) {
-    return value;
+  public IndexType convertFromText(String value, Class<?> targetType, String optionContext) {
+    switch (value.toLowerCase()) {
+      case "range":
+        return IndexType.FUNCTIONAL;
+      case "key":
+        return IndexType.PRIMARY_KEY;
+      case "hash":
+        return IndexType.HASH;
+    }
+
+    throw new IllegalArgumentException("invalid index type: " + value);
   }
 
   @Override
   public boolean getAllPossibleValues(List<Completion> completions, Class<?> targetType,
       String existingData, String optionContext, MethodTarget target) {
-
-    if (String.class.equals(targetType) && ConverterHint.INDEX_TYPE.equals(optionContext)) {
-      completions.add(new Completion("range"));
-      completions.add(new Completion("key"));
-      completions.add(new Completion("hash"));
-    }
-    return !completions.isEmpty();
+    completions.add(new Completion("range"));
+    completions.add(new Completion("key"));
+    completions.add(new Completion("hash"));
+    return true;
   }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
index 209b088..86398d5 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserAutoCompletionTest.java
@@ -17,14 +17,16 @@ package org.apache.geode.management.internal.cli;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertTrue;
 
-import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.test.junit.rules.GfshParserRule;
-import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.springframework.shell.core.Completion;
 
+import org.apache.geode.cache.query.IndexType;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
 @Category(IntegrationTest.class)
 public class GfshParserAutoCompletionTest {
   @ClassRule
@@ -275,10 +277,12 @@ public class GfshParserAutoCompletionTest {
     assertThat(parser.getCommandManager().obtainHelp(command)).isEqualTo(helpString);
   }
 
-  private String getCompleted(String buffer, int cursor, Completion completed) {
-    return buffer.substring(0, cursor) + completed.getValue();
+  @Test
+  public void testIndexType() throws Exception {
+    buffer = "create index --type=";
+    candidate = parser.complete(buffer);
+    assertThat(candidate.size()).isEqualTo(IndexType.values().length);
+    assertThat(candidate.getFirstCandidate()).isEqualTo(buffer + "hash");
   }
 
-
-
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
index 5ba89af..38e4118 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
@@ -25,8 +25,8 @@ import org.junit.experimental.categories.Category;
 import org.springframework.shell.event.ParseResult;
 
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.test.junit.rules.GfshParserRule;
 import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
 
 @Category(IntegrationTest.class)
 public class GfshParserParsingTest {
@@ -354,4 +354,12 @@ public class GfshParserParsingTest {
     assertThat(result.getParamValue("name")).isNull();
     assertThat(result.getParamValue("bind-address")).isEqualTo("123");
   }
+
+  @Test
+  public void testMultiLineCommand() throws Exception {
+    String command = "start server " + GfshParser.LINE_SEPARATOR + "--name=test";
+    GfshParseResult result = parser.parse(command);
+    assertThat(result.getParamValue("name")).isEqualTo("test");
+    assertThat(result.getCommandName()).isEqualTo("start server");
+  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommandTest.java
new file mode 100644
index 0000000..9d8acba
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateIndexCommandTest.java
@@ -0,0 +1,134 @@
+/*
+ * 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 static org.apache.geode.management.cli.Result.Status.ERROR;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.mockito.ArgumentCaptor;
+
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.cache.query.IndexType;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
+@Category(UnitTest.class)
+public class CreateIndexCommandTest {
+  @Rule
+  public GfshParserRule gfshParser = new GfshParserRule();
+
+  private CreateIndexCommand command;
+  private CommandResult result;
+  private ResultCollector rc;
+
+  @Before
+  public void before() throws Exception {
+    command = spy(CreateIndexCommand.class);
+    rc = mock(ResultCollector.class);
+    when(rc.getResult()).thenReturn(Collections.emptyList());
+    doReturn(rc).when(command).createIndexOnMember(any(), any(), any(), any(), any());
+  }
+
+  @Test
+  public void missingName() throws Exception {
+    result = gfshParser.executeCommandWithInstance(command,
+        "create index --expression=abc --region=abc");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("Invalid command");
+  }
+
+  @Test
+  public void missingExpression() throws Exception {
+    result = gfshParser.executeCommandWithInstance(command, "create index --name=abc --region=abc");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("Invalid command");
+  }
+
+  @Test
+  public void missingRegion() throws Exception {
+    result =
+        gfshParser.executeCommandWithInstance(command, "create index --name=abc --expression=abc");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("Invalid command");
+  }
+
+  @Test
+  public void invalidIndexType() throws Exception {
+    result = gfshParser.executeCommandWithInstance(command,
+        "create index --name=abc --expression=abc --region=abc --type=abc");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("Invalid command");
+  }
+
+  @Test
+  public void validIndexType() throws Exception {
+    doReturn(Collections.EMPTY_SET).when(command).findMembers(any(), any());
+    result = gfshParser.executeCommandWithInstance(command,
+        "create index --name=abc --expression=abc --region=abc --type=range");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("No Members Found");
+  }
+
+  @Test
+  public void validIndexType2() throws Exception {
+    doReturn(Collections.EMPTY_SET).when(command).findMembers(any(), any());
+    result = gfshParser.executeCommandWithInstance(command,
+        "create index --name=abc --expression=abc --region=abc --type=hash");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("No Members Found");
+  }
+
+  @Test
+  public void noMemberFound() throws Exception {
+    doReturn(Collections.EMPTY_SET).when(command).findMembers(any(), any());
+    result = gfshParser.executeCommandWithInstance(command,
+        "create index --name=abc --expression=abc --region=abc");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("No Members Found");
+  }
+
+  @Test
+  public void defaultInexType() throws Exception {
+    DistributedMember member = mock(DistributedMember.class);
+    doReturn(Collections.singleton(member)).when(command).findMembers(any(), any());
+
+    ArgumentCaptor<IndexType> indexTypeCaptor = ArgumentCaptor.forClass(IndexType.class);
+    result = gfshParser.executeCommandWithInstance(command,
+        "create index --name=abc --expression=abc --region=abc");
+
+    verify(command).createIndexOnMember(eq("abc"), eq("abc"), eq("/abc"), indexTypeCaptor.capture(),
+        eq(Collections.singleton(member)));
+
+    assertThat(indexTypeCaptor.getValue()).isEqualTo(IndexType.FUNCTIONAL);
+  }
+
+
+}
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsIntegrationTest.java
similarity index 63%
rename from geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsDUnitTest.java
rename to geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsIntegrationTest.java
index be0f338..359fc52 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsIntegrationTest.java
@@ -14,13 +14,14 @@
  */
 package org.apache.geode.management.internal.cli.commands;
 
+import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
-import java.util.Properties;
+import java.util.Collection;
 
+import org.junit.After;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
@@ -30,84 +31,82 @@ import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.query.Index;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.domain.Stock;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
-import org.apache.geode.test.dunit.rules.CleanupDUnitVMsRule;
-import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
-import org.apache.geode.test.dunit.rules.MemberVM;
-import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.Server;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+/**
+ * this test class test: CreateIndexCommand, DestroyIndexCommand, ListIndexCommand
+ */
+@Category(IntegrationTest.class)
+public class IndexCommandsIntegrationTest {
+  private static final String regionName = "regionA";
+  private static final String groupName = "groupA";
+  private static final String indexName = "indexA";
+
 
-@Category(DistributedTest.class)
-public class IndexCommandsDUnitTest {
-  // This test seemed to be leaving behind state that caused ConnectCommandWithSSLTest to fail if it
-  // was run afterwards. So, this rule ensures we leave behind a clean state.
   @ClassRule
-  public static CleanupDUnitVMsRule cleanupDUnitVMsRule = new CleanupDUnitVMsRule();
+  public static ServerStarterRule server =
+      new ServerStarterRule().withProperty(GROUPS, groupName).withJMXManager().withAutoStart();
+
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    InternalCache cache = server.getCache();
+    Region region = createPartitionedRegion(regionName, cache, String.class, Stock.class);
+    region.put("VMW", new Stock("VMW", 98));
+    region.put("APPL", new Stock("APPL", 600));
+  }
 
   @Rule
   public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
 
-  @Rule
-  public LocatorServerStartupRule startupRule = new LocatorServerStartupRule();
-  private static final String partitionedRegionName = "partitionedRegion";
-  private static final String indexName = "index1";
-  private static final String groupName = "group1";
-
   @Before
   public void before() throws Exception {
-    Properties props = new Properties();
-    props.setProperty("groups", groupName);
-    MemberVM serverVM = startupRule.startServerAsJmxManager(0, props);
-    serverVM.invoke(() -> {
-      InternalCache cache = LocatorServerStartupRule.serverStarter.getCache();
-      Region parReg =
-          createPartitionedRegion(partitionedRegionName, cache, String.class, Stock.class);
-      parReg.put("VMW", new Stock("VMW", 98));
-      parReg.put("APPL", new Stock("APPL", 600));
+    connect(server);
+  }
+
+  @After
+  public void after() throws Exception {
+    // destroy all existing indexes
+    Collection<Index> indices = server.getCache().getQueryService().getIndexes();
+    indices.stream().map(Index::getName).forEach(indexName -> {
+      gfsh.executeAndVerifyCommand("destroy index --name=" + indexName);
     });
-    connect(serverVM);
+
+    gfsh.executeAndVerifyCommand("list index");
+    assertThat(gfsh.getGfshOutput()).contains("No Indexes Found");
   }
 
-  public void connect(MemberVM serverVM) throws Exception {
-    gfsh.connectAndVerify(serverVM.getJmxPort(), GfshShellConnectionRule.PortType.jmxManager);
+  public void connect(Server server) throws Exception {
+    gfsh.connectAndVerify(server.getJmxPort(), GfshShellConnectionRule.PortType.jmxManager);
   }
 
   @Test
-  public void testCreateAndDestroyIndex() throws Exception {
-    // Create an index
-    CommandStringBuilder createStringBuilder = new CommandStringBuilder(CliStrings.CREATE_INDEX);
-    createStringBuilder.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
-    createStringBuilder.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "key");
-    createStringBuilder.addOption(CliStrings.CREATE_INDEX__REGION, "/" + partitionedRegionName);
-    gfsh.executeAndVerifyCommand(createStringBuilder.toString());
-
-    assertTrue(indexIsListed());
-
-    // Destroy the index
-    CommandStringBuilder destroyStringBuilder = new CommandStringBuilder(CliStrings.DESTROY_INDEX);
-    destroyStringBuilder.addOption(CliStrings.DESTROY_INDEX__NAME, indexName);
-    destroyStringBuilder.addOption(CliStrings.DESTROY_INDEX__REGION, "/" + partitionedRegionName);
-    gfsh.executeAndVerifyCommand(destroyStringBuilder.toString());
-
-    assertFalse(indexIsListed());
+  public void testCreate() throws Exception {
+    createSimpleIndexA();
   }
 
   @Test
   public void testCreateIndexWithMultipleIterators() throws Exception {
     CommandStringBuilder createStringBuilder = new CommandStringBuilder(CliStrings.CREATE_INDEX);
-    createStringBuilder.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
+    createStringBuilder.addOption(CliStrings.CREATE_INDEX__NAME, "indexA");
     createStringBuilder.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "\"h.low\"");
     createStringBuilder.addOption(CliStrings.CREATE_INDEX__REGION,
-        "\"/" + partitionedRegionName + " s, s.history h\"");
+        "\"/" + regionName + " s, s.history h\"");
 
     gfsh.executeAndVerifyCommand(createStringBuilder.toString());
+    assertThat(gfsh.getGfshOutput()).contains("Index successfully created");
 
-    assertTrue(indexIsListed());
+    gfsh.executeAndVerifyCommand("list index");
+    assertThat(gfsh.getGfshOutput()).contains("indexA");
   }
 
   @Test
@@ -116,22 +115,24 @@ public class IndexCommandsDUnitTest {
     createStringBuilder.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
     createStringBuilder.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "\"h.low\"");
     createStringBuilder.addOption(CliStrings.CREATE_INDEX__REGION,
-        "\"/" + partitionedRegionName + " s, s.history h\"");
+        "\"/" + regionName + " s, s.history h\"");
 
     gfsh.executeAndVerifyCommand(createStringBuilder.toString());
+    assertThat(gfsh.getGfshOutput()).contains("Index successfully created");
 
-    assertTrue(indexIsListedAsValid());
+    gfsh.executeAndVerifyCommand("list index");
+    assertThat(gfsh.getGfshOutput()).contains("indexA");
   }
 
   @Test
   public void testCannotCreateIndexWithExistingIndexName() throws Exception {
-    createBaseIndexForTesting();
+    createSimpleIndexA();
 
     // CREATE the same index
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.CREATE_INDEX);
     csb.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
     csb.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "key");
-    csb.addOption(CliStrings.CREATE_INDEX__REGION, "/" + partitionedRegionName);
+    csb.addOption(CliStrings.CREATE_INDEX__REGION, "/" + regionName);
     csb.addOption(CliStrings.CREATE_INDEX__TYPE, "hash");
 
     CommandResult result = gfsh.executeCommand(csb.toString());
@@ -139,6 +140,19 @@ public class IndexCommandsDUnitTest {
   }
 
   @Test
+  public void creatIndexWithNoBeginningSlash() throws Exception {
+    CommandStringBuilder csb = new CommandStringBuilder(CliStrings.CREATE_INDEX);
+    csb.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
+    csb.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "key");
+    csb.addOption(CliStrings.CREATE_INDEX__REGION, regionName);
+    gfsh.executeAndVerifyCommand(csb.toString());
+    assertThat(gfsh.getGfshOutput()).contains("Index successfully created");
+
+    gfsh.executeAndVerifyCommand("list index");
+    assertThat(gfsh.getGfshOutput()).contains("indexA");
+  }
+
+  @Test
   public void testCannotCreateIndexInIncorrectRegion() throws Exception {
     // Create index on a wrong regionPath
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.CREATE_INDEX);
@@ -149,8 +163,16 @@ public class IndexCommandsDUnitTest {
 
     CommandResult result = gfsh.executeCommand(csb.toString());
     assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
+    assertThat(gfsh.getGfshOutput()).contains("Region not found : \"/InvalidRegionName\"");
   }
 
+  @Test
+  public void cannotCreateWithTheSameName() throws Exception {
+    createSimpleIndexA();
+    gfsh.execute("create index --name=indexA --expression=key --region=/regionA");
+    assertThat(gfsh.getGfshOutput())
+        .contains("Index \"indexA\" already exists.  Create failed due to duplicate name");
+  }
 
   @Test
   public void testCannotCreateIndexWithInvalidIndexExpression() throws Exception {
@@ -158,7 +180,7 @@ public class IndexCommandsDUnitTest {
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.CREATE_INDEX);
     csb.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
     csb.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "InvalidExpressionOption");
-    csb.addOption(CliStrings.CREATE_INDEX__REGION, "/" + partitionedRegionName);
+    csb.addOption(CliStrings.CREATE_INDEX__REGION, "/" + regionName);
     csb.addOption(CliStrings.CREATE_INDEX__TYPE, "hash");
 
     CommandResult result = gfsh.executeCommand(csb.toString());
@@ -166,22 +188,7 @@ public class IndexCommandsDUnitTest {
   }
 
   @Test
-  public void testCannotCreateIndexWithInvalidIndexType() throws Exception {
-    // Create index with wrong type
-    CommandStringBuilder csb = new CommandStringBuilder(CliStrings.CREATE_INDEX);
-    csb.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
-    csb.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "key");
-    csb.addOption(CliStrings.CREATE_INDEX__REGION, "/" + partitionedRegionName);
-    csb.addOption(CliStrings.CREATE_INDEX__TYPE, "InvalidIndexType");
-
-    CommandResult result = gfsh.executeCommand(csb.toString());
-    assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
-  }
-
-  @Test
   public void testCannotDestroyIndexWithInvalidIndexName() throws Exception {
-    createBaseIndexForTesting();
-
     // Destroy index with incorrect indexName
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESTROY_INDEX);
     csb.addOption(CliStrings.DESTROY_INDEX__NAME, "IncorrectIndexName");
@@ -193,22 +200,17 @@ public class IndexCommandsDUnitTest {
 
   @Test
   public void testCannotDestroyIndexWithInvalidRegion() throws Exception {
-    createBaseIndexForTesting();
-
     // Destroy index with incorrect region
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESTROY_INDEX);
     csb.addOption(CliStrings.DESTROY_INDEX__NAME, indexName);
     csb.addOption(CliStrings.DESTROY_INDEX__REGION, "IncorrectRegion");
     CommandResult result = gfsh.executeCommand(csb.toString());
     assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
-    assertThat(gfsh.getGfshOutput()).contains(
-        CliStrings.format(CliStrings.DESTROY_INDEX__REGION__NOT__FOUND, "IncorrectRegion"));
+    assertThat(gfsh.getGfshOutput()).contains("Region \"IncorrectRegion\" not found.");
   }
 
   @Test
   public void testCannotDestroyIndexWithInvalidMember() throws Exception {
-    createBaseIndexForTesting();
-
     // Destroy index with incorrect member name
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESTROY_INDEX);
     csb.addOption(CliStrings.DESTROY_INDEX__NAME, indexName);
@@ -216,59 +218,46 @@ public class IndexCommandsDUnitTest {
     csb.addOption(CliStrings.MEMBER, "InvalidMemberName");
     CommandResult result = gfsh.executeCommand(csb.toString());
     assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
+    assertThat(gfsh.getGfshOutput()).contains("No Members Found");
   }
 
   @Test
   public void testCannotDestroyIndexWithNoOptions() throws Exception {
-    createBaseIndexForTesting();
-
     // Destroy index with no option
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESTROY_INDEX);
     CommandResult result = gfsh.executeCommand(csb.toString());
     assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
+    assertThat(gfsh.getGfshOutput()).contains("requires that one or more parameters be provided.");
   }
 
   @Test
   public void testDestroyIndexViaRegion() throws Exception {
-    createBaseIndexForTesting();
+    createSimpleIndexA();
 
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESTROY_INDEX);
-    csb.addOption(CliStrings.DESTROY_INDEX__REGION, partitionedRegionName);
+    csb.addOption(CliStrings.DESTROY_INDEX__REGION, regionName);
     gfsh.executeAndVerifyCommand(csb.toString());
-
-    assertFalse(indexIsListed());
+    assertThat(gfsh.getGfshOutput()).contains("Indexes on region : regionA successfully destroyed");
   }
 
   @Test
   public void testDestroyIndexViaGroup() throws Exception {
-    createBaseIndexForTesting();
+    createSimpleIndexA();
 
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.DESTROY_INDEX);
     csb.addOption(CliStrings.GROUP, groupName);
     gfsh.executeAndVerifyCommand(csb.toString());
 
-    assertFalse(indexIsListed());
-
+    assertThat(gfsh.getGfshOutput()).contains("Indexes successfully destroyed");
   }
 
-  private void createBaseIndexForTesting() throws Exception {
+  private void createSimpleIndexA() throws Exception {
     CommandStringBuilder csb = new CommandStringBuilder(CliStrings.CREATE_INDEX);
     csb.addOption(CliStrings.CREATE_INDEX__NAME, indexName);
     csb.addOption(CliStrings.CREATE_INDEX__EXPRESSION, "key");
-    csb.addOption(CliStrings.CREATE_INDEX__REGION, "/" + partitionedRegionName);
-    csb.addOption(CliStrings.CREATE_INDEX__TYPE, "hash");
+    csb.addOption(CliStrings.CREATE_INDEX__REGION, "/" + regionName);
     gfsh.executeAndVerifyCommand(csb.toString());
-    assertTrue(indexIsListed());
-  }
-
-  private boolean indexIsListed() throws Exception {
-    gfsh.executeAndVerifyCommand(CliStrings.LIST_INDEX);
-    return gfsh.getGfshOutput().contains(indexName);
-  }
-
-  private boolean indexIsListedAsValid() throws Exception {
-    gfsh.executeAndVerifyCommand(CliStrings.LIST_INDEX);
-    return gfsh.getGfshOutput().contains("true");
+    assertThat(gfsh.getGfshOutput()).contains("Index successfully created");
   }
 
   private static Region<?, ?> createPartitionedRegion(String regionName, Cache cache,
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverterTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverterTest.java
new file mode 100644
index 0000000..861b639
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/converters/IndexTypeConverterTest.java
@@ -0,0 +1,64 @@
+/*
+ * 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.converters;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.query.IndexType;
+import org.apache.geode.management.cli.ConverterHint;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class IndexTypeConverterTest {
+
+  IndexTypeConverter typeConverter;
+  EnumConverter enumConverter;
+
+  @Before
+  public void before() throws Exception {
+    typeConverter = new IndexTypeConverter();
+    enumConverter = new EnumConverter();
+  }
+
+  @Test
+  public void supports() throws Exception {
+    assertThat(typeConverter.supports(IndexType.class, ConverterHint.INDEX_TYPE)).isTrue();
+    assertThat(typeConverter.supports(Enum.class, ConverterHint.INDEX_TYPE)).isFalse();
+    assertThat(typeConverter.supports(IndexType.class, "")).isFalse();
+
+    assertThat(enumConverter.supports(IndexType.class, "")).isTrue();
+    assertThat(enumConverter.supports(Enum.class, "")).isTrue();
+    assertThat(enumConverter.supports(IndexType.class, ConverterHint.INDEX_TYPE)).isFalse();
+    assertThat(enumConverter.supports(Enum.class, ConverterHint.DISABLE_ENUM_CONVERTER)).isFalse();
+  }
+
+  @Test
+  public void convert() throws Exception {
+    assertThat(typeConverter.convertFromText("hash", IndexType.class, ""))
+        .isEqualTo(IndexType.HASH);
+    assertThat(typeConverter.convertFromText("range", IndexType.class, ""))
+        .isEqualTo(IndexType.FUNCTIONAL);
+    assertThat(typeConverter.convertFromText("key", IndexType.class, ""))
+        .isEqualTo(IndexType.PRIMARY_KEY);
+    assertThatThrownBy(() -> typeConverter.convertFromText("invalid", IndexType.class, ""))
+        .isInstanceOf(IllegalArgumentException.class);
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java b/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java
index ba0be92..9e1b220 100644
--- a/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java
+++ b/geode-core/src/test/java/org/apache/geode/test/junit/rules/GfshParserRule.java
@@ -16,6 +16,15 @@ package org.apache.geode.test.junit.rules;
 
 import static org.mockito.Mockito.spy;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.rules.ExternalResource;
+import org.springframework.shell.core.Completion;
+import org.springframework.shell.core.Converter;
+import org.springframework.util.ReflectionUtils;
+
 import org.apache.geode.internal.ClassPathLoader;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.Result;
@@ -25,14 +34,6 @@ import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.GfshParser;
 import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.management.internal.cli.result.ResultBuilder;
-import org.junit.rules.ExternalResource;
-import org.springframework.shell.core.Completion;
-import org.springframework.shell.core.Converter;
-import org.springframework.util.ReflectionUtils;
-
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Set;
 
 public class GfshParserRule extends ExternalResource {
 
diff --git a/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandOverHttpTest.java b/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandOverHttpTest.java
index 23f7a27..64b2707 100644
--- a/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandOverHttpTest.java
+++ b/geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandOverHttpTest.java
@@ -17,14 +17,14 @@ package org.apache.geode.management.internal.cli.commands;
 
 import org.junit.experimental.categories.Category;
 
+import org.apache.geode.test.junit.categories.IntegrationTest;
 import org.apache.geode.test.junit.rules.GfshShellConnectionRule;
-import org.apache.geode.test.dunit.rules.MemberVM;
-import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.Server;
 
-@Category(DistributedTest.class)
-public class IndexCommandOverHttpTest extends IndexCommandsDUnitTest {
+@Category(IntegrationTest.class)
+public class IndexCommandOverHttpTest extends IndexCommandsIntegrationTest {
   @Override
-  public void connect(MemberVM serverVM) throws Exception {
-    gfsh.connectAndVerify(serverVM.getHttpPort(), GfshShellConnectionRule.PortType.http);
+  public void connect(Server server) throws Exception {
+    gfsh.connectAndVerify(server.getHttpPort(), GfshShellConnectionRule.PortType.http);
   }
 }

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

Mime
View raw message