[
https://issues.apache.org/jira/browse/GEODE-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16278886#comment-16278886
]
ASF GitHub Bot commented on GEODE-4049:
---------------------------------------
jinmeiliao closed pull request #1118: GEODE-4049: refactor CreateRegionCommand
and DestoryRegionCommand status
URL: https://github.com/apache/geode/pull/1118
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/cache/execute/FunctionContext.java
b/geode-core/src/main/java/org/apache/geode/cache/execute/FunctionContext.java
index b4a357f85a..733296dec2 100755
---
a/geode-core/src/main/java/org/apache/geode/cache/execute/FunctionContext.java
+++
b/geode-core/src/main/java/org/apache/geode/cache/execute/FunctionContext.java
@@ -14,7 +14,10 @@
*/
package org.apache.geode.cache.execute;
+import org.apache.logging.log4j.util.Strings;
+
import org.apache.geode.cache.Cache;
+import org.apache.geode.distributed.DistributedMember;
/**
* Defines the execution context of a {@link Function}. It is required by the
@@ -77,4 +80,22 @@
public boolean isPossibleDuplicate();
public Cache getCache();
+
+ /**
+ * a convenience method to get the name of the member this function executes
on. call this
+ * function once in your function execution to avoid performance issues.
+ *
+ * @return member name or id if name is blank
+ */
+ public default String getMemberName() {
+ DistributedMember member =
getCache().getDistributedSystem().getDistributedMember();
+
+ // if this member has name, use it, otherwise, use the id
+ String memberName = member.getName();
+ if (!Strings.isBlank(memberName)) {
+ return memberName;
+ }
+
+ return member.getId();
+ }
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java
index 0b46c0b239..ea0a636387 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommand.java
@@ -24,8 +24,6 @@
import org.springframework.shell.core.annotation.CliOption;
import org.apache.geode.cache.asyncqueue.internal.AsyncEventQueueFactoryImpl;
-import org.apache.geode.cache.execute.Function;
-import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.management.cli.ConverterHint;
import org.apache.geode.management.cli.Result;
@@ -119,7 +117,7 @@ public Result createAsyncEventQueue(
gatewaySubstitutionListener, listener, listenerProperties,
forwardExpirationDestroy);
CreateAsyncEventQueueFunction function = new
CreateAsyncEventQueueFunction();
- List<CliFunctionResult> results = execute(function, aeqArgs,
targetMembers);
+ List<CliFunctionResult> results = executeAndGetFunctionResult(function,
aeqArgs, targetMembers);
if (results.size() == 0) {
throw new RuntimeException("No results received.");
@@ -149,9 +147,4 @@ public Result createAsyncEventQueue(
return commandResult;
}
- List<CliFunctionResult> execute(Function function, Object args,
- Set<DistributedMember> targetMembers) {
- ResultCollector rc = executeFunction(function, args, targetMembers);
- return CliFunctionResult.cleanResults((List<?>) rc.getResult());
- }
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
index 5eeed3f637..732ca09210 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
@@ -344,35 +344,30 @@ public Result createRegion(
}
}
- ResultCollector<?, ?> resultCollector =
- executeFunction(RegionCreateFunction.INSTANCE, functionArgs,
membersToCreateRegionOn);
- @SuppressWarnings("unchecked")
- List<CliFunctionResult> regionCreateResults =
- (List<CliFunctionResult>) resultCollector.getResult();
+ List<CliFunctionResult> regionCreateResults = executeAndGetFunctionResult(
+ RegionCreateFunction.INSTANCE, functionArgs, membersToCreateRegionOn);
TabularResultData tabularResultData =
ResultBuilder.createTabularResultData();
- final String errorPrefix = "ERROR: ";
+ boolean regionCreated = false;
for (CliFunctionResult regionCreateResult : regionCreateResults) {
- boolean success = regionCreateResult.isSuccessful();
tabularResultData.accumulate("Member",
regionCreateResult.getMemberIdOrName());
- tabularResultData.accumulate("Status",
- (success ? "" : errorPrefix) + regionCreateResult.getMessage());
-
- if (success) {
+ if (regionCreateResult.isSuccessful()) {
+ tabularResultData.accumulate("Status",
regionCreateResult.getMessage());
xmlEntity.set(regionCreateResult.getXmlEntity());
+ regionCreated = true;
} else {
- tabularResultData.setStatus(Result.Status.ERROR);
+ tabularResultData.accumulate("Status", "ERROR: " +
regionCreateResult.getErrorMessage());
}
}
+ tabularResultData.setStatus(regionCreated ? Result.Status.OK :
Result.Status.ERROR);
result = ResultBuilder.buildResult(tabularResultData);
- verifyDistributedRegionMbean(cache, regionPath);
- if (xmlEntity.get() != null) {
+ if (regionCreated) {
+ verifyDistributedRegionMbean(cache, regionPath);
persistClusterConfiguration(result,
() -> getSharedConfiguration().addXmlEntity(xmlEntity.get(),
groups));
}
-
return result;
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
index 42d1ecde70..22db28d1b4 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyDiskStoreCommand.java
@@ -22,12 +22,10 @@
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
-import org.apache.geode.cache.execute.ResultCollector;
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.DestroyDiskStoreFunction;
import
org.apache.geode.management.internal.cli.functions.DestroyDiskStoreFunctionArgs;
@@ -54,7 +52,7 @@ public Result destroyDiskStore(
TabularResultData tabularData = ResultBuilder.createTabularResultData();
- Set<DistributedMember> targetMembers = CliUtil.findMembers(groups, null);
+ Set<DistributedMember> targetMembers = findMembers(groups, null);
if (targetMembers.isEmpty()) {
return
ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
@@ -62,9 +60,8 @@ public Result destroyDiskStore(
DestroyDiskStoreFunctionArgs functionArgs = new
DestroyDiskStoreFunctionArgs(name, ifExist);
- ResultCollector<?, ?> rc =
- CliUtil.executeFunction(new DestroyDiskStoreFunction(), functionArgs,
targetMembers);
- List<CliFunctionResult> results = CliFunctionResult.cleanResults((List<?>)
rc.getResult());
+ List<CliFunctionResult> results =
+ executeAndGetFunctionResult(new DestroyDiskStoreFunction(),
functionArgs, targetMembers);
AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
for (CliFunctionResult result : results) {
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommand.java
index 13947ec469..e999ccca29 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommand.java
@@ -21,15 +21,17 @@
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
-import org.apache.geode.cache.execute.ResultCollector;
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.exceptions.EntityNotFoundException;
import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import
org.apache.geode.management.internal.cli.functions.RegionDestroyFunction;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.CommandResult;
import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.TabularResultData;
import org.apache.geode.management.internal.configuration.domain.XmlEntity;
import org.apache.geode.management.internal.security.ResourceOperation;
import org.apache.geode.security.ResourcePermission;
@@ -49,52 +51,49 @@ public Result destroyRegion(
// regionPath should already be converted to have "/" in front of it.
AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
+ // this finds all the members that host this region. destroy will be
called on each of these
+ // members since the region might be a scope.LOCAL region
Set<DistributedMember> regionMembersList =
findMembersForRegion(getCache(), regionPath);
if (regionMembersList.size() == 0) {
- if (ifExists) {
- return ResultBuilder.createInfoResult("");
- } else {
- return ResultBuilder.createUserErrorResult(
-
CliStrings.format(CliStrings.DESTROY_REGION__MSG__COULD_NOT_FIND_REGIONPATH_0_IN_GEODE,
- regionPath, "jmx-manager-update-rate milliseconds"));
- }
+ String message =
+
CliStrings.format(CliStrings.DESTROY_REGION__MSG__COULD_NOT_FIND_REGIONPATH_0_IN_GEODE,
+ regionPath, "jmx-manager-update-rate milliseconds");
+ throw new EntityNotFoundException(message, ifExists);
}
- ResultCollector<?, ?> resultCollector =
- executeFunction(RegionDestroyFunction.INSTANCE, regionPath,
regionMembersList);
- List<CliFunctionResult> resultsList = (List<CliFunctionResult>)
resultCollector.getResult();
+ List<CliFunctionResult> resultsList =
+ executeAndGetFunctionResult(RegionDestroyFunction.INSTANCE,
regionPath, regionMembersList);
+
+ // destroy is called on each member. If the region destroy is successful
on one member, we
+ // deem the destroy action successful, since if one member destroy
successfully, the subsequent
+ // destroy on a another member would probably throw
RegionDestroyedException
+ TabularResultData tabularData = ResultBuilder.createTabularResultData();
- // destroy is called on each member, if any error happens in any one of
the member, we should
- // deem the destroy not successful.
- String errorMessage = null;
+ boolean regionDestroyed = false;
for (CliFunctionResult functionResult : resultsList) {
+ tabularData.accumulate("Member", functionResult.getMemberIdOrName());
if (functionResult.isSuccessful()) {
- xmlEntity.set(functionResult.getXmlEntity());
- } else {
- if (functionResult.getThrowable() != null) {
- throw functionResult.getThrowable();
- }
- if (functionResult.getMessage() != null) {
- errorMessage = functionResult.getMessage();
- } else {
- errorMessage = "Destroy failed on one member";
+ if (xmlEntity.get() == null) {
+ xmlEntity.set(functionResult.getXmlEntity());
}
- // if any error occurred, break out without looking further
- break;
+ tabularData.accumulate("Status", functionResult.getMessage());
+ regionDestroyed = true;
+ } else {
+ tabularData.accumulate("Status", "Error: " +
functionResult.getErrorMessage());
}
}
- if (errorMessage != null) {
- return ResultBuilder.createGemFireErrorResult(errorMessage);
- }
+ tabularData.setStatus(regionDestroyed ? Result.Status.OK :
Result.Status.ERROR);
- Result result =
- ResultBuilder.createInfoResult(String.format("\"%s\" destroyed
successfully.", regionPath));
- if (xmlEntity.get() != null) {
+ CommandResult result = ResultBuilder.buildResult(tabularData);
+
+ // if at least one member returns with successful deletion, we will need
to update cc
+ if (regionDestroyed) {
persistClusterConfiguration(result,
() -> getSharedConfiguration().deleteXmlEntity(xmlEntity.get(),
null));
}
return result;
}
+
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
index f94b6d1109..b251d64804 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshCommand.java
@@ -15,6 +15,7 @@
package org.apache.geode.management.internal.cli.commands;
import java.util.Collections;
+import java.util.List;
import java.util.Set;
import org.springframework.shell.core.CommandMarker;
@@ -33,6 +34,7 @@
import org.apache.geode.management.cli.Result;
import org.apache.geode.management.internal.cli.CliUtil;
import
org.apache.geode.management.internal.cli.exceptions.EntityNotFoundException;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.shell.Gfsh;
@@ -180,6 +182,12 @@ default ManagementService getManagementService() {
return executeFunction(function, args,
Collections.singleton(targetMember));
}
+ default List<CliFunctionResult> executeAndGetFunctionResult(Function
function, Object args,
+ Set<DistributedMember> targetMembers) {
+ ResultCollector rc = executeFunction(function, args, targetMembers);
+ return CliFunctionResult.cleanResults((List<?>) rc.getResult());
+ }
+
default Set<DistributedMember> findAnyMembersForRegion(InternalCache cache,
String regionPath) {
return CliUtil.getRegionAssociatedMembers(regionPath, cache, false);
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
index 1c7baef4d1..80985660b7 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
@@ -74,8 +74,7 @@ public void execute(FunctionContext context) {
ResultSender<Object> resultSender = context.getResultSender();
Cache cache = context.getCache();
- String memberNameOrId =
-
CliUtil.getMemberNameOrId(cache.getDistributedSystem().getDistributedMember());
+ String memberNameOrId = context.getMemberName();
RegionFunctionArgs regionCreateArgs = (RegionFunctionArgs)
context.getArguments();
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunction.java
index 1aae3383a4..ef235223e9 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunction.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunction.java
@@ -16,10 +16,12 @@
import org.apache.geode.cache.Cache;
import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionDestroyedException;
import org.apache.geode.cache.execute.Function;
import org.apache.geode.cache.execute.FunctionContext;
import org.apache.geode.internal.InternalEntity;
import org.apache.geode.internal.cache.xmlcache.CacheXml;
+import org.apache.geode.internal.logging.LogService;
import org.apache.geode.management.internal.configuration.domain.XmlEntity;
/**
@@ -41,6 +43,8 @@ public boolean hasResult() {
@Override
public void execute(FunctionContext context) {
String regionPath = null;
+ String memberName = context.getMemberName();
+
try {
String functionId = context.getFunctionId();
Object arguments = context.getArguments();
@@ -54,24 +58,31 @@ public void execute(FunctionContext context) {
regionPath = (String) arguments;
Cache cache = context.getCache();
Region<?, ?> region = cache.getRegion(regionPath);
- // if the region is a distributed region, and is already destroyed by
another member
+ // the region is already destroyed by another member
if (region == null) {
- context.getResultSender().lastResult(new CliFunctionResult("", true,
"SUCCESS"));
+ context.getResultSender().lastResult(new CliFunctionResult(memberName,
true,
+ String.format("Region '%s' already destroyed", regionPath)));
return;
}
region.destroyRegion();
+
String regionName =
regionPath.startsWith(Region.SEPARATOR) ? regionPath.substring(1) :
regionPath;
XmlEntity xmlEntity = new XmlEntity(CacheXml.REGION, "name", regionName);
- context.getResultSender().lastResult(new CliFunctionResult("",
xmlEntity, regionPath));
+ context.getResultSender().lastResult(new CliFunctionResult(memberName,
xmlEntity,
+ String.format("Region '%s' destroyed successfully", regionPath)));
} catch (IllegalStateException ex) {
- // user is trying to destroy something that can't destroyed.
- context.getResultSender().lastResult(new CliFunctionResult("", false,
ex.getMessage()));
- } catch (Exception ex) {
+ // user is trying to destroy something that can't destroyed, like
co-location
context.getResultSender()
- .lastResult(new CliFunctionResult("", ex, "failed to destroy " +
regionPath));
+ .lastResult(new CliFunctionResult(memberName, false,
ex.getMessage()));
+ } catch (RegionDestroyedException ex) {
+ context.getResultSender().lastResult(new CliFunctionResult(memberName,
true,
+ String.format("Region '%s' already destroyed", regionPath)));
+ } catch (Exception ex) {
+ LogService.getLogger().error(ex.getMessage(), ex);
+ context.getResultSender().lastResult(new CliFunctionResult(memberName,
ex, ex.getMessage()));
}
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandTest.java
index 3fd59af89c..3073eaffa5 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateAsyncEventQueueCommandTest.java
@@ -128,8 +128,8 @@ public void buildResult_all_success() throws Exception {
// this is only to make the code pass that member check
doReturn(Collections.emptySet()).when(command).getMembers(any(), any());
- doReturn(functionResults).when(command).execute(isA(Function.class),
isA(Object.class),
- isA(Set.class));
+
doReturn(functionResults).when(command).executeAndGetFunctionResult(isA(Function.class),
+ isA(Object.class), isA(Set.class));
gfsh.executeAndAssertThat(command,
MINIUM_COMMAND).statusIsSuccess().persisted()
.tableHasRowCount("Member", 2)
@@ -151,8 +151,8 @@ public void buildResult_all_failure() throws Exception {
// this is only to make the code pass that member check
doReturn(Collections.emptySet()).when(command).getMembers(any(), any());
- doReturn(functionResults).when(command).execute(isA(Function.class),
isA(Object.class),
- isA(Set.class));
+
doReturn(functionResults).when(command).executeAndGetFunctionResult(isA(Function.class),
+ isA(Object.class), isA(Set.class));
gfsh.executeAndAssertThat(command,
MINIUM_COMMAND).statusIsSuccess().persisted() // need to make
// sure
@@ -182,8 +182,8 @@ public void buildResult_one_failure_one_success() throws
Exception {
// this is only to make the code pass that member check
doReturn(Collections.emptySet()).when(command).getMembers(any(), any());
- doReturn(functionResults).when(command).execute(isA(Function.class),
isA(Object.class),
- isA(Set.class));
+
doReturn(functionResults).when(command).executeAndGetFunctionResult(isA(Function.class),
+ isA(Object.class), isA(Set.class));
gfsh.executeAndAssertThat(command,
MINIUM_COMMAND).statusIsSuccess().persisted()
.tableHasRowCount("Member", 2)
@@ -202,8 +202,8 @@ public void
command_succeeded_but_no_cluster_config_service() throws Exception {
List<CliFunctionResult> functionResults = new ArrayList<>();
XmlEntity xmlEntity = mock(XmlEntity.class);
functionResults.add(new CliFunctionResult("member1", xmlEntity,
"SUCCESS"));
- doReturn(functionResults).when(command).execute(isA(Function.class),
isA(Object.class),
- isA(Set.class));
+
doReturn(functionResults).when(command).executeAndGetFunctionResult(isA(Function.class),
+ isA(Object.class), isA(Set.class));
gfsh.executeAndAssertThat(command,
MINIUM_COMMAND).statusIsSuccess().failToPersist();
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandDUnitTest.java
index 51e2102b08..55dbff3e6c 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandDUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandDUnitTest.java
@@ -27,13 +27,10 @@
import org.junit.experimental.categories.Category;
import org.apache.geode.cache.Cache;
-import org.apache.geode.cache.PartitionAttributesFactory;
-import org.apache.geode.cache.RegionFactory;
-import org.apache.geode.cache.RegionShortcut;
-import org.apache.geode.cache.Scope;
import org.apache.geode.distributed.Locator;
import org.apache.geode.distributed.internal.ClusterConfigurationService;
import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.management.internal.configuration.domain.Configuration;
import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.categories.DistributedTest;
@@ -52,7 +49,7 @@
@BeforeClass
public static void beforeClass() throws Exception {
locator = lsRule.startLocatorVM(0);
- server1 = lsRule.startServerVM(1, locator.getPort());
+ server1 = lsRule.startServerVM(1, "group1", locator.getPort());
server2 = lsRule.startServerVM(2, locator.getPort());
server3 = lsRule.startServerVM(3, locator.getPort());
}
@@ -64,91 +61,84 @@ public void before() throws Exception {
@Test
public void testDestroyDistributedRegion() {
- MemberVM.invokeInEveryMember(() -> {
- Cache cache = LocatorServerStartupRule.getCache();
- RegionFactory<Object, Object> factory =
cache.createRegionFactory(RegionShortcut.PARTITION);
- factory.create("Customer");
+ gfsh.executeAndAssertThat("create region --name=Customer
--type=PARTITION").statusIsSuccess();
+ gfsh.executeAndAssertThat(
+ "create region --name=Order --type=PARTITION
--colocated-with=Customer").statusIsSuccess();
- PartitionAttributesFactory paFactory = new PartitionAttributesFactory();
- paFactory.setColocatedWith("Customer");
- factory.setPartitionAttributes(paFactory.create());
- factory.create("Order");
- }, server1, server2);
-
- locator.waitTillRegionsAreReadyOnServers("/Customer", 2);
- locator.waitTillRegionsAreReadyOnServers("/Order", 2);
+ locator.waitTillRegionsAreReadyOnServers("/Customer", 3);
+ locator.waitTillRegionsAreReadyOnServers("/Order", 3);
// Test unable to destroy with co-location
gfsh.executeAndAssertThat("destroy region
--name=/Customer").statusIsError()
+ .tableHasRowCount("Member", 3)
.containsOutput("The parent region [/Customer] in colocation chain
cannot be destroyed");
// Test success
gfsh.executeAndAssertThat("destroy region --name=/Order").statusIsSuccess()
- .containsOutput("destroyed successfully");
+ .tableHasRowCount("Member", 3).containsOutput("destroyed
successfully");
gfsh.executeAndAssertThat("destroy region
--name=/Customer").statusIsSuccess()
- .containsOutput("destroyed successfully");
+ .tableHasRowCount("Member", 3).containsOutput("destroyed
successfully");
// destroy something that's not exist anymore
gfsh.executeAndAssertThat("destroy region
--name=/Customer").statusIsError()
.containsOutput("Could not find a Region");
- gfsh.executeAndAssertThat("destroy region --name=/Customer
--if-exists").statusIsSuccess();
+ gfsh.executeAndAssertThat("destroy region --name=/Customer --if-exists")
+ .containsOutput("Skipping: Could not find a Region").statusIsSuccess();
}
@Test
public void testDestroyLocalRegions() {
- MemberVM.invokeInEveryMember(() -> {
- Cache cache = LocatorServerStartupRule.getCache();
- RegionFactory<Object, Object> factory =
cache.createRegionFactory(RegionShortcut.REPLICATE);
- factory.setScope(Scope.LOCAL);
- factory.create("Customer");
- }, server1, server2, server3);
+ gfsh.executeAndAssertThat("create region --name=region1
--type=LOCAL").statusIsSuccess();
- locator.waitTillRegionsAreReadyOnServers("/Customer", 3);
+ locator.waitTillRegionsAreReadyOnServers("/region1", 3);
- gfsh.executeAndAssertThat("destroy region
--name=Customer").statusIsSuccess()
- .containsOutput("destroyed successfully");
+ gfsh.executeAndAssertThat("destroy region
--name=region1").statusIsSuccess()
+ .tableHasRowCount("Member", 3).containsOutput("destroyed
successfully");
MemberVM.invokeInEveryMember(() -> {
Cache cache = LocatorServerStartupRule.getCache();
- assertThat(cache.getRegion("Customer")).isNull();
+ assertThat(cache.getRegion("region1")).isNull();
}, server1, server2, server3);
}
@Test
public void testDestroyLocalAndDistributedRegions() {
- MemberVM.invokeInEveryMember(() -> {
- Cache cache = LocatorServerStartupRule.getCache();
- RegionFactory<Object, Object> factory =
cache.createRegionFactory(RegionShortcut.PARTITION);
- factory.create("Customer");
- factory.create("Customer_2");
- factory.create("Customer_3");
- }, server1, server2);
+ gfsh.executeAndAssertThat("create region --name=region1 --type=LOCAL
--group=group1")
+ .statusIsSuccess();
+ gfsh.executeAndAssertThat("create region --name=region1
--type=REPLICATE").statusIsSuccess();
- server3.invoke(() -> {
- Cache cache = LocatorServerStartupRule.getCache();
- RegionFactory<Object, Object> factory =
cache.createRegionFactory(RegionShortcut.REPLICATE);
- factory.setScope(Scope.LOCAL);
- factory.create("Customer");
- factory.create("Customer_2");
- factory.create("Customer_3");
+ locator.waitTillRegionsAreReadyOnServers("/region1", 3);
+
+ locator.invoke(() -> {
+ ClusterConfigurationService service =
+ LocatorServerStartupRule.getLocator().getSharedConfiguration();
+ Configuration group1Config = service.getConfiguration("group1");
+ assertThat(group1Config.getCacheXmlContent())
+ .contains("<region name=\"region1\">\n" + " <region-attributes
scope=\"local\"/>");
+
+ Configuration clusterConfig = service.getConfiguration("cluster");
+ assertThat(clusterConfig.getCacheXmlContent()).contains("<region
name=\"region1\">\n"
+ + " <region-attributes data-policy=\"replicate\"
scope=\"distributed-ack\"/>");
});
- locator.waitTillRegionsAreReadyOnServers("/Customer", 3);
- locator.waitTillRegionsAreReadyOnServers("/Customer_2", 3);
- locator.waitTillRegionsAreReadyOnServers("/Customer_3", 3);
+ gfsh.executeAndAssertThat("destroy region
--name=region1").statusIsSuccess()
+ .tableHasRowCount("Member", 3).containsOutput("destroyed
successfully");
- gfsh.executeAndAssertThat("destroy region
--name=Customer").statusIsSuccess()
- .containsOutput("destroyed successfully");
- gfsh.executeAndAssertThat("destroy region
--name=Customer_2").statusIsSuccess()
- .containsOutput("destroyed successfully");
- gfsh.executeAndAssertThat("destroy region
--name=Customer_3").statusIsSuccess()
- .containsOutput("destroyed successfully");
+ // verify that all cc entries are deleted, no matter what the scope is
+ locator.invoke(() -> {
+ ClusterConfigurationService service =
+ LocatorServerStartupRule.getLocator().getSharedConfiguration();
+ Configuration group1Config = service.getConfiguration("group1");
+ assertThat(group1Config.getCacheXmlContent()).doesNotContain("region1");
+ Configuration clusterConfig = service.getConfiguration("cluster");
+ assertThat(clusterConfig.getCacheXmlContent()).doesNotContain("region1");
+ });
+
+ // verify that all regions are destroyed, no matter what the scope is
MemberVM.invokeInEveryMember(() -> {
Cache cache = LocatorServerStartupRule.getCache();
- assertThat(cache.getRegion("Customer")).isNull();
- assertThat(cache.getRegion("Customer_2")).isNull();
- assertThat(cache.getRegion("Customer_3")).isNull();
+ assertThat(cache.getRegion("region1")).isNull();
}, server1, server2, server3);
}
@@ -156,20 +146,14 @@ public void testDestroyLocalAndDistributedRegions() {
public void testDestroyRegionWithSharedConfig() throws IOException {
gfsh.executeAndAssertThat("create region --name=Customer
--type=REPLICATE").statusIsSuccess();
+ // Make sure the region exists in the cluster config
locator.invoke(() -> {
- // Make sure the region exists in the cluster config
ClusterConfigurationService sharedConfig =
((InternalLocator) Locator.getLocator()).getSharedConfiguration();
assertThat(sharedConfig.getConfiguration("cluster").getCacheXmlContent())
.contains("Customer");
});
- // make sure region does exists
- MemberVM.invokeInEveryMember(() -> {
- Cache cache = LocatorServerStartupRule.getCache();
- assertThat(cache.getRegion("Customer")).isNotNull();
- }, server1, server2, server3);
-
// destroy the region
gfsh.executeAndAssertThat("destroy region
--name=Customer").statusIsSuccess()
.containsOutput("destroyed successfully");
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
index 12f43efee0..374ec2a9da 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DestroyRegionCommandTest.java
@@ -15,6 +15,7 @@
package org.apache.geode.management.internal.cli.commands;
+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;
@@ -33,9 +34,10 @@
import org.junit.Test;
import org.junit.experimental.categories.Category;
-import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.ClusterConfigurationService;
import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.internal.cli.GfshParseResult;
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;
@@ -51,19 +53,24 @@
private DestroyRegionCommand command;
private CommandResult result;
private CliFunctionResult result1, result2;
+ private ClusterConfigurationService ccService;
+ XmlEntity xmlEntity;
@Before
public void before() throws Exception {
+ xmlEntity = mock(XmlEntity.class);
command = spy(DestroyRegionCommand.class);
+ ccService = mock(ClusterConfigurationService.class);
+ doReturn(ccService).when(command).getSharedConfiguration();
doReturn(mock(InternalCache.class)).when(command).getCache();
- ResultCollector collector = mock(ResultCollector.class);
- doReturn(collector).when(command).executeFunction(any(), any(),
any(Set.class));
-
List<CliFunctionResult> functionResults = new ArrayList<>();
- doReturn(functionResults).when(collector).getResult();
+ doReturn(functionResults).when(command).executeAndGetFunctionResult(any(),
any(),
+ any(Set.class));
result1 = mock(CliFunctionResult.class);
result2 = mock(CliFunctionResult.class);
+ when(result1.getMemberIdOrName()).thenReturn("member1");
+ when(result2.getMemberIdOrName()).thenReturn("member2");
functionResults.add(result1);
functionResults.add(result2);
}
@@ -80,6 +87,12 @@ public void invalidRegion() throws Exception {
.containsOutput("Invalid command");
}
+ @Test
+ public void regionConverterApplied() {
+ GfshParseResult parseResult = parser.parse("destroy region --name=test");
+ assertThat(parseResult.getParamValue("name")).isEqualTo("/test");
+ }
+
@Test
public void whenNoRegionIsFoundOnAnyMembers() throws Exception {
doReturn(Collections.emptySet()).when(command).findMembersForRegion(any(),
any());
@@ -91,41 +104,60 @@ public void whenNoRegionIsFoundOnAnyMembers() throws
Exception {
}
@Test
- public void multipleResultReturnedWithOneError() throws Exception {
+ public void multipleResultReturned_oneSucess_oneFailed() throws Exception {
// mock this to pass the member search call
doReturn(Collections.singleton(DistributedMember.class)).when(command)
.findMembersForRegion(any(), any());
when(result1.isSuccessful()).thenReturn(true);
when(result1.getMessage()).thenReturn("result1 message");
- when(result1.getXmlEntity()).thenReturn(mock(XmlEntity.class));
+ when(result1.getXmlEntity()).thenReturn(xmlEntity);
when(result2.isSuccessful()).thenReturn(false);
- when(result2.getMessage()).thenReturn("result2 message");
+ when(result2.getErrorMessage()).thenReturn("result2 message");
- parser.executeAndAssertThat(command, "destroy region
--name=test").statusIsError()
- .containsOutput("result2 message");
+ parser.executeAndAssertThat(command, "destroy region
--name=test").statusIsSuccess()
+ .containsOutput("result1 message").containsOutput("result2 message");
- // verify that xmlEntiry returned by the result1 is not saved to Cluster
config
- verify(command, never()).persistClusterConfiguration(any(), any());
+ // verify that xmlEntiry returned by the result1 is saved to Cluster config
+ verify(ccService).deleteXmlEntity(xmlEntity, null);
}
@Test
- public void multipleResultReturnedWithOneException() throws Exception {
+ public void multipleResultReturned_oneSuccess_oneException() throws
Exception {
// mock this to pass the member search call
doReturn(Collections.singleton(DistributedMember.class)).when(command)
.findMembersForRegion(any(), any());
when(result1.isSuccessful()).thenReturn(true);
when(result1.getMessage()).thenReturn("result1 message");
- when(result1.getXmlEntity()).thenReturn(mock(XmlEntity.class));
+ when(result1.getXmlEntity()).thenReturn(xmlEntity);
+
+ when(result2.isSuccessful()).thenReturn(false);
+ when(result2.getErrorMessage()).thenReturn("something happened");
+
+ parser.executeAndAssertThat(command, "destroy region
--name=test").statusIsSuccess()
+ .containsOutput("result1 message").containsOutput("something
happened");
+
+
+ // verify that xmlEntiry returned by the result1 is saved to Cluster config
+ verify(ccService).deleteXmlEntity(xmlEntity, null);
+ }
+
+ @Test
+ public void multipleResultReturned_all_failed() throws Exception {
+ // mock this to pass the member search call
+ doReturn(Collections.singleton(DistributedMember.class)).when(command)
+ .findMembersForRegion(any(), any());
+ when(result1.isSuccessful()).thenReturn(false);
+ when(result1.getErrorMessage()).thenReturn("result1 message");
when(result2.isSuccessful()).thenReturn(false);
- when(result2.getThrowable()).thenReturn(new
IllegalArgumentException("something happened"));
+ when(result2.getErrorMessage()).thenReturn("something happened");
parser.executeAndAssertThat(command, "destroy region
--name=test").statusIsError()
- .containsOutput("something happened");
+ .containsOutput("result1 message").containsOutput("something
happened");
- // verify that xmlEntiry returned by the result1 is not saved to Cluster
config
+ // verify that xmlEntiry returned by the result1 is saved to Cluster config
verify(command, never()).persistClusterConfiguration(any(), any());
}
}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunctionTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunctionTest.java
index 60885099b2..31f3b12d15 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunctionTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/RegionDestroyFunctionTest.java
@@ -29,6 +29,7 @@
import org.mockito.ArgumentCaptor;
import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionDestroyedException;
import org.apache.geode.cache.execute.FunctionContext;
import org.apache.geode.cache.execute.ResultSender;
import org.apache.geode.internal.cache.InternalCache;
@@ -76,7 +77,24 @@ public void regionAlreadyDestroyed() throws Exception {
CliFunctionResult result = resultCaptor.getValue();
assertThat(result.isSuccessful()).isTrue();
- assertThat(result.getMessage()).contains("SUCCESS");
+ assertThat(result.getThrowable()).isNull();
+ assertThat(result.getMessage()).contains("Region 'testRegion' already
destroyed");
+ }
+
+ @Test
+ public void regionAlreadyDestroyed_throwException() throws Exception {
+
when(context.getFunctionId()).thenReturn(RegionDestroyFunction.class.getName());
+ Region region = mock(Region.class);
+ when(cache.getRegion(any())).thenReturn(region);
+ doThrow(mock(RegionDestroyedException.class)).when(region).destroyRegion();
+ function.execute(context);
+
+ verify(resultSender).lastResult(resultCaptor.capture());
+ CliFunctionResult result = resultCaptor.getValue();
+
+ assertThat(result.isSuccessful()).isTrue();
+ assertThat(result.getThrowable()).isNull();
+ assertThat(result.getMessage()).contains("Region 'testRegion' already
destroyed");
}
@Test
----------------------------------------------------------------
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
> Command that executes function on multiple members should report the status
> of execution on each member
> -------------------------------------------------------------------------------------------------------
>
> Key: GEODE-4049
> URL: https://issues.apache.org/jira/browse/GEODE-4049
> Project: Geode
> Issue Type: Bug
> Reporter: Jinmei Liao
>
> "destroy region", even though executes function on multiple members, do not
> report the result as tabular results. Need to change that.
> The tabular results of these commands should be successful if at least one
> member executes successfully. Cluster config should be updated if one
> function gets executed successfully.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
