Throwing IllegalArgumentException

Author: Michael-A-McMahon

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

@@ -31,6 +31,7 @@
 import reactor.core.publisher.Mono;
 
 import javax.sql.DataSource;
+import java.time.Duration;
 
 /**
  * <p>
@@ -48,30 +49,43 @@
  * </p>
  * <h3 id="required_options">Required Options</h3><p>
  * This implementation requires the following options for connection creation:
- * </p><ul>
- *   <li>{@link ConnectionFactoryOptions#DRIVER}</li>
- *   <li>{@link ConnectionFactoryOptions#HOST}</li>
- * </ul>
+ * </p><dl>
+ *   <dt>{@link ConnectionFactoryOptions#DRIVER}</dt>
+ *   <dd>Must have the value "oracle"</dd>
+ *   <dt>{@link ConnectionFactoryOptions#HOST}</dt>
+ *   <dd>IP address or hostname of an Oracle Database</dd>
+ * </dl>
  * <h3 id="supported_options">Supported Options</h3><p>
  * This implementation supports the following options for connection creation:
- * </p><ul>
- *   <li>{@link ConnectionFactoryOptions#PORT}</li>
- *   <li>{@link ConnectionFactoryOptions#DATABASE}</li>
- *   <li>{@link ConnectionFactoryOptions#USER}</li>
- *   <li>{@link ConnectionFactoryOptions#PASSWORD}</li>
- *   <li>{@link ConnectionFactoryOptions#CONNECT_TIMEOUT}</li>
- *   <li>{@link ConnectionFactoryOptions#SSL}</li>
- * </ul><p>
- * A value set for {@code CONNECT_TIMEOUT} will be rounded up to the nearest
- * whole second. When a value is set, any connection request that exceeds the
- * specified duration of seconds will automatically be cancelled. The
- * cancellation will result in an {@code onError} signal delivering an
- * {@link io.r2dbc.spi.R2dbcTimeoutException} to a connection {@code
- * Subscriber}.
- * </p><p>
- * A value of {@code true} set for {@code SSL} will configure the Oracle
- * JDBC Driver to connect using the TCPS protocol (ie: SSL/TLS).
- * </p>
+ * </p><dl>
+ *   <dt>{@link ConnectionFactoryOptions#PORT}</dt>
+ *   <dd>Port number of an Oracle Database</dd>
+ *   <dt>{@link ConnectionFactoryOptions#DATABASE}</dt>
+ *   <dd>Service name (not an SID) of an Oracle Database</dd>
+ *   <dt>{@link ConnectionFactoryOptions#USER}</dt>
+ *   <dd>Name of an Oracle Database user</dd>
+ *   <dt>{@link ConnectionFactoryOptions#PASSWORD}</dt>
+ *   <dd>
+ *     Password of an Oracle Database user. The value may be an instance
+ *     of a mutable {@link CharSequence}, such {@link java.nio.CharBuffer},
+ *     that may be cleared after creating an instance of
+ *     {@code OracleConnectionFactoryImpl}.
+ *   </dd>
+ *   <dt>{@link ConnectionFactoryOptions#CONNECT_TIMEOUT}</dt>
+ *   <dd>
+ *     Maximum wait time when requesting a {@code Connection}. If the
+ *     duration expires, a {@code Connection} {@code Subscriber} receives
+ *     {@code onError} with an {@link io.r2dbc.spi.R2dbcTimeoutException}.
+ *     The duration is rounded up to the nearest whole second. The query
+ *     section of an R2DBC URL may provide a value in the format specified by
+ *     {@link Duration#parse(CharSequence)}.
+ *   </dd>
+ *   <dt>{@link ConnectionFactoryOptions#SSL}</dt>
+ *   <dd>
+ *     If set to {@code true}, the driver connects to Oracle Database using
+ *     TCPS (ie: SSL/TLS).
+ *   </dd>
+ * </dl>
  *
  * @author  harayuanwang, michael-a-mcmahon
  * @since   0.1.0
@@ -121,14 +135,19 @@ final class OracleConnectionFactoryImpl implements ConnectionFactory {
    * </p>
    *
    * @param options Options applied when opening a connection to a database.
+   *
    * @throws IllegalArgumentException If the value of a required option is
    * null.
+   *
    * @throws IllegalStateException If the value of a required option is not
    * specified.
+   *
+   * @throws IllegalArgumentException If the {@code oracle-net-descriptor}
+   * {@code Option} is provided with any other options that might have
+   * conflicting values, such as {@link ConnectionFactoryOptions#HOST}.
    */
   OracleConnectionFactoryImpl(ConnectionFactoryOptions options) {
     OracleR2dbcExceptions.requireNonNull(options, "options is null.");
-
     adapter = ReactiveJdbcAdapter.getOracleAdapter();
     dataSource = adapter.createDataSource(options);
   }

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

@@ -27,6 +27,7 @@
 
 import java.util.ServiceLoader;
 
+import static io.r2dbc.spi.ConnectionFactoryOptions.DRIVER;
 import static oracle.r2dbc.impl.OracleR2dbcExceptions.requireNonNull;
 
 /**
@@ -83,8 +84,13 @@ public OracleConnectionFactoryProviderImpl() { }
    * {@code ConnectionFactory} that applies values specified by the {@code
    * options} parameter, when opening connections to an Oracle Database.
    * </p>
+   *
    * @throws IllegalStateException If any option required by
    * {@link OracleConnectionFactoryImpl} is not specified by {@code options}.
+   *
+   * @throws IllegalArgumentException If the {@code oracle-net-descriptor}
+   * {@code Option} is provided with any other options that might have
+   * conflicting values, such as {@link ConnectionFactoryOptions#HOST}.
    */
   @Override
   public ConnectionFactory create(ConnectionFactoryOptions options) {
@@ -103,8 +109,7 @@ public ConnectionFactory create(ConnectionFactoryOptions options) {
   @Override
   public boolean supports(ConnectionFactoryOptions options) {
     requireNonNull(options, "options must not be null.");
-    return DRIVER_IDENTIFIER.equals(
-      options.getValue(ConnectionFactoryOptions.DRIVER));
+    return DRIVER_IDENTIFIER.equals(options.getValue(DRIVER));
   }
 
   /**

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

@@ -420,6 +420,10 @@ static OracleReactiveJdbcAdapter getInstance() {
    * 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 oracle-net-descriptor}
+   * {@code Option} is provided with any other options that might have
+   * conflicting values, such as {@link ConnectionFactoryOptions#HOST}.
    */
   @Override
   public DataSource createDataSource(ConnectionFactoryOptions options) {
@@ -442,6 +446,9 @@ public DataSource createDataSource(ConnectionFactoryOptions options) {
    * {@link #createDataSource(ConnectionFactoryOptions)}
    * @param options R2DBC options. Not null.
    * @return An Oracle JDBC URL composed from R2DBC options
+   * @throws IllegalArgumentException If the {@code oracle-net-descriptor}
+   * {@code Option} is provided with any other options that might have
+   * conflicting values, such as {@link ConnectionFactoryOptions#HOST}.
    */
   private static String composeJdbcUrl(ConnectionFactoryOptions options) {
     Object descriptor = options.getValue(DESCRIPTOR);
@@ -468,10 +475,10 @@ private static String composeJdbcUrl(ConnectionFactoryOptions options) {
   /**
    * Validates {@code options} when the {@link #DESCRIPTOR} {@code Option} is
    * present. It is invalid to specify any other options having information
-   * that overlaps with information in the descriptor, such as
+   * that potentially conflicts with information in the descriptor, such as
    * {@link ConnectionFactoryOptions#HOST}.
    * @param options Options to validate
-   * @throws IllegalStateException If {@code options} are invalid
+   * @throws IllegalArgumentException If {@code options} are invalid
    */
   private static void validateDescriptorOptions(
     ConnectionFactoryOptions options) {
@@ -488,8 +495,8 @@ private static void validateDescriptorOptions(
         .toArray(Option[]::new);
 
     if (overlappingOptions.length != 0) {
-      throw new IllegalStateException(DESCRIPTOR.name()
-        + " Option has been specified with overlapping Options: "
+      throw new IllegalArgumentException(DESCRIPTOR.name()
+        + " Option has been specified with potentially conflicting Options: "
         + Arrays.toString(overlappingOptions));
     }
   }

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

@@ -61,6 +61,7 @@
 import static oracle.r2dbc.util.Awaits.awaitNone;
 import static oracle.r2dbc.util.Awaits.awaitOne;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Verifies that
@@ -173,10 +174,11 @@ public void testCreateDataSource() throws SQLException {
   /**
    * Verifies that
    * {@link OracleReactiveJdbcAdapter#createDataSource(ConnectionFactoryOptions)}
-   * handles the {@link OracleConnection#CONNECTION_PROPERTY_TNS_ADMIN} property
-   * correctly. This property configures a file system path to a
+   * handles Oracle Net Descriptors and the
+   * {@link OracleConnection#CONNECTION_PROPERTY_TNS_ADMIN} property
+   * correctly. The TNS ADMIN property configures a file system path to a
    * directory containing tnsnames.ora and ojdbc.properties files. If the TNS
-   * admin property is handled correctly, then TNS descriptors should be read
+   * admin property is handled correctly, then descriptors should be read
    * from tnsnames.ora, and connection properties should be read from
    * ojdbc.properties.
    */
@@ -231,6 +233,30 @@ public void testTnsAdmin() throws IOException {
           .create())
         .close());
 
+      // Expect IllegalArgumentException if HOST, PORT, DATABASE, or SSL options
+      // are provided with a descriptor
+      assertThrows(IllegalArgumentException.class, () ->
+        ConnectionFactories.get(
+          "r2dbc:oracle://"+host()+"?oracle-net-descriptor="+descriptor));
+      assertThrows(IllegalArgumentException.class, () ->
+        ConnectionFactories.get("r2dbc:oracle://"
+            +host()+":"+port()+"?oracle-net-descriptor="+descriptor));
+      assertThrows(IllegalArgumentException.class, () ->
+        ConnectionFactories.get("r2dbc:oracle://"+host()+":"+port()+"/"
+          +serviceName()+"?oracle-net-descriptor="+descriptor));
+      assertThrows(IllegalArgumentException.class, () ->
+        ConnectionFactories.get("r2dbc:oracle://"+host()+"/"
+          +serviceName()+"?oracle-net-descriptor="+descriptor));
+      assertThrows(IllegalArgumentException.class, () ->
+        ConnectionFactories.get("r2dbc:oracle:///"
+            +serviceName()+"?oracle-net-descriptor="+descriptor));
+      assertThrows(IllegalArgumentException.class, () ->
+        ConnectionFactories.get("r2dbcs:oracle://" + // r2dbcs is SSL=true
+          "?oracle-net-descriptor="+descriptor));
+      assertThrows(IllegalArgumentException.class, () ->
+        ConnectionFactories.get(
+          "r2dbc:oracle://?oracle-net-descriptor="+descriptor+"&ssl=true"));
+
       // Create an ojdbc.properties file containing the user name
       Files.writeString(Path.of("ojdbc.properties"),
         String.format("user=%s", user()),