From 03a4023725dbfb08dce66874179fa8a25a6fdd97 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Thu, 30 Jun 2022 07:46:46 +0200 Subject: [PATCH] Introduce DoStep --- dbump.go | 113 +++++++++------------------------ dbump_test.go | 169 ++++++++++---------------------------------------- mock_test.go | 67 +++++--------------- 3 files changed, 78 insertions(+), 271 deletions(-) diff --git a/dbump.go b/dbump.go index bf7b204..8afc573 100644 --- a/dbump.go +++ b/dbump.go @@ -51,6 +51,7 @@ type Migrator interface { LockDB(ctx context.Context) error // UnlockDB to allow running other migrators later. UnlockDB(ctx context.Context) error + // Init the dbump database where database state is saved. // What is created by this method completely depends on migrator implementation // and might be different between databases. @@ -58,21 +59,16 @@ type Migrator interface { // Version of the migration. Used only once in the beginning. Version(ctx context.Context) (version int, err error) - // SetVersion is run after each migration. - SetVersion(ctx context.Context, version int) error - - // Begin the transaction before migration. - // Might be no-op if DisableTx is set or transaction are not supported by database. - Begin(ctx context.Context) error - // Commit the transaction after migration. - // Might be no-op if DisableTx is set or transaction are not supported by database. - Commit(ctx context.Context) error - // Rollback the transaction on migration fail. - // Might be no-op if DisableTx is set or transaction are not supported by database. - Rollback(ctx context.Context) error - - // Exec the given query and params. - Exec(ctx context.Context, query string, args ...interface{}) error + + // DoStep runs the given query and sets a new version on success. + DoStep(ctx context.Context, step Step) error +} + +// Step represents exact thing that is going to run. +type Step struct { + Version int + Query string + DisableTx bool } // Loader returns migrations to be applied on a database. @@ -197,50 +193,13 @@ func (m *mig) runMigrationsLocked(ctx context.Context, ms []*Migration) error { } for _, step := range m.prepareSteps(curr, target, ms) { - if err := m.execStep(ctx, step); err != nil { + if err := m.DoStep(ctx, step); err != nil { return err } } return nil } -func (m *mig) execStep(ctx context.Context, step step) error { - if m.Config.DisableTx { - return m.execSimpleStep(ctx, step) - } - return m.execStepSafely(ctx, step) -} - -func (m *mig) execStepSafely(ctx context.Context, step step) (err error) { - if err := m.Begin(ctx); err != nil { - return fmt.Errorf("begin tx: %w", err) - } - - defer func() { - if err != nil { - if errRollback := m.Rollback(ctx); errRollback != nil { - err = fmt.Errorf("(rollback tx: %v): %w", errRollback, err) - } - } - }() - - err = m.execSimpleStep(ctx, step) - if err == nil { - err = m.Commit(ctx) - } - return err -} - -func (m *mig) execSimpleStep(ctx context.Context, step step) error { - if err := m.Exec(ctx, step.Query); err != nil { - return fmt.Errorf("exec: %w", err) - } - if err := m.SetVersion(ctx, step.Version); err != nil { - return fmt.Errorf("set version: %w", err) - } - return nil -} - func (m *mig) getCurrAndTargetVersions(ctx context.Context, migrations int) (curr, target int, err error) { curr, err = m.Version(ctx) if err != nil { @@ -278,11 +237,11 @@ func (m *mig) getCurrAndTargetVersions(ctx context.Context, migrations int) (cur return curr, target, nil } -func (m *mig) prepareSteps(curr, target int, ms []*Migration) []step { +func (m *mig) prepareSteps(curr, target int, ms []*Migration) []Step { if curr == target { return nil } - steps := []step{} + steps := []Step{} direction := 1 if curr > target { @@ -296,34 +255,28 @@ func (m *mig) prepareSteps(curr, target int, ms []*Migration) []step { idx-- } - steps = append(steps, ms[idx].toStep(isUp)) + steps = append(steps, ms[idx].toStep(isUp, m.DisableTx)) if m.ZigZag { steps = append(steps, - ms[idx].toStep(!isUp), - ms[idx].toStep(isUp)) + ms[idx].toStep(!isUp, m.DisableTx), + ms[idx].toStep(isUp, m.DisableTx)) } } return steps } -type step struct { - Version int - IsQuery bool - Query string -} - -func (m *Migration) toStep(up bool) step { +func (m *Migration) toStep(up, disableTx bool) Step { if up { - return step{ - Version: m.ID, - IsQuery: m.Apply != "", - Query: m.Apply, + return Step{ + Version: m.ID, + Query: m.Apply, + DisableTx: disableTx, } } - return step{ - Version: m.ID - 1, - IsQuery: m.Revert != "", - Query: m.Revert, + return Step{ + Version: m.ID - 1, + Query: m.Revert, + DisableTx: disableTx, } } @@ -331,21 +284,15 @@ type locklessMigrator struct { m Migrator } -func (llm *locklessMigrator) Init(ctx context.Context) error { return llm.m.Init(ctx) } func (llm *locklessMigrator) LockDB(ctx context.Context) error { return nil } func (llm *locklessMigrator) UnlockDB(ctx context.Context) error { return nil } +func (llm *locklessMigrator) Init(ctx context.Context) error { return llm.m.Init(ctx) } + func (llm *locklessMigrator) Version(ctx context.Context) (version int, err error) { return llm.m.Version(ctx) } -func (llm *locklessMigrator) SetVersion(ctx context.Context, version int) error { - return llm.m.SetVersion(ctx, version) -} - -func (llm *locklessMigrator) Begin(ctx context.Context) error { return llm.m.Begin(ctx) } -func (llm *locklessMigrator) Commit(ctx context.Context) error { return llm.m.Commit(ctx) } -func (llm *locklessMigrator) Rollback(ctx context.Context) error { return llm.m.Rollback(ctx) } -func (llm *locklessMigrator) Exec(ctx context.Context, query string, args ...interface{}) error { - return llm.m.Exec(ctx, query, args...) +func (llm *locklessMigrator) DoStep(ctx context.Context, step Step) error { + return llm.m.DoStep(ctx, step) } diff --git a/dbump_test.go b/dbump_test.go index 513138b..6c282c9 100644 --- a/dbump_test.go +++ b/dbump_test.go @@ -47,11 +47,11 @@ func TestRunCheck(t *testing.T) { func TestMigrateUp(t *testing.T) { wantLog := []string{ "lockdb", "init", "getversion", - "exec", "SELECT 1;", "[]", "setversion", "1", - "exec", "SELECT 2;", "[]", "setversion", "2", - "exec", "SELECT 3;", "[]", "setversion", "3", - "exec", "SELECT 4;", "[]", "setversion", "4", - "exec", "SELECT 5;", "[]", "setversion", "5", + "dostep", "{v:1 q:'SELECT 1;' notx:true}", + "dostep", "{v:2 q:'SELECT 2;' notx:true}", + "dostep", "{v:3 q:'SELECT 3;' notx:true}", + "dostep", "{v:4 q:'SELECT 4;' notx:true}", + "dostep", "{v:5 q:'SELECT 5;' notx:true}", "unlockdb", } @@ -91,9 +91,7 @@ func TestMigrateUpOne(t *testing.T) { currVersion := 3 wantLog := []string{ "lockdb", "init", "getversion", - "begin", - "exec", "SELECT 4;", "[]", "setversion", "4", - "commit", + "dostep", "{v:4 q:'SELECT 4;' notx:false}", "unlockdb", } @@ -115,11 +113,11 @@ func TestMigrateUpOne(t *testing.T) { func TestMigrateDown(t *testing.T) { wantLog := []string{ "lockdb", "init", "getversion", - "exec", "SELECT 50;", "[]", "setversion", "4", - "exec", "SELECT 40;", "[]", "setversion", "3", - "exec", "SELECT 30;", "[]", "setversion", "2", - "exec", "SELECT 20;", "[]", "setversion", "1", - "exec", "SELECT 10;", "[]", "setversion", "0", + "dostep", "{v:4 q:'SELECT 50;' notx:true}", + "dostep", "{v:3 q:'SELECT 40;' notx:true}", + "dostep", "{v:2 q:'SELECT 30;' notx:true}", + "dostep", "{v:1 q:'SELECT 20;' notx:true}", + "dostep", "{v:0 q:'SELECT 10;' notx:true}", "unlockdb", } @@ -163,9 +161,7 @@ func TestMigrateDownOne(t *testing.T) { currVersion := 3 wantLog := []string{ "lockdb", "init", "getversion", - "begin", - "exec", "SELECT 30;", "[]", "setversion", "2", - "commit", + "dostep", "{v:2 q:'SELECT 30;' notx:false}", "unlockdb", } @@ -188,8 +184,8 @@ func TestUseForce(t *testing.T) { currVersion := 3 wantLog := []string{ "lockdb", "unlockdb", "lockdb", "init", "getversion", - "exec", "SELECT 4;", "[]", "setversion", "4", - "exec", "SELECT 5;", "[]", "setversion", "5", + "dostep", "{v:4 q:'SELECT 4;' notx:true}", + "dostep", "{v:5 q:'SELECT 5;' notx:true}", "unlockdb", } @@ -225,25 +221,25 @@ func TestUseForce(t *testing.T) { func TestZigZag(t *testing.T) { wantLog := []string{ "lockdb", "init", "getversion", - "exec", "SELECT 1;", "[]", "setversion", "1", - "exec", "SELECT 10;", "[]", "setversion", "0", - "exec", "SELECT 1;", "[]", "setversion", "1", + "dostep", "{v:1 q:'SELECT 1;' notx:true}", + "dostep", "{v:0 q:'SELECT 10;' notx:true}", + "dostep", "{v:1 q:'SELECT 1;' notx:true}", - "exec", "SELECT 2;", "[]", "setversion", "2", - "exec", "SELECT 20;", "[]", "setversion", "1", - "exec", "SELECT 2;", "[]", "setversion", "2", + "dostep", "{v:2 q:'SELECT 2;' notx:true}", + "dostep", "{v:1 q:'SELECT 20;' notx:true}", + "dostep", "{v:2 q:'SELECT 2;' notx:true}", - "exec", "SELECT 3;", "[]", "setversion", "3", - "exec", "SELECT 30;", "[]", "setversion", "2", - "exec", "SELECT 3;", "[]", "setversion", "3", + "dostep", "{v:3 q:'SELECT 3;' notx:true}", + "dostep", "{v:2 q:'SELECT 30;' notx:true}", + "dostep", "{v:3 q:'SELECT 3;' notx:true}", - "exec", "SELECT 4;", "[]", "setversion", "4", - "exec", "SELECT 40;", "[]", "setversion", "3", - "exec", "SELECT 4;", "[]", "setversion", "4", + "dostep", "{v:4 q:'SELECT 4;' notx:true}", + "dostep", "{v:3 q:'SELECT 40;' notx:true}", + "dostep", "{v:4 q:'SELECT 4;' notx:true}", - "exec", "SELECT 5;", "[]", "setversion", "5", - "exec", "SELECT 50;", "[]", "setversion", "4", - "exec", "SELECT 5;", "[]", "setversion", "5", + "dostep", "{v:5 q:'SELECT 5;' notx:true}", + "dostep", "{v:4 q:'SELECT 50;' notx:true}", + "dostep", "{v:5 q:'SELECT 5;' notx:true}", "unlockdb", } @@ -298,9 +294,7 @@ func TestFailOnUnlockDB(t *testing.T) { currVersion := 4 wantLog := []string{ "lockdb", "init", "getversion", - "begin", - "exec", "SELECT 5;", "[]", "setversion", "5", - "commit", + "dostep", "{v:5 q:'SELECT 5;' notx:false}", "unlockdb", } mm := &MockMigrator{ @@ -340,16 +334,14 @@ func TestFailOnGetVersionError(t *testing.T) { mustEqual(t, mm.log, wantLog) } -func TestFailOnSetVersionError(t *testing.T) { +func TestFailOnDoStepError(t *testing.T) { wantLog := []string{ "lockdb", "init", "getversion", - "begin", - "exec", "SELECT 1;", "[]", "setversion", "1", - "rollback", + "dostep", "{v:1 q:'SELECT 1;' notx:false}", "unlockdb", } mm := &MockMigrator{ - SetVersionFn: func(ctx context.Context, version int) error { + DoStepFn: func(ctx context.Context, step Step) error { return errors.New("no access") }, } @@ -363,101 +355,6 @@ func TestFailOnSetVersionError(t *testing.T) { mustEqual(t, mm.log, wantLog) } -func TestFailOnBegin(t *testing.T) { - wantLog := []string{ - "lockdb", "init", "getversion", - "begin", - "unlockdb", - } - mm := &MockMigrator{ - BeginFn: func(ctx context.Context) error { - return errors.New("timeout") - }, - } - cfg := Config{ - Migrator: mm, - Loader: NewSliceLoader(testdataMigrations), - Mode: ModeUp, - } - - failIfOk(t, Run(context.Background(), cfg)) - mustEqual(t, mm.log, wantLog) -} - -func TestFailOnExec(t *testing.T) { - wantLog := []string{ - "lockdb", "init", "getversion", - "begin", - "exec", "SELECT 1;", "[]", - "rollback", - "unlockdb", - } - mm := &MockMigrator{ - ExecFn: func(ctx context.Context, query string, args ...interface{}) error { - return errors.New("syntax error") - }, - } - cfg := Config{ - Migrator: mm, - Loader: NewSliceLoader(testdataMigrations), - Mode: ModeUp, - } - - failIfOk(t, Run(context.Background(), cfg)) - mustEqual(t, mm.log, wantLog) -} - -func TestFailOnCommit(t *testing.T) { - wantLog := []string{ - "lockdb", "init", "getversion", - "begin", - "exec", "SELECT 1;", "[]", "setversion", "1", - "commit", - "rollback", - "unlockdb", - } - mm := &MockMigrator{ - CommitFn: func(ctx context.Context) error { - return errors.New("constraint violation") - }, - } - cfg := Config{ - Migrator: mm, - Loader: NewSliceLoader(testdataMigrations), - Mode: ModeUp, - } - - failIfOk(t, Run(context.Background(), cfg)) - mustEqual(t, mm.log, wantLog) -} - -func TestFailOnRollback(t *testing.T) { - wantLog := []string{ - "lockdb", "init", "getversion", - "begin", - "exec", "SELECT 1;", "[]", "setversion", "1", - "commit", - "rollback", - "unlockdb", - } - mm := &MockMigrator{ - CommitFn: func(ctx context.Context) error { - return errors.New("constraint violation") - }, - RollbackFn: func(ctx context.Context) error { - return errors.New("timeout") - }, - } - cfg := Config{ - Migrator: mm, - Loader: NewSliceLoader(testdataMigrations), - Mode: ModeUp, - } - - failIfOk(t, Run(context.Background(), cfg)) - mustEqual(t, mm.log, wantLog) -} - func TestFailOnLoad(t *testing.T) { cfg := Config{ Migrator: &MockMigrator{}, diff --git a/mock_test.go b/mock_test.go index 45e1c6d..a038eaa 100644 --- a/mock_test.go +++ b/mock_test.go @@ -3,7 +3,6 @@ package dbump import ( "context" "fmt" - "strconv" ) var _ Migrator = &MockMigrator{} @@ -11,25 +10,11 @@ var _ Migrator = &MockMigrator{} type MockMigrator struct { log []string - InitFn func(ctx context.Context) error LockDBFn func(ctx context.Context) error UnlockDBFn func(ctx context.Context) error - - VersionFn func(ctx context.Context) (version int, err error) - SetVersionFn func(ctx context.Context, version int) error - - BeginFn func(ctx context.Context) error - CommitFn func(ctx context.Context) error - RollbackFn func(ctx context.Context) error - ExecFn func(ctx context.Context, query string, args ...interface{}) error -} - -func (mm *MockMigrator) Init(ctx context.Context) error { - mm.log = append(mm.log, "init") - if mm.InitFn == nil { - return nil - } - return mm.InitFn(ctx) + InitFn func(ctx context.Context) error + VersionFn func(ctx context.Context) (version int, err error) + DoStepFn func(ctx context.Context, step Step) error } func (mm *MockMigrator) LockDB(ctx context.Context) error { @@ -48,6 +33,14 @@ func (mm *MockMigrator) UnlockDB(ctx context.Context) error { return mm.UnlockDBFn(ctx) } +func (mm *MockMigrator) Init(ctx context.Context) error { + mm.log = append(mm.log, "init") + if mm.InitFn == nil { + return nil + } + return mm.InitFn(ctx) +} + func (mm *MockMigrator) Version(ctx context.Context) (version int, err error) { mm.log = append(mm.log, "getversion") if mm.VersionFn == nil { @@ -56,42 +49,12 @@ func (mm *MockMigrator) Version(ctx context.Context) (version int, err error) { return mm.VersionFn(ctx) } -func (mm *MockMigrator) SetVersion(ctx context.Context, version int) error { - mm.log = append(mm.log, "setversion", strconv.Itoa(version)) - if mm.SetVersionFn == nil { - return nil - } - return mm.SetVersionFn(ctx, version) -} - -func (mm *MockMigrator) Begin(ctx context.Context) error { - mm.log = append(mm.log, "begin") - if mm.BeginFn == nil { - return nil - } - return mm.BeginFn(ctx) -} -func (mm *MockMigrator) Commit(ctx context.Context) error { - mm.log = append(mm.log, "commit") - if mm.CommitFn == nil { - return nil - } - return mm.CommitFn(ctx) -} -func (mm *MockMigrator) Rollback(ctx context.Context) error { - mm.log = append(mm.log, "rollback") - if mm.RollbackFn == nil { - return nil - } - return mm.RollbackFn(ctx) -} - -func (mm *MockMigrator) Exec(ctx context.Context, query string, args ...interface{}) error { - mm.log = append(mm.log, "exec", query, fmt.Sprintf("%+v", args)) - if mm.ExecFn == nil { +func (mm *MockMigrator) DoStep(ctx context.Context, step Step) error { + mm.log = append(mm.log, "dostep", fmt.Sprintf("{v:%d q:'%s' notx:%v}", step.Version, step.Query, step.DisableTx)) + if mm.DoStepFn == nil { return nil } - return mm.ExecFn(ctx, query, args...) + return mm.DoStepFn(ctx, step) } type MockLoader struct {