From 2faa7ce749eebbe748fd702540bd8df613e3b77a Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Sun, 1 Dec 2024 19:15:31 +0000 Subject: [PATCH] Rework GetLatestCommitStatusForPairs query using a subquery for PG compatibility (#6113) ## Scope Intended to fix #6099; and moves related tests to integration tests (per. request https://codeberg.org/forgejo/forgejo/pulls/6105#issuecomment-2486228) ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. **not applicable** - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... **not applicable** - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] I do not want this change to show in the release notes. - Rationale: bug-fix for a change that hasn't been released yet. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6113 Reviewed-by: Gusted Reviewed-by: Earl Warren Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- models/git/commit_status.go | 17 +- models/git/commit_status_test.go | 179 -------------------- tests/integration/commit_status_test.go | 206 ++++++++++++++++++++++++ 3 files changed, 220 insertions(+), 182 deletions(-) create mode 100644 tests/integration/commit_status_test.go diff --git a/models/git/commit_status.go b/models/git/commit_status.go index eae2af2f90..f6ac1dcc54 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -302,10 +302,21 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map conds = append(conds, builder.Eq{"repo_id": repoSHA.RepoID, "sha": repoSHA.SHA}) } - sess := db.GetEngine(ctx).Table(&CommitStatus{}). - Select("MAX(`index`) AS `index`, *"). + subquery := builder.Dialect(db.BuilderDialect()). + Select("context_hash, repo_id, sha, MAX(`index`) AS max_index"). + From("commit_status"). Where(builder.Or(conds...)). - GroupBy("context_hash, repo_id, sha").OrderBy("MAX(`index`) DESC") + GroupBy("context_hash, repo_id, sha") + + sess := db.GetEngine(ctx). + Table(&CommitStatus{}). + Alias("c"). + Join( + "INNER", + subquery, + "c.context_hash = commit_status.context_hash AND c.repo_id = commit_status.repo_id AND c.sha = commit_status.sha AND c.`index` = commit_status.max_index", + ). + OrderBy("c.`index` DESC") err := sess.Find(&results) if err != nil { return nil, err diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index 2adedee119..9a039bfc39 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -269,182 +269,3 @@ func TestCommitStatusesHideActionsURL(t *testing.T) { assert.Empty(t, statuses[0].TargetURL) assert.Equal(t, "https://mycicd.org/1", statuses[1].TargetURL) } - -func TestGetLatestCommitStatusForPairs(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - t.Run("Empty", func(t *testing.T) { - pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, nil) - require.NoError(t, err) - assert.EqualValues(t, map[int64][]*git_model.CommitStatus{}, pairs) - }) - - t.Run("Repo 1", func(t *testing.T) { - pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{1, "1234123412341234123412341234123412341234"}}) - require.NoError(t, err) - - assert.EqualValues(t, map[int64][]*git_model.CommitStatus{ - 1: { - { - ID: 7, - Index: 6, - RepoID: 1, - State: structs.CommitStatusPending, - SHA: "1234123412341234123412341234123412341234", - TargetURL: "https://example.com/builds/", - Description: "My awesome deploy service", - ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", - Context: "deploy/awesomeness", - CreatorID: 2, - }, - { - ID: 4, - Index: 4, - State: structs.CommitStatusFailure, - TargetURL: "https://example.com/builds/", - Description: "My awesome CI-service", - Context: "ci/awesomeness", - CreatorID: 2, - RepoID: 1, - SHA: "1234123412341234123412341234123412341234", - ContextHash: "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", - }, - { - ID: 3, - Index: 3, - State: structs.CommitStatusSuccess, - TargetURL: "https://example.com/coverage/", - Description: "My awesome Coverage service", - Context: "cov/awesomeness", - CreatorID: 2, - RepoID: 1, - SHA: "1234123412341234123412341234123412341234", - ContextHash: "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", - }, - }, - }, pairs) - }) - t.Run("Repo 62", func(t *testing.T) { - pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{62, "774f93df12d14931ea93259ae93418da4482fcc1"}}) - require.NoError(t, err) - - assert.EqualValues(t, map[int64][]*git_model.CommitStatus{ - 62: { - { - ID: 8, - Index: 2, - RepoID: 62, - State: structs.CommitStatusError, - TargetURL: "/user2/test_workflows/actions", - Description: "My awesome deploy service - v2", - Context: "deploy/awesomeness", - SHA: "774f93df12d14931ea93259ae93418da4482fcc1", - ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", - CreatorID: 2, - }, - }, - }, pairs) - }) - - t.Run("Repo 62 non-existent sha", func(t *testing.T) { - pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{62, "774f93df12d14931ea93259ae93418da4482fcc"}}) - require.NoError(t, err) - - assert.EqualValues(t, map[int64][]*git_model.CommitStatus{}, pairs) - }) - - t.Run("SHA with non-existent repo id", func(t *testing.T) { - pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{1, "774f93df12d14931ea93259ae93418da4482fcc1"}}) - require.NoError(t, err) - - assert.EqualValues(t, map[int64][]*git_model.CommitStatus{}, pairs) - }) -} - -func TestGetLatestCommitStatusForRepoCommitIDs(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - t.Run("Empty", func(t *testing.T) { - repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 62, nil) - require.NoError(t, err) - assert.EqualValues(t, map[string][]*git_model.CommitStatus{}, repoStatuses) - }) - - t.Run("Repo 1", func(t *testing.T) { - repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 1, []string{"1234123412341234123412341234123412341234"}) - require.NoError(t, err) - assert.EqualValues(t, map[string][]*git_model.CommitStatus{ - "1234123412341234123412341234123412341234": { - { - ID: 3, - Index: 3, - State: structs.CommitStatusSuccess, - TargetURL: "https://example.com/coverage/", - Description: "My awesome Coverage service", - Context: "cov/awesomeness", - CreatorID: 2, - RepoID: 1, - SHA: "1234123412341234123412341234123412341234", - ContextHash: "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", - }, - { - ID: 4, - Index: 4, - State: structs.CommitStatusFailure, - TargetURL: "https://example.com/builds/", - Description: "My awesome CI-service", - Context: "ci/awesomeness", - CreatorID: 2, - RepoID: 1, - SHA: "1234123412341234123412341234123412341234", - ContextHash: "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", - }, - { - ID: 7, - Index: 6, - RepoID: 1, - State: structs.CommitStatusPending, - SHA: "1234123412341234123412341234123412341234", - TargetURL: "https://example.com/builds/", - Description: "My awesome deploy service", - ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", - Context: "deploy/awesomeness", - CreatorID: 2, - }, - }, - }, repoStatuses) - }) - - t.Run("Repo 62", func(t *testing.T) { - repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 62, []string{"774f93df12d14931ea93259ae93418da4482fcc1"}) - require.NoError(t, err) - assert.EqualValues(t, map[string][]*git_model.CommitStatus{ - "774f93df12d14931ea93259ae93418da4482fcc1": { - { - ID: 8, - Index: 2, - RepoID: 62, - State: structs.CommitStatusError, - TargetURL: "/user2/test_workflows/actions", - Description: "My awesome deploy service - v2", - Context: "deploy/awesomeness", - SHA: "774f93df12d14931ea93259ae93418da4482fcc1", - ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", - CreatorID: 2, - }, - }, - }, repoStatuses) - }) - - t.Run("Repo 62 non-existent sha", func(t *testing.T) { - repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 62, []string{"774f93df12d14931ea93259ae93418da4482fcc"}) - require.NoError(t, err) - assert.EqualValues(t, map[string][]*git_model.CommitStatus{}, repoStatuses) - }) - - t.Run("non-existent repo ID", func(t *testing.T) { - repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 1, []string{"774f93df12d14931ea93259ae93418da4482fcc"}) - require.NoError(t, err) - assert.EqualValues(t, map[string][]*git_model.CommitStatus{}, repoStatuses) - }) -} diff --git a/tests/integration/commit_status_test.go b/tests/integration/commit_status_test.go new file mode 100644 index 0000000000..924fc5b074 --- /dev/null +++ b/tests/integration/commit_status_test.go @@ -0,0 +1,206 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetLatestCommitStatusForPairs(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("Empty", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, nil) + require.NoError(t, err) + assert.EqualValues(t, map[int64][]*git_model.CommitStatus{}, pairs) + }) + + t.Run("Repo 1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{RepoID: 1, SHA: "1234123412341234123412341234123412341234"}}) + require.NoError(t, err) + + assert.EqualValues(t, map[int64][]*git_model.CommitStatus{ + 1: { + { + ID: 7, + Index: 6, + RepoID: 1, + State: structs.CommitStatusPending, + SHA: "1234123412341234123412341234123412341234", + TargetURL: "https://example.com/builds/", + Description: "My awesome deploy service", + ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", + Context: "deploy/awesomeness", + CreatorID: 2, + }, + { + ID: 4, + Index: 4, + State: structs.CommitStatusFailure, + TargetURL: "https://example.com/builds/", + Description: "My awesome CI-service", + Context: "ci/awesomeness", + CreatorID: 2, + RepoID: 1, + SHA: "1234123412341234123412341234123412341234", + ContextHash: "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", + }, + { + ID: 3, + Index: 3, + State: structs.CommitStatusSuccess, + TargetURL: "https://example.com/coverage/", + Description: "My awesome Coverage service", + Context: "cov/awesomeness", + CreatorID: 2, + RepoID: 1, + SHA: "1234123412341234123412341234123412341234", + ContextHash: "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", + }, + }, + }, pairs) + }) + t.Run("Repo 62", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{RepoID: 62, SHA: "774f93df12d14931ea93259ae93418da4482fcc1"}}) + require.NoError(t, err) + + assert.EqualValues(t, map[int64][]*git_model.CommitStatus{ + 62: { + { + ID: 8, + Index: 2, + RepoID: 62, + State: structs.CommitStatusError, + TargetURL: "/user2/test_workflows/actions", + Description: "My awesome deploy service - v2", + Context: "deploy/awesomeness", + SHA: "774f93df12d14931ea93259ae93418da4482fcc1", + ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", + CreatorID: 2, + }, + }, + }, pairs) + }) + + t.Run("Repo 62 non-existent sha", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{RepoID: 62, SHA: "774f93df12d14931ea93259ae93418da4482fcc"}}) + require.NoError(t, err) + + assert.EqualValues(t, map[int64][]*git_model.CommitStatus{}, pairs) + }) + + t.Run("SHA with non-existent repo id", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, []git_model.RepoSHA{{RepoID: 1, SHA: "774f93df12d14931ea93259ae93418da4482fcc1"}}) + require.NoError(t, err) + + assert.EqualValues(t, map[int64][]*git_model.CommitStatus{}, pairs) + }) +} + +func TestGetLatestCommitStatusForRepoCommitIDs(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("Empty", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 62, nil) + require.NoError(t, err) + assert.EqualValues(t, map[string][]*git_model.CommitStatus{}, repoStatuses) + }) + + t.Run("Repo 1", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 1, []string{"1234123412341234123412341234123412341234"}) + require.NoError(t, err) + assert.EqualValues(t, map[string][]*git_model.CommitStatus{ + "1234123412341234123412341234123412341234": { + { + ID: 3, + Index: 3, + State: structs.CommitStatusSuccess, + TargetURL: "https://example.com/coverage/", + Description: "My awesome Coverage service", + Context: "cov/awesomeness", + CreatorID: 2, + RepoID: 1, + SHA: "1234123412341234123412341234123412341234", + ContextHash: "3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe", + }, + { + ID: 4, + Index: 4, + State: structs.CommitStatusFailure, + TargetURL: "https://example.com/builds/", + Description: "My awesome CI-service", + Context: "ci/awesomeness", + CreatorID: 2, + RepoID: 1, + SHA: "1234123412341234123412341234123412341234", + ContextHash: "c65f4d64a3b14a3eced0c9b36799e66e1bd5ced7", + }, + { + ID: 7, + Index: 6, + RepoID: 1, + State: structs.CommitStatusPending, + SHA: "1234123412341234123412341234123412341234", + TargetURL: "https://example.com/builds/", + Description: "My awesome deploy service", + ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", + Context: "deploy/awesomeness", + CreatorID: 2, + }, + }, + }, repoStatuses) + }) + + t.Run("Repo 62", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 62, []string{"774f93df12d14931ea93259ae93418da4482fcc1"}) + require.NoError(t, err) + assert.EqualValues(t, map[string][]*git_model.CommitStatus{ + "774f93df12d14931ea93259ae93418da4482fcc1": { + { + ID: 8, + Index: 2, + RepoID: 62, + State: structs.CommitStatusError, + TargetURL: "/user2/test_workflows/actions", + Description: "My awesome deploy service - v2", + Context: "deploy/awesomeness", + SHA: "774f93df12d14931ea93259ae93418da4482fcc1", + ContextHash: "ae9547713a6665fc4261d0756904932085a41cf2", + CreatorID: 2, + }, + }, + }, repoStatuses) + }) + + t.Run("Repo 62 non-existent sha", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 62, []string{"774f93df12d14931ea93259ae93418da4482fcc"}) + require.NoError(t, err) + assert.EqualValues(t, map[string][]*git_model.CommitStatus{}, repoStatuses) + }) + + t.Run("non-existent repo ID", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + repoStatuses, err := git_model.GetLatestCommitStatusForRepoCommitIDs(db.DefaultContext, 1, []string{"774f93df12d14931ea93259ae93418da4482fcc"}) + require.NoError(t, err) + assert.EqualValues(t, map[string][]*git_model.CommitStatus{}, repoStatuses) + }) +}