feat(arr): convert non-SRT grabs instead of refusing
#199 shipped the manual grab before #213 existed, so srt_text answered 422 for any provider not serving SRT. Fetched::to_srt already decodes and converts, so this is that call. A format with no parser still fails rather than reaching the disk. Leaves the alass half of #214 open; that waits on #194.
This commit is contained in:
@@ -29,7 +29,7 @@ use arr_core::subs::{rank, SubtitleTarget, SubtitleVerdict};
|
|||||||
use arr_core::{layout, Language};
|
use arr_core::{layout, Language};
|
||||||
use arr_db::subtitles as db;
|
use arr_db::subtitles as db;
|
||||||
use arr_db::SubtitleOrigin;
|
use arr_db::SubtitleOrigin;
|
||||||
use arr_subs::{CandidateId, MediaFile, MediaRef, SearchRequest, SubtitleFormat};
|
use arr_subs::{CandidateId, MediaFile, MediaRef, SearchRequest};
|
||||||
use axum::extract::rejection::JsonRejection;
|
use axum::extract::rejection::JsonRejection;
|
||||||
use axum::extract::{Path as UrlPath, State};
|
use axum::extract::{Path as UrlPath, State};
|
||||||
use axum::http::StatusCode;
|
use axum::http::StatusCode;
|
||||||
@@ -852,14 +852,11 @@ async fn claim_path(state: &AppState, destination: &Path) -> Result<(), ApiError
|
|||||||
/// issue (#213), so anything else is refused rather than written under a
|
/// issue (#213), so anything else is refused rather than written under a
|
||||||
/// `.srt` name it does not honour.
|
/// `.srt` name it does not honour.
|
||||||
fn srt_text(fetched: &arr_subs::Fetched) -> Result<String, ApiError> {
|
fn srt_text(fetched: &arr_subs::Fetched) -> Result<String, ApiError> {
|
||||||
if fetched.format != SubtitleFormat::Srt {
|
// Sidecars are SRT (§15), but a provider serving VTT or ASS is converted
|
||||||
return Err(ApiError::Invalid(format!(
|
// rather than refused (#213). Decoding happens inside `to_srt`, and a
|
||||||
"candidate_id: the provider served {}, and sidecars are SRT",
|
// format with no parser still fails here rather than reaching the disk.
|
||||||
fetched.format
|
|
||||||
)));
|
|
||||||
}
|
|
||||||
fetched
|
fetched
|
||||||
.decode()
|
.to_srt()
|
||||||
.map_err(|error| ApiError::SubtitleUpstream(error.to_string()))
|
.map_err(|error| ApiError::SubtitleUpstream(error.to_string()))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -929,6 +926,7 @@ mod tests {
|
|||||||
struct StubProvider {
|
struct StubProvider {
|
||||||
id: ProviderId,
|
id: ProviderId,
|
||||||
format: SubtitleFormat,
|
format: SubtitleFormat,
|
||||||
|
body: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl StubProvider {
|
impl StubProvider {
|
||||||
@@ -936,6 +934,7 @@ mod tests {
|
|||||||
Self {
|
Self {
|
||||||
id: ProviderId::new(name),
|
id: ProviderId::new(name),
|
||||||
format: SubtitleFormat::Srt,
|
format: SubtitleFormat::Srt,
|
||||||
|
body: SRT.to_owned(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -943,6 +942,17 @@ mod tests {
|
|||||||
Self {
|
Self {
|
||||||
id: ProviderId::new(name),
|
id: ProviderId::new(name),
|
||||||
format,
|
format,
|
||||||
|
body: SRT.to_owned(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Serve a format together with a body actually in that format, so a
|
||||||
|
/// conversion test exercises the parser rather than the error path.
|
||||||
|
fn serving_body(name: &str, format: SubtitleFormat, body: &str) -> Self {
|
||||||
|
Self {
|
||||||
|
id: ProviderId::new(name),
|
||||||
|
format,
|
||||||
|
body: body.to_owned(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -994,7 +1004,7 @@ mod tests {
|
|||||||
id: id.clone(),
|
id: id.clone(),
|
||||||
language: Language::PortuguesePortugal,
|
language: Language::PortuguesePortugal,
|
||||||
format: self.format.clone(),
|
format: self.format.clone(),
|
||||||
content: SRT.as_bytes().to_vec(),
|
content: self.body.as_bytes().to_vec(),
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@@ -1340,19 +1350,56 @@ mod tests {
|
|||||||
assert_eq!(fixture.subtitle_rows().await.len(), 1);
|
assert_eq!(fixture.subtitle_rows().await.len(), 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Sidecars are SRT (§15); converting other containers is #213.
|
/// Sidecars are SRT (§15), so a provider serving ASS or VTT is converted
|
||||||
|
/// on the way to disk (#213) rather than refused.
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn a_provider_serving_something_other_than_srt_is_refused() {
|
async fn a_provider_serving_ass_is_converted_to_srt() {
|
||||||
|
const ASS: &str = "[Events]\n\
|
||||||
|
Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text\n\
|
||||||
|
Dialogue: 0,0:00:01.00,0:00:02.00,Default,,0,0,0,,{\\an8}olá\n";
|
||||||
let fixture = application(
|
let fixture = application(
|
||||||
vec![Arc::new(StubProvider::serving(
|
vec![Arc::new(StubProvider::serving_body(
|
||||||
"opensubtitles",
|
"opensubtitles",
|
||||||
SubtitleFormat::Ass,
|
SubtitleFormat::Ass,
|
||||||
|
ASS,
|
||||||
))],
|
))],
|
||||||
vec![],
|
vec![],
|
||||||
)
|
)
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
let (status, body) = grab(&fixture, pt()).await;
|
let (status, body) = grab(&fixture, pt()).await;
|
||||||
assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY, "{body}");
|
|
||||||
|
assert_eq!(status, StatusCode::CREATED, "{body}");
|
||||||
|
let sidecar = fixture
|
||||||
|
.folder
|
||||||
|
.join("Dune (2021) [tmdbid-438631] - [2160p][WEB-DL].pt-PT.srt");
|
||||||
|
let written = std::fs::read_to_string(&sidecar).expect("the sidecar is written");
|
||||||
|
assert!(
|
||||||
|
written.starts_with("1\n00:00:01,000 --> 00:00:02,000"),
|
||||||
|
"{written}"
|
||||||
|
);
|
||||||
|
// The override tag is styling, dropped by the conversion.
|
||||||
|
assert!(
|
||||||
|
written.contains("olá") && !written.contains("an8"),
|
||||||
|
"{written}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A format with no parser is still refused — converting is not guessing.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_provider_serving_a_format_with_no_parser_is_refused() {
|
||||||
|
let fixture = application(
|
||||||
|
vec![Arc::new(StubProvider::serving(
|
||||||
|
"opensubtitles",
|
||||||
|
SubtitleFormat::Other("sub".to_owned()),
|
||||||
|
))],
|
||||||
|
vec![],
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let (status, body) = grab(&fixture, pt()).await;
|
||||||
|
|
||||||
|
assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE, "{body}");
|
||||||
assert!(fixture.subtitle_rows().await.is_empty());
|
assert!(fixture.subtitle_rows().await.is_empty());
|
||||||
assert!(!fixture
|
assert!(!fixture
|
||||||
.folder
|
.folder
|
||||||
|
|||||||
Reference in New Issue
Block a user