Merge pull request 'Fix unconditional DB queries in commit status fetches' (#6105) from mfenniak/forgejo:fix-6101 into forgejo

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

models/git/commit_status.go

@@ -289,6 +289,12 @@ 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) {
 	results := []*CommitStatus{}
+	repoStatuses := make(map[int64][]*CommitStatus)
+
+	if len(repoSHAs) == 0 {
+		// Avoid performing query when there will be no query conditions added.
+		return repoStatuses, nil
+	}
 
 	// Create a disjunction of conditions for each repoID and SHA pair
 	conds := make([]builder.Cond, 0, len(repoSHAs))
@@ -305,8 +311,6 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map
 		return nil, err
 	}
 
-	repoStatuses := make(map[int64][]*CommitStatus)
-
 	// Group the statuses by repo ID
 	for _, status := range results {
 		repoStatuses[status.RepoID] = append(repoStatuses[status.RepoID], status)
@@ -321,6 +325,12 @@ func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, co
 		Index int64
 		SHA   string
 	}
+	repoStatuses := make(map[string][]*CommitStatus)
+
+	if len(commitIDs) == 0 {
+		// Avoid performing query when there will be no `sha` query conditions added.
+		return repoStatuses, nil
+	}
 
 	getBase := func() *xorm.Session {
 		return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID)
@@ -340,8 +350,6 @@ func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, co
 		return nil, err
 	}
 
-	repoStatuses := make(map[string][]*CommitStatus)
-
 	if len(results) > 0 {
 		statuses := make([]*CommitStatus, 0, len(results))
 

models/git/commit_status_test.go

@@ -273,64 +273,10 @@ func TestCommitStatusesHideActionsURL(t *testing.T) {
 func TestGetLatestCommitStatusForPairs(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())
 
-	t.Run("All", func(t *testing.T) {
+	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)
-		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) {
@@ -414,3 +360,91 @@ func TestGetLatestCommitStatusForPairs(t *testing.T) {
 		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)
+	})
+}