fix: handle migration precedence appropriately

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
 	})
 }

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
 }

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],

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())

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)