You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jc...@apache.org on 2019/01/23 20:18:32 UTC

[geode] branch feature/GEODE-6273 updated: Added Partitioned group test to CreateMappingDUnitTest

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

jchen21 pushed a commit to branch feature/GEODE-6273
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-6273 by this push:
     new 742a9e5  Added Partitioned group test to CreateMappingDUnitTest
742a9e5 is described below

commit 742a9e5afafe7d80056711f3ec689b0cf8f9c205
Author: Ben Ross <br...@pivotal.io>
AuthorDate: Wed Jan 23 12:15:43 2019 -0800

    Added Partitioned group test to CreateMappingDUnitTest
    
    - Added additional servers to CreateMappingDUnitTest
    - Removed unnessessary code from CliUtil, GfshCommand, and
    CommandExecutor
    
    Co-authored-by: Ben Ross <br...@pivotal.io>
    Co-authored-by: Jianxia Chen <jc...@pivotal.io>
---
 .../cli/CreateMappingCommandDUnitTest.java         | 118 +++++++++++++++++----
 .../jdbc/internal/cli/CreateMappingCommand.java    |   4 +-
 .../apache/geode/management/cli/GfshCommand.java   |   4 -
 .../geode/management/internal/cli/CliUtil.java     |  36 ++++---
 .../internal/cli/remote/CommandExecutor.java       |   7 +-
 5 files changed, 123 insertions(+), 46 deletions(-)

diff --git a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
index 1cee130..25f25c2 100644
--- a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
+++ b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommandDUnitTest.java
@@ -60,8 +60,10 @@ import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
 public class CreateMappingCommandDUnitTest {
 
   private static final String TEST_REGION = "testRegion";
+  private static final String GROUP1_REGION = "group1Region";
+  private static final String GROUP2_REGION = "group2Region";
   private static final String TEST_GROUP1 = "testGroup1";
-  // private static final String TEST_GROUP2 = "testGroup2";
+  private static final String TEST_GROUP2 = "testGroup2";
 
   @Rule
   public transient GfshCommandRule gfsh = new GfshCommandRule();
@@ -75,15 +77,16 @@ public class CreateMappingCommandDUnitTest {
   private MemberVM locator;
   private MemberVM server1;
   private MemberVM server2;
+  private MemberVM server3;
+  private MemberVM server4;
 
   @Before
   public void before() throws Exception {
     locator = startupRule.startLocatorVM(0);
     server1 = startupRule.startServerVM(1, locator.getPort());
     server2 = startupRule.startServerVM(2, TEST_GROUP1, locator.getPort());
-    // TODO:
-    // create a TEST_GROUP2 to contain server3
-    // create a TEST_GROUP3 to contain both server2 and server3
+    server3 = startupRule.startServerVM(3, TEST_GROUP2, locator.getPort());
+    server4 = startupRule.startServerVM(4, TEST_GROUP1 + "," + TEST_GROUP2, locator.getPort());
 
     gfsh.connectAndVerify(locator);
 
@@ -94,6 +97,7 @@ public class CreateMappingCommandDUnitTest {
     startupRule.stop(1);
     startupRule.stop(2);
     startupRule.stop(3);
+    startupRule.stop(4);
     gfsh.disconnect();
   }
 
@@ -110,9 +114,9 @@ public class CreateMappingCommandDUnitTest {
   }
 
   // TODO: might need a parameter for server group names
-  private void setupGroupReplicate(String regionName) {
+  private void setupGroupReplicate(String regionName, String groupNames) {
     gfsh.executeAndAssertThat(
-        "create region --name=" + regionName + " --type=REPLICATE --groups=" + TEST_GROUP1)
+        "create region --name=" + regionName + " --type=REPLICATE --groups=" + groupNames)
         .statusIsSuccess();
   }
 
@@ -121,6 +125,11 @@ public class CreateMappingCommandDUnitTest {
         .statusIsSuccess();
   }
 
+  private void setupGroupPartition(String regionName, String groupNames) {
+    gfsh.executeAndAssertThat("create region --name=" + regionName + " --type=PARTITION --groups=" + groupNames)
+            .statusIsSuccess();
+  }
+
   private void setupAsyncEventQueue(String regionName) {
     gfsh.executeAndAssertThat(
         "create async-event-queue --id="
@@ -209,9 +218,9 @@ public class CreateMappingCommandDUnitTest {
   }
 
   @Test
-  @Parameters({TEST_REGION, "/" + TEST_REGION})
-  public void createMappingUpdatesServiceAndClusterConfigForServerGroup(String regionName) {
-    setupGroupReplicate(regionName);
+  @Parameters({GROUP1_REGION, "/" + GROUP1_REGION})
+  public void createMappingReplicatedUpdatesServiceAndClusterConfigForServerGroup(String regionName) {
+    setupGroupReplicate(regionName, TEST_GROUP1);
     CommandStringBuilder csb = new CommandStringBuilder(CREATE_MAPPING);
     csb.addOption(REGION_NAME, regionName);
     csb.addOption(DATA_SOURCE_NAME, "connection");
@@ -224,7 +233,7 @@ public class CreateMappingCommandDUnitTest {
 
     gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();
 
-    // TEST_GROUP1 only contains server2
+    // TEST_GROUP1 only contains server2 and server 4
     server2.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
       assertThat(mapping.getDataSourceName()).isEqualTo("connection");
@@ -237,17 +246,26 @@ public class CreateMappingCommandDUnitTest {
       validateAsyncEventQueueCreatedOnServer(regionName, false);
     });
 
+    server4.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isEqualTo("myTable");
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+      assertThat(mapping.getIds()).isEqualTo("myId");
+      assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
+      assertThat(mapping.getSchema()).isEqualTo("mySchema");
+      validateRegionAlteredOnServer(regionName, false);
+      validateAsyncEventQueueCreatedOnServer(regionName, false);
+    });
+
     server1.invoke(() -> {
       RegionMapping mapping = getRegionMappingFromService(regionName);
       assertThat(mapping).isNull();
-      // assertThat(mapping.getDataSourceName()).isEqualTo("connection");
-      // assertThat(mapping.getTableName()).isEqualTo("myTable");
-      // assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
-      // assertThat(mapping.getIds()).isEqualTo("myId");
-      // assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
-      // assertThat(mapping.getSchema()).isEqualTo("mySchema");
-      // validateRegionAlteredOnServer(regionName, false);
-      // validateAsyncEventQueueCreatedOnServer(regionName, false);
+    });
+
+    server3.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping).isNull();
     });
 
     locator.invoke(() -> {
@@ -264,6 +282,70 @@ public class CreateMappingCommandDUnitTest {
   }
 
   @Test
+  @Parameters({GROUP2_REGION, "/" + GROUP2_REGION})
+  public void createMappingParitionedUpdatesServiceAndClusterConfigForServerGroup(String regionName) {
+    setupGroupPartition(regionName, TEST_GROUP2);
+    CommandStringBuilder csb = new CommandStringBuilder(CREATE_MAPPING);
+    csb.addOption(REGION_NAME, regionName);
+    csb.addOption(DATA_SOURCE_NAME, "connection");
+    csb.addOption(TABLE_NAME, "myTable");
+    csb.addOption(PDX_NAME, "myPdxClass");
+    csb.addOption(ID_NAME, "myId");
+    csb.addOption(CATALOG_NAME, "myCatalog");
+    csb.addOption(SCHEMA_NAME, "mySchema");
+    csb.addOption(GROUP_NAME, TEST_GROUP2);
+
+    gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();
+
+    // TEST_GROUP2 only contains server3 and server4
+    server3.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isEqualTo("myTable");
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+      assertThat(mapping.getIds()).isEqualTo("myId");
+      assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
+      assertThat(mapping.getSchema()).isEqualTo("mySchema");
+      validateRegionAlteredOnServer(regionName, false);
+      validateAsyncEventQueueCreatedOnServer(regionName, true);
+    });
+
+    server4.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(mapping.getTableName()).isEqualTo("myTable");
+      assertThat(mapping.getPdxName()).isEqualTo("myPdxClass");
+      assertThat(mapping.getIds()).isEqualTo("myId");
+      assertThat(mapping.getCatalog()).isEqualTo("myCatalog");
+      assertThat(mapping.getSchema()).isEqualTo("mySchema");
+      validateRegionAlteredOnServer(regionName, false);
+      validateAsyncEventQueueCreatedOnServer(regionName, true);
+    });
+
+    server1.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping).isNull();
+    });
+
+    server2.invoke(() -> {
+      RegionMapping mapping = getRegionMappingFromService(regionName);
+      assertThat(mapping).isNull();
+    });
+
+    locator.invoke(() -> {
+      RegionMapping regionMapping = getRegionMappingFromClusterConfig(regionName, TEST_GROUP2);
+      assertThat(regionMapping.getDataSourceName()).isEqualTo("connection");
+      assertThat(regionMapping.getTableName()).isEqualTo("myTable");
+      assertThat(regionMapping.getPdxName()).isEqualTo("myPdxClass");
+      assertThat(regionMapping.getIds()).isEqualTo("myId");
+      assertThat(regionMapping.getCatalog()).isEqualTo("myCatalog");
+      assertThat(regionMapping.getSchema()).isEqualTo("mySchema");
+      validateRegionAlteredInClusterConfig(regionName, TEST_GROUP2, false);
+      validateAsyncEventQueueCreatedInClusterConfig(regionName, TEST_GROUP2, true);
+    });
+  }
+
+  @Test
   @Parameters({TEST_REGION, "/" + TEST_REGION})
   public void createMappingUpdatesServiceAndClusterConfig(String regionName) {
     setupReplicate(regionName);
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
index a0f2c6c..7ed9490 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateMappingCommand.java
@@ -112,9 +112,7 @@ public class CreateMappingCommand extends SingleGfshCommand {
     if (regionName.startsWith("/")) {
       regionName = regionName.substring(1);
     }
-    // group1,group2 => [group1, group2]
-    // input
-    // Set<DistributedMember> targetMembers = findMembersForRegion(regionName, groups);
+
     Set<DistributedMember> targetMembers = findMembers(groups, null);
     RegionMapping mapping =
         new RegionMapping(regionName, pdxName, table, dataSourceName, id, catalog, schema,
diff --git a/geode-core/src/main/java/org/apache/geode/management/cli/GfshCommand.java b/geode-core/src/main/java/org/apache/geode/management/cli/GfshCommand.java
index d82f2bf..09b5471 100644
--- a/geode-core/src/main/java/org/apache/geode/management/cli/GfshCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/cli/GfshCommand.java
@@ -185,10 +185,6 @@ public abstract class GfshCommand implements CommandMarker {
     return CliUtil.getRegionAssociatedMembers(regionPath, cache, true);
   }
 
-  public Set<DistributedMember> findMembersForRegion(String regionPath, String serverGroup) {
-    return CliUtil.getRegionAssociatedMembers(regionPath, cache, true, serverGroup);
-  }
-
   public Set<DistributedMember> findAnyMembersForRegion(String regionPath) {
     return CliUtil.getRegionAssociatedMembers(regionPath, cache, false);
   }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
index 7e8d15a..6f1dd9c 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/CliUtil.java
@@ -49,6 +49,7 @@ import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.FunctionService;
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.distributed.internal.DistributionManager;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
 import org.apache.geode.internal.ClassPathLoader;
@@ -112,15 +113,7 @@ public class CliUtil {
    * @return a Set of DistributedMember for members that have the specified <code>region</code>.
    */
   public static Set<DistributedMember> getRegionAssociatedMembers(String region,
-      final InternalCache cache,
-      boolean returnAll) {
-    return getRegionAssociatedMembers(region, cache, returnAll, null);
-  }
-
-  public static Set<DistributedMember> getRegionAssociatedMembers(String region,
-      final InternalCache cache,
-      boolean returnAll,
-      String serverGroup) {
+      final InternalCache cache, boolean returnAll) {
     if (region == null || region.isEmpty()) {
       return Collections.emptySet();
     }
@@ -142,9 +135,9 @@ public class CliUtil {
     allClusterMembers.add(cache.getDistributedSystem().getDistributedMember());
 
     for (DistributedMember member : allClusterMembers) {
-      List<String> regionAssociatedMemberNamesSet = Arrays.asList(regionAssociatedMemberNames);
+      List<String> regionAssociatedMemberNamesList = Arrays.asList(regionAssociatedMemberNames);
       String name = MBeanJMXAdapter.getMemberNameOrUniqueId(member);
-      if (regionAssociatedMemberNamesSet.contains(name)) {
+      if (regionAssociatedMemberNamesList.contains(name)) {
         matchedMembers.add(member);
         if (!returnAll) {
           return matchedMembers;
@@ -160,8 +153,7 @@ public class CliUtil {
    * @param returnAll if true, returns all matching members, otherwise, returns only one.
    */
   public static Set<DistributedMember> getQueryRegionsAssociatedMembers(Set<String> regions,
-      final InternalCache cache,
-      boolean returnAll) {
+      final InternalCache cache, boolean returnAll) {
     Set<DistributedMember> results = regions.stream()
         .map(region -> getRegionAssociatedMembers(region, cache, true)).reduce((s1, s2) -> {
           s1.retainAll(s2);
@@ -255,8 +247,7 @@ public class CliUtil {
    * groups or members.
    */
   public static Set<DistributedMember> findMembersIncludingLocators(String[] groups,
-      String[] members,
-      InternalCache cache) {
+      String[] members, InternalCache cache) {
     Set<DistributedMember> allMembers = getAllMembers(cache);
     return findMembers(allMembers, groups, members);
   }
@@ -272,6 +263,18 @@ public class CliUtil {
     return findMembers(allNormalMembers, groups, members);
   }
 
+  /**
+   * Finds all Servers which belong to the given arrays of groups or members. Does not include
+   * locators.
+   */
+  public static Set<DistributedMember> findMembers(String[] groups, String[] members,
+      DistributionManager distributionManager) {
+    Set<DistributedMember> allNormalMembers = new HashSet<DistributedMember>(
+        distributionManager.getNormalDistributionManagerIds());
+
+    return findMembers(allNormalMembers, groups, members);
+  }
+
   private static Set<DistributedMember> findMembers(Set<DistributedMember> membersToConsider,
       String[] groups, String[] members) {
     if (groups == null) {
@@ -563,9 +566,8 @@ public class CliUtil {
     } catch (IOException | InterruptedException e) {
       Gfsh.printlnErr(e.getMessage());
     } finally {
-      if (file != null) {
+      if (file != null)
         file.delete();
-      }
     }
   }
 
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
index de056ad..d9e223b 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandExecutor.java
@@ -145,12 +145,11 @@ public class CommandExecutor {
     }
 
     List<String> groupsToUpdate;
-    String groupsInput = parseResult.getParamValueAsString("group");
-
+    String groupInput = parseResult.getParamValueAsString("group");
     TabularResultModel table = null;
 
-    if (!StringUtils.isBlank(groupsInput)) {
-      groupsToUpdate = Arrays.asList(groupsInput.split(","));
+    if (!StringUtils.isBlank(groupInput)) {
+      groupsToUpdate = Arrays.asList(groupInput.split(","));
     } else if (gfshCommand instanceof UpdateAllConfigurationGroupsMarker) {
       groupsToUpdate = ccService.getGroups().stream().collect(Collectors.toList());
       table = resultModel.addTable(GROUP_STATUS_SECTION);