Fix for offline mode
This commit is contained in:
@@ -144,6 +144,17 @@ impl OnlineRepository {
|
||||
|
||||
/// Make authenticated GET request
|
||||
async fn get_json<T: for<'de> Deserialize<'de>>(&self, endpoint: &str) -> Result<T, RepoError> {
|
||||
// Fast-fail when connectivity is known-offline. Without this every request
|
||||
// still runs the full HTTP retry/backoff cycle (~7s) before giving up,
|
||||
// which stalls cache-miss paths and makes offline browsing feel janky.
|
||||
// The offline recovery probe (connectivity monitor) flips us back to
|
||||
// reachable the moment the server returns, so this never sticks.
|
||||
if let Some(reporter) = &self.connectivity {
|
||||
if !reporter.is_reachable().await {
|
||||
return Err(RepoError::Offline);
|
||||
}
|
||||
}
|
||||
|
||||
let result = self.get_json_inner(endpoint).await;
|
||||
self.report_outcome(&result).await;
|
||||
result
|
||||
@@ -1413,12 +1424,43 @@ impl MediaRepository for OnlineRepository {
|
||||
quality: &str,
|
||||
media_source_id: Option<&str>,
|
||||
) -> String {
|
||||
let mut url = format!("{}/Videos/{}/download", self.server_url, item_id);
|
||||
// NOTE: Jellyfin's `/Videos/{id}/download` endpoint is not universally
|
||||
// available (returns 404 on many server configs), which silently broke
|
||||
// every movie/TV download. Use the progressive `stream.mp4` endpoint
|
||||
// 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)];
|
||||
|
||||
// Add quality parameter if not "original"
|
||||
if quality != "original" {
|
||||
params.push(format!("quality={}", quality));
|
||||
// Map the frontend quality preset to concrete transcode params. For
|
||||
// "original" we request a direct static copy (no transcode) which is
|
||||
// byte-range resumable; other presets ask the server to transcode.
|
||||
match quality {
|
||||
"high" => {
|
||||
params.push("videoBitrate=8000000".to_string());
|
||||
params.push("maxHeight=1080".to_string());
|
||||
params.push("audioBitrate=384000".to_string());
|
||||
params.push("videoCodec=h264".to_string());
|
||||
params.push("audioCodec=aac".to_string());
|
||||
}
|
||||
"medium" => {
|
||||
params.push("videoBitrate=4000000".to_string());
|
||||
params.push("maxHeight=720".to_string());
|
||||
params.push("audioBitrate=256000".to_string());
|
||||
params.push("videoCodec=h264".to_string());
|
||||
params.push("audioCodec=aac".to_string());
|
||||
}
|
||||
"low" => {
|
||||
params.push("videoBitrate=1500000".to_string());
|
||||
params.push("maxHeight=480".to_string());
|
||||
params.push("audioBitrate=128000".to_string());
|
||||
params.push("videoCodec=h264".to_string());
|
||||
params.push("audioCodec=aac".to_string());
|
||||
}
|
||||
// "original" (and any unknown value) → direct, resumable copy.
|
||||
_ => {
|
||||
params.push("Static=true".to_string());
|
||||
}
|
||||
}
|
||||
|
||||
// Add media source ID if provided
|
||||
@@ -1426,10 +1468,8 @@ impl MediaRepository for OnlineRepository {
|
||||
params.push(format!("mediaSourceId={}", source_id));
|
||||
}
|
||||
|
||||
if !params.is_empty() {
|
||||
url.push('?');
|
||||
url.push_str(¶ms.join("&"));
|
||||
}
|
||||
url.push('?');
|
||||
url.push_str(¶ms.join("&"));
|
||||
|
||||
url
|
||||
}
|
||||
@@ -1759,6 +1799,25 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// When connectivity is known-offline, `get_json` must fast-fail with
|
||||
/// `RepoError::Offline` instead of running the full HTTP retry cycle (~7s).
|
||||
/// This is what keeps offline browsing snappy. `test.server.com` is
|
||||
/// unroutable, so if the guard were absent this would hang on retries; the
|
||||
/// assertion returning promptly with `Offline` proves the short-circuit.
|
||||
#[tokio::test]
|
||||
async fn test_get_json_fast_fails_when_offline() {
|
||||
let (repo, reporter) = create_test_repository_with_connectivity();
|
||||
reporter.mark_unreachable_for_test().await;
|
||||
assert!(!reporter.is_reachable().await, "precondition: offline");
|
||||
|
||||
let result: Result<serde_json::Value, RepoError> = repo.get_json("/System/Info").await;
|
||||
assert!(
|
||||
matches!(result, Err(RepoError::Offline)),
|
||||
"known-offline get_json should return Offline immediately, got {:?}",
|
||||
result
|
||||
);
|
||||
}
|
||||
|
||||
/// A network error routes through the debounced path. A single failure stays
|
||||
/// online (debounce window not yet elapsed).
|
||||
#[tokio::test]
|
||||
@@ -1887,6 +1946,74 @@ mod tests {
|
||||
assert_eq!(tags.primary(), None);
|
||||
}
|
||||
|
||||
// ===== 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.
|
||||
//
|
||||
// @req-test: DR-013 - Repository pattern for online/offline data access
|
||||
|
||||
#[test]
|
||||
fn test_video_download_url_uses_stream_not_download_endpoint() {
|
||||
let repo = create_test_repository();
|
||||
let url = repo.get_video_download_url("item123", "original", None);
|
||||
|
||||
// Must NOT use the /download endpoint (404 on real servers).
|
||||
assert!(
|
||||
!url.contains("/download"),
|
||||
"download URL must not use the broken /Videos/{{id}}/download endpoint: {url}"
|
||||
);
|
||||
// Must use the progressive, range-resumable stream endpoint.
|
||||
assert!(
|
||||
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}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_video_download_url_original_is_static_direct_copy() {
|
||||
let repo = create_test_repository();
|
||||
let url = repo.get_video_download_url("item123", "original", None);
|
||||
|
||||
// "original" must request a direct static copy (byte-range resumable),
|
||||
// with no transcode params.
|
||||
assert!(url.contains("Static=true"), "url: {url}");
|
||||
assert!(!url.contains("videoBitrate"), "original must not transcode: {url}");
|
||||
assert!(!url.contains("maxHeight"), "original must not transcode: {url}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_video_download_url_quality_presets_transcode() {
|
||||
let repo = create_test_repository();
|
||||
|
||||
for (quality, height) in [("high", "1080"), ("medium", "720"), ("low", "480")] {
|
||||
let url = repo.get_video_download_url("item123", quality, None);
|
||||
assert!(
|
||||
url.contains("/Videos/item123/stream.mp4"),
|
||||
"{quality} must use stream.mp4: {url}"
|
||||
);
|
||||
assert!(url.contains("videoBitrate="), "{quality} must set bitrate: {url}");
|
||||
assert!(
|
||||
url.contains(&format!("maxHeight={height}")),
|
||||
"{quality} must cap height at {height}: {url}"
|
||||
);
|
||||
assert!(url.contains("videoCodec=h264"), "{quality}: {url}");
|
||||
// Transcoded presets must not also ask for a static copy.
|
||||
assert!(!url.contains("Static=true"), "{quality} must not be Static: {url}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_video_download_url_passes_media_source_id() {
|
||||
let repo = create_test_repository();
|
||||
let url = repo.get_video_download_url("item123", "original", Some("src-42"));
|
||||
assert!(url.contains("mediaSourceId=src-42"), "url: {url}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_jellyfin_item_deserialize_with_image_tags() {
|
||||
// Test full JellyfinItem deserialization with ImageTags
|
||||
|
||||
Reference in New Issue
Block a user