Skip to content

Commit

Permalink
Remove Placeholder again (#9)
Browse files Browse the repository at this point in the history
We only need it for the version argument, and that's safe to use with string concatenation instead.
  • Loading branch information
markuswustenberg authored Jun 17, 2021
1 parent 85e2dc9 commit 8282b29
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 25 deletions.
24 changes: 9 additions & 15 deletions migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,20 @@ import (
)

var (
upMatcher = regexp.MustCompile(`^([\w]+).up.sql$`)
downMatcher = regexp.MustCompile(`^([\w]+).down.sql`)
)

const (
PlaceholderDollar = "$1"
PlaceholderQuestion = "?"
upMatcher = regexp.MustCompile(`^([\w-]+).up.sql$`)
downMatcher = regexp.MustCompile(`^([\w-]+).down.sql`)
)

type Migrator struct {
DB *sql.DB
FS fs.FS
Placeholder string
DB *sql.DB
FS fs.FS
}

// New Migrator with default options.
// Migrator.Placeholder default is PlaceholderDollar. You can also use PlaceholderQuestion.
func New(db *sql.DB, fs fs.FS) *Migrator {
return &Migrator{
DB: db,
FS: fs,
Placeholder: PlaceholderDollar,
DB: db,
FS: fs,
}
}

Expand Down Expand Up @@ -109,7 +101,9 @@ func (m *Migrator) apply(ctx context.Context, name, version string) error {
return err
}
return m.inTransaction(ctx, func(tx *sql.Tx) error {
if _, err := tx.ExecContext(ctx, `update migrations set version = `+m.Placeholder, version); err != nil {
// Normally we wouldn't just string interpolate the version like this,
// but because we know the version has been matched against the regexes, we know it's safe.
if _, err := tx.ExecContext(ctx, `update migrations set version = '`+version+`'`); err != nil {
return err
}
if _, err := tx.ExecContext(ctx, string(content)); err != nil {
Expand Down
13 changes: 3 additions & 10 deletions migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ func TestMigrator(t *testing.T) {
tests := []struct {
flavor string
createDatabase func(*testing.T) (*sql.DB, func())
placeholder string
}{
{"postgres", createPostgresDatabase, migrate.PlaceholderDollar},
{"maria", createMariaDatabase, migrate.PlaceholderQuestion},
{"sqlite", createSQLiteDatabase, migrate.PlaceholderDollar},
{"postgres", createPostgresDatabase},
{"maria", createMariaDatabase},
{"sqlite", createSQLiteDatabase},
}

for _, test := range tests {
Expand All @@ -39,7 +38,6 @@ func TestMigrator(t *testing.T) {
is := is.New(t)

m := migrate.New(db, fstest.MapFS{})
m.Placeholder = test.placeholder

err := m.MigrateUp(context.Background())
is.NoErr(err)
Expand All @@ -56,7 +54,6 @@ func TestMigrator(t *testing.T) {
is := is.New(t)

m := migrate.New(db, mustSub(t, testdata, "testdata/good"))
m.Placeholder = test.placeholder

err := m.MigrateUp(context.Background())
is.NoErr(err)
Expand All @@ -78,7 +75,6 @@ func TestMigrator(t *testing.T) {
is := is.New(t)

m := migrate.New(db, mustSub(t, testdata, "testdata/good"))
m.Placeholder = test.placeholder

err := m.MigrateUp(context.Background())
is.NoErr(err)
Expand All @@ -93,7 +89,6 @@ func TestMigrator(t *testing.T) {
is := is.New(t)

m := migrate.New(db, mustSub(t, testdata, "testdata/bad"))
m.Placeholder = test.placeholder

err := m.MigrateUp(context.Background())
is.True(err != nil)
Expand All @@ -110,7 +105,6 @@ func TestMigrator(t *testing.T) {
is := is.New(t)

m := migrate.New(db, mustSub(t, testdata, "testdata/good"))
m.Placeholder = test.placeholder

err := m.MigrateUp(context.Background())
is.NoErr(err)
Expand All @@ -134,7 +128,6 @@ func TestMigrator(t *testing.T) {
is := is.New(t)

m := migrate.New(db, mustSub(t, testdata, "testdata/good"))
m.Placeholder = test.placeholder

err := m.MigrateDown(context.Background())
is.NoErr(err)
Expand Down

0 comments on commit 8282b29

Please sign in to comment.