From ea1992333b29dcbc243a5112aa75ba38a35fc520 Mon Sep 17 00:00:00 2001 From: Roman Langolf Date: Sun, 13 Oct 2024 16:13:28 +0700 Subject: [PATCH] ensure default schema appears first in search_path, add tests --- .../shared/src/main/scala/dumbo/Dumbo.scala | 29 ++++++++-------- .../src/main/scala/ffstest/FFramework.scala | 5 ++- .../db/test_default_schema/V1__test.sql | 1 + .../src/test/scala/DumboMigrationSpec.scala | 33 +++++++++++++++++++ 4 files changed, 54 insertions(+), 14 deletions(-) create mode 100644 modules/tests/shared/src/test/resources/db/test_default_schema/V1__test.sql diff --git a/modules/core/shared/src/main/scala/dumbo/Dumbo.scala b/modules/core/shared/src/main/scala/dumbo/Dumbo.scala index f789723..408758d 100644 --- a/modules/core/shared/src/main/scala/dumbo/Dumbo.scala +++ b/modules/core/shared/src/main/scala/dumbo/Dumbo.scala @@ -142,7 +142,7 @@ final class DumboWithResourcesPartiallyApplied[F[_]](reader: ResourceReader[F]) defaultSchema: String, schemas: Set[String], )(implicit T: Temporal[F], C: Console[F], TRC: Tracer[F], N: Network[F]) = { - val searchPath = (schemas + defaultSchema).mkString(",") + val searchPath = Dumbo.toSearchPath(defaultSchema, schemas) val params = Session.DefaultConnectionParameters ++ Map("search_path" -> searchPath) Session.single[F]( @@ -173,7 +173,7 @@ class Dumbo[F[_]: Sync: Console]( ) { import Dumbo.* - private[dumbo] val allSchemas = Set(defaultSchema) ++ schemas + private[dumbo] val allSchemas = defaultSchema :: schemas.toList private[dumbo] val historyTable = s"${defaultSchema}.${schemaHistoryTable}" private val dumboHistory = History(historyTable) @@ -350,27 +350,26 @@ class Dumbo[F[_]: Sync: Console]( // 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 { + allSchemas.diff(spSchemas.toList) match { case Nil => ().pure[F] case missing => - val newSearchPath = allSchemas.mkString(",") + val newSearchPath = toSearchPath(defaultSchema, schemas) 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 + ) >> session.execute(sql"SET search_path TO #${newSearchPath}".command).void } } dbVersion <- session.unique(sql"SELECT version()".query(text)) _ <- Console[F].println(s"Starting migration on $dbVersion") schemaRes <- - allSchemas.toList - .flatTraverse(schema => - session.execute(initSchemaCmd(schema)).attempt.map { - case Right(Completion.CreateSchema) => List(schema) - case Left(e: skunk.exception.PostgresErrorException) if duplicateErrorCodes.contains(e.code) => Nil - case _ => Nil - } - ) + allSchemas.flatTraverse(schema => + session.execute(initSchemaCmd(schema)).attempt.map { + case Right(Completion.CreateSchema) => List(schema) + case Left(e: skunk.exception.PostgresErrorException) if duplicateErrorCodes.contains(e.code) => Nil + case _ => Nil + } + ) _ <- session.execute(dumboHistory.createTableCommand).void.recover { case e: skunk.exception.PostgresErrorException if duplicateErrorCodes.contains(e.code) => () } @@ -569,4 +568,8 @@ object Dumbo extends internal.DumboPlatform { .compile .drain } yield crc32.getValue().toInt + + // need to ensure that default schema appears first in the search_path + private[dumbo] def toSearchPath(defaultSchema: String, schemas: Set[String]) = + (defaultSchema :: schemas.toList).mkString(",") } diff --git a/modules/tests/shared/src/main/scala/ffstest/FFramework.scala b/modules/tests/shared/src/main/scala/ffstest/FFramework.scala index 0a1306e..b8685bb 100644 --- a/modules/tests/shared/src/main/scala/ffstest/FFramework.scala +++ b/modules/tests/shared/src/main/scala/ffstest/FFramework.scala @@ -27,7 +27,10 @@ trait FTest extends CatsEffectSuite with FTestPlatform { def dbTest(name: String)(f: => IO[Unit]): Unit = test(name)(dropSchemas >> f) - def someSchemaName = s"schema_${Random.alphanumeric.take(10).mkString}" + def someSchemaName: String = { + val chars = "abcdefghijklmnopqrstuvwxyz" + LazyList.continually(chars.charAt(Random.nextInt(chars.length))).take(15).mkString + } lazy val connectionConfig: ConnectionConfig = ConnectionConfig( host = "localhost", diff --git a/modules/tests/shared/src/test/resources/db/test_default_schema/V1__test.sql b/modules/tests/shared/src/test/resources/db/test_default_schema/V1__test.sql new file mode 100644 index 0000000..69bd5cb --- /dev/null +++ b/modules/tests/shared/src/test/resources/db/test_default_schema/V1__test.sql @@ -0,0 +1 @@ +CREATE TABLE test_default_schema(); \ No newline at end of file diff --git a/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala b/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala index 747323f..050779d 100644 --- a/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala +++ b/modules/tests/shared/src/test/scala/DumboMigrationSpec.scala @@ -9,6 +9,8 @@ import scala.concurrent.duration.* import cats.data.Validated.Invalid import cats.implicits.* import ffstest.TestConsole +import skunk.codec.all.* +import skunk.implicits.* trait DumboMigrationSpec extends ffstest.FTest { def db: Db @@ -208,6 +210,37 @@ trait DumboMigrationSpec extends ffstest.FTest { } yield () } + dbTest("default schema is used when no schema is specified in migration sripts") { + val withResources = dumboWithResources("db/test_default_schema") + + (1 to 5).toList.traverse_ { _ => + val schemaDefault = someSchemaName + val schemas = List.fill(scala.util.Random.nextInt(10))(someSchemaName) + + def assertDefaultSchemaHasTable = + session() + .use( + _.execute(sql"""|SELECT table_schema::text + |FROM information_schema.tables + |WHERE table_name = 'test_default_schema'""".stripMargin.query(text)) + ) + .map { schemas => + assertEquals(schemas, List(schemaDefault)) + } + + for { + _ <- dropSchemas + // migrate by connection config + _ <- dumboMigrate(schemaDefault, withResources, schemas) + _ <- assertDefaultSchemaHasTable + _ <- dropSchemas + // migrate by custom session + _ <- dumboMigrateWithSession(schemaDefault, withResources, session(), schemas) + _ <- assertDefaultSchemaHasTable + } yield () + } + } + { val withResources = dumboWithResources("db/test_long_running") def logMatch(s: String): Boolean = s.startsWith("Awaiting query with pid")