fix: let a media file belong to more than one entry

media.entry_id modelled ownership as one entry per file, but link_to_entry
reassigns it on every save. Embedding the same image in a second entry
therefore moved the claim, and deleting that second entry deleted the file
out from under the first — a broken image in the entry and a 404 on its
share link.

A join table replaces the column. Deleting an entry now removes only the
files no other entry embeds, and link_to_entry runs in a transaction so an
entry is never briefly attached to nothing while its share link is being
read.

migrations/0002_entry_media.sql

@@ -0,0 +1,18 @@
+-- `media.entry_id` claimed a file for exactly one entry. Embedding the same
+-- file in a second entry silently moved the claim, and deleting that second
+-- entry then deleted a file the first one still displayed. A join table lets a
+-- file belong to as many entries as embed it.
+
+CREATE TABLE entry_media (
+    entry_id INTEGER NOT NULL REFERENCES entries (id) ON DELETE CASCADE,
+    media_id TEXT    NOT NULL REFERENCES media (id)   ON DELETE CASCADE,
+    PRIMARY KEY (entry_id, media_id)
+) WITHOUT ROWID;
+
+CREATE INDEX idx_entry_media_media_id ON entry_media (media_id);
+
+INSERT INTO entry_media (entry_id, media_id)
+    SELECT entry_id, id FROM media WHERE entry_id IS NOT NULL;
+
+DROP INDEX idx_media_entry_id;
+ALTER TABLE media DROP COLUMN entry_id;

src/routes/entries.rs

@@ -212,13 +212,9 @@ pub async fn remove(
     State(state): State<AppState>,
     Path(id): Path<i64>,
 ) -> AppResult<Json<serde_json::Value>> {
-    let media_ids: Vec<String> = sqlx::query("SELECT id FROM media WHERE entry_id = ?1")
-        .bind(id)
-        .fetch_all(&state.db)
-        .await?
-        .into_iter()
-        .map(|row| row.get("id"))
-        .collect();
+    // Collected before the delete, which takes the entry_media rows with it.
+    // Files another entry also embeds are left alone.
+    let orphans = media::exclusive_media(&state.db, id).await?;
 
     let affected = sqlx::query("DELETE FROM entries WHERE id = ?1")
         .bind(id)
@@ -230,7 +226,7 @@ pub async fn remove(
         return Err(AppError::NotFound);
     }
 
-    for media_id in media_ids {
+    for media_id in orphans {
         media::delete_media(&state, &media_id).await?;
     }
 

src/routes/media.rs

@@ -1,4 +1,4 @@
-use std::sync::LazyLock;
+use std::{collections::BTreeSet, sync::LazyLock};
 
 use axum::{
     body::Body,
@@ -67,11 +67,21 @@ pub struct Media {
     pub size: i64,
     pub created_at: i64,
     pub url: String,
-    pub entry_id: Option<i64>,
+    /// Every entry that embeds this file, which may be none or several.
+    pub entry_ids: Vec<i64>,
 }
 
 fn row_to_media(row: &sqlx::sqlite::SqliteRow) -> Media {
     let id: String = row.get("id");
+    // `group_concat` gives "3,7" or NULL; queries that do not ask for it at all
+    // (a fresh upload) get an empty list.
+    let entry_ids = row
+        .try_get::<Option<String>, _>("entry_ids")
+        .ok()
+        .flatten()
+        .map(|joined| joined.split(',').filter_map(|n| n.parse().ok()).collect())
+        .unwrap_or_default();
+
     Media {
         url: format!("/api/media/{id}"),
         id,
@@ -79,7 +89,7 @@ fn row_to_media(row: &sqlx::sqlite::SqliteRow) -> Media {
         mime: row.get("mime"),
         size: row.get("size"),
         created_at: row.get("created_at"),
-        entry_id: row.get("entry_id"),
+        entry_ids,
     }
 }
 
@@ -131,7 +141,7 @@ pub async fn upload(
         let row = sqlx::query(
             "INSERT INTO media (id, filename, mime, size, created_at)
              VALUES (?1, ?2, ?3, ?4, ?5)
-             RETURNING id, entry_id, filename, mime, size, created_at",
+             RETURNING id, filename, mime, size, created_at",
         )
         .bind(&id)
         .bind(&filename)
@@ -152,8 +162,10 @@ pub async fn upload(
 
 pub async fn list(_: Session, State(state): State<AppState>) -> AppResult<Json<Vec<Media>>> {
     let rows = sqlx::query(
-        "SELECT id, entry_id, filename, mime, size, created_at
-         FROM media ORDER BY created_at DESC LIMIT 500",
+        "SELECT m.id, m.filename, m.mime, m.size, m.created_at,
+                (SELECT group_concat(em.entry_id)
+                 FROM entry_media em WHERE em.media_id = m.id) AS entry_ids
+         FROM media m ORDER BY m.created_at DESC LIMIT 500",
     )
     .fetch_all(&state.db)
     .await?;
@@ -237,27 +249,58 @@ pub async fn delete_media(state: &AppState, id: &str) -> AppResult<()> {
 
 /// Attach every media file the body embeds to this entry, and detach the ones
 /// it no longer mentions so they stop being reachable through its share link.
+/// Other entries keep whatever they embed; this only rewrites one entry's row.
 pub async fn link_to_entry(db: &SqlitePool, entry_id: i64, body: &str) -> AppResult<()> {
-    let referenced: Vec<String> = MEDIA_REF
+    let referenced: BTreeSet<String> = MEDIA_REF
         .captures_iter(body)
         .map(|c| c[1].to_lowercase())
         .collect();
 
-    sqlx::query("UPDATE media SET entry_id = NULL WHERE entry_id = ?1")
+    // One transaction, so an entry is never momentarily attached to nothing —
+    // which would blank its images for anyone reading its share link.
+    let mut tx = db.begin().await?;
+
+    sqlx::query("DELETE FROM entry_media WHERE entry_id = ?1")
         .bind(entry_id)
-        .execute(db)
+        .execute(&mut *tx)
         .await?;
 
-    for id in referenced {
-        sqlx::query("UPDATE media SET entry_id = ?1 WHERE id = ?2")
+    for id in &referenced {
+        // Selecting from `media` rather than binding the id directly means a
+        // body that still mentions a since-deleted file saves fine.
+        sqlx::query(
+            "INSERT OR IGNORE INTO entry_media (entry_id, media_id)
+             SELECT ?1, id FROM media WHERE id = ?2",
+        )
         .bind(entry_id)
         .bind(id)
-            .execute(db)
+        .execute(&mut *tx)
         .await?;
     }
+
+    tx.commit().await?;
     Ok(())
 }
 
+/// The files this entry embeds that no other entry does — the ones that become
+/// unreachable once it is gone.
+pub async fn exclusive_media(db: &SqlitePool, entry_id: i64) -> AppResult<Vec<String>> {
+    let rows = sqlx::query(
+        "SELECT em.media_id AS id
+         FROM entry_media em
+         WHERE em.entry_id = ?1
+           AND NOT EXISTS (
+               SELECT 1 FROM entry_media other
+               WHERE other.media_id = em.media_id AND other.entry_id <> ?1
+           )",
+    )
+    .bind(entry_id)
+    .fetch_all(db)
+    .await?;
+
+    Ok(rows.iter().map(|row| row.get("id")).collect())
+}
+
 fn urlencode(value: &str) -> String {
     value
         .bytes()

src/routes/share.rs

@@ -104,9 +104,9 @@ pub async fn serve_media(
 ) -> AppResult<Response> {
     let allowed = sqlx::query(
         "SELECT 1 AS ok
-         FROM media m
-         JOIN entries e ON e.id = m.entry_id
-         WHERE m.id = ?1 AND e.share_token = ?2",
+         FROM entry_media em
+         JOIN entries e ON e.id = em.entry_id
+         WHERE em.media_id = ?1 AND e.share_token = ?2",
     )
     .bind(&id)
     .bind(&token)

web/src/lib/MediaBrowser.svelte

@@ -5,8 +5,13 @@
   import { diary } from './store.svelte'
   import { formatBytes, formatStamp } from './util'
 
-  async function remove(id: string, filename: string) {
-    if (!confirm(`delete ${filename}? entries that embed it will break.`)) return
+  const usage = (entryIds: number[]) =>
+    entryIds.length === 0 ? 'unattached' : entryIds.map((id) => `entry:${id}`).join(' ')
+
+  async function remove(id: string, filename: string, entryIds: number[]) {
+    const used = entryIds.length === 1 ? '1 entry' : `${entryIds.length} entries`
+    const warning = entryIds.length > 0 ? ` it is embedded in ${used}, which will break.` : ''
+    if (!confirm(`delete ${filename}?${warning}`)) return
     await diary.guard(async () => {
       await api.deleteMedia(id)
       await diary.loadMedia()
@@ -36,7 +41,7 @@
           <span class="name">{file.filename}</span>
           <span class="faint">
             {formatBytes(file.size)} · {formatStamp(file.created_at)} ·
-            {file.entry_id ? `entry:${file.entry_id}` : 'unattached'}
+            {usage(file.entry_ids)}
           </span>
         </div>
 
@@ -50,7 +55,7 @@
             >
           {/if}
           <button onclick={() => diary.copy(location.origin + file.url)}>copy url</button>
-          <button onclick={() => remove(file.id, file.filename)}>rm</button>
+          <button onclick={() => remove(file.id, file.filename, file.entry_ids)}>rm</button>
         </div>
       </li>
     {:else}

web/src/lib/api.ts

@@ -33,7 +33,8 @@ export interface MediaFile {
   size: number
   created_at: number
   url: string
-  entry_id: number | null
+  /** Every entry that embeds this file — a file may be used by several. */
+  entry_ids: number[]
 }
 
 export class ApiError extends Error {