Merge branch 'main' into message-blank-line

Author: wxiaoguang

.github/workflows/pull-db-tests.yml

@@ -17,7 +17,7 @@ jobs:
     runs-on: ubuntu-latest
     services:
       pgsql:
-        image: postgres:12
+        image: postgres:14
         env:
           POSTGRES_DB: test
           POSTGRES_PASSWORD: postgres

cmd/hook.go

@@ -24,7 +24,7 @@ import (
 )
 
 const (
-	hookBatchSize = 30
+	hookBatchSize = 500
 )
 
 var (

models/actions/runner.go

@@ -5,6 +5,7 @@ package actions
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -298,6 +299,23 @@ func DeleteRunner(ctx context.Context, id int64) error {
 	return err
 }
 
+// DeleteEphemeralRunner deletes a ephemeral runner by given ID.
+func DeleteEphemeralRunner(ctx context.Context, id int64) error {
+	runner, err := GetRunnerByID(ctx, id)
+	if err != nil {
+		if errors.Is(err, util.ErrNotExist) {
+			return nil
+		}
+		return err
+	}
+	if !runner.Ephemeral {
+		return nil
+	}
+
+	_, err = db.DeleteByID[ActionRunner](ctx, id)
+	return err
+}
+
 // CreateRunner creates new runner.
 func CreateRunner(ctx context.Context, t *ActionRunner) error {
 	if t.OwnerID != 0 && t.RepoID != 0 {

models/actions/task.go

@@ -336,6 +336,11 @@ func UpdateTask(ctx context.Context, task *ActionTask, cols ...string) error {
 		sess.Cols(cols...)
 	}
 	_, err := sess.Update(task)
+
+	// Automatically delete the ephemeral runner if the task is done
+	if err == nil && task.Status.IsDone() && util.SliceContainsString(cols, "status") {
+		return DeleteEphemeralRunner(ctx, task.RunnerID)
+	}
 	return err
 }
 

models/fixtures/action_runner.yml

@@ -38,3 +38,14 @@
   repo_id: 0
   description: "This runner is going to be deleted"
   agent_labels: '["runner_to_be_deleted","linux"]'
+-
+  id: 34350
+  name: runner_to_be_deleted-org-ephemeral
+  uuid: 3FF231BD-FBB7-4E4B-9602-E6F28363EF20
+  token_hash: 3FF231BD-FBB7-4E4B-9602-E6F28363EF20
+  ephemeral: true
+  version: "1.0.0"
+  owner_id: 3
+  repo_id: 0
+  description: "This runner is going to be deleted"
+  agent_labels: '["runner_to_be_deleted","linux"]'

models/fixtures/action_task.yml

@@ -117,6 +117,26 @@
   log_length: 707
   log_size: 90179
   log_expired: 0
+-
+  id: 52
+  job_id: 196
+  attempt: 1
+  runner_id: 34350
+  status: 6 # running
+  started: 1683636528
+  stopped: 1683636626
+  repo_id: 4
+  owner_id: 1
+  commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
+  is_fork_pull_request: 0
+  token_hash: f8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784222
+  token_salt: ffffffffff
+  token_last_eight: ffffffff
+  log_filename: artifact-test2/2f/47.log
+  log_in_storage: 1
+  log_length: 707
+  log_size: 90179
+  log_expired: 0
 -
   id: 53
   job_id: 198

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

models/git/commit_status.go

@@ -230,18 +230,24 @@ func (status *CommitStatus) HideActionsURL(ctx context.Context) {
 
 // CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc
 func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus {
+	// This function is widely used, but it is not quite right.
+	// Ideally it should return something like "CommitStatusSummary" with properly aggregated state.
+	// GitHub's behavior: if all statuses are "skipped", GitHub will return "success" as the combined status.
 	var lastStatus *CommitStatus
 	state := api.CommitStatusSuccess
 	for _, status := range statuses {
-		if status.State.NoBetterThan(state) {
+		if state == status.State || status.State.HasHigherPriorityThan(state) {
 			state = status.State
 			lastStatus = status
 		}
 	}
 	if lastStatus == nil {
 		if len(statuses) > 0 {
+			// FIXME: a bad case: Gitea just returns the first commit status, its status is "skipped" in this case.
 			lastStatus = statuses[0]
 		} else {
+			// FIXME: another bad case: if the "statuses" slice is empty, the returned value is an invalid CommitStatus, all its fields are empty.
+			// Frontend code (tmpl&vue) sometimes depend on the empty fields to skip rendering commit status elements (need to double check in the future)
 			lastStatus = &CommitStatus{}
 		}
 	}
@@ -298,27 +304,37 @@ type CommitStatusIndex struct {
 	MaxIndex int64  `xorm:"index"`
 }
 
+func makeRepoCommitQuery(ctx context.Context, repoID int64, sha string) *xorm.Session {
+	return db.GetEngine(ctx).Table(&CommitStatus{}).
+		Where("repo_id = ?", repoID).And("sha = ?", sha)
+}
+
 // GetLatestCommitStatus returns all statuses with a unique context for a given commit.
-func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
-	getBase := func() *xorm.Session {
-		return db.GetEngine(ctx).Table(&CommitStatus{}).
-			Where("repo_id = ?", repoID).And("sha = ?", sha)
-	}
+func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, error) {
 	indices := make([]int64, 0, 10)
-	sess := getBase().Select("max( `index` ) as `index`").
-		GroupBy("context_hash").OrderBy("max( `index` ) desc")
+	sess := makeRepoCommitQuery(ctx, repoID, sha).
+		Select("max( `index` ) as `index`").
+		GroupBy("context_hash").
+		OrderBy("max( `index` ) desc")
 	if !listOptions.IsListAll() {
 		sess = db.SetSessionPagination(sess, &listOptions)
 	}
-	count, err := sess.FindAndCount(&indices)
-	if err != nil {
-		return nil, count, err
+	if err := sess.Find(&indices); err != nil {
+		return nil, err
 	}
 	statuses := make([]*CommitStatus, 0, len(indices))
 	if len(indices) == 0 {
-		return statuses, count, nil
+		return statuses, nil
 	}
-	return statuses, count, getBase().And(builder.In("`index`", indices)).Find(&statuses)
+	err := makeRepoCommitQuery(ctx, repoID, sha).And(builder.In("`index`", indices)).Find(&statuses)
+	return statuses, err
+}
+
+func CountLatestCommitStatus(ctx context.Context, repoID int64, sha string) (int64, error) {
+	return makeRepoCommitQuery(ctx, repoID, sha).
+		Select("count(context_hash)").
+		GroupBy("context_hash").
+		Count()
 }
 
 // GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs

models/git/commit_status_summary.go

@@ -55,11 +55,15 @@ func GetLatestCommitStatusForRepoAndSHAs(ctx context.Context, repoSHAs []RepoSHA
 }
 
 func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) error {
-	commitStatuses, _, err := GetLatestCommitStatus(ctx, repoID, sha, db.ListOptionsAll)
+	commitStatuses, err := GetLatestCommitStatus(ctx, repoID, sha, db.ListOptionsAll)
 	if err != nil {
 		return err
 	}
-	state := CalcCommitStatus(commitStatuses)
+	// it guarantees that commitStatuses is not empty because this function is always called after a commit status is created
+	if len(commitStatuses) == 0 {
+		setting.PanicInDevOrTesting("no commit statuses found for repo %d and sha %s", repoID, sha)
+	}
+	state := CalcCommitStatus(commitStatuses) // non-empty commitStatuses is guaranteed
 	// mysql will return 0 when update a record which state hasn't been changed which behaviour is different from other database,
 	// so we need to use insert in on duplicate
 	if setting.Database.Type.IsMySQL() {

models/git/commit_status_test.go

@@ -26,7 +26,7 @@ func TestGetCommitStatuses(t *testing.T) {
 
 	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 
-	sha1 := "1234123412341234123412341234123412341234"
+	sha1 := "1234123412341234123412341234123412341234" // the mocked commit ID in test fixtures
 
 	statuses, maxResults, err := db.FindAndCount[git_model.CommitStatus](db.DefaultContext, &git_model.CommitStatusOptions{
 		ListOptions: db.ListOptions{Page: 1, PageSize: 50},
@@ -256,3 +256,26 @@ func TestCommitStatusesHideActionsURL(t *testing.T) {
 	assert.Empty(t, statuses[0].TargetURL)
 	assert.Equal(t, "https://mycicd.org/1", statuses[1].TargetURL)
 }
+
+func TestGetCountLatestCommitStatus(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+
+	sha1 := "1234123412341234123412341234123412341234" // the mocked commit ID in test fixtures
+
+	commitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo1.ID, sha1, db.ListOptions{
+		Page:     1,
+		PageSize: 2,
+	})
+	assert.NoError(t, err)
+	assert.Len(t, commitStatuses, 2)
+	assert.Equal(t, structs.CommitStatusFailure, commitStatuses[0].State)
+	assert.Equal(t, "ci/awesomeness", commitStatuses[0].Context)
+	assert.Equal(t, structs.CommitStatusError, commitStatuses[1].State)
+	assert.Equal(t, "deploy/awesomeness", commitStatuses[1].Context)
+
+	count, err := git_model.CountLatestCommitStatus(db.DefaultContext, repo1.ID, sha1)
+	assert.NoError(t, err)
+	assert.EqualValues(t, 3, count)
+}