From e2bf97db99c56cc0f5bb137831d2d85494fb38fb Mon Sep 17 00:00:00 2001 From: pmphfm <82890237+pmphfm@users.noreply.github.com> Date: Tue, 19 Oct 2021 19:31:17 -0700 Subject: [PATCH] Code cleanup (#148) Fixed all most all clippy warnings. Test: cargo test && cargo clippy --- src/app/ddns/manager.rs | 1 - src/app/index/metadata.rs | 16 ++++------ src/app/index/mod.rs | 8 +++-- src/app/index/query.rs | 3 +- src/app/index/update/cleaner.rs | 1 - src/app/index/update/collector.rs | 7 ++--- src/app/index/update/inserter.rs | 50 ++++++++++++------------------- src/app/index/update/traverser.rs | 26 +++++++--------- src/app/lastfm/manager.rs | 6 ++-- src/app/playlist/manager.rs | 3 +- src/app/settings/manager.rs | 1 - src/app/thumbnail/generate.rs | 2 +- src/app/user/manager.rs | 3 +- src/app/user/preferences.rs | 1 - src/app/vfs/manager.rs | 2 +- src/app/vfs/mod.rs | 3 +- src/db/mod.rs | 5 ++-- src/main.rs | 2 +- src/paths.rs | 2 +- src/service/actix/api.rs | 15 +++++----- src/service/dto.rs | 2 ++ src/test.rs | 2 +- src/utils.rs | 1 + 23 files changed, 68 insertions(+), 94 deletions(-) diff --git a/src/app/ddns/manager.rs b/src/app/ddns/manager.rs index 6e4aef2..fd14fcb 100644 --- a/src/app/ddns/manager.rs +++ b/src/app/ddns/manager.rs @@ -3,7 +3,6 @@ use diesel::prelude::*; use log::{error, info}; use std::thread; use std::time; -use ureq; use super::*; use crate::db::DB; diff --git a/src/app/index/metadata.rs b/src/app/index/metadata.rs index 8a10f48..f0051fc 100644 --- a/src/app/index/metadata.rs +++ b/src/app/index/metadata.rs @@ -1,12 +1,6 @@ use anyhow::*; -use ape; -use id3; use lewton::inside_ogg::OggStreamReader; use log::error; -use metaflac; -use mp3_duration; -use mp4ameta; -use opus_headers; use regex::Regex; use std::fs; use std::path::Path; @@ -52,13 +46,13 @@ impl From for SongTags { let label = tag.get_text("TPUB"); SongTags { + disc_number, + track_number, + title, + duration, artist, album_artist, album, - title, - duration, - disc_number, - track_number, year, has_artwork, lyricist, @@ -296,7 +290,7 @@ fn read_flac(path: &Path) -> Result { let tag = metaflac::Tag::read_from_path(path)?; let vorbis = tag .vorbis_comments() - .ok_or(anyhow!("Missing Vorbis comments"))?; + .ok_or_else(|| anyhow!("Missing Vorbis comments"))?; let disc_number = vorbis .get("DISCNUMBER") .and_then(|d| d[0].parse::().ok()); diff --git a/src/app/index/mod.rs b/src/app/index/mod.rs index f4b4ffd..c166d51 100644 --- a/src/app/index/mod.rs +++ b/src/app/index/mod.rs @@ -1,4 +1,3 @@ -use diesel; use log::error; use std::sync::{Arc, Condvar, Mutex}; use std::time::Duration; @@ -31,7 +30,12 @@ impl Index { db, vfs_manager, settings_manager, - pending_reindex: Arc::new((Mutex::new(false), Condvar::new())), + + pending_reindex: Arc::new(( + #[allow(clippy::clippy::mutex_atomic)] + Mutex::new(false), + Condvar::new(), + )), }; let commands_index = index.clone(); diff --git a/src/app/index/query.rs b/src/app/index/query.rs index f2689f1..a27978a 100644 --- a/src/app/index/query.rs +++ b/src/app/index/query.rs @@ -1,5 +1,4 @@ use anyhow::*; -use diesel; use diesel::dsl::sql; use diesel::prelude::*; use diesel::sql_types; @@ -89,7 +88,7 @@ impl Index { .virtual_to_real(virtual_path) .map_err(|_| QueryError::VFSPathNotFound)?; let song_path_filter = { - let mut path_buf = real_path.clone(); + let mut path_buf = real_path; path_buf.push("%"); path_buf.as_path().to_string_lossy().into_owned() }; diff --git a/src/app/index/update/cleaner.rs b/src/app/index/update/cleaner.rs index 2089e17..96fc99a 100644 --- a/src/app/index/update/cleaner.rs +++ b/src/app/index/update/cleaner.rs @@ -1,5 +1,4 @@ use anyhow::*; -use diesel; use diesel::prelude::*; use rayon::prelude::*; use std::path::Path; diff --git a/src/app/index/update/collector.rs b/src/app/index/update/collector.rs index 8de238a..60a5382 100644 --- a/src/app/index/update/collector.rs +++ b/src/app/index/update/collector.rs @@ -24,11 +24,8 @@ impl Collector { } pub fn collect(&self) { - loop { - match self.receiver.recv() { - Ok(directory) => self.collect_directory(directory), - Err(_) => break, - } + while let Ok(directory) = self.receiver.recv() { + self.collect_directory(directory); } } diff --git a/src/app/index/update/inserter.rs b/src/app/index/update/inserter.rs index eaef071..2b26943 100644 --- a/src/app/index/update/inserter.rs +++ b/src/app/index/update/inserter.rs @@ -1,6 +1,5 @@ use anyhow::*; use crossbeam_channel::Receiver; -use diesel; use diesel::prelude::*; use log::error; @@ -57,19 +56,16 @@ impl Inserter { let new_directories = Vec::with_capacity(INDEX_BUILDING_INSERT_BUFFER_SIZE); let new_songs = Vec::with_capacity(INDEX_BUILDING_INSERT_BUFFER_SIZE); Self { - db, receiver, new_directories, new_songs, + db, } } pub fn insert(&mut self) { - loop { - match self.receiver.recv() { - Ok(item) => self.insert_item(item), - Err(_) => break, - } + while let Ok(item) = self.receiver.recv() { + self.insert_item(item); } } @@ -91,34 +87,26 @@ impl Inserter { } fn flush_directories(&mut self) { - if self - .db - .connect() - .and_then(|connection| { - diesel::insert_into(directories::table) - .values(&self.new_directories) - .execute(&*connection) // TODO https://github.com/diesel-rs/diesel/issues/1822 - .map_err(Error::new) - }) - .is_err() - { + let res = self.db.connect().and_then(|connection| { + diesel::insert_into(directories::table) + .values(&self.new_directories) + .execute(&*connection) // TODO https://github.com/diesel-rs/diesel/issues/1822 + .map_err(Error::new) + }); + if res.is_err() { error!("Could not insert new directories in database"); } self.new_directories.clear(); } fn flush_songs(&mut self) { - if self - .db - .connect() - .and_then(|connection| { - diesel::insert_into(songs::table) - .values(&self.new_songs) - .execute(&*connection) // TODO https://github.com/diesel-rs/diesel/issues/1822 - .map_err(Error::new) - }) - .is_err() - { + let res = self.db.connect().and_then(|connection| { + diesel::insert_into(songs::table) + .values(&self.new_songs) + .execute(&*connection) // TODO https://github.com/diesel-rs/diesel/issues/1822 + .map_err(Error::new) + }); + if res.is_err() { error!("Could not insert new songs in database"); } self.new_songs.clear(); @@ -127,10 +115,10 @@ impl Inserter { impl Drop for Inserter { fn drop(&mut self) { - if self.new_directories.len() > 0 { + if !self.new_directories.is_empty() { self.flush_directories(); } - if self.new_songs.len() > 0 { + if !self.new_songs.is_empty() { self.flush_songs(); } } diff --git a/src/app/index/update/traverser.rs b/src/app/index/update/traverser.rs index e5533a9..be12e4b 100644 --- a/src/app/index/update/traverser.rs +++ b/src/app/index/update/traverser.rs @@ -49,7 +49,7 @@ impl Traverser { let num_threads = std::env::var_os(key) .map(|v| v.to_string_lossy().to_string()) .and_then(|v| usize::from_str(&v).ok()) - .unwrap_or(min(num_cpus::get(), 4)); + .unwrap_or_else(|| min(num_cpus::get(), 4)); info!("Browsing collection using {} threads", num_threads); let mut threads = Vec::new(); @@ -107,14 +107,12 @@ impl Worker { if self.is_all_work_done() { return None; } + if let Ok(w) = self + .work_item_receiver + .recv_timeout(Duration::from_millis(100)) { - if let Ok(w) = self - .work_item_receiver - .recv_timeout(Duration::from_millis(100)) - { - return Some(w); - } - }; + return Some(w); + } } } @@ -167,12 +165,10 @@ impl Worker { if path.is_dir() { sub_directories.push(path); + } else if let Some(metadata) = metadata::read(&path) { + songs.push(Song { path, metadata }); } else { - if let Some(metadata) = metadata::read(&path) { - songs.push(Song { path, metadata }); - } else { - other_files.push(path); - } + other_files.push(path); } } @@ -180,7 +176,7 @@ impl Worker { self.emit_directory(Directory { path: work_item.path.to_owned(), - parent: work_item.parent.map(|p| p.to_owned()), + parent: work_item.parent, songs, other_files, created, @@ -195,7 +191,7 @@ impl Worker { } fn get_date_created(path: &Path) -> Option { - if let Ok(t) = fs::metadata(path).and_then(|m| m.created().or(m.modified())) { + if let Ok(t) = fs::metadata(path).and_then(|m| m.created().or_else(|_| m.modified())) { t.duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs() as i32) .ok() diff --git a/src/app/lastfm/manager.rs b/src/app/lastfm/manager.rs index 843f021..546ec2e 100644 --- a/src/app/lastfm/manager.rs +++ b/src/app/lastfm/manager.rs @@ -61,7 +61,7 @@ impl Manager { } pub fn link(&self, username: &str, lastfm_token: &str) -> Result<()> { - let mut scrobbler = Scrobbler::new(LASTFM_API_KEY.into(), LASTFM_API_SECRET.into()); + let mut scrobbler = Scrobbler::new(LASTFM_API_KEY, LASTFM_API_SECRET); let auth_response = scrobbler.authenticate_with_token(lastfm_token)?; self.user_manager @@ -74,7 +74,7 @@ impl Manager { } pub fn scrobble(&self, username: &str, track: &Path) -> Result<()> { - let mut scrobbler = Scrobbler::new(LASTFM_API_KEY.into(), LASTFM_API_SECRET.into()); + let mut scrobbler = Scrobbler::new(LASTFM_API_KEY, LASTFM_API_SECRET); let scrobble = self.scrobble_from_path(track)?; let auth_token = self.user_manager.get_lastfm_session_key(username)?; scrobbler.authenticate_with_session_key(&auth_token); @@ -83,7 +83,7 @@ impl Manager { } pub fn now_playing(&self, username: &str, track: &Path) -> Result<()> { - let mut scrobbler = Scrobbler::new(LASTFM_API_KEY.into(), LASTFM_API_SECRET.into()); + let mut scrobbler = Scrobbler::new(LASTFM_API_KEY, LASTFM_API_SECRET); let scrobble = self.scrobble_from_path(track)?; let auth_token = self.user_manager.get_lastfm_session_key(username)?; scrobbler.authenticate_with_session_key(&auth_token); diff --git a/src/app/playlist/manager.rs b/src/app/playlist/manager.rs index f8e56ba..ded563c 100644 --- a/src/app/playlist/manager.rs +++ b/src/app/playlist/manager.rs @@ -1,6 +1,5 @@ use anyhow::Result; use core::clone::Clone; -use diesel; use diesel::prelude::*; use diesel::sql_types; use diesel::BelongingToDsl; @@ -171,7 +170,7 @@ impl Manager { ORDER BY ps.ordering "#, ); - let query = query.clone().bind::(playlist.id); + let query = query.bind::(playlist.id); songs = query.get_results(&connection).map_err(anyhow::Error::new)?; } diff --git a/src/app/settings/manager.rs b/src/app/settings/manager.rs index 0a9cfc4..f62db1c 100644 --- a/src/app/settings/manager.rs +++ b/src/app/settings/manager.rs @@ -1,4 +1,3 @@ -use diesel; use diesel::prelude::*; use regex::Regex; use std::convert::TryInto; diff --git a/src/app/thumbnail/generate.rs b/src/app/thumbnail/generate.rs index 94c56bc..38b3720 100644 --- a/src/app/thumbnail/generate.rs +++ b/src/app/thumbnail/generate.rs @@ -23,7 +23,7 @@ pub fn generate_thumbnail(image_path: &Path, options: &Options) -> Result 0) + Ok(!results.is_empty()) } pub fn is_admin(&self, username: &str) -> Result { diff --git a/src/app/user/preferences.rs b/src/app/user/preferences.rs index 2b18a74..4b01ba1 100644 --- a/src/app/user/preferences.rs +++ b/src/app/user/preferences.rs @@ -1,5 +1,4 @@ use anyhow::Result; -use diesel; use diesel::prelude::*; use serde::{Deserialize, Serialize}; diff --git a/src/app/vfs/manager.rs b/src/app/vfs/manager.rs index d405215..f4a5ea3 100644 --- a/src/app/vfs/manager.rs +++ b/src/app/vfs/manager.rs @@ -30,7 +30,7 @@ impl Manager { Ok(mount_dirs) } - pub fn set_mount_dirs(&self, mount_dirs: &Vec) -> Result<()> { + pub fn set_mount_dirs(&self, mount_dirs: &[MountDir]) -> Result<()> { use self::mount_points::dsl::*; let connection = self.db.connect()?; diesel::delete(mount_points).execute(&connection)?; diff --git a/src/app/vfs/mod.rs b/src/app/vfs/mod.rs index 0ae9d9f..b214256 100644 --- a/src/app/vfs/mod.rs +++ b/src/app/vfs/mod.rs @@ -34,11 +34,12 @@ impl From for Mount { let source = PathBuf::from(path_string.deref()); Self { name: m.name, - source: source, + source, } } } +#[allow(clippy::upper_case_acronyms)] pub struct VFS { mounts: Vec, } diff --git a/src/db/mod.rs b/src/db/mod.rs index c99e598..c12a26f 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -2,7 +2,6 @@ use anyhow::*; use diesel::r2d2::{self, ConnectionManager, PooledConnection}; use diesel::sqlite::SqliteConnection; use diesel::RunQueryDsl; -use diesel_migrations; use std::path::Path; mod schema; @@ -34,7 +33,7 @@ impl diesel::r2d2::CustomizeConnection ); query .execute(connection) - .map_err(|e| diesel::r2d2::Error::QueryError(e))?; + .map_err(diesel::r2d2::Error::QueryError)?; Ok(()) } } @@ -46,7 +45,7 @@ impl DB { let pool = diesel::r2d2::Pool::builder() .connection_customizer(Box::new(ConnectionCustomizer {})) .build(manager)?; - let db = DB { pool: pool }; + let db = DB { pool }; db.migrate_up()?; Ok(db) } diff --git a/src/main.rs b/src/main.rs index 2495d1d..fc9faa2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -62,7 +62,7 @@ fn init_logging>(log_level: LevelFilter, log_file_path: &Option, new_mount_dirs: Json>, ) -> Result { - let new_mount_dirs = new_mount_dirs + let new_mount_dirs: Vec = new_mount_dirs .to_owned() .into_iter() .map(|m| m.into()) @@ -515,10 +516,8 @@ async fn update_user( user_update: Json, ) -> Result { if let Some(auth) = &admin_rights.auth { - if auth.username == name.as_str() { - if user_update.new_is_admin == Some(false) { - return Err(APIError::OwnAdminPrivilegeRemoval); - } + if auth.username == name.as_str() && user_update.new_is_admin == Some(false) { + return Err(APIError::OwnAdminPrivilegeRemoval); } } diff --git a/src/service/dto.rs b/src/service/dto.rs index b5fa39c..47c0184 100644 --- a/src/service/dto.rs +++ b/src/service/dto.rs @@ -1,6 +1,7 @@ use serde::{Deserialize, Serialize}; use crate::app::{config, ddns, settings, thumbnail, user, vfs}; +use std::convert::From; pub const API_MAJOR_VERSION: i32 = 6; pub const API_MINOR_VERSION: i32 = 0; @@ -60,6 +61,7 @@ pub enum ThumbnailSize { Native, } +#[allow(clippy::clippy::clippy::from_over_into)] impl Into> for ThumbnailSize { fn into(self) -> Option { match self { diff --git a/src/test.rs b/src/test.rs index c910d38..2e27bdc 100644 --- a/src/test.rs +++ b/src/test.rs @@ -7,7 +7,7 @@ macro_rules! test_name { let file_name = file_name.replace("/", "-"); let file_name = file_name.replace("\\", "-"); format!("{}-line-{}", file_name, line!()) - }}; + }}; } pub fn prepare_test_directory>(test_name: T) -> PathBuf { diff --git a/src/utils.rs b/src/utils.rs index 818ce4a..a4d6d90 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -12,6 +12,7 @@ macro_rules! match_ignore_case { } pub use crate::match_ignore_case; +#[allow(clippy::upper_case_acronyms)] #[derive(Debug, PartialEq)] pub enum AudioFormat { AIFF,