From 5eadba377e66d0488f5e18d917469f5a4a58a11e Mon Sep 17 00:00:00 2001 From: Aneesh Sathe Date: Sun, 18 Oct 2026 13:39:45 -0700 Subject: [PATCH 1/2] Duplicate-upload guard: same file, same item returns the existing row (#7) POST /api/media with an item_id hashes the bytes (SHA-256) before the R2 write. A row on that item with the same hash, MIME type and placement is returned as 200 { id, url, mime, duplicate: true } and nothing is stored. Migration 0028 adds a nullable media.sha256 (no backfill) with an (item_id, sha256) index, and recreates change_media_update in the remote-safe shape with the new column watched. Co-Authored-By: Claude Opus 5.5 --- docs/api.md | 2 +- migrations/0028_media_sha256.sql | 21 ++++++++++ openapi.json | 33 ++++++++++++++++ sdk/generated/types.gen.ts | 9 +++++ sdk/generated/zod.gen.ts | 21 ++++++---- src/api.ts | 21 +++++++--- src/contract/routes.ts | 2 + src/model.ts | 13 ++++++- test/mcp-media.oracle.test.ts | 24 ++++++++---- test/media-visibility-security.oracle.test.ts | 7 ++-- test/media.test.ts | 38 +++++++++++++++++++ 11 files changed, 164 insertions(+), 27 deletions(-) create mode 100644 migrations/0028_media_sha256.sql diff --git a/docs/api.md b/docs/api.md index 82ce842..01badc8 100644 --- a/docs/api.md +++ b/docs/api.md @@ -27,7 +27,7 @@ Consult `openapi.json` for every field and response. | Signals | `GET /signals` | `PUT` or `DELETE /signals/{sub}/{remoteId}` | | Mentions | `GET /mentions?direction=inbound` or `outbound` | `PATCH /mentions/{id}` with `hidden` | | Settings | `GET /settings` | `PATCH /settings` | -| Media | Included in item detail | `POST /media` with a multipart file (`inline=true` when the client places it in the text), `DELETE /media/{id}` (detaches; deletes the file only when no published version shows it) | +| Media | Included in item detail | `POST /media` with a multipart file (`inline=true` when the client places it in the text; the same file sent again to the same item in the same placement returns the existing row with `200` and `duplicate: true`), `DELETE /media/{id}` (detaches; deletes the file only when no published version shows it) | | Imported items | `GET /imports/{sub}/{id}`, `GET /imports/{sub}/{id}/history`, `GET /imports/{sub}/{id}/versions/{v}` (history and public versions, read from the origin) | The subscription importer manages these items | | Reading | `GET /reading`, `GET /reading/imported` (imported items only, cursor-paged; see below) | Read only | | Read state | `readVersion` on each imported Reading entry | `PUT`/`DELETE /reading/{sub}/{remoteId}/read`, `POST /reading/read`, `POST /reading/unread` | diff --git a/migrations/0028_media_sha256.sql b/migrations/0028_media_sha256.sql new file mode 100644 index 0000000..9041894 --- /dev/null +++ b/migrations/0028_media_sha256.sql @@ -0,0 +1,21 @@ +-- The upload guard (blygger-studio#7): a SHA-256 of an attachment's bytes, so +-- an identical file uploaded again to the same item returns the existing row +-- instead of storing a second copy. NULL for rows uploaded before this +-- migration (not backfilled) and for uploads not attached to an item. +ALTER TABLE media ADD COLUMN sha256 TEXT; +CREATE INDEX media_item_sha256 ON media(item_id, sha256); + +-- The polling cache's change trigger (0024) lists every column it watches, and +-- an oracle test holds it to the table's full column set. Recreated with +-- `sha256` added; otherwise identical to 0024's. +DROP TRIGGER IF EXISTS change_media_update; +CREATE TRIGGER change_media_update +AFTER UPDATE ON media +WHEN OLD."alt" IS NOT NEW."alt" OR OLD."created" IS NOT NEW."created" OR OLD."id" IS NOT NEW."id" OR OLD."inline" IS NOT NEW."inline" OR OLD."item_id" IS NOT NEW."item_id" OR OLD."mime" IS NOT NEW."mime" OR OLD."r2_key" IS NOT NEW."r2_key" OR OLD."sha256" IS NOT NEW."sha256" +BEGIN + SELECT RAISE(ABORT, 'missing change state') WHERE NOT EXISTS(SELECT 1 FROM change_state WHERE id=1); + UPDATE change_state SET + items=items+(1), + feed=feed+(1) + WHERE id=1; +END; diff --git a/openapi.json b/openapi.json index 1ad20a4..cfb2449 100644 --- a/openapi.json +++ b/openapi.json @@ -5417,6 +5417,39 @@ } }, "responses": { + "200": { + "description": "Already attached", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "url": { + "type": "string" + }, + "mime": { + "type": "string" + }, + "duplicate": { + "type": "boolean", + "enum": [ + true + ] + } + }, + "required": [ + "id", + "url", + "mime", + "duplicate" + ] + } + } + } + }, "201": { "description": "Success", "content": { diff --git a/sdk/generated/types.gen.ts b/sdk/generated/types.gen.ts index 67ca1d3..8d3ea3d 100644 --- a/sdk/generated/types.gen.ts +++ b/sdk/generated/types.gen.ts @@ -1800,6 +1800,15 @@ export type UploadMediaErrors = { export type UploadMediaError = UploadMediaErrors[keyof UploadMediaErrors]; export type UploadMediaResponses = { + /** + * Already attached + */ + 200: { + id: string; + url: string; + mime: string; + duplicate: true; + }; /** * Success */ diff --git a/sdk/generated/zod.gen.ts b/sdk/generated/zod.gen.ts index d103ace..faa50d9 100644 --- a/sdk/generated/zod.gen.ts +++ b/sdk/generated/zod.gen.ts @@ -858,14 +858,19 @@ export const zUploadMediaBody = z.object({ inline: z.enum(['true', 'false']).optional() }); -/** - * Success - */ -export const zUploadMediaResponse = z.object({ - id: z.string(), - url: z.string(), - mime: z.string() -}); +export const zUploadMediaResponse = z.union([ + z.object({ + id: z.string(), + url: z.string(), + mime: z.string(), + duplicate: z.literal(true) + }), + z.object({ + id: z.string(), + url: z.string(), + mime: z.string() + }) +]); export const zDeleteMediaPath = z.object({ id: z.string().min(1) diff --git a/src/api.ts b/src/api.ts index 3b96a1b..9a705da 100644 --- a/src/api.ts +++ b/src/api.ts @@ -23,6 +23,7 @@ import { getMedia, getSettings, getVersion, + findDuplicateMedia, insertMedia, pinVersion, publish, @@ -47,7 +48,7 @@ import { BLOCKING, staleThreads, threadFreshness } from "./freshness.ts"; import { draftChangeNote } from "./change-note.ts"; import { platformFetchFor } from "./importer/http.ts"; import { reconcileIndex } from "./importer/poll.ts"; -import { isFollowableUrl, isValidTimeZone, newMediaId, normalizeMount, nowIso } from "./util.ts"; +import { hex, isFollowableUrl, isValidTimeZone, newMediaId, normalizeMount, nowIso } from "./util.ts"; const MEDIA_TYPES: Record = { "image/png": "png", @@ -408,9 +409,19 @@ api.openapi(routes.uploadMedia, async (c) => { if (typeof itemId === "string" && itemId && !(await getItem(c.env.DB, itemId))) { return c.json({ error: "item_id not found" }, 404); } + const bytes = await file.arrayBuffer(); + // The studio sends inline=true for an upload it places in the text + // (studio#24): shown only where its line is, never appended. + const inline = form?.get("inline") === "true" ? 1 : 0; + // The same file (bytes and type) uploaded to the same item in the same + // placement is the same attachment (studio#7): return the row already + // there, store nothing. + const sha256 = typeof itemId === "string" && itemId ? hex(new Uint8Array(await crypto.subtle.digest("SHA-256", bytes))) : null; + const existing = sha256 ? await findDuplicateMedia(c.env.DB, itemId as string, sha256, file.type, inline) : null; + if (existing) return c.json({ id: existing.id, url: existing.r2_key, mime: existing.mime, duplicate: true }, 200); const id = newMediaId(); const r2Key = `media/${id}.${ext}`; - await c.env.MEDIA.put(r2Key, await file.arrayBuffer(), { httpMetadata: { contentType: file.type } }); + await c.env.MEDIA.put(r2Key, bytes, { httpMetadata: { contentType: file.type } }); const alt = form?.get("alt"); const row = await insertMedia(c.env.DB, { id, @@ -418,10 +429,8 @@ api.openapi(routes.uploadMedia, async (c) => { r2_key: r2Key, mime: file.type, alt: typeof alt === "string" ? alt : null, - // The studio sends inline=true for an upload it places in the text - // (studio#24): shown only where its line is, never appended. - inline: form?.get("inline") === "true" ? 1 : 0, - }, c.get('draftOnlyMedia') === true); + inline, + }, c.get('draftOnlyMedia') === true, sha256); if (!row) { await c.env.MEDIA.delete(r2Key); c.header('WWW-Authenticate', 'Bearer error="insufficient_scope", scope="owner:draft owner:publish"'); diff --git a/src/contract/routes.ts b/src/contract/routes.ts index df153f3..c30b38e 100644 --- a/src/contract/routes.ts +++ b/src/contract/routes.ts @@ -136,5 +136,7 @@ export const routes = { // Read routes contributed by the extensions in this repository (extensions/catalog.ts). ...extensionRoutes, }; +// An identical file uploaded again to the same item returns the existing row (studio#7). +routes.uploadMedia.responses[200] = { description: "Already attached", content: json(z.object({ id: z.string(), url: z.string(), mime: z.string(), duplicate: z.literal(true) })) }; // The resolve/confirm operation has two successful response shapes and statuses. routes.createSubscription.responses[201] = { description: "Subscribed", content: json(subscribed) }; diff --git a/src/model.ts b/src/model.ts index 2101503..bcefe2c 100644 --- a/src/model.ts +++ b/src/model.ts @@ -648,17 +648,26 @@ export async function insertMedia( db: D1Database, row: Omit, unpublishedOnly = false, + sha256: string | null = null, ): Promise { const created = nowIso(); // Evaluate publication and insert in one statement. A publisher can run while // the R2 upload awaits; a separate earlier read is not an authorization gate. const result = await db - .prepare("INSERT INTO media (id, item_id, r2_key, mime, alt, created, inline) SELECT ?, ?, ?, ?, ?, ?, ? WHERE ? = 0 OR ? IS NULL OR EXISTS (SELECT 1 FROM items WHERE id = ? AND version = 0)") - .bind(row.id, row.item_id, row.r2_key, row.mime, row.alt, created, row.inline, unpublishedOnly ? 1 : 0, row.item_id, row.item_id) + .prepare("INSERT INTO media (id, item_id, r2_key, mime, alt, created, inline, sha256) SELECT ?, ?, ?, ?, ?, ?, ?, ? WHERE ? = 0 OR ? IS NULL OR EXISTS (SELECT 1 FROM items WHERE id = ? AND version = 0)") + .bind(row.id, row.item_id, row.r2_key, row.mime, row.alt, created, row.inline, sha256, unpublishedOnly ? 1 : 0, row.item_id, row.item_id) .run(); return result.meta.changes ? { ...row, created } : null; } +/** The item's earlier upload of the same file (bytes and type) in the same placement, if any (migration 0028, studio#7). */ +export async function findDuplicateMedia(db: D1Database, itemId: string, sha256: string, mime: string, inline: number): Promise { + return db + .prepare("SELECT id, item_id, r2_key, mime, alt, created, inline FROM media WHERE item_id = ? AND sha256 = ? AND mime = ? AND inline = ? ORDER BY created ASC LIMIT 1") + .bind(itemId, sha256, mime, inline) + .first(); +} + export async function getMedia(db: D1Database, id: string): Promise { return db.prepare("SELECT * FROM media WHERE id = ?").bind(id).first(); } diff --git a/test/mcp-media.oracle.test.ts b/test/mcp-media.oracle.test.ts index b03b616..e2119b5 100644 --- a/test/mcp-media.oracle.test.ts +++ b/test/mcp-media.oracle.test.ts @@ -73,7 +73,14 @@ describe('MCP multipart media transport parity', () => { expect(await (await d.fetch('/blyg/items/' + item.id + '.json')).json()).toEqual(before); expect(await storedCount()).toEqual({ r2: 0, rows: 0 }); const permitted = await d.credential(['owner:draft', 'owner:publish']); - expect((await result(await d.upload(permitted, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id }))).error).toBe(false); + const attached = await result(await d.upload(permitted, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id })); + expect(attached.error).toBe(false); + // The same file again under draft scope is the attached row (studio#7): returned, nothing written. + const stored = await storedCount(), after = await (await d.fetch('/blyg/items/' + item.id + '.json')).json(); + const again = await result(await d.upload(token, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id })); + expect(again.error).toBe(false); expect(again.value).toEqual({ ...attached.value, duplicate: true }); + expect(await storedCount()).toEqual(stored); + expect(await (await d.fetch('/blyg/items/' + item.id + '.json')).json()).toEqual(after); }); it('discovers uploadMedia for draft scopes', async () => { @@ -84,16 +91,19 @@ describe('MCP multipart media transport parity', () => { }); it('uses the same bytes, MIME types and attachment metadata as multipart REST', async () => { const d = await driver(), token = await d.credential(['owner:draft']); - const item = await (await d.owner('/api/items', { content_md: 'media parity' })).json() as any; + // One item per transport: the same file sent twice to one item is a + // duplicate (studio#7) and would not exercise MCP's own write. + const restItem = await (await d.owner('/api/items', { content_md: 'media parity (REST)' })).json() as any; + const mcpItem = await (await d.owner('/api/items', { content_md: 'media parity (MCP)' })).json() as any; const bytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 1, 2, 3]); for (const contentType of ['image/png', 'image/jpeg', 'image/gif', 'image/webp', 'image/svg+xml']) { - const extra = { item_id: item.id, alt: 'a diagram', inline: 'true' }; - const rest = await d.restUpload(bytes, contentType, extra); expect(rest.status).toBe(201); + const extra = { alt: 'a diagram', inline: 'true' }; + const rest = await d.restUpload(bytes, contentType, { item_id: restItem.id, ...extra }); expect(rest.status).toBe(201); const expected = await rest.json() as any; - const observed = await result(await d.upload(token, { file: { filename: 'same-file.dat', contentType, dataBase64: base64(bytes) }, ...extra })); - expect(observed.error).toBe(false); expect(observed.value.mime).toBe(expected.mime); + const observed = await result(await d.upload(token, { file: { filename: 'same-file.dat', contentType, dataBase64: base64(bytes) }, item_id: mcpItem.id, ...extra })); + expect(observed.error).toBe(false); expect(observed.value.mime).toBe(expected.mime); expect(observed.value.duplicate).toBeUndefined(); expect(observed.value.url.split('.').pop()).toBe(expected.url.split('.').pop()); - for (const value of [expected, observed.value]) { + for (const [value, item] of [[expected, restItem], [observed.value, mcpItem]]) { expect((await d.fetch('/blyg/' + value.url)).status).toBe(404); const publicBytes = await d.fetch('/blyg/' + value.url, { headers: { cookie: d.cookie } }); expect(publicBytes.status).toBe(200); expect(publicBytes.headers.get('content-type')).toBe(contentType); diff --git a/test/media-visibility-security.oracle.test.ts b/test/media-visibility-security.oracle.test.ts index 8233205..4092b2e 100644 --- a/test/media-visibility-security.oracle.test.ts +++ b/test/media-visibility-security.oracle.test.ts @@ -28,8 +28,8 @@ async function fixture() { const login = await receive('/blyg/studio/login', { method: 'POST', headers: { 'CF-Connecting-IP': 'fd00:' + crypto.randomUUID().replaceAll('-', '').match(/.{4}/g)!.slice(0,7).join(':') }, body: new URLSearchParams({ password: env.OWNER_PASSWORD }) }); expect(login.status).toBe(302); const cookie = login.headers.get('set-cookie')!.split(';')[0]; const owner = (path: string, method = 'GET', body?: unknown) => receive(path, { method, headers: { cookie, 'Content-Type': 'application/json' }, ...(body === undefined ? {} : { body: JSON.stringify(body) }) }); - const upload = async (item?: string, inline = false) => { - const form = new FormData(); form.set('file', new File(['private fixture bytes'], 'private.png', { type: 'image/png' })); + const upload = async (item?: string, inline = false, bytes = 'private fixture bytes') => { + const form = new FormData(); form.set('file', new File([bytes], 'private.png', { type: 'image/png' })); if (item) form.set('item_id', item); if (inline) form.set('inline', 'true'); const response = await receive('/api/media', { method: 'POST', headers: { cookie }, body: form }); expect(response.status).toBe(201); return response.json() as Promise<{id:string;url:string}>; @@ -75,7 +75,8 @@ it.each([false, true])('withdrawal keeps published media and leaves later upload // Attaching to an ever-published item is itself a publish act (publish scope); // an inline upload is public only once published text shows it. if (inline) { - const later = await f.upload(id, true); + // Other bytes: the same file again would be the published row (studio#7). + const later = await f.upload(id, true, 'later fixture bytes'); expect((await receive('/blyg/' + later.url)).status, 'an inline upload no published text showed stays private on a withdrawn item').toBe(404); } }); diff --git a/test/media.test.ts b/test/media.test.ts index f55bb77..e9f5b0e 100644 --- a/test/media.test.ts +++ b/test/media.test.ts @@ -84,3 +84,41 @@ describe("avatar resolution (session 19)", () => { }); }); + +describe("duplicate uploads (studio#7, migration 0028)", () => { + const png = (...b: number[]) => new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, ...b])], "p.png", { type: "image/png" }); + + it("returns the existing row for the same bytes on the same item, and stores nothing", async () => { + const cookie = await login(); + const id = await createAndPublish(cookie, "fragment with a repeated image"); + const first = await upload(cookie, png(1, 2), { item_id: id, alt: "first" }); + expect(first.status).toBe(201); + const again = await upload(cookie, png(1, 2), { item_id: id, alt: "second" }); + expect(again.status).toBe(200); + expect(again.json).toEqual({ id: first.json.id, url: first.json.url, mime: "image/png", duplicate: true }); + + const item = await (await getPublic(`/blyg/items/${id}.json`)).json(); + expect(item.media).toEqual([{ url: first.json.url, mime: "image/png", alt: "first" }]); + }); + + it("stores a new row for different bytes, another type, another item, another placement, or no item", async () => { + const cookie = await login(); + const a = await createAndPublish(cookie, "item a"); + const b = await createAndPublish(cookie, "item b"); + const base = await upload(cookie, png(3), { item_id: a }); + expect(base.status).toBe(201); + const others = [ + await upload(cookie, png(4), { item_id: a }), + await upload(cookie, new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, 3])], "p.webp", { type: "image/webp" }), { item_id: a }), + await upload(cookie, png(3), { item_id: b }), + await upload(cookie, png(3), { item_id: a, inline: "true" }), + await upload(cookie, png(3)), + await upload(cookie, png(3)), + ]; + for (const r of others) { + expect(r.status).toBe(201); + expect(r.json.duplicate).toBeUndefined(); + } + expect(new Set([base, ...others].map((r) => r.json.id)).size).toBe(7); + }); +}); From 1c44ac8bc62a28a42ecd8c40281c29e050320ae2 Mon Sep 17 00:00:00 2001 From: Aneesh Sathe Date: Sun, 18 Oct 2026 14:41:25 -0700 Subject: [PATCH 2/2] Attachments per version: a removal drops from the next version only (#7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration 0031 adds version_media(item_id, version, media_id), written at publish with the version and never changed. The item document, RSS, feed cards and item pages read the version's own list, so a later version can go without an attachment while the earlier ones keep listing and serving it (§5.4). The backfill keeps every live document as it was. DELETE /api/media/{id} now detaches anything a version lists, not only what a version's text shows: an appended attachment's bytes were deleted while its version still listed them. An upload to a published item is listed from the next version on and marks the item dirty. GET /api/items/{id} gains published_media, and the editor offers "drop from the next version". Co-Authored-By: Claude Opus 5.5 --- docs/api.md | 2 +- e2e/studio.spec.ts | 36 +++++++++ migrations/0031_version_media.sql | 29 ++++++++ openapi.json | 11 ++- sdk/generated/types.gen.ts | 7 ++ sdk/generated/zod.gen.ts | 1 + src/api.ts | 16 +++- src/contract/routes.ts | 2 +- src/index.ts | 11 ++- src/item-data.ts | 7 +- src/model.ts | 20 ++++- src/pages.ts | 12 +-- src/protocol.ts | 12 +-- src/public-feed.ts | 10 ++- src/read-api.ts | 2 +- src/ui/authoring.tsx | 21 +++++- src/ui/data-schemas.ts | 1 + src/ui/data.ts | 4 +- test-ui/source-collections.test.ts | 2 +- test/media.test.ts | 3 + test/pages.test.ts | 2 + test/public-page-performance.test.ts | 4 +- test/version-media.test.ts | 107 +++++++++++++++++++++++++++ 23 files changed, 286 insertions(+), 36 deletions(-) create mode 100644 migrations/0031_version_media.sql create mode 100644 test/version-media.test.ts diff --git a/docs/api.md b/docs/api.md index 01badc8..cf378dc 100644 --- a/docs/api.md +++ b/docs/api.md @@ -27,7 +27,7 @@ Consult `openapi.json` for every field and response. | Signals | `GET /signals` | `PUT` or `DELETE /signals/{sub}/{remoteId}` | | Mentions | `GET /mentions?direction=inbound` or `outbound` | `PATCH /mentions/{id}` with `hidden` | | Settings | `GET /settings` | `PATCH /settings` | -| Media | Included in item detail | `POST /media` with a multipart file (`inline=true` when the client places it in the text; the same file sent again to the same item in the same placement returns the existing row with `200` and `duplicate: true`), `DELETE /media/{id}` (detaches; deletes the file only when no published version shows it) | +| Media | Included in item detail: `media` is the working copy's attachments, `published_media` what the live version lists | `POST /media` with a multipart file (`inline=true` when the client places it in the text; the same file sent again to the same item in the same placement returns the existing row with `200` and `duplicate: true`). Attachments are per version: one added to a published item is listed from its next version on. `DELETE /media/{id}` detaches it from the working copy, so the next version goes without it while the versions that listed it keep it; the file is deleted only when no published version lists or shows it | | Imported items | `GET /imports/{sub}/{id}`, `GET /imports/{sub}/{id}/history`, `GET /imports/{sub}/{id}/versions/{v}` (history and public versions, read from the origin) | The subscription importer manages these items | | Reading | `GET /reading`, `GET /reading/imported` (imported items only, cursor-paged; see below) | Read only | | Read state | `readVersion` on each imported Reading entry | `PUT`/`DELETE /reading/{sub}/{remoteId}/read`, `POST /reading/read`, `POST /reading/unread` | diff --git a/e2e/studio.spec.ts b/e2e/studio.spec.ts index 2c7e39c..fa00dd6 100644 --- a/e2e/studio.spec.ts +++ b/e2e/studio.spec.ts @@ -281,6 +281,42 @@ test("an uploaded image leaves with its line, and an unused attachment can be re expect(errors).toEqual([]); }); +test("an attachment the live version lists is dropped from the next version only", async ({ page }) => { + const errors: string[] = []; + page.on("pageerror", (error) => errors.push(error.message)); + await page.goto("/studio/login"); + await page.locator('[name="password"]').fill("test-password"); + await page.getByRole("button", { name: "log in", exact: true }).click(); + await expect(page.locator("#composer-text")).toBeVisible(); + // An appended attachment (another tool's upload, not placed in the text), + // published. In-page fetches, so the owner cookie (Secure) rides along. + const id = await page.evaluate(async (stamp) => { + const json = (method: string, path: string, body: unknown) => + fetch(path, { method, headers: { "content-type": "application/json" }, body: JSON.stringify(body) }); + const { id } = await (await json("POST", "/api/items", { content_md: `Per-version fixture ${stamp}` })).json(); + const form = new FormData(); + form.set("item_id", id); + form.set("alt", "the figure"); + form.set("file", new File([`PNG fixture ${stamp}`], "fig.png", { type: "image/png" })); + if ((await fetch("/api/media", { method: "POST", body: form })).status !== 201) throw new Error("upload failed"); + if ((await json("POST", `/api/items/${id}/publish`, {})).status !== 200) throw new Error("publish failed"); + return id as string; + }, `${test.info().project.name} ${Date.now()}`); + await page.goto(`/studio/edit/${id}`); + await openCard(page, "attachments"); + const row = page.locator('.media:not([data-dropped]) .attachment'); + await expect(row).toContainText("shown below the post"); + const drop = row.locator('[data-action="remove-media"]'); + await expect(drop).toHaveText("drop from the next version"); + const removed = page.waitForResponse((r) => r.url().includes("/api/media/") && r.request().method() === "DELETE"); + await drop.click(); + expect((await removed).status()).toBe(200); + const dropped = page.locator("[data-dropped] .attachment"); + await expect(dropped).toContainText("the figure"); + await expect(dropped).toContainText("leaves with the next one"); + expect(errors).toEqual([]); +}); + test("the composer autosaves real text, and not a stray keystroke", async ({ page }) => { const errors: string[] = []; page.on("pageerror", (error) => errors.push(error.message)); diff --git a/migrations/0031_version_media.sql b/migrations/0031_version_media.sql new file mode 100644 index 0000000..304b069 --- /dev/null +++ b/migrations/0031_version_media.sql @@ -0,0 +1,29 @@ +-- Attachments per version (blygger-studio#7). §5.4 makes media part of a +-- version: each version's document carries its own `media`, and a later +-- version may leave an attachment out while the earlier ones keep serving it. +-- Until now every row attached to an item was listed on every version, so a +-- removed attachment could only leave by detaching it from the live version +-- too. A row here says the version showed that attachment; it is written at +-- publish and never changed, like the version itself. Reads join `media` by +-- id, so a row detached since still lists on the versions that showed it. +CREATE TABLE version_media ( + item_id TEXT NOT NULL, + version INTEGER NOT NULL, + media_id TEXT NOT NULL, + PRIMARY KEY (item_id, version, media_id) +); +CREATE INDEX version_media_media ON version_media(media_id); + +-- Backfill, so no live document changes at upgrade: the latest published +-- version keeps every attachment it lists today (appended, or placed in its +-- text). An earlier version gets only what it can have shown, an attachment +-- uploaded no later than it was. Rows detached before this migration are not +-- recovered; they were already off every version. +INSERT OR IGNORE INTO version_media (item_id, version, media_id) +SELECT v.item_id, v.version, m.id +FROM versions v +JOIN items i ON i.id = v.item_id +JOIN media m ON m.item_id = v.item_id +WHERE v.content_html IS NOT NULL AND v.content_html <> '' + AND (v.version = i.version OR julianday(m.created) <= julianday(v.published_at)) + AND (m.inline = 0 OR instr(v.content_html, '/' || m.r2_key) > 0 OR instr(v.content_html, '"' || m.r2_key) > 0); diff --git a/openapi.json b/openapi.json index cfb2449..59d4643 100644 --- a/openapi.json +++ b/openapi.json @@ -4061,7 +4061,15 @@ "type": "array", "items": { "$ref": "#/components/schemas/Media" - } + }, + "description": "The working copy's attachments: what the next version lists." + }, + "published_media": { + "type": "array", + "items": { + "$ref": "#/components/schemas/Media" + }, + "description": "What the live version lists, including attachments removed from the working copy since (studio#7)." }, "versions": { "type": "array", @@ -4097,6 +4105,7 @@ "fork_cite", "authored_kind", "media", + "published_media", "versions", "published" ] diff --git a/sdk/generated/types.gen.ts b/sdk/generated/types.gen.ts index 8d3ea3d..63b070b 100644 --- a/sdk/generated/types.gen.ts +++ b/sdk/generated/types.gen.ts @@ -1186,7 +1186,14 @@ export type GetItemResponses = { forked_from: VersionReference | null; fork_cite: Citation | null; authored_kind: 'fragment' | 'thread'; + /** + * The working copy's attachments: what the next version lists. + */ media: Array; + /** + * What the live version lists, including attachments removed from the working copy since (studio#7). + */ + published_media: Array; versions: Array; published: Version | null; }; diff --git a/sdk/generated/zod.gen.ts b/sdk/generated/zod.gen.ts index faa50d9..fd0cf8c 100644 --- a/sdk/generated/zod.gen.ts +++ b/sdk/generated/zod.gen.ts @@ -720,6 +720,7 @@ export const zGetItemResponse = z.object({ fork_cite: zCitation.nullable(), authored_kind: z.enum(['fragment', 'thread']), media: z.array(zMedia), + published_media: z.array(zMedia), versions: z.array(zVersion), published: zVersion.nullable() }); diff --git a/src/api.ts b/src/api.ts index 9a705da..f35e83f 100644 --- a/src/api.ts +++ b/src/api.ts @@ -436,6 +436,8 @@ api.openapi(routes.uploadMedia, async (c) => { c.header('WWW-Authenticate', 'Bearer error="insufficient_scope", scope="owner:draft owner:publish"'); return c.json({ error: 'owner:publish required to attach media to a published item' }, 403); } + // A published item lists it from its next version on (migration 0031). + if (row.item_id) await c.env.DB.prepare("UPDATE items SET dirty = 1 WHERE id = ? AND status = 'public'").bind(row.item_id).run(); c.header("Location", `/${normalizeMount(c.env.MOUNT).replace(/^\//, "")}/${row.r2_key}`.replace(/^\/\//, "/")); return c.json({ id: row.id, url: row.r2_key, mime: row.mime }, 201); }); @@ -443,8 +445,9 @@ api.openapi(routes.uploadMedia, async (c) => { /** * Remove an attachment from its item (studio#24, #7). A media URL is a promise * to serve the same bytes (§5.4), so bytes any published version still shows - * are kept and the row is only detached — it leaves the item, its page and its - * `media` list. Bytes nothing published references are deleted outright. + * are kept and the row is only detached: it leaves the working copy, so the + * next version goes without it, while the versions that listed it keep it + * (migration 0031). Bytes nothing published references are deleted outright. * The avatar is not an attachment and is refused. */ api.openapi(routes.deleteMedia, async (c) => { @@ -452,9 +455,14 @@ api.openapi(routes.deleteMedia, async (c) => { const row = await getMedia(db, c.req.param("id")); if (!row) return c.json({ error: "not found" }, 404); if ((await getSettings(db)).avatar_media_id === row.id) return c.json({ error: "this is the avatar; change it in Settings" }, 409); - const published = await db.prepare("SELECT 1 FROM versions WHERE instr(content_html, ?) > 0 LIMIT 1").bind(row.r2_key).first(); + // An appended attachment is in no version's text, only in its list. + const published = await db.prepare("SELECT 1 WHERE EXISTS (SELECT 1 FROM version_media WHERE media_id = ?) OR EXISTS (SELECT 1 FROM versions WHERE instr(content_html, ?) > 0)").bind(row.id, row.r2_key).first(); if (published) { - await db.prepare("UPDATE media SET item_id = NULL WHERE id = ?").bind(row.id).run(); + // The working copy now differs from the live version, which still lists it. + await db.batch([ + db.prepare("UPDATE items SET dirty = 1 WHERE id = ? AND status = 'public' AND EXISTS (SELECT 1 FROM version_media WHERE item_id = ? AND version = items.version AND media_id = ?)").bind(row.item_id, row.item_id, row.id), + db.prepare("UPDATE media SET item_id = NULL WHERE id = ?").bind(row.id), + ]); return c.json({ ok: true, outcome: "detached" as const }); } await c.env.MEDIA.delete(row.r2_key); diff --git a/src/contract/routes.ts b/src/contract/routes.ts index c30b38e..dd1d66b 100644 --- a/src/contract/routes.ts +++ b/src/contract/routes.ts @@ -108,7 +108,7 @@ export const routes = { markUnreadBatch: route("markUnreadBatch", "post", "/reading/unread", ok.extend({ received: z.number().int().nonnegative() }), z.object({ items: z.array(unreadMark).max(READ_BATCH_MAX) })), updateMention: route("updateMention", "patch", "/mentions/{id}", ok.extend({ hidden: z.boolean() }), z.object({ hidden: z.boolean() })), listItems: route("listItems", "get", "/items", z.object({ items: z.array(ItemSchema.extend({ pins: z.array(z.object({ version: z.number().int().positive(), kind: z.enum(["fragment", "thread"]) })).optional() })), total: z.number(), offset: z.number(), limit: z.number() }), undefined, 200, page), - getItem: route("getItem", "get", "/items/{id}", z.object({ ...ItemSchema.shape, authored_kind: z.enum(["fragment", "thread"]), media: z.array(MediaSchema), versions: z.array(VersionSchema), published: z.union([VersionSchema, z.null()]) })), + getItem: route("getItem", "get", "/items/{id}", z.object({ ...ItemSchema.shape, authored_kind: z.enum(["fragment", "thread"]), media: z.array(MediaSchema).describe("The working copy's attachments: what the next version lists."), published_media: z.array(MediaSchema).describe("What the live version lists, including attachments removed from the working copy since (studio#7)."), versions: z.array(VersionSchema), published: z.union([VersionSchema, z.null()]) })), getSettings: route("getSettings", "get", "/settings", SettingsSchema), listSubscriptions: route("listSubscriptions", "get", "/subscriptions", collection(SubscriptionSchema), undefined, 200, page), getSubscription: route("getSubscription", "get", "/subscriptions/{id}", SubscriptionSchema), diff --git a/src/index.ts b/src/index.ts index 51bfaee..b0c0fb6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -305,15 +305,18 @@ export function makeApp(mount: string) { if (!media || media.r2_key !== `media/${file}`) return c.notFound(); // §5.4: a published media URL MUST always serve the same bytes, and §9 // withdrawal does not cascade. Public uses: the avatar, an attachment on an - // item ever published (attaching there needs publish scope), and anything - // a published version's text shows. Unused uploads stay owner-only. - // Indexed checks run first; the scan of every version is the last resort. + // item ever published (attaching there needs publish scope), anything a + // published version lists (migration 0031: detached rows included), and + // anything a published version's text shows. Unused uploads stay + // owner-only. Indexed checks run first; the scan of every version is the + // last resort. const publicUse = await c.env.DB.prepare(`SELECT 1 WHERE EXISTS (SELECT 1 FROM settings WHERE key='avatar_media_id' AND value=?) OR EXISTS (SELECT 1 FROM items WHERE id=? AND version>0 AND ?<>1) + OR EXISTS (SELECT 1 FROM version_media WHERE media_id=?) OR EXISTS (SELECT 1 FROM versions WHERE item_id=? AND instr(content_html, ?) > 0) OR EXISTS (SELECT 1 FROM versions WHERE instr(content_html, ?) > 0)`) - .bind(media.id, media.item_id, media.inline ?? 0, media.item_id, media.r2_key, media.r2_key).first(); + .bind(media.id, media.item_id, media.inline ?? 0, media.id, media.item_id, media.r2_key, media.r2_key).first(); if (!publicUse) { c.header('Cache-Control', 'no-store'); const url = new URL(c.req.url); diff --git a/src/item-data.ts b/src/item-data.ts index ffed5ae..410046f 100644 --- a/src/item-data.ts +++ b/src/item-data.ts @@ -9,9 +9,10 @@ const detailSql = `WITH selected AS (SELECT * FROM items WHERE id = ?) SELECT 'item' AS type, ${projection(ItemRowSchema, "i")}, 0 AS section, 0 AS version_order, '' AS created_order FROM selected i UNION ALL SELECT 'media', ${projection(MediaRowSchema, "m")}, 1, 0, m.created FROM media m WHERE m.item_id IN (SELECT id FROM selected) UNION ALL SELECT 'version', ${projection(VersionRowSchema, "v")}, 2, v.version, '' FROM versions v WHERE v.item_id IN (SELECT id FROM selected) + UNION ALL SELECT 'published_media', ${projection(MediaRowSchema, "m")}, 3, 0, m.created FROM selected i JOIN version_media vm ON vm.item_id = i.id AND vm.version = i.version JOIN media m ON m.id = vm.media_id ORDER BY section, version_order, created_order`; -/** One D1 query, with one result row per item, attachment, or history version. */ +/** One D1 query, with one result row per item, attachment, history version, or attachment the live version lists. */ export async function itemDetail(db: D1Database, id: string) { const { results } = await db.prepare(detailSql).bind(id).all & { type: string }>(); const itemRow = results.find((row) => row.type === "item"); @@ -19,7 +20,9 @@ export async function itemDetail(db: D1Database, id: string) { const item = ItemRowSchema.parse(itemRow); const media = results.filter((row) => row.type === "media").map((row) => MediaRowSchema.parse(row)); const versions = results.filter((row) => row.type === "version").map((row) => VersionRowSchema.parse(row)); + // What the live version lists (migration 0031), detached rows included. + const publishedMedia = results.filter((row) => row.type === "published_media").map((row) => MediaRowSchema.parse(row)); const published = versions.find((v) => v.version === item.version) ?? null; const kind = item.kind === "thread" || (item.kind === "withdrawn" && versions.find((v) => v.version === item.version - 1)?.transclusions) ? "thread" : "fragment"; - return { item, kind, media, versions, published }; + return { item, kind, media, versions, published, publishedMedia }; } diff --git a/src/model.ts b/src/model.ts index bcefe2c..0652e38 100644 --- a/src/model.ts +++ b/src/model.ts @@ -20,7 +20,7 @@ import { import type { ForkedFrom, ItemRow, MediaRow, ScopeProvenance, Settings, StubCite, StubOf, Transclusion, VersionRow } from "./types.ts"; import { FRAGMENT_MAX_CHARS } from "./types.ts"; import { parseEnabledExtensions } from "./extensions/names.ts"; -import { absolutizeHtml, authoredText, contentHash, isFollowableUrl, newId, nowIso } from "./util.ts"; +import { absolutizeHtml, authoredText, contentHash, isFollowableUrl, newId, nowIso, visibleMedia } from "./util.ts"; export { TkPublishError, TransclusionResolveError }; @@ -459,10 +459,17 @@ export async function publish(db: D1Database, item: ItemRow, note: string | null const citeJson = agreed ? JSON.stringify(await composeStubCite(db, agreed, normalizedOrigin(origin), settings.site_title, now)) : null; const hash = await contentHash(strippedMd); + // The attachments this version shows (§5.4, studio#7): appended ones, and + // inline ones its text places. Frozen with the version, so dropping one from + // the working copy changes the next version and never this one. + const shown = JSON.stringify(visibleMedia(await listMediaForItem(db, item.id), contentHtml).map((m) => m.id)); await db.batch([ db.prepare( "INSERT INTO versions (item_id, version, content_md, content_html, content_hash, published_at, note, transclusions, generated_json, stub_of, stub_cite, note_generated) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", ).bind(item.id, version, strippedMd, contentHtml, hash, now, note, transclusionsJson, generatedJson, stubJson, citeJson, note && noteGenerated ? 1 : 0), + // Rows still attached at the write, so a removal racing the publish wins. + db.prepare("INSERT INTO version_media (item_id, version, media_id) SELECT ?, ?, m.id FROM json_each(?) s JOIN media m ON m.id = s.value WHERE m.item_id = ?") + .bind(item.id, version, shown, item.id), // The agreed citation goes back into the working copy as well, or the copy // keeps citing the older version and differs from what it just published. db.prepare("UPDATE items SET status = 'public', kind = ?, version = ?, dirty = 0, updated = ?, stub_of = ? WHERE id = ?") @@ -672,6 +679,17 @@ export async function getMedia(db: D1Database, id: string): Promise(); } +/** The attachments a published version shows (migration 0031), detached rows included: what that version's document and pages list. */ +export async function listVersionMedia(db: D1Database, itemId: string, version: number): Promise { + return ( + await db + .prepare("SELECT m.* FROM version_media vm JOIN media m ON m.id = vm.media_id WHERE vm.item_id = ? AND vm.version = ? ORDER BY m.created ASC") + .bind(itemId, version) + .all() + ).results; +} + +/** The working copy's attachments: what the next version will show. */ export async function listMediaForItem(db: D1Database, itemId: string): Promise { return ( await db.prepare("SELECT * FROM media WHERE item_id = ? ORDER BY created ASC").bind(itemId).all() diff --git a/src/pages.ts b/src/pages.ts index d034e92..6b4a22a 100644 --- a/src/pages.ts +++ b/src/pages.ts @@ -13,7 +13,7 @@ import { blygItemUrl } from "./importer/util.ts"; import { parseStoredCite, parseStoredFork, parseStoredStub } from "./stub.ts"; import { excerptFromHtml } from "./markdown.ts"; import { clampText, leadingHeading, stripTransclusionQuotes } from "./preview.ts"; -import { authoredKind, getMedia, itemShowsResponses, listMediaForItem, publishedVersion } from "./model.ts"; +import { authoredKind, getMedia, itemShowsResponses, listVersionMedia, publishedVersion } from "./model.ts"; import type { ItemRow, MediaRow, Settings, SubscriptionRow, Transclusion, VersionRow } from "./types.ts"; import { loadFeedData, sourceKey, type FeedCardData, type FeedProvenance, type FeedItem } from "./public-feed.ts"; import { WEBMENTION_PATH } from "./types.ts"; @@ -1156,7 +1156,7 @@ ${permalinkLink(item.id, false, mount)} */ async function fragmentBlock(db: D1Database, item: FeedItem, mount: string, tz: string, titleLink = false, loaded?: FeedCardData): Promise { const latest = loaded ? loaded.latest : await publishedVersion(db, item); - const media = loaded ? loaded.media : await listMediaForItem(db, item.id); + const media = loaded ? loaded.media : await listVersionMedia(db, item.id, item.version); const html = latest?.content_html ?? ""; return renderFragment( item, @@ -1307,7 +1307,7 @@ async function threadCard(db: D1Database, item: FeedItem, mount: string, tz: str const href = `${mount}/t/${item.id}/`; const transclusions = parseTransclusions(latest?.transclusions); const html = injectProvenance(await sanitizeHtml(latest?.content_html ?? ""), await transclusionProvenance(db, transclusions, mount, provenance)); - const media = loaded ? loaded.media : await listMediaForItem(db, item.id); + const media = loaded ? loaded.media : await listVersionMedia(db, item.id, item.version); // The kind line is apparatus, on its own line above the item, never inline // with the author's first sentence (session 30). The quote count says what // the card may be cutting off. @@ -1330,7 +1330,7 @@ async function threadBlock(db: D1Database, item: ItemRow, mount: string, tz: str const latest = await publishedVersion(db, item); const transclusions = parseTransclusions(latest?.transclusions); const html = injectProvenance(await sanitizeHtml(latest?.content_html ?? ""), await transclusionProvenance(db, transclusions, mount)); - const media = await listMediaForItem(db, item.id); + const media = await listVersionMedia(db, item.id, item.version); return `
${stubCitation(latest, tz)} ${forkLineage(item, tz)} @@ -1843,7 +1843,7 @@ export async function permalinkPage(db: D1Database, settings: Settings, item: It ); } const latest = await publishedVersion(db, item); - const media = await listMediaForItem(db, item.id); + const media = await listVersionMedia(db, item.id, item.version); const head = itemHead(latest?.content_html ?? "", settings); const body = `${await blygPage(db, settings, mount, `${await fragmentBlock(db, item, mount, settings.timezone)} ${await responsesSection(db, item, settings, mount)}`)} @@ -1878,7 +1878,7 @@ export async function threadPage(db: D1Database, settings: Settings, item: ItemR ); } const latest = await publishedVersion(db, item); - const media = await listMediaForItem(db, item.id); + const media = await listVersionMedia(db, item.id, item.version); const head = itemHead(latest?.content_html ?? "", settings, respondsToLabel(latest)); const body = `${await blygPage(db, settings, mount, `${await threadBlock(db, item, mount, settings.timezone)} ${await responsesSection(db, item, settings, mount)}`)} diff --git a/src/protocol.ts b/src/protocol.ts index adaff3a..e30885c 100644 --- a/src/protocol.ts +++ b/src/protocol.ts @@ -11,7 +11,7 @@ import { feedEvents, getMedia, lastUpdated, - listMediaForItem, + listVersionMedia, listPublic, listVersions, publishedVersion, @@ -59,7 +59,7 @@ export async function buildItemJson(db: D1Database, settings: Settings, item: It const latest = await publishedVersion(db, item); const contentMd = isWithdrawn ? "" : (latest?.content_md ?? ""); const contentHtml = isWithdrawn ? "" : (latest?.content_html ?? ""); - const media = isWithdrawn ? [] : await listMediaForItem(db, item.id); + const media = isWithdrawn ? [] : await listVersionMedia(db, item.id, item.version); const changelog = (await listVersions(db, item.id)).map((v) => ({ version: v.version, at: v.published_at, @@ -226,12 +226,14 @@ export async function buildFeedXml(db: D1Database, settings: Settings, origin: s // friends reported "invalid feed"). Same bytes out, fixed query count. const liveItems = events.filter(({ item }) => item.kind !== "withdrawn"); const livePairs = JSON.stringify([...new Map(liveItems.map(({ item }) => [item.id, { id: item.id, version: item.version }])).values()]); - const allIds = JSON.stringify([...new Set(events.map(({ item }) => item.id))]); const withdrawnIds = JSON.stringify([...new Map(events.filter(({ item }) => item.kind === "withdrawn").map(({ item }) => [item.id, { id: item.id, version: item.version }])).values()]); const [latestRes, mediaRes, prevRes] = await db.batch([ db.prepare(`SELECT v.item_id, v.content_md, v.content_html, v.transclusions, v.stub_of, v.stub_cite FROM json_each(?) s JOIN versions v ON v.item_id = json_extract(s.value, '$.id') AND v.version = json_extract(s.value, '$.version')`).bind(livePairs), - db.prepare(`SELECT m.* FROM json_each(?) s JOIN media m ON m.item_id = s.value ORDER BY m.created ASC`).bind(allIds), + // Each live item's latest version's own attachments (migration 0031). + db.prepare(`SELECT vm.item_id, m.r2_key, m.alt, m.inline FROM json_each(?) s JOIN version_media vm + ON vm.item_id = json_extract(s.value, '$.id') AND vm.version = json_extract(s.value, '$.version') + JOIN media m ON m.id = vm.media_id ORDER BY m.created ASC`).bind(livePairs), db.prepare(`SELECT v.item_id, v.transclusions FROM json_each(?) s JOIN versions v ON v.item_id = json_extract(s.value, '$.id') AND v.version = json_extract(s.value, '$.version') - 1`).bind(withdrawnIds), ]); @@ -240,7 +242,7 @@ export async function buildFeedXml(db: D1Database, settings: Settings, origin: s latestById.set(v.item_id, v); } const mediaById = new Map>(); - for (const m of mediaRes.results as unknown as Array<{ item_id: string; r2_key: string; alt: string | null }>) { + for (const m of mediaRes.results as unknown as Array<{ item_id: string; r2_key: string; alt: string | null; inline: number }>) { const list = mediaById.get(m.item_id) ?? []; list.push(m); mediaById.set(m.item_id, list); diff --git a/src/public-feed.ts b/src/public-feed.ts index 87ea44e..c2d9df8 100644 --- a/src/public-feed.ts +++ b/src/public-feed.ts @@ -52,20 +52,22 @@ export interface FeedCardData { */ export async function loadFeedData(db: D1Database, items: FeedItem[], avatarId: string) { const selected = JSON.stringify(items.map(({ id, version }) => ({ id, version }))); - const ids = JSON.stringify(items.filter(i => i.kind !== "withdrawn").map(i => i.id)); + const live = JSON.stringify(items.filter(i => i.kind !== "withdrawn").map(({ id, version }) => ({ id, version }))); const [versions, pins, media, avatar] = await db.batch([ db.prepare(`SELECT v.item_id, v.content_html, v.note, v.transclusions, v.stub_of, v.stub_cite, v.generated_json FROM json_each(?) s JOIN versions v ON v.item_id = json_extract(s.value, '$.id') AND v.version = json_extract(s.value, '$.version')`).bind(selected), db.prepare(`SELECT v.item_id, v.version FROM json_each(?) s JOIN versions v ON v.item_id = json_extract(s.value, '$.id') WHERE v.pinned = 1 ORDER BY v.version`).bind(selected), - db.prepare(`SELECT m.* FROM json_each(?) s JOIN media m ON m.item_id = s.value - ORDER BY m.created ASC`).bind(ids), + db.prepare(`SELECT vm.item_id AS version_item, m.* FROM json_each(?) s JOIN version_media vm + ON vm.item_id = json_extract(s.value, '$.id') AND vm.version = json_extract(s.value, '$.version') + JOIN media m ON m.id = vm.media_id ORDER BY m.created ASC`).bind(live), db.prepare("SELECT * FROM media WHERE id = ?").bind(avatarId), ]); const cards = new Map(items.map(i => [i.id, { latest: null, pins: [], media: [] }])); for (const v of versions.results as unknown as FeedVersion[]) cards.get(v.item_id)!.latest = v; for (const pin of pins.results as unknown as { item_id: string; version: number }[]) cards.get(pin.item_id)!.pins.push(pin.version); - for (const m of media.results as unknown as MediaRow[]) cards.get(m.item_id!)!.media.push(m); + // Keyed by the version's item: a row detached since still belongs to the version that showed it. + for (const { version_item, ...m } of media.results as unknown as (MediaRow & { version_item: string })[]) cards.get(version_item)!.media.push(m); const provenance = await loadProvenance( db, items.filter(i => i.kind === "thread").map(i => JSON.parse(cards.get(i.id)!.latest?.transclusions || "[]") as Transclusion[]), diff --git a/src/read-api.ts b/src/read-api.ts index bc02bad..fdabd49 100644 --- a/src/read-api.ts +++ b/src/read-api.ts @@ -56,7 +56,7 @@ async function presentVersion(row: Parameters[0]) { } readApi.openapi(routes.getItem, async (c) => { const detail = await itemDetail(c.env.DB, c.req.param("id")); - return detail ? c.json({ ...itemResource(detail.item), authored_kind: detail.kind, media: detail.media.map(mediaResource), versions: await Promise.all(detail.versions.map(presentVersion)), published: detail.published ? await presentVersion(detail.published) : null }) : c.json({ error: "not found" }, 404); + return detail ? c.json({ ...itemResource(detail.item), authored_kind: detail.kind, media: detail.media.map(mediaResource), published_media: detail.publishedMedia.map(mediaResource), versions: await Promise.all(detail.versions.map(presentVersion)), published: detail.published ? await presentVersion(detail.published) : null }) : c.json({ error: "not found" }, 404); }); readApi.openapi(routes.getSettings, async (c) => c.json(settingsResource(await getSettings(c.env.DB)))); readApi.openapi(routes.listSubscriptions, async (c) => { diff --git a/src/ui/authoring.tsx b/src/ui/authoring.tsx index da07cdf..f67b97a 100644 --- a/src/ui/authoring.tsx +++ b/src/ui/authoring.tsx @@ -1577,6 +1577,8 @@ function Editor({ item }: { item: Detail }) { }); const isThread = item.authored_kind === 'thread'; const isPublic = item.status === 'public'; + // Attachments the live version lists that the working copy no longer has. + const dropped = item.published_media.filter((m) => !item.media.some((w) => w.id === m.id)); // Own items can go in hoppers beside what you read (session 47); "~own" is // the store's marker for them (src/importer/store.ts OWN_SUBSCRIPTION). const addToHopper = async () => { @@ -2012,6 +2014,9 @@ function Editor({ item }: { item: Detail }) {
{item.media.map((media) => { const inText = text.includes(media.url); + // The live version keeps what it lists (studio#7): removing it + // here takes it off the next version only. + const live = item.published_media.some((m) => m.id === media.id); return (

- remove + {live ? 'drop from the next version' : 'remove'} ) : null} @@ -2056,9 +2061,21 @@ function Editor({ item }: { item: Detail }) { ); })}

- ) : !upload.attached ? ( + ) : !upload.attached && !dropped.length ? (

None.

) : null} + {dropped.length ? ( +
+ {dropped.map((media) => ( +

+ + {media.alt || media.url} + {' '} + · in the live version, leaves with the next one +

+ ))} +
+ ) : null} {isPublic && isThread ? ( diff --git a/src/ui/data-schemas.ts b/src/ui/data-schemas.ts index ad19269..1ab3ae4 100644 --- a/src/ui/data-schemas.ts +++ b/src/ui/data-schemas.ts @@ -25,6 +25,7 @@ export const sourceSchemas = { id: true, authored_kind: true, media: true, + published_media: true, versions: true, published: true, }), diff --git a/src/ui/data.ts b/src/ui/data.ts index 69f66f5..cd12ccb 100644 --- a/src/ui/data.ts +++ b/src/ui/data.ts @@ -366,9 +366,9 @@ export const itemHistory = createCollection( const ids = subsetIds(context); if (!ids) throw new Error('Item history requires an id predicate'); return existing(ids, async (id) => { - const { authored_kind, media, versions, published } = + const { authored_kind, media, published_media, versions, published } = await readItem(id, generation); - return { id, authored_kind, media, versions, published }; + return { id, authored_kind, media, published_media, versions, published }; }); }), select: (response) => response.data, diff --git a/test-ui/source-collections.test.ts b/test-ui/source-collections.test.ts index 257f48a..4e6222a 100644 --- a/test-ui/source-collections.test.ts +++ b/test-ui/source-collections.test.ts @@ -20,7 +20,7 @@ let writeGate: Promise | undefined; const requests: { path: string; params: URLSearchParams }[] = []; const views: { cleanup(): Promise | void }[] = []; const item = () => ({ id: 'a', content_md: text, kind: 'fragment' as const, status: 'draft' as const, created: '2026-10-06', updated: '2026-10-06', version: 0, dirty: false, responses: 'default' as const, highlight: 'default' as const, provenance: [], stub_of: null, forked_from: null, fork_cite: null, pins: [] }); -const detail = (): Detail => ({ ...item(), authored_kind: 'fragment', media: [], versions: [], published: null }); +const detail = (): Detail => ({ ...item(), authored_kind: 'fragment', media: [], published_media: [], versions: [], published: null }); const hopper = { id: 'h', name: 'Hopper', public: false, slug: null, created: '2026-10-06', slug_frozen: false, description: null }; const contents = () => ({ hopper, items: Array.from({ length: 4 }, (_, i) => ({ subscription_id: 's', remote_id: String(i), content_html: `${text} ${i}`, content_md: text, kind: 'fragment', state: 'current', version: 1, created: null, updated: null, observed_at: '2026-10-06', content_hash: null, author_json: null, media_json: null, transclusions_json: null, l0: false, pinned_version_retained: null, page: null, stub_of_json: null, forked_from_json: null, generated_json: null })), memberships: Array.from({ length: 4 }, (_, i) => ({ hopper_id: 'h', subscription_id: 's', remote_id: String(i), added_at: '2026-10-06' })), total: 4, source_count: 1 }); const entry = (key: string, kind = 'fragment') => ({ key, source: 'own', kind, withdrawn: false, l0: false, contentHtml: text, displayAt: '2026-10-06' }); diff --git a/test/media.test.ts b/test/media.test.ts index e9f5b0e..e7fe183 100644 --- a/test/media.test.ts +++ b/test/media.test.ts @@ -46,6 +46,8 @@ describe("media (§3.3)", () => { item_id: id, alt: "the alt text", }); + // An attachment joins the item with its next version (migration 0031). + expect((await apiJson(cookie, "POST", `/api/items/${id}/publish`, {})).status).toBe(200); const item = await (await getPublic(`/blyg/items/${id}.json`)).json(); expect(item.media).toEqual([{ url: json.url, mime: "image/png", alt: "the alt text" }]); @@ -96,6 +98,7 @@ describe("duplicate uploads (studio#7, migration 0028)", () => { const again = await upload(cookie, png(1, 2), { item_id: id, alt: "second" }); expect(again.status).toBe(200); expect(again.json).toEqual({ id: first.json.id, url: first.json.url, mime: "image/png", duplicate: true }); + expect((await apiJson(cookie, "POST", `/api/items/${id}/publish`, {})).status).toBe(200); const item = await (await getPublic(`/blyg/items/${id}.json`)).json(); expect(item.media).toEqual([{ url: first.json.url, mime: "image/png", alt: "first" }]); diff --git a/test/pages.test.ts b/test/pages.test.ts index b3d1974..359ae5b 100644 --- a/test/pages.test.ts +++ b/test/pages.test.ts @@ -311,6 +311,8 @@ describe("social / meta tags (session 19)", () => { const up = (await ( await SELF.fetch(`${BASE}/api/media`, { method: "POST", headers: { cookie }, body: form }) ).json()) as any; + // An attachment joins the item with its next version (migration 0031). + expect((await apiJson(cookie, "POST", `/api/items/${id}/publish`, {})).status).toBe(200); const withImage = await (await getPublic(`/blyg/f/${id}/`)).text(); expect(withImage).toContain(``); expect(withImage).toContain(''); diff --git a/test/public-page-performance.test.ts b/test/public-page-performance.test.ts index 3385978..acf6b62 100644 --- a/test/public-page-performance.test.ts +++ b/test/public-page-performance.test.ts @@ -35,11 +35,12 @@ async function fixture(count: number) { } describe("public homepage read budget", () => { - beforeEach(async () => { await env.DB.batch(["versions", "media", "items"].map(table => env.DB.prepare(`DELETE FROM ${table}`))); }); + beforeEach(async () => { await env.DB.batch(["versions", "media", "version_media", "items"].map(table => env.DB.prepare(`DELETE FROM ${table}`))); }); it("preserves mixed-card order, published text, pins, images, metadata and the older link within five render queries", async () => { await fixture(100); await env.DB.prepare("INSERT INTO versions (item_id,version,content_md,content_html,content_hash,published_at,pinned) VALUES ('card0',1,'old','

old body

','h','2026-01-01',1)").run(); await env.DB.prepare("INSERT INTO media (id,item_id,r2_key,mime,alt,created) VALUES ('img','card0','media/img.png','image/png','my image','2026-01-01')").run(); + await env.DB.prepare("INSERT INTO version_media (item_id,version,media_id) VALUES ('card0',2,'img')").run(); const settings = await getSettings(env.DB), items = await listFeedItems(env.DB, 100), meter = measured(env.DB); const html = await feedPage(meter.db, settings, items, true, "/blyg", "https://example.com/blyg/"); expect(html).toContain('href="/blyg/f/card0/v1/"'); @@ -86,6 +87,7 @@ describe("public homepage read budget", () => { await env.DB.prepare("INSERT INTO subscriptions (id,kind,origin,feed_url,title,created,in_blogroll) VALUES ('sub','blyg','https://remote.example/','https://remote.example/feed.xml','Live source','2026-01-01',1)").run(); await env.DB.prepare("INSERT INTO imported_items (subscription_id,remote_id,kind,state,version,observed_at,page) VALUES ('sub','remote','thread','current',1,'2026-01-01','/custom')").run(); await env.DB.prepare("INSERT INTO media (id,item_id,r2_key,mime,alt,created) VALUES ('threadimg','card3','media/thread.png','image/png','thread image','2026-01-01')").run(); + await env.DB.prepare("INSERT INTO version_media (item_id,version,media_id) VALUES ('card3',2,'threadimg')").run(); await env.DB.prepare("INSERT INTO media (id,r2_key,mime,created) VALUES ('avatar','media/avatar.png','image/png','2026-01-01')").run(); const settings = { ...await getSettings(env.DB), avatar_media_id: 'avatar' }, meter = measured(env.DB); const html = await feedPage(meter.db, settings, await listFeedItems(env.DB, 101), false, '/mounted', 'https://example.com/mounted/'); diff --git a/test/version-media.test.ts b/test/version-media.test.ts new file mode 100644 index 0000000..e28a224 --- /dev/null +++ b/test/version-media.test.ts @@ -0,0 +1,107 @@ +// Attachments per version (blygger-studio#7, migration 0031). §5.4 makes media +// part of a version: each version's document lists its own `media`, a later +// version may leave one out, and the earlier versions keep serving its bytes. +import { env, SELF } from "cloudflare:test"; +import { describe, expect, it } from "vitest"; +import migration0031 from "../migrations/0031_version_media.sql?raw"; +import { apiJson, BASE, createAndPublish, getPublic, login } from "./helpers.ts"; + +async function upload(cookie: string, item: string, bytes: number[], extra: Record = {}) { + const form = new FormData(); + form.set("file", new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, ...bytes])], "p.png", { type: "image/png" })); + form.set("item_id", item); + for (const [k, v] of Object.entries(extra)) form.set(k, v); + const res = await SELF.fetch(`${BASE}/api/media`, { method: "POST", headers: { cookie }, body: form }); + expect(res.status).toBe(201); + return (await res.json()) as { id: string; url: string }; +} +const publish = async (cookie: string, id: string) => expect((await apiJson(cookie, "POST", `/api/items/${id}/publish`, {})).status).toBe(200); +const remove = async (cookie: string, media: string) => (await apiJson(cookie, "DELETE", `/api/media/${media}`)).json.outcome; +const document = async (id: string) => (await (await getPublic(`/blyg/items/${id}.json`)).json()) as { version: number; media: { url: string }[] }; +const urls = async (id: string) => (await document(id)).media.map((m) => m.url); +const dirty = async (id: string) => (await env.DB.prepare("SELECT dirty FROM items WHERE id = ?").bind(id).first<{ dirty: number }>())!.dirty; + +describe("attachments per version (studio#7)", () => { + it("drops an attachment from the next version only, and keeps its bytes", async () => { + const cookie = await login(); + const id = await createAndPublish(cookie, "two pictures below"); + const a = await upload(cookie, id, [1], { alt: "a" }); + const b = await upload(cookie, id, [2], { alt: "b" }); + await publish(cookie, id); + expect(await urls(id)).toEqual([a.url, b.url]); + + // An appended attachment is in no version's text, only in its list, and + // removing it used to delete the bytes the live version still showed. + expect(await remove(cookie, a.id)).toBe("detached"); + expect(await urls(id), "the live version keeps what it published").toEqual([a.url, b.url]); + expect(await dirty(id), "the working copy now differs").toBe(1); + expect((await getPublic(`/blyg/${a.url}`)).status).toBe(200); + + await publish(cookie, id); + const next = await document(id); + expect(next.media.map((m) => m.url)).toEqual([b.url]); + expect((await getPublic(`/blyg/${a.url}`)).status, "an earlier version still lists it (§5.4)").toBe(200); + const page = await (await getPublic(`/blyg/f/${id}/`)).text(); + expect(page).toContain(b.url); + expect(page).not.toContain(a.url); + const feed = await (await getPublic("/blyg/feed.xml")).text(); + const entry = feed.slice(feed.indexOf(``), feed.indexOf("")); + expect(feed).toContain(b.url); + expect(entry).not.toContain(a.url); + }); + + it("lists an attachment added to a published item from its next version on", async () => { + const cookie = await login(); + const id = await createAndPublish(cookie, "a picture is coming"); + const a = await upload(cookie, id, [3]); + expect(await urls(id)).toEqual([]); + expect(await dirty(id)).toBe(1); + await publish(cookie, id); + expect(await urls(id)).toEqual([a.url]); + }); + + it("deletes outright what no version listed", async () => { + const cookie = await login(); + const id = await createAndPublish(cookie, "nothing attached yet"); + const a = await upload(cookie, id, [4]); + expect(await remove(cookie, a.id)).toBe("deleted"); + expect(await env.MEDIA.get(a.url)).toBeNull(); + }); + + it("an inline attachment follows its version's text", async () => { + const cookie = await login(); + const created = await apiJson(cookie, "POST", "/api/items", { content_md: "draft" }); + const id = created.json.id as string; + const a = await upload(cookie, id, [5], { inline: "true" }); + expect((await apiJson(cookie, "PATCH", `/api/items/${id}`, { content_md: `![a](/blyg/${a.url})` })).status).toBe(200); + await publish(cookie, id); + expect(await urls(id)).toEqual([a.url]); + expect((await apiJson(cookie, "PATCH", `/api/items/${id}`, { content_md: "no picture now" })).status).toBe(200); + await publish(cookie, id); + expect(await urls(id)).toEqual([]); + }); +}); + +describe("migration 0031 backfill", () => { + it("keeps every live document as it was, and gives an earlier version only what existed then", async () => { + const cookie = await login(); + const id = await createAndPublish(cookie, "backfilled"); + const early = await upload(cookie, id, [6]); + await publish(cookie, id); // v2, shows early + const late = await upload(cookie, id, [7]); // attached after v2, never published + const v = await env.DB.prepare("SELECT version, published_at FROM versions WHERE item_id = ? ORDER BY version").bind(id).all<{ version: number; published_at: string }>(); + const [v1, v2] = v.results; + // Deterministic order: early after v1, late after v2. + await env.DB.prepare("UPDATE media SET created = ? WHERE id = ?").bind(new Date(Date.parse(v1.published_at) + 1000).toISOString().replace(/\.\d{3}Z$/, "Z"), early.id).run(); + await env.DB.prepare("UPDATE media SET created = ? WHERE id = ?").bind(new Date(Date.parse(v2.published_at) + 1000).toISOString().replace(/\.\d{3}Z$/, "Z"), late.id).run(); + + await env.DB.prepare("DELETE FROM version_media WHERE item_id = ?").bind(id).run(); + const backfill = migration0031.slice(migration0031.indexOf("INSERT OR IGNORE INTO version_media")); + const statement = backfill.split("\n").filter((line) => !line.trim().startsWith("--")).join("\n").replace(/;\s*$/, ""); + await env.DB.prepare(statement).run(); + + const rows = await env.DB.prepare("SELECT version, media_id FROM version_media WHERE item_id = ? ORDER BY version, media_id").bind(id).all(); + // v1 predates both; v2 is live, so it keeps everything attached today, as before. + expect(rows.results).toEqual([{ version: v2.version, media_id: early.id }, { version: v2.version, media_id: late.id }].sort((x, y) => x.media_id.localeCompare(y.media_id))); + }); +});