diff --git a/src/commands.rs b/src/commands.rs index d9319b8..6695ed4 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -113,8 +113,8 @@ impl CommandManager { match cmd { Command::Noop => Ok(None), Command::Quit => { - let queue = self.queue.queue.read().expect("can't readlock queue"); - self.config.with_state_mut(move |mut s| { + let queue = self.queue.queue.read().unwrap(); + self.config.with_state_mut(move |s| { debug!( "saving state, {} items, current track: {:?}", queue.len(), diff --git a/src/config.rs b/src/config.rs index ed5958e..51e6732 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,12 +1,12 @@ use std::collections::HashMap; use std::error::Error; use std::path::PathBuf; -use std::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard}; +use std::sync::{RwLock, RwLockReadGuard}; use std::{fs, process}; use cursive::theme::Theme; use log::{debug, error}; -use ncspot::CONFIGURATION_FILE_NAME; +use ncspot::{CONFIGURATION_FILE_NAME, USER_STATE_FILE_NAME}; use platform_dirs::AppDirs; use crate::command::{SortDirection, SortKey}; @@ -209,7 +209,7 @@ impl Config { }); let mut userstate = { - let path = config_path("userstate.cbor"); + let path = config_path(USER_STATE_FILE_NAME); CBOR.load_or_generate_default(path, || Ok(UserState::default()), true) .expect("could not load user state") }; @@ -235,36 +235,36 @@ impl Config { /// Get the user configuration values. pub fn values(&self) -> RwLockReadGuard { - self.values.read().expect("can't readlock config values") + self.values.read().unwrap() } /// Get the runtime user state values. pub fn state(&self) -> RwLockReadGuard { - self.state.read().expect("can't readlock user state") + self.state.read().unwrap() } /// Modify the internal user state through a shared reference using a closure. pub fn with_state_mut(&self, cb: F) where - F: Fn(RwLockWriteGuard), + F: Fn(&mut UserState), { - let state_guard = self.state.write().expect("can't writelock user state"); - cb(state_guard); + let mut state_guard = self.state.write().unwrap(); + cb(&mut state_guard); } /// Update the version number of the runtime user state. This should be done before saving it to /// disk. fn update_state_cache_version(&self) { - self.with_state_mut(|mut state| state.cache_version = CACHE_VERSION); + self.with_state_mut(|state| state.cache_version = CACHE_VERSION); } /// Save runtime state to the user configuration directory. pub fn save_state(&self) { self.update_state_cache_version(); - let path = config_path("userstate.cbor"); + let path = config_path(USER_STATE_FILE_NAME); debug!("saving user state to {}", path.display()); - if let Err(e) = CBOR.write(path, self.state().clone()) { + if let Err(e) = CBOR.write(path, &*self.state()) { error!("Could not save user state: {}", e); } } diff --git a/src/events.rs b/src/events.rs index fda79f8..9921b4b 100644 --- a/src/events.rs +++ b/src/events.rs @@ -39,14 +39,12 @@ impl EventManager { /// Send a new event to be handled. pub fn send(&self, event: Event) { - self.tx.send(event).expect("could not send event"); + self.tx.send(event).unwrap(); self.trigger(); } /// Send a no-op to the Cursive event loop to trigger immediate processing of events. pub fn trigger(&self) { - self.cursive_sink - .send(Box::new(Cursive::noop)) - .expect("could not send no-op event to cursive"); + self.cursive_sink.send(Box::new(Cursive::noop)).unwrap(); } } diff --git a/src/ipc.rs b/src/ipc.rs index 692ffce..dd8a845 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -70,7 +70,7 @@ impl IpcSocket { mode: event.clone(), playable, }; - self.tx.send(status).expect("Error publishing IPC update"); + self.tx.send(status).unwrap(); } async fn worker(listener: UnixListener, ev: EventManager, tx: Receiver) { diff --git a/src/lib.rs b/src/lib.rs index 16c8e23..8a67b7b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,7 @@ use librespot_playback::audio_backend; pub const AUTHOR: &str = "Henrik Friedrichsen and contributors"; pub const BIN_NAME: &str = "ncspot"; pub const CONFIGURATION_FILE_NAME: &str = "config.toml"; +pub const USER_STATE_FILE_NAME: &str = "userstate.cbor"; /// Return the [Command](clap::Command) that models the program's command line arguments. The /// command can be used to parse the actual arguments passed to the program, or to automatically diff --git a/src/library.rs b/src/library.rs index 9fdddb2..5e3be13 100644 --- a/src/library.rs +++ b/src/library.rs @@ -133,7 +133,7 @@ impl Library { /// Append `updated` to the local playlists or update the local version if it exists. Return the /// index of the appended/updated playlist. fn append_or_update(&self, updated: Playlist) -> usize { - let mut store = self.playlists.write().expect("can't writelock playlists"); + let mut store = self.playlists.write().unwrap(); for (index, local) in store.iter_mut().enumerate() { if local.id == updated.id { *local = updated; @@ -159,10 +159,7 @@ impl Library { if let Some(position) = position { if self.spotify.api.delete_playlist(id).is_ok() { - self.playlists - .write() - .expect("can't writelock playlists") - .remove(position); + self.playlists.write().unwrap().remove(position); self.save_cache( &config::cache_path(CACHE_PLAYLISTS), &self.playlists.read().unwrap(), @@ -581,7 +578,7 @@ impl Library { /// If there is a local version of the playlist, update it and rewrite the cache. pub fn playlist_update(&self, updated: &Playlist) { { - let mut playlists = self.playlists.write().expect("can't writelock playlists"); + let mut playlists = self.playlists.write().unwrap(); if let Some(playlist) = playlists.iter_mut().find(|p| p.id == updated.id) { *playlist = updated.clone(); } diff --git a/src/mpris.rs b/src/mpris.rs index 470f1ae..ef10145 100644 --- a/src/mpris.rs +++ b/src/mpris.rs @@ -372,9 +372,8 @@ impl MprisPlayer { fn open_uri(&self, uri: &str) { let spotify_url = if uri.contains("open.spotify.com") { SpotifyUrl::from_url(uri) - } else if UriType::from_uri(uri).is_some() { + } else if let Ok(uri_type) = uri.parse() { let id = &uri[uri.rfind(':').unwrap_or(0) + 1..uri.len()]; - let uri_type = UriType::from_uri(uri).unwrap(); Some(SpotifyUrl::new(id, uri_type)) } else { None diff --git a/src/queue.rs b/src/queue.rs index a981d06..dce7b13 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -423,7 +423,7 @@ impl Queue { /// Set the current repeat behavior and save it to the configuration. pub fn set_repeat(&self, new: RepeatSetting) { - self.cfg.with_state_mut(|mut s| s.repeat = new); + self.cfg.with_state_mut(|s| s.repeat = new); } /// Get the current shuffle behavior. @@ -457,7 +457,7 @@ impl Queue { /// Set the current shuffle behavior. pub fn set_shuffle(&self, new: bool) { - self.cfg.with_state_mut(|mut s| s.shuffle = new); + self.cfg.with_state_mut(|s| s.shuffle = new); if new { self.generate_random_order(); } else { diff --git a/src/spotify.rs b/src/spotify.rs index d6555ef..9d1d099 100644 --- a/src/spotify.rs +++ b/src/spotify.rs @@ -1,7 +1,8 @@ -use std::env; +use std::error::Error; use std::str::FromStr; use std::sync::{Arc, RwLock}; use std::time::{Duration, SystemTime}; +use std::{env, fmt}; use futures::channel::oneshot; use librespot_core::authentication::Credentials; @@ -56,8 +57,6 @@ pub struct Spotify { since: Arc>>, /// Channel to send commands to the worker thread. channel: Arc>>>, - /// The username of the logged in user. - user: Option, } impl Spotify { @@ -71,19 +70,18 @@ impl Spotify { elapsed: Arc::new(RwLock::new(None)), since: Arc::new(RwLock::new(None)), channel: Arc::new(RwLock::new(None)), - user: None, }; let (user_tx, user_rx) = oneshot::channel(); spotify.start_worker(Some(user_tx)); - spotify.user = ASYNC_RUNTIME.get().unwrap().block_on(user_rx).ok(); + let user = ASYNC_RUNTIME.get().unwrap().block_on(user_rx).ok(); let volume = cfg.state().volume; spotify.set_volume(volume); spotify.api.set_worker_channel(spotify.channel.clone()); spotify.api.update_token(); - spotify.api.set_user(spotify.user.clone()); + spotify.api.set_user(user); spotify } @@ -92,10 +90,7 @@ impl Spotify { /// in user. pub fn start_worker(&self, user_tx: Option>) { let (tx, rx) = mpsc::unbounded_channel(); - *self - .channel - .write() - .expect("can't writelock worker channel") = Some(tx); + *self.channel.write().unwrap() = Some(tx); { let worker_channel = self.channel.clone(); let cfg = self.cfg.clone(); @@ -144,9 +139,10 @@ impl Spotify { credentials: Credentials, ) -> Result { let librespot_cache_path = config::cache_path("librespot"); - let audio_cache_path = match cfg.values().audio_cache.unwrap_or(true) { - true => Some(librespot_cache_path.join("files")), - false => None, + let audio_cache_path = if let Some(false) = cfg.values().audio_cache { + None + } else { + Some(librespot_cache_path.join("files")) }; let cache = Cache::new( Some(librespot_cache_path.clone()), @@ -243,18 +239,13 @@ impl Spotify { worker.run_loop().await; error!("worker thread died, requesting restart"); - *worker_channel - .write() - .expect("can't writelock worker channel") = None; + *worker_channel.write().unwrap() = None; events.send(Event::SessionDied) } /// Get the current playback status of the [Player]. pub fn get_current_status(&self) -> PlayerEvent { - let status = self - .status - .read() - .expect("could not acquire read lock on playback status"); + let status = self.status.read().unwrap(); (*status).clone() } @@ -268,34 +259,22 @@ impl Spotify { } fn set_elapsed(&self, new_elapsed: Option) { - let mut elapsed = self - .elapsed - .write() - .expect("could not acquire write lock on elapsed time"); + let mut elapsed = self.elapsed.write().unwrap(); *elapsed = new_elapsed; } fn get_elapsed(&self) -> Option { - let elapsed = self - .elapsed - .read() - .expect("could not acquire read lock on elapsed time"); + let elapsed = self.elapsed.read().unwrap(); *elapsed } fn set_since(&self, new_since: Option) { - let mut since = self - .since - .write() - .expect("could not acquire write lock on since time"); + let mut since = self.since.write().unwrap(); *since = new_since; } fn get_since(&self) -> Option { - let since = self - .since - .read() - .expect("could not acquire read lock on since time"); + let since = self.since.read().unwrap(); *since } @@ -329,10 +308,7 @@ impl Spotify { } } - let mut status = self - .status - .write() - .expect("could not acquire write lock on player status"); + let mut status = self.status.write().unwrap(); *status = new_status; } @@ -361,7 +337,7 @@ impl Spotify { /// Send a [WorkerCommand] to the worker thread. fn send_worker(&self, cmd: WorkerCommand) { info!("sending command to worker: {:?}", cmd); - let channel = self.channel.read().expect("can't readlock worker channel"); + let channel = self.channel.read().unwrap(); match channel.as_ref() { Some(channel) => { if let Err(e) = channel.send(cmd) { @@ -407,7 +383,7 @@ impl Spotify { /// Set the current volume of the [Player]. pub fn set_volume(&self, volume: u16) { info!("setting volume to {}", volume); - self.cfg.with_state_mut(|mut s| s.volume = volume); + self.cfg.with_state_mut(|s| s.volume = volume); self.send_worker(WorkerCommand::SetVolume(volume)); } @@ -434,23 +410,59 @@ pub enum UriType { Episode, } -impl UriType { - /// Try to create a [UriType] from the given string. - pub fn from_uri(s: &str) -> Option { +#[derive(Debug)] +pub struct UriParseError; + +impl fmt::Display for UriParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("invalid Spotify URI") + } +} + +impl Error for UriParseError {} + +impl FromStr for UriType { + type Err = UriParseError; + + fn from_str(s: &str) -> Result { if s.starts_with("spotify:album:") { - Some(Self::Album) + Ok(Self::Album) } else if s.starts_with("spotify:artist:") { - Some(Self::Artist) + Ok(Self::Artist) } else if s.starts_with("spotify:track:") { - Some(Self::Track) + Ok(Self::Track) } else if s.starts_with("spotify:") && s.contains(":playlist:") { - Some(Self::Playlist) + Ok(Self::Playlist) } else if s.starts_with("spotify:show:") { - Some(Self::Show) + Ok(Self::Show) } else if s.starts_with("spotify:episode:") { - Some(Self::Episode) + Ok(Self::Episode) } else { - None + Err(UriParseError) } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_album_uri() { + let uri_type = "spotify:album:29F5MF6Q9VYlryDsYEQz6a".parse(); + assert!(matches!(uri_type, Ok(UriType::Album))); + } + + #[test] + fn parse_invalid_uri() { + let uri_type: Result = "kayava".parse(); + assert!(matches!(uri_type, Err(UriParseError))); + } + + #[test] + + fn parse_playlist_uri() { + let uri_type = "spotify:playlist:37i9dQZF1DX36Xw4IJIVKA".parse(); + assert!(matches!(uri_type, Ok(UriType::Playlist))); + } +} diff --git a/src/spotify_api.rs b/src/spotify_api.rs index a2a7c16..77a6bd4 100644 --- a/src/spotify_api.rs +++ b/src/spotify_api.rs @@ -95,26 +95,18 @@ impl WebApi { let (token_tx, token_rx) = std::sync::mpsc::channel(); let cmd = WorkerCommand::RequestToken(token_tx); - if let Some(channel) = self - .worker_channel - .read() - .expect("can't readlock worker channel") - .as_ref() - { - channel.send(cmd).expect("can't send message to worker"); + if let Some(channel) = self.worker_channel.read().unwrap().as_ref() { + channel.send(cmd).unwrap(); let token_option = token_rx.recv().unwrap(); if let Some(token) = token_option { - *self.api.token.lock().expect("can't writelock api token") = Some(Token { + *self.api.token.lock().unwrap() = Some(Token { access_token: token.access_token, expires_in: chrono::Duration::seconds(token.expires_in.into()), scopes: HashSet::from_iter(token.scope), expires_at: None, refresh_token: None, }); - *self - .token_expiration - .write() - .expect("could not writelock token") = + *self.token_expiration.write().unwrap() = Utc::now() + ChronoDuration::seconds(token.expires_in.into()); } else { error!("Failed to update token"); @@ -231,7 +223,7 @@ impl WebApi { None }; - if let Some(()) = self.api_with_retry(|api| { + let replace_items = self.api_with_retry(|api| { let playable_ids: Vec = tracks .iter() .filter_map(|playable| playable.into()) @@ -240,7 +232,9 @@ impl WebApi { PlaylistId::from_id(id).unwrap(), playable_ids.iter().map(|p| p.as_ref()), ) - }) { + }); + + if replace_items.is_some() { debug!("saved {} tracks to playlist {}", tracks.len(), id); while let Some(ref mut tracks) = remainder.clone() { // grab the next set of 100 tracks diff --git a/src/ui/playlist.rs b/src/ui/playlist.rs index 92a25bf..73752a7 100644 --- a/src/ui/playlist.rs +++ b/src/ui/playlist.rs @@ -93,7 +93,7 @@ impl ViewExt for PlaylistView { } if let Command::Sort(key, direction) = cmd { - self.library.cfg.with_state_mut(|mut state| { + self.library.cfg.with_state_mut(|state| { let order = crate::config::SortingOrder { key: key.clone(), direction: direction.clone(), diff --git a/src/ui/search_results.rs b/src/ui/search_results.rs index 0b48ae3..7d68e07 100644 --- a/src/ui/search_results.rs +++ b/src/ui/search_results.rs @@ -394,7 +394,7 @@ impl SearchResultsView { self.spotify.api.update_token(); // is the query a Spotify URI? - if let Some(uritype) = UriType::from_uri(&query) { + if let Ok(uritype) = query.parse() { match uritype { UriType::Track => { self.perform_search(