Skip to content

Commit

Permalink
Return error from PlanMigration when unknown migration in the database
Browse files Browse the repository at this point in the history
As discussed in GitHub issue #43 the PlanMigration function would not
allow running migrations when the there are already applied migrations
in the database which are not part of the migration source. In such
cases the user have to decide what to do.

This also fixes a bug where running the `up` migration would result in
success exit status while nothing has been done.
  • Loading branch information
ironsmile committed Mar 2, 2018
1 parent 081fe17 commit dd5fcca
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 0 deletions.
32 changes: 32 additions & 0 deletions migrate.go
Expand Up @@ -29,6 +29,26 @@ var tableName = "gorp_migrations"
var schemaName = ""
var numberPrefixRegex = regexp.MustCompile(`^(\d+).*$`)

// PlanError happens where no migration plan could be created between the sets
// of already applied migrations and the currently found. For example, when the database
// contains a migration which is not among the migrations list found for an operation.
type PlanError struct {
Migration *Migration
ErrorMessag string
}

func newPlanError(migration *Migration, errorMessage string) error {
return &PlanError{
Migration: migration,
ErrorMessag: errorMessage,
}
}

func (p *PlanError) Error() string {
return fmt.Sprintf("Unable to create migration plan because of %s: %s",
p.Migration.Id, p.ErrorMessag)
}

// TxError is returned when any error is encountered during a database
// transaction. It contains the relevant *Migration and notes it's Id in the
// Error function output.
Expand Down Expand Up @@ -445,6 +465,18 @@ func PlanMigration(db *sql.DB, dialect string, m MigrationSource, dir MigrationD
}
sort.Sort(byId(existingMigrations))

// Make sure all migrations in the database are among the found migrations which
// are to be applied.
migrationsSearch := make(map[string]struct{})
for _, migration := range migrations {
migrationsSearch[migration.Id] = struct{}{}
}
for _, existingMigration := range existingMigrations {
if _, ok := migrationsSearch[existingMigration.Id]; !ok {
return nil, nil, newPlanError(existingMigration, "unknown migration in database")
}
}

// Get last migration that was run
record := &Migration{}
if len(existingMigrations) > 0 {
Expand Down
85 changes: 85 additions & 0 deletions migrate_test.go
Expand Up @@ -482,3 +482,88 @@ func (s *SqliteMigrateSuite) TestLess(c *C) {
Less(&Migration{Id: "20160126_1100"}), Equals, false)

}

func (s *SqliteMigrateSuite) TestPlanMigrationWithUnknownDatabaseMigrationApplied(c *C) {
migrations := &MemoryMigrationSource{
Migrations: []*Migration{
&Migration{
Id: "1_create_table.sql",
Up: []string{"CREATE TABLE people (id int)"},
Down: []string{"DROP TABLE people"},
},
&Migration{
Id: "2_alter_table.sql",
Up: []string{"ALTER TABLE people ADD COLUMN first_name text"},
Down: []string{"SELECT 0"}, // Not really supported
},
&Migration{
Id: "10_add_last_name.sql",
Up: []string{"ALTER TABLE people ADD COLUMN last_name text"},
Down: []string{"ALTER TABLE people DROP COLUMN last_name"},
},
},
}
n, err := Exec(s.Db, "sqlite3", migrations, Up)
c.Assert(err, IsNil)
c.Assert(n, Equals, 3)

// Note that migration 10_add_last_name.sql is missing from the new migrations source
// so it is considered an "unknown" migration for the planner.
migrations.Migrations = append(migrations.Migrations[:2], &Migration{
Id: "10_add_middle_name.sql",
Up: []string{"ALTER TABLE people ADD COLUMN middle_name text"},
Down: []string{"ALTER TABLE people DROP COLUMN middle_name"},
})

_, _, err = PlanMigration(s.Db, "sqlite3", migrations, Up, 0)
c.Assert(err, NotNil, Commentf("Up migrations should not have been applied when there "+
"is an unknown migration in the database"))
c.Assert(err, FitsTypeOf, &PlanError{})

_, _, err = PlanMigration(s.Db, "sqlite3", migrations, Down, 0)
c.Assert(err, NotNil, Commentf("Down migrations should not have been applied when there "+
"is an unknown migration in the database"))
c.Assert(err, FitsTypeOf, &PlanError{})
}

// TestExecWithUnknownMigrationInDatabase makes sure that problems found with planning the
// migrations are propagated and returned by Exec.
func (s *SqliteMigrateSuite) TestExecWithUnknownMigrationInDatabase(c *C) {
migrations := &MemoryMigrationSource{
Migrations: sqliteMigrations[:2],
}

// Executes two migrations
n, err := Exec(s.Db, "sqlite3", migrations, Up)
c.Assert(err, IsNil)
c.Assert(n, Equals, 2)

// Then create a new migration source with one of the migrations missing
var newSqliteMigrations = []*Migration{
&Migration{
Id: "124_other",
Up: []string{"ALTER TABLE people ADD COLUMN middle_name text"},
Down: []string{"ALTER TABLE people DROP COLUMN middle_name"},
},
&Migration{
Id: "125",
Up: []string{"ALTER TABLE people ADD COLUMN age int"},
Down: []string{"ALTER TABLE people DROP COLUMN age"},
},
}
migrations = &MemoryMigrationSource{
Migrations: append(sqliteMigrations[:1], newSqliteMigrations...),
}

n, err = Exec(s.Db, "sqlite3", migrations, Up)
c.Assert(err, NotNil, Commentf("Migrations should not have been applied when there "+
"is an unknown migration in the database"))
c.Assert(err, FitsTypeOf, &PlanError{})
c.Assert(n, Equals, 0)

// Make sure the new columns are not actually created
_, err = s.DbMap.Exec("SELECT middle_name FROM people")
c.Assert(err, NotNil)
_, err = s.DbMap.Exec("SELECT age FROM people")
c.Assert(err, NotNil)
}

0 comments on commit dd5fcca

Please sign in to comment.