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

MySQLContainer does not have getter for the databaseName field #473

Closed
dmatej opened this issue Oct 18, 2017 · 3 comments
Closed

MySQLContainer does not have getter for the databaseName field #473

dmatej opened this issue Oct 18, 2017 · 3 comments

Comments

@dmatej
Copy link

dmatej commented Oct 18, 2017

In Java EE containers like Payara is JDBC pool and JDBC driver configured via properties, not URL. It is more useful to provide username, password, serverName, portName and databaseName etc. than URL.
getJdbcUrl is higher abstraction layer over it.

@bsideup
Copy link
Member

bsideup commented Nov 6, 2017

Hi @dmatej,

At least in TestContainers 1.4.3, there is getUsername, getPassword, getContainerIpAddress and getFirstMappedPort/getExposedPort(0) for that.

DB name ATM is static and has test value. There is also withDatabaseName to override it, but then you know it upfront :)

@dmatej
Copy link
Author

dmatej commented Nov 6, 2017

The problem is not about setting the test container but with using it's active configuration for the creation of the JDBC resource, this is why URL is not useful. I would like to simply use getters to configure the JDBC pool from the test container's getters but now I have to duplicate values or use sometimes getters sometimes static values. The code is inconsistent and programming not intuitive, I had to find out where can I get values I needed.

getFirstMappedPort has another implementation than getUrl uses - this is the reason why I wrote the getUrl method parsing port from the url instead (but now I see both ways give same result, ok).

It would be much more transparent to have standard JDBC getters and let user to access all useful values. Or maybe to create some transfer object for that and provide getJdbcConfiguration?.

    LOG.info("Starting the MySql docker service...");
    mysql = new MySQLContainer<>().withDatabaseName(DB_NAME)//
        .withEnv("MYSQL_LOG_QUERIES_ENABLED", "1")//
        .withEnv("TC_INITFUNCTION", ArquillianDaoUnitTest.class.getCanonicalName() + "::initCollation")//
        .withLogConsumer(new Slf4jLogConsumer(LoggerFactory.getLogger(ArquillianDaoUnitTest.class)));//
    mysql.start();

      runCommand("create-jdbc-connection-pool", "--ping", "--restype", "javax.sql.DataSource", //
          "--datasourceclassname", "com.mysql.cj.jdbc.MysqlDataSource", "--property",
          "user=" + mysql.getUsername() + ":password=" + mysql.getPassword() + ":DatabaseName=" + DB_NAME //
              + ":ServerName=" + mysql.getContainerIpAddress() + ":port=" + getJdbcPort()
              + ":zeroDateTimeBehavior=convertToNull:useUnicode=true:useJDBCCompliantTimezoneShift=true"
              + ":useLegacyDatetimeCode=true:serverTimezone=UTC:characterEncoding=UTF-8"
              + ":useInformationSchema=true:nullCatalogMeansCurrent=true:nullNamePatternMatchesAll=false", //
          "xxx-test");
      runCommand("list-jdbc-connection-pools", "--echo=true", "--terse=true");
      runCommand("set", "resources.jdbc-connection-pool.xxx-test.steady-pool-size=5");
      runCommand("set", "resources.jdbc-connection-pool.xxx-test.max-pool-size=20");
      runCommand("set", "resources.jdbc-connection-pool.xxx-test.connection-validation-method=auto-commit");
      runCommand("set", "resources.jdbc-connection-pool.xxx-test.is-connection-validation-required=true");
      runCommand("set", "resources.jdbc-connection-pool.xxx-test.fail-all-connections=true");
      runCommand("create-jdbc-resource", "--connectionpoolid", "xxx-test", "jdbc/xxx-jta");

@kiview
Copy link
Member

kiview commented Nov 7, 2017

@bsideup I suppose there is no damage in adding a getter for databaseName, so we have a consistent API?

dbyron0 referenced this issue in locationlabs/testcontainers-java Dec 14, 2017
dbyron0 referenced this issue in locationlabs/testcontainers-java Dec 15, 2017
@kiview kiview closed this as completed in fcefad3 Dec 16, 2017
rnorth pushed a commit that referenced this issue Dec 18, 2017
Container classes which want to support this feature need to override getDatabaseName() method.
Fixes #473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants