diff --git a/.sqlx/query-6eaeee584dc98b28b9605faabab0eecf7839b7755c956731617b475921481701.json b/.sqlx/query-3d48b78768bf8dc2e337d0935889a5654089f38bb7ee2a15b7e19f4ed5540262.json similarity index 71% rename from .sqlx/query-6eaeee584dc98b28b9605faabab0eecf7839b7755c956731617b475921481701.json rename to .sqlx/query-3d48b78768bf8dc2e337d0935889a5654089f38bb7ee2a15b7e19f4ed5540262.json index 6d94a9b..b32d119 100644 --- a/.sqlx/query-6eaeee584dc98b28b9605faabab0eecf7839b7755c956731617b475921481701.json +++ b/.sqlx/query-3d48b78768bf8dc2e337d0935889a5654089f38bb7ee2a15b7e19f4ed5540262.json @@ -1,6 +1,6 @@ { "db_name": "SQLite", - "query": "INSERT INTO subtitle_files\n (media_file_id, language, origin, provider, candidate_id, engine,\n forced, sdh, synced, sync_rejected, path)\n VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)\n ON CONFLICT DO NOTHING\n RETURNING id AS \"id!: i64\"", + "query": "INSERT INTO subtitle_files\n (media_file_id, language, origin, provider, candidate_id, engine,\n forced, sdh, synced, sync_rejected, path)\n VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)\n ON CONFLICT (path) DO NOTHING\n ON CONFLICT (media_file_id, language, forced, sdh)\n WHERE origin = 'embedded' DO NOTHING\n RETURNING id AS \"id!: i64\"", "describe": { "columns": [ { @@ -22,5 +22,5 @@ null ] }, - "hash": "6eaeee584dc98b28b9605faabab0eecf7839b7755c956731617b475921481701" + "hash": "3d48b78768bf8dc2e337d0935889a5654089f38bb7ee2a15b7e19f4ed5540262" } diff --git a/.sqlx/query-e17b71f67d8b79c54d2d6f32d5e4197d77faac1763b360e8e609b0b9ba5820f1.json b/.sqlx/query-e17b71f67d8b79c54d2d6f32d5e4197d77faac1763b360e8e609b0b9ba5820f1.json new file mode 100644 index 0000000..b3a5a5a --- /dev/null +++ b/.sqlx/query-e17b71f67d8b79c54d2d6f32d5e4197d77faac1763b360e8e609b0b9ba5820f1.json @@ -0,0 +1,26 @@ +{ + "db_name": "SQLite", + "query": "SELECT path AS \"path!: String\" FROM subtitle_files\n WHERE media_file_id = ? AND language = ? AND path IS NOT NULL", + "describe": { + "columns": [ + { + "name": "path!: String", + "ordinal": 0, + "type_info": "Text", + "origin": { + "Table": { + "table": "subtitle_files", + "name": "path" + } + } + } + ], + "parameters": { + "Right": 2 + }, + "nullable": [ + true + ] + }, + "hash": "e17b71f67d8b79c54d2d6f32d5e4197d77faac1763b360e8e609b0b9ba5820f1" +} diff --git a/crates/arr-db/migrations/0029_one_sidecar_per_language.sql b/crates/arr-db/migrations/0029_one_sidecar_per_language.sql new file mode 100644 index 0000000..84d44c8 --- /dev/null +++ b/crates/arr-db/migrations/0029_one_sidecar_per_language.sql @@ -0,0 +1,41 @@ +-- #222. DESIGN.md §15, as amended: a language is satisfied by exactly one +-- sidecar. No filename segment distinguishes forced from plain from SDH, so +-- two sidecar rows for one (media file, language) describe two files +-- competing for one name. The schema says so now rather than leaving it to +-- the API's path check. +-- +-- Embedded rows are deliberately untouched. They describe tracks inside the +-- video, not files on disk, and several can legitimately coexist for one +-- language — a plain track and a forced one, say. Their key stays +-- `(media_file_id, language, forced, sdh)`. + +-- A database that predates the constraint may already hold a duplicate from +-- a manual grab that beat the path check: a fetched `.pt-PT.srt` beside a +-- translated `.pt-PT.mt.srt`, for instance. Resolve rather than fail. Of the +-- rows for one (media file, language): keep a real subtitle over a machine +-- translation, and of two of the same kind the newest. +-- +-- The files stay on disk. Deleting a viewer's subtitle during a migration is +-- worse than leaving an orphan, and the manual delete (#218, #223) cleans one +-- up on request. +DELETE FROM subtitle_files +WHERE path IS NOT NULL + AND id NOT IN ( + SELECT id + FROM ( + SELECT id, + ROW_NUMBER() OVER ( + PARTITION BY media_file_id, language + ORDER BY (origin = 'translated') ASC, + created_at DESC, + id DESC + ) AS place + FROM subtitle_files + WHERE path IS NOT NULL + ) + WHERE place = 1 + ); + +CREATE UNIQUE INDEX subtitle_files_one_sidecar + ON subtitle_files (media_file_id, language) + WHERE path IS NOT NULL; diff --git a/crates/arr-db/src/subtitles.rs b/crates/arr-db/src/subtitles.rs index e93176b..4625481 100644 --- a/crates/arr-db/src/subtitles.rs +++ b/crates/arr-db/src/subtitles.rs @@ -210,15 +210,22 @@ impl NewSubtitleFile { /// Record a subtitle, returning its row id. /// -/// Idempotent on both keys the schema carries: the sidecar path, and the -/// language an embedded track satisfies. A second probe of the same video, or -/// a re-import that finds the same sidecar, converges instead of duplicating. -/// The first row wins — §15 has no upgrade loop, and replacing a subtitle is -/// a manual action that deletes first. +/// Idempotent on the two keys that mean "arr already knows this": the sidecar +/// path, and the language an embedded track satisfies. A second probe of the +/// same video, or a re-import that finds the same sidecar, converges instead +/// of duplicating. The first row wins — §15 has no upgrade loop, and +/// replacing a subtitle is a manual action that deletes first. +/// +/// The third key is not swallowed. §15 gives a language exactly one sidecar, +/// so a *different* file claiming a language that already has one is a real +/// conflict — the caller wrote a second subtitle where only one may live — +/// and the unique violation is returned rather than resolved to some other +/// row's id. Callers that can refuse before writing anything do (#222). /// /// # Errors /// -/// If the insert fails. +/// If the insert fails, including when the one-sidecar-per-language +/// invariant rejects it. pub async fn record_file( pool: &SqlitePool, subtitle: &NewSubtitleFile, @@ -230,7 +237,9 @@ pub async fn record_file( (media_file_id, language, origin, provider, candidate_id, engine, forced, sdh, synced, sync_rejected, path) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - ON CONFLICT DO NOTHING + ON CONFLICT (path) DO NOTHING + ON CONFLICT (media_file_id, language, forced, sdh) + WHERE origin = 'embedded' DO NOTHING RETURNING id AS "id!: i64""#, subtitle.media_file_id, subtitle.language, @@ -251,8 +260,9 @@ pub async fn record_file( return Ok(id); } - // The insert hit one of the two unique keys. Which one is decided by the - // origin, because only an embedded row has no path. + // The insert hit one of the two keys named above — the one-sidecar + // index is not among them and would have raised. Which of the two is + // decided by the origin, because only an embedded row has no path. if let Some(path) = subtitle.path.as_deref() { sqlx::query_scalar!( r#"SELECT id AS "id!: i64" FROM subtitle_files WHERE path = ?"#, @@ -323,6 +333,32 @@ pub async fn files_for( .collect()) } +/// The one sidecar this media file already has in `language`, if any (§15). +/// +/// The same invariant the schema enforces, read instead of tripped over: a +/// caller about to fetch or translate a second subtitle for a language asks +/// this first, so it refuses before anything reaches the disk rather than +/// writing a file the insert then rejects. Embedded rows are not sidecars and +/// never answer here. +/// +/// # Errors +/// +/// If the query fails. +pub async fn sidecar_for( + pool: &SqlitePool, + media_file_id: i64, + language: &str, +) -> Result, sqlx::Error> { + sqlx::query_scalar!( + r#"SELECT path AS "path!: String" FROM subtitle_files + WHERE media_file_id = ? AND language = ? AND path IS NOT NULL"#, + media_file_id, + language + ) + .fetch_optional(pool) + .await +} + /// Record what `alass` did to a subtitle already on disk. /// /// A rejected sync flags the row (§15); it never deletes it, because the @@ -582,7 +618,8 @@ pub async fn pending(pool: &SqlitePool, limit: i64) -> Result = files_for(db.pool(), file) + .await + .unwrap() + .into_iter() + .filter_map(|row| row.path) + .collect(); + assert_eq!(kept, vec!["/m/old.en.2.srt", "/m/old.pt-PT.srt"]); + assert_eq!( + files_for(db.pool(), file) + .await + .unwrap() + .iter() + .filter(|row| row.origin == SubtitleOrigin::Embedded) + .count(), + 2, + "embedded rows are not sidecars and are left alone" + ); + } + #[tokio::test] async fn the_schema_ties_every_optional_column_to_the_origin() { let (db, _dir) = database().await;