Skip to content

Commit 90bd5b2

Browse files
committed
Try to use more simple SQL to get commit statuses
1 parent a3881ff commit 90bd5b2

File tree

3 files changed

+101
-68
lines changed

3 files changed

+101
-68
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: 20 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -323,53 +323,25 @@ func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOp
323323

324324
// GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs
325325
func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map[int64][]*CommitStatus, error) {
326-
type result struct {
327-
Index int64
328-
RepoID int64
329-
SHA string
330-
}
331-
332-
results := make([]result, 0, len(repoSHAs))
333-
334-
getBase := func() *xorm.Session {
335-
return db.GetEngine(ctx).Table(&CommitStatus{})
336-
}
337-
338326
// Create a disjunction of conditions for each repoID and SHA pair
339327
conds := make([]builder.Cond, 0, len(repoSHAs))
340328
for _, repoSHA := range repoSHAs {
341329
conds = append(conds, builder.Eq{"repo_id": repoSHA.RepoID, "sha": repoSHA.SHA})
342330
}
343-
sess := getBase().Where(builder.Or(conds...)).
344-
Select("max( `index` ) as `index`, repo_id, sha").
345-
GroupBy("context_hash, repo_id, sha").OrderBy("max( `index` ) desc")
346331

347-
err := sess.Find(&results)
348-
if err != nil {
332+
commitStatuses := make([]*CommitStatus, 0, len(repoSHAs))
333+
if err := db.GetEngine(ctx).
334+
Select("MAX(`index`) AS `index`, *").
335+
Where(builder.Or(conds...)).
336+
GroupBy("context_hash, repo_id, sha").OrderBy("MAX(`index`) DESC").
337+
Find(&commitStatuses); err != nil {
349338
return nil, err
350339
}
351340

352341
repoStatuses := make(map[int64][]*CommitStatus)
353-
354-
if len(results) > 0 {
355-
statuses := make([]*CommitStatus, 0, len(results))
356-
357-
conds = make([]builder.Cond, 0, len(results))
358-
for _, result := range results {
359-
cond := builder.Eq{
360-
"`index`": result.Index,
361-
"repo_id": result.RepoID,
362-
"sha": result.SHA,
363-
}
364-
conds = append(conds, cond)
365-
}
366-
err = getBase().Where(builder.Or(conds...)).Find(&statuses)
367-
if err != nil {
368-
return nil, err
369-
}
370-
342+
if len(commitStatuses) > 0 {
371343
// Group the statuses by repo ID
372-
for _, status := range statuses {
344+
for _, status := range commitStatuses {
373345
repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status)
374346
}
375347
}
@@ -379,45 +351,25 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map
379351

380352
// GetLatestCommitStatusForRepoCommitIDs returns all statuses with a unique context for a given list of repo-sha pairs
381353
func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, commitIDs []string) (map[string][]*CommitStatus, error) {
382-
type result struct {
383-
Index int64
384-
SHA string
385-
}
386-
387-
getBase := func() *xorm.Session {
388-
return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID)
389-
}
390-
results := make([]result, 0, len(commitIDs))
391-
392354
conds := make([]builder.Cond, 0, len(commitIDs))
393355
for _, sha := range commitIDs {
394356
conds = append(conds, builder.Eq{"sha": sha})
395357
}
396-
sess := getBase().And(builder.Or(conds...)).
397-
Select("max( `index` ) as `index`, sha").
398-
GroupBy("context_hash, sha").OrderBy("max( `index` ) desc")
399358

400-
err := sess.Find(&results)
401-
if err != nil {
359+
commitStatuses := make([]*CommitStatus, 0, len(commitIDs))
360+
if err := db.GetEngine(ctx).
361+
Select("MAX(`index`) AS `index`, *").
362+
Where("repo_id = ?", repoID).
363+
And(builder.Or(conds...)).
364+
GroupBy("context_hash, sha").OrderBy("max( `index` ) desc").
365+
Find(&commitStatuses); err != nil {
402366
return nil, err
403367
}
404368

405369
repoStatuses := make(map[string][]*CommitStatus)
406-
407-
if len(results) > 0 {
408-
statuses := make([]*CommitStatus, 0, len(results))
409-
410-
conds = make([]builder.Cond, 0, len(results))
411-
for _, result := range results {
412-
conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA})
413-
}
414-
err = getBase().And(builder.Or(conds...)).Find(&statuses)
415-
if err != nil {
416-
return nil, err
417-
}
418-
370+
if len(commitStatuses) > 0 {
419371
// Group the statuses by commit
420-
for _, status := range statuses {
372+
for _, status := range commitStatuses {
421373
repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status)
422374
}
423375
}
@@ -479,7 +431,7 @@ func NewCommitStatus(ctx context.Context, opts NewCommitStatusOptions) error {
479431
opts.CommitStatus.Index = idx
480432
log.Debug("NewCommitStatus[%s, %s]: %d", repoPath, opts.SHA, opts.CommitStatus.Index)
481433

482-
opts.CommitStatus.ContextHash = hashCommitStatusContext(opts.CommitStatus.Context)
434+
opts.CommitStatus.ContextHash = HashCommitStatusContext(opts.CommitStatus.Context)
483435

484436
// Insert new CommitStatus
485437
if _, err = db.GetEngine(ctx).Insert(opts.CommitStatus); err != nil {
@@ -517,8 +469,8 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig
517469
return newCommits
518470
}
519471

520-
// hashCommitStatusContext hash context
521-
func hashCommitStatusContext(context string) string {
472+
// HashCommitStatusContext hash context
473+
func HashCommitStatusContext(context string) string {
522474
return fmt.Sprintf("%x", sha1.Sum([]byte(context)))
523475
}
524476

tests/integration/repo_commits_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,13 @@ import (
1212
"testing"
1313

1414
auth_model "code.gitea.io/gitea/models/auth"
15+
"code.gitea.io/gitea/models/db"
16+
git_model "code.gitea.io/gitea/models/git"
17+
repo_model "code.gitea.io/gitea/models/repo"
18+
"code.gitea.io/gitea/models/unittest"
1519
"code.gitea.io/gitea/modules/json"
1620
"code.gitea.io/gitea/modules/setting"
21+
"code.gitea.io/gitea/modules/structs"
1722
api "code.gitea.io/gitea/modules/structs"
1823
"code.gitea.io/gitea/tests"
1924

@@ -203,3 +208,74 @@ func TestRepoCommitsStatusMultiple(t *testing.T) {
203208
sel := doc.doc.Find("#commits-table tbody tr td.message [data-tippy=\"commit-statuses\"] .commit-status")
204209
assert.Equal(t, 1, sel.Length())
205210
}
211+
212+
// Test_GetLatestCommitStatusForPairs tests GetLatestCommitStatusForPairs with multiple databases in integration tests
213+
// But unit tests in CI will only test with sqlite
214+
func Test_GetLatestCommitStatusForPairs(t *testing.T) {
215+
defer tests.PrepareTestEnv(t)()
216+
217+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
218+
repoStatuses, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{
219+
{RepoID: repo1.ID, SHA: "1234123412341234123412341234123412341234"},
220+
})
221+
assert.NoError(t, err)
222+
assert.Len(t, repoStatuses, 1)
223+
assert.Len(t, repoStatuses[repo1.ID], 3)
224+
225+
assert.EqualValues(t, 5, repoStatuses[repo1.ID][0].Index)
226+
assert.EqualValues(t, structs.CommitStatusError, repoStatuses[repo1.ID][0].State)
227+
assert.EqualValues(t, "deploy/awesomeness", repoStatuses[repo1.ID][0].Context)
228+
assert.EqualValues(t, "https://example.com/builds/", repoStatuses[repo1.ID][0].TargetURL)
229+
assert.EqualValues(t, "My awesome deploy service", repoStatuses[repo1.ID][0].Description)
230+
assert.EqualValues(t, "ae9547713a6665fc4261d0756904932085a41cf2", repoStatuses[repo1.ID][0].ContextHash)
231+
232+
assert.EqualValues(t, 4, repoStatuses[repo1.ID][1].Index)
233+
assert.EqualValues(t, structs.CommitStatusFailure, repoStatuses[repo1.ID][1].State)
234+
assert.EqualValues(t, "ci/awesomeness", repoStatuses[repo1.ID][1].Context)
235+
assert.EqualValues(t, "https://example.com/builds/", repoStatuses[repo1.ID][1].TargetURL)
236+
assert.EqualValues(t, "My awesome CI-service", repoStatuses[repo1.ID][1].Description)
237+
assert.EqualValues(t, "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", repoStatuses[repo1.ID][1].ContextHash)
238+
239+
assert.EqualValues(t, 3, repoStatuses[repo1.ID][2].Index)
240+
// warning + success = success
241+
assert.EqualValues(t, structs.CommitStatusSuccess, repoStatuses[repo1.ID][2].State)
242+
assert.EqualValues(t, "cov/awesomeness", repoStatuses[repo1.ID][2].Context)
243+
assert.EqualValues(t, "https://example.com/converage/", repoStatuses[repo1.ID][2].TargetURL)
244+
assert.EqualValues(t, "My awesome Coverage service", repoStatuses[repo1.ID][2].Description)
245+
assert.EqualValues(t, "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", repoStatuses[repo1.ID][2].ContextHash)
246+
}
247+
248+
func Test_GetLatestCommitStatusForRepoCommitIDs(t *testing.T) {
249+
defer tests.PrepareTestEnv(t)()
250+
251+
commitID := "1234123412341234123412341234123412341234"
252+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
253+
repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, repo1.ID, []string{
254+
commitID,
255+
})
256+
assert.NoError(t, err)
257+
assert.Len(t, repoStatuses, 1)
258+
assert.Len(t, repoStatuses[commitID], 3)
259+
260+
assert.EqualValues(t, 5, repoStatuses[commitID][0].Index)
261+
assert.EqualValues(t, structs.CommitStatusError, repoStatuses[commitID][0].State)
262+
assert.EqualValues(t, "deploy/awesomeness", repoStatuses[commitID][0].Context)
263+
assert.EqualValues(t, "https://example.com/builds/", repoStatuses[commitID][0].TargetURL)
264+
assert.EqualValues(t, "My awesome deploy service", repoStatuses[commitID][0].Description)
265+
assert.EqualValues(t, "ae9547713a6665fc4261d0756904932085a41cf2", repoStatuses[commitID][0].ContextHash)
266+
267+
assert.EqualValues(t, 4, repoStatuses[commitID][1].Index)
268+
assert.EqualValues(t, structs.CommitStatusFailure, repoStatuses[commitID][1].State)
269+
assert.EqualValues(t, "ci/awesomeness", repoStatuses[commitID][1].Context)
270+
assert.EqualValues(t, "https://example.com/builds/", repoStatuses[commitID][1].TargetURL)
271+
assert.EqualValues(t, "My awesome CI-service", repoStatuses[commitID][1].Description)
272+
assert.EqualValues(t, "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", repoStatuses[commitID][1].ContextHash)
273+
274+
assert.EqualValues(t, 3, repoStatuses[commitID][2].Index)
275+
// warning + success = success
276+
assert.EqualValues(t, structs.CommitStatusSuccess, repoStatuses[commitID][2].State)
277+
assert.EqualValues(t, "cov/awesomeness", repoStatuses[commitID][2].Context)
278+
assert.EqualValues(t, "https://example.com/converage/", repoStatuses[commitID][2].TargetURL)
279+
assert.EqualValues(t, "My awesome Coverage service", repoStatuses[commitID][2].Description)
280+
assert.EqualValues(t, "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", repoStatuses[commitID][2].ContextHash)
281+
}

0 commit comments

Comments
 (0)