diff --git a/internal/handlers/export_test.go b/internal/handlers/export_test.go index eb1b617..cff6c09 100644 --- a/internal/handlers/export_test.go +++ b/internal/handlers/export_test.go @@ -28,7 +28,7 @@ func (s *Handlers) BuildSlackTargetConfigForTest( // package. func (s *Handlers) BuildDatabaseTargetConfigForTest( w http.ResponseWriter, - r *http.Request, + expiry string, ) (string, error) { - return s.buildDatabaseTargetConfig(w, r) + return s.buildDatabaseTargetConfig(w, expiry) } diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index d5f9d73..8aae54a 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -4,8 +4,6 @@ import ( "context" "net/http" "net/http/httptest" - "net/url" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -189,25 +187,6 @@ func TestRenderTemplate(t *testing.T) { ) } -// databaseConfigRequest builds a POST request carrying the -// given expiry as a form value, as the add-target form does. -func databaseConfigRequest(expiry string) *http.Request { - form := url.Values{} - if expiry != "" { - form.Set("expiry", expiry) - } - - req := httptest.NewRequestWithContext( - context.Background(), http.MethodPost, "/", - strings.NewReader(form.Encode()), - ) - req.Header.Set( - "Content-Type", "application/x-www-form-urlencoded", - ) - - return req -} - func TestBuildDatabaseTargetConfig_Valid(t *testing.T) { t.Parallel() @@ -220,25 +199,19 @@ func TestBuildDatabaseTargetConfig_Valid(t *testing.T) { // Empty expiry: the keep-forever default, empty config. w := httptest.NewRecorder() - cfg, err := h.BuildDatabaseTargetConfigForTest( - w, databaseConfigRequest(""), - ) + cfg, err := h.BuildDatabaseTargetConfigForTest(w, "") require.NoError(t, err) assert.Empty(t, cfg) // Explicit never is stored as config. w = httptest.NewRecorder() - cfg, err = h.BuildDatabaseTargetConfigForTest( - w, databaseConfigRequest("never"), - ) + cfg, err = h.BuildDatabaseTargetConfigForTest(w, "never") require.NoError(t, err) assert.JSONEq(t, `{"expiry":"never"}`, cfg) // A positive duration is stored as config. w = httptest.NewRecorder() - cfg, err = h.BuildDatabaseTargetConfigForTest( - w, databaseConfigRequest("720h"), - ) + cfg, err = h.BuildDatabaseTargetConfigForTest(w, "720h") require.NoError(t, err) assert.JSONEq(t, `{"expiry":"720h"}`, cfg) } @@ -257,9 +230,7 @@ func TestBuildDatabaseTargetConfig_RejectsBadExpiry( for _, bad := range []string{"nonsense", "7d", "-5h"} { w := httptest.NewRecorder() - cfg, err := h.BuildDatabaseTargetConfigForTest( - w, databaseConfigRequest(bad), - ) + cfg, err := h.BuildDatabaseTargetConfigForTest(w, bad) require.Error(t, err, "expiry %q", bad) assert.Empty(t, cfg) diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index f5cb987..7d43ad5 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -816,6 +816,7 @@ func (h *Handlers) processTargetCreate( targetType := database.TargetType(r.FormValue("type")) targetURL := r.FormValue("url") maxRetriesStr := r.FormValue("max_retries") + expiry := r.FormValue("expiry") if name == "" { http.Error( @@ -835,7 +836,7 @@ func (h *Handlers) processTargetCreate( } configJSON, err := h.buildTargetConfig( - w, r, targetType, targetURL, + w, r, targetType, targetURL, expiry, ) if err != nil { return @@ -893,11 +894,13 @@ func parseNonNegativeInt(s string) int { } // buildTargetConfig builds the JSON config string for a target. +// The expiry form value is read by the caller (which bounds the +// request body) and applies to database targets only. func (h *Handlers) buildTargetConfig( w http.ResponseWriter, r *http.Request, targetType database.TargetType, - targetURL string, + targetURL, expiry string, ) (string, error) { switch targetType { case database.TargetTypeHTTP: @@ -905,7 +908,7 @@ func (h *Handlers) buildTargetConfig( case database.TargetTypeSlack: return h.buildSlackTargetConfig(w, r, targetURL) case database.TargetTypeDatabase: - return h.buildDatabaseTargetConfig(w, r) + return h.buildDatabaseTargetConfig(w, expiry) case database.TargetTypeLog: return "", nil default: @@ -1017,16 +1020,16 @@ func (h *Handlers) buildSlackTargetConfig( } // buildDatabaseTargetConfig builds config JSON for a database -// (archive) target. The optional expiry form value is validated -// here, at creation time, so an unparseable value is rejected -// with a 400 instead of failing every subsequent delivery. An -// empty expiry yields an empty config (the keep-forever -// default). +// (archive) target. The optional expiry (a form value read by +// the caller, which bounds the request body) is validated here, +// at creation time, so an unparseable value is rejected with a +// 400 instead of failing every subsequent delivery. An empty +// expiry yields an empty config (the keep-forever default). func (h *Handlers) buildDatabaseTargetConfig( w http.ResponseWriter, - r *http.Request, + expiry string, ) (string, error) { - expiry := strings.TrimSpace(r.FormValue("expiry")) + expiry = strings.TrimSpace(expiry) if expiry == "" { return "", nil }