You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by zh...@apache.org on 2019/05/22 18:53:45 UTC

[geode] branch feature/GEODE-6770 created (now 463f37a)

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

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


      at 463f37a  GEODE-6770: add optional deregister driver class when destroy data source.

This branch includes the following new commits:

     new 463f37a  GEODE-6770: add optional deregister driver class when destroy data source.

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-6770: add optional deregister driver class when destroy data source.

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

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

commit 463f37a2b53673108f233a66956b6f3c27c5017c
Author: zhouxh <gz...@pivotal.io>
AuthorDate: Wed May 22 10:19:58 2019 -0700

    GEODE-6770: add optional deregister driver class when destroy data source.
    
        Co-authored-by: Benjamin Ross <br...@pivotal.io>
        Co-authored-by: Donal Evans <do...@pivotal.io>
        Co-authored-by: Xiaojian Zhou <gz...@pivotal.io>
---
 .../internal/cli/DestroyDataSourceCommand.java     |  54 ++++++++-
 .../internal/cli/DestroyDataSourceCommandTest.java | 132 +++++++++++++++++++++
 .../apache/geode/internal/util/DriverJarUtil.java  |  24 ++++
 .../geode/internal/util/DriverJarUtilTest.java     |  16 +++
 4 files changed, 223 insertions(+), 3 deletions(-)

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 2b1be87..0188710 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
@@ -14,6 +14,7 @@
  */
 package org.apache.geode.connectors.jdbc.internal.cli;
 
+import java.sql.SQLException;
 import java.util.List;
 import java.util.Set;
 
@@ -28,12 +29,14 @@ 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.util.DriverJarUtil;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.SingleGfshCommand;
 import org.apache.geode.management.internal.cli.commands.CreateJndiBindingCommand;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.management.internal.cli.functions.DestroyJndiBindingFunction;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.model.InfoResultModel;
 import org.apache.geode.management.internal.cli.result.model.ResultModel;
 import org.apache.geode.management.internal.exceptions.EntityNotFoundException;
 import org.apache.geode.management.internal.security.ResourceOperation;
@@ -49,6 +52,10 @@ public class DestroyDataSourceCommand extends SingleGfshCommand {
       "Skip the destroy operation when the specified data source does "
           + "not exist. Without this option, an error results from the specification "
           + "of a data source that does not exist.";
+  static final String DEREGISTER_DRIVER = "deregister-driver";
+  static final String DEREGISTER_DRIVER_HELP =
+      "Indicates whether or not the driver class associated"
+          + "with the target data source should be deregistered during the destroy process.";
 
   @CliCommand(value = DESTROY_DATA_SOURCE, help = DESTROY_DATA_SOURCE_HELP)
   @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_REGION)
@@ -57,11 +64,15 @@ public class DestroyDataSourceCommand extends SingleGfshCommand {
   public ResultModel destroyDataSource(
       @CliOption(key = DATA_SOURCE_NAME, mandatory = true,
           help = DATA_SOURCE_NAME_HELP) String dataSourceName,
+      @CliOption(key = DEREGISTER_DRIVER,
+          help = DEREGISTER_DRIVER_HELP, specifiedDefaultValue = "true",
+          unspecifiedDefaultValue = "false") boolean deregisterDriver,
       @CliOption(key = CliStrings.IFEXISTS, help = IFEXISTS_HELP, specifiedDefaultValue = "true",
           unspecifiedDefaultValue = "false") boolean ifExists) {
 
     InternalConfigurationPersistenceService service =
         (InternalConfigurationPersistenceService) getConfigurationPersistenceService();
+    String driverClassName = null;
     if (service != null) {
       List<JndiBindingsType.JndiBinding> bindings =
           service.getCacheConfig("cluster").getJndiBindings();
@@ -86,6 +97,9 @@ public class DestroyDataSourceCommand extends SingleGfshCommand {
             dataSourceName, ex.getMessage()));
 
       }
+      if (deregisterDriver) {
+        driverClassName = binding.getJdbcDriverClass();
+      }
     }
 
     Set<DistributedMember> targetMembers = findMembers(null, null);
@@ -109,14 +123,27 @@ public class DestroyDataSourceCommand extends SingleGfshCommand {
       }
 
       ResultModel result = ResultModel.createMemberStatusResult(dataSourceDestroyResult);
+      InfoResultModel infoModel = result.addInfo();
+      String deregisterResult = deregisterDriver(deregisterDriver, driverClassName, dataSourceName);
+      if (deregisterResult != null) {
+        infoModel.addLine(deregisterResult);
+      }
       result.setConfigObject(dataSourceName);
 
       return result;
     } else {
       if (service != null) {
-        ResultModel result =
-            ResultModel
-                .createInfo("No members found, data source removed from cluster configuration.");
+        // ResultModel result =
+        // ResultModel
+        // .createInfo("No members found, data source removed from cluster configuration.");
+        ResultModel result = new ResultModel();
+        InfoResultModel infoModel = result.addInfo();
+        infoModel.addLine("No members found, data source removed from cluster configuration.");
+        String deregisterResult =
+            deregisterDriver(deregisterDriver, driverClassName, dataSourceName);
+        if (deregisterResult != null) {
+          infoModel.addLine(deregisterResult);
+        }
         result.setConfigObject(dataSourceName);
         return result;
       } else {
@@ -149,6 +176,27 @@ public class DestroyDataSourceCommand extends SingleGfshCommand {
         || CreateJndiBindingCommand.DATASOURCE_TYPE.POOLED.getType().equals(binding.getType());
   }
 
+  private String deregisterDriver(boolean deregisterDriver, String driverClassName,
+      String dataSourceName) {
+    if (deregisterDriver && driverClassName != null) {
+      DriverJarUtil util = createDriverJarUtil();
+      try {
+        util.deregisterDriver(driverClassName);
+      } catch (SQLException ex) {
+        return "Warning: deregistering \"" + driverClassName + "\" while destroying data source \""
+            + dataSourceName + "\" failed with exception: " + ex.getMessage();
+      }
+    } else if (deregisterDriver && driverClassName == null) {
+      return "Warning: deregistering \"" + driverClassName + "\" while destroying data source \""
+          + dataSourceName + "\" failed: No driver class name found";
+    }
+    return null;
+  }
+
+  DriverJarUtil createDriverJarUtil() {
+    return new DriverJarUtil();
+  }
+
   @Override
   public boolean updateConfigForGroup(String group, CacheConfig config, Object element) {
     CacheElement.removeElement(config.getJndiBindings(), (String) element);
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 4388e91..823b946 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
@@ -20,12 +20,14 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.isNotNull;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
@@ -47,6 +49,7 @@ 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;
+import org.apache.geode.internal.util.DriverJarUtil;
 import org.apache.geode.management.internal.cli.commands.CreateJndiBindingCommand;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.management.internal.cli.functions.DestroyJndiBindingFunction;
@@ -364,4 +367,133 @@ public class DestroyDataSourceCommandTest {
     assertThat(destroyingDataSource).isEqualTo(true);
     assertThat(targetMembers.getValue()).isEqualTo(members);
   }
+
+  @Test
+  public void destroyDataSourceWithDeregisterDriverSucceedsWithWarning() {
+    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();
+
+    Set<DistributedMember> members = new HashSet<>();
+    members.add(mock(DistributedMember.class));
+
+    CliFunctionResult result =
+        new CliFunctionResult("server1", true, "Data source \"name\" destroyed on \"server1\"");
+    List<CliFunctionResult> results = new ArrayList<>();
+    results.add(result);
+
+    doReturn(members).when(command).findMembers(any(), any());
+    doReturn(results).when(command).executeAndGetFunctionResult(any(), any(), any());
+
+    gfsh.executeAndAssertThat(command, COMMAND + " --name=name --deregister-driver")
+        .statusIsSuccess()
+        .tableHasColumnOnlyWithValues("Member", "server1")
+        .tableHasColumnOnlyWithValues("Status", "OK")
+        .tableHasColumnOnlyWithValues("Message", "Data source \"name\" destroyed on \"server1\"");
+
+    assertThat(cacheConfig.getJndiBindings().isEmpty()).isTrue();
+    verify(command).updateConfigForGroup(eq("cluster"), eq(cacheConfig), any());
+
+    ArgumentCaptor<DestroyJndiBindingFunction> function =
+        ArgumentCaptor.forClass(DestroyJndiBindingFunction.class);
+    ArgumentCaptor<Object[]> arguments = ArgumentCaptor.forClass(Object[].class);
+
+    ArgumentCaptor<Set<DistributedMember>> targetMembers = ArgumentCaptor.forClass(Set.class);
+    verify(command, times(1)).executeAndGetFunctionResult(function.capture(), arguments.capture(),
+        targetMembers.capture());
+
+    String jndiName = (String) arguments.getValue()[0];
+    boolean destroyingDataSource = (boolean) arguments.getValue()[1];
+
+    assertThat(function.getValue()).isInstanceOf(DestroyJndiBindingFunction.class);
+    assertThat(jndiName).isEqualTo("name");
+    assertThat(destroyingDataSource).isEqualTo(true);
+    assertThat(targetMembers.getValue()).isEqualTo(members);
+  }
+
+  @Test
+  public void destroyDataSourceThrowsSQLExceptpionWithRegisterDriverAndBadDriverClassName()
+      throws SQLException {
+    DriverJarUtil util = mock(DriverJarUtil.class);
+    doReturn(util).when(command).createDriverJarUtil();
+
+    String exceptionMessage = "Command failed as expected.";
+
+    doThrow(new SQLException(exceptionMessage)).when(util).deregisterDriver(any());
+
+    List<JndiBindingsType.JndiBinding> bindings = new ArrayList<>();
+    JndiBindingsType.JndiBinding jndiBinding = new JndiBindingsType.JndiBinding();
+    jndiBinding.setJndiName("name");
+    jndiBinding.setJdbcDriverClass("badDriverClassName");
+    jndiBinding.setType(CreateJndiBindingCommand.DATASOURCE_TYPE.SIMPLE.getType());
+    bindings.add(jndiBinding);
+    doReturn(bindings).when(cacheConfig).getJndiBindings();
+
+    Set<DistributedMember> members = new HashSet<>();
+    members.add(mock(DistributedMember.class));
+
+    CliFunctionResult result =
+        new CliFunctionResult("server1", true, "Data source \"name\" destroyed on \"server1\"");
+    List<CliFunctionResult> results = new ArrayList<>();
+    results.add(result);
+
+    doReturn(members).when(command).findMembers(any(), any());
+    doReturn(results).when(command).executeAndGetFunctionResult(any(), any(), any());
+
+    gfsh.executeAndAssertThat(command, COMMAND + " --name=name --deregister-driver")
+        .containsOutput(exceptionMessage);
+  }
+
+  @Test
+  public void destroyDataSourceWithDeregisterDriverSucceedsWithoutWarnings() {
+    DriverJarUtil util = mock(DriverJarUtil.class);
+
+    List<JndiBindingsType.JndiBinding> bindings = new ArrayList<>();
+    JndiBindingsType.JndiBinding jndiBinding = new JndiBindingsType.JndiBinding();
+    jndiBinding.setJndiName("name");
+    jndiBinding.setJdbcDriverClass("driverClassName");
+    jndiBinding.setType(CreateJndiBindingCommand.DATASOURCE_TYPE.SIMPLE.getType());
+    bindings.add(jndiBinding);
+    doReturn(bindings).when(cacheConfig).getJndiBindings();
+
+    Set<DistributedMember> members = new HashSet<>();
+    members.add(mock(DistributedMember.class));
+
+    CliFunctionResult result =
+        new CliFunctionResult("server1", true, "Data source \"name\" destroyed on \"server1\"");
+    List<CliFunctionResult> results = new ArrayList<>();
+    results.add(result);
+
+    doReturn(members).when(command).findMembers(any(), any());
+    doReturn(results).when(command).executeAndGetFunctionResult(any(), any(), any());
+
+    gfsh.executeAndAssertThat(command, COMMAND + " --name=name --deregister-driver")
+        .statusIsSuccess()
+        .tableHasColumnOnlyWithValues("Member", "server1")
+        .tableHasColumnOnlyWithValues("Status", "OK")
+        .tableHasColumnOnlyWithValues("Message", "Data source \"name\" destroyed on \"server1\"")
+        .doesNotContainOutput("Warning:");
+
+    assertThat(cacheConfig.getJndiBindings().isEmpty()).isTrue();
+    verify(command).updateConfigForGroup(eq("cluster"), eq(cacheConfig), any());
+
+    ArgumentCaptor<DestroyJndiBindingFunction> function =
+        ArgumentCaptor.forClass(DestroyJndiBindingFunction.class);
+    ArgumentCaptor<Object[]> arguments = ArgumentCaptor.forClass(Object[].class);
+
+    ArgumentCaptor<Set<DistributedMember>> targetMembers = ArgumentCaptor.forClass(Set.class);
+    verify(command, times(1)).executeAndGetFunctionResult(function.capture(), arguments.capture(),
+        targetMembers.capture());
+
+    String jndiName = (String) arguments.getValue()[0];
+    boolean destroyingDataSource = (boolean) arguments.getValue()[1];
+
+    assertThat(function.getValue()).isInstanceOf(DestroyJndiBindingFunction.class);
+    assertThat(jndiName).isEqualTo("name");
+    assertThat(destroyingDataSource).isEqualTo(true);
+    assertThat(targetMembers.getValue()).isEqualTo(members);
+  }
 }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/util/DriverJarUtil.java b/geode-core/src/main/java/org/apache/geode/internal/util/DriverJarUtil.java
index dc57bd6..aaac8ba 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/util/DriverJarUtil.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/util/DriverJarUtil.java
@@ -21,6 +21,7 @@ import java.sql.DriverManager;
 import java.sql.DriverPropertyInfo;
 import java.sql.SQLException;
 import java.sql.SQLFeatureNotSupportedException;
+import java.util.Enumeration;
 
 import org.apache.geode.internal.ClassPathLoader;
 
@@ -35,6 +36,17 @@ public class DriverJarUtil {
     registerDriverWithDriverManager(d);
   }
 
+  public void deregisterDriver(String driverClassName) throws SQLException {
+    Enumeration<Driver> driverEnumeration = getDrivers();
+    Driver driver;
+    while (driverEnumeration.hasMoreElements()) {
+      driver = driverEnumeration.nextElement();
+      if (compareDriverClassName(driver, driverClassName)) {
+        deregisterDriverWithDriverManager(driver);
+      }
+    }
+  }
+
   // The methods below are included to facilitate testing and to make the helper methods in this
   // class cleaner
   Driver getDriverInstanceByClassName(String driverClassName)
@@ -42,10 +54,22 @@ public class DriverJarUtil {
     return (Driver) ClassPathLoader.getLatest().forName(driverClassName).newInstance();
   }
 
+  Enumeration<Driver> getDrivers() {
+    return DriverManager.getDrivers();
+  }
+
+  boolean compareDriverClassName(Driver driver, String driverClassName) {
+    return driver.getClass().getName().equals(driverClassName);
+  }
+
   void registerDriverWithDriverManager(Driver driver) throws SQLException {
     DriverManager.registerDriver(driver);
   }
 
+  void deregisterDriverWithDriverManager(Driver driver) throws SQLException {
+    DriverManager.deregisterDriver(driver);
+  }
+
   // DriverManager only uses a driver loaded by system ClassLoader
   class DriverWrapper implements Driver {
 
diff --git a/geode-core/src/test/java/org/apache/geode/internal/util/DriverJarUtilTest.java b/geode-core/src/test/java/org/apache/geode/internal/util/DriverJarUtilTest.java
index 8cfcba2..de8102a 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/util/DriverJarUtilTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/util/DriverJarUtilTest.java
@@ -20,9 +20,11 @@ import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import java.sql.Driver;
 import java.sql.SQLException;
+import java.util.Enumeration;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -47,4 +49,18 @@ public class DriverJarUtilTest {
     util.registerDriver(driverName);
     verify(util).registerDriverWithDriverManager(any());
   }
+
+  @Test
+  public void deregisterDriverSucceedsWithClassName() throws SQLException {
+    String driverName = "driver-name";
+    Enumeration<Driver> drivers = mock(Enumeration.class);
+    doReturn(drivers).when(util).getDrivers();
+    Driver driver = mock(Driver.class);
+    when(drivers.hasMoreElements()).thenReturn(true).thenReturn(false);
+    when(drivers.nextElement()).thenReturn(driver);
+    doReturn(true).when(util).compareDriverClassName(driver, driverName);
+
+    util.deregisterDriver(driverName);
+    verify(util).deregisterDriverWithDriverManager(any());
+  }
 }