diff --git a/README.md b/README.md index e886758..f0f9b92 100644 --- a/README.md +++ b/README.md @@ -1719,8 +1719,11 @@ events should be forwarded. own archive database (`archive-{webhook_name}-{target_name}-{target_uuid}.db`) for long-term retention, with an optional creation-validated expiry (default: keep - forever). No external delivery and no retries; an archive write - failure fails the delivery. See the database target section under + forever). The new webhook form, the add target form and the target edit + form all offer the same expiries: never, 1h, 12h, 24h, 30d, 90d or 365d. + The target list shows the expiry in plain units, such as "30 days". No + external delivery and no retries; an archive write failure fails the + delivery. See the database target section under "Per-Webhook Event Databases" for the full semantics. - **`log`** — Write the event to the application log (stdout). Useful for debugging. diff --git a/internal/delivery/target_config_view.go b/internal/delivery/target_config_view.go index 1053eac..d6e3a84 100644 --- a/internal/delivery/target_config_view.go +++ b/internal/delivery/target_config_view.go @@ -1,9 +1,9 @@ package delivery import ( - "encoding/json" "fmt" "strconv" + "time" "sneak.berlin/go/webhooker/internal/database" ) @@ -197,37 +197,61 @@ func maxRetriesField(t *database.Target) ConfigField { } } -// databaseConfigFields describes an archive target. Its -// configuration is optional, and an absent or empty expiry -// means the archive is kept forever. An expiry that is set -// but not a valid duration is reported as unavailable rather -// than echoed back. +// databaseConfigFields describes an archive target by its +// expiry in plain units, such as "30 days", or "never" when +// the archive is kept forever. An expiry that is set but not +// a valid duration is reported as unavailable rather than +// echoed back. func databaseConfigFields(configJSON string) []ConfigField { - expiry := archiveExpiryNever + expiry, err := parseArchiveExpiry(configJSON) + if err != nil { + return unavailableConfigFields() + } - if configJSON != "" { - var cfg databaseTargetConfig - - err := json.Unmarshal([]byte(configJSON), &cfg) - if err != nil { - return unavailableConfigFields() - } - - if cfg.Expiry != "" { - if ValidateArchiveExpiry(cfg.Expiry) != nil { - return unavailableConfigFields() - } - - expiry = cfg.Expiry - } + value := archiveExpiryNever + if expiry > 0 { + value = plainDuration(expiry) } return []ConfigField{{ Label: "Archive Expiry", - Value: expiry, + Value: value, }} } +// plainDuration writes a positive duration as a count of the +// largest whole unit it divides into: "30 days", "12 hours", +// "1 minute". A duration with a fraction of a second is +// written as Go writes it. +func plainDuration(d time.Duration) string { + const day = 24 * time.Hour + + units := []struct { + size time.Duration + name string + }{ + {day, "day"}, + {time.Hour, "hour"}, + {time.Minute, "minute"}, + {time.Second, "second"}, + } + + for _, unit := range units { + if d%unit.size != 0 { + continue + } + + count := int64(d / unit.size) + if count == 1 { + return "1 " + unit.name + } + + return fmt.Sprintf("%d %ss", count, unit.name) + } + + return d.String() +} + // MaskedWebhookURL returns the Slack webhook URL reduced to // its scheme and host, with the path, query and any userinfo // elided. The path segments are the credential, so none of diff --git a/internal/delivery/target_config_view_test.go b/internal/delivery/target_config_view_test.go index c9e58f5..f763d86 100644 --- a/internal/delivery/target_config_view_test.go +++ b/internal/delivery/target_config_view_test.go @@ -305,14 +305,20 @@ func TestNewTargetViews_Database(t *testing.T) { }{ "empty config": {config: "", want: viewExpiryNever}, "empty expiry": {config: `{}`, want: viewExpiryNever}, - "explicit": { - config: `{"expiry":"720h"}`, - want: "720h", - }, "never literal": { config: `{"expiry":"` + viewExpiryNever + `"}`, want: viewExpiryNever, }, + "1h": {config: `{"expiry":"1h"}`, want: "1 hour"}, + "12h": {config: `{"expiry":"12h"}`, want: "12 hours"}, + "24h": {config: `{"expiry":"24h"}`, want: "1 day"}, + "720h": {config: `{"expiry":"720h"}`, want: "30 days"}, + "2160h": {config: `{"expiry":"2160h"}`, want: "90 days"}, + "8760h": {config: `{"expiry":"8760h"}`, want: "365 days"}, + "36h": {config: `{"expiry":"36h"}`, want: "36 hours"}, + "1h30m": {config: `{"expiry":"1h30m"}`, want: "90 minutes"}, + "45s": {config: `{"expiry":"45s"}`, want: "45 seconds"}, + "1.5s": {config: `{"expiry":"1.5s"}`, want: "1.5s"}, } for name, tc := range tests { diff --git a/internal/handlers/archive_expiry.go b/internal/handlers/archive_expiry.go new file mode 100644 index 0000000..060737d --- /dev/null +++ b/internal/handlers/archive_expiry.go @@ -0,0 +1,58 @@ +package handlers + +const ( + // archiveExpiryNever is the archive expiry that keeps archived + // events forever. A stored empty expiry means the same. + archiveExpiryNever = "never" + + // tmplKeyArchiveExpiryChoices is the template data key for the + // entries of a page's archive expiry select. + tmplKeyArchiveExpiryChoices = "ArchiveExpiryChoices" +) + +// archiveExpiryChoice is one entry of a database target's archive +// expiry select: the expiry stored, the label shown, and whether the +// select starts on it. +type archiveExpiryChoice struct { + Value string + Label string + Selected bool +} + +// archiveExpiryChoices lists the archive expiries offered by the new +// webhook page, the add target form and the target edit form. +func archiveExpiryChoices() []archiveExpiryChoice { + return []archiveExpiryChoice{ + {Value: archiveExpiryNever, Label: archiveExpiryNever}, + {Value: "1h", Label: "1h"}, + {Value: "12h", Label: "12h"}, + {Value: "24h", Label: "24h"}, + {Value: "720h", Label: "30d"}, + {Value: "2160h", Label: "90d"}, + {Value: "8760h", Label: "365d"}, + } +} + +// archiveExpiryOptions returns the choices with expiry selected; an +// empty expiry selects never. An expiry that is not one of the +// choices comes first as its own selected entry, so saving the form +// unchanged keeps it. +func archiveExpiryOptions(expiry string) []archiveExpiryChoice { + if expiry == "" { + expiry = archiveExpiryNever + } + + options := archiveExpiryChoices() + + for i := range options { + if options[i].Value == expiry { + options[i].Selected = true + + return options + } + } + + own := archiveExpiryChoice{Value: expiry, Label: expiry, Selected: true} + + return append([]archiveExpiryChoice{own}, options...) +} diff --git a/internal/handlers/archive_expiry_test.go b/internal/handlers/archive_expiry_test.go new file mode 100644 index 0000000..70bf476 --- /dev/null +++ b/internal/handlers/archive_expiry_test.go @@ -0,0 +1,166 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/database" +) + +// expiryNever is the archive expiry that keeps archived events +// forever. +const expiryNever = "never" + +// matched returns what the one group of pattern matched in page, at +// each match. +func matched(pattern, page string) []string { + matches := regexp.MustCompile(pattern).FindAllStringSubmatch(page, -1) + groups := make([]string, 0, len(matches)) + + for _, m := range matches { + groups = append(groups, m[1]) + } + + return groups +} + +// expiryShown returns the archive expiries the webhook page's target +// list shows. +func expiryShown( + t *testing.T, env *sourceTestEnv, webhookID string, +) []string { + t.Helper() + + w := httptest.NewRecorder() + env.handlers.HandleSourceDetail().ServeHTTP(w, getRequest( + t, "/hook/"+webhookID, env.cookies, + map[string]string{sourceIDParam: webhookID}, + )) + require.Equal(t, http.StatusOK, w.Code) + + return matched( + `Archive Expiry:\s*([^<]*)`, w.Body.String(), + ) +} + +// expirySelected returns the target edit page and the expiries its +// select starts on. +func expirySelected( + t *testing.T, env *sourceTestEnv, webhookID, targetID string, +) (string, []string) { + t.Helper() + + w := serveTarget( + env, http.MethodGet, + "/hook/"+webhookID+"/targets/"+targetID+"/edit", nil, + ) + require.Equal(t, http.StatusOK, w.Code) + + page := w.Body.String() + + return page, matched(``, page) +} + +// TestArchiveExpiryChoices adds a database target with each archive +// expiry the forms offer, and checks that it is stored as chosen, +// shown in plain units in the target list, and that the target edit +// form starts on it. +func TestArchiveExpiryChoices(t *testing.T) { + t.Parallel() + + env := setupSourceTest(t) + + choices := []struct{ value, shown string }{ + {expiryNever, expiryNever}, + {"1h", "1 hour"}, + {"12h", "12 hours"}, + {"24h", "1 day"}, + {"720h", "30 days"}, + {"2160h", "90 days"}, + {"8760h", "365 days"}, + } + + for _, choice := range choices { + t.Run(choice.value, func(t *testing.T) { + t.Parallel() + + webhook := seedWebhookWithRetention(t, env.db, 30) + + form := url.Values{} + form.Set("name", "archive") + form.Set("type", string(database.TargetTypeDatabase)) + form.Set("expiry", choice.value) + + w := serveTarget( + env, http.MethodPost, "/hook/"+webhook.ID+"/targets", form, + ) + require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String()) + + targets := targetsForWebhook(t, env.db, webhook.ID) + require.Len(t, targets, 1) + assert.JSONEq( + t, `{"expiry":"`+choice.value+`"}`, targets[0].Config, + ) + + assert.Equal( + t, []string{choice.shown}, + expiryShown(t, env, webhook.ID), + ) + + _, selected := expirySelected(t, env, webhook.ID, targets[0].ID) + assert.Equal(t, []string{choice.value}, selected) + }) + } +} + +// TestArchiveExpiryEditStartsOnStoredValue checks the edit form of a +// database target whose stored expiry is empty, which selects never, +// and of one whose expiry is not one of the choices, which is listed +// first as its own selected entry and saved unchanged. +func TestArchiveExpiryEditStartsOnStoredValue(t *testing.T) { + t.Parallel() + + env := setupSourceTest(t) + + webhook := seedWebhookWithRetention(t, env.db, 30) + empty := seedConfiguredTarget( + t, env.db, webhook.ID, database.TargetTypeDatabase, "", + ) + + _, selected := expirySelected(t, env, webhook.ID, empty.ID) + assert.Equal(t, []string{expiryNever}, selected) + + webhook = seedWebhookWithRetention(t, env.db, 30) + unlisted := seedConfiguredTarget( + t, env.db, webhook.ID, database.TargetTypeDatabase, + `{"expiry":"36h"}`, + ) + + assert.Equal(t, []string{"36 hours"}, expiryShown(t, env, webhook.ID)) + + page, selected := expirySelected(t, env, webhook.ID, unlisted.ID) + assert.Equal(t, []string{"36h"}, selected) + assert.Regexp( + t, + `\s*`+ + `36h\s*`+ + `never`, + page, + ) + assert.Contains(t, page, `365d`) + + form := url.Values{} + form.Set("name", unlisted.Name) + form.Set("expiry", "36h") + + w := submitTargetEdit(env, webhook.ID, unlisted.ID, form) + require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String()) + assert.JSONEq( + t, `{"expiry":"36h"}`, storedTarget(t, env, unlisted.ID).Config, + ) +} diff --git a/internal/handlers/source_detail_test.go b/internal/handlers/source_detail_test.go index ad79ed2..007f464 100644 --- a/internal/handlers/source_detail_test.go +++ b/internal/handlers/source_detail_test.go @@ -234,7 +234,7 @@ func TestHandleSourceDetail_RendersNamedTargetFields( assert.NotContains(t, body, "sekrit") assert.Contains(t, body, "Archive Expiry") - assert.Contains(t, body, "720h") + assert.Contains(t, body, "30 days") // An unknown type gets the neutral placeholder, never the // stored blob. diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index 5556e27..5c36d3a 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -312,6 +312,9 @@ func newSourceFormData( tmplKeyError: errMsg, "Form": in, "DefaultRetentionDays": database.DefaultRetentionDays, + tmplKeyArchiveExpiryChoices: archiveExpiryOptions( + in.ArchiveExpiry, + ), } } @@ -623,6 +626,9 @@ func (h *Handlers) renderSourceDetail( "Stats": h.loadWebhookStats(webhook.ID, entrypoints, targets), "TargetForm": targetForm, "TargetError": targetErr, + // The add target form's select starts on its expiry + // through Alpine, so no choice is selected here. + tmplKeyArchiveExpiryChoices: archiveExpiryChoices(), } status := http.StatusOK diff --git a/internal/handlers/target_edit.go b/internal/handlers/target_edit.go index 10a1e65..9855615 100644 --- a/internal/handlers/target_edit.go +++ b/internal/handlers/target_edit.go @@ -245,8 +245,9 @@ func (h *Handlers) renderTargetEdit( MaxRetries: target.MaxRetries, Config: cfg, }, - tmplKeyMaxTimeout: delivery.MaxTargetTimeoutSeconds, - tmplKeyError: errMsg, + tmplKeyMaxTimeout: delivery.MaxTargetTimeoutSeconds, + tmplKeyError: errMsg, + tmplKeyArchiveExpiryChoices: archiveExpiryOptions(cfg.Expiry), } h.renderTemplate(w, r, targetEditTemplate, data) diff --git a/internal/server/alpine_browser_test.go b/internal/server/alpine_browser_test.go index c2ea01f..acd0481 100644 --- a/internal/server/alpine_browser_test.go +++ b/internal/server/alpine_browser_test.go @@ -89,35 +89,8 @@ func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) { page := srv.URL + "/hook/" + webhook.ID checkAddEntrypoint(ctx, t, page) - - // Each target type, with the fields its add target form submits, in - // page order. Only http and slack have a url field. - targetTypes := []struct { - name string - fields string - values map[string]string - }{ - { - "http", "csrf_token name type url headers timeout max_retries", - map[string]string{"url": publicTargetURL}, - }, - { - "slack", "csrf_token name type url max_retries", - map[string]string{"url": publicTargetURL}, - }, - { - "database", "csrf_token name type expiry", - map[string]string{"expiry": "720h"}, - }, - {"log", "csrf_token name type", nil}, - } - - for _, tt := range targetTypes { - checkAddTarget( - ctx, t, page, tt.name, strings.Fields(tt.fields), tt.values, - ) - } - + checkAddEveryTarget(ctx, t, page) + checkArchiveExpiry(ctx, t, page) checkRefusedTarget(ctx, t, page) checkTargetDeliveries(ctx, t, page, target.Name, "0 in total, 0 in the last 24 hours", @@ -310,6 +283,40 @@ const ( document.querySelector('form[action$="/targets"]')).keys()]` ) +// checkAddEveryTarget adds a target of each type on a webhook page, +// each through checkAddTarget, in page order. +func checkAddEveryTarget(ctx context.Context, t *testing.T, page string) { + t.Helper() + + // Each target type, with the fields its add target form submits. + // Only http and slack have a url field. + targetTypes := []struct { + name string + fields string + values map[string]string + }{ + { + "http", "csrf_token name type url headers timeout max_retries", + map[string]string{"url": publicTargetURL}, + }, + { + "slack", "csrf_token name type url max_retries", + map[string]string{"url": publicTargetURL}, + }, + { + "database", "csrf_token name type expiry", + map[string]string{"expiry": "720h"}, + }, + {"log", "csrf_token name type", nil}, + } + + for _, tt := range targetTypes { + checkAddTarget( + ctx, t, page, tt.name, strings.Fields(tt.fields), tt.values, + ) + } +} + // checkAddTarget loads a webhook page and walks the add target form for // one target type. The form shows nothing until Add is clicked; Add // shows only the type choice; Cancel there closes it; Next shows the @@ -398,6 +405,43 @@ func chooseTargetType(ctx context.Context, t *testing.T, targetType string) { "%s: Add still shows while the form is open", targetType) } +// checkArchiveExpiry loads a webhook page and checks that the add +// target form's archive expiry starts on never, that the database +// target checkAddTarget added with 720h is listed as 30 days, and that +// its edit form starts on 720h. +func checkArchiveExpiry(ctx context.Context, t *testing.T, url string) { + t.Helper() + + const expiry = `form[action$="/targets"] select[name="expiry"]` + + row := `//span[text()="added-database"]/ancestor::div[@class="p-4"][1]` + + require.NoError(t, chromedp.Run(ctx, loadPage(url))) + + chooseTargetType(ctx, t, "database") + + var start, edited string + + require.NoError(t, chromedp.Run( + ctx, chromedp.Value(expiry, &start, chromedp.ByQuery), + )) + assert.Equal(t, "never", start, + "the add target form's archive expiry does not start on never") + + assert.True(t, shown(ctx, row+`//span[text()="Archive Expiry:"]`+ + `/following-sibling::span[text()="30 days"]`), + "a database target added with 720h is not listed as 30 days") + + click(ctx, t, row+`//a[text()="Edit"]`) + require.NoError(t, chromedp.Run( + ctx, + chromedp.WaitReady("#expiry", chromedp.ByQuery), + chromedp.Value("#expiry", &edited, chromedp.ByQuery), + )) + assert.Equal(t, "720h", edited, + "the edit form does not start on the stored archive expiry") +} + // checkRefusedTarget submits an http target the server refuses, a // loopback destination, and checks that the page comes back with the // form open on the http fields, the values entered and the reason, and diff --git a/templates/source_detail.html b/templates/source_detail.html index 2aae5d7..bdb181b 100644 --- a/templates/source_detail.html +++ b/templates/source_detail.html @@ -203,8 +203,15 @@ - - Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows. + + Archive expiry: + + {{range .ArchiveExpiryChoices}} + {{.Label}} + {{end}} + + + Archived events older than this are deleted from the archive; never keeps them all. diff --git a/templates/sources_new.html b/templates/sources_new.html index d4e7438..62f6103 100644 --- a/templates/sources_new.html +++ b/templates/sources_new.html @@ -50,13 +50,9 @@ Archive pruning - never - 1h - 12h - 24h - 30d - 90d - 365d + {{range .ArchiveExpiryChoices}} + {{.Label}} + {{end}} Archived events older than this are deleted from the archive; never keeps them all. diff --git a/templates/target_edit.html b/templates/target_edit.html index 59b2fec..aaad5d8 100644 --- a/templates/target_edit.html +++ b/templates/target_edit.html @@ -60,8 +60,12 @@ {{if eq .Target.Type "database"}} Archive Expiry - - "never" (the default when blank) keeps archived rows forever, or a Go duration like "720h" prunes older rows. + + {{range .ArchiveExpiryChoices}} + {{.Label}} + {{end}} + + Archived events older than this are deleted from the archive; never keeps them all. {{end}}
Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.
Archived events older than this are deleted from the archive; never keeps them all.
"never" (the default when blank) keeps archived rows forever, or a Go duration like "720h" prunes older rows.