merge: clear the clippy backlog and unify lock helpers (D1-warnings, D3)

51 clippy warnings -> 0, with 8 justified #[allow]s (IPC arity, specta wire
types, and the 9 test-only await-holding-lock sites). 27 raw lock calls moved to
the poison-tolerant helpers - all of them test code; production was already
clean.

Caught a non-neutral clippy --fix: removing the redundant 'use hostname;' in
credentials.rs orphaned its #[cfg(target_os = "linux")] onto SERVICE_NAME,
which would have cfg'd the constant out of every non-Linux build. Compiles clean
on Linux, so only Windows/macOS CI would have caught it.
This commit is contained in:
2026-08-16 23:06:33 +02:00
27 changed files with 173 additions and 109 deletions
+2 -2
View File
@@ -418,13 +418,13 @@ mod tests {
#[test] #[test]
fn test_auth_manager_wrapper_structure() { fn test_auth_manager_wrapper_structure() {
// Verify wrapper type exists and has correct structure // Verify wrapper type exists and has correct structure
assert_eq!(std::mem::size_of::<AuthManagerWrapper>() > 0, true); assert!(std::mem::size_of::<AuthManagerWrapper>() > 0);
} }
#[test] #[test]
fn test_session_verifier_wrapper_structure() { fn test_session_verifier_wrapper_structure() {
// Verify wrapper type exists and has correct structure // Verify wrapper type exists and has correct structure
assert_eq!(std::mem::size_of::<SessionVerifierWrapper>() > 0, true); assert!(std::mem::size_of::<SessionVerifierWrapper>() > 0);
} }
#[test] #[test]
+9 -8
View File
@@ -496,7 +496,7 @@ pub(crate) async fn requeue_mistyped_video_downloads(
.collect::<Vec<_>>() .collect::<Vec<_>>()
.join(", "); .join(", ");
let query = Query::new(&format!( let query = Query::new(format!(
"UPDATE downloads "UPDATE downloads
SET status = 'pending', stream_url = NULL, progress = 0, SET status = 'pending', stream_url = NULL, progress = 0,
bytes_downloaded = 0, started_at = NULL, completed_at = NULL bytes_downloaded = 0, started_at = NULL, completed_at = NULL
@@ -563,7 +563,7 @@ where
), ),
None => String::new(), None => String::new(),
}; };
let rows_query = Query::new(&format!( let rows_query = Query::new(format!(
"SELECT d.id, d.item_id, "SELECT d.id, d.item_id,
COALESCE( COALESCE(
d.media_type, d.media_type,
@@ -753,6 +753,7 @@ pub async fn resume_queued_downloads(
mod tests { mod tests {
use super::*; use super::*;
use crate::storage::db_service::RusqliteService; use crate::storage::db_service::RusqliteService;
use crate::utils::lock::MutexSafe;
use rusqlite::Connection; use rusqlite::Connection;
use std::sync::Mutex; use std::sync::Mutex;
@@ -1012,14 +1013,14 @@ mod tests {
resolve_pending_download_urls(&db, "/data", None, move |item_id, media_type, _q| { resolve_pending_download_urls(&db, "/data", None, move |item_id, media_type, _q| {
let seen = Arc::clone(&seen_c); let seen = Arc::clone(&seen_c);
async move { async move {
seen.lock().unwrap().push((item_id.clone(), media_type)); seen.lock_safe().push((item_id.clone(), media_type));
Some(format!("http://resolved/{item_id}")) Some(format!("http://resolved/{item_id}"))
} }
}) })
.await .await
.unwrap(); .unwrap();
let seen = seen.lock().unwrap().clone(); let seen = seen.lock_safe().clone();
let of = |id: &str| { let of = |id: &str| {
seen.iter() seen.iter()
.find(|(i, _)| i == id) .find(|(i, _)| i == id)
@@ -1045,14 +1046,14 @@ mod tests {
resolve_pending_download_urls(&db, "/data", None, move |_id, media_type, _q| { resolve_pending_download_urls(&db, "/data", None, move |_id, media_type, _q| {
let seen = Arc::clone(&seen_c); let seen = Arc::clone(&seen_c);
async move { async move {
*seen.lock().unwrap() = media_type; *seen.lock_safe() = media_type;
Some("http://x".to_string()) Some("http://x".to_string())
} }
}) })
.await .await
.unwrap(); .unwrap();
assert_eq!(*seen.lock().unwrap(), "audio"); assert_eq!(*seen.lock_safe(), "audio");
} }
/// An explicit `media_type` on the row always wins over the item's type. /// An explicit `media_type` on the row always wins over the item's type.
@@ -1069,14 +1070,14 @@ mod tests {
resolve_pending_download_urls(&db, "/data", None, move |_id, media_type, _q| { resolve_pending_download_urls(&db, "/data", None, move |_id, media_type, _q| {
let seen = Arc::clone(&seen_c); let seen = Arc::clone(&seen_c);
async move { async move {
*seen.lock().unwrap() = media_type; *seen.lock_safe() = media_type;
Some("http://x".to_string()) Some("http://x".to_string())
} }
}) })
.await .await
.unwrap(); .unwrap();
assert_eq!(*seen.lock().unwrap(), "video"); assert_eq!(*seen.lock_safe(), "video");
} }
/// Rows already downloaded under the audio default hold an audio-only /// Rows already downloaded under the audio default hold an audio-only
+1 -1
View File
@@ -97,6 +97,6 @@ mod tests {
// due to its dependencies, so we just test the wrapper type structure // due to its dependencies, so we just test the wrapper type structure
// This verifies the wrapper type exists and can hold Arc<Mutex> // This verifies the wrapper type exists and can hold Arc<Mutex>
assert_eq!(std::mem::size_of::<ConnectivityMonitorWrapper>() > 0, true); assert!(std::mem::size_of::<ConnectivityMonitorWrapper>() > 0);
} }
} }
+15 -3
View File
@@ -19,6 +19,10 @@ mod smart_cache;
pub use pinning::*; pub use pinning::*;
pub use smart_cache::*; pub use smart_cache::*;
/// One row of the series episode listing used when queueing a whole series:
/// `(id, name, season_name, index_number, parent_index_number)`.
type EpisodeRow = (String, String, Option<String>, Option<i32>, Option<i32>);
/// Wrapper for DownloadManager to be used as Tauri state /// Wrapper for DownloadManager to be used as Tauri state
pub struct DownloadManagerWrapper(pub Mutex<DownloadManager>); pub struct DownloadManagerWrapper(pub Mutex<DownloadManager>);
@@ -596,6 +600,10 @@ pub(crate) async fn queue_album_tracks(
/// TRACES: UR-018, UR-055 | DR-173 | UT-170 /// TRACES: UR-018, UR-055 | DR-173 | UT-170
#[tauri::command] #[tauri::command]
#[specta::specta] #[specta::specta]
// Three of the eight arguments are Tauri `State<'_, _>` injections plus the
// `AppHandle`, not caller input. Folding the rest into a struct would change the
// IPC contract and the generated TypeScript for no readability gain.
#[allow(clippy::too_many_arguments)]
pub async fn download_album( pub async fn download_album(
db: State<'_, DatabaseWrapper>, db: State<'_, DatabaseWrapper>,
repository: State<'_, crate::commands::repository::RepositoryManagerWrapper>, repository: State<'_, crate::commands::repository::RepositoryManagerWrapper>,
@@ -807,7 +815,7 @@ pub async fn download_series(
vec![QueryParam::String(series_id)], vec![QueryParam::String(series_id)],
); );
let episodes: Vec<(String, String, Option<String>, Option<i32>, Option<i32>)> = db_service let episodes: Vec<EpisodeRow> = db_service
.query_many(episodes_query, |row| { .query_many(episodes_query, |row| {
Ok(( Ok((
row.get(0)?, row.get(0)?,
@@ -912,6 +920,10 @@ pub async fn download_series(
/// Queue all episodes of a specific season for download /// Queue all episodes of a specific season for download
#[tauri::command] #[tauri::command]
#[specta::specta] #[specta::specta]
// One of the eight arguments is a Tauri `State<'_, _>` injection; the rest are
// the season's identifying fields. Folding them into a struct would change the
// IPC contract and the generated TypeScript for no readability gain.
#[allow(clippy::too_many_arguments)]
pub async fn download_season( pub async fn download_season(
db: State<'_, DatabaseWrapper>, db: State<'_, DatabaseWrapper>,
season_id: String, season_id: String,
@@ -2307,7 +2319,7 @@ pub async fn delete_downloads_under(
)"; )";
let file_query = Query::with_params( let file_query = Query::with_params(
&format!("SELECT d.file_path FROM downloads d WHERE {SCOPE}"), format!("SELECT d.file_path FROM downloads d WHERE {SCOPE}"),
vec![ vec![
QueryParam::String(user_id.clone()), QueryParam::String(user_id.clone()),
QueryParam::String(item_id.clone()), QueryParam::String(item_id.clone()),
@@ -2323,7 +2335,7 @@ pub async fn delete_downloads_under(
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
let delete_query = Query::with_params( let delete_query = Query::with_params(
&format!("DELETE FROM downloads WHERE id IN (SELECT d.id FROM downloads d WHERE {SCOPE})"), format!("DELETE FROM downloads WHERE id IN (SELECT d.id FROM downloads d WHERE {SCOPE})"),
vec![ vec![
QueryParam::String(user_id), QueryParam::String(user_id),
QueryParam::String(item_id.clone()), QueryParam::String(item_id.clone()),
+2 -1
View File
@@ -186,6 +186,7 @@ async fn run_drain(app: &tauri::AppHandle) -> Result<(), String> {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::utils::lock::MutexSafe;
use rusqlite::Connection; use rusqlite::Connection;
use std::sync::Mutex; use std::sync::Mutex;
@@ -211,7 +212,7 @@ mod tests {
} }
fn calls(&self) -> Vec<(String, bool)> { fn calls(&self) -> Vec<(String, bool)> {
let mut calls = self.calls.lock().unwrap().clone(); let mut calls = self.calls.lock_safe().clone();
calls.sort(); calls.sort();
calls calls
} }
+1 -1
View File
@@ -360,7 +360,7 @@ mod tests {
#[test] #[test]
fn test_playback_reporter_wrapper_structure() { fn test_playback_reporter_wrapper_structure() {
// Verify wrapper type can hold Arc<TokioMutex<Option<T>>> // Verify wrapper type can hold Arc<TokioMutex<Option<T>>>
assert_eq!(std::mem::size_of::<PlaybackReporterWrapper>() > 0, true); assert!(std::mem::size_of::<PlaybackReporterWrapper>() > 0);
} }
#[test] #[test]
+16 -9
View File
@@ -1490,6 +1490,10 @@ pub async fn player_seek_video(
/// TRACES: UR-021 | IR-019, DR-024 /// TRACES: UR-021 | IR-019, DR-024
#[tauri::command] #[tauri::command]
#[specta::specta] #[specta::specta]
// Two of the eight arguments are Tauri `State<'_, _>` injections, not caller
// input. Folding the rest into a struct would change the IPC contract and the
// generated TypeScript for no readability gain.
#[allow(clippy::too_many_arguments)]
pub async fn player_switch_audio_track( pub async fn player_switch_audio_track(
player: State<'_, PlayerStateWrapper>, player: State<'_, PlayerStateWrapper>,
repository_manager: State<'_, super::repository::RepositoryManagerWrapper>, repository_manager: State<'_, super::repository::RepositoryManagerWrapper>,
@@ -1569,6 +1573,10 @@ pub async fn player_switch_audio_track(
/// TRACES: UR-074 | DR-162 /// TRACES: UR-074 | DR-162
#[tauri::command] #[tauri::command]
#[specta::specta] #[specta::specta]
// Three of the nine arguments are Tauri `State<'_, _>` injections, not caller
// input. Folding the rest into a struct would change the IPC contract and the
// generated TypeScript for no readability gain.
#[allow(clippy::too_many_arguments)]
pub async fn player_set_stream_quality( pub async fn player_set_stream_quality(
player: State<'_, PlayerStateWrapper>, player: State<'_, PlayerStateWrapper>,
repository_manager: State<'_, super::repository::RepositoryManagerWrapper>, repository_manager: State<'_, super::repository::RepositoryManagerWrapper>,
@@ -1805,7 +1813,7 @@ pub async fn player_get_status(
let local_media = { let local_media = {
let queue_arc = controller.queue(); let queue_arc = controller.queue();
let queue = queue_arc.lock().map_err(|e| e.to_string())?; let queue = queue_arc.lock().map_err(|e| e.to_string())?;
queue.current().map(|item| MergedMediaItem::from(item)) queue.current().map(MergedMediaItem::from)
}; };
let local_is_playing = status.state.is_playing(); let local_is_playing = status.state.is_playing();
@@ -1829,10 +1837,7 @@ pub async fn player_get_status(
log::info!("[PlayerCommands] Merging remote session state"); log::info!("[PlayerCommands] Merging remote session state");
// Merge media item // Merge media item
status.merged_media = session status.merged_media = session.now_playing_item.as_ref().map(MergedMediaItem::from);
.now_playing_item
.as_ref()
.map(|item| MergedMediaItem::from(item));
// Merge isPlaying (NOT isPaused!) // Merge isPlaying (NOT isPaused!)
status.merged_is_playing = session status.merged_is_playing = session
@@ -2762,6 +2767,8 @@ pub async fn player_disable_jellyfin(player: State<'_, PlayerStateWrapper>) -> R
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::utils::lock::MutexSafe;
/// The subtitle list the frontend resolved must survive the IPC hop and end /// The subtitle list the frontend resolved must survive the IPC hop and end
/// up on the `MediaItem` the native backend loads. /// up on the `MediaItem` the native backend loads.
/// ///
@@ -3084,7 +3091,7 @@ mod tests {
let database = Database::open_in_memory().unwrap(); let database = Database::open_in_memory().unwrap();
{ {
let conn = database.connection(); let conn = database.connection();
let conn = conn.lock().unwrap(); let conn = conn.lock_safe();
conn.execute_batch(&format!( conn.execute_batch(&format!(
r#" r#"
INSERT INTO servers (id, name, url) VALUES ('srv', 'Test', 'http://test'); INSERT INTO servers (id, name, url) VALUES ('srv', 'Test', 'http://test');
@@ -3140,7 +3147,7 @@ mod tests {
assert_eq!(switched, 1, "only the download whose file exists switches"); assert_eq!(switched, 1, "only the download whose file exists switches");
let queue = controller.queue(); let queue = controller.queue();
let queue_lock = queue.lock().unwrap(); let queue_lock = queue.lock_safe();
match &queue_lock.items()[0].source { match &queue_lock.items()[0].source {
MediaSource::Local { MediaSource::Local {
file_path, file_path,
@@ -3169,7 +3176,7 @@ mod tests {
index_number: Option<i32>, index_number: Option<i32>,
} }
let mut tracks = vec![ let mut tracks = [
MockTrack { MockTrack {
id: "track1".to_string(), id: "track1".to_string(),
name: "Song 1".to_string(), name: "Song 1".to_string(),
@@ -3262,7 +3269,7 @@ mod tests {
} }
// Create tracks in random order (not sorted) // Create tracks in random order (not sorted)
let mut tracks = vec![ let mut tracks = [
MockTrack { MockTrack {
id: "id5".to_string(), id: "id5".to_string(),
name: "Track 5".to_string(), name: "Track 5".to_string(),
+4 -1
View File
@@ -67,6 +67,10 @@ pub struct RepositoryManagerWrapper(pub RepositoryManager);
/// Returns a handle (UUID) for accessing the repository /// Returns a handle (UUID) for accessing the repository
#[tauri::command] #[tauri::command]
#[specta::specta] #[specta::specta]
// Four of the eight arguments are Tauri `State<'_, _>` injections, not caller
// input. Folding the remaining four into a struct would change the IPC contract
// and the generated TypeScript for no readability gain.
#[allow(clippy::too_many_arguments)]
pub async fn repository_create( pub async fn repository_create(
manager: State<'_, RepositoryManagerWrapper>, manager: State<'_, RepositoryManagerWrapper>,
player: State<'_, crate::commands::player::PlayerStateWrapper>, player: State<'_, crate::commands::player::PlayerStateWrapper>,
@@ -1099,7 +1103,6 @@ mod tests {
let handle = format!("{}", uuid); let handle = format!("{}", uuid);
// UUID should convert to a non-empty string // UUID should convert to a non-empty string
assert!(!handle.is_empty()); assert!(!handle.is_empty());
assert!(handle.len() > 0);
} }
#[test] #[test]
+1 -1
View File
@@ -89,7 +89,7 @@ mod tests {
#[test] #[test]
fn test_session_poller_wrapper_structure() { fn test_session_poller_wrapper_structure() {
// Test that wrapper type structure is correct // Test that wrapper type structure is correct
assert_eq!(std::mem::size_of::<SessionPollerWrapper>() > 0, true); assert!(std::mem::size_of::<SessionPollerWrapper>() > 0);
} }
#[test] #[test]
+3 -3
View File
@@ -1658,19 +1658,19 @@ mod tests {
#[test] #[test]
fn test_database_wrapper_structure() { fn test_database_wrapper_structure() {
// Verify DatabaseWrapper can be created and holds Mutex<Database> // Verify DatabaseWrapper can be created and holds Mutex<Database>
assert_eq!(std::mem::size_of::<DatabaseWrapper>() > 0, true); assert!(std::mem::size_of::<DatabaseWrapper>() > 0);
} }
#[test] #[test]
fn test_credential_store_wrapper_structure() { fn test_credential_store_wrapper_structure() {
// Verify CredentialStoreWrapper can be created // Verify CredentialStoreWrapper can be created
assert_eq!(std::mem::size_of::<CredentialStoreWrapper>() > 0, true); assert!(std::mem::size_of::<CredentialStoreWrapper>() > 0);
} }
#[test] #[test]
fn test_thumbnail_cache_wrapper_structure() { fn test_thumbnail_cache_wrapper_structure() {
// Verify ThumbnailCacheWrapper holds Arc<ThumbnailCache> // Verify ThumbnailCacheWrapper holds Arc<ThumbnailCache>
assert_eq!(std::mem::size_of::<ThumbnailCacheWrapper>() > 0, true); assert!(std::mem::size_of::<ThumbnailCacheWrapper>() > 0);
} }
#[test] #[test]
+3 -2
View File
@@ -481,6 +481,7 @@ pub async fn sync_process_pending(app: tauri::AppHandle) -> Result<DrainReport,
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::utils::lock::MutexSafe;
use rusqlite::Connection; use rusqlite::Connection;
use std::sync::Mutex; use std::sync::Mutex;
@@ -517,7 +518,7 @@ mod tests {
} }
fn calls(&self) -> Vec<QueuedOp> { fn calls(&self) -> Vec<QueuedOp> {
self.calls.lock().unwrap().clone() self.calls.lock_safe().clone()
} }
} }
@@ -527,7 +528,7 @@ mod tests {
if let Some(err) = &self.fail_with { if let Some(err) = &self.fail_with {
return Err(err.clone()); return Err(err.clone());
} }
self.calls.lock().unwrap().push(op.clone()); self.calls.lock_safe().push(op.clone());
Ok(()) Ok(())
} }
} }
+2 -8
View File
@@ -20,9 +20,6 @@ use sha2::{Digest, Sha256};
use std::fs; use std::fs;
use std::path::PathBuf; use std::path::PathBuf;
#[cfg(target_os = "linux")]
use hostname;
#[cfg(not(target_os = "android"))] #[cfg(not(target_os = "android"))]
const SERVICE_NAME: &str = "com.dtourolle.jellytau"; const SERVICE_NAME: &str = "com.dtourolle.jellytau";
@@ -203,15 +200,12 @@ impl CredentialStore {
// secret-tool doesn't support --version, so we test with a search command // secret-tool doesn't support --version, so we test with a search command
// that will succeed even if no items are found // that will succeed even if no items are found
match Command::new("secret-tool") Command::new("secret-tool")
.arg("search") .arg("search")
.arg("service") .arg("service")
.arg("__nonexistent_test__") .arg("__nonexistent_test__")
.output() .output()
{ .is_ok()
Ok(_) => true, // If command runs (even with no results), secret-tool is available
Err(_) => false, // Command not found or can't execute
}
} }
#[cfg(all(not(target_os = "android"), not(target_os = "linux")))] #[cfg(all(not(target_os = "android"), not(target_os = "linux")))]
+6 -5
View File
@@ -119,11 +119,12 @@ mod tests {
use super::*; use super::*;
fn item(name: &str, kind: MediaKind) -> MediaItem { fn item(name: &str, kind: MediaKind) -> MediaItem {
let mut item = MediaItem::default(); MediaItem {
item.id = format!("id-{}-{:?}", name, kind); id: format!("id-{}-{:?}", name, kind),
item.name = name.to_string(); name: name.to_string(),
item.kind = kind; kind,
item ..MediaItem::default()
}
} }
fn names(items: &[MediaItem]) -> Vec<&str> { fn names(items: &[MediaItem]) -> Vec<&str> {
+14 -8
View File
@@ -389,14 +389,18 @@ mod tests {
#[test] #[test]
fn test_queue_precache_config() { fn test_queue_precache_config() {
let mut config = CacheConfig::default(); let config = CacheConfig {
config.queue_precache_enabled = false; queue_precache_enabled: false,
..CacheConfig::default()
};
let cache = SmartCache::new(config); let cache = SmartCache::new(config);
assert!(!cache.should_precache_queue()); assert!(!cache.should_precache_queue());
let mut new_config = CacheConfig::default(); let new_config = CacheConfig {
new_config.wifi_only = false; wifi_only: false,
..CacheConfig::default()
};
cache.update_config(new_config); cache.update_config(new_config);
assert!(cache.should_precache_queue()); assert!(cache.should_precache_queue());
@@ -407,9 +411,11 @@ mod tests {
// wifi_only must not short-circuit precaching: the network gate lives in // wifi_only must not short-circuit precaching: the network gate lives in
// the download pump, which checks the *actual* transport. Enabling // the download pump, which checks the *actual* transport. Enabling
// WiFi-only while on WiFi should still precache. // WiFi-only while on WiFi should still precache.
let mut config = CacheConfig::default(); let config = CacheConfig {
config.queue_precache_enabled = true; queue_precache_enabled: true,
config.wifi_only = true; wifi_only: true,
..CacheConfig::default()
};
let cache = SmartCache::new(config); let cache = SmartCache::new(config);
assert!(cache.should_precache_queue()); assert!(cache.should_precache_queue());
@@ -422,7 +428,7 @@ mod tests {
/// TRACES: UR-071 | DR-127 | UT-120 /// TRACES: UR-071 | DR-127 | UT-120
#[tokio::test] #[tokio::test]
async fn test_reclaim_expired_only_takes_expired_temporary_entries() { async fn test_reclaim_expired_only_takes_expired_temporary_entries() {
use crate::storage::db_service::{DatabaseService, RusqliteService}; use crate::storage::db_service::RusqliteService;
use rusqlite::Connection; use rusqlite::Connection;
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
+2 -2
View File
@@ -620,7 +620,7 @@ fn create_player_backend(
match MpvBackend::new(Some(_event_emitter), playback_reporter, position_throttler) { match MpvBackend::new(Some(_event_emitter), playback_reporter, position_throttler) {
Ok(backend) => { Ok(backend) => {
info!("Successfully initialized MPV backend for Linux"); info!("Successfully initialized MPV backend for Linux");
return Box::new(backend); Box::new(backend)
} }
Err(e) => { Err(e) => {
error!("\n========================================"); error!("\n========================================");
@@ -645,7 +645,7 @@ fn create_player_backend(
// still browse the library and manage downloads, and the frontend // still browse the library and manage downloads, and the frontend
// can show a "playback unavailable" notice via this event. // can show a "playback unavailable" notice via this event.
emit_backend_init_failed(&app_handle, "mpv", e.to_string()); emit_backend_init_failed(&app_handle, "mpv", e.to_string());
return Box::new(NullBackend::new()); Box::new(NullBackend::new())
} }
} }
} }
+10 -10
View File
@@ -624,7 +624,7 @@ impl PlaybackModeManager {
); );
// Log first few track IDs for debugging // Log first few track IDs for debugging
if queue_ids.len() > 0 { if !queue_ids.is_empty() {
let preview: Vec<&str> = queue_ids.iter().take(3).map(|s| s.as_str()).collect(); let preview: Vec<&str> = queue_ids.iter().take(3).map(|s| s.as_str()).collect();
debug!("[PlaybackMode] First track IDs: {:?}...", preview); debug!("[PlaybackMode] First track IDs: {:?}...", preview);
} }
@@ -914,7 +914,7 @@ mod tests {
impl PlayerEventEmitter for CapturingEmitter { impl PlayerEventEmitter for CapturingEmitter {
fn emit(&self, event: PlayerStatusEvent) { fn emit(&self, event: PlayerStatusEvent) {
self.events.lock().unwrap().push(event); self.events.lock_safe().push(event);
} }
} }
@@ -942,7 +942,7 @@ mod tests {
manager.set_mode(PlaybackMode::Local); manager.set_mode(PlaybackMode::Local);
manager.set_mode(PlaybackMode::Idle); manager.set_mode(PlaybackMode::Idle);
let events = emitter.events.lock().unwrap(); let events = emitter.events.lock_safe();
assert_eq!(events.len(), 3, "one event per real mode change"); assert_eq!(events.len(), 3, "one event per real mode change");
match &events[0] { match &events[0] {
@@ -975,10 +975,10 @@ mod tests {
impl RemoteVolumeControl for RecordingVolumeControl { impl RemoteVolumeControl for RecordingVolumeControl {
fn enable(&self, _initial_volume: i32) { fn enable(&self, _initial_volume: i32) {
self.calls.lock().unwrap().push("enable"); self.calls.lock_safe().push("enable");
} }
fn disable(&self) { fn disable(&self) {
self.calls.lock().unwrap().push("disable"); self.calls.lock_safe().push("disable");
} }
} }
@@ -1014,7 +1014,7 @@ mod tests {
manager.set_mode(PlaybackMode::Idle); manager.set_mode(PlaybackMode::Idle);
assert_eq!( assert_eq!(
*volume.calls.lock().unwrap(), *volume.calls.lock_safe(),
vec!["enable", "disable"], vec!["enable", "disable"],
"remote->idle must return volume control to the local speaker" "remote->idle must return volume control to the local speaker"
); );
@@ -1033,7 +1033,7 @@ mod tests {
manager.set_mode(PlaybackMode::Local); manager.set_mode(PlaybackMode::Local);
assert_eq!( assert_eq!(
*volume.calls.lock().unwrap(), *volume.calls.lock_safe(),
vec!["enable", "disable"], vec!["enable", "disable"],
"remote->local must return volume control to the local speaker" "remote->local must return volume control to the local speaker"
); );
@@ -1053,7 +1053,7 @@ mod tests {
manager.set_mode(PlaybackMode::Local); manager.set_mode(PlaybackMode::Local);
assert!( assert!(
volume.calls.lock().unwrap().is_empty(), volume.calls.lock_safe().is_empty(),
"local/idle transitions must not touch remote volume routing" "local/idle transitions must not touch remote volume routing"
); );
} }
@@ -1074,7 +1074,7 @@ mod tests {
}); });
assert_eq!( assert_eq!(
*volume.calls.lock().unwrap(), *volume.calls.lock_safe(),
vec!["enable", "enable"], vec!["enable", "enable"],
"remote->remote re-arms control without releasing it to local" "remote->remote re-arms control without releasing it to local"
); );
@@ -1091,7 +1091,7 @@ mod tests {
manager.set_mode(PlaybackMode::Local); manager.set_mode(PlaybackMode::Local);
assert_eq!( assert_eq!(
emitter.events.lock().unwrap().len(), emitter.events.lock_safe().len(),
1, 1,
"repeated identical mode set emits only once" "repeated identical mode set emits only once"
); );
+6
View File
@@ -6,6 +6,12 @@ use serde::{Deserialize, Serialize};
/// Autoplay decision result - determines what happens after playback ends /// Autoplay decision result - determines what happens after playback ends
#[derive(specta::Type, Debug, Clone, Serialize)] #[derive(specta::Type, Debug, Clone, Serialize)]
#[serde(tag = "action", rename_all = "camelCase")] #[serde(tag = "action", rename_all = "camelCase")]
// `ShowNextEpisodePopup` carries two `MediaItem`s, so it dwarfs the unit
// variants. Boxing them is not worth it here: this enum is constructed once per
// end-of-item (never in a hot loop or a large collection), and it is an IPC type
// — the indirection would have to stay invisible to serde/specta while every
// match arm gained a deref, for no measurable gain.
#[allow(clippy::large_enum_variant)]
pub enum AutoplayDecision { pub enum AutoplayDecision {
/// Stop playback (no next item or timer expired) /// Stop playback (no next item or timer expired)
Stop, Stop,
+6
View File
@@ -30,6 +30,12 @@ use super::{MediaSessionType, SleepTimerMode};
// queue_changed never reach the frontend, so the mini player never appears). // queue_changed never reach the frontend, so the mini player never appears).
// Keep serde and specta agreeing: snake_case fields, snake_case variant tags. // Keep serde and specta agreeing: snake_case fields, snake_case variant tags.
#[serde(tag = "type", rename_all = "snake_case")] #[serde(tag = "type", rename_all = "snake_case")]
// `ShowNextEpisodePopup` carries two `MediaItem`s, so it dwarfs the small
// position/state variants. Boxing them is rejected deliberately: this is a
// serde + specta wire type whose generated TypeScript must not shift, and the
// events are emitted a few times a second at most — never bulk-allocated — so
// the size difference costs nothing measurable.
#[allow(clippy::large_enum_variant)]
pub enum PlayerStatusEvent { pub enum PlayerStatusEvent {
/// Playback position updated (emitted periodically during playback) /// Playback position updated (emitted periodically during playback)
PositionUpdate { PositionUpdate {
+1 -1
View File
@@ -243,7 +243,7 @@ impl MpvBackend {
}); });
} }
} }
libmpv::events::Event::PropertyChange { name, .. } if name == "pause" => { libmpv::events::Event::PropertyChange { name: "pause", .. } => {
// Handle pause state changes // Handle pause state changes
if let Ok(is_paused) = mpv.get_property::<bool>("pause") { if let Ok(is_paused) = mpv.get_property::<bool>("pause") {
let media_id = state let media_id = state
+14 -13
View File
@@ -1,14 +1,15 @@
/// Tests for MpvBackend to prevent regressions //! Tests for MpvBackend to prevent regressions
/// //!
/// These tests are designed to catch common issues like: //! These tests are designed to catch common issues like:
/// - Tokio runtime panics when spawning async tasks from std::thread //! - Tokio runtime panics when spawning async tasks from std::thread
/// - Position update thread failures //! - Position update thread failures
/// - Event emission issues //! - Event emission issues
/// //!
/// TRACES: UR-003, UR-004 | IR-003 | IT-003, IT-004 //! TRACES: UR-003, UR-004 | IR-003 | IT-003, IT-004
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::utils::lock::MutexSafe;
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use tokio::sync::Mutex as TokioMutex; use tokio::sync::Mutex as TokioMutex;
@@ -67,14 +68,14 @@ mod tests {
if let Ok(handle) = tokio::runtime::Handle::try_current() { if let Ok(handle) = tokio::runtime::Handle::try_current() {
// Has runtime (shouldn't happen in this test) // Has runtime (shouldn't happen in this test)
handle.spawn(async move { handle.spawn(async move {
*counter_clone.lock().unwrap() += 1; *counter_clone.lock_safe() += 1;
}); });
} else { } else {
// No runtime - use fallback (should happen in this test) // No runtime - use fallback (should happen in this test)
std::thread::spawn(move || { std::thread::spawn(move || {
let rt = tokio::runtime::Runtime::new().unwrap(); let rt = tokio::runtime::Runtime::new().unwrap();
rt.block_on(async move { rt.block_on(async move {
*counter_clone.lock().unwrap() += 1; *counter_clone.lock_safe() += 1;
}); });
}); });
} }
@@ -85,7 +86,7 @@ mod tests {
// Wait for async task to complete // Wait for async task to complete
std::thread::sleep(std::time::Duration::from_millis(100)); std::thread::sleep(std::time::Duration::from_millis(100));
let count = *counter.lock().unwrap(); let count = *counter.lock_safe();
assert_eq!( assert_eq!(
count, 1, count, 1,
"Fallback pattern should execute async code successfully" "Fallback pattern should execute async code successfully"
@@ -109,13 +110,13 @@ mod tests {
let position = i as f64 * 0.25; let position = i as f64 * 0.25;
// Store position (simulating event emission) // Store position (simulating event emission)
positions_clone.lock().unwrap().push(position); positions_clone.lock_safe().push(position);
} }
}); });
handle.join().unwrap(); handle.join().unwrap();
let recorded_positions = positions.lock().unwrap(); let recorded_positions = positions.lock_safe();
assert_eq!( assert_eq!(
recorded_positions.len(), recorded_positions.len(),
5, 5,
+1 -2
View File
@@ -806,11 +806,10 @@ mod tests {
assert_eq!(queue.current_index(), Some(first_shuffled_index)); assert_eq!(queue.current_index(), Some(first_shuffled_index));
// Move through shuffle order // Move through shuffle order
for i in 1..shuffle_order.len() { for &expected_index in &shuffle_order[1..] {
assert!(queue.has_next()); assert!(queue.has_next());
let result = queue.next(); let result = queue.next();
assert!(result.is_some()); assert!(result.is_some());
let expected_index = shuffle_order[i];
assert_eq!(queue.current_index(), Some(expected_index)); assert_eq!(queue.current_index(), Some(expected_index));
} }
+3
View File
@@ -159,6 +159,9 @@ mod tests {
#[test] #[test]
fn test_end_reason_clone() { fn test_end_reason_clone() {
let reason = EndReason::Finished; let reason = EndReason::Finished;
// Deliberately exercising the derived `Clone` impl, not a plain copy:
// `EndReason` is also `Copy`, so clippy flags the call as redundant.
#[allow(clippy::clone_on_copy)]
let cloned = reason.clone(); let cloned = reason.clone();
assert_eq!(reason, cloned); assert_eq!(reason, cloned);
} }
@@ -196,7 +196,7 @@ mod tests {
impl PlayerEventEmitter for RecordingEmitter { impl PlayerEventEmitter for RecordingEmitter {
fn emit(&self, event: PlayerStatusEvent) { fn emit(&self, event: PlayerStatusEvent) {
self.events.lock().unwrap().push(event); self.events.lock_safe().push(event);
} }
} }
@@ -244,7 +244,7 @@ mod tests {
let (mut b, events) = backend(); let (mut b, events) = backend();
b.load(&test_media()).unwrap(); b.load(&test_media()).unwrap();
let ev = events.lock().unwrap(); let ev = events.lock_safe();
let load = ev let load = ev
.iter() .iter()
.find(|e| matches!(e, PlayerStatusEvent::WebviewAudioLoad { .. })) .find(|e| matches!(e, PlayerStatusEvent::WebviewAudioLoad { .. }))
@@ -263,7 +263,7 @@ mod tests {
b.pause().unwrap(); b.pause().unwrap();
b.seek(42.0).unwrap(); b.seek(42.0).unwrap();
let ev = events.lock().unwrap(); let ev = events.lock_safe();
assert!(ev.iter().any(|e| matches!( assert!(ev.iter().any(|e| matches!(
e, e,
PlayerStatusEvent::ControlCommand { action, .. } if action == "pause" PlayerStatusEvent::ControlCommand { action, .. } if action == "pause"
+10
View File
@@ -1134,6 +1134,16 @@ impl MediaRepository for HybridRepository {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
// `GATE_TEST_LOCK` below serialises the tests that flip the process-global
// `INCLUDE_CATALOG_BROWSE` flag, so its guard is deliberately held across
// the `.await` of the repository call under test — that await *is* the
// critical section. This is not the production deadlock hazard the lint
// targets: the lock is test-only, uncontended outside these tests, and each
// `#[tokio::test]` runs on its own single-threaded runtime, so a held guard
// cannot block another task on the same worker. Restructuring around it
// would reintroduce the flag race the lock exists to prevent.
#![allow(clippy::await_holding_lock)]
use super::*; use super::*;
use std::sync::Mutex; use std::sync::Mutex;
+12 -3
View File
@@ -2510,6 +2510,16 @@ impl MediaRepository for OfflineRepository {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
// `CATALOG_BROWSE_LOCK` below serialises the tests that flip the
// process-global `INCLUDE_CATALOG_BROWSE` flag, so its guard is deliberately
// held across the `.await` of the query under test — that await *is* the
// critical section. This is not the production deadlock hazard the lint
// targets: the lock is test-only, uncontended outside these tests, and each
// `#[tokio::test]` runs on its own single-threaded runtime, so a held guard
// cannot block another task on the same worker. Restructuring around it
// would reintroduce the flag race the lock exists to prevent.
#![allow(clippy::await_holding_lock)]
use super::*; use super::*;
use crate::storage::db_service::RusqliteService; use crate::storage::db_service::RusqliteService;
use rusqlite::Connection; use rusqlite::Connection;
@@ -2524,9 +2534,8 @@ mod tests {
static CATALOG_BROWSE_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); static CATALOG_BROWSE_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
fn lock_catalog_browse() -> std::sync::MutexGuard<'static, ()> { fn lock_catalog_browse() -> std::sync::MutexGuard<'static, ()> {
CATALOG_BROWSE_LOCK use crate::utils::lock::MutexSafe;
.lock() CATALOG_BROWSE_LOCK.lock_safe()
.unwrap_or_else(|poisoned| poisoned.into_inner())
} }
/// TRACES: UR-065 | DR-108 | UT-111 /// TRACES: UR-065 | DR-108 | UT-111
+22 -22
View File
@@ -990,7 +990,7 @@ struct JellyfinMediaSource {
} }
impl JellyfinItem { impl JellyfinItem {
fn to_media_item(self, server_id: String) -> MediaItem { fn into_media_item(self, server_id: String) -> MediaItem {
// Extract image tags before consuming self // Extract image tags before consuming self
let primary_tag = self.image_tags.as_ref().and_then(|tags| tags.primary()); let primary_tag = self.image_tags.as_ref().and_then(|tags| tags.primary());
let backdrop_tags = self.backdrop_image_tags; let backdrop_tags = self.backdrop_image_tags;
@@ -1129,7 +1129,7 @@ impl MediaRepository for OnlineRepository {
items: response items: response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect(), .collect(),
total_record_count: response.total_record_count, total_record_count: response.total_record_count,
}) })
@@ -1150,7 +1150,7 @@ impl MediaRepository for OnlineRepository {
let endpoint = format!("/Users/{}/Items/{}?Fields=BackdropImageTags,ParentBackdropImageTags,People,MediaStreams,MediaSources,PremiereDate,UserData", self.user_id, item_id); let endpoint = format!("/Users/{}/Items/{}?Fields=BackdropImageTags,ParentBackdropImageTags,People,MediaStreams,MediaSources,PremiereDate,UserData", self.user_id, item_id);
let item: JellyfinItem = self.get_json(&endpoint).await?; let item: JellyfinItem = self.get_json(&endpoint).await?;
let media_item = item.to_media_item(self.user_id.clone()); let media_item = item.into_media_item(self.user_id.clone());
Ok(media_item) Ok(media_item)
} }
@@ -1165,7 +1165,7 @@ impl MediaRepository for OnlineRepository {
let items: Vec<JellyfinItem> = self.get_json(&endpoint).await?; let items: Vec<JellyfinItem> = self.get_json(&endpoint).await?;
Ok(items Ok(items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect()) .collect())
} }
@@ -1196,7 +1196,7 @@ impl MediaRepository for OnlineRepository {
Ok(response Ok(response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect()) .collect())
} }
@@ -1215,7 +1215,7 @@ impl MediaRepository for OnlineRepository {
Ok(response Ok(response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect()) .collect())
} }
@@ -1235,7 +1235,7 @@ impl MediaRepository for OnlineRepository {
let items: Vec<MediaItem> = response let items: Vec<MediaItem> = response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect(); .collect();
debug!("[get_recently_played_audio] Fetched {} items", items.len()); debug!("[get_recently_played_audio] Fetched {} items", items.len());
@@ -1258,7 +1258,7 @@ impl MediaRepository for OnlineRepository {
"[get_recently_played_audio] Grouping item '{}' into album '{}'", "[get_recently_played_audio] Grouping item '{}' into album '{}'",
item.name, key item.name, key
); );
album_map.entry(key).or_insert_with(Vec::new).push(item); album_map.entry(key).or_default().push(item);
} else { } else {
debug!( debug!(
"[get_recently_played_audio] No album_id or album_name for item: '{}'", "[get_recently_played_audio] No album_id or album_name for item: '{}'",
@@ -1373,7 +1373,7 @@ impl MediaRepository for OnlineRepository {
Ok(response Ok(response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect()) .collect())
} }
@@ -1393,7 +1393,7 @@ impl MediaRepository for OnlineRepository {
Ok(response Ok(response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect()) .collect())
} }
@@ -1503,7 +1503,7 @@ impl MediaRepository for OnlineRepository {
items: response items: response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect(), .collect(),
total_record_count: response.total_record_count, total_record_count: response.total_record_count,
}) })
@@ -1881,7 +1881,7 @@ impl MediaRepository for OnlineRepository {
Ok(response Ok(response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.server_url.clone())) .map(|item| item.into_media_item(self.server_url.clone()))
.collect()) .collect())
} }
@@ -1894,7 +1894,7 @@ impl MediaRepository for OnlineRepository {
let items = response let items = response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.server_url.clone())) .map(|item| item.into_media_item(self.server_url.clone()))
.collect(); .collect();
Ok(SearchResult { Ok(SearchResult {
items, items,
@@ -2231,7 +2231,7 @@ impl MediaRepository for OnlineRepository {
items: response items: response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect(), .collect(),
total_record_count: response.total_record_count, total_record_count: response.total_record_count,
}) })
@@ -2374,7 +2374,7 @@ impl MediaRepository for OnlineRepository {
async fn get_person(&self, person_id: &str) -> Result<MediaItem, RepoError> { async fn get_person(&self, person_id: &str) -> Result<MediaItem, RepoError> {
let endpoint = format!("/Users/{}/Items/{}", self.user_id, person_id); let endpoint = format!("/Users/{}/Items/{}", self.user_id, person_id);
let item: JellyfinItem = self.get_json(&endpoint).await?; let item: JellyfinItem = self.get_json(&endpoint).await?;
Ok(item.to_media_item(self.user_id.clone())) Ok(item.into_media_item(self.user_id.clone()))
} }
/// A person's filmography — every item they are credited on. /// A person's filmography — every item they are credited on.
@@ -2407,7 +2407,7 @@ impl MediaRepository for OnlineRepository {
items: response items: response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect(), .collect(),
total_record_count: response.total_record_count, total_record_count: response.total_record_count,
}) })
@@ -2431,7 +2431,7 @@ impl MediaRepository for OnlineRepository {
items: response items: response
.items .items
.into_iter() .into_iter()
.map(|item| item.to_media_item(self.user_id.clone())) .map(|item| item.into_media_item(self.user_id.clone()))
.collect(), .collect(),
total_record_count: response.total_record_count, total_record_count: response.total_record_count,
}) })
@@ -2519,7 +2519,7 @@ impl MediaRepository for OnlineRepository {
.into_iter() .into_iter()
.map(|pi| PlaylistEntry { .map(|pi| PlaylistEntry {
playlist_item_id: pi.playlist_item_id, playlist_item_id: pi.playlist_item_id,
item: pi.item.to_media_item(self.user_id.clone()), item: pi.item.into_media_item(self.user_id.clone()),
}) })
.collect()) .collect())
} }
@@ -3017,7 +3017,7 @@ mod tests {
})) }))
.expect("fixture must deserialize"); .expect("fixture must deserialize");
let streams = item.to_media_item("server-1".to_string()).media_streams; let streams = item.into_media_item("server-1".to_string()).media_streams;
let streams = streams.expect("the item carries streams"); let streams = streams.expect("the item carries streams");
let deliverable = |index: i32| { let deliverable = |index: i32| {
streams streams
@@ -3612,7 +3612,7 @@ mod tests {
}"#; }"#;
let item: JellyfinItem = serde_json::from_str(json).expect("item should deserialize"); let item: JellyfinItem = serde_json::from_str(json).expect("item should deserialize");
let media = item.to_media_item("server1".to_string()); let media = item.into_media_item("server1".to_string());
let user_data = media.user_data.expect("user data should be mapped"); let user_data = media.user_data.expect("user data should be mapped");
assert_eq!(user_data.is_favorite, Some(true)); assert_eq!(user_data.is_favorite, Some(true));
@@ -3632,7 +3632,7 @@ mod tests {
let json = r#"{"Id": "x", "Name": "No User Data", "Type": "Movie"}"#; let json = r#"{"Id": "x", "Name": "No User Data", "Type": "Movie"}"#;
let item: JellyfinItem = serde_json::from_str(json).expect("item should deserialize"); let item: JellyfinItem = serde_json::from_str(json).expect("item should deserialize");
let media = item.to_media_item("server1".to_string()); let media = item.into_media_item("server1".to_string());
assert!(media.user_data.is_none()); assert!(media.user_data.is_none());
} }
@@ -3676,7 +3676,7 @@ mod tests {
}"#; }"#;
let jellyfin_item: JellyfinItem = serde_json::from_str(json).expect("Failed to parse"); let jellyfin_item: JellyfinItem = serde_json::from_str(json).expect("Failed to parse");
let media_item = jellyfin_item.to_media_item("test-server-id".to_string()); let media_item = jellyfin_item.into_media_item("test-server-id".to_string());
assert_eq!(media_item.id, "album456"); assert_eq!(media_item.id, "album456");
assert_eq!(media_item.name, "Love and Theft"); assert_eq!(media_item.name, "Love and Theft");
+4
View File
@@ -155,6 +155,10 @@ impl ThumbnailCache {
} }
/// Save thumbnail to cache /// Save thumbnail to cache
// The arguments are the cache key (item/type/tag) plus the payload and its
// dimensions — all independent scalars borrowed from the caller. A parameter
// struct would only move the same list one level down.
#[allow(clippy::too_many_arguments)]
pub async fn save_thumbnail( pub async fn save_thumbnail(
&self, &self,
db: Arc<RusqliteService>, db: Arc<RusqliteService>,