show manual cron run's last time (#27544) (#27577)

Backport #27544 by @earl-warren

- 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)

Co-authored-by: Earl Warren <109468362+earl-warren@users.noreply.github.com>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: silverwind <me@silverwind.io>

diff --git a/services/cron/cron.go b/services/cron/cron.go
index e3f31d08f0c9029a56017aea3a69b7f00070554b..63db75ab3b335dcf255061624e1bc867b5872fbd 100644 (file)
--- 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 ea1925c26c73851576cf18b4804a99ffe0f7bd64..d2c3d1d812c427648122d1442ff8c2db8256e814 100644 (file)
--- 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 6613d4b71598382ab8c772b481e8fa02c843dc4f..aae9ec4a24a2da6da1ab6615cacc841d4713bb8d 100644 (file)
--- 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,52 @@ 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, "28", resp.Header().Get("X-Total-Count"))
+
+               var crons []api.Cron
+               DecodeJSON(t, resp, &crons)
+               assert.Len(t, crons, 28)
+       })
+
+       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 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)
+
+               // 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))
+                       }
+               }
+       })
+}