From 40ea22cdfa4ca1ddf9a362ad123d63aa6b810496 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 21 Jul 2023 15:50:14 +0200 Subject: [PATCH 1/4] show manual cron run's last time - Currently in the cron tasks, the 'Previous Time' only displays the previous time of when the cron library executes the function, but not any of the manual executions of the task. - Store the last run's time in memory in the Task struct and use that, when that time is later than time that the cron library has executed this task. - This ensures that if an instance admin manually starts a task, there's feedback that this task is/has been run, because the task might be run that quick, that the status icon already has been changed to an checkmark, - Tasks that are executed at startup now reflect this as well, as the time of the execution of that task on startup is now being shown as 'Previous Time'. - Added integration tests for the API part, which is easier to test because querying the HTML table of cron tasks is non-trivial. - Resolves https://codeberg.org/forgejo/forgejo/issues/949 (cherry picked from commit fd34fdac1408ece6b7d9fe6a76501ed9a45d06fa) --- services/cron/cron.go | 6 ++++ services/cron/tasks.go | 9 +++++ tests/integration/api_admin_test.go | 51 +++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/services/cron/cron.go b/services/cron/cron.go index e3f31d08f0c9..63db75ab3b33 100644 --- a/services/cron/cron.go +++ b/services/cron/cron.go @@ -106,6 +106,12 @@ func ListTasks() TaskTable { next = e.NextRun() prev = e.PreviousRun() } + + // If the manual run is after the cron run, use that instead. + if prev.Before(task.LastRun) { + prev = task.LastRun + } + task.lock.Lock() tTable = append(tTable, &TaskTableRow{ Name: task.Name, diff --git a/services/cron/tasks.go b/services/cron/tasks.go index ea1925c26c73..d2c3d1d812c4 100644 --- a/services/cron/tasks.go +++ b/services/cron/tasks.go @@ -9,6 +9,7 @@ import ( "reflect" "strings" "sync" + "time" "code.gitea.io/gitea/models/db" system_model "code.gitea.io/gitea/models/system" @@ -37,6 +38,8 @@ type Task struct { LastMessage string LastDoer string ExecTimes int64 + // This stores the time of the last manual run of this task. + LastRun time.Time } // DoRunAtStart returns if this task should run at the start @@ -88,6 +91,12 @@ func (t *Task) RunWithUser(doer *user_model.User, config Config) { } }() graceful.GetManager().RunWithShutdownContext(func(baseCtx context.Context) { + // Store the time of this run, before the function is executed, so it + // matches the behavior of what the cron library does. + t.lock.Lock() + t.LastRun = time.Now() + t.lock.Unlock() + pm := process.GetManager() doerName := "" if doer != nil && doer.ID != -1 { diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 6613d4b71598..4e222e22e62f 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "testing" + "time" asymkey_model "code.gitea.io/gitea/models/asymkey" auth_model "code.gitea.io/gitea/models/auth" @@ -282,3 +283,53 @@ func TestAPIRenameUser(t *testing.T) { }) MakeRequest(t, req, http.StatusOK) } + +func TestAPICron(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // user1 is an admin user + session := loginUser(t, "user1") + + t.Run("List", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadAdmin) + urlStr := fmt.Sprintf("/api/v1/admin/cron?token=%s", token) + req := NewRequest(t, "GET", urlStr) + resp := MakeRequest(t, req, http.StatusOK) + + assert.Equal(t, "26", resp.Header().Get("X-Total-Count")) + + var crons []api.Cron + DecodeJSON(t, resp, &crons) + assert.Len(t, crons, 26) + }) + + t.Run("Execute", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + now := time.Now() + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteAdmin) + /// Archive cleanup is harmless, because in the text environment there are none + /// and is thus an NOOP operation and therefore doesn't interfere with any other + /// tests. + urlStr := fmt.Sprintf("/api/v1/admin/cron/archive_cleanup?token=%s", token) + req := NewRequest(t, "POST", urlStr) + MakeRequest(t, req, http.StatusNoContent) + + // Check for the latest run time for this cron, to ensure it + // has been run. + urlStr = fmt.Sprintf("/api/v1/admin/cron?token=%s", token) + req = NewRequest(t, "GET", urlStr) + resp := MakeRequest(t, req, http.StatusOK) + + var crons []api.Cron + DecodeJSON(t, resp, &crons) + + for _, cron := range crons { + if cron.Name == "archive_cleanup" { + assert.True(t, now.Before(cron.Prev)) + } + } + }) +} From c1397b233686fd8f4ed77aa37cab07295a5a4b39 Mon Sep 17 00:00:00 2001 From: Earl Warren <109468362+earl-warren@users.noreply.github.com> Date: Tue, 10 Oct 2023 13:38:45 +0200 Subject: [PATCH 2/4] Update tests/integration/api_admin_test.go Co-authored-by: KN4CK3R --- tests/integration/api_admin_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 4e222e22e62f..2b2ff83305f9 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -310,9 +310,9 @@ func TestAPICron(t *testing.T) { now := time.Now() token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteAdmin) - /// Archive cleanup is harmless, because in the text environment there are none - /// and is thus an NOOP operation and therefore doesn't interfere with any other - /// tests. + // Archive cleanup is harmless, because in the test environment there are none + // and is thus an NOOP operation and therefore doesn't interfere with any other + // tests. urlStr := fmt.Sprintf("/api/v1/admin/cron/archive_cleanup?token=%s", token) req := NewRequest(t, "POST", urlStr) MakeRequest(t, req, http.StatusNoContent) From 6742dcb2313d7259c29f79a265e4b5ae6869c428 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Tue, 10 Oct 2023 13:40:35 +0200 Subject: [PATCH 3/4] count is 28 --- tests/integration/api_admin_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 2b2ff83305f9..f083d4af27ec 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -298,11 +298,11 @@ func TestAPICron(t *testing.T) { req := NewRequest(t, "GET", urlStr) resp := MakeRequest(t, req, http.StatusOK) - assert.Equal(t, "26", resp.Header().Get("X-Total-Count")) + assert.Equal(t, "28", resp.Header().Get("X-Total-Count")) var crons []api.Cron DecodeJSON(t, resp, &crons) - assert.Len(t, crons, 26) + assert.Len(t, crons, 28) }) t.Run("Execute", func(t *testing.T) { From 668d5c2c8fe7b928a744b539bbf63998be86ff1e Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 10 Oct 2023 14:51:37 +0200 Subject: [PATCH 4/4] Update tests/integration/api_admin_test.go --- tests/integration/api_admin_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index f083d4af27ec..aae9ec4a24a2 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -317,8 +317,7 @@ func TestAPICron(t *testing.T) { req := NewRequest(t, "POST", urlStr) MakeRequest(t, req, http.StatusNoContent) - // Check for the latest run time for this cron, to ensure it - // has been run. + // Check for the latest run time for this cron, to ensure it has been run. urlStr = fmt.Sprintf("/api/v1/admin/cron?token=%s", token) req = NewRequest(t, "GET", urlStr) resp := MakeRequest(t, req, http.StatusOK)