GOMODULES
/
goose
mirror of https://github.com/pressly/goose.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: up and up -allow-missing behaviour (#524)

pull/527/head
Michael Fridman 2023-05-21 08:20:29 -04:00 committed by GitHub
parent 3717a9e315
commit 3d345c4b81
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 70 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
tests/e2e/allow_missing_test.go
View File

@ -315,6 +315,42 @@ func TestMigrateAllowMissingDown(t *testing.T) {
}
}
func TestWithWithoutAllowMissing(t *testing.T) {
// Test for https://github.com/pressly/goose/issues/521
// Apply 1,2,4,3 then run up without allow missing. If the next requested migration is
// 4 then it should not raise an error because it has already been applied.
// If goose attempts to apply 4 again then it will raise an error (SQLSTATE 42701) because it
// has already been applied. Specifically it will raise a error.
db := setupTestDB(t, 2)
migrations, err := goose.CollectMigrations(migrationsDir, 0, 4)
check.NoError(t, err)
err = migrations[3].Up(db) // version 4
check.NoError(t, err)
err = migrations[2].Up(db) // version 3
check.NoError(t, err)
err = goose.UpTo(db, migrationsDir, 4)
check.NoError(t, err)
err = goose.UpTo(db, migrationsDir, 4, goose.WithAllowMissing())
check.NoError(t, err)
// Rollback migration 3 because it is the last applied migration.
// But, we want to change this behaviour to apply rollback migration 4.
// See these issues for more details:
// https://github.com/pressly/goose/issues/523
// https://github.com/pressly/goose/issues/402
//
// Adding this test to ensure the behaviour is updated and captured in a test.
err = goose.Down(db, migrationsDir)
check.NoError(t, err)
version, err := goose.GetDBVersion(db)
check.NoError(t, err)
check.Number(t, version, 4)
}
// setupTestDB is helper to setup a DB and apply migrations
// up to the specified version.
func setupTestDB(t *testing.T, version int64) *sql.DB {

130
up.go
View File

@ -3,7 +3,6 @@ package goose
import (
"context"
"database/sql"
"errors"
"fmt"
"sort"
"strings"
@ -64,8 +63,14 @@ func UpTo(db *sql.DB, dir string, version int64, opts ...OptionsFunc) error {
if err != nil {
return err
}
dbMaxVersion := dbMigrations[len(dbMigrations)-1].Version
// lookupAppliedInDB is a map of all applied migrations in the database.
lookupAppliedInDB := make(map[int64]bool)
for _, m := range dbMigrations {
lookupAppliedInDB[m.Version] = true
}
missingMigrations := findMissingMigrations(dbMigrations, foundMigrations)
missingMigrations := findMissingMigrations(dbMigrations, foundMigrations, dbMaxVersion)
// feature(mf): It is very possible someone may want to apply ONLY new migrations
// and skip missing migrations altogether. At the moment this is not supported,
@ -79,37 +84,38 @@ func UpTo(db *sql.DB, dir string, version int64, opts ...OptionsFunc) error {
return fmt.Errorf("error: found %d missing migrations:\n\t%s",
len(missingMigrations), strings.Join(collected, "\n\t"))
}
var migrationsToApply Migrations
if option.allowMissing {
return upWithMissing(
db,
missingMigrations,
foundMigrations,
dbMigrations,
option,
)
migrationsToApply = missingMigrations
}
// filter all migrations with a version greater than the supplied version (min) and less than or
// equal to the requested version (max). Note, we do not need to filter out missing migrations
// because we are only appending "new" migrations that have a higher version than the current
// database max version, which inevitably means they are not "missing".
for _, m := range foundMigrations {
if lookupAppliedInDB[m.Version] {
continue
}
if m.Version > dbMaxVersion && m.Version <= version {
migrationsToApply = append(migrationsToApply, m)
}
}
var current int64
for {
var err error
current, err = GetDBVersion(db)
if err != nil {
return err
}
next, err := foundMigrations.Next(current)
if err != nil {
if errors.Is(err, ErrNoNextVersion) {
break
}
return fmt.Errorf("failed to find next migration: %v", err)
}
if err := next.Up(db); err != nil {
for _, m := range migrationsToApply {
if err := m.Up(db); err != nil {
return err
}
if option.applyUpByOne {
return nil
}
current = m.Version
}
if len(migrationsToApply) == 0 && option.applyUpByOne {
current, err = GetDBVersion(db)
if err != nil {
return err
}
}
// At this point there are no more migrations to apply. But we need to maintain
// the following behaviour:
@ -140,77 +146,6 @@ func upToNoVersioning(db *sql.DB, migrations Migrations, version int64) error {
return nil
}
func upWithMissing(
db *sql.DB,
missingMigrations Migrations,
foundMigrations Migrations,
dbMigrations Migrations,
option *options,
) error {
lookupApplied := make(map[int64]bool)
for _, found := range dbMigrations {
lookupApplied[found.Version] = true
}
// Apply all missing migrations first.
for _, missing := range missingMigrations {
if err := missing.Up(db); err != nil {
return err
}
// Apply one migration and return early.
if option.applyUpByOne {
return nil
}
// TODO(mf): do we need this check? It's a bit redundant, but we may
// want to keep it as a safe-guard. Maybe we should instead have
// the underlying query (if possible) return the current version as
// part of the same transaction.
current, err := GetDBVersion(db)
if err != nil {
return err
}
if current == missing.Version {
lookupApplied[missing.Version] = true
continue
}
return fmt.Errorf("error: missing migration:%d does not match current db version:%d",
current, missing.Version)
}
// We can no longer rely on the database version_id to be sequential because
// missing (out-of-order) migrations get applied before newer migrations.
for _, found := range foundMigrations {
// TODO(mf): instead of relying on this lookup, consider hitting
// the database directly?
// Alternatively, we can skip a bunch migrations and start the cursor
// at a version that represents 100% applied migrations. But this is
// risky, and we should aim to keep this logic simple.
if lookupApplied[found.Version] {
continue
}
if err := found.Up(db); err != nil {
return err
}
if option.applyUpByOne {
return nil
}
}
current, err := GetDBVersion(db)
if err != nil {
return err
}
// At this point there are no more migrations to apply. But we need to maintain
// the following behaviour:
// UpByOne returns an error to signifying there are no more migrations.
// Up and UpTo return nil
log.Printf("goose: no migrations to run. current version: %d\n", current)
if option.applyUpByOne {
return ErrNoNextVersion
}
return nil
}
// Up applies all available migrations.
func Up(db *sql.DB, dir string, opts ...OptionsFunc) error {
return UpTo(db, dir, maxVersion, opts...)
@ -246,15 +181,14 @@ func listAllDBVersions(ctx context.Context, db *sql.DB) (Migrations, error) {
// findMissingMigrations migrations returns all missing migrations.
// A migrations is considered missing if it has a version less than the
// current known max version.
func findMissingMigrations(knownMigrations, newMigrations Migrations) Migrations {
max := knownMigrations[len(knownMigrations)-1].Version
func findMissingMigrations(knownMigrations, newMigrations Migrations, dbMaxVersion int64) Migrations {
existing := make(map[int64]bool)
for _, known := range knownMigrations {
existing[known.Version] = true
}
var missing Migrations
for _, new := range newMigrations {
if !existing[new.Version] && new.Version < max {
if !existing[new.Version] && new.Version < dbMaxVersion {
missing = append(missing, new)
}
}

4
up_test.go
View File

@ -10,7 +10,7 @@ func TestFindMissingMigrations(t *testing.T) {
{Version: 3},
{Version: 4},
{Version: 5},
{Version: 7},
{Version: 7}, // <-- database max version_id
}
new := Migrations{
{Version: 1},
@ -22,7 +22,7 @@ func TestFindMissingMigrations(t *testing.T) {
{Version: 7}, // <-- database max version_id
{Version: 8}, // new migration
}
got := findMissingMigrations(known, new)
got := findMissingMigrations(known, new, 7)
if len(got) != 2 {
t.Fatalf("invalid migration count: got:%d want:%d", len(got), 2)
}