refactor: tighten DDD boundaries on the Feed aggregate

Address five modeling tensions in one pass:

- Encapsulation: the Feed aggregate no longer exposes raw config/metadata
  (a shallow Readonly still leaked mutable arrays). It now offers
  intention-revealing accessors that return copies, plus
  toConfigSnapshot/toMetadataSnapshot for the repository and summary() for
  the global registry.
- feeds:list consistency: FeedRepository.save/saveConfig upsert the registry
  entry from feed.summary(), so services no longer mirror title/description/
  expiry by hand (the old add/updateInList footgun is gone).
- domain/feed.ts: drop the dead applySenderPolicy, internalise resolveExpiresAt
  and trimToByteBudget into the aggregate; feed.ts keeps only the shared
  isExpired predicate used by the read-model routes.
- Single edit path: remove editDetails; edit(patch, deps) is the sole config
  mutation, with a systematic expired guard. Renaming an expired feed now 403s.
- FeedId flows through the application and infrastructure signatures;
  fromTrusted/parse happen once at the edge, .value only at the serialisation
  boundaries (urls, feed-generator, feed-keys, logs, JSON).

347 tests green, tsc clean, Worker bundle builds.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Julien Herr
2026-05-24 13:45:13 +02:00
parent b3d42f6c50
commit ad196f1761
24 changed files with 406 additions and 398 deletions
+29 -7
View File
@@ -1,6 +1,7 @@
import { describe, it, expect } from "vitest";
import { createMockEnv } from "../test/setup";
import { FeedRepository } from "./feed-repository";
import { Feed } from "../domain/feed.aggregate";
import { FeedId } from "../domain/value-objects/feed-id";
import type { Env, FeedConfig, FeedMetadata, EmailData } from "../types";
@@ -95,10 +96,29 @@ describe("FeedRepository emails", () => {
});
describe("FeedRepository feed list", () => {
it("adds, updates, lists and removes feeds with expiry", async () => {
const feedWith = (
id: string,
title: string,
opts: { description?: string; expires_at?: number } = {},
) =>
Feed.reconstitute(
fid(id),
{
title,
language: "en",
created_at: 1000,
description: opts.description,
expires_at: opts.expires_at,
},
{ emails: [] },
);
it("upserts the list entry from the aggregate on save/saveConfig", async () => {
const repo = new FeedRepository(mockEnv().EMAIL_STORAGE);
await repo.addToList(fid("a.b.42"), "One", "desc", 5000);
await repo.addToList(fid("c.d.99"), "Two");
await repo.save(
feedWith("a.b.42", "One", { description: "desc", expires_at: 5000 }),
);
await repo.save(feedWith("c.d.99", "Two"));
let feeds = await repo.listFeeds();
expect(feeds).toHaveLength(2);
@@ -107,8 +127,10 @@ describe("FeedRepository feed list", () => {
expires_at: 5000,
});
await repo.updateInList(fid("a.b.42"), "One-updated", undefined, undefined);
// saveConfig refreshes the same entry in place (no duplicate, expiry cleared).
await repo.saveConfig(feedWith("a.b.42", "One-updated"));
feeds = await repo.listFeeds();
expect(feeds.filter((f) => f.id === "a.b.42")).toHaveLength(1);
const updated = feeds.find((f) => f.id === "a.b.42");
expect(updated).toMatchObject({ title: "One-updated" });
expect(updated?.expires_at).toBeUndefined();
@@ -121,9 +143,9 @@ describe("FeedRepository feed list", () => {
it("bulk-removes only the matching ids", async () => {
const repo = new FeedRepository(mockEnv().EMAIL_STORAGE);
await repo.addToList(fid("a.b.42"), "One");
await repo.addToList(fid("c.d.99"), "Two");
await repo.addToList(fid("e.f.10"), "Three");
await repo.save(feedWith("a.b.42", "One"));
await repo.save(feedWith("c.d.99", "Two"));
await repo.save(feedWith("e.f.10", "Three"));
const removed = await repo.removeFromListBulk(["a.b.42", "e.f.10", "nope"]);
expect(removed.sort()).toEqual(["a.b.42", "e.f.10"]);
+34 -45
View File
@@ -72,28 +72,38 @@ export class FeedRepository {
return Feed.reconstitute(feedId, config, metadata ?? { emails: [] });
}
/** Persist both keys the aggregate owns (config + metadata). */
/**
* Persist both keys the aggregate owns (config + metadata) and keep the global
* `feeds:list` entry in sync. The registry projection is derived from
* `feed.summary()` here, so no caller has to remember to mirror it.
*/
async save(feed: Feed): Promise<void> {
await Promise.all([
this.putConfig(feed.id, feed.config),
this.putMetadata(feed.id, feed.metadata),
this.putConfig(feed.id, feed.toConfigSnapshot()),
this.putMetadata(feed.id, feed.toMetadataSnapshot()),
this.upsertListEntry(feed.summary()),
]);
}
/**
* Persist only the email index. Used by the ingest/delete paths where config
* is unchanged — avoids a redundant config write on the hot path.
* is unchanged — avoids a redundant config write on the hot path. The list
* projection (title/description/expiry) is untouched, so it is not rewritten.
*/
async saveMetadata(feed: Feed): Promise<void> {
await this.putMetadata(feed.id, feed.metadata);
await this.putMetadata(feed.id, feed.toMetadataSnapshot());
}
/**
* Persist only the config. Used by the rename/edit paths where metadata is
* unchanged — avoids re-writing (and risking clobbering) the email index.
* Persist only the config and refresh the `feeds:list` entry from it. Used by
* the rename/edit paths where metadata is unchanged — avoids re-writing (and
* risking clobbering) the email index.
*/
async saveConfig(feed: Feed): Promise<void> {
await this.putConfig(feed.id, feed.config);
await Promise.all([
this.putConfig(feed.id, feed.toConfigSnapshot()),
this.upsertListEntry(feed.summary()),
]);
}
// ── Feed config ───────────────────────────────────────────────────────────
@@ -156,50 +166,29 @@ export class FeedRepository {
}
}
async addToList(
feedId: FeedId,
title: string,
description?: string,
expires_at?: number,
): Promise<void> {
/**
* Insert-or-update a feed's entry in the global `feeds:list` registry from its
* aggregate summary. Idempotent by feed id. Private: callers persist a `Feed`
* via `save`/`saveConfig`, which keep the projection in sync — never mirror the
* list by hand. (Read-modify-write is not atomic under KV, unchanged from the
* prior add/update split.)
*/
private async upsertListEntry(summary: FeedListItem): Promise<void> {
try {
const feedList = ((await this.kv.get(FEEDS_LIST_KEY, {
type: "json",
})) as FeedList | null) || { feeds: [] };
feedList.feeds.push({ id: feedId.value, title, description, expires_at });
const index = feedList.feeds.findIndex((feed) => feed.id === summary.id);
if (index === -1) {
feedList.feeds.push(summary);
} else {
feedList.feeds[index] = summary;
}
await this.kv.put(FEEDS_LIST_KEY, JSON.stringify(feedList));
} catch (error) {
logger.error("Error adding feed to list", {
feedId: feedId.value,
error: String(error),
});
}
}
async updateInList(
feedId: FeedId,
title: string,
description?: string,
expires_at?: number,
): Promise<void> {
try {
const feedList = ((await this.kv.get(FEEDS_LIST_KEY, {
type: "json",
})) as FeedList | null) || { feeds: [] };
const feedIndex = feedList.feeds.findIndex(
(feed) => feed.id === feedId.value,
);
if (feedIndex !== -1) {
feedList.feeds[feedIndex].title = title;
feedList.feeds[feedIndex].description = description;
feedList.feeds[feedIndex].expires_at = expires_at;
await this.kv.put(FEEDS_LIST_KEY, JSON.stringify(feedList));
}
} catch (error) {
logger.error("Error updating feed in list", {
feedId: feedId.value,
logger.error("Error upserting feed in list", {
feedId: summary.id,
error: String(error),
});
}
@@ -1,19 +1,21 @@
import { describe, it, expect } from "vitest";
import { createMockEnv } from "../test/setup";
import { WebSubSubscriptionRepository } from "./websub-subscription-repository";
import { FeedId } from "../domain/value-objects/feed-id";
import type { Env, WebSubSubscription } from "../types";
const mockEnv = () => createMockEnv() as unknown as Env;
const fid = FeedId.fromTrusted("a.b.42");
describe("WebSubSubscriptionRepository", () => {
it("round-trips subscriptions and counts feeds with subscribers", async () => {
const repo = new WebSubSubscriptionRepository(mockEnv().EMAIL_STORAGE);
expect(await repo.get("a.b.42")).toEqual([]);
expect(await repo.get(fid)).toEqual([]);
const subs: WebSubSubscription[] = [
{ callbackUrl: "https://r.example/cb", expiresAt: 9999 },
];
await repo.save("a.b.42", subs);
expect(await repo.get("a.b.42")).toEqual(subs);
await repo.save(fid, subs);
expect(await repo.get(fid)).toEqual(subs);
expect(await repo.countKeys()).toBe(1);
});
});
@@ -1,5 +1,6 @@
import { Env, WebSubSubscription } from "../types";
import { feedKeys } from "../domain/feed-keys";
import { FeedId } from "../domain/value-objects/feed-id";
import { logger } from "./logger";
/**
@@ -12,16 +13,19 @@ export class WebSubSubscriptionRepository {
return new WebSubSubscriptionRepository(env.EMAIL_STORAGE);
}
async get(feedId: string): Promise<WebSubSubscription[]> {
const raw = await this.kv.get(feedKeys.websub(feedId), "json");
async get(feedId: FeedId): Promise<WebSubSubscription[]> {
const raw = await this.kv.get(feedKeys.websub(feedId.value), "json");
return (raw as WebSubSubscription[] | null) ?? [];
}
async save(
feedId: string,
feedId: FeedId,
subscriptions: WebSubSubscription[],
): Promise<void> {
await this.kv.put(feedKeys.websub(feedId), JSON.stringify(subscriptions));
await this.kv.put(
feedKeys.websub(feedId.value),
JSON.stringify(subscriptions),
);
}
/** Number of feeds that currently hold at least one WebSub subscription. */
+42 -40
View File
@@ -9,9 +9,11 @@ import {
verifyAndStoreSubscription,
verifyAndDeleteSubscription,
} from "./websub";
import { FeedId } from "../domain/value-objects/feed-id";
import type { Env, WebSubSubscription } from "../types";
const mockEnv = () => createMockEnv() as unknown as Env;
const fid = (value: string) => FeedId.fromTrusted(value);
describe("buildHmacSignature", () => {
it("returns sha256= prefixed hex", async () => {
@@ -35,7 +37,7 @@ describe("buildHmacSignature", () => {
describe("getSubscriptions / saveSubscriptions", () => {
it("returns empty array when no subs exist", async () => {
const env = mockEnv();
expect(await getSubscriptions("feed1", env)).toEqual([]);
expect(await getSubscriptions(fid("feed1"), env)).toEqual([]);
});
it("round-trips stored subscriptions", async () => {
@@ -46,13 +48,13 @@ describe("getSubscriptions / saveSubscriptions", () => {
expiresAt: Date.now() + 60000,
},
];
await saveSubscriptions("feed1", subs, env);
expect(await getSubscriptions("feed1", env)).toEqual(subs);
await saveSubscriptions(fid("feed1"), subs, env);
expect(await getSubscriptions(fid("feed1"), env)).toEqual(subs);
});
it("uses the correct KV key", async () => {
const env = mockEnv();
await saveSubscriptions("abc", [], env);
await saveSubscriptions(fid("abc"), [], env);
expect(
await env.EMAIL_STORAGE.get("websub:subs:abc", { type: "json" }),
).toEqual([]);
@@ -69,7 +71,7 @@ describe("notifySubscribers", () => {
return HttpResponse.text("ok");
}),
);
await notifySubscribers("feed1", env);
await notifySubscribers(fid("feed1"), env);
expect(called).toBe(false);
});
@@ -81,7 +83,7 @@ describe("notifySubscribers", () => {
expiresAt: Date.now() + 60000,
},
];
await saveSubscriptions("feed1", subs, env);
await saveSubscriptions(fid("feed1"), subs, env);
let called = false;
server.use(
http.post("https://reader.example/callback", () => {
@@ -89,7 +91,7 @@ describe("notifySubscribers", () => {
return HttpResponse.text("ok");
}),
);
await notifySubscribers("feed1", env);
await notifySubscribers(fid("feed1"), env);
expect(called).toBe(false);
});
@@ -113,7 +115,7 @@ describe("notifySubscribers", () => {
expiresAt: Date.now() + 60000,
},
];
await saveSubscriptions("feed1", subs, env);
await saveSubscriptions(fid("feed1"), subs, env);
let receivedBody = "";
let receivedContentType = "";
@@ -125,7 +127,7 @@ describe("notifySubscribers", () => {
}),
);
await notifySubscribers("feed1", env);
await notifySubscribers(fid("feed1"), env);
expect(receivedBody).toContain("<?xml");
expect(receivedContentType).toContain("application/rss+xml");
@@ -152,7 +154,7 @@ describe("notifySubscribers", () => {
secret: "mysecret",
},
];
await saveSubscriptions("feed1", subs, env);
await saveSubscriptions(fid("feed1"), subs, env);
let receivedSig256 = "";
let receivedSig = "";
@@ -164,7 +166,7 @@ describe("notifySubscribers", () => {
}),
);
await notifySubscribers("feed1", env);
await notifySubscribers(fid("feed1"), env);
expect(receivedSig256).toMatch(/^sha256=[0-9a-f]{64}$/);
expect(receivedSig).toBe(""); // legacy header should NOT be sent
});
@@ -190,7 +192,7 @@ describe("notifySubscribers", () => {
format: "atom",
},
];
await saveSubscriptions("feed1", subs, env);
await saveSubscriptions(fid("feed1"), subs, env);
let receivedContentType = "";
let receivedLink = "";
@@ -202,7 +204,7 @@ describe("notifySubscribers", () => {
}),
);
await notifySubscribers("feed1", env);
await notifySubscribers(fid("feed1"), env);
expect(receivedContentType).toContain("application/atom+xml");
expect(receivedLink).toContain(`/atom/feed1`);
expect(receivedLink).toContain(`rel="self"`);
@@ -234,7 +236,7 @@ describe("notifySubscribers", () => {
format: "atom",
},
];
await saveSubscriptions("feed1", subs, env);
await saveSubscriptions(fid("feed1"), subs, env);
const received: Record<string, string> = {};
server.use(
@@ -248,7 +250,7 @@ describe("notifySubscribers", () => {
}),
);
await notifySubscribers("feed1", env);
await notifySubscribers(fid("feed1"), env);
expect(received.rss).toContain("application/rss+xml");
expect(received.atom).toContain("application/atom+xml");
});
@@ -277,7 +279,7 @@ describe("notifySubscribers", () => {
expiresAt: Date.now() + 60000,
},
];
await saveSubscriptions("feed1", subs, env);
await saveSubscriptions(fid("feed1"), subs, env);
const notified: string[] = [];
server.use(
@@ -291,10 +293,10 @@ describe("notifySubscribers", () => {
}),
);
await notifySubscribers("feed1", env);
await notifySubscribers(fid("feed1"), env);
expect(notified).toEqual(["active"]);
const remaining = await getSubscriptions("feed1", env);
const remaining = await getSubscriptions(fid("feed1"), env);
expect(remaining).toHaveLength(1);
expect(remaining[0].callbackUrl).toBe("https://active.example/callback");
});
@@ -312,7 +314,7 @@ describe("verifyAndStoreSubscription", () => {
);
const result = await verifyAndStoreSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
undefined,
86400,
@@ -321,7 +323,7 @@ describe("verifyAndStoreSubscription", () => {
);
expect(result).toBe(true);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(1);
expect(subs[0].callbackUrl).toBe("https://reader.example/callback");
expect(subs[0].expiresAt).toBeGreaterThan(Date.now());
@@ -340,7 +342,7 @@ describe("verifyAndStoreSubscription", () => {
);
const result = await verifyAndStoreSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
undefined,
86400,
@@ -350,7 +352,7 @@ describe("verifyAndStoreSubscription", () => {
expect(result).toBe(true);
expect(receivedTopic).toContain("/atom/feed1");
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs[0].format).toBe("atom");
});
@@ -363,7 +365,7 @@ describe("verifyAndStoreSubscription", () => {
);
const result = await verifyAndStoreSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
undefined,
86400,
@@ -372,7 +374,7 @@ describe("verifyAndStoreSubscription", () => {
);
expect(result).toBe(false);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(0);
});
@@ -381,7 +383,7 @@ describe("verifyAndStoreSubscription", () => {
const existing: WebSubSubscription[] = [
{ callbackUrl: "https://reader.example/callback", expiresAt: 1000 },
];
await saveSubscriptions("feed1", existing, env);
await saveSubscriptions(fid("feed1"), existing, env);
server.use(
http.get("https://reader.example/callback", ({ request }) => {
@@ -392,7 +394,7 @@ describe("verifyAndStoreSubscription", () => {
);
const result = await verifyAndStoreSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
"newsecret",
3600,
@@ -401,7 +403,7 @@ describe("verifyAndStoreSubscription", () => {
);
expect(result).toBe(true);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(1);
expect(subs[0].secret).toBe("newsecret");
});
@@ -413,7 +415,7 @@ describe("verifyAndStoreSubscription", () => {
);
const result = await verifyAndStoreSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
undefined,
86400,
@@ -422,7 +424,7 @@ describe("verifyAndStoreSubscription", () => {
);
expect(result).toBe(false);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(0);
});
@@ -437,7 +439,7 @@ describe("verifyAndStoreSubscription", () => {
);
const result = await verifyAndStoreSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
undefined,
86400,
@@ -446,7 +448,7 @@ describe("verifyAndStoreSubscription", () => {
);
expect(result).toBe(false);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(0);
});
});
@@ -455,7 +457,7 @@ describe("verifyAndDeleteSubscription", () => {
it("removes subscription and returns true when callback echoes challenge", async () => {
const env = mockEnv();
await saveSubscriptions(
"feed1",
fid("feed1"),
[
{
callbackUrl: "https://reader.example/callback",
@@ -474,19 +476,19 @@ describe("verifyAndDeleteSubscription", () => {
);
const result = await verifyAndDeleteSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
env,
);
expect(result).toBe(true);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(0);
});
it("returns false and leaves subscription intact when callback returns wrong challenge", async () => {
const env = mockEnv();
await saveSubscriptions(
"feed1",
fid("feed1"),
[
{
callbackUrl: "https://reader.example/callback",
@@ -503,19 +505,19 @@ describe("verifyAndDeleteSubscription", () => {
);
const result = await verifyAndDeleteSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
env,
);
expect(result).toBe(false);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(1);
});
it("returns false and leaves subscription intact when callback fetch fails", async () => {
const env = mockEnv();
await saveSubscriptions(
"feed1",
fid("feed1"),
[
{
callbackUrl: "https://reader.example/callback",
@@ -530,12 +532,12 @@ describe("verifyAndDeleteSubscription", () => {
);
const result = await verifyAndDeleteSubscription(
"feed1",
fid("feed1"),
"https://reader.example/callback",
env,
);
expect(result).toBe(false);
const subs = await getSubscriptions("feed1", env);
const subs = await getSubscriptions(fid("feed1"), env);
expect(subs).toHaveLength(1);
});
});
+21 -17
View File
@@ -6,14 +6,14 @@ import { WebSubSubscriptionRepository } from "./websub-subscription-repository";
import { FeedId } from "../domain/value-objects/feed-id";
export async function getSubscriptions(
feedId: string,
feedId: FeedId,
env: Env,
): Promise<WebSubSubscription[]> {
return WebSubSubscriptionRepository.from(env).get(feedId);
}
export async function saveSubscriptions(
feedId: string,
feedId: FeedId,
subscriptions: WebSubSubscription[],
env: Env,
): Promise<void> {
@@ -43,22 +43,21 @@ export async function buildHmacSignature(
}
async function buildFeedXml(
feedId: string,
feedId: FeedId,
env: Env,
format: "rss" | "atom" = "rss",
): Promise<string | null> {
const repo = FeedRepository.from(env);
const id = FeedId.fromTrusted(feedId);
const [feedMetadata, rawConfig] = await Promise.all([
repo.getMetadata(id),
repo.getConfig(id),
repo.getMetadata(feedId),
repo.getConfig(feedId),
]);
if (!feedMetadata) return null;
const base = baseUrl(env);
const feedConfig: FeedConfig = rawConfig ?? {
title: `Newsletter Feed ${feedId}`,
title: `Newsletter Feed ${feedId.value}`,
description: "Converted email newsletter",
language: "en",
created_at: Date.now(),
@@ -74,15 +73,15 @@ async function buildFeedXml(
feedConfig,
emailsData,
base,
feedId,
feedAtomUrl(feedId, env),
feedId.value,
feedAtomUrl(feedId.value, env),
);
}
return generateRssFeed(feedConfig, emailsData, base, feedId);
return generateRssFeed(feedConfig, emailsData, base, feedId.value);
}
export async function notifySubscribers(
feedId: string,
feedId: FeedId,
env: Env,
): Promise<void> {
const subs = await getSubscriptions(feedId, env);
@@ -140,12 +139,17 @@ export async function notifySubscribers(
await Promise.allSettled([
...(rssFeed
? rssSubs.map((sub) =>
deliver(sub, rssFeed, "application/rss+xml", `/rss/${feedId}`),
deliver(sub, rssFeed, "application/rss+xml", `/rss/${feedId.value}`),
)
: []),
...(atomFeed
? atomSubs.map((sub) =>
deliver(sub, atomFeed, "application/atom+xml", `/atom/${feedId}`),
deliver(
sub,
atomFeed,
"application/atom+xml",
`/atom/${feedId.value}`,
),
)
: []),
]);
@@ -176,7 +180,7 @@ async function verifyCallback(
}
export async function verifyAndStoreSubscription(
feedId: string,
feedId: FeedId,
callbackUrl: string,
secret: string | undefined,
leaseSeconds: number,
@@ -185,7 +189,7 @@ export async function verifyAndStoreSubscription(
): Promise<boolean> {
const verified = await verifyCallback(callbackUrl, {
"hub.mode": "subscribe",
"hub.topic": feedUrl(format, feedId, env),
"hub.topic": feedUrl(format, feedId.value, env),
"hub.lease_seconds": String(leaseSeconds),
});
if (!verified) return false;
@@ -208,13 +212,13 @@ export async function verifyAndStoreSubscription(
}
export async function verifyAndDeleteSubscription(
feedId: string,
feedId: FeedId,
callbackUrl: string,
env: Env,
): Promise<boolean> {
const verified = await verifyCallback(callbackUrl, {
"hub.mode": "unsubscribe",
"hub.topic": feedRssUrl(feedId, env),
"hub.topic": feedRssUrl(feedId.value, env),
});
if (!verified) return false;