From 32d252c57d74cf0076f516661cab3eb2ff00dc6c Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 2 Aug 2016 22:04:15 +0100 Subject: [PATCH 1/2] Add a basic fix for #159 Extend test suite to include more rigorous testing of connection pools with containers --- .../jdbc/ContainerDatabaseDriver.java | 169 +++++++++--------- modules/mysql/pom.xml | 10 ++ .../testcontainers/jdbc/JDBCDriverTest.java | 33 ---- .../jdbc/JDBCDriverWithPoolTest.java | 165 +++++++++++++++++ 4 files changed, 261 insertions(+), 116 deletions(-) create mode 100644 modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverWithPoolTest.java diff --git a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java index cbcb5655556..c7bb19bdbc4 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java +++ b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java @@ -20,16 +20,16 @@ /** * Test Containers JDBC proxy driver. This driver will handle JDBC URLs of the form: - * + *

* jdbc:tc:type://host:port/database?querystring - * + *

* where type is a supported database type (e.g. mysql, postgresql, oracle). Behind the scenes a new * docker container will be launched running the required database engine. New JDBC connections will be created * using the database's standard driver implementation, connected to the container. - * + *

* If TC_INITSCRIPT is set in querystring, it will be used as the path for an init script that * should be run to initialize the database after the container is created. This should be a classpath resource. - * + *

* Similarly TC_INITFUNCTION may be a method reference for a function that can initialize the database. * Such a function must accept a javax.sql.Connection as its only parameter. * An example of a valid method reference would be com.myapp.SomeClass::initFunction @@ -48,9 +48,9 @@ public class ContainerDatabaseDriver implements Driver { private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(ContainerDatabaseDriver.class); private Driver delegate; - private final Map> containerConnections = new HashMap<>(); - private final Map jdbcUrlContainerCache = new HashMap<>(); - private final Set initializedContainers = new HashSet<>(); + private static final Map> containerConnections = new HashMap<>(); + private static final Map jdbcUrlContainerCache = new HashMap<>(); + private static final Set initializedContainers = new HashSet<>(); static { load(); @@ -72,90 +72,93 @@ public boolean acceptsURL(String url) throws SQLException { @Override public synchronized Connection connect(String url, final Properties info) throws SQLException { - /* - The driver should return "null" if it realizes it is the wrong kind of driver to connect to the given URL. - */ - if(!acceptsURL(url)) { - return null; - } - - String queryString = ""; /* - If we already have a running container for this exact connection string, we want to connect - to that rather than create a new container + The driver should return "null" if it realizes it is the wrong kind of driver to connect to the given URL. */ - JdbcDatabaseContainer container = jdbcUrlContainerCache.get(url); - if (container == null) { - /* - Extract from the JDBC connection URL: - * The database type (e.g. mysql, postgresql, ...) - * The docker tag, if provided. - * The URL query string, if provided - */ - Matcher urlMatcher = URL_MATCHING_PATTERN.matcher(url); - if (!urlMatcher.matches()) { - throw new IllegalArgumentException("JDBC URL matches jdbc:tc: prefix but the database or tag name could not be identified"); - } - String databaseType = urlMatcher.group(1); - String tag = urlMatcher.group(3); - if (tag == null) { - tag = "latest"; - } - - queryString = urlMatcher.group(4); - if (queryString == null) { - queryString = ""; - } + if (!acceptsURL(url)) { + return null; + } - Map parameters = getContainerParameters(url); + synchronized (jdbcUrlContainerCache) { + String queryString = ""; /* - Find a matching container type using ServiceLoader. + If we already have a running container for this exact connection string, we want to connect + to that rather than create a new container */ - ServiceLoader databaseContainers = ServiceLoader.load(JdbcDatabaseContainerProvider.class); - for (JdbcDatabaseContainerProvider candidateContainerType : databaseContainers) { - if (candidateContainerType.supports(databaseType)) { - container = candidateContainerType.newInstance(tag); - delegate = container.getJdbcDriverInstance(); - } - } + JdbcDatabaseContainer container = jdbcUrlContainerCache.get(url); if (container == null) { - throw new UnsupportedOperationException("Database name " + databaseType + " not supported"); - } + /* + Extract from the JDBC connection URL: + * The database type (e.g. mysql, postgresql, ...) + * The docker tag, if provided. + * The URL query string, if provided + */ + Matcher urlMatcher = URL_MATCHING_PATTERN.matcher(url); + if (!urlMatcher.matches()) { + throw new IllegalArgumentException("JDBC URL matches jdbc:tc: prefix but the database or tag name could not be identified"); + } + String databaseType = urlMatcher.group(1); + String tag = urlMatcher.group(3); + if (tag == null) { + tag = "latest"; + } - /* - Cache the container before starting to prevent race conditions when a connection - pool is started up - */ - jdbcUrlContainerCache.put(url, container); + queryString = urlMatcher.group(4); + if (queryString == null) { + queryString = ""; + } + + Map parameters = getContainerParameters(url); + + /* + Find a matching container type using ServiceLoader. + */ + ServiceLoader databaseContainers = ServiceLoader.load(JdbcDatabaseContainerProvider.class); + for (JdbcDatabaseContainerProvider candidateContainerType : databaseContainers) { + if (candidateContainerType.supports(databaseType)) { + container = candidateContainerType.newInstance(tag); + delegate = container.getJdbcDriverInstance(); + } + } + if (container == null) { + throw new UnsupportedOperationException("Database name " + databaseType + " not supported"); + } + + /* + Cache the container before starting to prevent race conditions when a connection + pool is started up + */ + jdbcUrlContainerCache.put(url, container); + + /* + Pass possible container-specific parameters + */ + container.setParameters(parameters); + + /* + Start the container + */ + container.start(); + } /* - Pass possible container-specific parameters + Create a connection using the delegated driver. The container must be ready to accept connections. */ - container.setParameters(parameters); + Connection connection = container.createConnection(queryString); /* - Start the container + If this container has not been initialized, AND + an init script or function has been specified, use it */ - container.start(); - } - - /* - Create a connection using the delegated driver. The container must be ready to accept connections. - */ - Connection connection = container.createConnection(queryString); + if (!initializedContainers.contains(container)) { + runInitScriptIfRequired(url, connection); + runInitFunctionIfRequired(url, connection); + initializedContainers.add(container); + } - /* - If this container has not been initialized, AND - an init script or function has been specified, use it - */ - if (!initializedContainers.contains(container)) { - runInitScriptIfRequired(url, connection); - runInitFunctionIfRequired(url, connection); - initializedContainers.add(container); + return wrapConnection(connection, container, url); } - - return wrapConnection(connection, container, url); } private Map getContainerParameters(String url) { @@ -174,18 +177,18 @@ private Map getContainerParameters(String url) { /** * Wrap the connection, setting up a callback to be called when the connection is closed. - * + *

* When there are no more open connections, the container itself will be stopped. * - * @param connection the new connection to be wrapped - * @param container the container which the connection is associated with - * @param url the testcontainers JDBC URL for this connection - * @return the connection, wrapped + * @param connection the new connection to be wrapped + * @param container the container which the connection is associated with + * @param url the testcontainers JDBC URL for this connection + * @return the connection, wrapped */ private Connection wrapConnection(final Connection connection, final JdbcDatabaseContainer container, final String url) { Set connections = containerConnections.get(container); - if(connections == null) { + if (connections == null) { connections = new HashSet<>(); containerConnections.put(container, connections); } @@ -206,7 +209,7 @@ private Connection wrapConnection(final Connection connection, final JdbcDatabas /** * Run an init script from the classpath. * - * @param url the JDBC URL to check for init script declarations. + * @param url the JDBC URL to check for init script declarations. * @param connection JDBC connection to apply init scripts to. * @throws SQLException on script or DB error */ @@ -231,7 +234,7 @@ private void runInitScriptIfRequired(String url, Connection connection) throws S /** * Run an init function (must be a public static method on an accessible class). * - * @param url the JDBC URL to check for init function declarations. + * @param url the JDBC URL to check for init function declarations. * @param connection JDBC connection to apply init functions to. * @throws SQLException on script or DB error */ diff --git a/modules/mysql/pom.xml b/modules/mysql/pom.xml index 6154275fdd1..a44756c3f1d 100644 --- a/modules/mysql/pom.xml +++ b/modules/mysql/pom.xml @@ -45,5 +45,15 @@ 1.6 test + + org.apache.tomcat + tomcat-jdbc + 8.5.4 + + + org.vibur + vibur-dbcp + 9.0 + \ No newline at end of file diff --git a/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java b/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java index f4bfee6ce75..76b6673ccaf 100644 --- a/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java +++ b/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java @@ -58,39 +58,6 @@ public void testMySQLWithClasspathInitFunction() throws SQLException { performTestForScriptedSchema("jdbc:tc:mysql://hostname/databasename?TC_INITFUNCTION=org.testcontainers.jdbc.JDBCDriverTest::sampleInitFunction"); } - @Test - public void testMySQLWithConnectionPoolUsingSameContainer() throws SQLException { - HikariDataSource dataSource = getDataSource("jdbc:tc:mysql://hostname/databasename?TC_INITFUNCTION=org.testcontainers.jdbc.JDBCDriverTest::sampleInitFunction", 10); - for (int i = 0; i < 100; i++) { - new QueryRunner(dataSource).insert("INSERT INTO my_counter (n) VALUES (5)", new ResultSetHandler() { - @Override - public Object handle(ResultSet rs) throws SQLException { - return true; - } - }); - } - - new QueryRunner(dataSource).query("SELECT COUNT(1) FROM my_counter", new ResultSetHandler() { - @Override - public Object handle(ResultSet rs) throws SQLException { - rs.next(); - int resultSetInt = rs.getInt(1); - assertEquals("Reuse of a datasource points to the same DB container", 100, resultSetInt); - return true; - } - }); - - new QueryRunner(dataSource).query("SELECT SUM(n) FROM my_counter", new ResultSetHandler() { - @Override - public Object handle(ResultSet rs) throws SQLException { - rs.next(); - int resultSetInt = rs.getInt(1); - assertEquals("Reuse of a datasource points to the same DB container", 500, resultSetInt); - return true; - } - }); - } - @Test public void testMySQLWithQueryParams() throws SQLException { performSimpleTestWithCharacterSet("jdbc:tc:mysql://hostname/databasename?useUnicode=yes&characterEncoding=utf8"); diff --git a/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverWithPoolTest.java b/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverWithPoolTest.java new file mode 100644 index 00000000000..4c7e119996d --- /dev/null +++ b/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverWithPoolTest.java @@ -0,0 +1,165 @@ +package org.testcontainers.jdbc; + +import com.zaxxer.hikari.HikariConfig; +import com.zaxxer.hikari.HikariDataSource; +import org.apache.commons.dbutils.QueryRunner; +import org.apache.commons.dbutils.ResultSetHandler; +import org.apache.tomcat.jdbc.pool.PoolProperties; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.vibur.dbcp.ViburDBCPDataSource; + +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; + +import static java.util.Arrays.asList; +import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals; + +/** + * + */ +@RunWith(Parameterized.class) +public class JDBCDriverWithPoolTest { + + public static final String URL = "jdbc:tc:mysql://hostname/databasename?TC_INITFUNCTION=org.testcontainers.jdbc.JDBCDriverWithPoolTest::sampleInitFunction"; + private final DataSource dataSource; + + @Parameterized.Parameters + public static Iterable> dataSourceSuppliers() { + return asList( + JDBCDriverWithPoolTest::getTomcatDataSourceWithDriverClassName, + JDBCDriverWithPoolTest::getTomcatDataSource, + JDBCDriverWithPoolTest::getHikariDataSourceWithDriverClassName, + JDBCDriverWithPoolTest::getHikariDataSource, + JDBCDriverWithPoolTest::getViburDataSourceWithDriverClassName, + JDBCDriverWithPoolTest::getViburDataSource + ); + } + + public JDBCDriverWithPoolTest(Supplier dataSourceSupplier) { + this.dataSource = dataSourceSupplier.get(); + } + + private ExecutorService executorService = Executors.newFixedThreadPool(5); + + @Test + public void testMySQLWithConnectionPoolUsingSameContainer() throws SQLException, InterruptedException { + + // Populate the database with some data in multiple threads, so that multiple connections from the pool will be used + for (int i = 0; i < 100; i++) { + executorService.submit(() -> { + try { + new QueryRunner(dataSource).insert("INSERT INTO my_counter (n) VALUES (5)", + (ResultSetHandler) rs -> true); + } catch (SQLException e) { + e.printStackTrace(); + } + }); + } + + // Complete population of the database + executorService.shutdown(); + executorService.awaitTermination(5, TimeUnit.MINUTES); + + // compare to expected results + int count = new QueryRunner(dataSource).query("SELECT COUNT(1) FROM my_counter", rs -> { + rs.next(); + return rs.getInt(1); + }); + assertEquals("Reuse of a datasource points to the same DB container", 100, count); + + + int sum = new QueryRunner(dataSource).query("SELECT SUM(n) FROM my_counter", rs -> { + rs.next(); + return rs.getInt(1); + }); + // 100 records * 5 = 500 expected + assertEquals("Reuse of a datasource points to the same DB container", 500, sum); + } + + + private static DataSource getTomcatDataSourceWithDriverClassName() { + PoolProperties poolProperties = new PoolProperties(); + poolProperties.setUrl(URL + ";TEST=TOMCAT_WITH_CLASSNAME"); // append a dummy URL element to ensure different DB per test + poolProperties.setValidationQuery("SELECT 1"); + poolProperties.setMinIdle(3); + poolProperties.setMaxActive(10); + poolProperties.setDriverClassName(ContainerDatabaseDriver.class.getName()); + + return new org.apache.tomcat.jdbc.pool.DataSource(poolProperties); + } + + private static DataSource getTomcatDataSource() { + PoolProperties poolProperties = new PoolProperties(); + poolProperties.setUrl(URL + ";TEST=TOMCAT"); // append a dummy URL element to ensure different DB per test + poolProperties.setValidationQuery("SELECT 1"); + poolProperties.setInitialSize(3); + poolProperties.setMaxActive(10); + + return new org.apache.tomcat.jdbc.pool.DataSource(poolProperties); + } + + private static HikariDataSource getHikariDataSourceWithDriverClassName() { + HikariConfig hikariConfig = new HikariConfig(); + hikariConfig.setJdbcUrl(URL + ";TEST=HIKARI_WITH_CLASSNAME"); // append a dummy URL element to ensure different DB per test + hikariConfig.setConnectionTestQuery("SELECT 1"); + hikariConfig.setMinimumIdle(3); + hikariConfig.setMaximumPoolSize(10); + hikariConfig.setDriverClassName(ContainerDatabaseDriver.class.getName()); + + return new HikariDataSource(hikariConfig); + } + + private static HikariDataSource getHikariDataSource() { + HikariConfig hikariConfig = new HikariConfig(); + hikariConfig.setJdbcUrl(URL + ";TEST=HIKARI"); // append a dummy URL element to ensure different DB per test + hikariConfig.setConnectionTestQuery("SELECT 1"); + hikariConfig.setMinimumIdle(3); + hikariConfig.setMaximumPoolSize(10); + + return new HikariDataSource(hikariConfig); + } + + private static DataSource getViburDataSourceWithDriverClassName() { + ViburDBCPDataSource ds = new ViburDBCPDataSource(); + + ds.setJdbcUrl(URL + ";TEST=VIBUR_WITH_CLASSNAME"); + ds.setPoolInitialSize(3); + ds.setPoolMaxSize(10); + ds.setTestConnectionQuery("SELECT 1"); + ds.setDriverClassName(ContainerDatabaseDriver.class.getName()); + + ds.start(); + + return ds; + } + + private static DataSource getViburDataSource() { + ViburDBCPDataSource ds = new ViburDBCPDataSource(); + ds.setJdbcUrl(URL + ";TEST=VIBUR"); + ds.setPoolInitialSize(3); + ds.setPoolMaxSize(10); + ds.setTestConnectionQuery("SELECT 1"); + + ds.start(); + + return ds; + } + + @SuppressWarnings("SqlNoDataSourceInspection") + public static void sampleInitFunction(Connection connection) throws SQLException { + connection.createStatement().execute("CREATE TABLE bar (\n" + + " foo VARCHAR(255)\n" + + ");"); + connection.createStatement().execute("INSERT INTO bar (foo) VALUES ('hello world');"); + connection.createStatement().execute("CREATE TABLE my_counter (\n" + + " n INT\n" + + ");"); + } +} From f1bc5c69bf3228e6feecfe450f728634c64af28c Mon Sep 17 00:00:00 2001 From: Richard North Date: Sun, 7 Aug 2016 21:40:11 +0100 Subject: [PATCH 2/2] Add convenience methods to clean up database containers. Refs #159. --- .../jdbc/ContainerDatabaseDriver.java | 45 ++++++++++++++++--- .../jdbc/ContainerDatabaseDriverTest.java | 10 ++--- .../testcontainers/jdbc/JDBCDriverTest.java | 10 ++++- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java index c7bb19bdbc4..937ca61536f 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java +++ b/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java @@ -48,9 +48,9 @@ public class ContainerDatabaseDriver implements Driver { private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(ContainerDatabaseDriver.class); private Driver delegate; - private static final Map> containerConnections = new HashMap<>(); + private static final Map> containerConnections = new HashMap<>(); private static final Map jdbcUrlContainerCache = new HashMap<>(); - private static final Set initializedContainers = new HashSet<>(); + private static final Set initializedContainers = new HashSet<>(); static { load(); @@ -151,10 +151,10 @@ public synchronized Connection connect(String url, final Properties info) throws If this container has not been initialized, AND an init script or function has been specified, use it */ - if (!initializedContainers.contains(container)) { + if (!initializedContainers.contains(container.getContainerId())) { runInitScriptIfRequired(url, connection); runInitFunctionIfRequired(url, connection); - initializedContainers.add(container); + initializedContainers.add(container.getContainerId()); } return wrapConnection(connection, container, url); @@ -186,11 +186,11 @@ private Map getContainerParameters(String url) { * @return the connection, wrapped */ private Connection wrapConnection(final Connection connection, final JdbcDatabaseContainer container, final String url) { - Set connections = containerConnections.get(container); + Set connections = containerConnections.get(container.getContainerId()); if (connections == null) { connections = new HashSet<>(); - containerConnections.put(container, connections); + containerConnections.put(container.getContainerId(), connections); } connections.add(connection); @@ -280,4 +280,37 @@ public boolean jdbcCompliant() { public Logger getParentLogger() throws SQLFeatureNotSupportedException { return delegate.getParentLogger(); } + + /** + * Utility method to kill ALL database containers directly from test support code. It shouldn't be necessary to use this, + * but it is provided for convenience - e.g. for situations where many different database containers are being + * tested and cleanup is needed to limit resource usage. + */ + public static void killContainers() { + synchronized (jdbcUrlContainerCache) { + jdbcUrlContainerCache.values().forEach(JdbcDatabaseContainer::stop); + jdbcUrlContainerCache.clear(); + containerConnections.clear(); + initializedContainers.clear(); + } + + } + + /** + * Utility method to kill a database container directly from test support code. It shouldn't be necessary to use this, + * but it is provided for convenience - e.g. for situations where many different database containers are being + * tested and cleanup is needed to limit resource usage. + * @param jdbcUrl the JDBC URL of the container which should be killed + */ + public static void killContainer(String jdbcUrl) { + synchronized (jdbcUrlContainerCache) { + JdbcDatabaseContainer container = jdbcUrlContainerCache.get(jdbcUrl); + if (container != null) { + container.stop(); + jdbcUrlContainerCache.remove(jdbcUrl); + containerConnections.remove(container.getContainerId()); + initializedContainers.remove(container.getContainerId()); + } + } + } } diff --git a/modules/jdbc/src/test/java/org/testcontainers/jdbc/ContainerDatabaseDriverTest.java b/modules/jdbc/src/test/java/org/testcontainers/jdbc/ContainerDatabaseDriverTest.java index 8b11722c357..d33e399db27 100644 --- a/modules/jdbc/src/test/java/org/testcontainers/jdbc/ContainerDatabaseDriverTest.java +++ b/modules/jdbc/src/test/java/org/testcontainers/jdbc/ContainerDatabaseDriverTest.java @@ -1,16 +1,16 @@ package org.testcontainers.jdbc; -import java.sql.Connection; -import java.sql.DriverManager; -import java.sql.SQLException; -import java.util.Properties; - import org.hamcrest.CoreMatchers; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.Properties; + public class ContainerDatabaseDriverTest { private static final String PLAIN_POSTGRESQL_JDBC_URL = "jdbc:postgresql://localhost:5432/test"; diff --git a/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java b/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java index 76b6673ccaf..0782810195e 100644 --- a/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java +++ b/modules/mysql/src/test/java/org/testcontainers/jdbc/JDBCDriverTest.java @@ -5,6 +5,7 @@ import org.apache.commons.dbutils.QueryRunner; import org.apache.commons.dbutils.ResultSetHandler; import org.apache.commons.lang.SystemUtils; +import org.junit.After; import org.junit.Test; import java.sql.Connection; @@ -12,14 +13,19 @@ import java.sql.SQLException; import java.sql.Statement; -import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals; import static org.junit.Assume.assumeFalse; +import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals; /** * */ public class JDBCDriverTest { + @After + public void testCleanup() { + ContainerDatabaseDriver.killContainers(); + } + @Test public void testMySQLWithVersion() throws SQLException { performSimpleTest("jdbc:tc:mysql:5.5.43://hostname/databasename"); @@ -32,7 +38,7 @@ public void testMySQLWithNoSpecifiedVersion() throws SQLException { @Test public void testMySQLWithCustomIniFile() throws SQLException { - assumeFalse(SystemUtils.IS_OS_WINDOWS); + assumeFalse(SystemUtils.IS_OS_WINDOWS); HikariDataSource ds = getDataSource("jdbc:tc:mysql:5.6://hostname/databasename?TC_MY_CNF=somepath/mysql_conf_override", 1); Statement statement = ds.getConnection().createStatement(); statement.execute("SELECT @@GLOBAL.innodb_file_format");