From 59adfd6c815230e3cb5a42d9acc501a68643097c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 8 Nov 2022 17:12:25 +0800 Subject: [PATCH 01/14] tests: add TestPrimaryKeys --- models/db/engine_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/models/db/engine_test.go b/models/db/engine_test.go index c26d94c3405e7..752d83abf3fff 100644 --- a/models/db/engine_test.go +++ b/models/db/engine_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "testing" + _ "code.gitea.io/gitea/cmd" // for TestPrimaryKeys "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" @@ -52,3 +53,34 @@ func TestDeleteOrphanedObjects(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, countBefore, countAfter) } + +func TestPrimaryKeys(t *testing.T) { + // Some dbs require that all tables have primary keys, see + // https://github.com/go-gitea/gitea/issues/21086 + // https://github.com/go-gitea/gitea/issues/16802 + // To avoid creating tables without primary key again, this test will check them. + // Import "code.gitea.io/gitea/cmd" to make sure each db.RegisterModel in init functions has been called. + + beans, err := db.NamesToBean() + if err != nil { + t.Fatal(err) + } + + whitelist := map[string]string{ + "the_table_name_to_skip_checking": "Write a note here to explain why", + } + + for _, bean := range beans { + table, err := db.TableInfo(bean) + if err != nil { + t.Fatal(err) + } + if why, ok := whitelist[table.Name]; ok { + t.Logf("ignore %q because %q", table.Name, why) + continue + } + if len(table.PrimaryKeys) == 0 { + t.Errorf("table %q has no primary key", table.Name) + } + } +} From 3d4e4cddf843a82cea2c4b8091b5aa24fe3f09db Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 8 Nov 2022 17:21:09 +0800 Subject: [PATCH 02/14] chore: format codes --- models/db/engine_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/db/engine_test.go b/models/db/engine_test.go index 752d83abf3fff..1bf022690a7a2 100644 --- a/models/db/engine_test.go +++ b/models/db/engine_test.go @@ -8,12 +8,13 @@ import ( "path/filepath" "testing" - _ "code.gitea.io/gitea/cmd" // for TestPrimaryKeys "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/setting" + _ "code.gitea.io/gitea/cmd" // for TestPrimaryKeys + "github.com/stretchr/testify/assert" ) From 961c87a313255a1124a7d78786d1a38f7add0e55 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Dec 2022 11:03:50 +0000 Subject: [PATCH 03/14] add primary key to foreignreference Signed-off-by: Andrew Thornton --- models/foreignreference/foreignreference.go | 2 ++ .../foreign_reference.yml | 25 ++++++++++++++ models/migrations/migrations.go | 2 ++ models/migrations/v1_19/v236.go | 21 ++++++++++++ models/migrations/v1_19/v236_test.go | 34 +++++++++++++++++++ 5 files changed, 84 insertions(+) create mode 100644 models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml create mode 100644 models/migrations/v1_19/v236.go create mode 100644 models/migrations/v1_19/v236_test.go diff --git a/models/foreignreference/foreignreference.go b/models/foreignreference/foreignreference.go index 2d2ad04c5a142..93f7879fd9922 100644 --- a/models/foreignreference/foreignreference.go +++ b/models/foreignreference/foreignreference.go @@ -19,6 +19,8 @@ const ( // ForeignReference represents external references type ForeignReference struct { + ID int64 `xorm:"pk autoincr"` + // RepoID is the first column in all indices. now we only need 2 indices: (repo, local) and (repo, foreign, type) RepoID int64 `xorm:"UNIQUE(repo_foreign_type) INDEX(repo_local)" ` LocalIndex int64 `xorm:"INDEX(repo_local)"` // the resource key inside Gitea, it can be IssueIndex, or some model ID. diff --git a/models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml b/models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml new file mode 100644 index 0000000000000..4512cef7964c9 --- /dev/null +++ b/models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml @@ -0,0 +1,25 @@ +- + repo_id: 1 + local_index: 1 + foreign_index: "foo" + type: "foo" +- + repo_id: 1 + local_index: 2 + foreign_index: "bar" + type: "foo" +- + repo_id: 2 + local_index: 2 + foreign_index: "foo" + type: "foo" +- + repo_id: 3 + local_index: 1024 + foreign_index: "1" + type: "normal" +- + repo_id: 2 + local_index: 1 + foreign_index: "bar" + type: "foo" diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index e718355f8349f..5f6eda0fd50b8 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -442,6 +442,8 @@ var migrations = []Migration{ NewMigration("Add package cleanup rule table", v1_19.CreatePackageCleanupRuleTable), // v235 -> v236 NewMigration("Add index for access_token", v1_19.AddIndexForAccessToken), + // v236 -> v237 + NewMigration("Add primary key to foreign reference", v1_19.AddPrimaryKeyToForeignReference), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_19/v236.go b/models/migrations/v1_19/v236.go new file mode 100644 index 0000000000000..4271217a4a99c --- /dev/null +++ b/models/migrations/v1_19/v236.go @@ -0,0 +1,21 @@ +// Copyright 2022 Gitea. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_19 //nolint + +import "xorm.io/xorm" + +func AddPrimaryKeyToForeignReference(x *xorm.Engine) error { + // ForeignReference represents external references + type ForeignReference struct { + ID int64 `xorm:"pk autoincr"` + + // RepoID is the first column in all indices. now we only need 2 indices: (repo, local) and (repo, foreign, type) + RepoID int64 `xorm:"UNIQUE(repo_foreign_type) INDEX(repo_local)" ` + LocalIndex int64 `xorm:"INDEX(repo_local)"` // the resource key inside Gitea, it can be IssueIndex, or some model ID. + ForeignIndex string `xorm:"INDEX UNIQUE(repo_foreign_type)"` + Type string `xorm:"VARCHAR(16) INDEX UNIQUE(repo_foreign_type)"` + } + + return x.Sync(new(ForeignReference)) +} diff --git a/models/migrations/v1_19/v236_test.go b/models/migrations/v1_19/v236_test.go new file mode 100644 index 0000000000000..d3f1098458d01 --- /dev/null +++ b/models/migrations/v1_19/v236_test.go @@ -0,0 +1,34 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_19 //nolint + +import ( + "testing" + + "code.gitea.io/gitea/models/migrations/base" + "github.com/stretchr/testify/assert" +) + +func Test_AddPrimaryKeyToForeignReference(t *testing.T) { + // ForeignReference represents external references + type ForeignReference struct { + // RepoID is the first column in all indices. now we only need 2 indices: (repo, local) and (repo, foreign, type) + RepoID int64 `xorm:"UNIQUE(repo_foreign_type) INDEX(repo_local)" ` + LocalIndex int64 `xorm:"INDEX(repo_local)"` // the resource key inside Gitea, it can be IssueIndex, or some model ID. + ForeignIndex string `xorm:"INDEX UNIQUE(repo_foreign_type)"` + Type string `xorm:"VARCHAR(16) INDEX UNIQUE(repo_foreign_type)"` + } + + // Prepare and load the testing database + x, deferable := base.PrepareTestEnv(t, 0, new(ForeignReference)) + defer deferable() + if x == nil || t.Failed() { + return + } + + if err := AddPrimaryKeyToForeignReference(x); err != nil { + assert.NoError(t, err) + return + } +} From e96ea3ab4687c27637646226d74763a260579495 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 4 Dec 2022 20:00:21 +0000 Subject: [PATCH 04/14] fix fmt Signed-off-by: Andrew Thornton --- models/migrations/v1_19/v236_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/migrations/v1_19/v236_test.go b/models/migrations/v1_19/v236_test.go index d3f1098458d01..bdd4e372e1e95 100644 --- a/models/migrations/v1_19/v236_test.go +++ b/models/migrations/v1_19/v236_test.go @@ -7,6 +7,7 @@ import ( "testing" "code.gitea.io/gitea/models/migrations/base" + "github.com/stretchr/testify/assert" ) From bed6b7513709ca6b427cac07bf75fb97233c5236 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 5 Dec 2022 22:33:15 +0000 Subject: [PATCH 05/14] use recreate-table Signed-off-by: Andrew Thornton --- models/migrations/v1_19/v236.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/models/migrations/v1_19/v236.go b/models/migrations/v1_19/v236.go index 4271217a4a99c..c42436df1d740 100644 --- a/models/migrations/v1_19/v236.go +++ b/models/migrations/v1_19/v236.go @@ -3,7 +3,10 @@ package v1_19 //nolint -import "xorm.io/xorm" +import ( + "code.gitea.io/gitea/models/migrations/base" + "xorm.io/xorm" +) func AddPrimaryKeyToForeignReference(x *xorm.Engine) error { // ForeignReference represents external references @@ -17,5 +20,13 @@ func AddPrimaryKeyToForeignReference(x *xorm.Engine) error { Type string `xorm:"VARCHAR(16) INDEX UNIQUE(repo_foreign_type)"` } - return x.Sync(new(ForeignReference)) + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + base.RecreateTable(sess, &ForeignReference{}) + + return sess.Commit() } From 0d720a17d6af81213b3bfa90d4bdc0fedab6846f Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 10:51:35 +0800 Subject: [PATCH 06/14] chore: rename file --- models/migrations/v1_19/{v236_test.go => v237_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/v1_19/{v236_test.go => v237_test.go} (100%) diff --git a/models/migrations/v1_19/v236_test.go b/models/migrations/v1_19/v237_test.go similarity index 100% rename from models/migrations/v1_19/v236_test.go rename to models/migrations/v1_19/v237_test.go From e70992f4832adfd34ff75d50ee951a2546bae03e Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 11:09:10 +0800 Subject: [PATCH 07/14] chore: fmt code --- models/migrations/v1_19/v237.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v1_19/v237.go b/models/migrations/v1_19/v237.go index 3490811985c3b..a57e9b457a48f 100644 --- a/models/migrations/v1_19/v237.go +++ b/models/migrations/v1_19/v237.go @@ -5,6 +5,7 @@ package v1_19 //nolint import ( "code.gitea.io/gitea/models/migrations/base" + "xorm.io/xorm" ) @@ -30,4 +31,3 @@ func AddPrimaryKeyToForeignReference(x *xorm.Engine) error { return sess.Commit() } - From 9e5539d515d2e2dcd63c2e76cff037a2397b63b4 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 14:17:13 +0800 Subject: [PATCH 08/14] Revert "chore: fmt code" This reverts commit e70992f4832adfd34ff75d50ee951a2546bae03e. --- models/migrations/v1_19/v237.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v1_19/v237.go b/models/migrations/v1_19/v237.go index a57e9b457a48f..3490811985c3b 100644 --- a/models/migrations/v1_19/v237.go +++ b/models/migrations/v1_19/v237.go @@ -5,7 +5,6 @@ package v1_19 //nolint import ( "code.gitea.io/gitea/models/migrations/base" - "xorm.io/xorm" ) @@ -31,3 +30,4 @@ func AddPrimaryKeyToForeignReference(x *xorm.Engine) error { return sess.Commit() } + From 63b55e7fdcb8efc0d3f01186cd816f06dd33666e Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 14:17:16 +0800 Subject: [PATCH 09/14] Revert "chore: rename file" This reverts commit 0d720a17d6af81213b3bfa90d4bdc0fedab6846f. --- models/migrations/v1_19/{v237_test.go => v236_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename models/migrations/v1_19/{v237_test.go => v236_test.go} (100%) diff --git a/models/migrations/v1_19/v237_test.go b/models/migrations/v1_19/v236_test.go similarity index 100% rename from models/migrations/v1_19/v237_test.go rename to models/migrations/v1_19/v236_test.go From 083ce6f754e7a86b92537d5ca3728d1840056118 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 14:19:34 +0800 Subject: [PATCH 10/14] Revert "fix fmt" This reverts commit e96ea3ab4687c27637646226d74763a260579495. --- models/migrations/v1_19/v236_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/models/migrations/v1_19/v236_test.go b/models/migrations/v1_19/v236_test.go index bdd4e372e1e95..d3f1098458d01 100644 --- a/models/migrations/v1_19/v236_test.go +++ b/models/migrations/v1_19/v236_test.go @@ -7,7 +7,6 @@ import ( "testing" "code.gitea.io/gitea/models/migrations/base" - "github.com/stretchr/testify/assert" ) From ca07851d29c268f54c16702352e1ad9cd2ee80ce Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 14:43:11 +0800 Subject: [PATCH 11/14] Revert "add primary key to foreignreference" This reverts commit 961c87a313255a1124a7d78786d1a38f7add0e55. --- models/foreignreference/foreignreference.go | 2 -- .../foreign_reference.yml | 25 -------------- models/migrations/migrations.go | 2 -- models/migrations/v1_19/v236_test.go | 34 ------------------- models/migrations/v1_19/v237.go | 33 ------------------ 5 files changed, 96 deletions(-) delete mode 100644 models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml delete mode 100644 models/migrations/v1_19/v236_test.go delete mode 100644 models/migrations/v1_19/v237.go diff --git a/models/foreignreference/foreignreference.go b/models/foreignreference/foreignreference.go index 93f7879fd9922..2d2ad04c5a142 100644 --- a/models/foreignreference/foreignreference.go +++ b/models/foreignreference/foreignreference.go @@ -19,8 +19,6 @@ const ( // ForeignReference represents external references type ForeignReference struct { - ID int64 `xorm:"pk autoincr"` - // RepoID is the first column in all indices. now we only need 2 indices: (repo, local) and (repo, foreign, type) RepoID int64 `xorm:"UNIQUE(repo_foreign_type) INDEX(repo_local)" ` LocalIndex int64 `xorm:"INDEX(repo_local)"` // the resource key inside Gitea, it can be IssueIndex, or some model ID. diff --git a/models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml b/models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml deleted file mode 100644 index 4512cef7964c9..0000000000000 --- a/models/migrations/fixtures/Test_AddPrimaryKeyToForeignReference/foreign_reference.yml +++ /dev/null @@ -1,25 +0,0 @@ -- - repo_id: 1 - local_index: 1 - foreign_index: "foo" - type: "foo" -- - repo_id: 1 - local_index: 2 - foreign_index: "bar" - type: "foo" -- - repo_id: 2 - local_index: 2 - foreign_index: "foo" - type: "foo" -- - repo_id: 3 - local_index: 1024 - foreign_index: "1" - type: "normal" -- - repo_id: 2 - local_index: 1 - foreign_index: "bar" - type: "foo" diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 199a7bb3ebebb..591bfa3e86a63 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -444,8 +444,6 @@ var migrations = []Migration{ NewMigration("Add index for access_token", v1_19.AddIndexForAccessToken), // v236 -> v237 NewMigration("Create secrets table", v1_19.CreateSecretsTable), - // v237 -> v238 - NewMigration("Add primary key to foreign reference", v1_19.AddPrimaryKeyToForeignReference), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_19/v236_test.go b/models/migrations/v1_19/v236_test.go deleted file mode 100644 index d3f1098458d01..0000000000000 --- a/models/migrations/v1_19/v236_test.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package v1_19 //nolint - -import ( - "testing" - - "code.gitea.io/gitea/models/migrations/base" - "github.com/stretchr/testify/assert" -) - -func Test_AddPrimaryKeyToForeignReference(t *testing.T) { - // ForeignReference represents external references - type ForeignReference struct { - // RepoID is the first column in all indices. now we only need 2 indices: (repo, local) and (repo, foreign, type) - RepoID int64 `xorm:"UNIQUE(repo_foreign_type) INDEX(repo_local)" ` - LocalIndex int64 `xorm:"INDEX(repo_local)"` // the resource key inside Gitea, it can be IssueIndex, or some model ID. - ForeignIndex string `xorm:"INDEX UNIQUE(repo_foreign_type)"` - Type string `xorm:"VARCHAR(16) INDEX UNIQUE(repo_foreign_type)"` - } - - // Prepare and load the testing database - x, deferable := base.PrepareTestEnv(t, 0, new(ForeignReference)) - defer deferable() - if x == nil || t.Failed() { - return - } - - if err := AddPrimaryKeyToForeignReference(x); err != nil { - assert.NoError(t, err) - return - } -} diff --git a/models/migrations/v1_19/v237.go b/models/migrations/v1_19/v237.go deleted file mode 100644 index 3490811985c3b..0000000000000 --- a/models/migrations/v1_19/v237.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package v1_19 //nolint - -import ( - "code.gitea.io/gitea/models/migrations/base" - "xorm.io/xorm" -) - -func AddPrimaryKeyToForeignReference(x *xorm.Engine) error { - // ForeignReference represents external references - type ForeignReference struct { - ID int64 `xorm:"pk autoincr"` - - // RepoID is the first column in all indices. now we only need 2 indices: (repo, local) and (repo, foreign, type) - RepoID int64 `xorm:"UNIQUE(repo_foreign_type) INDEX(repo_local)" ` - LocalIndex int64 `xorm:"INDEX(repo_local)"` // the resource key inside Gitea, it can be IssueIndex, or some model ID. - ForeignIndex string `xorm:"INDEX UNIQUE(repo_foreign_type)"` - Type string `xorm:"VARCHAR(16) INDEX UNIQUE(repo_foreign_type)"` - } - - sess := x.NewSession() - defer sess.Close() - if err := sess.Begin(); err != nil { - return err - } - - base.RecreateTable(sess, &ForeignReference{}) - - return sess.Commit() -} - From 76656013c986e739e381d34ca56bd6165b5e489d Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 14:59:39 +0800 Subject: [PATCH 12/14] chore: drop foreignreference --- models/fixtures/foreign_reference.yml | 1 - models/foreignreference/error.go | 52 ------------------ models/foreignreference/foreignreference.go | 31 ----------- models/issues/issue.go | 60 ++------------------- models/issues/issue_test.go | 34 ------------ models/migrate.go | 7 --- models/migrate_test.go | 12 ----- services/migrations/gitea_uploader.go | 11 ---- 8 files changed, 5 insertions(+), 203 deletions(-) delete mode 100644 models/fixtures/foreign_reference.yml delete mode 100644 models/foreignreference/error.go delete mode 100644 models/foreignreference/foreignreference.go diff --git a/models/fixtures/foreign_reference.yml b/models/fixtures/foreign_reference.yml deleted file mode 100644 index ca780a73aa0c1..0000000000000 --- a/models/fixtures/foreign_reference.yml +++ /dev/null @@ -1 +0,0 @@ -[] # empty diff --git a/models/foreignreference/error.go b/models/foreignreference/error.go deleted file mode 100644 index 07ed1052a63bf..0000000000000 --- a/models/foreignreference/error.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2022 Gitea. All rights reserved. -// SPDX-License-Identifier: MIT - -package foreignreference - -import ( - "fmt" - - "code.gitea.io/gitea/modules/util" -) - -// ErrLocalIndexNotExist represents a "LocalIndexNotExist" kind of error. -type ErrLocalIndexNotExist struct { - RepoID int64 - ForeignIndex int64 - Type string -} - -// ErrLocalIndexNotExist checks if an error is a ErrLocalIndexNotExist. -func IsErrLocalIndexNotExist(err error) bool { - _, ok := err.(ErrLocalIndexNotExist) - return ok -} - -func (err ErrLocalIndexNotExist) Error() string { - return fmt.Sprintf("repository %d has no LocalIndex for ForeignIndex %d of type %s", err.RepoID, err.ForeignIndex, err.Type) -} - -func (err ErrLocalIndexNotExist) Unwrap() error { - return util.ErrNotExist -} - -// ErrForeignIndexNotExist represents a "ForeignIndexNotExist" kind of error. -type ErrForeignIndexNotExist struct { - RepoID int64 - LocalIndex int64 - Type string -} - -// ErrForeignIndexNotExist checks if an error is a ErrForeignIndexNotExist. -func IsErrForeignIndexNotExist(err error) bool { - _, ok := err.(ErrForeignIndexNotExist) - return ok -} - -func (err ErrForeignIndexNotExist) Error() string { - return fmt.Sprintf("repository %d has no ForeignIndex for LocalIndex %d of type %s", err.RepoID, err.LocalIndex, err.Type) -} - -func (err ErrForeignIndexNotExist) Unwrap() error { - return util.ErrNotExist -} diff --git a/models/foreignreference/foreignreference.go b/models/foreignreference/foreignreference.go deleted file mode 100644 index 2d2ad04c5a142..0000000000000 --- a/models/foreignreference/foreignreference.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2022 Gitea. All rights reserved. -// SPDX-License-Identifier: MIT - -package foreignreference - -import ( - "code.gitea.io/gitea/models/db" -) - -// Type* are valid values for the Type field of ForeignReference -const ( - TypeIssue = "issue" - TypePullRequest = "pull_request" - TypeComment = "comment" - TypeReview = "review" - TypeReviewComment = "review_comment" - TypeRelease = "release" -) - -// ForeignReference represents external references -type ForeignReference struct { - // RepoID is the first column in all indices. now we only need 2 indices: (repo, local) and (repo, foreign, type) - RepoID int64 `xorm:"UNIQUE(repo_foreign_type) INDEX(repo_local)" ` - LocalIndex int64 `xorm:"INDEX(repo_local)"` // the resource key inside Gitea, it can be IssueIndex, or some model ID. - ForeignIndex string `xorm:"INDEX UNIQUE(repo_foreign_type)"` - Type string `xorm:"VARCHAR(16) INDEX UNIQUE(repo_foreign_type)"` -} - -func init() { - db.RegisterModel(new(ForeignReference)) -} diff --git a/models/issues/issue.go b/models/issues/issue.go index fc93fcf45465c..f45e635c0ecea 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -9,11 +9,9 @@ import ( "fmt" "regexp" "sort" - "strconv" "strings" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/foreignreference" "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" @@ -136,12 +134,11 @@ type Issue struct { UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` ClosedUnix timeutil.TimeStamp `xorm:"INDEX"` - Attachments []*repo_model.Attachment `xorm:"-"` - Comments []*Comment `xorm:"-"` - Reactions ReactionList `xorm:"-"` - TotalTrackedTime int64 `xorm:"-"` - Assignees []*user_model.User `xorm:"-"` - ForeignReference *foreignreference.ForeignReference `xorm:"-"` + Attachments []*repo_model.Attachment `xorm:"-"` + Comments []*Comment `xorm:"-"` + Reactions ReactionList `xorm:"-"` + TotalTrackedTime int64 `xorm:"-"` + Assignees []*user_model.User `xorm:"-"` // IsLocked limits commenting abilities to users on an issue // with write access @@ -321,29 +318,6 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) { return nil } -func (issue *Issue) loadForeignReference(ctx context.Context) (err error) { - if issue.ForeignReference != nil { - return nil - } - reference := &foreignreference.ForeignReference{ - RepoID: issue.RepoID, - LocalIndex: issue.Index, - Type: foreignreference.TypeIssue, - } - has, err := db.GetEngine(ctx).Get(reference) - if err != nil { - return err - } else if !has { - return foreignreference.ErrForeignIndexNotExist{ - RepoID: issue.RepoID, - LocalIndex: issue.Index, - Type: foreignreference.TypeIssue, - } - } - issue.ForeignReference = reference - return nil -} - // LoadMilestone load milestone of this issue. func (issue *Issue) LoadMilestone(ctx context.Context) (err error) { if (issue.Milestone == nil || issue.Milestone.ID != issue.MilestoneID) && issue.MilestoneID > 0 { @@ -406,10 +380,6 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) { } } - if err = issue.loadForeignReference(ctx); err != nil && !foreignreference.IsErrForeignIndexNotExist(err) { - return err - } - return issue.loadReactions(ctx) } @@ -1097,26 +1067,6 @@ func GetIssueByIndex(repoID, index int64) (*Issue, error) { return issue, nil } -// GetIssueByForeignIndex returns raw issue by foreign ID -func GetIssueByForeignIndex(ctx context.Context, repoID, foreignIndex int64) (*Issue, error) { - reference := &foreignreference.ForeignReference{ - RepoID: repoID, - ForeignIndex: strconv.FormatInt(foreignIndex, 10), - Type: foreignreference.TypeIssue, - } - has, err := db.GetEngine(ctx).Get(reference) - if err != nil { - return nil, err - } else if !has { - return nil, foreignreference.ErrLocalIndexNotExist{ - RepoID: repoID, - ForeignIndex: foreignIndex, - Type: foreignreference.TypeIssue, - } - } - return GetIssueByIndex(repoID, reference.LocalIndex) -} - // GetIssueWithAttrsByIndex returns issue by index in a repository. func GetIssueWithAttrsByIndex(repoID, index int64) (*Issue, error) { issue, err := GetIssueByIndex(repoID, index) diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index 6764a9e6260ef..de1da19ab9fad 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -7,13 +7,11 @@ import ( "context" "fmt" "sort" - "strconv" "sync" "testing" "time" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/foreignreference" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" @@ -501,38 +499,6 @@ func TestCorrectIssueStats(t *testing.T) { assert.EqualValues(t, issueStats.OpenCount, issueAmount) } -func TestIssueForeignReference(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 4}) - assert.NotEqualValues(t, issue.Index, issue.ID) // make sure they are different to avoid false positive - - // it is fine for an issue to not have a foreign reference - err := issue.LoadAttributes(db.DefaultContext) - assert.NoError(t, err) - assert.Nil(t, issue.ForeignReference) - - var foreignIndex int64 = 12345 - _, err = issues_model.GetIssueByForeignIndex(context.Background(), issue.RepoID, foreignIndex) - assert.True(t, foreignreference.IsErrLocalIndexNotExist(err)) - - err = db.Insert(db.DefaultContext, &foreignreference.ForeignReference{ - LocalIndex: issue.Index, - ForeignIndex: strconv.FormatInt(foreignIndex, 10), - RepoID: issue.RepoID, - Type: foreignreference.TypeIssue, - }) - assert.NoError(t, err) - - err = issue.LoadAttributes(db.DefaultContext) - assert.NoError(t, err) - - assert.EqualValues(t, issue.ForeignReference.ForeignIndex, strconv.FormatInt(foreignIndex, 10)) - - found, err := issues_model.GetIssueByForeignIndex(context.Background(), issue.RepoID, foreignIndex) - assert.NoError(t, err) - assert.EqualValues(t, found.Index, issue.Index) -} - func TestMilestoneList_LoadTotalTrackedTimes(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) miles := issues_model.MilestoneList{ diff --git a/models/migrate.go b/models/migrate.go index b1b5568126481..82cacd4a75b22 100644 --- a/models/migrate.go +++ b/models/migrate.go @@ -83,13 +83,6 @@ func insertIssue(ctx context.Context, issue *issues_model.Issue) error { } } - if issue.ForeignReference != nil { - issue.ForeignReference.LocalIndex = issue.Index - if _, err := sess.Insert(issue.ForeignReference); err != nil { - return err - } - } - return nil } diff --git a/models/migrate_test.go b/models/migrate_test.go index 48cd905e4c0d9..42102f9a7d22f 100644 --- a/models/migrate_test.go +++ b/models/migrate_test.go @@ -4,11 +4,9 @@ package models import ( - "strconv" "testing" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/foreignreference" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -48,7 +46,6 @@ func assertCreateIssues(t *testing.T, isPull bool) { UserID: owner.ID, } - foreignIndex := int64(12345) title := "issuetitle1" is := &issues_model.Issue{ RepoID: repo.ID, @@ -62,20 +59,11 @@ func assertCreateIssues(t *testing.T, isPull bool) { IsClosed: true, Labels: []*issues_model.Label{label}, Reactions: []*issues_model.Reaction{reaction}, - ForeignReference: &foreignreference.ForeignReference{ - ForeignIndex: strconv.FormatInt(foreignIndex, 10), - RepoID: repo.ID, - Type: foreignreference.TypeIssue, - }, } err := InsertIssues(is) assert.NoError(t, err) i := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: title}) - assert.Nil(t, i.ForeignReference) - err = i.LoadAttributes(db.DefaultContext) - assert.NoError(t, err) - assert.EqualValues(t, strconv.FormatInt(foreignIndex, 10), i.ForeignReference.ForeignIndex) unittest.AssertExistsAndLoadBean(t, &issues_model.Reaction{Type: "heart", UserID: owner.ID, IssueID: i.ID}) } diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index f3848e616cccd..23aa4ac2ca9d6 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/models/foreignreference" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -403,16 +402,6 @@ func (g *GiteaLocalUploader) CreateIssues(issues ...*base.Issue) error { Labels: labels, CreatedUnix: timeutil.TimeStamp(issue.Created.Unix()), UpdatedUnix: timeutil.TimeStamp(issue.Updated.Unix()), - ForeignReference: &foreignreference.ForeignReference{ - LocalIndex: issue.GetLocalIndex(), - ForeignIndex: strconv.FormatInt(issue.GetForeignIndex(), 10), - RepoID: g.repo.ID, - Type: foreignreference.TypeIssue, - }, - } - - if is.ForeignReference.ForeignIndex == "0" { - is.ForeignReference.ForeignIndex = strconv.FormatInt(is.Index, 10) } if err := g.remapUser(issue, &is); err != nil { From d10ad3ac4c9f18d13a75c7c8859bf1b989845721 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 15:20:03 +0800 Subject: [PATCH 13/14] feat: add dropping migration --- models/migrations/migrations.go | 2 ++ models/migrations/v1_19/v237.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 models/migrations/v1_19/v237.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 591bfa3e86a63..8b7485b275ef5 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -444,6 +444,8 @@ var migrations = []Migration{ NewMigration("Add index for access_token", v1_19.AddIndexForAccessToken), // v236 -> v237 NewMigration("Create secrets table", v1_19.CreateSecretsTable), + // v237 -> v238 + NewMigration("Drop ForeignReference table", v1_19.DropForeignReference), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_19/v237.go b/models/migrations/v1_19/v237.go new file mode 100644 index 0000000000000..773fb066aef3b --- /dev/null +++ b/models/migrations/v1_19/v237.go @@ -0,0 +1,15 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_19 //nolint + +import ( + "xorm.io/xorm" +) + +func DropForeignReference(x *xorm.Engine) error { + // Drop the table introduced in `v211`, it's considered badly designed and doesn't look like to be used. + // See: https://github.com/go-gitea/gitea/issues/21086#issuecomment-1318217453 + type ForeignReference struct{} + return x.DropTables(new(ForeignReference)) +} From 7d38df327b3878d248b746673748071f1e19f850 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Fri, 23 Dec 2022 15:41:51 +0800 Subject: [PATCH 14/14] chore: rename migration func --- models/migrations/migrations.go | 2 +- models/migrations/v1_19/v237.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 8b7485b275ef5..9d9c8f5165e47 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -445,7 +445,7 @@ var migrations = []Migration{ // v236 -> v237 NewMigration("Create secrets table", v1_19.CreateSecretsTable), // v237 -> v238 - NewMigration("Drop ForeignReference table", v1_19.DropForeignReference), + NewMigration("Drop ForeignReference table", v1_19.DropForeignReferenceTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_19/v237.go b/models/migrations/v1_19/v237.go index 773fb066aef3b..b23c765aa5aac 100644 --- a/models/migrations/v1_19/v237.go +++ b/models/migrations/v1_19/v237.go @@ -7,7 +7,7 @@ import ( "xorm.io/xorm" ) -func DropForeignReference(x *xorm.Engine) error { +func DropForeignReferenceTable(x *xorm.Engine) error { // Drop the table introduced in `v211`, it's considered badly designed and doesn't look like to be used. // See: https://github.com/go-gitea/gitea/issues/21086#issuecomment-1318217453 type ForeignReference struct{}