Add R2DBC support #2545

bsideup · 2020-04-09T12:27:08Z

This PR adds support for R2DBC.

URL support

The URL support is similar to the JDBC's, but:

Does not reuse containers by URL - it is unclear how to "hash" ConnectionFactoryOptions
The image tag must be explicitly provided via TC_IMAGE_TAG r2dbc option
TODO: daemon mode
TODO: alias support (see Support custom Docker images in the JDBC URL #4)

Example usage

The user must add the org.testcontainers:r2dbc module - unlike JDBC, we use compileOnly dependency on R2DBC, since it is a 3rd party dependency.

Once both the r2dbc and database's module are added, one can use:

String url = "r2dbc:tc:postgresql:///db?TC_IMAGE_TAG=10-alpine";
ConnectionFactory connectionFactory = ConnectionFactories.get(url);

Programmatic access

For those who start containers manually and want to obtain ConnectionFactoryOptions for already started containers, we can provide a helper static method:

try (PostgreSQLContainer<?> container = new PostgreSQLContainer<>()) {
    container.start();

    ConnectionFactory connectionFactory = ConnectionFactories.get(
        PostgreSQLR2DBCDatabaseContainer.getOptions(container)
    );
}

I decided to leave some generic way (e.g. R2dbcContainers.getOptions(container);) for future considerations because:

we will need to apply instanceof checks (or expose getR2dbcDriverType()-like method) on DatabaseContainer
we need some sort of a sorting to avoid postgres taking precedence before postgis and returning wrong factory
probably more :D

/cc @mp911de

OlegDokuka · 2020-04-09T12:39:44Z

modules/r2dbc/src/main/java/org/testcontainers/r2dbc/ConnectionPublisher.java

+
+            @Override
+            public void cancel() {
+                s.cancel();


might be a case when

future just completed -> void enter() { publisher.subscribe(this); } executed but subscription may be delivered later (See https://github.com/reactive-streams/reactive-streams-jvm/issues/486 for the clarification) so the next cancel which is waiting on the synchronized block will end-up with NPE.

The simples solution for that is to add flag and synchronization between request cancel and onsubscriber to ensure we are not ending up with NPE if the subscription has not arrived yet

OlegDokuka · 2020-04-09T12:41:13Z

modules/r2dbc/src/main/java/org/testcontainers/r2dbc/ConnectionPublisher.java

+
+            @Override
+            public void onSubscribe(Subscription s) {
+                this.s = s;


here we have to have a flag, e.g (canceled) and if so, immediately call s.cancel on the given one

simonbasle · 2020-04-09T12:40:54Z

modules/r2dbc/src/main/java/org/testcontainers/r2dbc/ConnectionPublisher.java

+            @Override
+            public void onSubscribe(Subscription s) {
+                this.s = s;
+                s.request(1);


@mp911de does the connection factory absolutely guarantees a Publisher with at most one onNext(Connection)? if not, @bsideup it might be worth it to consider guarding against flux-like publishers by adding a boolean guard in onNext (doesn't even have to be a volatile, since only onNext would read/write it)

it does (checked with Mark), yes. See the design notes in the Javadoc

it was unclear to me that comment was about the factory, since it mentions ConnectionPublisher (the current class). I would instead phrase it as Publisher<Connection> provided by R2DBC / the connectionFactory is guaranteed to emit at most one item

connectionFactory implements single element or failure semantics. Completion without an element or emitting multiple items violates the spec. It's a cold publisher that should allow multiple subscriptions.

simonbasle · 2020-04-09T12:52:19Z

modules/r2dbc/src/main/java/org/testcontainers/r2dbc/ConnectionPublisher.java

+            newState.enter();
+        }
+
+        abstract class SubscriptionState implements Subscription {


that pattern was a bit harder for me to follow because of the habit of seeing Subscription as self-sufficient and thus aggressively guarded, but after review it looks correct. I feel it wouldn't be too hard to collapse the various SubscriptionState into the StateMachineSubscription itself and deal with int/enum based states for transitions, but eh as long as it is correct...

Thanks for looking at it! FYI I just fixed a race reported by @OlegDokuka:
d3409df

mp911de · 2020-04-09T13:59:56Z

Right now, ConnectionFactoryOptions does not implement equals/hashCode. I filed r2dbc/r2dbc-spi#166. However, the reuse of options should follow a rather Testcontainers-specific identification mechanism.

Is it now possible to use the same container instance via URL from JDBC and R2DBC? The reuse of containers should be based on config options provided by both technologies.

bsideup · 2020-04-09T14:05:04Z

@mp911de

However, the reuse of options should follow a rather Testcontainers-specific identification mechanism.

In JDBC, we use the JDBC URL string to identify the same container. r2dbc/r2dbc-spi#166 would be handy indeed.

s it now possible to use the same container instance via URL from JDBC and R2DBC?

No, not possible (since we can't "match" them). If one wants to reuse (not talking about the reusable containers feature, btw) the same DB with r2dbc and jdbc, they should create an instance themselves and use the programmatic way.

mp911de · 2020-04-09T14:26:33Z

No, not possible (since we can't "match" them). If one wants to reuse (not talking about the reusable containers feature, btw) the same DB with r2dbc and jdbc

Probably I miss some context. I understood you're already extracting bits from the JDBC URL. If those values match the ones from the R2DBC config, then it should be possible to match those.

they should create an instance themselves and use the programmatic way.

Many utilities (Flyway, Liquibase) do not support R2DBC. When bootstrapping a database, the schema becomes essential. Launching a single container programmatically and wiring it up introduces additional burden on frameworks that aid integration testing and advertise URL-based Testcontainer integration.

bsideup · 2020-04-09T14:28:48Z

@mp911de

I understood you're already extracting bits from the JDBC URL

We don't, actually. We take the original string and use it as a key in the Map:

testcontainers-java/modules/jdbc/src/main/java/org/testcontainers/jdbc/ContainerDatabaseDriver.java

Lines 78 to 81 in a09f4d5

    
                       /* 
        
                         If we already have a running container for this exact connection string, we want to connect 
        
                         to that rather than create a new container 
        
                        */

rnorth · 2020-04-11T09:40:20Z

What shall we do for docs? It feels to me like we should break apart https://www.testcontainers.org/modules/databases/ and have a new page for each of the JDBC and R2DBC general concepts, plus also add updates to the MySQL, MariaDB and PostgreSQL module docs pages, as they're the places people probably want to look...

WDYT?

bsideup · 2020-04-11T09:45:44Z

@rnorth oh yes, I will add the docs (to this PR) once we agree on the API 👍

rnorth · 2020-04-11T09:45:56Z

...riadb/src/main/java/org/testcontainers/containers/MariaDBR2DBCDatabaseContainerProvider.java

+import org.testcontainers.r2dbc.R2DBCDatabaseContainer;
+import org.testcontainers.r2dbc.R2DBCDatabaseContainerProvider;
+
+@AutoService(R2DBCDatabaseContainerProvider.class)


Woah, TIL about @AutoService. I like it.

For the sake of consistency, we should probably propagate this out to everywhere else where we've currently hand-written a service descriptor file. This could be a subsequent PR, though.

kiview

I was mostly wondering about some code reuse for the tests. But we can refactor this later of course.

kiview · 2020-04-11T10:14:32Z

...s/mariadb/src/test/java/org/testcontainers/containers/MariaDBR2DBCDatabaseContainerTest.java

+
+import static org.junit.Assert.*;
+
+public class MariaDBR2DBCDatabaseContainerTest {


Just wondering if we should prefer an approach similar to our jdbc-test module, that runs the same test in a parameterized way?

@rnorth wanted to change it to "per module" for better parallelization, see #2520

Ah yes, I was assuming something like this actually 😬
Still, we could maybe extract an abstract base test class or something similar in a later change, to reduced code duplication.

Yep, we can split it per-module and have some common base test classes to keep things neat. That's how I'd started with my draft splitting of jdbc-test, and works nicely.

modules/r2dbc/src/main/resources/META-INF/services/io.r2dbc.spi.ConnectionFactoryProvider

# Conflicts: # modules/mariadb/build.gradle

docs/modules/databases/r2dbc.md

Co-Authored-By: Richard North <rich.north@gmail.com>

docs/modules/databases/r2dbc.md

Co-Authored-By: Richard North <rich.north@gmail.com>

docs/modules/databases/r2dbc.md

Co-Authored-By: Richard North <rich.north@gmail.com>

rnorth · 2020-04-13T19:47:31Z

This was released in https://github.com/testcontainers/testcontainers-java/releases/tag/1.14.0 🎉

Thanks for the contribution!

This PR adds support for [R2DBC](https://r2dbc.io). ## URL support The URL support is similar to the JDBC's, but: 1. Does not reuse containers by URL - it is unclear yet how to "hash" `ConnectionFactoryOptions` 1. The image tag must be explicitly provided via `TC_IMAGE_TAG` r2dbc option 1. TODO: daemon mode 1. TODO: alias support (see #4) ## Example usage The user **must add the `org.testcontainers:r2dbc` module** - unlike JDBC, we use `compileOnly` dependency on R2DBC, since it is a 3rd party dependency. Once both the `r2dbc` and database's module are added, one can use: ```java String url = "r2dbc:tc:postgresql:///db?TC_IMAGE_TAG=10-alpine"; ConnectionFactory connectionFactory = ConnectionFactories.get(url); ``` ## Programmatic access For those who start containers manually and want to obtain `ConnectionFactoryOptions` for already started containers, we provide a helper static method: ```java try (PostgreSQLContainer<?> container = new PostgreSQLContainer<>()) { container.start(); ConnectionFactory connectionFactory = ConnectionFactories.get( PostgreSQLR2DBCDatabaseContainer.getOptions(container) ); } ```

Add R2DBC URL support

98515fc

bsideup added the type/feature label Apr 9, 2020

bsideup added this to the next milestone Apr 9, 2020

bsideup requested a review from a team April 9, 2020 12:27

OlegDokuka reviewed Apr 9, 2020

View reviewed changes

simonbasle reviewed Apr 9, 2020

View reviewed changes

bsideup added 2 commits April 9, 2020 15:00

fix a race

d3409df

move javadoc to the top class

3e2b34b

bsideup added 4 commits April 9, 2020 16:30

use auto-service, add testUrlSupport test

84ce94d

Add MSSQL R2DBC support

7b4a8c6

Add MySQL

8903ed8

Add MariaDB

a3a5c5a

bsideup marked this pull request as ready for review April 10, 2020 19:56

bsideup requested review from kiview and rnorth as code owners April 10, 2020 19:56

rnorth reviewed Apr 11, 2020

View reviewed changes

kiview approved these changes Apr 11, 2020

View reviewed changes

bsideup and others added 6 commits April 11, 2020 12:39

Use @AutoService on TestcontainersR2DBCConnectionFactoryProvider

886c9e7

Merge branch 'master' into r2dbc_support

184da88

# Conflicts: # modules/mariadb/build.gradle

add AbstractR2DBCDatabaseContainerTest

834dae8

TC_IMAGE -> TC_IMAGE_TAG

937fe96

fix reusesUntilConnectionFactoryIsClosed

574df74

Merge branch 'master' into r2dbc_support

fa6a715

add docs

e145cc1

rnorth reviewed Apr 12, 2020

View reviewed changes

docs/modules/databases/r2dbc.md Outdated Show resolved Hide resolved

Update docs/modules/databases/r2dbc.md

830f483

Co-Authored-By: Richard North <rich.north@gmail.com>

rnorth reviewed Apr 12, 2020

View reviewed changes

docs/modules/databases/r2dbc.md Outdated Show resolved Hide resolved

Update docs/modules/databases/r2dbc.md

3db13dc

Co-Authored-By: Richard North <rich.north@gmail.com>

rnorth reviewed Apr 12, 2020

View reviewed changes

docs/modules/databases/r2dbc.md Outdated Show resolved Hide resolved

Update docs/modules/databases/r2dbc.md

38e81dc

Co-Authored-By: Richard North <rich.north@gmail.com>

rnorth approved these changes Apr 12, 2020

View reviewed changes

Merge branch 'master' into r2dbc_support

b28fb65

bsideup merged commit 6c45a20 into master Apr 12, 2020

delete-merged-branch bot deleted the r2dbc_support branch April 12, 2020 17:27

bsideup mentioned this pull request May 27, 2020

Add Promise style semantics to reactive streams reactive-streams/reactive-streams-jvm#268

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add R2DBC support #2545

Add R2DBC support #2545

bsideup commented Apr 9, 2020 •

edited

OlegDokuka Apr 9, 2020

simonbasle Apr 9, 2020

OlegDokuka Apr 9, 2020

simonbasle Apr 9, 2020

bsideup Apr 9, 2020

simonbasle Apr 9, 2020 •

edited

mp911de Apr 9, 2020

simonbasle Apr 9, 2020

bsideup Apr 9, 2020

mp911de commented Apr 9, 2020

bsideup commented Apr 9, 2020

mp911de commented Apr 9, 2020

bsideup commented Apr 9, 2020 •

edited

rnorth commented Apr 11, 2020

bsideup commented Apr 11, 2020

rnorth Apr 11, 2020

kiview left a comment

kiview Apr 11, 2020

bsideup Apr 11, 2020

kiview Apr 11, 2020

rnorth Apr 11, 2020

rnorth commented Apr 13, 2020


		import static org.junit.Assert.*;

		public class MariaDBR2DBCDatabaseContainerTest {

Add R2DBC support #2545

Add R2DBC support #2545

Conversation

bsideup commented Apr 9, 2020 • edited

URL support

Example usage

Programmatic access

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbasle Apr 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mp911de commented Apr 9, 2020

bsideup commented Apr 9, 2020

mp911de commented Apr 9, 2020

bsideup commented Apr 9, 2020 • edited

rnorth commented Apr 11, 2020

bsideup commented Apr 11, 2020

Choose a reason for hiding this comment

kiview left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnorth commented Apr 13, 2020

bsideup commented Apr 9, 2020 •

edited

simonbasle Apr 9, 2020 •

edited

bsideup commented Apr 9, 2020 •

edited