From bbff44470604531dd61e205af424671c439b7dfb Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Mon, 29 Apr 2019 22:31:58 -0300 Subject: [PATCH 1/3] User defer for transaction rollback where possible This is safer, since it'll be guaranteed to run even on panics, etc --- gormigrate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gormigrate.go b/gormigrate.go index 900bbc4..26d65cd 100644 --- a/gormigrate.go +++ b/gormigrate.go @@ -252,6 +252,7 @@ func (g *Gormigrate) RollbackTo(migrationID string) error { } g.begin() + defer g.rollback() for i := len(g.migrations) - 1; i >= 0; i-- { migration := g.migrations[i] @@ -260,12 +261,10 @@ func (g *Gormigrate) RollbackTo(migrationID string) error { } if g.migrationDidRun(migration) { if err := g.rollbackMigration(migration); err != nil { - g.rollback() return err } } } - return g.commit() } @@ -282,8 +281,9 @@ func (g *Gormigrate) getLastRunMigration() (*Migration, error) { // RollbackMigration undo a migration. func (g *Gormigrate) RollbackMigration(m *Migration) error { g.begin() + defer g.rollback() + if err := g.rollbackMigration(m); err != nil { - g.rollback() return err } return g.commit() From 2288c0531815d0b33821ec124d022d9539cb4bd8 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Mon, 29 Apr 2019 22:32:22 -0300 Subject: [PATCH 2/3] Small code style improvements --- gormigrate.go | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/gormigrate.go b/gormigrate.go index 26d65cd..a4e9051 100644 --- a/gormigrate.go +++ b/gormigrate.go @@ -8,7 +8,7 @@ import ( ) const ( - initSchemaMigrationId = "SCHEMA_INIT" + initSchemaMigrationID = "SCHEMA_INIT" ) // MigrateFunc is the func signature for migrating. @@ -179,7 +179,6 @@ func (g *Gormigrate) migrate(migrationID string) error { break } } - return g.commit() } @@ -193,7 +192,7 @@ func (g *Gormigrate) hasMigrations() bool { // For now there's only have one reserved ID, but there may be more in the future. func (g *Gormigrate) checkReservedID() error { for _, m := range g.migrations { - if m.ID == initSchemaMigrationId { + if m.ID == initSchemaMigrationID { return &ReservedIDError{ID: m.ID} } } @@ -299,17 +298,14 @@ func (g *Gormigrate) rollbackMigration(m *Migration) error { } sql := fmt.Sprintf("DELETE FROM %s WHERE %s = ?", g.options.TableName, g.options.IDColumnName) - if err := g.tx.Exec(sql, m.ID).Error; err != nil { - return err - } - return nil + return g.tx.Exec(sql, m.ID).Error } func (g *Gormigrate) runInitSchema() error { if err := g.initSchema(g.tx); err != nil { return err } - if err := g.insertMigration(initSchemaMigrationId); err != nil { + if err := g.insertMigration(initSchemaMigrationID); err != nil { return err } @@ -345,10 +341,7 @@ func (g *Gormigrate) createMigrationTableIfNotExists() error { } sql := fmt.Sprintf("CREATE TABLE %s (%s VARCHAR(%d) PRIMARY KEY)", g.options.TableName, g.options.IDColumnName, g.options.IDColumnSize) - if err := g.tx.Exec(sql).Error; err != nil { - return err - } - return nil + return g.tx.Exec(sql).Error } func (g *Gormigrate) migrationDidRun(m *Migration) bool { @@ -363,7 +356,7 @@ func (g *Gormigrate) migrationDidRun(m *Migration) bool { // The schema can be initialised only if it hasn't been initialised yet // and no other migration has been applied already. func (g *Gormigrate) canInitializeSchema() bool { - if g.migrationDidRun(&Migration{ID: initSchemaMigrationId}) { + if g.migrationDidRun(&Migration{ID: initSchemaMigrationID}) { return false } @@ -390,9 +383,7 @@ func (g *Gormigrate) begin() { func (g *Gormigrate) commit() error { if g.options.UseTransaction { - if err := g.tx.Commit().Error; err != nil { - return err - } + return g.tx.Commit().Error } return nil } From 03e245b3e5ef9e21a99a80c29c23586c898e3d63 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Mon, 29 Apr 2019 22:41:45 -0300 Subject: [PATCH 3/3] Check for errors --- gormigrate.go | 58 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/gormigrate.go b/gormigrate.go index a4e9051..e54c7eb 100644 --- a/gormigrate.go +++ b/gormigrate.go @@ -164,11 +164,17 @@ func (g *Gormigrate) migrate(migrationID string) error { return err } - if g.initSchema != nil && g.canInitializeSchema() { - if err := g.runInitSchema(); err != nil { + if g.initSchema != nil { + canInitializeSchema, err := g.canInitializeSchema() + if err != nil { return err } - return g.commit() + if canInitializeSchema { + if err := g.runInitSchema(); err != nil { + return err + } + return g.commit() + } } for _, migration := range g.migrations { @@ -258,7 +264,11 @@ func (g *Gormigrate) RollbackTo(migrationID string) error { if migration.ID == migrationID { break } - if g.migrationDidRun(migration) { + migrationRan, err := g.migrationRan(migration) + if err != nil { + return err + } + if migrationRan { if err := g.rollbackMigration(migration); err != nil { return err } @@ -270,7 +280,13 @@ func (g *Gormigrate) RollbackTo(migrationID string) error { func (g *Gormigrate) getLastRunMigration() (*Migration, error) { for i := len(g.migrations) - 1; i >= 0; i-- { migration := g.migrations[i] - if g.migrationDidRun(migration) { + + migrationRan, err := g.migrationRan(migration) + if err != nil { + return nil, err + } + + if migrationRan { return migration, nil } } @@ -323,7 +339,11 @@ func (g *Gormigrate) runMigration(migration *Migration) error { return ErrMissingID } - if !g.migrationDidRun(migration) { + migrationRan, err := g.migrationRan(migration) + if err != nil { + return err + } + if !migrationRan { if err := migration.Migrate(g.tx); err != nil { return err } @@ -344,28 +364,34 @@ func (g *Gormigrate) createMigrationTableIfNotExists() error { return g.tx.Exec(sql).Error } -func (g *Gormigrate) migrationDidRun(m *Migration) bool { +func (g *Gormigrate) migrationRan(m *Migration) (bool, error) { var count int - g.tx. + err := g.tx. Table(g.options.TableName). Where(fmt.Sprintf("%s = ?", g.options.IDColumnName), m.ID). - Count(&count) - return count > 0 + Count(&count). + Error + return count > 0, err } // The schema can be initialised only if it hasn't been initialised yet // and no other migration has been applied already. -func (g *Gormigrate) canInitializeSchema() bool { - if g.migrationDidRun(&Migration{ID: initSchemaMigrationID}) { - return false +func (g *Gormigrate) canInitializeSchema() (bool, error) { + migrationRan, err := g.migrationRan(&Migration{ID: initSchemaMigrationID}) + if err != nil { + return false, err + } + if migrationRan { + return false, nil } // If the ID doesn't exist, we also want the list of migrations to be empty var count int - g.tx. + err = g.tx. Table(g.options.TableName). - Count(&count) - return count == 0 + Count(&count). + Error + return count == 0, err } func (g *Gormigrate) insertMigration(id string) error {