Skip to content

Commit

Permalink
Merge pull request #366 from ramsayleung/ramsay_fix_negative_progress_ms
Browse files Browse the repository at this point in the history
Replace std::time::Duration with chrono::Duration to support negative duration
  • Loading branch information
marioortizmanero authored Dec 27, 2022
2 parents 48b74b4 + a49a5f9 commit 1a3a06e
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 52 deletions.
27 changes: 14 additions & 13 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 0.11.6 (2022.12.14)

- ([#331](https://github.com/ramsayleung/rspotify/pull/331)) `Market` is now `Copy`
- ([#366](https://github.com/ramsayleung/rspotify/pull/366)) Replace all `std::time::Duration` with `chrono::Duration` to support negative duration.

**Bugfixes**:
- ([#332](https://github.com/ramsayleung/rspotify/pull/332)) Fix typo in `RestrictionReason` enum values
Expand Down Expand Up @@ -117,19 +118,19 @@ Hopefully this will convince you that the new breaking changes are good; you'll

Here are a few examples of upgrades:

| Name | Old | New |
|----------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------|
| [Sync] device | [`examples/blocking/device.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/device.rs) | [`examples/ureq/device.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/device.rs) |
| [Sync] me | [`examples/blocking/me.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/me.rs) | [`examples/ureq/me.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/me.rs) |
| [Sync] search | [`examples/blocking/search_track.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/search_track.rs) | [`examples/ureq/search.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/search.rs) |
| [Sync] seek_track | [`examples/blocking/seek_track.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/seek_track.rs) | [`examples/ureq/seek_track.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/seek_track.rs) |
| [Sync] current_user_saved_tracks | [`examples/blocking/current_user_saved_tracks.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/current_user_saved_tracks.rs) | [`examples/pagination_sync.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/pagination_sync.rs) |
| [Async] current_user_saved_tracks | [`examples/current_user_saved_tracks.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_user_saved_tracks.rs) | [`examples/pagination_async.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/pagination_async.rs) |
| [Async] current_user_saved_tracks (manually) | [`examples/current_user_saved_tracks.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_user_saved_tracks.rs) | [`examples/pagination_manual.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/pagination_manual.rs) |
| [Async] current_playing | [`examples/current_playing.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_playing.rs) | [`examples/auth_code.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/auth_code.rs) |
| [Async] current_playback | [`examples/current_playback.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_playback.rs) | [`examples/auth_code_pkce.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/auth_code_pkce.rs) |
| [Async] album | [`examples/album.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/album.rs) | [`examples/client_creds.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/client_creds.rs) |
| [Async] webapp with Rocket | [`examples/webapp`](https://github.com/ramsayleung/rspotify/tree/4c1c3366630a8b2b37668a17878b746108c93fd0/examples/webapp) | [`examples/webapp`](https://github.com/ramsayleung/rspotify/tree/master/examples/webapp) |
| Name | Old | New |
| -------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- |
| [Sync] device | [`examples/blocking/device.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/device.rs) | [`examples/ureq/device.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/device.rs) |
| [Sync] me | [`examples/blocking/me.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/me.rs) | [`examples/ureq/me.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/me.rs) |
| [Sync] search | [`examples/blocking/search_track.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/search_track.rs) | [`examples/ureq/search.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/search.rs) |
| [Sync] seek_track | [`examples/blocking/seek_track.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/seek_track.rs) | [`examples/ureq/seek_track.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/ureq/seek_track.rs) |
| [Sync] current_user_saved_tracks | [`examples/blocking/current_user_saved_tracks.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/blocking/current_user_saved_tracks.rs) | [`examples/pagination_sync.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/pagination_sync.rs) |
| [Async] current_user_saved_tracks | [`examples/current_user_saved_tracks.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_user_saved_tracks.rs) | [`examples/pagination_async.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/pagination_async.rs) |
| [Async] current_user_saved_tracks (manually) | [`examples/current_user_saved_tracks.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_user_saved_tracks.rs) | [`examples/pagination_manual.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/pagination_manual.rs) |
| [Async] current_playing | [`examples/current_playing.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_playing.rs) | [`examples/auth_code.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/auth_code.rs) |
| [Async] current_playback | [`examples/current_playback.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/current_playback.rs) | [`examples/auth_code_pkce.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/auth_code_pkce.rs) |
| [Async] album | [`examples/album.rs`](https://github.com/ramsayleung/rspotify/blob/22a995a061dffbce9f5069fd603e266d7ed3a252/examples/album.rs) | [`examples/client_creds.rs`](https://github.com/ramsayleung/rspotify/blob/master/examples/client_creds.rs) |
| [Async] webapp with Rocket | [`examples/webapp`](https://github.com/ramsayleung/rspotify/tree/4c1c3366630a8b2b37668a17878b746108c93fd0/examples/webapp) | [`examples/webapp`](https://github.com/ramsayleung/rspotify/tree/master/examples/webapp) |

More in the [`examples` directory](https://github.com/ramsayleung/rspotify/tree/master/examples)!

Expand Down
2 changes: 1 addition & 1 deletion rspotify-model/src/audio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use serde::{Deserialize, Serialize};

use std::time::Duration;
use chrono::Duration;

use crate::{
custom_serde::{duration_ms, modality},
Expand Down
3 changes: 1 addition & 2 deletions rspotify-model/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
//! All objects related to context
use chrono::{DateTime, Utc};
use chrono::{DateTime, Duration, Utc};
use serde::{Deserialize, Deserializer, Serialize};

use std::collections::HashMap;
use std::time::Duration;

use crate::{
custom_serde::{millisecond_timestamp, option_duration_ms},
Expand Down
47 changes: 27 additions & 20 deletions rspotify-model/src/custom_serde.rs
Original file line number Diff line number Diff line change
@@ -1,45 +1,51 @@
//! Custom serialization methods used throughout the crate
pub mod duration_ms {
use chrono::Duration;
use serde::{de, Serializer};
use std::{fmt, time::Duration};
use std::convert::TryFrom;
use std::fmt;

/// Vistor to help deserialize duration represented as millisecond to
/// `std::time::Duration`.
/// `chrono::Duration`.
pub struct DurationVisitor;
impl<'de> de::Visitor<'de> for DurationVisitor {
type Value = Duration;
fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
write!(formatter, "a milliseconds represents std::time::Duration")
write!(formatter, "a milliseconds represents chrono::Duration")
}
fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(Duration::from_millis(v))
Ok(Duration::milliseconds(v))
}
fn visit_i64<E>(self, v: i64) -> Result<Self::Value, E>

// JSON deserializer calls visit_u64 for non-negative intgers
fn visit_u64<E>(self, v: u64) -> Result<Self::Value, E>
where
E: de::Error,
{
Ok(Duration::from_millis(v.max(0) as u64))
i64::try_from(v)
.map(Duration::milliseconds)
.map_err(E::custom)
}
}

/// Deserialize `std::time::Duration` from milliseconds (represented as u64)
/// Deserialize `chrono::Duration` from milliseconds (represented as i64)
pub fn deserialize<'de, D>(d: D) -> Result<Duration, D::Error>
where
D: de::Deserializer<'de>,
{
d.deserialize_u64(DurationVisitor)
d.deserialize_i64(DurationVisitor)
}

/// Serialize `std::time::Duration` to milliseconds (represented as u64)
/// Serialize `chrono::Duration` to milliseconds (represented as i64)
pub fn serialize<S>(x: &Duration, s: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
s.serialize_u64(x.as_millis() as u64)
s.serialize_i64(x.num_milliseconds())
}
}

Expand Down Expand Up @@ -96,11 +102,12 @@ pub mod millisecond_timestamp {

pub mod option_duration_ms {
use crate::custom_serde::duration_ms;
use chrono::Duration;
use serde::{de, Serializer};
use std::{fmt, time::Duration};
use std::fmt;

/// Vistor to help deserialize duration represented as milliseconds to
/// `Option<std::time::Duration>`
/// `Option<chrono::Duration>`
struct OptionDurationVisitor;

impl<'de> de::Visitor<'de> for OptionDurationVisitor {
Expand All @@ -109,7 +116,7 @@ pub mod option_duration_ms {
fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
write!(
formatter,
"a optional milliseconds represents std::time::Duration"
"a optional milliseconds represents chrono::Duration"
)
}

Expand All @@ -125,28 +132,28 @@ pub mod option_duration_ms {
D: de::Deserializer<'de>,
{
Ok(Some(
deserializer.deserialize_u64(duration_ms::DurationVisitor)?,
deserializer.deserialize_i64(duration_ms::DurationVisitor)?,
))
}
}

/// Deserialize `Option<std::time::Duration>` from milliseconds
/// (represented as u64)
/// Deserialize `Option<chrono::Duration>` from milliseconds
/// (represented as i64)
pub fn deserialize<'de, D>(d: D) -> Result<Option<Duration>, D::Error>
where
D: de::Deserializer<'de>,
{
d.deserialize_option(OptionDurationVisitor)
}

/// Serialize `Option<std::time::Duration>` to milliseconds (represented as
/// u64)
/// Serialize `Option<chrono::Duration>` to milliseconds (represented as
/// i64)
pub fn serialize<S>(x: &Option<Duration>, s: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
match *x {
Some(duration) => s.serialize_u64(duration.as_millis() as u64),
Some(duration) => s.serialize_i64(duration.num_milliseconds()),
None => s.serialize_none(),
}
}
Expand Down
4 changes: 2 additions & 2 deletions rspotify-model/src/show.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use serde::{Deserialize, Serialize};

use chrono::Duration;
use std::collections::HashMap;
use std::time::Duration;

use crate::{
custom_serde::duration_ms, CopyrightType, DatePrecision, EpisodeId, Image, Page, ShowId,
Expand Down Expand Up @@ -123,7 +123,7 @@ pub struct EpisodesPayload {
}

/// Resume point object
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)]
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct ResumePoint {
pub fully_played: bool,
#[serde(with = "duration_ms", rename = "resume_position_ms")]
Expand Down
5 changes: 3 additions & 2 deletions rspotify-model/src/track.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
//! All kinds of tracks object
use chrono::prelude::*;
use chrono::Duration;
use serde::{Deserialize, Serialize};

use std::{collections::HashMap, time::Duration};
use std::collections::HashMap;

use crate::{
custom_serde::duration_ms, PlayableId, Restriction, SimplifiedAlbum, SimplifiedArtist, TrackId,
Expand Down Expand Up @@ -56,7 +57,7 @@ pub struct FullTracks {
///
/// `is_playable`, `linked_from` and `restrictions` will only be present when
/// relinking is applied.
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)]
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
pub struct SimplifiedTrack {
pub artists: Vec<SimplifiedArtist>,
pub available_markets: Option<Vec<String>>,
Expand Down
19 changes: 9 additions & 10 deletions tests/test_models.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use chrono::{DateTime, NaiveDateTime, Utc};
use chrono::{DateTime, Duration, NaiveDateTime, Utc};
use rspotify::model::*;
use serde::de::DeserializeOwned;
use std::time::Duration;

#[track_caller]
fn deserialize<T>(data: impl AsRef<str>) -> T
Expand Down Expand Up @@ -51,7 +50,7 @@ fn test_simplified_track() {
"#;
let track: SimplifiedTrack = deserialize(json_str);
let duration = Duration::from_millis(276773);
let duration = Duration::milliseconds(276773);
assert_eq!(track.duration, duration);
}

Expand Down Expand Up @@ -195,7 +194,7 @@ fn test_simplified_episode() {
simplified_episode.release_date_precision,
DatePrecision::Day
);
let duration = Duration::from_millis(2685023);
let duration = Duration::milliseconds(2685023);
assert_eq!(simplified_episode.duration, duration);
}

Expand Down Expand Up @@ -263,7 +262,7 @@ fn test_full_episode() {
"#;
let full_episode: FullEpisode = deserialize(json_str);
assert_eq!(full_episode.release_date_precision, DatePrecision::Day);
let duration = Duration::from_millis(1502795);
let duration = Duration::milliseconds(1502795);
assert_eq!(full_episode.duration, duration);
}

Expand Down Expand Up @@ -518,7 +517,7 @@ fn test_audio_features() {
}
"#;
let audio_features: AudioFeatures = deserialize(json);
let duration = Duration::from_millis(255349);
let duration = Duration::milliseconds(255349);
assert_eq!(audio_features.duration, duration);
}

Expand Down Expand Up @@ -597,7 +596,7 @@ fn test_full_track() {
}
"#;
let full_track: FullTrack = deserialize(json);
let duration = Duration::from_millis(207959);
let duration = Duration::milliseconds(207959);
assert_eq!(full_track.duration, duration);
}

Expand All @@ -610,7 +609,7 @@ fn test_resume_point() {
}
"#;
let resume_point: ResumePoint = deserialize(json);
let duration = Duration::from_millis(423432);
let duration = Duration::milliseconds(423432);
assert_eq!(resume_point.resume_position, duration);
}

Expand All @@ -623,7 +622,7 @@ fn test_resume_point_negative() {
}
"#;
let resume_point: ResumePoint = deserialize(json);
let duration = Duration::default();
let duration = Duration::milliseconds(-1000);
assert_eq!(resume_point.resume_position, duration);
}

Expand Down Expand Up @@ -737,7 +736,7 @@ fn test_currently_playing_context() {
);
assert_eq!(currently_playing_context.timestamp, dt);

let duration = Duration::from_millis(22270);
let duration = Duration::milliseconds(22270);
assert_eq!(currently_playing_context.progress, Some(duration));
}

Expand Down
4 changes: 2 additions & 2 deletions tests/test_with_oauth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.as_millis() as u32);
let position = backup.progress.map(|p| p.num_milliseconds() as u32);
client
.start_uris_playback(uri, device, offset, position)
.await
Expand Down Expand Up @@ -535,7 +535,7 @@ async fn test_seek_track() {
}) = backup
{
client
.seek_track(progress.as_millis() as u32, None)
.seek_track(progress.num_milliseconds() as u32, None)
.await
.unwrap();
}
Expand Down

0 comments on commit 1a3a06e

Please sign in to comment.