You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2017/11/28 23:44:58 UTC

[geode] branch feature/GEODE-4018 updated (4cbdc3c -> 31144f3)

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

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


 discard 4cbdc3c  GEODE-3781: cleanup tests
 discard 56866a1  GEODE-4018: implement destroy jdbc-connection command
     new 73dfc4b  GEODE-4018: implement destroy jdbc-connection command
     new 31144f3  GEODE-3781: cleanup tests

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (4cbdc3c)
            \
             N -- N -- N   refs/heads/feature/GEODE-4018 (31144f3)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 2 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.


Summary of changes:
 .../geode/connectors/jdbc/internal/ConnectionManager.java   |  1 -
 .../jdbc/internal/cli/CreateConnectionCommand.java          | 13 +++++++------
 .../jdbc/internal/cli/CreateConnectionFunction.java         |  2 --
 .../jdbc/internal/cli/DestroyConnectionCommand.java         |  4 ++--
 .../jdbc/internal/PreparedStatementCacheTest.java           |  2 +-
 .../cli/DestroyConnectionCommandIntegrationTest.java        |  2 +-
 6 files changed, 11 insertions(+), 13 deletions(-)

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].

[geode] 01/02: GEODE-4018: implement destroy jdbc-connection command

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

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

commit 73dfc4bcdd9674e39e384af5a7ffc77af8510e1a
Author: Kirk Lund <kl...@apache.org>
AuthorDate: Tue Nov 28 12:00:34 2017 -0800

    GEODE-4018: implement destroy jdbc-connection command
---
 .../jdbc/internal/ConnectionManager.java           |  1 -
 .../internal/InternalJdbcConnectorService.java     |  2 +
 .../jdbc/internal/JdbcConnectorService.java        |  6 ++
 .../jdbc/internal/cli/CreateConnectionCommand.java | 18 ++--
 .../internal/cli/CreateConnectionFunction.java     | 34 ++++----
 .../internal/cli/DestroyConnectionCommand.java     | 88 ++++++++++++++++++++
 ...unction.java => DestroyConnectionFunction.java} | 55 +++++++------
 .../ExceptionHandler.java}                         | 27 +++---
 .../org.springframework.shell.core.CommandMarker   |  3 +-
 .../CreateConnectionCommandIntegrationTest.java    |  3 +-
 .../internal/cli/CreateConnectionFunctionTest.java | 44 +++++++---
 .../cli/DestroyConnectionCommandDUnitTest.java     | 96 ++++++++++++++++++++++
 ...> DestroyConnectionCommandIntegrationTest.java} | 43 +++++-----
 ...est.java => DestroyConnectionFunctionTest.java} | 56 +++++++++----
 14 files changed, 357 insertions(+), 119 deletions(-)

diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java
index 30d030f..8e7795b 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionManager.java
@@ -97,7 +97,6 @@ class ConnectionManager {
         pdxTypeId);
   }
 
-  // package protected for testing purposes only
   Connection getSQLConnection(ConnectionConfiguration config) throws SQLException {
     return DriverManager.getConnection(config.getUrl(), config.getConnectionProperties());
   }
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/InternalJdbcConnectorService.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/InternalJdbcConnectorService.java
index 882dd20..c7bdc16 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/InternalJdbcConnectorService.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/InternalJdbcConnectorService.java
@@ -23,6 +23,8 @@ public interface InternalJdbcConnectorService extends Extension<Cache>, CacheSer
   void createConnectionConfig(ConnectionConfiguration config)
       throws ConnectionConfigExistsException;
 
+  void destroyConnectionConfig(String connectionName);
+
   void addOrUpdateRegionMapping(RegionMapping mapping);
 
   ConnectionConfiguration getConnectionConfig(String connectionName);
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java
index 222345f..de4759b 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorService.java
@@ -53,6 +53,12 @@ public class JdbcConnectorService implements InternalJdbcConnectorService {
   }
 
   @Override
+  public void destroyConnectionConfig(String connectionName) {
+    registerAsExtension();
+    connectionsByName.remove(connectionName);
+  }
+
+  @Override
   public void addOrUpdateRegionMapping(RegionMapping mapping) {
     registerAsExtension();
     mappingsByRegion.put(mapping.getRegionName(), mapping);
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommand.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommand.java
index a3f676f..9bf9ba0 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommand.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommand.java
@@ -42,20 +42,21 @@ public class CreateConnectionCommand implements GfshCommand {
   private static final Logger logger = LogService.getLogger();
 
   static final String CREATE_CONNECTION = "create jdbc-connection";
-  static final String CREATE_CONNECTION__HELP = "Create JDBC connection for JDBC Connector.";
+  static final String CREATE_CONNECTION__HELP =
+      "Create a connection for communicating with a database through JDBC.";
   static final String CREATE_CONNECTION__NAME = "name";
-  static final String CREATE_CONNECTION__NAME__HELP = "Name of the JDBC connection to be created.";
+  static final String CREATE_CONNECTION__NAME__HELP = "Name of the connection to be created.";
   static final String CREATE_CONNECTION__URL = "url";
-  static final String CREATE_CONNECTION__URL__HELP = "URL location for the database";
+  static final String CREATE_CONNECTION__URL__HELP = "URL location for the database.";
   static final String CREATE_CONNECTION__USER = "user";
   static final String CREATE_CONNECTION__USER__HELP =
-      "Name of user to use when connecting to the database";
+      "User name to use when connecting to database.";
   static final String CREATE_CONNECTION__PASSWORD = "password";
   static final String CREATE_CONNECTION__PASSWORD__HELP =
-      "Password of user to use when connecting to the database";
+      "Password to use when connecting to database.";
   static final String CREATE_CONNECTION__PARAMS = "params";
   static final String CREATE_CONNECTION__PARAMS__HELP =
-      "Comma delimited list of additional parameters to use when connecting to the database";
+      "Additional parameters to use when connecting to the database.";
 
   private static final String ERROR_PREFIX = "ERROR: ";
 
@@ -85,11 +86,12 @@ public class CreateConnectionCommand implements GfshCommand {
 
     Object resultCollectorResult = resultCollector.getResult();
 
-    List<CliFunctionResult> regionCreateResults = (List<CliFunctionResult>) resultCollectorResult;
+    List<CliFunctionResult> connectionCreateResults =
+        (List<CliFunctionResult>) resultCollectorResult;
 
     AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
     TabularResultData tabularResultData = ResultBuilder.createTabularResultData();
-    for (CliFunctionResult regionCreateResult : regionCreateResults) {
+    for (CliFunctionResult regionCreateResult : connectionCreateResults) {
       boolean success = regionCreateResult.isSuccessful();
       tabularResultData.accumulate("Member", regionCreateResult.getMemberIdOrName());
       tabularResultData.accumulate("Status",
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunction.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunction.java
index 6dd5e57..72358b0 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunction.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunction.java
@@ -14,8 +14,6 @@
  */
 package org.apache.geode.connectors.jdbc.internal.cli;
 
-import org.apache.logging.log4j.Logger;
-
 import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
@@ -27,16 +25,24 @@ import org.apache.geode.connectors.jdbc.internal.xml.JdbcConnectorServiceXmlPars
 import org.apache.geode.internal.InternalEntity;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.xmlcache.CacheXml;
-import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.management.internal.configuration.domain.XmlEntity;
 
 public class CreateConnectionFunction implements Function<ConnectionConfiguration>, InternalEntity {
-  private static final Logger logger = LogService.getLogger();
 
   private static final String ID = CreateConnectionFunction.class.getName();
 
+  private final transient ExceptionHandler exceptionHandler;
+
+  public CreateConnectionFunction() {
+    this(new ExceptionHandler());
+  }
+
+  private CreateConnectionFunction(ExceptionHandler exceptionHandler) {
+    this.exceptionHandler = exceptionHandler;
+  }
+
   @Override
   public boolean isHA() {
     return false;
@@ -55,36 +61,24 @@ public class CreateConnectionFunction implements Function<ConnectionConfiguratio
     String memberNameOrId =
         CliUtil.getMemberNameOrId(cache.getDistributedSystem().getDistributedMember());
 
-    ConnectionConfiguration configuration = context.getArguments();
+    ConnectionConfiguration connectionConfig = context.getArguments();
 
     try {
       InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
-      service.createConnectionConfig(configuration);
+      service.createConnectionConfig(connectionConfig);
 
       XmlEntity xmlEntity = new XmlEntity(CacheXml.CACHE, JdbcConnectorServiceXmlGenerator.PREFIX,
           JdbcConnectorServiceXmlParser.NAMESPACE, ElementType.CONNECTION_SERVICE.getTypeName());
 
       resultSender.lastResult(new CliFunctionResult(memberNameOrId, xmlEntity,
-          "Created JDBC connection " + configuration.getName() + " on " + memberNameOrId));
+          "Created JDBC connection " + connectionConfig.getName() + " on " + memberNameOrId));
 
     } catch (Exception e) {
       String exceptionMsg = e.getMessage();
       if (exceptionMsg == null) {
         exceptionMsg = CliUtil.stackTraceAsString(e);
       }
-      resultSender.lastResult(handleException(memberNameOrId, exceptionMsg, e));
+      resultSender.lastResult(exceptionHandler.handleException(memberNameOrId, exceptionMsg, e));
     }
   }
-
-  private CliFunctionResult handleException(final String memberNameOrId, final String exceptionMsg,
-      final Exception e) {
-    if (e != null && logger.isDebugEnabled()) {
-      logger.debug(e.getMessage(), e);
-    }
-    if (exceptionMsg != null) {
-      return new CliFunctionResult(memberNameOrId, false, exceptionMsg);
-    }
-
-    return new CliFunctionResult(memberNameOrId);
-  }
 }
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommand.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommand.java
new file mode 100644
index 0000000..04217f4
--- /dev/null
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommand.java
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.connectors.jdbc.internal.cli;
+
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.springframework.shell.core.annotation.CliCommand;
+import org.springframework.shell.core.annotation.CliOption;
+
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.cli.CliMetaData;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.commands.GfshCommand;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.result.ResultBuilder;
+import org.apache.geode.management.internal.cli.result.TabularResultData;
+import org.apache.geode.management.internal.configuration.domain.XmlEntity;
+import org.apache.geode.management.internal.security.ResourceOperation;
+import org.apache.geode.security.ResourcePermission;
+
+public class DestroyConnectionCommand implements GfshCommand {
+
+  static final String DESTROY_CONNECTION = "destroy jdbc-connection";
+  static final String DESTROY_CONNECTION__HELP = "Destroy/Remove the specified jdbc connection.";
+  static final String DESTROY_CONNECTION__NAME = "name";
+  static final String DESTROY_CONNECTION__NAME__HELP =
+      "Name of the jdbc connection to be destroyed.";
+
+  private static final String ERROR_PREFIX = "ERROR: ";
+
+  @CliCommand(value = DESTROY_CONNECTION, help = DESTROY_CONNECTION__HELP)
+  @CliMetaData(relatedTopic = CliStrings.TOPIC_GEODE_REGION)
+  @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER,
+      operation = ResourcePermission.Operation.MANAGE)
+  public Result destroyConnection(@CliOption(key = DESTROY_CONNECTION__NAME, mandatory = true,
+      help = DESTROY_CONNECTION__NAME__HELP) String name) {
+
+    Set<DistributedMember> membersToDestroyConnectionOn = getMembers(null, null);
+
+    ResultCollector<?, ?> resultCollector =
+        executeFunction(new DestroyConnectionFunction(), name, membersToDestroyConnectionOn);
+
+    Object resultCollectorResult = resultCollector.getResult();
+
+    List<CliFunctionResult> connectionDestroyResults =
+        (List<CliFunctionResult>) resultCollectorResult;
+
+    AtomicReference<XmlEntity> xmlEntity = new AtomicReference<>();
+    TabularResultData tabularResultData = ResultBuilder.createTabularResultData();
+    for (CliFunctionResult connectionDestroyResult : connectionDestroyResults) {
+      boolean success = connectionDestroyResult.isSuccessful();
+      tabularResultData.accumulate("Member", connectionDestroyResult.getMemberIdOrName());
+      tabularResultData.accumulate("Status",
+          (success ? "" : ERROR_PREFIX) + connectionDestroyResult.getMessage());
+
+      if (success) {
+        xmlEntity.set(connectionDestroyResult.getXmlEntity());
+      } else {
+        tabularResultData.setStatus(Result.Status.ERROR);
+      }
+    }
+
+    Result result = ResultBuilder.buildResult(tabularResultData);
+
+    if (xmlEntity.get() != null) {
+      persistClusterConfiguration(result,
+          () -> getSharedConfiguration().addXmlEntity(xmlEntity.get(), null));
+    }
+
+    return result;
+  }
+}
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunction.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunction.java
similarity index 62%
copy from geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunction.java
copy to geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunction.java
index 6dd5e57..e0886cc 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunction.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunction.java
@@ -14,12 +14,9 @@
  */
 package org.apache.geode.connectors.jdbc.internal.cli;
 
-import org.apache.logging.log4j.Logger;
-
 import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
-import org.apache.geode.connectors.jdbc.internal.ConnectionConfiguration;
 import org.apache.geode.connectors.jdbc.internal.InternalJdbcConnectorService;
 import org.apache.geode.connectors.jdbc.internal.xml.ElementType;
 import org.apache.geode.connectors.jdbc.internal.xml.JdbcConnectorServiceXmlGenerator;
@@ -27,15 +24,24 @@ import org.apache.geode.connectors.jdbc.internal.xml.JdbcConnectorServiceXmlPars
 import org.apache.geode.internal.InternalEntity;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.xmlcache.CacheXml;
-import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.management.internal.cli.CliUtil;
 import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.configuration.domain.XmlEntity;
 
-public class CreateConnectionFunction implements Function<ConnectionConfiguration>, InternalEntity {
-  private static final Logger logger = LogService.getLogger();
+public class DestroyConnectionFunction implements Function<String>, InternalEntity {
+
+  private static final String ID = DestroyConnectionFunction.class.getName();
 
-  private static final String ID = CreateConnectionFunction.class.getName();
+  private final transient ExceptionHandler exceptionHandler;
+
+  public DestroyConnectionFunction() {
+    this(new ExceptionHandler());
+  }
+
+  private DestroyConnectionFunction(ExceptionHandler exceptionHandler) {
+    this.exceptionHandler = exceptionHandler;
+  }
 
   @Override
   public boolean isHA() {
@@ -48,43 +54,38 @@ public class CreateConnectionFunction implements Function<ConnectionConfiguratio
   }
 
   @Override
-  public void execute(FunctionContext<ConnectionConfiguration> context) {
+  public void execute(FunctionContext<String> context) {
     ResultSender<Object> resultSender = context.getResultSender();
 
     InternalCache cache = (InternalCache) context.getCache();
     String memberNameOrId =
         CliUtil.getMemberNameOrId(cache.getDistributedSystem().getDistributedMember());
 
-    ConnectionConfiguration configuration = context.getArguments();
+    String connectionName = context.getArguments();
 
     try {
       InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
-      service.createConnectionConfig(configuration);
 
-      XmlEntity xmlEntity = new XmlEntity(CacheXml.CACHE, JdbcConnectorServiceXmlGenerator.PREFIX,
-          JdbcConnectorServiceXmlParser.NAMESPACE, ElementType.CONNECTION_SERVICE.getTypeName());
+      if (service.getConnectionConfig(connectionName) == null) {
+        resultSender.lastResult(new CliFunctionResult(memberNameOrId, false,
+            CliStrings.format("Connection named \"{0}\" not found", connectionName)));
+
+      } else {
+        service.destroyConnectionConfig(connectionName);
 
-      resultSender.lastResult(new CliFunctionResult(memberNameOrId, xmlEntity,
-          "Created JDBC connection " + configuration.getName() + " on " + memberNameOrId));
+        XmlEntity xmlEntity = new XmlEntity(CacheXml.CACHE, JdbcConnectorServiceXmlGenerator.PREFIX,
+            JdbcConnectorServiceXmlParser.NAMESPACE, ElementType.CONNECTION_SERVICE.getTypeName());
+
+        resultSender.lastResult(new CliFunctionResult(memberNameOrId, xmlEntity,
+            "Destroyed JDBC connection " + connectionName + " on " + memberNameOrId));
+      }
 
     } catch (Exception e) {
       String exceptionMsg = e.getMessage();
       if (exceptionMsg == null) {
         exceptionMsg = CliUtil.stackTraceAsString(e);
       }
-      resultSender.lastResult(handleException(memberNameOrId, exceptionMsg, e));
+      resultSender.lastResult(exceptionHandler.handleException(memberNameOrId, exceptionMsg, e));
     }
   }
-
-  private CliFunctionResult handleException(final String memberNameOrId, final String exceptionMsg,
-      final Exception e) {
-    if (e != null && logger.isDebugEnabled()) {
-      logger.debug(e.getMessage(), e);
-    }
-    if (exceptionMsg != null) {
-      return new CliFunctionResult(memberNameOrId, false, exceptionMsg);
-    }
-
-    return new CliFunctionResult(memberNameOrId);
-  }
 }
diff --git a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/InternalJdbcConnectorService.java b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/ExceptionHandler.java
similarity index 53%
copy from geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/InternalJdbcConnectorService.java
copy to geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/ExceptionHandler.java
index 882dd20..8e44ec7 100644
--- a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/InternalJdbcConnectorService.java
+++ b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/ExceptionHandler.java
@@ -12,20 +12,25 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-package org.apache.geode.connectors.jdbc.internal;
+package org.apache.geode.connectors.jdbc.internal.cli;
 
-import org.apache.geode.cache.Cache;
-import org.apache.geode.internal.cache.CacheService;
-import org.apache.geode.internal.cache.extension.Extension;
+import org.apache.logging.log4j.Logger;
 
-public interface InternalJdbcConnectorService extends Extension<Cache>, CacheService {
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 
-  void createConnectionConfig(ConnectionConfiguration config)
-      throws ConnectionConfigExistsException;
+class ExceptionHandler {
+  private static final Logger logger = LogService.getLogger();
 
-  void addOrUpdateRegionMapping(RegionMapping mapping);
+  CliFunctionResult handleException(final String memberNameOrId, final String exceptionMsg,
+      final Exception e) {
+    if (e != null && logger.isDebugEnabled()) {
+      logger.debug(e.getMessage(), e);
+    }
+    if (exceptionMsg != null) {
+      return new CliFunctionResult(memberNameOrId, false, exceptionMsg);
+    }
 
-  ConnectionConfiguration getConnectionConfig(String connectionName);
-
-  RegionMapping getMappingForRegion(String regionName);
+    return new CliFunctionResult(memberNameOrId);
+  }
 }
diff --git a/geode-connectors/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker b/geode-connectors/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
index 148dd48..5ec0430 100644
--- a/geode-connectors/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
+++ b/geode-connectors/src/main/resources/META-INF/services/org.springframework.shell.core.CommandMarker
@@ -15,4 +15,5 @@
 # limitations under the License.
 #
 # Lucene Extensions command
-org.apache.geode.connectors.jdbc.internal.cli.CreateConnectionCommand
\ No newline at end of file
+org.apache.geode.connectors.jdbc.internal.cli.CreateConnectionCommand
+org.apache.geode.connectors.jdbc.internal.cli.DestroyConnectionCommand
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
index e6ea066..3d8933f 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
@@ -27,7 +27,6 @@ import org.apache.geode.connectors.jdbc.internal.ConnectionConfiguration;
 import org.apache.geode.connectors.jdbc.internal.InternalJdbcConnectorService;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 
 @Category(IntegrationTest.class)
@@ -85,7 +84,7 @@ public class CreateConnectionCommandIntegrationTest {
     ConnectionConfiguration connectionConfig = service.getConnectionConfig(name);
 
     Result result = createConnectionCommand.createConnection(name, url, user, password, params);
-    assertThat(((CommandResult) result).toJson()).contains("ERROR");
+    assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
 
     assertThat(service.getConnectionConfig(name)).isSameAs(connectionConfig);
   }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
index 7452d88..e63d4ed 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
@@ -16,23 +16,30 @@ package org.apache.geode.connectors.jdbc.internal.cli;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.io.Serializable;
+
+import org.apache.commons.lang.SerializationUtils;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.mockito.ArgumentCaptor;
 
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
 import org.apache.geode.connectors.jdbc.internal.ConnectionConfigBuilder;
+import org.apache.geode.connectors.jdbc.internal.ConnectionConfigExistsException;
 import org.apache.geode.connectors.jdbc.internal.ConnectionConfiguration;
 import org.apache.geode.connectors.jdbc.internal.InternalJdbcConnectorService;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.DistributedSystem;
 import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
@@ -41,21 +48,18 @@ public class CreateConnectionFunctionTest {
   private CreateConnectionFunction function;
   private FunctionContext<ConnectionConfiguration> context;
   private ResultSender<Object> resultSender;
-  private InternalCache cache;
-  private DistributedSystem system;
-  private DistributedMember member;
   private ConnectionConfiguration configuration;
   private InternalJdbcConnectorService service;
 
   @Before
-  public void setup() {
+  public void setUp() {
     configuration = new ConnectionConfigBuilder().build();
 
     context = mock(FunctionContext.class);
     resultSender = mock(ResultSender.class);
-    cache = mock(InternalCache.class);
-    system = mock(DistributedSystem.class);
-    member = mock(DistributedMember.class);
+    InternalCache cache = mock(InternalCache.class);
+    DistributedSystem system = mock(DistributedSystem.class);
+    DistributedMember member = mock(DistributedMember.class);
     service = mock(InternalJdbcConnectorService.class);
 
     when(context.getResultSender()).thenReturn(resultSender);
@@ -69,19 +73,39 @@ public class CreateConnectionFunctionTest {
   }
 
   @Test
-  public void isHAIsFalse() throws Exception {
+  public void isHAReturnsFalse() throws Exception {
     assertThat(function.isHA()).isFalse();
   }
 
   @Test
-  public void getId() throws Exception {
+  public void getIdReturnsNameOfClass() throws Exception {
     assertThat(function.getId()).isEqualTo(function.getClass().getName());
   }
 
   @Test
-  public void execute() throws Exception {
+  public void executeCreatesConnectionIfConfigNotFound() throws Exception {
     function.execute(context);
     verify(service, times(1)).createConnectionConfig(configuration);
   }
 
+  @Test
+  public void executeReportsErrorIfConnectionConfigFound() throws Exception {
+    doThrow(ConnectionConfigExistsException.class).when(service)
+        .createConnectionConfig(eq(configuration));
+
+    function.execute(context);
+
+    ArgumentCaptor<CliFunctionResult> argument = ArgumentCaptor.forClass(CliFunctionResult.class);
+    verify(resultSender, times(1)).lastResult(argument.capture());
+    assertThat(argument.getValue().getErrorMessage())
+        .contains(ConnectionConfigExistsException.class.getName());
+  }
+
+  @Test
+  public void serializes() throws Exception {
+    Serializable original = function;
+    Object copy = SerializationUtils.clone(original);
+
+    assertThat(copy).isNotSameAs(original).isInstanceOf(CreateConnectionFunction.class);
+  }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandDUnitTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandDUnitTest.java
new file mode 100644
index 0000000..9c40ae3
--- /dev/null
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandDUnitTest.java
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.connectors.jdbc.internal.cli;
+
+import static org.apache.geode.connectors.jdbc.internal.cli.DestroyConnectionCommand.DESTROY_CONNECTION;
+import static org.apache.geode.connectors.jdbc.internal.cli.DestroyConnectionCommand.DESTROY_CONNECTION__NAME;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.connectors.jdbc.internal.ConnectionConfigBuilder;
+import org.apache.geode.connectors.jdbc.internal.ConnectionConfiguration;
+import org.apache.geode.connectors.jdbc.internal.InternalJdbcConnectorService;
+import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+@Category(DistributedTest.class)
+public class DestroyConnectionCommandDUnitTest implements Serializable {
+
+  @Rule
+  public transient GfshCommandRule gfsh = new GfshCommandRule();
+
+  @Rule
+  public LocatorServerStartupRule startupRule = new LocatorServerStartupRule();
+
+  @Rule
+  public SerializableTestName testName = new SerializableTestName();
+
+  private MemberVM locator;
+  private MemberVM server;
+
+  private String connectionName;
+
+  @Before
+  public void before() throws Exception {
+    connectionName = "name";
+
+    locator = startupRule.startLocatorVM(0);
+    server = startupRule.startServerVM(1, locator.getPort());
+
+    server.invoke(() -> createConnection());
+
+    gfsh.connectAndVerify(locator);
+  }
+
+  @Test
+  public void destroysConnection() throws Exception {
+    CommandStringBuilder csb = new CommandStringBuilder(DESTROY_CONNECTION);
+    csb.addOption(DESTROY_CONNECTION__NAME, "name");
+
+    gfsh.executeAndAssertThat(csb.toString()).statusIsSuccess();
+
+    locator.invoke(() -> {
+      String xml = InternalLocator.getLocator().getSharedConfiguration().getConfiguration("cluster")
+          .getCacheXmlContent();
+      assertThat(xml).contains("jdbc:connector-service");
+    });
+
+    server.invoke(() -> {
+      InternalCache cache = LocatorServerStartupRule.getCache();
+      ConnectionConfiguration config =
+          cache.getService(InternalJdbcConnectorService.class).getConnectionConfig("name");
+      assertThat(config).isNull();
+    });
+  }
+
+  private void createConnection() {
+    InternalCache cache = LocatorServerStartupRule.getCache();
+    InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
+    service.createConnectionConfig(new ConnectionConfigBuilder().withName(connectionName).build());
+    assertThat(service.getConnectionConfig(connectionName)).isNotNull();
+  }
+}
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandIntegrationTest.java
similarity index 64%
copy from geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
copy to geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandIntegrationTest.java
index e6ea066..571ee8f 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandIntegrationTest.java
@@ -23,18 +23,18 @@ import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.connectors.jdbc.internal.ConnectionConfigBuilder;
 import org.apache.geode.connectors.jdbc.internal.ConnectionConfiguration;
 import org.apache.geode.connectors.jdbc.internal.InternalJdbcConnectorService;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.management.cli.Result;
-import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 
 @Category(IntegrationTest.class)
-public class CreateConnectionCommandIntegrationTest {
+public class DestroyConnectionCommandIntegrationTest {
 
   private InternalCache cache;
-  private CreateConnectionCommand createConnectionCommand;
+  private DestroyConnectionCommand destroyConnectionCommand;
 
   private String name;
   private String url;
@@ -42,16 +42,21 @@ public class CreateConnectionCommandIntegrationTest {
   private String password;
   private String[] params;
 
+  private ConnectionConfiguration connectionConfig;
+
   @Before
   public void setup() throws Exception {
     cache = (InternalCache) new CacheFactory().set(ENABLE_CLUSTER_CONFIGURATION, "true").create();
-    createConnectionCommand = new CreateConnectionCommand();
+    destroyConnectionCommand = new DestroyConnectionCommand();
 
     name = "name";
     url = "url";
     user = "user";
     password = "password";
     params = new String[] {"param1:value1", "param2:value2"};
+
+    connectionConfig = new ConnectionConfigBuilder().withName(name).withUrl(url).withUser(user)
+        .withPassword(password).withParameters(params).build();
   }
 
   @After
@@ -60,33 +65,25 @@ public class CreateConnectionCommandIntegrationTest {
   }
 
   @Test
-  public void createsConnectionConfigurationInService() throws Exception {
-    Result result = createConnectionCommand.createConnection(name, url, user, password, params);
+  public void destroysNamedConnection() throws Exception {
+    InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
+    service.createConnectionConfig(connectionConfig);
+    assertThat(service.getConnectionConfig(name)).isSameAs(connectionConfig);
 
+    Result result = destroyConnectionCommand.destroyConnection(name);
     assertThat(result.getStatus()).isSameAs(Result.Status.OK);
 
-    InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
-    ConnectionConfiguration connectionConfig = service.getConnectionConfig(name);
-
-    assertThat(connectionConfig).isNotNull();
-    assertThat(connectionConfig.getName()).isEqualTo(name);
-    assertThat(connectionConfig.getUrl()).isEqualTo(url);
-    assertThat(connectionConfig.getUser()).isEqualTo(user);
-    assertThat(connectionConfig.getPassword()).isEqualTo(password);
-    assertThat(connectionConfig.getConnectionProperties()).containsEntry("param1", "value1")
-        .containsEntry("param2", "value2");
+    assertThat(service.getConnectionConfig(name)).isNull();
   }
 
   @Test
-  public void createsConnectionOnceOnly() throws Exception {
-    createConnectionCommand.createConnection(name, url, user, password, params);
+  public void returnsErrorIfNamedConnectionNotFound() throws Exception {
     InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
+    assertThat(service.getConnectionConfig(name)).isNull();
 
-    ConnectionConfiguration connectionConfig = service.getConnectionConfig(name);
-
-    Result result = createConnectionCommand.createConnection(name, url, user, password, params);
-    assertThat(((CommandResult) result).toJson()).contains("ERROR");
+    Result result = destroyConnectionCommand.destroyConnection(name);
+    assertThat(result.getStatus()).isSameAs(Result.Status.ERROR);
 
-    assertThat(service.getConnectionConfig(name)).isSameAs(connectionConfig);
+    assertThat(service.getConnectionConfig(name)).isNull();
   }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunctionTest.java
similarity index 60%
copy from geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
copy to geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunctionTest.java
index 7452d88..5402808 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunctionTest.java
@@ -21,9 +21,13 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.io.Serializable;
+
+import org.apache.commons.lang.SerializationUtils;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.mockito.ArgumentCaptor;
 
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
@@ -33,55 +37,75 @@ import org.apache.geode.connectors.jdbc.internal.InternalJdbcConnectorService;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.DistributedSystem;
 import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
 import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
-public class CreateConnectionFunctionTest {
+public class DestroyConnectionFunctionTest {
+
+  private static final String connectionName = "connectionName";
 
-  private CreateConnectionFunction function;
-  private FunctionContext<ConnectionConfiguration> context;
+  private DestroyConnectionFunction function;
+  private FunctionContext<String> context;
   private ResultSender<Object> resultSender;
-  private InternalCache cache;
-  private DistributedSystem system;
-  private DistributedMember member;
   private ConnectionConfiguration configuration;
   private InternalJdbcConnectorService service;
 
   @Before
-  public void setup() {
+  public void setUp() {
     configuration = new ConnectionConfigBuilder().build();
 
     context = mock(FunctionContext.class);
     resultSender = mock(ResultSender.class);
-    cache = mock(InternalCache.class);
-    system = mock(DistributedSystem.class);
-    member = mock(DistributedMember.class);
+    InternalCache cache = mock(InternalCache.class);
+    DistributedSystem system = mock(DistributedSystem.class);
+    DistributedMember member = mock(DistributedMember.class);
     service = mock(InternalJdbcConnectorService.class);
 
     when(context.getResultSender()).thenReturn(resultSender);
     when(context.getCache()).thenReturn(cache);
     when(cache.getDistributedSystem()).thenReturn(system);
     when(system.getDistributedMember()).thenReturn(member);
-    when(context.getArguments()).thenReturn(configuration);
+    when(context.getArguments()).thenReturn(connectionName);
     when(cache.getService(eq(InternalJdbcConnectorService.class))).thenReturn(service);
 
-    function = new CreateConnectionFunction();
+    function = new DestroyConnectionFunction();
   }
 
   @Test
-  public void isHAIsFalse() throws Exception {
+  public void isHAReturnsFalse() throws Exception {
     assertThat(function.isHA()).isFalse();
   }
 
   @Test
-  public void getId() throws Exception {
+  public void getIdReturnsNameOfClass() throws Exception {
     assertThat(function.getId()).isEqualTo(function.getClass().getName());
   }
 
   @Test
-  public void execute() throws Exception {
+  public void executeReportsErrorIfConnectionConfigNotFound() throws Exception {
+    function.execute(context);
+
+    ArgumentCaptor<CliFunctionResult> argument = ArgumentCaptor.forClass(CliFunctionResult.class);
+    verify(resultSender, times(1)).lastResult(argument.capture());
+    assertThat(argument.getValue().getErrorMessage())
+        .contains("Connection named \"" + connectionName + "\" not found");
+  }
+
+  @Test
+  public void executeDestroysIfConnectionConfigFound() throws Exception {
+    when(service.getConnectionConfig(eq(connectionName))).thenReturn(configuration);
+
     function.execute(context);
-    verify(service, times(1)).createConnectionConfig(configuration);
+
+    verify(service, times(1)).destroyConnectionConfig(eq(connectionName));
   }
 
+  @Test
+  public void serializes() throws Exception {
+    Serializable original = function;
+    Object copy = SerializationUtils.clone(original);
+
+    assertThat(copy).isNotSameAs(original).isInstanceOf(DestroyConnectionFunction.class);
+  }
 }

-- 
To stop receiving notification emails like this one, please contact
"commits@geode.apache.org" <co...@geode.apache.org>.

[geode] 02/02: GEODE-3781: cleanup tests

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

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

commit 31144f3d8d5469d3897d2871624422dd546e7652
Author: Kirk Lund <kl...@apache.org>
AuthorDate: Tue Nov 28 15:03:52 2017 -0800

    GEODE-3781: cleanup tests
---
 .../geode/connectors/jdbc/JdbcAsyncWriterTest.java |   6 +-
 .../connectors/jdbc/JdbcLoaderIntegrationTest.java |   8 +-
 .../geode/connectors/jdbc/JdbcLoaderTest.java      |  22 ++++-
 .../geode/connectors/jdbc/JdbcWriterTest.java      |  29 ++----
 .../connectors/jdbc/internal/ColumnValueTest.java  |   1 +
 .../jdbc/internal/ConnectionConfigurationTest.java |  47 ++++++---
 .../jdbc/internal/ConnectionManagerUnitTest.java   | 106 +++++++++++----------
 .../jdbc/internal/JdbcConnectorServiceTest.java    |  27 +++---
 .../jdbc/internal/PreparedStatementCacheTest.java  |  31 +++---
 .../jdbc/internal/RegionMappingTest.java           |  71 +++++++++-----
 .../jdbc/internal/SqlStatementFactoryTest.java     |  21 ++--
 .../CreateConnectionCommandIntegrationTest.java    |   7 +-
 .../internal/cli/CreateConnectionFunctionTest.java |  13 ++-
 .../cli/DestroyConnectionCommandDUnitTest.java     |   2 +
 .../DestroyConnectionCommandIntegrationTest.java   |  27 ++----
 .../cli/DestroyConnectionFunctionTest.java         |  11 ++-
 .../internal/xml/ConnectionConfigBuilderTest.java  |  10 +-
 .../jdbc/internal/xml/ElementTypeTest.java         |  25 +++--
 ...onnectorServiceXmlGeneratorIntegrationTest.java |  51 +---------
 .../xml/JdbcConnectorServiceXmlGeneratorTest.java  |   1 +
 .../xml/JdbcConnectorServiceXmlParserTest.java     |   5 +-
 .../internal/xml/JdbcServiceConfigurationTest.java |   7 +-
 .../internal/xml/RegionMappingBuilderTest.java     |   5 +-
 23 files changed, 287 insertions(+), 246 deletions(-)

diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterTest.java
index ef285dd..4b99deb 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcAsyncWriterTest.java
@@ -40,11 +40,15 @@ import org.apache.geode.test.junit.categories.UnitTest;
 public class JdbcAsyncWriterTest {
 
   private SqlHandler sqlHandler;
+  private InternalRegion<Object, Object> region;
+
   private JdbcAsyncWriter writer;
 
   @Before
   public void setup() {
     sqlHandler = mock(SqlHandler.class);
+    region = mock(InternalRegion.class);
+
     writer = new JdbcAsyncWriter(sqlHandler);
   }
 
@@ -87,7 +91,7 @@ public class JdbcAsyncWriterTest {
 
   private AsyncEvent createMockEvent() {
     AsyncEvent event = mock(AsyncEvent.class);
-    when(event.getRegion()).thenReturn(mock(InternalRegion.class));
+    when(event.getRegion()).thenReturn(region);
     return event;
   }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java
index 3905531..1dbd693 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderIntegrationTest.java
@@ -14,6 +14,7 @@
  */
 package org.apache.geode.connectors.jdbc;
 
+import static org.apache.geode.cache.RegionShortcut.REPLICATE;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.sql.Connection;
@@ -30,7 +31,6 @@ import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionFactory;
-import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.connectors.jdbc.internal.SqlHandler;
 import org.apache.geode.connectors.jdbc.internal.TestConfigService;
 import org.apache.geode.connectors.jdbc.internal.TestableConnectionManager;
@@ -98,8 +98,8 @@ public class JdbcLoaderIntegrationTest {
 
   private Region<String, PdxInstance> createRegionWithJDBCLoader(String regionName) {
     JdbcLoader<String, PdxInstance> jdbcLoader = new JdbcLoader<>(createSqlHandler());
-    RegionFactory<String, PdxInstance> rf = cache.createRegionFactory(RegionShortcut.REPLICATE);
-    rf.setCacheLoader(jdbcLoader);
-    return rf.create(regionName);
+    RegionFactory<String, PdxInstance> regionFactory = cache.createRegionFactory(REPLICATE);
+    regionFactory.setCacheLoader(jdbcLoader);
+    return regionFactory.create(regionName);
   }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderTest.java
index 8b08587..620c9f8 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcLoaderTest.java
@@ -20,6 +20,7 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -31,14 +32,25 @@ import org.apache.geode.test.junit.categories.UnitTest;
 @Category(UnitTest.class)
 public class JdbcLoaderTest {
 
+  private SqlHandler sqlHandler;
+  private LoaderHelper loaderHelper;
+
+  private JdbcLoader<Object, Object> loader;
+
+  @Before
+  public void setUp() throws Exception {
+    sqlHandler = mock(SqlHandler.class);
+    loaderHelper = mock(LoaderHelper.class);
+
+    when(loaderHelper.getRegion()).thenReturn(mock(InternalRegion.class));
+
+    loader = new JdbcLoader<>(sqlHandler);
+  }
+
   @Test
   public void loadReadsFromSqlHandler() {
-    SqlHandler sqlHandler = mock(SqlHandler.class);
-    JdbcLoader<Object, Object> loader = new JdbcLoader<>(sqlHandler);
-    LoaderHelper loaderHelper = mock(LoaderHelper.class);
-    when(loaderHelper.getRegion()).thenReturn(mock(InternalRegion.class));
     loader.load(loaderHelper);
+
     verify(sqlHandler, times(1)).read(any(), any());
   }
-
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcWriterTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcWriterTest.java
index 1767c63..92cbf2b 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcWriterTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/JdbcWriterTest.java
@@ -41,24 +41,28 @@ public class JdbcWriterTest {
   private EntryEvent<Object, Object> entryEvent;
   private PdxInstance pdxInstance;
   private SqlHandler sqlHandler;
+  private SerializedCacheValue<Object> serializedNewValue;
+  private RegionEvent<Object, Object> regionEvent;
+
+  private JdbcWriter<Object, Object> writer;
 
   @Before
   public void setUp() {
     entryEvent = mock(EntryEvent.class);
     pdxInstance = mock(PdxInstance.class);
-    SerializedCacheValue<Object> serializedNewValue = mock(SerializedCacheValue.class);
     sqlHandler = mock(SqlHandler.class);
+    serializedNewValue = mock(SerializedCacheValue.class);
+    regionEvent = mock(RegionEvent.class);
 
     when(entryEvent.getRegion()).thenReturn(mock(InternalRegion.class));
     when(entryEvent.getSerializedNewValue()).thenReturn(serializedNewValue);
     when(serializedNewValue.getDeserializedValue()).thenReturn(pdxInstance);
 
+    writer = new JdbcWriter<>(sqlHandler);
   }
 
   @Test
   public void beforeUpdateWithPdxInstanceWritesToSqlHandler() {
-    JdbcWriter<Object, Object> writer = new JdbcWriter<>(sqlHandler);
-
     writer.beforeUpdate(entryEvent);
 
     verify(sqlHandler, times(1)).write(any(), any(), any(), eq(pdxInstance));
@@ -66,16 +70,7 @@ public class JdbcWriterTest {
 
   @Test
   public void beforeUpdateWithoutPdxInstanceWritesToSqlHandler() {
-    EntryEvent<Object, Object> entryEvent = mock(EntryEvent.class);
-    Object value = new Object();
-    SerializedCacheValue<Object> serializedNewValue = mock(SerializedCacheValue.class);
-    SqlHandler sqlHander = mock(SqlHandler.class);
-
-    when(entryEvent.getRegion()).thenReturn(mock(InternalRegion.class));
-    when(entryEvent.getSerializedNewValue()).thenReturn(serializedNewValue);
-    when(serializedNewValue.getDeserializedValue()).thenReturn(value);
-
-    JdbcWriter<Object, Object> writer = new JdbcWriter<>(sqlHander);
+    when(serializedNewValue.getDeserializedValue()).thenReturn(new Object());
 
     assertThatThrownBy(() -> writer.beforeUpdate(entryEvent))
         .isInstanceOf(IllegalArgumentException.class);
@@ -83,8 +78,6 @@ public class JdbcWriterTest {
 
   @Test
   public void beforeCreateWithPdxInstanceWritesToSqlHandler() {
-    JdbcWriter<Object, Object> writer = new JdbcWriter<>(sqlHandler);
-
     writer.beforeCreate(entryEvent);
 
     verify(sqlHandler, times(1)).write(any(), any(), any(), eq(pdxInstance));
@@ -92,8 +85,6 @@ public class JdbcWriterTest {
 
   @Test
   public void beforeDestroyWithPdxInstanceWritesToSqlHandler() {
-    JdbcWriter<Object, Object> writer = new JdbcWriter<>(sqlHandler);
-
     writer.beforeDestroy(entryEvent);
 
     verify(sqlHandler, times(1)).write(any(), any(), any(), eq(pdxInstance));
@@ -101,8 +92,6 @@ public class JdbcWriterTest {
 
   @Test
   public void beforeRegionDestroyDoesNotWriteToSqlHandler() {
-    JdbcWriter<Object, Object> writer = new JdbcWriter<>(sqlHandler);
-
     writer.beforeRegionDestroy(mock(RegionEvent.class));
 
     verifyZeroInteractions(sqlHandler);
@@ -110,8 +99,6 @@ public class JdbcWriterTest {
 
   @Test
   public void beforeRegionClearDoesNotWriteToSqlHandler() {
-    JdbcWriter<Object, Object> writer = new JdbcWriter<>(sqlHandler);
-
     writer.beforeRegionClear(mock(RegionEvent.class));
 
     verifyZeroInteractions(sqlHandler);
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnValueTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnValueTest.java
index a7468fa..2cdde6b 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnValueTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ColumnValueTest.java
@@ -24,6 +24,7 @@ import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class ColumnValueTest {
+
   private static final String COLUMN_NAME = "columnName";
   private static final Object VALUE = new Object();
 
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigurationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigurationTest.java
index ba55bda..ba3d7a4 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigurationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigurationTest.java
@@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -27,9 +28,30 @@ import org.apache.geode.test.junit.categories.UnitTest;
 @Category(UnitTest.class)
 public class ConnectionConfigurationTest {
 
+  private String name;
+  private String url;
+  private String user;
+  private String password;
+  private Map<String, String> parameters;
+
+  private ConnectionConfiguration config;
+
+  @Before
+  public void setUp() {
+    name = "name";
+    url = "url";
+    user = "username";
+    password = "secret";
+
+    parameters = new HashMap<>();
+    parameters.put("param1", "value1");
+    parameters.put("param2", "value2");
+
+    config = new ConnectionConfiguration(null, null, null, null, null);
+  }
+
   @Test
   public void initiatedWithNullValues() {
-    ConnectionConfiguration config = new ConnectionConfiguration(null, null, null, null, null);
     assertThat(config.getName()).isNull();
     assertThat(config.getUrl()).isNull();
     assertThat(config.getUser()).isNull();
@@ -38,39 +60,36 @@ public class ConnectionConfigurationTest {
 
   @Test
   public void hasCorrectName() {
-    String name = "name";
-    ConnectionConfiguration config = new ConnectionConfiguration(name, null, null, null, null);
+    config = new ConnectionConfiguration(name, null, null, null, null);
+
     assertThat(config.getName()).isEqualTo(name);
   }
 
   @Test
   public void hasCorrectUrl() {
-    String url = "url";
-    ConnectionConfiguration config = new ConnectionConfiguration(null, url, null, null, null);
+    config = new ConnectionConfiguration(null, url, null, null, null);
+
     assertThat(config.getUrl()).isEqualTo(url);
   }
 
   @Test
   public void hasCorrectUser() {
-    String user = "user";
-    ConnectionConfiguration config = new ConnectionConfiguration(null, null, user, null, null);
+    config = new ConnectionConfiguration(null, null, user, null, null);
+
     assertThat(config.getUser()).isEqualTo(user);
   }
 
   @Test
   public void hasCorrectPassword() {
-    String password = "password";
-    ConnectionConfiguration config = new ConnectionConfiguration(null, null, null, password, null);
+    config = new ConnectionConfiguration(null, null, null, password, null);
+
     assertThat(config.getPassword()).isEqualTo(password);
   }
 
   @Test
   public void hasCorrectProperties() {
-    Map<String, String> parameters = new HashMap<>();
-    parameters.put("param1", "value1");
-    parameters.put("param2", "value2");
-    ConnectionConfiguration config =
-        new ConnectionConfiguration(null, null, "username", "secret", parameters);
+    config = new ConnectionConfiguration(null, null, "username", "secret", parameters);
+
     assertThat(config.getConnectionProperties()).containsEntry("user", "username")
         .containsEntry("password", "secret").containsEntry("param1", "value1")
         .containsEntry("param2", "value2");
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionManagerUnitTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionManagerUnitTest.java
index b6b4a67..947559d 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionManagerUnitTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionManagerUnitTest.java
@@ -15,6 +15,7 @@
 package org.apache.geode.connectors.jdbc.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.doReturn;
@@ -31,10 +32,8 @@ import java.util.Arrays;
 import java.util.List;
 
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.rules.ExpectedException;
 
 import org.apache.geode.cache.Operation;
 import org.apache.geode.pdx.PdxInstance;
@@ -42,51 +41,56 @@ import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class ConnectionManagerUnitTest {
+
   private static final String REGION_NAME = "testRegion";
   private static final String TABLE_NAME = "testTable";
   private static final String CONFIG_NAME = "configName";
   private static final String KEY_COLUMN = "keyColumn";
 
-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
-
   private JdbcConnectorService configService;
   private ConnectionManager manager;
   private Connection connection;
-  private ConnectionConfiguration connectionConfig;
   private RegionMapping mapping;
-  private Object key = new Object();
-  private PdxInstance value = mock(PdxInstance.class);
 
+  private Object key;
+  private PdxInstance value;
+
+  private ConnectionConfiguration connectionConfig;
 
   @Before
   public void setup() throws Exception {
     configService = mock(JdbcConnectorService.class);
     manager = spy(new ConnectionManager(configService));
     connection = mock(Connection.class);
+    mapping = mock(RegionMapping.class);
+    value = mock(PdxInstance.class);
 
-    connectionConfig = getTestConnectionConfig("name", "url", null, null);
-    doReturn(connection).when(manager).getSQLConnection(connectionConfig);
+    connectionConfig = new ConnectionConfiguration("name", "url", null, null, null);
 
-    mapping = mock(RegionMapping.class);
     when(mapping.getTableName()).thenReturn(TABLE_NAME);
+    doReturn(connection).when(manager).getSQLConnection(connectionConfig);
+
+    key = new Object();
   }
 
   @Test
   public void getsCorrectMapping() {
     manager.getMappingForRegion(REGION_NAME);
+
     verify(configService).getMappingForRegion(REGION_NAME);
   }
 
   @Test
   public void getsCorrectConnectionConfig() {
     manager.getConnectionConfig(CONFIG_NAME);
+
     verify(configService).getConnectionConfig(CONFIG_NAME);
   }
 
   @Test
   public void retrievesANewConnection() throws Exception {
     Connection returnedConnection = manager.getConnection(connectionConfig);
+
     assertThat(returnedConnection).isNotNull().isSameAs(connection);
   }
 
@@ -94,6 +98,7 @@ public class ConnectionManagerUnitTest {
   public void retrievesSameConnectionForSameConnectionConfig() throws Exception {
     Connection returnedConnection = manager.getConnection(connectionConfig);
     Connection secondReturnedConnection = manager.getConnection(connectionConfig);
+
     assertThat(returnedConnection).isNotNull().isSameAs(connection);
     assertThat(secondReturnedConnection).isNotNull().isSameAs(connection);
   }
@@ -102,11 +107,12 @@ public class ConnectionManagerUnitTest {
   public void retrievesDifferentConnectionForEachConfig() throws Exception {
     Connection secondConnection = mock(Connection.class);
     ConnectionConfiguration secondConnectionConfig =
-        getTestConnectionConfig("newName", "url", null, null);
+        new ConnectionConfiguration("newName", "url", null, null, null);
     doReturn(secondConnection).when(manager).getSQLConnection(secondConnectionConfig);
 
     Connection returnedConnection = manager.getConnection(connectionConfig);
     Connection secondReturnedConnection = manager.getConnection(secondConnectionConfig);
+
     assertThat(returnedConnection).isNotNull().isSameAs(connection);
     assertThat(secondReturnedConnection).isNotNull().isSameAs(secondConnection);
     assertThat(returnedConnection).isNotSameAs(secondReturnedConnection);
@@ -116,10 +122,11 @@ public class ConnectionManagerUnitTest {
   public void retrievesANewConnectionIfCachedOneIsClosed() throws Exception {
     manager.getConnection(connectionConfig);
     when(connection.isClosed()).thenReturn(true);
-
     Connection secondConnection = mock(Connection.class);
     doReturn(secondConnection).when(manager).getSQLConnection(connectionConfig);
+
     Connection secondReturnedConnection = manager.getConnection(connectionConfig);
+
     assertThat(secondReturnedConnection).isSameAs(secondConnection);
   }
 
@@ -127,126 +134,129 @@ public class ConnectionManagerUnitTest {
   public void closesAllConnections() throws Exception {
     Connection secondConnection = mock(Connection.class);
     ConnectionConfiguration secondConnectionConfig =
-        getTestConnectionConfig("newName", "url", null, null);
-
+        new ConnectionConfiguration("newName", "url", null, null, null);
     doReturn(secondConnection).when(manager).getSQLConnection(secondConnectionConfig);
     manager.getConnection(connectionConfig);
     manager.getConnection(secondConnectionConfig);
 
     manager.close();
+
     verify(connection).close();
     verify(secondConnection).close();
   }
 
   @Test
   public void returnsCorrectColumnForDestroy() throws Exception {
-    ResultSet primaryKeys = getPrimaryKeysMetadData();
+    ResultSet primaryKeys = getPrimaryKeysMetaData();
     when(primaryKeys.next()).thenReturn(true).thenReturn(false);
 
     List<ColumnValue> columnValueList =
         manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.DESTROY);
+
     assertThat(columnValueList).hasSize(1);
     assertThat(columnValueList.get(0).getColumnName()).isEqualTo(KEY_COLUMN);
   }
 
   @Test
   public void returnsCorrectColumnForGet() throws Exception {
-    ResultSet primaryKeys = getPrimaryKeysMetadData();
+    ResultSet primaryKeys = getPrimaryKeysMetaData();
     when(primaryKeys.next()).thenReturn(true).thenReturn(false);
 
     List<ColumnValue> columnValueList =
         manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET);
+
     assertThat(columnValueList).hasSize(1);
     assertThat(columnValueList.get(0).getColumnName()).isEqualTo(KEY_COLUMN);
   }
 
   @Test
   public void throwsExceptionIfTableHasCompositePrimaryKey() throws Exception {
-    ResultSet primaryKeys = getPrimaryKeysMetadData();
+    ResultSet primaryKeys = getPrimaryKeysMetaData();
     when(primaryKeys.next()).thenReturn(true);
 
-    thrown.expect(IllegalStateException.class);
-    manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET);
+    assertThatThrownBy(
+        () -> manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET))
+            .isInstanceOf(IllegalStateException.class);
   }
 
   @Test
   public void throwsExceptionWhenTwoTablesHasCaseInsensitiveSameName() throws Exception {
     DatabaseMetaData metadata = mock(DatabaseMetaData.class);
-    when(connection.getMetaData()).thenReturn(metadata);
     ResultSet resultSet = mock(ResultSet.class);
+    when(connection.getMetaData()).thenReturn(metadata);
+    when(metadata.getTables(any(), any(), any(), any())).thenReturn(resultSet);
     when(resultSet.next()).thenReturn(true).thenReturn(true).thenReturn(false);
     when(resultSet.getString("TABLE_NAME")).thenReturn(TABLE_NAME);
     when(resultSet.getString("TABLE_NAME")).thenReturn(TABLE_NAME.toUpperCase());
-    when(metadata.getTables(any(), any(), any(), any())).thenReturn(resultSet);
 
-    thrown.expect(IllegalStateException.class);
-    thrown.expectMessage("Duplicate tables that match region name");
-    manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET);
+    assertThatThrownBy(
+        () -> manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET))
+            .isInstanceOf(IllegalStateException.class)
+            .hasMessage("Duplicate tables that match region name");
   }
 
   @Test
   public void throwsExceptionWhenDesiredTableNotFound() throws Exception {
     DatabaseMetaData metadata = mock(DatabaseMetaData.class);
-    when(connection.getMetaData()).thenReturn(metadata);
     ResultSet resultSet = mock(ResultSet.class);
+    when(connection.getMetaData()).thenReturn(metadata);
+    when(metadata.getTables(any(), any(), any(), any())).thenReturn(resultSet);
     when(resultSet.next()).thenReturn(true).thenReturn(false);
     when(resultSet.getString("TABLE_NAME")).thenReturn("otherTable");
-    when(metadata.getTables(any(), any(), any(), any())).thenReturn(resultSet);
 
-    thrown.expect(IllegalStateException.class);
-    manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET);
+    assertThatThrownBy(
+        () -> manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET))
+            .isInstanceOf(IllegalStateException.class);
   }
 
   @Test
   public void throwsExceptionWhenNoPrimaryKeyInTable() throws Exception {
-    ResultSet primaryKeys = getPrimaryKeysMetadData();
+    ResultSet primaryKeys = getPrimaryKeysMetaData();
     when(primaryKeys.next()).thenReturn(false);
 
-    thrown.expect(IllegalStateException.class);
-    manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET);
+    assertThatThrownBy(
+        () -> manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET))
+            .isInstanceOf(IllegalStateException.class);
   }
 
   @Test
   public void throwsExceptionWhenFailsToGetTableMetadata() throws Exception {
     when(connection.getMetaData()).thenThrow(SQLException.class);
 
-    thrown.expect(IllegalStateException.class);
-    manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET);
+    assertThatThrownBy(
+        () -> manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.GET))
+            .isInstanceOf(IllegalStateException.class);
   }
 
   @Test
   public void returnsCorrectColumnsForUpsertOperations() throws Exception {
-    ResultSet primaryKeys = getPrimaryKeysMetadData();
-    when(primaryKeys.next()).thenReturn(true).thenReturn(false);
-
+    ResultSet primaryKeys = getPrimaryKeysMetaData();
     String nonKeyColumn = "otherColumn";
     when(mapping.getColumnNameForField(KEY_COLUMN)).thenReturn(KEY_COLUMN);
     when(mapping.getColumnNameForField(nonKeyColumn)).thenReturn(nonKeyColumn);
+    when(primaryKeys.next()).thenReturn(true).thenReturn(false);
     when(value.getFieldNames()).thenReturn(Arrays.asList(KEY_COLUMN, nonKeyColumn));
 
     List<ColumnValue> columnValueList =
         manager.getColumnToValueList(connectionConfig, mapping, key, value, Operation.UPDATE);
+
     assertThat(columnValueList).hasSize(2);
     assertThat(columnValueList.get(0).getColumnName()).isEqualTo(nonKeyColumn);
     assertThat(columnValueList.get(1).getColumnName()).isEqualTo(KEY_COLUMN);
   }
 
-  private ConnectionConfiguration getTestConnectionConfig(String name, String url, String user,
-      String password) {
-    ConnectionConfiguration config = new ConnectionConfiguration(name, url, user, password, null);
-    return config;
-  }
-
-  private ResultSet getPrimaryKeysMetadData() throws SQLException {
+  private ResultSet getPrimaryKeysMetaData() throws SQLException {
     DatabaseMetaData metadata = mock(DatabaseMetaData.class);
-    when(connection.getMetaData()).thenReturn(metadata);
     ResultSet resultSet = mock(ResultSet.class);
-    when(resultSet.next()).thenReturn(true).thenReturn(false);
-    when(resultSet.getString("TABLE_NAME")).thenReturn(TABLE_NAME);
-    when(metadata.getTables(any(), any(), any(), any())).thenReturn(resultSet);
     ResultSet primaryKeys = mock(ResultSet.class);
+
+    when(connection.getMetaData()).thenReturn(metadata);
+    when(metadata.getTables(any(), any(), any(), any())).thenReturn(resultSet);
     when(metadata.getPrimaryKeys(any(), any(), anyString())).thenReturn(primaryKeys);
     when(primaryKeys.getString("COLUMN_NAME")).thenReturn(KEY_COLUMN);
+    when(resultSet.next()).thenReturn(true).thenReturn(false);
+    when(resultSet.getString("TABLE_NAME")).thenReturn(TABLE_NAME);
+
     return primaryKeys;
   }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java
index d32af99..da5f923 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/JdbcConnectorServiceTest.java
@@ -29,15 +29,29 @@ import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class JdbcConnectorServiceTest {
+
   private static final String TEST_CONFIG_NAME = "testConfig";
   private static final String TEST_REGION_NAME = "testRegion";
 
-  private JdbcConnectorService service = new JdbcConnectorService();
+  private ConnectionConfiguration config;
+  private ConnectionConfiguration config2;
+  private RegionMapping mapping;
+
+  private JdbcConnectorService service;
 
   @Before
   public void setup() {
     InternalCache cache = mock(InternalCache.class);
+    config = mock(ConnectionConfiguration.class);
+    mapping = mock(RegionMapping.class);
+    config2 = mock(ConnectionConfiguration.class);
+
     when(cache.getExtensionPoint()).thenReturn(mock(ExtensionPoint.class));
+    when(config.getName()).thenReturn(TEST_CONFIG_NAME);
+    when(config2.getName()).thenReturn(TEST_CONFIG_NAME);
+    when(mapping.getRegionName()).thenReturn(TEST_REGION_NAME);
+
+    service = new JdbcConnectorService();
     service.init(cache);
   }
 
@@ -53,8 +67,6 @@ public class JdbcConnectorServiceTest {
 
   @Test
   public void returnsCorrectConfig() {
-    ConnectionConfiguration config = mock(ConnectionConfiguration.class);
-    when(config.getName()).thenReturn(TEST_CONFIG_NAME);
     service.createConnectionConfig(config);
 
     assertThat(service.getConnectionConfig(TEST_CONFIG_NAME)).isSameAs(config);
@@ -62,7 +74,6 @@ public class JdbcConnectorServiceTest {
 
   @Test
   public void doesNotReturnConfigWithDifferentName() {
-    ConnectionConfiguration config = mock(ConnectionConfiguration.class);
     when(config.getName()).thenReturn("theOtherConfig");
     service.createConnectionConfig(config);
 
@@ -71,8 +82,6 @@ public class JdbcConnectorServiceTest {
 
   @Test
   public void returnsCorrectMapping() {
-    RegionMapping mapping = mock(RegionMapping.class);
-    when(mapping.getRegionName()).thenReturn(TEST_REGION_NAME);
     service.addOrUpdateRegionMapping(mapping);
 
     assertThat(service.getMappingForRegion(TEST_REGION_NAME)).isSameAs(mapping);
@@ -80,7 +89,6 @@ public class JdbcConnectorServiceTest {
 
   @Test
   public void doesNotReturnMappingForDifferentRegion() {
-    RegionMapping mapping = mock(RegionMapping.class);
     when(mapping.getRegionName()).thenReturn("theOtherMapping");
     service.addOrUpdateRegionMapping(mapping);
 
@@ -89,13 +97,8 @@ public class JdbcConnectorServiceTest {
 
   @Test
   public void createConnectionConfig_throwsIfConnectionExists() {
-    ConnectionConfiguration config = mock(ConnectionConfiguration.class);
-    when(config.getName()).thenReturn(TEST_CONFIG_NAME);
     service.createConnectionConfig(config);
 
-    ConnectionConfiguration config2 = mock(ConnectionConfiguration.class);
-    when(config2.getName()).thenReturn(TEST_CONFIG_NAME);
-
     assertThatThrownBy(() -> service.createConnectionConfig(config2))
         .isInstanceOf(ConnectionConfigExistsException.class).hasMessageContaining(TEST_CONFIG_NAME);
   }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/PreparedStatementCacheTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/PreparedStatementCacheTest.java
index ed78310..798a28d 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/PreparedStatementCacheTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/PreparedStatementCacheTest.java
@@ -14,6 +14,9 @@
  */
 package org.apache.geode.connectors.jdbc.internal;
 
+import static org.apache.geode.cache.Operation.REGION_CLOSE;
+import static org.apache.geode.cache.Operation.UPDATE;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -27,18 +30,13 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.rules.ExpectedException;
 
-import org.apache.geode.cache.Operation;
 import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class PreparedStatementCacheTest {
-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
 
   private PreparedStatementCache cache;
   private Connection connection;
@@ -48,35 +46,40 @@ public class PreparedStatementCacheTest {
   public void setup() throws SQLException {
     cache = new PreparedStatementCache();
     connection = mock(Connection.class);
+
     when(connection.prepareStatement(any())).thenReturn(mock(PreparedStatement.class));
+
     values.add(mock(ColumnValue.class));
   }
 
   @Test
   public void returnsSameStatementForIdenticalInputs() throws Exception {
-    cache.getPreparedStatement(connection, values, "table1", Operation.UPDATE, 1);
-    cache.getPreparedStatement(connection, values, "table1", Operation.UPDATE, 1);
+    cache.getPreparedStatement(connection, values, "table1", UPDATE, 1);
+    cache.getPreparedStatement(connection, values, "table1", UPDATE, 1);
+
     verify(connection, times(1)).prepareStatement(any());
   }
 
   @Test
   public void returnsDifferentStatementForNonIdenticalInputs() throws Exception {
-    cache.getPreparedStatement(connection, values, "table1", Operation.UPDATE, 1);
-    cache.getPreparedStatement(connection, values, "table2", Operation.UPDATE, 1);
+    cache.getPreparedStatement(connection, values, "table1", UPDATE, 1);
+    cache.getPreparedStatement(connection, values, "table2", UPDATE, 1);
+
     verify(connection, times(2)).prepareStatement(any());
   }
 
   @Test()
   public void throwsExceptionIfPreparingStatementFails() throws Exception {
     when(connection.prepareStatement(any())).thenThrow(SQLException.class);
-    thrown.expect(IllegalStateException.class);
-    cache.getPreparedStatement(connection, values, "table1", Operation.UPDATE, 1);
+
+    assertThatThrownBy(() -> cache.getPreparedStatement(connection, values, "table1", UPDATE, 1))
+        .isInstanceOf(IllegalStateException.class);
   }
 
   @Test
   public void throwsExceptionIfInvalidOperationGiven() throws Exception {
-    thrown.expect(IllegalArgumentException.class);
-    cache.getPreparedStatement(connection, values, "table1", Operation.REGION_CLOSE, 1);
-
+    assertThatThrownBy(
+        () -> cache.getPreparedStatement(connection, values, "table1", REGION_CLOSE, 1))
+            .isInstanceOf(IllegalArgumentException.class);
   }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
index 1c7201c..99433e9 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingTest.java
@@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -27,9 +28,31 @@ import org.apache.geode.test.junit.categories.UnitTest;
 @Category(UnitTest.class)
 public class RegionMappingTest {
 
+  private String name;
+  private String fieldName1;
+  private String columnName1;
+  private String fieldName2;
+  private String columnName2;
+
+  private Map<String, String> fieldMap;
+
+  private RegionMapping mapping;
+
+  @Before
+  public void setUp() {
+    name = "name";
+    fieldName1 = "myField1";
+    columnName1 = "myColumn1";
+    fieldName2 = "myField2";
+    columnName2 = "myColumn2";
+
+    fieldMap = new HashMap<>();
+
+    mapping = new RegionMapping(null, null, null, null, false, null);
+  }
+
   @Test
   public void initiatedWithNullValues() {
-    RegionMapping mapping = new RegionMapping(null, null, null, null, false, null);
     assertThat(mapping.getTableName()).isNull();
     assertThat(mapping.getRegionName()).isNull();
     assertThat(mapping.getConnectionConfigName()).isNull();
@@ -38,67 +61,63 @@ public class RegionMappingTest {
 
   @Test
   public void hasCorrectTableName() {
-    String name = "name";
-    RegionMapping mapping = new RegionMapping(null, null, name, null, false, null);
+    mapping = new RegionMapping(null, null, name, null, false, null);
+
     assertThat(mapping.getTableName()).isEqualTo(name);
   }
 
   @Test
   public void hasCorrectRegionName() {
-    String name = "name";
-    RegionMapping mapping = new RegionMapping(name, null, null, null, false, null);
+    mapping = new RegionMapping(name, null, null, null, false, null);
+
     assertThat(mapping.getRegionName()).isEqualTo(name);
   }
 
   @Test
   public void hasCorrectConfigName() {
-    String name = "name";
-    RegionMapping mapping = new RegionMapping(null, null, null, name, false, null);
+    mapping = new RegionMapping(null, null, null, name, false, null);
+
     assertThat(mapping.getConnectionConfigName()).isEqualTo(name);
   }
 
   @Test
   public void hasCorrectPdxClassName() {
-    String name = "name";
-    RegionMapping mapping = new RegionMapping(null, name, null, null, false, null);
+    mapping = new RegionMapping(null, name, null, null, false, null);
+
     assertThat(mapping.getPdxClassName()).isEqualTo(name);
   }
 
   @Test
   public void primaryKeyInValueSetCorrectly() {
-    RegionMapping mapping = new RegionMapping(null, null, null, null, true, null);
+    mapping = new RegionMapping(null, null, null, null, true, null);
+
     assertThat(mapping.isPrimaryKeyInValue()).isTrue();
   }
 
   @Test
   public void returnsFieldNameIfColumnNotMapped() {
-    String fieldName = "myField";
-    Map<String, String> fieldMap = new HashMap<>();
     fieldMap.put("otherField", "column");
-    RegionMapping mapping = new RegionMapping(null, null, null, null, true, fieldMap);
-    assertThat(mapping.getColumnNameForField(fieldName)).isEqualTo(fieldName);
+
+    mapping = new RegionMapping(null, null, null, null, true, fieldMap);
+
+    assertThat(mapping.getColumnNameForField(fieldName1)).isEqualTo(fieldName1);
   }
 
   @Test
   public void returnsMappedColumnNameForField() {
-    String fieldName = "myField";
-    String columnName = "myColumn";
-    Map<String, String> fieldMap = new HashMap<>();
-    fieldMap.put(fieldName, columnName);
-    RegionMapping mapping = new RegionMapping(null, null, null, null, true, fieldMap);
-    assertThat(mapping.getColumnNameForField(fieldName)).isEqualTo(columnName);
+    fieldMap.put(fieldName1, columnName1);
+
+    mapping = new RegionMapping(null, null, null, null, true, fieldMap);
+
+    assertThat(mapping.getColumnNameForField(fieldName1)).isEqualTo(columnName1);
   }
 
   @Test
   public void returnsAllMappings() {
-    String fieldName1 = "myField1";
-    String columnName1 = "myColumn1";
-    String fieldName2 = "myField2";
-    String columnName2 = "myColumn2";
-    Map<String, String> fieldMap = new HashMap<>();
     fieldMap.put(fieldName1, columnName1);
     fieldMap.put(fieldName2, columnName2);
-    RegionMapping mapping = new RegionMapping(null, null, null, null, true, fieldMap);
+
+    mapping = new RegionMapping(null, null, null, null, true, fieldMap);
 
     assertThat(mapping.getFieldToColumnMap().size()).isEqualTo(2);
     assertThat(mapping.getFieldToColumnMap()).containsOnlyKeys(fieldName1, fieldName2);
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java
index abfaf66..988d792 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/SqlStatementFactoryTest.java
@@ -27,6 +27,7 @@ import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class SqlStatementFactoryTest {
+
   private static final String TABLE_NAME = "testTable";
   private static final String KEY_COLUMN_NAME = "column1";
 
@@ -42,39 +43,47 @@ public class SqlStatementFactoryTest {
 
   @Test
   public void getSelectQueryString() throws Exception {
+    String expectedStatement =
+        String.format("SELECT * FROM %s WHERE %s = ?", TABLE_NAME, KEY_COLUMN_NAME);
     List<ColumnValue> keyColumn = new ArrayList<>();
     keyColumn.add(new ColumnValue(true, KEY_COLUMN_NAME, null));
+
     String statement = factory.createSelectQueryString(TABLE_NAME, keyColumn);
-    String expectedStatement =
-        String.format("SELECT * FROM %s WHERE %s = ?", TABLE_NAME, KEY_COLUMN_NAME);
+
     assertThat(statement).isEqualTo(expectedStatement);
   }
 
   @Test
   public void getDestroySqlString() throws Exception {
+    String expectedStatement =
+        String.format("DELETE FROM %s WHERE %s = ?", TABLE_NAME, KEY_COLUMN_NAME);
     List<ColumnValue> keyColumn = new ArrayList<>();
     keyColumn.add(new ColumnValue(true, KEY_COLUMN_NAME, null));
+
     String statement = factory.createDestroySqlString(TABLE_NAME, keyColumn);
-    String expectedStatement =
-        String.format("DELETE FROM %s WHERE %s = ?", TABLE_NAME, KEY_COLUMN_NAME);
+
     assertThat(statement).isEqualTo(expectedStatement);
   }
 
   @Test
   public void getUpdateSqlString() throws Exception {
-    String statement = factory.createUpdateSqlString(TABLE_NAME, columnValues);
     String expectedStatement = String.format("UPDATE %s SET %s = ?, %s = ? WHERE %s = ?",
         TABLE_NAME, columnValues.get(0).getColumnName(), columnValues.get(2).getColumnName(),
         KEY_COLUMN_NAME);
+
+    String statement = factory.createUpdateSqlString(TABLE_NAME, columnValues);
+
     assertThat(statement).isEqualTo(expectedStatement);
   }
 
   @Test
   public void getInsertSqlString() throws Exception {
-    String statement = factory.createInsertSqlString(TABLE_NAME, columnValues);
     String expectedStatement = String.format("INSERT INTO %s (%s, %s, %s) VALUES (?,?,?)",
         TABLE_NAME, columnValues.get(0).getColumnName(), columnValues.get(1).getColumnName(),
         columnValues.get(2).getColumnName());
+
+    String statement = factory.createInsertSqlString(TABLE_NAME, columnValues);
+
     assertThat(statement).isEqualTo(expectedStatement);
   }
 
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
index 3d8933f..e79100c 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionCommandIntegrationTest.java
@@ -43,14 +43,15 @@ public class CreateConnectionCommandIntegrationTest {
 
   @Before
   public void setup() throws Exception {
-    cache = (InternalCache) new CacheFactory().set(ENABLE_CLUSTER_CONFIGURATION, "true").create();
-    createConnectionCommand = new CreateConnectionCommand();
-
     name = "name";
     url = "url";
     user = "user";
     password = "password";
     params = new String[] {"param1:value1", "param2:value2"};
+
+    cache = (InternalCache) new CacheFactory().set(ENABLE_CLUSTER_CONFIGURATION, "true").create();
+
+    createConnectionCommand = new CreateConnectionCommand();
   }
 
   @After
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
index e63d4ed..5ab909d 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/CreateConnectionFunctionTest.java
@@ -45,16 +45,15 @@ import org.apache.geode.test.junit.categories.UnitTest;
 @Category(UnitTest.class)
 public class CreateConnectionFunctionTest {
 
-  private CreateConnectionFunction function;
-  private FunctionContext<ConnectionConfiguration> context;
-  private ResultSender<Object> resultSender;
   private ConnectionConfiguration configuration;
+  private FunctionContext<ConnectionConfiguration> context;
   private InternalJdbcConnectorService service;
+  private ResultSender<Object> resultSender;
+
+  private CreateConnectionFunction function;
 
   @Before
   public void setUp() {
-    configuration = new ConnectionConfigBuilder().build();
-
     context = mock(FunctionContext.class);
     resultSender = mock(ResultSender.class);
     InternalCache cache = mock(InternalCache.class);
@@ -62,6 +61,8 @@ public class CreateConnectionFunctionTest {
     DistributedMember member = mock(DistributedMember.class);
     service = mock(InternalJdbcConnectorService.class);
 
+    configuration = new ConnectionConfigBuilder().build();
+
     when(context.getResultSender()).thenReturn(resultSender);
     when(context.getCache()).thenReturn(cache);
     when(cache.getDistributedSystem()).thenReturn(system);
@@ -85,6 +86,7 @@ public class CreateConnectionFunctionTest {
   @Test
   public void executeCreatesConnectionIfConfigNotFound() throws Exception {
     function.execute(context);
+
     verify(service, times(1)).createConnectionConfig(configuration);
   }
 
@@ -104,6 +106,7 @@ public class CreateConnectionFunctionTest {
   @Test
   public void serializes() throws Exception {
     Serializable original = function;
+
     Object copy = SerializationUtils.clone(original);
 
     assertThat(copy).isNotSameAs(original).isInstanceOf(CreateConnectionFunction.class);
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandDUnitTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandDUnitTest.java
index 9c40ae3..380c6ed 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandDUnitTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandDUnitTest.java
@@ -90,7 +90,9 @@ public class DestroyConnectionCommandDUnitTest implements Serializable {
   private void createConnection() {
     InternalCache cache = LocatorServerStartupRule.getCache();
     InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
+
     service.createConnectionConfig(new ConnectionConfigBuilder().withName(connectionName).build());
+
     assertThat(service.getConnectionConfig(connectionName)).isNotNull();
   }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandIntegrationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandIntegrationTest.java
index 571ee8f..d6a8125 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandIntegrationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionCommandIntegrationTest.java
@@ -33,30 +33,23 @@ import org.apache.geode.test.junit.categories.IntegrationTest;
 @Category(IntegrationTest.class)
 public class DestroyConnectionCommandIntegrationTest {
 
-  private InternalCache cache;
-  private DestroyConnectionCommand destroyConnectionCommand;
-
   private String name;
-  private String url;
-  private String user;
-  private String password;
-  private String[] params;
-
+  private InternalCache cache;
   private ConnectionConfiguration connectionConfig;
 
+  private DestroyConnectionCommand destroyConnectionCommand;
+
   @Before
   public void setup() throws Exception {
-    cache = (InternalCache) new CacheFactory().set(ENABLE_CLUSTER_CONFIGURATION, "true").create();
-    destroyConnectionCommand = new DestroyConnectionCommand();
-
     name = "name";
-    url = "url";
-    user = "user";
-    password = "password";
-    params = new String[] {"param1:value1", "param2:value2"};
 
-    connectionConfig = new ConnectionConfigBuilder().withName(name).withUrl(url).withUser(user)
-        .withPassword(password).withParameters(params).build();
+    String[] params = new String[] {"param1:value1", "param2:value2"};
+    connectionConfig = new ConnectionConfigBuilder().withName(name).withUrl("url").withUser("user")
+        .withPassword("password").withParameters(params).build();
+
+    cache = (InternalCache) new CacheFactory().set(ENABLE_CLUSTER_CONFIGURATION, "true").create();
+
+    destroyConnectionCommand = new DestroyConnectionCommand();
   }
 
   @After
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunctionTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunctionTest.java
index 5402808..b28f499 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunctionTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyConnectionFunctionTest.java
@@ -53,14 +53,12 @@ public class DestroyConnectionFunctionTest {
 
   @Before
   public void setUp() {
-    configuration = new ConnectionConfigBuilder().build();
-
-    context = mock(FunctionContext.class);
-    resultSender = mock(ResultSender.class);
     InternalCache cache = mock(InternalCache.class);
-    DistributedSystem system = mock(DistributedSystem.class);
+    context = mock(FunctionContext.class);
     DistributedMember member = mock(DistributedMember.class);
+    resultSender = mock(ResultSender.class);
     service = mock(InternalJdbcConnectorService.class);
+    DistributedSystem system = mock(DistributedSystem.class);
 
     when(context.getResultSender()).thenReturn(resultSender);
     when(context.getCache()).thenReturn(cache);
@@ -69,6 +67,8 @@ public class DestroyConnectionFunctionTest {
     when(context.getArguments()).thenReturn(connectionName);
     when(cache.getService(eq(InternalJdbcConnectorService.class))).thenReturn(service);
 
+    configuration = new ConnectionConfigBuilder().build();
+
     function = new DestroyConnectionFunction();
   }
 
@@ -104,6 +104,7 @@ public class DestroyConnectionFunctionTest {
   @Test
   public void serializes() throws Exception {
     Serializable original = function;
+
     Object copy = SerializationUtils.clone(original);
 
     assertThat(copy).isNotSameAs(original).isInstanceOf(DestroyConnectionFunction.class);
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ConnectionConfigBuilderTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ConnectionConfigBuilderTest.java
index ffc8abc..cd83690 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ConnectionConfigBuilderTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ConnectionConfigBuilderTest.java
@@ -25,9 +25,11 @@ import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class ConnectionConfigBuilderTest {
+
   @Test
   public void createsAllNullObjectIfNothingSet() {
     ConnectionConfiguration config = new ConnectionConfigBuilder().build();
+
     assertThat(config.getName()).isNull();
     assertThat(config.getUrl()).isNull();
     assertThat(config.getUser()).isNull();
@@ -36,10 +38,10 @@ public class ConnectionConfigBuilderTest {
 
   @Test
   public void createsObjectWithCorrectValues() {
-    ConnectionConfigBuilder builder = new ConnectionConfigBuilder();
-    builder.withName("name").withUrl("url").withUser("user").withPassword("password")
-        .withParameters(new String[] {"param1:value1", "param2:value2"});
-    ConnectionConfiguration config = builder.build();
+    ConnectionConfiguration config = new ConnectionConfigBuilder().withName("name").withUrl("url")
+        .withUser("user").withPassword("password")
+        .withParameters(new String[] {"param1:value1", "param2:value2"}).build();
+
     assertThat(config.getName()).isEqualTo("name");
     assertThat(config.getUrl()).isEqualTo("url");
     assertThat(config.getUser()).isEqualTo("user");
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java
index 4877ac9..dcb0199 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java
@@ -52,17 +52,21 @@ import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class ElementTypeTest {
-  private Stack<Object> stack = new Stack<>();
+
   private Attributes attributes;
   private CacheCreation cacheCreation;
   private ExtensionPoint<Cache> extensionPoint;
+  private Stack<Object> stack;
 
   @Before
   public void setup() {
     attributes = mock(Attributes.class);
     cacheCreation = mock(CacheCreation.class);
     extensionPoint = mock(ExtensionPoint.class);
+
     when(cacheCreation.getExtensionPoint()).thenReturn(extensionPoint);
+
+    stack = new Stack<>();
   }
 
   @Test
@@ -76,13 +80,14 @@ public class ElementTypeTest {
 
   @Test
   public void gettingElementTypeThatDoesNotExistThrowsException() {
-    assertThatThrownBy(() -> ElementType.getTypeFromName("non-existant element"))
+    assertThatThrownBy(() -> ElementType.getTypeFromName("non-existent element"))
         .isInstanceOf(IllegalArgumentException.class);
   }
 
   @Test
   public void startElementConnectionServiceThrowsWithoutCacheCreation() {
     stack.push(new Object());
+
     assertThatThrownBy(() -> CONNECTION_SERVICE.startElement(stack, attributes))
         .isInstanceOf(CacheXmlException.class);
   }
@@ -90,7 +95,9 @@ public class ElementTypeTest {
   @Test
   public void startElementConnectionService() {
     stack.push(cacheCreation);
+
     CONNECTION_SERVICE.startElement(stack, attributes);
+
     verify(extensionPoint, times(1)).addExtension(any(JdbcServiceConfiguration.class));
     assertThat(stack.peek()).isInstanceOf(JdbcServiceConfiguration.class);
   }
@@ -98,13 +105,16 @@ public class ElementTypeTest {
   @Test
   public void endElementConnectionService() {
     stack.push(new Object());
+
     CONNECTION_SERVICE.endElement(stack);
+
     assertThat(stack).isEmpty();
   }
 
   @Test
   public void startElementConnectionThrowsWithoutJdbcServiceConfiguration() {
     stack.push(new Object());
+
     assertThatThrownBy(() -> CONNECTION.startElement(stack, attributes))
         .isInstanceOf(CacheXmlException.class);
   }
@@ -113,15 +123,14 @@ public class ElementTypeTest {
   public void startElementConnection() {
     JdbcServiceConfiguration serviceConfiguration = mock(JdbcServiceConfiguration.class);
     stack.push(serviceConfiguration);
-
     when(attributes.getValue(JdbcConnectorServiceXmlParser.NAME)).thenReturn("connectionName");
     when(attributes.getValue(JdbcConnectorServiceXmlParser.URL)).thenReturn("url");
     when(attributes.getValue(JdbcConnectorServiceXmlParser.USER)).thenReturn("username");
     when(attributes.getValue(JdbcConnectorServiceXmlParser.PASSWORD)).thenReturn("secret");
 
     CONNECTION.startElement(stack, attributes);
-    ConnectionConfiguration config = ((ConnectionConfigBuilder) stack.pop()).build();
 
+    ConnectionConfiguration config = ((ConnectionConfigBuilder) stack.pop()).build();
     assertThat(config.getName()).isEqualTo("connectionName");
     assertThat(config.getUrl()).isEqualTo("url");
     assertThat(config.getUser()).isEqualTo("username");
@@ -144,15 +153,14 @@ public class ElementTypeTest {
   @Test
   public void startElementRegionMappingThrowsWithoutJdbcServiceConfiguration() {
     stack.push(new Object());
+
     assertThatThrownBy(() -> REGION_MAPPING.startElement(stack, attributes))
         .isInstanceOf(CacheXmlException.class);
   }
 
   @Test
   public void startElementRegionMapping() {
-    JdbcServiceConfiguration serviceConfiguration = mock(JdbcServiceConfiguration.class);
-    stack.push(serviceConfiguration);
-
+    stack.push(mock(JdbcServiceConfiguration.class));
     when(attributes.getValue(REGION)).thenReturn("region");
     when(attributes.getValue(CONNECTION_NAME)).thenReturn("connectionName");
     when(attributes.getValue(TABLE)).thenReturn("table");
@@ -185,6 +193,7 @@ public class ElementTypeTest {
   @Test
   public void startElementFieldMappingThrowsWithoutRegionMappingBuilder() {
     stack.push(new Object());
+
     assertThatThrownBy(() -> FIELD_MAPPING.startElement(stack, attributes))
         .isInstanceOf(CacheXmlException.class);
   }
@@ -197,8 +206,8 @@ public class ElementTypeTest {
     when(attributes.getValue(COLUMN_NAME)).thenReturn("columnName");
 
     ElementType.FIELD_MAPPING.startElement(stack, attributes);
-    RegionMapping regionMapping = ((RegionMappingBuilder) stack.pop()).build();
 
+    RegionMapping regionMapping = ((RegionMappingBuilder) stack.pop()).build();
     assertThat(regionMapping.getColumnNameForField("fieldName")).isEqualTo("columnName");
   }
 
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java
index 6280d6f..dfe0da5 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java
@@ -35,14 +35,10 @@ import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.PrintWriter;
-import java.nio.file.Files;
 
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
-import javax.xml.xpath.XPath;
-import javax.xml.xpath.XPathConstants;
-import javax.xml.xpath.XPathFactory;
 
 import org.junit.After;
 import org.junit.Before;
@@ -61,10 +57,7 @@ import org.apache.geode.connectors.jdbc.internal.ConnectionConfiguration;
 import org.apache.geode.connectors.jdbc.internal.InternalJdbcConnectorService;
 import org.apache.geode.connectors.jdbc.internal.RegionMapping;
 import org.apache.geode.internal.cache.InternalCache;
-import org.apache.geode.internal.cache.xmlcache.CacheXml;
 import org.apache.geode.internal.cache.xmlcache.CacheXmlGenerator;
-import org.apache.geode.internal.cache.xmlcache.CacheXmlParser;
-import org.apache.geode.management.internal.configuration.utils.XmlUtils;
 import org.apache.geode.test.junit.categories.IntegrationTest;
 
 @Category(IntegrationTest.class)
@@ -96,7 +89,9 @@ public class JdbcConnectorServiceXmlGeneratorIntegrationTest {
   @Test
   public void serviceWithoutInformationDoesNotPersist() throws Exception {
     cache.getService(InternalJdbcConnectorService.class);
+
     generateXml();
+
     Document document = getCacheXmlDocument();
     NodeList elements = getElementsByName(document, ElementType.CONNECTION_SERVICE);
     assertThat(elements.getLength()).isZero();
@@ -168,50 +163,16 @@ public class JdbcConnectorServiceXmlGeneratorIntegrationTest {
         .withUser("username").withPassword("secret").build();
     service.createConnectionConfig(config);
     generateXml();
-
     cache.close();
+
     cache = (InternalCache) new CacheFactory().set(CACHE_XML_FILE, cacheXmlFile.getAbsolutePath())
         .create();
 
     service = cache.getService(InternalJdbcConnectorService.class);
-
     assertThat(service.getConnectionConfig("name")).isEqualTo(config);
   }
 
   @Test
-  public void generatedXmlWithConnectionConfigurationCanBeXPathed() throws Exception {
-    InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
-    ConnectionConfiguration config = new ConnectionConfigBuilder().withName("name").withUrl("url")
-        .withUser("username").withPassword("secret").build();
-    service.createConnectionConfig(config);
-    generateXml();
-
-    for (String line : Files.readAllLines(cacheXmlFile.toPath())) {
-      System.out.println(line);
-    }
-
-    DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-    factory.setNamespaceAware(true);
-
-    DocumentBuilder builder = factory.newDocumentBuilder();
-    builder.setEntityResolver(new CacheXmlParser());
-
-    Document document = builder.parse(cacheXmlFile);
-    System.out.println(document.getDocumentElement());
-    XmlUtils.XPathContext xpathContext = new XmlUtils.XPathContext();
-    xpathContext.addNamespace(CacheXml.PREFIX, CacheXml.GEODE_NAMESPACE);
-    xpathContext.addNamespace(PREFIX, NAMESPACE); // TODO: wrap this line with conditional
-    // Create an XPathContext here
-    XPath xpath = XPathFactory.newInstance().newXPath();
-    xpath.setNamespaceContext(xpathContext);
-    Object result = xpath.evaluate("//cache/jdbc:connector-service", document, XPathConstants.NODE);
-    // Node element = XmlUtils.querySingleElement(document, "//cache/jdbc:connector-service",
-    // xpathContext);
-    // Must copy to preserve namespaces.
-    System.out.println("RESULT = " + XmlUtils.elementToString((Element) result));
-  }
-
-  @Test
   public void generatedXmlWithRegionMappingCanBeParsed() throws Exception {
     InternalJdbcConnectorService service = cache.getService(InternalJdbcConnectorService.class);
     RegionMapping mapping = new RegionMappingBuilder().withRegionName("region")
@@ -220,13 +181,12 @@ public class JdbcConnectorServiceXmlGeneratorIntegrationTest {
         .withFieldToColumnMapping("field2", "columnMapping2").build();
     service.addOrUpdateRegionMapping(mapping);
     generateXml();
-
     cache.close();
+
     cache = (InternalCache) new CacheFactory().set(CACHE_XML_FILE, cacheXmlFile.getAbsolutePath())
         .create();
 
     service = cache.getService(InternalJdbcConnectorService.class);
-
     assertThat(service.getMappingForRegion("region")).isEqualTo(mapping);
   }
 
@@ -242,13 +202,12 @@ public class JdbcConnectorServiceXmlGeneratorIntegrationTest {
         .withFieldToColumnMapping("field2", "columnMapping2").build();
     service.addOrUpdateRegionMapping(mapping);
     generateXml();
-
     cache.close();
+
     cache = (InternalCache) new CacheFactory().set(CACHE_XML_FILE, cacheXmlFile.getAbsolutePath())
         .create();
 
     service = cache.getService(InternalJdbcConnectorService.class);
-
     assertThat(service.getConnectionConfig("name")).isEqualTo(config);
     assertThat(service.getMappingForRegion("region")).isEqualTo(mapping);
   }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorTest.java
index b91bd91..3ae5587 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorTest.java
@@ -28,6 +28,7 @@ public class JdbcConnectorServiceXmlGeneratorTest {
   @Test
   public void returnsCorrectNamespace() {
     JdbcConnectorServiceXmlGenerator generator = new JdbcConnectorServiceXmlGenerator(null, null);
+
     assertThat(generator.getNamespaceUri()).isEqualTo(NAMESPACE);
   }
 }
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlParserTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlParserTest.java
index 95a4d45..7db489b 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlParserTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlParserTest.java
@@ -36,17 +36,20 @@ import org.apache.geode.test.junit.categories.UnitTest;
 @Category(UnitTest.class)
 public class JdbcConnectorServiceXmlParserTest {
 
-  private Stack<Object> stack = new Stack<>();
   private Attributes attributes;
   private CacheCreation cacheCreation;
   private ExtensionPoint<Cache> extensionPoint;
+  private Stack<Object> stack;
 
   @Before
   public void setup() {
     attributes = mock(Attributes.class);
     cacheCreation = mock(CacheCreation.class);
     extensionPoint = mock(ExtensionPoint.class);
+
     when(cacheCreation.getExtensionPoint()).thenReturn(extensionPoint);
+
+    stack = new Stack<>();
   }
 
   @Test
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcServiceConfigurationTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcServiceConfigurationTest.java
index 13aff42..a268e69 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcServiceConfigurationTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcServiceConfigurationTest.java
@@ -37,8 +37,6 @@ import org.apache.geode.test.junit.categories.UnitTest;
 @Category(UnitTest.class)
 public class JdbcServiceConfigurationTest {
 
-  private JdbcServiceConfiguration configuration;
-
   private InternalCache cache;
   private InternalJdbcConnectorService service;
   private ConnectionConfiguration connection1;
@@ -46,15 +44,17 @@ public class JdbcServiceConfigurationTest {
   private RegionMapping mapping1;
   private RegionMapping mapping2;
 
+  private JdbcServiceConfiguration configuration;
+
   @Before
   public void setUp() {
     connection1 = mock(ConnectionConfiguration.class);
     connection2 = mock(ConnectionConfiguration.class);
     mapping1 = mock(RegionMapping.class);
     mapping2 = mock(RegionMapping.class);
-
     service = mock(InternalJdbcConnectorService.class);
     cache = mock(InternalCache.class);
+
     when(cache.getService(InternalJdbcConnectorService.class)).thenReturn(service);
 
     configuration = new JdbcServiceConfiguration();
@@ -108,6 +108,7 @@ public class JdbcServiceConfigurationTest {
   @Test
   public void onCreateWithNoConnectionsOrMappings() {
     configuration.onCreate(cache, cache);
+
     verifyZeroInteractions(service);
   }
 
diff --git a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/RegionMappingBuilderTest.java b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/RegionMappingBuilderTest.java
index 9953d95..c412319 100644
--- a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/RegionMappingBuilderTest.java
+++ b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/RegionMappingBuilderTest.java
@@ -38,9 +38,8 @@ public class RegionMappingBuilderTest {
 
   @Test
   public void createsMappingWithSpecifiedValues() {
-    RegionMappingBuilder builder = new RegionMappingBuilder();
-    RegionMapping regionMapping = builder.withTableName("tableName").withRegionName("regionName")
-        .withPrimaryKeyInValue("true").withPdxClassName("pdxClassName")
+    RegionMapping regionMapping = new RegionMappingBuilder().withTableName("tableName")
+        .withRegionName("regionName").withPrimaryKeyInValue("true").withPdxClassName("pdxClassName")
         .withConnectionConfigName("configName").withFieldToColumnMapping("fieldName", "columnName")
         .build();
 

-- 
To stop receiving notification emails like this one, please contact
"commits@geode.apache.org" <co...@geode.apache.org>.