From b28d0532985dd113a550dcd7befb9505bc2970ba Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Thu, 12 Sep 2024 22:12:26 +0200 Subject: [PATCH 01/12] fix(migration): Looks for pending migrations --- app/pkg/dbx/migrate.go | 55 +++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index 9204c5cf7..f232f5941 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -63,20 +63,23 @@ func Migrate(ctx context.Context, path string) error { totalMigrationsExecuted := 0 + pendingVersions, err := getPendingMigrations(versions) + if err != nil { + return errors.Wrap(err, "failed to get pending migrations") + } + // Apply all migrations - for _, version := range versions { - if version > lastVersion { - fileName := versionFiles[version] - log.Infof(ctx, "Running Version: @{Version} (@{FileName})", dto.Props{ - "Version": version, - "FileName": fileName, - }) - err := runMigration(ctx, version, path, fileName) - if err != nil { - return errors.Wrap(err, "failed to run migration '%s'", fileName) - } - totalMigrationsExecuted++ + for _, version := range pendingVersions { + fileName := versionFiles[version] + log.Infof(ctx, "Running Version: @{Version} (@{FileName})", dto.Props{ + "Version": version, + "FileName": fileName, + }) + err := runMigration(ctx, version, path, fileName) + if err != nil { + return errors.Wrap(err, "failed to run migration '%s'", fileName) } + totalMigrationsExecuted++ } if totalMigrationsExecuted > 0 { @@ -140,3 +143,31 @@ func getLastMigration() (int, error) { return int(lastVersion.Int64), nil } + +func getPendingMigrations(versions []int) ([]int, error) { + pendingMigrations := make([]int, 0) + versionStr := strconv.Itoa(versions[0]) + + for _, version := range versions { + versionStr = versionStr + "," + strconv.Itoa(version) + } + + dbVersionMap := make(map[int]bool) + rows, err := conn.Query("SELECT version FROM migrations_history WHERE version IN (" + versionStr + ")") + if err != nil { + return nil, err + } + for rows.Next() { + var version int + _ = rows.Scan(&version) + dbVersionMap[version] = true + } + + for _, version := range versions { + if !dbVersionMap[version] { + pendingMigrations = append(pendingMigrations, version) + } + } + + return pendingMigrations, nil +} From 58af2c6953286c12bd18e59bb94c523fc1b2e329 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Fri, 13 Sep 2024 07:18:10 +0200 Subject: [PATCH 02/12] refactor: Rewrite versions string --- app/pkg/dbx/migrate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index f232f5941..7142b4950 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -146,11 +146,11 @@ func getLastMigration() (int, error) { func getPendingMigrations(versions []int) ([]int, error) { pendingMigrations := make([]int, 0) - versionStr := strconv.Itoa(versions[0]) - + versionStr := "" for _, version := range versions { versionStr = versionStr + "," + strconv.Itoa(version) } + versionStr = versionStr[1:] dbVersionMap := make(map[int]bool) rows, err := conn.Query("SELECT version FROM migrations_history WHERE version IN (" + versionStr + ")") From a4d839b3eff5e1756ac0733bdc7f38b1b4e7f43f Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Fri, 13 Sep 2024 07:26:50 +0200 Subject: [PATCH 03/12] refactor: Get pending versions using slices --- app/pkg/dbx/migrate.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index 7142b4950..31cf1277a 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -5,6 +5,7 @@ import ( "database/sql" stdErrors "errors" "os" + "slices" "sort" "strconv" "strings" @@ -145,29 +146,25 @@ func getLastMigration() (int, error) { } func getPendingMigrations(versions []int) ([]int, error) { - pendingMigrations := make([]int, 0) versionStr := "" for _, version := range versions { versionStr = versionStr + "," + strconv.Itoa(version) } versionStr = versionStr[1:] - dbVersionMap := make(map[int]bool) rows, err := conn.Query("SELECT version FROM migrations_history WHERE version IN (" + versionStr + ")") if err != nil { return nil, err } + for rows.Next() { var version int _ = rows.Scan(&version) - dbVersionMap[version] = true - } - - for _, version := range versions { - if !dbVersionMap[version] { - pendingMigrations = append(pendingMigrations, version) + i := slices.Index(versions, version) + if i != -1 { + versions = slices.Delete(versions, i, i+1) } } - return pendingMigrations, nil + return versions, nil } From bd2865a7b048a1665f90ddc90529fe6e0c66e9f2 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 10:49:24 +0200 Subject: [PATCH 04/12] refactor: Copy versions slice to local variable --- app/pkg/dbx/migrate.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index 31cf1277a..7ee0dcf99 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -146,8 +146,9 @@ func getLastMigration() (int, error) { } func getPendingMigrations(versions []int) ([]int, error) { + pendingMigrations := versions versionStr := "" - for _, version := range versions { + for _, version := range pendingMigrations { versionStr = versionStr + "," + strconv.Itoa(version) } versionStr = versionStr[1:] @@ -160,11 +161,11 @@ func getPendingMigrations(versions []int) ([]int, error) { for rows.Next() { var version int _ = rows.Scan(&version) - i := slices.Index(versions, version) + i := slices.Index(pendingMigrations, version) if i != -1 { - versions = slices.Delete(versions, i, i+1) + pendingMigrations = slices.Delete(pendingMigrations, i, i+1) } } - return versions, nil + return pendingMigrations, nil } From b27ac39b79ed498e766d830ca6f4e2ba92f2d754 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 10:58:29 +0200 Subject: [PATCH 05/12] fix: Use pq.Array as SQL query param --- app/pkg/dbx/migrate.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index 7ee0dcf99..9ae70b495 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" stdErrors "errors" + "github.com/lib/pq" "os" "slices" "sort" @@ -147,13 +148,8 @@ func getLastMigration() (int, error) { func getPendingMigrations(versions []int) ([]int, error) { pendingMigrations := versions - versionStr := "" - for _, version := range pendingMigrations { - versionStr = versionStr + "," + strconv.Itoa(version) - } - versionStr = versionStr[1:] - rows, err := conn.Query("SELECT version FROM migrations_history WHERE version IN (" + versionStr + ")") + rows, err := conn.Query("SELECT version FROM migrations_history WHERE version = ANY($1)", pq.Array(pendingMigrations)) if err != nil { return nil, err } From 5c4681534d37f0ae21596dd0763070ee0b41c624 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:03:42 +0200 Subject: [PATCH 06/12] fix: Close connection using defer statement --- app/pkg/dbx/migrate.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index 9ae70b495..e749a7472 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -153,6 +153,7 @@ func getPendingMigrations(versions []int) ([]int, error) { if err != nil { return nil, err } + defer rows.Close() for rows.Next() { var version int From 1bd3b80293bf27fe7a9a845d4e845e102d068810 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:06:32 +0200 Subject: [PATCH 07/12] fix: Handle errors during version scan --- app/pkg/dbx/migrate.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index e749a7472..a7941402d 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -157,7 +157,9 @@ func getPendingMigrations(versions []int) ([]int, error) { for rows.Next() { var version int - _ = rows.Scan(&version) + if err := rows.Scan(&version); err != nil { + return nil, errors.Wrap(err, "failed to scan version") + } i := slices.Index(pendingMigrations, version) if i != -1 { pendingMigrations = slices.Delete(pendingMigrations, i, i+1) From a2e4d9a6e5e3d6bda67938985acc3318d57d55f1 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:48:26 +0200 Subject: [PATCH 08/12] test: Ensure migration will be executed even with past timestamp --- app/pkg/dbx/migrate_test.go | 21 +++++++++++++++++++ .../209901010000_create.sql | 1 + .../210001010002_delete.sql | 1 + 3 files changed, 23 insertions(+) create mode 100644 app/pkg/dbx/testdata/migration_success_with_new_migrations/209901010000_create.sql create mode 100644 app/pkg/dbx/testdata/migration_success_with_new_migrations/210001010002_delete.sql diff --git a/app/pkg/dbx/migrate_test.go b/app/pkg/dbx/migrate_test.go index 7ee8cc504..8719cda5c 100644 --- a/app/pkg/dbx/migrate_test.go +++ b/app/pkg/dbx/migrate_test.go @@ -39,6 +39,27 @@ func TestMigrate_Success(t *testing.T) { trx.MustRollback() } +func TestMigrate_SuccessWithPastMigration(t *testing.T) { + setupMigrationTest(t) + ctx := context.Background() + + err := dbx.Migrate(ctx, "/app/pkg/dbx/testdata/migration_success") + Expect(err).IsNil() + + err = dbx.Migrate(ctx, "/app/pkg/dbx/testdata/migration_success_with_new_migrations") + Expect(err).IsNil() + + trx, _ := dbx.BeginTx(ctx) + var version string + err = trx.Scalar(&version, "SELECT version FROM migrations_history WHERE version = '209901010000' LIMIT 1") + Expect(version).Equals("209901010000") + var count int + err = trx.Scalar(&count, "SELECT COUNT(*) FROM migrations_history where VERSION IN (209901010000,210001010002)") + Expect(err).IsNil() + Expect(count).Equals(2) + trx.MustRollback() +} + func TestMigrate_Failure(t *testing.T) { setupMigrationTest(t) ctx := context.Background() diff --git a/app/pkg/dbx/testdata/migration_success_with_new_migrations/209901010000_create.sql b/app/pkg/dbx/testdata/migration_success_with_new_migrations/209901010000_create.sql new file mode 100644 index 000000000..6f9bf34f9 --- /dev/null +++ b/app/pkg/dbx/testdata/migration_success_with_new_migrations/209901010000_create.sql @@ -0,0 +1 @@ +insert into dummy (id, description) values (400, 'Description 400A'); \ No newline at end of file diff --git a/app/pkg/dbx/testdata/migration_success_with_new_migrations/210001010002_delete.sql b/app/pkg/dbx/testdata/migration_success_with_new_migrations/210001010002_delete.sql new file mode 100644 index 000000000..2655a0b24 --- /dev/null +++ b/app/pkg/dbx/testdata/migration_success_with_new_migrations/210001010002_delete.sql @@ -0,0 +1 @@ +DELETE FROM dummy WHERE id = 400; \ No newline at end of file From 267a72baa7859e3b26f54a2c9a38e10e84cb6cd0 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:55:03 +0200 Subject: [PATCH 09/12] lint: Fix typo in sql request in tests --- app/pkg/dbx/migrate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pkg/dbx/migrate_test.go b/app/pkg/dbx/migrate_test.go index 8719cda5c..464aa8709 100644 --- a/app/pkg/dbx/migrate_test.go +++ b/app/pkg/dbx/migrate_test.go @@ -54,7 +54,7 @@ func TestMigrate_SuccessWithPastMigration(t *testing.T) { err = trx.Scalar(&version, "SELECT version FROM migrations_history WHERE version = '209901010000' LIMIT 1") Expect(version).Equals("209901010000") var count int - err = trx.Scalar(&count, "SELECT COUNT(*) FROM migrations_history where VERSION IN (209901010000,210001010002)") + err = trx.Scalar(&count, "SELECT COUNT(*) FROM migrations_history WHERE version IN (209901010000,210001010002)") Expect(err).IsNil() Expect(count).Equals(2) trx.MustRollback() From fbff6971514d1c18a9443faec585b6c5e93c4a60 Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:57:27 +0200 Subject: [PATCH 10/12] test: Check error while fetching versions --- app/pkg/dbx/migrate_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/pkg/dbx/migrate_test.go b/app/pkg/dbx/migrate_test.go index 464aa8709..83676e2bf 100644 --- a/app/pkg/dbx/migrate_test.go +++ b/app/pkg/dbx/migrate_test.go @@ -52,6 +52,8 @@ func TestMigrate_SuccessWithPastMigration(t *testing.T) { trx, _ := dbx.BeginTx(ctx) var version string err = trx.Scalar(&version, "SELECT version FROM migrations_history WHERE version = '209901010000' LIMIT 1") + Expect(err).IsNil() + Expect(version).Equals("209901010000") var count int err = trx.Scalar(&count, "SELECT COUNT(*) FROM migrations_history WHERE version IN (209901010000,210001010002)") From 9486347122c2abfff02c4bcbb1687734286ce54e Mon Sep 17 00:00:00 2001 From: quentinchampenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:06:53 +0200 Subject: [PATCH 11/12] lint: Light refactor --- app/pkg/dbx/migrate_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/pkg/dbx/migrate_test.go b/app/pkg/dbx/migrate_test.go index 83676e2bf..183107bc6 100644 --- a/app/pkg/dbx/migrate_test.go +++ b/app/pkg/dbx/migrate_test.go @@ -53,12 +53,13 @@ func TestMigrate_SuccessWithPastMigration(t *testing.T) { var version string err = trx.Scalar(&version, "SELECT version FROM migrations_history WHERE version = '209901010000' LIMIT 1") Expect(err).IsNil() - Expect(version).Equals("209901010000") + var count int err = trx.Scalar(&count, "SELECT COUNT(*) FROM migrations_history WHERE version IN (209901010000,210001010002)") Expect(err).IsNil() Expect(count).Equals(2) + trx.MustRollback() } From 1bbe867b252d5ab8a4ff6101b94936312f4b35d5 Mon Sep 17 00:00:00 2001 From: Quentin Champenois <26109239+Quentinchampenois@users.noreply.github.com> Date: Sun, 15 Sep 2024 09:19:01 +0200 Subject: [PATCH 12/12] feat: Copy versions array to local var Co-authored-by: Matt Roberts --- app/pkg/dbx/migrate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pkg/dbx/migrate.go b/app/pkg/dbx/migrate.go index a7941402d..eebe43154 100644 --- a/app/pkg/dbx/migrate.go +++ b/app/pkg/dbx/migrate.go @@ -147,7 +147,7 @@ func getLastMigration() (int, error) { } func getPendingMigrations(versions []int) ([]int, error) { - pendingMigrations := versions + pendingMigrations := append([]int(nil), versions...) rows, err := conn.Query("SELECT version FROM migrations_history WHERE version = ANY($1)", pq.Array(pendingMigrations)) if err != nil {