Merge pull request #2739 from hongwei1/feature/botswanaBranch

Feature/Unified Doobie's connection management with Lift

Author: simonredfern

obp-api/src/main/scala/code/api/util/DoobieTransactor.scala

@@ -1,11 +1,13 @@
 package code.api.util
 
-import cats.effect.{IO, Resource}
+import cats.effect.IO
 import cats.effect.unsafe.implicits.global
-import com.zaxxer.hikari.HikariConfig
+import code.util.Helper.MdcLoggable
 import doobie._
-import doobie.hikari.HikariTransactor
 import doobie.implicits._
+import doobie.util.transactor.Strategy
+import net.liftweb.common.Full
+import net.liftweb.db.DB
 
 import scala.concurrent.{ExecutionContext, Future}
 
@@ -16,16 +18,22 @@ import scala.concurrent.{ExecutionContext, Future}
  * This handles all JDBC types correctly, including SQL Server's NVARCHAR (type -9)
  * which Lift's DB.runQuery doesn't handle.
  *
- * IMPORTANT: This uses a SEPARATE HikariCP connection pool from Lift's pool.
- * This ensures complete isolation and safety - Doobie manages its own connections
- * without any interference with Lift's connection management.
+ * TRANSACTION UNIFICATION:
+ * When called within a Lift HTTP request context, Doobie uses the SAME Connection
+ * that Lift is holding for the current request transaction (via Transactor.fromConnection).
+ * This means Doobie queries participate in Lift's transaction boundary:
+ * - Same connection, same transaction, same commit/rollback
+ * - Doobie can see uncommitted Lift writes (same session)
+ * - If Lift rolls back, Doobie's operations are also rolled back
+ *
+ * When called outside a Lift request context (e.g., background tasks, schedulers),
+ * falls back to Lift's shared HikariCP connection pool via Transactor.fromDataSource.
  *
  * Benefits over DBUtil.runQuery:
  * - Type-safe query results via case classes
  * - Type-safe parameters (no SQL injection risk)
  * - Proper JDBC type handling for all databases
  * - Composable queries using cats-effect IO
- * - Complete isolation from Lift's connection pool
  *
  * Usage:
  * {{{
@@ -45,96 +53,99 @@ import scala.concurrent.{ExecutionContext, Future}
  * val result: List[TopApi] = DoobieUtil.runQuery(query)
  * }}}
  */
-object DoobieUtil {
+object DoobieUtil extends MdcLoggable {
 
   /**
-   * Lazy-initialized HikariCP transactor for Doobie.
-   *
-   * This creates a separate connection pool from Lift's pool, ensuring
-   * complete isolation and safety. The pool is initialized on first use
-   * and kept alive for the lifetime of the application.
+   * Fallback transactor that shares Lift's HikariCP connection pool.
+   * Used when no Lift request context is available (background tasks, schedulers).
+   * Strategy.void: Doobie will not call setAutoCommit/commit/rollback.
    */
-  private lazy val transactor: Transactor[IO] = {
-    val (dbUrl, dbUser, dbPassword) = DBUtil.getDbConnectionParameters
-
-    val hikariConfig = new HikariConfig()
-    hikariConfig.setJdbcUrl(dbUrl)
-    hikariConfig.setUsername(dbUser)
-    hikariConfig.setPassword(dbPassword)
-
-    // Pool configuration - conservative settings for safety
-    hikariConfig.setPoolName("doobie-pool")
-    hikariConfig.setMaximumPoolSize(
-      APIUtil.getPropsAsIntValue("doobie.hikari.maximumPoolSize", 10)
-    )
-    hikariConfig.setMinimumIdle(
-      APIUtil.getPropsAsIntValue("doobie.hikari.minimumIdle", 2)
-    )
-    hikariConfig.setConnectionTimeout(
-      APIUtil.getPropsAsLongValue("doobie.hikari.connectionTimeout", 30000L)
-    )
-    hikariConfig.setIdleTimeout(
-      APIUtil.getPropsAsLongValue("doobie.hikari.idleTimeout", 600000L)
-    )
-    hikariConfig.setMaxLifetime(
-      APIUtil.getPropsAsLongValue("doobie.hikari.maxLifetime", 1800000L)
+  private lazy val fallbackTransactor: Transactor[IO] = {
+    val liftDataSource = APIUtil.vendor.HikariDatasource.ds
+    logger.info("DoobieUtil: Initialized fallback transactor sharing Lift's HikariCP pool")
+    val xa = Transactor.fromDataSource[IO].apply(
+      liftDataSource,
+      ExecutionContext.global
     )
+    xa.copy(strategy0 = Strategy.void)
+  }
 
-    // Set driver class based on database type
-    if (dbUrl.contains("sqlserver")) {
-      hikariConfig.setDriverClassName("com.microsoft.sqlserver.jdbc.SQLServerDriver")
-    } else if (dbUrl.contains("postgresql")) {
-      hikariConfig.setDriverClassName("org.postgresql.Driver")
-    } else if (dbUrl.contains("mysql")) {
-      hikariConfig.setDriverClassName("com.mysql.cj.jdbc.Driver")
-    } else if (dbUrl.contains("h2")) {
-      hikariConfig.setDriverClassName("org.h2.Driver")
-    } else if (dbUrl.contains("oracle")) {
-      hikariConfig.setDriverClassName("oracle.jdbc.OracleDriver")
-    }
-    // For other databases, HikariCP will try to auto-detect the driver
+  /**
+   * Create a transactor that wraps an existing JDBC Connection.
+   * Strategy.void ensures Doobie does not interfere with Lift's transaction management.
+   */
+  private def transactorFromConnection(conn: java.sql.Connection): Transactor[IO] = {
+    val xa = Transactor.fromConnection[IO].apply(conn, None)
+    xa.copy(strategy0 = Strategy.void)
+  }
 
-    // Create the transactor - this allocates the pool immediately
-    // We use unsafeRunSync here because we need a stable, long-lived transactor
-    HikariTransactor.fromHikariConfig[IO](hikariConfig)
-      .allocated
-      .map(_._1)  // Get the transactor, discard the finalizer (pool lives for app lifetime)
-      .unsafeRunSync()
+  /**
+   * Try to get the current Lift request's Connection.
+   * Uses DB.currentConnection which peeks at the DynoVar without
+   * triggering reference counting or creating a new connection.
+   * Returns Some(connection) if inside a Lift HTTP request context,
+   * None otherwise (background tasks, schedulers, tests without request context).
+   */
+  private def liftCurrentConnection: Option[java.sql.Connection] = {
+    // DB.currentConnection returns Box[SuperConnection]
+    // SuperConnection has implicit conversion to java.sql.Connection
+    DB.currentConnection match {
+      case Full(superConn) =>
+        val conn: java.sql.Connection = superConn.connection
+        if (!conn.isClosed) Some(conn) else None
+      case _ => None
+    }
   }
 
   /**
-   * Run a Doobie query synchronously using the dedicated Doobie connection pool.
+   * Run a Doobie query synchronously, sharing Lift's transaction when available.
    *
-   * This uses a completely separate HikariCP pool from Lift's pool,
-   * ensuring no interference between Doobie and Lift's connection management.
+   * When called within a Lift HTTP request context:
+   * - Uses the SAME Connection that Lift holds for the current request
+   * - Doobie query participates in Lift's transaction (same commit/rollback)
+   * - Can see uncommitted Lift writes (same database session)
+   *
+   * When called outside a Lift request context (background tasks, schedulers):
+   * - Falls back to Lift's shared HikariCP pool (separate connection)
    *
    * @param query The Doobie ConnectionIO query to execute
    * @return The query result
    */
   def runQuery[A](query: ConnectionIO[A]): A = {
-    query.transact(transactor).unsafeRunSync()
+    liftCurrentConnection match {
+      case Some(conn) =>
+        // Inside Lift request: use the same connection for transaction unification
+        query.transact(transactorFromConnection(conn)).unsafeRunSync()
+      case None =>
+        // Outside Lift request: fallback to shared pool
+        logger.debug("DoobieUtil.runQuery: No Lift request context, using fallback pool transactor")
+        query.transact(fallbackTransactor).unsafeRunSync()
+    }
   }
 
   /**
    * Run a Doobie query asynchronously, returning a Future.
+   * Note: async queries always use the fallback pool transactor because
+   * Lift's request connection may not be available on a different thread.
    *
    * @param query The Doobie ConnectionIO query to execute
    * @param ec ExecutionContext for the Future
    * @return Future containing the query result
    */
   def runQueryAsync[A](query: ConnectionIO[A])(implicit ec: ExecutionContext): Future[A] = {
-    query.transact(transactor).unsafeToFuture()
+    query.transact(fallbackTransactor).unsafeToFuture()
   }
 
   /**
    * Run a Doobie query and return an IO.
-   * Useful when you want to compose with other cats-effect operations.
+   * Note: IO queries always use the fallback pool transactor because
+   * the IO may be evaluated outside the Lift request context.
    *
    * @param query The Doobie ConnectionIO query to execute
    * @return IO containing the query result
    */
   def runQueryIO[A](query: ConnectionIO[A]): IO[A] = {
-    query.transact(transactor)
+    query.transact(fallbackTransactor)
   }
 
   /**