Skip to content

Commit

Permalink
fix(migration): skip v20 if column sha256 already exists (#7354)
Browse files Browse the repository at this point in the history
  • Loading branch information
unknwon committed Feb 14, 2023
1 parent 8f9895a commit 0f8c71d
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -33,6 +33,7 @@ All notable changes to Gogs are documented in this file.
- Unable to choose "Lookup Avatar by mail" in user settings without deleting custom avatar. [#7267](https://github.com/gogs/gogs/pull/7267)
- Mistakenly include the "data" directory under the custom directory in the Docker setup. [#7343](https://github.com/gogs/gogs/pull/7343)
- Unable to render repository pages with implicit submodules (e.g. `get submodule "REDACTED": revision does not exist`). [#6436](https://github.com/gogs/gogs/issues/6436)
- Unable to start after data recovery with an outdated migration version. [#7125](https://github.com/gogs/gogs/issues/7125)

### Removed

Expand Down
18 changes: 15 additions & 3 deletions internal/db/migrations/migrations.go
Expand Up @@ -56,8 +56,17 @@ var migrations = []Migration{
NewMigration("migrate access tokens to store SHA56", migrateAccessTokenToSHA256),
// v20 -> v21:v0.13.0
NewMigration("add index to action.user_id", addIndexToActionUserID),
// v21 -> v22:v0.13.0
//
// NOTE: There was a bug in calculating the value of the `version.version`
// column after a migration is done, thus some instances are on v21 but some are
// on v22. Let's make a noop v22 to make sure every instance will not miss a
// real future migration.
NewMigration("noop", func(*gorm.DB) error { return nil }),
}

var errMigrationSkipped = errors.New("the migration has been skipped")

// Migrate migrates the database schema and/or data to the current version.
func Migrate(db *gorm.DB) error {
// NOTE: GORM has problem migrating tables that happen to have columns with the
Expand Down Expand Up @@ -121,13 +130,16 @@ In case you're stilling getting this notice, go through instructions again until
return db.Where("id = ?", current.ID).Updates(current).Error
}

for i, m := range migrations[current.Version-minDBVersion:] {
for _, m := range migrations[current.Version-minDBVersion:] {
log.Info("Migration: %s", m.Description())
if err = m.Migrate(db); err != nil {
return errors.Wrap(err, "do migrate")
if err != errMigrationSkipped {
return errors.Wrap(err, "do migrate")
}
log.Trace("The migration %q has been skipped", m.Description())
}

current.Version += int64(i) + 1
current.Version++
err = db.Where("id = ?", current.ID).Updates(current).Error
if err != nil {
return errors.Wrap(err, "update the version record")
Expand Down
14 changes: 9 additions & 5 deletions internal/db/migrations/v20.go
Expand Up @@ -12,14 +12,18 @@ import (
)

func migrateAccessTokenToSHA256(db *gorm.DB) error {
type accessToken struct {
ID int64
Sha1 string
SHA256 string `gorm:"TYPE:VARCHAR(64)"`
}

if db.Migrator().HasColumn(&accessToken{}, "SHA256") {
return errMigrationSkipped
}
return db.Transaction(func(tx *gorm.DB) error {
// 1. Add column without constraints because all rows have NULL values for the
// "sha256" column.
type accessToken struct {
ID int64
Sha1 string
SHA256 string `gorm:"TYPE:VARCHAR(64)"`
}
err := tx.Migrator().AddColumn(&accessToken{}, "SHA256")
if err != nil {
return errors.Wrap(err, "add column")
Expand Down
4 changes: 4 additions & 0 deletions internal/db/migrations/v20_test.go
Expand Up @@ -67,4 +67,8 @@ func TestMigrateAccessTokenToSHA256(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "73da7bb9d2a475bbc2ab79da7d4e94940cb9f9d5", got.Sha1)
assert.Equal(t, "ab144c7bd170691bb9bb995f1541c608e33a78b40174f30fc8a1616c0bc3a477", got.SHA256)

// Re-run should be skipped
err = migrateAccessTokenToSHA256(db)
require.Equal(t, errMigrationSkipped, err)
}
2 changes: 1 addition & 1 deletion internal/db/migrations/v21.go
Expand Up @@ -13,7 +13,7 @@ func addIndexToActionUserID(db *gorm.DB) error {
UserID string `gorm:"index"`
}
if db.Migrator().HasIndex(&action{}, "UserID") {
return nil
return errMigrationSkipped
}
return db.Migrator().CreateIndex(&action{}, "UserID")
}
4 changes: 4 additions & 0 deletions internal/db/migrations/v21_test.go
Expand Up @@ -79,4 +79,8 @@ func TestAddIndexToActionUserID(t *testing.T) {
err = addIndexToActionUserID(db)
require.NoError(t, err)
assert.True(t, db.Migrator().HasIndex(&actionV21{}, "UserID"))

// Re-run should be skipped
err = addIndexToActionUserID(db)
require.Equal(t, errMigrationSkipped, err)
}

0 comments on commit 0f8c71d

Please sign in to comment.