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
This commit is contained in:
Miguel Palhas
2026-08-25 11:42:59 +01:00
parent c454b3ec11
commit a0dc07f085
6 changed files with 281 additions and 8 deletions
@@ -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;
+171
View File
@@ -517,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;