Store a target's settings as columns (closes #543)

A target's settings move from the per-type JSON string in targets.config to columns: url, headers, timeout_seconds, forward_query, archive_expiry and archive_rotation. A column a type does not use stays empty. headers holds the header map as JSON text through GORM's JSON serializer, and archive_expiry holds a time.Duration, 0 meaning never. The forms check each setting once, with the same refusals and messages, and the edit form is filled from the columns, so the per-type config structs, their parsers and the "could not be read" branches are gone. A delivery task carries the six settings in place of the JSON string; tests check that each copy into a task, and back out of one, keeps all six. Model: opus-5-5
2026-10-15 23:02:39 +02:00
parent ded39aa158
commit 8406c77949
71 changed files with 1665 additions and 2168 deletions
@@ -3,7 +3,6 @@ package delivery_test
 import (
 	"bytes"
 	"context"
-	"encoding/json"
 	"fmt"
 	"io"
 	"log/slog"
@@ -97,17 +96,6 @@ func iSeedWebhookDB(
 	return db
 }

-func iHTTPConfig(url string) string {
-	cfg := delivery.HTTPTargetConfig{URL: url}
-
-	data, err := json.Marshal(cfg)
-	if err != nil {
-		panic("failed to marshal HTTPTargetConfig")
-	}
-
-	return string(data)
-}
-
 func iEngine(
 	t *testing.T, workers int,
 ) *delivery.Engine {
@@ -169,24 +157,24 @@ func iSeedDelivery(
 func iTask(
 	d database.Delivery,
 	event database.Event,
-	webhookID, targetID, name, config string,
+	webhookID, targetID, name, targetURL string,
 	maxAttempts, attemptNum int,
 	body *string,
 ) delivery.Task {
 	return delivery.Task{
-		DeliveryID:   d.ID,
-		EventID:      event.ID,
-		WebhookID:    webhookID,
-		TargetID:     targetID,
-		TargetName:   name,
-		TargetType:   database.TargetTypeHTTP,
-		TargetConfig: config,
-		MaxAttempts:  maxAttempts,
-		Method:       event.Method,
-		Headers:      event.Headers,
-		ContentType:  event.ContentType,
-		Body:         body,
-		AttemptNum:   attemptNum,
+		DeliveryID:  d.ID,
+		EventID:     event.ID,
+		WebhookID:   webhookID,
+		TargetID:    targetID,
+		TargetName:  name,
+		TargetType:  database.TargetTypeHTTP,
+		TargetURL:   targetURL,
+		MaxAttempts: maxAttempts,
+		Method:      event.Method,
+		Headers:     event.Headers,
+		ContentType: event.ContentType,
+		Body:        body,
+		AttemptNum:  attemptNum,
 	}
 }

@@ -239,10 +227,9 @@ func TestProcessNewTask_InlineBody(t *testing.T) {
 	)

 	bodyStr := event.Body
-	cfg := iHTTPConfig(ts.URL)
 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"test-target", cfg, 1, 1, &bodyStr,
+		"test-target", ts.URL, 1, 1, &bodyStr,
 	)

 	s.Engine.ExportProcessNewTask(
@@ -290,10 +277,9 @@ func TestProcessNewTask_LargeBody_FetchFromDB(
 		database.DeliveryStatusPending,
 	)

-	cfg := iHTTPConfig(ts.URL)
 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"test-large", cfg, 1, 1, nil,
+		"test-large", ts.URL, 1, 1, nil,
 	)

 	s.Engine.ExportProcessNewTask(
@@ -313,16 +299,16 @@ func TestProcessNewTask_InvalidWebhookID(t *testing.T) {
 	s := newISetup(t)

 	task := delivery.Task{
-		DeliveryID:   uuid.New().String(),
-		EventID:      uuid.New().String(),
-		WebhookID:    uuid.New().String(),
-		TargetID:     uuid.New().String(),
-		TargetName:   "test",
-		TargetType:   database.TargetTypeHTTP,
-		TargetConfig: iHTTPConfig("http://localhost:9999"),
-		MaxAttempts:  1,
-		Body:         nil,
-		AttemptNum:   1,
+		DeliveryID:  uuid.New().String(),
+		EventID:     uuid.New().String(),
+		WebhookID:   uuid.New().String(),
+		TargetID:    uuid.New().String(),
+		TargetName:  "test",
+		TargetType:  database.TargetTypeHTTP,
+		TargetURL:   "http://localhost:9999",
+		MaxAttempts: 1,
+		Body:        nil,
+		AttemptNum:  1,
 	}

 	s.Engine.ExportProcessNewTask(
@@ -363,7 +349,6 @@ func TestProcessRetryTask_SuccessfulRetry(t *testing.T) {
 	)

 	bodyStr := event.Body
-	cfg := iHTTPConfig(ts.URL)

 	// The target row exists because the engine confirms a scheduled
 	// retry's target has not been deleted before it runs it. A retry
@@ -372,12 +357,12 @@ func TestProcessRetryTask_SuccessfulRetry(t *testing.T) {
 	// task. See https://git.eeqj.de/sneak/webhooker/issues/107.
 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "retry-target",
-		database.TargetTypeHTTP, cfg, 5,
+		database.TargetTypeHTTP, ts.URL, 5,
 	)

 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"retry-target", cfg, 5, 2, &bodyStr,
+		"retry-target", ts.URL, 5, 2, &bodyStr,
 	)

 	s.Engine.ExportProcessRetryTask(
@@ -410,10 +395,9 @@ func TestProcessRetryTask_SkipsNonRetryingDelivery(
 	)

 	bodyStr := event.Body
-	cfg := iHTTPConfig("http://localhost:1")
 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"skip-target", cfg, 5, 2, &bodyStr,
+		"skip-target", "http://localhost:1", 5, 2, &bodyStr,
 	)

 	s.Engine.ExportProcessRetryTask(
@@ -460,16 +444,14 @@ func TestProcessRetryTask_LargeBody_FetchFromDB(
 		database.DeliveryStatusRetrying,
 	)

-	cfg := iHTTPConfig(ts.URL)
-
 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "retry-large",
-		database.TargetTypeHTTP, cfg, 5,
+		database.TargetTypeHTTP, ts.URL, 5,
 	)

 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"retry-large", cfg, 5, 2, nil,
+		"retry-large", ts.URL, 5, 2, nil,
 	)

 	s.Engine.ExportProcessRetryTask(
@@ -570,16 +552,15 @@ func TestWorkerLifecycle_ProcessesRetryChannel(
 	s.Engine.ExportStart()

 	bodyStr := event.Body
-	cfg := iHTTPConfig(ts.URL)

 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "retry-chan-test",
-		database.TargetTypeHTTP, cfg, 5,
+		database.TargetTypeHTTP, ts.URL, 5,
 	)

 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"retry-chan-test", cfg, 5, 2, &bodyStr,
+		"retry-chan-test", ts.URL, 5, 2, &bodyStr,
 	)

 	s.Engine.ExportRetryCh() <- task
@@ -685,7 +666,7 @@ func iCreateTarget(
 	mainDB *gorm.DB,
 	targetID, webhookID, name string,
 	targetType database.TargetType,
-	config string,
+	targetURL string,
 	maxAttempts int,
 ) {
 	t.Helper()
@@ -695,7 +676,7 @@ func iCreateTarget(
 		Name:        name,
 		Type:        targetType,
 		Active:      true,
-		Config:      config,
+		URL:         targetURL,
 		MaxAttempts: maxAttempts,
 	}
 	target.ID = targetID
@@ -736,7 +717,7 @@ func TestRecoverWebhookDeliveries_RetryingDeliveries(
 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "retry-recovery",
 		database.TargetTypeHTTP,
-		iHTTPConfig("http://example.com/hook"), 5,
+		"http://example.com/hook", 5,
 	)

 	event := iSeedEvent(
@@ -796,7 +777,7 @@ func iSeedRetryingWithType(

 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "mutated-target", targetType,
-		iHTTPConfig("http://example.com/hook"), 5,
+		"http://example.com/hook", 5,
 	)

 	event := iSeedEvent(
@@ -934,14 +915,14 @@ func TestFailUnretryableRetry_WritesNoTargetRow(

 	targetID := uuid.New().String()

-	// A Slack incoming-webhook URL: the target config IS the
+	// A Slack incoming-webhook URL: the target's URL IS the
 	// credential, which is what makes a leaked target row a
 	// disclosure rather than a curiosity.
 	hookURL := "https://hooks.slack.com/services/T00/B00/x"

 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "credential-bearing",
-		database.TargetTypeLog, iHTTPConfig(hookURL), 5,
+		database.TargetTypeLog, hookURL, 5,
 	)

 	event := iSeedEvent(
@@ -1186,7 +1167,7 @@ func TestRecoverDeliveries_SkipsAWebhookDeletedAfterTheListIsRead(
 	assert.False(t, dbMgr.DBExists(webhookID))
 }

-// --- HTTP Config with custom headers ---
+// --- HTTP target with custom headers ---

 func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
 	t.Parallel()
@@ -1208,16 +1189,6 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
 	)
 	defer ts.Close()

-	cfg := delivery.HTTPTargetConfig{
-		URL: ts.URL,
-		Headers: map[string]string{
-			"Authorization": "Bearer secret-token",
-		},
-	}
-
-	cfgJSON, err := json.Marshal(cfg)
-	require.NoError(t, err)
-
 	event := iSeedEvent(
 		t, s.WebhookDB, s.WebhookID,
 		`{"auth":"test"}`,
@@ -1232,9 +1203,12 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
 	bodyStr := event.Body
 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"auth-target", string(cfgJSON),
+		"auth-target", ts.URL,
 		1, 1, &bodyStr,
 	)
+	task.TargetHeaders = map[string]string{
+		headerAuthorization: "Bearer secret-token",
+	}

 	s.Engine.ExportProcessNewTask(
 		context.TODO(), &task,
@@ -1247,11 +1221,14 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {

 // --- HTTP delivery with custom timeout ---

+// TestDeliverHTTP_TargetTimeout proves a task's TimeoutSeconds reaches
+// the delivery through the engine. The destination answers after two
+// seconds, inside the engine's own five-second client timeout, so only
+// the task's one second can fail the delivery.
 func TestDeliverHTTP_TargetTimeout(t *testing.T) {
 	t.Parallel()

-	db := testWebhookDB(t)
-	e := iEngine(t, 1)
+	s := newISetup(t)

 	ts := httptest.NewServer(
 		http.HandlerFunc(
@@ -1263,31 +1240,30 @@ func TestDeliverHTTP_TargetTimeout(t *testing.T) {
 	)
 	defer ts.Close()

-	cfg := delivery.HTTPTargetConfig{
-		URL:     ts.URL,
-		Timeout: 1,
-	}
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"timeout":"test"}`,
+	)
+	targetID := uuid.New().String()

-	cfgJSON, err := json.Marshal(cfg)
-	require.NoError(t, err)
-
-	event, del := iSeedEventAndDelivery(
-		t, db, `{"timeout":"test"}`,
-		string(cfgJSON),
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
 	)

-	task, d := iHTTPTaskAndDelivery(
-		event, del, "timeout-target",
-		string(cfgJSON), 1, 1,
+	bodyStr := event.Body
+	task := iTask(
+		d, event, s.WebhookID, targetID,
+		"timeout-target", ts.URL, 1, 1, &bodyStr,
 	)
+	task.TimeoutSeconds = 1

-	e.ExportDeliverHTTP(context.TODO(), db, d, task)
+	s.Engine.ExportProcessNewTask(context.TODO(), &task)

-	iAssertStatus(t, db, del.ID,
+	iAssertStatus(t, s.WebhookDB, d.ID,
 		database.DeliveryStatusFailed,
 	)

-	iAssertResultFailed(t, db, del.ID)
+	iAssertResultFailed(t, s.WebhookDB, d.ID)
 }

 // TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog pins the size
@@ -1325,8 +1301,6 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
 	))
 	defer ts.Close()

-	cfgJSON := iHTTPConfig(ts.URL)
-
 	event := iSeedEvent(
 		t, s.WebhookDB, s.WebhookID, `{"cut":"test"}`,
 	)
@@ -1340,7 +1314,7 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
 	bodyStr := event.Body
 	task := iTask(
 		del, event, s.WebhookID, targetID,
-		"cut-target", cfgJSON, 1, 1, &bodyStr,
+		"cut-target", ts.URL, 1, 1, &bodyStr,
 	)

 	s.Engine.ExportProcessNewTask(context.TODO(), &task)
@@ -1372,7 +1346,7 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
 func iSeedEventAndDelivery(
 	t *testing.T,
 	db *gorm.DB,
-	body, _ string,
+	body string,
 ) (database.Event, database.Delivery) {
 	t.Helper()

@@ -1403,19 +1377,19 @@ func iSeedEventAndDelivery(
 func iHTTPTaskAndDelivery(
 	event database.Event,
 	del database.Delivery,
-	name, config string,
+	name, targetURL string,
 	maxAttempts, attemptNum int,
 ) (*delivery.Task, *database.Delivery) {
 	task := &delivery.Task{
-		DeliveryID:   del.ID,
-		EventID:      event.ID,
-		WebhookID:    event.WebhookID,
-		TargetID:     del.TargetID,
-		TargetName:   name,
-		TargetType:   database.TargetTypeHTTP,
-		TargetConfig: config,
-		MaxAttempts:  maxAttempts,
-		AttemptNum:   attemptNum,
+		DeliveryID:  del.ID,
+		EventID:     event.ID,
+		WebhookID:   event.WebhookID,
+		TargetID:    del.TargetID,
+		TargetName:  name,
+		TargetType:  database.TargetTypeHTTP,
+		TargetURL:   targetURL,
+		MaxAttempts: maxAttempts,
+		AttemptNum:  attemptNum,
 	}

 	d := &database.Delivery{
@@ -1426,7 +1400,7 @@ func iHTTPTaskAndDelivery(
 		Target: database.Target{
 			Name:        name,
 			Type:        database.TargetTypeHTTP,
-			Config:      config,
+			URL:         targetURL,
 			MaxAttempts: maxAttempts,
 		},
 	}
@@ -1455,53 +1429,27 @@ func iAssertResultFailed(
 	assert.NotEmpty(t, result.Error)
 }

-// --- HTTP request with invalid config ---
+// --- HTTP request with no URL ---

-func TestDeliverHTTP_InvalidConfig(t *testing.T) {
+// TestDeliverHTTP_NoURL proves an http target without a URL, which
+// the forms never store, fails its delivery and records why.
+func TestDeliverHTTP_NoURL(t *testing.T) {
 	t.Parallel()

 	db := testWebhookDB(t)
 	e := iEngine(t, 1)

-	event, del := iSeedEventAndDelivery(
-		t, db, `{"config":"invalid"}`, "",
-	)
+	event, del := iSeedEventAndDelivery(t, db, `{"url":"none"}`)

-	task, d := iHTTPTaskAndDelivery(
-		event, del, "bad-config", `not-json`, 1, 1,
-	)
+	task, d := iHTTPTaskAndDelivery(event, del, "no-url", "", 1, 1)

 	e.ExportDeliverHTTP(context.TODO(), db, d, task)

 	iAssertStatus(t, db, del.ID,
 		database.DeliveryStatusFailed,
 	)
-}

-// TestDeliverHTTP_InvalidConfigUnrecordedStaysPending: a delivery is
-// failed for an invalid config only once the reason is recorded.
-// Unrecorded, it stays pending, where the sweep finds it again.
-func TestDeliverHTTP_InvalidConfigUnrecordedStaysPending(t *testing.T) {
-	t.Parallel()
-
-	db := testWebhookDB(t)
-	e := testEngine(t, 1)
-
-	event, del := iSeedEventAndDelivery(
-		t, db, `{"config":"invalid"}`, "",
-	)
-
-	task, d := iHTTPTaskAndDelivery(
-		event, del, "bad-config", `not-json`, 1, 1,
-	)
-
-	require.NoError(t, db.Exec("drop table delivery_results").Error)
-
-	e.ExportDeliverHTTP(context.TODO(), db, d, task)
-
-	iAssertStatus(t, db, del.ID,
-		database.DeliveryStatusPending,
-	)
+	iAssertResultFailed(t, db, del.ID)
 }

 // --- Notify batching ---