Pass parameters by reference and use iterators wherever possible

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`.
+- ([#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`
+    + `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)
 

src/client.rs

@@ -297,7 +297,7 @@ impl Spotify {
     pub async fn artist_top_tracks(
         &self,
         artist_id: &ArtistId,
-        market: Market,
+        market: &Market,
     ) -> ClientResult<Vec<FullTrack>> {
         let params = build_map! {
             "market": market.as_ref()
@@ -377,7 +377,7 @@ impl Spotify {
     pub async fn search(
         &self,
         q: &str,
-        _type: SearchType,
+        _type: &SearchType,
         market: Option<&Market>,
         include_external: Option<&IncludeExternal>,
         limit: Option<u32>,
@@ -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<impl PlayableIdType>]>,
+        uris: Option<impl IntoIterator<Item = &'a Id<T>>>,
         range_start: Option<i32>,
         insert_before: Option<i32>,
         range_length: Option<u32>,
         snapshot_id: Option<&str>,
     ) -> ClientResult<PlaylistResult> {
-        let uris = uris.map(|u| u.iter().map(|id| id.uri()).collect::<Vec<_>>());
+        let uris = uris.map(|u| u.into_iter().map(|id| id.uri()).collect::<Vec<_>>());
         let params = build_json! {
             optional "uris": uris,
             optional "range_start": range_start,
@@ -854,18 +854,18 @@ 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<TrackPositions<'_>>,
+        tracks: impl IntoIterator<Item = &'a TrackPositions<'a>>,
         snapshot_id: Option<&str>,
     ) -> ClientResult<PlaylistResult> {
         let tracks = tracks
             .into_iter()
             .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(), json!(track.positions));
                 map
             })
             .collect::<Vec<_>>();
@@ -1390,7 +1390,7 @@ impl Spotify {
         &self,
         locale: Option<&str>,
         country: Option<&Market>,
-        timestamp: Option<DateTime<Utc>>,
+        timestamp: Option<&DateTime<Utc>>,
         limit: Option<u32>,
         offset: Option<u32>,
     ) -> ClientResult<FeaturedPlaylists> {
@@ -1569,17 +1569,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<String, Value>,
-        seed_artists: Option<Vec<&ArtistId>>,
-        seed_genres: Option<Vec<&str>>,
-        seed_tracks: Option<Vec<&TrackId>>,
+        seed_artists: Option<impl IntoIterator<Item = &'a ArtistId>>,
+        seed_genres: Option<impl IntoIterator<Item = &'a str>>,
+        seed_tracks: Option<impl IntoIterator<Item = &'a TrackId>>,
         market: Option<&Market>,
         limit: Option<u32>,
     ) -> ClientResult<Recommendations> {
         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::<Vec<_>>().join(","));
         let seed_tracks = seed_tracks.map(join_ids);
         let limit = limit.map(|x| x.to_string());
         let mut params = build_map! {
@@ -1590,7 +1590,6 @@ impl Spotify {
             optional "limit": limit.as_ref(),
         };
 
-        // TODO: this probably can be improved.
         let attributes = [
             "acousticness",
             "danceability",
@@ -1607,18 +1606,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(&param) {
-                    // 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(&param).map(|value| (param, value.to_string())),
+            )
+            .collect::<HashMap<String, String>>();
 
         for (ref key, ref value) in &map_to_hold_owned_value {
             params.insert(key, value);
@@ -1732,13 +1737,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<&Market>,
-        additional_types: Option<Vec<AdditionalType>>,
+        market: Option<&'a Market>,
+        additional_types: Option<impl IntoIterator<Item = &'a AdditionalType>>,
     ) -> ClientResult<Option<CurrentlyPlayingContext>> {
-        let additional_types =
-            additional_types.map(|x| x.iter().map(|x| x.as_ref()).collect::<Vec<_>>().join(","));
+        let additional_types = additional_types.map(|x| {
+            x.into_iter()
+                .map(|x| x.as_ref())
+                .collect::<Vec<_>>()
+                .join(",")
+        });
         let params = build_map! {
             optional "market": market.map(|x| x.as_ref()),
             optional "additional_types": additional_types.as_ref(),
@@ -1817,17 +1826,17 @@ impl Spotify {
     }
 
     #[maybe_async]
-    pub async fn start_uris_playback<T: PlayableIdType>(
+    pub async fn start_uris_playback<'a, T: PlayableIdType + 'a>(
         &self,
-        uris: &[&Id<T>],
+        uris: impl IntoIterator<Item = &'a Id<T>>,
         device_id: Option<&str>,
         offset: Option<super::model::Offset<T>>,
         position_ms: Option<u32>,
     ) -> ClientResult<()> {
         use super::model::Offset;
 
         let params = build_json! {
-            "uris": uris.iter().map(|id| id.uri()).collect::<Vec<_>>(),
+            "uris": uris.into_iter().map(|id| id.uri()).collect::<Vec<_>>(),
             optional "position_ms": position_ms,
             optional "offset": offset.map(|x| match x {
                 Offset::Position(position) => json!({ "position": position }),
@@ -1909,7 +1918,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,

tests/test_with_credential.rs

@@ -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();
 }

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::<Box<dyn Iterator<Item = &AdditionalType>>>)
         .await
         .unwrap();
 }
@@ -331,7 +332,7 @@ async fn test_featured_playlists() {
     let now: DateTime<Utc> = 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::<Box<dyn Iterator<Item = &str>>>,
             Some(seed_tracks),
             Some(&Market::Country(Country::UnitedStates)),
             Some(10),
@@ -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,7 +457,7 @@ async fn test_search_artist() {
         .await
         .search(
             query,
-            SearchType::Artist,
+            &SearchType::Artist,
             Some(&Market::Country(Country::UnitedStates)),
             None,
             Some(10),
@@ -475,7 +476,7 @@ async fn test_search_playlist() {
         .await
         .search(
             query,
-            SearchType::Playlist,
+            &SearchType::Playlist,
             Some(&Market::Country(Country::UnitedStates)),
             None,
             Some(10),
@@ -494,7 +495,7 @@ async fn test_search_track() {
         .await
         .search(
             query,
-            SearchType::Track,
+            &SearchType::Track,
             Some(&Market::Country(Country::UnitedStates)),
             None,
             Some(10),
@@ -526,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();
 }
@@ -675,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;
@@ -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();
 }