diff --git a/src-tauri/src/repository/capabilities.rs b/src-tauri/src/repository/capabilities.rs new file mode 100644 index 000000000..60c7a77c4 --- /dev/null +++ b/src-tauri/src/repository/capabilities.rs @@ -0,0 +1,416 @@ +//! What the server on the other end of the wire can actually do. +//! +//! One `ServerCapabilities` value is resolved per connection, from the version +//! the server already reports at `/System/Info/Public`, and every decision that +//! depends on the server generation reads a **named flag** from it. +//! +//! # Why flags and not version comparisons +//! +//! A `version < N` written at the point of use re-derives a domain fact where it +//! is consumed — the same error as a Jellyfin taxonomy in the frontend, and the +//! reason `check:boundary` exists. It is also unreadable by its second +//! occurrence (`< 11` says nothing about *what* changed), and it cannot express +//! a backport, where a behaviour appears in a patch release of an older line. +//! +//! So the version → flags mapping lives in exactly one function +//! ([`ServerCapabilities::for_version`]) and nothing else in the crate compares +//! a version number. +//! +//! # Why an unknown version resolves forward +//! +//! A server newer than this build resolves to the newest capability set we know +//! rather than being refused. Refusing would make every JellyTau release expire +//! the moment the server upgrades, which is the failure UR-085 exists to remove. +//! Refusal is reserved for a version *below* [`MINIMUM_SUPPORTED_MAJOR_MINOR`], +//! where failure is certain rather than merely likely. +//! +//! TRACES: UR-085 | IR-035, DR-280 + +use std::fmt; + +/// The oldest server this build will talk to, as `(major, minor)`. +/// +/// This is the current target and not a researched floor: no older server has +/// been tested against, so claiming support for one would be a guess. Lower it +/// when a real server has been exercised, not before. +pub const MINIMUM_SUPPORTED_MAJOR_MINOR: (u32, u32) = (10, 10); + +/// A parsed server version. +/// +/// Jellyfin reports things like `10.11.5`, `10.11.5.0` and occasionally a +/// build suffix (`10.11.5-rc1`). Only the leading numeric components are +/// meaningful here; anything after them is preserved in `raw` for logging and +/// otherwise ignored. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerVersion { + pub major: u32, + pub minor: u32, + pub patch: u32, + pub raw: String, +} + +impl ServerVersion { + /// Parse what `/System/Info/Public` reported. + /// + /// Returns `None` for anything without at least a numeric major, which is + /// treated as "unknown" rather than as an error — an unparseable version is + /// not a reason to refuse a server that may work perfectly well. + pub fn parse(raw: &str) -> Option { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return None; + } + + // Stop at the first character that cannot begin a numeric component, so + // `10.11.5-rc1` and `10.11.5+build7` both yield 10.11.5. + let numeric_prefix: String = trimmed + .chars() + .take_while(|c| c.is_ascii_digit() || *c == '.') + .collect(); + + let mut parts = numeric_prefix.split('.').filter(|p| !p.is_empty()); + let major = parts.next()?.parse().ok()?; + let minor = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0); + let patch = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0); + + Some(Self { + major, + minor, + patch, + raw: trimmed.to_string(), + }) + } + + fn is_below_floor(&self) -> bool { + (self.major, self.minor) < MINIMUM_SUPPORTED_MAJOR_MINOR + } +} + +impl fmt::Display for ServerVersion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}.{}.{}", self.major, self.minor, self.patch) + } +} + +/// How this build classified the server it is talking to. +/// +/// There are exactly two live cases, and the gap between them is not a typo: +/// **Jellyfin 11.0 does not exist and never did.** With 12.0 the project dropped +/// the leading `10` from its scheme, so what would have been 10.12.0 shipped as +/// `12.0` and the server reports `Version: "12.0.0"`. 12.0 is therefore *one* +/// release-branch step from 10.11, not two, and `major == 11` will never occur. +/// +/// Source: , which explicitly +/// flags version-string parsers as the thing to check before upgrading. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ServerGeneration { + /// The 10.x line — `major == 10`. What this client was built against. + V10_11, + /// The post-rename line — `major >= 12`. + V12Plus, + /// The server did not report a parseable version. Treated as the older + /// generation, which is the conservative choice: its flags are the ones that + /// also work on 12.x. + Unknown, +} + +/// The resolved answer, carried by `OnlineRepository` for the life of a +/// connection. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ServerCapabilities { + pub version: Option, + pub generation: ServerGeneration, + + /// Whether item queries go to `/Users/{userId}/Items` (`true`) or to + /// `/Items?userId=` (`false`). + /// + /// **`true` for every generation, and deliberately so.** The whole + /// `/Users/{userId}/…` family still exists and still works in 12.0 — only + /// six routes were removed anywhere, and the only user-scoped one is + /// `POST /Users/{userId}/EasyPassword`, which this client never called. + /// + /// What *did* change is policy: the family has carried `[Obsolete]` and been + /// hidden from the OpenAPI spec since 10.11.5, and 12.0 states in writing + /// that unspecified endpoints "can be removed in any major release without + /// warning". The replacements (`/Items?userId=` and friends) already exist + /// on 10.11.5, so migrating is a one-generation-compatible change whenever + /// it is wanted — which is why the route table carries both shapes even + /// though nothing selects the second one yet. See DR-282. + pub user_scoped_item_routes: bool, + + /// Whether the server honours the **audio codec** in a submitted + /// `DirectPlayProfile`. + /// + /// `false` on 10.11.5: it enforces the profile's container and video codec + /// but ignores its audio codec, so it offers direct play for an E-AC-3 track + /// the renderer cannot decode and the picture plays in silence. The client + /// therefore has to overrule the server's own direct-play offer. See + /// `device_profile::audio_forces_transcode` and DR-283. + /// + /// **Still `false` on 12.x, and that is an admission rather than a finding.** + /// A source-level diff of 12.0 could not establish whether the underlying + /// behaviour changed; it established only that 12.0 *reports* codec + /// mismatches in `TranscodeReasons` which 10.11.5 omitted, which is not the + /// same claim. Keeping the override on costs a transcode that might not be + /// needed; turning it off on a guess costs silent playback. Flip it only + /// against a running 12.x server. + pub honours_directplay_audio_codec: bool, + + /// Whether a source whose container is a *manifest* (`hls`, `applehttp`, + /// `dash`) may be direct-played. 12.0 makes such sources ineligible; on + /// 10.11.x they were eligible, which is what this client has assumed. + pub supports_manifest_container_direct_play: bool, + + /// Whether asking the image endpoint for a size larger than the stored image + /// returns that size. 10.11.x upscaled; 12.0 returns the original instead. + /// Governs layout expectation only — a smaller image is never an error. + pub image_endpoint_upscales: bool, +} + +impl ServerCapabilities { + /// The single place a version becomes behaviour. Nothing else in the crate + /// compares a version number. + pub fn for_version(version: Option) -> Self { + let generation = match &version { + None => ServerGeneration::Unknown, + // `major >= 12` and `major == 10` are the two live cases; 11 will + // never occur. A hypothetical 11 sorts with the older line, which is + // the conservative side. + Some(v) if v.major >= 12 => ServerGeneration::V12Plus, + Some(_) => ServerGeneration::V10_11, + }; + + let v12 = generation == ServerGeneration::V12Plus; + + Self { + version, + generation, + // Unchanged across both generations — see each flag's docs. Note the + // two genuinely breaking changes 12.0 introduced (the auth spelling + // and the `Recursive` default) are fixed by writing the request + // correctly for *both*, so neither appears here. A flag is a silent + // branch that outlives the reason it was added; keep them for + // genuine either/or behaviour only. + user_scoped_item_routes: true, + honours_directplay_audio_codec: false, + supports_manifest_container_direct_play: !v12, + image_endpoint_upscales: !v12, + } + } + + /// Resolve straight from what the server reported. + pub fn from_reported(raw_version: &str) -> Self { + Self::for_version(ServerVersion::parse(raw_version)) + } + + /// What this build assumes with no server to ask — the current target. + /// Used by offline paths and by tests that do not care. + pub fn assumed() -> Self { + Self::for_version(None) + } + + /// Whether the server is old enough that failure is certain rather than + /// likely. An unparseable version is never below the floor: we do not refuse + /// a server on the strength of not understanding its version string. + pub fn is_below_supported_floor(&self) -> bool { + self.version.as_ref().is_some_and(|v| v.is_below_floor()) + } +} + +impl Default for ServerCapabilities { + fn default() -> Self { + Self::assumed() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// TRACES: UR-085 | DR-280 + #[test] + fn parses_the_shapes_a_real_server_reports() { + assert_eq!( + ServerVersion::parse("10.11.5").unwrap().to_string(), + "10.11.5" + ); + // Four components: Jellyfin reports these, the fourth is ignored. + assert_eq!( + ServerVersion::parse("10.11.5.0").unwrap().to_string(), + "10.11.5" + ); + // A pre-release suffix must not defeat parsing. + assert_eq!( + ServerVersion::parse("10.11.5-rc1").unwrap().to_string(), + "10.11.5" + ); + assert_eq!( + ServerVersion::parse("10.11.5+build7").unwrap().to_string(), + "10.11.5" + ); + // Missing components default rather than failing. + assert_eq!(ServerVersion::parse("11").unwrap().to_string(), "11.0.0"); + assert_eq!( + ServerVersion::parse(" 10.10 ").unwrap().to_string(), + "10.10.0" + ); + } + + /// Nonsense is "unknown", never a panic and never a refusal. + /// + /// TRACES: UR-085 | DR-280, DR-286 + #[test] + fn unparseable_versions_are_unknown_not_fatal() { + for raw in ["", " ", "not-a-version", "v", "-", "..."] { + assert!( + ServerVersion::parse(raw).is_none(), + "{raw:?} should not parse" + ); + } + let caps = ServerCapabilities::from_reported("not-a-version"); + assert_eq!(caps.generation, ServerGeneration::Unknown); + assert!( + !caps.is_below_supported_floor(), + "an unreadable version must not refuse a server that may work" + ); + } + + /// A server newer than this build keeps working. Refusing it would make + /// every release expire the moment the server upgrades. + /// + /// TRACES: UR-085 | DR-286 + #[test] + fn a_newer_than_known_server_resolves_forward() { + let newer = ServerCapabilities::from_reported("99.0.0"); + assert_eq!(newer.generation, ServerGeneration::V12Plus); + assert!(!newer.is_below_supported_floor()); + + // It resolves to the newest known generation's flags; only the recorded + // version differs. + let known = ServerCapabilities::from_reported("12.0.0"); + assert_eq!( + newer, + ServerCapabilities { + version: newer.version.clone(), + ..known + } + ); + } + + /// The version scheme changed: 12.0 *is* 10.12 renamed, so 11 never occurs + /// and a parser must not assume a leading `10.`. + /// + /// TRACES: UR-085 | DR-280 + #[test] + fn the_two_live_generations_are_10_and_12_with_no_11() { + assert_eq!( + ServerCapabilities::from_reported("10.11.5").generation, + ServerGeneration::V10_11 + ); + assert_eq!( + ServerCapabilities::from_reported("12.0.0").generation, + ServerGeneration::V12Plus + ); + // 11 cannot be reported by any real server; if one somehow does, it + // sorts with the older line rather than being treated as newer. + assert_eq!( + ServerCapabilities::from_reported("11.0.0").generation, + ServerGeneration::V10_11 + ); + } + + /// The flags that genuinely differ, and only those. + /// + /// TRACES: UR-085 | DR-283 + #[test] + fn manifest_direct_play_and_upscaling_are_the_flags_that_differ() { + let old = ServerCapabilities::from_reported("10.11.5"); + let new = ServerCapabilities::from_reported("12.0.0"); + + assert!(old.supports_manifest_container_direct_play); + assert!(!new.supports_manifest_container_direct_play); + assert!(old.image_endpoint_upscales); + assert!(!new.image_endpoint_upscales); + + // The two breaking changes 12.0 introduced are NOT flags: they are fixed + // by writing the request correctly for both generations. + assert_eq!(old.user_scoped_item_routes, new.user_scoped_item_routes); + assert_eq!( + old.honours_directplay_audio_codec, new.honours_directplay_audio_codec, + "unestablished against a running 12.x server; must not be flipped on a guess" + ); + } + + /// Nothing may reintroduce an authentication spelling that 12.0 disables by + /// default. The header *value* is correct on both generations; only the + /// names were deprecated, so this is a structural guard. + /// + /// TRACES: UR-085 | DR-287 + #[test] + fn no_deprecated_auth_spelling_reaches_a_request_builder() { + let sources: &[(&str, &str)] = &[ + ("repository/online.rs", include_str!("online.rs")), + ("jellyfin/client.rs", include_str!("../jellyfin/client.rs")), + ( + "jellyfin/http_client.rs", + include_str!("../jellyfin/http_client.rs"), + ), + ("auth/mod.rs", include_str!("../auth/mod.rs")), + ]; + + for (name, src) in sources { + assert!( + !src.contains(r#".header("X-Emby-Authorization""#), + "{name}: X-Emby-Authorization is disabled by default on Jellyfin 12.0 \ + (a migration flips it on upgraded servers too). Use `Authorization` \ + with the same MediaBrowser value — ungated on both generations." + ); + assert!( + !src.contains(r#".header("X-Emby-Token""#) + && !src.contains(r#".header("X-MediaBrowser-Token""#), + "{name}: token headers are gated behind EnableLegacyAuthorization on 12.0" + ); + assert!( + !src.contains("api_key="), + "{name}: `api_key` as a query parameter is gated on 12.0. Use `ApiKey`, \ + ungated on both and what the server itself emits." + ); + } + } + + /// TRACES: UR-085 | DR-286 + #[test] + fn a_server_below_the_floor_is_refused() { + assert!(ServerCapabilities::from_reported("10.9.11").is_below_supported_floor()); + assert!(ServerCapabilities::from_reported("9.0.0").is_below_supported_floor()); + assert!(!ServerCapabilities::from_reported("10.10.0").is_below_supported_floor()); + assert!(!ServerCapabilities::from_reported("10.11.5").is_below_supported_floor()); + } + + /// The documented 10.11.5 behaviour, pinned so that flipping it later is a + /// deliberate act with a citation rather than a drive-by edit. + /// + /// TRACES: UR-085 | DR-283 + #[test] + fn the_current_target_does_not_honour_directplay_audio_codec() { + let caps = ServerCapabilities::from_reported("10.11.5"); + assert_eq!(caps.generation, ServerGeneration::V10_11); + assert!( + !caps.honours_directplay_audio_codec, + "10.11.5 ignores a DirectPlayProfile's audio codec; the client must overrule it" + ); + } + + /// No generation may quietly acquire an unverified route change. + /// + /// TRACES: UR-085 | DR-282 + #[test] + fn no_generation_yet_disables_user_scoped_routes() { + for raw in ["10.10.0", "10.11.5", "11.0.0", "12.0.0", "99.9.9"] { + assert!( + ServerCapabilities::from_reported(raw).user_scoped_item_routes, + "{raw}: flipping this needs a cited upstream source (DR-282), not a guess" + ); + } + } +} diff --git a/src-tauri/src/repository/endpoints.rs b/src-tauri/src/repository/endpoints.rs new file mode 100644 index 000000000..79f861e87 --- /dev/null +++ b/src-tauri/src/repository/endpoints.rs @@ -0,0 +1,858 @@ +//! Every Jellyfin route the online repository speaks, in one place. +//! +//! Before this module the endpoints were 57 inline `format!` literals scattered +//! through `online.rs`, query strings baked in at the point of use. That is +//! workable against exactly one server, and hostile to anything else: a second +//! route shape means a conditional at every one of them. +//! +//! Each function here takes `&ServerCapabilities` and returns a **path** +//! (`/Users/…`), except the handful documented as returning an absolute URL +//! because they are handed to a media player rather than to the JSON helpers. +//! +//! # Percent-encoding +//! +//! Values are encoded, syntax is not. A genre named `Drama & Romance` or a +//! search for `a?b` must not split into another parameter. [`Endpoint::param`] +//! encodes; [`Endpoint::raw_param`] does not and is for values this module +//! itself composed (numbers, and lists whose separator is meaningful to +//! Jellyfin — `IncludeItemTypes` splits on `,`, `Genres` on `|`, so the +//! separator survives while each element is encoded). +//! +//! TRACES: UR-085 | DR-279 + +use super::capabilities::ServerCapabilities; +use super::types::{GetItemsOptions, SearchScope}; + +/// A path plus query string, which knows whether it needs `?` or `&` next. +/// +/// The manual separator juggling this replaces produced the double-ampersand and +/// trailing-ampersand cases an earlier test file spent four assertions on. +/// Making it structural is cheaper than testing for it. +pub struct Endpoint { + buf: String, + has_query: bool, +} + +impl Endpoint { + pub fn new(path: &str) -> Self { + // A caller may hand in a path that already carries a query. + let has_query = path.contains('?'); + Self { + buf: path.to_string(), + has_query, + } + } + + fn separator(&mut self) -> char { + if self.has_query { + '&' + } else { + self.has_query = true; + '?' + } + } + + /// Append `key=value`, percent-encoding the value. + pub fn param(mut self, key: &str, value: &str) -> Self { + let sep = self.separator(); + self.buf + .push_str(&format!("{}{}={}", sep, key, urlencoding::encode(value))); + self + } + + /// Append `key=value` verbatim. Only for values this module composed. + pub fn raw_param(mut self, key: &str, value: &str) -> Self { + let sep = self.separator(); + self.buf.push_str(&format!("{}{}={}", sep, key, value)); + self + } + + pub fn build(self) -> String { + self.buf + } +} + +/// Encode each element of a list while keeping the separator Jellyfin splits on. +fn encode_list(values: impl IntoIterator>, separator: &str) -> String { + values + .into_iter() + .map(|v| urlencoding::encode(v.as_ref()).into_owned()) + .collect::>() + .join(separator) +} + +/// The base for a user-scoped item query. +/// +/// This is the one place the two route shapes differ, and the reason the route +/// table exists at all. `user_scoped_item_routes` is `true` for every generation +/// today — see the flag's own documentation for why flipping it needs a cited +/// source rather than a guess (DR-282). +fn user_items_root(caps: &ServerCapabilities, user_id: &str) -> Endpoint { + if caps.user_scoped_item_routes { + Endpoint::new(&format!("/Users/{}/Items", user_id)) + } else { + Endpoint::new("/Items").param("userId", user_id) + } +} + +/// The standard field set for a list view. `People` is deliberately absent — it +/// is only wanted in the detail view, and it is not small. +const LIST_FIELDS: &str = "BackdropImageTags,ParentBackdropImageTags,UserData"; + +/// As [`LIST_FIELDS`], plus what the offline store needs to derive genre lists +/// and per-genre counts from cached rows. +const LIST_FIELDS_WITH_GENRES: &str = + "BackdropImageTags,ParentBackdropImageTags,Genres,PremiereDate,UserData"; + +// ===== Libraries and items ===== + +/// The user's library views. +/// +/// TRACES: UR-007, UR-085 | JA-003, DR-279 +pub fn user_views(_caps: &ServerCapabilities, user_id: &str) -> String { + format!("/Users/{}/Views", user_id) +} + +/// One item, in detail. `People`, `MediaStreams` and `MediaSources` are named +/// here and nowhere else — the detail view is the only place they are wanted. +/// +/// TRACES: UR-007, UR-085 | JA-005, DR-279 +pub fn item_detail(caps: &ServerCapabilities, user_id: &str, item_id: &str) -> String { + let base = if caps.user_scoped_item_routes { + Endpoint::new(&format!( + "/Users/{}/Items/{}", + user_id, + urlencoding::encode(item_id) + )) + } else { + Endpoint::new(&format!("/Items/{}", urlencoding::encode(item_id))).param("userId", user_id) + }; + base.raw_param( + "Fields", + "BackdropImageTags,ParentBackdropImageTags,People,MediaStreams,MediaSources,PremiereDate,UserData", + ) + .build() +} + +/// A folder listing. +/// +/// Every value is percent-encoded before it goes into the query string: these +/// are values, not URL syntax, so a space or an `&` in one must not split it +/// into another parameter. +/// +/// TRACES: UR-007, UR-067, UR-085 | DR-116, DR-212, DR-279 | UT-104, UT-206 +pub fn get_items( + caps: &ServerCapabilities, + user_id: &str, + parent_id: &str, + options: Option<&GetItemsOptions>, +) -> String { + let mut ep = user_items_root(caps, user_id).param("ParentId", parent_id); + + if let Some(opts) = options { + if let Some(limit) = opts.limit { + ep = ep.raw_param("Limit", &limit.to_string()); + } + if let Some(start_index) = opts.start_index { + ep = ep.raw_param("StartIndex", &start_index.to_string()); + } + if let Some(types) = &opts.include_item_types { + // The comma is the list separator Jellyfin splits on, so encode + // each type rather than the joined string. + ep = ep.raw_param("IncludeItemTypes", &encode_list(types, ",")); + } + + // An explicit sort always wins; the container's default only fills the + // gap when the caller named none. A caller that names neither gets no + // SortBy at all, leaving the server's own order intact. + // + // TRACES: UR-007 | DR-257 | UT-229 + let default_sort = super::types::default_listing_sort(opts.parent_kind); + let sort_by = opts + .sort_by + .as_deref() + .or(default_sort.map(|(field, _)| field)); + let sort_order = opts + .sort_order + .as_deref() + .or(default_sort.map(|(_, order)| order)); + + if let Some(sort_by) = sort_by { + // SortBy is likewise comma-delimited ("ParentIndexNumber,IndexNumber, + // SortName"), so encode per field. + ep = ep.raw_param("SortBy", &encode_list(sort_by.split(','), ",")); + } + if let Some(sort_order) = sort_order { + ep = ep.param("SortOrder", sort_order); + } + // Jellyfin 12.0 defaults `recursive` to true when the parent is a + // library folder and `IncludeItemTypes` is set, where 10.11 listed only + // immediate children — the same request, a different result set. State + // it explicitly whenever a type filter is present so both generations + // agree, and state the behaviour that shipped rather than adopting the + // new server-side default silently. + // + // TRACES: UR-085 | DR-288 + let type_filtered = opts + .include_item_types + .as_ref() + .is_some_and(|types| !types.is_empty()); + match (opts.recursive, type_filtered) { + (Some(recursive), _) => ep = ep.raw_param("Recursive", &recursive.to_string()), + (None, true) => ep = ep.raw_param("Recursive", "false"), + (None, false) => {} + } + if let Some(genres) = &opts.genres { + if !genres.is_empty() { + // Genre names may contain spaces or ampersands; `|` is the + // separator Jellyfin splits this one on. + ep = ep.raw_param("Genres", &encode_list(genres, "|")); + } + } + // TRACES: UR-067 | DR-116 | UT-104 + if opts.favorites_only == Some(true) { + ep = ep.raw_param("Filters", "IsFavorite"); + } + } + + ep.raw_param("Fields", LIST_FIELDS_WITH_GENRES).build() +} + +/// A "recently added" listing. +/// +/// `GroupItems=true` is the load-bearing parameter: Jellyfin defaults it to +/// `false`, which returns each newly-added *leaf* separately, so importing one +/// 14-track album pushed 14 rows into "recently added" and buried everything +/// else. With grouping on, the server collapses children into the container +/// that was added — an album appears once, while movies (which have no such +/// container) are unaffected. +/// +/// TRACES: UR-024, UR-034, UR-085 | IR-024, JA-016, DR-279 +pub fn latest_items( + caps: &ServerCapabilities, + user_id: &str, + parent_id: &str, + limit: Option, +) -> String { + let base = if caps.user_scoped_item_routes { + Endpoint::new(&format!("/Users/{}/Items/Latest", user_id)) + } else { + Endpoint::new("/Items/Latest").param("userId", user_id) + }; + base.param("ParentId", parent_id) + .raw_param("Limit", &limit.unwrap_or(16).to_string()) + .raw_param("GroupItems", "true") + .raw_param("Fields", LIST_FIELDS) + .build() +} + +/// The resume ("Continue Watching") listing. +/// +/// TRACES: UR-019, UR-085 | JA-013, DR-279 +pub fn resume_items( + caps: &ServerCapabilities, + user_id: &str, + limit: usize, + include_item_types: Option<&str>, + parent_id: Option<&str>, +) -> String { + let base = if caps.user_scoped_item_routes { + Endpoint::new(&format!("/Users/{}/Items/Resume", user_id)) + } else { + Endpoint::new("/Items/Resume").param("userId", user_id) + }; + let ep = base + .raw_param("Limit", &limit.to_string()) + .raw_param("MediaTypes", "Video"); + let ep = match include_item_types { + Some(types) => ep.raw_param("IncludeItemTypes", types), + None => ep, + }; + let ep = ep.raw_param("Fields", LIST_FIELDS); + match parent_id { + Some(pid) => ep.param("ParentId", pid).build(), + None => ep.build(), + } +} + +/// A Next Up listing. +/// +/// `EnableResumable=false` is the point of this query: the server default is +/// `true`, which makes a partially-watched episode its own series' "next up" — +/// the very episode `/Items/Resume` returns — so Continue Watching and Next Up +/// end up showing the same cards. Servers predating the parameter ignore it, +/// which is why the frontend also drops in-progress entries (DR-197). +/// +/// TRACES: UR-023, UR-059, UR-085 | DR-197, DR-279, JA-014, JA-036 | UT-190, UT-191 +pub fn next_up( + _caps: &ServerCapabilities, + user_id: &str, + series_id: Option<&str>, + limit: Option, +) -> String { + let ep = Endpoint::new("/Shows/NextUp") + .param("UserId", user_id) + .raw_param("Limit", &limit.unwrap_or(16).to_string()) + .raw_param("EnableResumable", "false") + .raw_param("Fields", LIST_FIELDS); + + match series_id { + Some(sid) => ep.param("SeriesId", sid).build(), + None => ep.build(), + } +} + +/// A favourites listing. +/// +/// `scope` is expanded here — `SearchScope::All` yields `None`, and the +/// `IncludeItemTypes` filter is then **omitted entirely** rather than sent as a +/// union, which would silently drop every type nobody enumerated (see +/// `SearchScope::item_types`). +/// +/// TRACES: UR-067, UR-085 | DR-115, DR-279, JA-033 | UT-100 +pub fn favorites( + caps: &ServerCapabilities, + user_id: &str, + scope: SearchScope, + options: Option<&GetItemsOptions>, +) -> String { + let mut ep = user_items_root(caps, user_id) + .raw_param("Filters", "IsFavorite") + .raw_param("Recursive", "true"); + + if let Some(types) = scope.item_types() { + ep = ep.raw_param("IncludeItemTypes", &types.join(",")); + } + + // Jellyfin has no "date favourited", so name order is the only stable sort + // available; callers may still override it. + let sort_by = options + .and_then(|o| o.sort_by.as_deref()) + .unwrap_or("SortName"); + let sort_order = options + .and_then(|o| o.sort_order.as_deref()) + .unwrap_or("Ascending"); + ep = ep + .raw_param("SortBy", sort_by) + .raw_param("SortOrder", sort_order); + + if let Some(limit) = options.and_then(|o| o.limit) { + ep = ep.raw_param("Limit", &limit.to_string()); + } + if let Some(start_index) = options.and_then(|o| o.start_index) { + ep = ep.raw_param("StartIndex", &start_index.to_string()); + } + + ep.raw_param("Fields", LIST_FIELDS_WITH_GENRES).build() +} + +/// Items sorted by when they were last played, filtered to played ones. +/// +/// TRACES: UR-034, UR-085 | DR-279 +pub fn played_items_by_date( + caps: &ServerCapabilities, + user_id: &str, + include_item_types: &str, + limit: usize, + sort_order: &str, + parent_id: Option<&str>, +) -> String { + let ep = user_items_root(caps, user_id) + .raw_param("SortBy", "DatePlayed") + .raw_param("SortOrder", sort_order) + .raw_param("IncludeItemTypes", include_item_types) + .raw_param("Limit", &limit.to_string()) + .raw_param("Recursive", "true") + .raw_param("Filters", "IsPlayed") + .raw_param("Fields", LIST_FIELDS); + match parent_id { + Some(pid) => ep.param("ParentId", pid).build(), + None => ep.build(), + } +} + +/// Genres, with the item counts the frontend uses to pick a diverse subset. +/// +/// TRACES: UR-085 | DR-279 +pub fn genres( + _caps: &ServerCapabilities, + user_id: &str, + include_item_types: &str, + parent_id: Option<&str>, +) -> String { + let ep = Endpoint::new("/Genres") + .param("UserId", user_id) + .raw_param("IncludeItemTypes", include_item_types) + .raw_param("Recursive", "true") + .raw_param("Fields", "ItemCounts"); + match parent_id { + Some(pid) => ep.param("ParentId", pid).build(), + None => ep.build(), + } +} + +/// A search. +/// +/// TRACES: UR-085 | DR-279 +pub fn search( + caps: &ServerCapabilities, + user_id: &str, + term: &str, + limit: usize, + include_item_types: Option<&[String]>, +) -> String { + let ep = user_items_root(caps, user_id) + .param("SearchTerm", term) + .raw_param("Limit", &limit.to_string()) + .raw_param("Recursive", "true"); + match include_item_types { + Some(types) if !types.is_empty() => ep + .raw_param("IncludeItemTypes", &encode_list(types, ",")) + .build(), + _ => ep.build(), + } +} + +/// A person's filmography. +/// +/// TRACES: UR-036, UR-085 | JA-031, DR-279 +pub fn items_by_person( + caps: &ServerCapabilities, + user_id: &str, + person_id: &str, + limit: usize, + include_item_types: Option<&[String]>, +) -> String { + let ep = user_items_root(caps, user_id) + .param("PersonIds", person_id) + .raw_param("Limit", &limit.to_string()) + .raw_param("Recursive", "true") + .raw_param("Fields", LIST_FIELDS); + match include_item_types { + Some(types) if !types.is_empty() => ep + .raw_param("IncludeItemTypes", &encode_list(types, ",")) + .build(), + _ => ep.build(), + } +} + +/// A person as an item. +/// +/// Jellyfin serves people through the ordinary user-item endpoint rather than +/// anything under `/Persons`; the cast entries on an item's `People` field carry +/// the ids this is called with. +/// +/// TRACES: UR-035, UR-036, UR-085 | IR-022, JA-030, DR-279 +pub fn person(caps: &ServerCapabilities, user_id: &str, person_id: &str) -> String { + if caps.user_scoped_item_routes { + format!( + "/Users/{}/Items/{}", + user_id, + urlencoding::encode(person_id) + ) + } else { + Endpoint::new(&format!("/Items/{}", urlencoding::encode(person_id))) + .param("userId", user_id) + .build() + } +} + +/// Items similar to one item. +/// +/// TRACES: UR-085 | DR-279 +pub fn similar_items( + _caps: &ServerCapabilities, + item_id: &str, + user_id: &str, + limit: usize, +) -> String { + Endpoint::new(&format!("/Items/{}/Similar", urlencoding::encode(item_id))) + .param("UserId", user_id) + .raw_param("Limit", &limit.to_string()) + .raw_param("Fields", LIST_FIELDS) + .build() +} + +// ===== User data mutations ===== + +/// Favourite / un-favourite an item (POST to set, DELETE to clear). +/// +/// TRACES: UR-067, UR-085 | JA-033, DR-279 +pub fn favorite_item(_caps: &ServerCapabilities, user_id: &str, item_id: &str) -> String { + format!( + "/Users/{}/FavoriteItems/{}", + user_id, + urlencoding::encode(item_id) + ) +} + +/// Mark played / clear watch history (POST to set, DELETE to clear). +/// +/// TRACES: UR-025, UR-085 | JA-035, DR-279 +pub fn played_item(_caps: &ServerCapabilities, user_id: &str, item_id: &str) -> String { + format!( + "/Users/{}/PlayedItems/{}", + user_id, + urlencoding::encode(item_id) + ) +} + +// ===== Playback ===== + +/// Playback negotiation for one item. +/// +/// TRACES: UR-004, UR-085 | JA-021, DR-279 +pub fn playback_info(_caps: &ServerCapabilities, item_id: &str) -> String { + format!("/Items/{}/PlaybackInfo", urlencoding::encode(item_id)) +} + +/// Playback reporting. +/// +/// TRACES: UR-020, UR-085 | JA-010, JA-011, JA-012, DR-279 +pub fn sessions_playing(_caps: &ServerCapabilities) -> &'static str { + "/Sessions/Playing" +} +pub fn sessions_playing_progress(_caps: &ServerCapabilities) -> &'static str { + "/Sessions/Playing/Progress" +} +pub fn sessions_playing_stopped(_caps: &ServerCapabilities) -> &'static str { + "/Sessions/Playing/Stopped" +} + +/// Live TV channels. +/// +/// TRACES: UR-085 | DR-279 +pub fn live_tv_channels(_caps: &ServerCapabilities, user_id: &str) -> String { + Endpoint::new("/LiveTv/Channels") + .param("UserId", user_id) + .raw_param("Fields", "PrimaryImageAspectRatio,Overview") + .raw_param("EnableImageTypes", "Primary") + .build() +} + +/// Generic channels. +/// +/// TRACES: UR-085 | DR-279 +pub fn channels(_caps: &ServerCapabilities, user_id: &str) -> String { + Endpoint::new("/Channels").param("UserId", user_id).build() +} + +// ===== Playlists ===== + +/// TRACES: UR-062, UR-085 | DR-279 +pub fn playlists(_caps: &ServerCapabilities) -> &'static str { + "/Playlists" +} + +/// A playlist as an item — used for rename and delete, which are `/Items` +/// operations rather than `/Playlists` ones. +/// +/// TRACES: UR-062, UR-085 | DR-279 +pub fn playlist_as_item(_caps: &ServerCapabilities, playlist_id: &str) -> String { + format!("/Items/{}", urlencoding::encode(playlist_id)) +} + +/// TRACES: UR-062, UR-085 | DR-279 +pub fn playlist_items(_caps: &ServerCapabilities, playlist_id: &str, user_id: &str) -> String { + Endpoint::new(&format!( + "/Playlists/{}/Items", + urlencoding::encode(playlist_id) + )) + .param("UserId", user_id) + .raw_param( + "Fields", + "PrimaryImageTag,Artists,AlbumId,Album,AlbumArtist,RunTimeTicks,ArtistItems", + ) + .raw_param("StartIndex", "0") + .raw_param("Limit", "10000") + .build() +} + +/// TRACES: UR-062, UR-085 | DR-279 +pub fn playlist_items_add(_caps: &ServerCapabilities, playlist_id: &str, ids: &str) -> String { + Endpoint::new(&format!( + "/Playlists/{}/Items", + urlencoding::encode(playlist_id) + )) + .param("Ids", ids) + .build() +} + +/// TRACES: UR-062, UR-085 | DR-279 +pub fn playlist_items_remove( + _caps: &ServerCapabilities, + playlist_id: &str, + entry_ids: &str, +) -> String { + Endpoint::new(&format!( + "/Playlists/{}/Items", + urlencoding::encode(playlist_id) + )) + .param("EntryIds", entry_ids) + .build() +} + +/// TRACES: UR-062, UR-085 | DR-279 +pub fn playlist_item_move( + _caps: &ServerCapabilities, + playlist_id: &str, + item_id: &str, + new_index: u32, +) -> String { + format!( + "/Playlists/{}/Items/{}/Move/{}", + urlencoding::encode(playlist_id), + urlencoding::encode(item_id), + new_index + ) +} + +// ===== Plugin ===== + +/// The JRay plugin's per-item context. Not core Jellyfin; absent servers 404 and +/// the caller treats that as "no context", so it needs no capability flag. +/// +/// TRACES: UR-085 | DR-279 +pub fn jray_context(_caps: &ServerCapabilities, item_id: &str, position_seconds: f64) -> String { + format!( + "/Plugins/JRay/Items/{}/jray?t={}", + urlencoding::encode(item_id), + position_seconds + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn caps() -> ServerCapabilities { + ServerCapabilities::assumed() + } + + /// The builder must never emit a double or trailing separator, and must use + /// `?` exactly once. This is structural now rather than asserted at every + /// call site. + /// + /// TRACES: UR-085 | DR-279 + #[test] + fn query_separators_are_structural() { + let url = Endpoint::new("/Items") + .param("a", "1") + .param("b", "2") + .raw_param("c", "3") + .build(); + assert_eq!(url, "/Items?a=1&b=2&c=3"); + assert_eq!(url.matches('?').count(), 1); + assert!(!url.contains("&&")); + assert!(!url.ends_with('&')); + + // A path that already carries a query continues it rather than + // starting a second one. + let continued = Endpoint::new("/Items?x=0").param("y", "1").build(); + assert_eq!(continued, "/Items?x=0&y=1"); + assert_eq!(continued.matches('?').count(), 1); + + // No parameters at all means no `?`. + assert_eq!(Endpoint::new("/Items").build(), "/Items"); + } + + /// Values are encoded, list separators are not. + /// + /// TRACES: UR-007, UR-085 | DR-212, DR-279 | UT-206 + #[test] + fn values_are_encoded_but_list_separators_survive() { + let url = Endpoint::new("/x").param("SearchTerm", "a?b&c d").build(); + assert!(url.contains("SearchTerm=a%3Fb%26c%20d"), "{url}"); + + assert_eq!( + encode_list(["Drama & Romance", "Sci-Fi"], "|"), + "Drama%20%26%20Romance|Sci-Fi" + ); + assert_eq!(encode_list(["Movie", "Series"], ","), "Movie,Series"); + } + + /// The user-scoped split is the reason this module exists. Both shapes must + /// be well-formed, and the default must be byte-identical to what shipped. + /// + /// TRACES: UR-085 | DR-279, DR-282 + #[test] + fn both_user_scoped_route_shapes_are_well_formed() { + let legacy = caps(); + assert!(legacy.user_scoped_item_routes, "the shipped default"); + let url = get_items(&legacy, "u1", "lib-1", None); + assert!(url.starts_with("/Users/u1/Items?ParentId=lib-1"), "{url}"); + + let mut modern = caps(); + modern.user_scoped_item_routes = false; + let url = get_items(&modern, "u1", "lib-1", None); + assert!(url.starts_with("/Items?userId=u1&ParentId=lib-1"), "{url}"); + assert_eq!(url.matches('?').count(), 1, "{url}"); + assert!(!url.contains("/Users/"), "{url}"); + } + + /// Every route must be well-formed under *both* shapes — a flipped flag + /// must not produce a malformed URL anywhere. + /// + /// TRACES: UR-085 | DR-279, DR-282 + #[test] + fn no_route_is_malformed_under_either_shape() { + for user_scoped in [true, false] { + let mut c = caps(); + c.user_scoped_item_routes = user_scoped; + + let routes = vec![ + user_views(&c, "u1"), + item_detail(&c, "u1", "i1"), + get_items(&c, "u1", "p1", None), + latest_items(&c, "u1", "p1", Some(8)), + resume_items(&c, "u1", 10, None, None), + resume_items(&c, "u1", 10, Some("Movie"), Some("lib-9")), + next_up(&c, "u1", Some("s1"), Some(5)), + favorites(&c, "u1", SearchScope::All, None), + played_items_by_date(&c, "u1", "Audio", 20, "Descending", None), + genres(&c, "u1", "MusicAlbum", Some("lib-1")), + search(&c, "u1", "query", 25, Some(&["Movie".to_string()])), + items_by_person(&c, "u1", "p9", 50, None), + person(&c, "u1", "p9"), + similar_items(&c, "i1", "u1", 12), + favorite_item(&c, "u1", "i1"), + played_item(&c, "u1", "i1"), + playback_info(&c, "i1"), + live_tv_channels(&c, "u1"), + channels(&c, "u1"), + playlist_as_item(&c, "pl1"), + playlist_items(&c, "pl1", "u1"), + playlist_items_add(&c, "pl1", "a,b"), + playlist_items_remove(&c, "pl1", "e1"), + playlist_item_move(&c, "pl1", "i1", 3u32), + jray_context(&c, "i1", 42.5), + ]; + + for route in routes { + assert!(route.starts_with('/'), "{route}"); + assert!(!route.contains("&&"), "{route}"); + assert!(!route.contains("?&"), "{route}"); + assert!(!route.ends_with('&'), "{route}"); + assert!(!route.ends_with('?'), "{route}"); + assert!( + route.matches('?').count() <= 1, + "more than one query separator: {route}" + ); + } + } + } + + /// TRACES: UR-024, UR-034 | IR-024, JA-016 + #[test] + fn latest_items_groups_children_into_containers() { + let url = latest_items(&caps(), "u1", "lib-1", Some(16)); + assert!(url.contains("GroupItems=true"), "{url}"); + assert!(url.contains("ParentId=lib-1"), "{url}"); + assert!(url.contains("Limit=16"), "{url}"); + } + + /// TRACES: UR-059 | DR-197, JA-036 | UT-190, UT-191 + #[test] + fn next_up_excludes_resumable_and_scopes_to_series() { + let url = next_up(&caps(), "u1", None, Some(12)); + assert!(url.contains("EnableResumable=false"), "{url}"); + assert!(url.contains("UserId=u1"), "{url}"); + assert!(url.contains("Limit=12"), "{url}"); + assert!(!url.contains("SeriesId"), "{url}"); + + let scoped = next_up(&caps(), "u1", Some("series-a"), None); + assert!(scoped.contains("SeriesId=series-a"), "{scoped}"); + assert!(scoped.contains("Limit=16"), "default limit: {scoped}"); + } + + /// `All` must omit the type filter entirely rather than send a union, which + /// would silently drop every type nobody enumerated. + /// + /// TRACES: UR-067 | DR-115 | UT-100 + #[test] + fn favorites_all_scope_omits_the_type_filter() { + let url = favorites(&caps(), "u1", SearchScope::All, None); + assert!(!url.contains("IncludeItemTypes"), "{url}"); + assert!(url.contains("Filters=IsFavorite"), "{url}"); + } + + /// TRACES: UR-067 | DR-115 | UT-100 + #[test] + fn favorites_honours_paging_and_sort() { + let url = favorites( + &caps(), + "u1", + SearchScope::All, + Some(&GetItemsOptions { + limit: Some(20), + start_index: Some(40), + sort_by: Some("Random".to_string()), + sort_order: Some("Descending".to_string()), + ..Default::default() + }), + ); + assert!(url.contains("&Limit=20"), "{url}"); + assert!(url.contains("&StartIndex=40"), "{url}"); + assert!(url.contains("&SortBy=Random&SortOrder=Descending"), "{url}"); + } + + /// The detail view is the only caller that wants People/MediaStreams; a list + /// query must not drag them along. + /// + /// Jellyfin 12.0 changed `GetItems` to default `recursive` to **true** when + /// the parent is a library folder and `IncludeItemTypes` is set — so the + /// identical request returns a different result set on the two generations. + /// Sending an explicit value makes them agree, and `false` is what shipped. + /// + /// Source: `ItemsController.cs` in v12.0 — `if (folder is ICollectionFolder + /// && includeItemTypes.Length > 0) { recursive ??= true; }` + /// + /// TRACES: UR-085 | DR-288 + #[test] + fn a_type_filtered_listing_always_states_recursive() { + let filtered = get_items( + &caps(), + "u1", + "lib-1", + Some(&GetItemsOptions { + include_item_types: Some(vec!["Movie".to_string()]), + ..Default::default() + }), + ); + assert!( + filtered.contains("Recursive="), + "a type-filtered listing must state Recursive or 12.0 will infer a \ + different one than 10.11: {filtered}" + ); + assert!( + filtered.contains("Recursive=false"), + "and it must state the behaviour that shipped: {filtered}" + ); + + // An explicit choice by the caller still wins. + let explicit = get_items( + &caps(), + "u1", + "lib-1", + Some(&GetItemsOptions { + include_item_types: Some(vec!["Movie".to_string()]), + recursive: Some(true), + ..Default::default() + }), + ); + assert!(explicit.contains("Recursive=true"), "{explicit}"); + assert_eq!(explicit.matches("Recursive=").count(), 1, "{explicit}"); + + // No type filter, no inference to defend against, no parameter. + let plain = get_items(&caps(), "u1", "lib-1", None); + assert!(!plain.contains("Recursive="), "{plain}"); + } + + /// TRACES: UR-007 | DR-279 + #[test] + fn only_the_detail_route_requests_the_heavy_fields() { + assert!(item_detail(&caps(), "u1", "i1").contains("People")); + assert!(!get_items(&caps(), "u1", "p1", None).contains("People")); + assert!(!latest_items(&caps(), "u1", "p1", None).contains("MediaStreams")); + } +} diff --git a/src-tauri/src/repository/mod.rs b/src-tauri/src/repository/mod.rs index b490bc5fd..b8b2c07e8 100644 --- a/src-tauri/src/repository/mod.rs +++ b/src-tauri/src/repository/mod.rs @@ -1,10 +1,16 @@ +pub mod capabilities; pub mod device_profile; +pub mod endpoints; /// User-chosen browsing exclusions (UR-076 / DR-209). pub mod exclusions; +#[cfg(test)] +mod generation_tests; pub mod hybrid; pub mod offline; pub mod online; pub mod series_progress; +#[cfg(test)] +pub mod server_fixture; /// Backend-owned stream selection (UR-079 / DR-225). pub mod stream_selection; pub mod types; diff --git a/src-tauri/src/repository/online.rs b/src-tauri/src/repository/online.rs index 3d7398aeb..2a5dc4c61 100644 --- a/src-tauri/src/repository/online.rs +++ b/src-tauri/src/repository/online.rs @@ -5,6 +5,8 @@ use log::{debug, error, info, warn}; use serde::{Deserialize, Serialize}; use std::sync::{Arc, RwLock}; +use super::capabilities::ServerCapabilities; +use super::endpoints; use super::stream_selection::{ quality_options_for_source, PlaybackKind, Rendition, StreamSelection, Transport, }; @@ -188,6 +190,12 @@ pub struct OnlineRepository { /// This is the source of truth for the offline/online banner. `None` in /// tests / contexts where connectivity tracking isn't wired up. connectivity: Option, + /// What this server can do, resolved once from the version it reported at + /// connect. Every route and every version-dependent decision reads a named + /// flag from here; nothing compares a version number. + /// + /// TRACES: UR-085 | IR-035, DR-280 + capabilities: ServerCapabilities, } impl OnlineRepository { @@ -209,9 +217,34 @@ impl OnlineRepository { user_id, access_token, connectivity: None, + // Assumed until the caller supplies what the server reported. The + // assumption is the current target, which is what it will be in + // nearly every case. + capabilities: ServerCapabilities::assumed(), } } + /// Adopt the capabilities resolved from the version the server reported at + /// connect. Without this the repository assumes the current target. + /// + /// TRACES: UR-085 | IR-035, DR-280 + pub fn with_capabilities(mut self, capabilities: ServerCapabilities) -> Self { + self.capabilities = capabilities; + self + } + + /// What the server on the other end can do. + /// + /// Test-only: production reads the flags through the route table and the + /// playback paths rather than asking the repository for them, so exposing + /// this outside tests would be an accessor nobody calls. + /// + /// TRACES: UR-085 | DR-280 + #[cfg(test)] + pub fn capabilities(&self) -> &ServerCapabilities { + &self.capabilities + } + /// Attach a connectivity reporter so server outcomes drive the reachability /// state observed by the UI. See `report_outcome`. pub fn with_connectivity(mut self, reporter: ConnectivityReporter) -> Self { @@ -261,7 +294,7 @@ impl OnlineRepository { .http_client .client .get(url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .build() .map_err(|e| format!("Failed to build request: {}", e))?; @@ -298,11 +331,7 @@ impl OnlineRepository { item_id: &str, t: f64, ) -> Result, RepoError> { - let endpoint = format!( - "/Plugins/JRay/Items/{}/jray?t={}", - urlencoding::encode(item_id), - t - ); + let endpoint = endpoints::jray_context(&self.capabilities, item_id, t); match self.get_json::(&endpoint).await { Ok(context) => Ok(context.actors), // No plugin / no truth data for this item — not an error to the user. @@ -339,7 +368,7 @@ impl OnlineRepository { .http_client .client .get(&url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .build() .map_err(|e| RepoError::Network { message: format!("Failed to build request: {}", e), @@ -414,7 +443,7 @@ impl OnlineRepository { .client .post(&url) .header("Content-Type", "application/json") - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .json(body) .build() .map_err(|e| RepoError::Network { @@ -474,7 +503,7 @@ impl OnlineRepository { .client .post(&url) .header("Content-Type", "application/json") - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .json(body) .build() .map_err(|e| RepoError::Network { @@ -538,7 +567,7 @@ impl OnlineRepository { .http_client .client .delete(&url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .send(); match request.await { @@ -630,7 +659,7 @@ impl OnlineRepository { // TRACES: UR-004, UR-080 | DR-234 let (renderer_video_codecs, _) = super::device_profile::renderer_codecs(); let mut params = vec![ - ("api_key", self.access_token.clone()), + ("ApiKey", self.access_token.clone()), ("DeviceId", DEVICE_ID.to_string()), ("PlaySessionId", play_session_id), ("VideoCodec", renderer_video_codecs), @@ -724,7 +753,7 @@ impl OnlineRepository { ) -> Result { let mut params = vec![ ("UserId", self.user_id.clone()), - ("api_key", self.access_token.clone()), + ("ApiKey", self.access_token.clone()), ("DeviceId", DEVICE_ID.to_string()), // Progressive mp3 over HTTP — ExoPlayer-friendly; no HLS/ts. ("Container", "mp3".to_string()), @@ -783,7 +812,7 @@ impl OnlineRepository { &self, item_id: &str, ) -> Result<(NegotiatedSource, String), RepoError> { - let endpoint = format!("/Items/{}/PlaybackInfo", urlencoding::encode(item_id)); + let endpoint = endpoints::playback_info(&self.capabilities, item_id); // What the renderer that will decode this can play. One source, shared // with the transcode URL builder and the client-side audio override, so @@ -1043,7 +1072,7 @@ impl OnlineRepository { // (which is the *video* stream — the index is global across all // streams) only misleads servers that do honour it. let url = format!( - "{}/Videos/{}/stream?static=true&container=mp4&mediaSourceId={}&deviceId={}&api_key={}&userId={}", + "{}/Videos/{}/stream?static=true&container=mp4&mediaSourceId={}&deviceId={}&ApiKey={}&userId={}", self.server_url, item_id, effective_source_id, @@ -1191,119 +1220,26 @@ impl From for UserData { } } -/// Build the Jellyfin endpoint for a folder listing. +/// Test-only shim over [`endpoints::get_items`]. /// -/// Extracted from `get_items` so the query it produces — in particular the -/// favourites filter — can be asserted without standing up an HTTP server. -/// -/// TRACES: UR-007, UR-067 | DR-116 | UT-104 +/// The endpoint builders moved to `endpoints.rs` under DR-279. These wrappers +/// keep the existing requirement coverage (DR-116, DR-212, DR-257 and friends) +/// pointed at the production path rather than deleting it, and pin the *default* +/// capability shape — the URLs that shipped before the route table existed. +#[cfg(test)] fn build_get_items_endpoint( user_id: &str, parent_id: &str, options: Option<&GetItemsOptions>, ) -> String { - // Every value below is percent-encoded before it goes into the query - // string, the same way `Genres` and `SearchTerm` already are: these are - // values, not URL syntax, so a space or an `&` in one must not split it - // into another parameter. - // - // TRACES: UR-007 | DR-212 | UT-206 - let mut endpoint = format!( - "/Users/{}/Items?ParentId={}", - user_id, - urlencoding::encode(parent_id) - ); - - if let Some(opts) = options { - if let Some(limit) = opts.limit { - endpoint.push_str(&format!("&Limit={}", limit)); - } - if let Some(start_index) = opts.start_index { - endpoint.push_str(&format!("&StartIndex={}", start_index)); - } - if let Some(types) = &opts.include_item_types { - // Encode each type, not the joined string: the comma is the - // list separator Jellyfin splits on. - let encoded: Vec = types - .iter() - .map(|t| urlencoding::encode(t).into_owned()) - .collect(); - endpoint.push_str(&format!("&IncludeItemTypes={}", encoded.join(","))); - } - // An explicit sort always wins; the container's default only fills the - // gap when the caller named none. A caller that names neither gets no - // SortBy at all, leaving the server's own order intact. - // - // TRACES: UR-007 | DR-257 | UT-229 - let default_sort = default_listing_sort(opts.parent_kind); - let sort_by = opts - .sort_by - .as_deref() - .or(default_sort.map(|(field, _)| field)); - let sort_order = opts - .sort_order - .as_deref() - .or(default_sort.map(|(_, order)| order)); - - if let Some(sort_by) = sort_by { - // SortBy is likewise a comma-delimited list (`hybrid.rs` sends - // "ParentIndexNumber,IndexNumber,SortName"), so encode per field. - let encoded: Vec = sort_by - .split(',') - .map(|field| urlencoding::encode(field).into_owned()) - .collect(); - endpoint.push_str(&format!("&SortBy={}", encoded.join(","))); - } - if let Some(sort_order) = sort_order { - endpoint.push_str(&format!("&SortOrder={}", urlencoding::encode(sort_order))); - } - if let Some(recursive) = opts.recursive { - endpoint.push_str(&format!("&Recursive={}", recursive)); - } - if let Some(genres) = &opts.genres { - if !genres.is_empty() { - // Genre names may contain spaces/ampersands, so percent-encode each. - let encoded: Vec = genres - .iter() - .map(|g| urlencoding::encode(g).into_owned()) - .collect(); - endpoint.push_str(&format!("&Genres={}", encoded.join("|"))); - } - } - // TRACES: UR-067 | DR-116 | UT-104 - if opts.favorites_only == Some(true) { - endpoint.push_str("&Filters=IsFavorite"); - } - } - - // Request image fields for list views (People only needed in get_item - // detail view). Genres is needed so cached items carry their genres, - // which lets the offline store derive genre lists + per-genre counts. - endpoint - .push_str("&Fields=BackdropImageTags,ParentBackdropImageTags,Genres,PremiereDate,UserData"); - endpoint + endpoints::get_items(&ServerCapabilities::assumed(), user_id, parent_id, options) } -/// Build the Jellyfin endpoint for a "recently added" listing. -/// -/// `GroupItems=true` is the load-bearing parameter: Jellyfin defaults it to -/// `false`, which returns each newly-added *leaf* separately, so importing one -/// 14-track album pushed 14 rows into "recently added" and buried everything -/// else. With grouping on, the server collapses children into the container -/// that was added — an album appears once, while movies (which have no such -/// container) are unaffected. -/// -/// Pulled out of `get_latest_items` so the query can be asserted without an -/// HTTP server, matching `build_favorites_endpoint`. -/// -/// TRACES: UR-024, UR-034 | IR-024, JA-016 +/// Test-only shim over [`endpoints::latest_items`]. See +/// [`build_get_items_endpoint`]. +#[cfg(test)] fn build_latest_items_endpoint(user_id: &str, parent_id: &str, limit: Option) -> String { - format!( - "/Users/{}/Items/Latest?ParentId={}&Limit={}&GroupItems=true&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - user_id, - parent_id, - limit.unwrap_or(16) - ) + endpoints::latest_items(&ServerCapabilities::assumed(), user_id, parent_id, limit) } /// How many rows to ask the server for, given how many the row will show. @@ -1417,73 +1353,21 @@ fn album_from_track(track: &MediaItem, album_id: String) -> MediaItem { } } -/// Build the Jellyfin endpoint for a Next Up listing. -/// -/// `EnableResumable=false` is the point of this query: the server default is -/// `true`, which makes a partially-watched episode its own series' "next up" — -/// the very episode `/Items/Resume` returns — so Continue Watching and Next Up -/// end up showing the same cards. Next Up should only ever offer episodes the -/// viewer has not started. Servers predating the parameter ignore it, which is -/// why the frontend also drops in-progress entries (DR-197). -/// -/// Pulled out of `get_next_up_episodes` so the query can be asserted without an -/// HTTP server, matching `build_favorites_endpoint`. -/// -/// TRACES: UR-023, UR-059 | DR-197, JA-014, JA-036 | UT-190, UT-191 +/// Test-only shim over [`endpoints::next_up`]. See [`build_get_items_endpoint`]. +#[cfg(test)] fn build_next_up_endpoint(user_id: &str, series_id: Option<&str>, limit: Option) -> String { - let mut endpoint = format!( - "/Shows/NextUp?UserId={}&Limit={}&EnableResumable=false&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - user_id, - limit.unwrap_or(16) - ); - - if let Some(sid) = series_id { - endpoint.push_str(&format!("&SeriesId={}", sid)); - } - - endpoint + endpoints::next_up(&ServerCapabilities::assumed(), user_id, series_id, limit) } -/// Build the Jellyfin endpoint for a favourites listing. -/// -/// Pulled out of `get_favorites` so the query can be asserted without an HTTP -/// server. `scope` is expanded here — `SearchScope::All` yields `None`, and the -/// `IncludeItemTypes` filter is then **omitted entirely** rather than sent as a -/// union, which would silently drop every type nobody enumerated (see -/// `SearchScope::item_types`). -/// -/// TRACES: UR-067 | DR-115, JA-033 | UT-100 +/// Test-only shim over [`endpoints::favorites`]. See +/// [`build_get_items_endpoint`]. +#[cfg(test)] fn build_favorites_endpoint( user_id: &str, scope: SearchScope, options: Option<&GetItemsOptions>, ) -> String { - let mut endpoint = format!("/Users/{}/Items?Filters=IsFavorite&Recursive=true", user_id); - - if let Some(types) = scope.item_types() { - endpoint.push_str(&format!("&IncludeItemTypes={}", types.join(","))); - } - - // Jellyfin has no "date favourited", so name order is the only stable sort - // available; callers may still override it. - let sort_by = options - .and_then(|o| o.sort_by.as_deref()) - .unwrap_or("SortName"); - let sort_order = options - .and_then(|o| o.sort_order.as_deref()) - .unwrap_or("Ascending"); - endpoint.push_str(&format!("&SortBy={}&SortOrder={}", sort_by, sort_order)); - - if let Some(limit) = options.and_then(|o| o.limit) { - endpoint.push_str(&format!("&Limit={}", limit)); - } - if let Some(start_index) = options.and_then(|o| o.start_index) { - endpoint.push_str(&format!("&StartIndex={}", start_index)); - } - - endpoint - .push_str("&Fields=BackdropImageTags,ParentBackdropImageTags,Genres,PremiereDate,UserData"); - endpoint + endpoints::favorites(&ServerCapabilities::assumed(), user_id, scope, options) } // ImageTags from Jellyfin API - can be a HashMap with various image type keys @@ -1810,7 +1694,7 @@ impl MediaRepository for OnlineRepository { image_tags: Option, } - let endpoint = format!("/Users/{}/Views", self.user_id); + let endpoint = endpoints::user_views(&self.capabilities, &self.user_id); let response: LibrariesResponse = self.get_json(&endpoint).await?; Ok(response @@ -1832,7 +1716,12 @@ impl MediaRepository for OnlineRepository { parent_id: &str, options: Option, ) -> Result { - let endpoint = build_get_items_endpoint(&self.user_id, parent_id, options.as_ref()); + let endpoint = endpoints::get_items( + &self.capabilities, + &self.user_id, + parent_id, + options.as_ref(), + ); let response: ItemsResponse = self.get_json(&endpoint).await?; @@ -1858,7 +1747,7 @@ impl MediaRepository for OnlineRepository { /// /// TRACES: UR-021, UR-035 | IR-016, IR-022, JA-005, JA-009 async fn get_item(&self, item_id: &str) -> Result { - let endpoint = format!("/Users/{}/Items/{}?Fields=BackdropImageTags,ParentBackdropImageTags,People,MediaStreams,MediaSources,PremiereDate,UserData", self.user_id, urlencoding::encode(item_id)); + let endpoint = endpoints::item_detail(&self.capabilities, &self.user_id, item_id); let item: JellyfinItem = self.get_json(&endpoint).await?; let media_item = item.into_media_item(self.user_id.clone()); @@ -1879,7 +1768,8 @@ impl MediaRepository for OnlineRepository { limit: Option, ) -> Result, RepoError> { let limit_val = limit.unwrap_or(16); - let endpoint = build_latest_items_endpoint( + let endpoint = endpoints::latest_items( + &self.capabilities, &self.user_id, parent_id, Some(latest_items_fetch_limit(limit_val)), @@ -1909,16 +1799,14 @@ impl MediaRepository for OnlineRepository { parent_id: Option<&str>, limit: Option, ) -> Result, RepoError> { - let limit_str = limit.unwrap_or(16); - let mut endpoint = format!( - "/Users/{}/Items/Resume?Limit={}&MediaTypes=Video&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - self.user_id, limit_str + let endpoint = endpoints::resume_items( + &self.capabilities, + &self.user_id, + limit.unwrap_or(16), + None, + parent_id, ); - if let Some(pid) = parent_id { - endpoint.push_str(&format!("&ParentId={}", pid)); - } - let response: ItemsResponse = self.get_json(&endpoint).await?; Ok(response .items @@ -1936,7 +1824,7 @@ impl MediaRepository for OnlineRepository { series_id: Option<&str>, limit: Option, ) -> Result, RepoError> { - let endpoint = build_next_up_endpoint(&self.user_id, series_id, limit); + let endpoint = endpoints::next_up(&self.capabilities, &self.user_id, series_id, limit); let response: ItemsResponse = self.get_json(&endpoint).await?; Ok(response @@ -1953,9 +1841,13 @@ impl MediaRepository for OnlineRepository { let limit_val = limit.unwrap_or(12); // Fetch more items to account for grouping reducing the count let fetch_limit = limit_val * 3; - let endpoint = format!( - "/Users/{}/Items?SortBy=DatePlayed&SortOrder=Descending&IncludeItemTypes=Audio&Limit={}&Recursive=true&Filters=IsPlayed&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - self.user_id, fetch_limit + let endpoint = endpoints::played_items_by_date( + &self.capabilities, + &self.user_id, + "Audio", + fetch_limit, + "Descending", + None, ); let response: ItemsResponse = self.get_json(&endpoint).await?; @@ -2087,15 +1979,15 @@ impl MediaRepository for OnlineRepository { // Ask Jellyfin for played albums sorted by least-recently played first. // Filters=IsPlayed keeps only albums the user has actually listened to, // and SortBy=DatePlayed ascending surfaces the ones they've neglected. - let mut endpoint = format!( - "/Users/{}/Items?SortBy=DatePlayed&SortOrder=Ascending&IncludeItemTypes=MusicAlbum&Limit={}&Recursive=true&Filters=IsPlayed&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - self.user_id, limit_val + let endpoint = endpoints::played_items_by_date( + &self.capabilities, + &self.user_id, + "MusicAlbum", + limit_val, + "Ascending", + parent_id, ); - if let Some(pid) = parent_id { - endpoint.push_str(&format!("&ParentId={}", pid)); - } - let response: ItemsResponse = self.get_json(&endpoint).await?; Ok(response .items @@ -2110,10 +2002,12 @@ impl MediaRepository for OnlineRepository { /// /// TRACES: UR-019, UR-034 | IR-024, JA-013, JA-015 async fn get_resume_movies(&self, limit: Option) -> Result, RepoError> { - let limit_str = limit.unwrap_or(16); - let endpoint = format!( - "/Users/{}/Items/Resume?Limit={}&MediaTypes=Video&IncludeItemTypes=Movie&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - self.user_id, limit_str + let endpoint = endpoints::resume_items( + &self.capabilities, + &self.user_id, + limit.unwrap_or(16), + Some("Movie"), + None, ); let response: ItemsResponse = self.get_json(&endpoint).await?; @@ -2127,14 +2021,8 @@ impl MediaRepository for OnlineRepository { async fn get_genres(&self, parent_id: Option<&str>) -> Result, RepoError> { // Ask Jellyfin to scope counts to albums and include them, so the // frontend can rank genres by popularity without probing each one. - let mut endpoint = format!( - "/Genres?UserId={}&IncludeItemTypes=MusicAlbum&Recursive=true&Fields=ItemCounts", - self.user_id - ); - - if let Some(pid) = parent_id { - endpoint.push_str(&format!("&ParentId={}", pid)); - } + let endpoint = + endpoints::genres(&self.capabilities, &self.user_id, "MusicAlbum", parent_id); #[derive(Debug, Deserialize)] #[serde(rename_all = "PascalCase")] @@ -2201,28 +2089,12 @@ impl MediaRepository for OnlineRepository { // SearchTerm is arbitrary user input and must be percent-encoded so that // spaces, ampersands, etc. don't corrupt the query string (a multi-word // search like "Star Wars" would otherwise produce a malformed URL). - let mut endpoint = format!( - "/Users/{}/Items?SearchTerm={}&Limit={}&Recursive=true", - self.user_id, - urlencoding::encode(query), - limit - ); - - if let Some(opts) = options { - if let Some(types) = opts.include_item_types { - let encoded_types = types - .iter() - .map(|t| urlencoding::encode(t).into_owned()) - .collect::>() - .join(","); - endpoint.push_str(&format!("&IncludeItemTypes={}", encoded_types)); - } - } - - // Request image fields for list views (plus Genres so cached items - // carry genres for offline genre lists/counts). - endpoint.push_str( - "&Fields=BackdropImageTags,ParentBackdropImageTags,Genres,PremiereDate,UserData", + let endpoint = endpoints::search( + &self.capabilities, + &self.user_id, + query, + limit, + options.and_then(|o| o.include_item_types).as_deref(), ); let response: ItemsResponse = self.get_json(&endpoint).await?; @@ -2311,7 +2183,7 @@ impl MediaRepository for OnlineRepository { // serves the original file untouched, and pinning index 0 (the video // stream) only misleads servers that do honour it. format!( - "{}/Videos/{}/stream?static=true&container=mp4&mediaSourceId={}&deviceId=jellytau&api_key={}&userId={}", + "{}/Videos/{}/stream?static=true&container=mp4&mediaSourceId={}&deviceId=jellytau&ApiKey={}&userId={}", self.server_url, item_id, source.id, @@ -2335,7 +2207,7 @@ impl MediaRepository for OnlineRepository { async fn get_audio_stream_url(&self, item_id: &str) -> Result { // Construct direct audio stream URL let url = format!( - "{}/Audio/{}/stream?UserId={}&api_key={}&Static=true", + "{}/Audio/{}/stream?UserId={}&ApiKey={}&Static=true", self.server_url, item_id, self.user_id, self.access_token ); Ok(url) @@ -2360,10 +2232,7 @@ impl MediaRepository for OnlineRepository { async fn get_live_tv_channels(&self) -> Result, RepoError> { // Live TV channels (broadcast tuners / IPTV M3U). Returned as items with // type "TvChannel" — playable via open_live_stream. - let endpoint = format!( - "/LiveTv/Channels?UserId={}&Fields=PrimaryImageAspectRatio,Overview&EnableImageTypes=Primary", - self.user_id - ); + let endpoint = endpoints::live_tv_channels(&self.capabilities, &self.user_id); let response: ItemsResponse = self.get_json(&endpoint).await?; Ok(response .items @@ -2375,7 +2244,7 @@ impl MediaRepository for OnlineRepository { async fn get_channels(&self) -> Result { // Root list of plugin "Channels". Drill-down into a channel folder reuses // get_items(channel_id, ...). - let endpoint = format!("/Channels?UserId={}", self.user_id); + let endpoint = endpoints::channels(&self.capabilities, &self.user_id); let response: ItemsResponse = self.get_json(&endpoint).await?; let total = response.total_record_count; let items = response @@ -2426,7 +2295,7 @@ impl MediaRepository for OnlineRepository { live_stream_id: Option, } - let endpoint = format!("/Items/{}/PlaybackInfo", urlencoding::encode(item_id)); + let endpoint = endpoints::playback_info(&self.capabilities, item_id); let request = OpenLiveStreamRequest { user_id: self.user_id.clone(), auto_open_live_stream: true, @@ -2461,7 +2330,7 @@ impl MediaRepository for OnlineRepository { super::device_profile::without_server_chosen_subtitle(&url) ), None => format!( - "{}/Videos/{}/master.m3u8?api_key={}&MediaSourceId={}&LiveStreamId={}&VideoCodec=h264&AudioCodec=aac&TranscodingProtocol=hls&TranscodingContainer=ts&SubtitleStreamIndex={}", + "{}/Videos/{}/master.m3u8?ApiKey={}&MediaSourceId={}&LiveStreamId={}&VideoCodec=h264&AudioCodec=aac&TranscodingProtocol=hls&TranscodingContainer=ts&SubtitleStreamIndex={}", self.server_url, item_id, self.access_token, @@ -2503,7 +2372,8 @@ impl MediaRepository for OnlineRepository { is_paused: false, }; - self.post_json("/Sessions/Playing", &request).await + self.post_json(endpoints::sessions_playing(&self.capabilities), &request) + .await } async fn report_playback_progress( @@ -2525,7 +2395,11 @@ impl MediaRepository for OnlineRepository { is_paused: false, }; - self.post_json("/Sessions/Playing/Progress", &request).await + self.post_json( + endpoints::sessions_playing_progress(&self.capabilities), + &request, + ) + .await } async fn report_playback_stopped( @@ -2545,7 +2419,11 @@ impl MediaRepository for OnlineRepository { position_ticks, }; - self.post_json("/Sessions/Playing/Stopped", &request).await + self.post_json( + endpoints::sessions_playing_stopped(&self.capabilities), + &request, + ) + .await } fn get_image_url( @@ -2561,9 +2439,10 @@ impl MediaRepository for OnlineRepository { image_type.as_str() ); - // Authentication is handled by X-Emby-Authorization header in download_bytes() - // Do NOT include api_key here — some Jellyfin servers reject requests when - // api_key is present but the token doesn't match the expected format. + // Authentication is handled by the `Authorization` header in + // download_bytes(). Do NOT add a query-parameter token here — some + // Jellyfin servers reject requests carrying one whose format they do not + // expect, and this request can already authenticate by header. let mut params: Vec = Vec::new(); if let Some(opts) = options { @@ -2623,7 +2502,7 @@ impl MediaRepository for OnlineRepository { // instead — it is always present and supports HTTP Range, which the // download worker relies on for resume. let mut url = format!("{}/Videos/{}/stream.mp4", self.server_url, item_id); - let mut params = vec![format!("api_key={}", self.access_token)]; + let mut params = vec![format!("ApiKey={}", self.access_token)]; // Map the frontend quality preset to concrete transcode params. For // "original" we request a direct static copy (no transcode) which is @@ -2713,11 +2592,7 @@ impl MediaRepository for OnlineRepository { } async fn mark_favorite(&self, item_id: &str) -> Result<(), RepoError> { - let endpoint = format!( - "/Users/{}/FavoriteItems/{}", - self.user_id, - urlencoding::encode(item_id) - ); + let endpoint = endpoints::favorite_item(&self.capabilities, &self.user_id, item_id); self.post_json(&endpoint, &serde_json::json!({})).await } @@ -2727,7 +2602,8 @@ impl MediaRepository for OnlineRepository { scope: SearchScope, options: Option, ) -> Result { - let endpoint = build_favorites_endpoint(&self.user_id, scope, options.as_ref()); + let endpoint = + endpoints::favorites(&self.capabilities, &self.user_id, scope, options.as_ref()); let response: ItemsResponse = self.get_json(&endpoint).await?; Ok(SearchResult { @@ -2747,11 +2623,7 @@ impl MediaRepository for OnlineRepository { /// /// TRACES: UR-017 | JA-018, DR-021 async fn unmark_favorite(&self, item_id: &str) -> Result<(), RepoError> { - let endpoint = format!( - "/Users/{}/FavoriteItems/{}", - self.user_id, - urlencoding::encode(item_id) - ); + let endpoint = endpoints::favorite_item(&self.capabilities, &self.user_id, item_id); let url = format!("{}{}", self.server_url, endpoint); let result = async { @@ -2759,7 +2631,7 @@ impl MediaRepository for OnlineRepository { .http_client .client .delete(&url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .build() .map_err(|e| RepoError::Network { message: format!("Failed to build request: {}", e), @@ -2793,11 +2665,7 @@ impl MediaRepository for OnlineRepository { /// /// TRACES: UR-064 | DR-106, JA-033 async fn clear_watch_history(&self, item_id: &str) -> Result<(), RepoError> { - let endpoint = format!( - "/Users/{}/PlayedItems/{}", - self.user_id, - urlencoding::encode(item_id) - ); + let endpoint = endpoints::played_item(&self.capabilities, &self.user_id, item_id); let url = format!("{}{}", self.server_url, endpoint); let result = async { @@ -2805,7 +2673,7 @@ impl MediaRepository for OnlineRepository { .http_client .client .delete(&url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .build() .map_err(|e| RepoError::Network { message: format!("Failed to build request: {}", e), @@ -2838,11 +2706,7 @@ impl MediaRepository for OnlineRepository { /// /// TRACES: UR-025 | DR-131 | JA-035 async fn mark_played(&self, item_id: &str) -> Result<(), RepoError> { - let endpoint = format!( - "/Users/{}/PlayedItems/{}", - self.user_id, - urlencoding::encode(item_id) - ); + let endpoint = endpoints::played_item(&self.capabilities, &self.user_id, item_id); let url = format!("{}{}", self.server_url, endpoint); let result = async { @@ -2850,7 +2714,7 @@ impl MediaRepository for OnlineRepository { .http_client .client .post(&url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .header("Content-Length", "0") .build() .map_err(|e| RepoError::Network { @@ -2887,11 +2751,7 @@ impl MediaRepository for OnlineRepository { /// /// TRACES: UR-035, UR-036 | IR-022, JA-030 async fn get_person(&self, person_id: &str) -> Result { - let endpoint = format!( - "/Users/{}/Items/{}", - self.user_id, - urlencoding::encode(person_id) - ); + let endpoint = endpoints::person(&self.capabilities, &self.user_id, person_id); let item: JellyfinItem = self.get_json(&endpoint).await?; Ok(item.into_media_item(self.user_id.clone())) } @@ -2906,21 +2766,16 @@ impl MediaRepository for OnlineRepository { ) -> Result { let limit = options.as_ref().and_then(|o| o.limit).unwrap_or(100); - let mut endpoint = format!( - "/Users/{}/Items?PersonIds={}&Limit={}&Recursive=true&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - self.user_id, person_id, limit + let endpoint = endpoints::items_by_person( + &self.capabilities, + &self.user_id, + person_id, + limit, + options + .as_ref() + .and_then(|o| o.include_item_types.as_deref()), ); - // Add item type filtering if specified in options - if let Some(ref opts) = options { - if let Some(ref include_types) = opts.include_item_types { - if !include_types.is_empty() { - let types_param = include_types.join(","); - endpoint.push_str(&format!("&IncludeItemTypes={}", types_param)); - } - } - } - let response: ItemsResponse = self.get_json(&endpoint).await?; Ok(SearchResult { items: response @@ -2940,10 +2795,8 @@ impl MediaRepository for OnlineRepository { let limit_str = limit.unwrap_or(20); // Try the /Similar endpoint which works for most items - let endpoint = format!( - "/Items/{}/Similar?UserId={}&Limit={}&Fields=BackdropImageTags,ParentBackdropImageTags,UserData", - item_id, self.user_id, limit_str - ); + let endpoint = + endpoints::similar_items(&self.capabilities, item_id, &self.user_id, limit_str); let response: ItemsResponse = self.get_json(&endpoint).await?; Ok(SearchResult { @@ -2974,20 +2827,22 @@ impl MediaRepository for OnlineRepository { "MediaType": "Audio", "UserId": self.user_id, }); - let response: CreatePlaylistResponse = self.post_json_response("/Playlists", &body).await?; + let response: CreatePlaylistResponse = self + .post_json_response(endpoints::playlists(&self.capabilities), &body) + .await?; Ok(PlaylistCreatedResult { id: response.id }) } async fn delete_playlist(&self, playlist_id: &str) -> Result<(), RepoError> { info!("[OnlineRepo] Deleting playlist {}", playlist_id); - let endpoint = format!("/Items/{}", urlencoding::encode(playlist_id)); + let endpoint = endpoints::playlist_as_item(&self.capabilities, playlist_id); let url = format!("{}{}", self.server_url, endpoint); let request = self .http_client .client .delete(&url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .build() .map_err(|e| RepoError::Network { message: format!("Failed to build request: {}", e), @@ -3015,16 +2870,13 @@ impl MediaRepository for OnlineRepository { "[OnlineRepo] Renaming playlist {} to '{}'", playlist_id, name ); - let endpoint = format!("/Items/{}", urlencoding::encode(playlist_id)); + let endpoint = endpoints::playlist_as_item(&self.capabilities, playlist_id); self.post_json(&endpoint, &serde_json::json!({ "Name": name })) .await } async fn get_playlist_items(&self, playlist_id: &str) -> Result, RepoError> { - let endpoint = format!( - "/Playlists/{}/Items?UserId={}&Fields=PrimaryImageTag,Artists,AlbumId,Album,AlbumArtist,RunTimeTicks,ArtistItems&StartIndex=0&Limit=10000", - playlist_id, self.user_id - ); + let endpoint = endpoints::playlist_items(&self.capabilities, playlist_id, &self.user_id); let response: PlaylistItemsResponse = self.get_json(&endpoint).await?; debug!( @@ -3059,11 +2911,7 @@ impl MediaRepository for OnlineRepository { .map(|id| urlencoding::encode(id).into_owned()) .collect::>() .join(","); - let endpoint = format!( - "/Playlists/{}/Items?Ids={}", - urlencoding::encode(playlist_id), - ids_param - ); + let endpoint = endpoints::playlist_items_add(&self.capabilities, playlist_id, &ids_param); self.post_json(&endpoint, &serde_json::json!({})).await } @@ -3082,18 +2930,15 @@ impl MediaRepository for OnlineRepository { .map(|id| urlencoding::encode(id).into_owned()) .collect::>() .join(","); - let endpoint = format!( - "/Playlists/{}/Items?EntryIds={}", - urlencoding::encode(playlist_id), - ids_param - ); + let endpoint = + endpoints::playlist_items_remove(&self.capabilities, playlist_id, &ids_param); let url = format!("{}{}", self.server_url, endpoint); let request = self .http_client .client .delete(&url) - .header("X-Emby-Authorization", self.auth_header()) + .header("Authorization", self.auth_header()) .build() .map_err(|e| RepoError::Network { message: format!("Failed to build request: {}", e), @@ -3126,10 +2971,8 @@ impl MediaRepository for OnlineRepository { "[OnlineRepo] Moving item {} in playlist {} to index {}", item_id, playlist_id, new_index ); - let endpoint = format!( - "/Playlists/{}/Items/{}/Move/{}", - playlist_id, item_id, new_index - ); + let endpoint = + endpoints::playlist_item_move(&self.capabilities, playlist_id, item_id, new_index); self.post_json(&endpoint, &serde_json::json!({})).await } } @@ -3310,7 +3153,7 @@ mod tests { let url = result.unwrap(); assert_eq!( url, - "https://test.server.com/Audio/test-track-123/stream?UserId=test-user-id&api_key=test-access-token&Static=true" + "https://test.server.com/Audio/test-track-123/stream?UserId=test-user-id&ApiKey=test-access-token&Static=true" ); } @@ -3765,10 +3608,14 @@ mod tests { // ===== Video download URL (real impl) ===== // // These exercise the PRODUCTION `OnlineRepository::get_video_download_url`, - // not a mock. A prior mock in online_integration_test.rs used the correct - // `stream.mp4` endpoint while the real impl shipped `/Videos/{id}/download`, - // which returns 404 on real servers and silently broke every movie/TV - // download. Assert the real builder targets the resumable stream endpoint. + // not a mock. A prior mock used the correct `stream.mp4` endpoint while the + // real impl shipped `/Videos/{id}/download`, which returns 404 on real + // servers and silently broke every movie/TV download. That mock lived in + // `online_integration_test.rs`, which was never declared as a module and so + // never compiled — it was deleted for that reason, and this is the lesson it + // left: a mock that reimplements the builder asserts on itself, and passes + // just as happily when production is wrong. Assert the real builder targets + // the resumable stream endpoint. // // @req-test: DR-013 - Repository pattern for online/offline data access @@ -3787,7 +3634,7 @@ mod tests { url.contains("/Videos/item123/stream.mp4"), "download URL must target /Videos/{{id}}/stream.mp4: {url}" ); - assert!(url.contains("api_key=test-access-token"), "url: {url}"); + assert!(url.contains("ApiKey=test-access-token"), "url: {url}"); } #[test]