Fix incorrect pull request counter (#35819)

Fix #35781, #27472

The PR will not correct the wrong numbers automatically. 

There is a cron task `check_repo_stats` which will be run when Gitea
start or midnight. It will correct the numbers.

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",
 		},
 	})
 }

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}),

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)
+}

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
 }
 

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
 		}
 

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

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

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")

models/issues/issue_update.go

@@ -146,9 +146,20 @@ 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 {
+	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{
 		Type:  cmtType,
@@ -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 {

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},
 		)).

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

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},
 			)).

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
 	}

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",

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,11 +348,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)
 	})
 }
 

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,11 +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,
 		})
+		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)
@@ -242,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 {