Make FullTrack.id optional

.github/workflows/ci.yml

@@ -82,7 +82,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: clippy
-          args: -p rspotify -p rspotify-http -p rspotify-model -p rspotify-macros --no-default-features --features=${{ matrix.features }} -- -D warnings
+          args: -p rspotify -p rspotify-http -p rspotify-model -p rspotify-macros --no-default-features --features=${{ matrix.features }} --all-targets -- -D warnings
 
       - name: Run cargo test
         uses: actions-rs/cargo@v1

CHANGELOG.md

@@ -1,7 +1,8 @@
-## 0.11.3
+## 0.11.3 (unreleased)
 
 **Breaking changes:**
 - ([#286](https://github.com/ramsayleung/rspotify/pull/286)) Make `available_markets` optional for `FullAlbum`
+- ([#282](https://github.com/ramsayleung/rspotify/pull/282)) Make `id` optional for `FullTrack`
 
 ## 0.11 (2021.10.14)
 

examples/with_auto_reauth.rs

@@ -51,17 +51,15 @@ async fn client_creds_do_things(spotify: &ClientCredsSpotify) {
 async fn expire_token<S: BaseClient>(spotify: &S) {
     let token_mutex = spotify.get_token();
     let mut token = token_mutex.lock().await.unwrap();
-    assert!(token.is_some());
-    token.as_mut().map(|x| {
-        // In a regular case, the token would expire with time. Here we just do
-        // it manually.
-        let now = Utc::now().checked_sub_signed(Duration::seconds(10));
-        x.expires_at = now;
-        x.expires_in = Duration::seconds(0);
-        // We also use a garbage access token to make sure it's actually
-        // refreshed.
-        x.access_token = "garbage".to_owned();
-    });
+    let mut token = token.as_mut().expect("Token can't be empty as this point");
+    // In a regular case, the token would expire with time. Here we just do
+    // it manually.
+    let now = Utc::now().checked_sub_signed(Duration::seconds(10));
+    token.expires_at = now;
+    token.expires_in = Duration::seconds(0);
+    // We also use a garbage access token to make sure it's actually
+    // refreshed.
+    token.access_token = "garbage".to_owned();
 }
 
 async fn with_auth(creds: Credentials, oauth: OAuth, config: Config) {
@@ -112,8 +110,10 @@ async fn main() {
     env_logger::init();
 
     // Enabling automatic token refreshing in the config
-    let mut config = Config::default();
-    config.token_refreshing = true;
+    let config = Config {
+        token_refreshing: true,
+        ..Default::default()
+    };
 
     // The default credentials from the `.env` file will be used by default.
     let creds = Credentials::from_env().unwrap();

rspotify-macros/src/lib.rs

@@ -150,7 +150,7 @@ mod test {
         let artist = Some("The Wombats");
         let market: Option<i32> = None;
 
-        let market_str = market.clone().map(|x| x.to_string());
+        let market_str = market.map(|x| x.to_string());
         let with_macro = build_map! {
             // Mandatory (not an `Option<T>`)
             "id": id,

rspotify-model/src/device.rs

@@ -39,6 +39,6 @@ fn test_devices() {
             } ]
         }
 "#;
-    let payload: DevicePayload = serde_json::from_str(&json_str).unwrap();
+    let payload: DevicePayload = serde_json::from_str(json_str).unwrap();
     assert_eq!(payload.devices[0]._type, DeviceType::Computer)
 }

rspotify-model/src/lib.rs

@@ -54,11 +54,14 @@ pub enum PlayableItem {
 }
 
 impl PlayableItem {
-    /// Utility to get the ID from either variant in the enum
-    pub fn id(&self) -> &dyn PlayableId {
+    /// Utility to get the ID from either variant in the enum.
+    ///
+    /// Note that if it's a track and if it's local, it may not have an ID, in
+    /// which case this function will return `None`.
+    pub fn id(&self) -> Option<&dyn PlayableId> {
         match self {
-            PlayableItem::Track(t) => &t.id,
-            PlayableItem::Episode(e) => &e.id,
+            PlayableItem::Track(t) => t.id.as_ref().map(|t| t as &dyn PlayableId),
+            PlayableItem::Episode(e) => Some(&e.id),
         }
     }
 }

rspotify-model/src/track.rs

@@ -25,7 +25,8 @@ pub struct FullTrack {
     pub external_ids: HashMap<String, String>,
     pub external_urls: HashMap<String, String>,
     pub href: Option<String>,
-    pub id: TrackId,
+    /// Note that a track may not have an ID/URI if it's local
+    pub id: Option<TrackId>,
     pub is_local: bool,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub is_playable: Option<bool>,

tests/test_models.rs

@@ -34,7 +34,7 @@ fn test_simplified_track() {
   }
 
 "#;
-    let track: SimplifiedTrack = serde_json::from_str(&json_str).unwrap();
+    let track: SimplifiedTrack = serde_json::from_str(json_str).unwrap();
     let duration = Duration::from_millis(276773);
     assert_eq!(track.duration, duration);
 }
@@ -64,7 +64,7 @@ fn test_public_user() {
             "uri": "spotify:user:wizzler_with_underscores"
         }
         "#;
-    let user: PublicUser = serde_json::from_str(&json_str).unwrap();
+    let user: PublicUser = serde_json::from_str(json_str).unwrap();
     // This also makes sure user IDs can have other characters, such as
     // underscores.
     assert_eq!(
@@ -99,7 +99,7 @@ fn test_private_user() {
             "uri": "spotify:user:waq5aexykhm6nlv0cnwdieng0"
           } 
         "#;
-    let private_user: PrivateUser = serde_json::from_str(&json_str).unwrap();
+    let private_user: PrivateUser = serde_json::from_str(json_str).unwrap();
     assert_eq!(private_user.country.unwrap(), Country::UnitedStates);
 }
 
@@ -132,7 +132,7 @@ fn test_full_artist() {
             "uri": "spotify:artist:0OdUWJ0sBjDrqHygGUXeCF"
         }
         "#;
-    let full_artist: FullArtist = serde_json::from_str(&json_str).unwrap();
+    let full_artist: FullArtist = serde_json::from_str(json_str).unwrap();
     assert_eq!(full_artist.name, "Band of Horses");
     assert_eq!(full_artist.followers.total, 833247);
 }
@@ -174,7 +174,7 @@ fn test_simplified_episode() {
             "uri": "spotify:episode:3brfPv3PaUhspkm1T9ZVl8"
         }
         "#;
-    let simplified_episode: SimplifiedEpisode = serde_json::from_str(&json_str).unwrap();
+    let simplified_episode: SimplifiedEpisode = serde_json::from_str(json_str).unwrap();
     assert_eq!(
         simplified_episode.release_date_precision,
         DatePrecision::Day
@@ -245,7 +245,7 @@ fn test_full_episode() {
         "uri": "spotify:episode:512ojhOuo1ktJprKbVcKyQ"
     }
         "#;
-    let full_episode: FullEpisode = serde_json::from_str(&json_str).unwrap();
+    let full_episode: FullEpisode = serde_json::from_str(json_str).unwrap();
     assert_eq!(full_episode.release_date_precision, DatePrecision::Day);
     let duration = Duration::from_millis(1502795);
     assert_eq!(full_episode.duration, duration);
@@ -260,7 +260,7 @@ fn test_copyright() {
 	} ]
 
 "#;
-    let copyrights: Vec<Copyright> = serde_json::from_str(&json_str).unwrap();
+    let copyrights: Vec<Copyright> = serde_json::from_str(json_str).unwrap();
     assert_eq!(copyrights[0]._type, CopyrightType::Performance);
 }
 
@@ -282,8 +282,9 @@ fn test_audio_analysis_section() {
             "time_signature_confidence": 1
         }
         "#;
-    let session: AudioAnalysisSection = serde_json::from_str(&json_str).unwrap();
-    assert_eq!(session.time_interval.duration, 18.32542);
+    let session: AudioAnalysisSection = serde_json::from_str(json_str).unwrap();
+    // Comparison of floating point numbers
+    assert!((session.time_interval.duration - 18.32542).abs() < f32::EPSILON);
 }
 
 #[test]
@@ -305,8 +306,9 @@ fn test_audio_analysis_segments() {
             ]
             }
             "#;
-    let segment: AudioAnalysisSegment = serde_json::from_str(&json_str).unwrap();
-    assert_eq!(segment.time_interval.start, 252.15601);
+    let segment: AudioAnalysisSegment = serde_json::from_str(json_str).unwrap();
+    // Comparison of floating point numbers
+    assert!((segment.time_interval.start - 252.156).abs() < f32::EPSILON);
 }
 
 #[test]
@@ -318,7 +320,7 @@ fn test_actions() {
             }
         }
         "#;
-    let actions: Actions = serde_json::from_str(&json_str).unwrap();
+    let actions: Actions = serde_json::from_str(json_str).unwrap();
     assert_eq!(actions.disallows[0], DisallowKey::Resuming);
 }
 
@@ -334,7 +336,7 @@ fn test_recommendations_seed() {
             "type": "ARTIST"
         }        
         "#;
-    let seed: RecommendationsSeed = serde_json::from_str(&json_str).unwrap();
+    let seed: RecommendationsSeed = serde_json::from_str(json_str).unwrap();
     assert_eq!(seed._type, RecommendationsSeedType::Artist);
 }
 
@@ -578,7 +580,7 @@ fn test_full_track() {
   "uri": "spotify:track:11dFghVXANMlKmJXsNCbNl"
 }
     "#;
-    let full_track: FullTrack = serde_json::from_str(&json).unwrap();
+    let full_track: FullTrack = serde_json::from_str(json).unwrap();
     let duration = Duration::from_millis(207959);
     assert_eq!(full_track.duration, duration);
 }
@@ -591,7 +593,7 @@ fn test_resume_point() {
         "resume_position_ms": 423432
     }   
     "#;
-    let resume_point: ResumePoint = serde_json::from_str(&json).unwrap();
+    let resume_point: ResumePoint = serde_json::from_str(json).unwrap();
     let duration = Duration::from_millis(423432);
     assert_eq!(resume_point.resume_position, duration);
 }
@@ -604,7 +606,7 @@ fn test_resume_point_negative() {
         "resume_position_ms": -1000
     }
     "#;
-    let resume_point: ResumePoint = serde_json::from_str(&json).unwrap();
+    let resume_point: ResumePoint = serde_json::from_str(json).unwrap();
     let duration = Duration::default();
     assert_eq!(resume_point.resume_position, duration);
 }
@@ -709,7 +711,7 @@ fn test_currently_playing_context() {
   "is_playing": true
 }
     "#;
-    let currently_playing_context: CurrentlyPlayingContext = serde_json::from_str(&json).unwrap();
+    let currently_playing_context: CurrentlyPlayingContext = serde_json::from_str(json).unwrap();
     let timestamp = 1607769168429;
     let second: i64 = (timestamp - timestamp % 1000) / 1000;
     let nanosecond = (timestamp % 1000) * 1000000;
@@ -824,7 +826,7 @@ fn test_current_playback_context() {
   "is_playing": true
 }
     "#;
-    let current_playback_context: CurrentPlaybackContext = serde_json::from_str(&json).unwrap();
+    let current_playback_context: CurrentPlaybackContext = serde_json::from_str(json).unwrap();
     let timestamp = 1607774342714;
     let second: i64 = (timestamp - timestamp % 1000) / 1000;
     let nanosecond = (timestamp % 1000) * 1000000;
@@ -868,7 +870,7 @@ fn test_audio_analysis_track() {
     "rhythm_version": 1
   }
   "#;
-    let audio_analysis_track: AudioAnalysisTrack = serde_json::from_str(&json).unwrap();
+    let audio_analysis_track: AudioAnalysisTrack = serde_json::from_str(json).unwrap();
     assert_eq!(audio_analysis_track.mode, Modality::Minor);
 }
 
@@ -912,7 +914,7 @@ fn test_simplified_playlist() {
     "uri": "spotify:playlist:37i9dQZF1DX8mBRYewE6or"
   } 
   "#;
-    let simplified_playlist: SimplifiedPlaylist = serde_json::from_str(&json).unwrap();
+    let simplified_playlist: SimplifiedPlaylist = serde_json::from_str(json).unwrap();
     assert_eq!(
         simplified_playlist.tracks.href,
         "https://api.spotify.com/v1/playlists/37i9dQZF1DX8mBRYewE6or/tracks"

tests/test_oauth2.rs

@@ -51,7 +51,7 @@ async fn test_read_token_cache() {
         cache_path: PathBuf::from(".test_read_token_cache.json"),
         ..Default::default()
     };
-    let mut predefined_spotify = ClientCredsSpotify::from_token(tok.clone());
+    let mut predefined_spotify = ClientCredsSpotify::from_token(tok);
     predefined_spotify.config = config.clone();
 
     // write token data to cache_path

tests/test_with_oauth.rs

@@ -96,7 +96,7 @@ pub async fn oauth_client() -> AuthCodeSpotify {
 }
 
 #[maybe_async]
-async fn fetch_all<'a, T>(paginator: Paginator<'a, ClientResult<T>>) -> Vec<T> {
+async fn fetch_all<T>(paginator: Paginator<'_, ClientResult<T>>) -> Vec<T> {
     #[cfg(feature = "__async")]
     {
         use futures::stream::TryStreamExt;
@@ -253,7 +253,7 @@ async fn test_current_user_saved_tracks_add() {
     let all = fetch_all(client.current_user_saved_tracks(None)).await;
     let all = all
         .into_iter()
-        .filter_map(|saved| Some(saved.track.id))
+        .filter_map(|saved| saved.track.id)
         .collect::<Vec<_>>();
     // All the initial tracks should appear
     assert!(tracks_ids.iter().all(|track| all.contains(track)));
@@ -348,53 +348,50 @@ async fn test_playback() {
 
         // Starting playback of some songs
         client
-            .start_uris_playback(
-                uris.clone(),
-                Some(&device_id),
-                Some(Offset::for_position(0)),
-                None,
-            )
+            .start_uris_playback(uris, Some(device_id), Some(Offset::for_position(0)), None)
             .await
             .unwrap();
 
         for i in 0..uris.len() - 1 {
-            client.next_track(Some(&device_id)).await.unwrap();
+            client.next_track(Some(device_id)).await.unwrap();
 
             // Also trying to go to the previous track
             if i != 0 {
-                client.previous_track(Some(&device_id)).await.unwrap();
-                client.next_track(Some(&device_id)).await.unwrap();
+                client.previous_track(Some(device_id)).await.unwrap();
+                client.next_track(Some(device_id)).await.unwrap();
             }
 
             // Making sure pause/resume also works
             let playback = client.current_playback(None, None::<&[_]>).await.unwrap();
             if let Some(playback) = playback {
                 if playback.is_playing {
-                    client.pause_playback(Some(&device_id)).await.unwrap();
+                    client.pause_playback(Some(device_id)).await.unwrap();
                     client.resume_playback(None, None).await.unwrap();
                 } else {
                     client.resume_playback(None, None).await.unwrap();
-                    client.pause_playback(Some(&device_id)).await.unwrap();
+                    client.pause_playback(Some(device_id)).await.unwrap();
                 }
             }
         }
 
         client
-            .transfer_playback(&next_device_id, Some(true))
+            .transfer_playback(next_device_id, Some(true))
             .await
             .unwrap();
     }
 
     // Restore the original playback data
     if let Some(backup) = &backup {
         let uri = backup.item.as_ref().map(|item| item.id());
-        let offset = None;
-        let device = backup.device.id.as_deref();
-        let position = backup.progress.map(|p| p.as_millis() as u32);
-        client
-            .start_uris_playback(uri, device, offset, position)
-            .await
-            .unwrap();
+        if let Some(uri) = uri {
+            let offset = None;
+            let device = backup.device.id.as_deref();
+            let position = backup.progress.map(|p| p.as_millis() as u32);
+            client
+                .start_uris_playback(uri, device, offset, position)
+                .await
+                .unwrap();
+        }
     }
     // Pause the playback by default, unless it was playing before
     if !backup.map(|b| b.is_playing).unwrap_or(false) {