---
templates/repo/actions/runs_list.tmpl | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/templates/repo/actions/runs_list.tmpl b/templates/repo/actions/runs_list.tmpl
index 4ebedcd73b..a37731cffb 100644
--- a/templates/repo/actions/runs_list.tmpl
+++ b/templates/repo/actions/runs_list.tmpl
@@ -36,15 +36,23 @@
{{svg "octicon-calendar" 16}}{{DateUtils.TimeSince $run.Updated}}
{{svg "octicon-stopwatch" 16}}{{$run.Duration}}
- {{if and ($.AllowDeleteWorkflowRuns) ($run.Status.IsDone)}}
-
- {{end}}
+
+ {{svg "octicon-kebab-horizontal"}}
+
+
{{end}}
From 11ee7ff3bfb21fedf6a069d61149b0b3d51ce7f2 Mon Sep 17 00:00:00 2001
From: Bo-Yi Wu
Date: Tue, 27 May 2025 00:12:49 +0800
Subject: [PATCH 11/18] fix: return 201 Created for CreateVariable API
responses (#34517)
- Change CreateVariable API response status from 204 No Content to 201
Created
- Update related integration tests to expect 201 Created instead of 204
No Content
## :warning: BREAKING :warning:
Change the response status code of the Create Variable API under both
Org and Repo levels to `201` instead of 204.
API SDK: https://gitea.com/gitea/go-sdk/pulls/713
---------
Signed-off-by: Bo-Yi Wu
Signed-off-by: appleboy
Co-authored-by: delvh
---
routers/api/v1/org/action.go | 12 ++++----
routers/api/v1/repo/action.go | 10 +++----
routers/api/v1/user/action.go | 10 +++----
templates/swagger/v1_json.tmpl | 31 +++++++++-----------
tests/integration/api_repo_variables_test.go | 8 ++---
tests/integration/api_user_variables_test.go | 8 ++---
6 files changed, 37 insertions(+), 42 deletions(-)
diff --git a/routers/api/v1/org/action.go b/routers/api/v1/org/action.go
index 700a5ef8ea..5cd3ea07f4 100644
--- a/routers/api/v1/org/action.go
+++ b/routers/api/v1/org/action.go
@@ -384,13 +384,13 @@ func (Action) CreateVariable(ctx *context.APIContext) {
// "$ref": "#/definitions/CreateVariableOption"
// responses:
// "201":
- // description: response when creating an org-level variable
- // "204":
- // description: response when creating an org-level variable
+ // description: successfully created the org-level variable
// "400":
// "$ref": "#/responses/error"
- // "404":
- // "$ref": "#/responses/notFound"
+ // "409":
+ // description: variable name already exists.
+ // "500":
+ // "$ref": "#/responses/error"
opt := web.GetForm(ctx).(*api.CreateVariableOption)
@@ -419,7 +419,7 @@ func (Action) CreateVariable(ctx *context.APIContext) {
return
}
- ctx.Status(http.StatusNoContent)
+ ctx.Status(http.StatusCreated)
}
// UpdateVariable update an org-level variable
diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go
index 237250b2c5..3f64e2477d 100644
--- a/routers/api/v1/repo/action.go
+++ b/routers/api/v1/repo/action.go
@@ -339,12 +339,12 @@ func (Action) CreateVariable(ctx *context.APIContext) {
// responses:
// "201":
// description: response when creating a repo-level variable
- // "204":
- // description: response when creating a repo-level variable
// "400":
// "$ref": "#/responses/error"
- // "404":
- // "$ref": "#/responses/notFound"
+ // "409":
+ // description: variable name already exists.
+ // "500":
+ // "$ref": "#/responses/error"
opt := web.GetForm(ctx).(*api.CreateVariableOption)
@@ -373,7 +373,7 @@ func (Action) CreateVariable(ctx *context.APIContext) {
return
}
- ctx.Status(http.StatusNoContent)
+ ctx.Status(http.StatusCreated)
}
// UpdateVariable update a repo-level variable
diff --git a/routers/api/v1/user/action.go b/routers/api/v1/user/action.go
index 04097fcc95..7e4ffd9403 100644
--- a/routers/api/v1/user/action.go
+++ b/routers/api/v1/user/action.go
@@ -127,13 +127,11 @@ func CreateVariable(ctx *context.APIContext) {
// "$ref": "#/definitions/CreateVariableOption"
// responses:
// "201":
- // description: response when creating a variable
- // "204":
- // description: response when creating a variable
+ // description: successfully created the user-level variable
// "400":
// "$ref": "#/responses/error"
- // "404":
- // "$ref": "#/responses/notFound"
+ // "409":
+ // description: variable name already exists.
opt := web.GetForm(ctx).(*api.CreateVariableOption)
@@ -162,7 +160,7 @@ func CreateVariable(ctx *context.APIContext) {
return
}
- ctx.Status(http.StatusNoContent)
+ ctx.Status(http.StatusCreated)
}
// UpdateVariable update a user-level variable which is created by current doer
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index c97e525660..dd2048ddbb 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -2259,16 +2259,16 @@
],
"responses": {
"201": {
- "description": "response when creating an org-level variable"
- },
- "204": {
- "description": "response when creating an org-level variable"
+ "description": "successfully created the org-level variable"
},
"400": {
"$ref": "#/responses/error"
},
- "404": {
- "$ref": "#/responses/notFound"
+ "409": {
+ "description": "variable name already exists."
+ },
+ "500": {
+ "$ref": "#/responses/error"
}
}
},
@@ -5263,14 +5263,14 @@
"201": {
"description": "response when creating a repo-level variable"
},
- "204": {
- "description": "response when creating a repo-level variable"
- },
"400": {
"$ref": "#/responses/error"
},
- "404": {
- "$ref": "#/responses/notFound"
+ "409": {
+ "description": "variable name already exists."
+ },
+ "500": {
+ "$ref": "#/responses/error"
}
}
},
@@ -17863,16 +17863,13 @@
],
"responses": {
"201": {
- "description": "response when creating a variable"
- },
- "204": {
- "description": "response when creating a variable"
+ "description": "successfully created the user-level variable"
},
"400": {
"$ref": "#/responses/error"
},
- "404": {
- "$ref": "#/responses/notFound"
+ "409": {
+ "description": "variable name already exists."
}
}
},
diff --git a/tests/integration/api_repo_variables_test.go b/tests/integration/api_repo_variables_test.go
index 7847962b07..b5c88af279 100644
--- a/tests/integration/api_repo_variables_test.go
+++ b/tests/integration/api_repo_variables_test.go
@@ -35,11 +35,11 @@ func TestAPIRepoVariables(t *testing.T) {
},
{
Name: "_",
- ExpectedStatus: http.StatusNoContent,
+ ExpectedStatus: http.StatusCreated,
},
{
Name: "TEST_VAR",
- ExpectedStatus: http.StatusNoContent,
+ ExpectedStatus: http.StatusCreated,
},
{
Name: "test_var",
@@ -81,7 +81,7 @@ func TestAPIRepoVariables(t *testing.T) {
req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{
Value: "initial_val",
}).AddTokenAuth(token)
- MakeRequest(t, req, http.StatusNoContent)
+ MakeRequest(t, req, http.StatusCreated)
cases := []struct {
Name string
@@ -138,7 +138,7 @@ func TestAPIRepoVariables(t *testing.T) {
req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{
Value: "initial_val",
}).AddTokenAuth(token)
- MakeRequest(t, req, http.StatusNoContent)
+ MakeRequest(t, req, http.StatusCreated)
req = NewRequest(t, "DELETE", url).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNoContent)
diff --git a/tests/integration/api_user_variables_test.go b/tests/integration/api_user_variables_test.go
index 367b83e7d4..d430c9e21d 100644
--- a/tests/integration/api_user_variables_test.go
+++ b/tests/integration/api_user_variables_test.go
@@ -29,11 +29,11 @@ func TestAPIUserVariables(t *testing.T) {
},
{
Name: "_",
- ExpectedStatus: http.StatusNoContent,
+ ExpectedStatus: http.StatusCreated,
},
{
Name: "TEST_VAR",
- ExpectedStatus: http.StatusNoContent,
+ ExpectedStatus: http.StatusCreated,
},
{
Name: "test_var",
@@ -75,7 +75,7 @@ func TestAPIUserVariables(t *testing.T) {
req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{
Value: "initial_val",
}).AddTokenAuth(token)
- MakeRequest(t, req, http.StatusNoContent)
+ MakeRequest(t, req, http.StatusCreated)
cases := []struct {
Name string
@@ -132,7 +132,7 @@ func TestAPIUserVariables(t *testing.T) {
req := NewRequestWithJSON(t, "POST", url, api.CreateVariableOption{
Value: "initial_val",
}).AddTokenAuth(token)
- MakeRequest(t, req, http.StatusNoContent)
+ MakeRequest(t, req, http.StatusCreated)
req = NewRequest(t, "DELETE", url).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNoContent)
From 50d95650884a877936784fae01aaf3f7fea513c6 Mon Sep 17 00:00:00 2001
From: Markus Amshove
Date: Mon, 26 May 2025 18:37:38 +0200
Subject: [PATCH 12/18] Add sort option recentclose for issues and pulls
(#34525)
closes #34171
Adds a new sort option `recentclose` for issues and pull requests which
will return items in a descending order of when they were closed
---
models/issues/issue_search.go | 2 ++
models/issues/pull_list.go | 3 ++-
models/issues/pull_test.go | 42 ++++++++++++++++++++++++++++++++++
routers/api/v1/repo/pull.go | 2 +-
templates/swagger/v1_json.tmpl | 1 +
5 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go
index f9e1fbeb14..16f808acd1 100644
--- a/models/issues/issue_search.go
+++ b/models/issues/issue_search.go
@@ -88,6 +88,8 @@ func applySorts(sess *xorm.Session, sortType string, priorityRepoID int64) {
sess.Asc("issue.created_unix").Asc("issue.id")
case "recentupdate":
sess.Desc("issue.updated_unix").Desc("issue.created_unix").Desc("issue.id")
+ case "recentclose":
+ sess.Desc("issue.closed_unix").Desc("issue.created_unix").Desc("issue.id")
case "leastupdate":
sess.Asc("issue.updated_unix").Asc("issue.created_unix").Asc("issue.id")
case "mostcomment":
diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go
index b685175f8e..84f9f6166d 100644
--- a/models/issues/pull_list.go
+++ b/models/issues/pull_list.go
@@ -152,7 +152,8 @@ func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptio
applySorts(findSession, opts.SortType, 0)
findSession = db.SetSessionPagination(findSession, opts)
prs := make([]*PullRequest, 0, opts.PageSize)
- return prs, maxResults, findSession.Find(&prs)
+ found := findSession.Find(&prs)
+ return prs, maxResults, found
}
// PullRequestList defines a list of pull requests
diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go
index 8e09030215..53898cb42e 100644
--- a/models/issues/pull_test.go
+++ b/models/issues/pull_test.go
@@ -14,6 +14,7 @@ import (
"code.gitea.io/gitea/modules/setting"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestPullRequest_LoadAttributes(t *testing.T) {
@@ -76,6 +77,47 @@ func TestPullRequestsNewest(t *testing.T) {
}
}
+func TestPullRequests_Closed_RecentSortType(t *testing.T) {
+ // Issue ID | Closed At. | Updated At
+ // 2 | 1707270001 | 1707270001
+ // 3 | 1707271000 | 1707279999
+ // 11 | 1707279999 | 1707275555
+ tests := []struct {
+ sortType string
+ expectedIssueIDOrder []int64
+ }{
+ {"recentupdate", []int64{3, 11, 2}},
+ {"recentclose", []int64{11, 3, 2}},
+ }
+
+ assert.NoError(t, unittest.PrepareTestDatabase())
+ _, err := db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707270001, updated_unix = 1707270001, is_closed = true WHERE id = 2")
+ require.NoError(t, err)
+ _, err = db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707271000, updated_unix = 1707279999, is_closed = true WHERE id = 3")
+ require.NoError(t, err)
+ _, err = db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707279999, updated_unix = 1707275555, is_closed = true WHERE id = 11")
+ require.NoError(t, err)
+
+ for _, test := range tests {
+ t.Run(test.sortType, func(t *testing.T) {
+ prs, _, err := issues_model.PullRequests(db.DefaultContext, 1, &issues_model.PullRequestsOptions{
+ ListOptions: db.ListOptions{
+ Page: 1,
+ },
+ State: "closed",
+ SortType: test.sortType,
+ })
+ require.NoError(t, err)
+
+ if assert.Len(t, prs, len(test.expectedIssueIDOrder)) {
+ for i := range test.expectedIssueIDOrder {
+ assert.Equal(t, test.expectedIssueIDOrder[i], prs[i].IssueID)
+ }
+ }
+ })
+ }
+}
+
func TestLoadRequestedReviewers(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index f1ba06dd4a..d1bcaefb52 100644
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -73,7 +73,7 @@ func ListPullRequests(ctx *context.APIContext) {
// in: query
// description: Type of sort
// type: string
- // enum: [oldest, recentupdate, leastupdate, mostcomment, leastcomment, priority]
+ // enum: [oldest, recentupdate, recentclose, leastupdate, mostcomment, leastcomment, priority]
// - name: milestone
// in: query
// description: ID of the milestone
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index dd2048ddbb..71600e950c 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -12871,6 +12871,7 @@
"enum": [
"oldest",
"recentupdate",
+ "recentclose",
"leastupdate",
"mostcomment",
"leastcomment",
From ab9691291d313aedab7b459c42e1e1dd15f05461 Mon Sep 17 00:00:00 2001
From: Lunny Xiao
Date: Tue, 27 May 2025 01:09:14 +0800
Subject: [PATCH 13/18] Don't display error log when .git-blame-ignore-revs
doesn't exist (#34457)
Fix #34454
---
modules/git/blame.go | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/modules/git/blame.go b/modules/git/blame.go
index 6eb583a6b9..659dec34a1 100644
--- a/modules/git/blame.go
+++ b/modules/git/blame.go
@@ -132,18 +132,22 @@ func (r *BlameReader) Close() error {
}
// CreateBlameReader creates reader for given repository, commit and file
-func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath string, commit *Commit, file string, bypassBlameIgnore bool) (*BlameReader, error) {
- reader, stdout, err := os.Pipe()
- if err != nil {
- return nil, err
- }
+func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath string, commit *Commit, file string, bypassBlameIgnore bool) (rd *BlameReader, err error) {
+ var ignoreRevsFileName string
+ var ignoreRevsFileCleanup func()
+ defer func() {
+ if err != nil && ignoreRevsFileCleanup != nil {
+ ignoreRevsFileCleanup()
+ }
+ }()
cmd := NewCommandNoGlobals("blame", "--porcelain")
- var ignoreRevsFileName string
- var ignoreRevsFileCleanup func() // TODO: maybe it should check the returned err in a defer func to make sure the cleanup could always be executed correctly
if DefaultFeatures().CheckVersionAtLeast("2.23") && !bypassBlameIgnore {
- ignoreRevsFileName, ignoreRevsFileCleanup = tryCreateBlameIgnoreRevsFile(commit)
+ ignoreRevsFileName, ignoreRevsFileCleanup, err = tryCreateBlameIgnoreRevsFile(commit)
+ if err != nil && !IsErrNotExist(err) {
+ return nil, err
+ }
if ignoreRevsFileName != "" {
// Possible improvement: use --ignore-revs-file /dev/stdin on unix
// There is no equivalent on Windows. May be implemented if Gitea uses an external git backend.
@@ -154,6 +158,10 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath
cmd.AddDynamicArguments(commit.ID.String()).AddDashesAndList(file)
done := make(chan error, 1)
+ reader, stdout, err := os.Pipe()
+ if err != nil {
+ return nil, err
+ }
go func() {
stderr := bytes.Buffer{}
// TODO: it doesn't work for directories (the directories shouldn't be "blamed"), and the "err" should be returned by "Read" but not by "Close"
@@ -182,33 +190,29 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath
}, nil
}
-func tryCreateBlameIgnoreRevsFile(commit *Commit) (string, func()) {
+func tryCreateBlameIgnoreRevsFile(commit *Commit) (string, func(), error) {
entry, err := commit.GetTreeEntryByPath(".git-blame-ignore-revs")
if err != nil {
- log.Error("Unable to get .git-blame-ignore-revs file: GetTreeEntryByPath: %v", err)
- return "", nil
+ return "", nil, err
}
r, err := entry.Blob().DataAsync()
if err != nil {
- log.Error("Unable to get .git-blame-ignore-revs file data: DataAsync: %v", err)
- return "", nil
+ return "", nil, err
}
defer r.Close()
f, cleanup, err := setting.AppDataTempDir("git-repo-content").CreateTempFileRandom("git-blame-ignore-revs")
if err != nil {
- log.Error("Unable to get .git-blame-ignore-revs file data: CreateTempFileRandom: %v", err)
- return "", nil
+ return "", nil, err
}
filename := f.Name()
_, err = io.Copy(f, r)
_ = f.Close()
if err != nil {
cleanup()
- log.Error("Unable to get .git-blame-ignore-revs file data: Copy: %v", err)
- return "", nil
+ return "", nil, err
}
- return filename, cleanup
+ return filename, cleanup, nil
}
From 688da55f543f82265cc7df2bd1cf2bce53188b7a Mon Sep 17 00:00:00 2001
From: Lunny Xiao
Date: Tue, 27 May 2025 03:00:22 +0800
Subject: [PATCH 14/18] Split GetLatestCommitStatus as two functions (#34535)
Extract from #34531. This will reduce unnecessary count operation in
databases.
---------
Co-authored-by: wxiaoguang
---
models/fixtures/commit_status.yml | 5 +++
models/git/commit_status.go | 34 +++++++++++++-------
models/git/commit_status_summary.go | 2 +-
models/git/commit_status_test.go | 25 +++++++++++++-
routers/api/v1/repo/status.go | 11 +++++--
routers/web/repo/commit.go | 2 +-
routers/web/repo/pull.go | 6 ++--
routers/web/repo/release.go | 2 +-
routers/web/repo/view.go | 2 +-
services/actions/commit_status.go | 2 +-
services/git/commit.go | 2 +-
services/pull/commit_status.go | 2 +-
services/pull/pull.go | 2 +-
services/repository/gitgraph/graph_models.go | 2 +-
tests/integration/actions_trigger_test.go | 4 +--
15 files changed, 73 insertions(+), 30 deletions(-)
diff --git a/models/fixtures/commit_status.yml b/models/fixtures/commit_status.yml
index 20d57975ef..87c652e53a 100644
--- a/models/fixtures/commit_status.yml
+++ b/models/fixtures/commit_status.yml
@@ -7,6 +7,7 @@
target_url: https://example.com/builds/
description: My awesome CI-service
context: ci/awesomeness
+ context_hash: c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7
creator_id: 2
-
@@ -18,6 +19,7 @@
target_url: https://example.com/converage/
description: My awesome Coverage service
context: cov/awesomeness
+ context_hash: 3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe
creator_id: 2
-
@@ -29,6 +31,7 @@
target_url: https://example.com/converage/
description: My awesome Coverage service
context: cov/awesomeness
+ context_hash: 3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe
creator_id: 2
-
@@ -40,6 +43,7 @@
target_url: https://example.com/builds/
description: My awesome CI-service
context: ci/awesomeness
+ context_hash: c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7
creator_id: 2
-
@@ -51,4 +55,5 @@
target_url: https://example.com/builds/
description: My awesome deploy service
context: deploy/awesomeness
+ context_hash: ae9547713a6665fc4261d0756904932085a41cf2
creator_id: 2
diff --git a/models/git/commit_status.go b/models/git/commit_status.go
index b978476c4b..0e4e8215b0 100644
--- a/models/git/commit_status.go
+++ b/models/git/commit_status.go
@@ -298,27 +298,37 @@ type CommitStatusIndex struct {
MaxIndex int64 `xorm:"index"`
}
+func makeRepoCommitQuery(ctx context.Context, repoID int64, sha string) *xorm.Session {
+ return db.GetEngine(ctx).Table(&CommitStatus{}).
+ Where("repo_id = ?", repoID).And("sha = ?", sha)
+}
+
// GetLatestCommitStatus returns all statuses with a unique context for a given commit.
-func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
- getBase := func() *xorm.Session {
- return db.GetEngine(ctx).Table(&CommitStatus{}).
- Where("repo_id = ?", repoID).And("sha = ?", sha)
- }
+func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, error) {
indices := make([]int64, 0, 10)
- sess := getBase().Select("max( `index` ) as `index`").
- GroupBy("context_hash").OrderBy("max( `index` ) desc")
+ sess := makeRepoCommitQuery(ctx, repoID, sha).
+ Select("max( `index` ) as `index`").
+ GroupBy("context_hash").
+ OrderBy("max( `index` ) desc")
if !listOptions.IsListAll() {
sess = db.SetSessionPagination(sess, &listOptions)
}
- count, err := sess.FindAndCount(&indices)
- if err != nil {
- return nil, count, err
+ if err := sess.Find(&indices); err != nil {
+ return nil, err
}
statuses := make([]*CommitStatus, 0, len(indices))
if len(indices) == 0 {
- return statuses, count, nil
+ return statuses, nil
}
- return statuses, count, getBase().And(builder.In("`index`", indices)).Find(&statuses)
+ err := makeRepoCommitQuery(ctx, repoID, sha).And(builder.In("`index`", indices)).Find(&statuses)
+ return statuses, err
+}
+
+func CountLatestCommitStatus(ctx context.Context, repoID int64, sha string) (int64, error) {
+ return makeRepoCommitQuery(ctx, repoID, sha).
+ Select("count(context_hash)").
+ GroupBy("context_hash").
+ Count()
}
// GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs
diff --git a/models/git/commit_status_summary.go b/models/git/commit_status_summary.go
index 7603e7aa65..a3f440fd86 100644
--- a/models/git/commit_status_summary.go
+++ b/models/git/commit_status_summary.go
@@ -55,7 +55,7 @@ func GetLatestCommitStatusForRepoAndSHAs(ctx context.Context, repoSHAs []RepoSHA
}
func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) error {
- commitStatuses, _, err := GetLatestCommitStatus(ctx, repoID, sha, db.ListOptionsAll)
+ commitStatuses, err := GetLatestCommitStatus(ctx, repoID, sha, db.ListOptionsAll)
if err != nil {
return err
}
diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go
index 37d785e938..a01d030e71 100644
--- a/models/git/commit_status_test.go
+++ b/models/git/commit_status_test.go
@@ -26,7 +26,7 @@ func TestGetCommitStatuses(t *testing.T) {
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
- sha1 := "1234123412341234123412341234123412341234"
+ sha1 := "1234123412341234123412341234123412341234" // the mocked commit ID in test fixtures
statuses, maxResults, err := db.FindAndCount[git_model.CommitStatus](db.DefaultContext, &git_model.CommitStatusOptions{
ListOptions: db.ListOptions{Page: 1, PageSize: 50},
@@ -256,3 +256,26 @@ func TestCommitStatusesHideActionsURL(t *testing.T) {
assert.Empty(t, statuses[0].TargetURL)
assert.Equal(t, "https://mycicd.org/1", statuses[1].TargetURL)
}
+
+func TestGetCountLatestCommitStatus(t *testing.T) {
+ assert.NoError(t, unittest.PrepareTestDatabase())
+
+ repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+ sha1 := "1234123412341234123412341234123412341234" // the mocked commit ID in test fixtures
+
+ commitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo1.ID, sha1, db.ListOptions{
+ Page: 1,
+ PageSize: 2,
+ })
+ assert.NoError(t, err)
+ assert.Len(t, commitStatuses, 2)
+ assert.Equal(t, structs.CommitStatusFailure, commitStatuses[0].State)
+ assert.Equal(t, "ci/awesomeness", commitStatuses[0].Context)
+ assert.Equal(t, structs.CommitStatusError, commitStatuses[1].State)
+ assert.Equal(t, "deploy/awesomeness", commitStatuses[1].Context)
+
+ count, err := git_model.CountLatestCommitStatus(db.DefaultContext, repo1.ID, sha1)
+ assert.NoError(t, err)
+ assert.EqualValues(t, 3, count)
+}
diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go
index 756adcf3a3..40007ea1e5 100644
--- a/routers/api/v1/repo/status.go
+++ b/routers/api/v1/repo/status.go
@@ -258,19 +258,24 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) {
repo := ctx.Repo.Repository
- statuses, count, err := git_model.GetLatestCommitStatus(ctx, repo.ID, refCommit.Commit.ID.String(), utils.GetListOptions(ctx))
+ statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, refCommit.Commit.ID.String(), utils.GetListOptions(ctx))
if err != nil {
ctx.APIErrorInternal(fmt.Errorf("GetLatestCommitStatus[%s, %s]: %w", repo.FullName(), refCommit.CommitID, err))
return
}
+ count, err := git_model.CountLatestCommitStatus(ctx, repo.ID, refCommit.Commit.ID.String())
+ if err != nil {
+ ctx.APIErrorInternal(fmt.Errorf("CountLatestCommitStatus[%s, %s]: %w", repo.FullName(), refCommit.CommitID, err))
+ return
+ }
+ ctx.SetTotalCountHeader(count)
+
if len(statuses) == 0 {
ctx.JSON(http.StatusOK, &api.CombinedStatus{})
return
}
combiStatus := convert.ToCombinedStatus(ctx, statuses, convert.ToRepo(ctx, repo, ctx.Repo.Permission))
-
- ctx.SetTotalCountHeader(count)
ctx.JSON(http.StatusOK, combiStatus)
}
diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go
index 01a6cbc319..1ea881c2a7 100644
--- a/routers/web/repo/commit.go
+++ b/routers/web/repo/commit.go
@@ -377,7 +377,7 @@ func Diff(ctx *context.Context) {
ctx.Data["FileIconPoolHTML"] = renderedIconPool.RenderToHTML()
}
- statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll)
+ statuses, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll)
if err != nil {
log.Error("GetLatestCommitStatus: %v", err)
}
diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index 43ddc265cf..bbd2398bcb 100644
--- a/routers/web/repo/pull.go
+++ b/routers/web/repo/pull.go
@@ -291,7 +291,7 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue)
if len(compareInfo.Commits) != 0 {
sha := compareInfo.Commits[0].ID.String()
- commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, sha, db.ListOptionsAll)
+ commitStatuses, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, sha, db.ListOptionsAll)
if err != nil {
ctx.ServerError("GetLatestCommitStatus", err)
return nil
@@ -358,7 +358,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
return nil
}
- commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
+ commitStatuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
if err != nil {
ctx.ServerError("GetLatestCommitStatus", err)
return nil
@@ -454,7 +454,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
return nil
}
- commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
+ commitStatuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
if err != nil {
ctx.ServerError("GetLatestCommitStatus", err)
return nil
diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go
index 553bdbf6e5..2baf434e75 100644
--- a/routers/web/repo/release.go
+++ b/routers/web/repo/release.go
@@ -130,7 +130,7 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions)
}
if canReadActions {
- statuses, _, err := git_model.GetLatestCommitStatus(ctx, r.Repo.ID, r.Sha1, db.ListOptionsAll)
+ statuses, err := git_model.GetLatestCommitStatus(ctx, r.Repo.ID, r.Sha1, db.ListOptionsAll)
if err != nil {
return nil, err
}
diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go
index 2f01434684..68fa17bf07 100644
--- a/routers/web/repo/view.go
+++ b/routers/web/repo/view.go
@@ -131,7 +131,7 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool {
ctx.Data["LatestCommitVerification"] = verification
ctx.Data["LatestCommitUser"] = user_model.ValidateCommitWithEmail(ctx, latestCommit)
- statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, latestCommit.ID.String(), db.ListOptionsAll)
+ statuses, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, latestCommit.ID.String(), db.ListOptionsAll)
if err != nil {
log.Error("GetLatestCommitStatus: %v", err)
}
diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go
index eb15d16061..4541b3af4c 100644
--- a/services/actions/commit_status.go
+++ b/services/actions/commit_status.go
@@ -92,7 +92,7 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
}
ctxname := fmt.Sprintf("%s / %s (%s)", runName, job.Name, event)
state := toCommitStatus(job.Status)
- if statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll); err == nil {
+ if statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll); err == nil {
for _, v := range statuses {
if v.Context == ctxname {
if v.State == state {
diff --git a/services/git/commit.go b/services/git/commit.go
index 3faef76782..d3ebcab569 100644
--- a/services/git/commit.go
+++ b/services/git/commit.go
@@ -84,7 +84,7 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig
commit := &git_model.SignCommitWithStatuses{
SignCommit: c,
}
- statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptions{})
+ statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptionsAll)
if err != nil {
return nil, err
}
diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go
index 0bfff21746..553496713f 100644
--- a/services/pull/commit_status.go
+++ b/services/pull/commit_status.go
@@ -151,7 +151,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
return "", errors.Wrap(err, "LoadBaseRepo")
}
- commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
+ commitStatuses, err := git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
if err != nil {
return "", errors.Wrap(err, "GetLatestCommitStatus")
}
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 81be797832..f879f61136 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -1004,7 +1004,7 @@ func getAllCommitStatus(ctx context.Context, gitRepo *git.Repository, pr *issues
return nil, nil, shaErr
}
- statuses, _, err = git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
+ statuses, err = git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
lastStatus = git_model.CalcCommitStatus(statuses)
return statuses, lastStatus, err
}
diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go
index c45662836b..31379410b2 100644
--- a/services/repository/gitgraph/graph_models.go
+++ b/services/repository/gitgraph/graph_models.go
@@ -121,7 +121,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_
return repo_model.IsOwnerMemberCollaborator(ctx, repository, user.ID)
}, &keyMap)
- statuses, _, err := git_model.GetLatestCommitStatus(ctx, repository.ID, c.Commit.ID.String(), db.ListOptions{})
+ statuses, err := git_model.GetLatestCommitStatus(ctx, repository.ID, c.Commit.ID.String(), db.ListOptionsAll)
if err != nil {
log.Error("GetLatestCommitStatus: %v", err)
} else {
diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go
index e755481d09..a598cf64a5 100644
--- a/tests/integration/actions_trigger_test.go
+++ b/tests/integration/actions_trigger_test.go
@@ -633,7 +633,7 @@ jobs:
assert.NotEmpty(t, addFileResp)
sha = addFileResp.Commit.SHA
assert.Eventually(t, func() bool {
- latestCommitStatuses, _, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
+ latestCommitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
assert.NoError(t, err)
if len(latestCommitStatuses) == 0 {
return false
@@ -676,7 +676,7 @@ jobs:
}
func checkCommitStatusAndInsertFakeStatus(t *testing.T, repo *repo_model.Repository, sha string) {
- latestCommitStatuses, _, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
+ latestCommitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
assert.NoError(t, err)
assert.Len(t, latestCommitStatuses, 1)
assert.Equal(t, api.CommitStatusPending, latestCommitStatuses[0].State)
From 9f10885b2194b6ee19f9b165b06edda775b792b2 Mon Sep 17 00:00:00 2001
From: wxiaoguang
Date: Wed, 28 May 2025 00:49:05 +0800
Subject: [PATCH 15/18] Refactor commit reader (#34542)
---
modules/git/commit.go | 2 +-
modules/git/commit_reader.go | 138 ++++++++++++++----------------
modules/git/commit_sha256_test.go | 6 +-
modules/git/commit_test.go | 12 +--
4 files changed, 71 insertions(+), 87 deletions(-)
diff --git a/modules/git/commit.go b/modules/git/commit.go
index cd50c51151..44e8725bbe 100644
--- a/modules/git/commit.go
+++ b/modules/git/commit.go
@@ -34,7 +34,7 @@ type Commit struct {
// CommitSignature represents a git commit signature part.
type CommitSignature struct {
Signature string
- Payload string // TODO check if can be reconstruct from the rest of commit information to not have duplicate data
+ Payload string
}
// Message returns the commit message. Same as retrieving CommitMessage directly.
diff --git a/modules/git/commit_reader.go b/modules/git/commit_reader.go
index 228bbaf314..eb8f4c6322 100644
--- a/modules/git/commit_reader.go
+++ b/modules/git/commit_reader.go
@@ -6,10 +6,44 @@ package git
import (
"bufio"
"bytes"
+ "fmt"
"io"
- "strings"
)
+const (
+ commitHeaderGpgsig = "gpgsig"
+ commitHeaderGpgsigSha256 = "gpgsig-sha256"
+)
+
+func assignCommitFields(gitRepo *Repository, commit *Commit, headerKey string, headerValue []byte) error {
+ if len(headerValue) > 0 && headerValue[len(headerValue)-1] == '\n' {
+ headerValue = headerValue[:len(headerValue)-1] // remove trailing newline
+ }
+ switch headerKey {
+ case "tree":
+ objID, err := NewIDFromString(string(headerValue))
+ if err != nil {
+ return fmt.Errorf("invalid tree ID %q: %w", string(headerValue), err)
+ }
+ commit.Tree = *NewTree(gitRepo, objID)
+ case "parent":
+ objID, err := NewIDFromString(string(headerValue))
+ if err != nil {
+ return fmt.Errorf("invalid parent ID %q: %w", string(headerValue), err)
+ }
+ commit.Parents = append(commit.Parents, objID)
+ case "author":
+ commit.Author.Decode(headerValue)
+ case "committer":
+ commit.Committer.Decode(headerValue)
+ case commitHeaderGpgsig, commitHeaderGpgsigSha256:
+ // if there are duplicate "gpgsig" and "gpgsig-sha256" headers, then the signature must have already been invalid
+ // so we don't need to handle duplicate headers here
+ commit.Signature = &CommitSignature{Signature: string(headerValue)}
+ }
+ return nil
+}
+
// CommitFromReader will generate a Commit from a provided reader
// We need this to interpret commits from cat-file or cat-file --batch
//
@@ -21,90 +55,46 @@ func CommitFromReader(gitRepo *Repository, objectID ObjectID, reader io.Reader)
Committer: &Signature{},
}
- payloadSB := new(strings.Builder)
- signatureSB := new(strings.Builder)
- messageSB := new(strings.Builder)
- message := false
- pgpsig := false
-
- bufReader, ok := reader.(*bufio.Reader)
- if !ok {
- bufReader = bufio.NewReader(reader)
- }
-
-readLoop:
+ bufReader := bufio.NewReader(reader)
+ inHeader := true
+ var payloadSB, messageSB bytes.Buffer
+ var headerKey string
+ var headerValue []byte
for {
line, err := bufReader.ReadBytes('\n')
- if err != nil {
- if err == io.EOF {
- if message {
- _, _ = messageSB.Write(line)
+ if err != nil && err != io.EOF {
+ return nil, fmt.Errorf("unable to read commit %q: %w", objectID.String(), err)
+ }
+ if len(line) == 0 {
+ break
+ }
+
+ if inHeader {
+ inHeader = !(len(line) == 1 && line[0] == '\n') // still in header if line is not just a newline
+ k, v, _ := bytes.Cut(line, []byte{' '})
+ if len(k) != 0 || !inHeader {
+ if headerKey != "" {
+ if err = assignCommitFields(gitRepo, commit, headerKey, headerValue); err != nil {
+ return nil, fmt.Errorf("unable to parse commit %q: %w", objectID.String(), err)
+ }
}
- _, _ = payloadSB.Write(line)
- break readLoop
+ headerKey = string(k) // it also resets the headerValue to empty string if not inHeader
+ headerValue = v
+ } else {
+ headerValue = append(headerValue, v...)
}
- return nil, err
- }
- if pgpsig {
- if len(line) > 0 && line[0] == ' ' {
- _, _ = signatureSB.Write(line[1:])
- continue
- }
- pgpsig = false
- }
-
- if !message {
- // This is probably not correct but is copied from go-gits interpretation...
- trimmed := bytes.TrimSpace(line)
- if len(trimmed) == 0 {
- message = true
+ if headerKey != commitHeaderGpgsig && headerKey != commitHeaderGpgsigSha256 {
_, _ = payloadSB.Write(line)
- continue
- }
-
- split := bytes.SplitN(trimmed, []byte{' '}, 2)
- var data []byte
- if len(split) > 1 {
- data = split[1]
- }
-
- switch string(split[0]) {
- case "tree":
- commit.Tree = *NewTree(gitRepo, MustIDFromString(string(data)))
- _, _ = payloadSB.Write(line)
- case "parent":
- commit.Parents = append(commit.Parents, MustIDFromString(string(data)))
- _, _ = payloadSB.Write(line)
- case "author":
- commit.Author = &Signature{}
- commit.Author.Decode(data)
- _, _ = payloadSB.Write(line)
- case "committer":
- commit.Committer = &Signature{}
- commit.Committer.Decode(data)
- _, _ = payloadSB.Write(line)
- case "encoding":
- _, _ = payloadSB.Write(line)
- case "gpgsig":
- fallthrough
- case "gpgsig-sha256": // FIXME: no intertop, so only 1 exists at present.
- _, _ = signatureSB.Write(data)
- _ = signatureSB.WriteByte('\n')
- pgpsig = true
}
} else {
_, _ = messageSB.Write(line)
_, _ = payloadSB.Write(line)
}
}
- commit.CommitMessage = messageSB.String()
- commit.Signature = &CommitSignature{
- Signature: signatureSB.String(),
- Payload: payloadSB.String(),
- }
- if len(commit.Signature.Signature) == 0 {
- commit.Signature = nil
- }
+ commit.CommitMessage = messageSB.String()
+ if commit.Signature != nil {
+ commit.Signature.Payload = payloadSB.String()
+ }
return commit, nil
}
diff --git a/modules/git/commit_sha256_test.go b/modules/git/commit_sha256_test.go
index 64a0f53908..97ccecdacc 100644
--- a/modules/git/commit_sha256_test.go
+++ b/modules/git/commit_sha256_test.go
@@ -60,8 +60,7 @@ func TestGetFullCommitIDErrorSha256(t *testing.T) {
}
func TestCommitFromReaderSha256(t *testing.T) {
- commitString := `9433b2a62b964c17a4485ae180f45f595d3e69d31b786087775e28c6b6399df0 commit 1114
-tree e7f9e96dd79c09b078cac8b303a7d3b9d65ff9b734e86060a4d20409fd379f9e
+ commitString := `tree e7f9e96dd79c09b078cac8b303a7d3b9d65ff9b734e86060a4d20409fd379f9e
parent 26e9ccc29fad747e9c5d9f4c9ddeb7eff61cc45ef6a8dc258cbeb181afc055e8
author Adam Majer 1698676906 +0100
committer Adam Majer 1698676906 +0100
@@ -112,8 +111,7 @@ VAEUo6ecdDxSpyt2naeg9pKus/BRi7P6g4B1hkk/zZstUX/QP4IQuAJbXjkvsC+X
HKRr3NlRM/DygzTyj0gN74uoa0goCIbyAQhiT42nm0cuhM7uN/W0ayrlZjGF1cbR
8NCJUL2Nwj0ywKIavC99Ipkb8AsFwpVT6U6effs6
=xybZ
------END PGP SIGNATURE-----
-`, commitFromReader.Signature.Signature)
+-----END PGP SIGNATURE-----`, commitFromReader.Signature.Signature)
assert.Equal(t, `tree e7f9e96dd79c09b078cac8b303a7d3b9d65ff9b734e86060a4d20409fd379f9e
parent 26e9ccc29fad747e9c5d9f4c9ddeb7eff61cc45ef6a8dc258cbeb181afc055e8
author Adam Majer 1698676906 +0100
diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go
index f43e0081fd..81fb91dfc6 100644
--- a/modules/git/commit_test.go
+++ b/modules/git/commit_test.go
@@ -59,8 +59,7 @@ func TestGetFullCommitIDError(t *testing.T) {
}
func TestCommitFromReader(t *testing.T) {
- commitString := `feaf4ba6bc635fec442f46ddd4512416ec43c2c2 commit 1074
-tree f1a6cb52b2d16773290cefe49ad0684b50a4f930
+ commitString := `tree f1a6cb52b2d16773290cefe49ad0684b50a4f930
parent 37991dec2c8e592043f47155ce4808d4580f9123
author silverwind 1563741793 +0200
committer silverwind 1563741793 +0200
@@ -108,8 +107,7 @@ sD53z/f0J+We4VZjY+pidvA9BGZPFVdR3wd3xGs8/oH6UWaLJAMGkLG6dDb3qDLm
mfeFhT57UbE4qukTDIQ0Y0WM40UYRTakRaDY7ubhXgLgx09Cnp9XTVMsHgT6j9/i
1pxsB104XLWjQHTjr1JtiaBQEwFh9r2OKTcpvaLcbNtYpo7CzOs=
=FRsO
------END PGP SIGNATURE-----
-`, commitFromReader.Signature.Signature)
+-----END PGP SIGNATURE-----`, commitFromReader.Signature.Signature)
assert.Equal(t, `tree f1a6cb52b2d16773290cefe49ad0684b50a4f930
parent 37991dec2c8e592043f47155ce4808d4580f9123
author silverwind 1563741793 +0200
@@ -126,8 +124,7 @@ empty commit`, commitFromReader.Signature.Payload)
}
func TestCommitWithEncodingFromReader(t *testing.T) {
- commitString := `feaf4ba6bc635fec442f46ddd4512416ec43c2c2 commit 1074
-tree ca3fad42080dd1a6d291b75acdfc46e5b9b307e5
+ commitString := `tree ca3fad42080dd1a6d291b75acdfc46e5b9b307e5
parent 47b24e7ab977ed31c5a39989d570847d6d0052af
author KN4CK3R 1711702962 +0100
committer KN4CK3R 1711702962 +0100
@@ -172,8 +169,7 @@ SONRzusmu5n3DgV956REL7x62h7JuqmBz/12HZkr0z0zgXkcZ04q08pSJATX5N1F
yN+tWxTsWg+zhDk96d5Esdo9JMjcFvPv0eioo30GAERaz1hoD7zCMT4jgUFTQwgz
jw4YcO5u
=r3UU
------END PGP SIGNATURE-----
-`, commitFromReader.Signature.Signature)
+-----END PGP SIGNATURE-----`, commitFromReader.Signature.Signature)
assert.Equal(t, `tree ca3fad42080dd1a6d291b75acdfc46e5b9b307e5
parent 47b24e7ab977ed31c5a39989d570847d6d0052af
author KN4CK3R 1711702962 +0100
From 24a51059d7fffe75564f7dc2fffeba1f3789f55e Mon Sep 17 00:00:00 2001
From: Lunny Xiao
Date: Wed, 28 May 2025 02:25:34 +0800
Subject: [PATCH 16/18] Fix possible nil description of pull request when
migrating from CodeCommit (#34541)
Fix #34320
---
services/migrations/codecommit.go | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/services/migrations/codecommit.go b/services/migrations/codecommit.go
index 4b2634ef8a..8b79edc4e5 100644
--- a/services/migrations/codecommit.go
+++ b/services/migrations/codecommit.go
@@ -180,11 +180,15 @@ func (c *CodeCommitDownloader) GetPullRequests(ctx context.Context, page, perPag
continue
}
target := orig.PullRequestTargets[0]
+ description := ""
+ if orig.Description != nil {
+ description = *orig.Description
+ }
pr := &base.PullRequest{
Number: number,
Title: *orig.Title,
PosterName: c.getUsernameFromARN(*orig.AuthorArn),
- Content: *orig.Description,
+ Content: description,
State: "open",
Created: *orig.CreationDate,
Updated: *orig.LastActivityDate,
From 498088c0536f228eceb7c04ab53742f69a0024b4 Mon Sep 17 00:00:00 2001
From: Lunny Xiao
Date: Wed, 28 May 2025 02:53:27 +0800
Subject: [PATCH 17/18] Add webhook assigning test and fix possible bug
(#34420)
Co-authored-by: wxiaoguang
---
services/issue/assignee.go | 2 +
tests/integration/issue_test.go | 9 +
tests/integration/repo_webhook_test.go | 409 ++++++++++++++-----------
3 files changed, 235 insertions(+), 185 deletions(-)
diff --git a/services/issue/assignee.go b/services/issue/assignee.go
index c7e2495568..a9494b7ab4 100644
--- a/services/issue/assignee.go
+++ b/services/issue/assignee.go
@@ -54,6 +54,8 @@ func ToggleAssigneeWithNotify(ctx context.Context, issue *issues_model.Issue, do
if err != nil {
return false, nil, err
}
+ issue.AssigneeID = assigneeID
+ issue.Assignee = assignee
notify_service.IssueChangeAssignee(ctx, doer, issue, assignee, removed, comment)
diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go
index b403b3cf17..2e6a12df2c 100644
--- a/tests/integration/issue_test.go
+++ b/tests/integration/issue_test.go
@@ -151,6 +151,15 @@ func testNewIssue(t *testing.T, session *TestSession, user, repo, title, content
return issueURL
}
+func testIssueAssign(t *testing.T, session *TestSession, repoLink string, issueID, assigneeID int64) {
+ req := NewRequestWithValues(t, "POST", fmt.Sprintf(repoLink+"/issues/assignee?issue_ids=%d", issueID), map[string]string{
+ "_csrf": GetUserCSRFToken(t, session),
+ "id": strconv.FormatInt(assigneeID, 10),
+ "action": "", // empty action means assign
+ })
+ session.MakeRequest(t, req, http.StatusOK)
+}
+
func testIssueAddComment(t *testing.T, session *TestSession, issueURL, content, status string) int64 {
req := NewRequest(t, "GET", issueURL)
resp := session.MakeRequest(t, req, http.StatusOK)
diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go
index 13e3d198ea..438dd3211d 100644
--- a/tests/integration/repo_webhook_test.go
+++ b/tests/integration/repo_webhook_test.go
@@ -131,19 +131,19 @@ func (m *mockWebhookProvider) Close() {
}
func Test_WebhookCreate(t *testing.T) {
- var payloads []api.CreatePayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.CreatePayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = string(webhook_module.HookEventCreate)
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.CreatePayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.CreatePayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = string(webhook_module.HookEventCreate)
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -163,19 +163,19 @@ func Test_WebhookCreate(t *testing.T) {
}
func Test_WebhookDelete(t *testing.T) {
- var payloads []api.DeletePayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.DeletePayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "delete"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.DeletePayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.DeletePayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "delete"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -196,19 +196,19 @@ func Test_WebhookDelete(t *testing.T) {
}
func Test_WebhookFork(t *testing.T) {
- var payloads []api.ForkPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.ForkPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "fork"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.ForkPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.ForkPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "fork"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user1")
@@ -228,19 +228,19 @@ func Test_WebhookFork(t *testing.T) {
}
func Test_WebhookIssueComment(t *testing.T) {
- var payloads []api.IssueCommentPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.IssueCommentPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "issue_comment"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.IssueCommentPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.IssueCommentPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "issue_comment"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -312,19 +312,19 @@ func Test_WebhookIssueComment(t *testing.T) {
}
func Test_WebhookRelease(t *testing.T) {
- var payloads []api.ReleasePayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.ReleasePayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "release"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.ReleasePayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.ReleasePayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "release"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -345,19 +345,19 @@ func Test_WebhookRelease(t *testing.T) {
}
func Test_WebhookPush(t *testing.T) {
- var payloads []api.PushPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.PushPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "push"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.PushPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.PushPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "push"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -416,19 +416,19 @@ func Test_WebhookPushDevBranch(t *testing.T) {
}
func Test_WebhookIssue(t *testing.T) {
- var payloads []api.IssuePayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.IssuePayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "issues"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.IssuePayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.IssuePayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "issues"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -445,6 +445,45 @@ func Test_WebhookIssue(t *testing.T) {
assert.Equal(t, "user2/repo1", payloads[0].Issue.Repo.FullName)
assert.Equal(t, "Title1", payloads[0].Issue.Title)
assert.Equal(t, "Description1", payloads[0].Issue.Body)
+ assert.Positive(t, payloads[0].Issue.Created.Unix())
+ assert.Positive(t, payloads[0].Issue.Updated.Unix())
+ })
+}
+
+func Test_WebhookIssueAssign(t *testing.T) {
+ onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.PullRequestPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.PullRequestPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "pull_request_assign"
+ }, http.StatusOK)
+ defer provider.Close()
+
+ user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+ repo1 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 1})
+
+ // 1. create a new webhook with special webhook for repo1
+ session := loginUser(t, "user2")
+
+ testAPICreateWebhookForRepo(t, session, "user2", "repo1", provider.URL(), "pull_request_assign")
+
+ // 2. trigger the webhook, issue 2 is a pull request
+ testIssueAssign(t, session, repo1.Link(), 2, user2.ID)
+
+ // 3. validate the webhook is triggered
+ assert.Equal(t, "pull_request_assign", triggeredEvent)
+ assert.Len(t, payloads, 1)
+ assert.EqualValues(t, "assigned", payloads[0].Action)
+ assert.Equal(t, "repo1", payloads[0].PullRequest.Base.Repository.Name)
+ assert.Equal(t, "user2/repo1", payloads[0].PullRequest.Base.Repository.FullName)
+ assert.Equal(t, "issue2", payloads[0].PullRequest.Title)
+ assert.Equal(t, "content for the second issue", payloads[0].PullRequest.Body)
+ assert.Equal(t, user2.ID, payloads[0].PullRequest.Assignee.ID)
})
}
@@ -521,19 +560,19 @@ func Test_WebhookIssueMilestone(t *testing.T) {
}
func Test_WebhookPullRequest(t *testing.T) {
- var payloads []api.PullRequestPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.PullRequestPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "pull_request"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.PullRequestPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.PullRequestPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "pull_request"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -558,19 +597,19 @@ func Test_WebhookPullRequest(t *testing.T) {
}
func Test_WebhookPullRequestComment(t *testing.T) {
- var payloads []api.IssueCommentPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.IssueCommentPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "pull_request_comment"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.IssueCommentPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.IssueCommentPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "pull_request_comment"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -596,19 +635,19 @@ func Test_WebhookPullRequestComment(t *testing.T) {
}
func Test_WebhookWiki(t *testing.T) {
- var payloads []api.WikiPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.WikiPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "wiki"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.WikiPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.WikiPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "wiki"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -628,19 +667,19 @@ func Test_WebhookWiki(t *testing.T) {
}
func Test_WebhookRepository(t *testing.T) {
- var payloads []api.RepositoryPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.RepositoryPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "repository"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.RepositoryPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.RepositoryPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "repository"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user1")
@@ -660,19 +699,19 @@ func Test_WebhookRepository(t *testing.T) {
}
func Test_WebhookPackage(t *testing.T) {
- var payloads []api.PackagePayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- content, _ := io.ReadAll(r.Body)
- var payload api.PackagePayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "package"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.PackagePayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ content, _ := io.ReadAll(r.Body)
+ var payload api.PackagePayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "package"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user1")
@@ -697,24 +736,24 @@ func Test_WebhookPackage(t *testing.T) {
}
func Test_WebhookStatus(t *testing.T) {
- var payloads []api.CommitStatusPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- assert.Contains(t, r.Header["X-Github-Event-Type"], "status", "X-GitHub-Event-Type should contain status")
- assert.Contains(t, r.Header["X-Github-Hook-Installation-Target-Type"], "repository", "X-GitHub-Hook-Installation-Target-Type should contain repository")
- assert.Contains(t, r.Header["X-Gitea-Event-Type"], "status", "X-Gitea-Event-Type should contain status")
- assert.Contains(t, r.Header["X-Gitea-Hook-Installation-Target-Type"], "repository", "X-Gitea-Hook-Installation-Target-Type should contain repository")
- assert.Contains(t, r.Header["X-Gogs-Event-Type"], "status", "X-Gogs-Event-Type should contain status")
- content, _ := io.ReadAll(r.Body)
- var payload api.CommitStatusPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "status"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.CommitStatusPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ assert.Contains(t, r.Header["X-Github-Event-Type"], "status", "X-GitHub-Event-Type should contain status")
+ assert.Contains(t, r.Header["X-Github-Hook-Installation-Target-Type"], "repository", "X-GitHub-Hook-Installation-Target-Type should contain repository")
+ assert.Contains(t, r.Header["X-Gitea-Event-Type"], "status", "X-Gitea-Event-Type should contain status")
+ assert.Contains(t, r.Header["X-Gitea-Hook-Installation-Target-Type"], "repository", "X-Gitea-Hook-Installation-Target-Type should contain repository")
+ assert.Contains(t, r.Header["X-Gogs-Event-Type"], "status", "X-Gogs-Event-Type should contain status")
+ content, _ := io.ReadAll(r.Body)
+ var payload api.CommitStatusPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "status"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -750,16 +789,16 @@ func Test_WebhookStatus(t *testing.T) {
}
func Test_WebhookStatus_NoWrongTrigger(t *testing.T) {
- var trigger string
- provider := newMockWebhookProvider(func(r *http.Request) {
- assert.NotContains(t, r.Header["X-Github-Event-Type"], "status", "X-GitHub-Event-Type should not contain status")
- assert.NotContains(t, r.Header["X-Gitea-Event-Type"], "status", "X-Gitea-Event-Type should not contain status")
- assert.NotContains(t, r.Header["X-Gogs-Event-Type"], "status", "X-Gogs-Event-Type should not contain status")
- trigger = "push"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var trigger string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ assert.NotContains(t, r.Header["X-Github-Event-Type"], "status", "X-GitHub-Event-Type should not contain status")
+ assert.NotContains(t, r.Header["X-Gitea-Event-Type"], "status", "X-Gitea-Event-Type should not contain status")
+ assert.NotContains(t, r.Header["X-Gogs-Event-Type"], "status", "X-Gogs-Event-Type should not contain status")
+ trigger = "push"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
session := loginUser(t, "user2")
@@ -775,22 +814,22 @@ func Test_WebhookStatus_NoWrongTrigger(t *testing.T) {
}
func Test_WebhookWorkflowJob(t *testing.T) {
- var payloads []api.WorkflowJobPayload
- var triggeredEvent string
- provider := newMockWebhookProvider(func(r *http.Request) {
- assert.Contains(t, r.Header["X-Github-Event-Type"], "workflow_job", "X-GitHub-Event-Type should contain workflow_job")
- assert.Contains(t, r.Header["X-Gitea-Event-Type"], "workflow_job", "X-Gitea-Event-Type should contain workflow_job")
- assert.Contains(t, r.Header["X-Gogs-Event-Type"], "workflow_job", "X-Gogs-Event-Type should contain workflow_job")
- content, _ := io.ReadAll(r.Body)
- var payload api.WorkflowJobPayload
- err := json.Unmarshal(content, &payload)
- assert.NoError(t, err)
- payloads = append(payloads, payload)
- triggeredEvent = "workflow_job"
- }, http.StatusOK)
- defer provider.Close()
-
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+ var payloads []api.WorkflowJobPayload
+ var triggeredEvent string
+ provider := newMockWebhookProvider(func(r *http.Request) {
+ assert.Contains(t, r.Header["X-Github-Event-Type"], "workflow_job", "X-GitHub-Event-Type should contain workflow_job")
+ assert.Contains(t, r.Header["X-Gitea-Event-Type"], "workflow_job", "X-Gitea-Event-Type should contain workflow_job")
+ assert.Contains(t, r.Header["X-Gogs-Event-Type"], "workflow_job", "X-Gogs-Event-Type should contain workflow_job")
+ content, _ := io.ReadAll(r.Body)
+ var payload api.WorkflowJobPayload
+ err := json.Unmarshal(content, &payload)
+ assert.NoError(t, err)
+ payloads = append(payloads, payload)
+ triggeredEvent = "workflow_job"
+ }, http.StatusOK)
+ defer provider.Close()
+
// 1. create a new webhook with special webhook for repo1
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
session := loginUser(t, "user2")
From b0936f4f4120169fab31f21610e9fcd915e36fb4 Mon Sep 17 00:00:00 2001
From: Philip Peterson <1326208+philip-peterson@users.noreply.github.com>
Date: Tue, 27 May 2025 12:36:02 -0700
Subject: [PATCH 18/18] Do not mutate incoming options to RenderUserSearch and
SearchUsers (#34544)
This PR changes the `opts` argument in `SearchUsers()` to be passed by
value instead of by pointer, as its mutations do not escape the function
scope and are not used elsewhere. This simplifies reasoning about the
function and avoids unnecessary pointer usage.
This insight emerged during an initial attempt to refactor
`RenderUserSearch()`, which currently intermixes multiple concerns.
Co-authored-by: Philip Peterson
---
models/user/search.go | 4 ++--
models/user/user_test.go | 36 ++++++++++++++++++------------------
routers/api/v1/admin/org.go | 2 +-
routers/api/v1/admin/user.go | 2 +-
routers/api/v1/org/org.go | 2 +-
routers/api/v1/user/user.go | 2 +-
routers/web/admin/orgs.go | 2 +-
routers/web/admin/users.go | 2 +-
routers/web/explore/org.go | 2 +-
routers/web/explore/user.go | 4 ++--
routers/web/home.go | 2 +-
routers/web/user/search.go | 2 +-
12 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/models/user/search.go b/models/user/search.go
index f4436be09a..cfd0d011bc 100644
--- a/models/user/search.go
+++ b/models/user/search.go
@@ -137,7 +137,7 @@ func (opts *SearchUserOptions) toSearchQueryBase(ctx context.Context) *xorm.Sess
// SearchUsers takes options i.e. keyword and part of user name to search,
// it returns results in given range and number of total results.
-func SearchUsers(ctx context.Context, opts *SearchUserOptions) (users []*User, _ int64, _ error) {
+func SearchUsers(ctx context.Context, opts SearchUserOptions) (users []*User, _ int64, _ error) {
sessCount := opts.toSearchQueryBase(ctx)
defer sessCount.Close()
count, err := sessCount.Count(new(User))
@@ -152,7 +152,7 @@ func SearchUsers(ctx context.Context, opts *SearchUserOptions) (users []*User, _
sessQuery := opts.toSearchQueryBase(ctx).OrderBy(opts.OrderBy.String())
defer sessQuery.Close()
if opts.Page > 0 {
- sessQuery = db.SetSessionPagination(sessQuery, opts)
+ sessQuery = db.SetSessionPagination(sessQuery, &opts)
}
// the sql may contain JOIN, so we must only select User related columns
diff --git a/models/user/user_test.go b/models/user/user_test.go
index dd232abe2e..26192f8d55 100644
--- a/models/user/user_test.go
+++ b/models/user/user_test.go
@@ -88,7 +88,7 @@ func TestCanCreateOrganization(t *testing.T) {
func TestSearchUsers(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
- testSuccess := func(opts *user_model.SearchUserOptions, expectedUserOrOrgIDs []int64) {
+ testSuccess := func(opts user_model.SearchUserOptions, expectedUserOrOrgIDs []int64) {
users, _, err := user_model.SearchUsers(db.DefaultContext, opts)
assert.NoError(t, err)
cassText := fmt.Sprintf("ids: %v, opts: %v", expectedUserOrOrgIDs, opts)
@@ -100,61 +100,61 @@ func TestSearchUsers(t *testing.T) {
}
// test orgs
- testOrgSuccess := func(opts *user_model.SearchUserOptions, expectedOrgIDs []int64) {
+ testOrgSuccess := func(opts user_model.SearchUserOptions, expectedOrgIDs []int64) {
opts.Type = user_model.UserTypeOrganization
testSuccess(opts, expectedOrgIDs)
}
- testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1, PageSize: 2}},
+ testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1, PageSize: 2}},
[]int64{3, 6})
- testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 2, PageSize: 2}},
+ testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 2, PageSize: 2}},
[]int64{7, 17})
- testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 3, PageSize: 2}},
+ testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 3, PageSize: 2}},
[]int64{19, 25})
- testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 4, PageSize: 2}},
+ testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 4, PageSize: 2}},
[]int64{26, 41})
- testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 5, PageSize: 2}},
+ testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 5, PageSize: 2}},
[]int64{42})
- testOrgSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 6, PageSize: 2}},
+ testOrgSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 6, PageSize: 2}},
[]int64{})
// test users
- testUserSuccess := func(opts *user_model.SearchUserOptions, expectedUserIDs []int64) {
+ testUserSuccess := func(opts user_model.SearchUserOptions, expectedUserIDs []int64) {
opts.Type = user_model.UserTypeIndividual
testSuccess(opts, expectedUserIDs)
}
- testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
+ testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
- testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
+ testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
[]int64{9})
- testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
+ testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
- testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
+ testUserSuccess(user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
// order by name asc default
- testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
+ testUserSuccess(user_model.SearchUserOptions{Keyword: "user1", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
- testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: optional.Some(true)},
+ testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: optional.Some(true)},
[]int64{1})
- testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: optional.Some(true)},
+ testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: optional.Some(true)},
[]int64{29})
- testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: optional.Some(true)},
+ testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: optional.Some(true)},
[]int64{37})
- testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: optional.Some(true)},
+ testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: optional.Some(true)},
[]int64{24})
}
diff --git a/routers/api/v1/admin/org.go b/routers/api/v1/admin/org.go
index 8808a1587d..c7a4ae8419 100644
--- a/routers/api/v1/admin/org.go
+++ b/routers/api/v1/admin/org.go
@@ -101,7 +101,7 @@ func GetAllOrgs(ctx *context.APIContext) {
listOptions := utils.GetListOptions(ctx)
- users, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
+ users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Type: user_model.UserTypeOrganization,
OrderBy: db.SearchOrderByAlphabetically,
diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
index 3ba77604ec..85dd547ac1 100644
--- a/routers/api/v1/admin/user.go
+++ b/routers/api/v1/admin/user.go
@@ -423,7 +423,7 @@ func SearchUsers(ctx *context.APIContext) {
listOptions := utils.GetListOptions(ctx)
- users, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
+ users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Type: user_model.UserTypeIndividual,
LoginName: ctx.FormTrim("login_name"),
diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go
index c9208f4757..27c646896a 100644
--- a/routers/api/v1/org/org.go
+++ b/routers/api/v1/org/org.go
@@ -201,7 +201,7 @@ func GetAll(ctx *context.APIContext) {
listOptions := utils.GetListOptions(ctx)
- publicOrgs, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
+ publicOrgs, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
ListOptions: listOptions,
Type: user_model.UserTypeOrganization,
diff --git a/routers/api/v1/user/user.go b/routers/api/v1/user/user.go
index 757a548518..2b98fb5ac7 100644
--- a/routers/api/v1/user/user.go
+++ b/routers/api/v1/user/user.go
@@ -73,7 +73,7 @@ func Search(ctx *context.APIContext) {
if ctx.PublicOnly {
visible = []structs.VisibleType{structs.VisibleTypePublic}
}
- users, maxResults, err = user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
+ users, maxResults, err = user_model.SearchUsers(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Keyword: ctx.FormTrim("q"),
UID: uid,
diff --git a/routers/web/admin/orgs.go b/routers/web/admin/orgs.go
index 35e61efa17..e34f203aaf 100644
--- a/routers/web/admin/orgs.go
+++ b/routers/web/admin/orgs.go
@@ -27,7 +27,7 @@ func Organizations(ctx *context.Context) {
ctx.SetFormString("sort", UserSearchDefaultAdminSort)
}
- explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{
+ explore.RenderUserSearch(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Type: user_model.UserTypeOrganization,
IncludeReserved: true, // administrator needs to list all accounts include reserved
diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go
index e42cbb316c..c6e8f4eab2 100644
--- a/routers/web/admin/users.go
+++ b/routers/web/admin/users.go
@@ -64,7 +64,7 @@ func Users(ctx *context.Context) {
"SortType": sortType,
}
- explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{
+ explore.RenderUserSearch(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Type: user_model.UserTypeIndividual,
ListOptions: db.ListOptions{
diff --git a/routers/web/explore/org.go b/routers/web/explore/org.go
index 7bb71acfd7..f8f7f5c18c 100644
--- a/routers/web/explore/org.go
+++ b/routers/web/explore/org.go
@@ -44,7 +44,7 @@ func Organizations(ctx *context.Context) {
ctx.SetFormString("sort", sortOrder)
}
- RenderUserSearch(ctx, &user_model.SearchUserOptions{
+ RenderUserSearch(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Type: user_model.UserTypeOrganization,
ListOptions: db.ListOptions{PageSize: setting.UI.ExplorePagingNum},
diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go
index e1e1ec1cfd..af48e6fb79 100644
--- a/routers/web/explore/user.go
+++ b/routers/web/explore/user.go
@@ -32,7 +32,7 @@ func isKeywordValid(keyword string) bool {
}
// RenderUserSearch render user search page
-func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, tplName templates.TplName) {
+func RenderUserSearch(ctx *context.Context, opts user_model.SearchUserOptions, tplName templates.TplName) {
// Sitemap index for sitemap paths
opts.Page = int(ctx.PathParamInt64("idx"))
isSitemap := ctx.PathParam("idx") != ""
@@ -151,7 +151,7 @@ func Users(ctx *context.Context) {
ctx.SetFormString("sort", sortOrder)
}
- RenderUserSearch(ctx, &user_model.SearchUserOptions{
+ RenderUserSearch(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Type: user_model.UserTypeIndividual,
ListOptions: db.ListOptions{PageSize: setting.UI.ExplorePagingNum},
diff --git a/routers/web/home.go b/routers/web/home.go
index 208cc36dfb..c2d3184d39 100644
--- a/routers/web/home.go
+++ b/routers/web/home.go
@@ -68,7 +68,7 @@ func Home(ctx *context.Context) {
func HomeSitemap(ctx *context.Context) {
m := sitemap.NewSitemapIndex()
if !setting.Service.Explore.DisableUsersPage {
- _, cnt, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
+ _, cnt, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
Type: user_model.UserTypeIndividual,
ListOptions: db.ListOptions{PageSize: 1},
IsActive: optional.Some(true),
diff --git a/routers/web/user/search.go b/routers/web/user/search.go
index be5eee90a9..9acb9694d7 100644
--- a/routers/web/user/search.go
+++ b/routers/web/user/search.go
@@ -16,7 +16,7 @@ import (
// SearchCandidates searches candidate users for dropdown list
func SearchCandidates(ctx *context.Context) {
- users, _, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
+ users, _, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
Actor: ctx.Doer,
Keyword: ctx.FormTrim("q"),
Type: user_model.UserTypeIndividual,