From 364fa03f1ab7368305454a8dbc7c9232691a4aff Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Wed, 21 Apr 2021 16:47:20 +0200 Subject: [PATCH 01/18] relax lifetimes --- src/pagination/iter.rs | 8 ++------ src/pagination/stream.rs | 8 ++++---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/pagination/iter.rs b/src/pagination/iter.rs index 69475652..79db3575 100644 --- a/src/pagination/iter.rs +++ b/src/pagination/iter.rs @@ -8,13 +8,9 @@ pub trait Paginator: Iterator {} impl> Paginator for I {} /// This is used to handle paginated requests automatically. -pub fn paginate<'a, T, Request>( - req: Request, - page_size: u32, -) -> impl Iterator> + 'a +pub fn paginate(req: Request, page_size: u32) -> impl Iterator> where - T: 'a, - Request: Fn(u32, u32) -> ClientResult> + 'a, + Request: Fn(u32, u32) -> ClientResult>, { let pages = PageIterator { req, diff --git a/src/pagination/stream.rs b/src/pagination/stream.rs index dce0be01..b093ad63 100644 --- a/src/pagination/stream.rs +++ b/src/pagination/stream.rs @@ -10,14 +10,14 @@ pub trait Paginator: Stream {} impl> Paginator for I {} /// This is used to handle paginated requests automatically. -pub fn paginate<'a, T, Fut, Request>( +pub fn paginate( req: Request, page_size: u32, -) -> impl Stream> + 'a +) -> impl Stream> where - T: Unpin + 'a, + T: Unpin, Fut: Future>>, - Request: Fn(u32, u32) -> Fut + 'a, + Request: Fn(u32, u32) -> Fut, { use async_stream::stream; let mut offset = 0; From 5fc8c8e374d50061f2ab91d54f76c7b0c18bc486 Mon Sep 17 00:00:00 2001 From: Ramsay Date: Fri, 23 Apr 2021 21:11:07 +0800 Subject: [PATCH 02/18] Pass parameter by reference instead of by value. --- src/client.rs | 60 +++++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/src/client.rs b/src/client.rs index f3dd1dab..03f7b465 100644 --- a/src/client.rs +++ b/src/client.rs @@ -190,7 +190,7 @@ impl Spotify { pub async fn tracks<'a>( &self, track_ids: impl IntoIterator, - market: Option, + market: Option<&Market>, ) -> ClientResult> { let ids = join_ids(track_ids); let params = build_map! { @@ -250,7 +250,7 @@ impl Spotify { pub fn artist_albums<'a>( &'a self, artist_id: &'a ArtistId, - album_type: Option, + album_type: Option<&'a AlbumType>, market: Option<&'a Market>, ) -> impl Paginator> + 'a { paginate( @@ -266,7 +266,7 @@ impl Spotify { pub async fn artist_albums_manual( &self, artist_id: &ArtistId, - album_type: Option, + album_type: Option<&AlbumType>, market: Option<&Market>, limit: Option, offset: Option, @@ -297,7 +297,7 @@ impl Spotify { pub async fn artist_top_tracks( &self, artist_id: &ArtistId, - market: Market, + market: &Market, ) -> ClientResult> { let params = build_map! { required market => market.as_ref() @@ -377,9 +377,9 @@ impl Spotify { pub async fn search( &self, q: &str, - r#type: SearchType, - market: Option, - include_external: Option, + r#type: &SearchType, + market: Option<&Market>, + include_external: Option<&IncludeExternal>, limit: Option, offset: Option, ) -> ClientResult { @@ -465,7 +465,7 @@ impl Spotify { &self, playlist_id: &PlaylistId, fields: Option<&str>, - market: Option, + market: Option<&Market>, ) -> ClientResult { let params = build_map! { optional fields, @@ -856,10 +856,10 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-remove-tracks-playlist) #[maybe_async] - pub async fn playlist_remove_specific_occurrences_of_tracks( + pub async fn playlist_remove_specific_occurrences_of_tracks<'a>( &self, playlist_id: &PlaylistId, - tracks: Vec>, + tracks: impl IntoIterator>, snapshot_id: Option<&str>, ) -> ClientResult { let tracks = tracks @@ -867,7 +867,7 @@ impl Spotify { .map(|track| { let mut map = Map::new(); map.insert("uri".to_owned(), track.id.uri().into()); - map.insert("positions".to_owned(), track.positions.into()); + map.insert("positions".to_owned(), track.positions.clone().into()); map }) .collect::>(); @@ -1391,8 +1391,8 @@ impl Spotify { pub async fn featured_playlists( &self, locale: Option<&str>, - country: Option, - timestamp: Option>, + country: Option<&Market>, + timestamp: Option<&DateTime>, limit: Option, offset: Option, ) -> ClientResult { @@ -1571,17 +1571,17 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-get-recommendations) #[maybe_async] - pub async fn recommendations( + pub async fn recommendations<'a>( &self, payload: &Map, - seed_artists: Option>, - seed_genres: Option>, - seed_tracks: Option>, + seed_artists: Option>, + seed_genres: Option>, + seed_tracks: Option>, limit: Option, market: Option, ) -> ClientResult { let seed_artists = seed_artists.map(join_ids); - let seed_genres = seed_genres.map(|x| x.join(",")); + let seed_genres = seed_genres.map(|x| x.into_iter().collect::>().join(",")); let seed_tracks = seed_tracks.map(join_ids); let limit = limit.map(|x| x.to_string()); let mut params = build_map! { @@ -1730,13 +1730,17 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-get-recently-played) #[maybe_async] - pub async fn current_playing( + pub async fn current_playing<'a>( &self, - market: Option, - additional_types: Option>, + market: Option<&'a Market>, + additional_types: Option>, ) -> ClientResult> { - let additional_types = - additional_types.map(|x| x.iter().map(|x| x.as_ref()).collect::>().join(",")); + let additional_types = additional_types.map(|x| { + x.into_iter() + .map(|x| x.as_ref()) + .collect::>() + .join(",") + }); let params = build_map! { optional market => market.as_ref(), optional additional_types, @@ -1912,7 +1916,7 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-set-repeat-mode-on-users-playback) #[maybe_async] - pub async fn repeat(&self, state: RepeatState, device_id: Option<&str>) -> ClientResult<()> { + pub async fn repeat(&self, state: &RepeatState, device_id: Option<&str>) -> ClientResult<()> { let url = self.append_device_id( &format!("me/player/repeat?state={}", state.as_ref()), device_id, @@ -2067,7 +2071,7 @@ impl Spotify { pub async fn get_several_shows<'a>( &self, ids: impl IntoIterator, - market: Option, + market: Option<&Market>, ) -> ClientResult> { let ids = join_ids(ids); let params = build_map! { @@ -2143,7 +2147,7 @@ impl Spotify { pub async fn get_an_episode( &self, id: &EpisodeId, - market: Option, + market: Option<&Market>, ) -> ClientResult { let url = format!("episodes/{}", id.id()); let params = build_map! { @@ -2165,7 +2169,7 @@ impl Spotify { pub async fn get_several_episodes<'a>( &self, ids: impl IntoIterator, - market: Option, + market: Option<&Market>, ) -> ClientResult> { let ids = join_ids(ids); let params = build_map! { @@ -2209,7 +2213,7 @@ impl Spotify { pub async fn remove_users_saved_shows<'a>( &self, show_ids: impl IntoIterator, - country: Option, + country: Option<&Market>, ) -> ClientResult<()> { let url = format!("me/shows?ids={}", join_ids(show_ids)); let params = build_json! { From 94a792cfec8804805a7cd748bd42f261fd951f4c Mon Sep 17 00:00:00 2001 From: Ramsay Date: Fri, 23 Apr 2021 22:52:12 +0800 Subject: [PATCH 03/18] Update CHANGELOG.md --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fdce9b3..5aabfd61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -215,6 +215,22 @@ If we missed any change or there's something you'd like to discuss about this ve - No default values are set from Rspotify now, they will be left to the Spotify API. - ([#202](https://github.com/ramsayleung/rspotify/pull/202)) Add a `collaborative` parameter to `user_playlist_create`. - ([#202](https://github.com/ramsayleung/rspotify/pull/202)) Add a `uris` parameter to `playlist_reorder_tracks`. +- ([]()) Update the endpoint signatures to pass parameters by reference, affected endpoint list: + + `tracks` + + `artist_albums` + + `artist_albums_manual` + + `artist_top_tracks` + + `search` + + `playlist` + + `playlist_remove_specific_occurences_of_tracks` + + `featured_playlists` + + `recommendations` + + `current_playlist` + + `repeat` + + `get_seversal_shows` + + `get_an_episode` + + `get_several_episodes` + + `remove_users_saved_shows` ## 0.10 (2020/07/01) From 6e5206c74a4d78c4eb89535ad1fa3df012b89ea6 Mon Sep 17 00:00:00 2001 From: Ramsay Date: Sat, 24 Apr 2021 10:20:12 +0800 Subject: [PATCH 04/18] Update tests to pass parameters by reference --- tests/test_with_credential.rs | 4 +-- tests/test_with_oauth.rs | 48 +++++++++++++++++------------------ 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/test_with_credential.rs b/tests/test_with_credential.rs index 4df85a6c..81ec48af 100644 --- a/tests/test_with_credential.rs +++ b/tests/test_with_credential.rs @@ -85,7 +85,7 @@ async fn test_artists_albums() { .await .artist_albums_manual( birdy_uri, - Some(AlbumType::Album), + Some(&AlbumType::Album), Some(&Market::Country(Country::UnitedStates)), Some(10), None, @@ -109,7 +109,7 @@ async fn test_artist_top_tracks() { let birdy_uri = Id::from_uri("spotify:artist:2WX2uTcsvV5OnS0inACecP").unwrap(); creds_client() .await - .artist_top_tracks(birdy_uri, Market::Country(Country::UnitedStates)) + .artist_top_tracks(birdy_uri, &Market::Country(Country::UnitedStates)) .await .unwrap(); } diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index 67e3cff0..30cf6a4f 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -18,6 +18,7 @@ mod common; use common::maybe_async_test; use rspotify::model::offset::Offset; +use rspotify::model::AdditionalType; use rspotify::oauth2::{CredentialsBuilder, OAuthBuilder, TokenBuilder}; use rspotify::{ client::{Spotify, SpotifyBuilder}, @@ -144,7 +145,7 @@ async fn test_current_playback() { async fn test_current_playing() { oauth_client() .await - .current_playing(None, None) + .current_playing(None, None::>>) .await .unwrap(); } @@ -331,7 +332,7 @@ async fn test_featured_playlists() { let now: DateTime = Utc::now(); oauth_client() .await - .featured_playlists(None, None, Some(now), Some(10), Some(0)) + .featured_playlists(None, None, Some(&now), Some(10), Some(0)) .await .unwrap(); } @@ -415,7 +416,7 @@ async fn test_recommendations() { .recommendations( &payload, Some(seed_artists), - None, + None::>>, Some(seed_tracks), Some(10), Some(Market::Country(Country::UnitedStates)), @@ -430,7 +431,7 @@ async fn test_recommendations() { async fn test_repeat() { oauth_client() .await - .repeat(RepeatState::Context, None) + .repeat(&RepeatState::Context, None) .await .unwrap(); } @@ -442,7 +443,7 @@ async fn test_search_album() { let query = "album:arrival artist:abba"; oauth_client() .await - .search(query, SearchType::Album, None, None, Some(10), Some(0)) + .search(query, &SearchType::Album, None, None, Some(10), Some(0)) .await .unwrap(); } @@ -456,8 +457,8 @@ async fn test_search_artist() { .await .search( query, - SearchType::Artist, - Some(Market::Country(Country::UnitedStates)), + &SearchType::Artist, + Some(&Market::Country(Country::UnitedStates)), None, Some(10), Some(0), @@ -475,8 +476,8 @@ async fn test_search_playlist() { .await .search( query, - SearchType::Playlist, - Some(Market::Country(Country::UnitedStates)), + &SearchType::Playlist, + Some(&Market::Country(Country::UnitedStates)), None, Some(10), Some(0), @@ -494,8 +495,8 @@ async fn test_search_track() { .await .search( query, - SearchType::Track, - Some(Market::Country(Country::UnitedStates)), + &SearchType::Track, + Some(&Market::Country(Country::UnitedStates)), None, Some(10), Some(0), @@ -716,19 +717,18 @@ async fn test_playlist_remove_all_occurrences_of_tracks() { #[ignore] async fn test_playlist_remove_specific_occurrences_of_tracks() { let playlist_id = Id::from_id("5jAOgWXCBKuinsGiZxjDQ5").unwrap(); - let tracks = vec![ - TrackPositions::new( - Id::from_uri("spotify:track:4iV5W9uYEdYUVa79Axb7Rh").unwrap(), - vec![0, 3], - ), - TrackPositions::new( - Id::from_uri("spotify:track:1301WleyT98MSxVHPZCA6M").unwrap(), - vec![7], - ), - ]; - oauth_client() - .await - .playlist_remove_specific_occurrences_of_tracks(playlist_id, tracks, None) + let track_position_0_3 = TrackPositions::new( + Id::from_uri("spotify:track:4iV5W9uYEdYUVa79Axb7Rh").unwrap(), + vec![0, 3], + ); + let track_position_7 = TrackPositions::new( + Id::from_uri("spotify:track:1301WleyT98MSxVHPZCA6M").unwrap(), + vec![7], + ); + let tracks = vec![&track_position_0_3, &track_position_7]; + oauth_client() + .await + .playlist_remove_specific_occurrences_of_tracks(&playlist_id, tracks, None::<&str>) .await .unwrap(); } From 78962cd655958070f3d22c9c4a4bbe1812c51065 Mon Sep 17 00:00:00 2001 From: Ramsay Date: Sat, 24 Apr 2021 20:30:06 +0800 Subject: [PATCH 05/18] Replace for loop with iterator --- src/client.rs | 110 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 98 insertions(+), 12 deletions(-) diff --git a/src/client.rs b/src/client.rs index 03f7b465..d33253f4 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1592,7 +1592,6 @@ impl Spotify { optional limit, }; - // TODO: this probably can be improved. let attributes = [ "acousticness", "danceability", @@ -1609,18 +1608,24 @@ impl Spotify { "time_signature", "valence", ]; - let mut map_to_hold_owned_value = HashMap::new(); let prefixes = ["min", "max", "target"]; - for attribute in attributes.iter() { - for prefix in prefixes.iter() { - let param = format!("{}_{}", prefix, attribute); - if let Some(value) = payload.get(¶m) { - // TODO: not sure if this `to_string` is what we want. It - // might add quotes to the strings. - map_to_hold_owned_value.insert(param, value.to_string()); - } - } - } + + // This map is used to store the intermediate data which lives long enough + // to be borrowed into the `params` + let map_to_hold_owned_value = attributes + .iter() + // create cartesian product for attributes and prefixes + .flat_map(|attribute| { + prefixes + .iter() + .map(move |prefix| format!("{}_{}", prefix, attribute)) + }) + .filter_map( + // TODO: not sure if this `to_string` is what we want. It + // might add quotes to the strings. + |param| payload.get(¶m).map(|value| (param, value.to_string())), + ) + .collect::>(); for (ref key, ref value) in &map_to_hold_owned_value { params.insert(key, value); @@ -2262,4 +2267,85 @@ mod test { "me/player/shuffle?state=true&device_id=fdafdsadfa" ); } + + #[test] + fn test_cartesian_product() { + let attributes = [ + "acousticness", + "danceability", + "duration_ms", + "energy", + "instrumentalness", + "key", + "liveness", + "loudness", + "mode", + "popularity", + "speechiness", + "tempo", + "time_signature", + "valence", + ]; + let prefixes = ["min", "max", "target"]; + + let iterator_product_result = attributes + .iter() + .flat_map(|attribute| { + prefixes + .iter() + .map(move |prefix| format!("{}_{}", prefix, attribute)) + }) + .collect::>(); + let mut loop_product_result = vec![]; + for attribute in attributes.iter() { + for prefix in prefixes.iter() { + let param = format!("{}_{}", prefix, attribute); + loop_product_result.push(param); + } + } + assert!(iterator_product_result + .iter() + .eq(loop_product_result.iter())); + } + #[test] + fn test_cartesian_product_and_filter_map() { + let attributes = [ + "acousticness", + "danceability", + "duration_ms", + "energy", + "instrumentalness", + "key", + "liveness", + "loudness", + "mode", + "popularity", + "speechiness", + "tempo", + "time_signature", + "valence", + ]; + let prefixes = ["min", "max", "target"]; + let mut store = HashMap::new(); + store.insert("min_valence".to_owned(), "foo".to_owned()); + store.insert("max_tempo".to_owned(), "bar".to_owned()); + let found_key_value = attributes + .iter() + .flat_map(|attribute| { + prefixes + .iter() + .map(move |prefix| format!("{}_{}", prefix, attribute)) + }) + .filter_map(|param| store.get(¶m).map(|value| (param, value.to_owned()))) + .collect::>(); + assert_eq!(found_key_value.len(), 2); + assert_eq!( + found_key_value.get(&"min_valence".to_string()), + Some(&"foo".to_string()) + ); + assert_eq!( + found_key_value.get(&"max_tempo".to_string()), + Some(&"bar".to_string()) + ); + } } From 65ddd086337d78c03de33663d2a1973e2cd508fc Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Sat, 24 Apr 2021 18:09:03 +0200 Subject: [PATCH 06/18] apply review suggestions --- src/client.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/client.rs b/src/client.rs index c74ffe54..ea9e82b5 100644 --- a/src/client.rs +++ b/src/client.rs @@ -778,7 +778,6 @@ impl Spotify { ) -> ClientResult { let uris = uris.map(|u| u.iter().map(|id| id.uri()).collect::>()); let params = build_json! { - "playlist_id": playlist_id, optional "uris": uris, optional "range_start": range_start, optional "insert_before": insert_before, @@ -1061,7 +1060,7 @@ impl Spotify { ) -> ClientResult> { let limit = limit.map(|s| s.to_string()); let params = build_map! { - "r#type": Type::Artist.as_ref(), + "type": Type::Artist.as_ref(), optional "after": after, optional "limit": limit.as_deref(), }; @@ -1576,8 +1575,8 @@ impl Spotify { seed_artists: Option>, seed_genres: Option>, seed_tracks: Option>, - limit: Option, market: Option<&Market>, + limit: Option, ) -> ClientResult { let seed_artists = seed_artists.map(join_ids); let seed_genres = seed_genres.map(|x| x.join(",")); @@ -1758,19 +1757,18 @@ impl Spotify { /// /// Parameters: /// - device_id - transfer playback to this device - /// - force_play - true: after transfer, play. false: - /// keep current state. + /// - force_play - true: after transfer, play. false: keep current state. /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-transfer-a-users-playback) #[maybe_async] pub async fn transfer_playback( &self, device_id: &str, - force_play: Option, + play: Option, ) -> ClientResult<()> { let params = build_json! { "device_ids": [device_id], - optional "force_play": force_play, + optional "play": play, }; self.endpoint_put("me/player", ¶ms).await?; From 307c946b784b7efd6b093f1a75271dbc29c22c47 Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Sat, 24 Apr 2021 18:10:25 +0200 Subject: [PATCH 07/18] remove unnecessary type --- src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.rs b/src/client.rs index ea9e82b5..802c3a2d 100644 --- a/src/client.rs +++ b/src/client.rs @@ -271,7 +271,7 @@ impl Spotify { limit: Option, offset: Option, ) -> ClientResult> { - let limit: Option = limit.map(|x| x.to_string()); + let limit = limit.map(|x| x.to_string()); let offset = offset.map(|x| x.to_string()); let params = build_map! { optional "album_type": album_type.map(|x| x.as_ref()), From d48b41e9673696d0a3ce77c030921a17f88b0df8 Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Sat, 24 Apr 2021 18:21:22 +0200 Subject: [PATCH 08/18] use IntoIterator --- src/client.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client.rs b/src/client.rs index 802c3a2d..036cfcb6 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1698,13 +1698,13 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-get-information-about-the-users-current-playback) #[maybe_async] - pub async fn current_playback( + pub async fn current_playback<'a>( &self, country: Option<&Market>, - additional_types: Option>, + additional_types: Option>, ) -> ClientResult> { let additional_types = - additional_types.map(|x| x.iter().map(|x| x.as_ref()).collect::>().join(",")); + additional_types.map(|x| x.into_iter().map(|x| x.as_ref()).collect::>().join(",")); let params = build_map! { optional "country": country.map(|x| x.as_ref()), optional "additional_types": additional_types.as_ref(), From 87eb49cabf8ee32be1beb39f6c77834c12140142 Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Sat, 24 Apr 2021 18:39:48 +0200 Subject: [PATCH 09/18] fix some tests --- tests/test_with_oauth.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index f06422f0..bce2753c 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -133,7 +133,7 @@ async fn test_category_playlists() { async fn test_current_playback() { oauth_client() .await - .current_playback(None, None) + .current_playback(None, Option::<&[_]>::None) .await .unwrap(); } @@ -417,8 +417,8 @@ async fn test_recommendations() { Some(seed_artists), None, Some(seed_tracks), - Some(10), Some(&Market::Country(Country::UnitedStates)), + Some(10), ) .await .unwrap(); From 89f325028181d4b46c1699f5a276703852948313 Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Sat, 24 Apr 2021 18:42:03 +0200 Subject: [PATCH 10/18] somewhat prettier syntax --- src/client.rs | 18 +++++++++--------- tests/test_with_oauth.rs | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/client.rs b/src/client.rs index 036cfcb6..c21d52fb 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1698,13 +1698,17 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-get-information-about-the-users-current-playback) #[maybe_async] - pub async fn current_playback<'a>( + pub async fn current_playback<'a, A: IntoIterator>( &self, country: Option<&Market>, - additional_types: Option>, + additional_types: Option, ) -> ClientResult> { - let additional_types = - additional_types.map(|x| x.into_iter().map(|x| x.as_ref()).collect::>().join(",")); + let additional_types = additional_types.map(|x| { + x.into_iter() + .map(|x| x.as_ref()) + .collect::>() + .join(",") + }); let params = build_map! { optional "country": country.map(|x| x.as_ref()), optional "additional_types": additional_types.as_ref(), @@ -1761,11 +1765,7 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-transfer-a-users-playback) #[maybe_async] - pub async fn transfer_playback( - &self, - device_id: &str, - play: Option, - ) -> ClientResult<()> { + pub async fn transfer_playback(&self, device_id: &str, play: Option) -> ClientResult<()> { let params = build_json! { "device_ids": [device_id], optional "play": play, diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index bce2753c..73fedd5b 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -133,7 +133,7 @@ async fn test_category_playlists() { async fn test_current_playback() { oauth_client() .await - .current_playback(None, Option::<&[_]>::None) + .current_playback::<&[_]>(None, None) .await .unwrap(); } From 050215783e3954f837ff1dc9ff2da83a79281801 Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Sat, 24 Apr 2021 18:45:36 +0200 Subject: [PATCH 11/18] as_deref is clearer than as_ref --- src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.rs b/src/client.rs index c21d52fb..cb4dbc07 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1711,7 +1711,7 @@ impl Spotify { }); let params = build_map! { optional "country": country.map(|x| x.as_ref()), - optional "additional_types": additional_types.as_ref(), + optional "additional_types": additional_types.as_deref(), }; let result = self.endpoint_get("me/player", ¶ms).await?; From 0e6173b8d2c9f9c0e040bf7408a33621fb1a15e3 Mon Sep 17 00:00:00 2001 From: Mario Ortiz Manero Date: Mon, 26 Apr 2021 08:55:17 +0200 Subject: [PATCH 12/18] fix optional --- src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.rs b/src/client.rs index cb4dbc07..cf716b30 100644 --- a/src/client.rs +++ b/src/client.rs @@ -721,7 +721,7 @@ impl Spotify { let uris = track_ids.into_iter().map(|id| id.uri()).collect::>(); let params = build_json! { "uris": uris, - "position": position, + optional "position": position, }; let url = format!("playlists/{}/tracks", playlist_id.id()); From 29625fd70d987dacf1b982b6332aa907167f0ddb Mon Sep 17 00:00:00 2001 From: Ramsay Date: Mon, 26 Apr 2021 21:33:25 +0800 Subject: [PATCH 13/18] Replace slice reference with IntoIterator --- src/client.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/client.rs b/src/client.rs index 4cd28836..d1e98417 100644 --- a/src/client.rs +++ b/src/client.rs @@ -767,16 +767,16 @@ impl Spotify { /// /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#endpoint-reorder-or-replace-playlists-tracks) #[maybe_async] - pub async fn playlist_reorder_tracks( + pub async fn playlist_reorder_tracks<'a, T: PlayableIdType + 'a>( &self, playlist_id: &PlaylistId, - uris: Option<&[&Id]>, + uris: Option>>, range_start: Option, insert_before: Option, range_length: Option, snapshot_id: Option<&str>, ) -> ClientResult { - let uris = uris.map(|u| u.iter().map(|id| id.uri()).collect::>()); + let uris = uris.map(|u| u.into_iter().map(|id| id.uri()).collect::>()); let params = build_json! { optional "uris": uris, optional "range_start": range_start, @@ -1826,9 +1826,9 @@ impl Spotify { } #[maybe_async] - pub async fn start_uris_playback( + pub async fn start_uris_playback<'a, T: PlayableIdType + 'a>( &self, - uris: &[&Id], + uris: impl IntoIterator>, device_id: Option<&str>, offset: Option>, position_ms: Option, @@ -1836,7 +1836,7 @@ impl Spotify { use super::model::Offset; let params = build_json! { - "uris": uris.iter().map(|id| id.uri()).collect::>(), + "uris": uris.into_iter().map(|id| id.uri()).collect::>(), optional "position_ms": position_ms, optional "offset": offset.map(|x| match x { Offset::Position(position) => json!({ "position": position }), From 1488532825d88d279367fc105c09b338da44bdab Mon Sep 17 00:00:00 2001 From: Ramsay Date: Mon, 26 Apr 2021 21:44:13 +0800 Subject: [PATCH 14/18] Fix compilation error --- tests/test_with_oauth.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index d67e4f28..fba26f37 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -527,7 +527,7 @@ async fn test_start_playback() { let uris = vec![TrackId::from_uri("spotify:track:4iV5W9uYEdYUVa79Axb7Rh").unwrap()]; oauth_client() .await - .start_uris_playback(&uris, Some(device_id), Some(Offset::for_position(0)), None) + .start_uris_playback(uris, Some(device_id), Some(Offset::for_position(0)), None) .await .unwrap(); } @@ -676,7 +676,7 @@ async fn test_playlist_follow_playlist() { #[maybe_async_test] #[ignore] async fn test_playlist_recorder_tracks() { - let uris: Option<&[&EpisodeId]> = None; + let uris = Some(vec![EpisodeId::from_id("0lbiy3LKzIY2fnyjioC11p").unwrap()]); let playlist_id = Id::from_id("5jAOgWXCBKuinsGiZxjDQ5").unwrap(); let range_start = 0; let insert_before = 1; From df8d58ada15e77e1612872c1dc63db011a653e37 Mon Sep 17 00:00:00 2001 From: Ramsay Date: Mon, 26 Apr 2021 21:52:41 +0800 Subject: [PATCH 15/18] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5aabfd61..77eb8a1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -215,7 +215,7 @@ If we missed any change or there's something you'd like to discuss about this ve - No default values are set from Rspotify now, they will be left to the Spotify API. - ([#202](https://github.com/ramsayleung/rspotify/pull/202)) Add a `collaborative` parameter to `user_playlist_create`. - ([#202](https://github.com/ramsayleung/rspotify/pull/202)) Add a `uris` parameter to `playlist_reorder_tracks`. -- ([]()) Update the endpoint signatures to pass parameters by reference, affected endpoint list: +- ([#206](https://github.com/ramsayleung/rspotify/pull/206)) Update the endpoint signatures to pass parameters by reference, affected endpoint list: + `tracks` + `artist_albums` + `artist_albums_manual` From 926077c6ec85b17081cfaf1dcd083585025f3f74 Mon Sep 17 00:00:00 2001 From: Ramsay Date: Tue, 27 Apr 2021 22:24:13 +0800 Subject: [PATCH 16/18] Removed useless tests and optimize code --- src/client.rs | 83 +--------------------------------------- tests/test_with_oauth.rs | 2 +- 2 files changed, 2 insertions(+), 83 deletions(-) diff --git a/src/client.rs b/src/client.rs index d1e98417..0e12fcdf 100644 --- a/src/client.rs +++ b/src/client.rs @@ -865,7 +865,7 @@ impl Spotify { .map(|track| { let mut map = Map::new(); map.insert("uri".to_owned(), track.id.uri().into()); - map.insert("positions".to_owned(), track.positions.clone().into()); + map.insert("positions".to_owned(), json!(track.positions)); map }) .collect::>(); @@ -2264,85 +2264,4 @@ mod test { "me/player/shuffle?state=true&device_id=fdafdsadfa" ); } - - #[test] - fn test_cartesian_product() { - let attributes = [ - "acousticness", - "danceability", - "duration_ms", - "energy", - "instrumentalness", - "key", - "liveness", - "loudness", - "mode", - "popularity", - "speechiness", - "tempo", - "time_signature", - "valence", - ]; - let prefixes = ["min", "max", "target"]; - - let iterator_product_result = attributes - .iter() - .flat_map(|attribute| { - prefixes - .iter() - .map(move |prefix| format!("{}_{}", prefix, attribute)) - }) - .collect::>(); - let mut loop_product_result = vec![]; - for attribute in attributes.iter() { - for prefix in prefixes.iter() { - let param = format!("{}_{}", prefix, attribute); - loop_product_result.push(param); - } - } - assert!(iterator_product_result - .iter() - .eq(loop_product_result.iter())); - } - #[test] - fn test_cartesian_product_and_filter_map() { - let attributes = [ - "acousticness", - "danceability", - "duration_ms", - "energy", - "instrumentalness", - "key", - "liveness", - "loudness", - "mode", - "popularity", - "speechiness", - "tempo", - "time_signature", - "valence", - ]; - let prefixes = ["min", "max", "target"]; - let mut store = HashMap::new(); - store.insert("min_valence".to_owned(), "foo".to_owned()); - store.insert("max_tempo".to_owned(), "bar".to_owned()); - let found_key_value = attributes - .iter() - .flat_map(|attribute| { - prefixes - .iter() - .map(move |prefix| format!("{}_{}", prefix, attribute)) - }) - .filter_map(|param| store.get(¶m).map(|value| (param, value.to_owned()))) - .collect::>(); - assert_eq!(found_key_value.len(), 2); - assert_eq!( - found_key_value.get(&"min_valence".to_string()), - Some(&"foo".to_string()) - ); - assert_eq!( - found_key_value.get(&"max_tempo".to_string()), - Some(&"bar".to_string()) - ); - } } diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index fba26f37..affdd1ec 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -728,7 +728,7 @@ async fn test_playlist_remove_specific_occurrences_of_tracks() { let tracks = vec![&track_position_0_3, &track_position_7]; oauth_client() .await - .playlist_remove_specific_occurrences_of_tracks(&playlist_id, tracks, None::<&str>) + .playlist_remove_specific_occurrences_of_tracks(playlist_id, tracks, None::<&str>) .await .unwrap(); } From 51b8b4a0df6ec983e63a22a84696a33d39562a46 Mon Sep 17 00:00:00 2001 From: Ramsay Date: Tue, 27 Apr 2021 23:24:11 +0800 Subject: [PATCH 17/18] remove explicit type declaration --- src/client.rs | 2 +- tests/test_with_oauth.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/client.rs b/src/client.rs index 0e12fcdf..3adee6bc 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1623,7 +1623,7 @@ impl Spotify { // might add quotes to the strings. |param| payload.get(¶m).map(|value| (param, value.to_string())), ) - .collect::>(); + .collect::>(); for (ref key, ref value) in &map_to_hold_owned_value { params.insert(key, value); diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index affdd1ec..4dfeb031 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -18,7 +18,6 @@ mod common; use common::maybe_async_test; use rspotify::model::offset::Offset; -use rspotify::model::AdditionalType; use rspotify::oauth2::{CredentialsBuilder, OAuthBuilder, TokenBuilder}; use rspotify::{ client::{Spotify, SpotifyBuilder}, @@ -31,7 +30,6 @@ use rspotify::{ use chrono::prelude::*; use maybe_async::maybe_async; -use serde_json::map::Map; use std::env; /// Generating a new OAuth client for the requests. @@ -145,7 +143,7 @@ async fn test_current_playback() { async fn test_current_playing() { oauth_client() .await - .current_playing(None, None::>>) + .current_playing(None, None::<&[_]>) .await .unwrap(); } @@ -416,7 +414,7 @@ async fn test_recommendations() { .recommendations( &payload, Some(seed_artists), - None::>>, + None::<&[&str]>, Some(seed_tracks), Some(&Market::Country(Country::UnitedStates)), Some(10), From a09716eada8a085ff515747163f21c200e2bf6af Mon Sep 17 00:00:00 2001 From: Ramsay Date: Tue, 27 Apr 2021 23:33:50 +0800 Subject: [PATCH 18/18] Fix compile error --- tests/test_with_oauth.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index 4dfeb031..6a10236c 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -30,6 +30,7 @@ use rspotify::{ use chrono::prelude::*; use maybe_async::maybe_async; +use serde_json::map::Map; use std::env; /// Generating a new OAuth client for the requests. @@ -414,7 +415,7 @@ async fn test_recommendations() { .recommendations( &payload, Some(seed_artists), - None::<&[&str]>, + None::>, Some(seed_tracks), Some(&Market::Country(Country::UnitedStates)), Some(10),