From 859f73b0870917adfc33911aa3592c56c71ad1cf Mon Sep 17 00:00:00 2001 From: Keiichi Hirobe Date: Wed, 19 Jan 2022 12:27:54 +0900 Subject: [PATCH] Open and parse files lazily --- migrate.go | 147 +++++++++++++-------- migrate_test.go | 238 ++++++++++++++++++++-------------- sort_test.go | 19 +-- sql-migrate/command_common.go | 4 +- toapply_test.go | 19 +-- 5 files changed, 262 insertions(+), 165 deletions(-) diff --git a/migrate.go b/migrate.go index 8797f8d5..4914091e 100644 --- a/migrate.go +++ b/migrate.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "net/http" - "os" "path" "regexp" "sort" @@ -121,15 +120,61 @@ func SetIgnoreUnknown(v bool) { migSet.IgnoreUnknown = v } -type Migration struct { - Id string - Up []string - Down []string - +type MigrationData struct { + Up []string + Down []string DisableTransactionUp bool DisableTransactionDown bool } +type migrationFile struct { + dir http.FileSystem + root string + baseName string +} + +type Migration struct { + Id string + + // data may be nil if not parsed yet. + data *MigrationData + // file is information of migration file, which is used to parse later. + // file may be nil if migration file has already been parsed when Migration is initialized. + file *migrationFile +} + +// Data parses migration file if not yet, and returns *MigrationData +func (m *Migration) Data() (*MigrationData, error) { + if m.data != nil { + return m.data, nil + } + err := m.loadFile() + if err != nil { + return nil, err + } + return m.data, nil +} + +func (m *Migration) loadFile() error { + if m.file == nil { + return fmt.Errorf("Error m.file must not be nil when call loadFile") + } + root := m.file.root + name := m.file.baseName + file, err := m.file.dir.Open(path.Join(root, name)) + if err != nil { + return fmt.Errorf("Error while opening %s: %s", name, err) + } + defer func() { _ = file.Close() }() + + migrationData, err := ParseMigration(name, file) + if err != nil { + return fmt.Errorf("Error while parsing %s: %s", name, err) + } + m.data = migrationData + return nil +} + func (m Migration) Less(other *Migration) bool { switch { case m.isNumeric() && other.isNumeric() && m.VersionInt() != other.VersionInt(): @@ -266,11 +311,14 @@ func findMigrations(dir http.FileSystem, root string) ([]*Migration, error) { for _, info := range files { if strings.HasSuffix(info.Name(), ".sql") { - migration, err := migrationFromFile(dir, root, info) - if err != nil { - return nil, err + migration := &Migration{ + Id: info.Name(), + file: &migrationFile{ + dir: dir, + root: root, + baseName: info.Name(), + }, } - migrations = append(migrations, migration) } } @@ -281,21 +329,6 @@ func findMigrations(dir http.FileSystem, root string) ([]*Migration, error) { return migrations, nil } -func migrationFromFile(dir http.FileSystem, root string, info os.FileInfo) (*Migration, error) { - path := path.Join(root, info.Name()) - file, err := dir.Open(path) - if err != nil { - return nil, fmt.Errorf("Error while opening %s: %s", info.Name(), err) - } - defer func() { _ = file.Close() }() - - migration, err := ParseMigration(info.Name(), file) - if err != nil { - return nil, fmt.Errorf("Error while parsing %s: %s", info.Name(), err) - } - return migration, nil -} - // Migrations from a bindata asset set. type AssetMigrationSource struct { // Asset should return content of file in path if exists @@ -325,11 +358,14 @@ func (a AssetMigrationSource) FindMigrations() ([]*Migration, error) { return nil, err } - migration, err := ParseMigration(name, bytes.NewReader(file)) + migrationData, err := ParseMigration(name, bytes.NewReader(file)) if err != nil { return nil, err } - + migration := &Migration{ + Id: name, + data: migrationData, + } migrations = append(migrations, migration) } } @@ -382,11 +418,14 @@ func (p PackrMigrationSource) FindMigrations() ([]*Migration, error) { return nil, err } - migration, err := ParseMigration(name, bytes.NewReader(file)) + migrationData, err := ParseMigration(name, bytes.NewReader(file)) if err != nil { return nil, err } - + migration := &Migration{ + Id: name, + data: migrationData, + } migrations = append(migrations, migration) } } @@ -398,23 +437,18 @@ func (p PackrMigrationSource) FindMigrations() ([]*Migration, error) { } // Migration parsing -func ParseMigration(id string, r io.ReadSeeker) (*Migration, error) { - m := &Migration{ - Id: id, - } - +func ParseMigration(id string, r io.ReadSeeker) (*MigrationData, error) { parsed, err := sqlparse.ParseMigration(r) if err != nil { return nil, fmt.Errorf("Error parsing migration (%s): %s", id, err) } - m.Up = parsed.UpStatements - m.Down = parsed.DownStatements - - m.DisableTransactionUp = parsed.DisableTransactionUp - m.DisableTransactionDown = parsed.DisableTransactionDown - - return m, nil + return &MigrationData{ + Up: parsed.UpStatements, + Down: parsed.DownStatements, + DisableTransactionUp: parsed.DisableTransactionUp, + DisableTransactionDown: parsed.DisableTransactionDown, + }, nil } type SqlExecutor interface { @@ -575,7 +609,11 @@ func (ms MigrationSet) PlanMigration(db *sql.DB, dialect string, m MigrationSour // Add missing migrations up to the last run migration. // This can happen for example when merges happened. if len(existingMigrations) > 0 { - result = append(result, ToCatchup(migrations, existingMigrations, record)...) + catchUp, err := ToCatchup(migrations, existingMigrations, record) + if err != nil { + return nil, nil, err + } + result = append(result, catchUp...) } // Figure out which migrations to apply @@ -585,18 +623,21 @@ func (ms MigrationSet) PlanMigration(db *sql.DB, dialect string, m MigrationSour toApplyCount = max } for _, v := range toApply[0:toApplyCount] { - + migrationData, err := v.Data() + if err != nil { + return nil, nil, err + } if dir == Up { result = append(result, &PlannedMigration{ Migration: v, - Queries: v.Up, - DisableTransaction: v.DisableTransactionUp, + Queries: migrationData.Up, + DisableTransaction: migrationData.DisableTransactionUp, }) } else if dir == Down { result = append(result, &PlannedMigration{ Migration: v, - Queries: v.Down, - DisableTransaction: v.DisableTransactionDown, + Queries: migrationData.Down, + DisableTransaction: migrationData.DisableTransactionDown, }) } } @@ -683,7 +724,7 @@ func ToApply(migrations []*Migration, current string, direction MigrationDirecti panic("Not possible") } -func ToCatchup(migrations, existingMigrations []*Migration, lastRun *Migration) []*PlannedMigration { +func ToCatchup(migrations, existingMigrations []*Migration, lastRun *Migration) ([]*PlannedMigration, error) { missing := make([]*PlannedMigration, 0) for _, migration := range migrations { found := false @@ -694,14 +735,18 @@ func ToCatchup(migrations, existingMigrations []*Migration, lastRun *Migration) } } if !found && migration.Less(lastRun) { + migrationData, err := migration.Data() + if err != nil { + return nil, err + } missing = append(missing, &PlannedMigration{ Migration: migration, - Queries: migration.Up, - DisableTransaction: migration.DisableTransactionUp, + Queries: migrationData.Up, + DisableTransaction: migrationData.DisableTransactionUp, }) } } - return missing + return missing, nil } func GetMigrationRecords(db *sql.DB, dialect string) ([]*MigrationRecord, error) { diff --git a/migrate_test.go b/migrate_test.go index 0d6a0875..a1c593de 100644 --- a/migrate_test.go +++ b/migrate_test.go @@ -11,15 +11,19 @@ import ( ) var sqliteMigrations = []*Migration{ - &Migration{ - Id: "123", - Up: []string{"CREATE TABLE people (id int)"}, - Down: []string{"DROP TABLE people"}, + { + Id: "123", + data: &MigrationData{ + Up: []string{"CREATE TABLE people (id int)"}, + Down: []string{"DROP TABLE people"}, + }, }, - &Migration{ - Id: "124", - Up: []string{"ALTER TABLE people ADD COLUMN first_name text"}, - Down: []string{"SELECT 0"}, // Not really supported + { + Id: "124", + data: &MigrationData{ + Up: []string{"ALTER TABLE people ADD COLUMN first_name text"}, + Down: []string{"SELECT 0"}, // Not really supported + }, }, } @@ -281,10 +285,12 @@ func (s *SqliteMigrateSuite) TestMigrateTransaction(c *C) { Migrations: []*Migration{ sqliteMigrations[0], sqliteMigrations[1], - &Migration{ - Id: "125", - Up: []string{"INSERT INTO people (id, first_name) VALUES (1, 'Test')", "SELECT fail"}, - Down: []string{}, // Not important here + { + Id: "125", + data: &MigrationData{ + Up: []string{"INSERT INTO people (id, first_name) VALUES (1, 'Test')", "SELECT fail"}, + Down: []string{}, // Not important here + }, }, }, } @@ -303,20 +309,26 @@ func (s *SqliteMigrateSuite) TestMigrateTransaction(c *C) { func (s *SqliteMigrateSuite) TestPlanMigration(c *C) { migrations := &MemoryMigrationSource{ Migrations: []*Migration{ - &Migration{ - Id: "1_create_table.sql", - Up: []string{"CREATE TABLE people (id int)"}, - Down: []string{"DROP TABLE people"}, + { + Id: "1_create_table.sql", + data: &MigrationData{ + 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 + { + Id: "2_alter_table.sql", + data: &MigrationData{ + 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"}, + { + Id: "10_add_last_name.sql", + data: &MigrationData{ + Up: []string{"ALTER TABLE people ADD COLUMN last_name text"}, + Down: []string{"ALTER TABLE people DROP COLUMN last_name"}, + }, }, }, } @@ -325,9 +337,11 @@ func (s *SqliteMigrateSuite) TestPlanMigration(c *C) { c.Assert(n, Equals, 3) migrations.Migrations = append(migrations.Migrations, &Migration{ - Id: "11_add_middle_name.sql", - Up: []string{"ALTER TABLE people ADD COLUMN middle_name text"}, - Down: []string{"ALTER TABLE people DROP COLUMN middle_name"}, + Id: "11_add_middle_name.sql", + data: &MigrationData{ + Up: []string{"ALTER TABLE people ADD COLUMN middle_name text"}, + Down: []string{"ALTER TABLE people DROP COLUMN middle_name"}, + }, }) plannedMigrations, _, err := PlanMigration(s.Db, "sqlite3", migrations, Up, 0) @@ -346,20 +360,26 @@ func (s *SqliteMigrateSuite) TestPlanMigration(c *C) { func (s *SqliteMigrateSuite) TestSkipMigration(c *C) { migrations := &MemoryMigrationSource{ Migrations: []*Migration{ - &Migration{ - Id: "1_create_table.sql", - Up: []string{"CREATE TABLE people (id int)"}, - Down: []string{"DROP TABLE people"}, + { + Id: "1_create_table.sql", + data: &MigrationData{ + 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 + { + Id: "2_alter_table.sql", + data: &MigrationData{ + 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"}, + { + Id: "10_add_last_name.sql", + data: &MigrationData{ + Up: []string{"ALTER TABLE people ADD COLUMN last_name text"}, + Down: []string{"ALTER TABLE people DROP COLUMN last_name"}, + }, }, }, } @@ -386,15 +406,19 @@ func (s *SqliteMigrateSuite) TestPlanMigrationWithHoles(c *C) { down := "SELECT 1" migrations := &MemoryMigrationSource{ Migrations: []*Migration{ - &Migration{ - Id: "1", - Up: []string{up}, - Down: []string{down}, + { + Id: "1", + data: &MigrationData{ + Up: []string{up}, + Down: []string{down}, + }, }, - &Migration{ - Id: "3", - Up: []string{up}, - Down: []string{down}, + { + Id: "3", + data: &MigrationData{ + Up: []string{up}, + Down: []string{down}, + }, }, }, } @@ -403,21 +427,27 @@ func (s *SqliteMigrateSuite) TestPlanMigrationWithHoles(c *C) { c.Assert(n, Equals, 2) migrations.Migrations = append(migrations.Migrations, &Migration{ - Id: "2", - Up: []string{up}, - Down: []string{down}, + Id: "2", + data: &MigrationData{ + Up: []string{up}, + Down: []string{down}, + }, }) migrations.Migrations = append(migrations.Migrations, &Migration{ - Id: "4", - Up: []string{up}, - Down: []string{down}, + Id: "4", + data: &MigrationData{ + Up: []string{up}, + Down: []string{down}, + }, }) migrations.Migrations = append(migrations.Migrations, &Migration{ - Id: "5", - Up: []string{up}, - Down: []string{down}, + Id: "5", + data: &MigrationData{ + Up: []string{up}, + Down: []string{down}, + }, }) // apply all the missing migrations @@ -476,20 +506,26 @@ func (s *SqliteMigrateSuite) TestLess(c *C) { 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"}, + { + Id: "1_create_table.sql", + data: &MigrationData{ + 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 + { + Id: "2_alter_table.sql", + data: &MigrationData{ + 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"}, + { + Id: "10_add_last_name.sql", + data: &MigrationData{ + Up: []string{"ALTER TABLE people ADD COLUMN last_name text"}, + Down: []string{"ALTER TABLE people DROP COLUMN last_name"}, + }, }, }, } @@ -500,9 +536,11 @@ func (s *SqliteMigrateSuite) TestPlanMigrationWithUnknownDatabaseMigrationApplie // 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"}, + Id: "10_add_middle_name.sql", + data: &MigrationData{ + 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) @@ -519,20 +557,26 @@ func (s *SqliteMigrateSuite) TestPlanMigrationWithUnknownDatabaseMigrationApplie func (s *SqliteMigrateSuite) TestPlanMigrationWithIgnoredUnknownDatabaseMigrationApplied(c *C) { migrations := &MemoryMigrationSource{ Migrations: []*Migration{ - &Migration{ - Id: "1_create_table.sql", - Up: []string{"CREATE TABLE people (id int)"}, - Down: []string{"DROP TABLE people"}, + { + Id: "1_create_table.sql", + data: &MigrationData{ + 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 + { + Id: "2_alter_table.sql", + data: &MigrationData{ + 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"}, + { + Id: "10_add_last_name.sql", + data: &MigrationData{ + Up: []string{"ALTER TABLE people ADD COLUMN last_name text"}, + Down: []string{"ALTER TABLE people DROP COLUMN last_name"}, + }, }, }, } @@ -544,9 +588,11 @@ func (s *SqliteMigrateSuite) TestPlanMigrationWithIgnoredUnknownDatabaseMigratio // 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"}, + Id: "10_add_middle_name.sql", + data: &MigrationData{ + 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) @@ -571,15 +617,19 @@ func (s *SqliteMigrateSuite) TestExecWithUnknownMigrationInDatabase(c *C) { // 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"}, + { + Id: "124_other", + data: &MigrationData{ + 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"}, + { + Id: "125", + data: &MigrationData{ + Up: []string{"ALTER TABLE people ADD COLUMN age int"}, + Down: []string{"ALTER TABLE people DROP COLUMN age"}, + }, }, } migrations = &MemoryMigrationSource{ diff --git a/sort_test.go b/sort_test.go index f768f553..68723d3d 100644 --- a/sort_test.go +++ b/sort_test.go @@ -1,8 +1,9 @@ package migrate import ( - . "gopkg.in/check.v1" "sort" + + . "gopkg.in/check.v1" ) type SortSuite struct{} @@ -11,14 +12,14 @@ var _ = Suite(&SortSuite{}) func (s *SortSuite) TestSortMigrations(c *C) { var migrations = byId([]*Migration{ - &Migration{Id: "10_abc", Up: nil, Down: nil}, - &Migration{Id: "120_cde", Up: nil, Down: nil}, - &Migration{Id: "1_abc", Up: nil, Down: nil}, - &Migration{Id: "efg", Up: nil, Down: nil}, - &Migration{Id: "2_cde", Up: nil, Down: nil}, - &Migration{Id: "35_cde", Up: nil, Down: nil}, - &Migration{Id: "3_efg", Up: nil, Down: nil}, - &Migration{Id: "4_abc", Up: nil, Down: nil}, + {Id: "10_abc"}, + {Id: "120_cde"}, + {Id: "1_abc"}, + {Id: "efg"}, + {Id: "2_cde"}, + {Id: "35_cde"}, + {Id: "3_efg"}, + {Id: "4_abc"}, }) sort.Sort(migrations) diff --git a/sql-migrate/command_common.go b/sql-migrate/command_common.go index e5949c6a..4a13aba9 100644 --- a/sql-migrate/command_common.go +++ b/sql-migrate/command_common.go @@ -49,12 +49,12 @@ func ApplyMigrations(dir migrate.MigrationDirection, dryrun bool, limit int) err func PrintMigration(m *migrate.PlannedMigration, dir migrate.MigrationDirection) { if dir == migrate.Up { ui.Output(fmt.Sprintf("==> Would apply migration %s (up)", m.Id)) - for _, q := range m.Up { + for _, q := range m.Queries { ui.Output(q) } } else if dir == migrate.Down { ui.Output(fmt.Sprintf("==> Would apply migration %s (down)", m.Id)) - for _, q := range m.Down { + for _, q := range m.Queries { ui.Output(q) } } else { diff --git a/toapply_test.go b/toapply_test.go index e5a53a26..5b9e5559 100644 --- a/toapply_test.go +++ b/toapply_test.go @@ -1,14 +1,15 @@ package migrate import ( - . "gopkg.in/check.v1" "sort" + + . "gopkg.in/check.v1" ) var toapplyMigrations = []*Migration{ - &Migration{Id: "abc", Up: nil, Down: nil}, - &Migration{Id: "cde", Up: nil, Down: nil}, - &Migration{Id: "efg", Up: nil, Down: nil}, + {Id: "abc"}, + {Id: "cde"}, + {Id: "efg"}, } type ToApplyMigrateSuite struct { @@ -79,11 +80,11 @@ func (s *ToApplyMigrateSuite) TestDownAll(c *C) { func (s *ToApplyMigrateSuite) TestAlphaNumericMigrations(c *C) { var migrations = byId([]*Migration{ - &Migration{Id: "10_abc", Up: nil, Down: nil}, - &Migration{Id: "1_abc", Up: nil, Down: nil}, - &Migration{Id: "efg", Up: nil, Down: nil}, - &Migration{Id: "2_cde", Up: nil, Down: nil}, - &Migration{Id: "35_cde", Up: nil, Down: nil}, + {Id: "10_abc"}, + {Id: "1_abc"}, + {Id: "efg"}, + {Id: "2_cde"}, + {Id: "35_cde"}, }) sort.Sort(migrations)