From 43e65514ed77deba5931c07441c97daeb7f717df Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Tue, 25 Aug 2026 11:17:33 +0100 Subject: [PATCH] fix(api): reclassify on root and policy changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving a title to a root with a different policy, and pointing a root at a different policy via PUT /api/roots/{id}, both changed the effective policy without re-deriving stored verdicts — which §9.3's deck and the daemon's manual-grab gate read. Both now run the same reclassify the overrides path uses, inline in the request; §5.1 states the contract, and relocate.rs no longer claims the move alone makes the policy apply. PUT /api/policies/{id} has the same gap one level up; noted on #241 for its own issue. Closes #241 Co-Authored-By: Claude Fable 5 --- ...0839c26fbbea836aca6a55a43d75210ae4368.json | 26 +++ ...232fc8a45e31c1a74ae487198b2d286b81e96.json | 26 +++ DESIGN.md | 11 + crates/arr-api/src/movies.rs | 68 ++++++- crates/arr-api/src/reclassify.rs | 39 +++- crates/arr-api/src/relocate.rs | 5 +- crates/arr-api/src/roots.rs | 191 ++++++++++++++++++ crates/arr-api/src/series.rs | 69 ++++++- 8 files changed, 429 insertions(+), 6 deletions(-) create mode 100644 .sqlx/query-2bb8e5739bd8d16bc7c5a08f5530839c26fbbea836aca6a55a43d75210ae4368.json create mode 100644 .sqlx/query-cc5686ba4766a79d333e98ddcce232fc8a45e31c1a74ae487198b2d286b81e96.json diff --git a/.sqlx/query-2bb8e5739bd8d16bc7c5a08f5530839c26fbbea836aca6a55a43d75210ae4368.json b/.sqlx/query-2bb8e5739bd8d16bc7c5a08f5530839c26fbbea836aca6a55a43d75210ae4368.json new file mode 100644 index 0000000..26ddc1e --- /dev/null +++ b/.sqlx/query-2bb8e5739bd8d16bc7c5a08f5530839c26fbbea836aca6a55a43d75210ae4368.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT id AS \"id!: i64\" FROM movies WHERE root_id = ?", + "describe": { + "columns": [ + { + "name": "id!: i64", + "ordinal": 0, + "type_info": "Integer", + "origin": { + "Table": { + "table": "movies", + "name": "id" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + }, + "hash": "2bb8e5739bd8d16bc7c5a08f5530839c26fbbea836aca6a55a43d75210ae4368" +} diff --git a/.sqlx/query-cc5686ba4766a79d333e98ddcce232fc8a45e31c1a74ae487198b2d286b81e96.json b/.sqlx/query-cc5686ba4766a79d333e98ddcce232fc8a45e31c1a74ae487198b2d286b81e96.json new file mode 100644 index 0000000..9afc9d2 --- /dev/null +++ b/.sqlx/query-cc5686ba4766a79d333e98ddcce232fc8a45e31c1a74ae487198b2d286b81e96.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT id AS \"id!: i64\" FROM series WHERE root_id = ?", + "describe": { + "columns": [ + { + "name": "id!: i64", + "ordinal": 0, + "type_info": "Integer", + "origin": { + "Table": { + "table": "series", + "name": "id" + } + } + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + }, + "hash": "cc5686ba4766a79d333e98ddcce232fc8a45e31c1a74ae487198b2d286b81e96" +} diff --git a/DESIGN.md b/DESIGN.md index d6c46b2..b730786 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -199,6 +199,17 @@ fake profile per root and ignoring whatever it sends. Per-title `overrides` relax the root policy for one title. Same mechanism in both directions — `only_4k` tightens, `allow_english_audio` loosens. +**Stored verdicts follow the effective policy.** A release's verdict is +stamped by the search that found it, and both §9.3's deck and the daemon's +manual-grab gate read that stored column. So the three operator actions that +change a title's effective policy — editing its overrides, moving it to a +root with a different policy, and pointing a root at a different policy — +re-derive the stored verdicts of everything they touch, inline in the same +request. The operator is never left reading a verdict computed under a +policy that no longer applies. Inline is affordable even root-wide: +re-evaluation is pure and in-memory, and a row whose verdict does not move +is not rewritten, so a library-sized repoint costs reads, not writes. + ### 5.2 Language Requires a concept the release name does not carry: the **original language of diff --git a/crates/arr-api/src/movies.rs b/crates/arr-api/src/movies.rs index 1be1218..fabc819 100644 --- a/crates/arr-api/src/movies.rs +++ b/crates/arr-api/src/movies.rs @@ -475,7 +475,10 @@ pub async fn update( { crate::relocate::refresh_jellyfin(&state).await; } - if overrides_changed { + // A root change swaps the effective policy the same way an overrides + // edit does (§5.1), so both re-derive; `relocation` is `Some` exactly + // when the root changed. + if overrides_changed || relocation.is_some() { crate::reclassify::movie(&state, id).await?; } Ok(Json(load_movie(&state, id).await?)) @@ -1302,6 +1305,69 @@ mod tests { assert_eq!(releases[0]["verdict"], "waived"); } + /// #241: moving a title to a root with a different policy re-derives its + /// stored verdicts (§5.1) — an English-audio release a `main` root found + /// eligible is only a waiver under the `kids` root it moved to. + #[tokio::test] + async fn moving_a_movie_to_another_root_rederives_its_verdicts() { + let (_dir, state, base) = application().await; + let pool = state.database().expect("database").pool(); + let created: serde_json::Value = reqwest::Client::new() + .post(format!("{base}/api/movies")) + .json(&serde_json::json!({ + "tmdb_id": 693_134, "title": "Dune Part Two", "year": 2024, + "original_language": "en", "root_id": 1, "overrides": {} + })) + .send() + .await + .expect("create movie") + .json() + .await + .expect("movie json"); + let movie_id = created["id"].as_i64().expect("id"); + + let name = "Dune Part Two 2024 1080p WEB-DL ENGLISH x264-GROUP"; + let parsed = arr_parse::parse(name); + let size = 6_i64 * (1 << 30); + let release_id = sqlx::query_scalar::<_, i64>( + "INSERT INTO releases (indexer_id, guid, name, size, seeders, download_url, parsed, score, verdict) + VALUES (7, 'g', ?, ?, 40, 'url', ?, 0, 'eligible') RETURNING id", + ) + .bind(name) + .bind(size) + .bind(serde_json::to_string(&parsed).expect("parsed json")) + .fetch_one(pool) + .await + .expect("release"); + sqlx::query("INSERT INTO movie_releases (movie_id, release_id) VALUES (?, ?)") + .bind(movie_id) + .bind(release_id) + .execute(pool) + .await + .expect("association"); + + let response = reqwest::Client::new() + .patch(format!("{base}/api/movies/{movie_id}")) + .json(&serde_json::json!({"root_id": 2})) + .send() + .await + .expect("move root"); + assert_eq!(response.status(), StatusCode::OK); + + let releases: Vec = + reqwest::get(format!("{base}/api/movies/{movie_id}/releases")) + .await + .expect("releases") + .json() + .await + .expect("releases json"); + assert_eq!(releases.len(), 1); + assert_eq!( + releases[0]["verdict"], "waived", + "kids requires pt-PT, English is a soft fail: {releases:?}" + ); + } + /// A blacklisted release (§6.3) is not a policy opinion, so no override /// re-opens it. #[tokio::test] diff --git a/crates/arr-api/src/reclassify.rs b/crates/arr-api/src/reclassify.rs index 70fd9a8..e95d265 100644 --- a/crates/arr-api/src/reclassify.rs +++ b/crates/arr-api/src/reclassify.rs @@ -1,10 +1,15 @@ -//! Stored verdicts, re-derived when a title's overrides change (§9.3). +//! Stored verdicts, re-derived when a title's effective policy changes: +//! an overrides edit (§9.3), a move to a root with a different policy, or a +//! root pointed at a different policy (§5.1). //! //! A release's verdict is stamped once, by the search that found it. Both the //! deck and the daemon's manual-grab gate read that stored column, so an //! override written from the deck's one-click waive would change nothing //! until the next sweep — the row the operator just acted on would keep -//! reading `rejected` and the grab would be refused. +//! reading `rejected` and the grab would be refused. A root move and a +//! policy repoint invalidate the column the same way, just wider: nothing +//! else re-reads it, so the write that changed the effective policy is the +//! only place the correction can happen. //! //! So the rules run again here, over the releases already attached to the //! title. This is the same correction the daemon makes when a grab turns out @@ -141,6 +146,36 @@ pub(crate) async fn series(state: &AppState, series_id: i64) -> Result<(), ApiEr .await } +/// Re-evaluate every release of every title under one root, for a +/// `PUT /api/roots/{id}` that pointed the root at a different policy (§5.1). +/// +/// Title by title through [`movie`] and [`series`], so the skip rules — no +/// stored original language, the blacklist (§6.3) — stay in one place. Rows +/// whose verdict the new policy does not move are left unwritten, so the +/// usual case (most of a library re-evaluates to the same answer) costs +/// reads, not writes. +pub(crate) async fn root(state: &AppState, root_id: i64) -> Result<(), ApiError> { + let movie_ids = sqlx::query_scalar!( + r#"SELECT id AS "id!: i64" FROM movies WHERE root_id = ?"#, + root_id + ) + .fetch_all(pool(state)?) + .await?; + for movie_id in movie_ids { + movie(state, movie_id).await?; + } + let series_ids = sqlx::query_scalar!( + r#"SELECT id AS "id!: i64" FROM series WHERE root_id = ?"#, + root_id + ) + .fetch_all(pool(state)?) + .await?; + for series_id in series_ids { + series(state, series_id).await?; + } + Ok(()) +} + #[allow(clippy::too_many_arguments)] async fn apply( state: &AppState, diff --git a/crates/arr-api/src/relocate.rs b/crates/arr-api/src/relocate.rs index 3641efa..459b387 100644 --- a/crates/arr-api/src/relocate.rs +++ b/crates/arr-api/src/relocate.rs @@ -1,8 +1,9 @@ //! Moving library files when the layout under them changes: a title changing //! its `root_id` (issue #228), and a root changing its `path` (issue #236). //! Both rename §7.4 folders and rewrite the `media_files` rows to match, so -//! the layout keeps describing the disk and the root's policy (§5.1) applies -//! to a library the files are actually in. +//! the layout keeps describing the disk. This module only moves things: the +//! stored verdicts a root change invalidates (§5.1) are re-derived by the +//! calling handler through `reclassify`, after the row commits. //! //! Every root shares one filesystem — one ZFS dataset, bind-mounted — so this //! is a directory rename, never a copy. Hardlinked files keep their inodes diff --git a/crates/arr-api/src/roots.rs b/crates/arr-api/src/roots.rs index 758bd27..71ead45 100644 --- a/crates/arr-api/src/roots.rs +++ b/crates/arr-api/src/roots.rs @@ -236,6 +236,11 @@ pub async fn update( { crate::relocate::refresh_jellyfin(&state).await; } + // Verdicts already stored under this root were reached under the old + // policy; §9.3's deck and the grab gate both read them (`reclassify`). + if input.policy_id != current.policy_id { + crate::reclassify::root(&state, id).await?; + } Ok(Json(load_root(&state, id).await?)) } @@ -385,6 +390,192 @@ mod tests { .collect() } + async fn policy_id_named(base: &str, name: &str) -> i64 { + let policies: Vec = reqwest::get(format!("{base}/api/policies")) + .await + .expect("policies") + .json() + .await + .expect("policies json"); + policies + .iter() + .find(|policy| policy["name"] == name) + .and_then(|policy| policy["id"].as_i64()) + .expect("policy id") + } + + async fn stamped_eligible_release(state: &AppState, name: &str, size: i64) -> i64 { + let parsed = arr_parse::parse(name); + sqlx::query_scalar::<_, i64>( + "INSERT INTO releases (indexer_id, guid, name, size, seeders, download_url, parsed, score, verdict) + VALUES (7, ?, ?, ?, 40, 'url', ?, 0, 'eligible') RETURNING id", + ) + .bind(name) + .bind(name) + .bind(size) + .bind(serde_json::to_string(&parsed).expect("parsed json")) + .fetch_one(state.database().expect("database").pool()) + .await + .expect("release") + } + + /// A movie under the movie main root, its release stamped eligible. + async fn movie_with_eligible_release(state: &AppState, base: &str) -> i64 { + let movie: serde_json::Value = reqwest::Client::new() + .post(format!("{base}/api/movies")) + .json(&serde_json::json!({ + "tmdb_id": 693_134, "title": "Dune Part Two", "year": 2024, + "original_language": "en", "root_id": 1, "overrides": {} + })) + .send() + .await + .expect("create movie") + .json() + .await + .expect("movie json"); + let movie_id = movie["id"].as_i64().expect("movie id"); + let release_id = stamped_eligible_release( + state, + "Dune Part Two 2024 1080p WEB-DL ENGLISH x264-GROUP", + 6_i64 * (1 << 30), + ) + .await; + sqlx::query("INSERT INTO movie_releases (movie_id, release_id) VALUES (?, ?)") + .bind(movie_id) + .bind(release_id) + .execute(state.database().expect("database").pool()) + .await + .expect("movie association"); + movie_id + } + + /// A series under the given TV root, with a season 9 pack stamped + /// eligible. Ten episodes in 15 GiB: 1.5 GiB each, inside the 1080p band. + async fn series_with_eligible_pack(state: &AppState, base: &str, root_id: i64) -> i64 { + let client = reqwest::Client::new(); + let series: serde_json::Value = client + .post(format!("{base}/api/series")) + .json(&serde_json::json!({ + "tmdb_id": 82_728, "title": "Bluey", "year": 2018, + "original_language": "en", "root_id": root_id, + "auto_track": false + })) + .send() + .await + .expect("create series") + .json() + .await + .expect("series json"); + let series_id = series["id"].as_i64().expect("series id"); + let episodes: Vec = (1..=10) + .map(|number| { + serde_json::json!({ + "number": number, "title": format!("Episode {number}"), + "air_date": "2025-01-01" + }) + }) + .collect(); + let season: serde_json::Value = client + .post(format!("{base}/api/series/{series_id}/seasons")) + .json(&serde_json::json!({"number": 9, "episodes": episodes})) + .send() + .await + .expect("create season") + .json() + .await + .expect("season json"); + let season_id = season["id"].as_i64().expect("season id"); + let release_id = stamped_eligible_release( + state, + "Bluey S09 1080p WEB-DL ENGLISH x264-GROUP", + 15_i64 * (1 << 30), + ) + .await; + sqlx::query("INSERT INTO season_releases (season_id, release_id) VALUES (?, ?)") + .bind(season_id) + .bind(release_id) + .execute(state.database().expect("database").pool()) + .await + .expect("season association"); + series_id + } + + async fn verdict_of(url: String) -> serde_json::Value { + let releases: Vec = reqwest::get(url) + .await + .expect("releases") + .json() + .await + .expect("releases json"); + releases[0]["verdict"].clone() + } + + /// #241: `PUT /api/roots/{id}` writing a different `policy_id` + /// re-derives the stored verdicts of every title under the root (§5.1) + /// — and of nothing outside it. + #[tokio::test] + async fn repointing_a_root_at_another_policy_rederives_its_library() { + let (_dir, state, base) = application().await; + let pool = state.database().expect("database").pool(); + let client = reqwest::Client::new(); + let movie_id = movie_with_eligible_release(&state, &base).await; + let tv_main: i64 = + sqlx::query_scalar("SELECT id FROM roots WHERE kind = 'tv' AND audience = 'main'") + .fetch_one(pool) + .await + .expect("tv root"); + let series_id = series_with_eligible_pack(&state, &base, tv_main).await; + + // Point the movie main root at the kids policy; the path stays, so + // nothing on disk is touched. + let kids_policy = policy_id_named(&base, "Movies — kids").await; + let response = client + .put(format!("{base}/api/roots/1")) + .json(&serde_json::json!({ + "kind": "movie", "audience": "main", + "path": "/mnt/media/movies/main", "policy_id": kids_policy + })) + .send() + .await + .expect("repoint movie root"); + assert_eq!(response.status(), StatusCode::OK); + + assert_eq!( + verdict_of(format!("{base}/api/movies/{movie_id}/releases")).await, + "waived", + "kids requires pt-PT, English is a soft fail" + ); + // The TV root was not repointed, so its library is untouched. + assert_eq!( + verdict_of(format!("{base}/api/series/{series_id}/seasons/9/releases")).await, + "eligible" + ); + + // Now the TV root; the series' pack re-derives too. + let tv_kids_policy = policy_id_named(&base, "TV — kids").await; + let response = client + .put(format!("{base}/api/roots/{tv_main}")) + .json(&serde_json::json!({ + "kind": "tv", "audience": "main", + "path": "/mnt/media/tv/main", "policy_id": tv_kids_policy + })) + .send() + .await + .expect("repoint tv root"); + assert_eq!( + response.status(), + StatusCode::OK, + "{:?}", + response.text().await + ); + + assert_eq!( + verdict_of(format!("{base}/api/series/{series_id}/seasons/9/releases")).await, + "waived", + "kids requires pt-PT, English is a soft fail" + ); + } + #[tokio::test] async fn a_root_round_trips_through_create_and_update() { let (_dir, _state, base) = application().await; diff --git a/crates/arr-api/src/series.rs b/crates/arr-api/src/series.rs index 6ee9020..de95833 100644 --- a/crates/arr-api/src/series.rs +++ b/crates/arr-api/src/series.rs @@ -627,7 +627,10 @@ pub async fn update( { crate::relocate::refresh_jellyfin(&state).await; } - if overrides_changed { + // A root change swaps the effective policy the same way an overrides + // edit does (§5.1), so both re-derive; `relocation` is `Some` exactly + // when the root changed. + if overrides_changed || relocation.is_some() { crate::reclassify::series(&state, id).await?; } Ok(Json(load_series(&state, id).await?)) @@ -2043,6 +2046,70 @@ mod tests { assert_eq!(releases[0]["rejected_rule"], "size"); } + /// #241: moving a series to a root with a different policy re-derives + /// the stored verdicts of its whole deck (§5.1), season packs included. + #[tokio::test] + async fn moving_a_series_to_another_root_rederives_its_verdicts() { + 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 episodes: Vec = (1..=10) + .map(|number| { + serde_json::json!({ + "number": number, "title": format!("Episode {number}"), + "air_date": "2025-01-01" + }) + }) + .collect(); + let season = add_season(&base, series_id, 9, serde_json::json!(episodes)).await; + let season_id = season["id"].as_i64().expect("season id"); + + // Ten episodes in 15 GiB: 1.5 GiB each, inside the 1080p band. + let pool = state.database().expect("database").pool(); + let name = "Bluey S09 1080p WEB-DL ENGLISH x264-GROUP"; + let parsed = arr_parse::parse(name); + let size = 15_i64 * (1 << 30); + let release_id = sqlx::query_scalar::<_, i64>( + "INSERT INTO releases (indexer_id, guid, name, size, seeders, download_url, parsed, score, verdict) + VALUES (7, 'pack', ?, ?, 50, 'url', ?, 0, 'eligible') RETURNING id", + ) + .bind(name) + .bind(size) + .bind(serde_json::to_string(&parsed).expect("parsed json")) + .fetch_one(pool) + .await + .expect("release"); + sqlx::query("INSERT INTO season_releases (season_id, release_id) VALUES (?, ?)") + .bind(season_id) + .bind(release_id) + .execute(pool) + .await + .expect("association"); + + let response = reqwest::Client::new() + .patch(format!("{base}/api/series/{series_id}")) + .json(&serde_json::json!({"root_id": kids_root})) + .send() + .await + .expect("move root"); + assert_eq!(response.status(), StatusCode::OK); + + let releases: Vec = + reqwest::get(format!("{base}/api/series/{series_id}/seasons/9/releases")) + .await + .expect("releases") + .json() + .await + .expect("releases json"); + assert_eq!(releases.len(), 1); + assert_eq!( + releases[0]["verdict"], "waived", + "kids requires pt-PT, English is a soft fail: {releases:?}" + ); + } + #[tokio::test] async fn overrides_reject_a_key_the_policy_engine_has_no_rule_for() { let (_dir, state, base) = application().await;