feat(arr): one sidecar per language, in the schema
DESIGN.md §15 as amended: a language is satisfied by exactly one sidecar, and no filename segment distinguishes forced from plain from SDH. A unique index over sidecar rows says so; embedded rows keep their own key, since several tracks for one language can legitimately coexist inside a video. Existing databases may hold a duplicate from a manual grab that beat the API's path check, so the migration resolves them rather than failing: a real subtitle beats a machine translation, and of two of the same kind the newest wins. The files stay on disk for the manual delete to clean up. `record_file` no longer swallows every conflict — only the two that mean "arr already knows this file".
This commit is contained in:
+2
-2
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"db_name": "SQLite",
|
"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": {
|
"describe": {
|
||||||
"columns": [
|
"columns": [
|
||||||
{
|
{
|
||||||
@@ -22,5 +22,5 @@
|
|||||||
null
|
null
|
||||||
]
|
]
|
||||||
},
|
},
|
||||||
"hash": "6eaeee584dc98b28b9605faabab0eecf7839b7755c956731617b475921481701"
|
"hash": "3d48b78768bf8dc2e337d0935889a5654089f38bb7ee2a15b7e19f4ed5540262"
|
||||||
}
|
}
|
||||||
+26
@@ -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"
|
||||||
|
}
|
||||||
@@ -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;
|
||||||
+267
-11
@@ -210,15 +210,22 @@ impl NewSubtitleFile {
|
|||||||
|
|
||||||
/// Record a subtitle, returning its row id.
|
/// Record a subtitle, returning its row id.
|
||||||
///
|
///
|
||||||
/// Idempotent on both keys the schema carries: the sidecar path, and the
|
/// Idempotent on the two keys that mean "arr already knows this": the sidecar
|
||||||
/// language an embedded track satisfies. A second probe of the same video, or
|
/// path, and the language an embedded track satisfies. A second probe of the
|
||||||
/// a re-import that finds the same sidecar, converges instead of duplicating.
|
/// same video, or a re-import that finds the same sidecar, converges instead
|
||||||
/// The first row wins — §15 has no upgrade loop, and replacing a subtitle is
|
/// of duplicating. The first row wins — §15 has no upgrade loop, and
|
||||||
/// a manual action that deletes first.
|
/// 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
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// If the insert fails.
|
/// If the insert fails, including when the one-sidecar-per-language
|
||||||
|
/// invariant rejects it.
|
||||||
pub async fn record_file(
|
pub async fn record_file(
|
||||||
pool: &SqlitePool,
|
pool: &SqlitePool,
|
||||||
subtitle: &NewSubtitleFile,
|
subtitle: &NewSubtitleFile,
|
||||||
@@ -230,7 +237,9 @@ pub async fn record_file(
|
|||||||
(media_file_id, language, origin, provider, candidate_id, engine,
|
(media_file_id, language, origin, provider, candidate_id, engine,
|
||||||
forced, sdh, synced, sync_rejected, path)
|
forced, sdh, synced, sync_rejected, path)
|
||||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
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""#,
|
RETURNING id AS "id!: i64""#,
|
||||||
subtitle.media_file_id,
|
subtitle.media_file_id,
|
||||||
subtitle.language,
|
subtitle.language,
|
||||||
@@ -251,8 +260,9 @@ pub async fn record_file(
|
|||||||
return Ok(id);
|
return Ok(id);
|
||||||
}
|
}
|
||||||
|
|
||||||
// The insert hit one of the two unique keys. Which one is decided by the
|
// The insert hit one of the two keys named above — the one-sidecar
|
||||||
// origin, because only an embedded row has no path.
|
// 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() {
|
if let Some(path) = subtitle.path.as_deref() {
|
||||||
sqlx::query_scalar!(
|
sqlx::query_scalar!(
|
||||||
r#"SELECT id AS "id!: i64" FROM subtitle_files WHERE path = ?"#,
|
r#"SELECT id AS "id!: i64" FROM subtitle_files WHERE path = ?"#,
|
||||||
@@ -323,6 +333,32 @@ pub async fn files_for(
|
|||||||
.collect())
|
.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<Option<String>, 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.
|
/// Record what `alass` did to a subtitle already on disk.
|
||||||
///
|
///
|
||||||
/// A rejected sync flags the row (§15); it never deletes it, because the
|
/// 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<Vec<PendingSubtitl
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::{
|
use super::{
|
||||||
attempts_for, delete_file, files_for, mark_satisfied, mark_synced, pending, record_attempt,
|
attempts_for, delete_file, files_for, mark_satisfied, mark_synced, pending, record_attempt,
|
||||||
record_file, want, NewSubtitleFile, SubtitleOrigin, SubtitleState, SubtitleSync,
|
record_file, sidecar_for, want, NewSubtitleFile, SubtitleOrigin, SubtitleState,
|
||||||
|
SubtitleSync,
|
||||||
};
|
};
|
||||||
use crate::Db;
|
use crate::Db;
|
||||||
use sqlx::SqlitePool;
|
use sqlx::SqlitePool;
|
||||||
@@ -650,7 +687,10 @@ mod tests {
|
|||||||
let again = record_file(db.pool(), &fetched).await.unwrap();
|
let again = record_file(db.pool(), &fetched).await.unwrap();
|
||||||
assert_eq!(first, again);
|
assert_eq!(first, again);
|
||||||
|
|
||||||
let translated = NewSubtitleFile::translated(file, "pt-PT", "deepl", "/m/b.pt-PT.mt.srt")
|
// A different language: §15 gives pt-PT exactly one sidecar, so the
|
||||||
|
// machine translation that sits beside this one is `en`, not a
|
||||||
|
// second pt-PT (#222).
|
||||||
|
let translated = NewSubtitleFile::translated(file, "en", "deepl", "/m/b.en.mt.srt")
|
||||||
.sync(SubtitleSync::Synced);
|
.sync(SubtitleSync::Synced);
|
||||||
record_file(db.pool(), &translated).await.unwrap();
|
record_file(db.pool(), &translated).await.unwrap();
|
||||||
|
|
||||||
@@ -671,6 +711,222 @@ mod tests {
|
|||||||
assert_eq!(machine.sync, SubtitleSync::Synced);
|
assert_eq!(machine.sync, SubtitleSync::Synced);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// §15, as amended: a language is satisfied by exactly one sidecar, so a
|
||||||
|
/// second file claiming one is refused by the schema rather than resolved
|
||||||
|
/// to the first row's id (#222).
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_language_gets_exactly_one_sidecar() {
|
||||||
|
let (db, _dir) = database().await;
|
||||||
|
let file = media_file(db.pool(), "/m/one.mkv", "2026-01-01T00:00:00.000Z").await;
|
||||||
|
|
||||||
|
let fetched =
|
||||||
|
NewSubtitleFile::fetched(file, "pt-PT", "opensubtitles", "1", "/m/one.pt-PT.srt");
|
||||||
|
record_file(db.pool(), &fetched).await.unwrap();
|
||||||
|
|
||||||
|
// A different file, the same language: `.mt.srt` never sits beside a
|
||||||
|
// real subtitle.
|
||||||
|
let translated = NewSubtitleFile::translated(file, "pt-PT", "deepl", "/m/one.pt-PT.mt.srt");
|
||||||
|
let refused = record_file(db.pool(), &translated).await.unwrap_err();
|
||||||
|
assert!(
|
||||||
|
matches!(&refused, sqlx::Error::Database(error) if error.is_unique_violation()),
|
||||||
|
"{refused:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Another language on the same file, and the same language on another
|
||||||
|
// file, are both untouched by it.
|
||||||
|
record_file(
|
||||||
|
db.pool(),
|
||||||
|
&NewSubtitleFile::fetched(file, "en", "opensubtitles", "2", "/m/one.en.srt"),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let other = media_file(db.pool(), "/m/two.mkv", "2026-01-01T00:00:00.000Z").await;
|
||||||
|
record_file(
|
||||||
|
db.pool(),
|
||||||
|
&NewSubtitleFile::fetched(other, "pt-PT", "opensubtitles", "3", "/m/two.pt-PT.srt"),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(files_for(db.pool(), file).await.unwrap().len(), 2);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The embedded key is untouched by the sidecar one: a track inside the
|
||||||
|
/// video is not a file, and a plain and a forced track for one language
|
||||||
|
/// legitimately coexist (#222).
|
||||||
|
#[tokio::test]
|
||||||
|
async fn embedded_tracks_still_share_a_language() {
|
||||||
|
let (db, _dir) = database().await;
|
||||||
|
let file = media_file(db.pool(), "/m/tracks.mkv", "2026-01-01T00:00:00.000Z").await;
|
||||||
|
|
||||||
|
record_file(db.pool(), &NewSubtitleFile::embedded(file, "en"))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
record_file(db.pool(), &NewSubtitleFile::embedded(file, "en").forced())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
record_file(db.pool(), &NewSubtitleFile::embedded(file, "en").sdh())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
// And a sidecar for that same language still fits beside all three.
|
||||||
|
record_file(
|
||||||
|
db.pool(),
|
||||||
|
&NewSubtitleFile::extracted(file, "en", "/m/tracks.en.srt"),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(files_for(db.pool(), file).await.unwrap().len(), 4);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn sidecar_for_answers_with_the_file_not_the_track() {
|
||||||
|
let (db, _dir) = database().await;
|
||||||
|
let file = media_file(db.pool(), "/m/ask.mkv", "2026-01-01T00:00:00.000Z").await;
|
||||||
|
|
||||||
|
record_file(db.pool(), &NewSubtitleFile::embedded(file, "en"))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
sidecar_for(db.pool(), file, "en").await.unwrap(),
|
||||||
|
None,
|
||||||
|
"an embedded track is not a sidecar"
|
||||||
|
);
|
||||||
|
|
||||||
|
record_file(
|
||||||
|
db.pool(),
|
||||||
|
&NewSubtitleFile::extracted(file, "en", "/m/ask.en.srt"),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
sidecar_for(db.pool(), file, "en").await.unwrap().as_deref(),
|
||||||
|
Some("/m/ask.en.srt")
|
||||||
|
);
|
||||||
|
assert_eq!(sidecar_for(db.pool(), file, "pt-PT").await.unwrap(), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A sidecar row written straight in, so a test can build the duplicate
|
||||||
|
/// state the schema now forbids.
|
||||||
|
async fn raw_sidecar(
|
||||||
|
pool: &SqlitePool,
|
||||||
|
file: i64,
|
||||||
|
origin: &str,
|
||||||
|
language: &str,
|
||||||
|
path: &str,
|
||||||
|
created: &str,
|
||||||
|
) {
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO subtitle_files
|
||||||
|
(media_file_id, language, origin, provider, engine, path, created_at)
|
||||||
|
VALUES (?, ?, ?, ?, ?, ?, ?)",
|
||||||
|
)
|
||||||
|
.bind(file)
|
||||||
|
.bind(language)
|
||||||
|
.bind(origin)
|
||||||
|
.bind((origin == "provider").then_some("opensubtitles"))
|
||||||
|
.bind((origin == "translated").then_some("deepl"))
|
||||||
|
.bind(path)
|
||||||
|
.bind(created)
|
||||||
|
.execute(pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A database written before the constraint may hold duplicates from a
|
||||||
|
/// manual grab that beat the API's path check. Migration `0029` resolves
|
||||||
|
/// them instead of failing: a real subtitle beats a machine translation,
|
||||||
|
/// and of two of the same kind the newest wins (#222).
|
||||||
|
#[tokio::test]
|
||||||
|
#[allow(clippy::too_many_lines)]
|
||||||
|
async fn the_migration_resolves_duplicates_it_finds() {
|
||||||
|
let (db, _dir) = database().await;
|
||||||
|
let file = media_file(db.pool(), "/m/old.mkv", "2026-01-01T00:00:00.000Z").await;
|
||||||
|
|
||||||
|
// Stand the database back up as it was before `0029`.
|
||||||
|
sqlx::query("DROP INDEX subtitle_files_one_sidecar")
|
||||||
|
.execute(db.pool())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let pool = db.pool();
|
||||||
|
// Newer, but machine made: the real subtitle is the one to keep.
|
||||||
|
raw_sidecar(
|
||||||
|
pool,
|
||||||
|
file,
|
||||||
|
"provider",
|
||||||
|
"pt-PT",
|
||||||
|
"/m/old.pt-PT.srt",
|
||||||
|
"2026-01-01T00:00:00.000Z",
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
raw_sidecar(
|
||||||
|
pool,
|
||||||
|
file,
|
||||||
|
"translated",
|
||||||
|
"pt-PT",
|
||||||
|
"/m/old.pt-PT.mt.srt",
|
||||||
|
"2026-02-01T00:00:00.000Z",
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
// Two of the same kind: the newest wins.
|
||||||
|
raw_sidecar(
|
||||||
|
pool,
|
||||||
|
file,
|
||||||
|
"provider",
|
||||||
|
"en",
|
||||||
|
"/m/old.en.srt",
|
||||||
|
"2026-01-01T00:00:00.000Z",
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
raw_sidecar(
|
||||||
|
pool,
|
||||||
|
file,
|
||||||
|
"provider",
|
||||||
|
"en",
|
||||||
|
"/m/old.en.2.srt",
|
||||||
|
"2026-03-01T00:00:00.000Z",
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
// An embedded pair for one language is not a duplicate.
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO subtitle_files (media_file_id, language, origin, forced)
|
||||||
|
VALUES (?, 'en', 'embedded', 0), (?, 'en', 'embedded', 1)",
|
||||||
|
)
|
||||||
|
.bind(file)
|
||||||
|
.bind(file)
|
||||||
|
.execute(db.pool())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let migration = crate::MIGRATOR
|
||||||
|
.iter()
|
||||||
|
.find(|migration| migration.version == 29)
|
||||||
|
.expect("migration 0029");
|
||||||
|
sqlx::raw_sql(migration.sql.clone())
|
||||||
|
.execute(db.pool())
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let kept: Vec<_> = 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]
|
#[tokio::test]
|
||||||
async fn the_schema_ties_every_optional_column_to_the_origin() {
|
async fn the_schema_ties_every_optional_column_to_the_origin() {
|
||||||
let (db, _dir) = database().await;
|
let (db, _dir) = database().await;
|
||||||
|
|||||||
Reference in New Issue
Block a user