Merge branch 'main' of https://github.com/oracle/oracle-r2dbc into 40-ldap-support

Author: Michael-A-McMahon

README.md

@@ -306,13 +306,17 @@ supported by Oracle R2DBC:
     - Out of band (OOB) breaks effect statement timeouts. Set this to "true" if statement timeouts are not working correctly.
   - [oracle.jdbc.enableQueryResultCache](https://docs.oracle.com/en/database/oracle/oracle-database/21/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_ENABLE_QUERY_RESULT_CACHE)
     - Cached query results can cause phantom reads even if the serializable
-     transaction isolation level is set. Set this to "false" if using the
+      transaction isolation level is set. Set this to "false" if using the
       serializable isolation level.
   - [v$session.terminal](https://docs.oracle.com/en/database/oracle/oracle-database/21/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_VSESSION_TERMINAL)
   - [v$session.machine](https://docs.oracle.com/en/database/oracle/oracle-database/21/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_VSESSION_MACHINE)
   - [v$session.osuser](https://docs.oracle.com/en/database/oracle/oracle-database/21/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_VSESSION_OSUSER)
   - [v$session.program](https://docs.oracle.com/en/database/oracle/oracle-database/21/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_VSESSION_PROGRAM)
   - [v$session.process](https://docs.oracle.com/en/database/oracle/oracle-database/21/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_VSESSION_PROCESS)
+  - (For inclusion in the next release) [oracle.jdbc.timeZoneAsRegion](https://docs.oracle.com/en/database/oracle/oracle-database/21/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_TIMEZONE_AS_REGION)
+    - Setting this option to "false" may resolve "ORA-01882: timezone region not 
+      found". The ORA-01882 error happens when Oracle Database doesn't recognize 
+      the name returned by `java.util.TimeZone.getDefault().getId()`.
 
 ### Thread Safety and Parallel Execution
 Oracle R2DBC's `ConnectionFactory` and `ConnectionFactoryProvider` are the only 

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

@@ -229,6 +229,13 @@ private OracleR2dbcOptions() {}
    * {@link OracleConnection#CONNECTION_PROPERTY_THIN_VSESSION_PROCESS}
    */
   public static final Option<CharSequence> VSESSION_PROCESS;
+
+  /**
+   * Configures the Oracle JDBC Connection used by Oracle R2DBC as specified by:
+   * {@link OracleConnection#CONNECTION_PROPERTY_TIMEZONE_AS_REGION}
+   */
+  public static final Option<CharSequence> TIMEZONE_AS_REGION;
+
   /**
    * Configures the Oracle JDBC Connection used by Oracle R2DBC as specified by:
    * {@link OracleConnection#CONNECTION_PROPERTY_THIN_LDAP_SECURITY_AUTHENTICATION}
@@ -398,6 +405,8 @@ private OracleR2dbcOptions() {}
       OracleConnection.CONNECTION_PROPERTY_THIN_VSESSION_PROGRAM),
     VSESSION_PROCESS = Option.valueOf(
       OracleConnection.CONNECTION_PROPERTY_THIN_VSESSION_PROCESS),
+    TIMEZONE_AS_REGION = Option.valueOf(
+      OracleConnection.CONNECTION_PROPERTY_TIMEZONE_AS_REGION),
     LDAP_SECURITY_AUTHENTICATION = Option.valueOf(
       OracleConnection.CONNECTION_PROPERTY_THIN_LDAP_SECURITY_AUTHENTICATION),
     LDAP_SECURITY_PRINCIPAL = Option.valueOf(
@@ -446,4 +455,5 @@ private OracleR2dbcOptions() {}
   public static Set<Option<?>> options() {
     return OPTIONS;
   }
+
 }

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

@@ -53,13 +53,13 @@
 import java.time.Duration;
 import java.util.Arrays;
 import java.util.Objects;
-import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Flow;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.stream.Stream;
 
 import static io.r2dbc.spi.ConnectionFactoryOptions.CONNECT_TIMEOUT;
 import static io.r2dbc.spi.ConnectionFactoryOptions.DATABASE;
@@ -316,17 +316,12 @@ public AsyncLock getLock() {
    *   </li><li>
    *   {@linkplain OracleConnection#CONNECTION_PROPERTY_THIN_VSESSION_MACHINE
    *     v$session.machine}
+   *   </li><li>
+   *   {@linkplain OracleConnection#CONNECTION_PROPERTY_TIMEZONE_AS_REGION
+   *     oracle.jdbc.timezoneAsRegion}
    *   </li>
    * </ul>
    *
-   * @implNote The returned {@code DataSource} is configured to create
-   * connections that encode character bind values using the National
-   * Character Set of an Oracle Database. In 21c, the National Character Set
-   * must be either UTF-8 or UTF-16; This ensures that unicode bind data is
-   * properly encoded by Oracle JDBC. If the data source is not configured
-   * this way, the Oracle JDBC Driver uses the default character set of the
-   * database, which may not support Unicode characters.
-   *
    * @throws IllegalArgumentException If the {@code oracleNetDescriptor}
    * {@code Option} is provided with any other options that might have
    * conflicting values, such as {@link ConnectionFactoryOptions#HOST}.
@@ -391,7 +386,7 @@ private static String composeJdbcUrl(ConnectionFactoryOptions options) {
 
     if (descriptor != null) {
       validateDescriptorOptions(options);
-      return "jdbc:oracle:thin:@" + descriptor.toString();
+      return "jdbc:oracle:thin:@" + descriptor;
     }
     else {
       Object protocol =
@@ -423,8 +418,7 @@ private static String composeJdbcUrl(ConnectionFactoryOptions options) {
   private static void validateDescriptorOptions(
     ConnectionFactoryOptions options) {
     Option<?>[] conflictingOptions =
-      Set.of(HOST, PORT, DATABASE, SSL)
-        .stream()
+      Stream.of(HOST, PORT, DATABASE, SSL)
         .filter(options::hasOption)
         .filter(option ->
           // Ignore options having a value that can be represented as a

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

@@ -39,6 +39,7 @@
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.sql.SQLType;
 import java.sql.SQLWarning;
 import java.time.Duration;
@@ -55,6 +56,7 @@
 import java.util.function.Function;
 import java.util.stream.IntStream;
 
+import static java.sql.Statement.CLOSE_ALL_RESULTS;
 import static java.sql.Statement.KEEP_CURRENT_RESULT;
 import static java.sql.Statement.RETURN_GENERATED_KEYS;
 import static java.util.Objects.requireNonNullElse;
@@ -1158,26 +1160,56 @@ private OracleResultImpl getWarnings(OracleResultImpl result) {
      */
     private Publisher<Void> deallocate(Collection<OracleResultImpl> results) {
 
-      // Close the statement after all results are consumed
+      // Set up a counter that is decremented as each result is consumed.
       AtomicInteger unconsumed = new AtomicInteger(results.size());
+
+      // Set up a publisher that decrements the counter, and closes the
+      // statement when it reaches zero
       Publisher<Void> closeStatement = adapter.getLock().run(() -> {
         if (unconsumed.decrementAndGet() == 0)
-          preparedStatement.close();
+          closeStatement();
       });
 
+      // Tell each unconsumed result to decrement the unconsumed count, and then
+      // close the statement when the count reaches zero.
       for (OracleResultImpl result : results) {
         if (!result.onConsumed(closeStatement))
           unconsumed.decrementAndGet();
       }
 
-      // If all results have already been consumed, the returned
-      // publisher closes the statement
+      // If there are no results, or all results have already been consumed,
+      // then the returned publisher closes the statement.
       if (unconsumed.get() == 0)
-        addDeallocation(adapter.getLock().run(preparedStatement::close));
+        addDeallocation(adapter.getLock().run(this::closeStatement));
 
       return deallocators;
     }
 
+    /**
+     * Closes the JDBC {@link #preparedStatement}. This method should only be
+     * called while holding the
+     * {@linkplain ReactiveJdbcAdapter#getLock() connection lock}
+     * @throws SQLException If the statement fails to close.
+     */
+    private void closeStatement() throws SQLException {
+      try {
+        // Workaround Oracle JDBC bug #34545179: ResultSet references are
+        // retained even when the statement is closed. Calling getMoreResults
+        // with the CLOSE_ALL_RESULTS argument forces the driver to
+        // de-reference them.
+        preparedStatement.getMoreResults(CLOSE_ALL_RESULTS);
+      }
+      catch (SQLException sqlException) {
+        // It may be the case that the JDBC connection was closed, and so the
+        // statement was closed with it. Check for this, and ignore the
+        // SQLException if so.
+        if (!jdbcConnection.isClosed())
+          throw sqlException;
+      }
+
+      preparedStatement.close();
+    }
+
     /**
      * Sets the {@code value} of a {@code preparedStatement} parameter at the
      * specified {@code index}. If a non-null {@code type} is provided, then it is
@@ -1454,7 +1486,7 @@ private JdbcBatch(
      */
     @Override
     protected Publisher<Void> bind() {
-      @SuppressWarnings({"unchecked","rawtypes"})
+      @SuppressWarnings({"unchecked"})
       Publisher<Void>[] bindPublishers = new Publisher[batchSize];
       for (int i = 0; i < batchSize; i++) {
         bindPublishers[i] = Flux.concat(

src/oracle-r2dbc-master.iml

@@ -37,7 +37,6 @@
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
-import javax.naming.spi.NamingManager;
 import java.io.IOException;
 import java.nio.channels.ServerSocketChannel;
 import java.nio.channels.SocketChannel;
@@ -46,10 +45,12 @@
 import java.nio.file.StandardOpenOption;
 import java.sql.SQLException;
 import java.time.Duration;
+import java.time.ZonedDateTime;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Properties;
 import java.util.Set;
+import java.util.TimeZone;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
@@ -616,6 +617,53 @@ private static String createDescriptor() {
       serviceName());
   }
 
+  /**
+   * Verifies the {@link OracleR2dbcOptions#TIMEZONE_AS_REGION} option
+   */
+  @Test
+  public void testTimezoneAsRegion() {
+    // Set the timezone to that of Warsaw. When JDBC opens a connection, it will
+    // read the Warsaw timezone from TimeZone.getDefault().
+    TimeZone warsawTimeZone = TimeZone.getTimeZone("Europe/Warsaw");
+    TimeZone timeZoneRestored = TimeZone.getDefault();
+    TimeZone.setDefault(warsawTimeZone);
+    try {
+
+      // Configure the JDBC connection property with a URL parameter. This has
+      // JDBC express the session timezone as an offset of UTC (+02:00), rather
+      // than a name (Europe/Warsaw).
+      Connection connection = awaitOne(ConnectionFactories.get(
+        ConnectionFactoryOptions.parse(format(
+          "r2dbc:oracle://%s:%d/%s?oracle.jdbc.timezoneAsRegion=false",
+          host(), port(), serviceName()))
+          .mutate()
+          .option(USER, user())
+          .option(PASSWORD, password())
+          .build())
+        .create());
+      try {
+
+        // Query the session timezone, and expect it to be expressed as an
+        // offset, rather than a name.
+        assertEquals(
+          ZonedDateTime.now(warsawTimeZone.toZoneId())
+            .getOffset()
+            .toString(),
+          awaitOne(awaitOne(connection.createStatement(
+            "SELECT sessionTimeZone FROM sys.dual")
+            .execute())
+            .map(row ->
+              row.get(0, String.class))));
+      }
+      finally {
+        tryAwaitNone(connection.close());
+      }
+    }
+    finally {
+      TimeZone.setDefault(timeZoneRestored);
+    }
+  }
+
   /**
    * Verifies that an attempt to connect with a {@code listeningChannel}
    * results in an {@link R2dbcTimeoutException}.

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

@@ -24,6 +24,7 @@
 import io.r2dbc.spi.Blob;
 import io.r2dbc.spi.Clob;
 import io.r2dbc.spi.Connection;
+import io.r2dbc.spi.Parameters;
 import io.r2dbc.spi.Row;
 import io.r2dbc.spi.Statement;
 import oracle.sql.json.OracleJsonFactory;
@@ -50,6 +51,9 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static io.r2dbc.spi.R2dbcType.NCHAR;
+import static io.r2dbc.spi.R2dbcType.NCLOB;
+import static io.r2dbc.spi.R2dbcType.NVARCHAR;
 import static java.util.Arrays.asList;
 import static oracle.r2dbc.test.DatabaseConfig.connectTimeout;
 import static oracle.r2dbc.test.DatabaseConfig.databaseVersion;
@@ -95,6 +99,16 @@ public class TypeMappingTest {
    * <a href="https://r2dbc.io/spec/0.8.3.RELEASE/spec/html/#datatypes.mapping">
    * Table 4 of Section 12 of the R2DBC 0.8.3 Specification.
    * </a>
+   * </p><p>
+   * This test method makes use of {@link io.r2dbc.spi.R2dbcType#NCHAR} and
+   * {@link io.r2dbc.spi.R2dbcType#NVARCHAR} when binding Strings that contain
+   * non-ascii characters. By default, a String bind is mapped to the VARCHAR
+   * SQL type. This default mapping has the driver encode the value using the
+   * database character set. The database character set may not support
+   * non-ascii characters. Binding Strings with the NCHAR/NVARCHAR type
+   * configures the driver to encode the string using the national character set
+   * of the database. The national character set is either UTF16 or UTF8, and so
+   * it must support non-ascii characters.
    * </p>
    */
   @Test
@@ -112,11 +126,18 @@ public void testCharacterTypeMappings() {
 
       // Expect NCHAR and String to map
       verifyTypeMapping(connection,
-        String.format("%100s", "你好, Oracle"), "NCHAR(100)");
+        Parameters.in(NCHAR, String.format("%100s", "你好, Oracle")),
+        "NCHAR(100)",
+        (expected, actual) ->
+          assertEquals(expected.getValue(), actual));
 
       // Expect NVARCHAR and String to map. The Oracle type named "NVARCHAR2" is
       // equivalent to the standard type named "NVARCHAR"
-      verifyTypeMapping(connection, "नमस्कार, Oracle", "NVARCHAR2(100)");
+      verifyTypeMapping(connection,
+        Parameters.in(NVARCHAR, "नमस्कार, Oracle"),
+        "NVARCHAR2(100)",
+        (expected, actual) ->
+          assertEquals(expected.getValue(), actual));
 
       // Expect CLOB and String to map
       verifyTypeMapping(connection, "Hola, Oracle", "CLOB");
@@ -130,7 +151,11 @@ public void testCharacterTypeMappings() {
 
       // Expect NCLOB and String to map for bind values, but not for row values.
       // For row values, expect Oracle CLOB to be mapped to io.r2dbc.spi.Clob
-      verifyTypeMapping(connection, "こんにちは, Oracle", "NCLOB");
+      verifyTypeMapping(connection,
+        Parameters.in(NVARCHAR, "こんにちは, Oracle"),
+        "NCLOB",
+        (expected, actual) ->
+          assertEquals(expected.getValue(), actual));
 
       // Expect NCLOB and io.r2dbc.spi.Clob to map
       verifyTypeMapping(connection,