855b9de866
All checks were successful
check / check (push) Successful in 3m4s
RetentionDays carried gorm:"default:30", so GORM substituted 30 for a zero value while building the insert. A webhook could therefore never be configured to keep its events indefinitely: the reaper's retain-forever branch existed but was unreachable from the normal create and edit flows. Introduce database.RetentionForeverDays = 365 * 1000 as the sentinel for "retain forever" and a Webhook.BeforeSave hook that rewrites any non-positive RetentionDays to it. The rewrite has to live in the hook rather than at the call sites: GORM applies the column default while converting the model to insert values, which happens after BeforeSave, so anything later loses that race. Putting it on the model also means a future call site, such as the planned REST API, cannot bypass it. The reaper now skips a webhook when Webhook.RetainsForever reports true, which recognises the sentinel and keeps honouring the old <= 0 values for rows written before it existed. Without this the sentinel, being positive, would have produced a cutoff a thousand years in the past and a DELETE matching nothing on every sweep. Form handling is shared by create and edit through parseRetentionDays so the two cannot drift: an empty field keeps the previous behaviour (default on create, unchanged on edit), 0 is honoured, and an unparseable or negative value is a 400 that re-renders the form rather than a silently substituted default. The retention inputs drop max="365". That cap was not cosmetic: the edit form pre-fills the stored value, so a retain-forever webhook rendered 365000 into an input capped at 365 and browser validation would have blocked saving any edit to it. min becomes 0 with a hint explaining what 0 does, and the list and detail views render a RetentionLabel of "forever" instead of a raw day count. The 30-day default is consolidated into database.DefaultRetentionDays, referenced from the handler and from the create form's pre-filled value, with a test asserting it agrees with the struct tag that cannot reference it.
482 lines
11 KiB
Go
482 lines
11 KiB
Go
package handlers_test
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/go-chi/chi"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"gorm.io/gorm/clause"
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
"sneak.berlin/go/webhooker/internal/handlers"
|
|
"sneak.berlin/go/webhooker/internal/session"
|
|
)
|
|
|
|
const (
|
|
// sourceTestUserID is the session user id used by the webhook
|
|
// management tests.
|
|
sourceTestUserID = "source-test-user"
|
|
// sourceIDParam is the chi URL parameter naming a webhook.
|
|
sourceIDParam = "sourceID"
|
|
)
|
|
|
|
// formRequest builds an urlencoded POST to path carrying the given
|
|
// cookies, plus any chi URL parameters the handler reads.
|
|
func formRequest(
|
|
path string,
|
|
cookies []*http.Cookie,
|
|
form url.Values,
|
|
urlParams map[string]string,
|
|
) *http.Request {
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(),
|
|
http.MethodPost,
|
|
path,
|
|
strings.NewReader(form.Encode()),
|
|
)
|
|
req.Header.Set(
|
|
"Content-Type", "application/x-www-form-urlencoded",
|
|
)
|
|
|
|
for _, c := range cookies {
|
|
req.AddCookie(c)
|
|
}
|
|
|
|
rctx := chi.NewRouteContext()
|
|
for k, v := range urlParams {
|
|
rctx.URLParams.Add(k, v)
|
|
}
|
|
|
|
return req.WithContext(
|
|
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
|
|
)
|
|
}
|
|
|
|
// getRequest builds a GET to path carrying the given cookies, plus any
|
|
// chi URL parameters the handler reads.
|
|
func getRequest(
|
|
t *testing.T,
|
|
path string,
|
|
cookies []*http.Cookie,
|
|
urlParams map[string]string,
|
|
) *http.Request {
|
|
t.Helper()
|
|
|
|
req := httptest.NewRequestWithContext(
|
|
context.Background(), http.MethodGet, path, nil,
|
|
)
|
|
|
|
for _, c := range cookies {
|
|
req.AddCookie(c)
|
|
}
|
|
|
|
rctx := chi.NewRouteContext()
|
|
for k, v := range urlParams {
|
|
rctx.URLParams.Add(k, v)
|
|
}
|
|
|
|
return req.WithContext(
|
|
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
|
|
)
|
|
}
|
|
|
|
// submitCreate posts the webhook creation form with the given
|
|
// retention_days value (omitted entirely when retention is nil) and
|
|
// returns the recorder.
|
|
func submitCreate(
|
|
t *testing.T,
|
|
h *handlers.Handlers,
|
|
cookies []*http.Cookie,
|
|
name string,
|
|
retention *string,
|
|
) *httptest.ResponseRecorder {
|
|
t.Helper()
|
|
|
|
form := url.Values{}
|
|
form.Set("name", name)
|
|
|
|
if retention != nil {
|
|
form.Set("retention_days", *retention)
|
|
}
|
|
|
|
req := formRequest("/sources/new", cookies, form, nil)
|
|
w := httptest.NewRecorder()
|
|
|
|
h.HandleSourceCreateSubmit().ServeHTTP(w, req)
|
|
|
|
return w
|
|
}
|
|
|
|
// onlyWebhook loads the single webhook belonging to the test user.
|
|
func onlyWebhook(
|
|
t *testing.T,
|
|
db *database.Database,
|
|
) database.Webhook {
|
|
t.Helper()
|
|
|
|
var webhooks []database.Webhook
|
|
|
|
require.NoError(
|
|
t,
|
|
db.DB().Where("user_id = ?", sourceTestUserID).
|
|
Find(&webhooks).Error,
|
|
)
|
|
require.Len(t, webhooks, 1)
|
|
|
|
return webhooks[0]
|
|
}
|
|
|
|
// seedWebhook inserts a webhook owned by the test user with an exact
|
|
// stored retention value, bypassing Webhook.BeforeSave via a
|
|
// column-level update so that legacy rows can be planted too.
|
|
func seedWebhook(
|
|
t *testing.T,
|
|
db *database.Database,
|
|
retentionDays int,
|
|
) database.Webhook {
|
|
t.Helper()
|
|
|
|
wh := &database.Webhook{
|
|
UserID: sourceTestUserID,
|
|
Name: "seeded",
|
|
RetentionDays: retentionDays,
|
|
}
|
|
require.NoError(
|
|
t,
|
|
db.DB().Omit(clause.Associations).Create(wh).Error,
|
|
)
|
|
require.NoError(
|
|
t,
|
|
db.DB().Model(wh).
|
|
Update("retention_days", retentionDays).Error,
|
|
)
|
|
|
|
wh.RetentionDays = retentionDays
|
|
|
|
return *wh
|
|
}
|
|
|
|
// storedRetentionDays reads the retention_days column for a webhook.
|
|
func storedRetentionDays(
|
|
t *testing.T,
|
|
db *database.Database,
|
|
id string,
|
|
) int {
|
|
t.Helper()
|
|
|
|
var got int
|
|
|
|
require.NoError(
|
|
t,
|
|
db.DB().Model(&database.Webhook{}).
|
|
Where("id = ?", id).
|
|
Pluck("retention_days", &got).Error,
|
|
)
|
|
|
|
return got
|
|
}
|
|
|
|
// sourceTestEnv bundles the handler, session, and database a webhook
|
|
// management test drives.
|
|
type sourceTestEnv struct {
|
|
handlers *handlers.Handlers
|
|
db *database.Database
|
|
cookies []*http.Cookie
|
|
}
|
|
|
|
func setupSourceTest(t *testing.T) *sourceTestEnv {
|
|
t.Helper()
|
|
|
|
var h *handlers.Handlers
|
|
|
|
var sess *session.Session
|
|
|
|
var db *database.Database
|
|
|
|
app := newTestApp(t, &h, &sess, &db)
|
|
app.RequireStart()
|
|
|
|
t.Cleanup(app.RequireStop)
|
|
|
|
return &sourceTestEnv{
|
|
handlers: h,
|
|
db: db,
|
|
cookies: authenticatedCookies(
|
|
t, sess, sourceTestUserID, "sourceuser",
|
|
),
|
|
}
|
|
}
|
|
|
|
// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever is the core
|
|
// regression test for the bug: the create form's 0 must reach the
|
|
// database as the retain-forever sentinel rather than being replaced by
|
|
// the column's default of 30.
|
|
func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
zero := "0"
|
|
|
|
w := submitCreate(t, env.handlers, env.cookies, "forever", &zero)
|
|
require.Equal(t, http.StatusSeeOther, w.Code)
|
|
|
|
wh := onlyWebhook(t, env.db)
|
|
assert.Equal(
|
|
t,
|
|
database.RetentionForeverDays,
|
|
storedRetentionDays(t, env.db, wh.ID),
|
|
)
|
|
assert.True(t, wh.RetainsForever())
|
|
}
|
|
|
|
func TestHandleSourceCreateSubmit_OmittedRetentionUsesDefault(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
w := submitCreate(t, env.handlers, env.cookies, "defaulted", nil)
|
|
require.Equal(t, http.StatusSeeOther, w.Code)
|
|
|
|
wh := onlyWebhook(t, env.db)
|
|
assert.Equal(
|
|
t,
|
|
database.DefaultRetentionDays,
|
|
storedRetentionDays(t, env.db, wh.ID),
|
|
)
|
|
}
|
|
|
|
// TestHandleSourceCreate_PrefillsDefaultFromConstant keeps the create
|
|
// form's pre-filled retention from becoming a third hardcoded copy of
|
|
// the 30-day policy.
|
|
func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
w := httptest.NewRecorder()
|
|
env.handlers.HandleSourceCreate().ServeHTTP(
|
|
w, getRequest(t, "/sources/new", env.cookies, nil),
|
|
)
|
|
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
|
|
body := w.Body.String()
|
|
|
|
assert.Contains(
|
|
t, body,
|
|
`value="`+strconv.Itoa(database.DefaultRetentionDays)+`"`,
|
|
)
|
|
assert.NotContains(
|
|
t, body, `max="365"`,
|
|
"a max below the sentinel would block retain-forever",
|
|
)
|
|
assert.Contains(t, body, `min="0"`)
|
|
}
|
|
|
|
func TestHandleSourceCreateSubmit_InvalidRetentionIsRejected(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
for _, raw := range []string{"abc", "-1", "3.5"} {
|
|
t.Run(raw, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
|
|
w := submitCreate(
|
|
t, env.handlers, env.cookies, "bad", &raw,
|
|
)
|
|
|
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
|
assert.Contains(
|
|
t, w.Body.String(), "Retention must be",
|
|
)
|
|
|
|
var count int64
|
|
|
|
require.NoError(
|
|
t,
|
|
env.db.DB().Model(&database.Webhook{}).
|
|
Where("user_id = ?", sourceTestUserID).
|
|
Count(&count).Error,
|
|
)
|
|
assert.Zero(
|
|
t, count,
|
|
"no webhook may be created from a rejected form",
|
|
)
|
|
})
|
|
}
|
|
}
|
|
|
|
// submitEdit posts the webhook edit form for the given webhook.
|
|
func submitEdit(
|
|
t *testing.T,
|
|
env *sourceTestEnv,
|
|
wh database.Webhook,
|
|
retention string,
|
|
) *httptest.ResponseRecorder {
|
|
t.Helper()
|
|
|
|
form := url.Values{}
|
|
form.Set("name", wh.Name)
|
|
form.Set("description", wh.Description)
|
|
form.Set("retention_days", retention)
|
|
|
|
req := formRequest(
|
|
"/source/"+wh.ID+"/edit",
|
|
env.cookies,
|
|
form,
|
|
map[string]string{sourceIDParam: wh.ID},
|
|
)
|
|
w := httptest.NewRecorder()
|
|
|
|
env.handlers.HandleSourceEditSubmit().ServeHTTP(w, req)
|
|
|
|
return w
|
|
}
|
|
|
|
func TestHandleSourceEditSubmit_ZeroRetentionPersistsForever(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
|
|
|
|
w := submitEdit(t, env, wh, "0")
|
|
require.Equal(t, http.StatusSeeOther, w.Code)
|
|
|
|
assert.Equal(
|
|
t,
|
|
database.RetentionForeverDays,
|
|
storedRetentionDays(t, env.db, wh.ID),
|
|
)
|
|
}
|
|
|
|
func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
wh := seedWebhook(t, env.db, database.DefaultRetentionDays)
|
|
|
|
w := submitEdit(t, env, wh, "not-a-number")
|
|
|
|
assert.Equal(t, http.StatusBadRequest, w.Code)
|
|
assert.Contains(t, w.Body.String(), "Retention must be")
|
|
assert.Equal(
|
|
t,
|
|
database.DefaultRetentionDays,
|
|
storedRetentionDays(t, env.db, wh.ID),
|
|
"a rejected form must not change the stored retention",
|
|
)
|
|
}
|
|
|
|
func TestHandleSourceEditSubmit_EmptyRetentionLeavesValueUnchanged(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
wh := seedWebhook(t, env.db, 7)
|
|
|
|
w := submitEdit(t, env, wh, "")
|
|
require.Equal(t, http.StatusSeeOther, w.Code)
|
|
|
|
assert.Equal(t, 7, storedRetentionDays(t, env.db, wh.ID))
|
|
}
|
|
|
|
// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
|
|
// the removed max="365" cap used to break: render the edit form for a
|
|
// retain-forever webhook, confirm the pre-filled sentinel is not capped
|
|
// by browser validation, then submit that pre-filled value straight
|
|
// back and confirm the retention policy survives untouched.
|
|
func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
wh := seedWebhook(t, env.db, database.RetentionForeverDays)
|
|
|
|
req := getRequest(
|
|
t, "/source/"+wh.ID+"/edit", env.cookies,
|
|
map[string]string{sourceIDParam: wh.ID},
|
|
)
|
|
w := httptest.NewRecorder()
|
|
env.handlers.HandleSourceEdit().ServeHTTP(w, req)
|
|
|
|
require.Equal(t, http.StatusOK, w.Code)
|
|
|
|
sentinel := strconv.Itoa(database.RetentionForeverDays)
|
|
body := w.Body.String()
|
|
|
|
assert.Contains(
|
|
t, body, `value="`+sentinel+`"`,
|
|
"the edit form pre-fills the stored retention",
|
|
)
|
|
assert.NotContains(
|
|
t, body, `max="365"`,
|
|
"a max below the sentinel would block saving any edit",
|
|
)
|
|
assert.Contains(
|
|
t, body, "forever",
|
|
"the form explains what the sentinel means",
|
|
)
|
|
|
|
// Submit the pre-filled value back, exactly as a browser would.
|
|
post := submitEdit(t, env, wh, sentinel)
|
|
require.Equal(t, http.StatusSeeOther, post.Code)
|
|
|
|
assert.Equal(
|
|
t,
|
|
database.RetentionForeverDays,
|
|
storedRetentionDays(t, env.db, wh.ID),
|
|
)
|
|
}
|
|
|
|
// TestSourceListAndDetail_ShowForeverNotTheSentinelNumber checks that
|
|
// the retain-forever value is never rendered to the user as a raw day
|
|
// count on either read-only view.
|
|
func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
|
|
t *testing.T,
|
|
) {
|
|
t.Parallel()
|
|
|
|
env := setupSourceTest(t)
|
|
wh := seedWebhook(t, env.db, database.RetentionForeverDays)
|
|
sentinel := strconv.Itoa(database.RetentionForeverDays)
|
|
|
|
listW := httptest.NewRecorder()
|
|
env.handlers.HandleSourceList().ServeHTTP(
|
|
listW, getRequest(t, "/sources", env.cookies, nil),
|
|
)
|
|
|
|
require.Equal(t, http.StatusOK, listW.Code)
|
|
assert.Contains(t, listW.Body.String(), "Retention: forever")
|
|
assert.NotContains(t, listW.Body.String(), sentinel)
|
|
|
|
detailW := httptest.NewRecorder()
|
|
env.handlers.HandleSourceDetail().ServeHTTP(
|
|
detailW,
|
|
getRequest(
|
|
t, "/source/"+wh.ID, env.cookies,
|
|
map[string]string{sourceIDParam: wh.ID},
|
|
),
|
|
)
|
|
|
|
require.Equal(t, http.StatusOK, detailW.Code)
|
|
assert.Contains(t, detailW.Body.String(), "Retention: forever")
|
|
assert.NotContains(t, detailW.Body.String(), sentinel)
|
|
}
|