You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2018/01/31 14:07:52 UTC

qpid-broker-j git commit: QPID-8066: [Broker-J] Fix jdbc store deletion on removal of Virtual Host Node or/and Virtual Host

Repository: qpid-broker-j
Updated Branches:
  refs/heads/master e0e110af3 -> bce93dfb6


QPID-8066: [Broker-J] Fix jdbc store deletion on removal of Virtual Host Node or/and Virtual Host


Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/bce93dfb
Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/bce93dfb
Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/bce93dfb

Branch: refs/heads/master
Commit: bce93dfb642ef30a87227377d71c588b084d7d04
Parents: e0e110a
Author: Alex Rudyy <or...@apache.org>
Authored: Wed Jan 31 14:03:03 2018 +0000
Committer: Alex Rudyy <or...@apache.org>
Committed: Wed Jan 31 14:03:03 2018 +0000

----------------------------------------------------------------------
 ...stractDurableConfigurationStoreTestCase.java | 12 ++-
 .../qpid/server/store/MessageStoreTestCase.java |  7 +-
 .../store/derby/DerbyConfigurationStore.java    | 13 +++
 .../jdbc/AbstractJDBCConfigurationStore.java    | 47 ++--------
 .../store/jdbc/AbstractJDBCMessageStore.java    | 28 +-----
 .../jdbc/GenericJDBCConfigurationStore.java     | 85 +++++++++---------
 .../store/jdbc/GenericJDBCMessageStore.java     | 74 ++++++++-------
 .../qpid/server/store/jdbc/JdbcUtils.java       | 62 +++++++++++++
 .../jdbc/GenericJDBCConfigurationStoreTest.java | 29 +++++-
 .../server/store/jdbc/JDBCMessageStoreTest.java | 95 ++++----------------
 .../qpid/server/store/jdbc/TestJdbcUtils.java   | 94 +++++++++++++++++++
 11 files changed, 321 insertions(+), 225 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-core/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java
----------------------------------------------------------------------
diff --git a/broker-core/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java b/broker-core/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java
index 53185f4..4aa4c8c 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java
@@ -80,7 +80,7 @@ public abstract class AbstractDurableConfigurationStoreTestCase extends QpidTest
     private DurableConfigurationStore _configStore;
     private ConfiguredObjectFactoryImpl _factory;
 
-    private ConfiguredObject<?> _parent;
+    private VirtualHostNode<?> _parent;
 
     private ConfiguredObjectRecord _rootRecord;
 
@@ -120,6 +120,16 @@ public abstract class AbstractDurableConfigurationStoreTestCase extends QpidTest
         _configStore.create(_rootRecord);
     }
 
+    protected VirtualHostNode<?> getVirtualHostNode()
+    {
+        return _parent;
+    }
+
+    protected DurableConfigurationStore getConfigurationStore()
+    {
+        return _configStore;
+    }
+
     protected abstract VirtualHostNode createVirtualHostNode(String storeLocation, ConfiguredObjectFactory factory);
 
     @Override

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-core/src/test/java/org/apache/qpid/server/store/MessageStoreTestCase.java
----------------------------------------------------------------------
diff --git a/broker-core/src/test/java/org/apache/qpid/server/store/MessageStoreTestCase.java b/broker-core/src/test/java/org/apache/qpid/server/store/MessageStoreTestCase.java
index 7f7f16a..98093dd 100644
--- a/broker-core/src/test/java/org/apache/qpid/server/store/MessageStoreTestCase.java
+++ b/broker-core/src/test/java/org/apache/qpid/server/store/MessageStoreTestCase.java
@@ -51,7 +51,7 @@ import org.apache.qpid.test.utils.QpidTestCase;
 public abstract class MessageStoreTestCase extends QpidTestCase
 {
     private MessageStore _store;
-    private ConfiguredObject<?> _parent;
+    private VirtualHost<?> _parent;
     private MessageStore.MessageStoreReader _storeReader;
     private static final int BUFFER_SIZE = 10;
     private static final int POOL_SIZE = 20;
@@ -81,6 +81,11 @@ public abstract class MessageStoreTestCase extends QpidTestCase
         super.tearDown();
     }
 
+    protected VirtualHost<?> getVirtualHost()
+    {
+        return _parent;
+    }
+
     protected abstract VirtualHost createVirtualHost();
 
     protected abstract MessageStore createMessageStore();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java
----------------------------------------------------------------------
diff --git a/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java b/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java
index 1b6a2b4..ee6c8a2 100644
--- a/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java
+++ b/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java
@@ -231,6 +231,19 @@ public class DerbyConfigurationStore extends AbstractJDBCConfigurationStore
         }
 
         @Override
+        public void onDelete(final ConfiguredObject<?> parent)
+        {
+            try(Connection connection = DerbyConfigurationStore.this.getConnection())
+            {
+                onDelete(connection);
+            }
+            catch (SQLException e)
+            {
+                throw new StoreException("Cannot get connection to perform deletion", e);
+            }
+        }
+
+        @Override
         protected Logger getLogger()
         {
             return DerbyConfigurationStore.this.getLogger();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCConfigurationStore.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCConfigurationStore.java b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCConfigurationStore.java
index e10935a..1aac9c7 100644
--- a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCConfigurationStore.java
+++ b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCConfigurationStore.java
@@ -139,12 +139,12 @@ public abstract class AbstractJDBCConfigurationStore implements MessageStoreProv
         return _tableNamePrefix + CONFIGURATION_VERSION_TABLE_NAME_SUFFIX;
     }
 
-    private String getConfiguredObjectsTableName()
+    String getConfiguredObjectsTableName()
     {
         return _tableNamePrefix + CONFIGURED_OBJECTS_TABLE_NAME_SUFFIX;
     }
 
-    private String getConfiguredObjectHierarchyTableName()
+    String getConfiguredObjectHierarchyTableName()
     {
         return _tableNamePrefix + CONFIGURED_OBJECT_HIERARCHY_TABLE_NAME_SUFFIX;
     }
@@ -819,44 +819,11 @@ public abstract class AbstractJDBCConfigurationStore implements MessageStoreProv
 
     protected abstract String getBlobAsString(ResultSet rs, int col) throws SQLException;
 
-    @Override
-    public void onDelete(ConfiguredObject<?> parent)
+    void onDelete(final Connection conn) throws SQLException
     {
-        // TODO should probably check we are closed
-        try
-        {
-            Connection conn = newAutoCommitConnection();
-            try
-            {
-
-                for (String tableName : Arrays.asList(
-                        getConfiguredObjectsTableName(),
-                        getConfiguredObjectHierarchyTableName()))
-                {
-                    Statement stmt = conn.createStatement();
-                    try
-                    {
-                        stmt.execute("DROP TABLE " +  tableName);
-                    }
-                    catch(SQLException e)
-                    {
-                        getLogger().warn("Failed to drop table '" + tableName + "' :" + e);
-                    }
-                    finally
-                    {
-                        stmt.close();
-                    }
-                }
-            }
-            finally
-            {
-                conn.close();
-            }
-        }
-        catch(SQLException e)
-        {
-            getLogger().error("Exception while deleting store tables", e);
-        }
+        JdbcUtils.dropTables(conn,
+                             getLogger(),
+                             Arrays.asList(getConfiguredObjectsTableName(), getConfiguredObjectHierarchyTableName()));
     }
 
     private static final class ConfiguredObjectRecordImpl implements ConfiguredObjectRecord
@@ -913,7 +880,7 @@ public abstract class AbstractJDBCConfigurationStore implements MessageStoreProv
         }
     }
 
-    private final void assertState(State state)
+    protected final void assertState(State state)
     {
         synchronized (_lock)
         {

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCMessageStore.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCMessageStore.java b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCMessageStore.java
index aeff70e..c12e223 100644
--- a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCMessageStore.java
+++ b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCMessageStore.java
@@ -700,10 +700,6 @@ public abstract class AbstractJDBCMessageStore implements MessageStore
 
     }
 
-    public void onOpen(final ConfiguredObject<?> parent)
-    {
-    }
-
     protected void setTablePrefix(final String tablePrefix)
     {
         _tablePrefix = tablePrefix == null ? "" : tablePrefix;
@@ -1818,29 +1814,9 @@ public abstract class AbstractJDBCMessageStore implements MessageStore
 
     protected abstract void storedSizeChange(int storeSizeIncrease);
 
-    @Override
-    public void onDelete(ConfiguredObject<?> parent)
+    protected void onDelete(final Connection conn)
     {
-        // TODO should probably check we are closed
-        try (Connection conn = newAutoCommitConnection())
-        {
-
-            for (String tableName : getTableNames())
-            {
-                try (Statement stmt = conn.createStatement())
-                {
-                    stmt.execute("DROP TABLE " + tableName);
-                }
-                catch (SQLException e)
-                {
-                    getLogger().warn("Failed to drop table '{}'", tableName, e);
-                }
-            }
-        }
-        catch (SQLException e)
-        {
-            getLogger().error("Exception while deleting store tables", e);
-        }
+        JdbcUtils.dropTables(conn, getLogger(), getTableNames());
     }
 
     public List<String> getTableNames()

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStore.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStore.java b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStore.java
index a4d537f..a5f5fe1 100644
--- a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStore.java
+++ b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStore.java
@@ -30,10 +30,6 @@ import java.sql.Blob;
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -96,43 +92,7 @@ public class GenericJDBCConfigurationStore extends AbstractJDBCConfigurationStor
                              + " Using settings : " + details);
         }
 
-        String connectionPoolType = settings.getConnectionPoolType() == null
-                ? DefaultConnectionProviderFactory.TYPE
-                : settings.getConnectionPoolType();
-
-        JDBCConnectionProviderFactory connectionProviderFactory =
-                JDBCConnectionProviderFactory.FACTORIES.get(connectionPoolType);
-        if (connectionProviderFactory == null)
-        {
-            LOGGER.warn("Unknown connection pool type: "
-                        + connectionPoolType
-                        + ".  no connection pooling will be used");
-            connectionProviderFactory = new DefaultConnectionProviderFactory();
-        }
-
-        try
-        {
-            Map<String, String> providerAttributes = new HashMap<>();
-            Set<String> providerAttributeNames =
-                    new HashSet<String>(connectionProviderFactory.getProviderAttributeNames());
-            providerAttributeNames.retainAll(parent.getContextKeys(false));
-            for (String attr : providerAttributeNames)
-            {
-                providerAttributes.put(attr, parent.getContextValue(String.class, attr));
-            }
-
-            _connectionProvider = connectionProviderFactory.getConnectionProvider(_connectionURL,
-                                                                                  settings.getUsername(),
-                                                                                  settings.getPassword(),
-                                                                                  providerAttributes);
-        }
-        catch (SQLException e)
-        {
-            throw new StoreException(String.format(
-                    "Failed to create connection provider for connectionUrl: '%s' and username: '%s'",
-                    _connectionURL,
-                    settings.getUsername()), e);
-        }
+        _connectionProvider = JdbcUtils.createConnectionProvider(parent, LOGGER);
         _blobType = details.getBlobType();
         _varBinaryType = details.getVarBinaryType();
         _useBytesMethodsForBlob = details.isUseBytesMethodsForBlob();
@@ -169,6 +129,36 @@ public class GenericJDBCConfigurationStore extends AbstractJDBCConfigurationStor
     }
 
     @Override
+    public void onDelete(final ConfiguredObject<?> parent)
+    {
+        assertState(CLOSED);
+        ConnectionProvider connectionProvider = JdbcUtils.createConnectionProvider(parent, LOGGER);
+        try
+        {
+            try (Connection conn = connectionProvider.getConnection())
+            {
+                conn.setAutoCommit(true);
+                onDelete(conn);
+            }
+            catch (SQLException e)
+            {
+                getLogger().error("Exception while deleting store tables", e);
+            }
+        }
+        finally
+        {
+            try
+            {
+                connectionProvider.close();
+            }
+            catch (SQLException e)
+            {
+                LOGGER.warn("Unable to close connection provider ", e);
+            }
+        }
+    }
+
+    @Override
     protected String getSqlBlobType()
     {
         return _blobType;
@@ -277,6 +267,19 @@ public class GenericJDBCConfigurationStore extends AbstractJDBCConfigurationStor
         }
 
         @Override
+        public void onDelete(final ConfiguredObject<?> parent)
+        {
+            try(Connection connection = GenericJDBCConfigurationStore.this.getConnection())
+            {
+                onDelete(connection);
+            }
+            catch (SQLException e)
+            {
+                throw new StoreException("Cannot get connection to perform deletion", e);
+            }
+        }
+
+        @Override
         protected Logger getLogger()
         {
             return GenericJDBCConfigurationStore.this.getLogger();

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCMessageStore.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCMessageStore.java b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCMessageStore.java
index 8ac3cec..422b5df 100644
--- a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCMessageStore.java
+++ b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/GenericJDBCMessageStore.java
@@ -21,6 +21,8 @@
 package org.apache.qpid.server.store.jdbc;
 
 
+import static org.apache.qpid.server.store.jdbc.JdbcUtils.createConnectionProvider;
+
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.InputStream;
@@ -28,10 +30,6 @@ import java.sql.Blob;
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -78,40 +76,7 @@ public class GenericJDBCMessageStore extends GenericAbstractJDBCMessageStore
         _varBinaryType = details.getVarBinaryType();
         _useBytesMethodsForBlob = details.isUseBytesMethodsForBlob();
         _bigIntType = details.getBigintType();
-
-        String connectionPoolType = settings.getConnectionPoolType() == null ? DefaultConnectionProviderFactory.TYPE : settings.getConnectionPoolType();
-
-        JDBCConnectionProviderFactory connectionProviderFactory =
-                JDBCConnectionProviderFactory.FACTORIES.get(connectionPoolType);
-        if(connectionProviderFactory == null)
-        {
-            LOGGER.warn("Unknown connection pool type: " + connectionPoolType + ".  No connection pooling will be used");
-            connectionProviderFactory = new DefaultConnectionProviderFactory();
-        }
-
-        try
-        {
-            Map<String, String> providerAttributes = new HashMap<>();
-            Set<String> providerAttributeNames = new HashSet<>(connectionProviderFactory.getProviderAttributeNames());
-            providerAttributeNames.retainAll(parent.getContextKeys(false));
-            for(String attr : providerAttributeNames)
-            {
-                providerAttributes.put(attr, parent.getContextValue(String.class, attr));
-            }
-
-            _connectionProvider = connectionProviderFactory.getConnectionProvider(_connectionURL,
-                                                                                  settings.getUsername(),
-                                                                                  settings.getPassword(),
-                                                                                  providerAttributes);
-        }
-        catch (SQLException e)
-        {
-            throw new StoreException(String.format(
-                    "Failed to create connection provider for connectionUrl: '%s' and username: '%s'",
-                    _connectionURL,
-                    settings.getUsername()), e);
-        }
-
+        _connectionProvider = createConnectionProvider(parent, LOGGER);
     }
 
     @Override
@@ -191,4 +156,37 @@ public class GenericJDBCMessageStore extends GenericAbstractJDBCMessageStore
         return null;
     }
 
+    @Override
+    public void onDelete(final ConfiguredObject<?> parent)
+    {
+        if (isMessageStoreOpen())
+        {
+            throw new IllegalStateException("Cannot delete the store as the provided message store is still open");
+        }
+
+        ConnectionProvider connectionProvider = JdbcUtils.createConnectionProvider(parent, LOGGER);
+        try
+        {
+            try (Connection conn = connectionProvider.getConnection())
+            {
+                conn.setAutoCommit(true);
+                onDelete(conn);
+            }
+            catch (SQLException e)
+            {
+                getLogger().error("Exception while deleting store tables", e);
+            }
+        }
+        finally
+        {
+            try
+            {
+                connectionProvider.close();
+            }
+            catch (SQLException e)
+            {
+                LOGGER.warn("Unable to close connection provider ", e);
+            }
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JdbcUtils.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JdbcUtils.java b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JdbcUtils.java
index 9e7710c..b6d9f22 100644
--- a/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JdbcUtils.java
+++ b/broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/JdbcUtils.java
@@ -24,9 +24,18 @@ import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 
 import org.slf4j.Logger;
 
+import org.apache.qpid.server.model.ConfiguredObject;
+import org.apache.qpid.server.store.StoreException;
+
 public class JdbcUtils
 {
     public static void closeConnection(final Connection conn, final Logger logger)
@@ -71,6 +80,59 @@ public class JdbcUtils
 
     }
 
+    static ConnectionProvider createConnectionProvider(final ConfiguredObject<?> parent, final Logger logger)
+    {
+        JDBCSettings settings = (JDBCSettings) parent;
+        String connectionPoolType = settings.getConnectionPoolType() == null
+                ? DefaultConnectionProviderFactory.TYPE
+                : settings.getConnectionPoolType();
+
+        JDBCConnectionProviderFactory connectionProviderFactory =
+                JDBCConnectionProviderFactory.FACTORIES.get(connectionPoolType);
+        if (connectionProviderFactory == null)
+        {
+            logger.warn("Unknown connection pool type: {}.  No connection pooling will be used", connectionPoolType);
+            connectionProviderFactory = new DefaultConnectionProviderFactory();
+        }
+
+        try
+        {
+            Map<String, String> providerAttributes = new HashMap<>();
+            Set<String> providerAttributeNames = new HashSet<>(connectionProviderFactory.getProviderAttributeNames());
+            providerAttributeNames.retainAll(parent.getContextKeys(false));
+            for (String attr : providerAttributeNames)
+            {
+                providerAttributes.put(attr, parent.getContextValue(String.class, attr));
+            }
+
+            return connectionProviderFactory.getConnectionProvider(settings.getConnectionUrl(),
+                                                                   settings.getUsername(),
+                                                                   settings.getPassword(),
+                                                                   providerAttributes);
+        }
+        catch (SQLException e)
+        {
+            throw new StoreException(String.format(
+                    "Failed to create connection provider for connectionUrl: '%s' and username: '%s'",
+                    settings.getConnectionUrl(),
+                    settings.getUsername()), e);
+        }
+    }
+
+    static void dropTables(final Connection connection, final Logger logger, Collection<String> tableNames)
+    {
+        for (String tableName : tableNames)
+        {
+            try(Statement statement = connection.createStatement())
+            {
+                statement.execute(String.format("DROP TABLE %s",  tableName));
+            }
+            catch(SQLException e)
+            {
+                logger.warn("Failed to drop table '" + tableName + "' :" + e);
+            }
+        }
+    }
     private static boolean tableExistsCase(final String tableName, final DatabaseMetaData metaData) throws SQLException
     {
         try (ResultSet rs = metaData.getTables(null, null, tableName, null))

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStoreTest.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStoreTest.java b/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStoreTest.java
index 76f397b..c5d7de4 100644
--- a/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStoreTest.java
+++ b/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/GenericJDBCConfigurationStoreTest.java
@@ -21,9 +21,16 @@
 package org.apache.qpid.server.store.jdbc;
 
 
+import static org.apache.qpid.server.store.jdbc.TestJdbcUtils.assertTablesExistence;
+import static org.apache.qpid.server.store.jdbc.TestJdbcUtils.getTableNames;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.Collection;
+
 import org.apache.qpid.server.model.ConfiguredObjectFactory;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.model.VirtualHostNode;
@@ -47,11 +54,26 @@ public class GenericJDBCConfigurationStoreTest extends AbstractDurableConfigurat
         {
             if (_connectionURL != null)
             {
-                JDBCMessageStoreTest.shutdownDerby(_connectionURL);
+                TestJdbcUtils.shutdownDerby(_connectionURL);
             }
         }
     }
 
+    public void testOnDelete() throws Exception
+    {
+        try(Connection connection = openConnection())
+        {
+            GenericJDBCConfigurationStore store = (GenericJDBCConfigurationStore) getConfigurationStore();
+            Collection<String> expectedTables = Arrays.asList(store.getConfiguredObjectHierarchyTableName(),
+                                                              store.getConfiguredObjectsTableName());
+            assertTablesExistence(expectedTables, getTableNames(connection), true);
+            store.closeConfigurationStore();
+            assertTablesExistence(expectedTables, getTableNames(connection), true);
+            store.onDelete(getVirtualHostNode());
+            assertTablesExistence(expectedTables, getTableNames(connection), false);
+        }
+    }
+
     @Override
     protected VirtualHostNode createVirtualHostNode(final String storeLocation, final ConfiguredObjectFactory factory)
     {
@@ -66,4 +88,9 @@ public class GenericJDBCConfigurationStoreTest extends AbstractDurableConfigurat
     {
         return new GenericJDBCConfigurationStore(VirtualHost.class);
     }
+
+    private Connection openConnection() throws SQLException
+    {
+        return TestJdbcUtils.openConnection(_connectionURL);
+    }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java b/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java
index 279ee78..a2636a2 100644
--- a/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java
+++ b/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java
@@ -20,17 +20,14 @@
  */
 package org.apache.qpid.server.store.jdbc;
 
+import static org.apache.qpid.server.store.jdbc.TestJdbcUtils.assertTablesExistence;
+import static org.apache.qpid.server.store.jdbc.TestJdbcUtils.getTableNames;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.sql.Connection;
-import java.sql.DatabaseMetaData;
-import java.sql.DriverManager;
-import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
@@ -72,7 +69,7 @@ public class JDBCMessageStoreTest extends MessageStoreTestCase
         {
             if (_connectionURL != null)
             {
-                shutdownDerby(_connectionURL);
+                TestJdbcUtils.shutdownDerby(_connectionURL);
             }
         }
         finally
@@ -89,17 +86,24 @@ public class JDBCMessageStoreTest extends MessageStoreTestCase
         {
             assertTrue(String.format("Table '%s' does not start with expected prefix '%s'", expectedTable, TEST_TABLE_PREFIX), expectedTable.startsWith(TEST_TABLE_PREFIX));
         }
-        assertTablesExist(expectedTables, true);
+        try(Connection connection = openConnection())
+        {
+            assertTablesExistence(expectedTables, getTableNames(connection), true);
+        }
     }
 
     public void testOnDelete() throws Exception
     {
-        Collection<String> expectedTables = ((GenericJDBCMessageStore)getStore()).getTableNames();
-        assertTablesExist(expectedTables, true);
-        getStore().closeMessageStore();
-        assertTablesExist(expectedTables, true);
-        getStore().onDelete(mock(JDBCVirtualHost.class));
-        assertTablesExist(expectedTables, false);
+        try(Connection connection = openConnection())
+        {
+            GenericJDBCMessageStore store = (GenericJDBCMessageStore) getStore();
+            Collection<String> expectedTables = store.getTableNames();
+            assertTablesExistence(expectedTables, getTableNames(connection), true);
+            store.closeMessageStore();
+            assertTablesExistence(expectedTables, getTableNames(connection), true);
+            store.onDelete(getVirtualHost());
+            assertTablesExistence(expectedTables, getTableNames(connection), false);
+        }
     }
 
     public void testEnqueueTransactionCommitAsync() throws Exception
@@ -180,71 +184,8 @@ public class JDBCMessageStoreTest extends MessageStoreTestCase
         return new GenericJDBCMessageStore();
     }
 
-    private void assertTablesExist(Collection<String> expectedTables, boolean exists) throws SQLException
-    {
-        Set<String> existingTables = getTableNames();
-        for (String tableName : expectedTables)
-        {
-            assertEquals("Table " + tableName + (exists ? " is not found" : " actually exist"), exists,
-                    existingTables.contains(tableName));
-        }
-    }
-
-    private Set<String> getTableNames() throws SQLException
-    {
-        Set<String> tableNames = new HashSet<>();
-        Connection conn = null;
-        try
-        {
-            conn = openConnection();
-            DatabaseMetaData metaData = conn.getMetaData();
-            try (ResultSet tables = metaData.getTables(null, null, null, new String[]{"TABLE"}))
-            {
-                while (tables.next())
-                {
-                    tableNames.add(tables.getString("TABLE_NAME"));
-                }
-            }
-        }
-        finally
-        {
-            if (conn != null)
-            {
-                conn.close();
-            }
-        }
-        return tableNames;
-    }
-
     private Connection openConnection() throws SQLException
     {
-        return DriverManager.getConnection(_connectionURL);
-    }
-
-    static void shutdownDerby(String connectionURL) throws SQLException
-    {
-        Connection connection = null;
-        try
-        {
-            connection = DriverManager.getConnection(connectionURL + ";shutdown=true");
-        }
-        catch(SQLException e)
-        {
-            if (e.getSQLState().equalsIgnoreCase("08006"))
-            {
-                //expected and represents a clean shutdown of this database only, do nothing.
-            }
-            else
-            {
-                throw e;
-            }
-        }
-        finally
-        {
-            if (connection != null)
-            {
-                connection.close();
-            }
-        }
+        return TestJdbcUtils.openConnection(_connectionURL);
     }
 }

http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/bce93dfb/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/TestJdbcUtils.java
----------------------------------------------------------------------
diff --git a/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/TestJdbcUtils.java b/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/TestJdbcUtils.java
new file mode 100644
index 0000000..e1ee362
--- /dev/null
+++ b/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/TestJdbcUtils.java
@@ -0,0 +1,94 @@
+/*
+ *
+ * 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.qpid.server.store.jdbc;
+
+import static org.junit.Assert.assertEquals;
+
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+
+class TestJdbcUtils
+{
+
+    static void assertTablesExistence(Collection<String> expectedTables,
+                                      Collection<String> actualTables,
+                                      boolean exists) throws SQLException
+    {
+        for (String tableName : expectedTables)
+        {
+            assertEquals(String.format("Table %s %s", tableName, (exists ? " is not found" : " actually exist")),
+                         exists,
+                         actualTables.contains(tableName));
+        }
+    }
+
+    static Set<String> getTableNames(Connection connection) throws SQLException
+    {
+        Set<String> tableNames = new HashSet<>();
+        DatabaseMetaData metaData = connection.getMetaData();
+        try (ResultSet tables = metaData.getTables(null, null, null, new String[]{"TABLE"}))
+        {
+            while (tables.next())
+            {
+                tableNames.add(tables.getString("TABLE_NAME"));
+            }
+        }
+        return tableNames;
+    }
+
+    static void shutdownDerby(String connectionURL) throws SQLException
+    {
+        Connection connection = null;
+        try
+        {
+            connection = DriverManager.getConnection(connectionURL + ";shutdown=true");
+        }
+        catch(SQLException e)
+        {
+            if (e.getSQLState().equalsIgnoreCase("08006"))
+            {
+                //expected and represents a clean shutdown of this database only, do nothing.
+            }
+            else
+            {
+                throw e;
+            }
+        }
+        finally
+        {
+            if (connection != null)
+            {
+                connection.close();
+            }
+        }
+    }
+
+    static Connection openConnection(final String connectionURL) throws SQLException
+    {
+        return DriverManager.getConnection(connectionURL);
+    }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org