refactor(domain): purify the Feed aggregate (Track D — points 1, 4, 6b)

Remove the infrastructure Env leak and ambient time from the domain core, and
model the sender policy as a value object.

- Point 1: Feed.create/edit no longer receive Env. The application layer resolves
  the effective lifetime (parsing FEED_TTL_HOURS and applying the server override)
  via feed-service.resolveTtlHours and hands the domain a plain ttlHours.
  resolveExpiresAt(ttlHours, now) is now pure.
- Point 4: introduce a Clock port (systemClock default), injected at
  create/reconstitute. The aggregate uses clock.now() instead of Date.now().
  The isExpired edge helper keeps its Date.now() default for routes.
- Point 6b: extract SenderPolicy value object built once from the lists
  (decide(senders)) instead of re-parsing per sender; applySenderPolicy is now a
  thin wrapper over it.

Coverage moved with the logic: the FEED_TTL_HOURS override is now pinned by
feed-service.test.ts; aggregate tests use an injected fixed clock.

351 tests pass; tsc --noEmit clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Julien Herr
2026-05-24 09:55:55 +02:00
parent e324571122
commit 23dd0a0c96
8 changed files with 359 additions and 149 deletions
+12
View File
@@ -0,0 +1,12 @@
/**
* A source of "now", injected into the domain so aggregates never reach for
* ambient `Date.now()`. Production wires `systemClock`; tests can supply a fixed
* clock for deterministic expiry/timestamp assertions.
*/
export interface Clock {
now(): number;
}
export const systemClock: Clock = {
now: () => Date.now(),
};
+62 -19
View File
@@ -3,12 +3,14 @@ import { createMockEnv } from "../test/setup";
import { Feed, CreateFeedInput } from "./feed.aggregate";
import { FeedRepository } from "./feed-repository";
import { FeedId } from "./value-objects/feed-id";
import { Clock } from "./clock";
import type { Env, EmailMetadata } from "../types";
const FID = FeedId.fromTrusted("a.b.42");
const mockEnv = (overrides: Partial<Env> = {}) =>
({ ...createMockEnv(), ...overrides }) as unknown as Env;
const mockEnv = () => createMockEnv() as unknown as Env;
const fixedClock = (now: number): Clock => ({ now: () => now });
const createInput = (
overrides: Partial<CreateFeedInput> = {},
@@ -30,26 +32,31 @@ const entry = (overrides: Partial<EmailMetadata> = {}): EmailMetadata => ({
describe("Feed.create", () => {
it("builds a config with an empty email index and no expiry by default", () => {
const feed = Feed.create(FID, createInput(), mockEnv());
const feed = Feed.create(FID, createInput());
expect(feed.id.value).toBe("a.b.42");
expect(feed.config.title).toBe("News");
expect(feed.config.expires_at).toBeUndefined();
expect(feed.metadata.emails).toEqual([]);
});
it("resolves expiry from lifetimeHours", () => {
const feed = Feed.create(FID, createInput({ lifetimeHours: 1 }), mockEnv());
expect(feed.config.expires_at).toBeGreaterThan(Date.now());
it("resolves expiry from the supplied ttlHours using the injected clock", () => {
const NOW = 1_000_000;
const feed = Feed.create(FID, createInput(), {
clock: fixedClock(NOW),
ttlHours: 2,
});
expect(feed.config.created_at).toBe(NOW);
expect(feed.config.updated_at).toBe(NOW);
expect(feed.config.expires_at).toBe(NOW + 2 * 3_600_000);
});
it("lets FEED_TTL_HOURS override a client lifetime", () => {
const feed = Feed.create(
FID,
createInput({ lifetimeHours: 1000000 }),
mockEnv({ FEED_TTL_HOURS: "1" }),
);
const oneClientHour = Date.now() + 1000000 * 3_600_000;
expect(feed.config.expires_at).toBeLessThan(oneClientHour);
it("trusts only deps.ttlHours, not the client lifetimeHours field", () => {
// The aggregate no longer parses lifetime policy: the application resolves
// the effective ttlHours (env override etc.) and hands it in.
const feed = Feed.create(FID, createInput({ lifetimeHours: 9999 }), {
ttlHours: undefined,
});
expect(feed.config.expires_at).toBeUndefined();
});
});
@@ -64,6 +71,16 @@ describe("Feed.isExpired / accepts", () => {
expect(feed.isExpired(150)).toBe(true);
});
it("uses the injected clock when no instant is supplied", () => {
const feed = Feed.reconstitute(
FID,
{ title: "T", language: "en", created_at: 0, expires_at: 100 },
{ emails: [] },
fixedClock(150),
);
expect(feed.isExpired()).toBe(true);
});
it("applies the sender policy", () => {
const feed = Feed.reconstitute(
FID,
@@ -80,6 +97,36 @@ describe("Feed.isExpired / accepts", () => {
});
});
describe("Feed.edit", () => {
it("recomputes expiry only when asked", () => {
const NOW = 5_000_000;
const FUTURE = NOW + 10 * 3_600_000;
const feed = Feed.reconstitute(
FID,
{ title: "T", language: "en", created_at: 0, expires_at: FUTURE },
{ emails: [] },
fixedClock(NOW),
);
feed.edit({ title: "T2" }, { recomputeExpiry: false });
expect(feed.config.expires_at).toBe(FUTURE); // preserved
expect(feed.config.updated_at).toBe(NOW);
feed.edit({ title: "T3" }, { recomputeExpiry: true, ttlHours: 1 });
expect(feed.config.expires_at).toBe(NOW + 3_600_000);
});
it("refuses to edit an already-expired feed", () => {
const feed = Feed.reconstitute(
FID,
{ title: "T", language: "en", created_at: 0, expires_at: 100 },
{ emails: [] },
fixedClock(200),
);
expect(feed.edit({ title: "X" }).status).toBe("expired");
});
});
describe("Feed.ingest", () => {
it("prepends the entry, tracks icon/unsub and trims to the byte budget", () => {
const feed = Feed.reconstitute(
@@ -129,11 +176,7 @@ describe("Feed.removeEmails", () => {
describe("FeedRepository.load / save round-trip", () => {
it("persists a created feed and reflects later mutations", async () => {
const repo = new FeedRepository(mockEnv().EMAIL_STORAGE);
const created = Feed.create(
FID,
createInput({ title: "Round" }),
mockEnv(),
);
const created = Feed.create(FID, createInput({ title: "Round" }));
await repo.save(created);
const loaded = await repo.load(FID);
+60 -28
View File
@@ -1,12 +1,8 @@
import { Env, FeedConfig, FeedMetadata, EmailMetadata } from "../types";
import { FeedConfig, FeedMetadata, EmailMetadata } from "../types";
import { FeedId } from "./value-objects/feed-id";
import {
resolveExpiresAt,
isExpired,
applySenderPolicy,
trimToByteBudget,
SenderDecision,
} from "./feed";
import { SenderPolicy, SenderDecision } from "./value-objects/sender-policy";
import { Clock, systemClock } from "./clock";
import { resolveExpiresAt, isExpired, trimToByteBudget } from "./feed";
export interface CreateFeedInput {
title: string;
@@ -26,6 +22,28 @@ export interface UpdateFeedInput {
lifetimeHours?: number;
}
/**
* Dependencies the aggregate needs from the outside but must not reach for
* itself: a clock (never ambient `Date.now()`) and an already-resolved feed
* lifetime. The application layer decides the lifetime — parsing env config and
* applying any server-side `FEED_TTL_HOURS` override — and hands the result in.
*/
export interface CreateFeedDeps {
clock?: Clock;
/** Effective lifetime in hours, already resolved by the application. */
ttlHours?: number;
}
export interface EditFeedDeps {
/** Effective lifetime in hours, already resolved by the application. */
ttlHours?: number;
/**
* Whether to recompute expiry at all. False preserves the current expiry
* (mirrors the old "no server TTL and no client lifetime ⇒ leave as-is").
*/
recomputeExpiry?: boolean;
}
export interface IngestOptions {
maxBytes: number;
iconDomain?: string;
@@ -41,21 +59,28 @@ export interface IngestOptions {
* deliberately sit *outside* the aggregate — the caller flushes them alongside
* `FeedRepository.save`/`saveMetadata`.
*
* I/O-free: load and persist state through `FeedRepository`. KV has no multi-key
* transaction, so a future Durable Object keyed by feed id would wrap
* load→mutate→save to serialise concurrent writers (see email-processor.ts).
* I/O-free and time-free: load and persist state through `FeedRepository`; time
* comes from an injected `Clock`. KV has no multi-key transaction, so a future
* Durable Object keyed by feed id would wrap load→mutate→save to serialise
* concurrent writers (see email-processor.ts).
*/
export class Feed {
private constructor(
readonly id: FeedId,
private _config: FeedConfig,
private _metadata: FeedMetadata,
private readonly clock: Clock,
) {}
/** Mint a brand-new feed with an empty email index. */
static create(id: FeedId, input: CreateFeedInput, env: Env): Feed {
const now = Date.now();
const expiresAt = resolveExpiresAt(env, input.lifetimeHours);
static create(
id: FeedId,
input: CreateFeedInput,
deps: CreateFeedDeps = {},
): Feed {
const clock = deps.clock ?? systemClock;
const now = clock.now();
const expiresAt = resolveExpiresAt(deps.ttlHours, now);
const config: FeedConfig = {
title: input.title,
description: input.description,
@@ -66,7 +91,7 @@ export class Feed {
updated_at: now,
...(expiresAt !== undefined ? { expires_at: expiresAt } : {}),
};
return new Feed(id, config, { emails: [] });
return new Feed(id, config, { emails: [] }, clock);
}
/** Rebuild an aggregate from persisted state. */
@@ -74,8 +99,9 @@ export class Feed {
id: FeedId,
config: FeedConfig,
metadata: FeedMetadata,
clock: Clock = systemClock,
): Feed {
return new Feed(id, config, metadata);
return new Feed(id, config, metadata, clock);
}
get config(): Readonly<FeedConfig> {
@@ -86,12 +112,15 @@ export class Feed {
return this._metadata;
}
isExpired(now: number = Date.now()): boolean {
isExpired(now: number = this.clock.now()): boolean {
return isExpired(this._config, now);
}
accepts(senders: string[]): SenderDecision {
return applySenderPolicy(this._config, senders);
return SenderPolicy.fromLists(
this._config.allowed_senders,
this._config.blocked_senders,
).decide(senders);
}
/**
@@ -143,21 +172,24 @@ export class Feed {
if (patch.description !== undefined) {
this._config.description = patch.description;
}
this._config.updated_at = Date.now();
this._config.updated_at = this.clock.now();
}
/**
* Full edit: apply the patch and recompute expiry from `FEED_TTL_HOURS` or a
* supplied lifetime (an absent lifetime preserves the current expiry). Rejects
* an already-expired feed without mutating it.
* Full edit: apply the patch and recompute expiry from the application-supplied
* lifetime when asked (an absent recompute preserves the current expiry).
* Rejects an already-expired feed without mutating it.
*/
edit(patch: UpdateFeedInput, env: Env): { status: "ok" | "expired" } {
edit(
patch: UpdateFeedInput,
deps: EditFeedDeps = {},
): { status: "ok" | "expired" } {
if (this.isExpired()) return { status: "expired" };
const expiresAt =
env.FEED_TTL_HOURS || patch.lifetimeHours !== undefined
? resolveExpiresAt(env, patch.lifetimeHours)
: this._config.expires_at;
const now = this.clock.now();
const expiresAt = deps.recomputeExpiry
? resolveExpiresAt(deps.ttlHours, now)
: this._config.expires_at;
if (patch.title !== undefined) this._config.title = patch.title;
if (patch.description !== undefined) {
@@ -170,7 +202,7 @@ export class Feed {
if (patch.blockedSenders !== undefined) {
this._config.blocked_senders = patch.blockedSenders;
}
this._config.updated_at = Date.now();
this._config.updated_at = now;
this._config.expires_at = expiresAt;
return { status: "ok" };
+11 -20
View File
@@ -5,35 +5,26 @@ import {
applySenderPolicy,
trimToByteBudget,
} from "./feed";
import type { Env, FeedMetadata, EmailMetadata } from "../types";
const env = (overrides: Partial<Env> = {}): Env =>
({ FEED_TTL_HOURS: undefined, ...overrides }) as Env;
import type { FeedMetadata, EmailMetadata } from "../types";
describe("resolveExpiresAt", () => {
it("returns undefined when no lifetime applies", () => {
expect(resolveExpiresAt(env())).toBeUndefined();
expect(resolveExpiresAt(env(), 0)).toBeUndefined();
expect(resolveExpiresAt(env(), -5)).toBeUndefined();
const NOW = 1_000_000;
it("returns undefined when no positive lifetime applies", () => {
expect(resolveExpiresAt(undefined, NOW)).toBeUndefined();
expect(resolveExpiresAt(0, NOW)).toBeUndefined();
expect(resolveExpiresAt(-5, NOW)).toBeUndefined();
expect(resolveExpiresAt(NaN, NOW)).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);
it("computes expiry from a supplied lifetime relative to now", () => {
expect(resolveExpiresAt(2, NOW)).toBe(NOW + 2 * 3_600_000);
});
});
describe("isExpired", () => {
it("is false when no expiry is set", () => {
expect(isExpired({ expires_at: undefined })).toBe(false);
expect(isExpired({ expires_at: undefined }, 1000)).toBe(false);
});
it("is true at or past the expiry instant", () => {
+27 -80
View File
@@ -1,33 +1,38 @@
import { Env, FeedConfig, FeedMetadata, EmailMetadata } from "../types";
import { EmailAddress } from "./value-objects/email-address";
import { Domain } from "./value-objects/domain";
import { FeedConfig, FeedMetadata, EmailMetadata } from "../types";
import { SenderPolicy, SenderDecision } from "./value-objects/sender-policy";
const HOUR_MS = 3_600_000;
export type { SenderDecision };
/**
* 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.
* sender allow/block policy, and the email-size budget. No I/O and no ambient
* time or environment — callers pass `now` (from a Clock) and a resolved
* lifetime; persistence goes 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).
* Resolve a feed's `expires_at` from an already-resolved lifetime (hours) and a
* current instant. Returns undefined when no positive lifetime applies (i.e. the
* feed never expires). The policy decision of *which* lifetime applies (a client
* request vs. a server-side `FEED_TTL_HOURS` override, and parsing the env
* string) belongs to the application layer, not here.
*/
export function resolveExpiresAt(
env: Env,
lifetimeHours?: number,
ttlHours: number | undefined,
now: 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
return ttlHours !== undefined && Number.isFinite(ttlHours) && ttlHours > 0
? now + ttlHours * HOUR_MS
: undefined;
}
/** Whether a feed has reached its expiry instant. */
/**
* Whether a feed has reached its expiry instant. `now` defaults to the wall
* clock for convenience at the HTTP edge (routes); the aggregate always passes
* its injected clock so its own behaviour stays deterministic.
*/
export function isExpired(
config: Pick<FeedConfig, "expires_at">,
now: number = Date.now(),
@@ -35,77 +40,19 @@ export function isExpired(
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 toDomains(entries: string[]): Domain[] {
return entries
.map((e) => Domain.parse(e))
.filter((d): d is Domain => d !== null);
}
function evaluateSender(
sender: string,
allowedSenders: string[],
blockedSenders: string[],
): SenderMatch {
const parsed = EmailAddress.parse(sender);
const normalized = parsed ? parsed.normalized : normalizeEmail(sender);
const senderDomain = parsed?.domain ?? null;
const exactBlocked = blockedSenders.filter((e) => e.includes("@"));
const exactAllowed = allowedSenders.filter((e) => e.includes("@"));
const domainBlocked = toDomains(
blockedSenders.filter((e) => !e.includes("@")),
);
const domainAllowed = toDomains(
allowedSenders.filter((e) => !e.includes("@")),
);
if (exactBlocked.includes(normalized)) return "blocked";
if (exactAllowed.includes(normalized)) return "allowed";
if (senderDomain && domainBlocked.some((d) => d.matches(senderDomain)))
return "blocked";
if (senderDomain && domainAllowed.some((d) => d.matches(senderDomain)))
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.
* the message's candidate sender addresses. Thin wrapper over the `SenderPolicy`
* value object (which holds the matching semantics).
*/
export function applySenderPolicy(
config: Pick<FeedConfig, "allowed_senders" | "blocked_senders">,
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";
return SenderPolicy.fromLists(
config.allowed_senders,
config.blocked_senders,
).decide(senders);
}
/**
+84
View File
@@ -0,0 +1,84 @@
import { EmailAddress } from "./email-address";
import { Domain } from "./domain";
export type SenderDecision = "accepted" | "blocked";
type SenderMatch = "blocked" | "allowed" | "neutral";
function normalizeEmail(value: string): string {
return value.trim().toLowerCase();
}
function toDomains(entries: string[]): Domain[] {
return entries
.map((e) => Domain.parse(e))
.filter((d): d is Domain => d !== null);
}
/**
* The sender allow/block policy as a value object, built ONCE from a feed's
* lists. The exact-vs-domain split is pre-computed here so `decide` is a cheap
* lookup per candidate sender instead of re-parsing both lists for every one
* (the previous `applySenderPolicy` was O(senders × lists)).
*
* Semantics (unchanged): no lists ⇒ everything accepted; a blocklist hit always
* rejects; an allowlist (when present) must be matched by at least one sender.
*/
export class SenderPolicy {
private constructor(
private readonly exactAllowed: string[],
private readonly exactBlocked: string[],
private readonly domainAllowed: Domain[],
private readonly domainBlocked: Domain[],
private readonly hasAllowlist: boolean,
private readonly hasAnyRule: boolean,
) {}
static fromLists(
allowed: string[] = [],
blocked: string[] = [],
): SenderPolicy {
const allowedSenders = allowed.map(normalizeEmail).filter(Boolean);
const blockedSenders = blocked.map(normalizeEmail).filter(Boolean);
return new SenderPolicy(
allowedSenders.filter((e) => e.includes("@")),
blockedSenders.filter((e) => e.includes("@")),
toDomains(allowedSenders.filter((e) => !e.includes("@"))),
toDomains(blockedSenders.filter((e) => !e.includes("@"))),
allowedSenders.length > 0,
allowedSenders.length > 0 || blockedSenders.length > 0,
);
}
private evaluate(sender: string): SenderMatch {
const parsed = EmailAddress.parse(sender);
const normalized = parsed ? parsed.normalized : normalizeEmail(sender);
const senderDomain = parsed?.domain ?? null;
if (this.exactBlocked.includes(normalized)) return "blocked";
if (this.exactAllowed.includes(normalized)) return "allowed";
if (senderDomain && this.domainBlocked.some((d) => d.matches(senderDomain)))
return "blocked";
if (senderDomain && this.domainAllowed.some((d) => d.matches(senderDomain)))
return "allowed";
return "neutral";
}
/**
* Decide whether an inbound email is accepted, given its candidate sender
* addresses. A blocklist hit on any sender rejects; with an allowlist set, at
* least one sender must match it.
*/
decide(senders: string[]): SenderDecision {
if (!this.hasAnyRule) return "accepted";
const accepted = senders.some((sender) => {
const decision = this.evaluate(sender);
if (decision === "allowed") return true;
if (decision === "blocked") return false;
return !this.hasAllowlist;
});
return accepted ? "accepted" : "blocked";
}
}