Do not update PRs based on events that happened before they existed

* Split TestPullRequest out of AddTestPullRequestTask
* A Created field is added to the Issue table
* The Created field is set to the time (with nano resolution) on creation
* Record the nano time repo_module.PushUpdateOptions is created by the hook
* The decision to update a pull request created before a commit was
  pushed is based on the time (with nano resolution) the git hook
  was run and the Created field

It ensures the following happens:

* commit C is pushed
* the git hook queues AddTestPullRequestTask for processing and returns with success
* TestPullRequest is not called yet
* a pull request P with commit C as the head is created
* TestPullRequest runs and ignores P because it was created after the commit was received

When the "created" column is NULL, no verification is done, pull
requests that were created before the column was created in the
database cannot be newer than the latest call to a git hook.

Fixes: https://codeberg.org/forgejo/forgejo/issues/2009

6 files changed, 157 insertions, 2 deletions

diff --git a/models/issues/issue.go b/models/issues/issue.go
index 11256f788a..affd581929 100644
--- a/models/issues/issue.go
+++ b/models/issues/issue.go
@@ -124,6 +124,8 @@ type Issue struct {
 
 	DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"`
 
+	Created timeutil.TimeStampNano
+
 	CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
 	UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
 	ClosedUnix  timeutil.TimeStamp `xorm:"INDEX"`
diff --git a/models/issues/issue_index.go b/models/issues/issue_index.go
index 16274d0ef0..9386027f74 100644
--- a/models/issues/issue_index.go
+++ b/models/issues/issue_index.go
@@ -9,6 +9,14 @@ import (
 	"code.gitea.io/gitea/models/db"
 )
 
+func GetMaxIssueIndexForRepo(ctx context.Context, repoID int64) (int64, error) {
+	var max int64
+	if _, err := db.GetEngine(ctx).Select("MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&max); err != nil {
+		return 0, err
+	}
+	return max, nil
+}
+
 // RecalculateIssueIndexForRepo create issue_index for repo if not exist and
 // update it based on highest index of existing issues assigned to a repo
 func RecalculateIssueIndexForRepo(ctx context.Context, repoID int64) error {
@@ -18,8 +26,8 @@ func RecalculateIssueIndexForRepo(ctx context.Context, repoID int64) error {
 	}
 	defer committer.Close()
 
-	var max int64
-	if _, err = db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&max); err != nil {
+	max, err := GetMaxIssueIndexForRepo(ctx, repoID)
+	if err != nil {
 		return err
 	}
 
diff --git a/models/issues/issue_index_test.go b/models/issues/issue_index_test.go
new file mode 100644
index 0000000000..9937aac70e
--- /dev/null
+++ b/models/issues/issue_index_test.go
@@ -0,0 +1,38 @@
+// Copyright 2024 The Forgejo Authors
+// SPDX-License-Identifier: MIT
+
+package issues_test
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	issues_model "code.gitea.io/gitea/models/issues"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetMaxIssueIndexForRepo(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	maxPR, err := issues_model.GetMaxIssueIndexForRepo(db.DefaultContext, repo.ID)
+	assert.NoError(t, err)
+
+	issue := testCreateIssue(t, repo.ID, repo.OwnerID, "title1", "content1", false)
+	assert.Greater(t, issue.Index, maxPR)
+
+	maxPR, err = issues_model.GetMaxIssueIndexForRepo(db.DefaultContext, repo.ID)
+	assert.NoError(t, err)
+
+	pull := testCreateIssue(t, repo.ID, repo.OwnerID, "title2", "content2", true)
+	assert.Greater(t, pull.Index, maxPR)
+
+	maxPR, err = issues_model.GetMaxIssueIndexForRepo(db.DefaultContext, repo.ID)
+	assert.NoError(t, err)
+
+	assert.Equal(t, maxPR, pull.Index)
+}
diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go
index f20d552a1b..78e1f8e030 100644
--- a/models/issues/issue_update.go
+++ b/models/issues/issue_update.go
@@ -325,6 +325,8 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue
 		return fmt.Errorf("issue exist")
 	}
 
+	opts.Issue.Created = timeutil.TimeStampNanoNow()
+
 	if _, err := e.Insert(opts.Issue); err != nil {
 		return err
 	}
diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go
index de3eceed37..61b4168ea2 100644
--- a/models/issues/pull_list.go
+++ b/models/issues/pull_list.go
@@ -47,6 +47,14 @@ func listPullRequestStatement(ctx context.Context, baseRepoID int64, opts *PullR
 	return sess, nil
 }
 
+func GetUnmergedPullRequestsByHeadInfoMax(ctx context.Context, repoID, olderThan int64, branch string) ([]*PullRequest, error) {
+	prs := make([]*PullRequest, 0, 2)
+	sess := db.GetEngine(ctx).
+		Join("INNER", "issue", "issue.id = `pull_request`.issue_id").
+		Where("`pull_request`.head_repo_id = ? AND `pull_request`.head_branch = ? AND `pull_request`.has_merged = ? AND `issue`.is_closed = ? AND `pull_request`.flow = ? AND (`issue`.`created` IS NULL OR `issue`.`created` <= ?)", repoID, branch, false, false, PullRequestFlowGithub, olderThan)
+	return prs, sess.Find(&prs)
+}
+
 // GetUnmergedPullRequestsByHeadInfo returns all pull requests that are open and has not been merged
 func GetUnmergedPullRequestsByHeadInfo(ctx context.Context, repoID int64, branch string) ([]*PullRequest, error) {
 	prs := make([]*PullRequest, 0, 2)
diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go
index 675c90527d..a9d4edc8a5 100644
--- a/models/issues/pull_test.go
+++ b/models/issues/pull_test.go
@@ -4,7 +4,9 @@
 package issues_test
 
 import (
+	"fmt"
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
@@ -12,6 +14,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -156,6 +159,100 @@ func TestGetUnmergedPullRequestsByHeadInfo(t *testing.T) {
 	}
 }
 
+func TestGetUnmergedPullRequestsByHeadInfoMax(t *testing.T) {
+	defer tests.AddFixtures("models/fixtures/TestGetUnmergedPullRequestsByHeadInfoMax/")()
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repoID := int64(1)
+	olderThan := int64(0)
+
+	// for NULL created field the olderThan condition is ignored
+	prs, err := issues_model.GetUnmergedPullRequestsByHeadInfoMax(db.DefaultContext, repoID, olderThan, "branch2")
+	assert.NoError(t, err)
+	assert.Equal(t, int64(1), prs[0].HeadRepoID)
+
+	// test for when the created field is set
+	branch := "branchmax"
+	prs, err = issues_model.GetUnmergedPullRequestsByHeadInfoMax(db.DefaultContext, repoID, olderThan, branch)
+	assert.NoError(t, err)
+	assert.Len(t, prs, 0)
+	olderThan = time.Now().UnixNano()
+	assert.NoError(t, err)
+	prs, err = issues_model.GetUnmergedPullRequestsByHeadInfoMax(db.DefaultContext, repoID, olderThan, branch)
+	assert.NoError(t, err)
+	assert.Len(t, prs, 1)
+	for _, pr := range prs {
+		assert.Equal(t, int64(1), pr.HeadRepoID)
+		assert.Equal(t, branch, pr.HeadBranch)
+	}
+	pr := prs[0]
+
+	for _, testCase := range []struct {
+		table   string
+		field   string
+		id      int64
+		match   any
+		nomatch any
+	}{
+		{
+			table:   "issue",
+			field:   "is_closed",
+			id:      pr.IssueID,
+			match:   false,
+			nomatch: true,
+		},
+		{
+			table:   "pull_request",
+			field:   "flow",
+			id:      pr.ID,
+			match:   issues_model.PullRequestFlowGithub,
+			nomatch: issues_model.PullRequestFlowAGit,
+		},
+		{
+			table:   "pull_request",
+			field:   "head_repo_id",
+			id:      pr.ID,
+			match:   pr.HeadRepoID,
+			nomatch: 0,
+		},
+		{
+			table:   "pull_request",
+			field:   "head_branch",
+			id:      pr.ID,
+			match:   pr.HeadBranch,
+			nomatch: "something else",
+		},
+		{
+			table:   "pull_request",
+			field:   "has_merged",
+			id:      pr.ID,
+			match:   false,
+			nomatch: true,
+		},
+	} {
+		t.Run(testCase.field, func(t *testing.T) {
+			update := fmt.Sprintf("UPDATE `%s` SET `%s` = ? WHERE `id` = ?", testCase.table, testCase.field)
+
+			// expect no match
+			_, err = db.GetEngine(db.DefaultContext).Exec(update, testCase.nomatch, testCase.id)
+			assert.NoError(t, err)
+			prs, err = issues_model.GetUnmergedPullRequestsByHeadInfoMax(db.DefaultContext, repoID, olderThan, branch)
+			assert.NoError(t, err)
+			assert.Len(t, prs, 0)
+
+			// expect one match
+			_, err = db.GetEngine(db.DefaultContext).Exec(update, testCase.match, testCase.id)
+			assert.NoError(t, err)
+			prs, err = issues_model.GetUnmergedPullRequestsByHeadInfoMax(db.DefaultContext, repoID, olderThan, branch)
+			assert.NoError(t, err)
+			assert.Len(t, prs, 1)
+
+			// identical to the known PR
+			assert.Equal(t, pr.ID, prs[0].ID)
+		})
+	}
+}
+
 func TestGetUnmergedPullRequestsByBaseInfo(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(db.DefaultContext, 1, "master")