Name each database target's archive for its webhook and target (closes #376)

Each database target now has its own archive file,
archive-WEBHOOKNAME-TARGETNAME-TARGETID.db, instead of one
archive-WEBHOOKID.db per webhook. delivery.ArchiveFileName builds the
name: each name is lowercased, keeps ASCII letters and digits, turns
every other run of characters into one dash, and is cut to 40
characters.

A change of webhook or target name renames its archive files under the
archive writer's lock, before the new name is saved, and back again if
the save fails. A rename never replaces a file: if one already has the
new name, the edit is refused. Deleting a target evicts only that
target's writer. Archive files are never deleted, and nothing looks for
files under the old name.

Model: opus-5-5

internal/delivery/target_database_evict_test.go

@@ -17,85 +17,109 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
-// evictTestEngine builds an engine backed by a temporary data
-// directory and returns it along with that directory.
-func evictTestEngine(t *testing.T) (*delivery.Engine, string) {
+// deliverTo archives one event to a database target, leaving the
+// target's writer cached with its handle open.
+func deliverTo(
+	t *testing.T, env *archiveEnv, tgt *database.Target,
+) {
 	t.Helper()
 
-	dataDir := t.TempDir()
-
-	eng := delivery.NewTestEngineWithDB(
-		nil,
-		database.NewTestWebhookDBManager(dataDir),
-		archiveTestLogger(),
-		&http.Client{Timeout: 5 * time.Second},
-		1,
-	)
-
-	return eng, dataDir
-}
-
-// TestEvictWebhook_ClosesAndRemovesWriter proves that evicting
-// a webhook drops its archive writer from the registry and
-// closes the open archive handle, rather than leaving both
-// alive for the process lifetime.
-func TestEvictWebhook_ClosesAndRemovesWriter(t *testing.T) {
-	t.Parallel()
-
-	eng, dataDir := evictTestEngine(t)
-
 	webhookDB := testWebhookDB(t)
 	event := seedEvent(t, webhookDB, `{"archived":true}`)
-	d := seedDatabaseTargetDelivery(t, webhookDB, event, "")
 
-	eng.ExportDeliverDatabase(webhookDB, d)
-
-	webhookID := event.WebhookID
-
-	require.True(
-		t, eng.ExportHasArchiveWriter(webhookID),
-		"a delivery should have cached an archive writer",
-	)
-	require.True(
-		t, eng.ExportArchiveHandleOpen(webhookID),
-		"the writer should hold an open handle after a write",
+	env.eng.ExportDeliverDatabase(
+		webhookDB, seedDatabaseTargetDelivery(t, webhookDB, event, tgt),
 	)
+}
 
-	eng.EvictWebhook(webhookID)
+// TestEvictWebhook_ClosesAndRemovesWriter proves that evicting
+// a webhook drops the archive writers of its database targets
+// from the registry and closes their open handles, rather than
+// leaving them alive for the process lifetime, and leaves another
+// webhook's writer alone.
+func TestEvictWebhook_ClosesAndRemovesWriter(t *testing.T) {
+	t.Parallel()
 
-	assert.False(
-		t, eng.ExportHasArchiveWriter(webhookID),
-		"eviction should remove the registry entry",
-	)
-	assert.False(
-		t, eng.ExportArchiveHandleOpen(webhookID),
-		"eviction should close the archive handle",
-	)
+	env := setupArchiveTest(t)
+	first := env.seedDatabaseTarget(t, "")
+	second := env.addDatabaseTarget(t, first.WebhookID, "")
+	other := env.seedDatabaseTarget(t, "")
 
-	archivePath := filepath.Join(
-		dataDir, fmt.Sprintf("archive-%s.db", webhookID),
+	for _, tgt := range []*database.Target{first, second, other} {
+		deliverTo(t, env, tgt)
+
+		require.True(
+			t, env.eng.ExportArchiveHandleOpen(tgt.ID),
+			"the writer should hold an open handle after a write",
+		)
+	}
+
+	env.eng.EvictWebhook(first.WebhookID)
+
+	for _, tgt := range []*database.Target{first, second} {
+		assert.False(
+			t, env.eng.ExportHasArchiveWriter(tgt.ID),
+			"eviction should remove the registry entry",
+		)
+		assert.False(
+			t, env.eng.ExportArchiveHandleOpen(tgt.ID),
+			"eviction should close the archive handle",
+		)
+		assert.FileExists(
+			t, env.archivePath(tgt),
+			"eviction must not delete the archive file",
+		)
+	}
+
+	assert.True(
+		t, env.eng.ExportArchiveHandleOpen(other.ID),
+		"another webhook's writer must be left alone",
 	)
+}
+
+// TestEvictTarget_LeavesOtherTargets proves that evicting one
+// database target leaves the writer of another target of the same
+// webhook in place.
+func TestEvictTarget_LeavesOtherTargets(t *testing.T) {
+	t.Parallel()
+
+	env := setupArchiveTest(t)
+	doomed := env.seedDatabaseTarget(t, "")
+	kept := env.addDatabaseTarget(t, doomed.WebhookID, "")
+
+	deliverTo(t, env, doomed)
+	deliverTo(t, env, kept)
+
+	env.eng.EvictTarget(doomed.ID)
+
+	assert.False(t, env.eng.ExportHasArchiveWriter(doomed.ID))
 	assert.FileExists(
-		t, archivePath,
+		t, env.archivePath(doomed),
 		"eviction must not delete the archive file",
 	)
+	assert.True(
+		t, env.eng.ExportArchiveHandleOpen(kept.ID),
+		"the other target's writer must be left alone",
+	)
 }
 
 // TestEvictWebhook_UnknownWebhookIsNoOp proves eviction is safe
-// for the common case of a webhook that never had a database
-// target, and that repeating it does not panic.
+// for the common case of a webhook or target that never had an
+// archive writer, and that repeating it does not panic.
 func TestEvictWebhook_UnknownWebhookIsNoOp(t *testing.T) {
 	t.Parallel()
 
-	eng, _ := evictTestEngine(t)
+	env := setupArchiveTest(t)
 
 	assert.NotPanics(t, func() {
-		eng.EvictWebhook("no-such-webhook")
-		eng.EvictWebhook("no-such-webhook")
+		env.eng.EvictWebhook("no-such-webhook")
+		env.eng.EvictWebhook("no-such-webhook")
+		env.eng.EvictTarget("no-such-target")
+		env.eng.EvictTarget("no-such-target")
 	})
 
 	assert.False(
-		t, eng.ExportHasArchiveWriter("no-such-webhook"),
+		t, env.eng.ExportHasArchiveWriter("no-such-target"),
 		"eviction must not create a writer",
 	)
 }
@@ -289,17 +313,14 @@ func TestEvictWebhook_RacingWriteDoesNotReopenHandle(
 ) {
 	t.Parallel()
 
-	eng, _ := evictTestEngine(t)
-
-	webhookDB := testWebhookDB(t)
-	event := seedEvent(t, webhookDB, `{"archived":true}`)
-	d := seedDatabaseTargetDelivery(t, webhookDB, event, "")
+	env := setupArchiveTest(t)
+	tgt := env.seedDatabaseTarget(t, "")
 
 	// Prime the registry so the test can hold the very writer the
 	// eviction is about to detach.
-	eng.ExportDeliverDatabase(webhookDB, d)
+	deliverTo(t, env, tgt)
 
-	w := eng.ExportArchiveWriterFor(event.WebhookID)
+	w := env.eng.ExportArchiveWriterFor(tgt.ID)
 	require.NotNil(t, w)
 	require.True(t, w.HandleOpen())
 
@@ -309,7 +330,7 @@ func TestEvictWebhook_RacingWriteDoesNotReopenHandle(
 	// eviction has to contend for the writer's mutex.
 	race.awaitFirstWrite()
 
-	eng.EvictWebhook(event.WebhookID)
+	env.eng.EvictWebhook(tgt.WebhookID)
 
 	sawEvicted, otherErr := race.wait()
 
@@ -324,41 +345,33 @@ func TestEvictWebhook_RacingWriteDoesNotReopenHandle(
 			"been evicted",
 	)
 	assert.False(
-		t, eng.ExportHasArchiveWriter(event.WebhookID),
+		t, env.eng.ExportHasArchiveWriter(tgt.ID),
 		"the registry entry must stay gone",
 	)
 }
 
 // TestEvictWebhook_LaterDeliveryRecreatesWriter proves eviction
-// does not break archiving for a webhook that is still alive: a
+// does not break archiving for a target that is still alive: a
 // subsequent delivery gets a brand new writer from the registry.
 // It says nothing about the evicted writer itself — that is what
 // TestEvictedWriter_WriteDoesNotReopenFile covers.
 func TestEvictWebhook_LaterDeliveryRecreatesWriter(t *testing.T) {
 	t.Parallel()
 
-	eng, _ := evictTestEngine(t)
+	env := setupArchiveTest(t)
+	tgt := env.seedDatabaseTarget(t, "")
 
-	webhookDB := testWebhookDB(t)
-	event := seedEvent(t, webhookDB, `{"archived":true}`)
-	d := seedDatabaseTargetDelivery(t, webhookDB, event, "")
+	deliverTo(t, env, tgt)
+	require.True(t, env.eng.ExportHasArchiveWriter(tgt.ID))
 
-	eng.ExportDeliverDatabase(webhookDB, d)
-	require.True(
-		t, eng.ExportHasArchiveWriter(event.WebhookID),
-	)
+	env.eng.EvictWebhook(tgt.WebhookID)
 
-	eng.EvictWebhook(event.WebhookID)
-
-	// A fresh delivery for the same webhook gets a brand new
+	// A fresh delivery for the same target gets a brand new
 	// writer from the registry, so archiving keeps working.
-	second := seedDatabaseTargetDelivery(
-		t, webhookDB, event, "",
-	)
-	eng.ExportDeliverDatabase(webhookDB, second)
+	deliverTo(t, env, tgt)
 
 	assert.True(
-		t, eng.ExportHasArchiveWriter(event.WebhookID),
+		t, env.eng.ExportHasArchiveWriter(tgt.ID),
 		"a later delivery should recreate the writer",
 	)
 }
@@ -370,19 +383,16 @@ func TestEvictWebhook_LaterDeliveryRecreatesWriter(t *testing.T) {
 func TestEngineStop_WriteAfterStopIsRefused(t *testing.T) {
 	t.Parallel()
 
-	eng, _ := evictTestEngine(t)
+	env := setupArchiveTest(t)
+	tgt := env.seedDatabaseTarget(t, "")
 
-	webhookDB := testWebhookDB(t)
-	event := seedEvent(t, webhookDB, `{"archived":true}`)
-	d := seedDatabaseTargetDelivery(t, webhookDB, event, "")
+	deliverTo(t, env, tgt)
 
-	eng.ExportDeliverDatabase(webhookDB, d)
-
-	w := eng.ExportArchiveWriterFor(event.WebhookID)
+	w := env.eng.ExportArchiveWriterFor(tgt.ID)
 	require.NotNil(t, w)
 	require.True(t, w.HandleOpen())
 
-	require.NoError(t, eng.ExportStop(context.Background()))
+	require.NoError(t, env.eng.ExportStop(context.Background()))
 
 	err := w.Write(evictTestRow("ev-after-stop"), 0)
 
@@ -395,7 +405,7 @@ func TestEngineStop_WriteAfterStopIsRefused(t *testing.T) {
 		"a refused write must not reopen the archive",
 	)
 	assert.False(
-		t, eng.ExportHasArchiveWriter(event.WebhookID),
+		t, env.eng.ExportHasArchiveWriter(tgt.ID),
 		"the stop should empty the registry",
 	)