feat(repository): a route table and resolved server capabilities
Endpoints were 57 inline format! literals with their 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 would mean a conditional at every one of
them. They now live in repository/endpoints.rs, one function each, taking
&ServerCapabilities.
Two things fall out of the move:
- A small Endpoint builder replaces the manual ?/& juggling, so a double or
trailing separator is structurally impossible rather than something four
assertions in a deleted test file used to watch for.
- Both user-scoped route shapes (/Users/{uid}/Items and /Items?userId=) are
built and tested, though nothing selects the second yet. The family still
works on 12.0, so migrating is optional; having both means it is a one-line
change if 13.0 removes them, as the newly written removal policy allows.
ServerCapabilities is resolved once per connection from the version the server
already reported at connect. The version-to-flags mapping lives in exactly one
function and nothing else in the crate compares a version number: a `version < N`
at the point of use re-derives a domain fact where it is consumed, is unreadable
by its second occurrence, and cannot express a backport.
An unrecognised version resolves forward to the newest known generation rather
than being refused, because refusing would make every release expire the moment
the server upgrades. Only a version below the floor is refused.
This commit also carries the two fixes that are NOT capability branches, because
they live in the same files:
- Authorization replaces X-Emby-Authorization, and ApiKey replaces the api_key
query parameter. Jellyfin 12.0 disables both legacy spellings by default and
a migration flips them on upgraded servers too, so this is what actually
breaks against 12.0. The header value this app already built was always the
correct MediaBrowser scheme, and both new spellings are ungated on 10.11.x —
so it is a rename, not a branch. The query-parameter spelling is load-bearing
rather than cosmetic: stream URLs go to mpv, ExoPlayer and the webview's
<video>, none of which can send a header.
- A type-filtered listing now states Recursive explicitly. 12.0 defaults it to
true for a library parent with IncludeItemTypes where 10.11 returned
immediate children, so the identical request returned a different result set
with nothing in the response to say which rule applied. The value sent is the
one that shipped, so this is a compatibility fix and not a silent behaviour
change.
A structural test refuses any deprecated auth spelling reaching a request
builder, verified to fail when one is reintroduced. Behaviour is otherwise
preserved: the four previous endpoint builders become test-only shims over the
new table, so the ~20 existing tests encoding DR-116/DR-212/DR-257 now exercise
the production path rather than being deleted.
TRACES: UR-085 | IR-035, DR-279, DR-280, DR-287, DR-288
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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<Self> {
|
||||
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: <https://jellyfin.org/posts/jellyfin-release-12.0>, 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<ServerVersion>,
|
||||
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<ServerVersion>) -> 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"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user