fix(subs): ask OpenSubtitles for SRT at download
ci / web (push) Successful in 35s
ci / rust (push) Successful in 1m44s
e2e / e2e (push) Failing after 1m51s
ci / image (push) Successful in 2m23s

A search entry often carries no `format`, so `format_of(None)` produced
`Other("")`, that was stashed at search time and handed back as the
download's format, and conversion had no parser to pick:

    4073669 could not be converted from  to SRT: no parser for ""

`POST /download` takes `sub_format`, and the API converts on its side, so
asking for srt makes the answer srt whatever the uploader posted. The
format guessed from the search entry is dead weight and goes with it.

The download mock matched only `file_id`, so it never exercised what the
real API returns.

Closes #272

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Miguel Palhas
2026-08-31 10:10:06 +01:00
parent 7bdbecd68a
commit d97cdfd557
2 changed files with 14 additions and 34 deletions
+8 -33
View File
@@ -117,7 +117,6 @@ impl OpenSubtitlesConfig {
#[derive(Clone, Debug)]
struct CandidateMeta {
language: Language,
format: SubtitleFormat,
}
/// The OpenSubtitles.com client behind the [`Provider`] trait.
@@ -380,7 +379,6 @@ impl OpenSubtitles {
offered.candidate.id.clone(),
CandidateMeta {
language: offered.candidate.language.clone(),
format: offered.format.clone(),
},
)
}));
@@ -411,7 +409,11 @@ impl OpenSubtitles {
})?;
let token = self.current_token().await?;
let body = serde_json::json!({ "file_id": file_id });
// Ask for SRT rather than take whatever the uploader posted (#272).
// A search entry often carries no `format` at all, which left the
// download labelled `Other("")` and the conversion with no parser to
// pick. The API converts on its side, so the answer is always SRT.
let body = serde_json::json!({ "file_id": file_id, "sub_format": "srt" });
let response = self
.send(Method::POST, "download", None, Some(&body), Some(&token))
@@ -431,7 +433,7 @@ impl OpenSubtitles {
Ok(Fetched {
id: id.clone(),
language: meta.language,
format: meta.format,
format: SubtitleFormat::Srt,
content,
})
}
@@ -496,7 +498,6 @@ fn truncate(body: &str) -> String {
/// One API entry turned into arr's terms, plus the facts download will need.
struct Offered {
candidate: Candidate,
format: SubtitleFormat,
}
/// The wire shape of `POST /login`: the user token everything that touches
@@ -531,8 +532,6 @@ impl SubtitleEntry {
let file_id = attributes.files.first()?.file_id;
let language = language_of(attributes.language.as_deref()?)?;
let release_name = attributes.release.filter(|release| !release.is_empty());
let format = format_of(attributes.format.as_deref());
Some(Offered {
candidate: Candidate {
provider: ProviderId::new(PROVIDER_NAME),
@@ -547,7 +546,6 @@ impl SubtitleEntry {
sdh: attributes.hearing_impaired,
release_name,
},
format,
})
}
}
@@ -565,7 +563,6 @@ struct SubtitleAttributes {
hearing_impaired: bool,
#[serde(default)]
foreign_parts_only: bool,
format: Option<String>,
files: Vec<SubtitleFileRef>,
}
@@ -649,15 +646,6 @@ fn source_of_release(release: &str) -> Option<arr_core::Source> {
arr_parse::parse(release).source.map(Into::into)
}
fn format_of(format: Option<&str>) -> SubtitleFormat {
match format.unwrap_or_default().to_ascii_lowercase().as_str() {
"srt" | "subrip" => SubtitleFormat::Srt,
"ass" | "ssa" => SubtitleFormat::Ass,
"vtt" | "webvtt" => SubtitleFormat::Vtt,
other => SubtitleFormat::Other(other.to_owned()),
}
}
/// Order the candidates best first through the pure ranker (#185), building
/// the target from the same facts the search ran on: the file's own hash and
/// whatever the release name claims about group and source.
@@ -691,9 +679,9 @@ fn rank_candidates(
#[cfg(test)]
mod tests {
use super::{format_of, language_of, moviehash, rank_candidates, search_params, PROVIDER_NAME};
use super::{language_of, moviehash, rank_candidates, search_params, PROVIDER_NAME};
use crate::SearchRequest;
use crate::{Candidate, CandidateId, MediaFile, MediaRef, ProviderId, SubtitleFormat};
use crate::{Candidate, CandidateId, MediaFile, MediaRef, ProviderId};
use arr_core::Language;
#[test]
@@ -817,19 +805,6 @@ mod tests {
);
}
#[test]
fn formats_spell_the_way_providers_do() {
assert_eq!(format_of(Some("srt")), SubtitleFormat::Srt);
assert_eq!(format_of(Some("subrip")), SubtitleFormat::Srt);
assert_eq!(format_of(Some("ass")), SubtitleFormat::Ass);
assert_eq!(format_of(Some("vtt")), SubtitleFormat::Vtt);
assert_eq!(
format_of(Some("idx")),
SubtitleFormat::Other("idx".to_owned())
);
assert_eq!(format_of(None), SubtitleFormat::Other(String::new()));
}
fn candidate(id: u64, hash_match: bool, downloads: u64) -> Candidate {
Candidate {
provider: ProviderId::new(PROVIDER_NAME),
+6 -1
View File
@@ -267,7 +267,12 @@ async fn mount_download_link(server: &MockServer, token: &'static str, file_id:
Mock::given(method("POST"))
.and(path("/api/v1/download"))
.and(header("Authorization", format!("Bearer {token}")))
.and(body_partial_json(serde_json::json!({ "file_id": file_id })))
// `sub_format` is what makes the answer SRT (#272). A search entry
// often carries no format at all, so without it the download was
// labelled `Other("")` and no parser matched.
.and(body_partial_json(
serde_json::json!({ "file_id": file_id, "sub_format": "srt" }),
))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"link": format!("{}/file/{file_id}.srt", server.uri()),
"file_name": "subtitle.srt",