Skip to content

Commit

Permalink
Fix composeSetTransaction and update OracleTestKit
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael-A-McMahon committed Apr 14, 2021
1 parent 284e958 commit db1ab50
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 73 deletions.
2 changes: 1 addition & 1 deletion src/main/java/oracle/r2dbc/impl/OracleConnectionImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ private String composeSetTransaction(TransactionDefinition definition) {
" getTransactionIsoaltionLevel returns: " + jdbcIsolationLevel);
}

setTransactionBuilder.append(" ISOLATION LEVEL = READ COMMITTED");
setTransactionBuilder.append(" ISOLATION LEVEL READ COMMITTED");
}
else {
// TODO: Only supporting READ COMMITTED
Expand Down
169 changes: 97 additions & 72 deletions src/test/java/oracle/r2dbc/OracleTestKit.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
* Subclass implementation of the R2DBC {@link TestKit} for Oracle Database.
* This test kit implementation overrides super class test methods that are
* fundamentally incompatible with Oracle Database. The javadoc of each
* overriden test method describes why it must be overriden when interacting
* overridden test method describes why it must be overriden when interacting
* with an Oracle Database.
* </p><p>
* The developers of the Oracle R2DBC Driver are mindful of the fact that
Expand Down Expand Up @@ -123,6 +123,76 @@ public ConnectionFactory getConnectionFactory() {
return connectionFactory;
}

/**
* {@inheritDoc}
* <p>
* Overrides the standard {@link TestKit} implementation for compatibility
* with Oracle Database.
* </p><p>
* Oracle Database converts unquoted alias values to uppercase. This
* conflicts with the {@link #rowMetadata()} test where unquoted aliases are
* expected to retain their original lower case values. To resolve this
* conflict, the SQL statement returned for
* {@link TestStatement#SELECT_VALUE_ALIASED_COLUMNS} enquotes the alias
* names that appear in in the {@code SELECT} statement.
* </p>
* @param statement
* @return
*/
@Override
public String doGetSql(TestStatement statement) {
switch (statement) {
case SELECT_VALUE_ALIASED_COLUMNS:
// Enquote alias names to retain their lower case values
return "SELECT col1 AS \"b\", col1 AS \"c\", col1 AS \"a\"" +
" FROM test_two_column";
default:
return statement.getSql();
}
}

/**
* {@inheritDoc}
* <p>
* Overrides the standard {@link TestKit} implementation for compatibility
* with Oracle Database.
* </p><p>
* Oracle Database does not support the {@code INTEGER} datatype declared
* with {@link TestStatement#CREATE_TABLE},
* {@link TestStatement#CREATE_TABLE_AUTOGENERATED_KEY}, and
* {@link TestStatement#CREATE_TABLE_TWO_COLUMNS}. When the {@code INTEGER}
* type appears in these {@code CREATE TABLE} statements, Oracle Database
* creates columns of type {@code NUMBER(38)}, as specified in Oracle's
* <a href="https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/Data-Types.html#GUID-0BC16006-32F1-42B1-B45E-F27A494963FF">
* SQL Language Reference
* </a>. Oracle R2DBC converts NUMBER values into {@link BigDecimal}
* objects, and this conflicts with the {@code TestKit} verifications that
* expect the standard mapping of {@code INTEGER} to {@link Integer}. The
* {@code extractColumn} method is overridden to resolve this conflict by
* converting {@code BigDecimal} objects to {@code int}.
* </p>
*/
@Override
public Object extractColumn(Row row) {
return extractColumn("value", row);
}

/**
* Extracts the value of a column identified by {@code name} from a
* {@code row}. The extracted value is converted as specified by
* {@link OracleTestKit#extractColumn(Row)}.
* @param name Column name
* @return Column value
*/
private Object extractColumn(String name, Row row) {
Object value = row.get("value");

if (value instanceof BigDecimal)
return ((BigDecimal)value).intValue();
else
return value;
}

@Override
public String getCreateTableWithAutogeneratedKey() {
return "CREATE TABLE test (" +
Expand All @@ -144,42 +214,40 @@ public Integer getIdentifier(int index) {
return index;
}


/**
* {@inheritDoc}
* <p>
* Overrides the default implementation to expect BigDecimal as the default
* Java type mapping for INTEGER columns. The default implementation of this
* test in {@link TestKit#changeAutoCommitCommitsTransaction()}expects the
* R2DBC Specification's default type mapping guideline for INTEGER
* columns, which is java.lang.Integer.
* Overrides the default implementation to use
* {@link #extractColumn(String, Row)} rather than {@link Row#get(String)}.
* This override is necessary because {@link Row#get(String)} returns
* an instance of {@link BigDecimal} for columns declared as {@code INTEGER}
* by {@link TestStatement#INSERT_TWO_COLUMNS}. See
* {@link OracleTestKit#extractColumn(Row)} for details.
* </p><p>
* This override is necessary because the Oracle Database describes INTEGER
* type columns as NUMBER types with a precision of 38. This description
* does not provide enough information for the Oracle R2DBC Driver to
* distinguish between the NUMBER and INTEGER type, so it uses the default
* type mapping for NUMBER, which is BigDecimal.
* </p><p>
* This override does not prevent this test from verifying that
* setAutoCommit(boolean) will commit an active transaction.
* </p>
* This override does not prevent this test from verifying a case-insensitive
* name match is implemented by Row.get(String) when duplicate column names
* are present.
*/
@Test
@Override
public void changeAutoCommitCommitsTransaction() {
public void duplicateColumnNames() {
getJdbcOperations().execute(expand(TestStatement.INSERT_TWO_COLUMNS));

Mono.from(getConnectionFactory().create())
.flatMapMany(connection ->
Flux.from(connection.setAutoCommit(false))
.thenMany(connection.beginTransaction())
.thenMany(connection.createStatement("INSERT INTO test VALUES(200)").execute())
.flatMap(Result::getRowsUpdated)
.thenMany(connection.setAutoCommit(true))
.thenMany(connection.createStatement("SELECT value FROM test").execute())
.flatMap(it -> it.map((row, metadata) -> row.get("value")))
.concatWith(close(connection))
)
.flatMapMany(connection -> Flux.from(connection

.createStatement(expand(TestStatement.SELECT_VALUE_TWO_COLUMNS))
.execute())

.flatMap(result -> result
.map((row, rowMetadata) -> Arrays.asList(
extractColumn("value", row), extractColumn("VALUE", row))))
.flatMapIterable(Function.identity())

.concatWith(close(connection)))
.as(StepVerifier::create)
// Note the overridden behavior below: Expect the BigDecimal type
.expectNext(BigDecimal.valueOf(200)).as("autoCommit(true) committed the transaction. Expecting a value to be present")
.expectNext(100).as("value from col1")
.expectNext(100).as("value from col1 (upper case)")
.verifyComplete();
}

Expand Down Expand Up @@ -233,49 +301,6 @@ public void columnMetadata() {
.verifyComplete();
}

/**
* {@inheritDoc}
* <p>
* Overrides the default implementation to expect BigDecimal as the default
* Java type mapping for INTEGER columns. The default implementation of this
* test in {@link TestKit#duplicateColumnNames()} expects the R2DBC
* Specification's default type mapping guideline for INTEGER columns,
* which is java.lang.Integer.
* </p><p>
* This override is necessary because the Oracle Database describes INTEGER
* type columns as NUMBER types with a precision of 38. This description
* does not provide enough information for the Oracle R2DBC Driver to
* distinguish between the NUMBER and INTEGER type, so it uses the default
* type mapping for NUMBER, which is BigDecimal.
* </p><p>
* This override does not prevent this test from verifying a case-insensitive
* name match is implemented by Row.get(String) when duplicate column names
* are present.
* </p>
*/
@Test
@Override
public void duplicateColumnNames() {
getJdbcOperations().execute("INSERT INTO test_two_column VALUES (100, 'hello')");

Mono.from(getConnectionFactory().create())
.flatMapMany(connection -> Flux.from(connection

.createStatement("SELECT col1 AS value, col2 AS value FROM test_two_column")
.execute())

.flatMap(result -> result
.map((row, rowMetadata) -> Arrays.asList(row.get("value"), row.get("VALUE"))))
.flatMapIterable(Function.identity())

.concatWith(close(connection)))
.as(StepVerifier::create)
// Note the overridden behavior below: Expect the BigDecimal type
.expectNext(BigDecimal.valueOf(100L)).as("value from col1")
.expectNext(BigDecimal.valueOf(100L)).as("value from col1 (upper case)")
.verifyComplete();
}

/**
* {@inheritDoc}
* <p>
Expand Down
7 changes: 7 additions & 0 deletions src/test/java/oracle/r2dbc/util/SharedConnectionFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import io.r2dbc.spi.IsolationLevel;
import io.r2dbc.spi.R2dbcException;
import io.r2dbc.spi.Statement;
import io.r2dbc.spi.TransactionDefinition;
import io.r2dbc.spi.ValidationDepth;
import org.reactivestreams.Publisher;
import reactor.core.publisher.Flux;
Expand Down Expand Up @@ -279,6 +280,12 @@ public Publisher<Void> beginTransaction() {
return connection.beginTransaction();
}

@Override
public Publisher<Void> beginTransaction(TransactionDefinition definition) {
requireNotClosed();
return connection.beginTransaction(definition);
}

@Override
public Publisher<Void> commitTransaction() {
requireNotClosed();
Expand Down

0 comments on commit db1ab50

Please sign in to comment.