From 0202589c5e55b51f08060ef2b54c75e5f458a7b8 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Thu, 25 Jun 2020 15:41:08 -0400 Subject: [PATCH 01/19] Add reversed actions when using reverse --- .../slick/migration/api/TableMigration.scala | 2 +- .../scala/slick/migration/api/DbTest.scala | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/main/scala/slick/migration/api/TableMigration.scala b/src/main/scala/slick/migration/api/TableMigration.scala index de1a1d47..53151f8b 100644 --- a/src/main/scala/slick/migration/api/TableMigration.scala +++ b/src/main/scala/slick/migration/api/TableMigration.scala @@ -63,7 +63,7 @@ object TableMigration { override def sql = underlying.sql override def reverse = { val tm0 = underlying.modActions(_ => List.empty[Action]) - underlying.actions.foldLeft(tm0) { (tm, action) => + underlying.actions.reverse.foldLeft(tm0) { (tm, action) => action match { case Action.CreateTable => tm.modActions(Action.DropTable :: _) case Action.RenameTableTo(to) => tm.modActions(Action.RenameTableFrom(to) :: _) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index e23ddc5f..df1ae3c2 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -245,6 +245,25 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) runMigration(tm.drop) } } + + test("reverse") { + val tm = TableMigration(TestTable) + + val migration = tm + .create + .addColumns(_.id) + .addPrimaryKeys(_.compoundPK) + + assert(migration.actions.head.sort == 0) + + runMigration(migration) + + val reversed = migration.reverse + + assert(reversed.actions.last.sort == 0) + + runMigration(reversed) + } } trait CompleteDbTest { this: DbTest[_ <: JdbcProfile] => From 8150610c6f7c91a22ea1edd16aac924f25211d64 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Thu, 25 Jun 2020 19:32:10 -0400 Subject: [PATCH 02/19] Fix tests --- src/main/scala/slick/migration/api/TableMigration.scala | 2 +- src/test/scala/slick/migration/api/DbTest.scala | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/scala/slick/migration/api/TableMigration.scala b/src/main/scala/slick/migration/api/TableMigration.scala index 53151f8b..de1a1d47 100644 --- a/src/main/scala/slick/migration/api/TableMigration.scala +++ b/src/main/scala/slick/migration/api/TableMigration.scala @@ -63,7 +63,7 @@ object TableMigration { override def sql = underlying.sql override def reverse = { val tm0 = underlying.modActions(_ => List.empty[Action]) - underlying.actions.reverse.foldLeft(tm0) { (tm, action) => + underlying.actions.foldLeft(tm0) { (tm, action) => action match { case Action.CreateTable => tm.modActions(Action.DropTable :: _) case Action.RenameTableTo(to) => tm.modActions(Action.RenameTableFrom(to) :: _) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index df1ae3c2..a6263e46 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -172,7 +172,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) try { val addOtherColumns = tm.addColumns(_.int1Nullable, _.strWithDefault) - assert(addOtherColumns.reverse == tm.dropColumns(_.strWithDefault, _.int1Nullable)) + assert(addOtherColumns.reverse == tm.dropColumns(_.int1Nullable, _.strWithDefault)) withBeforeAndAfter(addOtherColumns)(getColumns(TestTable)) { (before, after) => assert(before.length === 1) @@ -199,7 +199,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(i2(Some("INDEX1") -> false) === Vector(Some("INT1"))) assert(i2(Some("INDEX2") -> true) === Vector(Some("INT2"), Some("INT3"))) - assert(createIndexes.reverse == tm.dropIndexes(_.index2, _.index1)) + assert(createIndexes.reverse == tm.dropIndexes(_.index1, _.index2)) withBeforeAndAfter(createIndexes.reverse)(indexes) { (_, i4) => assert(i4.keys.flatMap(_._1).exists(Set("INDEX1", "INDEX2")) === false) @@ -336,7 +336,7 @@ trait CompleteDbTest { this: DbTest[_ <: JdbcProfile] => finally runMigration(tm.drop) - assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.int1, _.id)) + assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.id, _.int1)) } test("alterColumnTypes") { From d61b289f22e9c202af24ea53f78645292507ccc7 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Thu, 25 Jun 2020 19:36:37 -0400 Subject: [PATCH 03/19] Add reverse back --- src/main/scala/slick/migration/api/TableMigration.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/scala/slick/migration/api/TableMigration.scala b/src/main/scala/slick/migration/api/TableMigration.scala index de1a1d47..53151f8b 100644 --- a/src/main/scala/slick/migration/api/TableMigration.scala +++ b/src/main/scala/slick/migration/api/TableMigration.scala @@ -63,7 +63,7 @@ object TableMigration { override def sql = underlying.sql override def reverse = { val tm0 = underlying.modActions(_ => List.empty[Action]) - underlying.actions.foldLeft(tm0) { (tm, action) => + underlying.actions.reverse.foldLeft(tm0) { (tm, action) => action match { case Action.CreateTable => tm.modActions(Action.DropTable :: _) case Action.RenameTableTo(to) => tm.modActions(Action.RenameTableFrom(to) :: _) From d9537f0a29ca50abd32601c0dc2e464fb5dce6b5 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Thu, 25 Jun 2020 19:56:17 -0400 Subject: [PATCH 04/19] Reverse last/head --- src/test/scala/slick/migration/api/DbTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index a6263e46..b7d7aca7 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -254,13 +254,13 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) .addColumns(_.id) .addPrimaryKeys(_.compoundPK) - assert(migration.actions.head.sort == 0) + assert(migration.actions.last.sort == 0) runMigration(migration) val reversed = migration.reverse - assert(reversed.actions.last.sort == 0) + assert(reversed.actions.head.sort == 0) runMigration(reversed) } From 7001df37f0391a8be31273d6f193a2574115c7b2 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 10:25:46 -0400 Subject: [PATCH 05/19] Fix underlying sql reversal --- build.sbt | 2 +- .../scala/slick/migration/api/Dialect.scala | 7 +++-- .../slick/migration/api/TableMigration.scala | 9 +++--- .../scala/slick/migration/api/DbTest.scala | 30 +++++++++++-------- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/build.sbt b/build.sbt index d3b541ca..83bb3c26 100644 --- a/build.sbt +++ b/build.sbt @@ -21,7 +21,7 @@ libraryDependencies += "org.apache.derby" % "derby" % "10.11.1 libraryDependencies += "org.hsqldb" % "hsqldb" % "2.5.0" % "test" -libraryDependencies += "org.postgresql" % "postgresql" % "42.2.14" % "test" +libraryDependencies += "org.postgresql" % "postgresql" % "42.2.14" % "test" libraryDependencies += "mysql" % "mysql-connector-java" % "8.0.16" % "test" diff --git a/src/main/scala/slick/migration/api/Dialect.scala b/src/main/scala/slick/migration/api/Dialect.scala index 280a0eb4..ce8a0fcf 100644 --- a/src/main/scala/slick/migration/api/Dialect.scala +++ b/src/main/scala/slick/migration/api/Dialect.scala @@ -142,7 +142,7 @@ class Dialect[-P <: JdbcProfile] extends AstHelpers { toB.andThen(b => (b :: bs, as)).applyOrElse(a, (_: A) => (bs, a :: as)) } - def migrateTable(table: TableInfo, actions: List[TableMigration.Action]): List[String] = { + def migrateTable(table: TableInfo, actions: List[TableMigration.Action], reverse: Boolean = false): List[String] = { def loop(actions: List[TableMigration.Action]): List[String] = actions match { case Nil => Nil case CreateTable :: rest => @@ -181,7 +181,10 @@ class Dialect[-P <: JdbcProfile] extends AstHelpers { case RenameIndexFrom(currentInfo, from) :: rest => renameIndex(currentInfo.copy(name = from), currentInfo.name) ::: loop(rest) } - loop(actions.reverse.sortBy(_.sort)) + loop(actions.reverse.sortBy(_.sort)( + if (reverse) Ordering.Int.reverse + else Ordering.Int.min + )) } } diff --git a/src/main/scala/slick/migration/api/TableMigration.scala b/src/main/scala/slick/migration/api/TableMigration.scala index 53151f8b..ae5ad8fb 100644 --- a/src/main/scala/slick/migration/api/TableMigration.scala +++ b/src/main/scala/slick/migration/api/TableMigration.scala @@ -58,12 +58,13 @@ object TableMigration { } } - implicit class Reversible[T <: JdbcProfile#Table[_]](val underlying: TableMigration[T, Action.Reversible]) + implicit class Reversible[T <: JdbcProfile#Table[_], A <: Action](val underlying: TableMigration[T, Action.Reversible])( + implicit dialect: Dialect[_], withActions: TableMigration.WithActions[A]) extends ReversibleMigration with SqlMigration { - override def sql = underlying.sql + override def sql = dialect.migrateTable(underlying.tableInfo, underlying.actions, reverse = true) override def reverse = { val tm0 = underlying.modActions(_ => List.empty[Action]) - underlying.actions.reverse.foldLeft(tm0) { (tm, action) => + underlying.actions.foldLeft(tm0) { (tm, action) => action match { case Action.CreateTable => tm.modActions(Action.DropTable :: _) case Action.RenameTableTo(to) => tm.modActions(Action.RenameTableFrom(to) :: _) @@ -87,7 +88,7 @@ object TableMigration { override def toString = underlying.toString } - implicit def toReversible[T <: JdbcProfile#Table[_]]: ToReversible[TableMigration[T, Action.Reversible]] = + implicit def toReversible[T <: JdbcProfile#Table[_]](implicit dialect: Dialect[_]): ToReversible[TableMigration[T, Action.Reversible]] = new ToReversible[TableMigration[T, Action.Reversible]](self => new Reversible(self)) def apply[T <: JdbcProfile#Table[_]](table: T) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index b7d7aca7..3d65531a 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -172,7 +172,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) try { val addOtherColumns = tm.addColumns(_.int1Nullable, _.strWithDefault) - assert(addOtherColumns.reverse == tm.dropColumns(_.int1Nullable, _.strWithDefault)) + assert(addOtherColumns.reverse == tm.dropColumns(_.strWithDefault, _.int1Nullable)) withBeforeAndAfter(addOtherColumns)(getColumns(TestTable)) { (before, after) => assert(before.length === 1) @@ -199,7 +199,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(i2(Some("INDEX1") -> false) === Vector(Some("INT1"))) assert(i2(Some("INDEX2") -> true) === Vector(Some("INT2"), Some("INT3"))) - assert(createIndexes.reverse == tm.dropIndexes(_.index1, _.index2)) + assert(createIndexes.reverse == tm.dropIndexes(_.index2, _.index1)) withBeforeAndAfter(createIndexes.reverse)(indexes) { (_, i4) => assert(i4.keys.flatMap(_._1).exists(Set("INDEX1", "INDEX2")) === false) @@ -248,19 +248,25 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) test("reverse") { val tm = TableMigration(TestTable) + val createTable = tm.create.addColumns(_.id, _.strWithDefault) - val migration = tm - .create - .addColumns(_.id) - .addPrimaryKeys(_.compoundPK) - - assert(migration.actions.last.sort == 0) + assert(createTable.sql === List( + """create table "TEST_TABLE( + | "ID" BIGSERIAL NOT NULL PRIMARY KEY, "STR_WITH_DEFAULT" VARCHAR DEFAULT "abc" + |)""".stripMargin + )) - runMigration(migration) + runMigration(createTable) - val reversed = migration.reverse + val reversed = createTable.reverse - assert(reversed.actions.head.sort == 0) + assert(reversed.sql === List( + s"""alter table "TEST_TABLE" + | drop column "STR_WITH_DEFAULT"""".stripMargin, + s"""alter table "TEST_TABLE" + | drop column "ID"""".stripMargin, + s"""drop table "TEST_TABLE"""" + )) runMigration(reversed) } @@ -336,7 +342,7 @@ trait CompleteDbTest { this: DbTest[_ <: JdbcProfile] => finally runMigration(tm.drop) - assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.id, _.int1)) + assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.int1, _.id)) } test("alterColumnTypes") { From 5ddf20ee288c72bc5db817e6a33bcaa77e7586cf Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 10:45:16 -0400 Subject: [PATCH 06/19] Fix tests --- .../scala/slick/migration/api/DbTest.scala | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 3d65531a..0e98d8d4 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -250,23 +250,17 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) val tm = TableMigration(TestTable) val createTable = tm.create.addColumns(_.id, _.strWithDefault) - assert(createTable.sql === List( - """create table "TEST_TABLE( - | "ID" BIGSERIAL NOT NULL PRIMARY KEY, "STR_WITH_DEFAULT" VARCHAR DEFAULT "abc" - |)""".stripMargin - )) + assert(createTable.sql.size === 1) + assert(createTable.sql.head.contains("create table")) runMigration(createTable) val reversed = createTable.reverse - assert(reversed.sql === List( - s"""alter table "TEST_TABLE" - | drop column "STR_WITH_DEFAULT"""".stripMargin, - s"""alter table "TEST_TABLE" - | drop column "ID"""".stripMargin, - s"""drop table "TEST_TABLE"""" - )) + assert(reversed.sql.size === 3) + assert(reversed.sql.head.contains("STR_WITH_DEFAULT")) + assert(reversed.sql(1).contains("ID")) + assert(reversed.sql(2).contains("drop table")) runMigration(reversed) } From 4af4dd847500a3a108b9c7f582f9009e2b385d84 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 10:53:08 -0400 Subject: [PATCH 07/19] Fix tests again --- src/test/scala/slick/migration/api/DbTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 0e98d8d4..4d42c318 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -258,8 +258,8 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) val reversed = createTable.reverse assert(reversed.sql.size === 3) - assert(reversed.sql.head.contains("STR_WITH_DEFAULT")) - assert(reversed.sql(1).contains("ID")) + assert(reversed.sql.head.contains("ID")) + assert(reversed.sql(1).contains("STR_WITH_DEFAULT")) assert(reversed.sql(2).contains("drop table")) runMigration(reversed) From d37441bebdad83b798450f4c289a147fa7925ecb Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 13:37:25 -0400 Subject: [PATCH 08/19] Change test to work for all dbs --- src/test/scala/slick/migration/api/DbTest.scala | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 4d42c318..1db5bb1e 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -7,7 +7,7 @@ import scala.concurrent.Await import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.duration.Duration import scala.util.control.NonFatal -import slick.jdbc.{HsqldbProfile, JdbcProfile, OracleProfile} +import slick.jdbc.{HsqldbProfile, JdbcProfile, OracleProfile, PostgresProfile} import slick.jdbc.meta._ import slick.lifted.AbstractTable import com.typesafe.slick.testkit.util.JdbcTestDB @@ -246,7 +246,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) } } - test("reverse") { + test("reverse multiple columns") { val tm = TableMigration(TestTable) val createTable = tm.create.addColumns(_.id, _.strWithDefault) @@ -262,7 +262,17 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(reversed.sql(1).contains("STR_WITH_DEFAULT")) assert(reversed.sql(2).contains("drop table")) - runMigration(reversed) + try withBeforeAndAfter(reversed)(getTable(TestTable).asTry) { (before, after) => + // Only postgres can remove all columns from a table + if (this.profile.isInstanceOf[PostgresProfile]) { + assert(before.isSuccess) + assert(after.isSuccess) + } else { + assert(before.isSuccess) + assert(after.isFailure) + } + } + finally runMigration(tm.drop) } } From a3ad768c333bad7beefcf8f3ff0f219190712c73 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 13:44:52 -0400 Subject: [PATCH 09/19] Guard against everything except postgres --- src/test/scala/slick/migration/api/DbTest.scala | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 1db5bb1e..79fe754f 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -267,12 +267,15 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) if (this.profile.isInstanceOf[PostgresProfile]) { assert(before.isSuccess) assert(after.isSuccess) - } else { - assert(before.isSuccess) - assert(after.isFailure) } + } { + case ex: Throwable => + if (!this.profile.isInstanceOf[PostgresProfile]) { + runMigration(tm.drop) + } else { + throw ex; + } } - finally runMigration(tm.drop) } } From aa0e6017241ef5bae0e4edebbb1d1208a2b8c022 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 13:57:35 -0400 Subject: [PATCH 10/19] Catch everything else --- src/test/scala/slick/migration/api/DbTest.scala | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 79fe754f..ca2d34f6 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -268,13 +268,8 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(before.isSuccess) assert(after.isSuccess) } - } { - case ex: Throwable => - if (!this.profile.isInstanceOf[PostgresProfile]) { - runMigration(tm.drop) - } else { - throw ex; - } + } catch { + case _: Throwable => runMigration(tm.drop) } } } From 0809bcac26d405f862a7dcf06d9b12b5f179ee4e Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 15:08:34 -0400 Subject: [PATCH 11/19] Add better check for table --- src/test/scala/slick/migration/api/DbTest.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index ca2d34f6..ab74a103 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -246,7 +246,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) } } - test("reverse multiple columns") { + test("reverse") { val tm = TableMigration(TestTable) val createTable = tm.create.addColumns(_.id, _.strWithDefault) @@ -262,11 +262,11 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(reversed.sql(1).contains("STR_WITH_DEFAULT")) assert(reversed.sql(2).contains("drop table")) - try withBeforeAndAfter(reversed)(getTable(TestTable).asTry) { (before, after) => + try withBeforeAndAfter(reversed)(getTables) { (before, after) => // Only postgres can remove all columns from a table if (this.profile.isInstanceOf[PostgresProfile]) { - assert(before.isSuccess) - assert(after.isSuccess) + assert(before.contains("TEST_TABLE")) + assert(!after.contains("TEST_TABLE")) } } catch { case _: Throwable => runMigration(tm.drop) From 311481fe05c7c9b8784769d0c8430c53862985fd Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Fri, 26 Jun 2020 15:24:57 -0400 Subject: [PATCH 12/19] Only revert if postgres --- src/test/scala/slick/migration/api/DbTest.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index ab74a103..8ebfb456 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -269,7 +269,10 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(!after.contains("TEST_TABLE")) } } catch { - case _: Throwable => runMigration(tm.drop) + case _: Throwable => + if (!this.profile.isInstanceOf[PostgresProfile]) { + runMigration(tm.drop) + } } } } From ab468e026a076932c6d63ecbdb55979ce86fb1ed Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Sun, 28 Jun 2020 21:30:59 -0400 Subject: [PATCH 13/19] Reset sort weights instead --- .../scala/slick/migration/api/Dialect.scala | 7 +-- .../slick/migration/api/TableMigration.scala | 51 +++++++++---------- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/main/scala/slick/migration/api/Dialect.scala b/src/main/scala/slick/migration/api/Dialect.scala index ce8a0fcf..280a0eb4 100644 --- a/src/main/scala/slick/migration/api/Dialect.scala +++ b/src/main/scala/slick/migration/api/Dialect.scala @@ -142,7 +142,7 @@ class Dialect[-P <: JdbcProfile] extends AstHelpers { toB.andThen(b => (b :: bs, as)).applyOrElse(a, (_: A) => (bs, a :: as)) } - def migrateTable(table: TableInfo, actions: List[TableMigration.Action], reverse: Boolean = false): List[String] = { + def migrateTable(table: TableInfo, actions: List[TableMigration.Action]): List[String] = { def loop(actions: List[TableMigration.Action]): List[String] = actions match { case Nil => Nil case CreateTable :: rest => @@ -181,10 +181,7 @@ class Dialect[-P <: JdbcProfile] extends AstHelpers { case RenameIndexFrom(currentInfo, from) :: rest => renameIndex(currentInfo.copy(name = from), currentInfo.name) ::: loop(rest) } - loop(actions.reverse.sortBy(_.sort)( - if (reverse) Ordering.Int.reverse - else Ordering.Int.min - )) + loop(actions.reverse.sortBy(_.sort)) } } diff --git a/src/main/scala/slick/migration/api/TableMigration.scala b/src/main/scala/slick/migration/api/TableMigration.scala index ae5ad8fb..80112cb6 100644 --- a/src/main/scala/slick/migration/api/TableMigration.scala +++ b/src/main/scala/slick/migration/api/TableMigration.scala @@ -10,27 +10,27 @@ object TableMigration { object Action { sealed abstract class Reversible(sort: Int) extends Action(sort) - case object DropTable extends Action(0) - case object CreateTable extends Reversible(1) - case class RenameTableTo(to: String) extends Reversible(2) - case class RenameTableFrom(from: String) extends Reversible(2) - case class AddColumn(info: ColumnInfo) extends Reversible(3) - case class AddColumnAndSetInitialValue(info: ColumnInfo, rawSqlExpr: String) extends Reversible(3) - case class DropColumn(info: ColumnInfo) extends Reversible(4) - case class DropColumnOfName(name: String) extends Action(4) - case class RenameColumnTo(originalInfo: ColumnInfo, to: String) extends Reversible(5) - case class RenameColumnFrom(currentInfo: ColumnInfo, from: String) extends Reversible(5) - case class AlterColumnType(info: ColumnInfo) extends Action(6) - case class AlterColumnDefault(info: ColumnInfo) extends Action(6) - case class AlterColumnNullable(info: ColumnInfo) extends Action(6) - case class DropPrimaryKey(info: PrimaryKeyInfo) extends Reversible(7) - case class DropForeignKey(fk: ForeignKey) extends Reversible(7) - case class DropIndex(info: IndexInfo) extends Reversible(7) - case class AddPrimaryKey(info: PrimaryKeyInfo) extends Reversible(8) - case class AddForeignKey(fk: ForeignKey) extends Reversible(8) - case class CreateIndex(info: IndexInfo) extends Reversible(8) - case class RenameIndexTo(originalInfo: IndexInfo, to: String) extends Reversible(9) - case class RenameIndexFrom(currentInfo: IndexInfo, from: String) extends Reversible(9) + case object CreateTable extends Reversible(0) + case class RenameTableTo(to: String) extends Reversible(1) + case class AddColumn(info: ColumnInfo) extends Reversible(2) + case class AddColumnAndSetInitialValue(info: ColumnInfo, rawSqlExpr: String) extends Reversible(2) + case class RenameColumnTo(originalInfo: ColumnInfo, to: String) extends Reversible(3) + case class AlterColumnType(info: ColumnInfo) extends Action(4) + case class AlterColumnDefault(info: ColumnInfo) extends Action(4) + case class AlterColumnNullable(info: ColumnInfo) extends Action(4) + case class AddPrimaryKey(info: PrimaryKeyInfo) extends Reversible(5) + case class AddForeignKey(fk: ForeignKey) extends Reversible(6) + case class CreateIndex(info: IndexInfo) extends Reversible(7) + case class RenameIndexTo(originalInfo: IndexInfo, to: String) extends Reversible(8) + case class DropPrimaryKey(info: PrimaryKeyInfo) extends Reversible(9) + case class DropForeignKey(fk: ForeignKey) extends Reversible(9) + case class DropIndex(info: IndexInfo) extends Reversible(10) + case class RenameColumnFrom(currentInfo: ColumnInfo, from: String) extends Reversible(11) + case class RenameIndexFrom(currentInfo: IndexInfo, from: String) extends Reversible(11) + case class RenameTableFrom(from: String) extends Reversible(11) + case class DropColumnOfName(name: String) extends Action(12) + case class DropColumn(info: ColumnInfo) extends Reversible(12) + case object DropTable extends Action(13) } sealed trait WithActions[A <: Action] { @@ -58,13 +58,12 @@ object TableMigration { } } - implicit class Reversible[T <: JdbcProfile#Table[_], A <: Action](val underlying: TableMigration[T, Action.Reversible])( - implicit dialect: Dialect[_], withActions: TableMigration.WithActions[A]) + implicit class Reversible[T <: JdbcProfile#Table[_]](val underlying: TableMigration[T, Action.Reversible]) extends ReversibleMigration with SqlMigration { - override def sql = dialect.migrateTable(underlying.tableInfo, underlying.actions, reverse = true) + override def sql = underlying.sql override def reverse = { val tm0 = underlying.modActions(_ => List.empty[Action]) - underlying.actions.foldLeft(tm0) { (tm, action) => + underlying.actions.reverse.foldLeft(tm0) { (tm, action) => action match { case Action.CreateTable => tm.modActions(Action.DropTable :: _) case Action.RenameTableTo(to) => tm.modActions(Action.RenameTableFrom(to) :: _) @@ -88,7 +87,7 @@ object TableMigration { override def toString = underlying.toString } - implicit def toReversible[T <: JdbcProfile#Table[_]](implicit dialect: Dialect[_]): ToReversible[TableMigration[T, Action.Reversible]] = + implicit def toReversible[T <: JdbcProfile#Table[_]]: ToReversible[TableMigration[T, Action.Reversible]] = new ToReversible[TableMigration[T, Action.Reversible]](self => new Reversible(self)) def apply[T <: JdbcProfile#Table[_]](table: T) From abc0a5ce29f7c2d9e809cf84b0f9430c003c996e Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Sun, 28 Jun 2020 21:39:35 -0400 Subject: [PATCH 14/19] Play with weights some more --- .../slick/migration/api/TableMigration.scala | 34 +++++++++---------- version.sbt | 2 +- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/main/scala/slick/migration/api/TableMigration.scala b/src/main/scala/slick/migration/api/TableMigration.scala index 80112cb6..68780bb3 100644 --- a/src/main/scala/slick/migration/api/TableMigration.scala +++ b/src/main/scala/slick/migration/api/TableMigration.scala @@ -14,23 +14,23 @@ object TableMigration { case class RenameTableTo(to: String) extends Reversible(1) case class AddColumn(info: ColumnInfo) extends Reversible(2) case class AddColumnAndSetInitialValue(info: ColumnInfo, rawSqlExpr: String) extends Reversible(2) - case class RenameColumnTo(originalInfo: ColumnInfo, to: String) extends Reversible(3) - case class AlterColumnType(info: ColumnInfo) extends Action(4) - case class AlterColumnDefault(info: ColumnInfo) extends Action(4) - case class AlterColumnNullable(info: ColumnInfo) extends Action(4) - case class AddPrimaryKey(info: PrimaryKeyInfo) extends Reversible(5) - case class AddForeignKey(fk: ForeignKey) extends Reversible(6) - case class CreateIndex(info: IndexInfo) extends Reversible(7) - case class RenameIndexTo(originalInfo: IndexInfo, to: String) extends Reversible(8) - case class DropPrimaryKey(info: PrimaryKeyInfo) extends Reversible(9) - case class DropForeignKey(fk: ForeignKey) extends Reversible(9) - case class DropIndex(info: IndexInfo) extends Reversible(10) - case class RenameColumnFrom(currentInfo: ColumnInfo, from: String) extends Reversible(11) - case class RenameIndexFrom(currentInfo: IndexInfo, from: String) extends Reversible(11) - case class RenameTableFrom(from: String) extends Reversible(11) - case class DropColumnOfName(name: String) extends Action(12) - case class DropColumn(info: ColumnInfo) extends Reversible(12) - case object DropTable extends Action(13) + case class RenameColumnTo(originalInfo: ColumnInfo, to: String) extends Reversible(2) + case class AlterColumnType(info: ColumnInfo) extends Action(3) + case class AlterColumnDefault(info: ColumnInfo) extends Action(3) + case class AlterColumnNullable(info: ColumnInfo) extends Action(3) + case class AddPrimaryKey(info: PrimaryKeyInfo) extends Reversible(4) + case class AddForeignKey(fk: ForeignKey) extends Reversible(4) + case class CreateIndex(info: IndexInfo) extends Reversible(5) + case class RenameIndexTo(originalInfo: IndexInfo, to: String) extends Reversible(5) + case class DropPrimaryKey(info: PrimaryKeyInfo) extends Reversible(6) + case class DropForeignKey(fk: ForeignKey) extends Reversible(6) + case class DropIndex(info: IndexInfo) extends Reversible(7) + case class RenameColumnFrom(currentInfo: ColumnInfo, from: String) extends Reversible(8) + case class RenameIndexFrom(currentInfo: IndexInfo, from: String) extends Reversible(8) + case class RenameTableFrom(from: String) extends Reversible(8) + case class DropColumnOfName(name: String) extends Action(9) + case class DropColumn(info: ColumnInfo) extends Reversible(9) + case object DropTable extends Action(10) } sealed trait WithActions[A <: Action] { diff --git a/version.sbt b/version.sbt index 6df83db0..01090220 100644 --- a/version.sbt +++ b/version.sbt @@ -1 +1 @@ -version := "0.8.0" +version := "0.9.0-SNAPSHOT" From 6d06302c185d039952da1e8ecee25ce3e0df95a2 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Sun, 28 Jun 2020 21:55:34 -0400 Subject: [PATCH 15/19] Revert other test changes --- src/test/scala/slick/migration/api/DbTest.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 8ebfb456..e6187b28 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -172,7 +172,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) try { val addOtherColumns = tm.addColumns(_.int1Nullable, _.strWithDefault) - assert(addOtherColumns.reverse == tm.dropColumns(_.strWithDefault, _.int1Nullable)) + assert(addOtherColumns.reverse == tm.dropColumns(_.int1Nullable, _.strWithDefault)) withBeforeAndAfter(addOtherColumns)(getColumns(TestTable)) { (before, after) => assert(before.length === 1) @@ -199,7 +199,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(i2(Some("INDEX1") -> false) === Vector(Some("INT1"))) assert(i2(Some("INDEX2") -> true) === Vector(Some("INT2"), Some("INT3"))) - assert(createIndexes.reverse == tm.dropIndexes(_.index2, _.index1)) + assert(createIndexes.reverse == tm.dropIndexes(_.index1, _.index2)) withBeforeAndAfter(createIndexes.reverse)(indexes) { (_, i4) => assert(i4.keys.flatMap(_._1).exists(Set("INDEX1", "INDEX2")) === false) @@ -347,7 +347,7 @@ trait CompleteDbTest { this: DbTest[_ <: JdbcProfile] => finally runMigration(tm.drop) - assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.int1, _.id)) + assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.id, _.int1)) } test("alterColumnTypes") { From c32e0d8fc5acb645724f2521ae2382524b806b71 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Sun, 28 Jun 2020 21:58:57 -0400 Subject: [PATCH 16/19] Switch reverse columns --- src/test/scala/slick/migration/api/DbTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index e6187b28..4a65c43a 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -258,8 +258,8 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) val reversed = createTable.reverse assert(reversed.sql.size === 3) - assert(reversed.sql.head.contains("ID")) - assert(reversed.sql(1).contains("STR_WITH_DEFAULT")) + assert(reversed.sql.head.contains("STR_WITH_DEFAULT")) + assert(reversed.sql(1).contains("ID")) assert(reversed.sql(2).contains("drop table")) try withBeforeAndAfter(reversed)(getTables) { (before, after) => From faf8d061a83d39ebf878b1791b547ee7e48574d2 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Sun, 28 Jun 2020 22:05:07 -0400 Subject: [PATCH 17/19] Revert snapshot version --- version.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.sbt b/version.sbt index 01090220..6df83db0 100644 --- a/version.sbt +++ b/version.sbt @@ -1 +1 @@ -version := "0.9.0-SNAPSHOT" +version := "0.8.0" From 173319f05f41e5ce1905a39007be14a0cfe81052 Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Sun, 28 Jun 2020 22:06:57 -0400 Subject: [PATCH 18/19] Rerevert test column placements --- src/main/scala/slick/migration/api/TableMigration.scala | 2 +- src/test/scala/slick/migration/api/DbTest.scala | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/scala/slick/migration/api/TableMigration.scala b/src/main/scala/slick/migration/api/TableMigration.scala index 68780bb3..5fd393e2 100644 --- a/src/main/scala/slick/migration/api/TableMigration.scala +++ b/src/main/scala/slick/migration/api/TableMigration.scala @@ -63,7 +63,7 @@ object TableMigration { override def sql = underlying.sql override def reverse = { val tm0 = underlying.modActions(_ => List.empty[Action]) - underlying.actions.reverse.foldLeft(tm0) { (tm, action) => + underlying.actions.foldLeft(tm0) { (tm, action) => action match { case Action.CreateTable => tm.modActions(Action.DropTable :: _) case Action.RenameTableTo(to) => tm.modActions(Action.RenameTableFrom(to) :: _) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 4a65c43a..25c35a15 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -172,7 +172,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) try { val addOtherColumns = tm.addColumns(_.int1Nullable, _.strWithDefault) - assert(addOtherColumns.reverse == tm.dropColumns(_.int1Nullable, _.strWithDefault)) + assert(addOtherColumns.reverse == tm.dropColumns(_.strWithDefault, _.int1Nullable)) withBeforeAndAfter(addOtherColumns)(getColumns(TestTable)) { (before, after) => assert(before.length === 1) @@ -199,7 +199,7 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) assert(i2(Some("INDEX1") -> false) === Vector(Some("INT1"))) assert(i2(Some("INDEX2") -> true) === Vector(Some("INT2"), Some("INT3"))) - assert(createIndexes.reverse == tm.dropIndexes(_.index1, _.index2)) + assert(createIndexes.reverse == tm.dropIndexes(_.index2, _.index1)) withBeforeAndAfter(createIndexes.reverse)(indexes) { (_, i4) => assert(i4.keys.flatMap(_._1).exists(Set("INDEX1", "INDEX2")) === false) @@ -347,7 +347,7 @@ trait CompleteDbTest { this: DbTest[_ <: JdbcProfile] => finally runMigration(tm.drop) - assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.id, _.int1)) + assert(tm.addColumns(_.id, _.int1).reverse == tm.dropColumns(_.int1, _.id)) } test("alterColumnTypes") { From 92ab4f37b5bf9d997ef716bb870e781ba5773b8c Mon Sep 17 00:00:00 2001 From: Matt Oliver Date: Sun, 28 Jun 2020 22:12:00 -0400 Subject: [PATCH 19/19] Switch back reverse test --- src/test/scala/slick/migration/api/DbTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/scala/slick/migration/api/DbTest.scala b/src/test/scala/slick/migration/api/DbTest.scala index 25c35a15..8ebfb456 100644 --- a/src/test/scala/slick/migration/api/DbTest.scala +++ b/src/test/scala/slick/migration/api/DbTest.scala @@ -258,8 +258,8 @@ abstract class DbTest[P <: JdbcProfile](val tdb: JdbcTestDB {val profile: P}) val reversed = createTable.reverse assert(reversed.sql.size === 3) - assert(reversed.sql.head.contains("STR_WITH_DEFAULT")) - assert(reversed.sql(1).contains("ID")) + assert(reversed.sql.head.contains("ID")) + assert(reversed.sql(1).contains("STR_WITH_DEFAULT")) assert(reversed.sql(2).contains("drop table")) try withBeforeAndAfter(reversed)(getTables) { (before, after) =>