From e8c2839582a2e6d0f147015850eeeebb12d269ec Mon Sep 17 00:00:00 2001 From: Roman Langolf Date: Sat, 12 Oct 2024 18:08:10 +0700 Subject: [PATCH] update search_path with warning, add tests --- README.md | 12 +++---- build.sbt | 2 +- .../shared/src/main/scala/dumbo/Dumbo.scala | 25 +++++++------ .../src/test/scala/DumboMigrationSpec.scala | 35 ++++++++++++++----- 4 files changed, 48 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index d151846..6944a0b 100644 --- a/README.md +++ b/README.md @@ -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, ) diff --git a/build.sbt b/build.sbt index e267034..d46ee3b 100644 --- a/build.sbt +++ b/build.sbt @@ -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`) diff --git a/modules/core/shared/src/main/scala/dumbo/Dumbo.scala b/modules/core/shared/src/main/scala/dumbo/Dumbo.scala index c1ad3a4..9191d3e 100644 --- a/modules/core/shared/src/main/scala/dumbo/Dumbo.scala +++ b/modules/core/shared/src/main/scala/dumbo/Dumbo.scala @@ -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 => diff --git a/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala b/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala index be80eb4..747323f 100644 --- a/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala +++ b/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala @@ -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 () }