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 f5ccba06c22b3..dd22901d70d56 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -184,6 +184,7 @@ func (run *ActionRun) IsSchedule() bool { 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..c65ee1521d995 --- /dev/null +++ b/models/actions/run_test.go @@ -0,0 +1,35 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + 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()) + + // 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) + + // 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) + assert.Equal(t, 3, repo.NumClosedActionRuns) +} 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/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/fixtures/repository.yml b/models/fixtures/repository.yml index 552a78cbd2773..dfa514db37f21 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: 3 is_private: false is_empty: false is_archived: false 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/issues/comment.go b/models/issues/comment.go index 3a4049700de1a..fd0500833e751 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -862,10 +862,7 @@ 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 - } + // 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 553e99aece290..0a320ffc56fec 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -146,8 +146,19 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User } // update repository's issue closed number - if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { - return nil, err + switch cmtType { + case CommentTypeClose, CommentTypeMergePull: + // only increase closed count + if err := IncrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { + return nil, err + } + case CommentTypeReopen: + // only decrease closed count + if err := DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false, true); err != nil { + return nil, err + } + default: + return nil, fmt.Errorf("invalid comment type: %d", cmtType) } return CreateComment(ctx, &CreateCommentOptions{ @@ -318,7 +329,6 @@ type NewIssueOptions struct { Issue *Issue LabelIDs []int64 Attachments []string // In UUID format. - IsPull bool } // NewIssueWithIndex creates issue with given index @@ -369,7 +379,8 @@ 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 { + // Update repository issue total count + if err := IncrRepoIssueNumbers(ctx, opts.Repo.ID, opts.Issue.IsPull, true); err != nil { return err } @@ -439,6 +450,42 @@ 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, 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 +} + +// 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)) + 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/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/issues/pull.go b/models/issues/pull.go index 7a37b627e1bd0..5669bed61e136 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -471,13 +471,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/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 } diff --git a/services/issue/issue.go b/services/issue/issue.go index 62b330f8e20be..85e70d0761814 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -270,16 +270,9 @@ 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 { + if err := issues_model.DecrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true, issue.IsClosed); err != nil { return nil, err } - // if the issue is closed, update the closed issue numbers - if issue.IsClosed { - if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { - return nil, err - } - } if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil { return nil, fmt.Errorf("error updating counters for milestone id %d: %w", diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 83babaca85bf4..17265c836a238 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -10,9 +10,12 @@ import ( "net/url" "path" "strings" + "sync" "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 +140,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) @@ -285,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 := 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)) + + // 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() @@ -300,8 +348,16 @@ 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").Run(t.Context(), &gitcmd.RunOpts{Dir: dstPath}) 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 c40c656a6860d..4020c880dfe11 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -99,12 +99,24 @@ 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], repo_model.MergeStyleMerge, 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) @@ -121,12 +133,24 @@ 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], repo_model.MergeStyleRebase, 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) @@ -143,12 +167,24 @@ 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], repo_model.MergeStyleRebaseMerge, 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) @@ -184,12 +220,24 @@ 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], repo_model.MergeStyleMerge, 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 {