You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2018/12/13 00:49:35 UTC

[geode] branch feature/GEODE-6102 updated: destroy data-source now fails if used by a jdbc-mapping

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

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


The following commit(s) were added to refs/heads/feature/GEODE-6102 by this push:
     new 01cf715  destroy data-source now fails if used by a jdbc-mapping
01cf715 is described below

commit 01cf71547dd4a14c87d25717fe4cd83e21c1d5c0
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Wed Dec 12 16:48:59 2018 -0800

    destroy data-source now fails if used by a jdbc-mapping
---
 .../cli/DestroyDataSourceCommandDUnitTest.java     | 31 ++++++++++----
 .../internal/cli/DestroyDataSourceCommand.java     | 30 +++++++++++++
 .../internal/cli/DestroyDataSourceCommandTest.java | 49 ++++++++++++++++++++++
 3 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java
index ff7af83..1f6c3b0 100644
--- a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java
+++ b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java
@@ -17,8 +17,8 @@ package org.apache.geode.connectors.jdbc.internal.cli;
 import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 
 import org.assertj.core.api.AssertionsForClassTypes;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
+import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -35,16 +35,16 @@ import org.apache.geode.test.junit.rules.GfshCommandRule;
 import org.apache.geode.test.junit.rules.VMProvider;
 
 public class DestroyDataSourceCommandDUnitTest {
-  private static MemberVM locator, server1, server2;
+  private MemberVM locator, server1, server2;
 
-  @ClassRule
-  public static ClusterStartupRule cluster = new ClusterStartupRule();
+  @Rule
+  public ClusterStartupRule cluster = new ClusterStartupRule();
 
-  @ClassRule
-  public static GfshCommandRule gfsh = new GfshCommandRule();
+  @Rule
+  public GfshCommandRule gfsh = new GfshCommandRule();
 
-  @BeforeClass
-  public static void before() throws Exception {
+  @Before
+  public void before() throws Exception {
     locator = cluster.startLocatorVM(0);
     server1 = cluster.startServerVM(1, locator.getPort());
     server2 = cluster.startServerVM(2, locator.getPort());
@@ -109,4 +109,17 @@ public class DestroyDataSourceCommandDUnitTest {
       AssertionsForClassTypes.assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(0);
     });
   }
+
+  @Test
+  public void destroyDataSourceFailsIfInUseByJdbcMapping() {
+    gfsh.executeAndAssertThat("create region --name=myRegion --type=REPLICATE").statusIsSuccess();
+    gfsh.executeAndAssertThat(
+        "create jdbc-mapping --data-source=datasource1 --pdx-name=myPdxClass --region=myRegion")
+        .statusIsSuccess();
+
+    gfsh.executeAndAssertThat("destroy data-source --name=datasource1").statusIsError()
+        .containsOutput(
+            "Data source named \"datasource1\" is still being used by region \"myRegion\"."
+                + " Use destroy jdbc-mapping --name=myRegion and then try again.");
+  }
 }
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommand.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommand.java
index 3d1a2ec..c66c8aa 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommand.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommand.java
@@ -24,6 +24,8 @@ 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.JndiBindingsType;
+import org.apache.geode.cache.configuration.RegionConfig;
+import org.apache.geode.connectors.jdbc.internal.configuration.RegionMapping;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
 import org.apache.geode.management.cli.CliMetaData;
@@ -75,6 +77,15 @@ public class DestroyDataSourceCommand extends SingleGfshCommand {
             "Data source named \"{0}\" does not exist. A jndi-binding was found with that name.",
             dataSourceName));
       }
+
+      try {
+        checkIfDataSourceIsInUse(service, dataSourceName);
+      } catch (IllegalStateException ex) {
+        return ResultModel.createError(CliStrings.format(
+            "Data source named \"{0}\" is still being used by region \"{1}\". Use destroy jdbc-mapping --name={1} and then try again.",
+            dataSourceName, ex.getMessage()));
+
+      }
     }
 
     Set<DistributedMember> targetMembers = findMembers(null, null);
@@ -114,6 +125,25 @@ public class DestroyDataSourceCommand extends SingleGfshCommand {
     }
   }
 
+  /**
+   * @throws IllegalStateException if the data source is used by a jdbc-mapping. The exception
+   *         message names the region using this data source
+   */
+  private void checkIfDataSourceIsInUse(InternalConfigurationPersistenceService service,
+      String dataSourceName) {
+    CacheConfig cacheConfig = service.getCacheConfig(null);
+    for (RegionConfig regionConfig : cacheConfig.getRegions()) {
+      for (CacheElement cacheElement : regionConfig.getCustomRegionElements()) {
+        if (cacheElement instanceof RegionMapping) {
+          RegionMapping regionMapping = (RegionMapping) cacheElement;
+          if (dataSourceName.equals(regionMapping.getDataSourceName())) {
+            throw new IllegalStateException(regionConfig.getName());
+          }
+        }
+      }
+    }
+  }
+
   private boolean isDataSource(JndiBindingsType.JndiBinding binding) {
     return CreateJndiBindingCommand.DATASOURCE_TYPE.SIMPLE.getType().equals(binding.getType())
         || CreateJndiBindingCommand.DATASOURCE_TYPE.POOLED.getType().equals(binding.getType());
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandTest.java
index c839428..67f9a96 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandTest.java
@@ -40,7 +40,10 @@ import org.mockito.ArgumentCaptor;
 
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.configuration.CacheConfig;
+import org.apache.geode.cache.configuration.CacheElement;
 import org.apache.geode.cache.configuration.JndiBindingsType;
+import org.apache.geode.cache.configuration.RegionConfig;
+import org.apache.geode.connectors.jdbc.internal.configuration.RegionMapping;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
 import org.apache.geode.internal.cache.InternalCache;
@@ -134,7 +137,53 @@ public class DestroyDataSourceCommandTest {
             "Data source named \\\"name\\\" does not exist. A jndi-binding was found with that name.");
   }
 
+  @Test
+  public void whenClusterConfigRunningAndDataSourceInUseByRegionThenError() {
+    String DATA_SOURCE_NAME = "myDataSourceName";
+    List<JndiBindingsType.JndiBinding> bindings = new ArrayList<>();
+    JndiBindingsType.JndiBinding jndiBinding = new JndiBindingsType.JndiBinding();
+    jndiBinding.setJndiName(DATA_SOURCE_NAME);
+    jndiBinding.setType(CreateJndiBindingCommand.DATASOURCE_TYPE.SIMPLE.getType());
+    bindings.add(jndiBinding);
+    doReturn(bindings).when(cacheConfig).getJndiBindings();
+    setupRegionConfigToUseDataSource(DATA_SOURCE_NAME);
 
+    gfsh.executeAndAssertThat(command, COMMAND + " --name=" + DATA_SOURCE_NAME).statusIsError()
+        .containsOutput("Data source named \\\"" + DATA_SOURCE_NAME
+            + "\\\" is still being used by region \\\"regionUsingDataSource\\\"."
+            + " Use destroy jdbc-mapping --name=regionUsingDataSource and then try again.");
+  }
+
+  private void setupRegionConfigToUseDataSource(String dataSourceName) {
+    RegionConfig regionConfig = mock(RegionConfig.class);
+    when(regionConfig.getName()).thenReturn("regionUsingDataSource");
+    List<RegionConfig> regionConfigList = new ArrayList<>();
+    regionConfigList.add(regionConfig);
+    when(cacheConfig.getRegions()).thenReturn(regionConfigList);
+    RegionMapping regionMapping = mock(RegionMapping.class);
+    when(regionMapping.getDataSourceName()).thenReturn(dataSourceName);
+    List<CacheElement> cacheElementList = new ArrayList<>();
+    cacheElementList.add(regionMapping);
+    when(regionConfig.getCustomRegionElements()).thenReturn(cacheElementList);
+  }
+
+  @Test
+  public void whenNoMembersFoundAndClusterConfigRunningAndRegionUsingOtherDataSourceThenUpdateClusterConfig() {
+    List<JndiBindingsType.JndiBinding> bindings = new ArrayList<>();
+    JndiBindingsType.JndiBinding jndiBinding = new JndiBindingsType.JndiBinding();
+    jndiBinding.setJndiName("name");
+    jndiBinding.setType(CreateJndiBindingCommand.DATASOURCE_TYPE.SIMPLE.getType());
+    bindings.add(jndiBinding);
+    doReturn(bindings).when(cacheConfig).getJndiBindings();
+    setupRegionConfigToUseDataSource("otherDataSource");
+
+    gfsh.executeAndAssertThat(command, COMMAND + " --name=name").statusIsSuccess()
+        .containsOutput("No members found, data source removed from cluster configuration.")
+        .containsOutput("Changes to configuration for group 'cluster' are persisted.");
+
+    verify(ccService).updateCacheConfig(any(), any());
+    verify(command).updateConfigForGroup(eq("cluster"), eq(cacheConfig), isNotNull());
+  }
 
   @Test
   public void whenNoMembersFoundAndClusterConfigRunningThenUpdateClusterConfig() {