Attachments per version: a removal drops from the next version only (#7)

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: `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` |

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));

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;

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);

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"
                   ]
@@ -5417,6 +5426,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": {

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<Media>;
+        /**
+         * What the live version lists, including attachments removed from the working copy since (studio#7).
+         */
+        published_media: Array<Media>;
         versions: Array<Version>;
         published: Version | null;
     };
@@ -1800,6 +1807,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
      */

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()
 });
@@ -858,14 +859,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)

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<string, string> = {
   "image/png": "png",
@@ -408,44 +409,60 @@ 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,
     item_id: typeof itemId === "string" && itemId ? itemId : null,
     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"');
     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);
 });
 
 /**
  * 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) => {
   const db = c.env.DB;
   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);

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),
@@ -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) };

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);