190cabe0f2
All checks were successful
check / check (push) Successful in 3m10s
The per-webhook archiveWriter registry in the database delivery target was never evicted, so a deleted webhook's writer -- and any archive file handle open within its debounce window -- lingered for the process lifetime. Separately, expiry pruning ran only when an archive was (re)opened, and reopens only happen on writes, so an archive belonging to a webhook that stopped receiving events kept its expired rows forever. Eviction: a new one-method delivery.WebhookEvictor interface (kept separate from Notifier: archiving lifecycle is not notification) is implemented by the Engine and injected into the handlers. Deleting a webhook, or deleting its last database target, drops the writer from the registry and closes its handle under the writer's own mutex, so eviction can never race an in-flight write. An evicted writer refuses further writes rather than reopening a file nothing holds. The archive file is deliberately left on disk: it is long-term storage an operator may want to keep or move away, and destroying it as a side effect of deleting a webhook would be unrecoverable. Idle sweep: a new ArchiveSweeper, modelled on the event RetentionReaper (fx lifecycle hooks, cancellable context, WaitGroup, ticker loop), prunes archives whose database target declares a positive expiry. It reuses the existing RETENTION_SWEEP_INTERVAL rather than adding a config key. It never creates an archive -- a missing file is skipped, and the reopen uses SQLite mode=rw so the file cannot be conjured even if it disappears mid-sweep -- routes the prune through the per-webhook writer so its mutex orders the sweep against concurrent writes, and leaves the archive closed so the move-the-file-away workflow keeps working. A failure for one webhook is logged and the sweep continues. Archives with no expiry or the expiry "never" are untouched.
131 lines
3.3 KiB
Go
131 lines
3.3 KiB
Go
package delivery_test
|
|
|
|
import (
|
|
"fmt"
|
|
"net/http"
|
|
"path/filepath"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"sneak.berlin/go/webhooker/internal/database"
|
|
"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) {
|
|
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",
|
|
)
|
|
|
|
eng.EvictWebhook(webhookID)
|
|
|
|
assert.False(
|
|
t, eng.ExportHasArchiveWriter(webhookID),
|
|
"eviction should remove the registry entry",
|
|
)
|
|
assert.False(
|
|
t, eng.ExportArchiveHandleOpen(webhookID),
|
|
"eviction should close the archive handle",
|
|
)
|
|
|
|
archivePath := filepath.Join(
|
|
dataDir, fmt.Sprintf("archive-%s.db", webhookID),
|
|
)
|
|
assert.FileExists(
|
|
t, archivePath,
|
|
"eviction must not delete the archive file",
|
|
)
|
|
}
|
|
|
|
// 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.
|
|
func TestEvictWebhook_UnknownWebhookIsNoOp(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
eng, _ := evictTestEngine(t)
|
|
|
|
assert.NotPanics(t, func() {
|
|
eng.EvictWebhook("no-such-webhook")
|
|
eng.EvictWebhook("no-such-webhook")
|
|
})
|
|
|
|
assert.False(
|
|
t, eng.ExportHasArchiveWriter("no-such-webhook"),
|
|
"eviction must not create a writer",
|
|
)
|
|
}
|
|
|
|
// TestEvictWebhook_EvictedWriterDoesNotReopen proves an evicted
|
|
// writer refuses further writes instead of silently reopening
|
|
// the archive file: nothing holds it any more, so a reopened
|
|
// handle would leak.
|
|
func TestEvictWebhook_EvictedWriterDoesNotReopen(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
eng, _ := evictTestEngine(t)
|
|
|
|
webhookDB := testWebhookDB(t)
|
|
event := seedEvent(t, webhookDB, `{"archived":true}`)
|
|
d := seedDatabaseTargetDelivery(t, webhookDB, event, "")
|
|
|
|
eng.ExportDeliverDatabase(webhookDB, d)
|
|
require.True(
|
|
t, eng.ExportHasArchiveWriter(event.WebhookID),
|
|
)
|
|
|
|
eng.EvictWebhook(event.WebhookID)
|
|
|
|
// A fresh delivery for the same webhook gets a brand new
|
|
// writer from the registry, so archiving keeps working.
|
|
second := seedDatabaseTargetDelivery(
|
|
t, webhookDB, event, "",
|
|
)
|
|
eng.ExportDeliverDatabase(webhookDB, second)
|
|
|
|
assert.True(
|
|
t, eng.ExportHasArchiveWriter(event.WebhookID),
|
|
"a later delivery should recreate the writer",
|
|
)
|
|
}
|