Modernize Oracle Container Logic

[KyleAure]

Now that we are using a standard Oracle image.
The Oracle Container logic needs to change to support the oracle database config options on gvenzl/oracle-xe

The following options are now configurable and no longer static:

• system / sys password
• Application username and password
• Pluggable database name

The following options are still static but need to be accounted for:

• SID is static and will always be XE

Additional method paths:

• Connect using SID, or service name (pluggable database name)

Finally:

• Add a waitStrategy that aligns with other JDBC containers.

Expanding on the work done under #4382

[sualeh]

Please merge this change - this is the only way to use Oracle 11 with Testcontainers, using "gvenzl/oracle-xe".

[KyleAure]

@rnorth let me know if you think this would be a good first PR towards getting the Oracle container on par with the others.

[sualeh]

@KyleAure - in a later PR, we could add "withSID" and "withServiceName". For now, your code works - I tried it out with Testcontainers 1.16.0.

[KyleAure]

@sualeh I am not sure we need withSID and withServiceName methods. Since this container architecture assumes you are using an Oracle XE image then the SID is static and will always be XE. I think it is reasonable that if a user of testcontainers is using a non-standard or non-XE oracle image then they will either extend this class, or create their own container class.

As for withServiceName the same logic applies. The Oracle XE image uses a standard service name, also XE.

Oracle has engineered these images to really only allow customization of pluggable databases. Therefore, withDatabaseName and getDatabaseName seem to be the standard.

This does contradict some of the suggestions I had on this PR: #4382
But I think aligning this container with what is actually configurable out of the box for this image makes the most sense.

[kiview]

Hey @KyleAure,
thanks for reiterating on this PR. With regards to the changes potentially coming through #4382, I would see the scope of this PR as brining all the capabilities of the gvenzl image to the class.

This means:

• Support for using another pluggable database
• Leveraging the faster LogMessageWaitStrategy for known log message

In addition, it would be great if we have tests for all main permutations of the container:

• no databaseName set, user/pw default
• databaseName set, user/pw default
• databaseName set, user/pw set
• no databaseName set, user/pw set

While you are right that the image does not support customizing serviceName or SID, custom user images might. So we can think about adding setters in a future PR (although we have no easy way to test them).

Since we currently assume strict compatibility with the gvenzl image, please add the following assertion, similarly to how it is done for e.g. the PostgreSQLContainer:

dockerImageName.assertCompatibleWith(DEFAULT_IMAGE_NAME);

@sualeh Which aspect of this PR do you require for making the module working with Oracle 11 and the gvenzl image?

[sualeh]

@kiview - these lines take care of Oracle 11 and above.

[KyleAure]

@kiview I have updated my PR based on the feedback you provided. Thank you.

[kiview]

@KyleAure Thanks, I will have another look.

@sualeh I don't understand this. Even without this change, the current test uses gvenzl/oracle-xe:18.4.0-slim and it works. But you are right that gvenzl/oracle-xe:11 does not work. So while versions above 11 (version 18 to be precise) are supported, version 11 is currently not. Is that what you meant?

[kiview]

I think the PR looks great, thanks a lot for the quick rework!
Just having small minor comments and questions, after that, it's good to go from my side.

@rnorth can you have an additional look?

[KyleAure]

@rnorth @kiview Updated the PR with your suggested changes. Thanks for the feedback 👍🏼

[sualeh]

@sualeh I don't understand this. Even without this change, the current test uses gvenzl/oracle-xe:18.4.0-slim and it works. But you are right that gvenzl/oracle-xe:11 does not work. So while versions above 11 (version 18 to be precise) are supported, version 11 is currently not. Is that what you meant?

@kiview - if you use the Oracle Testcontainer with a different tag (11) with SID, it works.

dbContainer =
      new OracleContainer(DockerImageName.parse("gvenzl/oracle-xe").withTag("11")).usingSid();

Basically, this PR will support both Oracle 18 and Oracle 11. That is why I am looking forward to getting it merged in.

[KyleAure]

@sualeh Is correct that the 11 docker image only supports connections using SID. Perhaps we could include startup logic to ensures that if the docker image is for 11 that SID must be enabled.

[sualeh]

@sualeh Is correct that the 11 docker image only supports connections using SID. Perhaps we could include startup logic to ensures that if the docker image is for 11 that SID must be enabled.

@KyleAure - the philosophy of Testcontainers seems to be (as far as I can tell) to create JDBC containers in such as way that the image can switched out. For this reason, I would not build in startup logic to check if the Docker image is "11".

On the other hand, @rnorth @kiview - what do you think about creating a sort of "base" JDBC container for each type of database system, where the image can be switched out, and then a concrete JDBC container as well, where the image cannot be switched out? The concrete JDBC container can be used with all the default settings by a casual Testcontainers user, lowering the barrier to entry. Taking Oracle as an example, there would be 3 JDBC containers - a "base" one looking like what we have now, an Oracle 11 one with gvenzl/oracle-xe and SID baked in, and an Oracle 18 one with gvenzl/oracle-xe baked in too, with a service name. Thoughts?

[kiview]

Testcontainers container implementation generally have a lot of assumptions about the underlying image and sometimes integrate against some slightly weak indicators or aspects, that might change in future versions or in custom images (the log message that is used now for waiting is one example for this). Therefore, we added the dockerImageName.assertCompatibleWith(DEFAULT_IMAGE_NAME); pattern, to be explicit to users if other images are used.

Let's try to not do too much auto-config for aspects users can easily specify if they know about the aspects of the underlying image (e.g. using SID in this case). This only created more weak integration points that might break and need to be maintained.

PR LGTM, thanks a @KyleAure and also thanks @sualeh for your feedback and explanations.