From b3391534baa91a018f6ffe63ad4daef09d10b40f Mon Sep 17 00:00:00 2001 From: Ramsay Leung Date: Thu, 7 Mar 2024 22:22:52 -0800 Subject: [PATCH 1/4] Revert the patch for Spotify API Bug after Spotify fixed the problem. --- rspotify-model/src/artist.rs | 4 +--- rspotify-model/src/data_type_patcher.rs | 28 ------------------------- rspotify-model/src/image.rs | 6 ------ rspotify-model/src/lib.rs | 5 +---- 4 files changed, 2 insertions(+), 41 deletions(-) delete mode 100644 rspotify-model/src/data_type_patcher.rs diff --git a/rspotify-model/src/artist.rs b/rspotify-model/src/artist.rs index 5eb59bcc..2906913e 100644 --- a/rspotify-model/src/artist.rs +++ b/rspotify-model/src/artist.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; -use crate::{data_type_patcher::as_u32, ArtistId, CursorBasedPage, Followers, Image}; +use crate::{ArtistId, CursorBasedPage, Followers, Image}; /// Simplified Artist Object #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)] @@ -25,8 +25,6 @@ pub struct FullArtist { pub id: ArtistId<'static>, pub images: Vec, pub name: String, - // TODO: remove this statement after Spotify fix the [issue](https://github.com/ramsayleung/rspotify/issues/452) - #[serde(deserialize_with = "as_u32")] pub popularity: u32, } diff --git a/rspotify-model/src/data_type_patcher.rs b/rspotify-model/src/data_type_patcher.rs deleted file mode 100644 index ff60f5e8..00000000 --- a/rspotify-model/src/data_type_patcher.rs +++ /dev/null @@ -1,28 +0,0 @@ -// Workaround for Spotify API bug which causes dome uint fields -// being return as floats -// TODO: remove this workaround after Spotify fix the [issue](https://github.com/ramsayleung/rspotify/issues/452) - -use serde::{Deserialize, Deserializer}; - -pub fn as_u32<'de, D>(deserializer: D) -> Result -where - D: Deserializer<'de>, -{ - let float_data: f64 = Deserialize::deserialize(deserializer)?; - - let u32_data = float_data as u32; - - Ok(u32_data) -} - -pub fn as_some_u32<'de, D>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - let float_data: Option = Deserialize::deserialize(deserializer)?; - - match float_data { - Some(f) => Ok(Some(f as u32)), - None => Ok(None), - } -} diff --git a/rspotify-model/src/image.rs b/rspotify-model/src/image.rs index 46dcf158..90c743e4 100644 --- a/rspotify-model/src/image.rs +++ b/rspotify-model/src/image.rs @@ -1,17 +1,11 @@ //! Image object -pub use crate::data_type_patcher::as_some_u32; - use serde::{Deserialize, Serialize}; /// Image object #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)] pub struct Image { - // TODO: remove this statement after Spotify fix the [issue](https://github.com/ramsayleung/rspotify/issues/452) - #[serde(deserialize_with = "as_some_u32")] pub height: Option, pub url: String, - // TODO: remove this statement after Spotify fix the [issue](https://github.com/ramsayleung/rspotify/issues/452) - #[serde(deserialize_with = "as_some_u32")] pub width: Option, } diff --git a/rspotify-model/src/lib.rs b/rspotify-model/src/lib.rs index 3979fb64..afa0466f 100644 --- a/rspotify-model/src/lib.rs +++ b/rspotify-model/src/lib.rs @@ -8,7 +8,6 @@ pub mod auth; pub mod category; pub mod context; pub(crate) mod custom_serde; -pub mod data_type_patcher; pub mod device; pub mod enums; pub mod error; @@ -25,7 +24,7 @@ pub mod track; pub mod user; pub use { - album::*, artist::*, audio::*, auth::*, category::*, context::*, data_type_patcher::as_u32, + album::*, artist::*, audio::*, auth::*, category::*, context::*, device::*, enums::*, error::*, idtypes::*, image::*, offset::*, page::*, playing::*, playlist::*, recommend::*, search::*, show::*, track::*, user::*, }; @@ -37,8 +36,6 @@ use serde::{Deserialize, Serialize}; pub struct Followers { // This field will always set to null, as the Web API does not support it at the moment. // pub href: Option, - // TODO: remove this statement after Spotify fix the [issue](https://github.com/ramsayleung/rspotify/issues/452) - #[serde(deserialize_with = "as_u32")] pub total: u32, } From 75ceb28c54aa9d29d4ce06cd91cc96f3b8ddcbde Mon Sep 17 00:00:00 2001 From: Ramsay Leung Date: Fri, 8 Mar 2024 00:22:51 -0800 Subject: [PATCH 2/4] Migrate the deprecated API in chrono to the recommended ones. --- Cargo.toml | 4 +- examples/with_auto_reauth.rs | 4 +- rspotify-model/src/auth.rs | 6 +-- rspotify-model/src/context.rs | 9 ++-- rspotify-model/src/custom_serde.rs | 79 +++++++++--------------------- rspotify-model/src/lib.rs | 6 +-- src/clients/mod.rs | 2 +- tests/test_models.rs | 28 +++++------ tests/test_oauth2.rs | 18 ++++--- tests/test_with_oauth.rs | 4 +- 10 files changed, 64 insertions(+), 96 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5ad68964..7d0ba017 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ authors = [ "Mario Ortiz Manero " ] name = "rspotify" -version = "0.12.0" +version = "0.13.0" license = "MIT" readme = "README.md" description = "Spotify API wrapper" @@ -33,7 +33,7 @@ rspotify-http = { path = "rspotify-http", version = "0.12.0", default-features = async-stream = { version = "0.3.2", optional = true } async-trait = { version = "0.1.51", optional = true } base64 = "0.22.0" -chrono = { version = "0.4.19", features = ["serde"] } +chrono = { version = "0.4.35", features = ["serde"] } dotenvy = { version = "0.15.0", optional = true } futures = { version = "0.3.17", optional = true } diff --git a/examples/with_auto_reauth.rs b/examples/with_auto_reauth.rs index 0e588752..6a5e3694 100644 --- a/examples/with_auto_reauth.rs +++ b/examples/with_auto_reauth.rs @@ -55,9 +55,9 @@ async fn expire_token(spotify: &S) { let 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)); + let now = Utc::now().checked_sub_signed(Duration::try_seconds(10).unwrap()); token.expires_at = now; - token.expires_in = Duration::seconds(0); + token.expires_in = Duration::try_seconds(0).unwrap(); // We also use a garbage access token to make sure it's actually // refreshed. token.access_token = "garbage".to_owned(); diff --git a/rspotify-model/src/auth.rs b/rspotify-model/src/auth.rs index 5a4a10f7..e48cdfd4 100644 --- a/rspotify-model/src/auth.rs +++ b/rspotify-model/src/auth.rs @@ -12,7 +12,7 @@ use std::{ path::Path, }; -use chrono::{DateTime, Duration, Utc}; +use chrono::{DateTime, Duration, TimeDelta, Utc}; use serde::{Deserialize, Serialize}; /// Spotify access token information @@ -46,7 +46,7 @@ impl Default for Token { fn default() -> Self { Self { access_token: String::new(), - expires_in: Duration::seconds(0), + expires_in: Duration::try_seconds(0).unwrap(), expires_at: Some(Utc::now()), refresh_token: None, scopes: HashSet::new(), @@ -81,7 +81,7 @@ impl Token { #[must_use] pub fn is_expired(&self) -> bool { self.expires_at.map_or(true, |expiration| { - Utc::now() + Duration::seconds(10) >= expiration + Utc::now() + TimeDelta::try_seconds(10).unwrap() >= expiration }) } diff --git a/rspotify-model/src/context.rs b/rspotify-model/src/context.rs index e0f9f869..ffb6a68e 100644 --- a/rspotify-model/src/context.rs +++ b/rspotify-model/src/context.rs @@ -1,13 +1,14 @@ //! All objects related to context +use chrono::serde::ts_milliseconds; use chrono::{DateTime, Duration, Utc}; use serde::{Deserialize, Deserializer, Serialize}; use std::collections::HashMap; use crate::{ - custom_serde::{millisecond_timestamp, option_duration_ms}, - CurrentlyPlayingType, Device, DisallowKey, PlayableItem, RepeatState, Type, + custom_serde::option_duration_ms, CurrentlyPlayingType, Device, DisallowKey, PlayableItem, + RepeatState, Type, }; /// Context object @@ -25,7 +26,7 @@ pub struct Context { #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct CurrentlyPlayingContext { pub context: Option, - #[serde(with = "millisecond_timestamp")] + #[serde(with = "ts_milliseconds")] pub timestamp: DateTime, #[serde(default)] #[serde(with = "option_duration_ms", rename = "progress_ms")] @@ -42,7 +43,7 @@ pub struct CurrentPlaybackContext { pub repeat_state: RepeatState, pub shuffle_state: bool, pub context: Option, - #[serde(with = "millisecond_timestamp")] + #[serde(with = "ts_milliseconds")] pub timestamp: DateTime, #[serde(default)] #[serde(with = "option_duration_ms", rename = "progress_ms")] diff --git a/rspotify-model/src/custom_serde.rs b/rspotify-model/src/custom_serde.rs index bafaa5e6..1a34ed15 100644 --- a/rspotify-model/src/custom_serde.rs +++ b/rspotify-model/src/custom_serde.rs @@ -18,7 +18,12 @@ pub mod duration_ms { where E: de::Error, { - Ok(Duration::milliseconds(v)) + Duration::try_milliseconds(v).ok_or_else(|| { + E::invalid_value( + serde::de::Unexpected::Signed(v), + &"an invalid duration in milliseconds", + ) + }) } // JSON deserializer calls visit_u64 for non-negative intgers @@ -26,9 +31,19 @@ pub mod duration_ms { where E: de::Error, { - i64::try_from(v) - .map(Duration::milliseconds) - .map_err(E::custom) + match i64::try_from(v) { + Ok(val) => Duration::try_milliseconds(val).ok_or_else(|| { + E::invalid_value( + serde::de::Unexpected::Signed(val), + &"a valid duration in + milliseconds", + ) + }), + Err(_) => Err(E::custom(format!( + "Conversion error: u64 to i64 conversion failed for value {}", + v + ))), + } } } @@ -49,57 +64,6 @@ pub mod duration_ms { } } -pub mod millisecond_timestamp { - use chrono::{DateTime, NaiveDateTime, Utc}; - use serde::{de, Serializer}; - use std::fmt; - - /// Vistor to help deserialize unix millisecond timestamp to - /// `chrono::DateTime`. - struct DateTimeVisitor; - - impl<'de> de::Visitor<'de> for DateTimeVisitor { - type Value = DateTime; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!( - formatter, - "an unix millisecond timestamp represents DataTime" - ) - } - - fn visit_u64(self, v: u64) -> Result - where - E: de::Error, - { - let second = (v - v % 1000) / 1000; - let nanosecond = ((v % 1000) * 1_000_000) as u32; - // The maximum value of i64 is large enough to hold milliseconds, - // so it would be safe to convert it i64. - match NaiveDateTime::from_timestamp_opt(second as i64, nanosecond) { - Some(ndt) => Ok(DateTime::::from_naive_utc_and_offset(ndt, Utc)), - None => Err(E::custom(format!("v is invalid second: {v}"))), - } - } - } - - /// Deserialize Unix millisecond timestamp to `DateTime` - pub fn deserialize<'de, D>(d: D) -> Result, D::Error> - where - D: de::Deserializer<'de>, - { - d.deserialize_u64(DateTimeVisitor) - } - - /// Serialize `DateTime` to Unix millisecond timestamp - pub fn serialize(x: &DateTime, s: S) -> Result - where - S: Serializer, - { - s.serialize_i64(x.timestamp_millis()) - } -} - pub mod option_duration_ms { use crate::custom_serde::duration_ms; use chrono::Duration; @@ -202,7 +166,10 @@ pub mod duration_second { D: de::Deserializer<'de>, { let duration: i64 = Deserialize::deserialize(d)?; - Ok(Duration::seconds(duration)) + Duration::try_seconds(duration).ok_or(serde::de::Error::invalid_value( + serde::de::Unexpected::Signed(duration), + &"an invalid duration in seconds", + )) } /// Serialize `chrono::Duration` to seconds (represented as u64) diff --git a/rspotify-model/src/lib.rs b/rspotify-model/src/lib.rs index afa0466f..11b6de7b 100644 --- a/rspotify-model/src/lib.rs +++ b/rspotify-model/src/lib.rs @@ -24,9 +24,9 @@ pub mod track; pub mod user; pub use { - album::*, artist::*, audio::*, auth::*, category::*, context::*, - device::*, enums::*, error::*, idtypes::*, image::*, offset::*, page::*, playing::*, - playlist::*, recommend::*, search::*, show::*, track::*, user::*, + album::*, artist::*, audio::*, auth::*, category::*, context::*, device::*, enums::*, error::*, + idtypes::*, image::*, offset::*, page::*, playing::*, playlist::*, recommend::*, search::*, + show::*, track::*, user::*, }; use serde::{Deserialize, Serialize}; diff --git a/src/clients/mod.rs b/src/clients/mod.rs index 3be8f02d..15083643 100644 --- a/src/clients/mod.rs +++ b/src/clients/mod.rs @@ -114,7 +114,7 @@ mod test { async fn test_auth_headers() { let tok = Token { access_token: "test-access_token".to_string(), - expires_in: Duration::seconds(1), + expires_in: Duration::try_seconds(1).unwrap(), expires_at: Some(Utc::now()), scopes: scopes!("playlist-read-private"), refresh_token: Some("...".to_string()), diff --git a/tests/test_models.rs b/tests/test_models.rs index c7b92325..2bb561bb 100644 --- a/tests/test_models.rs +++ b/tests/test_models.rs @@ -1,4 +1,4 @@ -use chrono::{DateTime, Duration, NaiveDateTime, Utc}; +use chrono::{DateTime, Duration}; use rspotify::model::*; use serde::de::DeserializeOwned; use wasm_bindgen_test::*; @@ -52,7 +52,7 @@ fn test_simplified_track() { "#; let track: SimplifiedTrack = deserialize(json_str); - let duration = Duration::milliseconds(276773); + let duration = Duration::try_milliseconds(276773).unwrap(); assert_eq!(track.duration, duration); } @@ -200,7 +200,7 @@ fn test_simplified_episode() { simplified_episode.release_date_precision, DatePrecision::Day ); - let duration = Duration::milliseconds(2685023); + let duration = Duration::try_milliseconds(2685023).unwrap(); assert_eq!(simplified_episode.duration, duration); } @@ -269,7 +269,7 @@ fn test_full_episode() { "#; let full_episode: FullEpisode = deserialize(json_str); assert_eq!(full_episode.release_date_precision, DatePrecision::Day); - let duration = Duration::milliseconds(1502795); + let duration = Duration::try_milliseconds(1502795).unwrap(); assert_eq!(full_episode.duration, duration); } @@ -531,7 +531,7 @@ fn test_audio_features() { } "#; let audio_features: AudioFeatures = deserialize(json); - let duration = Duration::milliseconds(255349); + let duration = Duration::try_milliseconds(255349).unwrap(); assert_eq!(audio_features.duration, duration); } @@ -611,7 +611,7 @@ fn test_full_track() { } "#; let full_track: FullTrack = deserialize(json); - let duration = Duration::milliseconds(207959); + let duration = Duration::try_milliseconds(207959).unwrap(); assert_eq!(full_track.duration, duration); } @@ -625,7 +625,7 @@ fn test_resume_point() { } "#; let resume_point: ResumePoint = deserialize(json); - let duration = Duration::milliseconds(423432); + let duration = Duration::try_milliseconds(423432).unwrap(); assert_eq!(resume_point.resume_position, duration); } @@ -639,7 +639,7 @@ fn test_resume_point_negative() { } "#; let resume_point: ResumePoint = deserialize(json); - let duration = Duration::milliseconds(-1000); + let duration = Duration::try_milliseconds(-1000).unwrap(); assert_eq!(resume_point.resume_position, duration); } @@ -748,13 +748,10 @@ fn test_currently_playing_context() { let timestamp = 1607769168429; let second: i64 = (timestamp - timestamp % 1000) / 1000; let nanosecond = (timestamp % 1000) * 1000000; - let dt = DateTime::::from_naive_utc_and_offset( - NaiveDateTime::from_timestamp_opt(second, nanosecond as u32).unwrap(), - Utc, - ); + let dt = DateTime::from_timestamp(second, nanosecond as u32).unwrap(); assert_eq!(currently_playing_context.timestamp, dt); - let duration = Duration::milliseconds(22270); + let duration = Duration::try_milliseconds(22270).unwrap(); assert_eq!(currently_playing_context.progress, Some(duration)); } @@ -864,10 +861,7 @@ fn test_current_playback_context() { let timestamp = 1607774342714; let second: i64 = (timestamp - timestamp % 1000) / 1000; let nanosecond = (timestamp % 1000) * 1000000; - let dt = DateTime::::from_naive_utc_and_offset( - NaiveDateTime::from_timestamp_opt(second, nanosecond as u32).unwrap(), - Utc, - ); + let dt = DateTime::from_timestamp(second, nanosecond as u32).unwrap(); assert_eq!(current_playback_context.timestamp, dt); assert!(current_playback_context.progress.is_none()); } diff --git a/tests/test_oauth2.rs b/tests/test_oauth2.rs index dcec1b73..0b4f2561 100644 --- a/tests/test_oauth2.rs +++ b/tests/test_oauth2.rs @@ -36,7 +36,7 @@ fn test_get_authorize_url() { #[maybe_async::test(feature = "__sync", async(feature = "__async", tokio::test))] async fn test_read_token_cache() { - let expires_in = Duration::seconds(3600); + let expires_in = Duration::try_seconds(3600).unwrap(); let expires_at = Some(Utc::now() + expires_in); let scopes = scopes!("playlist-read-private", "playlist-read-collaborative"); @@ -67,7 +67,10 @@ async fn test_read_token_cache() { let tok_from_file = spotify.read_token_cache().await.unwrap().unwrap(); assert_eq!(tok_from_file.scopes, scopes); assert_eq!(tok_from_file.refresh_token.unwrap(), "..."); - assert_eq!(tok_from_file.expires_in, Duration::seconds(3600)); + assert_eq!( + tok_from_file.expires_in, + Duration::try_seconds(3600).unwrap() + ); assert_eq!(tok_from_file.expires_at, expires_at); // delete cache file in the end @@ -81,7 +84,7 @@ async fn test_write_token() { let tok = Token { access_token: "test-access_token".to_owned(), - expires_in: Duration::seconds(3600), + expires_in: Duration::try_seconds(3600).unwrap(), expires_at: Some(now), scopes: scopes.clone(), refresh_token: Some("...".to_owned()), @@ -105,7 +108,10 @@ async fn test_write_token() { assert_eq!(tok_str, tok_str_file); let tok_from_file: Token = serde_json::from_str(&tok_str_file).unwrap(); assert_eq!(tok_from_file.scopes, scopes); - assert_eq!(tok_from_file.expires_in, Duration::seconds(3600)); + assert_eq!( + tok_from_file.expires_in, + Duration::try_seconds(3600).unwrap() + ); assert_eq!(tok_from_file.expires_at.unwrap(), now); // delete cache file in the end @@ -115,7 +121,7 @@ async fn test_write_token() { #[test] #[wasm_bindgen_test] fn test_token_is_expired() { - let expires_in = Duration::seconds(20); + let expires_in = Duration::try_seconds(20).unwrap(); let tok = Token { scopes: scopes!("playlist-read-private", "playlist-read-collaborative"), access_token: "test-access_token".to_owned(), @@ -125,7 +131,7 @@ fn test_token_is_expired() { }; assert!(!tok.is_expired()); - let expires_in = Duration::seconds(3); // There's a margin of 10 seconds + let expires_in = Duration::try_seconds(3).unwrap(); // There's a margin of 10 seconds let tok = Token { scopes: scopes!("playlist-read-private", "playlist-read-collaborative"), access_token: "test-access_token".to_owned(), diff --git a/tests/test_with_oauth.rs b/tests/test_with_oauth.rs index ac20f11b..e92f0c19 100644 --- a/tests/test_with_oauth.rs +++ b/tests/test_with_oauth.rs @@ -222,7 +222,7 @@ async fn test_current_user_playing_track() { )] #[ignore] async fn test_current_user_recently_played() { - let limit = TimeLimits::After(Utc::now() - Duration::days(2)); + let limit = TimeLimits::After(Utc::now() - Duration::try_days(2).unwrap()); oauth_client() .await .current_user_recently_played(Some(10), Some(limit)) @@ -635,7 +635,7 @@ async fn test_seek_track() { let backup = client.current_playback(None, None::<&[_]>).await.unwrap(); client - .seek_track(chrono::Duration::seconds(25), None) + .seek_track(chrono::Duration::try_seconds(25).unwrap(), None) .await .unwrap(); From cd55e43f30fc25dd90aadec5cfa408eb6b833825 Mon Sep 17 00:00:00 2001 From: Ramsay Leung Date: Fri, 8 Mar 2024 00:25:49 -0800 Subject: [PATCH 3/4] Fix cargo clippy error. --- examples/ureq/seek_track.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/ureq/seek_track.rs b/examples/ureq/seek_track.rs index b9c0eb06..24129eaf 100644 --- 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(chrono::Duration::seconds(25), None) { + match spotify.seek_track(chrono::Duration::try_seconds(25).unwrap(), None) { Ok(_) => println!("Change to previous playback successful"), Err(_) => eprintln!("Change to previous playback failed"), } From 55b263af473492286879c1eda994c1d7f3d8f2cd Mon Sep 17 00:00:00 2001 From: Ramsay Leung Date: Fri, 8 Mar 2024 00:29:10 -0800 Subject: [PATCH 4/4] Downgrade the version back to 0.12 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7d0ba017..cc86ff25 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ authors = [ "Mario Ortiz Manero " ] name = "rspotify" -version = "0.13.0" +version = "0.12.0" license = "MIT" readme = "README.md" description = "Spotify API wrapper"