Skip to content

Commit 688da55

Browse files
lunnywxiaoguang
lunny
and
wxiaoguang
authored
Split GetLatestCommitStatus as two functions (#34535)
Extract from #34531. This will reduce unnecessary count operation in databases. --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent ab96912 commit 688da55

File tree

15 files changed

+73
-30
lines changed

15 files changed

+73
-30
lines changed

‎models/fixtures/commit_status.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
target_url: https://example.com/builds/
88
description: My awesome CI-service
99
context: ci/awesomeness
10+
context_hash: c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7
1011
creator_id: 2
1112

1213
-
@@ -18,6 +19,7 @@
1819
target_url: https://example.com/converage/
1920
description: My awesome Coverage service
2021
context: cov/awesomeness
22+
context_hash: 3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe
2123
creator_id: 2
2224

2325
-
@@ -29,6 +31,7 @@
2931
target_url: https://example.com/converage/
3032
description: My awesome Coverage service
3133
context: cov/awesomeness
34+
context_hash: 3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe
3235
creator_id: 2
3336

3437
-
@@ -40,6 +43,7 @@
4043
target_url: https://example.com/builds/
4144
description: My awesome CI-service
4245
context: ci/awesomeness
46+
context_hash: c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7
4347
creator_id: 2
4448

4549
-
@@ -51,4 +55,5 @@
5155
target_url: https://example.com/builds/
5256
description: My awesome deploy service
5357
context: deploy/awesomeness
58+
context_hash: ae9547713a6665fc4261d0756904932085a41cf2
5459
creator_id: 2

‎models/git/commit_status.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -298,27 +298,37 @@ type CommitStatusIndex struct {
298298
MaxIndex int64 `xorm:"index"`
299299
}
300300

301+
func makeRepoCommitQuery(ctx context.Context, repoID int64, sha string) *xorm.Session {
302+
return db.GetEngine(ctx).Table(&CommitStatus{}).
303+
Where("repo_id = ?", repoID).And("sha = ?", sha)
304+
}
305+
301306
// GetLatestCommitStatus returns all statuses with a unique context for a given commit.
302-
func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
303-
getBase := func() *xorm.Session {
304-
return db.GetEngine(ctx).Table(&CommitStatus{}).
305-
Where("repo_id = ?", repoID).And("sha = ?", sha)
306-
}
307+
func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, error) {
307308
indices := make([]int64, 0, 10)
308-
sess := getBase().Select("max( `index` ) as `index`").
309-
GroupBy("context_hash").OrderBy("max( `index` ) desc")
309+
sess := makeRepoCommitQuery(ctx, repoID, sha).
310+
Select("max( `index` ) as `index`").
311+
GroupBy("context_hash").
312+
OrderBy("max( `index` ) desc")
310313
if !listOptions.IsListAll() {
311314
sess = db.SetSessionPagination(sess, &listOptions)
312315
}
313-
count, err := sess.FindAndCount(&indices)
314-
if err != nil {
315-
return nil, count, err
316+
if err := sess.Find(&indices); err != nil {
317+
return nil, err
316318
}
317319
statuses := make([]*CommitStatus, 0, len(indices))
318320
if len(indices) == 0 {
319-
return statuses, count, nil
321+
return statuses, nil
320322
}
321-
return statuses, count, getBase().And(builder.In("`index`", indices)).Find(&statuses)
323+
err := makeRepoCommitQuery(ctx, repoID, sha).And(builder.In("`index`", indices)).Find(&statuses)
324+
return statuses, err
325+
}
326+
327+
func CountLatestCommitStatus(ctx context.Context, repoID int64, sha string) (int64, error) {
328+
return makeRepoCommitQuery(ctx, repoID, sha).
329+
Select("count(context_hash)").
330+
GroupBy("context_hash").
331+
Count()
322332
}
323333

324334
// GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs

‎models/git/commit_status_summary.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func GetLatestCommitStatusForRepoAndSHAs(ctx context.Context, repoSHAs []RepoSHA
5555
}
5656

5757
func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) error {
58-
commitStatuses, _, err := GetLatestCommitStatus(ctx, repoID, sha, db.ListOptionsAll)
58+
commitStatuses, err := GetLatestCommitStatus(ctx, repoID, sha, db.ListOptionsAll)
5959
if err != nil {
6060
return err
6161
}

‎models/git/commit_status_test.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TestGetCommitStatuses(t *testing.T) {
2626

2727
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
2828

29-
sha1 := "1234123412341234123412341234123412341234"
29+
sha1 := "1234123412341234123412341234123412341234" // the mocked commit ID in test fixtures
3030

3131
statuses, maxResults, err := db.FindAndCount[git_model.CommitStatus](db.DefaultContext, &git_model.CommitStatusOptions{
3232
ListOptions: db.ListOptions{Page: 1, PageSize: 50},
@@ -256,3 +256,26 @@ func TestCommitStatusesHideActionsURL(t *testing.T) {
256256
assert.Empty(t, statuses[0].TargetURL)
257257
assert.Equal(t, "https://mycicd.org/1", statuses[1].TargetURL)
258258
}
259+
260+
func TestGetCountLatestCommitStatus(t *testing.T) {
261+
assert.NoError(t, unittest.PrepareTestDatabase())
262+
263+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
264+
265+
sha1 := "1234123412341234123412341234123412341234" // the mocked commit ID in test fixtures
266+
267+
commitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo1.ID, sha1, db.ListOptions{
268+
Page: 1,
269+
PageSize: 2,
270+
})
271+
assert.NoError(t, err)
272+
assert.Len(t, commitStatuses, 2)
273+
assert.Equal(t, structs.CommitStatusFailure, commitStatuses[0].State)
274+
assert.Equal(t, "ci/awesomeness", commitStatuses[0].Context)
275+
assert.Equal(t, structs.CommitStatusError, commitStatuses[1].State)
276+
assert.Equal(t, "deploy/awesomeness", commitStatuses[1].Context)
277+
278+
count, err := git_model.CountLatestCommitStatus(db.DefaultContext, repo1.ID, sha1)
279+
assert.NoError(t, err)
280+
assert.EqualValues(t, 3, count)
281+
}

‎routers/api/v1/repo/status.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,19 +258,24 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) {
258258

259259
repo := ctx.Repo.Repository
260260

261-
statuses, count, err := git_model.GetLatestCommitStatus(ctx, repo.ID, refCommit.Commit.ID.String(), utils.GetListOptions(ctx))
261+
statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, refCommit.Commit.ID.String(), utils.GetListOptions(ctx))
262262
if err != nil {
263263
ctx.APIErrorInternal(fmt.Errorf("GetLatestCommitStatus[%s, %s]: %w", repo.FullName(), refCommit.CommitID, err))
264264
return
265265
}
266266

267+
count, err := git_model.CountLatestCommitStatus(ctx, repo.ID, refCommit.Commit.ID.String())
268+
if err != nil {
269+
ctx.APIErrorInternal(fmt.Errorf("CountLatestCommitStatus[%s, %s]: %w", repo.FullName(), refCommit.CommitID, err))
270+
return
271+
}
272+
ctx.SetTotalCountHeader(count)
273+
267274
if len(statuses) == 0 {
268275
ctx.JSON(http.StatusOK, &api.CombinedStatus{})
269276
return
270277
}
271278

272279
combiStatus := convert.ToCombinedStatus(ctx, statuses, convert.ToRepo(ctx, repo, ctx.Repo.Permission))
273-
274-
ctx.SetTotalCountHeader(count)
275280
ctx.JSON(http.StatusOK, combiStatus)
276281
}

‎routers/web/repo/commit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ func Diff(ctx *context.Context) {
377377
ctx.Data["FileIconPoolHTML"] = renderedIconPool.RenderToHTML()
378378
}
379379

380-
statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll)
380+
statuses, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll)
381381
if err != nil {
382382
log.Error("GetLatestCommitStatus: %v", err)
383383
}

‎routers/web/repo/pull.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue)
291291

292292
if len(compareInfo.Commits) != 0 {
293293
sha := compareInfo.Commits[0].ID.String()
294-
commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, sha, db.ListOptionsAll)
294+
commitStatuses, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, sha, db.ListOptionsAll)
295295
if err != nil {
296296
ctx.ServerError("GetLatestCommitStatus", err)
297297
return nil
@@ -358,7 +358,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
358358
ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
359359
return nil
360360
}
361-
commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
361+
commitStatuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
362362
if err != nil {
363363
ctx.ServerError("GetLatestCommitStatus", err)
364364
return nil
@@ -454,7 +454,7 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C
454454
return nil
455455
}
456456

457-
commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
457+
commitStatuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll)
458458
if err != nil {
459459
ctx.ServerError("GetLatestCommitStatus", err)
460460
return nil

‎routers/web/repo/release.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions)
130130
}
131131

132132
if canReadActions {
133-
statuses, _, err := git_model.GetLatestCommitStatus(ctx, r.Repo.ID, r.Sha1, db.ListOptionsAll)
133+
statuses, err := git_model.GetLatestCommitStatus(ctx, r.Repo.ID, r.Sha1, db.ListOptionsAll)
134134
if err != nil {
135135
return nil, err
136136
}

‎routers/web/repo/view.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool {
131131
ctx.Data["LatestCommitVerification"] = verification
132132
ctx.Data["LatestCommitUser"] = user_model.ValidateCommitWithEmail(ctx, latestCommit)
133133

134-
statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, latestCommit.ID.String(), db.ListOptionsAll)
134+
statuses, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, latestCommit.ID.String(), db.ListOptionsAll)
135135
if err != nil {
136136
log.Error("GetLatestCommitStatus: %v", err)
137137
}

‎services/actions/commit_status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
9292
}
9393
ctxname := fmt.Sprintf("%s / %s (%s)", runName, job.Name, event)
9494
state := toCommitStatus(job.Status)
95-
if statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll); err == nil {
95+
if statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptionsAll); err == nil {
9696
for _, v := range statuses {
9797
if v.Context == ctxname {
9898
if v.State == state {

‎services/git/commit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig
8484
commit := &git_model.SignCommitWithStatuses{
8585
SignCommit: c,
8686
}
87-
statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptions{})
87+
statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptionsAll)
8888
if err != nil {
8989
return nil, err
9090
}

‎services/pull/commit_status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullR
151151
return "", errors.Wrap(err, "LoadBaseRepo")
152152
}
153153

154-
commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
154+
commitStatuses, err := git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
155155
if err != nil {
156156
return "", errors.Wrap(err, "GetLatestCommitStatus")
157157
}

‎services/pull/pull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@ func getAllCommitStatus(ctx context.Context, gitRepo *git.Repository, pr *issues
10041004
return nil, nil, shaErr
10051005
}
10061006

1007-
statuses, _, err = git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
1007+
statuses, err = git_model.GetLatestCommitStatus(ctx, pr.BaseRepo.ID, sha, db.ListOptionsAll)
10081008
lastStatus = git_model.CalcCommitStatus(statuses)
10091009
return statuses, lastStatus, err
10101010
}

‎services/repository/gitgraph/graph_models.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_
121121
return repo_model.IsOwnerMemberCollaborator(ctx, repository, user.ID)
122122
}, &keyMap)
123123

124-
statuses, _, err := git_model.GetLatestCommitStatus(ctx, repository.ID, c.Commit.ID.String(), db.ListOptions{})
124+
statuses, err := git_model.GetLatestCommitStatus(ctx, repository.ID, c.Commit.ID.String(), db.ListOptionsAll)
125125
if err != nil {
126126
log.Error("GetLatestCommitStatus: %v", err)
127127
} else {

‎tests/integration/actions_trigger_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,7 @@ jobs:
633633
assert.NotEmpty(t, addFileResp)
634634
sha = addFileResp.Commit.SHA
635635
assert.Eventually(t, func() bool {
636-
latestCommitStatuses, _, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
636+
latestCommitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
637637
assert.NoError(t, err)
638638
if len(latestCommitStatuses) == 0 {
639639
return false
@@ -676,7 +676,7 @@ jobs:
676676
}
677677

678678
func checkCommitStatusAndInsertFakeStatus(t *testing.T, repo *repo_model.Repository, sha string) {
679-
latestCommitStatuses, _, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
679+
latestCommitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll)
680680
assert.NoError(t, err)
681681
assert.Len(t, latestCommitStatuses, 1)
682682
assert.Equal(t, api.CommitStatusPending, latestCommitStatuses[0].State)

0 commit comments

Comments
 (0)