From 5d66558180428bcb1b8cba6daef5d6afd4957519 Mon Sep 17 00:00:00 2001 From: Savanni D'Gerinel Date: Wed, 20 Nov 2024 09:52:26 -0500 Subject: [PATCH 1/6] Set up a test to validate the function which gets available images There's a lot of work here that sets up dependency injection traits which will make it easier for me to keep writing tests and will make it easier for me to separate the Core from the support infrastructure. --- Cargo.lock | 98 +++++++++++++++++++++------------- visions/server/Cargo.toml | 2 + visions/server/src/asset_db.rs | 90 +++++++++++++++++++++++++++++++ visions/server/src/core.rs | 70 ++++++++++++++++++------ visions/server/src/handlers.rs | 10 ++-- visions/server/src/main.rs | 6 +-- visions/server/src/types.rs | 1 + 7 files changed, 217 insertions(+), 60 deletions(-) create mode 100644 visions/server/src/asset_db.rs diff --git a/Cargo.lock b/Cargo.lock index e982ac4..ae9f83c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -258,7 +258,7 @@ checksum = "721cae7de5c34fbb2acd27e21e6d2cf7b886dce0c27388d46c4e6c47ea4318dd" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -287,7 +287,7 @@ dependencies = [ "sha2", "sqlformat", "sqlx", - "thiserror", + "thiserror 1.0.64", "tokio", "uuid 0.4.0", ] @@ -368,7 +368,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -470,7 +470,7 @@ dependencies = [ "glib", "libc", "once_cell", - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -605,7 +605,7 @@ dependencies = [ "heck 0.5.0", "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -652,7 +652,7 @@ dependencies = [ "cool_asserts", "serde 1.0.210", "serde_json", - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -712,7 +712,7 @@ version = "0.1.0" dependencies = [ "nom", "proptest", - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -955,7 +955,7 @@ checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -984,7 +984,7 @@ dependencies = [ "serde_derive", "serde_json", "tempfile", - "thiserror", + "thiserror 1.0.64", "uuid 0.8.2", ] @@ -1130,7 +1130,7 @@ dependencies = [ "serde_json", "sha2", "tempdir", - "thiserror", + "thiserror 1.0.64", "tokio", "uuid 0.4.0", "warp", @@ -1153,7 +1153,7 @@ dependencies = [ "glib-build-tools 0.18.0", "gtk4", "libadwaita", - "thiserror", + "thiserror 1.0.64", "tokio", ] @@ -1230,7 +1230,7 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a530c4694a6a8d528794ee9bbd8ba0122e779629ac908d15ad5a7ae7763a33d" dependencies = [ - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -1373,7 +1373,7 @@ checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -1540,7 +1540,7 @@ dependencies = [ "once_cell", "pin-project-lite", "smallvec", - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -1576,7 +1576,7 @@ dependencies = [ "memchr", "once_cell", "smallvec", - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -1608,7 +1608,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -2120,7 +2120,7 @@ version = "0.1.0" dependencies = [ "chrono", "serde 1.0.210", - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -2801,7 +2801,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -2836,7 +2836,7 @@ dependencies = [ "serde 1.0.210", "serde_json", "sgf", - "thiserror", + "thiserror 1.0.64", "uuid 0.8.2", ] @@ -3044,7 +3044,7 @@ checksum = "3c0f5fad0874fc7abcd4d750e76917eaebbecaa2c20bde22e1dbeeba8beb758c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -3084,7 +3084,7 @@ dependencies = [ "nix", "once_cell", "pipewire-sys", - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -3584,7 +3584,7 @@ dependencies = [ name = "result-extended" version = "0.1.0" dependencies = [ - "thiserror", + "thiserror 1.0.64", ] [[package]] @@ -3776,7 +3776,7 @@ checksum = "243902eda00fad750862fc144cea25caca5e20d615af0a81bee94ca738f1df1f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -3836,7 +3836,7 @@ dependencies = [ "nary_tree", "nom", "serde 1.0.210", - "thiserror", + "thiserror 1.0.64", "typeshare", "uuid 0.8.2", ] @@ -4025,7 +4025,7 @@ dependencies = [ "sha2", "smallvec", "sqlformat", - "thiserror", + "thiserror 1.0.64", "tokio", "tokio-stream", "tracing", @@ -4107,7 +4107,7 @@ dependencies = [ "smallvec", "sqlx-core", "stringprep", - "thiserror", + "thiserror 1.0.64", "tracing", "whoami", ] @@ -4145,7 +4145,7 @@ dependencies = [ "smallvec", "sqlx-core", "stringprep", - "thiserror", + "thiserror 1.0.64", "tracing", "whoami", ] @@ -4209,9 +4209,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.79" +version = "2.0.87" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89132cd0bf050864e1d38dc3bbc07a0eb8e7530af26344d3d2bbbef83499f590" +checksum = "25aa4ce346d03a6dcd68dd8b4010bcb74e54e62c90c573f394c46eae99aba32d" dependencies = [ "proc-macro2", "quote", @@ -4302,7 +4302,16 @@ version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d50af8abc119fb8bb6dbabcfa89656f46f84aa0ac7688088608076ad2b459a84" dependencies = [ - "thiserror-impl", + "thiserror-impl 1.0.64", +] + +[[package]] +name = "thiserror" +version = "2.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c006c85c7651b3cf2ada4584faa36773bd07bac24acfb39f3c431b36d7e667aa" +dependencies = [ + "thiserror-impl 2.0.3", ] [[package]] @@ -4313,7 +4322,18 @@ checksum = "08904e7672f5eb876eaaf87e0ce17857500934f4981c4a0ab2b4aa98baac7fc3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f077553d607adc1caf65430528a576c757a71ed73944b66ebb58ef2bbd243568" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.87", ] [[package]] @@ -4438,7 +4458,7 @@ checksum = "693d596312e88961bc67d7f1f97af8a70227d9f90c31bba5806eec004978d752" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -4558,7 +4578,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -4600,7 +4620,7 @@ dependencies = [ "log 0.4.22", "rand 0.8.5", "sha1", - "thiserror", + "thiserror 1.0.64", "url 2.5.2", "utf-8", ] @@ -4654,7 +4674,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a615d6c2764852a2e88a4f16e9ce1ea49bb776b5872956309e170d63a042a34f" dependencies = [ "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] @@ -4857,12 +4877,14 @@ name = "visions" version = "0.1.0" dependencies = [ "authdb", + "cool_asserts", "futures", "http 1.1.0", "mime 0.3.17", "mime_guess 2.0.5", "serde 1.0.210", "serde_json", + "thiserror 2.0.3", "tokio", "tokio-stream", "typeshare", @@ -4958,7 +4980,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", "wasm-bindgen-shared", ] @@ -4992,7 +5014,7 @@ checksum = "afc340c74d9005395cf9dd098506f7f44e38f2b4a21c6aaacf9a105ea5e1e836" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -5263,7 +5285,7 @@ checksum = "fa4f8080344d4671fb4e831a13ad1e68092748387dfc4f55e356242fae12ce3e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.79", + "syn 2.0.87", ] [[package]] diff --git a/visions/server/Cargo.toml b/visions/server/Cargo.toml index 4c2c69f..51dfaf0 100644 --- a/visions/server/Cargo.toml +++ b/visions/server/Cargo.toml @@ -19,3 +19,5 @@ futures = "0.3.31" tokio-stream = "0.1.16" typeshare = "1.0.4" urlencoding = "2.1.3" +cool_asserts = "2.0.3" +thiserror = "2.0.3" diff --git a/visions/server/src/asset_db.rs b/visions/server/src/asset_db.rs new file mode 100644 index 0000000..c88a4a0 --- /dev/null +++ b/visions/server/src/asset_db.rs @@ -0,0 +1,90 @@ +use std::{ + collections::HashMap, + fmt::{self, Display}, +}; + +use thiserror::Error; + +#[derive(Debug, Error)] +enum Error { + #[error("Asset could not be found: {0}")] + AssetNotFound(AssetId), +} + +#[derive(Clone, Debug, Hash, Eq, PartialEq)] +pub struct AssetId(String); + +impl Display for AssetId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "AssetId({}", self.0) + } +} + +impl From<&str> for AssetId { + fn from(s: &str) -> Self { + AssetId(s.to_owned()) + } +} + +pub trait Assets { + fn assets(&self) -> Vec; + + fn get(&self, asset_id: AssetId) -> Result, Error>; +} + +pub struct FsAssets { + assets: HashMap, +} + +impl FsAssets { + pub fn new() -> Self { + Self { + assets: HashMap::new(), + } + } + + fn assets<'a>(&'a self) -> impl Iterator { + self.assets.keys() + } +} + +impl Assets for FsAssets { + fn assets(&self) -> Vec { + self.assets.keys().cloned().collect() + } + + fn get(&self, asset_id: AssetId) -> Result, Error> { + unimplemented!() + } +} + +#[cfg(test)] +pub mod mocks { + use std::collections::HashMap; + + use super::*; + + pub struct MemoryAssets { + assets: HashMap, + } + + impl MemoryAssets { + pub fn new(data: Vec<(AssetId, String)>) -> Self { + let mut m = HashMap::new(); + data.into_iter().for_each(|(asset, path)| { + m.insert(asset, path); + }); + Self { assets: m } + } + } + + impl Assets for MemoryAssets { + fn assets(&self) -> Vec { + self.assets.keys().cloned().collect() + } + + fn get(&self, asset_id: AssetId) -> Result, Error> { + unimplemented!() + } + } +} diff --git a/visions/server/src/core.rs b/visions/server/src/core.rs index a399112..95f93ef 100644 --- a/visions/server/src/core.rs +++ b/visions/server/src/core.rs @@ -9,32 +9,42 @@ use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender}; use urlencoding::decode; use uuid::Uuid; -use crate::types::{AppError, Message, Tabletop, RGB}; +use crate::{ + asset_db::{AssetId, Assets}, + types::{AppError, Message, Tabletop, RGB}, +}; #[derive(Debug)] struct WebsocketClient { sender: Option>, } -#[derive(Debug)] pub struct AppState { - pub image_base: PathBuf, - pub tabletop: Tabletop, + pub assets: Box, pub clients: HashMap, + + pub tabletop: Tabletop, } -#[derive(Clone, Debug)] -pub struct Core(pub Arc>); +#[derive(Clone)] +pub struct Core(Arc>); impl Core { - pub fn new() -> Self { + pub fn new(assetdb: A) -> Self + where + A: Assets + Sync + Send + 'static, + { Self(Arc::new(RwLock::new(AppState { - image_base: PathBuf::from("/home/savanni/Pictures"), + assets: Box::new(assetdb), + clients: HashMap::new(), tabletop: Tabletop { - background_color: RGB{ red: 0xca, green: 0xb9, blue: 0xbb }, + background_color: RGB { + red: 0xca, + green: 0xb9, + blue: 0xbb, + }, background_image: None, }, - clients: HashMap::new(), }))) } @@ -68,21 +78,29 @@ impl Core { } } - pub fn get_file(&self, file_name: String) -> Vec { + pub fn tabletop(&self) -> Tabletop { + self.0.read().unwrap().tabletop.clone() + } + + pub async fn get_file(&self, file_name: String) -> Vec { + /* let file_name = decode(&file_name).expect("UTF-8"); let mut full_path = self.0.read().unwrap().image_base.clone(); full_path.push(file_name.to_string()); - let mut content: Vec = Vec::new(); let mut file = std::fs::File::open(&full_path).unwrap(); file.read_to_end(&mut content).unwrap(); content + */ + unimplemented!() } - pub fn available_images(&self) -> Vec { - std::fs::read_dir(&self.0.read().unwrap().image_base) + pub fn available_images(&self) -> Vec { + self.0.read().unwrap().assets.assets() + /* + Ok(std::fs::read_dir(&self.0.read().unwrap().image_base) .unwrap() .filter_map(|entry| match entry { Ok(entry_) => match mime_guess::from_path(entry_.path()).first() { @@ -98,7 +116,8 @@ impl Core { }, Err(_) => None, }) - .collect() + .collect()) + */ } pub fn set_background_image(&self, path: String) -> Result<(), AppError> { @@ -121,3 +140,24 @@ impl Core { }); } } + +#[cfg(test)] +mod test { + use super::*; + use crate::asset_db::mocks::MemoryAssets; + + #[tokio::test] + async fn it_lists_available_images() { + let assets = MemoryAssets::new(vec![ + (AssetId::from("asset_1"), "asset_1".to_owned()), + (AssetId::from("asset_2"), "asset_2".to_owned()), + (AssetId::from("asset_3"), "asset_3".to_owned()), + (AssetId::from("asset_4"), "asset_4".to_owned()), + (AssetId::from("asset_5"), "asset_5".to_owned()), + ]); + let core = Core::new(assets); + + let image_paths = core.available_images(); + assert_eq!(image_paths.len(), 5); + } +} diff --git a/visions/server/src/handlers.rs b/visions/server/src/handlers.rs index 90ab6cf..de3ed83 100644 --- a/visions/server/src/handlers.rs +++ b/visions/server/src/handlers.rs @@ -33,7 +33,7 @@ pub async fn handle_auth( pub async fn handle_file(core: Core, file_name: String) -> impl Reply { let mimetype = mime_guess::from_path(&file_name).first().unwrap(); - let bytes = core.get_file(file_name); + let bytes = core.get_file(file_name).await; Response::builder() .header("application-type", mimetype.to_string()) .body(bytes) @@ -41,10 +41,13 @@ pub async fn handle_file(core: Core, file_name: String) -> impl Reply { } pub async fn handle_available_images(core: Core) -> impl Reply { + let image_paths: Vec = core.available_images().into_iter() + .map(|path| format!("{}", path)).collect(); + Response::builder() .header("Access-Control-Allow-Origin", "*") .header("Content-Type", "application/json") - .body(serde_json::to_string(&core.available_images()).unwrap()) + .body(serde_json::to_string(&image_paths).unwrap()) .unwrap() } @@ -82,7 +85,6 @@ pub async fn handle_connect_websocket( ws: warp::ws::Ws, client_id: String, ) -> impl Reply { - // println!("handle_connect_websocket: {}", client_id); ws.on_upgrade(move |socket| { let core = core.clone(); async move { @@ -90,7 +92,7 @@ pub async fn handle_connect_websocket( let mut receiver = core.connect_client(client_id.clone()); tokio::task::spawn(async move { - let tabletop = core.0.read().unwrap().tabletop.clone(); + let tabletop = core.tabletop(); let _ = ws_sender .send(Message::text( serde_json::to_string(&crate::types::Message::UpdateTabletop(tabletop)) diff --git a/visions/server/src/main.rs b/visions/server/src/main.rs index fc1e8b0..94644f0 100644 --- a/visions/server/src/main.rs +++ b/visions/server/src/main.rs @@ -1,3 +1,4 @@ +use asset_db::FsAssets; use authdb::AuthError; use handlers::{ handle_available_images, handle_connect_websocket, handle_file, handle_register_client, handle_set_background_image, handle_unregister_client, RegisterRequest @@ -13,10 +14,9 @@ use warp::{ Filter, }; +mod asset_db; mod core; - mod handlers; -// use handlers::handle_auth; mod types; @@ -96,7 +96,7 @@ async fn handle_rejection(err: warp::Rejection) -> Result Date: Thu, 21 Nov 2024 09:08:36 -0500 Subject: [PATCH 2/6] available_images now only lists image files from the asset database --- visions/server/src/asset_db.rs | 30 ++++++++++++++++++++------- visions/server/src/core.rs | 37 ++++++++++++++-------------------- visions/server/src/handlers.rs | 5 +---- 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/visions/server/src/asset_db.rs b/visions/server/src/asset_db.rs index c88a4a0..00a783c 100644 --- a/visions/server/src/asset_db.rs +++ b/visions/server/src/asset_db.rs @@ -1,5 +1,5 @@ use std::{ - collections::HashMap, + collections::{hash_map::Iter, HashMap}, fmt::{self, Display}, }; @@ -16,7 +16,7 @@ pub struct AssetId(String); impl Display for AssetId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "AssetId({}", self.0) + write!(f, "AssetId({})", self.0) } } @@ -26,8 +26,24 @@ impl From<&str> for AssetId { } } +impl From for AssetId { + fn from(s: String) -> Self { + AssetId(s) + } +} + +pub struct AssetIter<'a>(Iter<'a, AssetId, String>); + +impl <'a> Iterator for AssetIter<'a> { + type Item = (&'a AssetId, &'a String); + + fn next(&mut self) -> Option { + self.0.next() + } +} + pub trait Assets { - fn assets(&self) -> Vec; + fn assets<'a>(&'a self) -> AssetIter<'a>; fn get(&self, asset_id: AssetId) -> Result, Error>; } @@ -49,8 +65,8 @@ impl FsAssets { } impl Assets for FsAssets { - fn assets(&self) -> Vec { - self.assets.keys().cloned().collect() + fn assets<'a>(&'a self) -> AssetIter<'a> { + AssetIter(self.assets.iter()) } fn get(&self, asset_id: AssetId) -> Result, Error> { @@ -79,8 +95,8 @@ pub mod mocks { } impl Assets for MemoryAssets { - fn assets(&self) -> Vec { - self.assets.keys().cloned().collect() + fn assets<'a>(&'a self) -> AssetIter<'a> { + AssetIter(self.assets.iter()) } fn get(&self, asset_id: AssetId) -> Result, Error> { diff --git a/visions/server/src/core.rs b/visions/server/src/core.rs index 95f93ef..f1be756 100644 --- a/visions/server/src/core.rs +++ b/visions/server/src/core.rs @@ -20,7 +20,7 @@ struct WebsocketClient { } pub struct AppState { - pub assets: Box, + pub asset_db: Box, pub clients: HashMap, pub tabletop: Tabletop, @@ -35,7 +35,7 @@ impl Core { A: Assets + Sync + Send + 'static, { Self(Arc::new(RwLock::new(AppState { - assets: Box::new(assetdb), + asset_db: Box::new(assetdb), clients: HashMap::new(), tabletop: Tabletop { background_color: RGB { @@ -98,26 +98,19 @@ impl Core { } pub fn available_images(&self) -> Vec { - self.0.read().unwrap().assets.assets() - /* - Ok(std::fs::read_dir(&self.0.read().unwrap().image_base) + self.0 + .read() .unwrap() - .filter_map(|entry| match entry { - Ok(entry_) => match mime_guess::from_path(entry_.path()).first() { - Some(mime) if mime.type_() == mime::IMAGE => Some( - entry_ - .path() - .file_name() - .and_then(|filename| filename.to_str()) - .and_then(|filename| Some(filename.to_owned())) - .unwrap(), - ), + .asset_db + .assets() + .filter_map(|(asset_id, value)| { + println!("[{:?}] {}", mime_guess::from_path(&value).first(), value); + match mime_guess::from_path(&value).first() { + Some(mime) if mime.type_() == mime::IMAGE => Some(asset_id.clone()), _ => None, - }, - Err(_) => None, + } }) - .collect()) - */ + .collect() } pub fn set_background_image(&self, path: String) -> Result<(), AppError> { @@ -149,8 +142,8 @@ mod test { #[tokio::test] async fn it_lists_available_images() { let assets = MemoryAssets::new(vec![ - (AssetId::from("asset_1"), "asset_1".to_owned()), - (AssetId::from("asset_2"), "asset_2".to_owned()), + (AssetId::from("asset_1"), "asset_1.png".to_owned()), + (AssetId::from("asset_2"), "asset_2.jpg".to_owned()), (AssetId::from("asset_3"), "asset_3".to_owned()), (AssetId::from("asset_4"), "asset_4".to_owned()), (AssetId::from("asset_5"), "asset_5".to_owned()), @@ -158,6 +151,6 @@ mod test { let core = Core::new(assets); let image_paths = core.available_images(); - assert_eq!(image_paths.len(), 5); + assert_eq!(image_paths.len(), 2); } } diff --git a/visions/server/src/handlers.rs b/visions/server/src/handlers.rs index de3ed83..0796768 100644 --- a/visions/server/src/handlers.rs +++ b/visions/server/src/handlers.rs @@ -1,8 +1,5 @@ -use std::{pin::Pin, time::Duration}; - use futures::{SinkExt, StreamExt}; use serde::{Deserialize, Serialize}; -use tokio_stream::wrappers::UnboundedReceiverStream; use warp::{http::Response, reply::Reply, ws::Message}; use crate::core::Core; @@ -59,7 +56,7 @@ pub struct RegisterResponse { url: String, } -pub async fn handle_register_client(core: Core, request: RegisterRequest) -> impl Reply { +pub async fn handle_register_client(core: Core, _request: RegisterRequest) -> impl Reply { let client_id = core.register_client(); Response::builder() -- 2.44.1 From 0f42ebcc30f5480d3ba4d10f83b0ac44fa17fc8e Mon Sep 17 00:00:00 2001 From: Savanni D'Gerinel Date: Thu, 21 Nov 2024 18:46:05 -0500 Subject: [PATCH 3/6] Isolate error handling from Warp --- visions/server/src/asset_db.rs | 33 ++++++++-- visions/server/src/core.rs | 23 +++---- visions/server/src/handlers.rs | 111 ++++++++++++++++++++++----------- visions/server/src/main.rs | 4 +- visions/server/src/types.rs | 3 + 5 files changed, 118 insertions(+), 56 deletions(-) diff --git a/visions/server/src/asset_db.rs b/visions/server/src/asset_db.rs index 00a783c..6861dfa 100644 --- a/visions/server/src/asset_db.rs +++ b/visions/server/src/asset_db.rs @@ -1,14 +1,32 @@ use std::{ collections::{hash_map::Iter, HashMap}, fmt::{self, Display}, + io::Read, }; use thiserror::Error; #[derive(Debug, Error)] -enum Error { - #[error("Asset could not be found: {0}")] - AssetNotFound(AssetId), +pub enum Error { + #[error("Asset could not be found")] + NotFound, + #[error("Asset could not be opened")] + Inaccessible, + + #[error("An unexpected IO error occured when retrieving an asset {0}")] + UnexpectedError(std::io::Error), +} + +impl From for Error { + fn from(err: std::io::Error) -> Error { + use std::io::ErrorKind::*; + + match err.kind() { + NotFound => Error::NotFound, + PermissionDenied | UnexpectedEof => Error::Inaccessible, + _ => Error::UnexpectedError(err), + } + } } #[derive(Clone, Debug, Hash, Eq, PartialEq)] @@ -70,7 +88,14 @@ impl Assets for FsAssets { } fn get(&self, asset_id: AssetId) -> Result, Error> { - unimplemented!() + let path = match self.assets.get(&asset_id) { + Some(asset) => Ok(asset), + None => Err(Error::NotFound), + }?; + let mut content: Vec = Vec::new(); + let mut file = std::fs::File::open(&path)?; + file.read_to_end(&mut content)?; + Ok(content) } } diff --git a/visions/server/src/core.rs b/visions/server/src/core.rs index f1be756..4299603 100644 --- a/visions/server/src/core.rs +++ b/visions/server/src/core.rs @@ -10,7 +10,7 @@ use urlencoding::decode; use uuid::Uuid; use crate::{ - asset_db::{AssetId, Assets}, + asset_db::{self, AssetId, Assets}, types::{AppError, Message, Tabletop, RGB}, }; @@ -82,19 +82,14 @@ impl Core { self.0.read().unwrap().tabletop.clone() } - pub async fn get_file(&self, file_name: String) -> Vec { - /* - let file_name = decode(&file_name).expect("UTF-8"); - - let mut full_path = self.0.read().unwrap().image_base.clone(); - full_path.push(file_name.to_string()); - - let mut content: Vec = Vec::new(); - let mut file = std::fs::File::open(&full_path).unwrap(); - file.read_to_end(&mut content).unwrap(); - content - */ - unimplemented!() + pub async fn get_asset(&self, asset_id: AssetId) -> Result, AppError> { + self.0.read().unwrap().asset_db.get(asset_id.clone()).map_err(|err| { + match err { + asset_db::Error::NotFound => AppError::NotFound(format!("{}", asset_id)), + asset_db::Error::Inaccessible => AppError::Inaccessible(format!("{}", asset_id)), + asset_db::Error::UnexpectedError(err) => AppError::Inaccessible(format!("{}", err)), + } + }) } pub fn available_images(&self) -> Vec { diff --git a/visions/server/src/handlers.rs b/visions/server/src/handlers.rs index 0796768..e58fc6b 100644 --- a/visions/server/src/handlers.rs +++ b/visions/server/src/handlers.rs @@ -1,8 +1,10 @@ +use std::future::Future; + use futures::{SinkExt, StreamExt}; use serde::{Deserialize, Serialize}; -use warp::{http::Response, reply::Reply, ws::Message}; +use warp::{http::Response, http::StatusCode, reply::Reply, ws::Message}; -use crate::core::Core; +use crate::{asset_db::AssetId, core::Core, types::AppError}; /* pub async fn handle_auth( @@ -28,24 +30,52 @@ pub async fn handle_auth( } */ -pub async fn handle_file(core: Core, file_name: String) -> impl Reply { - let mimetype = mime_guess::from_path(&file_name).first().unwrap(); - let bytes = core.get_file(file_name).await; - Response::builder() - .header("application-type", mimetype.to_string()) - .body(bytes) - .unwrap() +pub async fn handler(f: F) -> impl Reply +where + F: Future>, AppError>>, +{ + match f.await { + Ok(response) => response, + Err(AppError::NotFound(_)) => Response::builder() + .status(StatusCode::NOT_FOUND) + .body(vec![]) + .unwrap(), + Err(_) => Response::builder() + .status(StatusCode::INTERNAL_SERVER_ERROR) + .body(vec![]) + .unwrap(), + } +} + +pub async fn handle_file(core: Core, asset_id: AssetId) -> impl Reply { + handler(async move { + let mimetype = mime_guess::from_path(&format!("{}", asset_id)) + .first() + .unwrap(); + let bytes = core.get_asset(asset_id).await?; + Ok(Response::builder() + .header("application-type", mimetype.to_string()) + .body(bytes) + .unwrap()) + }) + .await } pub async fn handle_available_images(core: Core) -> impl Reply { - let image_paths: Vec = core.available_images().into_iter() - .map(|path| format!("{}", path)).collect(); + handler(async move { + let image_paths: Vec = core + .available_images() + .into_iter() + .map(|path| format!("{}", path)) + .collect(); - Response::builder() - .header("Access-Control-Allow-Origin", "*") - .header("Content-Type", "application/json") - .body(serde_json::to_string(&image_paths).unwrap()) - .unwrap() + Ok(Response::builder() + .header("Access-Control-Allow-Origin", "*") + .header("Content-Type", "application/json") + .body(serde_json::to_vec(&image_paths).unwrap()) + .unwrap()) + }) + .await } #[derive(Deserialize, Serialize)] @@ -57,24 +87,30 @@ pub struct RegisterResponse { } pub async fn handle_register_client(core: Core, _request: RegisterRequest) -> impl Reply { - let client_id = core.register_client(); + handler(async move { + let client_id = core.register_client(); - Response::builder() - .header("Access-Control-Allow-Origin", "*") - .header("Content-Type", "application/json") - .body( - serde_json::to_string(&RegisterResponse { - url: format!("ws://127.0.0.1:8001/ws/{}", client_id), - }) - .unwrap(), - ) - .unwrap() + Ok(Response::builder() + .header("Access-Control-Allow-Origin", "*") + .header("Content-Type", "application/json") + .body( + serde_json::to_vec(&RegisterResponse { + url: format!("ws://127.0.0.1:8001/ws/{}", client_id), + }) + .unwrap(), + ) + .unwrap()) + }) + .await } pub async fn handle_unregister_client(core: Core, client_id: String) -> impl Reply { - core.unregister_client(client_id); + handler(async move { + core.unregister_client(client_id); - warp::reply::reply() + Ok(Response::builder().status(StatusCode::NO_CONTENT).body(vec![]).unwrap()) + }) + .await } pub async fn handle_connect_websocket( @@ -109,12 +145,15 @@ pub async fn handle_connect_websocket( } pub async fn handle_set_background_image(core: Core, image_name: String) -> impl Reply { - let _ = core.set_background_image(image_name); + handler(async move { + let _ = core.set_background_image(image_name); - Response::builder() - .header("Access-Control-Allow-Origin", "*") - .header("Access-Control-Allow-Methods", "*") - .header("Content-Type", "application/json") - .body("") - .unwrap() + Ok(Response::builder() + .header("Access-Control-Allow-Origin", "*") + .header("Access-Control-Allow-Methods", "*") + .header("Content-Type", "application/json") + .body(vec![]) + .unwrap()) + }) + .await } diff --git a/visions/server/src/main.rs b/visions/server/src/main.rs index 94644f0..36d5323 100644 --- a/visions/server/src/main.rs +++ b/visions/server/src/main.rs @@ -1,4 +1,4 @@ -use asset_db::FsAssets; +use asset_db::{AssetId, FsAssets}; use authdb::AuthError; use handlers::{ handle_available_images, handle_connect_websocket, handle_file, handle_register_client, handle_set_background_image, handle_unregister_client, RegisterRequest @@ -103,7 +103,7 @@ pub async fn main() { .and(warp::get()) .then({ let core = core.clone(); - move |file_name| handle_file(core.clone(), file_name) + move |file_name| handle_file(core.clone(), AssetId::from(file_name)) }); let route_available_images = warp::path!("api" / "v1" / "image").and(warp::get()).then({ diff --git a/visions/server/src/types.rs b/visions/server/src/types.rs index 45c518f..f47a25b 100644 --- a/visions/server/src/types.rs +++ b/visions/server/src/types.rs @@ -1,10 +1,13 @@ use serde::{Deserialize, Serialize}; + use typeshare::typeshare; #[derive(Debug)] pub enum AppError { NotFound(String), + Inaccessible(String), JsonError(serde_json::Error), + UnexpectedError(String), } #[derive(Clone, Debug, Deserialize, Serialize)] -- 2.44.1 From 71b114c9b209ae1f10a49b78edf9ea588410483f Mon Sep 17 00:00:00 2001 From: Savanni D'Gerinel Date: Sun, 24 Nov 2024 09:21:58 -0500 Subject: [PATCH 4/6] Set up some asset retrieval tests. --- visions/server/Cargo.toml | 4 +- visions/server/src/asset_db.rs | 40 +++++++++++++------- visions/server/src/core.rs | 67 +++++++++++++++++++++++++++------- visions/server/src/handlers.rs | 7 +--- 4 files changed, 85 insertions(+), 33 deletions(-) diff --git a/visions/server/Cargo.toml b/visions/server/Cargo.toml index 51dfaf0..0a185f8 100644 --- a/visions/server/Cargo.toml +++ b/visions/server/Cargo.toml @@ -19,5 +19,7 @@ futures = "0.3.31" tokio-stream = "0.1.16" typeshare = "1.0.4" urlencoding = "2.1.3" -cool_asserts = "2.0.3" thiserror = "2.0.3" + +[dev-dependencies] +cool_asserts = "2.0.3" diff --git a/visions/server/src/asset_db.rs b/visions/server/src/asset_db.rs index 6861dfa..23a7a4d 100644 --- a/visions/server/src/asset_db.rs +++ b/visions/server/src/asset_db.rs @@ -4,6 +4,7 @@ use std::{ io::Read, }; +use mime::Mime; use thiserror::Error; #[derive(Debug, Error)] @@ -52,7 +53,7 @@ impl From for AssetId { pub struct AssetIter<'a>(Iter<'a, AssetId, String>); -impl <'a> Iterator for AssetIter<'a> { +impl<'a> Iterator for AssetIter<'a> { type Item = (&'a AssetId, &'a String); fn next(&mut self) -> Option { @@ -63,7 +64,7 @@ impl <'a> Iterator for AssetIter<'a> { pub trait Assets { fn assets<'a>(&'a self) -> AssetIter<'a>; - fn get(&self, asset_id: AssetId) -> Result, Error>; + fn get(&self, asset_id: AssetId) -> Result<(Mime, Vec), Error>; } pub struct FsAssets { @@ -87,15 +88,16 @@ impl Assets for FsAssets { AssetIter(self.assets.iter()) } - fn get(&self, asset_id: AssetId) -> Result, Error> { + fn get(&self, asset_id: AssetId) -> Result<(Mime, Vec), Error> { let path = match self.assets.get(&asset_id) { Some(asset) => Ok(asset), None => Err(Error::NotFound), }?; + let mime = mime_guess::from_path(&path).first().unwrap(); let mut content: Vec = Vec::new(); let mut file = std::fs::File::open(&path)?; file.read_to_end(&mut content)?; - Ok(content) + Ok((mime, content)) } } @@ -106,26 +108,38 @@ pub mod mocks { use super::*; pub struct MemoryAssets { - assets: HashMap, + asset_paths: HashMap, + assets: HashMap>, } impl MemoryAssets { - pub fn new(data: Vec<(AssetId, String)>) -> Self { - let mut m = HashMap::new(); - data.into_iter().for_each(|(asset, path)| { - m.insert(asset, path); + pub fn new(data: Vec<(AssetId, String, Vec)>) -> Self { + let mut asset_paths = HashMap::new(); + let mut assets = HashMap::new(); + data.into_iter().for_each(|(asset, path, data)| { + asset_paths.insert(asset.clone(), path); + assets.insert(asset, data); }); - Self { assets: m } + Self { + asset_paths, + assets, + } } } impl Assets for MemoryAssets { fn assets<'a>(&'a self) -> AssetIter<'a> { - AssetIter(self.assets.iter()) + AssetIter(self.asset_paths.iter()) } - fn get(&self, asset_id: AssetId) -> Result, Error> { - unimplemented!() + fn get(&self, asset_id: AssetId) -> Result<(Mime, Vec), Error> { + match (self.asset_paths.get(&asset_id), self.assets.get(&asset_id)) { + (Some(path), Some(data)) => { + let mime = mime_guess::from_path(&path).first().unwrap(); + Ok((mime, data.to_vec())) + } + _ => Err(Error::NotFound), + } } } } diff --git a/visions/server/src/core.rs b/visions/server/src/core.rs index 4299603..aac0c15 100644 --- a/visions/server/src/core.rs +++ b/visions/server/src/core.rs @@ -5,6 +5,7 @@ use std::{ sync::{Arc, RwLock}, }; +use mime::Mime; use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender}; use urlencoding::decode; use uuid::Uuid; @@ -82,14 +83,17 @@ impl Core { self.0.read().unwrap().tabletop.clone() } - pub async fn get_asset(&self, asset_id: AssetId) -> Result, AppError> { - self.0.read().unwrap().asset_db.get(asset_id.clone()).map_err(|err| { - match err { + pub async fn get_asset(&self, asset_id: AssetId) -> Result<(Mime, Vec), AppError> { + self.0 + .read() + .unwrap() + .asset_db + .get(asset_id.clone()) + .map_err(|err| match err { asset_db::Error::NotFound => AppError::NotFound(format!("{}", asset_id)), asset_db::Error::Inaccessible => AppError::Inaccessible(format!("{}", asset_id)), asset_db::Error::UnexpectedError(err) => AppError::Inaccessible(format!("{}", err)), - } - }) + }) } pub fn available_images(&self) -> Vec { @@ -132,20 +136,55 @@ impl Core { #[cfg(test)] mod test { use super::*; + + use cool_asserts::assert_matches; + use crate::asset_db::mocks::MemoryAssets; + fn test_core() -> Core { + let assets = MemoryAssets::new(vec![ + ( + AssetId::from("asset_1"), + "asset_1.png".to_owned(), + String::from("abcdefg").into_bytes(), + ), + ( + AssetId::from("asset_2"), + "asset_2.jpg".to_owned(), + String::from("abcdefg").into_bytes(), + ), + ( + AssetId::from("asset_3"), + "asset_3".to_owned(), + String::from("abcdefg").into_bytes(), + ), + ( + AssetId::from("asset_4"), + "asset_4".to_owned(), + String::from("abcdefg").into_bytes(), + ), + ( + AssetId::from("asset_5"), + "asset_5".to_owned(), + String::from("abcdefg").into_bytes(), + ), + ]); + Core::new(assets) + } + #[tokio::test] async fn it_lists_available_images() { - let assets = MemoryAssets::new(vec![ - (AssetId::from("asset_1"), "asset_1.png".to_owned()), - (AssetId::from("asset_2"), "asset_2.jpg".to_owned()), - (AssetId::from("asset_3"), "asset_3".to_owned()), - (AssetId::from("asset_4"), "asset_4".to_owned()), - (AssetId::from("asset_5"), "asset_5".to_owned()), - ]); - let core = Core::new(assets); - + let core = test_core(); let image_paths = core.available_images(); assert_eq!(image_paths.len(), 2); } + + #[tokio::test] + async fn it_retrieves_an_asset() { + let core = test_core(); + assert_matches!(core.get_asset(AssetId::from("asset_1")).await, Ok((mime, data)) => { + assert_eq!(mime.type_(), mime::IMAGE); + assert_eq!(data, "abcdefg".as_bytes()); + }); + } } diff --git a/visions/server/src/handlers.rs b/visions/server/src/handlers.rs index e58fc6b..bf94976 100644 --- a/visions/server/src/handlers.rs +++ b/visions/server/src/handlers.rs @@ -49,12 +49,9 @@ where pub async fn handle_file(core: Core, asset_id: AssetId) -> impl Reply { handler(async move { - let mimetype = mime_guess::from_path(&format!("{}", asset_id)) - .first() - .unwrap(); - let bytes = core.get_asset(asset_id).await?; + let (mime, bytes) = core.get_asset(asset_id).await?; Ok(Response::builder() - .header("application-type", mimetype.to_string()) + .header("application-type", mime.to_string()) .body(bytes) .unwrap()) }) -- 2.44.1 From cadb3ab435b7c30f3f5347f3b7dd652d9d93548f Mon Sep 17 00:00:00 2001 From: Savanni D'Gerinel Date: Sun, 24 Nov 2024 09:35:25 -0500 Subject: [PATCH 5/6] Verify that the tabletop can be set and retrieved --- visions/server/src/asset_db.rs | 3 ++- visions/server/src/core.rs | 35 +++++++++++++++++++++++++++------- visions/server/src/handlers.rs | 2 +- visions/server/src/types.rs | 6 ++++-- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/visions/server/src/asset_db.rs b/visions/server/src/asset_db.rs index 23a7a4d..d32fa82 100644 --- a/visions/server/src/asset_db.rs +++ b/visions/server/src/asset_db.rs @@ -5,6 +5,7 @@ use std::{ }; use mime::Mime; +use serde::{Deserialize, Serialize}; use thiserror::Error; #[derive(Debug, Error)] @@ -30,7 +31,7 @@ impl From for Error { } } -#[derive(Clone, Debug, Hash, Eq, PartialEq)] +#[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] pub struct AssetId(String); impl Display for AssetId { diff --git a/visions/server/src/core.rs b/visions/server/src/core.rs index aac0c15..d5bcaf0 100644 --- a/visions/server/src/core.rs +++ b/visions/server/src/core.rs @@ -15,6 +15,12 @@ use crate::{ types::{AppError, Message, Tabletop, RGB}, }; +const DEFAULT_BACKGROUND_COLOR: RGB = RGB { + red: 0xca, + green: 0xb9, + blue: 0xbb, +}; + #[derive(Debug)] struct WebsocketClient { sender: Option>, @@ -39,11 +45,7 @@ impl Core { asset_db: Box::new(assetdb), clients: HashMap::new(), tabletop: Tabletop { - background_color: RGB { - red: 0xca, - green: 0xb9, - blue: 0xbb, - }, + background_color: DEFAULT_BACKGROUND_COLOR, background_image: None, }, }))) @@ -112,10 +114,10 @@ impl Core { .collect() } - pub fn set_background_image(&self, path: String) -> Result<(), AppError> { + pub fn set_background_image(&self, asset: AssetId) -> Result<(), AppError> { let tabletop = { let mut state = self.0.write().unwrap(); - state.tabletop.background_image = Some(path.clone()); + state.tabletop.background_image = Some(asset.clone()); state.tabletop.clone() }; self.publish(Message::UpdateTabletop(tabletop)); @@ -187,4 +189,23 @@ mod test { assert_eq!(data, "abcdefg".as_bytes()); }); } + + #[tokio::test] + async fn it_can_retrieve_the_default_tabletop() { + let core = test_core(); + assert_matches!(core.tabletop(), Tabletop{ background_color, background_image } => { + assert_eq!(background_color, DEFAULT_BACKGROUND_COLOR); + assert_eq!(background_image, None); + }); + } + + #[tokio::test] + async fn it_can_change_the_tabletop_background() { + let core = test_core(); + assert_matches!(core.set_background_image(AssetId::from("asset_1")), Ok(())); + assert_matches!(core.tabletop(), Tabletop{ background_color, background_image } => { + assert_eq!(background_color, DEFAULT_BACKGROUND_COLOR); + assert_eq!(background_image, Some(AssetId::from("asset_1"))); + }); + } } diff --git a/visions/server/src/handlers.rs b/visions/server/src/handlers.rs index bf94976..ae31f61 100644 --- a/visions/server/src/handlers.rs +++ b/visions/server/src/handlers.rs @@ -143,7 +143,7 @@ pub async fn handle_connect_websocket( pub async fn handle_set_background_image(core: Core, image_name: String) -> impl Reply { handler(async move { - let _ = core.set_background_image(image_name); + let _ = core.set_background_image(AssetId::from(image_name)); Ok(Response::builder() .header("Access-Control-Allow-Origin", "*") diff --git a/visions/server/src/types.rs b/visions/server/src/types.rs index f47a25b..f5c5307 100644 --- a/visions/server/src/types.rs +++ b/visions/server/src/types.rs @@ -2,6 +2,8 @@ use serde::{Deserialize, Serialize}; use typeshare::typeshare; +use crate::asset_db::AssetId; + #[derive(Debug)] pub enum AppError { NotFound(String), @@ -10,7 +12,7 @@ pub enum AppError { UnexpectedError(String), } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] #[typeshare] pub struct RGB { @@ -24,7 +26,7 @@ pub struct RGB { #[typeshare] pub struct Tabletop { pub background_color: RGB, - pub background_image: Option, + pub background_image: Option, } #[derive(Clone, Debug, Deserialize, Serialize)] -- 2.44.1 From c79610bd79292fc793eb0b850a57171a4e408aae Mon Sep 17 00:00:00 2001 From: Savanni D'Gerinel Date: Sun, 24 Nov 2024 09:50:20 -0500 Subject: [PATCH 6/6] Add a test for update notifications --- visions/server/src/core.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/visions/server/src/core.rs b/visions/server/src/core.rs index d5bcaf0..d157a65 100644 --- a/visions/server/src/core.rs +++ b/visions/server/src/core.rs @@ -208,4 +208,23 @@ mod test { assert_eq!(background_image, Some(AssetId::from("asset_1"))); }); } + + #[tokio::test] + async fn it_sends_notices_to_clients_on_tabletop_change() { + let core = test_core(); + let client_id = core.register_client(); + let mut receiver = core.connect_client(client_id); + + assert_matches!(core.set_background_image(AssetId::from("asset_1")), Ok(())); + match receiver.recv().await { + Some(Message::UpdateTabletop(Tabletop { + background_color, + background_image, + })) => { + assert_eq!(background_color, DEFAULT_BACKGROUND_COLOR); + assert_eq!(background_image, Some(AssetId::from("asset_1"))); + } + None => panic!("receiver did not get a message"), + } + } } -- 2.44.1