Merge pull request 'Fix incorrect webhook time and use relative-time to display it (#24477)' (#946) from earl-warren/forgejo:wip-v1.19-webhook into v1.19/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/946

Makefile

@@ -784,7 +784,7 @@ merge-locales:
 
 .PHONY: security-check
 security-check:
-	go run $(GOVULNCHECK_PACKAGE) -v ./...
+	go run $(GOVULNCHECK_PACKAGE) ./...
 
 $(EXECUTABLE): $(GO_SOURCES) $(TAGS_PREREQ)
 	CGO_CFLAGS="$(CGO_CFLAGS)" $(GO) build $(GOFLAGS) $(EXTRA_GOFLAGS) -tags '$(TAGS)' -ldflags '-s -w $(LDFLAGS)' -o $@

models/webhook/hooktask.go

@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/timeutil"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
 	gouuid "github.com/google/uuid"
@@ -40,15 +41,14 @@ type HookResponse struct {
 
 // HookTask represents a hook task.
 type HookTask struct {
-	ID              int64  `xorm:"pk autoincr"`
+	ID             int64  `xorm:"pk autoincr"`
-	HookID          int64  `xorm:"index"`
+	HookID         int64  `xorm:"index"`
-	UUID            string `xorm:"unique"`
+	UUID           string `xorm:"unique"`
-	api.Payloader   `xorm:"-"`
+	api.Payloader  `xorm:"-"`
-	PayloadContent  string `xorm:"LONGTEXT"`
+	PayloadContent string `xorm:"LONGTEXT"`
-	EventType       webhook_module.HookEventType
+	EventType      webhook_module.HookEventType
-	IsDelivered     bool
+	IsDelivered    bool
-	Delivered       int64
+	Delivered      timeutil.TimeStampNano
-	DeliveredString string `xorm:"-"`
 
 	// History info.
 	IsSucceed       bool
@@ -75,8 +75,6 @@ func (t *HookTask) BeforeUpdate() {
 
 // AfterLoad updates the webhook object upon setting a column
 func (t *HookTask) AfterLoad() {
-	t.DeliveredString = time.Unix(0, t.Delivered).Format("2006-01-02 15:04:05 MST")
-
 	if len(t.RequestContent) == 0 {
 		return
 	}
@@ -115,12 +113,17 @@ func HookTasks(hookID int64, page int) ([]*HookTask, error) {
 // CreateHookTask creates a new hook task,
 // it handles conversion from Payload to PayloadContent.
 func CreateHookTask(ctx context.Context, t *HookTask) (*HookTask, error) {
-	data, err := t.Payloader.JSONPayload()
-	if err != nil {
-		return nil, err
-	}
 	t.UUID = gouuid.New().String()
-	t.PayloadContent = string(data)
+	if t.Payloader != nil {
+		data, err := t.Payloader.JSONPayload()
+		if err != nil {
+			return nil, err
+		}
+		t.PayloadContent = string(data)
+	}
+	if t.Delivered == 0 {
+		t.Delivered = timeutil.TimeStampNanoNow()
+	}
 	return t, db.Insert(ctx, t)
 }
 
@@ -161,13 +164,11 @@ func ReplayHookTask(ctx context.Context, hookID int64, uuid string) (*HookTask,
 		}
 	}
 
-	newTask := &HookTask{
+	return CreateHookTask(ctx, &HookTask{
-		UUID:           gouuid.New().String(),
 		HookID:         task.HookID,
 		PayloadContent: task.PayloadContent,
 		EventType:      task.EventType,
-	}
+	})
-	return newTask, db.Insert(ctx, newTask)
 }
 
 // FindUndeliveredHookTaskIDs will find the next 100 undelivered hook tasks with ID greater than the provided lowerID

models/webhook/webhook_test.go

@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/json"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
@@ -222,7 +223,6 @@ func TestUpdateHookTask(t *testing.T) {
 
 	hook := unittest.AssertExistsAndLoadBean(t, &HookTask{ID: 1})
 	hook.PayloadContent = "new payload content"
-	hook.DeliveredString = "new delivered string"
 	hook.IsDelivered = true
 	unittest.AssertNotExistsBean(t, hook)
 	assert.NoError(t, UpdateHookTask(hook))
@@ -235,7 +235,7 @@ func TestCleanupHookTaskTable_PerWebhook_DeletesDelivered(t *testing.T) {
 		HookID:      3,
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
-		Delivered:   time.Now().UnixNano(),
+		Delivered:   timeutil.TimeStampNanoNow(),
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -268,7 +268,7 @@ func TestCleanupHookTaskTable_PerWebhook_LeavesMostRecentTask(t *testing.T) {
 		HookID:      4,
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
-		Delivered:   time.Now().UnixNano(),
+		Delivered:   timeutil.TimeStampNanoNow(),
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -285,7 +285,7 @@ func TestCleanupHookTaskTable_OlderThan_DeletesDelivered(t *testing.T) {
 		HookID:      3,
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
-		Delivered:   time.Now().AddDate(0, 0, -8).UnixNano(),
+		Delivered:   timeutil.TimeStampNano(time.Now().AddDate(0, 0, -8).UnixNano()),
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)
@@ -318,7 +318,7 @@ func TestCleanupHookTaskTable_OlderThan_LeavesTaskEarlierThanAgeToDelete(t *test
 		HookID:      4,
 		Payloader:   &api.PushPayload{},
 		IsDelivered: true,
-		Delivered:   time.Now().AddDate(0, 0, -6).UnixNano(),
+		Delivered:   timeutil.TimeStampNano(time.Now().AddDate(0, 0, -6).UnixNano()),
 	}
 	unittest.AssertNotExistsBean(t, hookTask)
 	_, err := CreateHookTask(db.DefaultContext, hookTask)

modules/timeutil/timestampnano.go

@@ -0,0 +1,28 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package timeutil
+
+import (
+	"time"
+
+	"code.gitea.io/gitea/modules/setting"
+)
+
+// TimeStampNano is for nano time in database, do not use it unless there is a real requirement.
+type TimeStampNano int64
+
+// TimeStampNanoNow returns now nano int64
+func TimeStampNanoNow() TimeStampNano {
+	return TimeStampNano(time.Now().UnixNano())
+}
+
+// AsTime convert timestamp as time.Time in Local locale
+func (tsn TimeStampNano) AsTime() (tm time.Time) {
+	return tsn.AsTimeInLocation(setting.DefaultUILocation)
+}
+
+// AsTimeInLocation convert timestamp as time.Time in Local locale
+func (tsn TimeStampNano) AsTimeInLocation(loc *time.Location) time.Time {
+	return time.Unix(0, int64(tsn)).In(loc)
+}

services/webhook/deliver.go

@@ -26,6 +26,7 @@ import (
 	"code.gitea.io/gitea/modules/proxy"
 	"code.gitea.io/gitea/modules/queue"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/timeutil"
 	webhook_module "code.gitea.io/gitea/modules/webhook"
 
 	"github.com/gobwas/glob"
@@ -179,7 +180,7 @@ func Deliver(ctx context.Context, t *webhook_model.HookTask) error {
 
 	// All code from this point will update the hook task
 	defer func() {
-		t.Delivered = time.Now().UnixNano()
+		t.Delivered = timeutil.TimeStampNanoNow()
 		if t.IsSucceed {
 			log.Trace("Hook delivered: %s", t.UUID)
 		} else if !w.IsActive {

templates/repo/settings/webhook/history.tmpl

@@ -21,7 +21,7 @@
 						<a class="ui primary sha label toggle button" data-target="#info-{{.ID}}">{{.UUID}}</a>
 						<div class="ui right">
 							<span class="text grey time">
-								{{.DeliveredString}}
+								{{TimeSince .Delivered.AsTime $.locale}}
 							</span>
 						</div>
 					</div>