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).
This commit is contained in:
Miguel Palhas
2026-08-25 02:29:13 +01:00
parent 7b4cff1874
commit 2fa74137f9
4 changed files with 279 additions and 3 deletions
@@ -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"
}
@@ -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"
}
@@ -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"
}
+201 -3
View File
@@ -647,8 +647,9 @@ enum FileScope {
/// subfolders, sidecar subtitles and artwork go with it. A season or a single /// 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 /// 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 /// 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 /// have to be re-derived to be named, which §2 forbids. Subtitle sidecars are
/// removed episode therefore stay; they are not rows this service wrote. /// 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 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. /// 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. /// 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> { async fn remove_library_files(state: &AppState, scope: FileScope) -> Result<(), ApiError> {
let root = scope_root(state, scope).await?; 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<std::path::PathBuf> = Vec::new(); let mut targets: Vec<std::path::PathBuf> = Vec::new();
for path in &paths { for path in &paths {
@@ -777,6 +779,50 @@ async fn scope_paths(state: &AppState, scope: FileScope) -> Result<Vec<String>,
}) })
} }
/// 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<Vec<String>, 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 /// 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 /// does not, which is the guard that keeps a sub-series delete inside the
/// library it belongs to. /// library it belongs to.
@@ -3163,6 +3209,42 @@ mod tests {
file 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) { async fn point_root_at(state: &AppState, root_id: i64, path: &std::path::Path) {
sqlx::query("UPDATE roots SET path = ? WHERE id = ?") sqlx::query("UPDATE roots SET path = ? WHERE id = ?")
.bind(path.to_str().expect("utf-8 root")) .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"); 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. /// #174: removal is not conditional on there being anything to remove.
/// A scope with no files still clears intent, and still answers 204. /// A scope with no files still clears intent, and still answers 204.
#[tokio::test] #[tokio::test]