From c7427d1b6cfd39f0427529f2d40db1df9610e619 Mon Sep 17 00:00:00 2001 From: Davide Polonio Date: Sun, 17 Jul 2022 17:07:59 +0200 Subject: [PATCH] feat: add tests, start using traits * Fix ContentKind bug --- Cargo.toml | 4 +- Dockerfile | 2 +- src/search/mod.rs | 47 +++++------- src/search/spotify/mod.rs | 26 +++++-- src/search/tests.rs | 147 ++++++++++++++++++++++++++++++++++++++ src/search/youtube/mod.rs | 31 +++++++- src/tgformatter/mod.rs | 8 +-- 7 files changed, 222 insertions(+), 43 deletions(-) create mode 100644 src/search/tests.rs diff --git a/Cargo.toml b/Cargo.toml index b5546c6..20c965f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "songlify" -version = "0.3.3-beta" +version = "0.3.4" edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html @@ -15,6 +15,8 @@ sentry = "0.27.0" invidious = "0.2.1" chrono = "0.4.19" itertools = "0.10.3" +async-trait = "0.1.56" [dev-dependencies] tokio-test = "0.4.2" +mockall = "0.11.1" diff --git a/Dockerfile b/Dockerfile index 01a5337..aa4c59a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM rust:1.59.0-slim-bullseye as builder +FROM rust:1.62.0-slim-bullseye as builder WORKDIR /build diff --git a/src/search/mod.rs b/src/search/mod.rs index 1d85b71..a631ab9 100644 --- a/src/search/mod.rs +++ b/src/search/mod.rs @@ -7,6 +7,9 @@ use youtube::Video; pub mod spotify; mod youtube; +#[cfg(test)] +mod tests; + pub(crate) trait ArtistComposed { fn get_artists_name(&self) -> HashSet; } @@ -48,14 +51,14 @@ impl ArtistComposed for TrackItem { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub(crate) struct AlbumItem { - pub(crate) spotify_track: Option, + pub(crate) spotify_album: Option, } impl ArtistComposed for AlbumItem { fn get_artists_name(&self) -> HashSet { - if self.spotify_track.is_some() { + if self.spotify_album.is_some() { return self - .spotify_track + .spotify_album .clone() .unwrap() .artists @@ -90,23 +93,25 @@ impl ArtistComposed for PlaylistItem { // This struct will allow us in the future to search, cache and store data and metadata regarding // tracks, albums and playlists -#[derive(Debug, Clone)] pub(crate) struct Engine { - spotify: spotify::Client, - youtube: youtube::Client, + spotify: Box, + youtube: Box, } impl Engine { pub(crate) async fn new() -> Self { Engine { - spotify: spotify::Client::new().await, - youtube: youtube::Client::new().await, + spotify: Box::new(spotify::Client::new().await), + youtube: Box::new(youtube::Client::new().await), } } + #[allow(dead_code)] + #[allow(unused_variables)] + #[cfg(test)] pub(crate) fn new_with_dependencies( - spotify_client: spotify::Client, - youtube_client: youtube::Client, + spotify_client: Box, + youtube_client: Box, ) -> Self { Engine { spotify: spotify_client, @@ -173,7 +178,7 @@ impl Engine { }; AlbumItem { - spotify_track: album_info, + spotify_album: album_info, } } @@ -197,23 +202,3 @@ impl Engine { pub(crate) fn get_spotify_kind(spotify_id: &str) -> Option { get_entry_kind(spotify_id) } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn should_search_track_by_spotify_id() { - todo!("Implement me!") - } - - #[test] - fn should_search_album_by_spotify_id() { - todo!("Implement me!") - } - - #[test] - fn should_search_playlist_by_spotify_id() { - todo!("Implement me!") - } -} diff --git a/src/search/spotify/mod.rs b/src/search/spotify/mod.rs index 5b6e3ca..36e86a3 100644 --- a/src/search/spotify/mod.rs +++ b/src/search/spotify/mod.rs @@ -1,3 +1,6 @@ +use async_trait::async_trait; +#[cfg(test)] +use mockall::{automock, mock, predicate::*}; use rspotify::model::PlayableItem::{Episode, Track}; use rspotify::model::{AlbumId, PlaylistId, TrackId}; use rspotify::prelude::*; @@ -5,6 +8,14 @@ use rspotify::{ClientCredsSpotify, Credentials}; use std::sync::Arc; use std::time::Duration; +#[cfg_attr(test, automock)] +#[async_trait] +pub(crate) trait SearchableClient { + async fn get_track(&self, id: &str) -> Option; + async fn get_album(&self, id: &str) -> Option; + async fn get_playlist(&self, id: &str) -> Option; +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum ContentKind { Track(String), @@ -69,13 +80,19 @@ impl Client { } } + #[allow(dead_code)] + #[allow(unused_variables)] + #[cfg(test)] pub(crate) fn new_with_dependencies(client: ClientCredsSpotify) -> Self { Client { client: Arc::new(client), } } +} - pub async fn get_track(&self, id: &str) -> Option { +#[async_trait] +impl SearchableClient for Client { + async fn get_track(&self, id: &str) -> Option { // FIXME should we really return Option here? We're hiding a possible error or a entry not found let track_id = match TrackId::from_id(id) { Ok(track) => track, @@ -92,7 +109,7 @@ impl Client { } } - pub async fn get_album(&self, id: &str) -> Option { + async fn get_album(&self, id: &str) -> Option { let album_id = match AlbumId::from_id(id) { Ok(album) => album, Err(_e) => return None, @@ -118,7 +135,7 @@ impl Client { } } - pub async fn get_playlist(&self, id: &str) -> Option { + async fn get_playlist(&self, id: &str) -> Option { let playlist_id = match PlaylistId::from_id(id) { Ok(playlist) => playlist, Err(_e) => return None, @@ -186,6 +203,7 @@ fn get_id_in_uri(uri: &str) -> Option<&str> { } pub fn get_entry_kind(uri: &str) -> Option { + // TODO WE SHOULD PROPERLY TEST THIS FUNCTION if uri.contains("spotify:track:") { let track_id = get_id_in_uri(uri); return match track_id { @@ -217,7 +235,7 @@ pub fn get_entry_kind(uri: &str) -> Option { if uri.contains("spotify:playlist:") { let track_id = get_id_in_uri(uri); return match track_id { - Some(id) => Some(ContentKind::Album(id.to_string())), + Some(id) => Some(ContentKind::Playlist(id.to_string())), None => None, }; } diff --git a/src/search/tests.rs b/src/search/tests.rs new file mode 100644 index 0000000..cc5c9fb --- /dev/null +++ b/src/search/tests.rs @@ -0,0 +1,147 @@ +use super::*; +use crate::search::youtube::VideoSearch; +use crate::spotify::PlayableKind; +use mockall::predicate; + +#[tokio::test] +async fn should_search_track_by_spotify_id() { + let spotify_id = "spotify:track:no-value-kek"; + let mut spotify_mock = spotify::MockSearchableClient::new(); + spotify_mock + .expect_get_track() + .with(predicate::eq("no-value-kek")) + .returning(|_id| { + Some(TrackInfo { + name: "A name".to_string(), + artists: vec!["Art1".to_string()], + duration: Default::default(), + }) + }); + let mut youtube_mock = youtube::MockSearchableClient::new(); + youtube_mock + .expect_search_video() + .returning(|_id, _sort_by| { + Ok(VideoSearch { + items: vec![Video { + title: "An example".to_string(), + video_id: "id123".to_string(), + author: "An Art".to_string(), + author_id: "artId123".to_string(), + author_url: "https://example.com".to_string(), + length_seconds: 42, + description: "A song".to_string(), + description_html: "A song 2".to_string(), + view_count: 0, + published: 0, + published_text: "".to_string(), + live_now: false, + paid: false, + premium: false, + }], + }) + }); + + let engine = Engine::new_with_dependencies(Box::new(spotify_mock), Box::new(youtube_mock)); + let got = engine.get_song_from_spotify_id(spotify_id).await; + + assert_eq!(true, got.spotify_track.is_some()); + let boxed_st = Box::new(got.spotify_track.unwrap()); + assert_eq!(1, boxed_st.artists.len()); + assert_eq!("Art1".to_string(), boxed_st.artists.get(0).unwrap().clone()); + assert_eq!("A name".to_string(), boxed_st.name); + + assert_eq!(true, got.youtube_track.is_some()); + let boxed_yt = Box::new(got.youtube_track.unwrap()); + assert_eq!(1, boxed_yt.len()); + let got_video = boxed_yt.get(0).unwrap(); + assert_eq!("An example".to_string(), got_video.title); +} + +#[tokio::test] +async fn should_search_album_by_spotify_id() { + let spotify_id = "spotify:album:no-value-kek"; + let mut spotify_mock = spotify::MockSearchableClient::new(); + spotify_mock + .expect_get_album() + .with(predicate::eq("no-value-kek")) + .returning(|_id| { + Some(AlbumInfo { + name: "An album".to_string(), + artists: vec!["Art1".to_string(), "Art2".to_string()], + genres: vec!["Rock".to_string(), "Hip-hop".to_string()], + tracks: vec![TrackInfo { + name: "Track info 1".to_string(), + artists: vec!["Art1".to_string()], + duration: Default::default(), + }], + }) + }); + let youtube_mock = youtube::MockSearchableClient::new(); + + let engine = Engine::new_with_dependencies(Box::new(spotify_mock), Box::new(youtube_mock)); + let got = engine.get_album_from_spotify_id(spotify_id).await; + + assert_eq!(true, got.spotify_album.is_some()); + let boxed_st = Box::new(got.spotify_album.unwrap()); + assert_eq!(2, boxed_st.artists.len()); + assert_eq!("Art1".to_string(), boxed_st.artists.get(0).unwrap().clone()); + assert_eq!("Art2".to_string(), boxed_st.artists.get(1).unwrap().clone()); + assert_eq!("An album".to_string(), boxed_st.name); + assert_eq!(2, boxed_st.genres.len()); + assert_eq!("Rock".to_string(), boxed_st.genres.get(0).unwrap().clone()); + assert_eq!( + "Hip-hop".to_string(), + boxed_st.genres.get(1).unwrap().clone() + ); + assert_eq!(1, boxed_st.tracks.len()); + assert_eq!( + TrackInfo { + name: "Track info 1".to_string(), + artists: vec!["Art1".to_string()], + duration: Default::default(), + }, + boxed_st.tracks.get(0).unwrap().clone() + ); +} + +#[tokio::test] +async fn should_search_playlist_by_spotify_id() { + let spotify_id = "spotify:playlist:no-value-kek"; + let mut spotify_mock = spotify::MockSearchableClient::new(); + spotify_mock + .expect_get_playlist() + .with(predicate::eq("no-value-kek")) + .returning(|_id| { + Some(PlaylistInfo { + name: "A playlist".to_string(), + artists: vec!["Art1".to_string(), "Art2".to_string()], + tracks: vec![PlayableKind::Track(TrackInfo { + name: "A track".to_string(), + artists: vec!["Art1".to_string()], + duration: Default::default(), + })], + owner: Some("Frodo".to_string()), + }) + }); + let youtube_mock = youtube::MockSearchableClient::new(); + + let engine = Engine::new_with_dependencies(Box::new(spotify_mock), Box::new(youtube_mock)); + let got = engine.get_playlist_from_spotify_id(spotify_id).await; + + assert_eq!(true, got.spotify_playlist.is_some()); + let boxed_st = Box::new(got.spotify_playlist.unwrap()); + assert_eq!(2, boxed_st.artists.len()); + assert_eq!("Art1".to_string(), boxed_st.artists.get(0).unwrap().clone()); + assert_eq!("Art2".to_string(), boxed_st.artists.get(1).unwrap().clone()); + assert_eq!("A playlist".to_string(), boxed_st.name); + assert_eq!(1, boxed_st.tracks.len()); + assert_eq!( + PlayableKind::Track(TrackInfo { + name: "A track".to_string(), + artists: vec!["Art1".to_string()], + duration: Default::default(), + }), + boxed_st.tracks.get(0).unwrap().clone() + ); + assert_eq!(Some("Frodo".to_string()), boxed_st.owner); +} diff --git a/src/search/youtube/mod.rs b/src/search/youtube/mod.rs index da645dc..e52bc35 100644 --- a/src/search/youtube/mod.rs +++ b/src/search/youtube/mod.rs @@ -1,6 +1,19 @@ +use async_trait::async_trait; +#[cfg(test)] +use mockall::{automock, mock, predicate::*}; use std::error::Error; use std::sync::Arc; +#[cfg_attr(test, automock)] +#[async_trait] +pub(crate) trait SearchableClient { + async fn search_video<'a>( + &self, + id: &str, + sort_by: Option<&'a SearchSortBy>, + ) -> Result>; +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub(crate) struct VideoSearch { pub(crate) items: Vec