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:46 UTC

[geode] 01/01: GEODE-6770: add optional deregister driver class when destroy data source.

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());
+  }
 }