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 2018/12/05 00:55:58 UTC

[geode] branch feature/GEODE-6142 created (now 59cbbde)

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

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


      at 59cbbde  GEODE-6142: Check JDBC mapping before destroy region

This branch includes the following new commits:

     new 59cbbde  GEODE-6142: Check JDBC mapping before destroy region

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[geode] 01/01: GEODE-6142: Check JDBC mapping before destroy region

Posted by jc...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 59cbbde7ef858d05ffe7ff0cf24b6c7967c1b831
Author: Jianxia Chen <jc...@apache.org>
AuthorDate: Tue Dec 4 16:54:51 2018 -0800

    GEODE-6142: Check JDBC mapping before destroy region
    
    Co-authored-by: Darrel Schneider <ds...@pivotal.io>
    Co-authored-by: Jianxia Chen <jc...@apache.org>
---
 .../cli/DestroyMappingCommandDunitTest.java        | 20 ++++++
 .../cli/commands/DestroyRegionCommand.java         | 41 ++++++++++++
 .../cli/commands/DestroyRegionCommandTest.java     | 76 +++++++++++++++++++---
 3 files changed, 129 insertions(+), 8 deletions(-)

diff --git a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyMappingCommandDunitTest.java b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyMappingCommandDunitTest.java
index a7b8d38..dc4247d 100644
--- a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyMappingCommandDunitTest.java
+++ b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyMappingCommandDunitTest.java
@@ -95,6 +95,26 @@ public class DestroyMappingCommandDunitTest implements Serializable {
   }
 
   @Test
+  public void destroyRegionThatHasSynchronousMappingFails() {
+    setupSynchronousMapping();
+
+    gfsh.executeAndAssertThat("destroy region --name=" + REGION_NAME).statusIsError()
+        .containsOutput("Cannot destroy region \"" + REGION_NAME
+            + "\" because JDBC mapping exists. Use \"destroy jdbc-mapping\" first.");
+  }
+
+  @Test
+  public void destroyRegionThatHadSynchronousMappingSucceeds() {
+    setupSynchronousMapping();
+    CommandStringBuilder csb = new CommandStringBuilder(DESTROY_MAPPING);
+    csb.addOption(DESTROY_MAPPING__REGION_NAME, REGION_NAME);
+    gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();
+
+    gfsh.executeAndAssertThat("destroy region --name=" + REGION_NAME).statusIsSuccess();
+  }
+
+
+  @Test
   public void destroysAsyncMapping() {
     setupAsyncMapping();
     CommandStringBuilder csb = new CommandStringBuilder(DESTROY_MAPPING);
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 ba84405..de36e99 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
@@ -14,12 +14,16 @@
  */
 package org.apache.geode.management.internal.cli.commands;
 
+import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
+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.DistributedMember;
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
 import org.apache.geode.management.cli.CliMetaData;
@@ -58,6 +62,12 @@ public class DestroyRegionCommand extends InternalGfshCommand {
       throw new EntityNotFoundException(message, ifExists);
     }
 
+    try {
+      checkForJDBCMapping(regionPath);
+    } catch (IllegalStateException e) {
+      return ResultBuilder.createGemFireErrorResult(e.getMessage());
+    }
+
     // 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
@@ -77,4 +87,35 @@ public class DestroyRegionCommand extends InternalGfshCommand {
     return result;
   }
 
+  void checkForJDBCMapping(String regionPath) {
+    String regionName = regionPath;
+    if (regionPath.startsWith("/")) {
+      regionName = regionPath.substring(1);
+    }
+    InternalConfigurationPersistenceService ccService = getConfigurationPersistenceService();
+    if (ccService == null) {
+      return;
+    }
+    CacheConfig cacheConfig = ccService.getCacheConfig(null);
+    if (cacheConfig == null) {
+      return;
+    }
+    RegionConfig regionConfig = findRegionConfig(cacheConfig, regionName);
+    if (regionConfig == null) {
+      return;
+    }
+    Iterator<CacheElement> iterator = regionConfig.getCustomRegionElements().iterator();
+    while (iterator.hasNext()) {
+      CacheElement element = iterator.next();
+      if (element != null && "jdbc-mapping".equals(element.getId())) {
+        throw new IllegalStateException("Cannot destroy region \"" + regionName
+            + "\" because JDBC mapping exists. Use \"destroy jdbc-mapping\" first.");
+      }
+    }
+  }
+
+  private RegionConfig findRegionConfig(CacheConfig cacheConfig, String regionName) {
+    return cacheConfig.getRegions().stream()
+        .filter(region -> region.getName().equals(regionName)).findFirst().orElse(null);
+  }
 }
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 b393275..b40ad5c 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
@@ -33,12 +33,14 @@ import org.junit.Before;
 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.DistributedMember;
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
 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;
 import org.apache.geode.test.junit.rules.GfshParserRule;
 
@@ -48,10 +50,14 @@ public class DestroyRegionCommandTest {
   public static GfshParserRule parser = new GfshParserRule();
 
   private DestroyRegionCommand command;
-  private CommandResult result;
   private CliFunctionResult result1, result2;
   private InternalConfigurationPersistenceService ccService;
   XmlEntity xmlEntity;
+  private CacheConfig cacheConfig = mock(CacheConfig.class);
+  private RegionConfig regionConfig = mock(RegionConfig.class);
+  private CacheElement cacheElement = mock(CacheElement.class);
+  private List<RegionConfig> regionConfigList = Collections.singletonList(regionConfig);
+  private List<CacheElement> cacheElementList = Collections.singletonList(cacheElement);
 
   @Before
   public void before() throws Exception {
@@ -73,7 +79,7 @@ public class DestroyRegionCommandTest {
   }
 
   @Test
-  public void invalidRegion() throws Exception {
+  public void invalidRegion() {
     parser.executeAndAssertThat(command, "destroy region").statusIsError()
         .containsOutput("Invalid command");
 
@@ -91,7 +97,7 @@ public class DestroyRegionCommandTest {
   }
 
   @Test
-  public void whenNoRegionIsFoundOnAnyMembers() throws Exception {
+  public void whenNoRegionIsFoundOnAnyMembers() {
     doReturn(Collections.emptySet()).when(command).findMembersForRegion(any());
     parser.executeAndAssertThat(command, "destroy region --name=test").statusIsError()
         .containsOutput("Could not find a Region with Region path");
@@ -101,7 +107,7 @@ public class DestroyRegionCommandTest {
   }
 
   @Test
-  public void multipleResultReturned_oneSucess_oneFailed() throws Exception {
+  public void multipleResultReturned_oneSucess_oneFailed() {
     // mock this to pass the member search call
     doReturn(Collections.singleton(DistributedMember.class)).when(command)
         .findMembersForRegion(any());
@@ -120,7 +126,7 @@ public class DestroyRegionCommandTest {
   }
 
   @Test
-  public void multipleResultReturned_oneSuccess_oneException() throws Exception {
+  public void multipleResultReturned_oneSuccess_oneException() {
     // mock this to pass the member search call
     doReturn(Collections.singleton(DistributedMember.class)).when(command)
         .findMembersForRegion(any());
@@ -140,7 +146,7 @@ public class DestroyRegionCommandTest {
   }
 
   @Test
-  public void multipleResultReturned_all_failed() throws Exception {
+  public void multipleResultReturned_all_failed() {
     // mock this to pass the member search call
     doReturn(Collections.singleton(DistributedMember.class)).when(command)
         .findMembersForRegion(any());
@@ -154,7 +160,61 @@ public class DestroyRegionCommandTest {
         .containsOutput("result1 message").containsOutput("something happened");
 
 
-    // verify that xmlEntiry returned by the result1 is saved to Cluster config
+    // verify that xmlEntry returned by the result1 is saved to Cluster config
     verify(command, never()).persistClusterConfiguration(any(), any());
   }
+
+  private void setupJDBCMappingOnRegion(String regionName) {
+    doReturn(cacheConfig).when(ccService).getCacheConfig(null);
+    doReturn(regionConfigList).when(cacheConfig).getRegions();
+    doReturn(regionName).when(regionConfig).getName();
+    doReturn(cacheElementList).when(regionConfig).getCustomRegionElements();
+    doReturn("jdbc-mapping").when(cacheElement).getId();
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void checkForJDBCMappingWithRegionPathThrowsIllegalStateException() {
+    setupJDBCMappingOnRegion("regionName");
+
+    command.checkForJDBCMapping("/regionName");
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void checkForJDBCMappingWithRegionNameThrowsIllegalStateException() {
+    setupJDBCMappingOnRegion("regionName");
+
+    command.checkForJDBCMapping("regionName");
+  }
+
+  @Test
+  public void checkForJDBCMappingWithNoClusterConfigDoesNotThrowException() {
+    setupJDBCMappingOnRegion("regionName");
+    doReturn(null).when(command).getConfigurationPersistenceService();
+
+    command.checkForJDBCMapping("regionName");
+  }
+
+  @Test
+  public void checkForJDBCMappingWithNoCacheConfigDoesNotThrowException() {
+    setupJDBCMappingOnRegion("regionName");
+    doReturn(null).when(ccService).getCacheConfig(null);
+
+    command.checkForJDBCMapping("regionName");
+  }
+
+  @Test
+  public void checkForJDBCMappingWithNoRegionConfigDoesNotThrowException() {
+    setupJDBCMappingOnRegion("regionName");
+    doReturn(Collections.emptyList()).when(cacheConfig).getRegions();
+
+    command.checkForJDBCMapping("regionName");
+  }
+
+  @Test
+  public void checkForJDBCMappingWithNoJDBCMappingDoesNotThrowException() {
+    setupJDBCMappingOnRegion("regionName");
+    doReturn("something").when(cacheElement).getId();
+
+    command.checkForJDBCMapping("regionName");
+  }
 }