From 2d0ff00823bf92ee32df107416fd30010638f21d Mon Sep 17 00:00:00 2001
From: Jason Song <i@wolfogre.com>
Date: Wed, 10 May 2023 19:54:18 +0800
Subject: [PATCH] Improve updating Actions tasks (#24600)

Co-authored-by: Giteabot <teabot@gitea.io>
---
 models/actions/task.go               | 17 +++++++++++------
 routers/api/actions/runner/runner.go | 23 +----------------------
 2 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/models/actions/task.go b/models/actions/task.go
index ffec4c92aa..66cd2bbea1 100644
--- a/models/actions/task.go
+++ b/models/actions/task.go
@@ -291,7 +291,7 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask
 	}
 
 	task.LogFilename = logFileName(job.Run.Repo.FullName(), task.ID)
-	if _, err := e.ID(task.ID).Cols("log_filename").Update(task); err != nil {
+	if err := UpdateTask(ctx, task, "log_filename"); err != nil {
 		return nil, false, err
 	}
 
@@ -367,9 +367,18 @@ func UpdateTaskByState(ctx context.Context, state *runnerv1.TaskState) (*ActionT
 		return nil, util.ErrNotExist
 	}
 
+	if task.Status.IsDone() {
+		// the state is final, do nothing
+		return task, nil
+	}
+
+	// state.Result is not unspecified means the task is finished
 	if state.Result != runnerv1.Result_RESULT_UNSPECIFIED {
 		task.Status = Status(state.Result)
 		task.Stopped = timeutil.TimeStamp(state.StoppedAt.AsTime().Unix())
+		if err := UpdateTask(ctx, task, "status", "stopped"); err != nil {
+			return nil, err
+		}
 		if _, err := UpdateRunJob(ctx, &ActionRunJob{
 			ID:      task.JobID,
 			Status:  task.Status,
@@ -379,10 +388,6 @@ func UpdateTaskByState(ctx context.Context, state *runnerv1.TaskState) (*ActionT
 		}
 	}
 
-	if _, err := e.ID(task.ID).Update(task); err != nil {
-		return nil, err
-	}
-
 	if err := task.LoadAttributes(ctx); err != nil {
 		return nil, err
 	}
@@ -440,7 +445,7 @@ func StopTask(ctx context.Context, taskID int64, status Status) error {
 		return err
 	}
 
-	if _, err := e.ID(task.ID).Update(task); err != nil {
+	if err := UpdateTask(ctx, task, "status", "stopped"); err != nil {
 		return err
 	}
 
diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go
index 73c6b746a0..895b281725 100644
--- a/routers/api/actions/runner/runner.go
+++ b/routers/api/actions/runner/runner.go
@@ -10,7 +10,6 @@ import (
 
 	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/modules/actions"
-	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/util"
 	actions_service "code.gitea.io/gitea/services/actions"
@@ -120,27 +119,7 @@ func (s *Service) UpdateTask(
 	ctx context.Context,
 	req *connect.Request[runnerv1.UpdateTaskRequest],
 ) (*connect.Response[runnerv1.UpdateTaskResponse], error) {
-	{
-		// to debug strange runner behaviors, it could be removed if all problems have been solved.
-		stateMsg, _ := json.Marshal(req.Msg.State)
-		log.Trace("update task with state: %s", stateMsg)
-	}
-
-	// Get Task first
-	task, err := actions_model.GetTaskByID(ctx, req.Msg.State.Id)
-	if err != nil {
-		return nil, status.Errorf(codes.Internal, "can't find the task: %v", err)
-	}
-	if task.Status.IsCancelled() {
-		return connect.NewResponse(&runnerv1.UpdateTaskResponse{
-			State: &runnerv1.TaskState{
-				Id:     req.Msg.State.Id,
-				Result: task.Status.AsResult(),
-			},
-		}), nil
-	}
-
-	task, err = actions_model.UpdateTaskByState(ctx, req.Msg.State)
+	task, err := actions_model.UpdateTaskByState(ctx, req.Msg.State)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "update task: %v", err)
 	}