From 8d99e7284c5231b43591b38cf6699c5ebc39acf2 Mon Sep 17 00:00:00 2001 From: Andy O'Neill Date: Tue, 16 Apr 2024 00:29:00 -0400 Subject: [PATCH] get queue API working, add first tests - Make the queue API return the current queue - Add JSON errors for e.g. 404 - Add once_cell and refactor build_test_app to fix Sled locking problems - Write first queue API tests, needs some more. --- rs/Cargo.lock | 1 + rs/Cargo.toml | 1 + rs/src/api/list.rs | 3 +- rs/src/api/queue.rs | 50 +++++++++++------------- rs/src/error.rs | 31 +++++++++++++++ rs/src/filemanager/collection.rs | 19 +++------ rs/src/filemanager/collection_builder.rs | 6 +-- rs/src/filemanager/list.rs | 2 +- rs/src/filemanager/model.rs | 1 - rs/src/lib.rs | 1 + rs/tests/helpers.rs | 12 ++++-- rs/tests/queue_e2e.rs | 49 +++++++++++++++++++++++ 12 files changed, 123 insertions(+), 53 deletions(-) create mode 100644 rs/src/error.rs create mode 100644 rs/tests/queue_e2e.rs diff --git a/rs/Cargo.lock b/rs/Cargo.lock index fcf5015..1fd517e 100644 --- a/rs/Cargo.lock +++ b/rs/Cargo.lock @@ -1322,6 +1322,7 @@ dependencies = [ "factori", "lazy_static", "log", + "once_cell", "rand", "regex", "rodio", diff --git a/rs/Cargo.toml b/rs/Cargo.toml index 0e7345b..43fbee3 100644 --- a/rs/Cargo.toml +++ b/rs/Cargo.toml @@ -20,6 +20,7 @@ walkdir = "^2" rand = "^0" regex = "^1" lazy_static = "^1" +once_cell = "1.19.0" [dev-dependencies] assert_matches = "^1" diff --git a/rs/src/api/list.rs b/rs/src/api/list.rs index 914b050..8262790 100644 --- a/rs/src/api/list.rs +++ b/rs/src/api/list.rs @@ -4,6 +4,7 @@ use serde::Serialize; use crate::app_state::AppState; +// TODO fill this out #[derive(Debug, Serialize)] struct ApiCategory { name: String, @@ -20,7 +21,7 @@ pub struct ApiTrack { } impl ApiTrack { - fn from_track(track: &crate::filemanager::model::Track) -> Self { + pub fn from_track(track: &crate::filemanager::model::Track) -> Self { ApiTrack { id: track.id.clone(), title: track.name.clone(), diff --git a/rs/src/api/queue.rs b/rs/src/api/queue.rs index 5507c31..84492b3 100644 --- a/rs/src/api/queue.rs +++ b/rs/src/api/queue.rs @@ -1,28 +1,11 @@ -use actix_web::{get, post, web, HttpResponse, Responder}; +use actix_web::{ + get, http::StatusCode, post, web, Responder, Result, +}; use serde::{Deserialize, Serialize}; -use crate::{api::list::ApiTrack, app_state::AppState, filemanager::model::Track}; +use crate::error::AppError; use crate::filemanager::dto::CollectionLocation; - -fn get_first_track(app_state: &AppState) -> &Track { - // TODO for now let's just look for the first track, it seems to work on our demo music - return app_state - .collection - .values() - .next() - .unwrap() - .artists - .values() - .next() - .unwrap() - .albums - .values() - .next() - .unwrap() - .tracks - .first() - .unwrap(); -} +use crate::{api::list::ApiTrack, app_state::AppState}; // TODO extract 'ApiCollectionLocaltion out of here #[derive(Deserialize, Debug)] @@ -43,18 +26,24 @@ struct ApiQueueResponse { } #[post("/queue/add")] -pub async fn add(data: web::Data, input: web::Json) -> impl Responder { +pub async fn add( + data: web::Data, + input: web::Json, +) -> Result { let collection_location = CollectionLocation { category: input.category.clone(), artist: input.artist.clone(), album: input.album.clone(), - disc: input.track.clone(), + disc: input.disc.clone(), track: input.track.clone(), }; let maybe_tracks = data.collection.get_tracks_under(&collection_location); if maybe_tracks.is_none() { - return HttpResponse::NotFound().body("Not found"); + return Err(AppError::new( + "No matching music found in the collection", + StatusCode::NOT_FOUND, + )); } let tracks = maybe_tracks.unwrap(); @@ -64,12 +53,17 @@ pub async fn add(data: web::Data, input: web::Json) -> queue.clear(); } queue.add_tracks(tracks.into_iter().cloned().collect()); - HttpResponse::Ok().body("ok") + Ok(web::Json(ApiQueueResponse { + tracks: queue.tracks.iter().map(ApiTrack::from_track).collect(), + position: queue.position, + })) } #[get("/queue")] pub async fn get_queue(data: web::Data) -> impl Responder { let queue = data.queue.lock().unwrap(); - queue.print_tracks(); - HttpResponse::Ok().body("ok") + web::Json(ApiQueueResponse { + tracks: queue.tracks.iter().map(ApiTrack::from_track).collect(), + position: queue.position, + }) } diff --git a/rs/src/error.rs b/rs/src/error.rs new file mode 100644 index 0000000..b3dbfc6 --- /dev/null +++ b/rs/src/error.rs @@ -0,0 +1,31 @@ +use actix_web::{http::StatusCode, HttpResponse, ResponseError}; +use std::fmt; + +#[derive(Debug)] +pub struct AppError { + pub msg: String, + pub status: StatusCode, +} + +impl AppError { + pub fn new(msg: &str, status: StatusCode) -> AppError { + AppError { + msg: msg.to_string(), + status, + } + } +} + +impl fmt::Display for AppError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:?}", self) + } +} + +impl ResponseError for AppError { + // builds the actual response to send back when an error occurs + fn error_response(&self) -> HttpResponse { + let err_json = serde_json::json!({ "error": self.msg }); + HttpResponse::build(self.status).json(err_json) + } +} diff --git a/rs/src/filemanager/collection.rs b/rs/src/filemanager/collection.rs index cca2e39..534d366 100644 --- a/rs/src/filemanager/collection.rs +++ b/rs/src/filemanager/collection.rs @@ -1,22 +1,13 @@ -use std::{ - collections::{BTreeMap}, -}; - - - +use std::collections::BTreeMap; use super::{ cache, - collection_builder::{build}, + collection_builder::build, dto::CollectionLocation, model::{Category, Track}, options::CollectionOptions, }; -const DEFAULT_CATEGORY: &str = "Music"; -const CATEGORY_PREFIX: &str = "_"; -const CD_REGEX_STR: &str = r"(?i)(cd|dis(c|k)) ?\d"; - pub struct Collection { pub categories: BTreeMap, } @@ -107,7 +98,8 @@ impl Collection { return maybe_tracks.and_then(|tracks| { tracks .iter() - .find(|track| track.name == *location.track.as_ref().unwrap()).map(|track| vec![track.clone()]) + .find(|track| track.name == *location.track.as_ref().unwrap()) + .map(|track| vec![*track]) }); } } @@ -116,9 +108,8 @@ impl Collection { mod tests { use super::*; use crate::filemanager::model::factories::*; - + use factori::create; - #[test] fn test_add_category() { diff --git a/rs/src/filemanager/collection_builder.rs b/rs/src/filemanager/collection_builder.rs index d0c4c64..7b8fd74 100644 --- a/rs/src/filemanager/collection_builder.rs +++ b/rs/src/filemanager/collection_builder.rs @@ -1,8 +1,4 @@ -use std::{ - borrow::Cow, - collections::{VecDeque}, - path::PathBuf, -}; +use std::{borrow::Cow, collections::VecDeque, path::PathBuf}; use lazy_static::lazy_static; use regex::Regex; diff --git a/rs/src/filemanager/list.rs b/rs/src/filemanager/list.rs index 7def749..0af301e 100644 --- a/rs/src/filemanager/list.rs +++ b/rs/src/filemanager/list.rs @@ -41,7 +41,7 @@ fn list_album(album: &Album, indent: usize) { let space = " ".repeat(indent); log::info!("{space}[al]{}", album.name); - for (_disc_name, disc) in &album.discs { + for disc in album.discs.values() { list_album(disc, indent + 2); } diff --git a/rs/src/filemanager/model.rs b/rs/src/filemanager/model.rs index 2dce8b0..bf4533c 100644 --- a/rs/src/filemanager/model.rs +++ b/rs/src/filemanager/model.rs @@ -149,7 +149,6 @@ pub mod factories { }); pub fn random_string() -> String { - thread_rng() .sample_iter(&Alphanumeric) .take(30) diff --git a/rs/src/lib.rs b/rs/src/lib.rs index 66e52f7..d94a139 100644 --- a/rs/src/lib.rs +++ b/rs/src/lib.rs @@ -1,5 +1,6 @@ pub mod api; pub mod app_state; +pub mod error; pub mod filemanager; pub mod player; pub mod queue; diff --git a/rs/tests/helpers.rs b/rs/tests/helpers.rs index b596596..bf01419 100644 --- a/rs/tests/helpers.rs +++ b/rs/tests/helpers.rs @@ -4,7 +4,10 @@ use actix_web::{ web::Data, App, Error, }; -use pickup::{build_app, build_app_state, filemanager::options::CollectionOptions, ServeOptions}; +use pickup::{ + app_state::AppState, build_app, build_app_state, filemanager::options::CollectionOptions, + ServeOptions, +}; pub fn build_test_app() -> App< impl ServiceFactory< @@ -15,6 +18,10 @@ pub fn build_test_app() -> App< Error = Error, >, > { + build_app(build_test_app_state()) +} + +pub fn build_test_app_state() -> Data { let options = ServeOptions { collection_options: CollectionOptions { dir: String::from("../music"), @@ -23,6 +30,5 @@ pub fn build_test_app() -> App< port: 3001, }; - let app_state = Data::new(build_app_state(&options)); - build_app(app_state) + Data::new(build_app_state(&options)) } diff --git a/rs/tests/queue_e2e.rs b/rs/tests/queue_e2e.rs new file mode 100644 index 0000000..55a0cd0 --- /dev/null +++ b/rs/tests/queue_e2e.rs @@ -0,0 +1,49 @@ +use actix_web::{ + test::{self, read_body_json}, + web::Data, +}; + +mod helpers; + +use helpers::build_test_app_state; +use once_cell::sync::Lazy; +use pickup::{app_state::AppState, build_app}; +use serde_json::{self, json}; + +// Make a shared app state for use in all tests, because Sled won't let us open multiple instances of the DB file +// from differet threads. +static APP_STATE: Lazy> = Lazy::new(build_test_app_state); + +#[actix_web::test] +async fn test_not_found() { + let app = test::init_service(build_app((*APP_STATE).clone())).await; + let req = test::TestRequest::post() + .uri("/queue/add") + .set_json(json!({"category": "DoesNotExist"})) + .to_request(); + + let resp = test::call_service(&app, req).await; + + assert_eq!(resp.status(), actix_web::http::StatusCode::NOT_FOUND); + let resp_json: serde_json::Value = read_body_json(resp).await; + assert_eq!( + resp_json, + json!({ "error": "No matching music found in the collection" }) + ); +} + +#[actix_web::test] +async fn test_add_artist() { + let app = test::init_service(build_app((*APP_STATE).clone())).await; + let req = test::TestRequest::post() + .uri("/queue/add") + .set_json(json!({ + "category": "Music", + })) + .to_request(); + + let resp: serde_json::Value = test::call_and_read_body_json(&app, req).await; + + assert_eq!(resp.get("position").unwrap().as_u64().unwrap(), 0); + assert_eq!(resp.get("tracks").unwrap().as_array().unwrap().len(), 5); +}