GEODE-6306: add capability to check cache element compatibility (apa…

…che#3695)

geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementDunitTest.java

@@ -15,12 +15,12 @@
 
 package org.apache.geode.management.internal.rest;
 
+import static org.apache.geode.test.junit.assertions.ClusterManagementResultAssert.assertManagementResult;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.util.List;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
@@ -33,79 +33,70 @@
 import org.apache.geode.cache.configuration.RegionConfig;
 import org.apache.geode.cache.configuration.RegionType;
 import org.apache.geode.management.api.ClusterManagementResult;
+import org.apache.geode.management.api.ClusterManagementService;
+import org.apache.geode.management.client.ClusterManagementServiceBuilder;
 import org.apache.geode.management.configuration.RuntimeRegionConfig;
 import org.apache.geode.test.dunit.IgnoredException;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 import org.apache.geode.test.dunit.rules.MemberVM;
 import org.apache.geode.test.junit.rules.GeodeDevRestClient;
-import org.apache.geode.util.internal.GeodeJsonMapper;
 
 public class RegionManagementDunitTest {
 
   @ClassRule
   public static ClusterStartupRule cluster = new ClusterStartupRule();
 
-  private static MemberVM locator, server;
+  private static MemberVM locator, server1, server2, server3;
 
   private static GeodeDevRestClient restClient;
+  private static ClusterManagementService cms;
 
   @BeforeClass
   public static void beforeClass() throws Exception {
     locator = cluster.startLocatorVM(0, l -> l.withHttpService());
-    server = cluster.startServerVM(1, locator.getPort());
+    server1 = cluster.startServerVM(1, "group1", locator.getPort());
+    server2 = cluster.startServerVM(2, "group2", locator.getPort());
+    server3 = cluster.startServerVM(3, "group2,group3", locator.getPort());
+
     restClient =
         new GeodeDevRestClient("/geode-management/v2", "localhost", locator.getHttpPort(), false);
+    cms = ClusterManagementServiceBuilder.buildWithHostAddress()
+        .setHostAddress("localhost", locator.getHttpPort())
+        .build();
   }
 
   @Test
   public void createsRegion() throws Exception {
     RegionConfig regionConfig = new RegionConfig();
     regionConfig.setName("customers");
+    regionConfig.setGroup("group1");
     regionConfig.setType(RegionType.REPLICATE);
-    ObjectMapper mapper = new ObjectMapper();
-    String json = mapper.writeValueAsString(regionConfig);
 
-    ClusterManagementResult result =
-        restClient.doPostAndAssert("/regions", json)
-            .hasStatusCode(201)
-            .getClusterManagementResult();
+    ClusterManagementResult result = cms.create(regionConfig);
 
     assertThat(result.isSuccessful()).isTrue();
     assertThat(result.getMemberStatuses()).containsKeys("server-1").hasSize(1);
 
     // make sure region is created
-    server.invoke(() -> verifyRegionCreated("customers", "REPLICATE"));
+    server1.invoke(() -> verifyRegionCreated("customers", "REPLICATE"));
 
     // make sure region is persisted
-    locator.invoke(() -> verifyRegionPersisted("customers", "REPLICATE"));
-
-    // verify that additional server can be started with the cluster configuration
-    MemberVM server2 = cluster.startServerVM(2, locator.getPort());
-    server2.invoke(() -> verifyRegionCreated("customers", "REPLICATE"));
-
-    // stop the 2nd server to avoid test pollution
-    server2.stop();
+    locator.invoke(() -> verifyRegionPersisted("customers", "REPLICATE", "group1"));
   }
 
   @Test
   public void createRegionWithKeyValueConstraint() throws Exception {
     RegionConfig config = new RegionConfig();
     config.setName("customers2");
+    config.setGroup("group1");
     config.setType(RegionType.PARTITION);
     RegionAttributesType type = new RegionAttributesType();
     type.setKeyConstraint("java.lang.Boolean");
     type.setValueConstraint("java.lang.Integer");
     config.setRegionAttributes(type);
+    cms.create(config);
 
-    restClient.doPostAndAssert("/regions", GeodeJsonMapper.getMapper().writeValueAsString(config))
-        .statusIsOk();
-
-    locator.waitUntilRegionIsReadyOnExactlyThisManyServers("/customers2", 1);
-
-    List<RuntimeRegionConfig> result =
-        restClient.doGetAndAssert("/regions?id=customers2").statusIsOk()
-            .getClusterManagementResult().getResult(
-                RuntimeRegionConfig.class);
+    List<RuntimeRegionConfig> result = cms.get(config).getResult(RuntimeRegionConfig.class);
 
     assertThat(result).hasSize(1);
     RuntimeRegionConfig config1 = result.get(0);
@@ -114,7 +105,7 @@ public void createRegionWithKeyValueConstraint() throws Exception {
     assertThat(config1.getRegionAttributes().getValueConstraint()).isEqualTo("java.lang.Integer");
     assertThat(config1.getRegionAttributes().getKeyConstraint()).isEqualTo("java.lang.Boolean");
 
-    server.invoke(() -> {
+    server1.invoke(() -> {
       Region customers2 = ClusterStartupRule.getCache().getRegionByPath("/customers2");
       assertThatThrownBy(() -> customers2.put("key", 2)).isInstanceOf(ClassCastException.class)
           .hasMessageContaining("does not satisfy keyConstraint");
@@ -126,7 +117,7 @@ public void createRegionWithKeyValueConstraint() throws Exception {
 
   @Test
   public void createsAPartitionedRegion() throws Exception {
-    String json = "{\"name\": \"orders\", \"type\": \"PARTITION\"}";
+    String json = "{\"name\": \"orders\", \"type\": \"PARTITION\", \"group\": \"group1\"}";
 
     ClusterManagementResult result = restClient.doPostAndAssert("/regions", json)
         .hasStatusCode(201)
@@ -136,10 +127,10 @@ public void createsAPartitionedRegion() throws Exception {
     assertThat(result.getMemberStatuses()).containsKeys("server-1").hasSize(1);
 
     // make sure region is created
-    server.invoke(() -> verifyRegionCreated("orders", "PARTITION"));
+    server1.invoke(() -> verifyRegionCreated("orders", "PARTITION"));
 
     // make sure region is persisted
-    locator.invoke(() -> verifyRegionPersisted("orders", "PARTITION"));
+    locator.invoke(() -> verifyRegionPersisted("orders", "PARTITION", "group1"));
 
     // create the same region 2nd time
     result = restClient.doPostAndAssert("/regions", json)
@@ -160,10 +151,10 @@ public void noNameInConfig() throws Exception {
     assertThat(result.isSuccessful()).isFalse();
   }
 
-  static void verifyRegionPersisted(String regionName, String type) {
+  static void verifyRegionPersisted(String regionName, String type, String group) {
     CacheConfig cacheConfig =
         ClusterStartupRule.getLocator().getConfigurationPersistenceService()
-            .getCacheConfig("cluster");
+            .getCacheConfig(group);
     RegionConfig regionConfig = CacheElement.findElement(cacheConfig.getRegions(), regionName);
     assertThat(regionConfig.getType()).isEqualTo(type);
   }
@@ -174,4 +165,59 @@ static void verifyRegionCreated(String regionName, String type) {
     assertThat(region).isNotNull();
     assertThat(region.getAttributes().getDataPolicy().toString()).isEqualTo(type);
   }
+
+  @Test
+  public void createSameRegionOnDisjointGroups() throws Exception {
+    RegionConfig regionConfig = new RegionConfig();
+    regionConfig.setName("disJoint");
+    regionConfig.setGroup("group1");
+    regionConfig.setType(RegionType.REPLICATE);
+    assertManagementResult(cms.create(regionConfig)).isSuccessful();
+
+    regionConfig.setName("disJoint");
+    regionConfig.setGroup("group2");
+    regionConfig.setType(RegionType.REPLICATE);
+    assertManagementResult(cms.create(regionConfig)).isSuccessful();
+  }
+
+  @Test
+  public void createSameRegionOnGroupsWithCommonMember() throws Exception {
+    RegionConfig regionConfig = new RegionConfig();
+    regionConfig.setName("commonMember");
+    regionConfig.setGroup("group2");
+    regionConfig.setType(RegionType.REPLICATE);
+    assertManagementResult(cms.create(regionConfig)).isSuccessful();
+
+    assertManagementResult(cms.create(regionConfig)).failed().hasStatusCode(
+        ClusterManagementResult.StatusCode.ENTITY_EXISTS)
+        .containsStatusMessage("server-2")
+        .containsStatusMessage("server-3")
+        .containsStatusMessage("already has this element created");
+
+    regionConfig.setGroup("group3");
+    assertManagementResult(cms.create(regionConfig)).failed().hasStatusCode(
+        ClusterManagementResult.StatusCode.ENTITY_EXISTS)
+        .containsStatusMessage("Member(s) server-3 already has this element created");
+  }
+
+  @Test
+  public void createIncompatibleRegionOnDisjointGroups() throws Exception {
+    RegionConfig regionConfig = new RegionConfig();
+    regionConfig.setName("incompatible");
+    regionConfig.setGroup("group4");
+    regionConfig.setType(RegionType.REPLICATE);
+    assertManagementResult(cms.create(regionConfig)).isSuccessful();
+
+    regionConfig.setName("incompatible");
+    regionConfig.setGroup("group5");
+    regionConfig.setType(RegionType.PARTITION);
+    assertManagementResult(cms.create(regionConfig)).failed().hasStatusCode(
+        ClusterManagementResult.StatusCode.ILLEGAL_ARGUMENT);
+
+    regionConfig.setName("incompatible");
+    regionConfig.setGroup("group5");
+    regionConfig.setType(RegionType.REPLICATE_PROXY);
+    assertManagementResult(cms.create(regionConfig)).isSuccessful();
+
+  }
 }

geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/RegionManagementRestSecurityDUnitTest.java

@@ -116,7 +116,8 @@ public void createRegionWithCredentials_CreatesRegion() throws Exception {
     server.invoke(() -> RegionManagementDunitTest.verifyRegionCreated("customers", "REPLICATE"));
 
     // make sure region is persisted
-    locator.invoke(() -> RegionManagementDunitTest.verifyRegionPersisted("customers", "REPLICATE"));
+    locator.invoke(() -> RegionManagementDunitTest.verifyRegionPersisted("customers", "REPLICATE",
+        "cluster"));
 
     // verify that additional server can be started with the cluster configuration
     cluster.startServerVM(2, config, locator.getPort());

geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java

@@ -179,7 +179,7 @@ public ClusterManagementResult delete(CacheElement config) {
     }
 
     String[] groupsWithThisElement =
-        memberValidator.findGroupsWithThisElement(config, configurationManager);
+        memberValidator.findGroupsWithThisElement(config.getId(), configurationManager);
     if (groupsWithThisElement.length == 0) {
       throw new EntityNotFoundException("Cache element '" + config.getId() + "' does not exist");
     }

geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/ConfigurationManager.java

@@ -31,6 +31,10 @@
  */
 @Experimental
 public interface ConfigurationManager<T extends CacheElement> {
+  /**
+   * specify how to add the config to the existing cache config. Note at this point, the config
+   * should have passed all the validations already.
+   */
   void add(T config, CacheConfig existing);
 
   void update(T config, CacheConfig existing);
@@ -40,4 +44,15 @@ public interface ConfigurationManager<T extends CacheElement> {
   List<? extends T> list(T filterConfig, CacheConfig existing);
 
   T get(String id, CacheConfig existing);
+
+  /**
+   * @param incoming the one that's about to be persisted
+   * @param group the group name of the existing cache element
+   * @param existing the existing cache element on another group
+   * @throws IllegalArgumentException if the incoming CacheElement is not compatible with the
+   *         existing
+   *
+   *         Note: incoming and exiting should have the same ID already
+   */
+  default void checkCompatibility(T incoming, String group, T existing) {};
 }

geode-core/src/main/java/org/apache/geode/management/internal/configuration/mutators/RegionConfigManager.java

@@ -93,4 +93,32 @@ public List<RuntimeRegionConfig> list(RegionConfig filter, CacheConfig existing)
   public RegionConfig get(String id, CacheConfig existing) {
     return CacheElement.findElement(existing.getRegions(), id);
   }
+
+  @Override
+  public void checkCompatibility(RegionConfig incoming, String group, RegionConfig existing) {
+    // if their types are the same, then they are compatible
+    if (incoming.getType().equals(existing.getType())) {
+      return;
+    }
+
+    // one has to be the proxy of the other's main type
+    if (!incoming.getType().contains("PROXY") && !existing.getType().contains("PROXY")) {
+      throw new IllegalArgumentException(getDescription(incoming) + " is not compatible with "
+          + group + "'s existing regionConfig "
+          + getDescription(existing));
+    }
+
+    // the beginning part of the type has to be the same
+    String incomingType = incoming.getType().split("_")[0];
+    String existingType = existing.getType().split("_")[0];
+    if (!incomingType.equals(existingType)) {
+      throw new IllegalArgumentException(
+          getDescription(incoming) + " is not compatible with " + group + "'s existing "
+              + getDescription(existing));
+    }
+  }
+
+  private String getDescription(RegionConfig regionConfig) {
+    return "Region " + regionConfig.getName() + " of type " + regionConfig.getType();
+  }
 }

geode-core/src/main/java/org/apache/geode/management/internal/configuration/validators/MemberValidator.java

@@ -15,11 +15,11 @@
 
 package org.apache.geode.management.internal.configuration.validators;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -47,12 +47,14 @@ public MemberValidator(InternalCache cache, ConfigurationPersistenceService pers
 
   public void validateCreate(CacheElement config, ConfigurationManager manager) {
 
-    String[] groupWithThisElement = findGroupsWithThisElement(config, manager);
-    if (groupWithThisElement.length == 0) {
+    Map<String, CacheElement> existingElementsAndTheirGroups =
+        findCacheElement(config.getId(), manager);
+    if (existingElementsAndTheirGroups.size() == 0) {
       return;
     }
 
-    Set<DistributedMember> membersOfExistingGroups = findMembers(groupWithThisElement);
+    Set<DistributedMember> membersOfExistingGroups =
+        findMembers(existingElementsAndTheirGroups.keySet().toArray(new String[0]));
     Set<DistributedMember> membersOfNewGroup = findMembers(config.getConfigGroup());
     Collection<DistributedMember> intersection =
         CollectionUtils.intersection(membersOfExistingGroups, membersOfNewGroup);
@@ -62,18 +64,34 @@ public void validateCreate(CacheElement config, ConfigurationManager manager) {
       throw new EntityExistsException(
           "Member(s) " + members + " already has this element created.");
     }
+
+    // if there is no common member, we still need to verify if the new config is compatible with
+    // the existing ones.
+    for (Map.Entry<String, CacheElement> existing : existingElementsAndTheirGroups.entrySet()) {
+      manager.checkCompatibility(config, existing.getKey(), existing.getValue());
+    }
+  }
+
+  public String[] findGroupsWithThisElement(String id, ConfigurationManager manager) {
+    return findCacheElement(id, manager).keySet().toArray(new String[0]);
   }
 
-  public String[] findGroupsWithThisElement(CacheElement config, ConfigurationManager manager) {
-    // if the same element exists in some groups already, make sure the groups has no common members
-    List<String> groupWithThisElement = new ArrayList<>();
+  /**
+   * this returns a map of CacheElement with this id, with the group as the key of the map
+   */
+  public Map<String, CacheElement> findCacheElement(String id, ConfigurationManager manager) {
+    Map<String, CacheElement> results = new HashMap<>();
     for (String group : persistenceService.getGroups()) {
       CacheConfig cacheConfig = persistenceService.getCacheConfig(group);
-      if (cacheConfig != null && manager.get(config.getId(), cacheConfig) != null) {
-        groupWithThisElement.add(group);
+      if (cacheConfig == null) {
+        continue;
+      }
+      CacheElement existing = manager.get(id, cacheConfig);
+      if (existing != null) {
+        results.put(group, existing);
       }
     }
-    return groupWithThisElement.toArray(new String[0]);
+    return results;
   }
 
   /**

geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java

@@ -129,13 +129,14 @@ public void create_validatorIsCalledCorrectly() throws Exception {
   @Test
   public void delete_validatorIsCalledCorrectly() throws Exception {
     doReturn(Collections.emptySet()).when(memberValidator).findMembers(anyString());
-    doReturn(new String[] {"cluster"}).when(memberValidator).findGroupsWithThisElement(regionConfig,
+    doReturn(new String[] {"cluster"}).when(memberValidator).findGroupsWithThisElement(
+        regionConfig.getId(),
         regionManager);
     doNothing().when(persistenceService).updateCacheConfig(any(), any());
     service.delete(regionConfig);
     verify(cacheElementValidator).validate(CacheElementOperation.DELETE, regionConfig);
     verify(regionValidator).validate(CacheElementOperation.DELETE, regionConfig);
-    verify(memberValidator).findGroupsWithThisElement(regionConfig, regionManager);
+    verify(memberValidator).findGroupsWithThisElement(regionConfig.getId(), regionManager);
     verify(memberValidator).findMembers("cluster");
   }