From 6b511737220886bd070a9092f03871292303725f Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Sat, 23 May 2026 23:59:15 +0200 Subject: [PATCH] refactor(domain): consolidate Feed aggregate invariants in domain/feed.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gather the feed's scattered business rules — expiry, sender allow/block policy, and the email byte-size budget — into one framework-agnostic module. Expiry was duplicated across feed-service, email-processor and the rss/atom/entries routes; the sender policy and trim loop lived inline in email-processor. Each now calls a single function (isExpired, applySenderPolicy, trimToByteBudget, resolveExpiresAt). Drops the now-unused MAX_METADATA_EMAILS constant. Behaviour-preserving; adds feed.test.ts covering every invariant. Co-Authored-By: Claude Opus 4.7 --- src/config/constants.ts | 3 - src/domain/feed.test.ts | 108 +++++++++++++++++++++++++++++++++ src/domain/feed.ts | 119 +++++++++++++++++++++++++++++++++++++ src/lib/email-processor.ts | 90 +++++----------------------- src/lib/feed-service.ts | 26 +------- src/routes/atom.ts | 6 +- src/routes/entries.ts | 6 +- src/routes/rss.ts | 6 +- 8 files changed, 250 insertions(+), 114 deletions(-) create mode 100644 src/domain/feed.test.ts create mode 100644 src/domain/feed.ts diff --git a/src/config/constants.ts b/src/config/constants.ts index 5d491dc..da0ff9b 100644 --- a/src/config/constants.ts +++ b/src/config/constants.ts @@ -16,9 +16,6 @@ export const ADMIN_COOKIE_MAX_AGE = 60 * 60 * 24 * 7; // 1 week /** Maximum number of feed items exposed in RSS/Atom responses. */ export const MAX_FEED_ITEMS = 20; -/** Maximum number of email entries kept in feed metadata. */ -export const MAX_METADATA_EMAILS = 50; - /** Default WebSub lease duration (seconds). */ export const DEFAULT_LEASE_SECONDS = 86400; // 24 hours diff --git a/src/domain/feed.test.ts b/src/domain/feed.test.ts new file mode 100644 index 0000000..6fee9b6 --- /dev/null +++ b/src/domain/feed.test.ts @@ -0,0 +1,108 @@ +import { describe, it, expect } from "vitest"; +import { + resolveExpiresAt, + isExpired, + applySenderPolicy, + trimToByteBudget, +} from "./feed"; +import type { Env, FeedMetadata, EmailMetadata } from "../types"; + +const env = (overrides: Partial = {}): Env => + ({ FEED_TTL_HOURS: undefined, ...overrides }) as Env; + +describe("resolveExpiresAt", () => { + it("returns undefined when no lifetime applies", () => { + expect(resolveExpiresAt(env())).toBeUndefined(); + expect(resolveExpiresAt(env(), 0)).toBeUndefined(); + expect(resolveExpiresAt(env(), -5)).toBeUndefined(); + }); + + it("computes expiry from a supplied lifetime", () => { + const before = Date.now(); + const result = resolveExpiresAt(env(), 2)!; + expect(result).toBeGreaterThanOrEqual(before + 2 * 3_600_000); + }); + + it("lets a server-side FEED_TTL_HOURS override the client value", () => { + const before = Date.now(); + const result = resolveExpiresAt(env({ FEED_TTL_HOURS: "1" }), 999)!; + // Uses 1h (server), not 999h (client). + expect(result).toBeLessThan(before + 2 * 3_600_000); + }); +}); + +describe("isExpired", () => { + it("is false when no expiry is set", () => { + expect(isExpired({ expires_at: undefined })).toBe(false); + }); + + it("is true at or past the expiry instant", () => { + expect(isExpired({ expires_at: 1000 }, 1000)).toBe(true); + expect(isExpired({ expires_at: 1000 }, 1001)).toBe(true); + expect(isExpired({ expires_at: 1000 }, 999)).toBe(false); + }); +}); + +describe("applySenderPolicy", () => { + it("accepts everything when no lists are configured", () => { + expect(applySenderPolicy({}, ["anyone@example.com"])).toBe("accepted"); + }); + + it("requires an allowlist match when an allowlist is set", () => { + const config = { allowed_senders: ["news@example.com"] }; + expect(applySenderPolicy(config, ["news@example.com"])).toBe("accepted"); + expect(applySenderPolicy(config, ["other@example.com"])).toBe("blocked"); + }); + + it("matches an allowlist by domain", () => { + const config = { allowed_senders: ["example.com"] }; + expect(applySenderPolicy(config, ["anyone@example.com"])).toBe("accepted"); + }); + + it("blocks a blocklisted sender even when allowlisted", () => { + const config = { + allowed_senders: ["example.com"], + blocked_senders: ["spam@example.com"], + }; + expect(applySenderPolicy(config, ["spam@example.com"])).toBe("blocked"); + expect(applySenderPolicy(config, ["ok@example.com"])).toBe("accepted"); + }); + + it("with only a blocklist, accepts everything else", () => { + const config = { blocked_senders: ["bad.com"] }; + expect(applySenderPolicy(config, ["x@bad.com"])).toBe("blocked"); + expect(applySenderPolicy(config, ["x@good.com"])).toBe("accepted"); + }); +}); + +describe("trimToByteBudget", () => { + const entry = (key: string, size: number): EmailMetadata => ({ + key, + subject: key, + receivedAt: 1, + size, + }); + + it("keeps everything within budget", () => { + const meta: FeedMetadata = { emails: [entry("a", 10), entry("b", 10)] }; + const { dropped } = trimToByteBudget(meta, 100); + expect(dropped).toEqual([]); + expect(meta.emails).toHaveLength(2); + }); + + it("drops the oldest entries (from the tail) until within budget", () => { + const meta: FeedMetadata = { + emails: [entry("new", 30), entry("mid", 30), entry("old", 30)], + }; + const { dropped } = trimToByteBudget(meta, 50); + expect(dropped.map((e) => e.key)).toEqual(["old", "mid"]); + expect(meta.emails.map((e) => e.key)).toEqual(["new"]); + }); + + it("always keeps at least one entry, even when oversized", () => { + const meta: FeedMetadata = { emails: [entry("only", 999)] }; + const { dropped } = trimToByteBudget(meta, 1); + expect(dropped).toEqual([]); + expect(meta.emails).toHaveLength(1); + }); +}); diff --git a/src/domain/feed.ts b/src/domain/feed.ts new file mode 100644 index 0000000..513d955 --- /dev/null +++ b/src/domain/feed.ts @@ -0,0 +1,119 @@ +import { Env, FeedConfig, FeedMetadata, EmailMetadata } from "../types"; + +const HOUR_MS = 3_600_000; + +/** + * The Feed aggregate's invariants, in one framework-agnostic place: expiry, + * sender allow/block policy, and the email-size budget. No I/O — callers load + * and persist state through the FeedRepository. + */ + +/** + * Resolve a feed's `expires_at` from a requested lifetime (hours). A server-side + * `FEED_TTL_HOURS` always overrides the client-supplied value. Returns undefined + * when no positive lifetime applies (i.e. the feed never expires). + */ +export function resolveExpiresAt( + env: Env, + lifetimeHours?: number, +): number | undefined { + const hours = env.FEED_TTL_HOURS + ? parseInt(env.FEED_TTL_HOURS, 10) + : (lifetimeHours ?? NaN); + return Number.isFinite(hours) && hours > 0 + ? Date.now() + hours * HOUR_MS + : undefined; +} + +/** Whether a feed has reached its expiry instant. */ +export function isExpired( + config: Pick, + now: number = Date.now(), +): boolean { + return config.expires_at !== undefined && config.expires_at <= now; +} + +export type SenderDecision = "accepted" | "blocked"; + +function normalizeEmail(value: string): string { + return value.trim().toLowerCase(); +} + +type SenderMatch = "blocked" | "allowed" | "neutral"; + +function evaluateSender( + sender: string, + allowedSenders: string[], + blockedSenders: string[], +): SenderMatch { + const normalized = normalizeEmail(sender); + const domain = normalized.split("@")[1] || ""; + + const normalizeDomain = (e: string) => (e.startsWith("@") ? e.slice(1) : e); + + const exactBlocked = blockedSenders.filter((e) => e.includes("@")); + const exactAllowed = allowedSenders.filter((e) => e.includes("@")); + const domainBlocked = blockedSenders + .filter((e) => !e.includes("@")) + .map(normalizeDomain); + const domainAllowed = allowedSenders + .filter((e) => !e.includes("@")) + .map(normalizeDomain); + + if (exactBlocked.includes(normalized)) return "blocked"; + if (exactAllowed.includes(normalized)) return "allowed"; + if (domain && domainBlocked.includes(domain)) return "blocked"; + if (domain && domainAllowed.includes(domain)) return "allowed"; + return "neutral"; +} + +/** + * Decide whether an inbound email is accepted, given the feed's sender lists and + * the message's candidate sender addresses. With no lists configured everything + * is accepted; a blocklist hit always rejects; an allowlist (when present) must + * be matched by at least one sender. + */ +export function applySenderPolicy( + config: Pick, + senders: string[], +): SenderDecision { + const allowedSenders = (config.allowed_senders || []) + .map(normalizeEmail) + .filter(Boolean); + const blockedSenders = (config.blocked_senders || []) + .map(normalizeEmail) + .filter(Boolean); + + if (allowedSenders.length === 0 && blockedSenders.length === 0) { + return "accepted"; + } + + const hasAllowlist = allowedSenders.length > 0; + const accepted = senders.some((sender) => { + const decision = evaluateSender(sender, allowedSenders, blockedSenders); + if (decision === "allowed") return true; + if (decision === "blocked") return false; + return !hasAllowlist; + }); + + return accepted ? "accepted" : "blocked"; +} + +/** + * Enforce the per-feed byte budget by dropping the oldest emails (mutating + * `metadata.emails`) until the total fits, always keeping at least one entry. + * Returns the dropped entries so the caller can purge their KV/R2 storage. + */ +export function trimToByteBudget( + metadata: FeedMetadata, + maxBytes: number, +): { dropped: EmailMetadata[] } { + let totalSize = metadata.emails.reduce((sum, e) => sum + (e.size ?? 0), 0); + const dropped: EmailMetadata[] = []; + while (totalSize > maxBytes && metadata.emails.length > 1) { + const entry = metadata.emails.pop()!; + totalSize -= entry.size ?? 0; + dropped.push(entry); + } + return { dropped }; +} diff --git a/src/lib/email-processor.ts b/src/lib/email-processor.ts index 2ac0850..27aceba 100644 --- a/src/lib/email-processor.ts +++ b/src/lib/email-processor.ts @@ -9,6 +9,7 @@ import { import { parseOneClickUnsubscribe } from "../utils/unsubscribe"; import { getAttachmentBucket } from "../utils/attachments"; import { FeedRepository } from "../domain/feed-repository"; +import { isExpired, applySenderPolicy, trimToByteBudget } from "../domain/feed"; import { logger } from "./logger"; import { FEED_MAX_BYTES } from "../config/constants"; @@ -34,38 +35,6 @@ type ValidationSuccess = { ok: true; feedId: string; feedConfig: FeedConfig }; type ValidationFailure = { ok: false; response: Response }; type ValidationResult = ValidationSuccess | ValidationFailure; -function normalizeEmail(value: string): string { - return value.trim().toLowerCase(); -} - -type SenderDecision = "blocked" | "allowed" | "neutral"; - -function evaluateSender( - sender: string, - allowedSenders: string[], - blockedSenders: string[], -): SenderDecision { - const normalized = normalizeEmail(sender); - const domain = normalized.split("@")[1] || ""; - - const normalizeDomain = (e: string) => (e.startsWith("@") ? e.slice(1) : e); - - const exactBlocked = blockedSenders.filter((e) => e.includes("@")); - const exactAllowed = allowedSenders.filter((e) => e.includes("@")); - const domainBlocked = blockedSenders - .filter((e) => !e.includes("@")) - .map(normalizeDomain); - const domainAllowed = allowedSenders - .filter((e) => !e.includes("@")) - .map(normalizeDomain); - - if (exactBlocked.includes(normalized)) return "blocked"; - if (exactAllowed.includes(normalized)) return "allowed"; - if (domain && domainBlocked.includes(domain)) return "blocked"; - if (domain && domainAllowed.includes(domain)) return "allowed"; - return "neutral"; -} - async function uploadAttachments( attachments: RawAttachment[], bucket: R2Bucket, @@ -113,10 +82,7 @@ export async function validateEmail( response: new Response("Feed does not exist", { status: 404 }), }; } - if ( - feedConfig.expires_at !== undefined && - feedConfig.expires_at <= Date.now() - ) { + if (isExpired(feedConfig)) { logger.warn("Rejected email: feed expired", { feedId }); return { ok: false, @@ -124,36 +90,19 @@ export async function validateEmail( }; } - const allowedSenders = (feedConfig.allowed_senders || []) - .map(normalizeEmail) - .filter(Boolean); - const blockedSenders = (feedConfig.blocked_senders || []) - .map(normalizeEmail) - .filter(Boolean); - - if (allowedSenders.length > 0 || blockedSenders.length > 0) { - const hasAllowlist = allowedSenders.length > 0; - const accepted = input.senders.some((sender) => { - const decision = evaluateSender(sender, allowedSenders, blockedSenders); - if (decision === "allowed") return true; - if (decision === "blocked") return false; - return !hasAllowlist; + if (applySenderPolicy(feedConfig, input.senders) === "blocked") { + logger.warn("Rejected email: sender filter", { + feedId, + senders: input.senders, + allowedSenders: feedConfig.allowed_senders, + blockedSenders: feedConfig.blocked_senders, }); - - if (!accepted) { - logger.warn("Rejected email: sender filter", { - feedId, - senders: input.senders, - allowedSenders, - blockedSenders, - }); - return { - ok: false, - response: new Response("Sender not allowed for this feed", { - status: 403, - }), - }; - } + return { + ok: false, + response: new Response("Sender not allowed for this feed", { + status: 403, + }), + }; } return { ok: true, feedId, feedConfig }; @@ -231,16 +180,7 @@ export async function storeEmail( }; } - let totalSize = feedMetadata.emails.reduce( - (sum, e) => sum + (e.size ?? 0), - 0, - ); - const toDelete: EmailMetadata[] = []; - while (totalSize > maxBytes && feedMetadata.emails.length > 1) { - const dropped = feedMetadata.emails.pop()!; - totalSize -= dropped.size ?? 0; - toDelete.push(dropped); - } + const { dropped: toDelete } = trimToByteBudget(feedMetadata, maxBytes); const r2Deletions = attachmentBucket && toDelete.length > 0 diff --git a/src/lib/feed-service.ts b/src/lib/feed-service.ts index a7f7984..5cf3e6a 100644 --- a/src/lib/feed-service.ts +++ b/src/lib/feed-service.ts @@ -6,30 +6,12 @@ import { waitUntilSafe } from "../utils/worker"; import { sendUnsubscribes } from "../utils/unsubscribe"; import { getAttachmentBucket } from "../utils/attachments"; import { FeedRepository } from "../domain/feed-repository"; +import { resolveExpiresAt, isExpired } from "../domain/feed"; import { purgeFeedKeysStep, collectUnsubscribeUrls, } from "../routes/admin/helpers"; -const HOUR_MS = 3_600_000; - -/** - * Resolve a feed's `expires_at` from a requested lifetime (hours). A server-side - * `FEED_TTL_HOURS` always overrides the client-supplied value. Returns undefined - * when no positive lifetime applies (i.e. the feed never expires). - */ -function resolveExpiresAt( - env: Env, - lifetimeHours?: number, -): number | undefined { - const hours = env.FEED_TTL_HOURS - ? parseInt(env.FEED_TTL_HOURS, 10) - : (lifetimeHours ?? NaN); - return Number.isFinite(hours) && hours > 0 - ? Date.now() + hours * HOUR_MS - : undefined; -} - export interface CreateFeedInput { title: string; description?: string; @@ -113,11 +95,7 @@ export async function updateFeedRecord( if (!existing) return { status: "not_found" }; - if ( - !options.inPlace && - existing.expires_at !== undefined && - existing.expires_at <= Date.now() - ) { + if (!options.inPlace && isExpired(existing)) { return { status: "expired" }; } diff --git a/src/routes/atom.ts b/src/routes/atom.ts index bb1bf52..eae26d1 100644 --- a/src/routes/atom.ts +++ b/src/routes/atom.ts @@ -3,6 +3,7 @@ import { Env } from "../types"; import { generateAtomFeed } from "../utils/feed-generator"; import { fetchFeedData } from "../utils/feed-fetcher"; import { baseUrl, feedAtomUrl } from "../utils/urls"; +import { isExpired } from "../domain/feed"; export async function handle(c: Context<{ Bindings: Env }>): Promise { try { @@ -15,10 +16,7 @@ export async function handle(c: Context<{ Bindings: Env }>): Promise { if (!feedData) { return new Response("Feed not found", { status: 404 }); } - if ( - feedData.feedConfig.expires_at !== undefined && - feedData.feedConfig.expires_at <= Date.now() - ) { + if (isExpired(feedData.feedConfig)) { return new Response("Feed has expired", { status: 410 }); } diff --git a/src/routes/entries.ts b/src/routes/entries.ts index 032f305..98f908d 100644 --- a/src/routes/entries.ts +++ b/src/routes/entries.ts @@ -4,6 +4,7 @@ import { Env } from "../types"; import { processEmailContent } from "../utils/html-processor"; import { formatBytes } from "../utils/format"; import { FeedRepository } from "../domain/feed-repository"; +import { isExpired } from "../domain/feed"; export async function handle(c: Context<{ Bindings: Env }>): Promise { const feedId = c.req.param("feedId"); @@ -22,10 +23,7 @@ export async function handle(c: Context<{ Bindings: Env }>): Promise { if (!feedMetadata) { return new Response("Feed not found", { status: 404 }); } - if ( - feedConfig?.expires_at !== undefined && - feedConfig.expires_at <= Date.now() - ) { + if (feedConfig && isExpired(feedConfig)) { return new Response("Feed has expired", { status: 410 }); } diff --git a/src/routes/rss.ts b/src/routes/rss.ts index 98cd58d..df85eba 100644 --- a/src/routes/rss.ts +++ b/src/routes/rss.ts @@ -3,6 +3,7 @@ import { Env } from "../types"; import { generateRssFeed } from "../utils/feed-generator"; import { fetchFeedData } from "../utils/feed-fetcher"; import { baseUrl, feedRssUrl } from "../utils/urls"; +import { isExpired } from "../domain/feed"; export async function handle(c: Context<{ Bindings: Env }>): Promise { try { @@ -15,10 +16,7 @@ export async function handle(c: Context<{ Bindings: Env }>): Promise { if (!feedData) { return new Response("Feed not found", { status: 404 }); } - if ( - feedData.feedConfig.expires_at !== undefined && - feedData.feedConfig.expires_at <= Date.now() - ) { + if (isExpired(feedData.feedConfig)) { return new Response("Feed has expired", { status: 410 }); }