This is an automated email from the ASF dual-hosted git repository.

jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 804ecfa  GEODE-4625: name collision check when create region through 
gfsh (#1483)
804ecfa is described below

commit 804ecfabef7a1e661626027e1e01e5652901c036
Author: jinmeiliao <jil...@pivotal.io>
AuthorDate: Thu Feb 22 15:50:11 2018 -0800

    GEODE-4625: name collision check when create region through gfsh (#1483)
---
 .../org/apache/geode/cache/RegionShortcut.java     | 26 +++++-
 .../geode/management/DistributedRegionMXBean.java  |  5 ++
 .../internal/beans/DistributedRegionBridge.java    |  2 +-
 .../beans/stats/RegionClusterStatsMonitor.java     |  2 +
 .../internal/cli/commands/CreateRegionCommand.java | 56 ++++++++++---
 .../cli/exceptions/EntityExistsException.java      | 25 ++++++
 .../sanctioned-geode-core-serializables.txt        |  1 +
 .../org/apache/geode/cache/RegionShortcutTest.java | 41 +++++++++
 .../cli/commands/CreateRegionCommandDUnitTest.java | 96 +++++++++++++++++++++-
 .../cli/commands/CreateRegionCommandTest.java      | 20 +++++
 .../cli/commands/CreateRegionSecurityTest.java     | 68 +++++++++++++++
 .../commands/DestroyRegionCommandDUnitTest.java    |  6 +-
 12 files changed, 328 insertions(+), 20 deletions(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/RegionShortcut.java 
b/geode-core/src/main/java/org/apache/geode/cache/RegionShortcut.java
index 6e77941..6ba796e 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/RegionShortcut.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/RegionShortcut.java
@@ -215,5 +215,29 @@ public enum RegionShortcut {
    * {@link DataPolicy} to {@link DataPolicy#EMPTY} and {@link Scope} to
    * {@link Scope#DISTRIBUTED_ACK}.
    */
-  REPLICATE_PROXY,
+  REPLICATE_PROXY;
+
+  public boolean isProxy() {
+    return name().contains("PROXY");
+  }
+
+  public boolean isLocal() {
+    return name().contains("LOCAL");
+  }
+
+  public boolean isPartition() {
+    return name().contains("PARTITION");
+  }
+
+  public boolean isReplicate() {
+    return name().contains("REPLICATE");
+  }
+
+  public boolean isPersistent() {
+    return name().contains("PERSISTENT");
+  }
+
+  public boolean isOverflow() {
+    return name().contains("OVERFLOW");
+  }
 }
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/DistributedRegionMXBean.java
 
b/geode-core/src/main/java/org/apache/geode/management/DistributedRegionMXBean.java
index f1b58cf..7a934e4 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/DistributedRegionMXBean.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/DistributedRegionMXBean.java
@@ -40,16 +40,20 @@ public interface DistributedRegionMXBean {
   /**
    * Returns the number of members hosting/using the Region.
    */
+  @ResourceOperation()
   int getMemberCount();
 
   /**
    * Returns a list of names/IDs of the members hosting the Region.
    */
+  @ResourceOperation()
   String[] getMembers();
 
   /**
    * Returns the type (data policy) of the Region.
+   * CreateRegionCommand will use this attribute
    */
+  @ResourceOperation()
   String getRegionType();
 
   /**
@@ -297,5 +301,6 @@ public interface DistributedRegionMXBean {
   /**
    * Returns the number of members whose entry count is 0.
    */
+  @ResourceOperation()
   int getEmptyNodes();
 }
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedRegionBridge.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedRegionBridge.java
index 6ff8f76..1893f79 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedRegionBridge.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/DistributedRegionBridge.java
@@ -330,7 +330,7 @@ public class DistributedRegionBridge {
 
   /**
    *
-   * @return type of the region
+   * @return data policy of the region
    */
   public String getRegionType() {
     return monitor.getRegionType();
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/RegionClusterStatsMonitor.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/RegionClusterStatsMonitor.java
index 12b69b4..0a6b06e 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/RegionClusterStatsMonitor.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/RegionClusterStatsMonitor.java
@@ -117,6 +117,7 @@ public class RegionClusterStatsMonitor {
 
   private String parentRegion;
 
+  // this is actually the data policy of the region
   private String regionType;
 
   private String fullPath;
@@ -436,6 +437,7 @@ public class RegionClusterStatsMonitor {
     return this.parentRegion;
   }
 
+  // returns the data policy of the region
   public String getRegionType() {
     return this.regionType;
   }
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 43869f5..c24a50e 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
@@ -15,6 +15,7 @@
 package org.apache.geode.management.internal.cli.commands;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -22,6 +23,7 @@ import java.util.stream.Collectors;
 
 import javax.management.ObjectName;
 
+import org.apache.commons.lang.StringUtils;
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
@@ -52,6 +54,7 @@ import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.LogWrapper;
 import org.apache.geode.management.internal.cli.domain.ClassName;
+import 
org.apache.geode.management.internal.cli.exceptions.EntityExistsException;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import 
org.apache.geode.management.internal.cli.functions.FetchRegionAttributesFunction;
 import 
org.apache.geode.management.internal.cli.functions.RegionAttributesWrapper;
@@ -193,7 +196,44 @@ public class CreateRegionCommand implements GfshCommand {
 
     InternalCache cache = getCache();
 
-    // validating the region path
+    // adding name collision check for regions created with regionShortcut only
+    DistributedRegionMXBean regionBean =
+        getManagementService().getDistributedRegionMXBean(regionPath);
+
+    if (regionBean != null && regionShortcut != null) {
+      // when creating a non-proxy region and there is already a non empty node
+      if (!regionShortcut.isProxy() && regionBean.getMemberCount() > 
regionBean.getEmptyNodes()) {
+        throw new EntityExistsException(
+            String.format("Region %s already exists on the cluster.", 
regionPath), ifNotExists);
+      }
+
+      // proxy regions can only be created on members not having this 
regionName already defined
+      if (regionShortcut.isProxy()) {
+        Set<String> membersWithThisRegion =
+            Arrays.stream(regionBean.getMembers()).collect(Collectors.toSet());
+        Set<String> membersWithinGroup = findMembers(groups, null).stream()
+            .map(DistributedMember::getName).collect(Collectors.toSet());
+        if (!Collections.disjoint(membersWithinGroup, membersWithThisRegion)) {
+          throw new EntityExistsException(String.format(
+              "Region %s already exists on these members: %s. You can only 
create "
+                  + "proxy regions with the same name on other members.",
+              regionPath, StringUtils.join(membersWithThisRegion, ",")), 
ifNotExists);
+        }
+      }
+
+      // then check if the existing region's data policy is compatible
+      if (regionShortcut.isPartition() && 
!regionBean.getRegionType().contains("PARTITION")) {
+        throw new EntityExistsException("The existing region is not a 
partitioned region",
+            ifNotExists);
+      }
+      if (regionShortcut.isReplicate() && 
!(regionBean.getRegionType().equals("EMPTY")
+          || regionBean.getRegionType().contains("REPLICATE"))) {
+        throw new EntityExistsException("The existing region is not a 
replicate region",
+            ifNotExists);
+      }
+    }
+
+    // validating the parent region
     RegionPath regionPathData = new RegionPath(regionPath);
     String parentRegionPath = regionPathData.getParent();
     if (parentRegionPath != null && 
!Region.SEPARATOR.equals(parentRegionPath)) {
@@ -367,7 +407,8 @@ public class CreateRegionCommand implements GfshCommand {
     }
 
     // additional authorization
-    if (isPersistentShortcut(functionArgs.getRegionShortcut())
+    if ((functionArgs.getRegionShortcut() != null
+        && functionArgs.getRegionShortcut().isPersistent())
         || isAttributePersistent(functionArgs.getRegionAttributes())) {
       getSecurityService().authorize(ResourcePermission.Resource.CLUSTER,
           ResourcePermission.Operation.WRITE, ResourcePermission.Target.DISK);
@@ -535,17 +576,6 @@ public class CreateRegionCommand implements GfshCommand {
     return false;
   }
 
-  private boolean isPersistentShortcut(RegionShortcut shortcut) {
-    return shortcut == RegionShortcut.LOCAL_PERSISTENT
-        || shortcut == RegionShortcut.LOCAL_PERSISTENT_OVERFLOW
-        || shortcut == RegionShortcut.PARTITION_PERSISTENT
-        || shortcut == RegionShortcut.PARTITION_PERSISTENT_OVERFLOW
-        || shortcut == RegionShortcut.PARTITION_REDUNDANT_PERSISTENT
-        || shortcut == RegionShortcut.PARTITION_REDUNDANT_PERSISTENT_OVERFLOW
-        || shortcut == RegionShortcut.REPLICATE_PERSISTENT
-        || shortcut == RegionShortcut.REPLICATE_PERSISTENT_OVERFLOW;
-  }
-
   private boolean isAttributePersistent(RegionAttributes attributes) {
     return attributes != null && attributes.getDataPolicy() != null
         && attributes.getDataPolicy().toString().contains("PERSISTENT");
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/EntityExistsException.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/EntityExistsException.java
new file mode 100644
index 0000000..49bcc05
--- /dev/null
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/EntityExistsException.java
@@ -0,0 +1,25 @@
+/*
+ * 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.exceptions;
+
+public class EntityExistsException extends EntityNotFoundException {
+
+  public EntityExistsException(String message, boolean statusOK) {
+    super(message, statusOK);
+  }
+}
diff --git 
a/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt
 
b/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt
index 599ad6f..ad79f57 100644
--- 
a/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt
+++ 
b/geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt
@@ -503,6 +503,7 @@ 
org/apache/geode/management/internal/cli/domain/RegionDescriptionPerMember,true,
 
org/apache/geode/management/internal/cli/domain/RegionInformation,true,1,dataPolicy:org/apache/geode/cache/DataPolicy,isRoot:boolean,name:java/lang/String,parentRegion:java/lang/String,path:java/lang/String,scope:org/apache/geode/cache/Scope,subRegionInformationSet:java/util/Set
 
org/apache/geode/management/internal/cli/domain/StackTracesPerMember,true,1,memberNameOrId:java/lang/String,stackTraces:byte[]
 
org/apache/geode/management/internal/cli/domain/SubscriptionQueueSizeResult,true,1,subscriptionQueueSize:long
+org/apache/geode/management/internal/cli/exceptions/EntityExistsException,false
 
org/apache/geode/management/internal/cli/exceptions/EntityNotFoundException,false,statusOK:boolean
 
org/apache/geode/management/internal/cli/functions/AlterRuntimeConfigFunction,true,1
 
org/apache/geode/management/internal/cli/functions/AsyncEventQueueFunctionArgs,true,-6524494645663740872,asyncEventQueueId:java/lang/String,batchSize:int,batchTimeInterval:int,diskStoreName:java/lang/String,diskSynchronous:boolean,dispatcherThreads:int,enableBatchConflation:boolean,forwardExpirationDestroy:boolean,gatewayEventFilters:java/lang/String[],gatewaySubstitutionFilter:java/lang/String,isParallel:boolean,listenerClassName:java/lang/String,listenerProperties:java/util/Properties,
 [...]
diff --git 
a/geode-core/src/test/java/org/apache/geode/cache/RegionShortcutTest.java 
b/geode-core/src/test/java/org/apache/geode/cache/RegionShortcutTest.java
new file mode 100644
index 0000000..bd6c7de
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/cache/RegionShortcutTest.java
@@ -0,0 +1,41 @@
+/*
+ * 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.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.test.junit.categories.UnitTest;
+
+
+@Category(UnitTest.class)
+public class RegionShortcutTest {
+
+  @Test
+  public void isPersistent() {
+    assertThat(RegionShortcut.LOCAL.isPersistent()).isFalse();
+    assertThat(RegionShortcut.LOCAL_HEAP_LRU.isPersistent()).isFalse();
+    assertThat(RegionShortcut.LOCAL_OVERFLOW.isPersistent()).isFalse();
+    assertThat(RegionShortcut.LOCAL_PERSISTENT.isPersistent()).isTrue();
+    
assertThat(RegionShortcut.PARTITION_PERSISTENT_OVERFLOW.isPersistent()).isTrue();
+    assertThat(RegionShortcut.PARTITION_HEAP_LRU.isPersistent()).isFalse();
+    assertThat(RegionShortcut.REPLICATE_PERSISTENT.isPersistent()).isTrue();
+  }
+}
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
index 02438c3..9f0b18f 100644
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
@@ -71,8 +71,8 @@ public class CreateRegionCommandDUnitTest {
   @BeforeClass
   public static void before() throws Exception {
     locator = lsRule.startLocatorVM(0);
-    server1 = lsRule.startServerVM(1, locator.getPort());
-    server2 = lsRule.startServerVM(2, locator.getPort());
+    server1 = lsRule.startServerVM(1, "group1", locator.getPort());
+    server2 = lsRule.startServerVM(2, "group2", locator.getPort());
 
     gfsh.connectAndVerify(locator);
   }
@@ -313,6 +313,98 @@ public class CreateRegionCommandDUnitTest {
     gfsh.executeAndAssertThat("destroy region 
--name=/TEMPLATE").statusIsSuccess();
   }
 
+  @Test
+  public void startWithNonProxyRegion() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat("create region --type=REPLICATE --group=group1 
--name=" + regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-1");
+    gfsh.executeAndAssertThat("create region --type=REPLICATE --group=group2 
--name=" + regionName)
+        .statusIsError().containsOutput("Region /" + regionName + " already 
exists on the cluster");
+
+    gfsh.executeAndAssertThat("create region --type=PARTITION --group=group2 
--name=" + regionName)
+        .statusIsError().containsOutput("Region /" + regionName + " already 
exists on the cluster");
+
+    gfsh.executeAndAssertThat(
+        "create region --type=REPLICATE_PROXY --group=group2 --name=" + 
regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-2");
+
+    locator.waitTillRegionsAreReadyOnServers("/" + regionName, 2);
+
+    gfsh.executeAndAssertThat(
+        "create region --type=PARTITION_PROXY --group=group2 --name=" + 
regionName).statusIsError()
+        .containsOutput("You can only create proxy regions with the same name 
on other members");
+  }
+
+  @Test
+  public void startWithReplicateProxyRegion() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat(
+        "create region --type=REPLICATE_PROXY --group=group1 --name=" + 
regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-1");
+    gfsh.executeAndAssertThat("create region --type=REPLICATE --group=group2 
--name=" + regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-2");
+
+    locator.waitTillRegionsAreReadyOnServers("/" + regionName, 2);
+    // the following two should fail with name check on locator, not on server
+    gfsh.executeAndAssertThat("create region --type=PARTITION --group=group2 
--name=" + regionName)
+        .statusIsError().containsOutput("Region /" + regionName + " already 
exists on the cluster");
+    gfsh.executeAndAssertThat(
+        "create region --type=PARTITION_PROXY --group=group2 --name=" + 
regionName).statusIsError()
+        .containsOutput("You can only create proxy regions with the same name 
on other members");
+  }
+
+  @Test
+  public void startWithReplicateProxyThenPartitionRegion() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat(
+        "create region --type=REPLICATE_PROXY --group=group1 --name=" + 
regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-1");
+    gfsh.executeAndAssertThat("create region --type=PARTITION --group=group2 
--name=" + regionName)
+        .statusIsError().containsOutput("The existing region is not a 
partitioned region");
+  }
+
+  @Test
+  public void startWithPartitionThenReplicateProxy() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat("create region --type=PARTITION --group=group1 
--name=" + regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-1");
+    gfsh.executeAndAssertThat(
+        "create region --type=REPLICATE_PROXY --group=group2 --name=" + 
regionName).statusIsError()
+        .containsOutput("The existing region is not a replicate region");
+  }
+
+  @Test
+  public void startWithPartitionProxyRegion() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat(
+        "create region --type=PARTITION_PROXY --group=group1 --name=" + 
regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-1");
+    gfsh.executeAndAssertThat("create region --type=PARTITION --group=group2 
--name=" + regionName)
+        .statusIsSuccess().tableHasRowWithValues("Member", "server-2");
+
+    locator.waitTillRegionsAreReadyOnServers("/" + regionName, 2);
+    // the following two should fail with name check on locator, not on server
+    gfsh.executeAndAssertThat("create region --type=PARTITION --group=group2 
--name=" + regionName)
+        .statusIsError().containsOutput("Region /" + regionName + " already 
exists on the cluster");
+    gfsh.executeAndAssertThat(
+        "create region --type=REPLICATE_PROXY --group=group2 --name=" + 
regionName).statusIsError()
+        .containsOutput("You can only create proxy regions with the same name 
on other members");
+  }
+
+  @Test
+  public void startWithLocalRegion() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat("create region --type=LOCAL --group=group1 
--name=" + regionName)
+        .statusIsSuccess();
+    gfsh.executeAndAssertThat("create region --type=REPLICATE --name=" + 
regionName)
+        .statusIsError();
+    gfsh.executeAndAssertThat("create region --type=PARTITION --group=group2 
--name=" + regionName)
+        .statusIsError();
+    gfsh.executeAndAssertThat(
+        "create region --type=PARTITION_PROXY --group=group2 --name=" + 
regionName).statusIsError()
+        .containsOutput("The existing region is not a partitioned region");
+  }
+
   private String getUniversalClassCode(String classname) {
     String code = "package io.pivotal;" + "import 
org.apache.geode.cache.CacheLoader;"
         + "import org.apache.geode.cache.CacheLoaderException;"
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
index fc52514..48f3859 100644
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
@@ -37,7 +37,9 @@ import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.DistributedRegionMXBean;
 import org.apache.geode.management.DistributedSystemMXBean;
+import org.apache.geode.management.ManagementService;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.GfshParseResult;
 import org.apache.geode.management.internal.cli.domain.ClassName;
@@ -54,6 +56,8 @@ public class CreateRegionCommandTest {
 
   private CreateRegionCommand command;
   private InternalCache cache;
+  private DistributedRegionMXBean regionMXBean;
+  ManagementService service;
 
   private static String COMMAND = "create region --name=region 
--type=REPLICATE ";
 
@@ -62,6 +66,11 @@ public class CreateRegionCommandTest {
     command = spy(CreateRegionCommand.class);
     cache = mock(InternalCache.class);
     doReturn(cache).when(command).getCache();
+
+    service = mock(ManagementService.class);
+    doReturn(service).when(command).getManagementService();
+    regionMXBean = mock(DistributedRegionMXBean.class);
+    when(service.getDistributedRegionMXBean(any())).thenReturn(regionMXBean);
   }
 
   @Test
@@ -152,6 +161,7 @@ public class CreateRegionCommandTest {
     
doReturn(Collections.singleton(mock(DistributedMember.class))).when(command).findMembers(any(),
         any());
     doReturn(true).when(command).verifyDistributedRegionMbean(any(), any());
+    when(service.getDistributedRegionMXBean(any())).thenReturn(null);
 
     parser.executeCommandWithInstance(command, "create region --name=A 
--type=REPLICATE");
     ArgumentCaptor<RegionFunctionArgs> argsCaptor =
@@ -372,4 +382,14 @@ public class CreateRegionCommandTest {
     parser.executeAndAssertThat(command, COMMAND + 
"--entry-idle-time-custom-expiry=abc")
         .statusIsError().containsOutput("Statistics must be enabled for 
expiration");
   }
+
+  @Test
+  public void nameCollisionCheck() {
+    when(regionMXBean.getMemberCount()).thenReturn(2);
+    when(regionMXBean.getEmptyNodes()).thenReturn(1);
+    parser.executeAndAssertThat(command, COMMAND).statusIsError()
+        .containsOutput("Region /region already exists on the cluster");
+    parser.executeAndAssertThat(command, COMMAND + " 
--if-not-exists").statusIsSuccess()
+        .containsOutput("Skipping: Region /region already exists on the 
cluster");
+  }
 }
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionSecurityTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionSecurityTest.java
new file mode 100644
index 0000000..c5b7c5c
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionSecurityTest.java
@@ -0,0 +1,68 @@
+/*
+ * 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.test.junit.rules.GfshCommandRule.PortType.jmxManager;
+
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestName;
+
+import org.apache.geode.security.SimpleTestSecurityManager;
+import org.apache.geode.test.junit.rules.ConnectionConfiguration;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+public class CreateRegionSecurityTest {
+  @ClassRule
+  public static ServerStarterRule server = new ServerStarterRule()
+      
.withSecurityManager(SimpleTestSecurityManager.class).withJMXManager().withAutoStart();
+
+  @Rule
+  public GfshCommandRule gfsh = new GfshCommandRule(server::getJmxPort, 
jmxManager);
+
+  @Rule
+  public TestName testName = new SerializableTestName();
+
+  @Test
+  @ConnectionConfiguration(user = "cluster", password = "cluster")
+  public void clusterNotAuthorized() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat("create region --type=REPLICATE --name=" + 
regionName).statusIsError()
+        .containsOutput("cluster not authorized for DATA:MANAGE");
+  }
+
+  // this test is to make sure that getting bean info to check name collision 
does not need
+  // further permission.
+  @Test
+  @ConnectionConfiguration(user = "dataManage", password = "dataManage")
+  public void dataManageAuthorzied() {
+    String regionName = testName.getMethodName();
+    gfsh.executeAndAssertThat("create region --type=REPLICATE --name=" + 
regionName)
+        .statusIsSuccess();
+
+    gfsh.executeAndAssertThat("create region --type=REPLICATE --name=" + 
regionName).statusIsError()
+        .containsOutput("Region /dataManageAuthorzied already exists on the 
cluster");
+
+    gfsh.executeAndAssertThat("create region --type=REPLICATE_PROXY --name=" + 
regionName)
+        .statusIsError()
+        .containsOutput("You can only create proxy regions with the same name 
on other members");
+  }
+}
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 9326850..98aa82f 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
@@ -104,7 +104,7 @@ public class DestroyRegionCommandDUnitTest {
 
   @Test
   public void testDestroyLocalAndDistributedRegions() {
-    gfsh.executeAndAssertThat("create region --name=region1 --type=LOCAL 
--group=group1")
+    gfsh.executeAndAssertThat("create region --name=region1 
--type=REPLICATE_PROXY --group=group1")
         .statusIsSuccess();
     gfsh.executeAndAssertThat("create region --name=region1 
--type=REPLICATE").statusIsSuccess();
 
@@ -114,8 +114,8 @@ public class DestroyRegionCommandDUnitTest {
       ClusterConfigurationService service =
           ClusterStartupRule.getLocator().getSharedConfiguration();
       Configuration group1Config = service.getConfiguration("group1");
-      assertThat(group1Config.getCacheXmlContent())
-          .contains("<region name=\"region1\">\n" + "    <region-attributes 
scope=\"local\"/>");
+      assertThat(group1Config.getCacheXmlContent()).contains("<region 
name=\"region1\">\n"
+          + "    <region-attributes data-policy=\"empty\" 
scope=\"distributed-ack\"/>");
 
       Configuration clusterConfig = service.getConfiguration("cluster");
       assertThat(clusterConfig.getCacheXmlContent()).contains("<region 
name=\"region1\">\n"

-- 
To stop receiving notification emails like this one, please contact
jinmeil...@apache.org.

Reply via email to