This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5728-9cede6f4393ed2822fa1e103706b3f73e14af8b5 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 990b7cb75330ea64efd76af26046368dce4900a3 Author: Matthew B. <[email protected]> AuthorDate: Tue Jun 16 09:51:36 2026 -0700 test(workflow-operator): pin ConnUtil URL composition without DriverManager (#5728) ### What changes were proposed in this PR? - `PostgreSQLConnUtilSpec` / `MySQLConnUtilSpec` were flaky: they ran a multi-step sequence against the process-global `java.sql.DriverManager` registry (deregister the real driver, register a capturing stub, restore), and the module's tests run un-forked with sbt's default `parallelExecution = true`, so the suites raced any concurrent suite that touched JDBC in the same JVM. - Extracted the JDBC URL composition (the only application-controlled logic) into a pure `buildJdbcUrl(host, port, database)` in `PostgreSQLConnUtil` and `MySQLConnUtil`; the existing `connect` now calls it, so the public signature and runtime behavior are unchanged. - Rewrote both specs to assert URL composition directly on `buildJdbcUrl`, with no `DriverManager` mutation and no `BeforeAndAfterAll`, so the suites are deterministic under parallel execution. - Dropped the former `setReadOnly` / credential / `SQLException` assertions: those exercise JDBC plumbing rather than application logic and were what forced the global-registry manipulation behind the flake. ### Trade-off: the dropped assertions The previous specs reached `connect` by mutating the global `DriverManager`; that seam, not the assertions, is what caused the race. The three dropped behaviors break down as: | Behavior | Regression risk | Notes | | --- | --- | --- | | `buildJdbcUrl` composition | Meaningful (kept) | The only logic with string composition / ordering that can plausibly regress. | | `setReadOnly(true)` | Low | A single constant line with no inputs, but a genuine read-only/efficiency contract. | | Credential propagation | Negligible | `connect` forwards `username`/`password` straight to `DriverManager`; a test mostly asserts the injected seam, not app logic. | | `SQLException` propagation | Negligible | No `try`/`catch` in `connect`; the test pins "Scala lets exceptions bubble." | These three could be restored *without* reintroducing the race, via a dependency-injection seam (a package-private `openConnection(url, user, pass, opener = DriverManager.getConnection)` that tests call with a fake opener plus a `Proxy`-backed `Connection`): no global registry mutation, so nothing for parallel suites to race on. > [!NOTE] > Deliberately did **not** add the DI seam here. For these utilities, the seam would be permanent production complexity in service of tests that mostly verify the mock. ### Any related issues, documentation, discussions? Closes: #5727 ### How was this PR tested? - Run `sbt "WorkflowOperator/testOnly *PostgreSQLConnUtilSpec *MySQLConnUtilSpec"` (under JDK 17); expect 12 tests succeeded, 0 failed across 2 suites. - Run `sbt scalafmtCheckAll`; expect success (no formatting diffs). - The original race was timing-dependent and not reproducible on demand; the fix removes the shared `DriverManager` state entirely, so there is no longer cross-suite state to race. Reviewers can confirm by grepping the two specs for `DriverManager` and `BeforeAndAfterAll`, both now absent. ### Was this PR authored or co-authored using generative AI tooling? Co-authored with Claude Opus 4.8 in compliance with ASF --- .../operator/source/sql/mysql/MySQLConnUtil.scala | 8 +- .../source/sql/postgresql/PostgreSQLConnUtil.scala | 7 +- .../source/sql/mysql/MySQLConnUtilSpec.scala | 234 +++----------------- .../sql/postgresql/PostgreSQLConnUtilSpec.scala | 235 ++------------------- 4 files changed, 65 insertions(+), 419 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtil.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtil.scala index 3e1ea93826..120f5dfe51 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtil.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtil.scala @@ -22,6 +22,11 @@ package org.apache.texera.amber.operator.source.sql.mysql import java.sql.{Connection, DriverManager, SQLException} object MySQLConnUtil { + + // Builds the JDBC URL. + private[mysql] def buildJdbcUrl(host: String, port: String, database: String): String = + "jdbc:mysql://" + host + ":" + port + "/" + database + "?autoReconnect=true&useSSL=true" + @throws[SQLException] def connect( host: String, @@ -30,8 +35,7 @@ object MySQLConnUtil { username: String, password: String ): Connection = { - val url = - "jdbc:mysql://" + host + ":" + port + "/" + database + "?autoReconnect=true&useSSL=true" + val url = buildJdbcUrl(host, port, database) val connection = DriverManager.getConnection(url, username, password) // set to readonly to improve efficiency connection.setReadOnly(true) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtil.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtil.scala index c6fb4ff792..ba201a2ef7 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtil.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtil.scala @@ -22,6 +22,11 @@ package org.apache.texera.amber.operator.source.sql.postgresql import java.sql.{Connection, DriverManager, SQLException} object PostgreSQLConnUtil { + + // Builds the JDBC URL. + private[postgresql] def buildJdbcUrl(host: String, port: String, database: String): String = + "jdbc:postgresql://" + host + ":" + port + "/" + database + @throws[SQLException] def connect( host: String, @@ -30,7 +35,7 @@ object PostgreSQLConnUtil { username: String, password: String ): Connection = { - val url = "jdbc:postgresql://" + host + ":" + port + "/" + database + val url = buildJdbcUrl(host, port, database) val connection = DriverManager.getConnection(url, username, password) // set to readonly to improve efficiency connection.setReadOnly(true) diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtilSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtilSpec.scala index e4bd71cb18..99f3f38445 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtilSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/mysql/MySQLConnUtilSpec.scala @@ -20,241 +20,71 @@ package org.apache.texera.amber.operator.source.sql.mysql import org.scalatest.flatspec.AnyFlatSpec -import org.scalatest.BeforeAndAfterAll -import java.lang.reflect.{InvocationHandler, Method, Proxy} -import java.sql.{Connection, Driver, DriverManager, DriverPropertyInfo, SQLException} -import java.util.Properties -import java.util.logging.Logger -import scala.collection.mutable.ArrayBuffer -import scala.jdk.CollectionConverters._ +class MySQLConnUtilSpec extends AnyFlatSpec { -class MySQLConnUtilSpec extends AnyFlatSpec with BeforeAndAfterAll { + // Strategy: same as PostgreSQLConnUtilSpec. Pin the JDBC URL composition + // (the only application-logic in this util) without a real DB. // --------------------------------------------------------------------------- - // Strategy — same capturing-driver pattern as PostgreSQLConnUtilSpec. - // The MySQL driver may or may not be present transitively, so we - // proactively deregister anything that claims jdbc:mysql: and swap in a - // capturing driver that records each URL and returns a Proxy-backed - // Connection so the production code can call `setReadOnly(true)`. + // URL composition - host/port/database // --------------------------------------------------------------------------- - private object CapturingMySQLDriver extends Driver { - val seenUrls: ArrayBuffer[String] = ArrayBuffer.empty - val seenProps: ArrayBuffer[Properties] = ArrayBuffer.empty - val readOnlyCalls: ArrayBuffer[Boolean] = ArrayBuffer.empty - - override def connect(url: String, info: Properties): Connection = { - if (!acceptsURL(url)) return null - seenUrls += url - seenProps += info - Proxy - .newProxyInstance( - getClass.getClassLoader, - Array(classOf[Connection]), - new InvocationHandler { - override def invoke(p: Any, m: Method, args: Array[AnyRef]): AnyRef = - m.getName match { - case "setReadOnly" => - readOnlyCalls += args(0).asInstanceOf[java.lang.Boolean].booleanValue() - null - case "equals" => java.lang.Boolean.valueOf(p eq args(0)) - case "hashCode" => java.lang.Integer.valueOf(System.identityHashCode(p)) - case "toString" => - "CapturingMySQLDriver.StubConnection@" + System.identityHashCode(p) - case "isWrapperFor" => java.lang.Boolean.FALSE - case "close" => null - case _ => null - } - } - ) - .asInstanceOf[Connection] - } - override def acceptsURL(url: String): Boolean = - url != null && url.startsWith("jdbc:mysql:") - override def getPropertyInfo(url: String, info: Properties): Array[DriverPropertyInfo] = - Array.empty - override def getMajorVersion: Int = 1 - override def getMinorVersion: Int = 0 - override def jdbcCompliant(): Boolean = false - override def getParentLogger: Logger = Logger.getLogger("test-mysql-capturing") - } - - private val savedRealDrivers: ArrayBuffer[Driver] = ArrayBuffer.empty - - private def safeAcceptsURL(d: Driver, url: String): Boolean = - try d.acceptsURL(url) - catch { case _: Throwable => false } - - override protected def beforeAll(): Unit = { - super.beforeAll() - // The probe URL mirrors the exact shape `MySQLConnUtil.connect` - // constructs (`jdbc:mysql://{host}:{port}/{database}?…`), including - // the canonical query parameters. A permissive third-party driver - // that returns `false` on a stripped-down probe but `true` on the - // real URL would otherwise slip past us. - try { - val others = DriverManager.getDrivers.asScala.toList.filter { d => - d != CapturingMySQLDriver && safeAcceptsURL( - d, - "jdbc:mysql://probe-host:3306/probe-db?autoReconnect=true&useSSL=true" - ) - } - others.foreach { d => - savedRealDrivers += d - DriverManager.deregisterDriver(d) - } - DriverManager.registerDriver(CapturingMySQLDriver) - } catch { - case t: Throwable => - savedRealDrivers.foreach { d => - try DriverManager.registerDriver(d) - catch { case _: Throwable => () } - } - throw t - } - } - - override protected def afterAll(): Unit = { - try { - try DriverManager.deregisterDriver(CapturingMySQLDriver) - catch { case _: Throwable => () } - savedRealDrivers.foreach { d => - try DriverManager.registerDriver(d) - catch { case _: Throwable => () } - } - } finally { - super.afterAll() - } - } - - private def clearCapture(): Unit = { - CapturingMySQLDriver.seenUrls.clear() - CapturingMySQLDriver.seenProps.clear() - CapturingMySQLDriver.readOnlyCalls.clear() - } - - // --------------------------------------------------------------------------- - // URL composition — host/port/database - // --------------------------------------------------------------------------- - - "MySQLConnUtil.connect" should - "build a JDBC URL of the form jdbc:mysql://{host}:{port}/{database}?…" in { - clearCapture() - val conn = MySQLConnUtil.connect("host-m", "3306", "db-m", "u", "p") - assert(conn != null) - assert(CapturingMySQLDriver.seenUrls.size == 1) - assert(CapturingMySQLDriver.seenUrls.head.startsWith("jdbc:mysql://host-m:3306/db-m")) + "MySQLConnUtil.buildJdbcUrl" should + "build a URL of the form jdbc:mysql://{host}:{port}/{database}?..." in { + assert( + MySQLConnUtil + .buildJdbcUrl("host-m", "3306", "db-m") + .startsWith("jdbc:mysql://host-m:3306/db-m") + ) } it should "interpolate distinct host/port/database values into the URL" in { - clearCapture() - MySQLConnUtil.connect("host-1", "3306", "db-1", "u", "p") - assert(CapturingMySQLDriver.seenUrls.head.startsWith("jdbc:mysql://host-1:3306/db-1")) - clearCapture() - MySQLConnUtil.connect("host-2", "33060", "db-2", "u", "p") - assert(CapturingMySQLDriver.seenUrls.head.startsWith("jdbc:mysql://host-2:33060/db-2")) + assert( + MySQLConnUtil + .buildJdbcUrl("host-1", "3306", "db-1") + .startsWith("jdbc:mysql://host-1:3306/db-1") + ) + assert( + MySQLConnUtil + .buildJdbcUrl("host-2", "33060", "db-2") + .startsWith("jdbc:mysql://host-2:33060/db-2") + ) } it should "place host BEFORE port" in { - clearCapture() - MySQLConnUtil.connect("a", "1", "x", "u", "p") - val url = CapturingMySQLDriver.seenUrls.head + val url = MySQLConnUtil.buildJdbcUrl("a", "1", "x") assert(url.contains("//a:1/")) assert(!url.contains("//1:a/")) } // --------------------------------------------------------------------------- - // Query parameters — autoReconnect=true and useSSL=true must be present + // Query parameters - autoReconnect=true and useSSL=true must be present // --------------------------------------------------------------------------- it should "include the `autoReconnect=true` query parameter" in { - clearCapture() - MySQLConnUtil.connect("h", "3306", "db", "u", "p") - val url = CapturingMySQLDriver.seenUrls.head + val url = MySQLConnUtil.buildJdbcUrl("h", "3306", "db") assert(url.contains("autoReconnect=true"), s"URL must include autoReconnect=true, got: $url") } it should "include the `useSSL=true` query parameter (TLS contract)" in { - clearCapture() - MySQLConnUtil.connect("h", "3306", "db", "u", "p") - val url = CapturingMySQLDriver.seenUrls.head + val url = MySQLConnUtil.buildJdbcUrl("h", "3306", "db") assert(url.contains("useSSL=true"), s"URL must include useSSL=true (TLS), got: $url") } - it should "use the canonical `?…&…` separator pattern" in { - clearCapture() - MySQLConnUtil.connect("h", "3306", "db", "u", "p") - val url = CapturingMySQLDriver.seenUrls.head + it should "use the canonical `?...&...` separator pattern" in { assert( - url == "jdbc:mysql://h:3306/db?autoReconnect=true&useSSL=true", - s"URL must match canonical pattern, got: $url" + MySQLConnUtil.buildJdbcUrl( + "h", + "3306", + "db" + ) == "jdbc:mysql://h:3306/db?autoReconnect=true&useSSL=true" ) } it should "use the `mysql` JDBC subprotocol (not e.g. `postgresql`)" in { - clearCapture() - MySQLConnUtil.connect("h", "3306", "db", "u", "p") - val url = CapturingMySQLDriver.seenUrls.head + val url = MySQLConnUtil.buildJdbcUrl("h", "3306", "db") assert(url.startsWith("jdbc:mysql://")) assert(!url.contains("jdbc:postgresql:")) } - - // --------------------------------------------------------------------------- - // Credentials propagation - // --------------------------------------------------------------------------- - - it should "pass username and password through DriverManager properties" in { - clearCapture() - MySQLConnUtil.connect("h", "3306", "db", "the-user", "the-pass") - val props = CapturingMySQLDriver.seenProps.head - assert(props.getProperty("user") == "the-user") - assert(props.getProperty("password") == "the-pass") - } - - // --------------------------------------------------------------------------- - // setReadOnly(true) — pinned via the captured proxy (parity with PG spec) - // --------------------------------------------------------------------------- - - it should "flip the returned Connection to read-only (query-efficiency contract)" in { - clearCapture() - MySQLConnUtil.connect("h", "3306", "db", "u", "p") - assert(CapturingMySQLDriver.readOnlyCalls == ArrayBuffer(true)) - } - - // --------------------------------------------------------------------------- - // SQLException propagation when the driver throws - // --------------------------------------------------------------------------- - - it should "propagate SQLException when the driver throws" in { - val throwingDriver = new Driver { - override def acceptsURL(url: String): Boolean = - url != null && url.startsWith("jdbc:mysql:") - // Follow the JDBC contract: return `null` if the URL isn't ours - // and throw only on a matching URL — keeps the helper from - // interfering with `DriverManager.getConnection` calls for any - // other scheme that might happen during the suite. - override def connect(url: String, info: Properties): Connection = { - if (!acceptsURL(url)) return null - throw new SQLException("forced-fail-for-test") - } - override def getPropertyInfo(url: String, info: Properties) = - Array.empty[DriverPropertyInfo] - override def getMajorVersion: Int = 99 - override def getMinorVersion: Int = 0 - override def jdbcCompliant(): Boolean = false - override def getParentLogger: Logger = Logger.getLogger("test-mysql-throwing") - } - DriverManager.deregisterDriver(CapturingMySQLDriver) - DriverManager.registerDriver(throwingDriver) - try { - val ex = intercept[SQLException] { - MySQLConnUtil.connect("h", "3306", "db", "u", "p") - } - assert(ex.getMessage.contains("forced-fail-for-test")) - } finally { - DriverManager.deregisterDriver(throwingDriver) - DriverManager.registerDriver(CapturingMySQLDriver) - } - } } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtilSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtilSpec.scala index 5081c11c26..be8ac359f9 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtilSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/sql/postgresql/PostgreSQLConnUtilSpec.scala @@ -20,244 +20,51 @@ package org.apache.texera.amber.operator.source.sql.postgresql import org.scalatest.flatspec.AnyFlatSpec -import org.scalatest.BeforeAndAfterAll -import java.lang.reflect.{InvocationHandler, Method, Proxy} -import java.sql.{Connection, Driver, DriverManager, DriverPropertyInfo, SQLException} -import java.util.Properties -import java.util.logging.Logger -import scala.collection.mutable.ArrayBuffer -import scala.jdk.CollectionConverters._ +class PostgreSQLConnUtilSpec extends AnyFlatSpec { -class PostgreSQLConnUtilSpec extends AnyFlatSpec with BeforeAndAfterAll { - - // --------------------------------------------------------------------------- - // Strategy — pin the JDBC URL composition (the only application-logic in + // Strategy: pin the JDBC URL composition (the only application-logic in // this util) without a real DB. - // - // The workflow-operator test classpath DOES include the real PostgreSQL - // driver (transitively), and that driver eats `jdbc:postgresql:` URLs - // before returning a generic "The connection attempt failed." exception. - // So we can't rely on `DriverManager.getConnection`'s default - // "No suitable driver" message. - // - // Instead, we deregister every driver claiming `jdbc:postgresql:`, - // register a capturing driver that records each URL it is asked to open - // (and returns a Proxy-backed Connection so the production code can call - // `setReadOnly`), run the assertions, then restore the real drivers - // in afterAll. - // --------------------------------------------------------------------------- - - private object CapturingPGDriver extends Driver { - val seenUrls: ArrayBuffer[String] = ArrayBuffer.empty - val seenProps: ArrayBuffer[Properties] = ArrayBuffer.empty - val readOnlyCalls: ArrayBuffer[Boolean] = ArrayBuffer.empty - - override def connect(url: String, info: Properties): Connection = { - if (!acceptsURL(url)) return null - seenUrls += url - seenProps += info - Proxy - .newProxyInstance( - getClass.getClassLoader, - Array(classOf[Connection]), - new InvocationHandler { - override def invoke(p: Any, m: Method, args: Array[AnyRef]): AnyRef = - m.getName match { - case "setReadOnly" => - readOnlyCalls += args(0).asInstanceOf[java.lang.Boolean].booleanValue() - null - // Object methods — required so `conn != null`, `conn.toString`, - // and identity HashMap-keying work without NPE on auto-unboxing. - case "equals" => java.lang.Boolean.valueOf(p eq args(0)) - case "hashCode" => java.lang.Integer.valueOf(System.identityHashCode(p)) - case "toString" => "CapturingPGDriver.StubConnection@" + System.identityHashCode(p) - case "isWrapperFor" => java.lang.Boolean.FALSE - case "close" => null - case _ => null - } - } - ) - .asInstanceOf[Connection] - } - override def acceptsURL(url: String): Boolean = - url != null && url.startsWith("jdbc:postgresql:") - override def getPropertyInfo(url: String, info: Properties): Array[DriverPropertyInfo] = - Array.empty - override def getMajorVersion: Int = 1 - override def getMinorVersion: Int = 0 - override def jdbcCompliant(): Boolean = false - override def getParentLogger: Logger = Logger.getLogger("test-pg-capturing") - } - - // Snapshot of real PG drivers temporarily deregistered in beforeAll. - // Restored in afterAll so other suites are not left with a broken - // JDBC driver registry. - private val savedRealDrivers: ArrayBuffer[Driver] = ArrayBuffer.empty - - /** `acceptsURL` is declared `throws SQLException`; treat any throw as - * "this driver doesn't claim our scheme" so a flaky third-party driver - * cannot abort the whole suite. - */ - private def safeAcceptsURL(d: Driver, url: String): Boolean = - try d.acceptsURL(url) - catch { case _: Throwable => false } - - override protected def beforeAll(): Unit = { - super.beforeAll() - // Remove every other driver that claims jdbc:postgresql: so our - // capturing driver is the only one DriverManager.getConnection sees. - // The probe URL mirrors the exact shape `PostgreSQLConnUtil.connect` - // constructs (`jdbc:postgresql://{host}:{port}/{database}`) so a - // permissive third-party driver that returns `false` on a stripped- - // down probe but `true` on the real URL can't slip past us. - // - // Wrapped in try/catch so that if any deregistration / registration - // step throws, we restore whatever we already deregistered before - // failing the suite — the alternative leaves the JVM's JDBC registry - // in an inconsistent state for the rest of the test run. - try { - val others = DriverManager.getDrivers.asScala.toList.filter { d => - d != CapturingPGDriver && safeAcceptsURL(d, "jdbc:postgresql://probe-host:5432/probe-db") - } - others.foreach { d => - savedRealDrivers += d - DriverManager.deregisterDriver(d) - } - DriverManager.registerDriver(CapturingPGDriver) - } catch { - case t: Throwable => - // Best-effort restore before re-throwing. - savedRealDrivers.foreach { d => - try DriverManager.registerDriver(d) - catch { case _: Throwable => () } - } - throw t - } - } - - override protected def afterAll(): Unit = { - try { - try DriverManager.deregisterDriver(CapturingPGDriver) - catch { case _: Throwable => () } - savedRealDrivers.foreach { d => - try DriverManager.registerDriver(d) - catch { case _: Throwable => () } - } - } finally { - super.afterAll() - } - } - - private def clearCapture(): Unit = { - CapturingPGDriver.seenUrls.clear() - CapturingPGDriver.seenProps.clear() - CapturingPGDriver.readOnlyCalls.clear() - } // --------------------------------------------------------------------------- - // URL composition — pin the exact JDBC URL the driver receives + // URL composition - pin the exact JDBC URL // --------------------------------------------------------------------------- - "PostgreSQLConnUtil.connect" should - "build a JDBC URL of the form jdbc:postgresql://{host}:{port}/{database}" in { - clearCapture() - val conn = PostgreSQLConnUtil.connect("host-a", "5432", "db-a", "u", "p") - assert(conn != null) - assert(CapturingPGDriver.seenUrls.size == 1) - assert(CapturingPGDriver.seenUrls.head == "jdbc:postgresql://host-a:5432/db-a") + "PostgreSQLConnUtil.buildJdbcUrl" should + "build a URL of the form jdbc:postgresql://{host}:{port}/{database}" in { + assert( + PostgreSQLConnUtil.buildJdbcUrl( + "host-a", + "5432", + "db-a" + ) == "jdbc:postgresql://host-a:5432/db-a" + ) } it should "interpolate distinct host/port/database values into the URL" in { - clearCapture() - PostgreSQLConnUtil.connect("h-1", "1234", "d-1", "u", "p") - assert(CapturingPGDriver.seenUrls.head == "jdbc:postgresql://h-1:1234/d-1") - clearCapture() - PostgreSQLConnUtil.connect("h-2", "9999", "d-2", "u", "p") - assert(CapturingPGDriver.seenUrls.head == "jdbc:postgresql://h-2:9999/d-2") + assert( + PostgreSQLConnUtil.buildJdbcUrl("h-1", "1234", "d-1") == "jdbc:postgresql://h-1:1234/d-1" + ) + assert( + PostgreSQLConnUtil.buildJdbcUrl("h-2", "9999", "d-2") == "jdbc:postgresql://h-2:9999/d-2" + ) } it should "place host BEFORE port (host-then-port, not port-then-host)" in { - clearCapture() - PostgreSQLConnUtil.connect("a", "1", "x", "u", "p") - val url = CapturingPGDriver.seenUrls.head + val url = PostgreSQLConnUtil.buildJdbcUrl("a", "1", "x") assert(url.contains("//a:1/"), s"expected //a:1/ ordering, got: $url") assert(!url.contains("//1:a/"), s"port-then-host ordering must NOT appear, got: $url") } it should "use the `postgresql` JDBC subprotocol (not e.g. `mysql`)" in { - clearCapture() - PostgreSQLConnUtil.connect("h", "5432", "db", "u", "p") - val url = CapturingPGDriver.seenUrls.head + val url = PostgreSQLConnUtil.buildJdbcUrl("h", "5432", "db") assert(url.startsWith("jdbc:postgresql://")) assert(!url.contains("jdbc:mysql:")) } it should "accept an empty database name and still produce a well-formed URL" in { - clearCapture() - PostgreSQLConnUtil.connect("h", "5432", "", "u", "p") // The resulting `jdbc:postgresql://h:5432/` is well-formed even if a // real driver would reject it. - assert(CapturingPGDriver.seenUrls.head == "jdbc:postgresql://h:5432/") - } - - // --------------------------------------------------------------------------- - // Credentials propagation - // --------------------------------------------------------------------------- - - it should "pass username and password through DriverManager properties" in { - clearCapture() - PostgreSQLConnUtil.connect("h", "5432", "db", "the-user", "the-pass") - val props = CapturingPGDriver.seenProps.head - assert(props.getProperty("user") == "the-user") - assert(props.getProperty("password") == "the-pass") - } - - // --------------------------------------------------------------------------- - // setReadOnly(true) — pinned via the captured proxy - // --------------------------------------------------------------------------- - - it should "flip the returned Connection to read-only (query-efficiency contract)" in { - clearCapture() - PostgreSQLConnUtil.connect("h", "5432", "db", "u", "p") - assert(CapturingPGDriver.readOnlyCalls == ArrayBuffer(true)) - } - - // --------------------------------------------------------------------------- - // SQLException propagation when the driver throws — pin the @throws contract - // --------------------------------------------------------------------------- - - it should "propagate SQLException when the driver throws" in { - // Swap in a one-shot throwing override of `connect`. We can't mutate - // CapturingPGDriver in-place, so register a higher-priority throwing - // driver and remove it after. - val throwingDriver = new Driver { - override def acceptsURL(url: String): Boolean = - url != null && url.startsWith("jdbc:postgresql:") - // Follow the JDBC contract: return `null` if the URL is not ours, - // then throw only on a matching URL. A future refactor that calls - // `DriverManager.getConnection` with a different scheme while - // this driver is registered would otherwise see a spurious throw. - override def connect(url: String, info: Properties): Connection = { - if (!acceptsURL(url)) return null - throw new SQLException("forced-fail-for-test") - } - override def getPropertyInfo(url: String, info: Properties) = Array.empty[DriverPropertyInfo] - override def getMajorVersion: Int = 99 - override def getMinorVersion: Int = 0 - override def jdbcCompliant(): Boolean = false - override def getParentLogger: Logger = Logger.getLogger("test-pg-throwing") - } - DriverManager.deregisterDriver(CapturingPGDriver) - DriverManager.registerDriver(throwingDriver) - try { - val ex = intercept[SQLException] { - PostgreSQLConnUtil.connect("h", "5432", "db", "u", "p") - } - assert(ex.getMessage.contains("forced-fail-for-test")) - } finally { - DriverManager.deregisterDriver(throwingDriver) - DriverManager.registerDriver(CapturingPGDriver) - } + assert(PostgreSQLConnUtil.buildJdbcUrl("h", "5432", "") == "jdbc:postgresql://h:5432/") } }
