From 8d6ada0fdaf105a3c2105d6dde3fefa890ce76cd Mon Sep 17 00:00:00 2001 From: Rafael Espinoza Date: Sun, 16 Jan 2022 15:56:46 -0800 Subject: [PATCH] refactor: Make API simpler Squashed commit of the following: commit 8a21e47a014e918f207b44b404313b7187c71925 Author: Rafael Espinoza Date: Sat Jan 15 17:51:04 2022 -0800 refactor: Simplify exported API There are too many exported identifiers in package godfish. A developer interested in making a new driver implementation really just needs to know about the Driver interface and not much else. Split up the code so that the only exposed functionality may build: - a Driver implementation (pkg godfish/drivers/...) - a standalone binary (pkg godfish/internal/cmd) The material that was in pkg godfish is important to get that working, but is probably not necessary for outside consumers. Most of these changes involve moving material from pkg godfish to pkg internal, adjusting callers, and ensuring that pkg godfish's exported API doesn't reference pkg internal. Other notable changes are: - Change UpdateSchemaMigrations signature. Putting the Direction type in a method signature complicates things IMO; in practice, it means up or down, so a boolean makes more sense here. The Direction type is useful for disambiguating labels for the direction in a migration filename, but that's not a concern for a Driver implementation. - Remove func CreateSchemaMigrations because it's not used anywhere and the functionality is already in a Driver method. - Move buildtime versioning info variables to pkg cmd because the Version type is now in pkg internal. - Move validation for direction label to ensure it's always validated, not just in the subcommand. - Move pkg info stuff into pkg internal, this simplifies dependencies. commit 88b0f7d001f5d44aae1e1434b00d0f51600f7394 Author: Rafael Espinoza Date: Fri Jan 7 19:53:33 2022 -0800 refactor: Clean up Migration constructor It's a little weird have syscall side effects in a constructor func, so remove the directory check (use of the Dirpath field) from that. Also clean up code some more. Static interface checks are deadweight; it's better to define a field with an interface type and assign a concrete type to that field to check if a type implements an interface. Clean up, fix tests --- Makefile | 18 +- driver.go | 2 +- drivers/cassandra/cassandra.go | 4 +- drivers/mysql/mysql.go | 4 +- drivers/postgres/postgres.go | 4 +- drivers/sqlite3/sqlite3.go | 4 +- godfish.go | 218 ++++++++++-------- godfish_test.go | 5 +- internal/cmd/cmd.go | 3 +- internal/cmd/create-migration.go | 41 +--- internal/cmd/info.go | 33 +-- internal/cmd/migrate.go | 19 +- internal/cmd/version.go | 36 +-- direction.go => internal/direction.go | 24 +- filename.go => internal/filename.go | 10 +- filename_test.go => internal/filename_test.go | 56 ++--- internal/info.go | 39 ++++ internal/info/info.go | 33 --- internal/{info => }/info_test.go | 19 +- internal/internal.go | 19 ++ migration.go => internal/migration.go | 76 +++--- internal/migration_test.go | 183 +++++++++++++++ internal/stub/appliedversions.go | 3 +- internal/stub/driver.go | 4 +- internal/stub/migration.go | 16 +- internal/stub/version.go | 6 +- internal/test/apply_migration.go | 63 ++--- internal/test/info.go | 10 +- internal/test/migrate.go | 9 +- internal/test/test.go | 28 +-- version.go => internal/version.go | 12 +- internal/version/version.go | 11 - migration_test.go | 168 -------------- 33 files changed, 606 insertions(+), 574 deletions(-) rename direction.go => internal/direction.go (78%) rename filename.go => internal/filename.go (70%) rename filename_test.go => internal/filename_test.go (74%) create mode 100644 internal/info.go delete mode 100644 internal/info/info.go rename internal/{info => }/info_test.go (81%) create mode 100644 internal/internal.go rename migration.go => internal/migration.go (68%) create mode 100644 internal/migration_test.go rename version.go => internal/version.go (82%) delete mode 100644 internal/version/version.go delete mode 100644 migration_test.go diff --git a/Makefile b/Makefile index f60946b..02de5ba 100644 --- a/Makefile +++ b/Makefile @@ -7,11 +7,11 @@ PKG_IMPORT_PATH=github.com/rafaelespinoza/godfish # inject this metadata when building a binary. define LDFLAGS --X $(PKG_IMPORT_PATH)/internal/version.BranchName=$(shell git rev-parse --abbrev-ref HEAD) \ --X $(PKG_IMPORT_PATH)/internal/version.BuildTime=$(shell date --rfc-3339=seconds --utc | tr ' ' 'T') \ --X $(PKG_IMPORT_PATH)/internal/version.CommitHash=$(shell git rev-parse --short=7 HEAD) \ --X $(PKG_IMPORT_PATH)/internal/version.GoVersion=$(shell $(GO) version | awk '{ print $$3 }') \ --X $(PKG_IMPORT_PATH)/internal/version.Tag=$(shell git describe --tag) +-X $(PKG_IMPORT_PATH)/internal/cmd.versionBranchName=$(shell git rev-parse --abbrev-ref HEAD) \ +-X $(PKG_IMPORT_PATH)/internal/cmd.versionBuildTime=$(shell date --rfc-3339=seconds --utc | tr ' ' 'T') \ +-X $(PKG_IMPORT_PATH)/internal/cmd.versionCommitHash=$(shell git rev-parse --short=7 HEAD) \ +-X $(PKG_IMPORT_PATH)/internal/cmd.versionGoVersion=$(shell $(GO) version | awk '{ print $$3 }') \ +-X $(PKG_IMPORT_PATH)/internal/cmd.versionTag=$(shell git describe --tag) endef test: @@ -23,7 +23,7 @@ clean: cassandra: $(GO) build -o $(BIN) -v \ -ldflags "$(LDFLAGS) \ - -X $(PKG_IMPORT_PATH)/internal/version.Driver=cassandra" \ + -X $(PKG_IMPORT_PATH)/internal/cmd.versionDriver=cassandra" \ ./drivers/cassandra/godfish cassandra-test: $(GO) test $(ARGS) ./drivers/cassandra @@ -31,7 +31,7 @@ cassandra-test: postgres: $(GO) build -o $(BIN) -v \ -ldflags "$(LDFLAGS) \ - -X $(PKG_IMPORT_PATH)/internal/version.Driver=postgres" \ + -X $(PKG_IMPORT_PATH)/internal/cmd.versionDriver=postgres" \ ./drivers/postgres/godfish postgres-test: $(GO) test $(ARGS) ./drivers/postgres @@ -39,7 +39,7 @@ postgres-test: mysql: $(GO) build -o $(BIN) -v \ -ldflags "$(LDFLAGS) \ - -X $(PKG_IMPORT_PATH)/internal/version.Driver=mysql" \ + -X $(PKG_IMPORT_PATH)/internal/cmd.versionDriver=mysql" \ ./drivers/mysql/godfish mysql-test: $(GO) test $(ARGS) ./drivers/mysql @@ -47,7 +47,7 @@ mysql-test: sqlite3: CGO_ENABLED=1 $(GO) build -o $(BIN) -v \ -ldflags "$(LDFLAGS) \ - -X $(PKG_IMPORT_PATH)/internal/version.Driver=sqlite3" \ + -X $(PKG_IMPORT_PATH)/internal/cmd.versionDriver=sqlite3" \ ./drivers/sqlite3/godfish sqlite3-test: $(GO) test $(ARGS) ./drivers/sqlite3 diff --git a/driver.go b/driver.go index 29f6295..5eb6a68 100644 --- a/driver.go +++ b/driver.go @@ -26,7 +26,7 @@ type Driver interface { // UpdateSchemaMigrations records a timestamped version of a migration that // has been successfully applied by adding a new row to the schema // migrations table. - UpdateSchemaMigrations(dir Direction, version string) error + UpdateSchemaMigrations(forward bool, version string) error } // AppliedVersions represents an iterative list of migrations that have been run diff --git a/drivers/cassandra/cassandra.go b/drivers/cassandra/cassandra.go index 00c17dd..9bd3cb2 100644 --- a/drivers/cassandra/cassandra.go +++ b/drivers/cassandra/cassandra.go @@ -94,9 +94,9 @@ func (d *driver) AppliedVersions() (out godfish.AppliedVersions, err error) { return } -func (d *driver) UpdateSchemaMigrations(dir godfish.Direction, version string) (err error) { +func (d *driver) UpdateSchemaMigrations(forward bool, version string) (err error) { conn := d.connection - if dir == godfish.DirForward { + if forward { err = conn.Query(` INSERT INTO schema_migrations (migration_id) VALUES (?)`, diff --git a/drivers/mysql/mysql.go b/drivers/mysql/mysql.go index 1ac669f..a79e72b 100644 --- a/drivers/mysql/mysql.go +++ b/drivers/mysql/mysql.go @@ -92,9 +92,9 @@ func (d *driver) AppliedVersions() (out godfish.AppliedVersions, err error) { return } -func (d *driver) UpdateSchemaMigrations(dir godfish.Direction, version string) (err error) { +func (d *driver) UpdateSchemaMigrations(forward bool, version string) (err error) { conn := d.connection - if dir == godfish.DirForward { + if forward { _, err = conn.Exec(` INSERT INTO schema_migrations (migration_id) VALUES (?)`, diff --git a/drivers/postgres/postgres.go b/drivers/postgres/postgres.go index 728bc31..e9d92dd 100644 --- a/drivers/postgres/postgres.go +++ b/drivers/postgres/postgres.go @@ -65,9 +65,9 @@ func (d *driver) AppliedVersions() (out godfish.AppliedVersions, err error) { return } -func (d *driver) UpdateSchemaMigrations(dir godfish.Direction, version string) (err error) { +func (d *driver) UpdateSchemaMigrations(forward bool, version string) (err error) { conn := d.connection - if dir == godfish.DirForward { + if forward { _, err = conn.Exec(` INSERT INTO schema_migrations (migration_id) VALUES ($1) diff --git a/drivers/sqlite3/sqlite3.go b/drivers/sqlite3/sqlite3.go index c603026..1e3e230 100644 --- a/drivers/sqlite3/sqlite3.go +++ b/drivers/sqlite3/sqlite3.go @@ -67,9 +67,9 @@ func (d *driver) AppliedVersions() (out godfish.AppliedVersions, err error) { return } -func (d *driver) UpdateSchemaMigrations(dir godfish.Direction, version string) (err error) { +func (d *driver) UpdateSchemaMigrations(forward bool, version string) (err error) { conn := d.connection - if dir == godfish.DirForward { + if forward { _, err = conn.Exec(` INSERT INTO schema_migrations (migration_id) VALUES ($1)`, diff --git a/godfish.go b/godfish.go index 20e91df..f215d2c 100644 --- a/godfish.go +++ b/godfish.go @@ -6,17 +6,35 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "sort" "strings" + + "github.com/rafaelespinoza/godfish/internal" ) +// CreateMigrationFiles takes care of setting up a new DB migration by +// generating empty migration files in a directory at dirpath. Passing in true +// for reversible means that a complementary file will be made for rollbacks. +// Names for directions in the filename could be overridden from their default +// values (forward and reverse) with the input vars fwdlabel, revlabel when +// non-empty. +func CreateMigrationFiles(migrationName string, reversible bool, dirpath, fwdlabel, revlabel string) (err error) { + params, err := internal.NewMigrationParams(migrationName, reversible, dirpath, fwdlabel, revlabel) + if err != nil { + return + } + err = params.GenerateFiles() + return +} + // Migrate executes all migrations at directoryPath in the specified direction. -func Migrate(driver Driver, directoryPath string, direction Direction, finishAtVersion string) (err error) { +func Migrate(driver Driver, directoryPath string, forward bool, finishAtVersion string) (err error) { var ( dsn string - migrations []Migration + migrations []internal.Migration ) if dsn, err = getDSN(); err != nil { return @@ -26,10 +44,15 @@ func Migrate(driver Driver, directoryPath string, direction Direction, finishAtV } defer driver.Close() - if finishAtVersion == "" && direction == DirForward { - finishAtVersion = maxVersion - } else if finishAtVersion == "" && direction == DirReverse { - finishAtVersion = minVersion + direction := internal.DirReverse + if forward { + direction = internal.DirForward + } + + if finishAtVersion == "" && direction == internal.DirForward { + finishAtVersion = internal.MaxVersion + } else if finishAtVersion == "" && direction == internal.DirReverse { + finishAtVersion = internal.MinVersion } finder := migrationFinder{ @@ -42,7 +65,7 @@ func Migrate(driver Driver, directoryPath string, direction Direction, finishAtV } for _, mig := range migrations { - pathToFile := directoryPath + "/" + MakeMigrationFilename(mig) + pathToFile := directoryPath + "/" + internal.MakeMigrationFilename(mig) if err = runMigration(driver, pathToFile, mig); err != nil { return } @@ -50,22 +73,17 @@ func Migrate(driver Driver, directoryPath string, direction Direction, finishAtV return } -var ( - // ErrSchemaMigrationsDoesNotExist means there is no database table to - // record migration status. - ErrSchemaMigrationsDoesNotExist = errors.New("schema migrations table does not exist") - - errNotFound = errors.New("not found") - errDataInvalid = errors.New("data invalid") -) +// ErrSchemaMigrationsDoesNotExist means there is no database table to +// record migration status. +var ErrSchemaMigrationsDoesNotExist = errors.New("schema migrations table does not exist") // ApplyMigration runs a migration at directoryPath with the specified version // and direction. -func ApplyMigration(driver Driver, directoryPath string, direction Direction, version string) (err error) { +func ApplyMigration(driver Driver, directoryPath string, forward bool, version string) (err error) { var ( dsn string pathToFile string - mig Migration + mig internal.Migration ) if dsn, err = getDSN(); err != nil { @@ -76,16 +94,16 @@ func ApplyMigration(driver Driver, directoryPath string, direction Direction, ve } defer driver.Close() - if direction == DirUnknown { - err = fmt.Errorf("unknown Direction %q", direction) - return + direction := internal.DirReverse + if forward { + direction = internal.DirForward } if version == "" { // attempt to find the next version to apply in the direction var limit string - if direction == DirForward { - limit = maxVersion + if direction == internal.DirForward { + limit = internal.MaxVersion } finder := migrationFinder{ direction: direction, @@ -96,7 +114,7 @@ func ApplyMigration(driver Driver, directoryPath string, direction Direction, ve err = fmt.Errorf("specified no version; error attempting to find one; %v", ierr) return } else if len(toApply) < 1 { - err = fmt.Errorf("version %w", errNotFound) + err = fmt.Errorf("version %w", internal.ErrNotFound) return } else { version = toApply[0].Version().String() @@ -106,28 +124,28 @@ func ApplyMigration(driver Driver, directoryPath string, direction Direction, ve if pathToFile, err = figureOutBasename(directoryPath, direction, version); err != nil { return } - fn := filename(directoryPath + "/" + pathToFile) - if mig, err = parseMigration(fn); err != nil { + fn := internal.Filename(directoryPath + "/" + pathToFile) + if mig, err = internal.ParseMigration(fn); err != nil { return } err = runMigration(driver, pathToFile, mig) return } -func figureOutBasename(directoryPath string, direction Direction, version string) (f string, e error) { +func figureOutBasename(directoryPath string, direction internal.Direction, version string) (f string, e error) { var filenames []string // glob as many filenames as possible that match the "version" segment, then // narrow it down from there. - glob := directoryPath + "/" + makeFilename(version, Indirection{}, "*") + glob := directoryPath + "/" + internal.MakeFilename(version, internal.Indirection{}, "*") if filenames, e = filepath.Glob(glob); e != nil { return } var directionNames []string - if direction == DirForward { - directionNames = ForwardDirections - } else if direction == DirReverse { - directionNames = ReverseDirections + if direction == internal.DirForward { + directionNames = internal.ForwardDirections + } else if direction == internal.DirReverse { + directionNames = internal.ReverseDirections } for _, fn := range filenames { @@ -139,7 +157,7 @@ func figureOutBasename(directoryPath string, direction Direction, version string } } if f == "" { - e = fmt.Errorf("files %w", errNotFound) + e = fmt.Errorf("files %w", internal.ErrNotFound) } return } @@ -159,13 +177,13 @@ func (e *runMigrationError) Error() string { // runMigration executes a migration against the database. The input, pathToFile // should be relative to the current working directory. -func runMigration(driver Driver, pathToFile string, mig Migration) (err error) { +func runMigration(driver Driver, pathToFile string, mig internal.Migration) (err error) { var data []byte if data, err = os.ReadFile(pathToFile); err != nil { return } gerund := "migrating" - if mig.Indirection().Value == DirReverse { + if mig.Indirection().Value == internal.DirReverse { gerund = "rolling back" } fmt.Printf("%s version %q ... ", gerund, mig.Version().String()) @@ -182,7 +200,7 @@ func runMigration(driver Driver, pathToFile string, mig Migration) (err error) { return } err = driver.UpdateSchemaMigrations( - mig.Indirection().Value, + mig.Indirection().Value == internal.DirForward, mig.Version().String(), ) if err == nil { @@ -191,10 +209,8 @@ func runMigration(driver Driver, pathToFile string, mig Migration) (err error) { return } -// CreateSchemaMigrationsTable creates a table to track status of migrations on -// the database. Running any migration will create the table, so you don't -// usually need to call this function. -func CreateSchemaMigrationsTable(driver Driver) (err error) { +// Info writes status of migrations to w in formats json or tsv. +func Info(driver Driver, directoryPath string, forward bool, finishAtVersion string, w io.Writer, format string) (err error) { var dsn string if dsn, err = getDSN(); err != nil { return @@ -203,34 +219,33 @@ func CreateSchemaMigrationsTable(driver Driver) (err error) { return err } defer driver.Close() - return driver.CreateSchemaMigrationsTable() -} -// Info displays the outputs of various helper functions. -func Info(driver Driver, directoryPath string, direction Direction, finishAtVersion string, p InfoPrinter) (err error) { - var dsn string - if dsn, err = getDSN(); err != nil { - return - } - if err = driver.Connect(dsn); err != nil { - return err + direction := internal.DirReverse + if forward { + direction = internal.DirForward } - defer driver.Close() + finder := migrationFinder{ direction: direction, directoryPath: directoryPath, finishAtVersion: finishAtVersion, - infoPrinter: p, + infoPrinter: choosePrinter(format, w), } _, err = finder.query(driver) return } -// Config is for various runtime settings. -type Config struct { - PathToFiles string `json:"path_to_files"` - ForwardLabel string `json:"forward_label"` - ReverseLabel string `json:"reverse_label"` +func choosePrinter(format string, w io.Writer) (out internal.InfoPrinter) { + if format == "json" { + out = internal.NewJSON(w) + return + } + + if format != "tsv" && format != "" { + fmt.Fprintf(os.Stderr, "unknown format %q, defaulting to tsv\n", format) + } + out = internal.NewTSV(w) + return } // Init creates a configuration file at pathToFile unless it already exists. @@ -245,7 +260,7 @@ func Init(pathToFile string) (err error) { } var data []byte - if data, err = json.MarshalIndent(Config{}, "", "\t"); err != nil { + if data, err = json.MarshalIndent(internal.Config{}, "", "\t"); err != nil { return err } return os.WriteFile( @@ -258,14 +273,14 @@ func Init(pathToFile string) (err error) { // migrationFinder is a collection of named parameters to use when searching // for migrations to apply. type migrationFinder struct { - direction Direction + direction internal.Direction directoryPath string finishAtVersion string - infoPrinter InfoPrinter + infoPrinter internal.InfoPrinter } // query returns a list of Migrations to apply. -func (m *migrationFinder) query(driver Driver) (out []Migration, err error) { +func (m *migrationFinder) query(driver Driver) (out []internal.Migration, err error) { available, err := m.available() if err != nil { return @@ -290,25 +305,25 @@ func (m *migrationFinder) query(driver Driver) (out []Migration, err error) { return } var useDefaultRollbackVersion bool - if m.finishAtVersion == "" && m.direction == DirForward { - m.finishAtVersion = maxVersion - } else if m.finishAtVersion == "" && m.direction == DirReverse { + if m.finishAtVersion == "" && m.direction == internal.DirForward { + m.finishAtVersion = internal.MaxVersion + } else if m.finishAtVersion == "" && m.direction == internal.DirReverse { if len(toApply) == 0 { return } useDefaultRollbackVersion = true m.finishAtVersion = toApply[0].Version().String() } - var finish Version - if finish, err = parseVersion(m.finishAtVersion); err != nil { + var finish internal.Version + if finish, err = internal.ParseVersion(m.finishAtVersion); err != nil { return } for _, mig := range toApply { version := mig.Version() - if m.direction == DirForward && finish.Before(version) { + if m.direction == internal.DirForward && finish.Before(version) { break } - if m.direction == DirReverse { + if m.direction == internal.DirReverse { if version.Before(finish) { break } @@ -327,12 +342,12 @@ func (m *migrationFinder) query(driver Driver) (out []Migration, err error) { } // available returns a list of Migration values in a specified direction. -func (m *migrationFinder) available() (out []Migration, err error) { +func (m *migrationFinder) available() (out []internal.Migration, err error) { var fileDir *os.File var filenames []string if fileDir, err = os.Open(m.directoryPath); err != nil { if _, ok := err.(*os.PathError); ok { - err = fmt.Errorf("path to migration files %q %w", m.directoryPath, errNotFound) + err = fmt.Errorf("path to migration files %q %w", m.directoryPath, internal.ErrNotFound) } return } @@ -340,14 +355,14 @@ func (m *migrationFinder) available() (out []Migration, err error) { if filenames, err = fileDir.Readdirnames(0); err != nil { return } - if m.direction == DirForward { + if m.direction == internal.DirForward { sort.Strings(filenames) } else { sort.Sort(sort.Reverse(sort.StringSlice(filenames))) } for _, fn := range filenames { - mig, ierr := parseMigration(filename(fn)) - if errors.Is(ierr, errDataInvalid) { + mig, ierr := internal.ParseMigration(internal.Filename(fn)) + if errors.Is(ierr, internal.ErrDataInvalid) { fmt.Println(ierr) continue } else if ierr != nil { @@ -363,7 +378,7 @@ func (m *migrationFinder) available() (out []Migration, err error) { return } -func scanAppliedVersions(driver Driver, directoryPath string) (out []Migration, err error) { +func scanAppliedVersions(driver Driver, directoryPath string) (out []internal.Migration, err error) { var rows AppliedVersions if rows, err = driver.AppliedVersions(); err != nil { return @@ -371,18 +386,18 @@ func scanAppliedVersions(driver Driver, directoryPath string) (out []Migration, defer rows.Close() for rows.Next() { var version, basename string - var mig Migration + var mig internal.Migration if err = rows.Scan(&version); err != nil { return } - basename, err = figureOutBasename(directoryPath, DirForward, version) - if errors.Is(err, errNotFound) { + basename, err = figureOutBasename(directoryPath, internal.DirForward, version) + if errors.Is(err, internal.ErrNotFound) { err = nil // swallow error and continue } else if err != nil { return } - mig, err = parseMigration(filename(basename)) - if errors.Is(err, errDataInvalid) { + mig, err = internal.ParseMigration(internal.Filename(basename)) + if errors.Is(err, internal.ErrDataInvalid) { err = nil // swallow error and continue } else if mig != nil { out = append(out, mig) @@ -393,15 +408,15 @@ func scanAppliedVersions(driver Driver, directoryPath string) (out []Migration, // filter compares lists of applied and available migrations, then selects a // list of migrations to apply. -func (m *migrationFinder) filter(applied, available []Migration) (out []Migration, err error) { - allVersions := make(map[int64]Migration) - uniqueToApplied := make(map[int64]Migration) +func (m *migrationFinder) filter(applied, available []internal.Migration) (out []internal.Migration, err error) { + allVersions := make(map[int64]internal.Migration) + uniqueToApplied := make(map[int64]internal.Migration) for _, mig := range applied { version := mig.Version().Value() uniqueToApplied[version] = mig allVersions[version] = mig } - uniqueToAvailable := make(map[int64]Migration) + uniqueToAvailable := make(map[int64]internal.Migration) for _, mig := range available { version := mig.Version().Value() if _, ok := uniqueToApplied[version]; ok { @@ -412,7 +427,7 @@ func (m *migrationFinder) filter(applied, available []Migration) (out []Migratio } } - if m.direction == DirForward { + if m.direction == internal.DirForward { for version, mig := range allVersions { _, isApplied := uniqueToApplied[version] _, isAvailable := uniqueToAvailable[version] @@ -430,25 +445,36 @@ func (m *migrationFinder) filter(applied, available []Migration) (out []Migratio // the original filename was, by assuming that the list of // forward directions is in the same order as the corresponding // reverse directions. It's kind of hacky, I know. - mut := &mutation{ - indirection: Indirection{Value: DirReverse}, - label: mig.Label(), - version: mig.Version(), + var mut internal.Migration + indirection := internal.Indirection{ + Value: internal.DirReverse, + Label: "reverse", // need to have something here, it gets restored later. } - for i, fwd := range ForwardDirections { + mut, err = newMigration(mig.Version().String(), indirection, mig.Label()) + if err != nil { + return + } + for i, fwd := range internal.ForwardDirections { + // Restore the direction label for reverse migration based + // on corresponding label for the known forward migration. + // // Another assumption, the filename format will never // change. If it does change, for example: it is // "${version}-${direction}-${label}", instead of // "${direction}-${version}-${label}", then this won't work. if mig.Indirection().Label == fwd { - mut.indirection.Label = ReverseDirections[i] + indirection.Label = internal.ReverseDirections[i] + mut, err = newMigration(mig.Version().String(), indirection, mig.Label()) + if err != nil { + return + } break } } - if mut.indirection.Label == "" { + if mut.Label() == "" { err = fmt.Errorf( "direction.Label empty; direction.Value: %q, version: %v, label: %q", - mut.indirection.Value, mut.version, mut.label, + mut.Indirection(), mut.Version().String(), mut.Label(), ) return } @@ -456,7 +482,7 @@ func (m *migrationFinder) filter(applied, available []Migration) (out []Migratio } } } - if m.direction == DirForward { + if m.direction == internal.DirForward { sort.Slice(out, func(i, j int) bool { return out[i].Version().Before(out[j].Version()) }) @@ -468,11 +494,13 @@ func (m *migrationFinder) filter(applied, available []Migration) (out []Migratio return } -type InfoPrinter interface { - PrintInfo(state string, migration Migration) error +func newMigration(version string, ind internal.Indirection, label string) (out internal.Migration, err error) { + fn := internal.MakeFilename(version, ind, label) + out, err = internal.ParseMigration(internal.Filename(fn)) + return } -func printMigrations(p InfoPrinter, state string, migrations []Migration) (err error) { +func printMigrations(p internal.InfoPrinter, state string, migrations []internal.Migration) (err error) { for i, mig := range migrations { if err = p.PrintInfo(state, mig); err != nil { err = fmt.Errorf("%w; item %d", err, i) diff --git a/godfish_test.go b/godfish_test.go index 35e242b..777fc1f 100644 --- a/godfish_test.go +++ b/godfish_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) const baseTestOutputDir = "/tmp/godfish_test" @@ -34,7 +35,7 @@ func TestInit(t *testing.T) { if err = godfish.Init(pathToFile); err != nil { t.Fatalf("something else is wrong with setup; %v", err) } - var conf godfish.Config + var conf internal.Config if data, err := os.ReadFile(pathToFile); err != nil { t.Fatal(err) } else if err = json.Unmarshal(data, &conf); err != nil { @@ -55,7 +56,7 @@ func TestInit(t *testing.T) { if err := godfish.Init(pathToFile); err != nil { t.Fatal(err) } - var conf2 godfish.Config + var conf2 internal.Config if data, err := os.ReadFile(pathToFile); err != nil { t.Fatal(err) } else if err = json.Unmarshal(data, &conf2); err != nil { diff --git a/internal/cmd/cmd.go b/internal/cmd/cmd.go index 3f7bfbb..4c5d774 100644 --- a/internal/cmd/cmd.go +++ b/internal/cmd/cmd.go @@ -11,6 +11,7 @@ import ( "github.com/rafaelespinoza/alf" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) var ( @@ -104,7 +105,7 @@ Examples: PrePerform: func(_ context.Context) error { // Look for config file and if present, merge those values with // input flag values. - var conf godfish.Config + var conf internal.Config if data, ierr := os.ReadFile(pathToConfig); ierr != nil { // probably no config file present, rely on arguments instead. } else if ierr = json.Unmarshal(data, &conf); ierr != nil { diff --git a/internal/cmd/create-migration.go b/internal/cmd/create-migration.go index 40b7b57..70721d8 100644 --- a/internal/cmd/create-migration.go +++ b/internal/cmd/create-migration.go @@ -8,6 +8,7 @@ import ( "github.com/rafaelespinoza/alf" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) func makeCreateMigration(subcmdName string) alf.Directive { @@ -37,13 +38,13 @@ func makeCreateMigration(subcmdName string) alf.Directive { flags.StringVar( &fwdlabelValue, fwdlabelFlagname, - godfish.ForwardDirections[0], + internal.ForwardDirections[0], "customize the directional part of the filename for forward migration", ) flags.StringVar( &revlabelValue, revlabelFlagname, - godfish.ReverseDirections[0], + internal.ReverseDirections[0], "customize the directional part of the filename for reverse migration", ) flags.Usage = func() { @@ -59,9 +60,9 @@ func makeCreateMigration(subcmdName string) alf.Directive { - %s - %s `, - bin, subcmdName, subcmdName, godfish.TimeFormat, + bin, subcmdName, subcmdName, internal.TimeFormat, fwdlabelFlagname, revlabelFlagname, - strings.Join(godfish.ForwardDirections, ", "), strings.Join(godfish.ReverseDirections, ", "), + strings.Join(internal.ForwardDirections, ", "), strings.Join(internal.ReverseDirections, ", "), ) printFlagDefaults(&p) printFlagDefaults(flags) @@ -90,37 +91,7 @@ func makeCreateMigration(subcmdName string) alf.Directive { revlabelValue = commonArgs.DefaultRevLabel } - // Should also consider values from the config file, so validate - // after allowing it the opportunity to set variable values. - if err := validateDirectionLabel(godfish.ForwardDirections, fwdlabelFlagname, fwdlabelValue); err != nil { - return err - } - if err := validateDirectionLabel(godfish.ReverseDirections, revlabelFlagname, revlabelValue); err != nil { - return err - } - - migration, err := godfish.NewMigrationParams(migrationName, reversible, commonArgs.Files, fwdlabelValue, revlabelValue) - if err != nil { - return err - } - return migration.GenerateFiles() + return godfish.CreateMigrationFiles(migrationName, reversible, commonArgs.Files, fwdlabelValue, revlabelValue) }, } } - -func validateDirectionLabel(okVals []string, flagName, flagVal string) (err error) { - var ok bool - for _, okVal := range okVals { - if flagVal == okVal { - ok = true - break - } - } - if !ok { - err = fmt.Errorf( - "invalid value (%q) for flag %q; should be one of: %s", - flagVal, flagName, strings.Join(okVals, ", "), - ) - } - return -} diff --git a/internal/cmd/info.go b/internal/cmd/info.go index a4f24ba..c5ced7a 100644 --- a/internal/cmd/info.go +++ b/internal/cmd/info.go @@ -4,13 +4,12 @@ import ( "context" "flag" "fmt" - "io" "os" "strings" "github.com/rafaelespinoza/alf" "github.com/rafaelespinoza/godfish" - "github.com/rafaelespinoza/godfish/internal/info" + "github.com/rafaelespinoza/godfish/internal" ) func makeInfo(name string) alf.Directive { @@ -36,7 +35,7 @@ func makeInfo(name string) alf.Directive { &version, "version", "", - fmt.Sprintf("timestamp of migration, format: %s", godfish.TimeFormat), + fmt.Sprintf("timestamp of migration, format: %s", internal.TimeFormat), ) flags.Usage = func() { fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags] @@ -63,31 +62,17 @@ func makeInfo(name string) alf.Directive { return flags }, Run: func(_ context.Context) error { - direction := whichDirection(direction) - printer := choosePrinter(format, os.Stdout) - return godfish.Info(theDriver, commonArgs.Files, direction, version, printer) + return godfish.Info(theDriver, commonArgs.Files, forward(direction), version, os.Stdout, format) }, } } -func whichDirection(input string) (direction godfish.Direction) { - direction = godfish.DirForward +func forward(input string) bool { d := strings.ToLower(input) - if strings.HasPrefix(d, "rev") || strings.HasPrefix(d, "back") { - direction = godfish.DirReverse + for _, prefix := range []string{"rev", "roll", "back", "down"} { + if strings.HasPrefix(d, prefix) { + return false + } } - return -} - -func choosePrinter(format string, w io.Writer) (printer godfish.InfoPrinter) { - if format == "json" { - printer = info.NewJSON(w) - return - } - - if format != "tsv" && format != "" { - fmt.Fprintf(os.Stderr, "unknown format %q, defaulting to tsv\n", format) - } - printer = info.NewTSV(w) - return + return true } diff --git a/internal/cmd/migrate.go b/internal/cmd/migrate.go index dfa7fe2..1c3b531 100644 --- a/internal/cmd/migrate.go +++ b/internal/cmd/migrate.go @@ -7,6 +7,7 @@ import ( "github.com/rafaelespinoza/alf" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) func makeMigrate(name string) alf.Directive { @@ -20,7 +21,7 @@ func makeMigrate(name string) alf.Directive { &version, "version", "", - fmt.Sprintf("timestamp of migration, format: %s", godfish.TimeFormat), + fmt.Sprintf("timestamp of migration, format: %s", internal.TimeFormat), ) flags.Usage = func() { fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags] @@ -32,7 +33,7 @@ func makeMigrate(name string) alf.Directive { The "files" flag can specify the path to a directory with migration files. `, - bin, name, name, godfish.TimeFormat, + bin, name, name, internal.TimeFormat, ) printFlagDefaults(&p) printFlagDefaults(flags) @@ -44,7 +45,7 @@ func makeMigrate(name string) alf.Directive { err := godfish.Migrate( theDriver, commonArgs.Files, - godfish.DirForward, + true, version, ) return err @@ -73,11 +74,11 @@ func makeRemigrate(name string) alf.Directive { return flags }, Run: func(_ context.Context) error { - err := godfish.ApplyMigration(theDriver, commonArgs.Files, godfish.DirReverse, "") + err := godfish.ApplyMigration(theDriver, commonArgs.Files, false, "") if err != nil { return err } - return godfish.ApplyMigration(theDriver, commonArgs.Files, godfish.DirForward, "") + return godfish.ApplyMigration(theDriver, commonArgs.Files, true, "") }, } } @@ -93,7 +94,7 @@ func makeRollback(name string) alf.Directive { &version, "version", "", - fmt.Sprintf("timestamp of migration, format: %s", godfish.TimeFormat), + fmt.Sprintf("timestamp of migration, format: %s", internal.TimeFormat), ) flags.Usage = func() { fmt.Printf(`Usage: %s [godfish-flags] %s [%s-flags] @@ -105,7 +106,7 @@ func makeRollback(name string) alf.Directive { The "files" flag can specify the path to a directory with migration files. `, - bin, name, name, godfish.TimeFormat, + bin, name, name, internal.TimeFormat, ) printFlagDefaults(&p) printFlagDefaults(flags) @@ -118,14 +119,14 @@ func makeRollback(name string) alf.Directive { err = godfish.ApplyMigration( theDriver, commonArgs.Files, - godfish.DirReverse, + false, version, ) } else { err = godfish.Migrate( theDriver, commonArgs.Files, - godfish.DirReverse, + false, version, ) } diff --git a/internal/cmd/version.go b/internal/cmd/version.go index c20b647..3ea6306 100644 --- a/internal/cmd/version.go +++ b/internal/cmd/version.go @@ -7,7 +7,17 @@ import ( "fmt" "github.com/rafaelespinoza/alf" - "github.com/rafaelespinoza/godfish/internal/version" +) + +// Pieces of version metadata that can be set through -ldflags at build time. +// TODO: Look into embedding this info when building with golang >= v1.18. +var ( + versionBranchName string + versionBuildTime string + versionDriver string + versionCommitHash string + versionGoVersion string + versionTag string ) func makeVersion(name string) alf.Directive { @@ -31,22 +41,22 @@ func makeVersion(name string) alf.Directive { }, Run: func(_ context.Context) error { if !formatJSON { - fmt.Printf("BranchName: %s\n", version.BranchName) - fmt.Printf("BuildTime: %s\n", version.BuildTime) - fmt.Printf("Driver: %s\n", version.Driver) - fmt.Printf("CommitHash: %s\n", version.CommitHash) - fmt.Printf("GoVersion: %s\n", version.GoVersion) - fmt.Printf("Tag: %s\n", version.Tag) + fmt.Printf("BranchName: %s\n", versionBranchName) + fmt.Printf("BuildTime: %s\n", versionBuildTime) + fmt.Printf("Driver: %s\n", versionDriver) + fmt.Printf("CommitHash: %s\n", versionCommitHash) + fmt.Printf("GoVersion: %s\n", versionGoVersion) + fmt.Printf("Tag: %s\n", versionTag) return nil } out, err := json.Marshal( map[string]string{ - "BranchName": version.BranchName, - "BuildTime": version.BuildTime, - "Driver": version.Driver, - "CommitHash": version.CommitHash, - "GoVersion": version.GoVersion, - "Tag": version.Tag, + "BranchName": versionBranchName, + "BuildTime": versionBuildTime, + "Driver": versionDriver, + "CommitHash": versionCommitHash, + "GoVersion": versionGoVersion, + "Tag": versionTag, }, ) if err != nil { diff --git a/direction.go b/internal/direction.go similarity index 78% rename from direction.go rename to internal/direction.go index 09e99dc..e58efc9 100644 --- a/direction.go +++ b/internal/direction.go @@ -1,6 +1,9 @@ -package godfish +package internal -import "strings" +import ( + "fmt" + "strings" +) // Direction describes which way the change goes. type Direction uint8 @@ -34,6 +37,23 @@ var ( } ) +func validateDirectionLabel(okVals []string, val string) (err error) { + var ok bool + for _, okVal := range okVals { + if val == okVal { + ok = true + break + } + } + if !ok { + err = fmt.Errorf( + "invalid value (%q); should be one of: %s", + val, strings.Join(okVals, ", "), + ) + } + return +} + // Indirection is some glue to help determine the direction of a migration, // usually from a filename with an alias for a direction. type Indirection struct { diff --git a/filename.go b/internal/filename.go similarity index 70% rename from filename.go rename to internal/filename.go index 7e97b5b..6944a9f 100644 --- a/filename.go +++ b/internal/filename.go @@ -1,16 +1,16 @@ -package godfish +package internal import "strings" const filenameDelimeter = "-" -// filename is just a string with a specific format to migration files. One part +// Filename is just a string with a specific format to migration files. One part // has a generated timestamp, one part has a direction, another has a label. -type filename string +type Filename string -// makeFilename creates a filename based on the independent parts. Format: +// MakeFilename creates a filename based on the independent parts. Format: // "${direction}-${version}-${label}.sql" -func makeFilename(version string, indirection Indirection, label string) string { +func MakeFilename(version string, indirection Indirection, label string) string { var dir string if indirection.Value == DirUnknown { dir = "*" + filenameDelimeter diff --git a/filename_test.go b/internal/filename_test.go similarity index 74% rename from filename_test.go rename to internal/filename_test.go index 18ea7f0..8c2c91d 100644 --- a/filename_test.go +++ b/internal/filename_test.go @@ -1,4 +1,4 @@ -package godfish +package internal import ( "testing" @@ -9,69 +9,69 @@ func TestFilename(t *testing.T) { version string direction Indirection label string - expOut filename + expOut Filename }{ { version: "20191118121314", direction: Indirection{Value: DirForward, Label: "forward"}, label: "test", - expOut: filename("forward-20191118121314-test.sql"), + expOut: Filename("forward-20191118121314-test.sql"), }, { version: "20191118121314", direction: Indirection{Value: DirReverse, Label: "reverse"}, label: "test", - expOut: filename("reverse-20191118121314-test.sql"), + expOut: Filename("reverse-20191118121314-test.sql"), }, // timestamp too long { version: "201911181213141516", direction: Indirection{Value: DirForward, Label: "forward"}, label: "test", - expOut: filename("forward-20191118121314-test.sql"), + expOut: Filename("forward-20191118121314-test.sql"), }, // timestamp too short { version: "1234", direction: Indirection{Value: DirForward, Label: "forward"}, label: "test", - expOut: filename("forward-1234-test.sql"), + expOut: Filename("forward-1234-test.sql"), }, // label has dashes { version: "20191118121314", direction: Indirection{Value: DirForward, Label: "forward"}, label: "foo-bar", - expOut: filename("forward-20191118121314-foo-bar.sql"), + expOut: Filename("forward-20191118121314-foo-bar.sql"), }, // alternative names { direction: Indirection{Value: DirForward, Label: "migrate"}, version: "20191118121314", label: "test", - expOut: filename("migrate-20191118121314-test.sql"), + expOut: Filename("migrate-20191118121314-test.sql"), }, { direction: Indirection{Value: DirForward, Label: "up"}, version: "20191118121314", label: "test", - expOut: filename("up-20191118121314-test.sql"), + expOut: Filename("up-20191118121314-test.sql"), }, { direction: Indirection{Value: DirReverse, Label: "rollback"}, version: "20191118121314", label: "test", - expOut: filename("rollback-20191118121314-test.sql"), + expOut: Filename("rollback-20191118121314-test.sql"), }, { direction: Indirection{Value: DirReverse, Label: "down"}, version: "20191118121314", label: "test", - expOut: filename("down-20191118121314-test.sql"), + expOut: Filename("down-20191118121314-test.sql"), }, } for i, test := range tests { - out := makeFilename(test.version, test.direction, test.label) + out := MakeFilename(test.version, test.direction, test.label) if out != string(test.expOut) { t.Errorf( "test %d; wrong filename; got %q, expected %q", @@ -82,7 +82,7 @@ func TestFilename(t *testing.T) { } func mustMakeMigration(version string, indirection Indirection, label string) Migration { - ver, err := parseVersion(version) + ver, err := ParseVersion(version) if err != nil { panic(err) } @@ -95,12 +95,12 @@ func mustMakeMigration(version string, indirection Indirection, label string) Mi func TestParseMigration(t *testing.T) { tests := []struct { - filename filename + filename Filename expOut Migration expErr bool }{ { - filename: filename("forward-20191118121314-test.sql"), + filename: Filename("forward-20191118121314-test.sql"), expOut: mustMakeMigration( "20191118121314", Indirection{Value: DirForward, Label: "forward"}, @@ -108,59 +108,59 @@ func TestParseMigration(t *testing.T) { ), }, { - filename: filename("reverse-20191118121314-test.sql"), + filename: Filename("reverse-20191118121314-test.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirReverse}, "test"), }, // no extension { - filename: filename("forward-20191118121314-test"), + filename: Filename("forward-20191118121314-test"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirForward}, "test"), }, // timestamp too long { - filename: filename("forward-201911181213141516-test.sql"), + filename: Filename("forward-201911181213141516-test.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirForward}, "516-test"), }, // timestamp short { - filename: filename("forward-1234-test.sql"), + filename: Filename("forward-1234-test.sql"), expOut: mustMakeMigration("1234", Indirection{Value: DirForward}, "test"), }, // unknown direction - {filename: filename("foo-20191118121314-bar.sql"), expErr: true}, + {filename: Filename("foo-20191118121314-bar.sql"), expErr: true}, // just bad - {filename: filename("foo-bar"), expErr: true}, + {filename: Filename("foo-bar"), expErr: true}, // label has a delimiter { - filename: filename("forward-20191118121314-foo-bar.sql"), + filename: Filename("forward-20191118121314-foo-bar.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirForward}, "foo-bar"), }, // alternative names for directions { - filename: filename("migrate-20191118121314-test.sql"), + filename: Filename("migrate-20191118121314-test.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirForward, Label: "migration"}, "test"), }, { - filename: filename("up-20191118121314-test.sql"), + filename: Filename("up-20191118121314-test.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirForward, Label: "up"}, "test"), }, { - filename: filename("rollback-20191118121314-test.sql"), + filename: Filename("rollback-20191118121314-test.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirReverse, Label: "rollback"}, "test"), }, { - filename: filename("down-20191118121314-test.sql"), + filename: Filename("down-20191118121314-test.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirReverse, Label: "down"}, "test"), }, // unix timestamp (seconds) as version { - filename: filename("forward-1574079194-test.sql"), + filename: Filename("forward-1574079194-test.sql"), expOut: mustMakeMigration("20191118121314", Indirection{Value: DirForward, Label: "forward"}, "test"), }, } for i, test := range tests { - actual, err := parseMigration(test.filename) + actual, err := ParseMigration(test.filename) if !test.expErr && err != nil { t.Errorf("test %d; %v", i, err) continue diff --git a/internal/info.go b/internal/info.go new file mode 100644 index 0000000..6f9b0e5 --- /dev/null +++ b/internal/info.go @@ -0,0 +1,39 @@ +package internal + +import ( + "fmt" + "io" +) + +// InfoPrinter outputs the state of one migration. +type InfoPrinter interface { + PrintInfo(state string, migration Migration) error +} + +// NewTSV constructs an InfoPrinter to write out tab separated values. +func NewTSV(w io.Writer) InfoPrinter { return &tsvPrinter{w} } + +// NewJSON constructs an InfoPrinter to write out JSON. +func NewJSON(w io.Writer) InfoPrinter { return &jsonPrinter{w} } + +type tsvPrinter struct{ w io.Writer } +type jsonPrinter struct{ w io.Writer } + +func (p *tsvPrinter) PrintInfo(state string, mig Migration) (e error) { + _, e = fmt.Fprintf( + p.w, + "%s\t%s\t%s\n", + state, mig.Version().String(), MakeMigrationFilename(mig), + ) + return +} + +func (p *jsonPrinter) PrintInfo(state string, mig Migration) (e error) { + _, e = fmt.Fprintf( + p.w, + `{"state":%q,"version":%q,"filename":%q} +`, // delimit each migration by a newline. + state, mig.Version().String(), MakeMigrationFilename(mig), + ) + return +} diff --git a/internal/info/info.go b/internal/info/info.go deleted file mode 100644 index c25d09a..0000000 --- a/internal/info/info.go +++ /dev/null @@ -1,33 +0,0 @@ -package info - -import ( - "fmt" - "io" - - "github.com/rafaelespinoza/godfish" -) - -func NewTSV(w io.Writer) godfish.InfoPrinter { return &tsvPrinter{w} } -func NewJSON(w io.Writer) godfish.InfoPrinter { return &jsonPrinter{w} } - -type tsvPrinter struct{ w io.Writer } -type jsonPrinter struct{ w io.Writer } - -func (p *tsvPrinter) PrintInfo(state string, mig godfish.Migration) (e error) { - _, e = fmt.Fprintf( - p.w, - "%s\t%s\t%s\n", - state, mig.Version().String(), godfish.MakeMigrationFilename(mig), - ) - return -} - -func (p *jsonPrinter) PrintInfo(state string, mig godfish.Migration) (e error) { - _, e = fmt.Fprintf( - p.w, - `{"state":%q,"version":%q,"filename":%q} -`, // delimit each migration by a newline. - state, mig.Version().String(), godfish.MakeMigrationFilename(mig), - ) - return -} diff --git a/internal/info/info_test.go b/internal/info_test.go similarity index 81% rename from internal/info/info_test.go rename to internal/info_test.go index 3e644b1..344e7e7 100644 --- a/internal/info/info_test.go +++ b/internal/info_test.go @@ -1,4 +1,4 @@ -package info_test +package internal_test import ( "bytes" @@ -10,8 +10,7 @@ import ( "strings" "testing" - "github.com/rafaelespinoza/godfish" - "github.com/rafaelespinoza/godfish/internal/info" + "github.com/rafaelespinoza/godfish/internal" "github.com/rafaelespinoza/godfish/internal/stub" ) @@ -19,7 +18,7 @@ func TestTSV(t *testing.T) { var buf bytes.Buffer names := []string{"alfa", "bravo", "charlie", "delta"} - if err := printMigrations(info.NewTSV(&buf), "up", mustMakeMigrations(names...)); err != nil { + if err := printMigrations(internal.NewTSV(&buf), "up", mustMakeMigrations(names...)); err != nil { t.Fatal(err) } @@ -63,7 +62,7 @@ func TestJSON(t *testing.T) { var buf bytes.Buffer names := []string{"alfa", "bravo", "charlie", "delta"} - if err := printMigrations(info.NewJSON(&buf), "up", mustMakeMigrations(names...)); err != nil { + if err := printMigrations(internal.NewJSON(&buf), "up", mustMakeMigrations(names...)); err != nil { t.Fatal(err) } @@ -106,26 +105,26 @@ func TestJSON(t *testing.T) { } } -func mustMakeMigrations(names ...string) []godfish.Migration { +func mustMakeMigrations(names ...string) []internal.Migration { dir, err := os.MkdirTemp(os.TempDir(), "godfish_test_*") if err != nil { panic(err) } - out := make([]godfish.Migration, len(names)) + out := make([]internal.Migration, len(names)) for i := 0; i < len(names); i++ { - params, err := godfish.NewMigrationParams(names[i], false, dir, "", "") + params, err := internal.NewMigrationParams(names[i], false, dir, "forward", "reverse") if err != nil { panic(err) } version := stub.NewVersion(strconv.Itoa((i + 1) * 1000)) - out[i] = stub.NewMigration(params.Forward, version, godfish.Indirection{}) + out[i] = stub.NewMigration(params.Forward, version, internal.Indirection{}) } return out } -func printMigrations(p godfish.InfoPrinter, state string, migrations []godfish.Migration) (err error) { +func printMigrations(p internal.InfoPrinter, state string, migrations []internal.Migration) (err error) { for i, mig := range migrations { if err = p.PrintInfo(state, mig); err != nil { err = fmt.Errorf("%w; item %d", err, i) diff --git a/internal/internal.go b/internal/internal.go new file mode 100644 index 0000000..e89e39d --- /dev/null +++ b/internal/internal.go @@ -0,0 +1,19 @@ +// Package internal defines common functionality available within the library. +package internal + +import ( + "errors" +) + +// Config is for various runtime settings. +type Config struct { + PathToFiles string `json:"path_to_files"` + ForwardLabel string `json:"forward_label"` + ReverseLabel string `json:"reverse_label"` +} + +// General error values to help shape behavior. +var ( + ErrNotFound = errors.New("not found") + ErrDataInvalid = errors.New("data invalid") +) diff --git a/migration.go b/internal/migration.go similarity index 68% rename from migration.go rename to internal/migration.go index dacb134..1eef962 100644 --- a/migration.go +++ b/internal/migration.go @@ -1,4 +1,4 @@ -package godfish +package internal import ( "fmt" @@ -24,30 +24,29 @@ type mutation struct { version Version } -var _ Migration = (*mutation)(nil) - func (m *mutation) Indirection() Indirection { return m.indirection } func (m *mutation) Label() string { return m.label } func (m *mutation) Version() Version { return m.version } -func parseMigration(name filename) (mig Migration, err error) { +// ParseMigration constructs a Migration from a Filename. +func ParseMigration(name Filename) (mig Migration, err error) { basename := filepath.Base(string(name)) indirection := parseIndirection(basename) if indirection.Value == DirUnknown { err = fmt.Errorf( "%w; could not parse Direction for filename %q", - errDataInvalid, name, + ErrDataInvalid, name, ) return } // index of the start of timestamp i := len(indirection.Label) + len(filenameDelimeter) - version, err := parseVersion(basename) + version, err := ParseVersion(basename) if err != nil { err = fmt.Errorf( "%w, could not parse version for filename %q; %v", - errDataInvalid, version, err, + ErrDataInvalid, version, err, ) return } @@ -71,44 +70,28 @@ type MigrationParams struct { Reverse Migration Reversible bool Dirpath string - directory *os.File } -// NewMigrationParams constructs a MigrationParams that's ready to use. Passing -// in true for reversible means that a complementary SQL file will be made for -// rolling back. The dirpath is the path to the directory for the files. An -// error is returned if dirpath doesn't actually represent a directory. Names -// for directions in the filename could be overridden from their default values -// (forward and reverse) with the input vars fwdLabel, revLabel when non-empty. -func NewMigrationParams(name string, reversible bool, dirpath, fwdLabel, revLabel string) (*MigrationParams, error) { - var ( - err error - info os.FileInfo - directory *os.File - now time.Time - version timestamp - ) - - if directory, err = os.Open(dirpath); err != nil { - return nil, err - } - defer directory.Close() - if info, err = directory.Stat(); err != nil { - return nil, err - } else if !info.IsDir() { - return nil, fmt.Errorf("dirpath %q should be a path to a directory", dirpath) - } - - now = time.Now().UTC() - version = timestamp{value: now.Unix(), label: now.Format(TimeFormat)} - +// NewMigrationParams constructs a MigrationParams that's ready to use. +func NewMigrationParams(name string, reversible bool, dirpath, fwdLabel, revLabel string) (out *MigrationParams, err error) { if fwdLabel == "" { fwdLabel = ForwardDirections[0] } + if err = validateDirectionLabel(ForwardDirections, fwdLabel); err != nil { + return + } + if revLabel == "" { revLabel = ReverseDirections[0] } - return &MigrationParams{ + if err = validateDirectionLabel(ReverseDirections, revLabel); err != nil { + return + } + + now := time.Now().UTC() + version := timestamp{value: now.Unix(), label: now.Format(TimeFormat)} + + out = &MigrationParams{ Reversible: reversible, Dirpath: dirpath, Forward: &mutation{ @@ -121,8 +104,8 @@ func NewMigrationParams(name string, reversible bool, dirpath, fwdLabel, revLabe label: name, version: &version, }, - directory: directory, - }, nil + } + return } // GenerateFiles creates the migration files. If the migration is reversible it @@ -131,24 +114,23 @@ func NewMigrationParams(name string, reversible bool, dirpath, fwdLabel, revLabe // when it's done. func (m *MigrationParams) GenerateFiles() (err error) { var forwardFile, reverseFile *os.File - defer func() { - forwardFile.Close() - reverseFile.Close() - }() - baseDir := m.directory.Name() - if forwardFile, err = newMigrationFile(m.Forward, baseDir); err != nil { + if forwardFile, err = newMigrationFile(m.Forward, m.Dirpath); err != nil { return } fmt.Println("created forward file:", forwardFile.Name()) + defer forwardFile.Close() + if !m.Reversible { fmt.Println("migration marked irreversible, did not create reverse file") return } - if reverseFile, err = newMigrationFile(m.Reverse, baseDir); err != nil { + + if reverseFile, err = newMigrationFile(m.Reverse, m.Dirpath); err != nil { return } fmt.Println("created reverse file:", reverseFile.Name()) + defer reverseFile.Close() return } @@ -158,7 +140,7 @@ func newMigrationFile(m Migration, baseDir string) (*os.File, error) { // MakeMigrationFilename converts a Migration m to a filename. func MakeMigrationFilename(m Migration) string { - return makeFilename( + return MakeFilename( m.Version().String(), m.Indirection(), m.Label(), diff --git a/internal/migration_test.go b/internal/migration_test.go new file mode 100644 index 0000000..0d422ed --- /dev/null +++ b/internal/migration_test.go @@ -0,0 +1,183 @@ +package internal_test + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/rafaelespinoza/godfish/internal" +) + +const baseTestOutputDir = "/tmp/godfish_test" + +func TestMigrationParams(t *testing.T) { + testOutputDir := baseTestOutputDir + "/" + t.Name() + if err := os.MkdirAll(testOutputDir, 0755); err != nil { + t.Fatal(err) + } + + type testCase struct { + name string + reversible bool + dirpath string + fwdLabel, revLabel string + expectedDirections []string + expectError bool + } + + runTest := func(t *testing.T, test testCase) { + var ( + migParams *internal.MigrationParams + err error + filesAfter []os.DirEntry + ) + + // construct params and test the fields. + migParams, err = internal.NewMigrationParams(test.name, test.reversible, test.dirpath, test.fwdLabel, test.revLabel) + if err != nil { + t.Fatal(err) + } + if migParams.Forward.Indirection().Value != internal.DirForward { + t.Errorf( + "wrong Direction; expected %s, got %s", + internal.DirForward, migParams.Forward.Indirection().Value, + ) + } + if migParams.Reverse.Indirection().Value != internal.DirReverse { + t.Errorf( + "wrong Direction; expected %s, got %s", + internal.DirReverse, migParams.Reverse.Indirection().Value, + ) + } + for i, mig := range []internal.Migration{migParams.Forward, migParams.Reverse} { + if i > 0 && !test.reversible { + continue + } + if mig.Label() != test.name { + t.Errorf("test [%d]; Name should be unchanged", i) + } + if mig.Version().String() == "" { + t.Errorf("test [%d]; got empty Timestamp", i) + } + } + + // generate files and test effects. + err = migParams.GenerateFiles() + if err != nil && !test.expectError { + t.Fatal(err) + } else if err == nil && test.expectError { + t.Fatalf("expected an error but got %v", err) + } else if err != nil && test.expectError { + return // test passes, no more things to check. + } + + if filesAfter, err = os.ReadDir(test.dirpath); err != nil { + t.Fatal(err) + } + + if len(filesAfter) != len(test.expectedDirections) { + t.Fatalf( + "expected to generate %d files, got %d", + len(test.expectedDirections), len(filesAfter), + ) + } + + // Some golang platforms seem to have differing output orders for + // reading filenames in a directory. So, just sort them first. + sort.Slice(filesAfter, func(i, j int) bool { return filesAfter[i].Name() < filesAfter[j].Name() }) + + // Sorting this makes sense here as long as the generated filenames + // begin with the direction label. + sort.Strings(test.expectedDirections) + + for i, dirEntry := range filesAfter { + patt := fmt.Sprintf("%s-[0-9]*-%s.sql", test.expectedDirections[i], test.name) + name := dirEntry.Name() + if match, err := filepath.Match(patt, name); err != nil { + t.Fatalf("test [%d]; %v", i, err) + } else if !match { + t.Errorf( + "test [%d]; expected filename %q to match pattern %q", + i, name, patt, + ) + } + } + } + + t.Run("reversible", func(t *testing.T) { + dirpath, err := os.MkdirTemp(testOutputDir, "") + if err != nil { + t.Fatal(err) + } + runTest(t, testCase{ + name: "foo", + reversible: true, + dirpath: dirpath, + fwdLabel: "forward", + revLabel: "reverse", + expectedDirections: []string{"forward", "reverse"}, + }) + }) + + t.Run("forward only", func(t *testing.T) { + dirpath, err := os.MkdirTemp(testOutputDir, "") + if err != nil { + t.Fatal(err) + } + runTest(t, testCase{ + name: "bar", + reversible: false, + dirpath: dirpath, + fwdLabel: "forward", + expectedDirections: []string{"forward"}, + }) + }) + + t.Run("delimiter in the name", func(t *testing.T) { + dirpath, err := os.MkdirTemp(testOutputDir, "") + if err != nil { + t.Fatal(err) + } + runTest(t, testCase{ + name: "delimiter-in-the-name", + reversible: false, + dirpath: dirpath, + fwdLabel: "forward", + revLabel: "reverse", + expectedDirections: []string{"forward"}, + }) + }) + + t.Run("alternative direction names", func(t *testing.T) { + dirpath, err := os.MkdirTemp(testOutputDir, "") + if err != nil { + t.Fatal(err) + } + runTest(t, testCase{ + name: "alternatives", + reversible: true, + dirpath: dirpath, + fwdLabel: "up", + revLabel: "down", + expectedDirections: []string{"up", "down"}, + }) + }) + + t.Run("err", func(t *testing.T) { + dirpath, err := os.MkdirTemp(testOutputDir, "") + if err != nil { + t.Fatal(err) + } + + runTest(t, testCase{ + name: "bad", + reversible: false, + dirpath: dirpath + "/this_should_not_exist", + fwdLabel: "forward", + revLabel: "reverse", + expectError: true, + }) + }) +} diff --git a/internal/stub/appliedversions.go b/internal/stub/appliedversions.go index d23467c..ad1f2ed 100644 --- a/internal/stub/appliedversions.go +++ b/internal/stub/appliedversions.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) type appliedVersions struct { @@ -13,7 +14,7 @@ type appliedVersions struct { // NewAppliedVersions constructs an in-memory AppliedVersions implementation for // testing purposes. -func NewAppliedVersions(migrations ...godfish.Migration) godfish.AppliedVersions { +func NewAppliedVersions(migrations ...internal.Migration) godfish.AppliedVersions { out := appliedVersions{ versions: make([]string, len(migrations)), } diff --git a/internal/stub/driver.go b/internal/stub/driver.go index 178b4a9..d9f4cee 100644 --- a/internal/stub/driver.go +++ b/internal/stub/driver.go @@ -34,7 +34,7 @@ func (d *driver) Execute(q string, a ...interface{}) error { return d.errorOnExecute } -func (d *driver) UpdateSchemaMigrations(direction godfish.Direction, version string) error { +func (d *driver) UpdateSchemaMigrations(forward bool, version string) error { var stubbedAV *appliedVersions av, err := d.AppliedVersions() if err != nil { @@ -50,7 +50,7 @@ func (d *driver) UpdateSchemaMigrations(direction godfish.Direction, version str "if you assign anything to this field, make it a %T", stubbedAV, ) } - if direction == godfish.DirForward { + if forward { stubbedAV.versions = append(stubbedAV.versions, version) } else { for i, v := range stubbedAV.versions { diff --git a/internal/stub/migration.go b/internal/stub/migration.go index 71db750..d7fdf66 100644 --- a/internal/stub/migration.go +++ b/internal/stub/migration.go @@ -1,16 +1,18 @@ package stub -import "github.com/rafaelespinoza/godfish" +import ( + "github.com/rafaelespinoza/godfish/internal" +) type migration struct { - indirection godfish.Indirection + indirection internal.Indirection label string - version godfish.Version + version internal.Version } // NewMigration constructs a migration that can be used to override the version // field so that the generated filename is unique enough for testing purposes. -func NewMigration(mig godfish.Migration, version godfish.Version, ind godfish.Indirection) godfish.Migration { +func NewMigration(mig internal.Migration, version internal.Version, ind internal.Indirection) internal.Migration { stub := migration{ indirection: mig.Indirection(), label: mig.Label(), @@ -22,6 +24,6 @@ func NewMigration(mig godfish.Migration, version godfish.Version, ind godfish.In return &stub } -func (m *migration) Indirection() godfish.Indirection { return m.indirection } -func (m *migration) Label() string { return m.label } -func (m *migration) Version() godfish.Version { return m.version } +func (m *migration) Indirection() internal.Indirection { return m.indirection } +func (m *migration) Label() string { return m.label } +func (m *migration) Version() internal.Version { return m.version } diff --git a/internal/stub/version.go b/internal/stub/version.go index 21a7a52..39e1746 100644 --- a/internal/stub/version.go +++ b/internal/stub/version.go @@ -3,15 +3,15 @@ package stub import ( "strconv" - "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) type version string // NewVersion converts the input to a Version for testing purposes. -func NewVersion(v string) godfish.Version { return version(v) } +func NewVersion(v string) internal.Version { return version(v) } -func (v version) Before(u godfish.Version) bool { +func (v version) Before(u internal.Version) bool { w := u.(version) // potential panic intended, keep tests simple return string(v) < string(w) } diff --git a/internal/test/apply_migration.go b/internal/test/apply_migration.go index 64b6d9b..0599853 100644 --- a/internal/test/apply_migration.go +++ b/internal/test/apply_migration.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { @@ -19,7 +20,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { // testInput is passed to ApplyMigration. type testInput struct { // direction is the direction to migrate. - direction godfish.Direction + direction internal.Direction // version is where to go when calling ApplyMigration. version string } @@ -42,7 +43,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { } defer teardown(driver, pathToFiles, "foos", "bars") - err = godfish.ApplyMigration(driver, pathToFiles, input.direction, input.version) + err = godfish.ApplyMigration(driver, pathToFiles, input.direction == internal.DirForward, input.version) if err != nil && !expected.err { t.Errorf("could not apply migration; %v", err) return @@ -84,7 +85,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: []testDriverStub{}, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "", }, expectedOutput{ @@ -99,7 +100,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: []testDriverStub{}, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "", }, expectedOutput{ @@ -117,7 +118,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: defaultStubs, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "", }, expectedOutput{ @@ -132,7 +133,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: defaultStubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "", }, expectedOutput{ @@ -151,7 +152,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: defaultStubs, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "", }, expectedOutput{ @@ -166,7 +167,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: defaultStubs, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "", }, expectedOutput{ @@ -181,7 +182,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: defaultStubs, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "", }, expectedOutput{ @@ -199,7 +200,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: defaultStubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "23450102030405", }, expectedOutput{ @@ -214,7 +215,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: defaultStubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "", }, expectedOutput{ @@ -245,7 +246,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { }, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "", }, expectedOutput{ @@ -278,7 +279,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { }, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "34560102030405", }, expectedOutput{ @@ -313,7 +314,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "43210102030405", }, expectedOutput{ @@ -329,7 +330,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "43210102030405", }, expectedOutput{ @@ -347,7 +348,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "34560102030405", }, expectedOutput{ @@ -365,7 +366,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "12340102030405", }, expectedOutput{ @@ -391,7 +392,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { }), }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "45670102030405", }, expectedOutput{ @@ -422,7 +423,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { }, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "3456", }, expectedOutput{ @@ -454,7 +455,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { }, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "12340102030405", }, expectedOutput{ @@ -481,7 +482,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { }, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "12340102030405", }, expectedOutput{ @@ -496,9 +497,9 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { var stubs = []testDriverStub{ { content: queries.CreateFoos, - indirectives: struct{ forward, reverse godfish.Indirection }{ - forward: godfish.Indirection{Label: "migrate"}, - reverse: godfish.Indirection{Label: "rollback"}, + indirectives: struct{ forward, reverse internal.Indirection }{ + forward: internal.Indirection{Label: "migrate"}, + reverse: internal.Indirection{Label: "rollback"}, }, version: formattedTime("12340102030405"), }, @@ -511,7 +512,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "12340102030405", }, expectedOutput{ @@ -526,7 +527,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "12340102030405", }, expectedOutput{ @@ -539,9 +540,9 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { var stubs = []testDriverStub{ { content: queries.CreateFoos, - indirectives: struct{ forward, reverse godfish.Indirection }{ - forward: godfish.Indirection{Label: "up"}, - reverse: godfish.Indirection{Label: "down"}, + indirectives: struct{ forward, reverse internal.Indirection }{ + forward: internal.Indirection{Label: "up"}, + reverse: internal.Indirection{Label: "down"}, }, version: formattedTime("12340102030405"), }, @@ -554,7 +555,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirForward, + direction: internal.DirForward, version: "12340102030405", }, expectedOutput{ @@ -569,7 +570,7 @@ func testApplyMigration(t *testing.T, driver godfish.Driver, queries Queries) { stubs: stubs, }, testInput{ - direction: godfish.DirReverse, + direction: internal.DirReverse, version: "12340102030405", }, expectedOutput{ diff --git a/internal/test/info.go b/internal/test/info.go index 13968ca..a1145a4 100644 --- a/internal/test/info.go +++ b/internal/test/info.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/rafaelespinoza/godfish" - "github.com/rafaelespinoza/godfish/internal/info" + "github.com/rafaelespinoza/godfish/internal" ) func testInfo(t *testing.T, driver godfish.Driver, queries Queries) { @@ -31,21 +31,21 @@ func testInfo(t *testing.T, driver godfish.Driver, queries Queries) { defer teardown(driver, path, "foos", "bars") t.Run("forward", func(t *testing.T) { - err := godfish.Info(driver, path, godfish.DirForward, "", info.NewTSV(os.Stderr)) + err := godfish.Info(driver, path, true, "", os.Stderr, "tsv") if err != nil { t.Errorf( "could not output info in %s Direction; %v", - godfish.DirForward, err, + internal.DirForward, err, ) } }) t.Run("reverse", func(t *testing.T) { - err := godfish.Info(driver, path, godfish.DirReverse, "", info.NewJSON(os.Stderr)) + err := godfish.Info(driver, path, false, "", os.Stderr, "json") if err != nil { t.Errorf( "could not output info in %s Direction; %v", - godfish.DirReverse, err, + internal.DirReverse, err, ) } }) diff --git a/internal/test/migrate.go b/internal/test/migrate.go index 6c6cd4f..c7d28ab 100644 --- a/internal/test/migrate.go +++ b/internal/test/migrate.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" ) func testMigrate(t *testing.T, driver godfish.Driver, queries Queries) { @@ -31,9 +32,9 @@ func testMigrate(t *testing.T, driver godfish.Driver, queries Queries) { // affect other tests. defer teardown(driver, path, "foos", "bars") - err = godfish.Migrate(driver, path, godfish.DirForward, "") + err = godfish.Migrate(driver, path, true, "") if err != nil { - t.Errorf("could not Migrate in %s Direction; %v", godfish.DirForward, err) + t.Errorf("could not Migrate in %s Direction; %v", internal.DirForward, err) } appliedVersions, err := collectAppliedVersions(driver) @@ -46,9 +47,9 @@ func testMigrate(t *testing.T, driver godfish.Driver, queries Queries) { t.Error(err) } - err = godfish.Migrate(driver, path, godfish.DirReverse, "12340102030405") + err = godfish.Migrate(driver, path, false, "12340102030405") if err != nil { - t.Errorf("could not Migrate in %s Direction; %v", godfish.DirReverse, err) + t.Errorf("could not Migrate in %s Direction; %v", internal.DirReverse, err) } appliedVersions, err = collectAppliedVersions(driver) diff --git a/internal/test/test.go b/internal/test/test.go index 3a9e047..42057d9 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/rafaelespinoza/godfish" + "github.com/rafaelespinoza/godfish/internal" "github.com/rafaelespinoza/godfish/internal/stub" ) @@ -76,7 +77,7 @@ func setup(driver godfish.Driver, testName string, stubs []testDriverStub, migra return } if migrateTo != skipMigration { - err = godfish.Migrate(driver, path, godfish.DirForward, migrateTo) + err = godfish.Migrate(driver, path, true, migrateTo) } return } @@ -111,21 +112,18 @@ func teardown(driver godfish.Driver, path string, tablesToDrop ...string) { driver.Close() } -func formattedTime(v string) godfish.Version { return stub.NewVersion(v) } +func formattedTime(v string) internal.Version { return stub.NewVersion(v) } // testDriverStub encompasses some data to use with interface tests. type testDriverStub struct { - migration godfish.Migration + migration internal.Migration content MigrationContent - indirectives struct{ forward, reverse godfish.Indirection } - version godfish.Version + indirectives struct{ forward, reverse internal.Indirection } + version internal.Version } func generateMigrationFiles(pathToTestDir string, stubs []testDriverStub) error { for i, stub := range stubs { - var err error - var params *godfish.MigrationParams - var reversible bool if stub.content.Forward != "" && stub.content.Reverse != "" { reversible = true @@ -133,21 +131,23 @@ func generateMigrationFiles(pathToTestDir string, stubs []testDriverStub) error panic(fmt.Errorf("test setup should have content in forward direction")) } - if params, err = godfish.NewMigrationParams(strconv.Itoa(i), reversible, pathToTestDir, "", ""); err != nil { + fwd, rev := stub.indirectives.forward, stub.indirectives.reverse + params, err := internal.NewMigrationParams(strconv.Itoa(i), reversible, pathToTestDir, fwd.Label, rev.Label) + if err != nil { return err } - // replace migrations before generating files, to maintain control of + // replace migrations before generating files to maintain control of // the timestamps, filenames, and migration content. - params.Forward = newMigrationStub(params.Forward, stub.version, stub.indirectives.forward) + params.Forward = newMigrationStub(params.Forward, stub.version, fwd) if params.Reversible { - params.Reverse = newMigrationStub(params.Reverse, stub.version, stub.indirectives.reverse) + params.Reverse = newMigrationStub(params.Reverse, stub.version, rev) } if err = params.GenerateFiles(); err != nil { return err } - for j, mig := range []godfish.Migration{params.Forward, params.Reverse} { + for j, mig := range []internal.Migration{params.Forward, params.Reverse} { if j > 0 && !params.Reversible { continue } @@ -180,7 +180,7 @@ func generateMigrationFiles(pathToTestDir string, stubs []testDriverStub) error return nil } -func newMigrationStub(mig godfish.Migration, version godfish.Version, ind godfish.Indirection) godfish.Migration { +func newMigrationStub(mig internal.Migration, version internal.Version, ind internal.Indirection) internal.Migration { return stub.NewMigration(mig, version, ind) } diff --git a/version.go b/internal/version.go similarity index 82% rename from version.go rename to internal/version.go index 2544d25..e35e633 100644 --- a/version.go +++ b/internal/version.go @@ -1,4 +1,4 @@ -package godfish +package internal import ( "regexp" @@ -18,8 +18,6 @@ type timestamp struct { label string } -var _ Version = (*timestamp)(nil) - func (v *timestamp) Before(u Version) bool { // Until there's more than 1 interface implementation, this is fine. So, // panic here? Yeah, maybe. Fail loudly, not silently. @@ -43,14 +41,16 @@ const ( unixTimestampSecLen = len("1574079194") ) +// Some reasonable lower, upper limits for migration versions. var ( - minVersion = time.Date(0, 1, 1, 0, 0, 0, 0, time.UTC).Format(TimeFormat) - maxVersion = time.Date(9999, 12, 31, 23, 59, 59, 0, time.UTC).Format(TimeFormat) + MinVersion = time.Date(0, 1, 1, 0, 0, 0, 0, time.UTC).Format(TimeFormat) + MaxVersion = time.Date(9999, 12, 31, 23, 59, 59, 0, time.UTC).Format(TimeFormat) ) var timeformatMatcher = regexp.MustCompile(`\d{4,14}`) -func parseVersion(basename string) (version Version, err error) { +// ParseVersion extracts Version info from a file's basename. +func ParseVersion(basename string) (version Version, err error) { written := timeformatMatcher.FindString(basename) if ts, perr := time.Parse(TimeFormat, written); perr != nil { err = perr // keep going diff --git a/internal/version/version.go b/internal/version/version.go deleted file mode 100644 index c136d63..0000000 --- a/internal/version/version.go +++ /dev/null @@ -1,11 +0,0 @@ -package version - -// These are pieces of version metadata that can be set through -ldflags. -var ( - BranchName string - BuildTime string - Driver string - CommitHash string - GoVersion string - Tag string -) diff --git a/migration_test.go b/migration_test.go deleted file mode 100644 index bf66da4..0000000 --- a/migration_test.go +++ /dev/null @@ -1,168 +0,0 @@ -package godfish_test - -import ( - "fmt" - "os" - "path/filepath" - "sort" - "testing" - - "github.com/rafaelespinoza/godfish" -) - -func TestNewMigrationParams(t *testing.T) { - testOutputDir := baseTestOutputDir + "/" + t.Name() - if err := os.MkdirAll(testOutputDir, 0755); err != nil { - t.Fatal(err) - } - - type testCase struct { - name string - reversible bool - dirpath string - expectError bool - } - - runTest := func(t *testing.T, test testCase) { - t.Helper() - - migParams, err := godfish.NewMigrationParams(test.name, test.reversible, test.dirpath, "", "") - if err != nil && !test.expectError { - t.Fatal(err) - } else if err == nil && test.expectError { - t.Fatalf("expected an error but got %v", err) - } else if err != nil && test.expectError { - return // test passes, no more things to check. - } - - if migParams.Forward.Indirection().Value != godfish.DirForward { - t.Errorf( - "wrong Direction; expected %s, got %s", - godfish.DirForward, migParams.Forward.Indirection().Value, - ) - } - if migParams.Reverse.Indirection().Value != godfish.DirReverse { - t.Errorf( - "wrong Direction; expected %s, got %s", - godfish.DirReverse, migParams.Reverse.Indirection().Value, - ) - } - for i, mig := range []godfish.Migration{migParams.Forward, migParams.Reverse} { - if i > 0 && !test.reversible { - continue - } - if mig.Label() != test.name { - t.Errorf("test [%d]; Name should be unchanged", i) - } - if mig.Version().String() == "" { - t.Errorf("test [%d]; got empty Timestamp", i) - } - } - } - - runTest(t, testCase{name: "foo", reversible: true, dirpath: testOutputDir}) - runTest(t, testCase{name: "bar", reversible: false, dirpath: testOutputDir}) - runTest(t, testCase{ - name: "bad", - reversible: false, - dirpath: testOutputDir + "/this_should_not_exist", - expectError: true, - }) -} - -func TestMigrationParamsGenerateFiles(t *testing.T) { - testOutputDir := baseTestOutputDir + "/" + t.Name() - if err := os.MkdirAll(testOutputDir, 0755); err != nil { - t.Fatal(err) - } - - type testCase struct { - name string - reversible bool - fwdLabel, revLabel string - expectedDirections []string - } - - runTest := func(t *testing.T, test testCase) { - t.Helper() - - var err error - var outdirPath string - var migParams *godfish.MigrationParams - - if outdirPath, err = os.MkdirTemp(testOutputDir, ""); err != nil { - t.Fatal(err) - } - - migParams, err = godfish.NewMigrationParams(test.name, test.reversible, outdirPath, test.fwdLabel, test.revLabel) - if err != nil { - t.Fatal(err) - } - - var filesBefore, filesAfter []os.DirEntry - if filesBefore, err = os.ReadDir(outdirPath); err != nil { - t.Fatal(err) - } - if err = migParams.GenerateFiles(); err != nil { - t.Error(err) - } - if filesAfter, err = os.ReadDir(outdirPath); err != nil { - t.Fatal(err) - } - - // Some golang platforms seem to have differing output orders for - // reading filenames in a directory. So, just sort them first. - sort.Slice(filesBefore, func(i, j int) bool { return filesBefore[i].Name() < filesBefore[j].Name() }) - sort.Slice(filesAfter, func(i, j int) bool { return filesAfter[i].Name() < filesAfter[j].Name() }) - - // Sorting this makes sense here as long as the generated filenames - // begin with the direction label. - sort.Strings(test.expectedDirections) - - actualNumFiles := len(filesAfter) - len(filesBefore) - if actualNumFiles != len(test.expectedDirections) { - t.Fatalf( - "expected to generate %d files, got %d", - len(test.expectedDirections), actualNumFiles, - ) - } - - for i, dirEntry := range filesAfter { - patt := fmt.Sprintf("%s-[0-9]*-%s.sql", test.expectedDirections[i], test.name) - name := dirEntry.Name() - if match, err := filepath.Match(patt, name); err != nil { - t.Fatalf("test [%d]; %v", i, err) - } else if !match { - t.Errorf( - "test [%d]; expected filename %q to match pattern %q", - i, name, patt, - ) - } - } - } - - runTest(t, testCase{ - name: "foo", - reversible: true, - expectedDirections: []string{"forward", "reverse"}, - }) - runTest(t, testCase{ - name: "bar", - reversible: false, - expectedDirections: []string{"forward"}, - }) - - runTest(t, testCase{ - name: "delimiter-in-the-name", - reversible: false, - expectedDirections: []string{"forward"}, - }) - - runTest(t, testCase{ - name: "alternatives", - reversible: true, - fwdLabel: "up", - revLabel: "down", - expectedDirections: []string{"up", "down"}, - }) -}