From 5cb0a5d9147932730b1a77b62790630fd3399aba Mon Sep 17 00:00:00 2001 From: Madraka <8898640+Madraka@users.noreply.github.com> Date: Wed, 11 Nov 2026 02:55:07 +0300 Subject: [PATCH 1/2] fix(core-ts): scope asset_refs by site and skip rebuilds on unchanged imports (#196) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC-0045 rollout step 0 (§2.5), independent of the RFC's new field types. - refsRebuild joins nodes so only same-site assets get rows, chunks its INSERT below 90 KB per statement (SQL plus parameters), drops OR IGNORE (rows are deduplicated in JS), and is emitted by the import batch only when that batch writes the node set. An unchanged import can no longer replace a concurrent edit's fresh row with plan-time rows (#196). - deleteAsset's guard and listAssets' unused filter read asset_refs by site_id, so legacy rows a foreign archive left in another site neither block the delete nor hide the asset. - The purge planner's asset rule moves to cache-purge-assets.ts and also reads typed field assets (fieldAssetRefs, the target snapshot's site definitions). planCachePurge shrinks 233 -> 207 lines (baseline tightened); assets.ts adds no net lines. - Tests: new shared suite testing-asset-refs.ts (#196 race, foreign archive, 5,000-row rebuild under the size limit); a field-image purge case; the two "import rebuilds refs" cases now edit after the export so the import writes the node set. Closes #196 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Madraka <8898640+Madraka@users.noreply.github.com> --- .changeset/asset-refs-by-site.md | 10 ++ packages/core-ts/src/assets.ts | 8 +- packages/core-ts/src/cache-purge-assets.ts | 72 +++++++++++ packages/core-ts/src/cache-purge.ts | 48 ++------ packages/core-ts/src/media/refs-rebuild.ts | 59 +++++++-- packages/core-ts/src/sync-plan.ts | 6 +- packages/core-ts/src/testing-asset-refs.ts | 132 +++++++++++++++++++++ packages/core-ts/src/testing-assets.ts | 5 +- packages/core-ts/src/testing-fields.ts | 2 + packages/core-ts/src/testing.ts | 2 + packages/core-ts/test/cache-purge.test.ts | 27 ++++- scripts/size-baseline.json | 2 +- 12 files changed, 312 insertions(+), 61 deletions(-) create mode 100644 .changeset/asset-refs-by-site.md create mode 100644 packages/core-ts/src/cache-purge-assets.ts create mode 100644 packages/core-ts/src/testing-asset-refs.ts diff --git a/.changeset/asset-refs-by-site.md b/.changeset/asset-refs-by-site.md new file mode 100644 index 00000000..45dff737 --- /dev/null +++ b/.changeset/asset-refs-by-site.md @@ -0,0 +1,10 @@ +--- +"@cloudhal/core-ts": patch +--- + +`asset_refs` is scoped by site (RFC-0045 §2.5, rollout step 0), and an unchanged import no longer rebuilds it (#196). + +- An import, push or pull planned `unchanged` used to rebuild the site's `asset_refs` from rows read at plan time. An edit that committed in between could lose its new row, so the asset it started using could be deleted and showed as unused. The rebuild now runs only in a batch that also writes the node set. +- The rebuild records only references to assets of the same site, and splits its `INSERT` so each statement stays below 90 KB (large sites could exceed D1's statement limit). Its statements count against the import's 1,000-statement cap. +- Deleting an asset and the media library's "unused" filter look only at references from the asset's own site, so a foreign archive naming another site's asset no longer blocks its delete or hides it. +- A publish that changes an asset now also purges the pages that show it through a typed `image` or `richText` field (RFC-0041), not only through covers, bodies and sections. diff --git a/packages/core-ts/src/assets.ts b/packages/core-ts/src/assets.ts index d482d3b0..2df6da1c 100644 --- a/packages/core-ts/src/assets.ts +++ b/packages/core-ts/src/assets.ts @@ -31,7 +31,7 @@ import { ID_PREFIX } from "./ids.ts"; import { RASTER_TYPES } from "./media/image.ts"; import { type InspectOptions, inspectUpload } from "./media/ingest.ts"; import { type AssetRefUse, assetRefsOf, canonicalImageRefs } from "./media/refs.ts"; -import { refsRebuild } from "./media/refs-rebuild.ts"; +import { refsRebuild, UNUSED_ASSET_SQL } from "./media/refs-rebuild.ts"; import { sniff } from "./media/sniff.ts"; import { assetKindFor, @@ -319,7 +319,7 @@ export class AssetEngine { where.push("json_extract(body, '$.uploadedBy.kind') = ?"); params.push(q.uploadedBy); } - if (q.unused) where.push("NOT EXISTS (SELECT 1 FROM asset_refs r WHERE r.asset_id = nodes.id)"); + if (q.unused) where.push(UNUSED_ASSET_SQL); if (q.missingAlt) where.push( "json_extract(body, '$.kind') = 'image' AND json_extract(body, '$.alt') IS NULL AND json_extract(body, '$.decorative') IS NULL", @@ -864,8 +864,8 @@ export class AssetEngine { */ async deleteAsset(a: Asset): Promise { const r = await this.#sql.run( - "DELETE FROM nodes WHERE id = ? AND rev = ? AND kind = 'asset' AND site_id = ? AND NOT EXISTS (SELECT 1 FROM asset_refs WHERE asset_id = ?)", - [a.id, a.rev, a.siteId, a.id], + "DELETE FROM nodes WHERE id = ? AND rev = ? AND kind = 'asset' AND site_id = ? AND NOT EXISTS (SELECT 1 FROM asset_refs WHERE site_id = ? AND asset_id = ?)", + [a.id, a.rev, a.siteId, a.siteId, a.id], ); if (r.changes === 1) return; const usage = (await this.assetUsage(a.siteId, [a.id])).get(a.id); diff --git a/packages/core-ts/src/cache-purge-assets.ts b/packages/core-ts/src/cache-purge-assets.ts new file mode 100644 index 00000000..a82b6f8c --- /dev/null +++ b/packages/core-ts/src/cache-purge-assets.ts @@ -0,0 +1,72 @@ +/** + * The purge planner's asset rule (RFC-0004 §7.4): an asset that left the live snapshot or changed + * purges its media URLs (`cloudhal-asset-`) and, when it changed, the responses that show it, + * since they resolve its URL, alt, caption and size when they render: the node tag of every entry + * and page that refers to it, and the term tag (with ancestors) of every term that does. An + * entry's typed fields count too (RFC-0045 §2.5): an `image` field's value and a `richText` + * field's `asset:` links, read through the target snapshot's own field definitions. A newly added + * asset purges nothing (404s are never cached, and whatever starts showing it changed itself). + * Part of `planCachePurge` (`cache-purge.ts`); reads through its counted, hash-keyed `read`. + */ +import type { Asset, ContentEntry, NodeRef, Site, Snapshot } from "@cloudhal/core-contracts"; +import { assetCacheTag, nodeCacheTag, termCacheTag } from "./cache-tags.ts"; +import { fieldAssetRefs, siteFieldDefinitions } from "./media/field-refs.ts"; +import { type AssetRefUse, assetRefsOf } from "./media/refs.ts"; + +export interface AssetPurgeInput { + /** Every node that differs between the two snapshots. */ + readonly changed: readonly { before?: NodeRef; after?: NodeRef }[]; + /** The snapshot going live. */ + readonly to: Snapshot; + /** The planner's node reader (counted against `maxReads`, cached by hash). */ + readonly read: (ref: NodeRef) => Promise; + /** A term and its ancestors in either snapshot. */ + readonly withAncestors: (id: string) => string[]; + /** Collected tags and log lines, added to in place. */ + readonly tags: Set; + readonly why: string[]; +} + +/** Adds the asset rule's tags and log lines to `input.tags` and `input.why`. */ +export async function planAssetPurge(input: AssetPurgeInput): Promise { + const { tags, why, read } = input; + const shown = new Set(); + for (const c of input.changed) { + if (!c.before || c.before.kind !== "asset") continue; + const id = c.before.id; + tags.add(assetCacheTag(id)); + if (!c.after) { + why.push(`asset ${id} was removed`); + continue; + } + const [a, b] = await Promise.all([read(c.before), read(c.after)]); + if (a.slug !== b.slug || a.status !== b.status) why.push(`asset ${id}: slug or status changed`); + else why.push(`asset ${id} changed`); + shown.add(id); + } + if (shown.size === 0) return; + const refsOf = await referencesIn(input.to, read); + for (const ref of input.to.nodes) { + if (ref.kind !== "contentEntry" && ref.kind !== "page" && ref.kind !== "term") continue; + const uses = refsOf(ref.kind, await read(ref)); + if (!uses.some((r) => shown.has(r.assetId))) continue; + if (ref.kind === "term") + for (const id of input.withAncestors(ref.id)) tags.add(termCacheTag(id)); + else tags.add(nodeCacheTag(ref.id)); + } +} + +/** A node's asset references in `snapshot`: its own, and an entry's typed fields' by its defs. */ +async function referencesIn( + snapshot: Snapshot, + read: AssetPurgeInput["read"], +): Promise<(kind: string, body: unknown) => AssetRefUse[]> { + const siteRef = snapshot.nodes.find((n) => n.kind === "site" && n.id === snapshot.siteId); + const defs = siteRef ? siteFieldDefinitions(await read(siteRef)) : {}; + return (kind, body) => { + const refs = assetRefsOf(kind, body); + if (kind !== "contentEntry") return refs; + const entry = body as ContentEntry; + return [...refs, ...fieldAssetRefs(defs[entry.collection] ?? [], entry.fields)]; + }; +} diff --git a/packages/core-ts/src/cache-purge.ts b/packages/core-ts/src/cache-purge.ts index c802f9b6..b99854c7 100644 --- a/packages/core-ts/src/cache-purge.ts +++ b/packages/core-ts/src/cache-purge.ts @@ -19,16 +19,16 @@ * * - an **asset** (RFC-0004 §7.4) that left the live snapshot or changed: its media URLs * (`cloudhal-asset-`), and — when it changed — the responses that show it: the node tag of - * every entry and page that refers to it, and the term tag of every term (archives, chips) that - * does. A newly added asset purges nothing (404s are never cached, and whatever starts showing - * it changed itself). Media never carry the site tag, so the site-wide purges above leave them. + * every entry and page that refers to it (typed fields included, RFC-0045 §2.5), and the term tag + * of every term (archives, chips) that does (`cache-purge-assets.ts`). A newly added asset purges + * nothing (404s are never cached, and whatever starts showing it changed itself). Media never + * carry the site tag, so the site-wide purges above leave them. * * Falls back to purging everything when there is no previous snapshot, a snapshot or node cannot * be read, reading the diff would take more than `maxReads` node reads, or the result has more * than `maxTags` tags. */ import type { - Asset, ContentEntry, NodeRef, Site, @@ -36,15 +36,9 @@ import type { Taxonomy, Term, } from "@cloudhal/core-contracts"; -import { - assetCacheTag, - listCacheTag, - nodeCacheTag, - siteCacheTag, - termCacheTag, -} from "./cache-tags.ts"; +import { planAssetPurge } from "./cache-purge-assets.ts"; +import { listCacheTag, nodeCacheTag, siteCacheTag, termCacheTag } from "./cache-tags.ts"; import { canonicalJson } from "./hash.ts"; -import { assetRefsOf } from "./media/refs.ts"; export interface SnapshotReader { getSnapshot(id: string): Promise; @@ -202,34 +196,8 @@ export async function planCachePurge( const tags = new Set(); const why: string[] = []; - // Media: an asset that left the live snapshot or changed (§7.4). Its own URLs, and the - // responses that show it (they resolve its URL, alt, caption and size when they render). - const assetChanges = changed.filter((c) => (c.after ?? c.before)?.kind === "asset" && c.before); - if (assetChanges.length > 0) { - const shown = new Set(); - for (const c of assetChanges) { - const id = (c.before as NodeRef).id; - tags.add(assetCacheTag(id)); - if (!c.after) { - why.push(`asset ${id} was removed`); - continue; - } - const [a, b] = await Promise.all([read(c.before as NodeRef), read(c.after)]); - if (a.slug !== b.slug || a.status !== b.status) - why.push(`asset ${id}: slug or status changed`); - else why.push(`asset ${id} changed`); - shown.add(id); - } - if (shown.size > 0) - for (const ref of (to as Snapshot).nodes) { - if (ref.kind !== "contentEntry" && ref.kind !== "page" && ref.kind !== "term") continue; - const body = await read(ref); - if (!assetRefsOf(ref.kind, body).some((r) => shown.has(r.assetId))) continue; - if (ref.kind === "term") - for (const id of withAncestors(ref.id)) tags.add(termCacheTag(id)); - else tags.add(nodeCacheTag(ref.id)); - } - } + // Media (§7.4): an asset that left the live snapshot or changed (`cache-purge-assets.ts`). + await planAssetPurge({ changed, to: to as Snapshot, read, withAncestors, tags, why }); for (const c of termChanges) { const a = c.before ? await read(c.before) : undefined; diff --git a/packages/core-ts/src/media/refs-rebuild.ts b/packages/core-ts/src/media/refs-rebuild.ts index cadbd04c..fbcc3cc5 100644 --- a/packages/core-ts/src/media/refs-rebuild.ts +++ b/packages/core-ts/src/media/refs-rebuild.ts @@ -1,21 +1,57 @@ /** * `asset_refs` for a whole site from its node rows (RFC-0004 §4.4): sync import and repair. Entry * rows also count their typed fields' references (RFC-0041 §2) when the site's own row is among - * them. Worker-safe. + * them. Only references to assets of the same site are recorded (RFC-0045 §2.5): the `INSERT` + * joins `nodes`, so a foreign archive naming another site's asset neither blocks that asset's + * delete nor hides it from that site's unused list. Run it only in a batch that also writes the + * site's node set (after the node `INSERT`s): a rebuild beside an unchanged node set would replace + * rows a concurrent write committed with plan-time ones (#196). Worker-safe. */ import type { ContentEntry, Site } from "@cloudhal/core-contracts"; import type { SqlStatement } from "@cloudhal/storage"; import { fieldAssetRefs, siteFieldDefinitions } from "./field-refs.ts"; import { assetRefsOf } from "./refs.ts"; -/** `asset_refs` for a whole site from node rows (DELETE + one INSERT). */ +/** Each rebuild `INSERT` stays below this many bytes, SQL and parameters together (D1: 100 KB). */ +export const REFS_STATEMENT_BYTES = 90_000; + +/** `listAssets`' `unused` filter: no reference from a node of the asset's own site (RFC-0045 §2.5). */ +export const UNUSED_ASSET_SQL = + "NOT EXISTS (SELECT 1 FROM asset_refs r WHERE r.site_id = nodes.site_id AND r.asset_id = nodes.id)"; + +const INSERT_SQL = `INSERT INTO asset_refs (site_id, asset_id, node_id, node_kind, field) + SELECT ?, a.id, json_extract(r.value, '$.n'), json_extract(r.value, '$.k'), json_extract(r.value, '$.f') + FROM json_each(?) r JOIN nodes a ON a.id = json_extract(r.value, '$.a') AND a.site_id = ? AND a.kind = 'asset'`; + +const encoder = new TextEncoder(); + +/** The JSON-encoded rows, in `json_each` arrays of at most `budget` bytes each. */ +function chunks(items: readonly string[], budget: number): string[] { + const out: string[] = []; + let open: string[] = []; + let bytes = 2; + for (const item of items) { + const size = encoder.encode(item).length + 1; + if (open.length > 0 && bytes + size > budget) { + out.push(`[${open.join(",")}]`); + open = []; + bytes = 2; + } + open.push(item); + bytes += size; + } + if (open.length > 0) out.push(`[${open.join(",")}]`); + return out; +} + +/** `asset_refs` for a whole site from node rows: one `DELETE`, then `INSERT`s below 90 KB each. */ export function refsRebuild( siteId: string, rows: readonly { id: string; kind: string; body: string }[], ): SqlStatement[] { const siteRow = rows.find((r) => r.kind === "site" && r.id === siteId); const defs = siteRow ? siteFieldDefinitions(JSON.parse(siteRow.body) as Site) : {}; - const all: { a: string; n: string; k: string; f: string }[] = []; + const all = new Set(); for (const r of rows) { const node = JSON.parse(r.body); const refs = assetRefsOf(r.kind, node); @@ -23,16 +59,15 @@ export function refsRebuild( const entry = node as ContentEntry; refs.push(...fieldAssetRefs(defs[entry.collection] ?? [], entry.fields)); } - for (const ref of refs) all.push({ a: ref.assetId, n: r.id, k: r.kind, f: ref.field }); + for (const ref of refs) + all.add(JSON.stringify({ a: ref.assetId, n: r.id, k: r.kind, f: ref.field })); } - const out: SqlStatement[] = [ + const fixed = encoder.encode(INSERT_SQL + siteId + siteId).length; + return [ { sql: "DELETE FROM asset_refs WHERE site_id = ?", params: [siteId] }, + ...chunks([...all], REFS_STATEMENT_BYTES - fixed).map((rows) => ({ + sql: INSERT_SQL, + params: [siteId, rows, siteId], + })), ]; - if (all.length > 0) - out.push({ - sql: `INSERT OR IGNORE INTO asset_refs (site_id, asset_id, node_id, node_kind, field) - SELECT ?, json_extract(value, '$.a'), json_extract(value, '$.n'), json_extract(value, '$.k'), json_extract(value, '$.f') FROM json_each(?)`, - params: [siteId, JSON.stringify(all)], - }); - return out; } diff --git a/packages/core-ts/src/sync-plan.ts b/packages/core-ts/src/sync-plan.ts index da411635..e6b3f680 100644 --- a/packages/core-ts/src/sync-plan.ts +++ b/packages/core-ts/src/sync-plan.ts @@ -303,9 +303,11 @@ function nodeBatch( inserted++; } batch.push(...extra.lists); + // The derived reference index follows the node set in the same batch (RFC-0004 §4.4), and + // only then: beside an unchanged node set its plan-time rows could replace a concurrent + // write's fresher ones (#196). Its chunked `INSERT`s count against the cap below. + batch.push(...refsRebuild(siteId, changes.rows)); } - // The derived reference index follows the node set in the same batch (RFC-0004 §4.4). - batch.push(...refsRebuild(siteId, changes.rows)); const written = changes.rows.find((n) => n.kind === "site" && n.id === siteId) ?? siteRow; batch.push(...hostStatements(extra.hosts, { siteId, hash: written.hash })); if (batch.length > 1000) diff --git a/packages/core-ts/src/testing-asset-refs.ts b/packages/core-ts/src/testing-asset-refs.ts new file mode 100644 index 00000000..06d6d24d --- /dev/null +++ b/packages/core-ts/src/testing-asset-refs.ts @@ -0,0 +1,132 @@ +/** + * RFC-0045 §2.5 (rollout step 0) behavioural suite: `asset_refs` is scoped by site. The delete + * guard and the unused filter read only the asset's own site; a rebuild records only same-site + * assets, in `INSERT`s below 90 KB; an import planned `unchanged` does not rebuild, so it cannot + * replace a concurrent edit's row (#196). Called by `describeSiteGraphStore`, so it runs on every + * storage backend (better-sqlite3 and Miniflare D1). + */ +import type { Actor, Asset, ContentEntry, Site } from "@cloudhal/core-contracts"; +import { describe, expect, it } from "vitest"; +import { InvariantError } from "./errors.ts"; +import { PNG_1X1 } from "./media/fixtures.ts"; +import { REFS_STATEMENT_BYTES, refsRebuild } from "./media/refs-rebuild.ts"; +import { seedStarterSite } from "./seed.ts"; +import { exportSite } from "./sync.ts"; +import { prepareImport } from "./sync-plan.ts"; +import type { SuiteContext } from "./testing-taxonomy.ts"; + +const robot: Actor = { kind: "agent", id: "agent_01HZX8K3Q9YZ", name: "Newsroom Agent" }; + +export function assetRefsSuite(ctx: SuiteContext): void { + describe("RFC-0045 §2.5 asset_refs by site", () => { + const store = () => ctx.store(); + const storage = () => ({ sql: ctx.sql(), blobs: ctx.blobs() }); + + async function site(slug: string): Promise<{ site: Site; entry: ContentEntry }> { + const seeded = await seedStarterSite(store(), { name: slug, slug }); + const entry = (await store().listContentEntries(seeded.id))[0] as ContentEntry; + return { site: (await store().getSite(seeded.id)) as Site, entry }; + } + const upload = async (siteId: string, filename = "dot.png"): Promise => + ( + await store().ingestAsset({ + siteId, + bytes: PNG_1X1, + filename, + uploadedBy: robot, + source: { kind: "upload" }, + fields: { decorative: true }, + }) + ).asset; + const cover = (asset: Asset) => + ({ assetId: asset.id, alt: "" }) as NonNullable; + const rowsOf = (siteId: string) => + ctx + .sql() + .all<{ asset_id: string; node_id: string }>( + "SELECT asset_id, node_id FROM asset_refs WHERE site_id = ? ORDER BY asset_id, node_id", + [siteId], + ); + const unused = async (siteId: string) => + (await store().listAssets(siteId, { unused: true })).assets.map((a) => a.id); + + it("#196: an unchanged import racing an asset-adding edit keeps the edit's row", async () => { + const { site: s, entry } = await site("race"); + const asset = await upload(s.id); + const bundle = await exportSite(storage(), s.id); + // Planned before the edit, applied after it: `importSite`'s step 3 with a write in between. + const { plan, writes } = await prepareImport(storage(), bundle, {}); + expect(plan.unchanged).toBe(true); + expect(writes.batch.some((st) => st.sql.includes("asset_refs"))).toBe(false); + await store().updateContentEntry({ ...entry, coverImage: cover(asset) }); + if (writes.batch.length > 0) await ctx.sql().batch(writes.batch); + expect(await rowsOf(s.id)).toEqual([{ asset_id: asset.id, node_id: entry.id }]); + expect(await unused(s.id)).not.toContain(asset.id); + const refused = await store() + .deleteAsset(asset) + .then( + () => undefined, + (e: unknown) => e, + ); + expect(refused).toBeInstanceOf(InvariantError); + expect((refused as InvariantError).code).toBe("in_use"); + }); + + it("an archive in one site naming another site's asset neither blocks its delete nor hides it", async () => { + const x = await site("xsite"); + const y = await site("ysite"); + const own = await upload(x.site.id, "own.png"); + const theirs = await upload(y.site.id, "theirs.png"); + // Site X's node rows as a foreign archive would carry them: its entry names Y's asset, its + // site node (the logo) its own. + type Row = { id: string; kind: string; body: string }; + const rows = await ctx + .sql() + .all("SELECT id, kind, body FROM nodes WHERE site_id = ?", [x.site.id]); + const edit = (kind: string, patch: Record) => { + const row = rows.find((r) => r.kind === kind) as Row; + row.body = JSON.stringify({ ...JSON.parse(row.body), ...patch }); + }; + edit("contentEntry", { coverImage: cover(theirs) }); + edit("site", { brand: { ...x.site.brand, logo: cover(own) } }); + await ctx.sql().batch(refsRebuild(x.site.id, rows)); + expect(await rowsOf(x.site.id)).toEqual([{ asset_id: own.id, node_id: x.site.id }]); + // A row an older rebuild wrote unscoped: Y's asset under X stays unused and deletable in Y. + await ctx + .sql() + .run( + "INSERT INTO asset_refs (site_id, asset_id, node_id, node_kind, field) VALUES (?, ?, ?, 'contentEntry', 'coverImage')", + [x.site.id, theirs.id, x.entry.id], + ); + expect(await unused(y.site.id)).toContain(theirs.id); + expect(await unused(x.site.id)).not.toContain(own.id); + await store().deleteAsset(theirs); + expect(await store().getAsset(theirs.id)).toBeUndefined(); + }); + + it("a large rebuild stays under the statement size limit, in few statements", async () => { + const { site: s } = await site("large"); + const asset = await upload(s.id); + const rows = Array.from({ length: 5000 }, (_, i) => ({ + id: `entry_large${String(i).padStart(6, "0")}`, + kind: "contentEntry", + body: JSON.stringify({ + collection: "posts", + body: { format: "html", value: "" }, + coverImage: cover(asset), + }), + })); + const statements = refsRebuild(s.id, rows); + const bytes = (st: (typeof statements)[number]) => + new TextEncoder().encode(st.sql + (st.params ?? []).join("")).length; + expect(statements.length).toBeGreaterThan(2); + expect(statements.length).toBeLessThan(12); + for (const st of statements) expect(bytes(st)).toBeLessThan(REFS_STATEMENT_BYTES); + await ctx.sql().batch(statements); + const [count] = await ctx + .sql() + .all<{ n: number }>("SELECT count(*) AS n FROM asset_refs WHERE site_id = ?", [s.id]); + expect(count?.n).toBe(5000); + }); + }); +} diff --git a/packages/core-ts/src/testing-assets.ts b/packages/core-ts/src/testing-assets.ts index 9904621b..1392473e 100644 --- a/packages/core-ts/src/testing-assets.ts +++ b/packages/core-ts/src/testing-assets.ts @@ -838,7 +838,10 @@ export function assetsSuite(ctx: SuiteContext): void { const bundle = await exportSite({ sql: ctx.sql(), blobs: ctx.blobs() }, w.site.id); expect(bundle.media?.map((m) => m.sha256)).toEqual([asset.sha256]); expect(bundle.purged?.map((p) => p.sha256)).toEqual([gone.asset.sha256]); - // Into the same storage under replace: the derived tables are rebuilt from the bundle. + // Into the same storage under replace: the derived tables are rebuilt from the bundle with + // the node set it writes (an edit since the export makes the import write it, #196). + const now = (await store.getContentEntry(w.entry.id)) as ContentEntry; + await store.updateContentEntry({ ...now, title: "Edited since the export" }); await ctx.sql().run("DELETE FROM asset_refs WHERE site_id = ?", [w.site.id]); await importSite({ sql: ctx.sql(), blobs: ctx.blobs() }, bundle, { mode: "replace" }); expect((await store.assetUsage(w.site.id, [asset.id])).get(asset.id)?.total).toBe(1); diff --git a/packages/core-ts/src/testing-fields.ts b/packages/core-ts/src/testing-fields.ts index 1075a928..bb72f0b5 100644 --- a/packages/core-ts/src/testing-fields.ts +++ b/packages/core-ts/src/testing-fields.ts @@ -410,6 +410,8 @@ export function fieldsSuite(ctx: SuiteContext): void { const { asset } = await upload(site); const e = await entry(site, { fields: { poster: asset.id } }); const bundle = await exportSite({ sql: ctx.sql(), blobs: ctx.blobs() }, site.id); + // An edit since the export: the import writes the node set, and the index with it (#196). + await store().updateContentEntry({ ...e, title: "Edited since the export" }); await ctx.sql().run("DELETE FROM asset_refs WHERE site_id = ?", [site.id]); await importSite({ sql: ctx.sql(), blobs: ctx.blobs() }, bundle, { mode: "replace" }); expect(await refsOf(e.id)).toEqual([`fields.poster=${asset.id}`]); diff --git a/packages/core-ts/src/testing.ts b/packages/core-ts/src/testing.ts index a3193bde..207b0bc5 100644 --- a/packages/core-ts/src/testing.ts +++ b/packages/core-ts/src/testing.ts @@ -8,6 +8,7 @@ import { GATES_ENFORCED } from "./availability.ts"; import { ConflictError, UniqueConstraintError } from "./errors.ts"; import { nodeHash } from "./hash.ts"; import { SiteGraphStore } from "./store.ts"; +import { assetRefsSuite } from "./testing-asset-refs.ts"; import { assetsSuite } from "./testing-assets.ts"; import { collectionsSuite } from "./testing-collections.ts"; import { entryQuerySuite } from "./testing-entry-query.ts"; @@ -269,6 +270,7 @@ export function describeSiteGraphStore(label: string, factory: StoreFactory): vo taxonomySuite(suite); menuSuite(suite); assetsSuite(suite); + assetRefsSuite(suite); permalinksSuite(suite); collectionsSuite(suite); fieldsSuite(suite); diff --git a/packages/core-ts/test/cache-purge.test.ts b/packages/core-ts/test/cache-purge.test.ts index da2d656f..4228b244 100644 --- a/packages/core-ts/test/cache-purge.test.ts +++ b/packages/core-ts/test/cache-purge.test.ts @@ -2,7 +2,7 @@ * planCachePurge (RFC-0002 §7): a publish purges the tags of what changed between the live and the * new snapshot — node, term and list tags — and the site tag only for structure. */ -import type { ContentEntry, Site, Taxonomy, Term } from "@cloudhal/core-contracts"; +import type { ContentEntry, FieldDefinition, Site, Taxonomy, Term } from "@cloudhal/core-contracts"; import { MemoryBlobStore } from "@cloudhal/storage"; import { BetterSqlite3Driver } from "@cloudhal/storage/sqlite"; import { beforeEach, describe, expect, it } from "vitest"; @@ -191,6 +191,31 @@ describe("planCachePurge", () => { expect(removed).not.toContain(siteCacheTag(site.id)); }); + it("media (RFC-0045 §2.5): a changed asset purges the entries showing it in a typed image field", async () => { + const { asset } = await store.ingestAsset({ + siteId: site.id, + bytes: PNG_1X1, + filename: "poster.png", + uploadedBy: human, + source: { kind: "upload" }, + fields: { decorative: true }, + }); + const poster = { + key: "poster", + type: "image", + labels: { en: { label: "Poster" } }, + localized: false, + } as FieldDefinition; + const s = (await store.getSite(site.id)) as Site; + const posts = { ...s.collections?.posts, fields: [poster] } as never; + await store.updateSite({ ...s, collections: { ...s.collections, posts } }); + await store.updateContentEntry({ ...hello, fields: { poster: asset.id } }); + await publish(); + // Only the field shows the asset: its new slug moves the URL the article renders. + await store.updateAsset({ ...asset, slug: "poster-renamed" }); + expect(await tagsOf()).toEqual([assetCacheTag(asset.id), nodeCacheTag(hello.id)].sort()); + }); + it("structure purges the site: pages, redirects, term slugs, the site's settings", async () => { const about = (await store.listPages(site.id)).find((p) => p.path === "/about"); await store.updatePage({ ...(about as NonNullable), title: "About us" }); diff --git a/scripts/size-baseline.json b/scripts/size-baseline.json index 7343e61f..6bb37d93 100644 --- a/scripts/size-baseline.json +++ b/scripts/size-baseline.json @@ -414,7 +414,7 @@ "params": 5 }, "packages/core-ts/src/cache-purge.ts::planCachePurge": { - "lines": 233 + "lines": 207 }, "packages/core-ts/src/media/image.ts::inspectAvif": { "depth": 6 From 1eda0f9adebe579811066d672fb225a971d44432 Mon Sep 17 00:00:00 2001 From: Madraka <8898640+Madraka@users.noreply.github.com> Date: Wed, 11 Nov 2026 03:07:57 +0300 Subject: [PATCH 2/2] fix(core-ts): read field assets through a running rename's fromKey (#196 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #200 review: the purge planner's asset rule read field values only by their definition key, so mid-rename (phase A's marker written, phase B stopped on its budget) entries still holding the old key were missed, and an asset change left their cached pages pointing at the old URL. - migratedFieldValues (media/field-refs.ts) reads a defined key the entry does not hold yet from migration.fromKey, as resolveFields' read() does. - The purge asset rule (cache-purge-assets.ts) uses it. - refsRebuild uses it too: otherwise an archive exported mid-rename would drop those entries' asset_refs rows, and an asset they still show could be deleted (RFC-0045 §2.5 point 4 asks the same of entryRefsRebuild). The writer path and fieldAssetIssues are unchanged. - Tests: a cache-purge case with a budget-stopped rename_field and three entries showing the asset; a shared-suite rebuild case with a marker. Both fail without the fallback. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Madraka <8898640+Madraka@users.noreply.github.com> --- .changeset/asset-refs-by-site.md | 1 + packages/core-ts/src/cache-purge-assets.ts | 8 ++-- packages/core-ts/src/media/field-refs.ts | 20 ++++++++++ packages/core-ts/src/media/refs-rebuild.ts | 17 ++++---- packages/core-ts/src/testing-asset-refs.ts | 31 +++++++++++++++ packages/core-ts/test/cache-purge.test.ts | 45 ++++++++++++++++++++++ 6 files changed, 112 insertions(+), 10 deletions(-) diff --git a/.changeset/asset-refs-by-site.md b/.changeset/asset-refs-by-site.md index 45dff737..24ffb614 100644 --- a/.changeset/asset-refs-by-site.md +++ b/.changeset/asset-refs-by-site.md @@ -8,3 +8,4 @@ - The rebuild records only references to assets of the same site, and splits its `INSERT` so each statement stays below 90 KB (large sites could exceed D1's statement limit). Its statements count against the import's 1,000-statement cap. - Deleting an asset and the media library's "unused" filter look only at references from the asset's own site, so a foreign archive naming another site's asset no longer blocks its delete or hides it. - A publish that changes an asset now also purges the pages that show it through a typed `image` or `richText` field (RFC-0041), not only through covers, bodies and sections. +- While a field rename runs, entries not rewritten yet still hold the old key. Both the rebuild and the purge planner now read those entries through the definition's `migration.fromKey`, as the renderer does. An archive exported mid-rename keeps those entries' rows, and an asset change purges their pages. diff --git a/packages/core-ts/src/cache-purge-assets.ts b/packages/core-ts/src/cache-purge-assets.ts index a82b6f8c..556a6d25 100644 --- a/packages/core-ts/src/cache-purge-assets.ts +++ b/packages/core-ts/src/cache-purge-assets.ts @@ -4,13 +4,14 @@ * since they resolve its URL, alt, caption and size when they render: the node tag of every entry * and page that refers to it, and the term tag (with ancestors) of every term that does. An * entry's typed fields count too (RFC-0045 §2.5): an `image` field's value and a `richText` - * field's `asset:` links, read through the target snapshot's own field definitions. A newly added + * field's `asset:` links, read through the target snapshot's own field definitions (and, while a + * rename runs, through its `migration.fromKey`, as the renderer reads them). A newly added * asset purges nothing (404s are never cached, and whatever starts showing it changed itself). * Part of `planCachePurge` (`cache-purge.ts`); reads through its counted, hash-keyed `read`. */ import type { Asset, ContentEntry, NodeRef, Site, Snapshot } from "@cloudhal/core-contracts"; import { assetCacheTag, nodeCacheTag, termCacheTag } from "./cache-tags.ts"; -import { fieldAssetRefs, siteFieldDefinitions } from "./media/field-refs.ts"; +import { fieldAssetRefs, migratedFieldValues, siteFieldDefinitions } from "./media/field-refs.ts"; import { type AssetRefUse, assetRefsOf } from "./media/refs.ts"; export interface AssetPurgeInput { @@ -67,6 +68,7 @@ async function referencesIn( const refs = assetRefsOf(kind, body); if (kind !== "contentEntry") return refs; const entry = body as ContentEntry; - return [...refs, ...fieldAssetRefs(defs[entry.collection] ?? [], entry.fields)]; + const own = defs[entry.collection] ?? []; + return [...refs, ...fieldAssetRefs(own, migratedFieldValues(own, entry.fields))]; }; } diff --git a/packages/core-ts/src/media/field-refs.ts b/packages/core-ts/src/media/field-refs.ts index 9ef62c11..e47a3a8c 100644 --- a/packages/core-ts/src/media/field-refs.ts +++ b/packages/core-ts/src/media/field-refs.ts @@ -35,6 +35,26 @@ export function fieldAssetRefs( ); } +/** + * `values` as readers see them while a rename runs (RFC-0041 §6, `resolveFields`' `read`): a + * defined key the entry does not hold yet reads its definition's `migration.fromKey`. For the + * whole-site readers of stored bodies (the `asset_refs` rebuild, the purge planner's asset rule); + * a write's own checks read the values it writes. + */ +export function migratedFieldValues( + defs: readonly FieldDefinition[], + values: Readonly> | undefined, +): Readonly> | undefined { + if (!values || !defs.some((d) => d.migration?.fromKey)) return values; + const out: Record = { ...values }; + for (const def of defs) { + const from = def.migration?.fromKey; + if (from && !Object.hasOwn(values, def.key) && Object.hasOwn(values, from)) + out[def.key] = values[from]; + } + return out; +} + /** Field definitions per collection of a site node (`{}` when it has none). */ export function siteFieldDefinitions(site: { collections?: Record | undefined; diff --git a/packages/core-ts/src/media/refs-rebuild.ts b/packages/core-ts/src/media/refs-rebuild.ts index fbcc3cc5..fd57d33b 100644 --- a/packages/core-ts/src/media/refs-rebuild.ts +++ b/packages/core-ts/src/media/refs-rebuild.ts @@ -1,15 +1,17 @@ /** * `asset_refs` for a whole site from its node rows (RFC-0004 §4.4): sync import and repair. Entry * rows also count their typed fields' references (RFC-0041 §2) when the site's own row is among - * them. Only references to assets of the same site are recorded (RFC-0045 §2.5): the `INSERT` - * joins `nodes`, so a foreign archive naming another site's asset neither blocks that asset's - * delete nor hides it from that site's unused list. Run it only in a batch that also writes the - * site's node set (after the node `INSERT`s): a rebuild beside an unchanged node set would replace - * rows a concurrent write committed with plan-time ones (#196). Worker-safe. + * them, read through a running rename's `migration.fromKey` like `resolveFields` (an archive + * exported mid-operation keeps the rows of entries not rewritten yet). Only references to assets + * of the same site are recorded (RFC-0045 §2.5): the `INSERT` joins `nodes`, so a foreign archive + * naming another site's asset neither blocks that asset's delete nor hides it from that site's + * unused list. Run it only in a batch that also writes the site's node set (after the node + * `INSERT`s): a rebuild beside an unchanged node set would replace rows a concurrent write + * committed with plan-time ones (#196). Worker-safe. */ import type { ContentEntry, Site } from "@cloudhal/core-contracts"; import type { SqlStatement } from "@cloudhal/storage"; -import { fieldAssetRefs, siteFieldDefinitions } from "./field-refs.ts"; +import { fieldAssetRefs, migratedFieldValues, siteFieldDefinitions } from "./field-refs.ts"; import { assetRefsOf } from "./refs.ts"; /** Each rebuild `INSERT` stays below this many bytes, SQL and parameters together (D1: 100 KB). */ @@ -57,7 +59,8 @@ export function refsRebuild( const refs = assetRefsOf(r.kind, node); if (r.kind === "contentEntry") { const entry = node as ContentEntry; - refs.push(...fieldAssetRefs(defs[entry.collection] ?? [], entry.fields)); + const own = defs[entry.collection] ?? []; + refs.push(...fieldAssetRefs(own, migratedFieldValues(own, entry.fields))); } for (const ref of refs) all.add(JSON.stringify({ a: ref.assetId, n: r.id, k: r.kind, f: ref.field })); diff --git a/packages/core-ts/src/testing-asset-refs.ts b/packages/core-ts/src/testing-asset-refs.ts index 06d6d24d..b7ad9898 100644 --- a/packages/core-ts/src/testing-asset-refs.ts +++ b/packages/core-ts/src/testing-asset-refs.ts @@ -104,6 +104,37 @@ export function assetRefsSuite(ctx: SuiteContext): void { expect(await store().getAsset(theirs.id)).toBeUndefined(); }); + it("a rebuild mid-rename reads entries not rewritten yet through the marker's fromKey", async () => { + const { site: s, entry } = await site("rename"); + const asset = await upload(s.id); + type Row = { id: string; kind: string; body: string }; + const rows = await ctx + .sql() + .all("SELECT id, kind, body FROM nodes WHERE site_id = ?", [s.id]); + // Phase A of `rename_field posts.poster → cover` ran; phase B has not reached the entry. + const cover = { + key: "cover", + type: "image", + labels: { en: { label: "Cover" } }, + localized: false, + migration: { id: "snap_01HZX8K3Q9CD", fromKey: "poster" }, + }; + const siteRow = rows.find((r) => r.kind === "site") as Row; + const body = JSON.parse(siteRow.body) as Site; + const posts = { ...body.collections?.[entry.collection], fields: [cover] }; + siteRow.body = JSON.stringify({ + ...body, + collections: { ...body.collections, [entry.collection]: posts }, + }); + const entryRow = rows.find((r) => r.id === entry.id) as Row; + entryRow.body = JSON.stringify({ + ...JSON.parse(entryRow.body), + fields: { poster: asset.id }, + }); + await ctx.sql().batch(refsRebuild(s.id, rows)); + expect(await rowsOf(s.id)).toEqual([{ asset_id: asset.id, node_id: entry.id }]); + }); + it("a large rebuild stays under the statement size limit, in few statements", async () => { const { site: s } = await site("large"); const asset = await upload(s.id); diff --git a/packages/core-ts/test/cache-purge.test.ts b/packages/core-ts/test/cache-purge.test.ts index 4228b244..3a67dd81 100644 --- a/packages/core-ts/test/cache-purge.test.ts +++ b/packages/core-ts/test/cache-purge.test.ts @@ -13,6 +13,7 @@ import { siteCacheTag, termCacheTag, } from "../src/cache-tags.ts"; +import { runFieldOperation } from "../src/fields-operations/index.ts"; import { PNG_1X1 } from "../src/media/fixtures.ts"; import { seedStarterSite } from "../src/seed.ts"; import { type PublishChange, SiteGraphStore } from "../src/store.ts"; @@ -216,6 +217,50 @@ describe("planCachePurge", () => { expect(await tagsOf()).toEqual([assetCacheTag(asset.id), nodeCacheTag(hello.id)].sort()); }); + it("media (RFC-0045 §2.5): mid-rename, entries still holding the old key are purged too", async () => { + const { asset } = await store.ingestAsset({ + siteId: site.id, + bytes: PNG_1X1, + filename: "poster.png", + uploadedBy: human, + source: { kind: "upload" }, + fields: { decorative: true }, + }); + const poster = { + key: "poster", + type: "image", + labels: { en: { label: "Poster" } }, + localized: false, + } as FieldDefinition; + const s = (await store.getSite(site.id)) as Site; + const posts = { ...s.collections?.posts, fields: [poster] } as never; + await store.updateSite({ ...s, collections: { ...s.collections, posts } }); + const shown = [hello.id]; + await store.updateContentEntry({ ...hello, fields: { poster: asset.id } }); + for (const slug of ["second", "third"]) { + const e = await entry(slug, hello.terms, "published"); + await store.updateContentEntry({ ...e, fields: { poster: asset.id } }); + shown.push(e.id); + } + // Phase A wrote the marker (`cover`, from `poster`); phase B stopped on its budget. + const part = await runFieldOperation( + store, + site.id, + { op: "rename_field", collection: "posts", from: "poster", to: "cover" }, + { actor: human, budget: { entries: 1 } }, + ); + expect(part.done).toBe(false); + const stale = []; + for (const id of shown) + if ((await store.getContentEntry(id))?.fields?.poster !== undefined) stale.push(id); + expect(stale.length).toBeGreaterThan(0); + await publish(); + await store.updateAsset({ ...asset, slug: "poster-renamed" }); + expect(await tagsOf()).toEqual( + [assetCacheTag(asset.id), ...shown.map((id) => nodeCacheTag(id))].sort(), + ); + }); + it("structure purges the site: pages, redirects, term slugs, the site's settings", async () => { const about = (await store.listPages(site.id)).find((p) => p.path === "/about"); await store.updatePage({ ...(about as NonNullable), title: "About us" });