Merge pull request 'feat: improve GetLatestCommitStatusForPairs' (#5983) from gusted/improve-commit-pairs into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5983
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>

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/coverage/
   description: My awesome Coverage service
   context: cov/awesomeness
+  context_hash: 3929ac7bccd3fa1bf9b38ddedb77973b1b9a8cfe
   creator_id: 2
 
 -
@@ -29,6 +31,7 @@
   target_url: https://example.com/coverage/
   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,15 +55,41 @@
   target_url: https://example.com/builds/
   description: My awesome deploy service
   context: deploy/awesomeness
+  context_hash: ae9547713a6665fc4261d0756904932085a41cf2
   creator_id: 2
 
 -
   id: 6
-  index: 6
+  index: 1
   repo_id: 62
   state: "failure"
   sha: "774f93df12d14931ea93259ae93418da4482fcc1"
   target_url: "/user2/test_workflows/actions"
   description: My awesome deploy service
   context: deploy/awesomeness
+  context_hash: ae9547713a6665fc4261d0756904932085a41cf2
+  creator_id: 2
+
+-
+  id: 7
+  index: 6
+  repo_id: 1
+  state: "pending"
+  sha: "1234123412341234123412341234123412341234"
+  target_url: https://example.com/builds/
+  description: My awesome deploy service
+  context: deploy/awesomeness
+  context_hash: ae9547713a6665fc4261d0756904932085a41cf2
+  creator_id: 2
+
+-
+  id: 8
+  index: 2
+  repo_id: 62
+  state: "error"
+  sha: "774f93df12d14931ea93259ae93418da4482fcc1"
+  target_url: "/user2/test_workflows/actions"
+  description: "My awesome deploy service - v2"
+  context: deploy/awesomeness
+  context_hash: ae9547713a6665fc4261d0756904932085a41cf2
   creator_id: 2

models/git/commit_status.go

@@ -288,27 +288,18 @@ 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 {
+	results := []*CommitStatus{}
-		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")
 
+	sess := db.GetEngine(ctx).Table(&CommitStatus{}).
+		Select("MAX(`index`) AS `index`, *").
+		Where(builder.Or(conds...)).
+		GroupBy("context_hash, repo_id, sha").OrderBy("MAX(`index`) DESC")
 	err := sess.Find(&results)
 	if err != nil {
 		return nil, err
@@ -316,27 +307,9 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map
 
 	repoStatuses := make(map[int64][]*CommitStatus)
 
-	if len(results) > 0 {
+	// Group the statuses by repo ID
-		statuses := make([]*CommitStatus, 0, len(results))
+	for _, status := range results {
-
+		repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status)
-		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
-		}
-
-		// Group the statuses by repo ID
-		for _, status := range statuses {
-			repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status)
-		}
 	}
 
 	return repoStatuses, nil

models/git/commit_status_test.go

@@ -35,8 +35,8 @@ func TestGetCommitStatuses(t *testing.T) {
 		SHA:         sha1,
 	})
 	require.NoError(t, err)
-	assert.Equal(t, 5, int(maxResults))
+	assert.EqualValues(t, 6, maxResults)
-	assert.Len(t, statuses, 5)
+	assert.Len(t, statuses, 6)
 
 	assert.Equal(t, "ci/awesomeness", statuses[0].Context)
 	assert.Equal(t, structs.CommitStatusPending, statuses[0].State)
@@ -58,13 +58,17 @@ func TestGetCommitStatuses(t *testing.T) {
 	assert.Equal(t, structs.CommitStatusError, statuses[4].State)
 	assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL(db.DefaultContext))
 
+	assert.Equal(t, "deploy/awesomeness", statuses[5].Context)
+	assert.Equal(t, structs.CommitStatusPending, statuses[5].State)
+	assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[5].APIURL(db.DefaultContext))
+
 	statuses, maxResults, err = db.FindAndCount[git_model.CommitStatus](db.DefaultContext, &git_model.CommitStatusOptions{
 		ListOptions: db.ListOptions{Page: 2, PageSize: 50},
 		RepoID:      repo1.ID,
 		SHA:         sha1,
 	})
 	require.NoError(t, err)
-	assert.Equal(t, 5, int(maxResults))
+	assert.EqualValues(t, 6, maxResults)
 	assert.Empty(t, statuses)
 }
 
@@ -265,3 +269,148 @@ 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("All", func(t *testing.T) {
+		pairs, err := git_model.GetLatestCommitStatusForPairs(db.DefaultContext, nil)
+		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",
+				},
+			},
+			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 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 nonexistant 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 existant 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)
+	})
+}