From 8e2747859bbbbe87cca56fd8e1f794e5cf0aa86e Mon Sep 17 00:00:00 2001 From: Maks1mS Date: Thu, 29 May 2025 22:39:53 +0200 Subject: [PATCH] fix: ensure consistent empty repository topics field (#7920) Resolves #7878 An empty repository topic was not stored consistently across databases, this caused the `ONLY_SHOW_RELEVANT_REPOS` feature to not work correctly. Always store empty repository topics as an empty array to fix this. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7920 Reviewed-by: Gusted Co-authored-by: Maks1mS Co-committed-by: Maks1mS --- models/fixtures/repository.yml | 65 ++++++++++++++++++- models/forgejo_migrations/migrate.go | 2 + models/forgejo_migrations/v31.go | 29 +++++++++ models/forgejo_migrations/v31_test.go | 38 +++++++++++ .../Test_SetTopicsAsEmptySlice/repository.yml | 9 +++ .../repository.yml | 1 + models/repo/repo.go | 9 ++- services/repository/create_test.go | 10 +++ tests/e2e/fixtures/repository.yml | 1 + .../TestAdminDeleteUser/repository.yml | 1 + 10 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 models/forgejo_migrations/v31.go create mode 100644 models/forgejo_migrations/v31_test.go create mode 100644 models/migrations/fixtures/Test_SetTopicsAsEmptySlice/repository.yml diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 0ba4d06e14..5941f82299 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -31,6 +31,8 @@ close_issues_via_commit_in_any_branch: false created_unix: 1731254961 updated_unix: 1731254961 + topics: '[]' + - id: 2 owner_id: 2 @@ -61,7 +63,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: true - + topics: '[]' - id: 3 owner_id: 3 @@ -94,6 +96,7 @@ close_issues_via_commit_in_any_branch: false created_unix: 1700000001 updated_unix: 1700000001 + topics: '[]' - id: 4 @@ -125,6 +128,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 5 @@ -158,6 +162,7 @@ close_issues_via_commit_in_any_branch: false created_unix: 1700000002 updated_unix: 1700000002 + topics: '[]' - id: 6 @@ -190,6 +195,7 @@ close_issues_via_commit_in_any_branch: false created_unix: 1710000001 updated_unix: 1710000001 + topics: '[]' - id: 7 @@ -222,6 +228,7 @@ close_issues_via_commit_in_any_branch: false created_unix: 1710000003 updated_unix: 1710000003 + topics: '[]' - id: 8 @@ -254,6 +261,7 @@ close_issues_via_commit_in_any_branch: false created_unix: 1710000002 updated_unix: 1710000002 + topics: '[]' - id: 9 @@ -284,6 +292,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 10 @@ -315,6 +324,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 11 @@ -346,6 +356,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 12 @@ -376,6 +387,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 13 @@ -406,6 +418,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 14 @@ -437,6 +450,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 15 @@ -468,6 +482,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 16 @@ -499,6 +514,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 17 @@ -529,6 +545,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 18 @@ -559,6 +576,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 19 @@ -589,6 +607,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 20 @@ -619,6 +638,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 21 @@ -649,6 +669,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 22 @@ -679,6 +700,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 23 @@ -709,6 +731,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 24 @@ -739,6 +762,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 25 @@ -769,6 +793,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 26 @@ -799,6 +824,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 27 @@ -829,6 +855,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 28 @@ -859,6 +886,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 29 @@ -889,6 +917,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 30 @@ -919,6 +948,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 31 @@ -950,6 +980,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 32 # org public repo @@ -982,6 +1013,7 @@ close_issues_via_commit_in_any_branch: false created_unix: 1700000003 updated_unix: 1700000003 + topics: '[]' - id: 33 @@ -1013,6 +1045,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 34 @@ -1043,6 +1076,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 35 @@ -1073,6 +1107,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 36 @@ -1104,6 +1139,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 37 @@ -1135,6 +1171,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 38 @@ -1166,6 +1203,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 39 @@ -1197,6 +1235,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 40 @@ -1228,6 +1267,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 41 @@ -1259,6 +1299,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 42 @@ -1290,6 +1331,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 43 @@ -1320,6 +1362,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 44 @@ -1351,6 +1394,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 45 @@ -1381,6 +1425,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 46 @@ -1412,6 +1457,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 47 @@ -1443,6 +1489,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 48 @@ -1474,6 +1521,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 49 @@ -1506,6 +1554,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 50 @@ -1537,6 +1586,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 51 @@ -1568,6 +1618,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 52 @@ -1599,6 +1650,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 53 @@ -1627,6 +1679,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 54 @@ -1639,6 +1692,7 @@ is_archived: false is_private: true status: 0 + topics: '[]' - id: 55 @@ -1651,6 +1705,7 @@ is_private: true num_issues: 1 status: 0 + topics: '[]' - id: 56 @@ -1664,6 +1719,7 @@ is_private: true status: 0 num_issues: 0 + topics: '[]' - id: 57 @@ -1677,6 +1733,7 @@ is_private: false status: 0 num_issues: 0 + topics: '[]' - id: 58 # org public repo @@ -1708,6 +1765,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 1059 @@ -1721,6 +1779,7 @@ is_private: false status: 0 num_issues: 0 + topics: '[]' - id: 59 @@ -1734,6 +1793,7 @@ is_private: true status: 0 num_issues: 0 + topics: '[]' - id: 60 @@ -1765,6 +1825,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 61 @@ -1796,6 +1857,7 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' - id: 62 owner_id: 2 @@ -1826,3 +1888,4 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index d79bb1e455..41197c434d 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -99,6 +99,8 @@ var migrations = []*Migration{ NewMigration("Add public key information to `FederatedUser` and `FederationHost`", AddPublicKeyInformationForFederation), // v29 -> v30 NewMigration("Migrate `User.NormalizedFederatedURI` column to extract port & schema into FederatedHost", MigrateNormalizedFederatedURI), + // v30 -> v31 + NewMigration("Normalize repository.topics to empty slice instead of null", SetTopicsAsEmptySlice), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v31.go b/models/forgejo_migrations/v31.go new file mode 100644 index 0000000000..3f31cd2e92 --- /dev/null +++ b/models/forgejo_migrations/v31.go @@ -0,0 +1,29 @@ +// Copyright 2025 The Forgejo Authors. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations //nolint:revive + +import ( + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +func SetTopicsAsEmptySlice(x *xorm.Engine) error { + var err error + if x.Dialect().URI().DBType == schemas.POSTGRES { + _, err = x.Exec("UPDATE `repository` SET topics = '[]' WHERE topics IS NULL OR topics::text = 'null'") + } else { + _, err = x.Exec("UPDATE `repository` SET topics = '[]' WHERE topics IS NULL") + } + + if err != nil { + return err + } + + type Repository struct { + ID int64 `xorm:"pk autoincr"` + Topics []string `xorm:"TEXT JSON NOT NULL"` + } + + return x.Sync(new(Repository)) +} diff --git a/models/forgejo_migrations/v31_test.go b/models/forgejo_migrations/v31_test.go new file mode 100644 index 0000000000..5b4aac2a60 --- /dev/null +++ b/models/forgejo_migrations/v31_test.go @@ -0,0 +1,38 @@ +// Copyright 2025 The Forgejo Authors. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations //nolint:revive + +import ( + "testing" + + migration_tests "forgejo.org/models/migrations/test" + + "github.com/stretchr/testify/require" +) + +func Test_SetTopicsAsEmptySlice(t *testing.T) { + type Repository struct { + ID int64 `xorm:"pk autoincr"` + Topics []string `xorm:"TEXT JSON"` + } + + x, deferable := migration_tests.PrepareTestEnv(t, 0, new(Repository)) + defer deferable() + if x == nil || t.Failed() { + return + } + + require.NoError(t, SetTopicsAsEmptySlice(x)) + + var repos []Repository + require.NoError(t, x.Find(&repos)) + + for _, repo := range repos { + if repo.ID == 2 { + require.Equal(t, []string{"go", "dev"}, repo.Topics, "Valid topics should remain unchanged") + } else { + require.Equal(t, []string{}, repo.Topics, "NULL topics should be set to empty array") + } + } +} diff --git a/models/migrations/fixtures/Test_SetTopicsAsEmptySlice/repository.yml b/models/migrations/fixtures/Test_SetTopicsAsEmptySlice/repository.yml new file mode 100644 index 0000000000..e37b0b25b5 --- /dev/null +++ b/models/migrations/fixtures/Test_SetTopicsAsEmptySlice/repository.yml @@ -0,0 +1,9 @@ +# type Repository struct { +# ID int64 `xorm:"pk autoincr"` +# Topics []string `xorm:"TEXT JSON"` +# } +- + id: 1 +- + id: 2 + topics: '["go", "dev"]' diff --git a/models/repo/TestSearchRepositoryIDsByCondition/repository.yml b/models/repo/TestSearchRepositoryIDsByCondition/repository.yml index 9ce830783d..b10fbc9226 100644 --- a/models/repo/TestSearchRepositoryIDsByCondition/repository.yml +++ b/models/repo/TestSearchRepositoryIDsByCondition/repository.yml @@ -28,3 +28,4 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]' diff --git a/models/repo/repo.go b/models/repo/repo.go index 6bf03d724c..688c03b3d5 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -183,7 +183,7 @@ type Repository struct { StatsIndexerStatus *RepoIndexerStatus `xorm:"-"` IsFsckEnabled bool `xorm:"NOT NULL DEFAULT true"` CloseIssuesViaCommitInAnyBranch bool `xorm:"NOT NULL DEFAULT false"` - Topics []string `xorm:"TEXT JSON"` + Topics []string `xorm:"TEXT JSON NOT NULL"` ObjectFormatName string `xorm:"VARCHAR(6) NOT NULL DEFAULT 'sha1'"` TrustModel TrustModelType @@ -196,6 +196,13 @@ type Repository struct { ArchivedUnix timeutil.TimeStamp `xorm:"DEFAULT 0"` } +// BeforeInsert will be invoked by XORM before updating a record +func (repo *Repository) BeforeInsert() { + if repo.Topics == nil { + repo.Topics = []string{} + } +} + func init() { db.RegisterModel(new(Repository)) } diff --git a/services/repository/create_test.go b/services/repository/create_test.go index 7eb3c0f805..0a6c34b6fe 100644 --- a/services/repository/create_test.go +++ b/services/repository/create_test.go @@ -147,3 +147,13 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) { } require.NoError(t, organization.DeleteOrganization(db.DefaultContext, org), "DeleteOrganization") } + +func TestCreateRepository(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + r, err := CreateRepositoryDirectly(db.DefaultContext, user, user, CreateRepoOptions{Name: "repo-last"}) + require.NoError(t, err) + require.NotNil(t, r.Topics) + require.Empty(t, r.Topics) +} diff --git a/tests/e2e/fixtures/repository.yml b/tests/e2e/fixtures/repository.yml index ac87721d6a..6afbd138e8 100644 --- a/tests/e2e/fixtures/repository.yml +++ b/tests/e2e/fixtures/repository.yml @@ -10,3 +10,4 @@ is_private: true status: 0 lfs_size: 8192 + topics: '[]' diff --git a/tests/integration/fixtures/TestAdminDeleteUser/repository.yml b/tests/integration/fixtures/TestAdminDeleteUser/repository.yml index 2c12c7e1de..d71232d834 100644 --- a/tests/integration/fixtures/TestAdminDeleteUser/repository.yml +++ b/tests/integration/fixtures/TestAdminDeleteUser/repository.yml @@ -28,3 +28,4 @@ size: 0 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false + topics: '[]'