From 8158db99a050bdd28d7175fb700fc5f0622a58c8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 31 Oct 2025 11:32:01 -0700 Subject: [PATCH 01/14] improvements --- models/issues/issue_update.go | 3 +-- models/issues/pull.go | 2 +- tests/integration/pull_create_test.go | 9 +++++++++ tests/integration/pull_merge_test.go | 3 +++ 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 553e99aece290..09b344ce94710 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -318,7 +318,6 @@ type NewIssueOptions struct { Issue *Issue LabelIDs []int64 Attachments []string // In UUID format. - IsPull bool } // NewIssueWithIndex creates issue with given index @@ -369,7 +368,7 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue } } - if err := repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.IsPull, false); err != nil { + if err := repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, false); err != nil { return err } diff --git a/models/issues/pull.go b/models/issues/pull.go index fb7dff3cc9e83..1ffcd683d563e 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -467,13 +467,13 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Iss issue.Index = idx issue.Title = util.EllipsisDisplayString(issue.Title, 255) + issue.IsPull = true if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{ Repo: repo, Issue: issue, LabelIDs: labelIDs, Attachments: uuids, - IsPull: true, }); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { return err diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index d9811d000f73f..6e9964854e224 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -13,6 +13,8 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git/gitcmd" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" @@ -137,8 +139,15 @@ func TestPullCreate(t *testing.T) { session := loginUser(t, "user1") testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo1.NumPulls) + assert.Equal(t, 3, repo1.NumOpenPulls) resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") + repo1 = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 4, repo1.NumPulls) + assert.Equal(t, 4, repo1.NumOpenPulls) + // check the redirected URL url := test.RedirectURL(resp) assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url) diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 062be3ae7a198..b7ed335225877 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -229,6 +229,9 @@ func TestPullSquashWithHeadCommitID(t *testing.T) { DeleteBranch: false, HeadCommitID: headBranch.CommitID, }) + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 4, baseRepo.NumPulls) + assert.Equal(t, 3, baseRepo.NumOpenPulls) hookTasks, err = webhook.HookTasks(t.Context(), 1, 1) assert.NoError(t, err) From 61ee5c864e6c4c3391d180ef6d3ee5c8b5c11eb6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 31 Oct 2025 17:13:15 -0700 Subject: [PATCH 02/14] improvements --- models/issues/comment.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 3a4049700de1a..90cf118ef32ee 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -862,10 +862,6 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil { return err } - case CommentTypeReopen, CommentTypeClose: - if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil { - return err - } } // update the issue's updated_unix column return UpdateIssueCols(ctx, opts.Issue, "updated_unix") From f9dbff34691eaa3c1c92bc0bb1a63d024cddabbd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 1 Nov 2025 19:10:45 -0700 Subject: [PATCH 03/14] Use increment and decrement to handle number of issues and pulls instead of update select count --- models/issues/issue_update.go | 17 +++++++++++++++-- services/issue/issue.go | 19 +++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 09b344ce94710..3c7f2a7ce5a4c 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -145,8 +145,17 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User } } + colName := util.Iif(issue.IsPull, "num_closed_pulls", "num_closed_issues") + dbSession := db.GetEngine(ctx) // update repository's issue closed number - if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { + if cmtType == CommentTypeClose { + dbSession.Incr(colName) + } else { + dbSession.Decr(colName) + } + if _, err := dbSession.ID(issue.RepoID). + NoAutoCondition().NoAutoTime(). + Update(new(repo_model.Repository)); err != nil { return nil, err } @@ -368,7 +377,11 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue } } - if err := repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, false); err != nil { + // Update repository issue count + colName := util.Iif(opts.Issue.IsPull, "num_pulls", "num_issues") + if _, err := db.GetEngine(ctx).Incr(colName).ID(opts.Repo.ID). + NoAutoCondition().NoAutoTime(). + Update(new(repo_model.Repository)); err != nil { return err } diff --git a/services/issue/issue.go b/services/issue/issue.go index 62b330f8e20be..0e273e0238534 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/util" notify_service "code.gitea.io/gitea/services/notify" ) @@ -270,15 +271,17 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, erro return nil, err } - // update the total issue numbers - if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { - return nil, err - } - // if the issue is closed, update the closed issue numbers + colName := util.Iif(issue.IsPull, "num_pulls", "num_issues") + closedColName := util.Iif(issue.IsPull, "num_closed_pulls", "num_closed_issues") + dbSession := db.GetEngine(ctx) if issue.IsClosed { - if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { - return nil, err - } + dbSession.Decr(closedColName) + } + // update repository's issue both total number and closed number + if _, err := dbSession.ID(issue.RepoID).Decr(colName). + NoAutoCondition().NoAutoTime(). + Update(new(repo_model.Repository)); err != nil { + return nil, err } if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil { From 4cd4541e7f27be252afffbb6442bf20540518e28 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 1 Nov 2025 21:35:04 -0700 Subject: [PATCH 04/14] Fix test --- models/issues/issue_update.go | 6 ++- tests/integration/pull_create_test.go | 8 ++++ tests/integration/pull_merge_test.go | 64 +++++++++++++++++++++++++-- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 3c7f2a7ce5a4c..07ab4b4eb7823 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -148,10 +148,12 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User colName := util.Iif(issue.IsPull, "num_closed_pulls", "num_closed_issues") dbSession := db.GetEngine(ctx) // update repository's issue closed number - if cmtType == CommentTypeClose { + if cmtType == CommentTypeClose || cmtType == CommentTypeMergePull { dbSession.Incr(colName) - } else { + } else if cmtType == CommentTypeReopen { dbSession.Decr(colName) + } else { + return nil, fmt.Errorf("invalid comment type: %d", cmtType) } if _, err := dbSession.ID(issue.RepoID). NoAutoCondition().NoAutoTime(). diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 6e9964854e224..d3149fd4eab60 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -309,11 +309,19 @@ func TestCreateAgitPullWithReadPermission(t *testing.T) { TreeFileContent: "temp content", })(t) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + err := gitcmd.NewCommand("push", "origin", "HEAD:refs/for/master", "-o"). AddDynamicArguments("topic=test-topic"). WithDir(dstPath). Run(t.Context()) assert.NoError(t, err) + + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 4, repo.NumOpenPulls) }) } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index b7ed335225877..f273d9fb3aacb 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -113,8 +113,16 @@ func TestPullMerge(t *testing.T) { testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 4, repo.NumOpenPulls) + elem := strings.Split(test.RedirectURL(resp), "/") assert.Equal(t, "pulls", elem[3]) testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{ @@ -122,6 +130,10 @@ func TestPullMerge(t *testing.T) { DeleteBranch: false, }) + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + hookTasks, err = webhook.HookTasks(t.Context(), 1, 1) assert.NoError(t, err) assert.Len(t, hookTasks, hookTasksLenBefore+1) @@ -138,8 +150,16 @@ func TestPullRebase(t *testing.T) { testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 4, repo.NumOpenPulls) + elem := strings.Split(test.RedirectURL(resp), "/") assert.Equal(t, "pulls", elem[3]) testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{ @@ -147,6 +167,10 @@ func TestPullRebase(t *testing.T) { DeleteBranch: false, }) + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + hookTasks, err = webhook.HookTasks(t.Context(), 1, 1) assert.NoError(t, err) assert.Len(t, hookTasks, hookTasksLenBefore+1) @@ -163,8 +187,16 @@ func TestPullRebaseMerge(t *testing.T) { testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 4, repo.NumOpenPulls) + elem := strings.Split(test.RedirectURL(resp), "/") assert.Equal(t, "pulls", elem[3]) testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{ @@ -172,6 +204,10 @@ func TestPullRebaseMerge(t *testing.T) { DeleteBranch: false, }) + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + hookTasks, err = webhook.HookTasks(t.Context(), 1, 1) assert.NoError(t, err) assert.Len(t, hookTasks, hookTasksLenBefore+1) @@ -215,6 +251,10 @@ func TestPullSquashWithHeadCommitID(t *testing.T) { testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited!)\n") + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) @@ -224,14 +264,19 @@ func TestPullSquashWithHeadCommitID(t *testing.T) { elem := strings.Split(test.RedirectURL(resp), "/") assert.Equal(t, "pulls", elem[3]) + + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 4, repo.NumOpenPulls) + testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{ Style: repo_model.MergeStyleSquash, DeleteBranch: false, HeadCommitID: headBranch.CommitID, }) - baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) - assert.Equal(t, 4, baseRepo.NumPulls) - assert.Equal(t, 3, baseRepo.NumOpenPulls) + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) hookTasks, err = webhook.HookTasks(t.Context(), 1, 1) assert.NoError(t, err) @@ -245,15 +290,28 @@ func TestPullCleanUpAfterMerge(t *testing.T) { testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited - TestPullCleanUpAfterMerge)\n") + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + resp := testPullCreate(t, session, "user1", "repo1", false, "master", "feature/test", "This is a pull title") elem := strings.Split(test.RedirectURL(resp), "/") assert.Equal(t, "pulls", elem[3]) + + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 4, repo.NumOpenPulls) + testPullMerge(t, session, elem[1], elem[2], elem[4], MergeOptions{ Style: repo_model.MergeStyleMerge, DeleteBranch: false, }) + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}) + assert.Equal(t, 4, repo.NumPulls) + assert.Equal(t, 3, repo.NumOpenPulls) + // Check PR branch deletion resp = testPullCleanUp(t, session, elem[1], elem[2], elem[4]) respJSON := struct { From 39ca8953b0a1d081fdc69b6de383d6f2ebf3701c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 1 Nov 2025 22:00:53 -0700 Subject: [PATCH 05/14] Fix lint --- models/issues/issue_update.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 07ab4b4eb7823..d07a8a77cd899 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -148,11 +148,12 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User colName := util.Iif(issue.IsPull, "num_closed_pulls", "num_closed_issues") dbSession := db.GetEngine(ctx) // update repository's issue closed number - if cmtType == CommentTypeClose || cmtType == CommentTypeMergePull { + switch cmtType { + case CommentTypeClose, CommentTypeMergePull: dbSession.Incr(colName) - } else if cmtType == CommentTypeReopen { + case CommentTypeReopen: dbSession.Decr(colName) - } else { + default: return nil, fmt.Errorf("invalid comment type: %d", cmtType) } if _, err := dbSession.ID(issue.RepoID). From 4d57c70156f05b5d2146dc5d378abe7dc71075f0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 1 Nov 2025 22:29:23 -0700 Subject: [PATCH 06/14] Add a pr creating parallel test --- tests/integration/pull_create_test.go | 39 +++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index d3149fd4eab60..625bc52cf6a31 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -10,6 +10,7 @@ import ( "net/url" "path" "strings" + "sync" "testing" auth_model "code.gitea.io/gitea/models/auth" @@ -294,6 +295,44 @@ func TestPullCreatePrFromBaseToFork(t *testing.T) { }) } +func TestPullCreateParallel(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + sessionFork := loginUser(t, "user1") + testRepoFork(t, sessionFork, "user2", "repo1", "user1", "repo1", "") + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 3, repo1.NumPulls) + assert.Equal(t, 3, repo1.NumOpenPulls) + + var wg sync.WaitGroup + for i := 0; i < 5; i++ { + wg.Go(func() { + branchName := fmt.Sprintf("new-branch-%d", i) + testEditFileToNewBranch(t, sessionFork, "user1", "repo1", "master", branchName, "README.md", fmt.Sprintf("Hello, World (Edited) %d\n", i)) + + // Create a PR + resp := testPullCreateDirectly(t, sessionFork, createPullRequestOptions{ + BaseRepoOwner: "user2", + BaseRepoName: "repo1", + BaseBranch: "master", + HeadRepoOwner: "user1", + HeadRepoName: "repo1", + HeadBranch: branchName, + Title: fmt.Sprintf("This is a pull title %d", i), + }) + // check the redirected URL + url := test.RedirectURL(resp) + assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url) + }) + } + wg.Wait() + + repo1 = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + assert.Equal(t, 8, repo1.NumPulls) + assert.Equal(t, 8, repo1.NumOpenPulls) + }) +} + func TestCreateAgitPullWithReadPermission(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { dstPath := t.TempDir() From e2d35677c79af07c2c8446e19fcd5f06141d2a60 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 2 Nov 2025 00:59:53 -0700 Subject: [PATCH 07/14] Fix checks --- tests/integration/pull_create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 625bc52cf6a31..ddafdf33b837d 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -305,7 +305,7 @@ func TestPullCreateParallel(t *testing.T) { assert.Equal(t, 3, repo1.NumOpenPulls) var wg sync.WaitGroup - for i := 0; i < 5; i++ { + for i := range 5 { wg.Go(func() { branchName := fmt.Sprintf("new-branch-%d", i) testEditFileToNewBranch(t, sessionFork, "user1", "repo1", "master", branchName, "README.md", fmt.Sprintf("Hello, World (Edited) %d\n", i)) From aa21dc3f4c6b03501b49d23ff534a495d66b66f7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 2 Nov 2025 01:21:22 -0800 Subject: [PATCH 08/14] Fix bug when updating actions numbers --- models/actions/main_test.go | 2 ++ models/actions/run.go | 2 ++ models/actions/run_test.go | 27 +++++++++++++++++++++++++++ models/fixtures/repository.yml | 2 ++ 4 files changed, 33 insertions(+) create mode 100644 models/actions/run_test.go diff --git a/models/actions/main_test.go b/models/actions/main_test.go index 5d5089e3bba88..4af483813ab53 100644 --- a/models/actions/main_test.go +++ b/models/actions/main_test.go @@ -13,6 +13,8 @@ func TestMain(m *testing.M) { unittest.MainTest(m, &unittest.TestOptions{ FixtureFiles: []string{ "action_runner_token.yml", + "action_run.yml", + "repository.yml", }, }) } diff --git a/models/actions/run.go b/models/actions/run.go index 4da6958e2d55a..be332d6857d89 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -193,9 +193,11 @@ func (run *ActionRun) IsSchedule() bool { return run.ScheduleID > 0 } +// UpdateRepoRunsNumbers updates the number of runs and closed runs of a repository. func UpdateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) error { _, err := db.GetEngine(ctx).ID(repo.ID). NoAutoTime(). + Cols("num_action_runs", "num_closed_action_runs"). SetExpr("num_action_runs", builder.Select("count(*)").From("action_run"). Where(builder.Eq{"repo_id": repo.ID}), diff --git a/models/actions/run_test.go b/models/actions/run_test.go new file mode 100644 index 0000000000000..6694e5853a5aa --- /dev/null +++ b/models/actions/run_test.go @@ -0,0 +1,27 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestUpdateRepoRunsNumbers(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + assert.Equal(t, 4, repo.NumActionRuns) + assert.Equal(t, 2, repo.NumClosedActionRuns) + + err := UpdateRepoRunsNumbers(t.Context(), repo) + assert.NoError(t, err) + repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + assert.Equal(t, 4, repo.NumActionRuns) + assert.Equal(t, 3, repo.NumClosedActionRuns) +} diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 552a78cbd2773..34f6995f44d16 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -110,6 +110,8 @@ num_closed_milestones: 0 num_projects: 0 num_closed_projects: 1 + num_action_runs: 4 + num_closed_action_runs: 2 # this is wrong, should be 3. It's for testing purpose of run_test.go is_private: false is_empty: false is_archived: false From 4761bf20127ca20c2f8819eb3294e6bcb0bf10a6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 2 Nov 2025 01:30:35 -0800 Subject: [PATCH 09/14] Fix bug when updating actions numbers --- models/asymkey/gpg_key_verify.go | 2 +- models/issues/label.go | 16 ++++++++++------ models/issues/milestone.go | 1 + models/migrations/v1_18/v229.go | 1 + models/repo/topic.go | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/models/asymkey/gpg_key_verify.go b/models/asymkey/gpg_key_verify.go index 55c64973b4121..5df0265c88082 100644 --- a/models/asymkey/gpg_key_verify.go +++ b/models/asymkey/gpg_key_verify.go @@ -78,7 +78,7 @@ func VerifyGPGKey(ctx context.Context, ownerID int64, keyID, token, signature st } key.Verified = true - if _, err := db.GetEngine(ctx).ID(key.ID).SetExpr("verified", true).Update(new(GPGKey)); err != nil { + if _, err := db.GetEngine(ctx).ID(key.ID).Cols("verified").Update(key); err != nil { return "", err } diff --git a/models/issues/label.go b/models/issues/label.go index 25d6f1303e8dc..af89e9df7d4df 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -495,19 +495,23 @@ func CountLabelsByOrgID(ctx context.Context, orgID int64) (int64, error) { } func updateLabelCols(ctx context.Context, l *Label, cols ...string) error { - _, err := db.GetEngine(ctx).ID(l.ID). - SetExpr("num_issues", + sess := db.GetEngine(ctx).ID(l.ID) + if slices.Contains(cols, "num_issues") { + sess.SetExpr("num_issues", builder.Select("count(*)").From("issue_label"). Where(builder.Eq{"label_id": l.ID}), - ). - SetExpr("num_closed_issues", + ) + } + if slices.Contains(cols, "num_closed_issues") { + sess.SetExpr("num_closed_issues", builder.Select("count(*)").From("issue_label"). InnerJoin("issue", "issue_label.issue_id = issue.id"). Where(builder.Eq{ "issue_label.label_id": l.ID, "issue.is_closed": true, }), - ). - Cols(cols...).Update(l) + ) + } + _, err := sess.Cols(cols...).Update(l) return err } diff --git a/models/issues/milestone.go b/models/issues/milestone.go index 373f39f4ffe82..82a82ac9132b2 100644 --- a/models/issues/milestone.go +++ b/models/issues/milestone.go @@ -181,6 +181,7 @@ func updateMilestone(ctx context.Context, m *Milestone) error { func UpdateMilestoneCounters(ctx context.Context, id int64) error { e := db.GetEngine(ctx) _, err := e.ID(id). + Cols("num_issues", "num_closed_issues"). SetExpr("num_issues", builder.Select("count(*)").From("issue").Where( builder.Eq{"milestone_id": id}, )). diff --git a/models/migrations/v1_18/v229.go b/models/migrations/v1_18/v229.go index bc15e01390862..1f69724365a8d 100644 --- a/models/migrations/v1_18/v229.go +++ b/models/migrations/v1_18/v229.go @@ -21,6 +21,7 @@ func UpdateOpenMilestoneCounts(x *xorm.Engine) error { for _, id := range openMilestoneIDs { _, err := x.ID(id). + Cols("num_issues", "num_closed_issues"). SetExpr("num_issues", builder.Select("count(*)").From("issue").Where( builder.Eq{"milestone_id": id}, )). diff --git a/models/repo/topic.go b/models/repo/topic.go index baeae01efaee6..f8f706fc1a58a 100644 --- a/models/repo/topic.go +++ b/models/repo/topic.go @@ -159,7 +159,7 @@ func RemoveTopicsFromRepo(ctx context.Context, repoID int64) error { builder.In("id", builder.Select("topic_id").From("repo_topic").Where(builder.Eq{"repo_id": repoID}), ), - ).Cols("repo_count").SetExpr("repo_count", "repo_count-1").Update(&Topic{}) + ).Decr("repo_count").Update(&Topic{}) if err != nil { return err } From c7a6ebf40203f357c2c35a8d999d20f82a709cd0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 2 Nov 2025 01:52:54 -0800 Subject: [PATCH 10/14] add more cols to avoid wrong columns to be updated --- models/activities/notification.go | 2 +- models/git/branch.go | 2 +- models/user/must_change_password.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/models/activities/notification.go b/models/activities/notification.go index b482e6020af2f..8a830c5aa26a8 100644 --- a/models/activities/notification.go +++ b/models/activities/notification.go @@ -386,7 +386,7 @@ func SetNotificationStatus(ctx context.Context, notificationID int64, user *user notification.Status = status - _, err = db.GetEngine(ctx).ID(notificationID).Update(notification) + _, err = db.GetEngine(ctx).ID(notificationID).Cols("status").Update(notification) return notification, err } diff --git a/models/git/branch.go b/models/git/branch.go index 54351649cc5ec..7fef9f5ca35e1 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -368,7 +368,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str } // 1. update branch in database - if n, err := sess.Where("repo_id=? AND name=?", repo.ID, from).Update(&Branch{ + if n, err := sess.Where("repo_id=? AND name=?", repo.ID, from).Cols("name").Update(&Branch{ Name: to, }); err != nil { return err diff --git a/models/user/must_change_password.go b/models/user/must_change_password.go index 686847c7d7cf6..e0de6f1a78c5a 100644 --- a/models/user/must_change_password.go +++ b/models/user/must_change_password.go @@ -45,5 +45,5 @@ func SetMustChangePassword(ctx context.Context, all, mustChangePassword bool, in cond = cond.And(builder.NotIn("lower_name", exclude)) } - return db.GetEngine(ctx).Where(cond).MustCols("must_change_password").Update(&User{MustChangePassword: mustChangePassword}) + return db.GetEngine(ctx).Where(cond).Cols("must_change_password").Update(&User{MustChangePassword: mustChangePassword}) } From 3454c0915db742a5baabdcf8f6d68128db1b6919 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 3 Nov 2025 10:01:51 -0800 Subject: [PATCH 11/14] revert some changes --- models/issues/label.go | 16 ++++++---------- models/user/must_change_password.go | 2 +- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/models/issues/label.go b/models/issues/label.go index af89e9df7d4df..25d6f1303e8dc 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -495,23 +495,19 @@ func CountLabelsByOrgID(ctx context.Context, orgID int64) (int64, error) { } func updateLabelCols(ctx context.Context, l *Label, cols ...string) error { - sess := db.GetEngine(ctx).ID(l.ID) - if slices.Contains(cols, "num_issues") { - sess.SetExpr("num_issues", + _, err := db.GetEngine(ctx).ID(l.ID). + SetExpr("num_issues", builder.Select("count(*)").From("issue_label"). Where(builder.Eq{"label_id": l.ID}), - ) - } - if slices.Contains(cols, "num_closed_issues") { - sess.SetExpr("num_closed_issues", + ). + SetExpr("num_closed_issues", builder.Select("count(*)").From("issue_label"). InnerJoin("issue", "issue_label.issue_id = issue.id"). Where(builder.Eq{ "issue_label.label_id": l.ID, "issue.is_closed": true, }), - ) - } - _, err := sess.Cols(cols...).Update(l) + ). + Cols(cols...).Update(l) return err } diff --git a/models/user/must_change_password.go b/models/user/must_change_password.go index e0de6f1a78c5a..686847c7d7cf6 100644 --- a/models/user/must_change_password.go +++ b/models/user/must_change_password.go @@ -45,5 +45,5 @@ func SetMustChangePassword(ctx context.Context, all, mustChangePassword bool, in cond = cond.And(builder.NotIn("lower_name", exclude)) } - return db.GetEngine(ctx).Where(cond).Cols("must_change_password").Update(&User{MustChangePassword: mustChangePassword}) + return db.GetEngine(ctx).Where(cond).MustCols("must_change_password").Update(&User{MustChangePassword: mustChangePassword}) } From a27061e269cc2d9c5c4033897b44e2046e5b2218 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 3 Nov 2025 10:27:06 -0800 Subject: [PATCH 12/14] improvements --- models/actions/run_test.go | 10 ++++++++- models/fixtures/repository.yml | 2 +- models/issues/comment.go | 1 + models/issues/issue_update.go | 41 +++++++++++++++++++++++----------- services/issue/issue.go | 12 +--------- 5 files changed, 40 insertions(+), 26 deletions(-) diff --git a/models/actions/run_test.go b/models/actions/run_test.go index 6694e5853a5aa..0986f87516e42 100644 --- a/models/actions/run_test.go +++ b/models/actions/run_test.go @@ -6,6 +6,7 @@ package actions import ( "testing" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -15,11 +16,18 @@ import ( func TestUpdateRepoRunsNumbers(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) + // update the number to a wrong one, the original is 3 + _, err := db.GetEngine(t.Context()).ID(4).Cols("num_closed_action_runs").Update(&repo_model.Repository{ + NumClosedActionRuns: 2, + }) + assert.NoError(t, err) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) assert.Equal(t, 4, repo.NumActionRuns) assert.Equal(t, 2, repo.NumClosedActionRuns) - err := UpdateRepoRunsNumbers(t.Context(), repo) + // now update will correct them, only num_actionr_runs and num_closed_action_runs should be updated + err = UpdateRepoRunsNumbers(t.Context(), repo) assert.NoError(t, err) repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) assert.Equal(t, 4, repo.NumActionRuns) diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 34f6995f44d16..dfa514db37f21 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -111,7 +111,7 @@ num_projects: 0 num_closed_projects: 1 num_action_runs: 4 - num_closed_action_runs: 2 # this is wrong, should be 3. It's for testing purpose of run_test.go + num_closed_action_runs: 3 is_private: false is_empty: false is_archived: false diff --git a/models/issues/comment.go b/models/issues/comment.go index 90cf118ef32ee..fd0500833e751 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -862,6 +862,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment if err = UpdateCommentAttachments(ctx, comment, opts.Attachments); err != nil { return err } + // comment type reopen and close event have their own logic to update numbers but not here } // update the issue's updated_unix column return UpdateIssueCols(ctx, opts.Issue, "updated_unix") diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index d07a8a77cd899..51b103919bda8 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -145,22 +145,19 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User } } - colName := util.Iif(issue.IsPull, "num_closed_pulls", "num_closed_issues") - dbSession := db.GetEngine(ctx) // update repository's issue closed number switch cmtType { case CommentTypeClose, CommentTypeMergePull: - dbSession.Incr(colName) + if err := DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { + return nil, err + } case CommentTypeReopen: - dbSession.Decr(colName) + if err := IncrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull); err != nil { + return nil, err + } default: return nil, fmt.Errorf("invalid comment type: %d", cmtType) } - if _, err := dbSession.ID(issue.RepoID). - NoAutoCondition().NoAutoTime(). - Update(new(repo_model.Repository)); err != nil { - return nil, err - } return CreateComment(ctx, &CreateCommentOptions{ Type: cmtType, @@ -381,10 +378,7 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue } // Update repository issue count - colName := util.Iif(opts.Issue.IsPull, "num_pulls", "num_issues") - if _, err := db.GetEngine(ctx).Incr(colName).ID(opts.Repo.ID). - NoAutoCondition().NoAutoTime(). - Update(new(repo_model.Repository)); err != nil { + if err := IncrRepoIssueNumbers(ctx, opts.Repo.ID, opts.Issue.IsPull); err != nil { return err } @@ -454,6 +448,27 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *Issue, la }) } +func IncrRepoIssueNumbers(ctx context.Context, repoID int64, isPull bool) error { + colName := util.Iif(isPull, "num_pulls", "num_issues") + _, err := db.GetEngine(ctx).Incr(colName).ID(repoID). + NoAutoCondition().NoAutoTime(). + Update(new(repo_model.Repository)) + return err +} + +func DecrRepoIssueNumbers(ctx context.Context, repoID int64, isPull, includeTotal bool) error { + closedColName := util.Iif(isPull, "num_closed_pulls", "num_closed_issues") + dbSession := db.GetEngine(ctx).Decr(closedColName) + if includeTotal { + colName := util.Iif(isPull, "num_pulls", "num_issues") + dbSession = dbSession.Decr(colName) + } + _, err := dbSession.ID(repoID). + NoAutoCondition().NoAutoTime(). + Update(new(repo_model.Repository)) + return err +} + // UpdateIssueMentions updates issue-user relations for mentioned users. func UpdateIssueMentions(ctx context.Context, issueID int64, mentions []*user_model.User) error { if len(mentions) == 0 { diff --git a/services/issue/issue.go b/services/issue/issue.go index 0e273e0238534..0ddc7fc4f9245 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -20,7 +20,6 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/storage" - "code.gitea.io/gitea/modules/util" notify_service "code.gitea.io/gitea/services/notify" ) @@ -271,16 +270,7 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, erro return nil, err } - colName := util.Iif(issue.IsPull, "num_pulls", "num_issues") - closedColName := util.Iif(issue.IsPull, "num_closed_pulls", "num_closed_issues") - dbSession := db.GetEngine(ctx) - if issue.IsClosed { - dbSession.Decr(closedColName) - } - // update repository's issue both total number and closed number - if _, err := dbSession.ID(issue.RepoID).Decr(colName). - NoAutoCondition().NoAutoTime(). - Update(new(repo_model.Repository)); err != nil { + if err := issues_model.DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { return nil, err } From d95580317b7dc5e76bdcd146b4f5b7c71a3b94e6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 3 Nov 2025 11:30:01 -0800 Subject: [PATCH 13/14] Fix bug --- models/issues/issue_update.go | 37 +++++++++++++++++++++++++---------- services/issue/issue.go | 2 +- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 51b103919bda8..4917d8950921e 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -148,11 +148,13 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User // update repository's issue closed number switch cmtType { case CommentTypeClose, CommentTypeMergePull: - if err := DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { + // only increase closed count + if err := IncrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { return nil, err } case CommentTypeReopen: - if err := IncrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull); err != nil { + // only decrease closed count + if err := DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false, true); err != nil { return nil, err } default: @@ -377,8 +379,8 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue } } - // Update repository issue count - if err := IncrRepoIssueNumbers(ctx, opts.Repo.ID, opts.Issue.IsPull); err != nil { + // Update repository issue total count + if err := IncrRepoIssueNumbers(ctx, opts.Repo.ID, opts.Issue.IsPull, true); err != nil { return err } @@ -448,21 +450,36 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *Issue, la }) } -func IncrRepoIssueNumbers(ctx context.Context, repoID int64, isPull bool) error { - colName := util.Iif(isPull, "num_pulls", "num_issues") - _, err := db.GetEngine(ctx).Incr(colName).ID(repoID). +// IncrRepoIssueNumbers increments repository issue numbers. +func IncrRepoIssueNumbers(ctx context.Context, repoID int64, isPull bool, totalOrClosed bool) error { + dbSession := db.GetEngine(ctx) + var colName string + if totalOrClosed { + colName = util.Iif(isPull, "num_pulls", "num_issues") + } else { + colName = util.Iif(isPull, "num_closed_pulls", "num_closed_issues") + } + _, err := dbSession.Incr(colName).ID(repoID). NoAutoCondition().NoAutoTime(). Update(new(repo_model.Repository)) return err } -func DecrRepoIssueNumbers(ctx context.Context, repoID int64, isPull, includeTotal bool) error { - closedColName := util.Iif(isPull, "num_closed_pulls", "num_closed_issues") - dbSession := db.GetEngine(ctx).Decr(closedColName) +// DecrRepoIssueNumbers decrements repository issue numbers. +func DecrRepoIssueNumbers(ctx context.Context, repoID int64, isPull, includeTotal, includeClosed bool) error { + if !includeTotal && !includeClosed { + return fmt.Errorf("no numbers to decrease for repo id %d", repoID) + } + + dbSession := db.GetEngine(ctx) if includeTotal { colName := util.Iif(isPull, "num_pulls", "num_issues") dbSession = dbSession.Decr(colName) } + if includeClosed { + closedColName := util.Iif(isPull, "num_closed_pulls", "num_closed_issues") + dbSession = dbSession.Decr(closedColName) + } _, err := dbSession.ID(repoID). NoAutoCondition().NoAutoTime(). Update(new(repo_model.Repository)) diff --git a/services/issue/issue.go b/services/issue/issue.go index 0ddc7fc4f9245..85e70d0761814 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -270,7 +270,7 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, erro return nil, err } - if err := issues_model.DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { + if err := issues_model.DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true, issue.IsClosed); err != nil { return nil, err } From e33653a78069221afd6b4a1fed46d2d92be24fef Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 3 Nov 2025 11:35:51 -0800 Subject: [PATCH 14/14] Fix checks --- models/issues/issue_update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 4917d8950921e..0a320ffc56fec 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -451,7 +451,7 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *Issue, la } // IncrRepoIssueNumbers increments repository issue numbers. -func IncrRepoIssueNumbers(ctx context.Context, repoID int64, isPull bool, totalOrClosed bool) error { +func IncrRepoIssueNumbers(ctx context.Context, repoID int64, isPull, totalOrClosed bool) error { dbSession := db.GetEngine(ctx) var colName string if totalOrClosed {