From 22b50d5768024690972a5b70f01c02ecf55708b7 Mon Sep 17 00:00:00 2001 From: aeneasr <3372410+aeneasr@users.noreply.github.com> Date: Sun, 7 Mar 2021 22:08:06 +0100 Subject: [PATCH 1/2] fix: filter duplicate migrations --- popx/migrator.go | 1 + 1 file changed, 1 insertion(+) diff --git a/popx/migrator.go b/popx/migrator.go index 4a53478e..ff8a5487 100644 --- a/popx/migrator.go +++ b/popx/migrator.go @@ -454,6 +454,7 @@ func (m *Migrator) Status(ctx context.Context) (MigrationStatuses, error) { migrations.Filter(func(mf Migration) bool { return m.MigrationIsCompatible(con.Dialect.Name(), mf) }) + sort.Sort(migrations) if len(migrations) == 0 { return nil, errors.Errorf("unable to find any migrations for dialect: %s", con.Dialect.Name()) From d523760ae8a252b4bae1ecfaaf90928cb94d36c1 Mon Sep 17 00:00:00 2001 From: aeneasr <3372410+aeneasr@users.noreply.github.com> Date: Mon, 8 Mar 2021 08:36:33 +0100 Subject: [PATCH 2/2] fix: handle migration precedence appropriately This patch makes sure that specific migrations (e.g. 1234_foobar.sqlite.up.sql) override generic migrations (e.g. 1234_foobar.up.sql). --- popx/migration_box.go | 6 ++++ popx/migration_info.go | 38 ++++++++++++++++++--- popx/migration_info_test.go | 68 +++++++++++++++++++++++-------------- popx/migrator.go | 18 ++-------- popx/test_migrator.go | 7 +--- 5 files changed, 86 insertions(+), 51 deletions(-) diff --git a/popx/migration_box.go b/popx/migration_box.go index 447ae39c..c80c1e4d 100644 --- a/popx/migration_box.go +++ b/popx/migration_box.go @@ -3,6 +3,7 @@ package popx import ( "embed" "io/fs" + "sort" "strings" "github.com/gobuffalo/pop/v5" @@ -120,6 +121,11 @@ func (fm *MigrationBox) findMigrations(runner func([]byte) func(mf Migration, c Runner: runner(content), } fm.Migrations[mf.Direction] = append(fm.Migrations[mf.Direction], mf) + mod := sortIdent(fm.Migrations[mf.Direction]) + if mf.Direction == "down" { + mod = sort.Reverse(mod) + } + sort.Sort(mod) return nil }) } diff --git a/popx/migration_info.go b/popx/migration_info.go index 9bfe1905..3feaffe6 100644 --- a/popx/migration_info.go +++ b/popx/migration_info.go @@ -2,6 +2,7 @@ package popx import ( "fmt" + "sort" "github.com/gobuffalo/pop/v5" ) @@ -52,12 +53,41 @@ func (mfs Migrations) Swap(i, j int) { mfs[i], mfs[j] = mfs[j], mfs[i] } -func (mfs *Migrations) Filter(f func(mf Migration) bool) { +func sortIdent(i sort.Interface) sort.Interface { + return i +} + +func (mfs Migrations) SortAndFilter(dialect string, modifiers ...func(sort.Interface) sort.Interface) Migrations { + // We need to sort mfs in order to push the dbType=="all" migrations + // to the back. + m := append(Migrations{}, mfs...) + sort.Sort(m) + vsf := make(Migrations, 0) - for _, v := range *mfs { - if f(v) { + for k, v := range m { + if v.DBType == "all" { + // Add "all" only if we can not find a more specific migration for the dialect. + var hasSpecific bool + for kk, vv := range m { + if v.Version == vv.Version && kk != k && vv.DBType == dialect { + hasSpecific = true + break + } + } + + if !hasSpecific { + vsf = append(vsf, v) + } + } else if v.DBType == dialect { vsf = append(vsf, v) } } - *mfs = vsf + + mod := sortIdent(vsf) + for _, m := range modifiers { + mod = m(mod) + } + + sort.Sort(mod) + return vsf } diff --git a/popx/migration_info_test.go b/popx/migration_info_test.go index 79e671d1..418439bb 100644 --- a/popx/migration_info_test.go +++ b/popx/migration_info_test.go @@ -7,34 +7,50 @@ import ( "github.com/stretchr/testify/assert" ) +var migrations = Migrations{ + { + Version: "1", + DBType: "all", + }, + { + Version: "1", + DBType: "postgres", + }, + { + Version: "2", + DBType: "cockroach", + }, + { + Version: "2", + DBType: "all", + }, + { + Version: "3", + DBType: "all", + }, + { + Version: "3", + DBType: "mysql", + }, +} + +func TestFilterMigrations(t *testing.T) { + t.Run("db=mysql", func(t *testing.T) { + assert.Equal(t, Migrations{ + migrations[0], + migrations[3], + migrations[5], + }, migrations.SortAndFilter("mysql")) + assert.Equal(t, Migrations{ + migrations[5], + migrations[3], + migrations[0], + }, migrations.SortAndFilter("mysql", sort.Reverse)) + }) +} + func TestSortingMigrations(t *testing.T) { t.Run("case=enforces precedence for specific migrations", func(t *testing.T) { - migrations := Migrations{ - { - Version: "1", - DBType: "all", - }, - { - Version: "1", - DBType: "postgres", - }, - { - Version: "2", - DBType: "cockroach", - }, - { - Version: "2", - DBType: "all", - }, - { - Version: "3", - DBType: "all", - }, - { - Version: "3", - DBType: "mysql", - }, - } expectedOrder := Migrations{ migrations[1], migrations[0], diff --git a/popx/migrator.go b/popx/migrator.go index ff8a5487..89af19e2 100644 --- a/popx/migrator.go +++ b/popx/migrator.go @@ -87,11 +87,7 @@ func (m *Migrator) UpTo(ctx context.Context, step int) (applied int, err error) c := m.Connection.WithContext(ctx) err = m.exec(ctx, func() error { mtn := m.migrationTableName(ctx, c) - mfs := m.Migrations["up"] - mfs.Filter(func(mf Migration) bool { - return m.MigrationIsCompatible(c.Dialect.Name(), mf) - }) - sort.Sort(mfs) + mfs := m.Migrations["up"].SortAndFilter(c.Dialect.Name()) for _, mi := range mfs { exists, err := c.Where("version = ?", mi.Version).Exists(mtn) if err != nil { @@ -178,11 +174,7 @@ func (m *Migrator) Down(ctx context.Context, step int) error { if err != nil { return errors.Wrap(err, "migration down: unable count existing migration") } - mfs := m.Migrations["down"] - mfs.Filter(func(mf Migration) bool { - return m.MigrationIsCompatible(c.Dialect.Name(), mf) - }) - sort.Sort(sort.Reverse(mfs)) + mfs := m.Migrations["down"].SortAndFilter(c.Dialect.Name(), sort.Reverse) // skip all ran migration if len(mfs) > count { mfs = mfs[len(mfs)-count:] @@ -450,11 +442,7 @@ func (m *Migrator) Status(ctx context.Context) (MigrationStatuses, error) { con := m.Connection.WithContext(ctx) - migrations := m.Migrations["up"] - migrations.Filter(func(mf Migration) bool { - return m.MigrationIsCompatible(con.Dialect.Name(), mf) - }) - sort.Sort(migrations) + migrations := m.Migrations["up"].SortAndFilter(con.Dialect.Name()) if len(migrations) == 0 { return nil, errors.Errorf("unable to find any migrations for dialect: %s", con.Dialect.Name()) diff --git a/popx/test_migrator.go b/popx/test_migrator.go index 4208d3f6..1927f259 100644 --- a/popx/test_migrator.go +++ b/popx/test_migrator.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "os" "path/filepath" - "sort" "strings" "testing" "time" @@ -58,11 +57,7 @@ func NewTestMigrator(t *testing.T, c *pop.Connection, migrationPath, testDataPat // find migration index if len(mf.Version) > 14 { - upMigrations := tm.Migrations["up"] - upMigrations.Filter(func(mf Migration) bool { - return tm.MigrationIsCompatible(c.Dialect.Name(), mf) - }) - sort.Sort(upMigrations) + upMigrations := tm.Migrations["up"].SortAndFilter(c.Dialect.Name()) mgs := upMigrations require.False(t, len(mgs) == 0)