From 90bd5b2b7dcb36f58cd93bb046928ec196e7652c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 21 Nov 2024 21:17:07 -0800 Subject: [PATCH 1/2] Try to use more simple SQL to get commit statuses --- models/fixtures/commit_status.yml | 5 ++ models/git/commit_status.go | 88 ++++++-------------------- tests/integration/repo_commits_test.go | 76 ++++++++++++++++++++++ 3 files changed, 101 insertions(+), 68 deletions(-) diff --git a/models/fixtures/commit_status.yml b/models/fixtures/commit_status.yml index 20d57975ef40d..87c652e53abc8 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 0579a41209e76..e4e3bd0b2ced7 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -323,53 +323,25 @@ func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOp // GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map[int64][]*CommitStatus, error) { - type result struct { - Index int64 - RepoID int64 - SHA string - } - - results := make([]result, 0, len(repoSHAs)) - - getBase := func() *xorm.Session { - return db.GetEngine(ctx).Table(&CommitStatus{}) - } - // Create a disjunction of conditions for each repoID and SHA pair conds := make([]builder.Cond, 0, len(repoSHAs)) for _, repoSHA := range repoSHAs { conds = append(conds, builder.Eq{"repo_id": repoSHA.RepoID, "sha": repoSHA.SHA}) } - sess := getBase().Where(builder.Or(conds...)). - Select("max( `index` ) as `index`, repo_id, sha"). - GroupBy("context_hash, repo_id, sha").OrderBy("max( `index` ) desc") - err := sess.Find(&results) - if err != nil { + commitStatuses := make([]*CommitStatus, 0, len(repoSHAs)) + if err := db.GetEngine(ctx). + Select("MAX(`index`) AS `index`, *"). + Where(builder.Or(conds...)). + GroupBy("context_hash, repo_id, sha").OrderBy("MAX(`index`) DESC"). + Find(&commitStatuses); err != nil { return nil, err } repoStatuses := make(map[int64][]*CommitStatus) - - if len(results) > 0 { - statuses := make([]*CommitStatus, 0, len(results)) - - conds = make([]builder.Cond, 0, len(results)) - for _, result := range results { - cond := builder.Eq{ - "`index`": result.Index, - "repo_id": result.RepoID, - "sha": result.SHA, - } - conds = append(conds, cond) - } - err = getBase().Where(builder.Or(conds...)).Find(&statuses) - if err != nil { - return nil, err - } - + if len(commitStatuses) > 0 { // Group the statuses by repo ID - for _, status := range statuses { + for _, status := range commitStatuses { repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status) } } @@ -379,45 +351,25 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map // GetLatestCommitStatusForRepoCommitIDs returns all statuses with a unique context for a given list of repo-sha pairs func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, commitIDs []string) (map[string][]*CommitStatus, error) { - type result struct { - Index int64 - SHA string - } - - getBase := func() *xorm.Session { - return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID) - } - results := make([]result, 0, len(commitIDs)) - conds := make([]builder.Cond, 0, len(commitIDs)) for _, sha := range commitIDs { conds = append(conds, builder.Eq{"sha": sha}) } - sess := getBase().And(builder.Or(conds...)). - Select("max( `index` ) as `index`, sha"). - GroupBy("context_hash, sha").OrderBy("max( `index` ) desc") - err := sess.Find(&results) - if err != nil { + commitStatuses := make([]*CommitStatus, 0, len(commitIDs)) + if err := db.GetEngine(ctx). + Select("MAX(`index`) AS `index`, *"). + Where("repo_id = ?", repoID). + And(builder.Or(conds...)). + GroupBy("context_hash, sha").OrderBy("max( `index` ) desc"). + Find(&commitStatuses); err != nil { return nil, err } repoStatuses := make(map[string][]*CommitStatus) - - if len(results) > 0 { - statuses := make([]*CommitStatus, 0, len(results)) - - conds = make([]builder.Cond, 0, len(results)) - for _, result := range results { - conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA}) - } - err = getBase().And(builder.Or(conds...)).Find(&statuses) - if err != nil { - return nil, err - } - + if len(commitStatuses) > 0 { // Group the statuses by commit - for _, status := range statuses { + for _, status := range commitStatuses { repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status) } } @@ -479,7 +431,7 @@ func NewCommitStatus(ctx context.Context, opts NewCommitStatusOptions) error { opts.CommitStatus.Index = idx log.Debug("NewCommitStatus[%s, %s]: %d", repoPath, opts.SHA, opts.CommitStatus.Index) - opts.CommitStatus.ContextHash = hashCommitStatusContext(opts.CommitStatus.Context) + opts.CommitStatus.ContextHash = HashCommitStatusContext(opts.CommitStatus.Context) // Insert new CommitStatus if _, err = db.GetEngine(ctx).Insert(opts.CommitStatus); err != nil { @@ -517,8 +469,8 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig return newCommits } -// hashCommitStatusContext hash context -func hashCommitStatusContext(context string) string { +// HashCommitStatusContext hash context +func HashCommitStatusContext(context string) string { return fmt.Sprintf("%x", sha1.Sum([]byte(context))) } diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index bb65d9e04a160..8baae644f7066 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -12,8 +12,13 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" @@ -203,3 +208,74 @@ func TestRepoCommitsStatusMultiple(t *testing.T) { sel := doc.doc.Find("#commits-table tbody tr td.message [data-tippy=\"commit-statuses\"] .commit-status") assert.Equal(t, 1, sel.Length()) } + +// Test_GetLatestCommitStatusForPairs tests GetLatestCommitStatusForPairs with multiple databases in integration tests +// But unit tests in CI will only test with sqlite +func Test_GetLatestCommitStatusForPairs(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + repoStatuses, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{ + {RepoID: repo1.ID, SHA: "1234123412341234123412341234123412341234"}, + }) + assert.NoError(t, err) + assert.Len(t, repoStatuses, 1) + assert.Len(t, repoStatuses[repo1.ID], 3) + + assert.EqualValues(t, 5, repoStatuses[repo1.ID][0].Index) + assert.EqualValues(t, structs.CommitStatusError, repoStatuses[repo1.ID][0].State) + assert.EqualValues(t, "deploy/awesomeness", repoStatuses[repo1.ID][0].Context) + assert.EqualValues(t, "https://example.com/builds/", repoStatuses[repo1.ID][0].TargetURL) + assert.EqualValues(t, "My awesome deploy service", repoStatuses[repo1.ID][0].Description) + assert.EqualValues(t, "ae9547713a6665fc4261d0756904932085a41cf2", repoStatuses[repo1.ID][0].ContextHash) + + assert.EqualValues(t, 4, repoStatuses[repo1.ID][1].Index) + assert.EqualValues(t, structs.CommitStatusFailure, repoStatuses[repo1.ID][1].State) + assert.EqualValues(t, "ci/awesomeness", repoStatuses[repo1.ID][1].Context) + assert.EqualValues(t, "https://example.com/builds/", repoStatuses[repo1.ID][1].TargetURL) + assert.EqualValues(t, "My awesome CI-service", repoStatuses[repo1.ID][1].Description) + assert.EqualValues(t, "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", repoStatuses[repo1.ID][1].ContextHash) + + assert.EqualValues(t, 3, repoStatuses[repo1.ID][2].Index) + // warning + success = success + assert.EqualValues(t, structs.CommitStatusSuccess, repoStatuses[repo1.ID][2].State) + assert.EqualValues(t, "cov/awesomeness", repoStatuses[repo1.ID][2].Context) + assert.EqualValues(t, "https://example.com/converage/", repoStatuses[repo1.ID][2].TargetURL) + assert.EqualValues(t, "My awesome Coverage service", repoStatuses[repo1.ID][2].Description) + assert.EqualValues(t, "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", repoStatuses[repo1.ID][2].ContextHash) +} + +func Test_GetLatestCommitStatusForRepoCommitIDs(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + commitID := "1234123412341234123412341234123412341234" + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, repo1.ID, []string{ + commitID, + }) + assert.NoError(t, err) + assert.Len(t, repoStatuses, 1) + assert.Len(t, repoStatuses[commitID], 3) + + assert.EqualValues(t, 5, repoStatuses[commitID][0].Index) + assert.EqualValues(t, structs.CommitStatusError, repoStatuses[commitID][0].State) + assert.EqualValues(t, "deploy/awesomeness", repoStatuses[commitID][0].Context) + assert.EqualValues(t, "https://example.com/builds/", repoStatuses[commitID][0].TargetURL) + assert.EqualValues(t, "My awesome deploy service", repoStatuses[commitID][0].Description) + assert.EqualValues(t, "ae9547713a6665fc4261d0756904932085a41cf2", repoStatuses[commitID][0].ContextHash) + + assert.EqualValues(t, 4, repoStatuses[commitID][1].Index) + assert.EqualValues(t, structs.CommitStatusFailure, repoStatuses[commitID][1].State) + assert.EqualValues(t, "ci/awesomeness", repoStatuses[commitID][1].Context) + assert.EqualValues(t, "https://example.com/builds/", repoStatuses[commitID][1].TargetURL) + assert.EqualValues(t, "My awesome CI-service", repoStatuses[commitID][1].Description) + assert.EqualValues(t, "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", repoStatuses[commitID][1].ContextHash) + + assert.EqualValues(t, 3, repoStatuses[commitID][2].Index) + // warning + success = success + assert.EqualValues(t, structs.CommitStatusSuccess, repoStatuses[commitID][2].State) + assert.EqualValues(t, "cov/awesomeness", repoStatuses[commitID][2].Context) + assert.EqualValues(t, "https://example.com/converage/", repoStatuses[commitID][2].TargetURL) + assert.EqualValues(t, "My awesome Coverage service", repoStatuses[commitID][2].Description) + assert.EqualValues(t, "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", repoStatuses[commitID][2].ContextHash) +} From 1bc55dfb6596696f041997105e1942a538b5efde Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 21 Nov 2024 21:31:31 -0800 Subject: [PATCH 2/2] Fix lint --- tests/integration/repo_commits_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index 8baae644f7066..509ad0c1308d2 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -18,7 +18,6 @@ import ( "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" @@ -223,14 +222,14 @@ func Test_GetLatestCommitStatusForPairs(t *testing.T) { assert.Len(t, repoStatuses[repo1.ID], 3) assert.EqualValues(t, 5, repoStatuses[repo1.ID][0].Index) - assert.EqualValues(t, structs.CommitStatusError, repoStatuses[repo1.ID][0].State) + assert.EqualValues(t, api.CommitStatusError, repoStatuses[repo1.ID][0].State) assert.EqualValues(t, "deploy/awesomeness", repoStatuses[repo1.ID][0].Context) assert.EqualValues(t, "https://example.com/builds/", repoStatuses[repo1.ID][0].TargetURL) assert.EqualValues(t, "My awesome deploy service", repoStatuses[repo1.ID][0].Description) assert.EqualValues(t, "ae9547713a6665fc4261d0756904932085a41cf2", repoStatuses[repo1.ID][0].ContextHash) assert.EqualValues(t, 4, repoStatuses[repo1.ID][1].Index) - assert.EqualValues(t, structs.CommitStatusFailure, repoStatuses[repo1.ID][1].State) + assert.EqualValues(t, api.CommitStatusFailure, repoStatuses[repo1.ID][1].State) assert.EqualValues(t, "ci/awesomeness", repoStatuses[repo1.ID][1].Context) assert.EqualValues(t, "https://example.com/builds/", repoStatuses[repo1.ID][1].TargetURL) assert.EqualValues(t, "My awesome CI-service", repoStatuses[repo1.ID][1].Description) @@ -238,7 +237,7 @@ func Test_GetLatestCommitStatusForPairs(t *testing.T) { assert.EqualValues(t, 3, repoStatuses[repo1.ID][2].Index) // warning + success = success - assert.EqualValues(t, structs.CommitStatusSuccess, repoStatuses[repo1.ID][2].State) + assert.EqualValues(t, api.CommitStatusSuccess, repoStatuses[repo1.ID][2].State) assert.EqualValues(t, "cov/awesomeness", repoStatuses[repo1.ID][2].Context) assert.EqualValues(t, "https://example.com/converage/", repoStatuses[repo1.ID][2].TargetURL) assert.EqualValues(t, "My awesome Coverage service", repoStatuses[repo1.ID][2].Description) @@ -258,14 +257,14 @@ func Test_GetLatestCommitStatusForRepoCommitIDs(t *testing.T) { assert.Len(t, repoStatuses[commitID], 3) assert.EqualValues(t, 5, repoStatuses[commitID][0].Index) - assert.EqualValues(t, structs.CommitStatusError, repoStatuses[commitID][0].State) + assert.EqualValues(t, api.CommitStatusError, repoStatuses[commitID][0].State) assert.EqualValues(t, "deploy/awesomeness", repoStatuses[commitID][0].Context) assert.EqualValues(t, "https://example.com/builds/", repoStatuses[commitID][0].TargetURL) assert.EqualValues(t, "My awesome deploy service", repoStatuses[commitID][0].Description) assert.EqualValues(t, "ae9547713a6665fc4261d0756904932085a41cf2", repoStatuses[commitID][0].ContextHash) assert.EqualValues(t, 4, repoStatuses[commitID][1].Index) - assert.EqualValues(t, structs.CommitStatusFailure, repoStatuses[commitID][1].State) + assert.EqualValues(t, api.CommitStatusFailure, repoStatuses[commitID][1].State) assert.EqualValues(t, "ci/awesomeness", repoStatuses[commitID][1].Context) assert.EqualValues(t, "https://example.com/builds/", repoStatuses[commitID][1].TargetURL) assert.EqualValues(t, "My awesome CI-service", repoStatuses[commitID][1].Description) @@ -273,7 +272,7 @@ func Test_GetLatestCommitStatusForRepoCommitIDs(t *testing.T) { assert.EqualValues(t, 3, repoStatuses[commitID][2].Index) // warning + success = success - assert.EqualValues(t, structs.CommitStatusSuccess, repoStatuses[commitID][2].State) + assert.EqualValues(t, api.CommitStatusSuccess, repoStatuses[commitID][2].State) assert.EqualValues(t, "cov/awesomeness", repoStatuses[commitID][2].Context) assert.EqualValues(t, "https://example.com/converage/", repoStatuses[commitID][2].TargetURL) assert.EqualValues(t, "My awesome Coverage service", repoStatuses[commitID][2].Description)