Improving Oracle-XE support

[justinwyer]

Current challenges:

• The module only connects using SID and not service name, connecting to PDBs in Oracle 12+ typically requires to use a service name.
• Username and password cannot be specified.

Changes:

• Detect SID : or service name / in the supplied TC JDBC URL and honour this in the underlying JDBC URL.
• Parse username and password from the supplied TC JDBC URL and pass to the underlying JDBC URL.
• Switched affected regex patterns to use named groups
• Enable tests
• Add tests for both SID and service name connections

Additional notes:

Currently Oracle only provides docker scripts to build your own version of 18.4.0 which has a very slow start time (~7 minutes on a decent laptop) in order to make this useful for test cycles it is typical to start this container and then run a docker commit on it. This reduces start time to around 10 seconds. The tests now make use of such a container, we should probably retag this container and put it somewhere else other than my docker hub account.

[justinwyer]

If you're happy with the PR in principal then I will be happy to update the JDBC documentation to include information on using Oracle XE.

[kiview]

Hi @justinwyer,

thanks for raising the PR, I think we wrote yesterday in the YT stream? But you probably misunderstood me, what I meant is, that we already have an open Oracle XE PR, that we would like to merge.

Could you maybe review the already open PR at #4298 and check if it missing anything, that your PR considers?
We especially wanted to use the gvenzl/oracle-xe image, since it is semi-official.

[justinwyer]

I think honouring the SID :and service name connectors / is a good feature as well as username and password in the URL. I am happy to rebase these once #4298 is merged.

[kiview]

Greats, let's proceed like this 👍

[kiview]

@justinwyer We have just merged #4298, so feel free to expand on this, if you think something is lacking 🙂

[justinwyer]

I've rebased, just have a question on the reasons for disallowing system / sys access?

Ah there is really no need with this image, it will create a user in the PDB. I will remove the SID connection.

[justinwyer]

@kiview I've updated the PR as follows:
Improving Oracle-XE support

Current challenges:

• Username and password cannot be specified using the JDBC url, this is imporant as the schema name in Oracle is the username
• The module only connects using service name and not SID
• It does not allow connecting using system or sys users

Changes:

• Detect SID : or service name / in the supplied TC jdbc URL and honor this in the underlying JDBC URL.
• Parse username and password from the supplied TC JDBC URL and pass to the underlying JDBC URL.
• Switched affected regex patterns to use named groups
• Add tests for both SID and service name connections
• Allow system and sys, and do not set APP_USER / APP_PASSWORD environment variables when it is the provided username in the JDBC url

[justinwyer]

@kiview quick thought, should we allow the full image name and tag as part of the JDBC url rather than just the image tag, given the nature of the Oracle image, there would no longer be a need for the testcontainers.properties at all.

[rnorth]

Re @justinwyer

quick thought, should we allow the full image name and tag as part of the JDBC url rather than just the image tag, given the nature of the Oracle image, there would no longer be a need for the testcontainers.properties at all.

This is something that we have discussed internally in the past but deferred. Since a docker image name can be a full host+port+path style string, fitting this into the JDBC URL could become messy unless we do it right.

I'm not saying we'll never do this, just that we've considered it and found it unpleasant enough not to want to rush into doing it. I'd suggest not worrying about it for this PR 😉

[kiview]

Hey @justinwyer, I am sorry for letting you and the PR waiting.
With regards to #4402, can we limit the changes in this PR to those in ConnectionUrl and OracleContainerProvider?

The SID and serviceName topic is a bit more complex I think and is better tackled in a future seperate PR and after this PR and #4402 are merged.

[rnorth]

I've taken the liberty of preparing a branch with the URL parsing extracted. I've split out the main change that deals with JDBC URL parsing, and added a bit of further tidying and a minor bugfix. I hope this is OK with you @justinwyer - I'm trying to ensure that you retain credit on the commits/PR.

[justinwyer]

Thanks guys, I will close this :)

[kiview]

Thanks a lot for your work and bringing this on track @justinwyer!