From c64c572781af0e60fe624fb4738c0793a60cc137 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 01:31:54 +0100 Subject: [PATCH 1/2] refactor(arr): give arr-api its own jellyfin client arr-daemon depends on arr-api, so a handler in arr-api can never reach the daemon's private JellyfinClient. Move it into arr-api and attach an instance to AppState, so a manual subtitle write can ask for the same refresh import already does (#195). --- Cargo.lock | 1 + crates/arr-api/Cargo.toml | 1 + .../{arr-daemon => arr-api}/src/jellyfin.rs | 16 +++++++++---- crates/arr-api/src/lib.rs | 1 + crates/arr-api/src/state.rs | 16 +++++++++++++ crates/arr-daemon/src/import.rs | 2 +- crates/arr-daemon/src/main.rs | 23 +++++++++++++------ 7 files changed, 48 insertions(+), 12 deletions(-) rename crates/{arr-daemon => arr-api}/src/jellyfin.rs (85%) diff --git a/Cargo.lock b/Cargo.lock index 1042d70..78bdfe6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -42,6 +42,7 @@ dependencies = [ "serde_json", "sqlx", "tempfile", + "thiserror", "tokio", "tracing", "utoipa", diff --git a/crates/arr-api/Cargo.toml b/crates/arr-api/Cargo.toml index 686ed94..b5da491 100644 --- a/crates/arr-api/Cargo.toml +++ b/crates/arr-api/Cargo.toml @@ -18,6 +18,7 @@ reqwest = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } sqlx = { workspace = true } +thiserror = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } utoipa = { workspace = true } diff --git a/crates/arr-daemon/src/jellyfin.rs b/crates/arr-api/src/jellyfin.rs similarity index 85% rename from crates/arr-daemon/src/jellyfin.rs rename to crates/arr-api/src/jellyfin.rs index dde3f47..e064d9a 100644 --- a/crates/arr-daemon/src/jellyfin.rs +++ b/crates/arr-api/src/jellyfin.rs @@ -1,8 +1,9 @@ -//! Jellyfin library refresh after import. See DESIGN.md §7.5. +//! Jellyfin library refresh after import or a subtitle write. See DESIGN.md +//! §7.5 and §15. //! -//! Jellyfin's own filesystem watcher misses the hardlinked file, so import -//! asks it to rescan directly. A refresh failure must not fail the import — -//! the caller logs and continues (§7.5). +//! Jellyfin's own filesystem watcher misses a hardlinked or sidecar file, so +//! callers ask it to rescan directly. A refresh failure must not fail the +//! caller — it logs and continues (§7.5). use std::time::Duration; @@ -27,6 +28,9 @@ pub struct JellyfinClient { } impl JellyfinClient { + /// # Errors + /// + /// If the underlying HTTP client cannot be built. pub fn new(base_url: impl Into, api_key: Option) -> Result { let client = Client::builder().timeout(REQUEST_TIMEOUT).build()?; Ok(Self { @@ -38,6 +42,10 @@ impl JellyfinClient { /// Trigger a full library scan. Jellyfin exposes no per-library refresh /// without knowing that library's ID, which this app never learns. + /// + /// # Errors + /// + /// If the request fails, or Jellyfin answers with a non-success status. pub async fn refresh(&self) -> Result<(), Error> { let url = format!("{}/Library/Refresh", self.base_url.trim_end_matches('/')); let mut request = self.client.post(url); diff --git a/crates/arr-api/src/lib.rs b/crates/arr-api/src/lib.rs index 38187b4..d42a89b 100644 --- a/crates/arr-api/src/lib.rs +++ b/crates/arr-api/src/lib.rs @@ -7,6 +7,7 @@ //! to compile, and the gate in DESIGN.md §12 fails with it. mod health; +pub mod jellyfin; mod metadata; mod movies; mod owners; diff --git a/crates/arr-api/src/state.rs b/crates/arr-api/src/state.rs index de8c4c0..e534120 100644 --- a/crates/arr-api/src/state.rs +++ b/crates/arr-api/src/state.rs @@ -7,6 +7,8 @@ use std::time::Duration; use arr_db::Db; use tokio::sync::mpsc; +use crate::jellyfin::JellyfinClient; + /// The TMDB API root. Not a bootstrap setting (DESIGN.md §10) — only the key /// is configurable, so this is a constant that tests point elsewhere. pub const DEFAULT_TMDB_URL: &str = "https://api.themoviedb.org/3"; @@ -76,6 +78,7 @@ pub struct AppState { pending_season_commands: Arc>>, metadata_commands: mpsc::Sender, pending_metadata_commands: Arc>>, + jellyfin: Option, } /// Work explicitly requested through the movie API. @@ -146,6 +149,7 @@ impl AppState { pending_season_commands: Arc::new(tokio::sync::Mutex::new(pending_season_commands)), metadata_commands, pending_metadata_commands: Arc::new(tokio::sync::Mutex::new(pending_metadata_commands)), + jellyfin: None, }) } @@ -156,6 +160,18 @@ impl AppState { self } + /// Attach the Jellyfin client, so a manual subtitle grab or translation + /// can trigger the same library refresh import does (§7.5, §15). + #[must_use] + pub fn with_jellyfin(mut self, jellyfin: JellyfinClient) -> Self { + self.jellyfin = Some(jellyfin); + self + } + + pub(crate) fn jellyfin(&self) -> Option<&JellyfinClient> { + self.jellyfin.as_ref() + } + /// Wait for the next manual movie action in the daemon's reconcile loop. /// /// # Errors diff --git a/crates/arr-daemon/src/import.rs b/crates/arr-daemon/src/import.rs index 80db79a..ebc6109 100644 --- a/crates/arr-daemon/src/import.rs +++ b/crates/arr-daemon/src/import.rs @@ -25,9 +25,9 @@ use arr_db::Db; use arr_dl::TransmissionClient; use arr_probe::Prober; -use crate::jellyfin::JellyfinClient; use crate::notify::Notifier; use crate::reconcile::{Action, ActionFuture, Outcome}; +use arr_api::jellyfin::JellyfinClient; /// A failure during one import tick. #[derive(Debug, thiserror::Error)] diff --git a/crates/arr-daemon/src/main.rs b/crates/arr-daemon/src/main.rs index 34c45c9..5264b36 100644 --- a/crates/arr-daemon/src/main.rs +++ b/crates/arr-daemon/src/main.rs @@ -6,7 +6,6 @@ mod config; mod grab; mod import; mod indexers; -mod jellyfin; mod manual; mod metadata; mod notify; @@ -94,7 +93,7 @@ enum Error { #[error("transmission client: {0}")] Transmission(#[from] arr_dl::Error), #[error("jellyfin client: {0}")] - Jellyfin(#[from] jellyfin::Error), + Jellyfin(#[from] arr_api::jellyfin::Error), #[error("ntfy client: {0}")] Notify(#[from] notify::NotifyError), #[error("bind {addr}: {source}")] @@ -124,6 +123,7 @@ async fn run() -> Result<(), Error> { None }; let notifier = Notifier::new(config.ntfy_url.clone())?; + let api_jellyfin = jellyfin_client(&config)?; let (reconcile, manual_grab, manual_tv) = reconcile_loop(&database, &config, &transmission, tmdb.as_ref(), ¬ifier)?; // Issue #176: the on-demand half of the metadata lane needs its own @@ -144,7 +144,9 @@ async fn run() -> Result<(), Error> { if let Some(tmdb_url) = config.tmdb_url { upstreams = upstreams.with_tmdb_url(tmdb_url); } - let state = AppState::new(upstreams)?.with_database(database.clone()); + let state = AppState::new(upstreams)? + .with_database(database.clone()) + .with_jellyfin(api_jellyfin); let app = arr_api::router(state.clone()) .merge(arr_compat::router(compat)) @@ -294,10 +296,7 @@ fn reconcile_loop( } else { tracing::warn!("TMDB is not configured: series metadata refresh is disabled"); } - let jellyfin = jellyfin::JellyfinClient::new( - config.jellyfin_url.clone(), - config.jellyfin_api_key.clone(), - )?; + let jellyfin = jellyfin_client(config)?; // Grab before import, so a download that completes on this tick is // imported on this tick. reconcile = reconcile.register( @@ -443,3 +442,13 @@ async fn shutdown() { tracing::info!("shutting down"); } + +/// The Jellyfin client, built fresh for each of its two independent callers: +/// import's own reconcile action, and the subtitle API's manual grab and +/// translate handlers (§7.5, §15). +fn jellyfin_client(config: &Config) -> Result { + Ok(arr_api::jellyfin::JellyfinClient::new( + config.jellyfin_url.clone(), + config.jellyfin_api_key.clone(), + )?) +} From bce3d3823dcaf2666f827687a3665f85a7ac6358 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 09:55:08 +0100 Subject: [PATCH 2/2] feat(api): move title files on root change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changing a movie's or series' root previously rewrote root_id and left the files behind, so the §7.4 layout stopped describing the disk and the root's policy applied to a library the files were not in. Series had no root control at all. All roots share one ZFS dataset, so the move is a rename of the title folder into the new root, never a copy — hardlinks and the seeding torrent survive it (§7.3). Disk first, row second: a destination that already holds the folder is a 409, a failed rename leaves the row unchanged, and a title with nothing on disk moves with no filesystem work. media_files rows are rewritten in the same transaction as the root_id, and a successful move triggers the §7.5 Jellyfin refresh. Issue #228 Co-Authored-By: Claude Fable 5 --- ...3621c69971e6dc0784f3d2c011c8e90a001d6.json | 12 + ...d95d2949fe2a0f1f59d55d6272cca1bd431aa.json | 38 +++ ...9ab5455e7a2d06fc86291e2c6c9696fd08953.json | 26 ++ ...a50df414a77bc191755d515e5d5905a651adc.json | 38 +++ DESIGN.md | 2 + crates/arr-api/src/lib.rs | 1 + crates/arr-api/src/movies.rs | 239 +++++++++++++++- crates/arr-api/src/relocate.rs | 259 ++++++++++++++++++ crates/arr-api/src/series.rs | 161 ++++++++++- 9 files changed, 770 insertions(+), 6 deletions(-) create mode 100644 .sqlx/query-20dd51ed5a7e54bb156d9a5eaf83621c69971e6dc0784f3d2c011c8e90a001d6.json create mode 100644 .sqlx/query-3be9c350ef24a69490540379225d95d2949fe2a0f1f59d55d6272cca1bd431aa.json create mode 100644 .sqlx/query-6ef45d7e6fc113b578fab91148f9ab5455e7a2d06fc86291e2c6c9696fd08953.json create mode 100644 .sqlx/query-89fb1aa140a11bcd9bb082b2dcba50df414a77bc191755d515e5d5905a651adc.json create mode 100644 crates/arr-api/src/relocate.rs diff --git a/.sqlx/query-20dd51ed5a7e54bb156d9a5eaf83621c69971e6dc0784f3d2c011c8e90a001d6.json b/.sqlx/query-20dd51ed5a7e54bb156d9a5eaf83621c69971e6dc0784f3d2c011c8e90a001d6.json new file mode 100644 index 0000000..9b13e30 --- /dev/null +++ b/.sqlx/query-20dd51ed5a7e54bb156d9a5eaf83621c69971e6dc0784f3d2c011c8e90a001d6.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "UPDATE media_files SET path = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", + "describe": { + "columns": [], + "parameters": { + "Right": 2 + }, + "nullable": [] + }, + "hash": "20dd51ed5a7e54bb156d9a5eaf83621c69971e6dc0784f3d2c011c8e90a001d6" +} diff --git a/.sqlx/query-3be9c350ef24a69490540379225d95d2949fe2a0f1f59d55d6272cca1bd431aa.json b/.sqlx/query-3be9c350ef24a69490540379225d95d2949fe2a0f1f59d55d6272cca1bd431aa.json new file mode 100644 index 0000000..d962efc --- /dev/null +++ b/.sqlx/query-3be9c350ef24a69490540379225d95d2949fe2a0f1f59d55d6272cca1bd431aa.json @@ -0,0 +1,38 @@ +{ + "db_name": "SQLite", + "query": "SELECT id AS \"id!: i64\", path AS \"path!: String\"\n FROM media_files WHERE owner_kind = 'movie' AND owner_id = ?", + "describe": { + "columns": [ + { + "name": "id!: i64", + "ordinal": 0, + "type_info": "Integer", + "origin": { + "Table": { + "table": "media_files", + "name": "id" + } + } + }, + { + "name": "path!: String", + "ordinal": 1, + "type_info": "Text", + "origin": { + "Table": { + "table": "media_files", + "name": "path" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + false + ] + }, + "hash": "3be9c350ef24a69490540379225d95d2949fe2a0f1f59d55d6272cca1bd431aa" +} diff --git a/.sqlx/query-6ef45d7e6fc113b578fab91148f9ab5455e7a2d06fc86291e2c6c9696fd08953.json b/.sqlx/query-6ef45d7e6fc113b578fab91148f9ab5455e7a2d06fc86291e2c6c9696fd08953.json new file mode 100644 index 0000000..5262844 --- /dev/null +++ b/.sqlx/query-6ef45d7e6fc113b578fab91148f9ab5455e7a2d06fc86291e2c6c9696fd08953.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT path AS \"path!: String\" FROM roots WHERE id = ?", + "describe": { + "columns": [ + { + "name": "path!: String", + "ordinal": 0, + "type_info": "Text", + "origin": { + "Table": { + "table": "roots", + "name": "path" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + }, + "hash": "6ef45d7e6fc113b578fab91148f9ab5455e7a2d06fc86291e2c6c9696fd08953" +} diff --git a/.sqlx/query-89fb1aa140a11bcd9bb082b2dcba50df414a77bc191755d515e5d5905a651adc.json b/.sqlx/query-89fb1aa140a11bcd9bb082b2dcba50df414a77bc191755d515e5d5905a651adc.json new file mode 100644 index 0000000..ade8330 --- /dev/null +++ b/.sqlx/query-89fb1aa140a11bcd9bb082b2dcba50df414a77bc191755d515e5d5905a651adc.json @@ -0,0 +1,38 @@ +{ + "db_name": "SQLite", + "query": "SELECT mf.id AS \"id!: i64\", mf.path AS \"path!: String\"\n FROM media_files mf\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 = ?", + "describe": { + "columns": [ + { + "name": "id!: i64", + "ordinal": 0, + "type_info": "Integer", + "origin": { + "Table": { + "table": "media_files", + "name": "id" + } + } + }, + { + "name": "path!: String", + "ordinal": 1, + "type_info": "Text", + "origin": { + "Table": { + "table": "media_files", + "name": "path" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false, + false + ] + }, + "hash": "89fb1aa140a11bcd9bb082b2dcba50df414a77bc191755d515e5d5905a651adc" +} diff --git a/DESIGN.md b/DESIGN.md index b269a9c..8d16fd3 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -488,6 +488,8 @@ Media kind first, hard audience boundary second, people nowhere. - **Release group is deliberately absent.** It is not a selection criterion and it makes filenames long enough to break a terminal. +Changing a title's root relocates its title folder into the new root; roots are assumed to share one filesystem, so the move is a rename, never a copy. + During transition, write into the existing roots so Jellyfin needs no reconfiguration and new content appears immediately. Radarr will not touch a folder it has no record of. diff --git a/crates/arr-api/src/lib.rs b/crates/arr-api/src/lib.rs index d42a89b..c912e04 100644 --- a/crates/arr-api/src/lib.rs +++ b/crates/arr-api/src/lib.rs @@ -13,6 +13,7 @@ mod movies; mod owners; mod policies; mod reclassify; +mod relocate; mod roots; mod search; mod series; diff --git a/crates/arr-api/src/movies.rs b/crates/arr-api/src/movies.rs index d7d13ac..96cdf48 100644 --- a/crates/arr-api/src/movies.rs +++ b/crates/arr-api/src/movies.rs @@ -430,9 +430,51 @@ pub async fn update( } let wanted = input.wanted.unwrap_or(current.wanted); let blocked = input.blocked.unwrap_or(current.blocked); - sqlx::query!("UPDATE movies SET title = ?, year = ?, original_language = ?, root_id = ?, wanted = ?, blocked = ?, overrides = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", title, year, original_language, root_id, wanted, blocked, overrides, id) - .execute(pool(&state)?) - .await?; + // A root change moves the §7.4 title folder with the row (issue #228). + // Disk first, row second: a failed rename leaves the row alone, so the + // operator sees the title where its files actually are and can retry — + // the same ordering `remove_library_files` documents. + let relocation = if root_id == current.root_id { + None + } else { + Some( + crate::relocate::relocate_title( + &state, + crate::relocate::TitleKind::Movie, + id, + current.root_id, + root_id, + ) + .await?, + ) + }; + let mut transaction = pool(&state)?.begin().await?; + let written: Result<(), sqlx::Error> = async { + sqlx::query!("UPDATE movies SET title = ?, year = ?, original_language = ?, root_id = ?, wanted = ?, blocked = ?, overrides = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", title, year, original_language, root_id, wanted, blocked, overrides, id) + .execute(&mut *transaction) + .await?; + if let Some(relocation) = &relocation { + relocation.rewrite_rows(&mut transaction).await?; + } + Ok(()) + } + .await; + let committed = match written { + Ok(()) => transaction.commit().await.map_err(ApiError::from), + Err(error) => Err(ApiError::from(error)), + }; + if let Err(error) = committed { + if let Some(relocation) = &relocation { + relocation.undo().await; + } + return Err(error); + } + if relocation + .as_ref() + .is_some_and(crate::relocate::Relocation::moved_files) + { + crate::relocate::refresh_jellyfin(&state).await; + } if overrides_changed { crate::reclassify::movie(&state, id).await?; } @@ -1417,6 +1459,197 @@ mod tests { ); } + /// Point one of the seeded roots at a real directory for the duration of + /// a test. + 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")) + .bind(root_id) + .execute(state.database().expect("database").pool()) + .await + .expect("point the root at the tempdir"); + } + + /// Issue #228: changing a movie's root renames its §7.4 folder into the + /// new root — sidecars ride along inside it — and the `media_files` rows + /// follow in the same write. + #[tokio::test] + async fn changing_root_moves_the_title_folder_and_its_rows() { + let (_dir, state, base) = application().await; + let movie = add_movie(&base, 693_134, 1).await; + let id = movie["id"].as_i64().expect("id"); + let source = tempfile::tempdir().expect("source root"); + let destination = tempfile::tempdir().expect("destination root"); + let folder = library_on_disk(&state, id, source.path()).await; + point_root_at(&state, 2, destination.path()).await; + + // The wrong kind of root is still rejected, before anything moves. + let rejected = reqwest::Client::new() + .patch(format!("{base}/api/movies/{id}")) + .json(&serde_json::json!({"root_id": 3})) + .send() + .await + .expect("move to a tv root"); + assert_eq!(rejected.status(), StatusCode::UNPROCESSABLE_ENTITY); + + let updated: serde_json::Value = reqwest::Client::new() + .patch(format!("{base}/api/movies/{id}")) + .json(&serde_json::json!({"root_id": 2})) + .send() + .await + .expect("move root") + .json() + .await + .expect("updated json"); + assert_eq!(updated["root_id"], 2); + + assert!(!folder.exists(), "the folder left the old root"); + let moved = destination + .path() + .join("Dune Part Two (2024) [tmdbid-693134]"); + assert!( + moved + .join("Dune Part Two (2024) [tmdbid-693134] - [2160p].mkv") + .exists(), + "the feature arrived in the new root" + ); + assert!( + moved.join("dune.pt.srt").exists(), + "sidecars travel inside the folder" + ); + + let path: String = sqlx::query_scalar( + "SELECT path FROM media_files WHERE owner_kind = 'movie' AND owner_id = ?", + ) + .bind(id) + .fetch_one(state.database().expect("database").pool()) + .await + .expect("media file row"); + assert!( + std::path::Path::new(&path).starts_with(destination.path()), + "the row follows the file: {path}" + ); + assert!( + std::path::Path::new(&path).exists(), + "the rewritten path describes the disk" + ); + } + + /// Issue #228: a title with nothing on disk changes root with no + /// filesystem work at all — the seeded root paths do not even exist. + #[tokio::test] + async fn a_movie_with_no_files_changes_root_cleanly() { + let (_dir, state, base) = application().await; + let movie = add_movie(&base, 693_134, 1).await; + let id = movie["id"].as_i64().expect("id"); + + let updated: serde_json::Value = reqwest::Client::new() + .patch(format!("{base}/api/movies/{id}")) + .json(&serde_json::json!({"root_id": 2})) + .send() + .await + .expect("move root") + .json() + .await + .expect("updated json"); + assert_eq!(updated["root_id"], 2); + + let root_id: i64 = sqlx::query_scalar("SELECT root_id FROM movies WHERE id = ?") + .bind(id) + .fetch_one(state.database().expect("database").pool()) + .await + .expect("movie row"); + assert_eq!(root_id, 2); + } + + /// Issue #228: a destination already holding a folder of that name is a + /// conflict, not an overwrite — and the row stays where the files are. + #[tokio::test] + async fn a_destination_collision_is_refused_and_the_row_unchanged() { + let (_dir, state, base) = application().await; + let movie = add_movie(&base, 693_134, 1).await; + let id = movie["id"].as_i64().expect("id"); + let source = tempfile::tempdir().expect("source root"); + let destination = tempfile::tempdir().expect("destination root"); + let folder = library_on_disk(&state, id, source.path()).await; + point_root_at(&state, 2, destination.path()).await; + let squatter = destination + .path() + .join("Dune Part Two (2024) [tmdbid-693134]"); + tokio::fs::create_dir_all(&squatter) + .await + .expect("pre-existing folder"); + tokio::fs::write(squatter.join("theirs.mkv"), b"not ours") + .await + .expect("write squatter file"); + + let response = reqwest::Client::new() + .patch(format!("{base}/api/movies/{id}")) + .json(&serde_json::json!({"root_id": 2})) + .send() + .await + .expect("move root"); + assert_eq!(response.status(), StatusCode::CONFLICT); + + assert!(folder.exists(), "the folder stayed in the old root"); + assert!( + squatter.join("theirs.mkv").exists(), + "the occupant was not overwritten" + ); + let (root_id, path): (i64, String) = sqlx::query_as( + "SELECT m.root_id, f.path FROM movies m + JOIN media_files f ON f.owner_kind = 'movie' AND f.owner_id = m.id + WHERE m.id = ?", + ) + .bind(id) + .fetch_one(state.database().expect("database").pool()) + .await + .expect("movie row"); + assert_eq!(root_id, 1, "the row is unchanged"); + assert!(std::path::Path::new(&path).starts_with(source.path())); + } + + /// Issue #228: if the rename fails, the row must not change — the + /// operator sees the title where its files actually are and can retry. + #[tokio::test] + async fn a_failed_rename_leaves_the_row_alone() { + let (_dir, state, base) = application().await; + let movie = add_movie(&base, 693_134, 1).await; + let id = movie["id"].as_i64().expect("id"); + let source = tempfile::tempdir().expect("source root"); + let destination = tempfile::tempdir().expect("destination root"); + let folder = library_on_disk(&state, id, source.path()).await; + // A destination whose parent does not exist makes the rename itself + // fail while the collision pre-check still passes. + point_root_at( + &state, + 2, + &destination.path().join("missing").join("library"), + ) + .await; + + let response = reqwest::Client::new() + .patch(format!("{base}/api/movies/{id}")) + .json(&serde_json::json!({"root_id": 2})) + .send() + .await + .expect("move root"); + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + + assert!(folder.exists(), "the folder never left the old root"); + let (root_id, path): (i64, String) = sqlx::query_as( + "SELECT m.root_id, f.path FROM movies m + JOIN media_files f ON f.owner_kind = 'movie' AND f.owner_id = m.id + WHERE m.id = ?", + ) + .bind(id) + .fetch_one(state.database().expect("database").pool()) + .await + .expect("movie row"); + assert_eq!(root_id, 1, "the row is unchanged"); + assert!(std::path::Path::new(&path).starts_with(source.path())); + } + #[test] fn a_title_target_is_the_folder_directly_under_the_root() { let root = "/mnt/media/movies/main"; diff --git a/crates/arr-api/src/relocate.rs b/crates/arr-api/src/relocate.rs new file mode 100644 index 0000000..f1d3f6f --- /dev/null +++ b/crates/arr-api/src/relocate.rs @@ -0,0 +1,259 @@ +//! Moving a title between roots (issue #228). Changing a title's `root_id` +//! renames its §7.4 folder into the new root and rewrites the `media_files` +//! rows to match, so the layout keeps describing the disk and the new root's +//! policy (§5.1) applies to a library the files are actually in. +//! +//! Every root shares one filesystem — one ZFS dataset, bind-mounted — so this +//! is a directory rename, never a copy. Hardlinked files keep their inodes +//! and the torrent keeps seeding against them (§7.3). +//! +//! Ordering mirrors `remove_library_files`: the disk is touched before the +//! row changes, so a failed rename leaves the title where its files actually +//! are and the operator can retry. + +use std::path::PathBuf; + +use crate::movies::{pool, title_target, ApiError}; +use crate::state::AppState; + +/// Which table owns the moving title's files. +#[derive(Debug, Clone, Copy)] +pub(crate) enum TitleKind { + Movie, + Series, +} + +/// One rename from the old root into the new one: a §7.4 title folder, or a +/// loose file sitting straight in the root. +#[derive(Debug)] +struct PlannedRename { + source: PathBuf, + destination: PathBuf, +} + +/// The renames already performed on disk and the row rewrites they imply. +/// The database half is the caller's transaction; [`Self::undo`] is for when +/// that transaction fails after the disk already changed. +#[derive(Debug)] +pub(crate) struct Relocation { + performed: Vec, + rewrites: Vec<(i64, String)>, +} + +/// Rename the title's folders into the new root. Called before the row is +/// written, and only when the root actually changes. +/// +/// A title with nothing on disk — no `media_files` rows, or rows whose +/// targets are already gone — changes root with no filesystem work at all. +/// +/// # Errors +/// +/// [`ApiError::Conflict`] when the destination already holds an entry of the +/// same name — a conflict, never an overwrite. [`ApiError::Filesystem`] when +/// a rename fails; whatever had already been renamed is moved back first, so +/// the row the caller then leaves unchanged still describes the disk. +pub(crate) async fn relocate_title( + state: &AppState, + kind: TitleKind, + title_id: i64, + old_root_id: i64, + new_root_id: i64, +) -> Result { + let old_root = root_path(state, old_root_id).await?; + let new_root = root_path(state, new_root_id).await?; + let files = title_files(state, kind, title_id).await?; + + let mut renames: Vec = Vec::new(); + let mut rewrites: Vec<(i64, String)> = Vec::new(); + for (file_id, path) in &files { + let Some(source) = title_target(&old_root, path) else { + // Outside its own root: not ours to move, and the row keeps + // pointing at where the file really is. + tracing::warn!(%path, %old_root, "media file is outside its root, not moved"); + continue; + }; + let Some(name) = source.file_name() else { + continue; + }; + let destination = std::path::Path::new(&new_root).join(name); + if !renames.iter().any(|rename| rename.source == source) { + renames.push(PlannedRename { + source, + destination, + }); + } + let relative = std::path::Path::new(path) + .strip_prefix(std::path::Path::new(&old_root)) + .map_err(|error| ApiError::Filesystem(error.to_string()))?; + let rewritten = std::path::Path::new(&new_root).join(relative); + let Some(rewritten) = rewritten.to_str() else { + return Err(ApiError::Filesystem(format!( + "non-UTF-8 path under {new_root}" + ))); + }; + rewrites.push((*file_id, rewritten.to_owned())); + } + + // Every destination is checked before anything is renamed, so a conflict + // never leaves a half-moved title behind. + for rename in &renames { + match tokio::fs::symlink_metadata(&rename.destination).await { + Ok(_) => { + return Err(ApiError::Conflict(format!( + "the destination root already has '{}'", + rename.destination.display() + ))) + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(ApiError::Filesystem(error.to_string())), + } + } + + let mut performed: Vec = Vec::new(); + for rename in renames { + // A recorded file with nothing on disk: the rows still follow the + // title, the same way a delete treats already-gone as done. Checked + // on the source, so a missing *destination* parent stays an error. + match tokio::fs::symlink_metadata(&rename.source).await { + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + tracing::warn!( + source = %rename.source.display(), + "nothing on disk to move for this title" + ); + continue; + } + Err(error) => { + let failed = ApiError::Filesystem(error.to_string()); + Relocation { + performed, + rewrites: Vec::new(), + } + .undo() + .await; + return Err(failed); + } + } + match tokio::fs::rename(&rename.source, &rename.destination).await { + Ok(()) => { + tracing::info!( + source = %rename.source.display(), + destination = %rename.destination.display(), + "moved title folder between roots" + ); + performed.push(rename); + } + Err(error) => { + let failed = ApiError::Filesystem(error.to_string()); + Relocation { + performed, + rewrites: Vec::new(), + } + .undo() + .await; + return Err(failed); + } + } + } + + Ok(Relocation { + performed, + rewrites, + }) +} + +impl Relocation { + /// Whether anything on disk actually moved — the trigger for the same + /// single Jellyfin refresh import performs (§7.5). + pub(crate) fn moved_files(&self) -> bool { + !self.performed.is_empty() + } + + /// Point the `media_files` rows at the new root, inside the caller's + /// transaction so they land together with the `root_id` change or not at + /// all. + pub(crate) async fn rewrite_rows( + &self, + transaction: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + ) -> Result<(), sqlx::Error> { + for (file_id, path) in &self.rewrites { + sqlx::query!( + "UPDATE media_files SET path = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", + path, + file_id + ) + .execute(&mut **transaction) + .await?; + } + Ok(()) + } + + /// Best-effort reversal of the renames, for when the disk moved but the + /// database write failed. A reversal that itself fails is logged: at that + /// point the operator's retry is the recovery path. + pub(crate) async fn undo(&self) { + for rename in self.performed.iter().rev() { + if let Err(error) = tokio::fs::rename(&rename.destination, &rename.source).await { + tracing::error!( + source = %rename.source.display(), + destination = %rename.destination.display(), + %error, + "could not move the title folder back after a failed root change" + ); + } + } + } +} + +/// §7.5 after a move: the same single best-effort refresh import performs. +/// Failure logs and never fails the write that already committed. +pub(crate) async fn refresh_jellyfin(state: &AppState) { + if let Some(jellyfin) = state.jellyfin() { + if let Err(error) = jellyfin.refresh().await { + tracing::warn!(%error, "jellyfin refresh after a root change failed"); + } + } +} + +async fn root_path(state: &AppState, root_id: i64) -> Result { + Ok(sqlx::query_scalar!( + r#"SELECT path AS "path!: String" FROM roots WHERE id = ?"#, + root_id + ) + .fetch_one(pool(state)?) + .await?) +} + +/// Every file the service recorded for the title: a movie's own rows, or the +/// rows of every episode below a series. +async fn title_files( + state: &AppState, + kind: TitleKind, + title_id: i64, +) -> Result, ApiError> { + Ok(match kind { + TitleKind::Movie => sqlx::query!( + r#"SELECT id AS "id!: i64", path AS "path!: String" + FROM media_files WHERE owner_kind = 'movie' AND owner_id = ?"#, + title_id + ) + .fetch_all(pool(state)?) + .await? + .into_iter() + .map(|row| (row.id, row.path)) + .collect(), + TitleKind::Series => sqlx::query!( + r#"SELECT mf.id AS "id!: i64", mf.path AS "path!: String" + FROM media_files mf + 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 = ?"#, + title_id + ) + .fetch_all(pool(state)?) + .await? + .into_iter() + .map(|row| (row.id, row.path)) + .collect(), + }) +} diff --git a/crates/arr-api/src/series.rs b/crates/arr-api/src/series.rs index a8c48f3..453f989 100644 --- a/crates/arr-api/src/series.rs +++ b/crates/arr-api/src/series.rs @@ -582,9 +582,51 @@ pub async fn update( let auto_track = input.auto_track.unwrap_or(current.auto_track); let upstream_ended = input.upstream_ended.unwrap_or(current.upstream_ended); let blocked = input.blocked.unwrap_or(current.blocked); - sqlx::query!("UPDATE series SET title = ?, year = ?, original_language = ?, root_id = ?, auto_track = ?, upstream_ended = ?, blocked = ?, overrides = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", title, year, original_language, root_id, auto_track, upstream_ended, blocked, overrides, id) - .execute(pool(&state)?) - .await?; + // A root change moves the §7.4 title folder with the row (issue #228). + // Disk first, row second: a failed rename leaves the row alone, so the + // operator sees the title where its files actually are and can retry — + // the same ordering `remove_library_files` documents. + let relocation = if root_id == current.root_id { + None + } else { + Some( + crate::relocate::relocate_title( + &state, + crate::relocate::TitleKind::Series, + id, + current.root_id, + root_id, + ) + .await?, + ) + }; + let mut transaction = pool(&state)?.begin().await?; + let written: Result<(), sqlx::Error> = async { + sqlx::query!("UPDATE series SET title = ?, year = ?, original_language = ?, root_id = ?, auto_track = ?, upstream_ended = ?, blocked = ?, overrides = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') WHERE id = ?", title, year, original_language, root_id, auto_track, upstream_ended, blocked, overrides, id) + .execute(&mut *transaction) + .await?; + if let Some(relocation) = &relocation { + relocation.rewrite_rows(&mut transaction).await?; + } + Ok(()) + } + .await; + let committed = match written { + Ok(()) => transaction.commit().await.map_err(ApiError::from), + Err(error) => Err(ApiError::from(error)), + }; + if let Err(error) = committed { + if let Some(relocation) = &relocation { + relocation.undo().await; + } + return Err(error); + } + if relocation + .as_ref() + .is_some_and(crate::relocate::Relocation::moved_files) + { + crate::relocate::refresh_jellyfin(&state).await; + } if overrides_changed { crate::reclassify::series(&state, id).await?; } @@ -3654,6 +3696,119 @@ mod tests { ); } + /// Issue #228: changing a series' root renames its §7.4 title folder — + /// season subfolders inside it — into the new root, and every episode's + /// `media_files` row follows. + #[tokio::test] + async fn changing_root_moves_the_series_folder_and_its_rows() { + let (_dir, state, base) = application().await; + let main_root = tv_root(&state, "main").await; + let kids_root = tv_root(&state, "kids").await; + let series = add_series(&base, main_root, false).await; + let series_id = series["id"].as_i64().expect("id"); + let season = add_season( + &base, + series_id, + 1, + serde_json::json!([{ "number": 1, "title": "Magic Xylophone" }]), + ) + .await; + let season_id = season["id"].as_i64().expect("season id"); + let episode_id: i64 = + sqlx::query_scalar("SELECT id FROM episodes WHERE season_id = ? AND number = 1") + .bind(season_id) + .fetch_one(state.database().expect("database").pool()) + .await + .expect("episode id"); + + let source = tempfile::tempdir().expect("source root"); + let destination = tempfile::tempdir().expect("destination root"); + let folder = source.path().join("Bluey (2018) [tmdbid-82728]"); + let episode_file = folder.join("Season 01").join("Bluey (2018) - S01E01.mkv"); + tokio::fs::create_dir_all(folder.join("Season 01")) + .await + .expect("create season folder"); + tokio::fs::write(&episode_file, b"episode") + .await + .expect("write episode"); + let pool = state.database().expect("database").pool(); + for (root, path) in [(main_root, source.path()), (kids_root, destination.path())] { + sqlx::query("UPDATE roots SET path = ? WHERE id = ?") + .bind(path.to_str().expect("utf-8 root")) + .bind(root) + .execute(pool) + .await + .expect("point the root at the tempdir"); + } + sqlx::query( + "INSERT INTO media_files (owner_kind, owner_id, path, size) VALUES ('episode', ?, ?, 7)", + ) + .bind(episode_id) + .bind(episode_file.to_str().expect("utf-8 path")) + .execute(pool) + .await + .expect("media file"); + + // The wrong kind of root is still rejected, before anything moves. + let rejected = reqwest::Client::new() + .patch(format!("{base}/api/series/{series_id}")) + .json(&serde_json::json!({"root_id": 1})) + .send() + .await + .expect("move to a movie root"); + assert_eq!(rejected.status(), StatusCode::UNPROCESSABLE_ENTITY); + + let updated: serde_json::Value = reqwest::Client::new() + .patch(format!("{base}/api/series/{series_id}")) + .json(&serde_json::json!({"root_id": kids_root})) + .send() + .await + .expect("move root") + .json() + .await + .expect("updated json"); + assert_eq!(updated["root_id"], kids_root); + + assert!(!folder.exists(), "the folder left the old root"); + let moved = destination + .path() + .join("Bluey (2018) [tmdbid-82728]") + .join("Season 01") + .join("Bluey (2018) - S01E01.mkv"); + assert!(moved.exists(), "the episode arrived, season folder intact"); + + let path: String = sqlx::query_scalar( + "SELECT path FROM media_files WHERE owner_kind = 'episode' AND owner_id = ?", + ) + .bind(episode_id) + .fetch_one(pool) + .await + .expect("media file row"); + assert_eq!(path, moved.to_str().expect("utf-8 path").to_owned()); + } + + /// Issue #228: a series with nothing on disk changes root with no + /// filesystem work at all. + #[tokio::test] + async fn a_series_with_no_files_changes_root_cleanly() { + let (_dir, state, base) = application().await; + let main_root = tv_root(&state, "main").await; + let kids_root = tv_root(&state, "kids").await; + let series = add_series(&base, main_root, false).await; + let series_id = series["id"].as_i64().expect("id"); + + let updated: serde_json::Value = reqwest::Client::new() + .patch(format!("{base}/api/series/{series_id}")) + .json(&serde_json::json!({"root_id": kids_root})) + .send() + .await + .expect("move root") + .json() + .await + .expect("updated json"); + assert_eq!(updated["root_id"], kids_root); + } + #[test] fn air_dates_parse_as_dates_and_as_timestamps() { assert_eq!(