Merge pull request #1745 from sideeffffect/refactor-hikari-config

Refactor Hikari config

modules/core/src/main/scala/doobie/util/transactor.scala

@@ -302,11 +302,7 @@ object transactor  {
         private def logHandler(implicit A: Applicative[M]): LogHandlerM[M] = maybeLogHandler.getOrElse(LogHandlerM.noop)
 
         def apply[A <: DataSource](dataSource: A, connectEC: ExecutionContext)(implicit ev: Async[M]): Transactor.Aux[M, A] = {
-          val connect = (dataSource: A) => {
-            val acquire = ev.evalOn(ev.delay(dataSource.getConnection()), connectEC)
-            def release(c: Connection) = ev.blocking(c.close())
-            Resource.make(acquire)(release)(ev)
-          }
+          val connect = (dataSource: A) => Resource.fromAutoCloseable(ev.evalOn(ev.delay(dataSource.getConnection()), connectEC))
           val interp  = KleisliInterpreter[M](logHandler).ConnectionInterpreter
           Transactor(dataSource, connect, interp, Strategy.default)
         }
@@ -358,11 +354,7 @@ object transactor  {
       )(implicit ev: Async[M]): Transactor.Aux[M, Unit] =
         Transactor(
           (),
-          _ => {
-            val acquire = ev.blocking{ Class.forName(driver); conn() }
-            def release(c: Connection) = ev.blocking{ c.close() }
-            Resource.make(acquire)(release)(ev)
-          },
+          _ => Resource.fromAutoCloseable(ev.blocking{ Class.forName(driver); conn() }),
           KleisliInterpreter[M](logHandler).ConnectionInterpreter,
           strategy
         )

modules/hikari/src/main/scala/doobie/hikari/Config.scala

@@ -21,13 +21,14 @@ import scala.concurrent.duration.Duration
   * which in turn is used to create `doobie.hikari.HikariTransactor`.
   * See the method `HikariTransactor.fromConfigAutoEc` */
 final case class Config(
+  jdbcUrl: String,
   catalog: Option[String] = None,
   connectionTimeout: Duration = Duration(30, TimeUnit.SECONDS),
   idleTimeout: Duration = Duration(10, TimeUnit.MINUTES),
   leakDetectionThreshold: Duration = Duration.Zero,
-  maximumPoolSize: Option[Int] = Some(10),
+  maximumPoolSize: Int = 10,
   maxLifetime: Duration = Duration(30, TimeUnit.MINUTES),
-  minimumIdle: Option[Int] = Some(10),
+  minimumIdle: Int = 10,
   password: Option[String] = None,
   poolName: Option[String] = None,
   username: Option[String] = None,
@@ -41,7 +42,6 @@ final case class Config(
   driverClassName: Option[String] = None,
   initializationFailTimeout: Duration = Duration(1, TimeUnit.MILLISECONDS),
   isolateInternalQueries: Boolean = false,
-  jdbcUrl: Option[String] = None,
   readOnly: Boolean = false,
   registerMbeans: Boolean = false,
   schema: Option[String] = None,
@@ -63,13 +63,15 @@ object Config {
     F.delay {
       val c = new HikariConfig()
 
+      c.setJdbcUrl(config.jdbcUrl)
+
       config.catalog.foreach(c.setCatalog)
       c.setConnectionTimeout(config.connectionTimeout.toMillis)
       c.setIdleTimeout(config.idleTimeout.toMillis)
       c.setLeakDetectionThreshold(config.leakDetectionThreshold.toMillis)
-      config.maximumPoolSize.foreach(c.setMaximumPoolSize)
+      c.setMaximumPoolSize(config.maximumPoolSize)
       c.setMaxLifetime(config.maxLifetime.toMillis)
-      config.minimumIdle.foreach(c.setMinimumIdle)
+      c.setMinimumIdle(config.minimumIdle)
       config.password.foreach(c.setPassword)
       config.poolName.foreach(c.setPoolName)
       config.username.foreach(c.setUsername)
@@ -84,7 +86,6 @@ object Config {
       config.driverClassName.foreach(c.setDriverClassName)
       c.setInitializationFailTimeout(config.initializationFailTimeout.toMillis)
       c.setIsolateInternalQueries(config.isolateInternalQueries)
-      config.jdbcUrl.foreach(c.setJdbcUrl)
       c.setReadOnly(config.readOnly)
       c.setRegisterMbeans(config.registerMbeans)
       config.schema.foreach(c.setSchema)

modules/hikari/src/main/scala/doobie/hikari/HikariTransactor.scala

@@ -26,24 +26,18 @@ object HikariTransactor {
   ): HikariTransactor[M] =
     Transactor.fromDataSource[M](hikariDataSource, connectEC)
 
-  private def createDataSourceResource[M[_]: Sync](factory: => HikariDataSource): Resource[M, HikariDataSource] = {
-    val alloc = Sync[M].delay(factory)
-    val free = (ds: HikariDataSource) => Sync[M].delay(ds.close())
-    Resource.make(alloc)(free)
-  }
-
   /** Resource yielding an unconfigured `HikariTransactor`. */
   def initial[M[_]: Async](
     connectEC: ExecutionContext
   ): Resource[M, HikariTransactor[M]] = {
-    createDataSourceResource(new HikariDataSource)
+    Resource.fromAutoCloseable(Sync[M].delay(new HikariDataSource))
       .map(Transactor.fromDataSource[M](_, connectEC))
   }
 
   /** Resource yielding a new `HikariTransactor` configured with the given Config.
-   * Unless you have a good reason, consider using `fromConfigAutoEc` which creates the `connectEC` for you.
+   * Unless you have a good reason, consider using `fromConfig` which creates the `connectEC` for you.
    */
-  def fromConfig[M[_]: Async](
+  def fromConfigCustomEc[M[_]: Async](
     config: Config,
     connectEC: ExecutionContext,
     dataSource: Option[DataSource] = None,
@@ -75,7 +69,7 @@ object HikariTransactor {
   /** Resource yielding a new `HikariTransactor` configured with the given Config.
    * The `connectEC` is created automatically, with the same size as the Hikari pool.
    */
-  def fromConfigAutoEc[M[_]: Async](
+  def fromConfig[M[_]: Async](
       config: Config,
       dataSource: Option[DataSource] = None,
       dataSourceProperties: Option[Properties] = None,
@@ -109,25 +103,28 @@ object HikariTransactor {
   def fromHikariConfig[M[_]: Async](
     hikariConfig: HikariConfig,
     connectEC: ExecutionContext
-  ): Resource[M, HikariTransactor[M]] = {
-    createDataSourceResource(new HikariDataSource(hikariConfig))
-      .map(Transactor.fromDataSource[M](_, connectEC))
-  }
+  ): Resource[M, HikariTransactor[M]] = Resource
+    .fromAutoCloseable(Sync[M].delay(new HikariDataSource(hikariConfig)))
+    .map(Transactor.fromDataSource[M](_, connectEC))
 
   /** Resource yielding a new `HikariTransactor` configured with the given HikariConfig.
    * The `connectEC` is created automatically, with the same size as the Hikari pool.
    */
   def fromHikariConfig[M[_]: Async](hikariConfig: HikariConfig): Resource[M, HikariTransactor[M]] =
   for {
-    _ <- Sync[M].delay(hikariConfig.validate()).toResource // to populate unset fields with default values, like `maximumPoolSize`
-    // Also note that the number of JDBC connections is usually limited by the underlying JDBC pool. You may therefore want to limit your connection pool to the same size as the underlying JDBC pool as any additional threads are guaranteed to be blocked.
+    // to populate unset fields with default values, like `maximumPoolSize`
+    _ <- Sync[M].delay(hikariConfig.validate()).toResource
+    // Note that the number of JDBC connections is usually limited by the underlying JDBC pool.
+    // You may therefore want to limit your connection pool to the same size as the underlying JDBC pool
+    // as any additional threads are guaranteed to be blocked.
     // https://tpolecat.github.io/doobie/docs/14-Managing-Connections.html#about-threading
     connectEC <- ExecutionContexts.fixedThreadPool(hikariConfig.getMaximumPoolSize)
-    result <- createDataSourceResource(new HikariDataSource(hikariConfig))
-      .map(Transactor.fromDataSource[M](_, connectEC))
+    result <- fromHikariConfig(hikariConfig, connectEC)
   } yield result
 
-  /** Resource yielding a new `HikariTransactor` configured with the given info. */
+  /** Resource yielding a new `HikariTransactor` configured with the given info.
+   * Consider using `fromConfig` for better configurability.
+   */
   def newHikariTransactor[M[_]: Async](
     driverClassName: String,
     url:             String,

modules/hikari/src/test/scala/doobie/hikari/ConfigSpec.scala

@@ -13,11 +13,16 @@ class ConfigSpec extends munit.FunSuite {
 
     import cats.effect.unsafe.implicits.global
 
-    val actual = Config.makeHikariConfig[IO](Config(jdbcUrl = Some("jdbcUrl"), poolName = Some("poolName"))).unsafeRunSync()
-    val expected = new HikariConfig()
-    expected.setJdbcUrl("jdbcUrl") // mandatory argument
-    expected.setPoolName("poolName") // otherwise the pool name is generated
-    expected.validate()
+    val actual = Config.makeHikariConfig[IO](Config("jdbcUrl", poolName = Some("poolName"))).unsafeRunSync()
+    val expected = {
+      val c = new HikariConfig()
+      c.setJdbcUrl("jdbcUrl") // mandatory argument
+      c.setPoolName("poolName") // otherwise the pool name is generated
+      c.validate()
+      c
+    }
+
+    assertEquals(actual.getJdbcUrl, expected.getJdbcUrl)
 
     assertEquals(actual.getCatalog, expected.getCatalog)
     assertEquals(actual.getConnectionTimeout, expected.getConnectionTimeout)
@@ -40,7 +45,6 @@ class ConfigSpec extends munit.FunSuite {
     assertEquals(actual.getDriverClassName, expected.getDriverClassName)
     assertEquals(actual.getInitializationFailTimeout, expected.getInitializationFailTimeout)
     assertEquals(actual.isIsolateInternalQueries, expected.isIsolateInternalQueries)
-    assertEquals(actual.getJdbcUrl, expected.getJdbcUrl)
     assertEquals(actual.isReadOnly, expected.isReadOnly)
     assertEquals(actual.isRegisterMbeans, expected.isRegisterMbeans)
     assertEquals(actual.getSchema, expected.getSchema)