fix: reduce noise for the v303 migration (#6591)

Using SELECT `%s` FROM `%s` WHERE 0 = 1 to assert the existence of a column is simple but noisy: it shows errors in the migrations that are confusing for Forgejo admins because they are not actual errors.

Use introspection instead, which is more complicated but leads to the same result.

Add a test that ensures it works as expected, for all database types. Although the migration is run for all database types, it does not account for various scenarios and is never tested in the case a column does not exist.

Refs: https://codeberg.org/forgejo/forgejo/issues/6583

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6591
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

authored by

Earl Warren

Earl Warren and committed by

Earl Warren 1 year ago 376a2e19 b2a3a041

+68 -8

2 changed files

expand all

models

migrations

v1_23

v303.go

v303_test.go

+27 -8

models/migrations/v1_23/v303.go

··· 1 - // Copyright 2024 The Forgejo Authors. 2 - // SPDX-License-Identifier: MIT 1 + // Copyright 2025 The Forgejo Authors. 2 + // SPDX-License-Identifier: GPL-3.0-or-later 3 3 4 4 package v1_23 //nolint 5 5 6 6 import ( 7 - "fmt" 8 - 9 7 "code.gitea.io/gitea/models/migrations/base" 10 8 11 9 "xorm.io/xorm" 10 + "xorm.io/xorm/schemas" 12 11 ) 13 12 14 13 func GiteaLastDrop(x *xorm.Engine) error { 14 + tables, err := x.DBMetas() 15 + if err != nil { 16 + return err 17 + } 18 + 15 19 sess := x.NewSession() 16 20 defer sess.Close() 17 21 18 22 for _, drop := range []struct { 19 - table string 20 - field string 23 + table string 24 + column string 21 25 }{ 22 26 {"badge", "slug"}, 23 27 {"oauth2_application", "skip_secondary_authorization"}, ··· 29 33 {"protected_branch", "force_push_allowlist_team_i_ds"}, 30 34 {"protected_branch", "force_push_allowlist_deploy_keys"}, 31 35 } { 32 - if _, err := sess.Exec(fmt.Sprintf("SELECT `%s` FROM `%s` WHERE 0 = 1", drop.field, drop.table)); err != nil { 36 + var table *schemas.Table 37 + found := false 38 + 39 + for _, table = range tables { 40 + if table.Name == drop.table { 41 + found = true 42 + break 43 + } 44 + } 45 + 46 + if !found { 33 47 continue 34 48 } 35 - if err := base.DropTableColumns(sess, drop.table, drop.field); err != nil { 49 + 50 + if table.GetColumn(drop.column) == nil { 51 + continue 52 + } 53 + 54 + if err := base.DropTableColumns(sess, drop.table, drop.column); err != nil { 36 55 return err 37 56 } 38 57 }

+41

models/migrations/v1_23/v303_test.go

··· 1 + // Copyright 2025 The Forgejo Authors. 2 + // SPDX-License-Identifier: GPL-3.0-or-later 3 + 4 + package v1_23 //nolint 5 + 6 + import ( 7 + "testing" 8 + 9 + migration_tests "code.gitea.io/gitea/models/migrations/test" 10 + 11 + "github.com/stretchr/testify/require" 12 + "xorm.io/xorm/schemas" 13 + ) 14 + 15 + func Test_GiteaLastDrop(t *testing.T) { 16 + type Badge struct { 17 + ID int64 `xorm:"pk autoincr"` 18 + Slug string 19 + } 20 + 21 + x, deferable := migration_tests.PrepareTestEnv(t, 0, new(Badge)) 22 + defer deferable() 23 + if x == nil || t.Failed() { 24 + return 25 + } 26 + 27 + getColumn := func() *schemas.Column { 28 + tables, err := x.DBMetas() 29 + require.NoError(t, err) 30 + require.Len(t, tables, 1) 31 + table := tables[0] 32 + require.Equal(t, "badge", table.Name) 33 + return table.GetColumn("slug") 34 + } 35 + 36 + require.NotNil(t, getColumn(), "slug column exists") 37 + require.NoError(t, GiteaLastDrop(x)) 38 + require.Nil(t, getColumn(), "slug column was deleted") 39 + // idempotent 40 + require.NoError(t, GiteaLastDrop(x)) 41 + }

Configure Feed

Configure Feed