You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by on...@apache.org on 2019/04/05 03:38:39 UTC

[geode] branch develop updated: GEODE-6560 - Geode Management REST supports creating regions by group (#3400)

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

onichols 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 0d31a85  GEODE-6560 - Geode Management REST supports creating regions by group (#3400)
0d31a85 is described below

commit 0d31a859219f118e671f3bdbf040d5657725725a
Author: Owen Nichols <34...@users.noreply.github.com>
AuthorDate: Thu Apr 4 20:38:24 2019 -0700

    GEODE-6560 - Geode Management REST supports creating regions by group (#3400)
    
    Co-authored-by: Owen Nichols <on...@pivotal.io>
---
 .../ClientClusterManagementServiceDunitTest.java   | 52 +++++++++++++++++++---
 .../api/LocatorClusterManagementService.java       | 24 +++++++---
 .../api/LocatorClusterManagementServiceTest.java   |  7 ++-
 .../internal/ClientClusterManagementService.java   |  5 ++-
 .../rest/RegionManagementIntegrationTest.java      |  2 +-
 .../RegionManagementSecurityIntegrationTest.java   |  2 +-
 .../controllers/RegionManagementController.java    |  6 ++-
 7 files changed, 76 insertions(+), 22 deletions(-)

diff --git a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClientClusterManagementServiceDunitTest.java b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClientClusterManagementServiceDunitTest.java
index 3af97a4..e8eb0d5 100644
--- a/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClientClusterManagementServiceDunitTest.java
+++ b/geode-assembly/src/distributedTest/java/org/apache/geode/management/internal/rest/ClientClusterManagementServiceDunitTest.java
@@ -7,7 +7,10 @@ import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
 
+import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.cache.configuration.CacheElement;
 import org.apache.geode.cache.configuration.RegionConfig;
+import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
 import org.apache.geode.management.api.ClusterManagementResult;
 import org.apache.geode.management.api.ClusterManagementService;
 import org.apache.geode.management.client.ClusterManagementServiceProvider;
@@ -32,16 +35,19 @@ import org.apache.geode.test.dunit.rules.MemberVM;
 
 public class ClientClusterManagementServiceDunitTest {
   @ClassRule
-  public static ClusterStartupRule cluster = new ClusterStartupRule(2);
+  public static ClusterStartupRule cluster = new ClusterStartupRule(4);
 
-  private static MemberVM locator, server;
+  private static MemberVM locator, server, serverWithGroupA;
   private static ClientVM client;
+
+  private static String groupA = "group-a";
   private static ClusterManagementService cmsClient;
 
   @BeforeClass
   public static void beforeClass() {
     locator = cluster.startLocatorVM(0, l -> l.withHttpService());
     server = cluster.startServerVM(1, locator.getPort());
+    serverWithGroupA = cluster.startServerVM(2, groupA, locator.getPort());
     cmsClient = ClusterManagementServiceProvider.getService("localhost", locator.getHttpPort());
   }
 
@@ -54,7 +60,7 @@ public class ClientClusterManagementServiceDunitTest {
 
     assertThat(result.isSuccessful()).isTrue();
     assertThat(result.getStatusCode()).isEqualTo(ClusterManagementResult.StatusCode.OK);
-    assertThat(result.getMemberStatuses()).containsKeys("server-1").hasSize(1);
+    assertThat(result.getMemberStatuses()).containsOnlyKeys("server-1", "server-2");
 
     result = cmsClient.create(region, "");
     assertThat(result.isSuccessful()).isFalse();
@@ -62,6 +68,19 @@ public class ClientClusterManagementServiceDunitTest {
   }
 
   @Test
+  public void createRegionWithNullGroup() {
+    RegionConfig region = new RegionConfig();
+    region.setName("orders");
+
+    ClusterManagementResult result = cmsClient.create(region, null);
+
+    assertThat(result.isSuccessful()).isTrue();
+    assertThat(result.getStatusCode()).isEqualTo(ClusterManagementResult.StatusCode.OK);
+    assertThat(result.getMemberStatuses()).containsOnlyKeys("server-1", "server-2");
+  }
+
+
+  @Test
   public void createRegionWithInvalidName() throws Exception {
     RegionConfig region = new RegionConfig();
     region.setName("__test");
@@ -73,9 +92,32 @@ public class ClientClusterManagementServiceDunitTest {
   }
 
   @Test
+  public void createRegionWithGroup() {
+    RegionConfig region = new RegionConfig();
+    region.setName("company");
+
+    ClusterManagementResult result = cmsClient.create(region, groupA);
+
+    assertThat(result.isSuccessful()).isTrue();
+    assertThat(result.getStatusCode()).isEqualTo(ClusterManagementResult.StatusCode.OK);
+
+    // server 1 should not be in the set
+    assertThat(result.getMemberStatuses()).containsOnlyKeys("server-2");
+
+    locator.invoke(() -> {
+      InternalConfigurationPersistenceService persistenceService =
+          ClusterStartupRule.getLocator().getConfigurationPersistenceService();
+      CacheConfig clusterCacheConfig = persistenceService.getCacheConfig("cluster", true);
+      CacheConfig groupACacheConfig = persistenceService.getCacheConfig("group-a");
+      assertThat(CacheElement.findElement(clusterCacheConfig.getRegions(), "company")).isNull();
+      assertThat(CacheElement.findElement(groupACacheConfig.getRegions(), "company")).isNotNull();
+    });
+  }
+
+  @Test
   public void invokeFromClientCacheWithLocatorPool() throws Exception {
     int locatorPort = locator.getPort();
-    client = cluster.startClientVM(2, c -> c.withLocatorConnection(locatorPort));
+    client = cluster.startClientVM(3, c -> c.withLocatorConnection(locatorPort));
 
     client.invoke(() -> {
       ClusterManagementService service = ClusterManagementServiceProvider.getService();
@@ -87,7 +129,7 @@ public class ClientClusterManagementServiceDunitTest {
   @Test
   public void invokeFromClientCacheWithServerPool() throws Exception {
     int serverPort = server.getPort();
-    client = cluster.startClientVM(2, c -> c.withServerConnection(serverPort));
+    client = cluster.startClientVM(3, c -> c.withServerConnection(serverPort));
 
     client.invoke(() -> {
       assertThatThrownBy(() -> ClusterManagementServiceProvider.getService())
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
index 511cc3f..c8997b6 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/api/LocatorClusterManagementService.java
@@ -21,8 +21,11 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.apache.commons.lang3.NotImplementedException;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.annotations.VisibleForTesting;
@@ -77,7 +80,7 @@ public class LocatorClusterManagementService implements ClusterManagementService
 
   @Override
   public ClusterManagementResult create(CacheElement config, String group) {
-    if (group == null) {
+    if (StringUtils.isBlank(group)) {
       group = "cluster";
     }
 
@@ -94,7 +97,6 @@ public class LocatorClusterManagementService implements ClusterManagementService
       validator.validate(config);
     }
 
-
     // exit early if config element already exists in cache config
     CacheConfig currentPersistedConfig = persistenceService.getCacheConfig(group, true);
     if (validator.exists(config, currentPersistedConfig)) {
@@ -102,9 +104,11 @@ public class LocatorClusterManagementService implements ClusterManagementService
     }
 
     // execute function on all members
-    Set<DistributedMember> targetedMembers = findMembers(null, null);
+    Set<DistributedMember> targetedMembers = findMembers(group);
+
     if (targetedMembers.size() == 0) {
-      return new ClusterManagementResult(false, "no members found to create cache element");
+      return new ClusterManagementResult(false,
+          "no members found in " + group + " to create cache element");
     }
 
     List<CliFunctionResult> functionResults = executeAndGetFunctionResult(
@@ -123,7 +127,7 @@ public class LocatorClusterManagementService implements ClusterManagementService
     }
 
     // persist configuration in cache config
-    String finalGroup = group;
+    final String finalGroup = group; // the below lambda requires a reference that is final
     persistenceService.updateCacheConfig(finalGroup, cacheConfigForGroup -> {
       try {
         configurationMutator.add(config, cacheConfigForGroup);
@@ -167,8 +171,14 @@ public class LocatorClusterManagementService implements ClusterManagementService
   }
 
   @VisibleForTesting
-  Set<DistributedMember> findMembers(String[] groups, String[] members) {
-    return CliUtil.findMembers(groups, members, cache);
+  Set<DistributedMember> findMembers(String group) {
+    Stream<DistributedMember> stream =
+        cache.getDistributionManager().getNormalDistributionManagerIds()
+            .stream().map(DistributedMember.class::cast);
+    if (!"cluster".equals(group)) {
+      stream = stream.filter(m -> m.getGroups().contains(group));
+    }
+    return stream.collect(Collectors.toSet());
   }
 
   @VisibleForTesting
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
index 2aaf71e..fdcde89 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/api/LocatorClusterManagementServiceTest.java
@@ -87,11 +87,11 @@ public class LocatorClusterManagementServiceTest {
   public void noMemberFound() throws Exception {
     regionConfig.setName("test");
     when(persistenceService.getCacheConfig("cluster", true)).thenReturn(new CacheConfig());
-    doReturn(Collections.emptySet()).when(service).findMembers(any(), any());
+    doReturn(Collections.emptySet()).when(service).findMembers(any());
     result = service.create(regionConfig, "cluster");
     assertThat(result.isSuccessful()).isFalse();
     assertThat(result.getStatusMessage())
-        .contains("no members found to create cache element");
+        .contains("no members found in cluster to create cache element");
   }
 
   @Test
@@ -101,8 +101,7 @@ public class LocatorClusterManagementServiceTest {
     functionResults.add(new CliFunctionResult("member2", false, "failed"));
     doReturn(functionResults).when(service).executeAndGetFunctionResult(any(), any(), any());
 
-    doReturn(Collections.singleton(mock(DistributedMember.class))).when(service).findMembers(any(),
-        any());
+    doReturn(Collections.singleton(mock(DistributedMember.class))).when(service).findMembers(any());
 
     when(persistenceService.getCacheConfig("cluster", true)).thenReturn(new CacheConfig());
     regionConfig.setName("test");
diff --git a/geode-management/src/main/java/org/apache/geode/management/internal/ClientClusterManagementService.java b/geode-management/src/main/java/org/apache/geode/management/internal/ClientClusterManagementService.java
index 26592f0..0fab185 100644
--- a/geode-management/src/main/java/org/apache/geode/management/internal/ClientClusterManagementService.java
+++ b/geode-management/src/main/java/org/apache/geode/management/internal/ClientClusterManagementService.java
@@ -107,9 +107,10 @@ public class ClientClusterManagementService implements ClusterManagementService
 
   @Override
   public ClusterManagementResult create(CacheElement config, String group) {
-    String endPoint = getEndpoint(config);
+    String endPoint = getEndpoint(config) + "?group={group}";
     // the response status code info is represented by the ClusterManagementResult.errorCode already
-    return restTemplate.postForEntity(VERSION + endPoint, config, ClusterManagementResult.class)
+    return restTemplate
+        .postForEntity(VERSION + endPoint, config, ClusterManagementResult.class, group)
         .getBody();
   }
 
diff --git a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementIntegrationTest.java b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementIntegrationTest.java
index 01788c6..07de9c2 100644
--- a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementIntegrationTest.java
+++ b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementIntegrationTest.java
@@ -67,7 +67,7 @@ public class RegionManagementIntegrationTest {
         .andExpect(status().isInternalServerError())
         .andExpect(jsonPath("$.statusCode", is("ERROR")))
         .andExpect(jsonPath("$.statusMessage",
-            is("no members found to create cache element")));
+            is("no members found in cluster to create cache element")));
   }
 
   @Test
diff --git a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementSecurityIntegrationTest.java b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementSecurityIntegrationTest.java
index ac7991a..1dc1a81 100644
--- a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementSecurityIntegrationTest.java
+++ b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/RegionManagementSecurityIntegrationTest.java
@@ -99,7 +99,7 @@ public class RegionManagementSecurityIntegrationTest {
         .andExpect(status().isInternalServerError())
         .andExpect(jsonPath("$.statusCode", is("ERROR")))
         .andExpect(jsonPath("$.statusMessage",
-            is("no members found to create cache element")));
+            is("no members found in cluster to create cache element")));
   }
 
 }
diff --git a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/RegionManagementController.java b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/RegionManagementController.java
index 45333b2..aec585c 100644
--- a/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/RegionManagementController.java
+++ b/geode-web-management/src/main/java/org/apache/geode/management/internal/rest/controllers/RegionManagementController.java
@@ -25,6 +25,7 @@ import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestParam;
 
 import org.apache.geode.cache.configuration.RegionConfig;
 import org.apache.geode.management.api.ClusterManagementResult;
@@ -36,9 +37,10 @@ public class RegionManagementController extends AbstractManagementController {
   @PreAuthorize("@securityService.authorize('DATA', 'MANAGE')")
   @RequestMapping(method = RequestMethod.POST, value = REGION_CONFIG_ENDPOINT)
   public ResponseEntity<ClusterManagementResult> createRegion(
-      @RequestBody RegionConfig regionConfig) {
+      @RequestBody RegionConfig regionConfig,
+      @RequestParam(required = false) String group) {
     ClusterManagementResult result =
-        clusterManagementService.create(regionConfig, "cluster");
+        clusterManagementService.create(regionConfig, group);
     return new ResponseEntity<>(result,
         result.isSuccessful() ? HttpStatus.CREATED : HttpStatus.INTERNAL_SERVER_ERROR);
   }