From ea918230355ed2e27e438ee7c89135d2e3e3d69c Mon Sep 17 00:00:00 2001 From: Richard North Date: Sun, 28 Apr 2019 13:05:02 +0100 Subject: [PATCH 1/6] WIP: Fail gracefully if no JDBC driver found Fixes #678 --- .../containers/ContainerLaunchException.java | 4 +- .../containers/JdbcDatabaseContainer.java | 44 ++++++++---- .../jdbc/MissingJdbcDriverTest.java | 69 +++++++++++++++++++ .../jdbc/src/test/resources/logback-test.xml | 25 +++++++ 4 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 modules/jdbc/src/test/java/org/testcontainers/jdbc/MissingJdbcDriverTest.java create mode 100644 modules/jdbc/src/test/resources/logback-test.xml diff --git a/core/src/main/java/org/testcontainers/containers/ContainerLaunchException.java b/core/src/main/java/org/testcontainers/containers/ContainerLaunchException.java index 9943d9f612b..bf407c16808 100644 --- a/core/src/main/java/org/testcontainers/containers/ContainerLaunchException.java +++ b/core/src/main/java/org/testcontainers/containers/ContainerLaunchException.java @@ -9,7 +9,7 @@ public ContainerLaunchException(String message) { super(message); } - public ContainerLaunchException(String message, Exception exception) { - super(message, exception); + public ContainerLaunchException(String message, Throwable cause) { + super(message, cause); } } diff --git a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java index e480070168f..a32377a1f3e 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java +++ b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java @@ -1,7 +1,7 @@ package org.testcontainers.containers; -import lombok.NonNull; import com.github.dockerjava.api.command.InspectContainerResponse; +import lombok.NonNull; import org.jetbrains.annotations.NotNull; import org.rnorth.ducttape.ratelimits.RateLimiter; import org.rnorth.ducttape.ratelimits.RateLimiterBuilder; @@ -20,6 +20,7 @@ import java.util.Properties; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; /** * Base class for containers that expose a JDBC connection @@ -34,9 +35,9 @@ public abstract class JdbcDatabaseContainer parameters = new HashMap<>(); private static final RateLimiter DB_CONNECT_RATE_LIMIT = RateLimiterBuilder.newBuilder() - .withRate(10, TimeUnit.SECONDS) - .withConstantThroughput() - .build(); + .withRate(10, TimeUnit.SECONDS) + .withConstantThroughput() + .build(); private int startupTimeoutSeconds = 120; private int connectTimeoutSeconds = 120; @@ -126,10 +127,15 @@ protected void waitUntilContainerStarted() { // Repeatedly try and open a connection to the DB and execute a test query logger().info("Waiting for database connection to become available at {} using query '{}'", getJdbcUrl(), getTestQueryString()); - Unreliables.retryUntilSuccess(getStartupTimeoutSeconds(), TimeUnit.SECONDS, () -> { + final AtomicBoolean retrying = new AtomicBoolean(true); + final long expiry = System.currentTimeMillis() + startupTimeoutSeconds * 1000; + Throwable failureReason = null; + + while (retrying.get() && System.currentTimeMillis() < expiry) { if (!isRunning()) { - throw new ContainerLaunchException("Container failed to start"); + failureReason = new ContainerLaunchException("Container failed to start", failureReason); + continue; // Don't attempt to connect } try (Connection connection = createConnection("")) { @@ -137,12 +143,18 @@ protected void waitUntilContainerStarted() { if (success) { logger().info("Obtained a connection to container ({})", JdbcDatabaseContainer.this.getJdbcUrl()); - return null; - } else { - throw new SQLException("Failed to execute test query"); + return; } + } catch (SQLException e) { + failureReason = e; + } catch (NoDriverFoundException e) { + failureReason = e; + break; // Fail fast, as this is an unrecoverable condition } - }); + } + + // if we reached this point, then we have failed to connect and must throw + throw new ContainerLaunchException("Failed to execute test query via JDBC connection to container", failureReason); } @Override @@ -155,14 +167,14 @@ protected void containerIsStarted(InspectContainerResponse containerInfo) { * * @return a JDBC Driver */ - public Driver getJdbcDriverInstance() { + public Driver getJdbcDriverInstance() throws NoDriverFoundException { synchronized (DRIVER_LOAD_MUTEX) { if (driver == null) { try { driver = (Driver) Class.forName(this.getDriverClassName()).newInstance(); } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) { - throw new RuntimeException("Could not get Driver", e); + throw new NoDriverFoundException("Could not get Driver", e); } } } @@ -178,7 +190,7 @@ public Driver getJdbcDriverInstance() { * @return a Connection * @throws SQLException if there is a repeated failure to create the connection */ - public Connection createConnection(String queryString) throws SQLException { + public Connection createConnection(String queryString) throws SQLException, NoDriverFoundException { final Properties info = new Properties(); info.put("user", this.getUsername()); info.put("password", this.getPassword()); @@ -256,4 +268,10 @@ protected int getConnectTimeoutSeconds() { protected DatabaseDelegate getDatabaseDelegate() { return new JdbcDatabaseDelegate(this, ""); } + + public static class NoDriverFoundException extends RuntimeException { + public NoDriverFoundException(String message, Throwable e) { + super(message, e); + } + } } diff --git a/modules/jdbc/src/test/java/org/testcontainers/jdbc/MissingJdbcDriverTest.java b/modules/jdbc/src/test/java/org/testcontainers/jdbc/MissingJdbcDriverTest.java new file mode 100644 index 00000000000..09aed9ab6f0 --- /dev/null +++ b/modules/jdbc/src/test/java/org/testcontainers/jdbc/MissingJdbcDriverTest.java @@ -0,0 +1,69 @@ +package org.testcontainers.jdbc; + +import com.google.common.base.Throwables; +import org.junit.Test; +import org.testcontainers.containers.JdbcDatabaseContainer; + +import java.sql.Connection; +import java.sql.SQLException; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals; +import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue; +import static org.rnorth.visibleassertions.VisibleAssertions.fail; + +public class MissingJdbcDriverTest { + + @Test + public void shouldFailFastIfNoDriverFound() { + + AtomicInteger connectionAttempts = new AtomicInteger(); + + // Anonymous inner class for the purposes of testing, with a known non-existent driver testFailFastIfNoDriverFound + final JdbcDatabaseContainer container = new JdbcDatabaseContainer("mysql:5.7.22") { + + @Override + public String getDriverClassName() { + return "nonexistent.ClassName"; + } + + @Override + public String getJdbcUrl() { + return ""; + } + + @Override + public String getUsername() { + return ""; + } + + @Override + public String getPassword() { + return ""; + } + + @Override + protected String getTestQueryString() { + return ""; + } + + @Override + public Connection createConnection(String queryString) throws SQLException, NoDriverFoundException { + connectionAttempts.incrementAndGet(); // test window: so we know how many times a connection was attempted + return super.createConnection(queryString); + } + }; + + try { + container.start(); + fail("The container is expected to fail to start"); + } catch (Exception e) { + final Throwable rootCause = Throwables.getRootCause(e); + assertTrue("ClassNotFoundException is the root cause", rootCause instanceof ClassNotFoundException); + } finally { + container.stop(); + } + + assertEquals("only one connection attempt should have been made", 1, connectionAttempts.get()); + } +} diff --git a/modules/jdbc/src/test/resources/logback-test.xml b/modules/jdbc/src/test/resources/logback-test.xml new file mode 100644 index 00000000000..b0f6b00e3e0 --- /dev/null +++ b/modules/jdbc/src/test/resources/logback-test.xml @@ -0,0 +1,25 @@ + + + + + + %d{HH:mm:ss.SSS} %-5level %logger - %msg%n + + + + + + + + + + + + + + + + + + From aea7ff999f11227b73484e9d9f60a602e6b7e652 Mon Sep 17 00:00:00 2001 From: Richard North Date: Sun, 28 Apr 2019 20:46:30 +0100 Subject: [PATCH 2/6] WIP: use Awaitility --- core/build.gradle | 1 + .../containers/JdbcDatabaseContainer.java | 58 ++++++++----------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/core/build.gradle b/core/build.gradle index 1f08c0159b2..183c3ae04ac 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -66,6 +66,7 @@ dependencies { compile ('org.rnorth.duct-tape:duct-tape:1.0.7') { exclude(group: 'org.jetbrains', module: 'annotations') } + compile 'org.awaitility:awaitility:3.1.6' compile 'org.rnorth.visible-assertions:visible-assertions:2.1.2' diff --git a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java index a32377a1f3e..8b6a025a82f 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java +++ b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java @@ -19,8 +19,9 @@ import java.util.Map; import java.util.Properties; import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.awaitility.Awaitility.await; /** * Base class for containers that expose a JDBC connection @@ -35,7 +36,7 @@ public abstract class JdbcDatabaseContainer parameters = new HashMap<>(); private static final RateLimiter DB_CONNECT_RATE_LIMIT = RateLimiterBuilder.newBuilder() - .withRate(10, TimeUnit.SECONDS) + .withRate(10, SECONDS) .withConstantThroughput() .build(); @@ -128,33 +129,24 @@ protected void waitUntilContainerStarted() { logger().info("Waiting for database connection to become available at {} using query '{}'", getJdbcUrl(), getTestQueryString()); - final AtomicBoolean retrying = new AtomicBoolean(true); - final long expiry = System.currentTimeMillis() + startupTimeoutSeconds * 1000; - Throwable failureReason = null; - - while (retrying.get() && System.currentTimeMillis() < expiry) { - if (!isRunning()) { - failureReason = new ContainerLaunchException("Container failed to start", failureReason); - continue; // Don't attempt to connect - } - - try (Connection connection = createConnection("")) { - boolean success = connection.createStatement().execute(JdbcDatabaseContainer.this.getTestQueryString()); - - if (success) { - logger().info("Obtained a connection to container ({})", JdbcDatabaseContainer.this.getJdbcUrl()); - return; - } - } catch (SQLException e) { - failureReason = e; - } catch (NoDriverFoundException e) { - failureReason = e; - break; // Fail fast, as this is an unrecoverable condition - } - } - - // if we reached this point, then we have failed to connect and must throw - throw new ContainerLaunchException("Failed to execute test query via JDBC connection to container", failureReason); + await().ignoreExceptionsMatching(e -> ! (e instanceof NoDriverFoundException)) + .timeout(startupTimeoutSeconds, SECONDS) + .until(() -> { + if (!isRunning()) { + return false; // Don't attempt to connect + } + + try (Connection connection = createConnection("")) { + boolean success = connection.createStatement().execute(JdbcDatabaseContainer.this.getTestQueryString()); + + if (success) { + logger().info("Obtained a connection to container ({})", JdbcDatabaseContainer.this.getJdbcUrl()); + return true; + } else { + return false; + } + } + }); } @Override @@ -199,9 +191,9 @@ public Connection createConnection(String queryString) throws SQLException, NoDr final Driver jdbcDriverInstance = getJdbcDriverInstance(); try { - return Unreliables.retryUntilSuccess(getConnectTimeoutSeconds(), TimeUnit.SECONDS, () -> - DB_CONNECT_RATE_LIMIT.getWhenReady(() -> - jdbcDriverInstance.connect(url, info))); + return Unreliables.retryUntilSuccess(getConnectTimeoutSeconds(), SECONDS, () -> + DB_CONNECT_RATE_LIMIT.getWhenReady(() -> + jdbcDriverInstance.connect(url, info))); } catch (Exception e) { throw new SQLException("Could not create new connection", e); } From a8a8248534bb12cf529684f3299fb3e04b75f58f Mon Sep 17 00:00:00 2001 From: Richard North Date: Sun, 28 Apr 2019 21:32:25 +0100 Subject: [PATCH 3/6] Use awaitility for createConnection method --- .../containers/JdbcDatabaseContainer.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java index 8b6a025a82f..a08fd5b9243 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java +++ b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java @@ -3,9 +3,6 @@ import com.github.dockerjava.api.command.InspectContainerResponse; import lombok.NonNull; import org.jetbrains.annotations.NotNull; -import org.rnorth.ducttape.ratelimits.RateLimiter; -import org.rnorth.ducttape.ratelimits.RateLimiterBuilder; -import org.rnorth.ducttape.unreliables.Unreliables; import org.testcontainers.containers.traits.LinkableContainer; import org.testcontainers.delegate.DatabaseDelegate; import org.testcontainers.ext.ScriptUtils; @@ -35,11 +32,6 @@ public abstract class JdbcDatabaseContainer parameters = new HashMap<>(); - private static final RateLimiter DB_CONNECT_RATE_LIMIT = RateLimiterBuilder.newBuilder() - .withRate(10, SECONDS) - .withConstantThroughput() - .build(); - private int startupTimeoutSeconds = 120; private int connectTimeoutSeconds = 120; @@ -191,9 +183,12 @@ public Connection createConnection(String queryString) throws SQLException, NoDr final Driver jdbcDriverInstance = getJdbcDriverInstance(); try { - return Unreliables.retryUntilSuccess(getConnectTimeoutSeconds(), SECONDS, () -> - DB_CONNECT_RATE_LIMIT.getWhenReady(() -> - jdbcDriverInstance.connect(url, info))); + return await() + .ignoreExceptions() + .atMost(connectTimeoutSeconds, SECONDS) + .pollDelay(0, SECONDS) + .pollInterval(5, SECONDS) + .until(() -> jdbcDriverInstance.connect(url, info), __ -> true); } catch (Exception e) { throw new SQLException("Could not create new connection", e); } From 3faff3a245aaddcdd5f5eb64911336d8e8d91131 Mon Sep 17 00:00:00 2001 From: Richard North Date: Sun, 28 Apr 2019 21:45:21 +0100 Subject: [PATCH 4/6] Catch correct exception class --- .../org/testcontainers/containers/JdbcDatabaseContainer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java index a08fd5b9243..b6a5b614231 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java +++ b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java @@ -2,6 +2,7 @@ import com.github.dockerjava.api.command.InspectContainerResponse; import lombok.NonNull; +import org.awaitility.core.ConditionTimeoutException; import org.jetbrains.annotations.NotNull; import org.testcontainers.containers.traits.LinkableContainer; import org.testcontainers.delegate.DatabaseDelegate; @@ -189,7 +190,7 @@ public Connection createConnection(String queryString) throws SQLException, NoDr .pollDelay(0, SECONDS) .pollInterval(5, SECONDS) .until(() -> jdbcDriverInstance.connect(url, info), __ -> true); - } catch (Exception e) { + } catch (ConditionTimeoutException e) { throw new SQLException("Could not create new connection", e); } } From 73756184f7b54626525e8c30b03f3ad9ef3e5fa1 Mon Sep 17 00:00:00 2001 From: Richard North Date: Wed, 12 Jun 2019 19:52:33 +0100 Subject: [PATCH 5/6] Extract connection testing logic to lambda as method reference --- .../containers/JdbcDatabaseContainer.java | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java index b6a5b614231..39ac0839039 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java +++ b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java @@ -124,22 +124,7 @@ protected void waitUntilContainerStarted() { await().ignoreExceptionsMatching(e -> ! (e instanceof NoDriverFoundException)) .timeout(startupTimeoutSeconds, SECONDS) - .until(() -> { - if (!isRunning()) { - return false; // Don't attempt to connect - } - - try (Connection connection = createConnection("")) { - boolean success = connection.createStatement().execute(JdbcDatabaseContainer.this.getTestQueryString()); - - if (success) { - logger().info("Obtained a connection to container ({})", JdbcDatabaseContainer.this.getJdbcUrl()); - return true; - } else { - return false; - } - } - }); + .until(this::isConnectable); } @Override @@ -257,6 +242,23 @@ protected DatabaseDelegate getDatabaseDelegate() { return new JdbcDatabaseDelegate(this, ""); } + private boolean isConnectable() throws SQLException { + if (!isRunning()) { + return false; // Don't attempt to connect + } + + try (Connection connection = createConnection("")) { + boolean success = connection.createStatement().execute(JdbcDatabaseContainer.this.getTestQueryString()); + + if (success) { + logger().info("Obtained a connection to container ({})", JdbcDatabaseContainer.this.getJdbcUrl()); + return true; + } else { + return false; + } + } + } + public static class NoDriverFoundException extends RuntimeException { public NoDriverFoundException(String message, Throwable e) { super(message, e); From f298e46546808fef8c3e2033f2f54ea27c8788cb Mon Sep 17 00:00:00 2001 From: Richard North Date: Sat, 6 Jul 2019 16:36:13 +0100 Subject: [PATCH 6/6] Shade awaitility dependency --- core/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/build.gradle b/core/build.gradle index 21816cbb9ec..870429172f1 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -66,7 +66,7 @@ dependencies { compile ('org.rnorth.duct-tape:duct-tape:1.0.7') { exclude(group: 'org.jetbrains', module: 'annotations') } - compile 'org.awaitility:awaitility:3.1.6' + shaded 'org.awaitility:awaitility:3.1.6' compile 'org.rnorth.visible-assertions:visible-assertions:2.1.2'