yolo/arr
3
0
Fork 0
Code Issues 12 Pull Requests Actions Packages Projects Releases Wiki Activity

Merge #222: one sidecar per language, enforced in the schema

Browse Source 
Closes #222
This commit is contained in:
Miguel Palhas
2026-08-25 08:35:58 +01:00
parent 2c5cd85499 d358c58844
commit 1126a52bbb
6 changed files with 450 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.sqlx/query-6eaeee584dc98b28b9605faabab0eecf7839b7755c956731617b475921481701.json → .sqlx/query-3d48b78768bf8dc2e337d0935889a5654089f38bb7ee2a15b7e19f4ed5540262.json
Generated
+2 -2
View File
@@ -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"
}
.sqlx/query-e17b71f67d8b79c54d2d6f32d5e4197d77faac1763b360e8e609b0b9ba5820f1.json
Generated
+26
View File
@@ -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"
}
crates/arr-api/src/subtitles.rs
+75 -4
View File
@@ -963,6 +963,7 @@ pub async fn grab(
.clone();
let destination = target.sidecar(&language, false)?;
claim_language(&state, target.media_file_id, &language).await?;
claim_path(&state, &destination).await?;
let fetched = provider
@@ -1054,6 +1055,7 @@ pub async fn translate(
.clone();
let destination = target.sidecar(&target_language, true)?;
claim_language(&state, target.media_file_id, &target_language).await?;
claim_path(&state, &destination).await?;
let raw = tokio::fs::read_to_string(&source_path)
@@ -1166,12 +1168,33 @@ async fn engine_name(state: &AppState, requested: Option<&str>) -> Result<String
.ok_or_else(|| ApiError::Invalid("engine: no translation engine is configured".into()))
}
/// Refuse a second subtitle for a language this file already has (#222).
///
/// §15 gives a language exactly one sidecar and no upgrade loop: replacing
/// one is delete-then-fetch, which the panel offers as a button (#223). The
/// schema carries the same invariant as a unique index, so this only decides
/// *when* the refusal happens — before the download and the write, rather
/// than on the insert with a stray file already on disk.
async fn claim_language(
state: &AppState,
media_file_id: i64,
language: &Language,
) -> Result<(), ApiError> {
let tag = language.to_string();
if let Some(existing) = db::sidecar_for(pool(state)?, media_file_id, &tag).await? {
return Err(ApiError::Conflict(format!(
"{tag} already has a subtitle at {existing}; delete it first to replace it"
)));
}
Ok(())
}
/// Refuse to write over a sidecar arr already knows about.
///
/// §15 has no upgrade loop and no in-place replacement: replacing a subtitle
/// is delete-then-fetch. Without this a second grab in the same language
/// would overwrite the file while the unique index on `path` kept the first
/// row, leaving the database describing a file that is no longer there.
/// [`claim_language`] covers the common case, and this still covers the rest:
/// the filename is the row's identity on disk, and two `media_files` rows for
/// one video — a re-import that left a stale row behind — derive the same
/// sidecar name from different ids. Two rows never race for one name.
async fn claim_path(state: &AppState, destination: &Path) -> Result<(), ApiError> {
let path = destination.to_string_lossy().into_owned();
let taken = sqlx::query_scalar!(
@@ -2198,6 +2221,54 @@ mod tests {
assert_eq!(fixture.subtitle_rows().await.len(), 1);
}
/// §15 as amended (#222): a language is satisfied by exactly one
/// sidecar. The second subtitle here lands on a *different* filename, so
/// only the invariant can refuse it — the path check cannot see it.
#[tokio::test]
async fn a_second_subtitle_for_a_satisfied_language_is_refused() {
let fixture = application(
vec![Arc::new(StubProvider::new("opensubtitles"))],
vec![Arc::new(StubBackend)],
)
.await;
let (_, source) = grab(&fixture, pt()).await;
// `.en.mt.srt` — English is satisfied by a machine translation.
let response = reqwest::Client::new()
.post(format!(
"{}/api/media-files/1/subtitles/translate",
fixture.base
))
.json(&serde_json::json!({
"source_subtitle_id": source["id"].as_i64().expect("source id"),
"target_language": "en",
"engine": "openai"
}))
.send()
.await
.expect("translate");
assert_eq!(response.status(), StatusCode::CREATED);
// A provider fetch for English would write `.en.srt`, a name nothing
// holds. §15 refuses it anyway, and says how to replace it instead.
let mut body = pt();
body["language"] = serde_json::json!("en");
let (status, refused) = grab(&fixture, body).await;
assert_eq!(status, StatusCode::CONFLICT, "{refused}");
let message = refused["error"].as_str().expect("message");
assert!(message.contains("delete it first"), "{message}");
assert!(message.contains("en.mt.srt"), "{message}");
assert_eq!(fixture.subtitle_rows().await.len(), 2);
assert!(
!fixture
.folder
.join("Dune (2021) [tmdbid-438631] - [2160p][WEB-DL].en.srt")
.exists(),
"the refusal lands before anything reaches the disk"
);
}
/// Write an executable fake `alass`. Its body receives the subtitle,
/// video and output paths as `$1`, `$2`, `$3`.
async fn fake_alass(dir: &std::path::Path, body: &str) -> PathBuf {
crates/arr-daemon/src/subtitles.rs
+39
View File
@@ -1494,6 +1494,45 @@ mod tests {
.contains("no text source to translate from"));
}
/// §15 gives a language exactly one sidecar and forced gets no segment
/// of its own, so nothing in the loop may ever write one. Ranking already
/// rejects a forced candidate (#185); this holds it to that at the end of
/// the pipeline, where the row would actually appear (#222).
#[tokio::test]
async fn a_forced_candidate_never_becomes_a_sidecar() {
let fixture = Fixture::new(&no_tracks()).await;
fixture.configure(r#"["pt-PT"]"#, Some("openai")).await;
let forced = Candidate {
forced: true,
hash_match: true,
download_count: Some(1_000_000),
..candidate("opensubtitles", "forced", Language::PortuguesePortugal)
};
let action = action(
vec![Arc::new(StubProvider::new("opensubtitles", vec![forced]))],
vec![Arc::new(StubBackend)],
);
action.run(&fixture.database).await.unwrap();
assert!(
fixture.files().await.is_empty(),
"a forced candidate is not a subtitle arr has"
);
assert!(
!fixture
.directory
.path()
.join("Movie (2024) - [1080p].pt-PT.srt")
.exists(),
"and nothing was written for it"
);
// The best candidate on offer being unusable is the same position as
// no candidate at all: nothing to fetch and nothing to translate from.
let attempt = fixture.attempt("pt-PT").await.unwrap();
assert_eq!(attempt.state, SubtitleState::Unavailable);
}
#[tokio::test]
async fn the_ranked_winner_is_fetched_synced_and_recorded() {
let fixture = Fixture::new(&no_tracks()).await;
crates/arr-db/migrations/0029_one_sidecar_per_language.sql
+41
View File
@@ -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;
crates/arr-db/src/subtitles.rs
+267 -11
View File
@@ -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<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.
///
/// 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 {
use super::{
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 sqlx::SqlitePool;
@@ -650,7 +687,10 @@ mod tests {
let again = record_file(db.pool(), &fetched).await.unwrap();
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);
record_file(db.pool(), &translated).await.unwrap();
@@ -671,6 +711,222 @@ mod tests {
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]
async fn the_schema_ties_every_optional_column_to_the_origin() {
let (db, _dir) = database().await;