From d98b645377dea0a6a0e33f87bfbe2bc9fb67101a Mon Sep 17 00:00:00 2001 From: aleidk Date: Fri, 26 Jul 2024 16:28:09 -0400 Subject: [PATCH] refactor(player): change to dependency injection Use this pattern to decouple the dependencies and allow to test te code and introduce new dependencies in the future --- src/file_explorer.rs | 45 ------------------------------------- src/file_handler.rs | 34 ++++++++++++++++++++++++++++ src/main.rs | 9 +++++--- src/player.rs | 53 ++++++++++++++++++++++++++------------------ 4 files changed, 71 insertions(+), 70 deletions(-) delete mode 100644 src/file_explorer.rs create mode 100644 src/file_handler.rs diff --git a/src/file_explorer.rs b/src/file_explorer.rs deleted file mode 100644 index 682a44f..0000000 --- a/src/file_explorer.rs +++ /dev/null @@ -1,45 +0,0 @@ -use ignore::types::TypesBuilder; -use ignore::WalkBuilder; -use std::error::Error; -use std::path::PathBuf; - -pub fn walk_dir(path: &PathBuf) -> Result, Box> { - let mut types_builder = TypesBuilder::new(); - types_builder.add_defaults(); - - let accepted_filetypes = ["mp3", "flac", "wav"]; - - for filetype in accepted_filetypes { - let _ = types_builder.add("sound", format!("*.{}", filetype).as_str()); - } - - types_builder.select("sound"); - - // let mut base_path = env::current_dir().expect("Error accesing the enviroment"); - // - // match path { - // Some(dir) => { - // search_path = base_path - // .join(dir) - // .canonicalize() - // .expect("Couldn't canonicalizice the path") - // } - // None => search_path = base_path.to_owned(), - // } - // - // // PathBuf.join() can override the hole path, this ensure we're not accessing files outside - // // base_dir - // if !search_path.starts_with(base_path) { - // return Err("Tried to access file or directory outside of server `base_path` config."); - // } - - let entries: Vec = WalkBuilder::new(path) - .types(types_builder.build().unwrap()) - .build() - .filter_map(|entry| entry.ok()) - .filter(|entry| !entry.path().is_dir()) - .map(|entry| entry.path().to_path_buf()) - .collect(); - - Ok(entries) -} diff --git a/src/file_handler.rs b/src/file_handler.rs new file mode 100644 index 0000000..ad6769b --- /dev/null +++ b/src/file_handler.rs @@ -0,0 +1,34 @@ +use ignore::types::TypesBuilder; +use ignore::WalkBuilder; +use std::path::PathBuf; + +pub trait FileExplorer { + fn get_files(path: &PathBuf) -> Vec; +} + +pub struct LocalFileSystem; + +impl FileExplorer for LocalFileSystem { + fn get_files(path: &PathBuf) -> Vec { + let mut types_builder = TypesBuilder::new(); + types_builder.add_defaults(); + + let accepted_filetypes = ["mp3", "flac", "wav"]; + + for filetype in accepted_filetypes { + let _ = types_builder.add("sound", format!("*.{}", filetype).as_str()); + } + + types_builder.select("sound"); + + let entries: Vec = WalkBuilder::new(path) + .types(types_builder.build().unwrap()) + .build() + .filter_map(|entry| entry.ok()) + .filter(|entry| !entry.path().is_dir()) + .map(|entry| entry.path().to_path_buf()) + .collect(); + + entries + } +} diff --git a/src/main.rs b/src/main.rs index 664e6fa..1cc5604 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,14 +8,15 @@ use tokio::sync::mpsc; use crate::player::Player; use self::configuration::{Commands, Config, ConfigMode}; +use self::file_handler::{FileExplorer, LocalFileSystem}; use self::grpc::server::GrpcServerMessage; mod configuration; -mod file_explorer; +mod file_handler; mod grpc; mod player; -async fn handle_message(player: &mut Player, message: GrpcServerMessage) { +async fn handle_message(player: &mut Player, message: GrpcServerMessage) { match message { GrpcServerMessage::Play { resp } => { player.play(); @@ -64,7 +65,9 @@ async fn init_server(config: Config) -> Result<(), Box> { volume = config_volume; }; - let mut player = Player::new(base_path, volume).expect("Error creating player"); + let mut player = Player::new(LocalFileSystem, base_path).expect("Error creating player"); + + player.set_volume(volume); println!("Listening for incomming messages..."); diff --git a/src/player.rs b/src/player.rs index ec9cc04..6354791 100644 --- a/src/player.rs +++ b/src/player.rs @@ -6,17 +6,18 @@ use std::path::PathBuf; use rodio::{OutputStream, Sink}; -use crate::file_explorer::walk_dir; +use crate::file_handler::FileExplorer; #[allow(dead_code)] -pub struct Player { +pub struct Player { queue: VecDeque, sink: Sink, stream: OutputStream, base_dir: PathBuf, + explorer: T, } -impl std::ops::Deref for Player { +impl std::ops::Deref for Player { type Target = Sink; fn deref(&self) -> &Self::Target { @@ -24,19 +25,19 @@ impl std::ops::Deref for Player { } } -impl Player { - pub fn new(base_dir: PathBuf, volume: f32) -> Result> { - let queue = walk_dir(&base_dir)?; +impl Player { + pub fn new(explorer: T, base_dir: PathBuf) -> Result> { + let queue = T::get_files(&base_dir); // stream needs to exist as long as sink to work let (stream, stream_handle) = OutputStream::try_default()?; let sink = Sink::try_new(&stream_handle)?; - sink.set_volume(volume); Ok(Player { queue: VecDeque::from(queue), sink, stream, base_dir, + explorer, }) } @@ -64,20 +65,6 @@ impl Player { } pub fn get_files(&mut self, path: &PathBuf) -> Result, Box> { - // let mut base_path = env::current_dir().expect("Error accesing the enviroment"); - // - // match path { - // Some(dir) => { - // } - // None => search_path = base_path.to_owned(), - // } - // - // // PathBuf.join() can override the hole path, this ensure we're not accessing files outside - // // base_dir - // if !search_path.starts_with(base_path) { - // return Err("Tried to access file or directory outside of server `base_path` config."); - // } - let search_path = self .base_dir .join(path) @@ -89,7 +76,7 @@ impl Player { panic!("Tried to access file or directory outside of server `base_path` config.") } - Ok(walk_dir(&search_path)?) + Ok(T::get_files(&search_path)) } pub fn play(&mut self) { @@ -124,4 +111,26 @@ impl Player { self.sink.append(rodio::Decoder::new(BufReader::new(file))?); Ok(()) } + + pub fn set_volume(&self, volume: f32) { + self.sink.set_volume(volume); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + struct MockFileExplorer; + + impl FileExplorer for MockFileExplorer { + fn get_files(_: &PathBuf) -> Vec { + return vec![]; + } + } + + #[test] + fn player_works() { + let _ = Player::new(MockFileExplorer, PathBuf::from(".")).expect("Error creating player"); + } }