Skip to content

Commit

Permalink
update search_path with warning, add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
rolang committed Oct 16, 2024
1 parent 77ca4f4 commit e8c2839
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 26 deletions.
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,23 +247,23 @@ dumboWithResouces.withMigrationStateLogAfter[IO](5.seconds)(
)

// in some cases you may want to provide a custom skunk session instead of connection config
// NOTE: given a connection config, dumbo will construct a session that will include provided schemas into the search path
// if you want to achieve the same with a custom session,
// then you need to add it to the search_path parameter of the session yourself
// NOTE: given a connection config, dumbo will construct a session that will include given schemas into the search path via session parameters
// for custom sessions the search_path will be updated via the SET command if it doesn't match given schemas config which is not recommended (see https://typelevel.org/skunk/reference/Sessions.html#session-parameters)
// you may consider adding the search_path to the parameters yourself in that case, dumbo will log it as a warning
dumboWithResouces.bySession(
defaultSchema = "schema_1",
defaultSchema = "schema_1", // consider updating the search_path for non default schema settings
sessionResource = skunk.Session.single[IO](
host = "localhost",
port = 5432,
user = "postgres",
database = "postgres",
password = Some("postgres"),
// add schemas to the search path if desired
// add schemas to the search path
// those are added by default when using a dumbo.ConnectionConfig
parameters = skunk.Session.DefaultConnectionParameters ++ Map(
"search_path" -> "schema_1"
),
// a strategy other than BuiltinsOnly should not be reuired for running migrations
// a strategy other than BuiltinsOnly should not be required for running migrations
// you may want to keep that
strategy = skunk.util.Typer.Strategy.BuiltinsOnly,
)
Expand Down
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import scala.scalanative.build.*
lazy val `scala-2.13` = "2.13.15"
lazy val `scala-3` = "3.3.4"

ThisBuild / tlBaseVersion := "0.4"
ThisBuild / tlBaseVersion := "0.5"
ThisBuild / startYear := Some(2023)
ThisBuild / scalaVersion := `scala-3`
ThisBuild / crossScalaVersions := Seq(`scala-3`, `scala-2.13`)
Expand Down
25 changes: 14 additions & 11 deletions modules/core/shared/src/main/scala/dumbo/Dumbo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -356,18 +356,21 @@ class Dumbo[F[_]: Sync: Console](
def runMigration: F[MigrationResult] = sessionResource.use(migrateBySession)

private def migrateBySession(session: Session[F]): F[Dumbo.MigrationResult] = for {
dbVersion <- session.unique(sql"SELECT version()".query(text))
searchPath <- session.unique(sql"SHOW search_path".query(text)).map(_.split(",").map(_.trim).toSet)
_ <- allSchemas.filterNot(searchPath.contains(_)).toList match {
case Nil => ().pure[F]
case missing =>
Console[F].println(
s"""|WARNING: Following schemas are not included in the search path: ${missing.mkString(", ")}.\n
|This may happen if you provide a custom session.\n
|If you want to include them in the search path, you need to add it to the "search_path" parameter of the session.""".stripMargin
)
// verify search_path
_ <- session.unique(sql"SHOW search_path".query(text)).flatMap { sp =>
val spSchemas = sp.split(",").map(_.trim).toSet
allSchemas.filterNot(spSchemas.contains(_)).toList match {
case Nil => ().pure[F]
case missing =>
val newSearchPath = allSchemas.mkString(",")
Console[F].println(
s"""|WARNING: Following schemas are not included in the search path '$sp': ${missing.mkString(", ")}.
|The search_path will be set to '${newSearchPath}'. Consider adding it to session parameters instead.""".stripMargin
) >> session.execute(sql"SET search_path = #${newSearchPath}".command).void
}
}
_ <- Console[F].println(s"Starting migration on $dbVersion")
dbVersion <- session.unique(sql"SELECT version()".query(text))
_ <- Console[F].println(s"Starting migration on $dbVersion")
schemaRes <-
allSchemas.toList
.flatTraverse(schema =>
Expand Down
35 changes: 27 additions & 8 deletions modules/tests/shared/src/test/scala/DumboMigrationSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -170,22 +170,41 @@ trait DumboMigrationSpec extends ffstest.FTest {

dbTest("warn if schemas are not included in the search path for custom sessions") {
val withResources = dumboWithResources("db/test_search_path")
val schemas = List("schema_1")
val schemas = List("schema_1", "schema_2")
val testConsole = new TestConsole
val warningMsg = "WARNING: Following schemas are not included in the search path: schema_1"
val hasWarning = (m: String) =>
m.contains(
"""WARNING: Following schemas are not included in the search path '"$user", public': schema_1, schema_2"""
) &&
m.contains("""The search_path will be set to 'schema_1,schema_2'""")

def migrateBySession(params: Map[String, String] = Map.empty) =
dumboMigrateWithSession(schemas.head, withResources, session(params))(testConsole).attempt
dumboMigrateWithSession(schemas.head, withResources, session(params), schemas.tail)(testConsole).attempt

for {
dumboRes <- migrateBySession()
_ = assert(dumboRes.isLeft)
_ = assert(testConsole.logs.get().exists(_.contains(warningMsg)))
_ = testConsole.flush()
_ = assert(dumboRes.isRight)
_ = assert(testConsole.logs.get().exists(hasWarning))
// reset
_ <- dropSchemas
_ = testConsole.flush()
// succeed without warning if search_path is set
dumboResB <- migrateBySession(Map("search_path" -> "schema_1"))
dumboResB <- migrateBySession(Map("search_path" -> "schema_1,schema_2"))
_ = assert(dumboResB.isRight)
_ = assert(!testConsole.logs.get().exists(_.contains(warningMsg)))
_ = assert(!testConsole.logs.get().exists(hasWarning))
} yield ()
}

dbTest("migrate by different schema using custom session") {
val withResources = dumboWithResources("db/test_1")
val schemaA = "test_a"
val schemaB = "test_b"

for {
resDumboA <- dumboMigrateWithSession(schemaA, withResources, session())
resDumboB <- dumboMigrateWithSession(schemaB, withResources, session())
_ = assertEquals(resDumboA.migrationsExecuted, 4)
_ = assertEquals(resDumboB.migrationsExecuted, 4)
} yield ()
}

Expand Down

0 comments on commit e8c2839

Please sign in to comment.