Merge pull request #155 from oracle/147-row-prefetch-option

Fetch Size and Proxy Client Name Options

Author: jeandelavarene

sample/src/main/java/oracle/r2dbc/samples/JdbcToR2dbc.java

@@ -26,7 +26,7 @@
 import io.r2dbc.spi.ConnectionFactoryOptions;
 import io.r2dbc.spi.R2dbcException;
 import io.r2dbc.spi.Result;
-import oracle.jdbc.pool.OracleDataSource;
+import oracle.jdbc.datasource.impl.OracleDataSource;
 import org.reactivestreams.Publisher;
 import reactor.core.publisher.Flux;
 
@@ -141,7 +141,8 @@ public final class JdbcToR2dbc {
    */
   static DataSource configureJdbc() throws SQLException {
 
-    OracleDataSource dataSource = new oracle.jdbc.pool.OracleDataSource();
+    OracleDataSource dataSource =
+      new oracle.jdbc.datasource.impl.OracleDataSource();
     dataSource.setDriverType("thin");
     dataSource.setServerName(DatabaseConfig.HOST);
     dataSource.setPortNumber(DatabaseConfig.PORT);

src/main/java/oracle/r2dbc/OracleR2dbcOptions.java

@@ -396,6 +396,18 @@ private OracleR2dbcOptions() {}
    */
   public static final Option<CharSequence> KERBEROS_JAAS_LOGIN_MODULE;
 
+  /**
+   * Configures the Oracle JDBC Connection used by Oracle R2DBC as specified by:
+   * {@link OracleConnection#CONNECTION_PROPERTY_DEFAULT_ROW_PREFETCH}
+   */
+  public static final Option<Integer> DEFAULT_FETCH_SIZE;
+
+  /**
+   * Configures the Oracle JDBC Connection used by Oracle R2DBC as specified by:
+   * {@link OracleConnection#CONNECTION_PROPERTY_PROXY_CLIENT_NAME}
+   */
+  public static final Option<CharSequence> PROXY_CLIENT_NAME;
+
   /** The unmodifiable set of all extended options */
   private static final Set<Option<?>> OPTIONS = Set.of(
     DESCRIPTOR = Option.valueOf("oracle.r2dbc.descriptor"),
@@ -509,7 +521,11 @@ private OracleR2dbcOptions() {}
     KERBEROS_REALM = Option.valueOf(
       OracleConnection.CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_KRB_REALM),
     KERBEROS_JAAS_LOGIN_MODULE = Option.valueOf(
-      OracleConnection.CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_KRB_JAAS_LOGIN_MODULE)
+      OracleConnection.CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_KRB_JAAS_LOGIN_MODULE),
+    DEFAULT_FETCH_SIZE = Option.valueOf(
+      OracleConnection.CONNECTION_PROPERTY_DEFAULT_ROW_PREFETCH),
+    PROXY_CLIENT_NAME = Option.valueOf(
+      OracleConnection.CONNECTION_PROPERTY_PROXY_CLIENT_NAME)
   );
 
   /**

src/main/java/oracle/r2dbc/impl/OracleReactiveJdbcAdapter.java

@@ -341,7 +341,7 @@ public AsyncLock getLock() {
   public DataSource createDataSource(ConnectionFactoryOptions options) {
 
     OracleDataSource oracleDataSource =
-      fromJdbc(oracle.jdbc.pool.OracleDataSource::new);
+      fromJdbc(oracle.jdbc.datasource.impl.OracleDataSource::new);
 
     runJdbc(() -> oracleDataSource.setURL(composeJdbcUrl(options)));
     configureStandardOptions(oracleDataSource, options);
@@ -632,15 +632,14 @@ private static void configureJdbcDefaults(OracleDataSource oracleDataSource) {
     // its effects. One effect is to have ResultSetMetaData describe
     // FLOAT columns as the FLOAT type, rather than the NUMBER type. This
     // effect allows the Oracle R2DBC Driver obtain correct metadata for
-    // FLOAT type columns. The property is deprecated, but the deprecation note
-    // explains that setting this to "false" is deprecated, and that it
-    // should be set to true; If not set, the 21c driver uses a default value
-    // of false.
-    @SuppressWarnings("deprecation")
-    String enableJdbcSpecCompliance =
-      OracleConnection.CONNECTION_PROPERTY_J2EE13_COMPLIANT;
+    // FLOAT type columns.
+    // The OracleConnection.CONNECTION_PROPERTY_J2EE13_COMPLIANT field is
+    // deprecated, so the String literal value of this field is used instead,
+    // just in case the field were to be removed in a future release of Oracle
+    // JDBC.
     runJdbc(() ->
-      oracleDataSource.setConnectionProperty(enableJdbcSpecCompliance, "true"));
+      oracleDataSource.setConnectionProperty(
+        "oracle.jdbc.J2EE13Compliant", "true"));
 
     // Cache PreparedStatements by default. The default value of the
     // OPEN_CURSORS parameter in the 21c and 19c databases is 50:

src/main/java/oracle/r2dbc/impl/OracleStatementImpl.java

@@ -597,8 +597,7 @@ private Publisher<JdbcStatement> createJdbcStatement() {
     return adapter.getLock().get(() -> {
       PreparedStatement preparedStatement =
         jdbcConnection.prepareStatement(sql);
-      preparedStatement.setFetchSize(currentFetchSize);
-      preparedStatement.setQueryTimeout(timeout);
+      configureJdbcStatement(preparedStatement, currentFetchSize, timeout);
       return new JdbcStatement(preparedStatement, currentBinds);
     });
   }
@@ -629,8 +628,7 @@ private Publisher<JdbcStatement> createJdbcBatch() {
     return adapter.getLock().get(() -> {
       PreparedStatement preparedStatement =
         jdbcConnection.prepareStatement(sql);
-      preparedStatement.setFetchSize(currentFetchSize);
-      preparedStatement.setQueryTimeout(timeout);
+      configureJdbcStatement(preparedStatement, currentFetchSize, timeout);
       return finalInvalidBinds == null
         ? new JdbcBatch(preparedStatement, currentBatch)
         : new JdbcBatchInvalidBinds(
@@ -649,8 +647,7 @@ private Publisher<JdbcStatement> createJdbcCall() {
 
     return adapter.getLock().get(() -> {
       CallableStatement callableStatement = jdbcConnection.prepareCall(sql);
-      callableStatement.setFetchSize(currentFetchSize);
-      callableStatement.setQueryTimeout(timeout);
+      configureJdbcStatement(callableStatement, currentFetchSize, timeout);
       return new JdbcCall(callableStatement, currentBinds, parameterNames);
     });
   }
@@ -671,12 +668,43 @@ private Publisher<JdbcStatement> createJdbcReturningGenerated() {
         currentGeneratedColumns.length == 0
           ? jdbcConnection.prepareStatement(sql, RETURN_GENERATED_KEYS)
           : jdbcConnection.prepareStatement(sql, currentGeneratedColumns);
-      preparedStatement.setFetchSize(currentFetchSize);
-      preparedStatement.setQueryTimeout(timeout);
+      configureJdbcStatement(preparedStatement, currentFetchSize, timeout);
       return new JdbcReturningGenerated(preparedStatement, currentBinds);
     });
   }
 
+  /**
+   * Configures a JDBC Statement with values that have been configured on an
+   * R2DBC Statement.
+   *
+   * @param statement The statement to configure. Not null.
+   *
+   * @param fetchSize Configuration of {@link #fetchSize(int)}, possibly 0 if
+   * a default size should be used.
+   *
+   * @param queryTimeout Configuration of {@link #timeout}, possibly 0 if no
+   * timeout should be used.
+   *
+   * @throws SQLException If the JDBC statement is closed.
+   */
+  private static void configureJdbcStatement(
+    java.sql.Statement statement, int fetchSize, int queryTimeout)
+    throws SQLException {
+
+    // It is noted that Oracle JDBC's feature of auto-tuning fetch sizes will
+    // be disabled if 0 is passed to setFetchSize. Perhaps similar behavior
+    // occurs with methods like setQueryTimeout as well? To be sure, don't call
+    // any methods unless non-default values are set.
+
+    if (fetchSize != 0) {
+      statement.setFetchSize(fetchSize);
+    }
+
+    if (queryTimeout != 0) {
+      statement.setQueryTimeout(queryTimeout);
+    }
+  }
+
   /**
    * Binds a {@code value} to all named parameters matching the specified
    * {@code name}. The match is case-sensitive.

src/test/java/oracle/r2dbc/impl/OracleReactiveJdbcAdapterTest.java

@@ -34,11 +34,11 @@
 import oracle.r2dbc.OracleR2dbcOptions;
 import oracle.r2dbc.test.DatabaseConfig;
 import oracle.r2dbc.util.TestContextFactory;
-import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Test;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
+import javax.sql.DataSource;
 import java.io.IOException;
 import java.nio.channels.ServerSocketChannel;
 import java.nio.channels.SocketChannel;
@@ -49,6 +49,7 @@
 import java.time.Duration;
 import java.time.ZonedDateTime;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Properties;
@@ -63,6 +64,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import static io.r2dbc.spi.ConnectionFactoryOptions.CONNECT_TIMEOUT;
@@ -78,7 +80,6 @@
 import static oracle.r2dbc.test.DatabaseConfig.connectTimeout;
 import static oracle.r2dbc.test.DatabaseConfig.connectionFactoryOptions;
 import static oracle.r2dbc.test.DatabaseConfig.host;
-import static oracle.r2dbc.test.DatabaseConfig.jdbcVersion;
 import static oracle.r2dbc.test.DatabaseConfig.password;
 import static oracle.r2dbc.test.DatabaseConfig.port;
 import static oracle.r2dbc.test.DatabaseConfig.protocol;
@@ -98,6 +99,7 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 /**
  * Verifies that
@@ -118,23 +120,7 @@ public void testCreateDataSource() throws SQLException {
     // properties. The defaultProperties variable contains properties that
     // are set to default values by OracleReactiveJdbcAdapter and the Oracle
     // JDBC Driver
-    Properties defaultProperties = new Properties();
-    defaultProperties.setProperty(
-      OracleConnection.CONNECTION_PROPERTY_J2EE13_COMPLIANT, "true");
-    defaultProperties.setProperty(
-      OracleConnection.CONNECTION_PROPERTY_IMPLICIT_STATEMENT_CACHE_SIZE, "25");
-    defaultProperties.setProperty(
-      OracleConnection.CONNECTION_PROPERTY_DEFAULT_LOB_PREFETCH_SIZE,
-      "1048576");
-    defaultProperties.setProperty(
-      OracleConnection.CONNECTION_PROPERTY_THIN_NET_USE_ZERO_COPY_IO,
-      "false");
-
-    if (jdbcVersion() == 21) {
-      // Oracle JDBC no longer sets this AC property by default in 23.3
-      defaultProperties.setProperty(
-        OracleConnection.CONNECTION_PROPERTY_ENABLE_AC_SUPPORT, "false");
-    }
+    Properties defaultProperties = getJdbcDefaultProperties();
 
     // Expect only default connection properties when no extended
     // options are supplied
@@ -672,7 +658,7 @@ public void testTimezoneAsRegion() {
    */
   @Test
   public void testEmptyProtocol() {
-    Assumptions.assumeTrue(
+    assumeTrue(
       DatabaseConfig.protocol() == null,
       "Test requires no PROTOCOL in config.properties");
 
@@ -704,6 +690,100 @@ public void testEmptyProtocol() {
     }
   }
 
+  @Test
+  public void testJdbcPropertyOptions() throws SQLException {
+
+    // Create a map where every Option of OracleR2dbcOptions is assigned to a
+    // value. The values are not necessarily valid, or even of the right class
+    // (every option is cast to Option<String>). That's OK because this test
+    // just wants to make sure the values are transferred to OracleDataSource,
+    // and it won't actually attempt to create a connection with these values.
+    Map<Option<String>, String> optionValues =
+      OracleR2dbcOptions.options()
+        .stream()
+        .map(option -> {
+          @SuppressWarnings("unchecked")
+          Option<String> stringOption = (Option<String>)option;
+          return stringOption;
+        })
+        .collect(Collectors.toMap(
+          Function.identity(),
+          option -> "VALUE OF " + option.name()
+        ));
+
+    ConnectionFactoryOptions.Builder optionsBuilder =
+      ConnectionFactoryOptions.builder();
+    optionValues.forEach(optionsBuilder::option);
+
+    DataSource dataSource =
+      OracleReactiveJdbcAdapter.getInstance()
+        .createDataSource(optionsBuilder.build());
+    assumeTrue(dataSource.isWrapperFor(OracleDataSource.class));
+
+    Properties actualProperties =
+      dataSource.unwrap(OracleDataSource.class)
+        .getConnectionProperties();
+
+    Properties expectedProperties = getJdbcDefaultProperties();
+    optionValues.forEach((option, value) ->
+      expectedProperties.setProperty(option.name(), value));
+
+    expectedProperties.entrySet()
+        .removeAll(actualProperties.entrySet());
+
+    // Don't expect OracleDataSource.getConnectionProperties() to have entries
+    // for options that Oracle R2DBC doesn't set as connection properties.
+    expectedProperties.entrySet()
+        .removeIf(entry ->
+          entry.getKey().toString().startsWith("oracle.r2dbc."));
+
+    // Don't expect OracleDataSource.getConnectionProperties() to have entries
+    // for options of security sensitive values.
+    expectedProperties.entrySet()
+      .removeIf(entry ->
+        entry.getKey().toString().toLowerCase().contains("password"));
+
+    assertTrue(
+      expectedProperties.isEmpty(),
+      "One or more properties were not set: " + expectedProperties);
+  }
+
+  /**
+   * Returns the connection properties that will be set by default when an
+   * {@link OracleDataSource} is created. Tests which verify the setting of
+   * properties can assume these default properties will be set as well.
+   *
+   * @return Properties that OracleDataSource sets by default.
+   */
+  private static Properties getJdbcDefaultProperties() throws SQLException {
+
+    // Start with any properties that JDBC will set by default. For example, the
+    // 21 driver would set CONNECTION_PROPERTY_ENABLE_AC_SUPPORT="false" by
+    // default.
+    Properties defaultProperties =
+      new oracle.jdbc.datasource.impl.OracleDataSource()
+        .getConnectionProperties();
+
+    if (defaultProperties == null)
+      defaultProperties = new Properties();
+
+    // Set the properties that Oracle R2DBC will set by default
+    // Not referencing the deprecated
+    // OracleConnection.CONNECTION_PROPERTY_J2EE13_COMPLIANT field, just in case
+    // it  gets removed in a future release of Oracle JDBC.
+    defaultProperties.setProperty("oracle.jdbc.J2EE13Compliant", "true");
+    defaultProperties.setProperty(
+      OracleConnection.CONNECTION_PROPERTY_IMPLICIT_STATEMENT_CACHE_SIZE, "25");
+    defaultProperties.setProperty(
+      OracleConnection.CONNECTION_PROPERTY_DEFAULT_LOB_PREFETCH_SIZE,
+      "1048576");
+    defaultProperties.setProperty(
+      OracleConnection.CONNECTION_PROPERTY_THIN_NET_USE_ZERO_COPY_IO,
+      "false");
+
+    return defaultProperties;
+  }
+
   /**
    * Returns an Oracle Net Descriptor having the values configured by
    * {@link DatabaseConfig}

src/test/java/oracle/r2dbc/test/OracleTestKit.java

@@ -93,7 +93,8 @@ public class OracleTestKit implements TestKit<Integer> {
   private final JdbcOperations jdbcOperations;
   {
     try {
-      OracleDataSource dataSource = new oracle.jdbc.pool.OracleDataSource();
+      OracleDataSource dataSource =
+        new oracle.jdbc.datasource.impl.OracleDataSource();
       dataSource.setURL(String.format("jdbc:oracle:thin:@%s%s:%d/%s",
         Optional.ofNullable(protocol())
           .map(protocol -> protocol + ":")