From 2fa74137f93cb2465e8e42842423f8ef35574588 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 02:29:13 +0100 Subject: [PATCH] fix(arr): delete subtitle sidecars with their file remove_library_files only resolved video paths from media_files, so a season/episode-scoped delete dropped subtitle_files rows via cascade but left the .srt sidecars on disk (#218). --- ...befa20fba22fc2d281295aed81a851a35ea07.json | 26 +++ ...deb24236c2c8afa25ffec1c7bbe519aac8803.json | 26 +++ ...8c825d3ca31eb631744a57594aa2fc08fb945.json | 26 +++ crates/arr-api/src/series.rs | 204 +++++++++++++++++- 4 files changed, 279 insertions(+), 3 deletions(-) create mode 100644 .sqlx/query-5f3a663eef0318b38cd864a2e06befa20fba22fc2d281295aed81a851a35ea07.json create mode 100644 .sqlx/query-b2d937e5b8381de44fc6c1915e3deb24236c2c8afa25ffec1c7bbe519aac8803.json create mode 100644 .sqlx/query-fa35de1c7e62c68dd1049d7e7e08c825d3ca31eb631744a57594aa2fc08fb945.json diff --git a/.sqlx/query-5f3a663eef0318b38cd864a2e06befa20fba22fc2d281295aed81a851a35ea07.json b/.sqlx/query-5f3a663eef0318b38cd864a2e06befa20fba22fc2d281295aed81a851a35ea07.json new file mode 100644 index 0000000..e7c8adc --- /dev/null +++ b/.sqlx/query-5f3a663eef0318b38cd864a2e06befa20fba22fc2d281295aed81a851a35ea07.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT sf.path AS \"path!: String\"\n FROM subtitle_files sf\n JOIN media_files mf ON mf.id = sf.media_file_id\n JOIN episodes e ON mf.owner_kind = 'episode' AND e.id = mf.owner_id\n JOIN seasons se ON se.id = e.season_id\n WHERE se.series_id = ? AND sf.path IS NOT NULL", + "describe": { + "columns": [ + { + "name": "path!: String", + "ordinal": 0, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_files", + "name": "path" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + true + ] + }, + "hash": "5f3a663eef0318b38cd864a2e06befa20fba22fc2d281295aed81a851a35ea07" +} diff --git a/.sqlx/query-b2d937e5b8381de44fc6c1915e3deb24236c2c8afa25ffec1c7bbe519aac8803.json b/.sqlx/query-b2d937e5b8381de44fc6c1915e3deb24236c2c8afa25ffec1c7bbe519aac8803.json new file mode 100644 index 0000000..3e27f73 --- /dev/null +++ b/.sqlx/query-b2d937e5b8381de44fc6c1915e3deb24236c2c8afa25ffec1c7bbe519aac8803.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT sf.path AS \"path!: String\"\n FROM subtitle_files sf\n JOIN media_files mf ON mf.id = sf.media_file_id\n JOIN episodes e ON mf.owner_kind = 'episode' AND e.id = mf.owner_id\n WHERE e.season_id = ? AND sf.path IS NOT NULL", + "describe": { + "columns": [ + { + "name": "path!: String", + "ordinal": 0, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_files", + "name": "path" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + true + ] + }, + "hash": "b2d937e5b8381de44fc6c1915e3deb24236c2c8afa25ffec1c7bbe519aac8803" +} diff --git a/.sqlx/query-fa35de1c7e62c68dd1049d7e7e08c825d3ca31eb631744a57594aa2fc08fb945.json b/.sqlx/query-fa35de1c7e62c68dd1049d7e7e08c825d3ca31eb631744a57594aa2fc08fb945.json new file mode 100644 index 0000000..05ce4ec --- /dev/null +++ b/.sqlx/query-fa35de1c7e62c68dd1049d7e7e08c825d3ca31eb631744a57594aa2fc08fb945.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT sf.path AS \"path!: String\"\n FROM subtitle_files sf\n JOIN media_files mf ON mf.id = sf.media_file_id\n WHERE mf.owner_kind = 'episode' AND mf.owner_id = ? AND sf.path IS NOT NULL", + "describe": { + "columns": [ + { + "name": "path!: String", + "ordinal": 0, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_files", + "name": "path" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + true + ] + }, + "hash": "fa35de1c7e62c68dd1049d7e7e08c825d3ca31eb631744a57594aa2fc08fb945" +} diff --git a/crates/arr-api/src/series.rs b/crates/arr-api/src/series.rs index cf78ba1..607feb8 100644 --- a/crates/arr-api/src/series.rs +++ b/crates/arr-api/src/series.rs @@ -647,8 +647,9 @@ enum FileScope { /// subfolders, sidecar subtitles and artwork go with it. A season or a single /// episode resolves to the recorded file and nothing else — the title folder /// holds the siblings this call must not touch, and a season subfolder would -/// have to be re-derived to be named, which §2 forbids. Sidecars beside a -/// removed episode therefore stay; they are not rows this service wrote. +/// have to be re-derived to be named, which §2 forbids. Subtitle sidecars are +/// rows this service wrote too (#186), so they are resolved and unlinked the +/// same way as the video they sit beside (#218). /// /// The torrent is untouched (§7.3). It keeps seeding under its own rule and /// the reaper deletes it; a hardlinked file loses only its library name. @@ -657,7 +658,8 @@ enum FileScope { /// recorded and can retry rather than losing the record of what is on disk. async fn remove_library_files(state: &AppState, scope: FileScope) -> Result<(), ApiError> { let root = scope_root(state, scope).await?; - let paths = scope_paths(state, scope).await?; + let mut paths = scope_paths(state, scope).await?; + paths.extend(subtitle_paths(state, scope).await?); let mut targets: Vec = Vec::new(); for path in &paths { @@ -777,6 +779,50 @@ async fn scope_paths(state: &AppState, scope: FileScope) -> Result, }) } +/// Every subtitle sidecar (#186) this service wrote for the scope. Mirrors +/// [`scope_paths`]: same owner filter, same guard against widening. An +/// embedded track has no `path` — it never touched disk — and is excluded. +async fn subtitle_paths(state: &AppState, scope: FileScope) -> Result, ApiError> { + Ok(match scope { + FileScope::Series(id) => { + sqlx::query_scalar!( + r#"SELECT sf.path AS "path!: String" + FROM subtitle_files sf + JOIN media_files mf ON mf.id = sf.media_file_id + JOIN episodes e ON mf.owner_kind = 'episode' AND e.id = mf.owner_id + JOIN seasons se ON se.id = e.season_id + WHERE se.series_id = ? AND sf.path IS NOT NULL"#, + id + ) + .fetch_all(pool(state)?) + .await? + } + FileScope::Season(id) => { + sqlx::query_scalar!( + r#"SELECT sf.path AS "path!: String" + FROM subtitle_files sf + JOIN media_files mf ON mf.id = sf.media_file_id + JOIN episodes e ON mf.owner_kind = 'episode' AND e.id = mf.owner_id + WHERE e.season_id = ? AND sf.path IS NOT NULL"#, + id + ) + .fetch_all(pool(state)?) + .await? + } + FileScope::Episode(id) => { + sqlx::query_scalar!( + r#"SELECT sf.path AS "path!: String" + FROM subtitle_files sf + JOIN media_files mf ON mf.id = sf.media_file_id + WHERE mf.owner_kind = 'episode' AND mf.owner_id = ? AND sf.path IS NOT NULL"#, + id + ) + .fetch_all(pool(state)?) + .await? + } + }) +} + /// The one recorded file, when it really sits inside the root. `None` when it /// does not, which is the guard that keeps a sub-series delete inside the /// library it belongs to. @@ -3163,6 +3209,42 @@ mod tests { file } + /// Same as [`episode_file_on_disk`], plus a §15 sidecar next to it with a + /// `subtitle_files` row pointing at the video's `media_files` row (#218). + async fn episode_file_with_sidecar_on_disk( + state: &AppState, + episode_id: i64, + root: &std::path::Path, + season: i64, + name: &str, + ) -> (std::path::PathBuf, std::path::PathBuf) { + let file = episode_file_on_disk(state, episode_id, root, season, name).await; + let pool = state.database().expect("database").pool(); + let media_file_id: i64 = sqlx::query_scalar( + "SELECT id FROM media_files WHERE owner_kind = 'episode' AND owner_id = ? AND path = ?", + ) + .bind(episode_id) + .bind(file.to_str().expect("utf-8 path")) + .fetch_one(pool) + .await + .expect("media file id"); + + let sidecar = file.with_extension("pt-PT.srt"); + tokio::fs::write(&sidecar, b"subs") + .await + .expect("write sidecar"); + sqlx::query( + "INSERT INTO subtitle_files (media_file_id, language, origin, provider, path) + VALUES (?, 'pt-PT', 'provider', 'opensubtitles', ?)", + ) + .bind(media_file_id) + .bind(sidecar.to_str().expect("utf-8 path")) + .execute(pool) + .await + .expect("subtitle file"); + (file, sidecar) + } + async fn point_root_at(state: &AppState, root_id: i64, path: &std::path::Path) { sqlx::query("UPDATE roots SET path = ? WHERE id = ?") .bind(path.to_str().expect("utf-8 root")) @@ -3365,6 +3447,122 @@ mod tests { assert_eq!(remaining, vec![second], "only the episode's row goes"); } + /// #218: a season-scoped delete unlinks its subtitle sidecars too — the + /// `subtitle_files` rows go with the video's `media_files` row, but the + /// files on disk do not follow without this. + #[tokio::test] + async fn removing_a_season_takes_its_subtitle_sidecars() { + let (_dir, state, base) = application().await; + let root_id = tv_root(&state, "main").await; + let series = add_series(&base, root_id, true).await; + let series_id = series["id"].as_i64().expect("id"); + let root = tempfile::tempdir().expect("root"); + point_root_at(&state, root_id, root.path()).await; + + let first = add_season( + &base, + series_id, + 1, + serde_json::json!([{"number": 1, "title": "The Magic Xylophone"}]), + ) + .await; + let second = add_season( + &base, + series_id, + 2, + serde_json::json!([{"number": 1, "title": "Dance Mode"}]), + ) + .await; + let s01e01 = first["episodes"][0]["id"].as_i64().expect("id"); + let s02e01 = second["episodes"][0]["id"].as_i64().expect("id"); + + let (_, in_scope_sidecar) = + episode_file_with_sidecar_on_disk(&state, s01e01, root.path(), 1, "Bluey - S01E01.mkv") + .await; + let (_, other_sidecar) = + episode_file_with_sidecar_on_disk(&state, s02e01, root.path(), 2, "Bluey - S02E01.mkv") + .await; + + let response = reqwest::Client::new() + .delete(format!("{base}/api/series/{series_id}/seasons/1/files")) + .send() + .await + .expect("delete season files"); + assert_eq!(response.status(), StatusCode::NO_CONTENT); + + assert!(!in_scope_sidecar.exists(), "its sidecar is gone with it"); + assert!( + other_sidecar.exists(), + "another season's sidecar is not in scope" + ); + + let pool = state.database().expect("database").pool(); + let orphans: i64 = sqlx::query_scalar("SELECT count(*) FROM subtitle_files") + .fetch_one(pool) + .await + .expect("count subtitle rows"); + assert_eq!( + orphans, 1, + "only the deleted season's subtitle row cascades" + ); + } + + /// #218: the same fix at the narrower episode scope, leaving a sibling + /// episode's sidecar untouched. + #[tokio::test] + async fn removing_one_episode_takes_its_subtitle_sidecar() { + let (_dir, state, base) = application().await; + let root_id = tv_root(&state, "main").await; + let series = add_series(&base, root_id, true).await; + let series_id = series["id"].as_i64().expect("id"); + let root = tempfile::tempdir().expect("root"); + point_root_at(&state, root_id, root.path()).await; + + let season = add_season( + &base, + series_id, + 1, + serde_json::json!([ + {"number": 1, "title": "The Magic Xylophone"}, + {"number": 2, "title": "Hospital"} + ]), + ) + .await; + let episodes = season["episodes"].as_array().expect("episodes"); + let first = episodes + .iter() + .find(|episode| episode["number"] == 1) + .expect("s01e01")["id"] + .as_i64() + .expect("id"); + let second = episodes + .iter() + .find(|episode| episode["number"] == 2) + .expect("s01e02")["id"] + .as_i64() + .expect("id"); + + let (_, first_sidecar) = + episode_file_with_sidecar_on_disk(&state, first, root.path(), 1, "Bluey - S01E01.mkv") + .await; + let (_, second_sidecar) = + episode_file_with_sidecar_on_disk(&state, second, root.path(), 1, "Bluey - S01E02.mkv") + .await; + + let response = reqwest::Client::new() + .delete(format!("{base}/api/episodes/{first}/files")) + .send() + .await + .expect("delete episode files"); + assert_eq!(response.status(), StatusCode::NO_CONTENT); + + assert!(!first_sidecar.exists(), "the episode's sidecar is gone"); + assert!( + second_sidecar.exists(), + "its sibling's sidecar is not in scope" + ); + } + /// #174: removal is not conditional on there being anything to remove. /// A scope with no files still clears intent, and still answers 204. #[tokio::test]