Compare commits

...

8 Commits

Author SHA1 Message Date
Miguel Palhas c78c73ec6c docs: reconcile design coherence issues in §5.7 and §9.5
§9.5 restated the pre-#226 decision-bar rule without the 30-day window,
giving the document two versions of the same rule. Make it defer to
§5.7 instead.

§5.7 used two phrasings for the season-queuing predicate in one
paragraph (existence-of-file vs. state check) though both readers
implement the state check; picked the state phrasing throughout. Also
fixed the arithmetically confusing "seen twice ... third face"
sentence, and rewrapped the single unwrapped ~450-character line in
§7.4 to the document's ~78-column width.

Ref #240. Ran full `just ci` through the gate (517 tests, exit 0).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-25 11:49:43 +01:00
Miguel Palhas 58a45fc98e Merge #211: let a waiver name the rule it relaxed
Closes #211
2026-08-25 11:47:25 +01:00
Miguel Palhas a0dc07f085 feat(db): let a waiver name the rule it relaxed
`releases` forbade a rule name on anything but a rejection, so §9.3's
deck showed a bare `waived` beside rejections that each named their own,
and §5.7's "watchable but not what was asked" lost the half that says
what was not asked for. Since #210 that is the ordinary outcome of
waiving a size rejection, not a rare one.

0032 rebuilds the table with `CHECK (verdict != 'rejected' OR
rejected_rule IS NOT NULL)`, and the daemon and arr-api's
reclassification both store the waived rule. Existing rows keep NULL and
read as they do today.

`releases` is a parent — `grabs`, `movie_releases`, `episode_releases`
and `season_releases` point at it, three ON DELETE CASCADE — so the
rebuild runs `-- no-transaction` with foreign keys off around one
explicit transaction, per SQLite's own procedure. Verified against a
real database: the pre-0032 binary created and populated it, this build
migrated a copy, and every release row, child row and created_at came
through byte-identical with `PRAGMA foreign_key_check` clean.

Refs #211
2026-08-25 11:44:05 +01:00
Miguel Palhas de6c35cce3 Merge #232: plainer words for grab, blocked, deck
Closes #232
2026-08-25 11:43:55 +01:00
Miguel Palhas 9a413b10ed Merge #246: re-derive verdicts on a policy edit
Closes #246
2026-08-25 11:42:10 +01:00
Miguel Palhas bbb6d2f4a4 feat(api): re-derive verdicts on a policy edit
PUT /api/policies/{id} changed the rule every title under every root
pointing at the policy is judged by, and re-derived nothing, so §9.3's
deck and the daemon's grab gate kept reading verdicts computed under
rules that no longer existed.

Drives #241's walker from a policy id: root by root through
reclassify::root, so the skip rules and the leave-unchanged-rows-alone
rule stay in one place. A rename touches no rule and walks nothing.

Inline still holds at this width. Measured on a release build over 2000
titles and 10 000 stored releases across two roots sharing one policy:
0.36 s when no verdict moves, 2.7 s when all 10 000 do. DESIGN.md §5.1
now names four actions and carries those numbers.

just ci passed through the gate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 11:40:32 +01:00
Miguel Palhas c454b3ec11 Merge #244: normalise stored root paths
Closes #244
2026-08-25 11:29:13 +01:00
Miguel Palhas c4d4ade4da fix(api): normalise stored root paths
#243 normalised the incoming path but compared it against the value read
raw from the database, so a root stored with a trailing separator never
compared equal. Every edit of it -- a policy change included -- took the
relocation branch, where each planned destination is its own source and
the pre-check refuses. That root could not be edited at all.

`update` now normalises both sides, and hands `relocate_root` the
normalised stored value. `path_is_free` normalises the stored side in SQL
and `create` goes through it too, so `/mnt/x` and `/mnt/x/` cannot be two
roots for one directory -- the unique index compares raw strings and
cannot see that.

Migration 0031 strips the separator from rows already written. It skips
any row whose stripped form another row would also hold, rather than
tripping the unique index: a migration that cannot apply stops the daemon
booting, which is worse than two roots naming one directory.

Also from the same review: `undo` recorded only the leaf directory, so a
failed move into `/mnt/media-v2/tv/kids` left `tv` behind. It now records
every level `create_dir_all` materialised, deepest first, and still never
touches one that was already on disk.

Refs #244.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-25 11:28:35 +01:00
14 changed files with 1195 additions and 74 deletions
@@ -0,0 +1,98 @@
{
"db_name": "SQLite",
"query": "SELECT required_audio AS \"required_audio!: String\",\n dub_blacklist AS \"dub_blacklist!: String\",\n hdr_rules AS \"hdr_rules!: String\",\n size_bands AS \"size_bands!: String\",\n resolution_pref AS \"resolution_pref!: String\",\n source_weights AS \"source_weights!: String\",\n score_weights AS \"score_weights!: String\"\n FROM policies WHERE id = ?",
"describe": {
"columns": [
{
"name": "required_audio!: String",
"ordinal": 0,
"type_info": "Text",
"origin": {
"Table": {
"table": "policies",
"name": "required_audio"
}
}
},
{
"name": "dub_blacklist!: String",
"ordinal": 1,
"type_info": "Text",
"origin": {
"Table": {
"table": "policies",
"name": "dub_blacklist"
}
}
},
{
"name": "hdr_rules!: String",
"ordinal": 2,
"type_info": "Text",
"origin": {
"Table": {
"table": "policies",
"name": "hdr_rules"
}
}
},
{
"name": "size_bands!: String",
"ordinal": 3,
"type_info": "Text",
"origin": {
"Table": {
"table": "policies",
"name": "size_bands"
}
}
},
{
"name": "resolution_pref!: String",
"ordinal": 4,
"type_info": "Text",
"origin": {
"Table": {
"table": "policies",
"name": "resolution_pref"
}
}
},
{
"name": "source_weights!: String",
"ordinal": 5,
"type_info": "Text",
"origin": {
"Table": {
"table": "policies",
"name": "source_weights"
}
}
},
{
"name": "score_weights!: String",
"ordinal": 6,
"type_info": "Text",
"origin": {
"Table": {
"table": "policies",
"name": "score_weights"
}
}
}
],
"parameters": {
"Right": 1
},
"nullable": [
false,
false,
false,
false,
false,
false,
false
]
},
"hash": "100e5e9297a9a917eb107c673ac492efe0df0123d9def212d84b88e57aaa0484"
}
@@ -1,10 +1,10 @@
{
"db_name": "SQLite",
"query": "SELECT id FROM roots WHERE path = ? AND id <> ?",
"query": "SELECT id AS \"id!: i64\" FROM roots WHERE policy_id = ?",
"describe": {
"columns": [
{
"name": "id",
"name": "id!: i64",
"ordinal": 0,
"type_info": "Integer",
"origin": {
@@ -16,11 +16,11 @@
}
],
"parameters": {
"Right": 2
"Right": 1
},
"nullable": [
false
]
},
"hash": "fa3d1e4a6cae94780daf8fe20062a107963ba6ab2bcef2c8cfb9a1efbd905b59"
"hash": "a28caf97dfb6a7e6b69543df0693e15c746702c36a0d4068fa70d28b950e7562"
}
@@ -0,0 +1,26 @@
{
"db_name": "SQLite",
"query": "SELECT id AS \"id!: i64\" FROM roots\n WHERE CASE WHEN rtrim(path, '/') = '' THEN '/' ELSE rtrim(path, '/') END = ?\n AND (? IS NULL OR id <> ?)",
"describe": {
"columns": [
{
"name": "id!: i64",
"ordinal": 0,
"type_info": "Integer",
"origin": {
"Table": {
"table": "roots",
"name": "id"
}
}
}
],
"parameters": {
"Right": 3
},
"nullable": [
false
]
},
"hash": "bda8991590c009ca7084fed36fae088f1b6ae12ad95f972724a417711ad37fa1"
}
+30 -16
View File
@@ -201,14 +201,23 @@ 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
manual-grab gate read that stored column. So the four 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.
root with a different policy, pointing a root at a different policy, and
editing the contents of a policy some root points at — 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.
The fourth is the widest: a repoint moves one library, a policy edit moves
every library sharing the policy. Inline still holds there. Re-evaluation is
pure and in-memory, a row whose verdict does not move is not rewritten, and
the ceiling is the whole database rather than something that grows with the
number of roots — roots partition titles, and a title has exactly one root.
Measured over 2000 titles and 10 000 stored releases split across two roots
sharing one policy: 0.36 s when the edit moves no verdict, 2.7 s in the
pathological case where it moves all 10 000. A rename changes no rule and
re-derives nothing.
### 5.2 Language
@@ -413,13 +422,13 @@ guard retries at worst weekly (§6.2) — and stays queued for exactly as long a
it is genuinely broken. Nothing is dismissed by hand and no acknowledgement
state is stored, so there is no second thing to keep correct.
**Only a target still waiting for a file is queued.** The count and the window
are one rule seen twice — the queue is meant to be visited and emptied — and
this is its third face: a failure history queues a target only while that
target still has a gap to fill. A movie or an episode is queued while it is
`wanted` and not `available`. A season holds no intent of its own (§4.1), so it
is queued while at least one of its episodes is still wanted and still without
a file. A season pack that hard-failed twice, fell back to per-episode grabbing
**Only a target still waiting for a file is queued.** The count and the
window already express one rule — bounded attention — and this is another
face of it: a failure history queues a target only while that target still
has a gap to fill. A movie or an episode is queued while it is `wanted` and
not `available`. A season holds no intent of its own (§4.1), so it is queued
while at least one of its episodes is still `wanted` and not `available`. A
season pack that hard-failed twice, fell back to per-episode grabbing
exactly as §6.2 says it should, and was then fully acquired leaves at once
rather than waiting out the 30 days — that is the system working, not a
decision. A target that is still broken keeps producing failures and stays.
@@ -539,7 +548,12 @@ 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. Changing a root's path is the same move over every title under it, and it is all or nothing: one folder that cannot move puts back the ones that already did and leaves the root's path alone, so the stored path always describes the disk.
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.
Changing a root's path is the same move over every title under it, and it
is all or nothing: one folder that cannot move puts back the ones that
already did and leaves the root's path alone, so the stored path always
describes the disk.
During transition, write into the existing roots so Jellyfin needs no
reconfiguration and new content appears immediately. Radarr will not touch a
@@ -649,7 +663,7 @@ notifying on everything and being muted within a week.
- **Imported** → to the title's owners. The only good-news notification.
- **Needs a decision** → to the operator alone. Entered the no-PT-source queue,
or hard-failed twice on different releases.
or the needs-a-decision queue (§5.7).
- **Broken** → to the operator alone. Prowlarr, Transmission or TMDB
unreachable, disk full.
+10
View File
@@ -76,6 +76,12 @@ pub struct Release {
pub parsed: serde_json::Value,
pub score: Option<f64>,
pub verdict: Option<String>,
// The rule behind the verdict: the one that killed a `rejected` row, or
// the one a `waived` row relaxed (#211). Null on an `eligible` row, and
// on a `waived` row stored before migration 0032, which could not record
// it. A plain comment, not a doc comment: doc comments here become
// OpenAPI descriptions and would put the generated client in web/ out of
// date, which #227 and #232 own.
pub rejected_rule: Option<String>,
}
@@ -1303,6 +1309,10 @@ mod tests {
.expect("releases json");
assert_eq!(releases.len(), 1);
assert_eq!(releases[0]["verdict"], "waived");
// #211: and it says which rule the waiver relaxed, rather than
// sitting in the deck as a bare `waived` beside rejections that each
// name their own.
assert_eq!(releases[0]["rejected_rule"], "size");
}
/// #241: moving a title to a root with a different policy re-derives its
+332 -9
View File
@@ -173,6 +173,33 @@ struct PolicyColumns {
score_weights: String,
}
/// Everything in a policy row that a verdict depends on — the name is the
/// one column that does not.
#[derive(PartialEq, Eq)]
struct PolicyRules {
required_audio: String,
dub_blacklist: String,
hdr_rules: String,
size_bands: String,
resolution_pref: String,
source_weights: String,
score_weights: String,
}
impl PolicyColumns {
fn rules(&self) -> PolicyRules {
PolicyRules {
required_audio: self.required_audio.clone(),
dub_blacklist: self.dub_blacklist.clone(),
hdr_rules: self.hdr_rules.clone(),
size_bands: self.size_bands.clone(),
resolution_pref: self.resolution_pref.clone(),
source_weights: self.source_weights.clone(),
score_weights: self.score_weights.clone(),
}
}
}
fn column<T: serde::de::DeserializeOwned>(
column: &'static str,
value: &str,
@@ -376,6 +403,8 @@ pub async fn update(
input.validate().map_err(ApiError::Invalid)?;
let name = input.name.trim().to_owned();
let columns = input.into_columns(name)?;
let after = columns.rules();
let before = stored_rules(&state, id).await?;
let result = sqlx::query!(
r#"UPDATE policies SET
name = ?, required_audio = ?, dub_blacklist = ?, hdr_rules = ?,
@@ -405,9 +434,44 @@ pub async fn update(
if result.rows_affected() == 0 {
return Err(ApiError::PolicyNotFound);
}
// Every verdict stored under every root pointing here was reached under
// the rules this write just replaced; §9.3's deck and the grab gate both
// read them (`reclassify`). A rename leaves the rules alone, so it walks
// nothing.
if before.is_none_or(|before| before != after) {
crate::reclassify::policy(&state, id).await?;
}
Ok(Json(load_policy(&state, id).await?))
}
/// The rule columns of a policy as they stand, or `None` when there is no
/// such row. Compared against what the write is about to store, so an edit
/// that only moves the name does not re-derive a library.
async fn stored_rules(state: &AppState, id: i64) -> Result<Option<PolicyRules>, ApiError> {
let row = sqlx::query!(
r#"SELECT required_audio AS "required_audio!: String",
dub_blacklist AS "dub_blacklist!: String",
hdr_rules AS "hdr_rules!: String",
size_bands AS "size_bands!: String",
resolution_pref AS "resolution_pref!: String",
source_weights AS "source_weights!: String",
score_weights AS "score_weights!: String"
FROM policies WHERE id = ?"#,
id
)
.fetch_optional(pool(state)?)
.await?;
Ok(row.map(|row| PolicyRules {
required_audio: row.required_audio,
dub_blacklist: row.dub_blacklist,
hdr_rules: row.hdr_rules,
size_bands: row.size_bands,
resolution_pref: row.resolution_pref,
source_weights: row.source_weights,
score_weights: row.score_weights,
}))
}
#[utoipa::path(
delete, path = "/api/policies/{policy_id}", tag = "policies",
params(("policy_id" = i64, Path, description = "Policy row id")),
@@ -450,7 +514,7 @@ mod tests {
use crate::{router, Upstreams};
use axum::http::StatusCode;
async fn application() -> (tempfile::TempDir, String) {
async fn application() -> (tempfile::TempDir, AppState, String) {
let dir = tempfile::tempdir().expect("tempdir");
let database = arr_db::Db::connect(dir.path().join("arr.db"))
.await
@@ -466,9 +530,9 @@ mod tests {
.await
.expect("bind");
let address = listener.local_addr().expect("address");
let app = router(state);
let app = router(state.clone());
tokio::spawn(async move { axum::serve(listener, app).await.expect("serve") });
(dir, format!("http://{address}"))
(dir, state, format!("http://{address}"))
}
fn valid_input(name: &str) -> serde_json::Value {
@@ -501,7 +565,7 @@ mod tests {
#[tokio::test]
async fn crud_round_trips_a_policy() {
let (_dir, base) = application().await;
let (_dir, _state, base) = application().await;
let created: serde_json::Value = create(&base, valid_input("test policy"))
.await
@@ -557,7 +621,7 @@ mod tests {
#[tokio::test]
async fn a_referenced_policy_refuses_to_die() {
let (_dir, base) = application().await;
let (_dir, _state, base) = application().await;
let roots: Vec<serde_json::Value> = reqwest::get(format!("{base}/api/roots"))
.await
.expect("roots")
@@ -582,7 +646,7 @@ mod tests {
#[tokio::test]
async fn an_unknown_resolution_is_a_422_naming_the_field() {
let (_dir, base) = application().await;
let (_dir, _state, base) = application().await;
let mut payload = valid_input("bad bands");
payload["size_bands"]["1440p"] = serde_json::json!({ "floor_gib": 2, "target_gib": 6, "penalty_points_per_gib_over": 60 });
@@ -596,7 +660,7 @@ mod tests {
#[tokio::test]
async fn every_field_validates_by_name() {
let (_dir, base) = application().await;
let (_dir, _state, base) = application().await;
let with = |patch: &dyn Fn(&mut serde_json::Value)| {
let mut payload = valid_input("validation probe");
patch(&mut payload);
@@ -645,7 +709,7 @@ mod tests {
#[tokio::test]
async fn malformed_json_is_422_not_400_or_500() {
let (_dir, base) = application().await;
let (_dir, _state, base) = application().await;
let client = reqwest::Client::new();
let response = client
.post(format!("{base}/api/policies"))
@@ -667,8 +731,267 @@ mod tests {
#[tokio::test]
async fn a_duplicate_name_conflicts() {
let (_dir, base) = application().await;
let (_dir, _state, base) = application().await;
let response = create(&base, valid_input("Movies — main")).await;
assert_eq!(response.status(), StatusCode::CONFLICT);
}
async fn policy_id_named(base: &str, name: &str) -> i64 {
let policies: Vec<serde_json::Value> = 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")
}
/// A movie under `root_id`, with one English 1080p release stamped
/// `verdict` as a search would have stamped it.
async fn movie_with_release(
state: &AppState,
base: &str,
root_id: i64,
tmdb_id: i64,
verdict: &str,
) -> i64 {
let movie: serde_json::Value = reqwest::Client::new()
.post(format!("{base}/api/movies"))
.json(&serde_json::json!({
"tmdb_id": tmdb_id, "title": format!("Title {tmdb_id}"), "year": 2024,
"original_language": "en", "root_id": root_id, "overrides": {}
}))
.send()
.await
.expect("create movie")
.json()
.await
.expect("movie json");
let movie_id = movie["id"].as_i64().expect("movie id");
let name = format!("Title {tmdb_id} 2024 1080p WEB-DL ENGLISH x264-GROUP");
let release_id = stamped_release(state, &name, 6_i64 * (1 << 30), verdict).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
}
/// One release row, verdict stamped by hand. `waived` carries no rule,
/// which the `releases` CHECK allows — only `rejected` needs one.
async fn stamped_release(state: &AppState, name: &str, size: i64, verdict: &str) -> 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, ?) RETURNING id",
)
.bind(name)
.bind(name)
.bind(size)
.bind(serde_json::to_string(&parsed).expect("parsed json"))
.bind(verdict)
.fetch_one(state.database().expect("database").pool())
.await
.expect("release")
}
/// A series under `root_id` with a ten-episode season 9 pack stamped
/// `waived`. Under its own policy the pack is eligible, so the stamp
/// only survives if nothing re-derived it.
async fn series_with_waived_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<serde_json::Value> = (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");
// Ten episodes in 15 GiB: 1.5 GiB each, inside the 1080p band.
let release_id = stamped_release(
state,
"Bluey S09 1080p WEB-DL ENGLISH x264-GROUP",
15_i64 * (1 << 30),
"waived",
)
.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_at(url: String) -> serde_json::Value {
let releases: Vec<serde_json::Value> = reqwest::get(url)
.await
.expect("releases")
.json()
.await
.expect("releases json");
releases[0]["verdict"].clone()
}
async fn movie_verdict(base: &str, movie_id: i64) -> serde_json::Value {
verdict_at(format!("{base}/api/movies/{movie_id}/releases")).await
}
async fn root_id_of(state: &AppState, kind: &str, audience: &str) -> i64 {
sqlx::query_scalar("SELECT id FROM roots WHERE kind = ? AND audience = ?")
.bind(kind)
.bind(audience)
.fetch_one(state.database().expect("database").pool())
.await
.expect("root id")
}
/// The same policy document with a different required-audio rule, so an
/// English release that was eligible becomes a waiver.
fn requires_portuguese(name: &str) -> serde_json::Value {
let mut payload = valid_input(name);
payload["required_audio"] = serde_json::json!({ "require": "any_of", "langs": ["pt-PT"] });
payload
}
/// #246: editing a policy re-derives the stored verdicts of every title
/// under every root pointing at it (§5.1) — both libraries when two
/// roots share it, and nothing under a root that does not.
#[tokio::test]
async fn editing_a_policy_rederives_every_root_that_points_at_it() {
let (_dir, state, base) = application().await;
let client = reqwest::Client::new();
let shared = policy_id_named(&base, "Movies — main").await;
let main_root = root_id_of(&state, "movie", "main").await;
let kids_root = root_id_of(&state, "movie", "kids").await;
// Two roots on one policy: the edit has two libraries to reach
// rather than one. The path stays, so nothing on disk moves.
let response = client
.put(format!("{base}/api/roots/{kids_root}"))
.json(&serde_json::json!({
"kind": "movie", "audience": "kids",
"path": "/mnt/media/movies/kids", "policy_id": shared,
}))
.send()
.await
.expect("repoint kids root");
assert_eq!(
response.status(),
StatusCode::OK,
"{:?}",
response.text().await
);
let here = movie_with_release(&state, &base, main_root, 693_134, "eligible").await;
let there = movie_with_release(&state, &base, kids_root, 27_205, "eligible").await;
// On the TV main root, a different policy, so the edit must not
// reach it. Stamped against its own policy's answer, so a walk that
// did reach it would show.
let tv_root = root_id_of(&state, "tv", "main").await;
let series_id = series_with_waived_pack(&state, &base, tv_root).await;
let response = client
.put(format!("{base}/api/policies/{shared}"))
.json(&requires_portuguese("Movies — main"))
.send()
.await
.expect("edit policy");
assert_eq!(
response.status(),
StatusCode::OK,
"{:?}",
response.text().await
);
assert_eq!(
movie_verdict(&base, here).await,
"waived",
"the first root's library re-derives"
);
assert_eq!(
movie_verdict(&base, there).await,
"waived",
"and so does the second root's, sharing the policy"
);
assert_eq!(
verdict_at(format!("{base}/api/series/{series_id}/seasons/9/releases")).await,
"waived",
"a root on another policy keeps the verdict it was stamped with"
);
}
/// Renaming a policy changes no rule, so it re-derives nothing — the
/// stamped verdict survives even though the rules would not produce it.
#[tokio::test]
async fn renaming_a_policy_leaves_verdicts_alone() {
let (_dir, state, base) = application().await;
let client = reqwest::Client::new();
let kids = policy_id_named(&base, "Movies — kids").await;
let kids_root = root_id_of(&state, "movie", "kids").await;
// Write the rules through the API once, so the rename that follows
// stores byte-identical rule columns and the only change is the name.
let response = client
.put(format!("{base}/api/policies/{kids}"))
.json(&valid_input("Movies — kids"))
.send()
.await
.expect("normalise policy");
assert_eq!(response.status(), StatusCode::OK);
// Deliberately the wrong answer: these rules make an English release
// a waiver, so a re-derivation would move this row.
let movie = movie_with_release(&state, &base, kids_root, 157_336, "eligible").await;
let response = client
.put(format!("{base}/api/policies/{kids}"))
.json(&valid_input("Movies — children"))
.send()
.await
.expect("rename policy");
assert_eq!(
response.status(),
StatusCode::OK,
"{:?}",
response.text().await
);
assert_eq!(
movie_verdict(&base, movie).await,
"eligible",
"a rename touches no rule, so it walks nothing"
);
}
}
+33 -7
View File
@@ -1,6 +1,7 @@
//! 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).
//! an overrides edit (§9.3), a move to a root with a different policy, a
//! root pointed at a different policy, or an edit to the contents of a
//! policy some root points at (§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
@@ -9,7 +10,8 @@
//! 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.
//! only place the correction can happen. A policy edit invalidates it wider
//! still: every root pointing at that policy, not just one.
//!
//! 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
@@ -176,6 +178,30 @@ pub(crate) async fn root(state: &AppState, root_id: i64) -> Result<(), ApiError>
Ok(())
}
/// Re-evaluate every release of every title under every root that points at
/// one policy, for a `PUT /api/policies/{id}` that changed its rules (§5.1).
///
/// Root by root through [`root`], which is title by title through [`movie`]
/// and [`series`]: one walker, one set of skip rules, one place that decides
/// a row does not need rewriting.
///
/// This is the widest of the four re-derivations — a root repoint moves one
/// library, a policy edit moves every library sharing the policy — but the
/// ceiling is the whole database rather than something that grows with it,
/// since roots partition titles and a title has exactly one root.
pub(crate) async fn policy(state: &AppState, policy_id: i64) -> Result<(), ApiError> {
let root_ids = sqlx::query_scalar!(
r#"SELECT id AS "id!: i64" FROM roots WHERE policy_id = ?"#,
policy_id
)
.fetch_all(pool(state)?)
.await?;
for root_id in root_ids {
root(state, root_id).await?;
}
Ok(())
}
#[allow(clippy::too_many_arguments)]
async fn apply(
state: &AppState,
@@ -208,11 +234,11 @@ async fn apply(
episodes,
runtime_minutes,
);
// `releases` allows a rule name only on a rejected row
// (`CHECK ((verdict = 'rejected') = (rejected_rule IS NOT NULL))`),
// which is also how the daemon writes a waiver.
// A waiver records the rule it relaxed, the same as a rejection
// (#211). Migration 0032 relaxed `releases` to
// `CHECK (verdict != 'rejected' OR rejected_rule IS NOT NULL)` so it
// can, and the daemon writes waivers the same way.
let (verdict, rule) = verdict_columns(&evaluation.verdict);
let rule = if verdict == "rejected" { rule } else { None };
if release.verdict.as_deref() == Some(verdict) && release.rejected_rule == rule {
continue;
}
+46 -25
View File
@@ -50,11 +50,14 @@ struct PlannedRename {
pub(crate) struct Relocation {
performed: Vec<PlannedRename>,
rewrites: Vec<(i64, String)>,
/// The new root directory, set only when this request created it (it did
/// not already exist). [`Self::undo`] removes it, so it is cleaned up
/// exactly when the move it was created for does not complete; a move
/// that commits never calls `undo` and the directory stays.
created_root: Option<PathBuf>,
/// Every directory level this request materialised for the new root,
/// deepest first. `create_dir_all` can make more than one — moving a
/// root to `/mnt/media-v2/tv/kids` when `/mnt/media-v2` is all that
/// exists creates both `tv` and `kids` — and [`Self::undo`] removes all
/// of them, so a failed move leaves nothing behind (#244). Levels that
/// were already on disk are never in this list and are never touched. A
/// move that commits never calls `undo` and the directories stay.
created_dirs: Vec<PathBuf>,
}
/// Whether the destination root is a directory that must already be there.
@@ -179,20 +182,18 @@ async fn relocate_files(
}
}
// Tracked only when this call is the one that created the directory, so
// a failed move can remove it again without ever touching a root path
// that already existed on disk.
let mut created_root: Option<PathBuf> = None;
// Tracked only for the levels this call is the one to create, so a
// failed move can remove them again without ever touching a directory
// that already existed on disk. Recorded before `create_dir_all`, since
// afterwards there is no way to tell which levels it made.
let mut created_dirs: Vec<PathBuf> = Vec::new();
if destination == Destination::Create && !renames.is_empty() {
let already_there = tokio::fs::symlink_metadata(new_root).await.is_ok();
created_dirs = missing_levels(std::path::Path::new(new_root)).await;
if let Err(error) = tokio::fs::create_dir_all(new_root).await {
return Err(ApiError::Filesystem(format!(
"could not create '{new_root}': {error}"
)));
}
if !already_there {
created_root = Some(PathBuf::from(new_root));
}
}
let mut performed: Vec<PlannedRename> = Vec::new();
@@ -209,7 +210,7 @@ async fn relocate_files(
);
continue;
}
Err(error) => return Err(failed(&rename, &error, performed, created_root).await),
Err(error) => return Err(failed(&rename, &error, performed, created_dirs).await),
}
match tokio::fs::rename(&rename.source, &rename.destination).await {
Ok(()) => {
@@ -220,31 +221,47 @@ async fn relocate_files(
);
performed.push(rename);
}
Err(error) => return Err(failed(&rename, &error, performed, created_root).await),
Err(error) => return Err(failed(&rename, &error, performed, created_dirs).await),
}
}
Ok(Relocation {
performed,
rewrites,
created_root,
created_dirs,
})
}
/// The levels of `path` that are not on disk, deepest first — exactly what a
/// following `create_dir_all` will materialise. The walk stops at the first
/// ancestor that exists, so nothing already there is ever listed.
async fn missing_levels(path: &std::path::Path) -> Vec<PathBuf> {
let mut missing = Vec::new();
for ancestor in path.ancestors() {
// `ancestors` ends in an empty path for a relative input; there is
// no level above that to create.
if ancestor.as_os_str().is_empty() || tokio::fs::symlink_metadata(ancestor).await.is_ok() {
break;
}
missing.push(ancestor.to_path_buf());
}
missing
}
/// One rename failed: move back everything that had already moved, remove
/// the new root if this request is the one that created it, and name the
/// folder that stopped the move so the operator knows which title to look at
/// before retrying.
/// every directory level this request created, and name the folder that
/// stopped the move so the operator knows which title to look at before
/// retrying.
async fn failed(
rename: &PlannedRename,
error: &std::io::Error,
performed: Vec<PlannedRename>,
created_root: Option<PathBuf>,
created_dirs: Vec<PathBuf>,
) -> ApiError {
Relocation {
performed,
rewrites: Vec::new(),
created_root,
created_dirs,
}
.undo()
.await;
@@ -297,13 +314,17 @@ impl Relocation {
// `remove_dir` rather than `remove_dir_all`: it only succeeds on an
// empty directory, so anything unexpected left inside it — this
// request's own undo failing, say — is a reason to leave it alone.
if let Some(root) = &self.created_root {
if let Err(error) = tokio::fs::remove_dir(root).await {
// Deepest first, since a parent cannot go while its child is there;
// the first level that will not go stops the walk, because every
// level above it now has content and refusing is the right answer.
for directory in &self.created_dirs {
if let Err(error) = tokio::fs::remove_dir(directory).await {
tracing::warn!(
path = %root.display(),
path = %directory.display(),
%error,
"could not remove the directory created for a move that did not complete"
"could not remove a directory created for a move that did not complete"
);
break;
}
}
}
+228 -9
View File
@@ -152,6 +152,7 @@ pub async fn create(
input.validate().map_err(ApiError::Invalid)?;
input.policy_exists(&state).await?;
let path = normalize_path(&input.path);
path_is_free(&state, None, &path).await?;
let result = sqlx::query!(
"INSERT INTO roots (kind, audience, path, policy_id) VALUES (?, ?, ?, ?)",
input.kind,
@@ -188,17 +189,25 @@ pub async fn update(
input.policy_exists(&state).await?;
let current = load_root(&state, id).await?;
let path = normalize_path(&input.path);
// #244: rows written before #243 can hold a trailing separator, so the
// stored value is normalised too. Comparing a normalised payload against
// a raw stored value means the row can never compare equal: every edit,
// policy changes included, takes the relocation branch, and there each
// planned destination is its own source. The normalised value is what
// `relocate_root` gets as well, so no planned path carries a doubled
// separator.
let current_path = normalize_path(&current.path);
// A path change moves every §7.4 title folder under this root with the
// row (issue #236), the same way changing a title's root moves one
// (#228). Disk first, row second: a failed rename leaves the root row
// alone, so the operator sees the library where its files actually are
// and can retry. A path already taken is refused before any of it, since
// the write would fail afterwards anyway.
let relocation = if path == current.path {
let relocation = if path == current_path {
None
} else {
path_is_free(&state, id, &path).await?;
Some(crate::relocate::relocate_root(&state, id, &current.path, &path).await?)
path_is_free(&state, Some(id), &path).await?;
Some(crate::relocate::relocate_root(&state, id, &current_path, &path).await?)
};
let mut transaction = pool(&state)?.begin().await?;
let written: Result<(), sqlx::Error> = async {
@@ -245,12 +254,25 @@ pub async fn update(
}
/// The unique index on `path` would catch this after the move; catching it
/// first keeps a doomed write from touching the disk at all.
async fn path_is_free(state: &AppState, id: i64, path: &str) -> Result<(), ApiError> {
let taken: Option<i64> =
sqlx::query_scalar!("SELECT id FROM roots WHERE path = ? AND id <> ?", path, id)
.fetch_optional(pool(state)?)
.await?;
/// first keeps a doomed write from touching the disk at all. `except` is the
/// row being updated, or `None` when creating.
///
/// The stored side is normalised in SQL, mirroring [`normalize_path`], so
/// `/mnt/media/x` and `/mnt/media/x/` cannot be two roots for one directory
/// (#244). The unique index cannot see that — it compares the raw strings —
/// and migration 0031 leaves any pair that already collides alone rather
/// than failing to apply, so such a row can still be on disk.
async fn path_is_free(state: &AppState, except: Option<i64>, path: &str) -> Result<(), ApiError> {
let taken: Option<i64> = sqlx::query_scalar!(
r#"SELECT id AS "id!: i64" FROM roots
WHERE CASE WHEN rtrim(path, '/') = '' THEN '/' ELSE rtrim(path, '/') END = ?
AND (? IS NULL OR id <> ?)"#,
path,
except,
except
)
.fetch_optional(pool(state)?)
.await?;
if taken.is_some() {
return Err(ApiError::Conflict(
"a root with this path already exists".into(),
@@ -749,6 +771,17 @@ mod tests {
assert_eq!(response.status(), StatusCode::NO_CONTENT);
}
/// A path written straight into the row, no normalisation — how a root
/// created before #243 could end up holding a trailing separator.
async fn point_root_at_raw(state: &AppState, root_id: i64, path: &str) {
sqlx::query("UPDATE roots SET path = ? WHERE id = ?")
.bind(path)
.bind(root_id)
.execute(state.database().expect("database").pool())
.await
.expect("store the raw path");
}
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"))
@@ -1257,6 +1290,192 @@ mod tests {
);
}
/// Issue #244: a root *stored* with a trailing separator — creatable
/// through the API at any point before #243 — could not be edited at
/// all. The payload was normalised and the stored value was not, so no
/// payload compared equal: every edit took the relocation branch, where
/// every planned destination is its own source and the pre-check 409s.
#[tokio::test]
async fn a_root_stored_with_a_trailing_separator_can_still_be_edited() {
let (_dir, state, base) = application().await;
let old = tempfile::tempdir().expect("old root");
let unslashed = old.path().to_str().expect("utf-8").to_owned();
point_root_at_raw(&state, 1, &format!("{unslashed}/")).await;
let id = add_movie(&base, 100, "Dune", 1).await;
let folder = library_folder(&state, id, old.path(), "Dune").await;
// What the settings view sends back: the path exactly as stored,
// separator included, with only the policy changed.
let kids_policy = policy_id_named(&base, "Movies — kids").await;
let mut payload = root_payload(&base, 1, &format!("{unslashed}/")).await;
payload["policy_id"] = kids_policy.into();
let response = reqwest::Client::new()
.put(format!("{base}/api/roots/1"))
.json(&payload)
.send()
.await
.expect("edit the root");
assert_eq!(
response.status(),
StatusCode::OK,
"{:?}",
response.text().await
);
assert!(folder.join("feature.mkv").exists(), "nothing on disk moved");
assert_eq!(
stored_path(&base, 1).await,
unslashed,
"the row is left normalised, so the next edit compares equal too"
);
// And the same payload without the separator is not a relocation
// either.
let response = reqwest::Client::new()
.put(format!("{base}/api/roots/1"))
.json(&root_payload(&base, 1, &unslashed).await)
.send()
.await
.expect("edit the root again");
assert_eq!(response.status(), StatusCode::OK);
assert!(folder.join("feature.mkv").exists(), "still nothing moved");
}
/// Issue #244: a real path change from a root stored with a trailing
/// separator plans from the normalised value, so neither a destination
/// nor a rewritten row carries a doubled separator.
#[tokio::test]
async fn relocating_a_slash_stored_root_plans_no_doubled_separator() {
let (_dir, state, base) = application().await;
let old = tempfile::tempdir().expect("old root");
let home = tempfile::tempdir().expect("home of the new path");
let new = home.path().join("relocated-main");
let unslashed = old.path().to_str().expect("utf-8").to_owned();
point_root_at_raw(&state, 1, &format!("{unslashed}/")).await;
let id = add_movie(&base, 100, "Dune", 1).await;
let folder = library_folder(&state, id, old.path(), "Dune").await;
let response = reqwest::Client::new()
.put(format!("{base}/api/roots/1"))
.json(&root_payload(&base, 1, new.to_str().expect("utf-8")).await)
.send()
.await
.expect("move the root");
assert_eq!(
response.status(),
StatusCode::OK,
"{:?}",
response.text().await
);
assert!(!folder.exists(), "the folder left the old path");
assert!(new.join("Dune").join("feature.mkv").exists());
let paths = file_paths(&state).await;
assert!(
!paths[0].contains("//"),
"no doubled separator in the rewritten row: {}",
paths[0]
);
assert!(
std::path::Path::new(&paths[0]).exists(),
"the rewritten path describes the disk: {}",
paths[0]
);
assert_eq!(stored_path(&base, 1).await, new.to_str().expect("utf-8"));
}
/// Issue #244: `create_dir_all` can materialise more than one level for
/// a root path pointed somewhere fresh. A failed move removes every
/// level it created, not only the leaf — and still nothing that was
/// already on disk.
#[cfg(unix)]
#[tokio::test]
async fn a_failed_move_removes_every_level_it_created() {
use std::os::unix::fs::PermissionsExt;
let (_dir, state, base) = application().await;
let old = tempfile::tempdir().expect("old root");
let home = tempfile::tempdir().expect("home of the new path");
// Three levels below a directory that is already there.
let top = home.path().join("media-v2");
let new = top.join("tv").join("kids");
point_root_at(&state, 1, old.path()).await;
let id = add_movie(&base, 100, "Dune", 1).await;
let stuck = library_folder(&state, id, old.path(), "Dune").await;
// Moving a directory to another parent rewrites its `..`, which
// needs write permission on the directory itself.
tokio::fs::set_permissions(&stuck, std::fs::Permissions::from_mode(0o555))
.await
.expect("freeze the title folder");
let response = reqwest::Client::new()
.put(format!("{base}/api/roots/1"))
.json(&root_payload(&base, 1, new.to_str().expect("utf-8")).await)
.send()
.await
.expect("move the root");
assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
tokio::fs::set_permissions(&stuck, std::fs::Permissions::from_mode(0o755))
.await
.expect("thaw the title folder");
assert!(
!top.exists(),
"every level the failed move created is gone, not only the leaf"
);
assert!(
home.path().exists(),
"the level that was already there is left alone"
);
assert!(
stuck.join("feature.mkv").exists(),
"the folder is still where the row says it is"
);
assert_eq!(
stored_path(&base, 1).await,
old.path().to_str().expect("utf-8")
);
}
/// Issue #244: two roots naming one directory, differing only by a
/// trailing separator, are not two roots. The unique index compares raw
/// strings and cannot see it, so the check normalises both sides.
#[tokio::test]
async fn a_slash_stored_path_is_not_free_for_another_root() {
let (_dir, state, base) = application().await;
point_root_at_raw(&state, 2, "/mnt/media/movies/archive/").await;
// An update onto the stripped form of a path another root holds.
let response = reqwest::Client::new()
.put(format!("{base}/api/roots/1"))
.json(&root_payload(&base, 1, "/mnt/media/movies/archive").await)
.send()
.await
.expect("update onto the other root's path");
assert_eq!(response.status(), StatusCode::CONFLICT);
// And a create, which the unique index would have let through.
let policy_ids = first_policy_ids(&base).await;
let mut payload = root_input(policy_ids[0]);
payload["kind"] = serde_json::json!("tv");
payload["audience"] = serde_json::json!("main");
payload["path"] = serde_json::json!("/mnt/media/movies/archive");
let deleted = reqwest::Client::new()
.delete(format!("{base}/api/roots/3"))
.send()
.await
.expect("free the (tv, main) pair");
assert_eq!(deleted.status(), StatusCode::NO_CONTENT);
let response = reqwest::Client::new()
.post(format!("{base}/api/roots"))
.json(&payload)
.send()
.await
.expect("create onto the other root's path");
assert_eq!(response.status(), StatusCode::CONFLICT);
}
/// Issue #236: a path another root already holds is refused before the
/// disk is touched at all.
#[tokio::test]
+4 -3
View File
@@ -2014,9 +2014,10 @@ mod tests {
releases[0]["verdict"], "waived",
"a waived grab stays a waiver; nothing here makes it eligible"
);
// The row's rule name goes with the rejection; what survives is the
// dashed `waived` verdict the deck reads (§9.3).
assert!(releases[0]["rejected_rule"].is_null());
// #211: the waiver keeps the rule it relaxed, so the deck names it
// the way it names a rejection (§9.3) instead of showing a bare
// `waived`.
assert_eq!(releases[0]["rejected_rule"], "size");
// The grab the deck's one click sends is now accepted.
let response = reqwest::Client::new()
+24 -1
View File
@@ -1534,10 +1534,17 @@ fn search_query(movie: &PendingMovie) -> String {
)
}
/// The `verdict` and `rejected_rule` columns for a verdict.
///
/// A waiver names the rule it relaxed (#211). §5.7 calls a soft fail
/// "watchable but not what was asked", and which rule was relaxed is the
/// whole content of that sentence, so §9.3's deck can name it the way it
/// names a rejection. Rows written before 0032 hold `NULL` there and stay
/// readable.
fn verdict_columns(verdict: &Verdict) -> (&'static str, Option<String>) {
match verdict {
Verdict::Eligible => ("eligible", None),
Verdict::Waived(_) => ("waived", None),
Verdict::Waived(rule) => ("waived", Some(rule.name())),
Verdict::Rejected(rule) => ("rejected", Some(rule.name())),
}
}
@@ -2029,6 +2036,22 @@ mod tests {
);
}
/// #211: a waiver names the rule it relaxed, the same as a rejection, so
/// §9.3's deck shows what was given up instead of a bare `waived`.
/// Migration 0032 relaxed the constraint that forbade it.
#[test]
fn a_waiver_records_the_rule_it_relaxed() {
assert_eq!(
verdict_columns(&Verdict::Waived(arr_core::Rule::Size)),
("waived", Some("size".to_owned()))
);
assert_eq!(verdict_columns(&Verdict::Eligible), ("eligible", None));
assert_eq!(
verdict_columns(&Verdict::Rejected(arr_core::Rule::RequiredAudio)),
("rejected", Some("required_audio".to_owned()))
);
}
/// Every candidate is cached with its verdict, which is what the manual
/// search view and the attention queues read (§9.3).
#[tokio::test]
@@ -0,0 +1,34 @@
-- Issue #244. A root's path could be stored with a trailing separator until
-- #243 normalised the incoming value. `roots::update` normalises the payload
-- and compares it against the stored value, so such a row never compares
-- equal: every edit, a policy change included, takes the relocation branch,
-- and there every planned destination is its own source. Normalising the
-- payload alone fixed the half that cannot bite; this is the other half.
--
-- `rtrim` strips every trailing separator at once, so '/mnt/x//' normalises
-- in one pass. A bare '/' rtrims to the empty string and is put back, which
-- is what `normalize_path` in arr-api does.
--
-- Guarded, because `roots.path` is UNIQUE and a migration that cannot apply
-- stops the daemon booting -- worse than the bug it fixes. A row is
-- normalised only when no other row shares its normalised path: neither a
-- row already holding the stripped value, nor another trailing-separator row
-- that would strip to the same thing. Every row in such a group is left
-- exactly as it is. That leaves two roots naming one directory, which is a
-- settings mistake for the operator to resolve by hand, not a reason to
-- refuse to boot.
--
-- This cannot introduce a collision either. An updated row's new value is a
-- normalised path no other row normalises to, and a row left alone whose raw
-- path equalled that value would have had the same normalised path, which is
-- the case the guard excludes.
UPDATE roots
SET path = CASE WHEN rtrim(path, '/') = '' THEN '/' ELSE rtrim(path, '/') END
WHERE path <> CASE WHEN rtrim(path, '/') = '' THEN '/' ELSE rtrim(path, '/') END
AND NOT EXISTS (
SELECT 1
FROM roots AS other
WHERE other.id <> roots.id
AND CASE WHEN rtrim(other.path, '/') = '' THEN '/' ELSE rtrim(other.path, '/') END
= CASE WHEN rtrim(roots.path, '/') = '' THEN '/' ELSE rtrim(roots.path, '/') END
);
@@ -0,0 +1,68 @@
-- no-transaction
-- #211: a waived release could not record which rule it relaxed. The old
-- constraint made the rule name an exact synonym for rejection:
--
-- CHECK ((verdict = 'rejected') = (rejected_rule IS NOT NULL))
--
-- so §9.3's deck showed a bare `waived` beside rejected rows that each named
-- their own rule, and §5.7's "watchable but not what was asked" lost the half
-- that says what was not asked for. The relaxed form still demands a rule on
-- a rejection and stops demanding its absence elsewhere.
--
-- SQLite cannot alter a CHECK, so the table is rebuilt (see 0014). Unlike the
-- rebuilds there, `releases` is a parent: `grabs`, `movie_releases`,
-- `episode_releases` and `season_releases` all point at it, three of them
-- ON DELETE CASCADE. Dropping the old table with foreign keys enforced would
-- delete those children (or, for `grabs`, refuse outright), so this follows
-- SQLite's own procedure — foreign keys off, the rebuild in one transaction,
-- foreign keys back on. `PRAGMA foreign_keys` is a no-op inside a
-- transaction, which is why the file opens `-- no-transaction` and manages
-- its own; the migration is still all-or-nothing.
--
-- Rows are copied verbatim. Every existing `waived` row keeps its NULL and
-- goes on reading as it does today; only rows written after this migration
-- carry a waived rule.
PRAGMA foreign_keys = OFF;
BEGIN;
CREATE TABLE releases_new (
id INTEGER PRIMARY KEY,
-- Prowlarr's indexer id. Not a foreign key: indexers live in Prowlarr.
indexer_id INTEGER NOT NULL,
guid TEXT NOT NULL,
name TEXT NOT NULL,
size INTEGER NOT NULL,
seeders INTEGER,
publish_date TEXT,
download_url TEXT NOT NULL,
-- §5.6. What the release name claims, before anything is downloaded.
parsed TEXT NOT NULL CHECK (json_valid(parsed)),
score REAL,
-- §9.3. Three buckets. `rejected_rule` names the rule that killed it so
-- an over-strict filter is visible without reading release names, and
-- §5.7's waiver names the rule it relaxed for the same reason.
verdict TEXT CHECK (verdict IN ('eligible', 'waived', 'rejected')),
rejected_rule TEXT,
created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
UNIQUE (indexer_id, guid),
CHECK (verdict != 'rejected' OR rejected_rule IS NOT NULL)
) STRICT;
INSERT INTO releases_new (
id, indexer_id, guid, name, size, seeders, publish_date, download_url,
parsed, score, verdict, rejected_rule, created_at
)
SELECT
id, indexer_id, guid, name, size, seeders, publish_date, download_url,
parsed, score, verdict, rejected_rule, created_at
FROM releases;
DROP TABLE releases;
ALTER TABLE releases_new RENAME TO releases;
CREATE INDEX releases_verdict ON releases (verdict, score);
COMMIT;
PRAGMA foreign_keys = ON;
+258
View File
@@ -347,6 +347,93 @@ mod tests {
assert_eq!(renamed_from_imported, "available");
}
/// The four seeded roots, up to but not including migration 0031, with
/// the given legacy paths written straight into the rows.
async fn roots_before_normalisation(paths: &[(i64, &str)]) -> (tempfile::TempDir, Db) {
let dir = tempfile::tempdir().expect("tempdir");
let db = Db::connect(dir.path().join("arr.db"))
.await
.expect("connect");
MIGRATOR
.run_to(30, db.pool())
.await
.expect("migrations before #244");
for (id, path) in paths {
sqlx::query("UPDATE roots SET path = ? WHERE id = ?")
.bind(path)
.bind(id)
.execute(db.pool())
.await
.expect("legacy path, valid under the unique index");
}
(dir, db)
}
async fn root_path(db: &Db, id: i64) -> String {
sqlx::query_scalar("SELECT path FROM roots WHERE id = ?")
.bind(id)
.fetch_one(db.pool())
.await
.expect("root path")
}
/// #244: a root's path could be stored with a trailing separator until
/// #243 normalised the incoming value, and `roots::update` compares a
/// normalised payload against the stored value — so such a row could
/// never be edited again. Migration 0031 strips the separator, and skips
/// a row whose stripped form another row already holds rather than
/// tripping the unique index and refusing to apply.
#[tokio::test]
async fn root_paths_are_normalised_but_never_onto_a_path_in_use() {
let (_dir, db) = roots_before_normalisation(&[
(1, "/mnt/media/movies/main/"),
(2, "/mnt/collide"),
(3, "/mnt/collide/"),
(4, "/mnt/media/tv/kids///"),
])
.await;
db.migrate()
.await
.expect("0031 applies with a collision present");
assert_eq!(root_path(&db, 1).await, "/mnt/media/movies/main");
assert_eq!(
root_path(&db, 4).await,
"/mnt/media/tv/kids",
"every trailing separator goes in one pass"
);
assert_eq!(root_path(&db, 2).await, "/mnt/collide");
assert_eq!(
root_path(&db, 3).await,
"/mnt/collide/",
"left exactly as it is: normalising it would collide with root 2"
);
}
/// #244: the harder half of the same guard. Two rows that strip to the
/// same path, *neither* of which already holds the stripped value, still
/// have to be left alone — normalising them would collide with each
/// other, and a migration that cannot apply stops the daemon booting.
#[tokio::test]
async fn two_rows_that_would_collide_with_each_other_stop_nothing() {
let (_dir, db) =
roots_before_normalisation(&[(1, "/mnt/one/"), (2, "/mnt/dup/"), (3, "/mnt/dup//")])
.await;
db.migrate()
.await
.expect("0031 applies with a mutually colliding pair present");
assert_eq!(root_path(&db, 2).await, "/mnt/dup/");
assert_eq!(root_path(&db, 3).await, "/mnt/dup//");
assert_eq!(
root_path(&db, 1).await,
"/mnt/one",
"the rest of the table is still normalised"
);
}
/// #155: the table-rebuild migrations (0007, 0014, 0021) run outside a
/// transaction so `PRAGMA foreign_keys = OFF` holds and dropping the old
/// tables does not cascade-delete `movie_releases`/`episode_releases`.
@@ -430,6 +517,177 @@ mod tests {
);
}
/// #211: 0032 rebuilds `releases`, which is a parent — `grabs`,
/// `movie_releases`, `episode_releases` and `season_releases` all point
/// at it, three of them ON DELETE CASCADE. The rebuild runs with foreign
/// keys off so dropping the old table neither cascades those children
/// away nor is refused by `grabs`.
#[tokio::test]
async fn the_releases_rebuild_preserves_children_and_rows() {
let (_dir, db) = a_deck_with_every_child_row().await;
db.migrate().await.expect("remaining migrations");
for (label, query, expected) in [
("releases", "SELECT count(*) FROM releases", 3),
("movie_releases", "SELECT count(*) FROM movie_releases", 1),
(
"episode_releases",
"SELECT count(*) FROM episode_releases",
1,
),
("season_releases", "SELECT count(*) FROM season_releases", 1),
("grabs", "SELECT count(*) FROM grabs", 1),
] {
let rows: i64 = sqlx::query_scalar(query)
.fetch_one(db.pool())
.await
.expect("count");
assert_eq!(rows, expected, "{label} survives the releases rebuild");
}
// An existing waived row keeps its NULL and reads as it did before.
let waived: Option<String> =
sqlx::query_scalar("SELECT rejected_rule FROM releases WHERE guid = 'waived-guid'")
.fetch_one(db.pool())
.await
.expect("waived row");
assert_eq!(waived, None);
let rejected: Option<String> =
sqlx::query_scalar("SELECT rejected_rule FROM releases WHERE guid = 'rejected-guid'")
.fetch_one(db.pool())
.await
.expect("rejected row");
assert_eq!(rejected.as_deref(), Some("size"));
// Foreign keys are back on for the connection the migration used.
let foreign_keys: i64 = sqlx::query_scalar("PRAGMA foreign_keys")
.fetch_one(db.pool())
.await
.expect("foreign_keys");
assert_eq!(foreign_keys, 1);
}
/// A database migrated to just before 0032, holding one release per
/// verdict and one row in every table that points at `releases`.
async fn a_deck_with_every_child_row() -> (tempfile::TempDir, Db) {
let dir = tempfile::tempdir().expect("tempdir");
let db = Db::connect(dir.path().join("arr.db"))
.await
.expect("connect");
MIGRATOR
.run_to(31, db.pool())
.await
.expect("migrations before the releases rebuild");
let movie_id = sqlx::query(
"INSERT INTO movies (tmdb_id, title, root_id)
SELECT 693134, 'Dune Part Two', id FROM roots WHERE kind = 'movie' LIMIT 1",
)
.execute(db.pool())
.await
.expect("movie")
.last_insert_rowid();
let series_id = sqlx::query(
"INSERT INTO series (tmdb_id, title, root_id)
SELECT 82728, 'Bluey', id FROM roots WHERE kind = 'tv' LIMIT 1",
)
.execute(db.pool())
.await
.expect("series")
.last_insert_rowid();
let season_id = sqlx::query("INSERT INTO seasons (series_id, number) VALUES (?, 1)")
.bind(series_id)
.execute(db.pool())
.await
.expect("season")
.last_insert_rowid();
let episode_id =
sqlx::query("INSERT INTO episodes (season_id, number, title) VALUES (?, 1, 'x')")
.bind(season_id)
.execute(db.pool())
.await
.expect("episode")
.last_insert_rowid();
// One row per verdict, including the waived row this issue is about,
// which under the old constraint could only hold NULL.
for (guid, verdict, rule) in [
("eligible-guid", "eligible", None),
("waived-guid", "waived", None),
("rejected-guid", "rejected", Some("size")),
] {
sqlx::query(
"INSERT INTO releases (indexer_id, guid, name, size, download_url, parsed, verdict, rejected_rule)
VALUES (1, ?, ?, 1024, 'http://x', '{}', ?, ?)",
)
.bind(guid)
.bind(guid)
.bind(verdict)
.bind(rule)
.execute(db.pool())
.await
.expect("release");
}
let release_id: i64 =
sqlx::query_scalar("SELECT id FROM releases WHERE guid = 'eligible-guid'")
.fetch_one(db.pool())
.await
.expect("release id");
sqlx::query("INSERT INTO movie_releases (movie_id, release_id) VALUES (?, ?)")
.bind(movie_id)
.bind(release_id)
.execute(db.pool())
.await
.expect("movie link");
sqlx::query("INSERT INTO episode_releases (episode_id, release_id) VALUES (?, ?)")
.bind(episode_id)
.bind(release_id)
.execute(db.pool())
.await
.expect("episode link");
sqlx::query("INSERT INTO season_releases (season_id, release_id) VALUES (?, ?)")
.bind(season_id)
.bind(release_id)
.execute(db.pool())
.await
.expect("season link");
sqlx::query(
"INSERT INTO grabs (release_id, target_kind, target_id, infohash)
VALUES (?, 'movie', ?, 'infohash-1')",
)
.bind(release_id)
.bind(movie_id)
.execute(db.pool())
.await
.expect("grab");
(dir, db)
}
/// The relaxed constraint (#211) admits a rule on a waiver and still
/// refuses a rejection without one.
#[tokio::test]
async fn a_waiver_may_name_the_rule_it_relaxed() {
let (_dir, db) = fresh().await;
sqlx::query(
"INSERT INTO releases (indexer_id, guid, name, size, download_url, parsed, verdict, rejected_rule)
VALUES (1, 'waived', 'Some.Release', 1024, 'http://x', '{}', 'waived', 'size')",
)
.execute(db.pool())
.await
.expect("a waiver names its rule");
sqlx::query(
"INSERT INTO releases (indexer_id, guid, name, size, download_url, parsed, verdict)
VALUES (1, 'rejected', 'Some.Release', 1024, 'http://x', '{}', 'rejected')",
)
.execute(db.pool())
.await
.expect_err("a rejection still has to name its rule");
}
#[tokio::test]
async fn seeds_two_tv_roots_with_distinct_policies() {
let (_dir, db) = fresh().await;