Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added additional url params in JdbcDatabaseContainer (#1802) #1874

Merged
merged 19 commits into from May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0af8b12
Add additional url params in MySQLContainer (#1802)
eaxdev Sep 15, 2019
456d477
Merge branch 'master' into eaxdev_add_with_url_param
rnorth Oct 8, 2019
00536ce
Merge branch 'master' into eaxdev_add_with_url_param
rnorth Oct 8, 2019
e55eb25
Change additional url params in MySQLContainer. Create common method:…
eaxdev Oct 16, 2019
5417319
Add additional url params in CockroachContainer (#1802)
eaxdev Oct 16, 2019
8af81ac
Add additional url params in Db2Container (#1802)
eaxdev Oct 16, 2019
96c955e
Add additional url params in MSSQLServerContainer (#1802)
eaxdev Oct 16, 2019
709ad1e
Add additional url params in MariaDBContainer (#1802)
eaxdev Oct 16, 2019
722f32b
Add additional url params in PostgreSQLContainer (#1802)
eaxdev Oct 16, 2019
062070f
Fix typo in test (#1802)
eaxdev Oct 17, 2019
2022e5d
Move common method withUrlParam() to parent abstract class (#1802)
eaxdev Oct 17, 2019
701a1b0
Fixed build url params in DB2 container. DB2 require ';' symbol in th…
eaxdev Oct 17, 2019
21f63e2
Merge branch 'master' into eaxdev_add_with_url_param
eaxdev Oct 18, 2019
7eadac4
Merge branch 'master' into eaxdev_add_with_url_param
rnorth Dec 23, 2019
d89543e
Merge remote-tracking branch 'origin/master' into eaxdev_add_with_url…
rnorth May 16, 2020
128b69f
Merge remote-tracking branch 'origin/master' into eaxdev_add_with_url…
rnorth May 16, 2020
fd56078
Reinstate tests lost during merge
rnorth May 16, 2020
f005def
Merge branch 'master' into eaxdev_add_with_url_param
rnorth May 21, 2020
4f1d18f
Fix erroneous assertion
rnorth May 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -66,4 +66,8 @@ public String getTestQueryString() {
return TEST_QUERY;
}

@Override
public ClickHouseContainer withUrlParam(String paramName, String paramValue) {
throw new UnsupportedOperationException("The ClickHouse does not support this");
}
}
Expand Up @@ -43,7 +43,9 @@ public String getDriverClassName() {

@Override
public String getJdbcUrl() {
return JDBC_URL_PREFIX + "://" + getHost() + ":" + getMappedPort(DB_PORT) + "/" + databaseName;
String additionalUrlParams = constructUrlParameters("?", "&");
return JDBC_URL_PREFIX + "://" + getHost() + ":" + getMappedPort(DB_PORT) +
"/" + databaseName + additionalUrlParams;
}

@Override
Expand Down
Expand Up @@ -9,6 +9,8 @@
import java.util.logging.Level;
import java.util.logging.LogManager;

import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;
import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;

public class SimpleCockroachDBTest extends AbstractContainerDatabaseTest {
Expand Down Expand Up @@ -42,4 +44,22 @@ public void testExplicitInitScript() throws SQLException {
assertEquals("Value from init script should equal real value", "hello world", firstColumnValue);
}
}

@Test
public void testWithAdditionalUrlParamInJdbcUrl() {
CockroachContainer cockroach = new CockroachContainer()
.withUrlParam("sslmode", "disable")
.withUrlParam("application_name", "cockroach");

try {
cockroach.start();
String jdbcUrl = cockroach.getJdbcUrl();
assertThat(jdbcUrl, containsString("?"));
assertThat(jdbcUrl, containsString("&"));
assertThat(jdbcUrl, containsString("sslmode=disable"));
assertThat(jdbcUrl, containsString("application_name=cockroach"));
} finally {
cockroach.stop();
}
}
}
Expand Up @@ -73,7 +73,9 @@ public String getDriverClassName() {

@Override
public String getJdbcUrl() {
return "jdbc:db2://" + getHost() + ":" + getMappedPort(DB2_PORT) + "/" + databaseName;
String additionalUrlParams = constructUrlParameters(":", ";", ";");
return "jdbc:db2://" + getHost() + ":" + getMappedPort(DB2_PORT) +
"/" + databaseName + additionalUrlParams;
}

@Override
Expand Down
Expand Up @@ -7,6 +7,8 @@
import java.sql.ResultSet;
import java.sql.SQLException;

import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;
import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;

public class SimpleDb2Test extends AbstractContainerDatabaseTest {
Expand All @@ -24,4 +26,17 @@ public void testSimple() throws SQLException {
assertEquals("A basic SELECT query succeeds", 1, resultSetInt);
}
}

@Test
public void testWithAdditionalUrlParamInJdbcUrl() {
try (Db2Container db2 = new Db2Container()
.withUrlParam("sslConnection", "false")
.acceptLicense()) {

db2.start();

String jdbcUrl = db2.getJdbcUrl();
assertThat(jdbcUrl, containsString(":sslConnection=false;"));
}
}
}
Expand Up @@ -2,6 +2,7 @@

import com.github.dockerjava.api.command.InspectContainerResponse;
import lombok.NonNull;
import org.apache.commons.lang.StringUtils;
import org.jetbrains.annotations.NotNull;
import org.testcontainers.containers.traits.LinkableContainer;
import org.testcontainers.delegate.DatabaseDelegate;
Expand All @@ -17,6 +18,8 @@
import java.util.Properties;
import java.util.concurrent.Future;

import static java.util.stream.Collectors.joining;

/**
* Base class for containers that expose a JDBC connection
*
Expand All @@ -28,6 +31,7 @@ public abstract class JdbcDatabaseContainer<SELF extends JdbcDatabaseContainer<S
private Driver driver;
private String initScriptPath;
protected Map<String, String> parameters = new HashMap<>();
protected Map<String, String> urlParameters = new HashMap<>();

private int startupTimeoutSeconds = 120;
private int connectTimeoutSeconds = 120;
Expand Down Expand Up @@ -82,7 +86,11 @@ public SELF withPassword(String password) {

public SELF withDatabaseName(String dbName) {
throw new UnsupportedOperationException();
}

public SELF withUrlParam(String paramName, String paramValue) {
urlParameters.put(paramName, paramValue);
return self();
}

/**
Expand Down Expand Up @@ -223,6 +231,21 @@ protected String constructUrlForConnection(String queryString) {
return getJdbcUrl() + queryString;
}

protected String constructUrlParameters(String startCharacter, String delimiter) {
return constructUrlParameters(startCharacter, delimiter, StringUtils.EMPTY);
}

protected String constructUrlParameters(String startCharacter, String delimiter, String endCharacter) {
String urlParameters = "";
if (!this.urlParameters.isEmpty()) {
String additionalParameters = this.urlParameters.entrySet().stream()
.map(Object::toString)
.collect(joining(delimiter));
urlParameters = startCharacter + additionalParameters + endCharacter;
}
return urlParameters;
}

protected void optionallyMapResourceParameterAsVolume(@NotNull String paramName, @NotNull String pathNameInContainer, @NotNull String defaultResource) {
String resourceName = parameters.getOrDefault(paramName, defaultResource);

Expand Down
Expand Up @@ -60,7 +60,9 @@ public String getDriverClassName() {

@Override
public String getJdbcUrl() {
return "jdbc:mariadb://" + getHost() + ":" + getMappedPort(MARIADB_PORT) + "/" + databaseName;
String additionalUrlParams = constructUrlParameters("?", "&");
return "jdbc:mariadb://" + getHost() + ":" + getMappedPort(MARIADB_PORT) +
"/" + databaseName + additionalUrlParams;
}

@Override
Expand Down
Expand Up @@ -8,6 +8,8 @@
import java.sql.ResultSet;
import java.sql.SQLException;

import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;
import static org.junit.Assume.assumeFalse;
import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;
Expand Down Expand Up @@ -68,4 +70,22 @@ public void testMariaDBWithCommandOverride() throws SQLException {
assertEquals("Auto increment increment should be overriden by command line", "10", result);
}
}

@Test
public void testWithAdditionalUrlParamInJdbcUrl() {
MariaDBContainer mariaDBContainer = (MariaDBContainer) new MariaDBContainer()
.withUrlParam("connectTimeout", "40000")
.withUrlParam("rewriteBatchedStatements", "true");

try {
mariaDBContainer.start();
String jdbcUrl = mariaDBContainer.getJdbcUrl();
assertThat(jdbcUrl, containsString("?"));
assertThat(jdbcUrl, containsString("&"));
assertThat(jdbcUrl, containsString("rewriteBatchedStatements=true"));
assertThat(jdbcUrl, containsString("connectTimeout=40000"));
} finally {
mariaDBContainer.stop();
}
}
}
Expand Up @@ -62,7 +62,8 @@ public String getDriverClassName() {

@Override
public String getJdbcUrl() {
return "jdbc:sqlserver://" + getHost() + ":" + getMappedPort(MS_SQL_SERVER_PORT);
String additionalUrlParams = constructUrlParameters(";", ";");
return "jdbc:sqlserver://" + getHost() + ":" + getMappedPort(MS_SQL_SERVER_PORT) + additionalUrlParams;
}

@Override
Expand Down
Expand Up @@ -9,6 +9,8 @@
import java.sql.SQLException;
import java.sql.Statement;

import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;
import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;

public class SimpleMSSQLServerTest extends AbstractContainerDatabaseTest {
Expand All @@ -24,6 +26,20 @@ public void testSimple() throws SQLException {
}
}

@Test
public void testWithAdditionalUrlParamInJdbcUrl() {
try (MSSQLServerContainer<?> mssqlServer = new MSSQLServerContainer<>()
.withUrlParam("integratedSecurity", "false")
.withUrlParam("applicationName", "MyApp")) {

mssqlServer.start();

String jdbcUrl = mssqlServer.getJdbcUrl();
assertThat(jdbcUrl, containsString(";integratedSecurity=false;applicationName=MyApp"));
}
}


@Test
public void testSetupDatabase() throws SQLException {
try (MSSQLServerContainer<?> mssqlServer = new MSSQLServerContainer<>()) {
Expand Down
Expand Up @@ -5,6 +5,8 @@
import java.util.HashSet;
import java.util.Set;

import static java.util.stream.Collectors.joining;

/**
* @author richardnorth
*/
Expand Down Expand Up @@ -70,7 +72,9 @@ public String getDriverClassName() {

@Override
public String getJdbcUrl() {
return "jdbc:mysql://" + getHost() + ":" + getMappedPort(MYSQL_PORT) + "/" + databaseName;
String additionalUrlParams = constructUrlParameters("?", "&");
return "jdbc:mysql://" + getHost() + ":" + getMappedPort(MYSQL_PORT) +
"/" + databaseName + additionalUrlParams;
}

@Override
Expand Down
Expand Up @@ -9,9 +9,17 @@
import org.testcontainers.containers.output.Slf4jLogConsumer;
import org.testcontainers.db.AbstractContainerDatabaseTest;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;

import java.sql.Statement;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;

import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeFalse;
import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
Expand Down Expand Up @@ -142,4 +150,77 @@ public void testEmptyPasswordWithRootUser() throws SQLException {
assertEquals("A basic SELECT query succeeds", 1, resultSetInt);
}
}

@Test
public void testWithAdditionalUrlParamTimeZone() throws SQLException {
MySQLContainer mysql = (MySQLContainer) new MySQLContainer()
.withUrlParam("serverTimezone", "Europe/Zurich")
.withEnv("TZ", "Europe/Zurich")
.withLogConsumer(new Slf4jLogConsumer(logger));
mysql.start();

try(Connection connection = mysql.createConnection("")) {
Statement statement = connection.createStatement();
statement.execute("SELECT NOW();");
try (ResultSet resultSet = statement.getResultSet()) {
resultSet.next();

// checking that the time_zone MySQL is Europe/Zurich
LocalDateTime localDateTime = resultSet.getObject(1, LocalDateTime.class);
ZonedDateTime actualDateTime = localDateTime.atZone(ZoneId.of("Europe/Zurich"))
.truncatedTo(ChronoUnit.MINUTES);
ZonedDateTime expectedDateTime = ZonedDateTime.now(ZoneId.of("Europe/Zurich"))
.truncatedTo(ChronoUnit.MINUTES);

String message = String.format("MySQL time zone is not Europe/Zurich. MySQL date:%s, current date:%s",
actualDateTime, expectedDateTime);
assertTrue(message, actualDateTime.equals(expectedDateTime));
}
} finally {
mysql.stop();
}
}

@Test
public void testWithAdditionalUrlParamMultiQueries() throws SQLException {
MySQLContainer mysql = (MySQLContainer) new MySQLContainer()
.withUrlParam("allowMultiQueries", "true")
.withLogConsumer(new Slf4jLogConsumer(logger));
mysql.start();

try(Connection connection = mysql.createConnection("")) {
Statement statement = connection.createStatement();
String multiQuery = "DROP TABLE IF EXISTS bar; " +
"CREATE TABLE bar (foo VARCHAR(20)); " +
"INSERT INTO bar (foo) VALUES ('hello world');";
statement.execute(multiQuery);
statement.execute("SELECT foo FROM bar;");
try(ResultSet resultSet = statement.getResultSet()) {
resultSet.next();
String firstColumnValue = resultSet.getString(1);
assertEquals("Value from bar should equal real value", "hello world", firstColumnValue);
}
} finally {
mysql.stop();
}
}

@Test
public void testWithAdditionalUrlParamInJdbcUrl() {
MySQLContainer mysql = (MySQLContainer) new MySQLContainer()
.withUrlParam("allowMultiQueries", "true")
.withUrlParam("rewriteBatchedStatements", "true")
.withLogConsumer(new Slf4jLogConsumer(logger));

try {
mysql.start();
String jdbcUrl = mysql.getJdbcUrl();
assertThat(jdbcUrl, containsString("?"));
assertThat(jdbcUrl, containsString("&"));
assertThat(jdbcUrl, containsString("rewriteBatchedStatements=true"));
assertThat(jdbcUrl, containsString("allowMultiQueries=true"));
} finally {
mysql.stop();
}
}
}
Expand Up @@ -89,6 +89,11 @@ public OracleContainer withPassword(String password) {
return self();
}

@Override
public OracleContainer withUrlParam(String paramName, String paramValue) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since some DBs like Oracle don't support URL parameters, but do support Properties parameters, I think it is worth considering renaming this method to withParam() or withJdbcParam(), so that client code doesn't have to call withUrlParameter() for some DBs and something like withPropertiesParam() for others (assuming that will be also supported in the future). I just think this would be worth considering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not entirely correct. What do you think about that, @rnorth ? Thx!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to take so long to come back to this PR to break the tie...

I think the current form is reasonable; Oracle does not support JDBC URL parameters (in the normal sense of a URL parameter, anyway). Also, we don't support setting connection properties for the Oracle driver.

It's good to think about the future case of being agnostic of whether the connection params are set in the URL or as properties. However, I think that:

  1. the params themselves are so database-specific that it's rarely going to be possible to write them generically, and
  2. trying to support both URL params and properties via one method is going to tie us in knots if we ever actually support pass-through of properties - we're not going to know which params go where.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Let's tackle that problem with connection properties separately. From my POV this commit looks good to go!

throw new UnsupportedOperationException("The OracleDb does not support this");
}

@SuppressWarnings("SameReturnValue")
public String getSid() {
return "xe";
Expand Down
Expand Up @@ -55,6 +55,8 @@ protected Set<Integer> getLivenessCheckPorts() {

@Override
protected void configure() {
// Disable Postgres driver use of java.util.logging to reduce noise at startup time
withUrlParam("loggerLevel", "OFF");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't investigated what exactly this does, but it looks a bit weird. I can understand if by default the logging level should not be DEBUG or so, but I would definitely like to see errors and probably also warnings by default (i.e. without having to investigate how to change this manually).

Please disregard if I missed something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be in the getJdbcUrl method like this:

return "jdbc:postgresql://" + getContainerIpAddress() + ":" + getMappedPort(POSTGRESQL_PORT) + "/" + databaseName + "?loggerLevel=OFF";

I just moved it to the configure() method.

I'm guessing the logger was turned off on purpose to remove the noise at the start of the container.

addEnv("POSTGRES_DB", databaseName);
addEnv("POSTGRES_USER", username);
addEnv("POSTGRES_PASSWORD", password);
Expand All @@ -67,25 +69,9 @@ public String getDriverClassName() {

@Override
public String getJdbcUrl() {
// Disable Postgres driver use of java.util.logging to reduce noise at startup time
return format("jdbc:postgresql://%s:%d/%s?loggerLevel=OFF", getHost(), getMappedPort(POSTGRESQL_PORT), databaseName);
}

@Override
protected String constructUrlForConnection(String queryString) {
String baseUrl = getJdbcUrl();

if ("".equals(queryString)) {
return baseUrl;
}

if (!queryString.startsWith("?")) {
throw new IllegalArgumentException("The '?' character must be included");
}

return baseUrl.contains("?")
? baseUrl + QUERY_PARAM_SEPARATOR + queryString.substring(1)
: baseUrl + queryString;
String additionalUrlParams = constructUrlParameters("?", "&");
return "jdbc:postgresql://" + getContainerIpAddress() + ":" + getMappedPort(POSTGRESQL_PORT)
+ "/" + databaseName + additionalUrlParams;
}

@Override
Expand Down