diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index a4cbca70c1..b2103ca997 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -146,19 +146,29 @@ func Migrate(x *xorm.Engine) error { return fmt.Errorf("sync: %w", err) } - currentVersion := &ForgejoVersion{ID: 1} - has, err := x.Get(currentVersion) - if err != nil { - return fmt.Errorf("get: %w", err) - } else if !has { - // If the version record does not exist we think - // it is a fresh installation and we can skip all migrations. - currentVersion.ID = 0 - currentVersion.Version = ExpectedVersion() - - if _, err = x.InsertOne(currentVersion); err != nil { + var versionRecords []*ForgejoVersion + if err := x.Find(&versionRecords); err != nil { + return fmt.Errorf("find: %w", err) + } + if len(versionRecords) == 0 { + // If the version record does not exist we think it is a fresh installation and we can skip all migrations; + // engine init calls `SyncAllTables` which will create the fresh database. + upToDate := &ForgejoVersion{ID: 1, Version: ExpectedVersion()} + if _, err := x.InsertOne(upToDate); err != nil { return fmt.Errorf("insert: %w", err) } + // continue with the migration routine, but nothing will be applied; this allows transition into the newer + // forgejo_migration library and for it to be configured and populated. + versionRecords = []*ForgejoVersion{upToDate} + } else if len(versionRecords) > 1 { + return fmt.Errorf( + "corrupt migrations: Forgejo database has unexpected records in the table `forgejo_version`; a single record is expected w/ ID=1, but %d records were found", + len(versionRecords)) + } + currentVersion := versionRecords[0] + if currentVersion.ID != 1 { + return fmt.Errorf( + "corrupt migrations: Forgejo database has corrupted records in the table `forgejo_version`; a single record with ID=1 is expected, but a record with ID=%d was found instead", currentVersion.ID) } v := currentVersion.Version @@ -177,7 +187,7 @@ func Migrate(x *xorm.Engine) error { // Some migration tasks depend on the git command if git.DefaultContext == nil { - if err = git.InitSimple(context.Background()); err != nil { + if err := git.InitSimple(context.Background()); err != nil { return err } } @@ -187,11 +197,11 @@ func Migrate(x *xorm.Engine) error { log.Info("Migration[%d]: %s", v+int64(i), m.description) // Reset the mapper between each migration - migrations are not supposed to depend on each other x.SetMapper(names.GonicMapper{}) - if err = m.migrate(x); err != nil { + if err := m.migrate(x); err != nil { return fmt.Errorf("migration[%d]: %s failed: %w", v+int64(i), m.description, err) } currentVersion.Version = v + int64(i) + 1 - if _, err = x.ID(1).Update(currentVersion); err != nil { + if _, err := x.ID(1).Update(currentVersion); err != nil { return err } } diff --git a/models/forgejo_migrations/migrate_test.go b/models/forgejo_migrations/migrate_test.go index 20653929a3..cc7e4cea7e 100644 --- a/models/forgejo_migrations/migrate_test.go +++ b/models/forgejo_migrations/migrate_test.go @@ -8,6 +8,7 @@ import ( migration_tests "forgejo.org/models/migrations/test" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -37,3 +38,38 @@ func TestEnsureUpToDate(t *testing.T) { err = EnsureUpToDate(x) require.Error(t, err) } + +func TestMigrateFreshDB(t *testing.T) { + x, deferable := migration_tests.PrepareTestEnv(t, 0, new(ForgejoVersion)) + defer deferable() + require.NotNil(t, x) + + err := Migrate(x) + require.NoError(t, err) + + var versionRecords []*ForgejoVersion + err = x.Find(&versionRecords) + require.NoError(t, err) + require.Len(t, versionRecords, 1) + v := versionRecords[0] + assert.EqualValues(t, 1, v.ID) + assert.EqualValues(t, 28, v.Version) +} + +func TestMigrateFailWithCorruption(t *testing.T) { + x, deferable := migration_tests.PrepareTestEnv(t, 0, new(ForgejoVersion)) + defer deferable() + require.NotNil(t, x) + + // ID != 1 + _, err := x.InsertOne(&ForgejoVersion{ID: 100, Version: 100}) + require.NoError(t, err) + err = Migrate(x) + require.ErrorContains(t, err, "corrupted records in the table `forgejo_version`") + + // Two versions... + _, err = x.InsertOne(&ForgejoVersion{ID: 1, Version: 1000}) + require.NoError(t, err) + err = Migrate(x) + require.ErrorContains(t, err, "unexpected records in the table `forgejo_version`") +} diff --git a/models/migrations/main_test.go b/models/migrations/main_test.go new file mode 100644 index 0000000000..c1fa2af9b3 --- /dev/null +++ b/models/migrations/main_test.go @@ -0,0 +1,14 @@ +// Copyright 2023 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package migrations + +import ( + "testing" + + migration_tests "forgejo.org/models/migrations/test" +) + +func TestMain(m *testing.M) { + migration_tests.MainTest(m) +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 11933014d7..837e437e03 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -447,27 +447,38 @@ func Migrate(x *xorm.Engine) error { } var previousVersion int64 - currentVersion := &Version{ID: 1} - has, err := x.Get(currentVersion) - if err != nil { - return fmt.Errorf("get: %w", err) - } else if !has { - // If the version record does not exist, it is a fresh installation, and we can skip all migrations. - // XORM model framework will create all tables when initializing. - currentVersion.ID = 0 - currentVersion.Version = maxDBVer - if _, err = x.InsertOne(currentVersion); err != nil { + var versionRecords []*Version + if err := x.Find(&versionRecords); err != nil { + return fmt.Errorf("find: %w", err) + } + if len(versionRecords) == 0 { + // If the version record does not exist we think it is a fresh installation and we can skip all migrations; + // engine init calls `SyncAllTables` which will create the fresh database. + upToDate := &Version{ID: 1, Version: maxDBVer} + if _, err := x.InsertOne(upToDate); err != nil { return fmt.Errorf("insert: %w", err) } + // continue with the migration routine, but nothing will be applied; this allows transition into the newer + // forgejo library and for it to be configured and populated. + versionRecords = []*Version{upToDate} + } else if len(versionRecords) > 1 { + return fmt.Errorf( + "corrupt migrations: Forgejo database has unexpected records in the table `version`; a single record is expected w/ ID=1, but %d records were found", + len(versionRecords)) } else { - previousVersion = currentVersion.Version + previousVersion = versionRecords[0].Version + } + currentVersion := versionRecords[0] + if currentVersion.ID != 1 { + return fmt.Errorf( + "corrupt migrations: Forgejo database has corrupted records in the table `version`; a single record with ID=1 is expected, but a record with ID=%d was found instead", currentVersion.ID) } curDBVer := currentVersion.Version // Outdated Forgejo database version is not supported if curDBVer < minDBVersion { log.Fatal(`Forgejo no longer supports auto-migration from your previously installed version. -Please try upgrading to a lower version first (suggested v1.6.4), then upgrade to this version.`) + Please try upgrading to a lower version first (suggested v1.6.4), then upgrade to this version.`) return nil } @@ -484,7 +495,7 @@ Please try upgrading to a lower version first (suggested v1.6.4), then upgrade t // Some migration tasks depend on the git command if git.DefaultContext == nil { - if err = git.InitSimple(context.Background()); err != nil { + if err := git.InitSimple(context.Background()); err != nil { return err } } @@ -498,11 +509,11 @@ Please try upgrading to a lower version first (suggested v1.6.4), then upgrade t log.Info("Migration[%d]: %s", m.idNumber, m.description) // Reset the mapper between each migration - migrations are not supposed to depend on each other x.SetMapper(names.GonicMapper{}) - if err = m.Migrate(x); err != nil { + if err := m.Migrate(x); err != nil { return fmt.Errorf("migration[%d]: %s failed: %w", m.idNumber, m.description, err) } currentVersion.Version = migrationIDNumberToDBVersion(m.idNumber) - if _, err = x.ID(1).Update(currentVersion); err != nil { + if _, err := x.ID(1).Update(currentVersion); err != nil { return err } } diff --git a/models/migrations/migrations_test.go b/models/migrations/migrations_test.go index ea941b9a09..494772993d 100644 --- a/models/migrations/migrations_test.go +++ b/models/migrations/migrations_test.go @@ -6,9 +6,11 @@ package migrations import ( "testing" + migration_tests "forgejo.org/models/migrations/test" "forgejo.org/modules/test" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMigrations(t *testing.T) { @@ -25,3 +27,38 @@ func TestMigrations(t *testing.T) { assert.EqualValues(t, []*migration{{idNumber: 71}}, getPendingMigrations(71, preparedMigrations)) assert.EqualValues(t, []*migration{}, getPendingMigrations(72, preparedMigrations)) } + +func TestMigrateFreshDB(t *testing.T) { + x, deferable := migration_tests.PrepareTestEnv(t, 0, new(Version)) + defer deferable() + require.NotNil(t, x) + + err := Migrate(x) + require.NoError(t, err) + + var versionRecords []*Version + err = x.Find(&versionRecords) + require.NoError(t, err) + require.Len(t, versionRecords, 1) + v := versionRecords[0] + assert.EqualValues(t, 1, v.ID) + assert.EqualValues(t, 305, v.Version) +} + +func TestMigrateFailWithCorruption(t *testing.T) { + x, deferable := migration_tests.PrepareTestEnv(t, 0, new(Version)) + defer deferable() + require.NotNil(t, x) + + // ID != 1 + _, err := x.InsertOne(&Version{ID: 100, Version: 100}) + require.NoError(t, err) + err = Migrate(x) + require.ErrorContains(t, err, "corrupted records in the table `version`") + + // Two versions... + _, err = x.InsertOne(&Version{ID: 1, Version: 1000}) + require.NoError(t, err) + err = Migrate(x) + require.ErrorContains(t, err, "unexpected records in the table `version`") +}