[ 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)