Move the archive files into their own package (closes #541)

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

cmd/webhooker/main.go

@@ -189,8 +189,8 @@ func newApp() *fx.App {
 		fx.Provide(
 			globals.New,
 			logger.New,
-			// The plain logger the session, the middleware, the
+			// The plain logger the session, the middleware and the
-			// webhook database manager and the archive writers take.
+			// webhook database manager take.
 			func(l *logger.Logger) *slog.Logger { return l.Get() },
 			config.New,
 			database.New,

internal/archive/archive.go

@@ -26,8 +26,8 @@ const archiveNameMaxLen = 40
 // target releases the cached archive writers, whose open file
 // handles would otherwise outlive them; renaming one renames the
 // archive files, which are named for the webhook and the target
-// (see FileName). RenameLock returns the lock every rename runs under
+// (see FileName). Lock and Unlock take and release the lock every
-// (see Writers.RenameLock).
+// rename runs under (see Writers.Lock).
 //
 // It is an interface so that the handlers' tests can stand in for
 // Writers.
@@ -35,7 +35,7 @@ const archiveNameMaxLen = 40
 // Neither eviction deletes an archive file. Both are idempotent
 // and are no-ops for a webhook or target with no writer.
 type Archives interface {
-	RenameLock() *sync.Mutex
+	sync.Locker
 	EvictWebhook(webhookID string)
 	EvictTarget(targetID string)
 	Rename(targetID, webhookName, targetName string) error
@@ -74,7 +74,7 @@ type Writers struct {
 	// 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. RenameLock returns it.
+	// stored then and opens it. Lock and Unlock take and release it.
 	renameMu sync.Mutex
 }
 
@@ -88,11 +88,15 @@ func NewWriters(params WritersParams) *Writers {
 	}
 }
 
-// RenameLock implements Archives. It returns the lock every rename of
+// Lock takes the lock every rename of archive files runs under: see
-// archive files runs under: see renameMu for who holds it and for how
+// renameMu for who holds it and for how long.
-// long. Archive writes and Rename itself do not take it.
+func (ws *Writers) Lock() {
-func (ws *Writers) RenameLock() *sync.Mutex {
+	ws.renameMu.Lock()
-	return &ws.renameMu
+}
+
+// Unlock releases the lock Lock takes.
+func (ws *Writers) Unlock() {
+	ws.renameMu.Unlock()
 }
 
 // FileName returns the file name of a database target's

internal/archive/archive_test.go

@@ -711,63 +711,3 @@ func TestArchiveWriter_RenameMovesBackEveryFile(t *testing.T) {
 
 	assert.Equal(t, oldPath, w.Path())
 }
-
-// TestRenameLock_HeldByOneEditAtATime holds the rename lock, as an edit
-// does from loading the stored names until it has saved them. While it
-// is held, the edit's rename and an archive write still finish, since
-// neither takes it, and the event ends up under the new name whichever
-// ran first. A second edit waits for the lock until the first lets go.
-func TestRenameLock_HeldByOneEditAtATime(t *testing.T) {
-	t.Parallel()
-
-	env := setupArchiveTest(t)
-	tgt := env.seedDatabaseTarget(t, "")
-	event := newEvent(`{"n":1}`)
-
-	env.writers.RenameLock().Lock()
-
-	finished := make(chan error, 2)
-
-	go func() {
-		finished <- env.writers.Rename(tgt.ID, "Orders", "Long Term")
-	}()
-	go func() {
-		finished <- env.writers.Write(databaseTargetDelivery(event, tgt))
-	}()
-
-	for range 2 {
-		select {
-		case err := <-finished:
-			require.NoError(t, err)
-		case <-time.After(10 * time.Second):
-			t.Fatal("a rename or a write waited for the rename lock")
-		}
-	}
-
-	newPath := filepath.Join(
-		env.dataDir, "archive-orders-long-term-"+tgt.ID+".db",
-	)
-	assert.Equal(t, []string{event.ID}, archivedEventIDs(t, newPath))
-
-	secondEdit := make(chan struct{})
-
-	go func() {
-		env.writers.RenameLock().Lock()
-		close(secondEdit)
-		env.writers.RenameLock().Unlock()
-	}()
-
-	select {
-	case <-secondEdit:
-		t.Fatal("a second edit took the rename lock while it was held")
-	case <-time.After(100 * time.Millisecond):
-	}
-
-	env.writers.RenameLock().Unlock()
-
-	select {
-	case <-secondEdit:
-	case <-time.After(10 * time.Second):
-		t.Fatal("a second edit did not get the rename lock once released")
-	}
-}

internal/archive/download.go

@@ -106,7 +106,7 @@ type exportedName struct {
 // NewExport lists a database target's archive files for export,
 // given the path Path gives it (see archiveFiles). It opens none
 // of them. Its caller holds lock, which every rename of the target's
-// files runs under (Writers.RenameLock returns it), from reading the names
+// files runs under (Writers.Lock takes it), from reading the names
 // path is made of until it returns, so the files it lists are the ones
 // those names give.
 //

internal/handlers/handlers_test.go

@@ -61,7 +61,7 @@ func (n *recordingNotifier) Tasks() []delivery.Task {
 // 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.
-// RenameLock returns a plain mutex, as in archive.Writers.
+// Lock and Unlock are a plain mutex, as in archive.Writers.
 type recordingArchives struct {
 	renameMu       sync.Mutex
 	mu             sync.Mutex
@@ -89,8 +89,12 @@ type archiveRename struct {
 	TargetName  string
 }
 
-func (r *recordingArchives) RenameLock() *sync.Mutex {
+func (r *recordingArchives) Lock() {
-	return &r.renameMu
+	r.renameMu.Lock()
+}
+
+func (r *recordingArchives) Unlock() {
+	r.renameMu.Unlock()
 }
 
 func (r *recordingArchives) EvictWebhook(webhookID string) {

internal/handlers/target_create.go

@@ -28,8 +28,8 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 
 		sourceID := chi.URLParam(r, "sourceID")
 
-		h.archives.RenameLock().Lock()
+		h.archives.Lock()
-		defer h.archives.RenameLock().Unlock()
+		defer h.archives.Unlock()
 
 		var webhook database.Webhook
 

internal/handlers/target_download.go

@@ -82,7 +82,7 @@ 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 the rename lock, which every archive rename runs under,
+// 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
@@ -91,8 +91,8 @@ func (h *Handlers) listTargetArchive(
 	w http.ResponseWriter,
 	r *http.Request,
 ) (database.Webhook, *database.Target, *archive.Export, bool) {
-	h.archives.RenameLock().Lock()
+	h.archives.Lock()
-	defer h.archives.RenameLock().Unlock()
+	defer h.archives.Unlock()
 
 	webhook, target, ok := h.ownedTarget(w, r)
 	if !ok {
@@ -107,7 +107,7 @@ func (h *Handlers) listTargetArchive(
 
 	export, err := archive.NewExport(
 		archive.Path(h.dbMgr, &webhook, target),
-		h.archives.RenameLock(),
+		h.archives,
 		func() (string, error) {
 			return h.storedArchivePath(webhook.ID, target.ID)
 		},
@@ -124,7 +124,7 @@ func (h *Handlers) listTargetArchive(
 
 // storedArchivePath returns the path archive.Path gives a
 // database target under the names stored for it and its webhook now.
-// Its caller holds the rename lock. 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,

internal/handlers/target_edit.go

@@ -97,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.archives.RenameLock().Lock()
+		h.archives.Lock()
-		defer h.archives.RenameLock().Unlock()
+		defer h.archives.Unlock()
 
 		webhook, target, ok := h.ownedTarget(w, r)
 		if !ok {

internal/handlers/webhook_edit.go

@@ -59,8 +59,8 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
 
 		sourceID := chi.URLParam(r, "sourceID")
 
-		h.archives.RenameLock().Lock()
+		h.archives.Lock()
-		defer h.archives.RenameLock().Unlock()
+		defer h.archives.Unlock()
 
 		var webhook database.Webhook
 

internal/resetpw/resetpw_test.go

@@ -136,11 +136,7 @@ type noopNotifier struct{}
 func (n *noopNotifier) Notify([]delivery.Task) {}
 
 type noopArchives struct {
-	renameMu sync.Mutex
+	sync.Mutex
-}
-
-func (n *noopArchives) RenameLock() *sync.Mutex {
-	return &n.renameMu
 }
 
 func (n *noopArchives) EvictWebhook(string) {}

internal/server/routes_test.go

@@ -54,13 +54,9 @@ func (n *noopNotifier) Notify([]delivery.Task) {}
 
 // noopArchives satisfies handlers.New's archive.Archives
 // dependency. No test here checks what gets evicted or renamed, so
-// it records nothing. Its rename lock is a plain mutex.
+// it records nothing. Its lock is a plain mutex.
 type noopArchives struct {
-	renameMu sync.Mutex
+	sync.Mutex
-}
-
-func (e *noopArchives) RenameLock() *sync.Mutex {
-	return &e.renameMu
 }
 
 func (e *noopArchives) EvictWebhook(string) {}