testcontainers-java icon indicating copy to clipboard operation
testcontainers-java copied to clipboard

Add getR2dbcUrl helper method to JdbcDatabaseContainer

Open mipo256 opened this issue 1 year ago • 9 comments

Module

Core

Problem

I might've been chosen the wrong module, sorry for that, haven't found one for all RDBMS containers.

There is a convenient method, getJdbcUrl in JdbcDatabaseContainer. The problem however, if I'm working with R2DBC the connection string schema is different. Currently, I have to create an R2DBC connection string myself, which is a bit tedious.

Solution

It would be great to have a similar method in JdbcDatabaseContainer, like getR2dbcUrl that would construct the R2DBC compliant connection string.

Benefit

This would make integration with R2DBC drivers more seamless and provide better developer experience.

Alternatives

The alternative, I think, is to create the connection string manually, which might be fine, not a big deal, until you have to do it over and over again, in which case it became really frustrating.

Would you like to help contributing this feature?

Yes

mipo256 avatar Jun 23 '24 13:06 mipo256

  • I would say this sounds very magical and has little practical value. Although most databases will only have one R2DBC Driver, there may actually be multiple JDBC Drivers for a single database. For example, for the third-party PostgreSQL JDBC Driver https://github.com/awslabs/aws-postgresql-jdbc , there is obviously only jdbc:postgresql:aws://<host>:5432/<database>, but there is no corresponding r2dbc:postgresql:aws://<host>:5432/<database>.

linghengqian avatar Jul 21 '24 07:07 linghengqian

@linghengqian I do not understand your point at all. R2DBC is a spec. It is a spec. The format of such URL is predefined for each implementation. Therefore, it does not matter whether one RDBMS has 0, 1 or multiple R2DBC drivers - they still will comply to the standard.

I think adding this is utterly clear and portable, in contrast to example with JDBC URL which is not standardized by a single spec. If we care about the user, who is trying to get R2DBC URL for the database, which actually does not have the R2DBC driver - who's fault that actually is?

mipo256 avatar Jul 22 '24 11:07 mipo256

Hi, thanks for the suggestion. I don't think this is feasible nowadays, the change must be done in R2DBCDatabaseContainer. But, it would be used R2DBCDatabaseContainer.getR2dbcUrl(PostgreSQLR2DBCDatabaseContainer.getOptions(postgresContainer)), which I don't think is friendly enough.

Testcontainers R2dbc integrations in spring-boot and micronaut test-resources are based on PostgreSQLR2DBCDatabaseContainer.getOptions(postgresContainer)

Other alternative is to implement getR2dbcUrl in each implementation but given the spec I would rather prefer the having a general method that applies to all.

eddumelendez avatar Aug 13 '24 04:08 eddumelendez

  • Given the ecological situation of r2dbc, I don't think r2dbc will continue to develop healthily, and testcontainers do not need to consider the interoperability of r2dbc and jdbc. An example is that https://central.sonatype.com/artifact/io.r2dbc/r2dbc-h2 , which is used to replace the h2database jdbc driver, has not had a new version for more than a year.

  • From the perspective of 2024, the ADBC ​​Driver involved in https://github.com/apache/arrow-adbc is more likely to become the real future of database clients, because ADBC ​​Driver is not designed just for Java, it also has specifications for a number of programming languages ​​such as C/Glib, C++, C#, Go, etc. for designing and implementing drivers. See https://arrow.apache.org/docs/format/ADBC.html .

linghengqian avatar Oct 04 '24 10:10 linghengqian

I honestly do not understand how the case for comparison of arrow-adbc and r2dbc specification can be made. These are meant for different things, that does not make any sense.

Apart from that, solving this issue does not seem to be that complicated (but I might be wrong). However, it is indeed a bit of shame that so much time spent for discussion and arguments for such a small feature.

mipo256 avatar Oct 04 '24 10:10 mipo256

The Testcontainers team has no negative sentiment towards R2DBC and no opinion with regards to its future direction. We aren't not opposed to the feature request from a user perspective at all, we just need to agree on the design/API.

For me it sounds like it could be possible to implement getR2dbcUrl() in JdbcDatabaseContainer and then have the individual container classes implement some schema specific getters. These are also totally fine to be opinionated (i.e. with regards to the driver), users can still opt to manually create the connection string instead and our getJdbcUrl() method is also highly opinionated.

Hi, thanks for the suggestion. I don't think this is feasible nowadays, the change must be done in R2DBCDatabaseContainer.

@eddumelendez Maybe you can elaborate why it would not be feasible with the current design?

kiview avatar Oct 07 '24 14:10 kiview

I took another look and the following method can be provided

Example using MSSQLR2DBCDatabaseContainer

public static String getR2dbcUrl(MSSQLServerContainer<?> container) {
    ConnectionFactoryOptions options = getOptions(container);
    return String.format("...", options.getValue(Option.valueOf("")));
}

every R2DBCDatabaseContainer implementation must provide the static method.

@eddumelendez Maybe you can elaborate why it would not be feasible with the current design?

I would like to enforce the implementation by declaring the method in the interface but more breaking changes would be require. I think the suggestion above will cover the current need.

eddumelendez avatar Oct 08 '24 03:10 eddumelendez

We could implement the method getR2dbcUrl() in the R2DBCDatabaseContainer interface with a default implementation that throws an UnsupportedOperationException, to avoid it being a breaking change to potential 3rd party implementations of the interface out there.

@mipo256 For your use case, is having this method on R2DBCDatabaseContainer appropriate, or would you want/need it on JdbcDatabaseContainer? I think also on JdbcDatabaseContainer, we could follow such an approach with a default UnsupportedOperationException implementation to avoid breaking changes.

kiview avatar Oct 14 '24 14:10 kiview

We have agreed on adding a new static method on each R2DBCDatabaseContainer implementation

For example, under MySQLR2DBCDatabaseContainer

public static String getR2dbcUrl(MySQLContainer<?> container) {
    return ...;
}

Current users used to do MySQLR2DBCDatabaseContainer.getOptions(container), so, in order to make this discoverable we decided to proceed with static method.

Please free free to submit a PR.

eddumelendez avatar Oct 17 '24 18:10 eddumelendez

submit a PR https://github.com/testcontainers/testcontainers-java/pull/9569

DHKIM-0511 avatar Nov 26 '24 11:11 DHKIM-0511