fix(arr): close the subtitle ranking seam
#184 and #185 were built in parallel and their candidate types did not meet. Ranking returned no identity for a candidate, so the winner of a rank() could not be handed back to Provider::download -- rank() reorders, so the caller could not recover it by position either. RankedSubtitle now carries the index of the candidate in the slice it was given, and Candidate::to_core is the one place the two shapes are mapped: hash_match against a compared hash, sdh against hearing_impaired, group against release_group, and the two optional tiebreakers defaulted to sort last rather than block a candidate.
This commit is contained in:
@@ -72,6 +72,11 @@ pub enum SubtitleVerdict {
|
|||||||
/// One candidate paired with the verdict that placed it in the ranking.
|
/// One candidate paired with the verdict that placed it in the ranking.
|
||||||
#[derive(Clone, Copy, Debug)]
|
#[derive(Clone, Copy, Debug)]
|
||||||
pub struct RankedSubtitle<'a> {
|
pub struct RankedSubtitle<'a> {
|
||||||
|
/// Position of this candidate in the slice handed to [`rank`]. Ranking
|
||||||
|
/// reorders, and a candidate carries no identity of its own, so this is
|
||||||
|
/// how a caller maps a result back to the provider candidate it came
|
||||||
|
/// from — and therefore to the id it must ask the provider to download.
|
||||||
|
pub index: usize,
|
||||||
pub candidate: SubtitleCandidate<'a>,
|
pub candidate: SubtitleCandidate<'a>,
|
||||||
pub verdict: SubtitleVerdict,
|
pub verdict: SubtitleVerdict,
|
||||||
}
|
}
|
||||||
@@ -128,7 +133,9 @@ pub fn rank<'a>(
|
|||||||
) -> Vec<RankedSubtitle<'a>> {
|
) -> Vec<RankedSubtitle<'a>> {
|
||||||
let mut ranked: Vec<RankedSubtitle<'a>> = candidates
|
let mut ranked: Vec<RankedSubtitle<'a>> = candidates
|
||||||
.iter()
|
.iter()
|
||||||
.map(|candidate| RankedSubtitle {
|
.enumerate()
|
||||||
|
.map(|(index, candidate)| RankedSubtitle {
|
||||||
|
index,
|
||||||
candidate: *candidate,
|
candidate: *candidate,
|
||||||
verdict: if candidate.forced {
|
verdict: if candidate.forced {
|
||||||
SubtitleVerdict::Rejected(SubtitleRule::Forced)
|
SubtitleVerdict::Rejected(SubtitleRule::Forced)
|
||||||
@@ -171,6 +178,38 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn ranking_reports_where_each_candidate_came_from() {
|
||||||
|
// Ranking reorders, and a candidate carries no id of its own, so the
|
||||||
|
// index is the only way back to the provider candidate — and so to
|
||||||
|
// the id the provider is asked to download.
|
||||||
|
let weak = plain();
|
||||||
|
let strong = SubtitleCandidate {
|
||||||
|
moviehash: Some("abc123"),
|
||||||
|
..plain()
|
||||||
|
};
|
||||||
|
let ranked = rank(&target(), &[weak, strong]);
|
||||||
|
|
||||||
|
assert_eq!(ranked[0].index, 1);
|
||||||
|
assert_eq!(ranked[1].index, 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_rejected_candidate_still_reports_its_index() {
|
||||||
|
let forced = SubtitleCandidate {
|
||||||
|
forced: true,
|
||||||
|
..plain()
|
||||||
|
};
|
||||||
|
let ranked = rank(&target(), &[forced, plain()]);
|
||||||
|
|
||||||
|
assert_eq!(ranked[0].index, 1);
|
||||||
|
assert_eq!(ranked[1].index, 0);
|
||||||
|
assert_eq!(
|
||||||
|
ranked[1].verdict,
|
||||||
|
SubtitleVerdict::Rejected(SubtitleRule::Forced)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_moviehash_match_wins_outright_over_every_other_tier() {
|
fn a_moviehash_match_wins_outright_over_every_other_tier() {
|
||||||
let hash_match = SubtitleCandidate {
|
let hash_match = SubtitleCandidate {
|
||||||
|
|||||||
@@ -152,6 +152,46 @@ pub struct Candidate {
|
|||||||
pub sdh: bool,
|
pub sdh: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl Candidate {
|
||||||
|
/// The same candidate in the shape `arr_core::subs::rank` eats.
|
||||||
|
///
|
||||||
|
/// The two types are deliberately separate — this crate reports what a
|
||||||
|
/// provider said, `arr-core` decides what it means — so the mapping
|
||||||
|
/// between them is decided here, once, rather than reinvented by every
|
||||||
|
/// caller:
|
||||||
|
///
|
||||||
|
/// * `hash_match` is a boolean here because that is what a provider
|
||||||
|
/// answers, while ranking compares hashes. A hash match is therefore
|
||||||
|
/// expressed by handing ranking `target_moviehash`; no match is `None`,
|
||||||
|
/// which never compares equal.
|
||||||
|
/// * `rating` and `download_count` are optional here because a provider
|
||||||
|
/// may not report them. Both are tiebreakers, so a missing one sorts
|
||||||
|
/// last rather than blocking the candidate: `0.0` and `0`.
|
||||||
|
///
|
||||||
|
/// `language` is dropped: ranking runs over one wanted language at a
|
||||||
|
/// time, so the caller has already filtered by it.
|
||||||
|
#[must_use]
|
||||||
|
pub fn to_core<'a>(
|
||||||
|
&'a self,
|
||||||
|
target_moviehash: Option<&'a str>,
|
||||||
|
) -> arr_core::subs::SubtitleCandidate<'a> {
|
||||||
|
arr_core::subs::SubtitleCandidate {
|
||||||
|
forced: self.forced,
|
||||||
|
hearing_impaired: self.sdh,
|
||||||
|
moviehash: if self.hash_match {
|
||||||
|
target_moviehash
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
},
|
||||||
|
release_name: self.release_name.as_deref(),
|
||||||
|
release_group: self.group.as_deref(),
|
||||||
|
source: self.source,
|
||||||
|
uploader_rating: f64::from(self.rating.unwrap_or(0.0)),
|
||||||
|
download_count: self.download_count.unwrap_or(0),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// The wire format of a downloaded subtitle.
|
/// The wire format of a downloaded subtitle.
|
||||||
///
|
///
|
||||||
/// Sidecars on disk are SRT (DESIGN.md §15), but providers serve other things
|
/// Sidecars on disk are SRT (DESIGN.md §15), but providers serve other things
|
||||||
@@ -200,7 +240,54 @@ pub struct Fetched {
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::{CandidateId, ProviderId, SubtitleFormat};
|
use arr_core::Language;
|
||||||
|
|
||||||
|
use super::{Candidate, CandidateId, ProviderId, SubtitleFormat};
|
||||||
|
|
||||||
|
fn candidate() -> Candidate {
|
||||||
|
Candidate {
|
||||||
|
provider: ProviderId::new("opensubtitles"),
|
||||||
|
id: CandidateId::new("1"),
|
||||||
|
language: Language::PortuguesePortugal,
|
||||||
|
hash_match: false,
|
||||||
|
release_name: Some("Movie.2024.1080p.WEB-DL-GROUP".into()),
|
||||||
|
group: Some("GROUP".into()),
|
||||||
|
source: None,
|
||||||
|
rating: None,
|
||||||
|
download_count: None,
|
||||||
|
forced: false,
|
||||||
|
sdh: true,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_hash_match_is_handed_to_ranking_as_the_target_hash() {
|
||||||
|
let matched = Candidate {
|
||||||
|
hash_match: true,
|
||||||
|
..candidate()
|
||||||
|
};
|
||||||
|
assert_eq!(matched.to_core(Some("abc123")).moviehash, Some("abc123"));
|
||||||
|
// No match must never compare equal, whatever the target carries.
|
||||||
|
let unmatched = candidate();
|
||||||
|
assert_eq!(unmatched.to_core(Some("abc123")).moviehash, None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn missing_tiebreakers_sort_last_rather_than_blocking_a_candidate() {
|
||||||
|
let candidate = candidate();
|
||||||
|
let core = candidate.to_core(None);
|
||||||
|
assert!((core.uploader_rating - 0.0).abs() < f64::EPSILON);
|
||||||
|
assert_eq!(core.download_count, 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn the_sdh_and_group_facts_survive_the_rename_across_the_seam() {
|
||||||
|
let candidate = candidate();
|
||||||
|
let core = candidate.to_core(None);
|
||||||
|
assert!(core.hearing_impaired);
|
||||||
|
assert!(!core.forced);
|
||||||
|
assert_eq!(core.release_group, Some("GROUP"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn ids_render_as_the_string_they_wrap() {
|
fn ids_render_as_the_string_they_wrap() {
|
||||||
|
|||||||
Reference in New Issue
Block a user