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 9ebc4f7  Fix for GEODE-3898: Defined Indexes are not Persisted in 
Cluster Configuration. (#1022)
9ebc4f7 is described below

commit 9ebc4f728c308633069de43b621d8c1e7c16f41d
Author: Juan José Ramos <[email protected]>
AuthorDate: Tue Nov 14 08:35:49 2017 -0800

    Fix for GEODE-3898: Defined Indexes are not Persisted in Cluster 
Configuration. (#1022)
    
    * GEODE-3898: Defined Indexes are not Persisted in Cluster Configuration.
      . Added JUnit and DUnit tests.
      . CreateDefinedIndexesFunction returns a list of CliFunctionResult and 
XmlEntity with the indexes created.
      . CreateDefinedIndexesCommand parses the list of results and persists the 
changes in the cluster configuration service. This is done only once per region 
whenever at least one member was able to successfully create the index on the 
region.
---
 .../cli/commands/CreateDefinedIndexesCommand.java  |  27 ++-
 .../functions/CreateDefinedIndexesFunction.java    |  83 ++++++--
 .../CreateDefinedIndexesCommandDUnitTest.java      | 218 ++++++++++++++++++++
 .../commands/CreateDefinedIndexesCommandTest.java  | 166 +++++++++++++++
 .../CreateDefinedIndexesFunctionTest.java          | 226 +++++++++++++++++++++
 .../java/org/apache/geode/test/fake/Fakes.java     |   2 +
 6 files changed, 694 insertions(+), 28 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommand.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommand.java
index 0b34a26..da67ad3 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommand.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommand.java
@@ -15,12 +15,12 @@
 
 package org.apache.geode.management.internal.cli.commands;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
@@ -30,7 +30,6 @@ import org.apache.geode.distributed.DistributedMember;
 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.CliUtil;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import 
org.apache.geode.management.internal.cli.functions.CreateDefinedIndexesFunction;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
@@ -61,7 +60,7 @@ public class CreateDefinedIndexesCommand implements 
GfshCommand {
           help = CliStrings.CREATE_DEFINED_INDEXES__GROUP__HELP) final 
String[] group) {
 
     Result result;
-    AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
+    List<XmlEntity> xmlEntities = new ArrayList<>();
 
     if (IndexDefinition.indexDefinitions.isEmpty()) {
       final InfoResultData infoResult = ResultBuilder.createInfoResultData();
@@ -70,13 +69,13 @@ public class CreateDefinedIndexesCommand implements 
GfshCommand {
     }
 
     try {
-      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);
       }
 
-      final ResultCollector<?, ?> rc = 
CliUtil.executeFunction(createDefinedIndexesFunction,
+      final ResultCollector<?, ?> rc = 
executeFunction(createDefinedIndexesFunction,
           IndexDefinition.indexDefinitions, targetMembers);
 
       final List<Object> funcResults = (List<Object>) rc.getResult();
@@ -90,8 +89,11 @@ public class CreateDefinedIndexesCommand implements 
GfshCommand {
           if (cliFunctionResult.isSuccessful()) {
             successfulMembers.add(cliFunctionResult.getMemberIdOrName());
 
-            if (xmlEntity.get() == null) {
-              xmlEntity.set(cliFunctionResult.getXmlEntity());
+            // Only add the XmlEntity if it wasn't previously added from the 
result of another
+            // successful member.
+            XmlEntity resultEntity = cliFunctionResult.getXmlEntity();
+            if ((null != resultEntity) && 
(!xmlEntities.contains(resultEntity))) {
+              xmlEntities.add(cliFunctionResult.getXmlEntity());
             }
           } else {
             final String exceptionMessage = cliFunctionResult.getMessage();
@@ -100,12 +102,18 @@ public class CreateDefinedIndexesCommand implements 
GfshCommand {
             if (failedMembers == null) {
               failedMembers = new TreeSet<>();
             }
+
             failedMembers.add(cliFunctionResult.getMemberIdOrName());
             indexOpFailMap.put(exceptionMessage, failedMembers);
           }
         }
       }
 
+      // TODO: GEODE-3916.
+      // The index creation might succeed in some members and fail in others, 
the current logic only
+      // reports to the user the members on which the operation was 
successful, giving no details
+      // about the failures. We should report the exact details of what 
failed/succeeded, and
+      // where/why.
       if (!successfulMembers.isEmpty()) {
         final InfoResultData infoResult = ResultBuilder.createInfoResultData();
         infoResult.addLine(CliStrings.CREATE_DEFINED_INDEXES__SUCCESS__MSG);
@@ -143,10 +151,11 @@ public class CreateDefinedIndexesCommand implements 
GfshCommand {
       result = ResultBuilder.createGemFireErrorResult(e.getMessage());
     }
 
-    if (xmlEntity.get() != null) {
+    for (XmlEntity xmlEntity : xmlEntities) {
       persistClusterConfiguration(result,
-          () -> getSharedConfiguration().addXmlEntity(xmlEntity.get(), group));
+          () -> getSharedConfiguration().addXmlEntity(xmlEntity, group));
     }
+
     return result;
   }
 }
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CreateDefinedIndexesFunction.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CreateDefinedIndexesFunction.java
index 1f30db6..5d9a294 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CreateDefinedIndexesFunction.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/CreateDefinedIndexesFunction.java
@@ -14,6 +14,8 @@
  */
 package org.apache.geode.management.internal.cli.functions;
 
+import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -21,32 +23,47 @@ import java.util.Set;
 import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.execute.FunctionAdapter;
 import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultSender;
 import org.apache.geode.cache.query.Index;
 import org.apache.geode.cache.query.IndexType;
 import org.apache.geode.cache.query.MultiIndexCreationException;
 import org.apache.geode.cache.query.QueryService;
 import org.apache.geode.internal.InternalEntity;
+import org.apache.geode.internal.cache.xmlcache.CacheXml;
 import org.apache.geode.management.internal.cli.domain.IndexInfo;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
 
 public class CreateDefinedIndexesFunction extends FunctionAdapter implements 
InternalEntity {
-
   private static final long serialVersionUID = 1L;
 
   @Override
+  public String getId() {
+    return CreateDefinedIndexesFunction.class.getName();
+  }
+
+  XmlEntity createXmlEntity(final String regionName) {
+    return new XmlEntity(CacheXml.REGION, "name", regionName);
+  }
+
+  @Override
   public void execute(FunctionContext context) {
-    String memberId = null;
-    List<Index> indexes = null;
     Cache cache;
+    String memberId = null;
+    boolean lastResultSent = Boolean.FALSE;
+
     try {
       cache = context.getCache();
-      memberId = cache.getDistributedSystem().getDistributedMember().getId();
+      ResultSender sender = context.getResultSender();
       QueryService queryService = cache.getQueryService();
+      memberId = cache.getDistributedSystem().getDistributedMember().getId();
       Set<IndexInfo> indexDefinitions = (Set<IndexInfo>) 
context.getArguments();
+
       for (IndexInfo indexDefinition : indexDefinitions) {
         String indexName = indexDefinition.getIndexName();
-        String indexedExpression = indexDefinition.getIndexedExpression();
         String regionPath = indexDefinition.getRegionPath();
+        String indexedExpression = indexDefinition.getIndexedExpression();
+
         if (indexDefinition.getIndexType() == IndexType.PRIMARY_KEY) {
           queryService.defineKeyIndex(indexName, indexedExpression, 
regionPath);
         } else if (indexDefinition.getIndexType() == IndexType.HASH) {
@@ -55,26 +72,54 @@ public class CreateDefinedIndexesFunction extends 
FunctionAdapter implements Int
           queryService.defineIndex(indexName, indexedExpression, regionPath);
         }
       }
-      indexes = queryService.createDefinedIndexes();
-      context.getResultSender().lastResult(new CliFunctionResult(memberId));
-    } catch (MultiIndexCreationException e) {
+
+      List<Index> indexes = queryService.createDefinedIndexes();
+      // Build the results with one XmlEntity per region.
+      List<String> processedRegions = new ArrayList<>();
+      List<CliFunctionResult> functionResults = new ArrayList<>();
+
+      for (Index index : indexes) {
+        String regionName = index.getRegion().getName();
+
+        if (!processedRegions.contains(regionName)) {
+          XmlEntity xmlEntity = createXmlEntity(regionName);
+          functionResults.add(new CliFunctionResult(memberId, xmlEntity));
+          processedRegions.add(regionName);
+        }
+      }
+
+      for (Iterator<CliFunctionResult> iterator = functionResults.iterator(); 
iterator.hasNext();) {
+        CliFunctionResult cliFunctionResult = iterator.next();
+
+        if (iterator.hasNext()) {
+          sender.sendResult(cliFunctionResult);
+        } else {
+          sender.lastResult(cliFunctionResult);
+          lastResultSent = Boolean.TRUE;
+        }
+      }
+
+      if (!lastResultSent) {
+        // No indexes were created and no exceptions were thrown during the 
process.
+        // We still need to make sure the function returns to the caller.
+        sender.lastResult(
+            new CliFunctionResult(memberId, true, 
CliStrings.DEFINE_INDEX__FAILURE__MSG));
+      }
+    } catch (MultiIndexCreationException multiIndexCreationException) {
       StringBuffer sb = new StringBuffer();
       sb.append("Index creation failed for indexes: ").append("\n");
-      for (Map.Entry<String, Exception> failedIndex : 
e.getExceptionsMap().entrySet()) {
+      for (Map.Entry<String, Exception> failedIndex : 
multiIndexCreationException.getExceptionsMap()
+          .entrySet()) {
         sb.append(failedIndex.getKey()).append(" : 
").append(failedIndex.getValue().getMessage())
             .append("\n");
       }
-      context.getResultSender().lastResult(new CliFunctionResult(memberId, e, 
sb.toString()));
-    } catch (Exception e) {
+      context.getResultSender()
+          .lastResult(new CliFunctionResult(memberId, 
multiIndexCreationException, sb.toString()));
+    } catch (Exception exception) {
       String exceptionMessage = 
CliStrings.format(CliStrings.EXCEPTION_CLASS_AND_MESSAGE,
-          e.getClass().getName(), e.getMessage());
-      context.getResultSender().lastResult(new CliFunctionResult(memberId, e, 
exceptionMessage));
+          exception.getClass().getName(), exception.getMessage());
+      context.getResultSender()
+          .lastResult(new CliFunctionResult(memberId, exception, 
exceptionMessage));
     }
   }
-
-  @Override
-  public String getId() {
-    return CreateDefinedIndexesFunction.class.getName();
-  }
-
 }
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommandDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommandDUnitTest.java
new file mode 100644
index 0000000..ddc9637
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommandDUnitTest.java
@@ -0,0 +1,218 @@
+/*
+ * 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.assertj.core.api.Assertions.assertThat;
+
+import java.util.Properties;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.query.QueryService;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.distributed.internal.ClusterConfigurationService;
+import org.apache.geode.distributed.internal.InternalLocator;
+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.rules.GfshShellConnectionRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@Category(DistributedTest.class)
+public class CreateDefinedIndexesCommandDUnitTest {
+  private MemberVM locator, server1, server2, server3;
+
+  @Rule
+  public GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+  @Rule
+  public LocatorServerStartupRule locatorServerStartupRule = new 
LocatorServerStartupRule();
+
+  @Rule
+  public TestName testName = new SerializableTestName();
+
+  @Before
+  public void before() throws Exception {
+    locator = locatorServerStartupRule.startLocatorVM(0);
+    server1 = locatorServerStartupRule.startServerVM(1, locator.getPort());
+    server2 = locatorServerStartupRule.startServerVM(2, locator.getPort());
+
+    Properties server3Properties = new Properties();
+    server3Properties.setProperty(ConfigurationProperties.LOCATORS,
+        "localhost[" + locator.getPort() + "]");
+    server3Properties.setProperty(ConfigurationProperties.GROUPS, "group1");
+    server3 = locatorServerStartupRule.startServerVM(3, server3Properties);
+
+    gfsh.connectAndVerify(locator);
+    gfsh.executeAndAssertThat("clear defined indexes").statusIsSuccess()
+        .containsOutput("Index definitions successfully cleared");
+  }
+
+  @Test
+  public void noDefinitions() {
+    gfsh.executeAndAssertThat("create defined indexes").statusIsSuccess()
+        .containsOutput("No indexes defined");
+  }
+
+  @Test
+  public void nonexistentRegion() {
+    String regionName = testName.getMethodName();
+
+    MemberVM.invokeInEveryMember(() -> {
+      Cache cache = LocatorServerStartupRule.getCache();
+      assertThat(cache.getRegion(regionName)).isNull();
+    }, server1, server2, server3);
+
+    gfsh.executeAndAssertThat("define index --name=index_" + regionName
+        + " --expression=value1 --region=" + regionName + 
"1").statusIsSuccess()
+        .containsOutput("Index successfully defined");
+
+    gfsh.executeAndAssertThat("create defined indexes").statusIsError()
+        .containsOutput("RegionNotFoundException");
+
+    MemberVM.invokeInEveryMember(() -> {
+      Cache cache = LocatorServerStartupRule.getCache();
+      QueryService queryService = cache.getQueryService();
+
+      assertThat(queryService.getIndexes().isEmpty()).isTrue();
+    }, server1, server2, server3);
+  }
+
+  @Test
+  public void multipleIndexesOnMultipleRegionsClusterWide() {
+    String region1Name = testName.getMethodName() + "1";
+    String region2Name = testName.getMethodName() + "2";
+    String index1Name = "index_" + region1Name;
+    String index2Name = "index_" + region2Name;
+
+    gfsh.executeAndAssertThat("create region --name=" + region1Name + " 
--type=REPLICATE")
+        .statusIsSuccess().containsOutput("Region \"/" + region1Name + "\" 
created on \"server-1\"")
+        .containsOutput("Region \"/" + region1Name + "\" created on 
\"server-2\"")
+        .containsOutput("Region \"/" + region1Name + "\" created on 
\"server-3\"");
+
+    gfsh.executeAndAssertThat("create region --name=" + region2Name + " 
--type=REPLICATE")
+        .statusIsSuccess().containsOutput("Region \"/" + region2Name + "\" 
created on \"server-1\"")
+        .containsOutput("Region \"/" + region2Name + "\" created on 
\"server-2\"")
+        .containsOutput("Region \"/" + region2Name + "\" created on 
\"server-3\"");
+
+    MemberVM.invokeInEveryMember(() -> {
+      Cache cache = LocatorServerStartupRule.getCache();
+      assertThat(cache.getRegion(region1Name)).isNotNull();
+      assertThat(cache.getRegion(region2Name)).isNotNull();
+    }, server1, server2, server3);
+
+    gfsh.executeAndAssertThat(
+        "define index --name=" + index1Name + " --expression=value1 --region=" 
+ region1Name)
+        .statusIsSuccess().containsOutput("Index successfully defined");
+
+    gfsh.executeAndAssertThat(
+        "define index --name=" + index2Name + " --expression=value2 --region=" 
+ region2Name)
+        .statusIsSuccess().containsOutput("Index successfully defined");
+
+    gfsh.executeAndAssertThat("create defined indexes").statusIsSuccess()
+        .containsOutput("Indexes successfully created");
+
+    MemberVM.invokeInEveryMember(() -> {
+      Cache cache = LocatorServerStartupRule.getCache();
+      QueryService queryService = cache.getQueryService();
+      Region region1 = cache.getRegion(region1Name);
+      Region region2 = cache.getRegion(region2Name);
+
+      assertThat(queryService.getIndexes(region1).size()).isEqualTo(1);
+      assertThat(queryService.getIndexes(region2).size()).isEqualTo(1);
+      assertThat(queryService.getIndex(region1, index1Name)).isNotNull();
+      assertThat(queryService.getIndex(region2, index2Name)).isNotNull();
+    }, server1, server2, server3);
+
+    locator.invoke(() -> {
+      // Make sure the indexes exist in the cluster config
+      ClusterConfigurationService sharedConfig =
+          ((InternalLocator) Locator.getLocator()).getSharedConfiguration();
+      
assertThat(sharedConfig.getConfiguration("cluster").getCacheXmlContent()).contains(index1Name,
+          index2Name);
+      assertThat(sharedConfig.getConfiguration("group1")).isNull();
+    });
+  }
+
+  @Test
+  public void multipleIndexesOnMultipleRegionsInMemberGroup() {
+    String region1Name = testName.getMethodName() + "1";
+    String region2Name = testName.getMethodName() + "2";
+    String index1Name = "index_" + region1Name;
+    String index2Name = "index_" + region2Name;
+
+    gfsh.executeAndAssertThat(
+        "create region --name=" + region1Name + " --type=REPLICATE 
--group=group1")
+        .statusIsSuccess()
+        .containsOutput("Region \"/" + region1Name + "\" created on 
\"server-3\"");
+
+    gfsh.executeAndAssertThat(
+        "create region --name=" + region2Name + " --type=REPLICATE 
--group=group1")
+        .statusIsSuccess()
+        .containsOutput("Region \"/" + region2Name + "\" created on 
\"server-3\"");
+
+    MemberVM.invokeInEveryMember(() -> {
+      Cache cache = LocatorServerStartupRule.getCache();
+      assertThat(cache.getRegion(region1Name)).isNull();
+      assertThat(cache.getRegion(region2Name)).isNull();
+    }, server1, server2);
+
+    MemberVM.invokeInEveryMember(() -> {
+      Cache cache = LocatorServerStartupRule.getCache();
+      assertThat(cache.getRegion(region1Name)).isNotNull();
+      assertThat(cache.getRegion(region2Name)).isNotNull();
+    }, server3);
+
+    gfsh.executeAndAssertThat(
+        "define index --name=" + index1Name + " --expression=value1 --region=" 
+ region1Name)
+        .statusIsSuccess().containsOutput("Index successfully defined");
+
+    gfsh.executeAndAssertThat(
+        "define index --name=" + index2Name + " --expression=value1 --region=" 
+ region2Name)
+        .statusIsSuccess().containsOutput("Index successfully defined");
+
+    gfsh.executeAndAssertThat("create defined indexes 
--group=group1").statusIsSuccess()
+        .containsOutput("Indexes successfully created");
+
+    MemberVM.invokeInEveryMember(() -> {
+      Cache cache = LocatorServerStartupRule.getCache();
+      QueryService queryService = cache.getQueryService();
+      Region region1 = cache.getRegion(region1Name);
+      Region region2 = cache.getRegion(region2Name);
+
+      assertThat(queryService.getIndexes(region1).size()).isEqualTo(1);
+      assertThat(queryService.getIndexes(region2).size()).isEqualTo(1);
+      assertThat(queryService.getIndex(region1, index1Name)).isNotNull();
+      assertThat(queryService.getIndex(region2, index2Name)).isNotNull();
+    }, server3);
+
+    locator.invoke(() -> {
+      // Make sure the indexes exist in the cluster config
+      ClusterConfigurationService sharedConfig =
+          ((InternalLocator) Locator.getLocator()).getSharedConfiguration();
+      
assertThat(sharedConfig.getConfiguration("group1").getCacheXmlContent()).contains(index2Name,
+          index1Name);
+      
assertThat(sharedConfig.getConfiguration("cluster").getCacheXmlContent()).isNullOrEmpty();
+    });
+  }
+}
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommandTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommandTest.java
new file mode 100644
index 0000000..3435d0c
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateDefinedIndexesCommandTest.java
@@ -0,0 +1,166 @@
+/*
+ * 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.apache.geode.management.cli.Result.Status.OK;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.mockito.Mockito;
+
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.cache.query.IndexType;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.ClusterConfigurationService;
+import org.apache.geode.management.internal.cli.domain.IndexInfo;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.test.junit.rules.GfshParserRule;
+
+@Category(UnitTest.class)
+public class CreateDefinedIndexesCommandTest {
+  @Rule
+  public GfshParserRule gfshParser = new GfshParserRule();
+
+  private CommandResult result;
+  private ResultCollector resultCollector;
+  private CreateDefinedIndexesCommand command;
+
+  @Before
+  public void setUp() throws Exception {
+    IndexDefinition.indexDefinitions.clear();
+    resultCollector = mock(ResultCollector.class);
+    command = spy(CreateDefinedIndexesCommand.class);
+    doReturn(resultCollector).when(command).executeFunction(any(), any(), 
any(Set.class));
+  }
+
+  @Test
+  public void noDefinitions() throws Exception {
+    result = gfshParser.executeCommandWithInstance(command, "create defined 
indexes");
+    assertThat(result.getStatus()).isEqualTo(OK);
+    assertThat(result.getContent().toString()).contains("No indexes defined");
+  }
+
+  @Test
+  public void noMembers() throws Exception {
+    IndexDefinition.indexDefinitions
+        .add(new IndexInfo("indexName", "indexedExpression", "TestRegion", 
IndexType.FUNCTIONAL));
+    doReturn(Collections.EMPTY_SET).when(command).findMembers(any(), any());
+    result = gfshParser.executeCommandWithInstance(command, "create defined 
indexes");
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    assertThat(result.getContent().toString()).contains("No Members Found");
+  }
+
+  @Test
+  public void creationFailure() throws Exception {
+    DistributedMember member = mock(DistributedMember.class);
+    when(member.getId()).thenReturn("memberId");
+    ClusterConfigurationService mockService = 
mock(ClusterConfigurationService.class);
+
+    doReturn(mockService).when(command).getSharedConfiguration();
+    doReturn(Collections.singleton(member)).when(command).findMembers(any(), 
any());
+    doReturn(Arrays.asList(new CliFunctionResult(member.getId(), new 
Exception("MockException"),
+        "Exception Message."))).when(resultCollector).getResult();
+
+    IndexDefinition.indexDefinitions
+        .add(new IndexInfo("index1", "value1", "TestRegion", 
IndexType.FUNCTIONAL));
+    result = gfshParser.executeCommandWithInstance(command, "create defined 
indexes");
+
+    assertThat(result.getStatus()).isEqualTo(ERROR);
+    verify(command, never()).persistClusterConfiguration(any(), any());
+  }
+
+  @Test
+  public void creationSuccess() throws Exception {
+    XmlEntity xmlEntity = mock(XmlEntity.class);
+    DistributedMember member = mock(DistributedMember.class);
+    when(member.getId()).thenReturn("memberId");
+    ClusterConfigurationService mockService = 
mock(ClusterConfigurationService.class);
+
+    doReturn(mockService).when(command).getSharedConfiguration();
+    doReturn(Collections.singleton(member)).when(command).findMembers(any(), 
any());
+    doReturn(Arrays.asList(new CliFunctionResult(member.getId(), 
xmlEntity))).when(resultCollector)
+        .getResult();
+
+    IndexDefinition.indexDefinitions
+        .add(new IndexInfo("index1", "value1", "TestRegion", 
IndexType.FUNCTIONAL));
+    result = gfshParser.executeCommandWithInstance(command, "create defined 
indexes");
+
+    assertThat(result.getStatus()).isEqualTo(OK);
+    assertThat(result.failedToPersist()).isFalse();
+    verify(command, Mockito.times(1)).persistClusterConfiguration(any(), 
any());
+    assertThat(result.getContent().toString()).contains("Indexes successfully 
created");
+  }
+
+  @Test
+  public void multipleIndexesOnMultipleRegions() throws Exception {
+    XmlEntity xmlEntityRegion1 = mock(XmlEntity.class);
+    XmlEntity xmlEntityRegion2 = mock(XmlEntity.class);
+    DistributedMember member1 = mock(DistributedMember.class);
+    DistributedMember member2 = mock(DistributedMember.class);
+    when(member1.getId()).thenReturn("memberId_1");
+    when(member2.getId()).thenReturn("memberId_2");
+
+    ClusterConfigurationService mockService = 
mock(ClusterConfigurationService.class);
+    CliFunctionResult member1Region1Result =
+        new CliFunctionResult(member1.getId(), xmlEntityRegion1);
+    CliFunctionResult member1Region2Result =
+        new CliFunctionResult(member1.getId(), xmlEntityRegion2);
+    CliFunctionResult member2Region1Result =
+        new CliFunctionResult(member2.getId(), xmlEntityRegion2);
+    CliFunctionResult member2Region2Result =
+        new CliFunctionResult(member2.getId(), xmlEntityRegion2);
+
+    doReturn(mockService).when(command).getSharedConfiguration();
+    doReturn(new HashSet<>(Arrays.asList(new DistributedMember[] {member1, 
member2}))).when(command)
+        .findMembers(any(), any());
+    doReturn(Arrays.asList(new CliFunctionResult[] {member1Region1Result, 
member1Region2Result,
+        member2Region1Result, 
member2Region2Result})).when(resultCollector).getResult();
+
+    IndexDefinition.indexDefinitions
+        .add(new IndexInfo("index1", "value1", "TestRegion1", 
IndexType.FUNCTIONAL));
+    IndexDefinition.indexDefinitions
+        .add(new IndexInfo("index2", "value2", "TestRegion2", 
IndexType.FUNCTIONAL));
+
+    result = gfshParser.executeCommandWithInstance(command, "create defined 
indexes");
+
+    assertThat(result.getStatus()).isEqualTo(OK);
+    assertThat(result.failedToPersist()).isFalse();
+
+    // The command will receive 4 results from 2 members, but we need to 
persist only 2 (#regions)
+    // of them.
+    verify(command, Mockito.times(2)).persistClusterConfiguration(any(), 
any());
+    assertThat(result.getContent().toString()).contains("Indexes successfully 
created");
+  }
+}
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/CreateDefinedIndexesFunctionTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/CreateDefinedIndexesFunctionTest.java
new file mode 100644
index 0000000..330e465
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/CreateDefinedIndexesFunctionTest.java
@@ -0,0 +1,226 @@
+/*
+ * 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.functions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.*;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.cache.execute.ResultSender;
+import org.apache.geode.cache.query.Index;
+import org.apache.geode.cache.query.IndexCreationException;
+import org.apache.geode.cache.query.IndexType;
+import org.apache.geode.cache.query.MultiIndexCreationException;
+import org.apache.geode.cache.query.QueryService;
+import org.apache.geode.cache.query.internal.InternalQueryService;
+import org.apache.geode.cache.query.internal.index.CompactMapRangeIndex;
+import org.apache.geode.cache.query.internal.index.HashIndex;
+import org.apache.geode.cache.query.internal.index.PrimaryKeyIndex;
+import org.apache.geode.internal.cache.execute.FunctionContextImpl;
+import org.apache.geode.management.internal.cli.domain.IndexInfo;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.test.fake.Fakes;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class CreateDefinedIndexesFunctionTest {
+  private Cache cache;
+  private Region region1;
+  private Region region2;
+  private FunctionContext context;
+  private QueryService queryService;
+  private TestResultSender resultSender;
+  private Set<IndexInfo> indexDefinitions;
+  private CreateDefinedIndexesFunction function;
+
+  @Before
+  public void setUp() throws Exception {
+    cache = Fakes.cache();
+    resultSender = new TestResultSender();
+    queryService = spy(InternalQueryService.class);
+    region1 = Fakes.region("Region1", cache);
+    region2 = Fakes.region("Region2", cache);
+    function = spy(CreateDefinedIndexesFunction.class);
+    doReturn(queryService).when(cache).getQueryService();
+
+    indexDefinitions = new HashSet<>();
+    indexDefinitions.add(new IndexInfo("index1", "value1", "/Region1", 
IndexType.HASH));
+    indexDefinitions.add(new IndexInfo("index2", "value2", "/Region2", 
IndexType.FUNCTIONAL));
+    indexDefinitions.add(new IndexInfo("index3", "value3", "/Region1", 
IndexType.PRIMARY_KEY));
+  }
+
+  @Test
+  public void noIndexDefinitionsAsFunctionArgument() throws Exception {
+    context = new FunctionContextImpl(cache, 
CreateDefinedIndexesFunction.class.getName(),
+        Collections.emptySet(), resultSender);
+
+    function.execute(context);
+    List<?> results = resultSender.getResults();
+
+    assertThat(results).isNotNull();
+    assertThat(results.size()).isEqualTo(1);
+    Object firstResult = results.get(0);
+    assertThat(firstResult).isInstanceOf(CliFunctionResult.class);
+    assertThat(((CliFunctionResult) firstResult).isSuccessful()).isTrue();
+    assertThat(((CliFunctionResult) firstResult).getMessage()).isNotEmpty();
+    assertThat(((CliFunctionResult) firstResult).getMessage()).contains("No 
indexes defined");
+  }
+
+  @Test
+  public void noIndexPreviouslyDefinedInQueryService() throws Exception {
+    
when(queryService.createDefinedIndexes()).thenReturn(Collections.emptyList());
+    context = new FunctionContextImpl(cache, 
CreateDefinedIndexesFunction.class.getName(),
+        indexDefinitions, resultSender);
+
+    function.execute(context);
+    List<?> results = resultSender.getResults();
+
+    assertThat(results).isNotNull();
+    assertThat(results.size()).isEqualTo(1);
+    Object firstResult = results.get(0);
+    assertThat(firstResult).isInstanceOf(CliFunctionResult.class);
+    assertThat(((CliFunctionResult) firstResult).isSuccessful()).isTrue();
+    assertThat(((CliFunctionResult) firstResult).getMessage()).isNotEmpty();
+    assertThat(((CliFunctionResult) firstResult).getMessage()).contains("No 
indexes defined");
+  }
+
+  @Test
+  public void multiIndexCreationExceptionThrowByQueryService() throws 
Exception {
+    HashMap<String, Exception> exceptions = new HashMap<>();
+    exceptions.put("index1", new IndexCreationException("Mock Failure."));
+    exceptions.put("index3", new IndexCreationException("Another Mock 
Failure."));
+    when(queryService.createDefinedIndexes())
+        .thenThrow(new MultiIndexCreationException(exceptions));
+    context = new FunctionContextImpl(cache, 
CreateDefinedIndexesFunction.class.getName(),
+        indexDefinitions, resultSender);
+
+    function.execute(context);
+    List<?> results = resultSender.getResults();
+
+    assertThat(results).isNotNull();
+    assertThat(results.size()).isEqualTo(1);
+    Object firstResult = results.get(0);
+    assertThat(firstResult).isNotNull();
+    assertThat(firstResult).isInstanceOf(CliFunctionResult.class);
+    assertThat(((CliFunctionResult) firstResult).isSuccessful()).isFalse();
+    assertThat(((CliFunctionResult) 
firstResult).getSerializables().length).isEqualTo(1);
+    assertThat(((CliFunctionResult) 
firstResult).getSerializables()[0]).isNotNull();
+    assertThat(((CliFunctionResult) 
firstResult).getSerializables()[0].toString()).contains(
+        "Index creation failed for indexes", "index1", "Mock Failure", 
"index3",
+        "Another Mock Failure");
+  }
+
+  @Test
+  public void unexpectedExceptionThrowByQueryService() throws Exception {
+    when(queryService.createDefinedIndexes()).thenThrow(new 
RuntimeException("Mock Exception"));
+    context = new FunctionContextImpl(cache, 
CreateDefinedIndexesFunction.class.getName(),
+        indexDefinitions, resultSender);
+
+    function.execute(context);
+    List<?> results = resultSender.getResults();
+
+    assertThat(results).isNotNull();
+    assertThat(results.size()).isEqualTo(1);
+    Object firstResult = results.get(0);
+    assertThat(firstResult).isNotNull();
+    assertThat(firstResult).isInstanceOf(CliFunctionResult.class);
+    assertThat(((CliFunctionResult) firstResult).isSuccessful()).isFalse();
+    assertThat(((CliFunctionResult) 
firstResult).getSerializables().length).isEqualTo(1);
+    assertThat(((CliFunctionResult) 
firstResult).getSerializables()[0]).isNotNull();
+    assertThat(((CliFunctionResult) 
firstResult).getSerializables()[0].toString())
+        .contains("RuntimeException", "Mock Exception");
+  }
+
+  @Test
+  public void creationSuccess() throws Exception {
+    Index index1 = mock(HashIndex.class);
+    when(index1.getName()).thenReturn("index1");
+    when(index1.getRegion()).thenReturn(region1);
+
+    Index index2 = mock(CompactMapRangeIndex.class);
+    when(index2.getName()).thenReturn("index2");
+    when(index2.getRegion()).thenReturn(region2);
+
+    Index index3 = mock(PrimaryKeyIndex.class);
+    when(index3.getName()).thenReturn("index3");
+    when(index3.getRegion()).thenReturn(region1);
+
+    doReturn(mock(XmlEntity.class)).when(function).createXmlEntity(any());
+    when(queryService.createDefinedIndexes()).thenReturn(Arrays.asList(index1, 
index2, index3));
+    context = new FunctionContextImpl(cache, 
CreateDefinedIndexesFunction.class.getName(),
+        indexDefinitions, resultSender);
+
+    function.execute(context);
+    List<?> results = resultSender.getResults();
+
+    assertThat(results).isNotNull();
+    assertThat(results.size()).isEqualTo(2);
+
+    Object firstIndex = results.get(0);
+    assertThat(firstIndex).isNotNull();
+    assertThat(firstIndex).isInstanceOf(CliFunctionResult.class);
+    assertThat(((CliFunctionResult) firstIndex).isSuccessful());
+
+    Object secondIndex = results.get(0);
+    assertThat(secondIndex).isNotNull();
+    assertThat(secondIndex).isInstanceOf(CliFunctionResult.class);
+    assertThat(((CliFunctionResult) secondIndex).isSuccessful()).isTrue();
+    assertThat(((CliFunctionResult) secondIndex).getXmlEntity()).isNotNull();
+  }
+
+  private static class TestResultSender implements ResultSender {
+
+    private final List<Object> results = new LinkedList<>();
+
+    private Exception t;
+
+    protected List<Object> getResults() throws Exception {
+      if (t != null) {
+        throw t;
+      }
+      return Collections.unmodifiableList(results);
+    }
+
+    @Override
+    public void lastResult(final Object lastResult) {
+      results.add(lastResult);
+    }
+
+    @Override
+    public void sendResult(final Object oneResult) {
+      results.add(oneResult);
+    }
+
+    @Override
+    public void sendException(final Throwable t) {
+      this.t = (Exception) t;
+    }
+  }
+}
diff --git a/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java 
b/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java
index 5148f5f..cc4d3d1 100644
--- a/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java
+++ b/geode-core/src/test/java/org/apache/geode/test/fake/Fakes.java
@@ -121,6 +121,8 @@ public class Fakes {
     when(attributes.getDataPolicy()).thenReturn(policy);
     when(region.getCache()).thenReturn(cache);
     when(region.getRegionService()).thenReturn(cache);
+    when(region.getName()).thenReturn(name);
+
     return region;
   }
 

-- 
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].

Reply via email to