Enhance SQL Client SPI

[jkuhn1]

Enhance SQL Client Driver SPI

Hi,

I think the current way SQL Client Drivers are exposed through Service Provider Interfaces presents some problems and should be improved.

I see actually several issues with current Driver interface which prevents the SPI to fullfil its purpose:

1. There is no way to set product specific connection options since these must be set on a SqlConnectOptions implementation specific to the driver which is usually not accessible when using the SPI. Basically, if we need to have vertx-pg-client on the classpath, the SPI becomes less interesting and we can directly manipulate Pg* types to create a SQL client.

2. There is no way to pick the right driver corresponding to a static configuration (eg. uri, properties, json...). Using a SPI we expect to be able to pick a driver that would understand a particular abstract configuration. Currently the Driver defines the acceptsOptions(SqlConnectOptions connectOptions) which is of little interest since all implementations returns true when a SqlConnectOptions instance is provided. So for this to be useful we must supply a specific SqlConnectOptions instance, in which case we could also directly create the specific SQL client and ignore the SPI.

The Pool#pool(SqlConnectOptions database, PoolOptions options) method provides a way to pick the right implementation based on the type of SqlConnectOptions but this actually doesn't solve the issue.

3. Driver#createPool(), Driver#createConnectionFactory() and Driver#acceptsOptions() implementations are not consistent. All implementations actually wraps options in createPool() and createConnectionFactory() without checking whether the options is accepted by the driver. We can for instance invoke PgDriver#createPool() with a DB2ConnectOptions instance that will eventually be wrapped in a PgConnectOptions whereas PgDriver#acceptsOptions() returns false since the driver only accept PgConnectOptions and SqlConnectOptions instances.

The purpose of the SPI is to be able to create a SQL client instance from an abstract static configuration, we basically don't know which data store we want to connect to. The client code should never be aware of the actual implementation, the Driver implementation to use should be given by configuration in a URI scheme for example.

Issue #767 describe a similar issue, I propose to solve these issues without dispersing the SPI by exposing methods based on a connection URI in the sole Driver interface in order to abstract SqlConnectOptions implementation as well as Pool and ConnectionFactory.

We could expose the following methods in Driver:

// Determines whether the driver support the connection URI based on the scheme for instance
boolean acceptsUri(String connectionUri);

// Returns a specific SqlConnectOptions instance created from the connection URI
<T extends SqlConnectOptions> T getConnectOptions(String connectionUri);

// Returns a specific SqlConnectOptions instance created from the connection URI and populated with the json
<T extends SqlConnectOptions> T getConnectOptions(String connectionUri, JsonObject json);

A DriveLoader class in the spi package could also be created to implement the ServiceLoader logic and expose drivers. I'm not sure about the Pool#pool(SqlConnectOptions database, PoolOptions options) which imho is part of the SPI but not in an obvious way, I'll prefer to say that the Driver interface and more generally the SPI package are the main entry points rather than dispersing the SPI logic in the SQL client API

This represents some changes in the API and shall surely be discussed. I'm making a corresponding PR right now to make things more concrete, please let me know what you think.

[vietj]

@jkuhn1 sorry for the late delay, I think this is something we can improve based on your proposal

can you provide a PR with these changes and then we can iterate from there ?

[jkuhn1]

@vietj sorry I had not properly linked PR #1029 to the issue, it is correct now. Let me know what you think.

[vietj]

I didn't spot it either :-) , I'll start reviewing next week