diff --git a/crates/arr-api/src/subtitles.rs b/crates/arr-api/src/subtitles.rs index 1df4353..9e97ccf 100644 --- a/crates/arr-api/src/subtitles.rs +++ b/crates/arr-api/src/subtitles.rs @@ -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 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 {