[ 
https://issues.apache.org/jira/browse/GEODE-3898?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16251681#comment-16251681
 ] 

ASF GitHub Bot commented on GEODE-3898:
---------------------------------------

jinmeiliao closed pull request #1022: Fix for GEODE-3898: Defined Indexes are 
not Persisted in Cluster Configuration.
URL: https://github.com/apache/geode/pull/1022
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 0b34a269f1..da67ad352d 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.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 Result createDefinedIndexes(
           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 Result createDefinedIndexes(
     }
 
     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 Result createDefinedIndexes(
           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 Result createDefinedIndexes(
             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 Result createDefinedIndexes(
       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 1f30db61f9..5d9a2943b4 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 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 void execute(FunctionContext context) {
           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 0000000000..ddc9637c22
--- /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 0000000000..3435d0c831
--- /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 0000000000..330e4657d1
--- /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 cd75a04ddb..4bb84e776a 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
@@ -120,6 +120,8 @@ public static Region region(String name, Cache cache) {
     when(attributes.getDataPolicy()).thenReturn(policy);
     when(region.getCache()).thenReturn(cache);
     when(region.getRegionService()).thenReturn(cache);
+    when(region.getName()).thenReturn(name);
+
     return region;
   }
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Indexes created through the `create defined indexes` command are not 
> persisted to the cluster configuration service
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: GEODE-3898
>                 URL: https://issues.apache.org/jira/browse/GEODE-3898
>             Project: Geode
>          Issue Type: Bug
>          Components: querying
>            Reporter: Juan José Ramos Cassella
>            Assignee: Juan José Ramos Cassella
>              Labels: configuration, gfsh
>
> When using the cluster configuration service, indexes created through {{gfsh 
> define index}} + {{gfsh create defined indexes}} are not persisted to the 
> cluster configuration service.
> Steps to reproduce:
> # Start a locator with {{enable-cluster-configuration-enabled=true}}.
> # Start a server with {{enable-cluster-configuration-enabled=true}}.
> # Create a sample region: {{gfsh create region --name=TestRegion 
> --type=REPLICATE}}.
> # Define an index: {{gfsh define index --name=index1 --expression=value1 
> --region=TestRegion}}.
> # Created the defined indexes: {{gfsh create defined indexes}}.
> # Restart the cluster.
> # Execute {{list indexes}}. The command returns no indexes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to