Merge pull request #375 from ramsayleung/more-chrono-duration

ramsayleung · Jan 3, 2023 · 55255bb · 55255bb
2 parents b6e453e + a77a7bc
commit 55255bb
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.11.7 (Unreleased)
+
+- ([#375](https://github.com/ramsayleung/rspotify/pull/375)) We now use `chrono::Duration` in more places for consistency and usability: `start_uris_playback`, `start_context_playback`, `rspotify_model::Offset`, `resume_playback`, `seek_track`. Some of these fields have been renamed from `position_ms` to `position`.
+
 ## 0.11.6 (2022.12.14)
 
 - ([#331](https://github.com/ramsayleung/rspotify/pull/331)) `Market` is now `Copy`

diff --git a/examples/ureq/seek_track.rs b/examples/ureq/seek_track.rs
@@ -16,7 +16,7 @@ fn main() {
     // This function requires the `cli` feature enabled.
     spotify.prompt_for_token(&url).unwrap();
 
-    match spotify.seek_track(25000, None) {
+    match spotify.seek_track(chrono::Duration::seconds(25), None) {
         Ok(_) => println!("Change to previous playback successful"),
         Err(_) => eprintln!("Change to previous playback failed"),
     }

diff --git a/rspotify-model/src/offset.rs b/rspotify-model/src/offset.rs
@@ -1,8 +1,10 @@
 //! Offset object
 
+use chrono::Duration;
+
 /// Offset object
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum Offset {
-    Position(u32),
+    Position(Duration),
     Uri(String),
 }
diff --git a/src/clients/oauth.rs b/src/clients/oauth.rs
@@ -11,7 +11,7 @@ use crate::{
     ClientResult, OAuth, Token,
 };
 
-use std::{collections::HashMap, time};
+use std::collections::HashMap;
 
 use maybe_async::maybe_async;
 use rspotify_model::idtypes::{PlayContextId, PlayableId};
@@ -301,12 +301,12 @@ pub trait OAuthClient: BaseClient {
         &self,
         playlist_id: PlaylistId<'_>,
         items: impl IntoIterator<Item = PlayableId<'a>> + Send + 'a,
-        position: Option<i32>,
+        position: Option<chrono::Duration>,
     ) -> ClientResult<PlaylistResult> {
         let uris = items.into_iter().map(|id| id.uri()).collect::<Vec<_>>();
         let params = JsonBuilder::new()
             .required("uris", uris)
-            .optional("position", position)
+            .optional("position", position.map(|p| p.num_milliseconds()))
             .build();
 
         let url = format!("playlists/{}/tracks", playlist_id.id());
@@ -752,8 +752,8 @@ pub trait OAuthClient: BaseClient {
     ///
     /// Parameters:
     /// - limit - the number of entities to return
-    /// - time_limit - a Unix timestamp in milliseconds. Returns all items after
-    /// or before (but not including) this cursor position.
+    /// - time_limit - a timestamp. The endpoint will return all items after
+    ///   or before (but not including) this cursor position.
     ///
     /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#/operations/get-recently-played)
     async fn current_user_recently_played(
@@ -1029,26 +1029,28 @@ pub trait OAuthClient: BaseClient {
     /// - context_uri - spotify context uri to play
     /// - uris - spotify track uris
     /// - offset - offset into context by index or track
-    /// - position_ms - Indicates from what position to start playback.
+    /// - position - Indicates from what position to start playback.
     ///
     /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#/operations/start-a-users-playback)
     async fn start_context_playback(
         &self,
         context_uri: PlayContextId<'_>,
         device_id: Option<&str>,
         offset: Option<Offset>,
-        position_ms: Option<time::Duration>,
+        position: Option<chrono::Duration>,
     ) -> ClientResult<()> {
         let params = JsonBuilder::new()
             .required("context_uri", context_uri.uri())
             .optional(
                 "offset",
                 offset.map(|x| match x {
-                    Offset::Position(position) => json!({ "position": position }),
+                    Offset::Position(position) => {
+                        json!({ "position": position.num_milliseconds() })
+                    }
                     Offset::Uri(uri) => json!({ "uri": uri }),
                 }),
             )
-            .optional("position_ms", position_ms)
+            .optional("position_ms", position.map(|p| p.num_milliseconds()))
             .build();
 
         let url = append_device_id("me/player/play", device_id);
@@ -1063,26 +1065,28 @@ pub trait OAuthClient: BaseClient {
     /// - uris
     /// - device_id
     /// - offset
-    /// - position_ms
+    /// - position
     ///
     /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#/operations/start-a-users-playback)
     async fn start_uris_playback<'a>(
         &self,
         uris: impl IntoIterator<Item = PlayableId<'a>> + Send + 'a,
         device_id: Option<&str>,
         offset: Option<crate::model::Offset>,
-        position_ms: Option<u32>,
+        position: Option<chrono::Duration>,
     ) -> ClientResult<()> {
         let params = JsonBuilder::new()
             .required(
                 "uris",
                 uris.into_iter().map(|id| id.uri()).collect::<Vec<_>>(),
             )
-            .optional("position_ms", position_ms)
+            .optional("position_ms", position.map(|p| p.num_milliseconds()))
             .optional(
                 "offset",
                 offset.map(|x| match x {
-                    Offset::Position(position) => json!({ "position": position }),
+                    Offset::Position(position) => {
+                        json!({ "position": position.num_milliseconds() })
+                    }
                     Offset::Uri(uri) => json!({ "uri": uri }),
                 }),
             )
@@ -1111,16 +1115,16 @@ pub trait OAuthClient: BaseClient {
     ///
     /// Parameters:
     /// - device_id - device target for playback
-    /// - position_ms
+    /// - position
     ///
     /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#/operations/start-a-users-playback)
     async fn resume_playback(
         &self,
         device_id: Option<&str>,
-        position_ms: Option<u32>,
+        position: Option<chrono::Duration>,
     ) -> ClientResult<()> {
         let params = JsonBuilder::new()
-            .optional("position_ms", position_ms)
+            .optional("position_ms", position.map(|p| p.num_milliseconds()))
             .build();
 
         let url = append_device_id("me/player/play", device_id);
@@ -1158,13 +1162,17 @@ pub trait OAuthClient: BaseClient {
     /// Seek To Position In Currently Playing Track.
     ///
     /// Parameters:
-    /// - position_ms - position in milliseconds to seek to
+    /// - position - position in milliseconds to seek to
     /// - device_id - device target for playback
     ///
     /// [Reference](https://developer.spotify.com/documentation/web-api/reference/#/operations/seek-to-position-in-currently-playing-track)
-    async fn seek_track(&self, position_ms: u32, device_id: Option<&str>) -> ClientResult<()> {
+    async fn seek_track(
+        &self,
+        position: chrono::Duration,
+        device_id: Option<&str>,
+    ) -> ClientResult<()> {
         let url = append_device_id(
-            &format!("me/player/seek?position_ms={position_ms}"),
+            &format!("me/player/seek?position_ms={}", position.num_milliseconds()),
             device_id,
         );
         self.endpoint_put(&url, &json!({})).await?;

diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs
@@ -349,7 +349,7 @@ async fn test_playback() {
             .start_uris_playback(
                 uris.iter().map(PlayableId::as_ref),
                 Some(device_id),
-                Some(Offset::Position(0)),
+                Some(Offset::Position(chrono::Duration::zero())),
                 None,
             )
             .await
@@ -389,7 +389,7 @@ async fn test_playback() {
         if let Some(uri) = uri {
             let offset = None;
             let device = backup.device.id.as_deref();
-            let position = backup.progress.map(|p| p.num_milliseconds() as u32);
+            let position = backup.progress;
             client
                 .start_uris_playback(uri, device, offset, position)
                 .await
@@ -527,17 +527,17 @@ async fn test_seek_track() {
     // Saving the previous state to restore it later
     let backup = client.current_playback(None, None::<&[_]>).await.unwrap();
 
-    client.seek_track(25000, None).await.unwrap();
+    client
+        .seek_track(chrono::Duration::seconds(25), None)
+        .await
+        .unwrap();
 
     if let Some(CurrentPlaybackContext {
         progress: Some(progress),
         ..
     }) = backup
     {
-        client
-            .seek_track(progress.num_milliseconds() as u32, None)
-            .await
-            .unwrap();
+        client.seek_track(progress, None).await.unwrap();
     }
 }