Skip to content

Conversation

@schauder
Copy link
Contributor

An Oracle database is started in Docker using Testcontainers.
Tests that don't work yet are skipped using the TestDatabaseFeatures which allows central control, which database supports which feature.

A suitable oracle image is deployed on DockerHub as "springci/spring-data-oracle-xe-prebuild:18.4.0".
The official Oracle docker images as described here https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md are not suitable since it needs about 15min to start and also TestContainers seems to break it by trying to us it to early.
The referenced image was created based on these instructions: https://github.com/oracle/docker-images/tree/master/OracleDatabase/SingleInstance/samples/prebuiltdb

The following features don't yet work for Oracle:

  • Loading of date like properties.
  • Ids with custom names.
  • Array support.
  • Boolean properties.
  • BigDecimals and BigInteger is limited to what fits into Oracles NUMBER data type.
  • Entities that use a join during load time, i.e. with a one to one relationship.

See DATAJDBC-256.

Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few comments. That ResultSet.getObject(…) returns driver-internal types is unfortunate. We could make use of JdbcUtils.getResultSetValue(…) and the actual property type to delegate result value conversion to the driver is we can make sure the requested type is a simple JDBC type (e.g. by verifying against JdbcSimpleTypes.HOLDER.isSimpleType(…)) otherwise, fall back to ResultSet.getObject(…). WDYT?

@Test // DATAJDBC-112
public void saveAndLoadAnEntityWithReferencedEntityById() {

features.supportsQuotedIds();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please switch to an annotation-based model (e.g. @EnabledOnFeature(Features.QUOTED_IDS))? All these calls to features obfuscate the test code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I have the very opposite impression. The direct call to features is shorter both in the underlying implementation and on the use case site and easier to understand.

I'll prepare a commit switching to a rule based approach.

</systemPropertyVariables>
</configuration>
</execution>
<execution>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Container startup fails with a TNS error message quite regularly. That causes a lot of the CI builds to fail.

I'm wondering whether we should run DB2 and Oracle tests only in a nightly job.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do you see these regular failures? In Jenkins I see one failure in the last 14 builds which would gives me hope that just waiting a little longer might fix the problem, especially since it is a TNS failure and not a "your account has been locked".

@schauder
Copy link
Contributor Author

schauder commented Jul 8, 2020

We could make use of JdbcUtils.getResultSetValue(…) and the actual property type to delegate result value conversion to the driver

Sounds very promising. I didn't know that method! But I'd leave that to DATAJDBC-569 and limit this PR to the just getting the very basics working as agreed before if that is ok.

schauder and others added 5 commits July 10, 2020 13:47
An Oracle database is started in Docker using Testcontainers. Tests that don't work yet are skipped using the TestDatabaseFeatures which allows central control, which database supports which feature. A suitable oracle image is deployed on DockerHub as "springci/spring-data-oracle-xe-prebuild:18.4.0". The official Oracle docker images as described here https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md are not suitable since it needs about 15min to start and also TestContainers seems to break it by trying to us it to early. The referenced image was created based on these instructions: https://github.com/oracle/docker-images/tree/master/OracleDatabase/SingleInstance/samples/prebuiltdb The following features don't yet work for Oracle: * Loading of date like properties. * Ids with custom names. * Array support. * Boolean properties. * BigDecimals and BigInteger is limited to what fits into Oracles NUMBER data type. * Entities that use a join during load time, i.e. with a one to one relationship.
Rename IdGeneration method and added Javadoc. Fixes copyright statement in OracleDataSourceConfiguration. Fixes JavaDoc comment for OracleDialect. Increases the maximum wait time for the Oracle database to start.
@schauder schauder force-pushed the issue/DATAJDBC-256 branch from 35b4dd6 to 8089b2c Compare July 10, 2020 12:26
@schauder
Copy link
Contributor Author

@mp911de
I addressed the various comments.

I left the migration to annotation based assumptions in a separate commit.
It actually doesn't look as bad as I expected :-)

I also rebased on the current master and removed the alias for join with as since that is already resolved.

For the CI build stability I increased the maximum wait time to 5minutes for the Oracle db to start up.
Let's see if that helps or if we have to come up with something else.

mp911de pushed a commit that referenced this pull request Jul 13, 2020
mp911de pushed a commit that referenced this pull request Jul 13, 2020
An Oracle database is started in Docker using Testcontainers. Tests that don't work yet are skipped using the TestDatabaseFeatures which allows central control, which database supports which feature. A suitable oracle image is deployed on DockerHub as "springci/spring-data-oracle-xe-prebuild:18.4.0". The official Oracle docker images as described here https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md are not suitable since it needs about 15min to start and also TestContainers seems to break it by trying to us it to early. The referenced image was created based on these instructions: https://github.com/oracle/docker-images/tree/master/OracleDatabase/SingleInstance/samples/prebuiltdb The following features don't yet work for Oracle: * Loading of date like properties. * Ids with custom names. * Array support. * Boolean properties. * BigDecimals and BigInteger is limited to what fits into Oracles NUMBER data type. * Entities that use a join during load time, i.e. with a one to one relationship. Original pull request: #232.
mp911de added a commit that referenced this pull request Jul 13, 2020
Rename RequiredFeature to EnabledOnFeature to align with JUnit terminology. Use RunWith instead of Spring rules. Reformat code. Original pull request: #232.
@mp911de
Copy link
Member

mp911de commented Jul 13, 2020

That's merged and polished now.

@mp911de mp911de closed this Jul 13, 2020
@mp911de mp911de deleted the issue/DATAJDBC-256 branch July 13, 2020 12:13
mp911de added a commit that referenced this pull request Jul 13, 2020
Rename RequiredFeature to EnabledOnFeature to align with JUnit terminology. Use RunWith instead of Spring rules. Reformat code. Update documentation. Original pull request: #232.
mp911de added a commit that referenced this pull request Feb 21, 2022
…cessException if row does not exist. We now emit a TransientDataAccessException if an object with a provided Id yields no affected rows. Such an arrangement is typically an indicator for a bug where calling code expects the object to be inserted with a provided Id.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants