Name each database target's archive for its webhook and target (closes #376)
check / check (push) Successful in 3m14s
check / check (push) Successful in 3m14s
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. Renaming a webhook or a target renames its archive files under the archive writer's lock, before the new name is saved, and back again if the save fails. 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
This commit is contained in:
@@ -62,7 +62,7 @@ type HandlersParams struct {
|
||||
Session *session.Session
|
||||
Middleware *middleware.Middleware
|
||||
Notifier delivery.Notifier
|
||||
Evictor delivery.WebhookEvictor
|
||||
Archives delivery.Archives
|
||||
SSRFGuard *delivery.Guard
|
||||
}
|
||||
|
||||
@@ -77,7 +77,7 @@ type Handlers struct {
|
||||
session *session.Session
|
||||
mw *middleware.Middleware
|
||||
notifier delivery.Notifier
|
||||
evictor delivery.WebhookEvictor
|
||||
archives delivery.Archives
|
||||
mtr *metrics.Set
|
||||
templates map[string]*template.Template
|
||||
|
||||
@@ -128,7 +128,7 @@ func New(
|
||||
s.session = params.Session
|
||||
s.mw = params.Middleware
|
||||
s.notifier = params.Notifier
|
||||
s.evictor = params.Evictor
|
||||
s.archives = params.Archives
|
||||
s.mtr = metrics.Default()
|
||||
s.ssrf = params.SSRFGuard
|
||||
|
||||
|
||||
@@ -51,23 +51,67 @@ func (n *recordingNotifier) Tasks() []delivery.Task {
|
||||
return out
|
||||
}
|
||||
|
||||
// recordingEvictor is a delivery.WebhookEvictor that records
|
||||
// the webhook ids it was asked to evict, so a test can prove
|
||||
// that a deletion path reached the delivery engine.
|
||||
type recordingEvictor struct {
|
||||
mu sync.Mutex
|
||||
evicted []string
|
||||
// recordingArchives is a delivery.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
|
||||
// rename fails with the given error.
|
||||
type recordingArchives struct {
|
||||
mu sync.Mutex
|
||||
evicted []string
|
||||
evictedTargets []string
|
||||
renames []archiveRename
|
||||
renameErr error
|
||||
}
|
||||
|
||||
func (r *recordingEvictor) EvictWebhook(webhookID string) {
|
||||
// errInjectedRename is the failure a test hands FailRenames.
|
||||
var errInjectedRename = errors.New("injected rename failure")
|
||||
|
||||
// archiveRename is one recorded RenameArchive call.
|
||||
type archiveRename struct {
|
||||
TargetID string
|
||||
WebhookName string
|
||||
TargetName string
|
||||
}
|
||||
|
||||
func (r *recordingArchives) EvictWebhook(webhookID string) {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
r.evicted = append(r.evicted, webhookID)
|
||||
}
|
||||
|
||||
func (r *recordingArchives) EvictTarget(targetID string) {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
r.evictedTargets = append(r.evictedTargets, targetID)
|
||||
}
|
||||
|
||||
func (r *recordingArchives) RenameArchive(
|
||||
targetID, webhookName, targetName string,
|
||||
) error {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
r.renames = append(r.renames, archiveRename{
|
||||
TargetID: targetID,
|
||||
WebhookName: webhookName,
|
||||
TargetName: targetName,
|
||||
})
|
||||
|
||||
return r.renameErr
|
||||
}
|
||||
|
||||
// FailRenames makes every later rename fail with err.
|
||||
func (r *recordingArchives) FailRenames(err error) {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
r.renameErr = err
|
||||
}
|
||||
|
||||
// Evicted returns a copy of the recorded webhook ids.
|
||||
func (r *recordingEvictor) Evicted() []string {
|
||||
func (r *recordingArchives) Evicted() []string {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
@@ -77,6 +121,28 @@ func (r *recordingEvictor) Evicted() []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// EvictedTargets returns a copy of the recorded target ids.
|
||||
func (r *recordingArchives) EvictedTargets() []string {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
out := make([]string, len(r.evictedTargets))
|
||||
copy(out, r.evictedTargets)
|
||||
|
||||
return out
|
||||
}
|
||||
|
||||
// Renames returns a copy of the recorded renames.
|
||||
func (r *recordingArchives) Renames() []archiveRename {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
|
||||
out := make([]archiveRename, len(r.renames))
|
||||
copy(out, r.renames)
|
||||
|
||||
return out
|
||||
}
|
||||
|
||||
func newTestApp(
|
||||
t *testing.T,
|
||||
targets ...any,
|
||||
@@ -103,10 +169,10 @@ func newTestApp(
|
||||
func(n *recordingNotifier) delivery.Notifier {
|
||||
return n
|
||||
},
|
||||
func() *recordingEvictor {
|
||||
return &recordingEvictor{}
|
||||
func() *recordingArchives {
|
||||
return &recordingArchives{}
|
||||
},
|
||||
func(r *recordingEvictor) delivery.WebhookEvictor {
|
||||
func(r *recordingArchives) delivery.Archives {
|
||||
return r
|
||||
},
|
||||
middleware.New,
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"gorm.io/gorm"
|
||||
"gorm.io/gorm/clause"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
"sneak.berlin/go/webhooker/internal/delivery"
|
||||
"sneak.berlin/go/webhooker/internal/handlers"
|
||||
"sneak.berlin/go/webhooker/internal/session"
|
||||
)
|
||||
@@ -147,18 +148,19 @@ func failDeleteOnTable(
|
||||
}
|
||||
|
||||
// archivePathFor returns the archive database path the
|
||||
// delivery engine would use for a webhook: beside the webhook's
|
||||
// event database in the data directory.
|
||||
// delivery engine would use for a database target: beside the
|
||||
// webhook's event database in the data directory.
|
||||
func archivePathFor(
|
||||
t *testing.T,
|
||||
mgr *database.WebhookDBManager,
|
||||
webhookID string,
|
||||
wh *database.Webhook,
|
||||
tgt *database.Target,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
return filepath.Join(
|
||||
filepath.Dir(mgr.DBPath(webhookID)),
|
||||
"archive-"+webhookID+".db",
|
||||
filepath.Dir(mgr.DBPath(wh.ID)),
|
||||
delivery.ArchiveFileName(wh.Name, tgt.Name, tgt.ID),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -195,8 +197,8 @@ func postRequest(
|
||||
|
||||
// TestHandleSourceDelete_EvictsArchiveWriter proves that
|
||||
// deleting a webhook reaches the delivery engine and releases
|
||||
// the webhook's archive writer, exercised through the real
|
||||
// deletion handler rather than by calling the evictor directly.
|
||||
// the webhook's archive writers, exercised through the real
|
||||
// deletion handler rather than by calling the engine directly.
|
||||
func TestHandleSourceDelete_EvictsArchiveWriter(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -204,7 +206,7 @@ func TestHandleSourceDelete_EvictsArchiveWriter(t *testing.T) {
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
ev *recordingEvictor
|
||||
ev *recordingArchives
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db, &ev)
|
||||
@@ -254,9 +256,10 @@ func TestHandleSourceDelete_KeepsArchiveFile(t *testing.T) {
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
wh := seedWebhook(t, db)
|
||||
tgt := seedTarget(t, db, wh.ID, database.TargetTypeDatabase)
|
||||
|
||||
// Place an archive file where the delivery engine would.
|
||||
archivePath := archivePathFor(t, mgr, wh.ID)
|
||||
archivePath := archivePathFor(t, mgr, wh, tgt)
|
||||
require.NoError(
|
||||
t,
|
||||
writeArchivePlaceholder(archivePath),
|
||||
@@ -435,68 +438,17 @@ func TestHandleSourceDelete_RemovesConfigAndEventDatabase(
|
||||
)
|
||||
}
|
||||
|
||||
// TestHandleTargetDelete_EvictsWhenLastDatabaseTargetGone
|
||||
// proves that removing the last database target releases the
|
||||
// archive writer.
|
||||
func TestHandleTargetDelete_EvictsWhenLastDatabaseTargetGone(
|
||||
t *testing.T,
|
||||
) {
|
||||
// TestHandleTargetDelete_EvictsThatTarget proves that deleting a
|
||||
// database target releases that target's archive writer and no
|
||||
// other: the webhook's other database target keeps its own.
|
||||
func TestHandleTargetDelete_EvictsThatTarget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
ev *recordingEvictor
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db, &ev)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
wh := seedWebhook(t, db)
|
||||
tgt := seedTarget(
|
||||
t, db, wh.ID, database.TargetTypeDatabase,
|
||||
)
|
||||
|
||||
cookies := authenticatedCookies(
|
||||
t, sess, deleteTestUserID, deleteTestUsername,
|
||||
)
|
||||
|
||||
req := postRequest(
|
||||
"/hook/"+wh.ID+"/targets/"+tgt.ID+"/delete",
|
||||
cookies,
|
||||
map[string]string{
|
||||
paramSourceID: wh.ID,
|
||||
paramTargetID: tgt.ID,
|
||||
},
|
||||
)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
h.HandleTargetDelete().ServeHTTP(w, req)
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
assert.Equal(
|
||||
t, []string{wh.ID}, ev.Evicted(),
|
||||
"removing the last database target should evict",
|
||||
)
|
||||
}
|
||||
|
||||
// TestHandleTargetDelete_KeepsWriterWhenDatabaseTargetRemains
|
||||
// proves that deleting one of several database targets leaves
|
||||
// the still-needed archive writer alone: the surviving target
|
||||
// keeps archiving to the same file, so the writer must stay.
|
||||
func TestHandleTargetDelete_KeepsWriterWhenDatabaseTargetRemains(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
var (
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
ev *recordingEvictor
|
||||
ev *recordingArchives
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db, &ev)
|
||||
@@ -527,17 +479,17 @@ func TestHandleTargetDelete_KeepsWriterWhenDatabaseTargetRemains(
|
||||
h.HandleTargetDelete().ServeHTTP(w, req)
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
assert.Empty(
|
||||
t, ev.Evicted(),
|
||||
"a second database target still needs the writer",
|
||||
assert.Equal(
|
||||
t, []string{doomed.ID}, ev.EvictedTargets(),
|
||||
"deleting a database target should evict its writer",
|
||||
)
|
||||
assert.Empty(t, ev.Evicted(), "the webhook is not deleted")
|
||||
}
|
||||
|
||||
// TestHandleTargetDelete_KeepsWriterWhenOtherTypeDeleted proves
|
||||
// that deleting a target of an unrelated type leaves a
|
||||
// still-needed archive writer alone: the webhook's database
|
||||
// target is untouched, so its writer must stay.
|
||||
func TestHandleTargetDelete_KeepsWriterWhenOtherTypeDeleted(
|
||||
// TestHandleTargetDelete_IgnoresAnotherWebhooksTarget proves that
|
||||
// a target id from the URL that is not a target of the webhook
|
||||
// deletes nothing and so evicts nothing.
|
||||
func TestHandleTargetDelete_IgnoresAnotherWebhooksTarget(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
@@ -546,7 +498,7 @@ func TestHandleTargetDelete_KeepsWriterWhenOtherTypeDeleted(
|
||||
h *handlers.Handlers
|
||||
sess *session.Session
|
||||
db *database.Database
|
||||
ev *recordingEvictor
|
||||
ev *recordingArchives
|
||||
)
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db, &ev)
|
||||
@@ -555,19 +507,20 @@ func TestHandleTargetDelete_KeepsWriterWhenOtherTypeDeleted(
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
wh := seedWebhook(t, db)
|
||||
seedTarget(t, db, wh.ID, database.TargetTypeDatabase)
|
||||
other := seedTarget(t, db, wh.ID, database.TargetTypeLog)
|
||||
elsewhere := seedTarget(
|
||||
t, db, seedWebhook(t, db).ID, database.TargetTypeDatabase,
|
||||
)
|
||||
|
||||
cookies := authenticatedCookies(
|
||||
t, sess, deleteTestUserID, deleteTestUsername,
|
||||
)
|
||||
|
||||
req := postRequest(
|
||||
"/hook/"+wh.ID+"/targets/"+other.ID+"/delete",
|
||||
"/hook/"+wh.ID+"/targets/"+elsewhere.ID+"/delete",
|
||||
cookies,
|
||||
map[string]string{
|
||||
paramSourceID: wh.ID,
|
||||
paramTargetID: other.ID,
|
||||
paramTargetID: elsewhere.ID,
|
||||
},
|
||||
)
|
||||
w := httptest.NewRecorder()
|
||||
@@ -576,7 +529,7 @@ func TestHandleTargetDelete_KeepsWriterWhenOtherTypeDeleted(
|
||||
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
assert.Empty(
|
||||
t, ev.Evicted(),
|
||||
"a surviving database target must keep its writer",
|
||||
t, ev.EvictedTargets(),
|
||||
"another webhook's target must not be evicted",
|
||||
)
|
||||
}
|
||||
|
||||
@@ -559,6 +559,7 @@ func (h *Handlers) applyWebhookEdit(
|
||||
return
|
||||
}
|
||||
|
||||
oldName := webhook.Name
|
||||
webhook.Name = name
|
||||
webhook.Description = r.PostFormValue("description")
|
||||
|
||||
@@ -581,8 +582,24 @@ func (h *Handlers) applyWebhookEdit(
|
||||
|
||||
webhook.RetentionDays = retentionDays
|
||||
|
||||
err := h.db.DB().Save(webhook).Error
|
||||
// The archive files are renamed before the new name is saved
|
||||
// (see delivery.Engine.RenameArchive). If either step fails,
|
||||
// they go back to the name that is still stored.
|
||||
err := h.renameWebhookArchives(webhook.ID, webhook.Name)
|
||||
if err == nil {
|
||||
err = h.db.DB().Save(webhook).Error
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
restoreErr := h.renameWebhookArchives(webhook.ID, oldName)
|
||||
if restoreErr != nil {
|
||||
h.log.Error(
|
||||
"failed to rename archives back",
|
||||
"webhook_id", webhook.ID,
|
||||
"error", restoreErr,
|
||||
)
|
||||
}
|
||||
|
||||
h.serverError(w, "failed to update webhook", err)
|
||||
|
||||
return
|
||||
@@ -717,11 +734,11 @@ func (h *Handlers) commitWebhookDeletion(
|
||||
return tx.Commit().Error
|
||||
}
|
||||
|
||||
// evictArchiveWriter asks the delivery engine to drop its
|
||||
// cached archive writer for a webhook, closing the archive file
|
||||
// handle.
|
||||
// evictArchiveWriter asks the delivery engine to drop the cached
|
||||
// archive writers of a webhook's database targets, closing their
|
||||
// archive file handles.
|
||||
//
|
||||
// The archive database file is NOT deleted. Unlike the event
|
||||
// The archive database files are NOT deleted. Unlike the event
|
||||
// database — which is per-webhook working storage and is
|
||||
// hard-deleted with the webhook — an archive is explicitly
|
||||
// long-term storage that an operator may want to keep or move
|
||||
@@ -729,50 +746,57 @@ func (h *Handlers) commitWebhookDeletion(
|
||||
// deleting a webhook would be a surprising and unrecoverable
|
||||
// data loss, so the file is left for the operator to handle.
|
||||
func (h *Handlers) evictArchiveWriter(webhookID string) {
|
||||
if h.evictor == nil {
|
||||
if h.archives == nil {
|
||||
return
|
||||
}
|
||||
|
||||
h.evictor.EvictWebhook(webhookID)
|
||||
h.archives.EvictWebhook(webhookID)
|
||||
}
|
||||
|
||||
// evictArchiveWriterIfUnused releases a webhook's archive
|
||||
// writer once the webhook has no database target left to feed
|
||||
// it.
|
||||
//
|
||||
// It is called after any child resource of a webhook is
|
||||
// deleted, and is correct without knowing which kind was: it
|
||||
// evicts only when no database target remains, so deleting one
|
||||
// of several database targets — or deleting an unrelated
|
||||
// target type — leaves a still-needed writer alone. When no
|
||||
// database target ever existed there is no writer and eviction
|
||||
// is a no-op. Soft-deleted targets are excluded by GORM's
|
||||
// default scope, so the row just deleted is not counted.
|
||||
func (h *Handlers) evictArchiveWriterIfUnused(webhookID string) {
|
||||
var remaining int64
|
||||
// evictTargetArchiveWriter is evictArchiveWriter for one deleted
|
||||
// target, and leaves its archive file on disk for the same reason.
|
||||
// A target that is not a database target has no writer, and
|
||||
// evicting it does nothing.
|
||||
func (h *Handlers) evictTargetArchiveWriter(targetID string) {
|
||||
if h.archives == nil {
|
||||
return
|
||||
}
|
||||
|
||||
h.archives.EvictTarget(targetID)
|
||||
}
|
||||
|
||||
// renameWebhookArchives renames the archive file of every database
|
||||
// target of a webhook for the webhook name webhookName, keeping
|
||||
// each target's own name. It stops at the first failure.
|
||||
func (h *Handlers) renameWebhookArchives(
|
||||
webhookID, webhookName string,
|
||||
) error {
|
||||
if h.archives == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
var targets []database.Target
|
||||
|
||||
err := h.db.DB().
|
||||
Model(&database.Target{}).
|
||||
Where(
|
||||
"webhook_id = ? AND type = ?",
|
||||
webhookID, database.TargetTypeDatabase,
|
||||
).
|
||||
Count(&remaining).Error
|
||||
Find(&targets).Error
|
||||
if err != nil {
|
||||
h.log.Error(
|
||||
"failed to count remaining database targets",
|
||||
"webhook_id", webhookID,
|
||||
"error", err,
|
||||
return err
|
||||
}
|
||||
|
||||
for i := range targets {
|
||||
err = h.archives.RenameArchive(
|
||||
targets[i].ID, webhookName, targets[i].Name,
|
||||
)
|
||||
|
||||
return
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if remaining > 0 {
|
||||
return
|
||||
}
|
||||
|
||||
h.evictArchiveWriter(webhookID)
|
||||
return nil
|
||||
}
|
||||
|
||||
// ownedWebhook resolves the request's sourceID parameter to a
|
||||
@@ -1649,27 +1673,26 @@ func (h *Handlers) HandleEntrypointDelete() http.HandlerFunc {
|
||||
)
|
||||
}
|
||||
|
||||
// HandleTargetDelete handles deleting a target. Deleting the
|
||||
// last database target of a webhook leaves its archive writer
|
||||
// with nothing to write, so the writer is evicted and its
|
||||
// handle closed; the archive file is left on disk.
|
||||
// HandleTargetDelete handles deleting a target. A deleted
|
||||
// database target's archive writer is evicted and its handle
|
||||
// closed; the archive file is left on disk.
|
||||
func (h *Handlers) HandleTargetDelete() http.HandlerFunc {
|
||||
return h.deleteChildResource(
|
||||
"targetID", &database.Target{},
|
||||
"failed to delete target",
|
||||
h.evictArchiveWriterIfUnused,
|
||||
h.evictTargetArchiveWriter,
|
||||
)
|
||||
}
|
||||
|
||||
// deleteChildResource returns a handler that deletes a child
|
||||
// resource (entrypoint or target) belonging to a webhook. The
|
||||
// optional afterDelete hook runs with the webhook's id once the
|
||||
// delete has succeeded, before the redirect.
|
||||
// optional afterDelete hook runs with the child's id once the
|
||||
// delete has removed it, before the redirect.
|
||||
func (h *Handlers) deleteChildResource(
|
||||
idParam string,
|
||||
model any,
|
||||
errMsg string,
|
||||
afterDelete func(webhookID string),
|
||||
afterDelete func(childID string),
|
||||
) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
userID, ok := h.getUserID(r)
|
||||
@@ -1709,8 +1732,10 @@ func (h *Handlers) deleteChildResource(
|
||||
return
|
||||
}
|
||||
|
||||
if afterDelete != nil {
|
||||
afterDelete(webhook.ID)
|
||||
// Only for a row this webhook really had: the id came from
|
||||
// the URL and may name another webhook's child.
|
||||
if afterDelete != nil && result.RowsAffected > 0 {
|
||||
afterDelete(childID)
|
||||
}
|
||||
|
||||
http.Redirect(
|
||||
|
||||
@@ -187,6 +187,7 @@ func storedRetentionDays(
|
||||
type sourceTestEnv struct {
|
||||
handlers *handlers.Handlers
|
||||
db *database.Database
|
||||
archives *recordingArchives
|
||||
cookies []*http.Cookie
|
||||
}
|
||||
|
||||
@@ -199,7 +200,9 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
|
||||
|
||||
var db *database.Database
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db)
|
||||
var archives *recordingArchives
|
||||
|
||||
app := newTestApp(t, &h, &sess, &db, &archives)
|
||||
app.RequireStart()
|
||||
|
||||
t.Cleanup(app.RequireStop)
|
||||
@@ -207,6 +210,7 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
|
||||
return &sourceTestEnv{
|
||||
handlers: h,
|
||||
db: db,
|
||||
archives: archives,
|
||||
cookies: authenticatedCookies(
|
||||
t, sess, sourceTestUserID, "sourceuser",
|
||||
),
|
||||
@@ -498,6 +502,74 @@ func TestHandleSourceEditSubmit_EmptyRetentionLeavesValueUnchanged(
|
||||
assert.Equal(t, 7, storedRetentionDays(t, env.db, wh.ID))
|
||||
}
|
||||
|
||||
// renamedWebhookName is the name the rename tests give a webhook.
|
||||
const renamedWebhookName = "Renamed"
|
||||
|
||||
// TestHandleSourceEditSubmit_RenamesArchives proves that renaming a
|
||||
// webhook renames the archive of each of its database targets, and
|
||||
// asks nothing of its other targets.
|
||||
func TestHandleSourceEditSubmit_RenamesArchives(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
wh := seedWebhookWithRetention(t, env.db, 7)
|
||||
first := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
|
||||
second := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
|
||||
seedTarget(t, env.db, wh.ID, database.TargetTypeLog)
|
||||
|
||||
wh.Name = renamedWebhookName
|
||||
|
||||
w := submitEdit(t, env, wh, "")
|
||||
require.Equal(t, http.StatusSeeOther, w.Code)
|
||||
|
||||
assert.ElementsMatch(
|
||||
t,
|
||||
[]archiveRename{
|
||||
{first.ID, renamedWebhookName, first.Name},
|
||||
{second.ID, renamedWebhookName, second.Name},
|
||||
},
|
||||
env.archives.Renames(),
|
||||
)
|
||||
}
|
||||
|
||||
// TestHandleSourceEditSubmit_FailedRenameKeepsTheName proves that a
|
||||
// webhook whose archive cannot be renamed keeps its stored name, so
|
||||
// the name on disk and the name in the UI do not part, and that the
|
||||
// handler puts back what it may already have moved.
|
||||
func TestHandleSourceEditSubmit_FailedRenameKeepsTheName(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
wh := seedWebhookWithRetention(t, env.db, 7)
|
||||
tgt := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
|
||||
|
||||
env.archives.FailRenames(errInjectedRename)
|
||||
|
||||
oldName := wh.Name
|
||||
wh.Name = renamedWebhookName
|
||||
|
||||
w := submitEdit(t, env, wh, "")
|
||||
require.Equal(t, http.StatusInternalServerError, w.Code)
|
||||
|
||||
var stored database.Webhook
|
||||
|
||||
require.NoError(
|
||||
t, env.db.DB().First(&stored, "id = ?", wh.ID).Error,
|
||||
)
|
||||
assert.Equal(t, oldName, stored.Name)
|
||||
|
||||
assert.Equal(
|
||||
t,
|
||||
[]archiveRename{
|
||||
{tgt.ID, renamedWebhookName, tgt.Name},
|
||||
{tgt.ID, oldName, tgt.Name},
|
||||
},
|
||||
env.archives.Renames(),
|
||||
)
|
||||
}
|
||||
|
||||
// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
|
||||
// the removed max="365" cap used to break: render the edit form for a
|
||||
// retain-forever webhook, confirm the pre-filled sentinel is not capped
|
||||
|
||||
@@ -152,11 +152,30 @@ func (h *Handlers) applyTargetEdit(
|
||||
target.MaxRetries = retries
|
||||
}
|
||||
|
||||
oldName := target.Name
|
||||
target.Name = name
|
||||
target.Config = configJSON
|
||||
|
||||
err = h.db.DB().Save(target).Error
|
||||
// The archive file is renamed before the new name is saved (see
|
||||
// delivery.Engine.RenameArchive). If either step fails, it goes
|
||||
// back to the name that is still stored.
|
||||
err = h.renameTargetArchive(target, webhook.Name, name)
|
||||
if err == nil {
|
||||
err = h.db.DB().Save(target).Error
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
restoreErr := h.renameTargetArchive(
|
||||
target, webhook.Name, oldName,
|
||||
)
|
||||
if restoreErr != nil {
|
||||
h.log.Error(
|
||||
"failed to rename archive back",
|
||||
"target_id", target.ID,
|
||||
"error", restoreErr,
|
||||
)
|
||||
}
|
||||
|
||||
h.serverError(w, "failed to update target", err)
|
||||
|
||||
return
|
||||
@@ -167,6 +186,23 @@ func (h *Handlers) applyTargetEdit(
|
||||
)
|
||||
}
|
||||
|
||||
// renameTargetArchive renames a database target's archive file for
|
||||
// the given webhook and target names. Other target types have no
|
||||
// archive.
|
||||
func (h *Handlers) renameTargetArchive(
|
||||
target *database.Target,
|
||||
webhookName, targetName string,
|
||||
) error {
|
||||
if h.archives == nil ||
|
||||
target.Type != database.TargetTypeDatabase {
|
||||
return nil
|
||||
}
|
||||
|
||||
return h.archives.RenameArchive(
|
||||
target.ID, webhookName, targetName,
|
||||
)
|
||||
}
|
||||
|
||||
// renderTargetEdit renders the target edit page with an optional
|
||||
// error message.
|
||||
func (h *Handlers) renderTargetEdit(
|
||||
|
||||
@@ -635,3 +635,48 @@ func assertWebhookOfAnotherUser404s(
|
||||
|
||||
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||
}
|
||||
|
||||
// TestHandleTargetEditSubmit_RenamesArchive proves that renaming a
|
||||
// database target renames its archive, that a target of another type
|
||||
// has no archive to rename, and that a target whose archive cannot be
|
||||
// renamed keeps its stored name.
|
||||
func TestHandleTargetEditSubmit_RenamesArchive(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
env := setupSourceTest(t)
|
||||
wh := seedWebhookWithRetention(t, env.db, 7)
|
||||
archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
|
||||
|
||||
w := submitTargetEdit(
|
||||
env, wh.ID, archive.ID, url.Values{"name": {"Long Term"}},
|
||||
)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
||||
assert.Equal(
|
||||
t,
|
||||
[]archiveRename{{archive.ID, wh.Name, "Long Term"}},
|
||||
env.archives.Renames(),
|
||||
)
|
||||
|
||||
httpWebhook, httpTarget := seedHTTPTarget(t, env, "", "")
|
||||
|
||||
w = submitTargetEdit(
|
||||
env, httpWebhook.ID, httpTarget.ID,
|
||||
editForm(editOriginalURL, "", ""),
|
||||
)
|
||||
require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
|
||||
assert.Len(
|
||||
t, env.archives.Renames(), 1,
|
||||
"an HTTP target has no archive to rename",
|
||||
)
|
||||
|
||||
env.archives.FailRenames(errInjectedRename)
|
||||
|
||||
w = submitTargetEdit(
|
||||
env, wh.ID, archive.ID, url.Values{"name": {"Again"}},
|
||||
)
|
||||
require.Equal(t, http.StatusInternalServerError, w.Code)
|
||||
assert.Equal(
|
||||
t, "Long Term", storedTarget(t, env, archive.ID).Name,
|
||||
"a target whose archive was not renamed keeps its name",
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user