Allow retention_days of 0 to mean retain forever (closes #79)
Some checks failed
check / check (push) Has been cancelled

Rewrites retention_days=0 to the RetentionForeverDays sentinel (365 * 1000)
in Webhook.BeforeSave, so the GORM column default cannot win the race. The
reaper skips retain-forever webhooks before building any query.

Also bounds the reaper's cutoff arithmetic: a time.Duration is int64
nanoseconds, so day counts above MaxFiniteRetentionDays (106751) overflowed
and wrapped the cutoff into the future, where created_at < cutoff matched
every row and the sweep deleted everything. parseRetentionDays now rejects
finite values above the ceiling, and retentionCutoff saturates so rows
written by older versions cannot reach it either.

Views render RetentionLabel() rather than the raw sentinel.
This commit was merged in pull request #96.
This commit is contained in:
2026-08-11 14:35:34 +02:00
parent c2cd2c440b
commit e50a79ced9
13 changed files with 1258 additions and 61 deletions

View File

@@ -77,7 +77,7 @@ func createWebhook(
wh := &database.Webhook{
UserID: uuid.New().String(),
Name: "test-webhook",
Name: testWebhookName,
RetentionDays: retentionDays,
}
require.NoError(
@@ -85,10 +85,11 @@ func createWebhook(
db.Omit(clause.Associations).Create(wh).Error,
)
// The RetentionDays column carries a GORM default of 30, so a
// zero (or negative) value passed to Create is replaced by that
// default. Force the requested value explicitly so the
// retain-forever (<= 0) path can be exercised.
// Webhook.BeforeSave rewrites a non-positive RetentionDays to the
// retain-forever sentinel, and the column's GORM default would
// otherwise substitute 30. Force the requested value with a
// column-level update so tests can plant legacy rows that predate
// the sentinel and still carry a literal 0 or negative value.
require.NoError(
t,
db.Model(wh).
@@ -98,6 +99,30 @@ func createWebhook(
return wh.ID
}
// createWebhookNormally inserts a webhook through the ordinary Create
// path, with no column-level forcing, so Webhook.BeforeSave applies
// exactly as it does in production. Passing 0 therefore yields a row
// holding the RetentionForeverDays sentinel.
func createWebhookNormally(
t *testing.T,
db *gorm.DB,
retentionDays int,
) string {
t.Helper()
wh := &database.Webhook{
UserID: uuid.New().String(),
Name: testWebhookName,
RetentionDays: retentionDays,
}
require.NoError(
t,
db.Omit(clause.Associations).Create(wh).Error,
)
return wh.ID
}
// eventChain is the set of row IDs seeded for a single event.
type eventChain struct {
eventID string
@@ -256,12 +281,111 @@ func TestRetentionReaper_ReapsExpiredKeepsRecent(t *testing.T) {
assertChainPresent(t, db, recent)
}
// TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep covers the
// end-to-end retain-forever path: a webhook created the normal way with
// a requested retention of 0 lands on the RetentionForeverDays
// sentinel, and the reaper leaves its ancient events alone while still
// reaping a finite-retention webhook in the very same sweep.
func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
t *testing.T,
) {
t.Parallel()
env := setupRetentionTest(t)
foreverID := createWebhookNormally(t, env.mainDB.DB(), 0)
var stored database.Webhook
require.NoError(
t,
env.mainDB.DB().Where("id = ?", foreverID).
First(&stored).Error,
)
require.Equal(
t,
database.RetentionForeverDays,
stored.RetentionDays,
"a requested retention of 0 must persist as the sentinel",
)
finiteID := createWebhookNormally(t, env.mainDB.DB(), 30)
foreverDB, err := env.mgr.GetDB(foreverID)
require.NoError(t, err)
finiteDB, err := env.mgr.GetDB(finiteID)
require.NoError(t, err)
ancient := time.Now().Add(-365 * 24 * time.Hour)
kept := seedEventChain(t, foreverDB, foreverID, ancient)
doomed := seedEventChain(t, finiteDB, finiteID, ancient)
env.reaper.ExportSweep(context.Background())
assertChainPresent(t, foreverDB, kept)
assertChainGone(t, finiteDB, doomed)
}
// TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents pins the
// overflow that made a large finite retention destroy everything.
//
// The cutoff is a time.Duration, an int64 nanosecond count. A day
// count above MaxFiniteRetentionDays multiplied out unclamped wraps
// negative, so subtracting it moves the cutoff into the far future,
// where "created_at < cutoff" matches every row: an event created a
// moment ago, and its delivery and delivery result, were all deleted
// on the first sweep. 200000 is inside that band and below the
// retain-forever sentinel, so it is treated as a finite policy and
// really does reach the arithmetic.
//
// The row is planted at the column level because such a value can no
// longer be submitted through the form; the point of the test is that
// a row from an older version, or a future call site, still cannot
// trigger the wipe.
func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
t *testing.T,
) {
t.Parallel()
env := setupRetentionTest(t)
const overflowingRetentionDays = 200000
require.Greater(
t,
overflowingRetentionDays,
database.MaxFiniteRetentionDays,
"the test value must exceed what the cutoff can represent",
)
require.Less(
t,
overflowingRetentionDays,
database.RetentionForeverDays,
"the test value must not be rescued by the forever skip",
)
webhookID := createWebhook(
t, env.mainDB.DB(), overflowingRetentionDays,
)
db, err := env.mgr.GetDB(webhookID)
require.NoError(t, err)
fresh := seedEventChain(t, db, webhookID, time.Now())
env.reaper.ExportSweep(context.Background())
assertChainPresent(t, db, fresh)
}
func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) {
t.Parallel()
env := setupRetentionTest(t)
// RetentionDays of zero means retain forever.
// A legacy row written before the sentinel existed still carries a
// literal 0; the <= 0 guard must keep honouring it.
webhookID := createWebhook(t, env.mainDB.DB(), 0)
db, err := env.mgr.GetDB(webhookID)