Move the archive files into their own package (closes #541)
check / check (push) Canceled after 0s

The SQLite archive files the database target writes are now managed by
internal/archive: file names, rotation, the writer of each target,
pruning, renames, the gzipped JSON export, the idle sweep, and the
lock renames run under, which was Handlers.renameMu. The database
target stays in internal/delivery and writes through archive.Writers;
the engine still closes the writers when it stops, so the stop order
is unchanged. The handlers take the lock, rename and evict through
archive.Archives.

Exported names drop their Archive prefix in the new package. The
tests moved with the code; the two that check a delivery's recorded
attempt stay in delivery.

Model: opus-5-5
This commit is contained in:
2026-10-15 18:10:02 +00:00
parent f10440b7cc
commit ee1cb0a0c6
42 changed files with 2116 additions and 1770 deletions
+2 -2
View File
@@ -12,8 +12,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// rotationNone is the archive rotation that keeps one file.
@@ -181,7 +181,7 @@ func TestArchiveFileView_Rotated(t *testing.T) {
`{"rotation":"daily"}`,
)
path := delivery.ArchivePath(env.dbMgr, &webhook, target)
path := archive.Path(env.dbMgr, &webhook, target)
stem := strings.TrimSuffix(path, ".db")
written := time.Date(2026, 10, 2, 9, 0, 0, 0, time.UTC)
+1 -1
View File
@@ -374,7 +374,7 @@ func assertFloodBounded(t *testing.T, label, out string) {
// this service's three gorm.Open sites: the main database behind the
// two unauthenticated lookups, and the per-webhook database the
// WebhookDBManager opens. The third, the archive writer, is pinned in
// internal/delivery, where its type lives.
// internal/archive, where its type lives.
//
// What each assertion is worth, since two of the three would pass
// against a service that had never been fixed if the capture were set
+3 -14
View File
@@ -10,11 +10,11 @@ import (
"html/template"
"log/slog"
"net/http"
"sync"
"sync/atomic"
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
@@ -65,7 +65,7 @@ type HandlersParams struct {
Session *session.Session
Middleware *middleware.Middleware
Notifier delivery.Notifier
Archives delivery.Archives
Archives archive.Archives
CircuitBreakers delivery.CircuitBreakers
SSRFGuard *delivery.Guard
Metrics *metrics.Set
@@ -83,7 +83,7 @@ type Handlers struct {
session *session.Session
mw *middleware.Middleware
notifier delivery.Notifier
archives delivery.Archives
archives archive.Archives
breakers delivery.CircuitBreakers
mtr *metrics.Set
templates map[string]*template.Template
@@ -93,17 +93,6 @@ type Handlers struct {
// is one delivery will actually attempt.
ssrf *delivery.Guard
// renameMu makes the webhook edit, the target edit and target
// creation run one at a time, each held from loading the stored
// names through the archive rename, the save and any move back.
// Interleaved, one could rename an archive between another's
// rename and save, leaving the file named for one edit and the
// stored names from the other. An archive download holds it while
// it reads the stored names and lists the files they give, and
// again for each file while it finds the file under the names
// stored then and opens it.
renameMu sync.Mutex
// dummyVerifications counts the equivalent-cost verifications
// charged for usernames that do not exist. It exists so a test
// can prove that path runs without measuring wall-clock time.
+16 -5
View File
@@ -16,6 +16,7 @@ import (
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
@@ -55,12 +56,14 @@ func (n *recordingNotifier) Tasks() []delivery.Task {
return out
}
// recordingArchives is a delivery.Archives that records what it
// recordingArchives is an archive.Archives that records what it
// was asked to do, so a test can prove that a deletion or rename
// path reached the delivery engine. After FailRenames, every
// path reached the archive writers. After FailRenames, every
// rename of that target fails with the given error. After
// BlockNextRename, the next rename is recorded and then waits.
// Lock and Unlock are a plain mutex, as in archive.Writers.
type recordingArchives struct {
renameMu sync.Mutex
mu sync.Mutex
evicted []string
evictedTargets []string
@@ -73,10 +76,10 @@ type recordingArchives struct {
// errInjectedRename is the failure a test hands FailRenames.
var errInjectedRename = errors.New("injected rename failure")
// errNameTaken is what the delivery engine returns when a file
// errNameTaken is what the archive writers return when a file
// already has an archive's new name, here archive-taken.db.
var errNameTaken = fmt.Errorf(
"%w: archive-taken.db", delivery.ErrArchiveNameTaken,
"%w: archive-taken.db", archive.ErrNameTaken,
)
// archiveRename is one recorded Rename call.
@@ -86,6 +89,14 @@ type archiveRename struct {
TargetName string
}
func (r *recordingArchives) Lock() {
r.renameMu.Lock()
}
func (r *recordingArchives) Unlock() {
r.renameMu.Unlock()
}
func (r *recordingArchives) EvictWebhook(webhookID string) {
r.mu.Lock()
defer r.mu.Unlock()
@@ -265,7 +276,7 @@ func newTestAppWithConfig(
func() *recordingArchives {
return &recordingArchives{}
},
func(r *recordingArchives) delivery.Archives {
func(r *recordingArchives) archive.Archives {
return r
},
func() *testCircuitBreakers {
+7 -7
View File
@@ -16,8 +16,8 @@ import (
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
@@ -180,7 +180,7 @@ func failSaveOnTable(
}
// archivePathFor returns the archive database path the
// delivery engine would use for a database target: beside the
// archive writers would use for a database target: beside the
// webhook's event database in the data directory.
func archivePathFor(
t *testing.T,
@@ -192,7 +192,7 @@ func archivePathFor(
return filepath.Join(
filepath.Dir(mgr.DBPath(wh.ID)),
delivery.ArchiveFileName(wh.Name, tgt.Name, tgt.ID),
archive.FileName(wh.Name, tgt.Name, tgt.ID),
)
}
@@ -228,9 +228,9 @@ func postRequest(
}
// TestHandleSourceDelete_EvictsArchiveWriter proves that
// deleting a webhook reaches the delivery engine and releases
// the webhook's archive writers, exercised through the real
// deletion handler rather than by calling the engine directly.
// deleting a webhook releases the webhook's archive writers,
// exercised through the real deletion handler rather than by
// calling the archive writers directly.
func TestHandleSourceDelete_EvictsArchiveWriter(t *testing.T) {
t.Parallel()
@@ -290,7 +290,7 @@ func TestHandleSourceDelete_KeepsArchiveFile(t *testing.T) {
wh := seedWebhook(t, db)
tgt := seedTarget(t, db, wh.ID, database.TargetTypeDatabase)
// Place an archive file where the delivery engine would.
// Place an archive file where the archive writers would.
archivePath := archivePathFor(t, mgr, wh, tgt)
require.NoError(
t,
+5 -4
View File
@@ -9,6 +9,7 @@ import (
"strings"
"github.com/go-chi/chi"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
@@ -27,8 +28,8 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
sourceID := chi.URLParam(r, "sourceID")
h.renameMu.Lock()
defer h.renameMu.Unlock()
h.archives.Lock()
defer h.archives.Unlock()
var webhook database.Webhook
@@ -361,12 +362,12 @@ func buildDatabaseTargetConfig(
) (string, string, error) {
expiry = strings.TrimSpace(expiry)
err := delivery.ValidateArchiveExpiry(expiry)
err := archive.ValidateExpiry(expiry)
if err != nil {
return "", fmt.Sprintf("Invalid archive expiry: %v", err), nil
}
err = delivery.ValidateArchiveRotation(rotation)
err = archive.ValidateRotation(rotation)
if err != nil {
return "", fmt.Sprintf("Invalid archive rotation: %v", err), nil
}
+16 -16
View File
@@ -6,8 +6,8 @@ import (
"net/http"
"time"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// downloadWriteTimeout is how long one write of a download may wait
@@ -16,7 +16,7 @@ const downloadWriteTimeout = 60 * time.Second
// HandleTargetDownload serves a database target's archive as one
// gzipped JSON file, named for the webhook, the target and the time;
// see delivery.ArchiveExport.WriteGzipJSON for what it holds. Other
// see archive.Export.WriteGzipJSON for what it holds. Other
// target types have no archive and are a 404.
//
// A download runs for as long as the client keeps reading: it reads
@@ -37,7 +37,7 @@ func (h *Handlers) HandleTargetDownload() http.HandlerFunc {
w.Header().Set("Content-Type", "application/gzip")
w.Header().Set(
"Content-Disposition",
`attachment; filename="`+delivery.ArchiveExportFileName(
`attachment; filename="`+archive.ExportFileName(
webhook.Name, target.Name, now,
)+`"`,
)
@@ -82,17 +82,17 @@ func (d downloadWriter) Write(b []byte) (int, error) {
// listTargetArchive lists the archive files of the request's database
// target for export. It reports false once it has written the response.
//
// It holds renameMu, which every archive rename runs under, while it
// reads the stored names and lists the files, so the files it lists
// are the ones those names give. It lets go before the export is
// streamed, which takes renameMu again for each file only while it
// It holds the archives' lock, which every archive rename runs under,
// while it reads the stored names and lists the files, so the files it
// lists are the ones those names give. It lets go before the export is
// streamed, which takes the lock again for each file only while it
// finds the file under the names stored then and opens it.
func (h *Handlers) listTargetArchive(
w http.ResponseWriter,
r *http.Request,
) (database.Webhook, *database.Target, *delivery.ArchiveExport, bool) {
h.renameMu.Lock()
defer h.renameMu.Unlock()
) (database.Webhook, *database.Target, *archive.Export, bool) {
h.archives.Lock()
defer h.archives.Unlock()
webhook, target, ok := h.ownedTarget(w, r)
if !ok {
@@ -105,9 +105,9 @@ func (h *Handlers) listTargetArchive(
return database.Webhook{}, nil, nil, false
}
export, err := delivery.NewArchiveExport(
delivery.ArchivePath(h.dbMgr, &webhook, target),
&h.renameMu,
export, err := archive.NewExport(
archive.Path(h.dbMgr, &webhook, target),
h.archives,
func() (string, error) {
return h.storedArchivePath(webhook.ID, target.ID)
},
@@ -122,9 +122,9 @@ func (h *Handlers) listTargetArchive(
return webhook, target, export, true
}
// storedArchivePath returns the path delivery.ArchivePath gives a
// storedArchivePath returns the path archive.Path gives a
// database target under the names stored for it and its webhook now.
// Its caller holds renameMu. A webhook or target deleted since is still
// Its caller holds the archives' lock. A webhook or target deleted since is still
// found, since deleting one leaves its archive files under their names.
func (h *Handlers) storedArchivePath(
webhookID, targetID string,
@@ -143,5 +143,5 @@ func (h *Handlers) storedArchivePath(
return "", err
}
return delivery.ArchivePath(h.dbMgr, &webhook, &target), nil
return archive.Path(h.dbMgr, &webhook, &target), nil
}
+11 -11
View File
@@ -21,9 +21,9 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/middleware/middlewaretest"
)
@@ -223,8 +223,8 @@ func TestHandleTargetDownload_FindsFilesAfterRename(t *testing.T) {
env := setupSourceTest(t)
wh := seedWebhookWithRetention(t, env.db, 7)
archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
oldPath := delivery.ArchivePath(env.dbMgr, &wh, archive)
target := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
oldPath := archive.Path(env.dbMgr, &wh, target)
month := func(path string) string {
return strings.TrimSuffix(path, ".db") + "-2026-10.db"
@@ -233,19 +233,19 @@ func TestHandleTargetDownload_FindsFilesAfterRename(t *testing.T) {
seedArchive(t, oldPath, 1, 16)
seedArchive(t, month(oldPath), 1, 16)
sw, downloaded := startStalledDownload(t, env, wh.ID, archive.ID)
sw, downloaded := startStalledDownload(t, env, wh.ID, target.ID)
require.Equal(t,
http.StatusSeeOther, renameTarget(env, wh.ID, archive.ID).Code,
http.StatusSeeOther, renameTarget(env, wh.ID, target.ID).Code,
)
// The test's archives record a rename without moving any file, so
// the files are moved here, as the delivery engine moves them.
// the files are moved here, as the archive writers move them.
var renamed database.Target
require.NoError(t, env.db.DB().First(&renamed, "id = ?", archive.ID).Error)
require.NoError(t, env.db.DB().First(&renamed, "id = ?", target.ID).Error)
newPath := delivery.ArchivePath(env.dbMgr, &wh, &renamed)
newPath := archive.Path(env.dbMgr, &wh, &renamed)
require.NoError(t, os.Rename(oldPath, newPath))
require.NoError(t, os.Rename(month(oldPath), month(newPath)))
@@ -355,16 +355,16 @@ func TestHandleTargetDownload_OutlastsTheRequestLimit(t *testing.T) {
env := setupSourceTest(t)
wh := seedWebhookWithRetention(t, env.db, 7)
archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
target := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
seedArchive(
t, delivery.ArchivePath(env.dbMgr, &wh, archive), rows, bodySize,
t, archive.Path(env.dbMgr, &wh, target), rows, bodySize,
)
srv, accessLog := limitedServer(t, env, limit)
req, err := http.NewRequestWithContext(
t.Context(), http.MethodGet,
srv.URL+downloadPath(wh.ID, archive.ID), nil,
srv.URL+downloadPath(wh.ID, target.ID), nil,
)
require.NoError(t, err)
+5 -4
View File
@@ -6,6 +6,7 @@ import (
"strconv"
"github.com/go-chi/chi"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
@@ -96,8 +97,8 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
// HandleTargetEditSubmit handles the target edit form submission.
func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
h.renameMu.Lock()
defer h.renameMu.Unlock()
h.archives.Lock()
defer h.archives.Unlock()
webhook, target, ok := h.ownedTarget(w, r)
if !ok {
@@ -157,7 +158,7 @@ func (h *Handlers) applyTargetEdit(
}
// A new name renames the archive file before it is saved (see
// delivery.Engine.Rename). If either step fails, it goes back to
// archive.Writers.Rename). If either step fails, it goes back to
// the name that is still stored.
err = h.renameTargetArchive(
target, webhook.Name, target.Name, edited.Name,
@@ -178,7 +179,7 @@ func (h *Handlers) applyTargetEdit(
)
}
if errors.Is(err, delivery.ErrArchiveNameTaken) {
if errors.Is(err, archive.ErrNameTaken) {
h.renderTargetEdit(
w, r, webhook, target, in,
"Not saved: "+err.Error()+
+4 -3
View File
@@ -10,6 +10,7 @@ import (
"github.com/dustin/go-humanize"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
@@ -242,7 +243,7 @@ func readTargetDeliveries(
func (h *Handlers) archiveFileView(
webhook *database.Webhook, target *database.Target, now time.Time,
) *ArchiveFileView {
current, err := delivery.ArchivePathAt(h.dbMgr, webhook, target, now)
current, err := archive.PathAt(h.dbMgr, webhook, target, now)
if err != nil {
return h.archiveUnreadable(&ArchiveFileView{}, target, err)
}
@@ -254,8 +255,8 @@ func (h *Handlers) archiveFileView(
view.Note = "not created yet"
}
files, err := delivery.StatArchive(
delivery.ArchivePath(h.dbMgr, webhook, target),
files, err := archive.Stat(
archive.Path(h.dbMgr, webhook, target),
)
switch {
+3 -3
View File
@@ -10,8 +10,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/session"
@@ -38,10 +38,10 @@ func TestHandleSourceDetail_ShowsArchiveFile(t *testing.T) {
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
archive := seedTarget(t, db, wh.ID, database.TargetTypeDatabase)
target := seedTarget(t, db, wh.ID, database.TargetTypeDatabase)
seedTarget(t, db, wh.ID, database.TargetTypeLog)
path := delivery.ArchivePath(dbMgr, wh, archive)
path := archive.Path(dbMgr, wh, target)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.Equal(t, 1, strings.Count(body, "Archive file:"))
+5 -5
View File
@@ -77,8 +77,8 @@ func (h *Handlers) deleteWebhookResources(
"user_id", userID,
)
// Release the delivery engine's per-webhook archiving state
// so a deleted webhook's archive writer (and any handle open
// Release the webhook's archive writers so a deleted
// webhook's archive writer (and any handle open
// within its debounce window) does not linger for the
// process lifetime. The archive file itself is deliberately
// left on disk; see evictArchiveWriter.
@@ -150,9 +150,9 @@ func (h *Handlers) commitWebhookDeletion(
return tx.Commit().Error
}
// evictArchiveWriter asks the delivery engine to drop the cached
// archive writers of a webhook's database targets, closing their
// archive file handles.
// evictArchiveWriter asks the archive writers to drop the cached
// writers of a webhook's database targets, closing their archive
// file handles.
//
// The archive database files are NOT deleted. Unlike the event
// database — which is per-webhook working storage and is
+5 -5
View File
@@ -6,8 +6,8 @@ import (
"strconv"
"github.com/go-chi/chi"
"sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// HandleSourceEdit shows the form to edit a webhook.
@@ -59,8 +59,8 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
sourceID := chi.URLParam(r, "sourceID")
h.renameMu.Lock()
defer h.renameMu.Unlock()
h.archives.Lock()
defer h.archives.Unlock()
var webhook database.Webhook
@@ -130,7 +130,7 @@ func (h *Handlers) applyWebhookEdit(
edited.RetentionDays = retentionDays
// A new name renames the archive files before it is saved (see
// delivery.Engine.Rename). If either step fails, the same targets'
// archive.Writers.Rename). If either step fails, the same targets'
// archives go back to the name that is still stored, without
// reading the main database again.
targets, err := h.renameWebhookArchives(
@@ -150,7 +150,7 @@ func (h *Handlers) applyWebhookEdit(
)
}
if errors.Is(err, delivery.ErrArchiveNameTaken) {
if errors.Is(err, archive.ErrNameTaken) {
h.renderWebhookEdit(
w, r, webhook, name, description, retention,
"Not saved: "+err.Error()+