-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow setting database name and user when using JDBC URL #594
Changes from 13 commits
63343fe
6a1726a
6f151da
6daa5b0
8e313f1
b3d290a
ffb84f4
4006d4f
c10f4b6
d807546
1be1c16
d6dcc89
5d8f12a
f162081
8fde60e
e29a339
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,3 +46,9 @@ node_modules/ | |
|
||
.gradle/ | ||
build/ | ||
|
||
# Eclipse IDE files | ||
**/.project | ||
**/.classpath | ||
**/.settings | ||
**/bin/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,10 @@ protected boolean isApplicable() { | |
return true; | ||
} | ||
|
||
protected boolean isPersistable() { | ||
return true; | ||
} | ||
|
||
/** | ||
* @return highest to lowest priority value | ||
*/ | ||
|
@@ -93,7 +97,10 @@ public static DockerClientProviderStrategy getFirstValidStrategy(List<DockerClie | |
LOGGER.warn("Can't instantiate a strategy from {}", it, e); | ||
return Stream.empty(); | ||
} | ||
}), | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the formatting in this Stream.concat... block just 'idea' being weird? doesnt seem to follow the 4 space indents everywhere else. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class is not modified and related for this PR, probably upstream has changed and so you see the difference. I would leave it as-is for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// Ignore persisted strategy if it's not persistable anymore | ||
.filter(DockerClientProviderStrategy::isPersistable) | ||
.peek(strategy -> LOGGER.info("Loaded {} from ~/.testcontainers.properties, will try it first", strategy.getClass().getName())), | ||
strategies | ||
.stream() | ||
.filter(DockerClientProviderStrategy::isApplicable) | ||
|
@@ -104,7 +111,9 @@ public static DockerClientProviderStrategy getFirstValidStrategy(List<DockerClie | |
strategy.test(); | ||
LOGGER.info("Found Docker environment with {}", strategy.getDescription()); | ||
|
||
TestcontainersConfiguration.getInstance().updateGlobalConfig("docker.client.strategy", strategy.getClass().getName()); | ||
if (strategy.isPersistable()) { | ||
TestcontainersConfiguration.getInstance().updateGlobalConfig("docker.client.strategy", strategy.getClass().getName()); | ||
} | ||
|
||
return Stream.of(strategy); | ||
} catch (Exception | ExceptionInInitializerError | NoClassDefFoundError e) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,8 +37,8 @@ public static Iterable<Object[]> data() { | |
return asList( | ||
new Object[][]{ | ||
{"jdbc:tc:mysql:5.5.43://hostname/databasename", false, false, false}, | ||
{"jdbc:tc:mysql://hostname/databasename?TC_INITSCRIPT=somepath/init_mysql.sql", true, false, false}, | ||
{"jdbc:tc:mysql://hostname/databasename?TC_INITFUNCTION=org.testcontainers.jdbc.JDBCDriverTest::sampleInitFunction", true, false, false}, | ||
{"jdbc:tc:mysql://hostname/databasename?user=someuser&password=somepwd&TC_INITSCRIPT=somepath/init_mysql.sql", true, false, false}, | ||
{"jdbc:tc:mysql://hostname/databasename?user=someuser&password=somepwd&TC_INITFUNCTION=org.testcontainers.jdbc.JDBCDriverTest::sampleInitFunction", true, false, false}, | ||
{"jdbc:tc:mysql://hostname/databasename?useUnicode=yes&characterEncoding=utf8", false, true, false}, | ||
{"jdbc:tc:mysql://hostname/databasename", false, false, false}, | ||
{"jdbc:tc:mysql://hostname/databasename?useSSL=false", false, false, false}, | ||
|
@@ -100,7 +100,21 @@ private void performTestForScriptedSchema(String jdbcUrl) throws SQLException { | |
assertEquals("A basic SELECT query succeeds where the schema has been applied from a script", "hello world", resultSetString); | ||
return true; | ||
}); | ||
|
||
|
||
result = new QueryRunner(dataSource).query("select CURRENT_USER()", rs -> { | ||
rs.next(); | ||
String resultUser = rs.getString(1); | ||
assertEquals("User from query param is created.", "someuser@%", resultUser); | ||
return true; | ||
}); | ||
|
||
result = new QueryRunner(dataSource).query("SELECT DATABASE()", rs -> { | ||
rs.next(); | ||
String resultDB = rs.getString(1); | ||
assertEquals("Database name from URL String is used.", "databasename", resultDB); | ||
return true; | ||
}); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just a trivial comment, but please could you reformat using 4 spaces for indentation? IntelliJ/Eclipse defaults ought to be reasonably close. I'll add automatic formatting soon... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I'll do that. Thanks for reviewing! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have updated formatting in recent commit. Thanks! |
||
assertTrue("The database returned a record as expected", result); | ||
|
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,208 @@ | ||
package org.testcontainers.jdbc; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.Optional; | ||
import java.util.StringJoiner; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
|
||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
|
||
/** | ||
* This is an Immutable class holding JDBC Connection Url and its parsed components, used by {@link ContainerDatabaseDriver}. | ||
* | ||
* {@link ConnectionUrl#parseUrl()} method must be called after instantiating this class. | ||
* | ||
* @author manikmagar | ||
* | ||
*/ | ||
public class ConnectionUrl { | ||
|
||
@Getter | ||
private String url; | ||
|
||
private String databaseType; | ||
|
||
@Getter | ||
private String imageTag = "latest"; | ||
|
||
private String dbHostString; | ||
|
||
@Getter | ||
private boolean inDaemonMode = false; | ||
|
||
@Getter | ||
private Optional<String> databaseHost = Optional.empty(); | ||
|
||
@Getter | ||
private Optional<Integer> databasePort = Optional.empty(); | ||
|
||
@Getter | ||
private Optional<String> databaseName = Optional.empty(); | ||
|
||
@Getter | ||
private Optional<String> initScriptPath = Optional.empty(); | ||
|
||
@Getter | ||
private Optional<InitFunctionDef> initFunction = Optional.empty(); | ||
|
||
@Getter | ||
private Optional<String> queryString; | ||
|
||
private ConnectionUrl() { | ||
//Not Allowed here | ||
} | ||
|
||
public ConnectionUrl(final String url) { | ||
this.url = Objects.requireNonNull(url, "Connection URL cannot be null"); | ||
} | ||
|
||
public String getDatabaseType() { | ||
return Objects.requireNonNull(this.databaseType, "Database Type cannot be null. Have you called parseUrl() method?"); | ||
} | ||
|
||
|
||
/** | ||
* This is a part of the connection string that may specify host:port/databasename. | ||
* It may vary for different clients and so clients can parse it as needed. | ||
* @return | ||
*/ | ||
public String getDbHostString() { | ||
return Objects.requireNonNull(this.dbHostString, "Database Host String cannot be null. Have you called parseUrl() method?"); | ||
} | ||
|
||
public static boolean accepts(final String url) { | ||
return url.startsWith("jdbc:tc:"); | ||
} | ||
|
||
public void parseUrl() { | ||
/* | ||
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 = Patterns.URL_MATCHING_PATTERN.matcher(this.getUrl()); | ||
if (!urlMatcher.matches()) { | ||
//Try for Oracle pattern | ||
urlMatcher = Patterns.ORACLE_URL_MATCHING_PATTERN.matcher(this.getUrl()); | ||
if(!urlMatcher.matches()) { | ||
throw new IllegalArgumentException("JDBC URL matches jdbc:tc: prefix but the database or tag name could not be identified"); | ||
} | ||
} | ||
databaseType = urlMatcher.group(1); | ||
|
||
imageTag = Optional.ofNullable(urlMatcher.group(3)).orElse("latest"); | ||
|
||
//String like hostname:port/database name, which may vary based on target database. | ||
//Clients can further parse it as needed. | ||
dbHostString = urlMatcher.group(4); | ||
|
||
//In case it matches to the default pattern | ||
Matcher dbInstanceMatcher = Patterns.DB_INSTANCE_MATCHING_PATTERN.matcher(dbHostString); | ||
if(dbInstanceMatcher.matches()) { | ||
databaseHost = Optional.of(dbInstanceMatcher.group(1)); | ||
databasePort = Optional.ofNullable(dbInstanceMatcher.group(3)).map(value -> Integer.valueOf(value)); | ||
databaseName = Optional.of(dbInstanceMatcher.group(4)); | ||
} | ||
|
||
queryString = Optional.ofNullable(urlMatcher.group(5)); | ||
getQueryParameters(); | ||
|
||
Matcher matcher = Patterns.INITSCRIPT_MATCHING_PATTERN.matcher(this.getUrl()); | ||
if(matcher.matches()) { | ||
initScriptPath = Optional.ofNullable(matcher.group(2)); | ||
} | ||
|
||
Matcher funcMatcher = Patterns.INITFUNCTION_MATCHING_PATTERN.matcher(this.getUrl()); | ||
if(funcMatcher.matches()) { | ||
initFunction = Optional.of(new InitFunctionDef(funcMatcher.group(2), funcMatcher.group(4))); | ||
} | ||
|
||
Matcher daemonMatcher = Patterns.DAEMON_MATCHING_PATTERN.matcher(this.getUrl()); | ||
inDaemonMode = daemonMatcher.matches() ? Boolean.parseBoolean(daemonMatcher.group(2)) : false; | ||
|
||
} | ||
|
||
/** | ||
* Get the TestContainers Parameters such as Init Function, Init Script path etc. | ||
* @return {@link Map} | ||
*/ | ||
public Map<String, String> getContainerParameters() { | ||
|
||
Map<String, String> results = new HashMap<>(); | ||
|
||
Matcher matcher = Patterns.TC_PARAM_MATCHING_PATTERN.matcher(this.getUrl()); | ||
while (matcher.find()) { | ||
String key = matcher.group(1); | ||
String value = matcher.group(2); | ||
results.put(key, value); | ||
} | ||
|
||
return results; | ||
} | ||
|
||
/** | ||
* Get all Query paramters specified in the Connection URL after ?. This also includes TestContainers parameters. | ||
* @return {@link Map} | ||
*/ | ||
public Map<String, String> getQueryParameters() { | ||
|
||
Map<String, String> results = new HashMap<>(); | ||
StringJoiner query = new StringJoiner("&"); | ||
Matcher matcher = Patterns.QUERY_PARAM_MATCHING_PATTERN.matcher(this.getQueryString().orElse("")); | ||
while (matcher.find()) { | ||
String key = matcher.group(1); | ||
String value = matcher.group(2); | ||
if(!key.startsWith("TC_")) query.add(key + "=" + value); | ||
results.put(key, value); | ||
} | ||
|
||
queryString = Optional.of("?" + query.toString()); | ||
return results; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object obj) { | ||
if(Objects.isNull(obj) || !(obj instanceof ConnectionUrl)) return false; | ||
return this.getUrl().equals(((ConnectionUrl)obj).getUrl()); | ||
} | ||
|
||
/** | ||
* This interface defines the Regex Patterns used by {@link ConnectionUrl}. | ||
* | ||
* @author manikmagar | ||
* | ||
*/ | ||
public interface Patterns { | ||
final Pattern URL_MATCHING_PATTERN = Pattern.compile("jdbc:tc:([a-z]+)(:([^:]+))?://([^\\?]+)(\\?.*)?"); | ||
|
||
final Pattern ORACLE_URL_MATCHING_PATTERN = Pattern.compile("jdbc:tc:([a-z]+)(:([^(thin:)]+))?:thin:@([^\\?]+)(\\?.*)?"); | ||
|
||
//Matches to part of string - hostname:port/databasename | ||
final Pattern DB_INSTANCE_MATCHING_PATTERN = Pattern.compile("([^:]+)(:([0-9]+))?/([^\\\\?]+)"); | ||
|
||
final Pattern DAEMON_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_DAEMON=([^\\?&]+).*"); | ||
final Pattern INITSCRIPT_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_INITSCRIPT=([^\\?&]+).*"); | ||
final Pattern INITFUNCTION_MATCHING_PATTERN = Pattern.compile(".*([\\?&]?)TC_INITFUNCTION=" + | ||
"((\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*\\.)*\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)" + | ||
"::" + | ||
"(\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*)" + | ||
".*"); | ||
|
||
final Pattern TC_PARAM_MATCHING_PATTERN = Pattern.compile("(TC_[A-Z_]+)=([^\\?&]+)"); | ||
|
||
final Pattern QUERY_PARAM_MATCHING_PATTERN = Pattern.compile("([^\\?&=]+)=([^\\?&]+)"); | ||
|
||
} | ||
|
||
@Getter | ||
@AllArgsConstructor | ||
public class InitFunctionDef { | ||
private String className; | ||
private String methodName; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please could you update the changelog to describe this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated Changelog with items that I feel related to this PR. Please take a look and let me know/feel free to modify as you seem right.