From 1e0f2f879208ba67a43f9f3b7b7156cb744b95f7 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sat, 5 Sep 2026 09:06:22 +0200 Subject: [PATCH] fix: let a media file belong to more than one entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 18 ++++++++ src/routes/entries.rs | 12 ++---- src/routes/media.rs | 73 ++++++++++++++++++++++++++------- src/routes/share.rs | 6 +-- web/src/lib/MediaBrowser.svelte | 13 ++++-- web/src/lib/api.ts | 3 +- 6 files changed, 94 insertions(+), 31 deletions(-) create mode 100644 migrations/0002_entry_media.sql diff --git a/migrations/0002_entry_media.sql b/migrations/0002_entry_media.sql new file mode 100644 index 0000000..7fd21c2 --- /dev/null +++ b/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; diff --git a/src/routes/entries.rs b/src/routes/entries.rs index 93dcb3e..7aff5e4 100644 --- a/src/routes/entries.rs +++ b/src/routes/entries.rs @@ -212,13 +212,9 @@ pub async fn remove( State(state): State, Path(id): Path, ) -> AppResult> { - let media_ids: Vec = 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?; } diff --git a/src/routes/media.rs b/src/routes/media.rs index a4787c5..4aeecaa 100644 --- a/src/routes/media.rs +++ b/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, + /// Every entry that embeds this file, which may be none or several. + pub entry_ids: Vec, } 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::, _>("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) -> AppResult>> { 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 = MEDIA_REF + let referenced: BTreeSet = 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") - .bind(entry_id) - .bind(id) - .execute(db) - .await?; + 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(&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> { + 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() diff --git a/src/routes/share.rs b/src/routes/share.rs index c7be043..8ffcaff 100644 --- a/src/routes/share.rs +++ b/src/routes/share.rs @@ -104,9 +104,9 @@ pub async fn serve_media( ) -> AppResult { 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) diff --git a/web/src/lib/MediaBrowser.svelte b/web/src/lib/MediaBrowser.svelte index 6831336..7f0a94a 100644 --- a/web/src/lib/MediaBrowser.svelte +++ b/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 @@ {file.filename} {formatBytes(file.size)} · {formatStamp(file.created_at)} · - {file.entry_id ? `entry:${file.entry_id}` : 'unattached'} + {usage(file.entry_ids)} @@ -50,7 +55,7 @@ > {/if} - + {:else} diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 3ad20b0..a3eca16 100644 --- a/web/src/lib/api.ts +++ b/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 {