From 88f7cdbb44f4d3d1bd10bbdb74963ccb48c9c695 Mon Sep 17 00:00:00 2001 From: eladyn <59307989+eladyn@users.noreply.github.com> Date: Thu, 28 Jul 2022 18:46:16 +0200 Subject: [PATCH] Fix playlist metadata fields parsing (#1019) Some fields were wrongly parsed as `SpotifyId`s, although they do not always encode exactly 16 bytes in practice. Also, some optional fields caused `[]` to be parsed as `SpotifyId`, which obviously failed as well. --- core/src/cdn_url.rs | 4 ++-- core/src/date.rs | 15 ++++----------- metadata/src/playlist/attribute.rs | 14 +++++++------- metadata/src/playlist/item.rs | 2 +- metadata/src/playlist/list.rs | 24 ++++++++++++++---------- metadata/src/track.rs | 2 +- 6 files changed, 29 insertions(+), 32 deletions(-) diff --git a/core/src/cdn_url.rs b/core/src/cdn_url.rs index 7257a9a5..9df43ea9 100644 --- a/core/src/cdn_url.rs +++ b/core/src/cdn_url.rs @@ -1,5 +1,5 @@ use std::{ - convert::{TryFrom, TryInto}, + convert::TryFrom, ops::{Deref, DerefMut}, }; @@ -152,7 +152,7 @@ impl TryFrom for MaybeExpiringUrls { Ok(MaybeExpiringUrl( cdn_url.to_owned(), - Some(expiry.try_into()?), + Some(Date::from_timestamp_ms(expiry * 1000)?), )) } else { Ok(MaybeExpiringUrl(cdn_url.to_owned(), None)) diff --git a/core/src/date.rs b/core/src/date.rs index 3c78f265..c9aadce8 100644 --- a/core/src/date.rs +++ b/core/src/date.rs @@ -28,12 +28,12 @@ impl Deref for Date { } impl Date { - pub fn as_timestamp(&self) -> i64 { - self.0.unix_timestamp() + pub fn as_timestamp_ms(&self) -> i64 { + (self.0.unix_timestamp_nanos() / 1_000_000) as i64 } - pub fn from_timestamp(timestamp: i64) -> Result { - let date_time = OffsetDateTime::from_unix_timestamp(timestamp)?; + pub fn from_timestamp_ms(timestamp: i64) -> Result { + let date_time = OffsetDateTime::from_unix_timestamp_nanos(timestamp as i128 * 1_000_000)?; Ok(Self(date_time)) } @@ -79,10 +79,3 @@ impl From for Date { Self(datetime) } } - -impl TryFrom for Date { - type Error = crate::Error; - fn try_from(timestamp: i64) -> Result { - Self::from_timestamp(timestamp) - } -} diff --git a/metadata/src/playlist/attribute.rs b/metadata/src/playlist/attribute.rs index eb4fb577..d271bc54 100644 --- a/metadata/src/playlist/attribute.rs +++ b/metadata/src/playlist/attribute.rs @@ -7,7 +7,7 @@ use std::{ use crate::{image::PictureSizes, util::from_repeated_enum}; -use librespot_core::{date::Date, SpotifyId}; +use librespot_core::date::Date; use librespot_protocol as protocol; use protocol::playlist4_external::FormatListAttribute as PlaylistFormatAttributeMessage; @@ -24,7 +24,7 @@ use protocol::playlist4_external::UpdateListAttributes as PlaylistUpdateAttribut pub struct PlaylistAttributes { pub name: String, pub description: String, - pub picture: SpotifyId, + pub picture: Vec, pub is_collaborative: bool, pub pl3_version: String, pub is_deleted_by_owner: bool, @@ -63,7 +63,7 @@ pub struct PlaylistItemAttributes { pub seen_at: Date, pub is_public: bool, pub format_attributes: PlaylistFormatAttribute, - pub item_id: SpotifyId, + pub item_id: Vec, } #[derive(Debug, Clone)] @@ -113,7 +113,7 @@ impl TryFrom<&PlaylistAttributesMessage> for PlaylistAttributes { Ok(Self { name: attributes.get_name().to_owned(), description: attributes.get_description().to_owned(), - picture: attributes.get_picture().try_into()?, + picture: attributes.get_picture().to_owned(), is_collaborative: attributes.get_collaborative(), pl3_version: attributes.get_pl3_version().to_owned(), is_deleted_by_owner: attributes.get_deleted_by_owner(), @@ -146,11 +146,11 @@ impl TryFrom<&PlaylistItemAttributesMessage> for PlaylistItemAttributes { fn try_from(attributes: &PlaylistItemAttributesMessage) -> Result { Ok(Self { added_by: attributes.get_added_by().to_owned(), - timestamp: attributes.get_timestamp().try_into()?, - seen_at: attributes.get_seen_at().try_into()?, + timestamp: Date::from_timestamp_ms(attributes.get_timestamp())?, + seen_at: Date::from_timestamp_ms(attributes.get_seen_at())?, is_public: attributes.get_public(), format_attributes: attributes.get_format_attributes().into(), - item_id: attributes.get_item_id().try_into()?, + item_id: attributes.get_item_id().to_owned(), }) } } diff --git a/metadata/src/playlist/item.rs b/metadata/src/playlist/item.rs index dbd5fda2..20f94a0b 100644 --- a/metadata/src/playlist/item.rs +++ b/metadata/src/playlist/item.rs @@ -94,7 +94,7 @@ impl TryFrom<&PlaylistMetaItemMessage> for PlaylistMetaItem { revision: item.try_into()?, attributes: item.get_attributes().try_into()?, length: item.get_length(), - timestamp: item.get_timestamp().try_into()?, + timestamp: Date::from_timestamp_ms(item.get_timestamp())?, owner_username: item.get_owner_username().to_owned(), has_abuse_reporting: item.get_abuse_reporting_enabled(), capabilities: item.get_capabilities().into(), diff --git a/metadata/src/playlist/list.rs b/metadata/src/playlist/list.rs index 0a359694..a8ef677b 100644 --- a/metadata/src/playlist/list.rs +++ b/metadata/src/playlist/list.rs @@ -39,12 +39,12 @@ impl Deref for Geoblocks { #[derive(Debug, Clone)] pub struct Playlist { pub id: NamedSpotifyId, - pub revision: SpotifyId, + pub revision: Vec, pub length: i32, pub attributes: PlaylistAttributes, pub contents: PlaylistItemList, - pub diff: PlaylistDiff, - pub sync_result: PlaylistDiff, + pub diff: Option, + pub sync_result: Option, pub resulting_revisions: Playlists, pub has_multiple_heads: bool, pub is_up_to_date: bool, @@ -77,12 +77,12 @@ impl Deref for RootPlaylist { #[derive(Debug, Clone)] pub struct SelectedListContent { - pub revision: SpotifyId, + pub revision: Vec, pub length: i32, pub attributes: PlaylistAttributes, pub contents: PlaylistItemList, - pub diff: PlaylistDiff, - pub sync_result: PlaylistDiff, + pub diff: Option, + pub sync_result: Option, pub resulting_revisions: Playlists, pub has_multiple_heads: bool, pub is_up_to_date: bool, @@ -202,17 +202,21 @@ impl TryFrom<&::Message> for SelectedListContent { type Error = librespot_core::Error; fn try_from(playlist: &::Message) -> Result { Ok(Self { - revision: playlist.get_revision().try_into()?, + revision: playlist.get_revision().to_owned(), length: playlist.get_length(), attributes: playlist.get_attributes().try_into()?, contents: playlist.get_contents().try_into()?, - diff: playlist.get_diff().try_into()?, - sync_result: playlist.get_sync_result().try_into()?, + diff: playlist.diff.as_ref().map(TryInto::try_into).transpose()?, + sync_result: playlist + .sync_result + .as_ref() + .map(TryInto::try_into) + .transpose()?, resulting_revisions: playlist.get_resulting_revisions().try_into()?, has_multiple_heads: playlist.get_multiple_heads(), is_up_to_date: playlist.get_up_to_date(), nonces: playlist.get_nonces().into(), - timestamp: playlist.get_timestamp().try_into()?, + timestamp: Date::from_timestamp_ms(playlist.get_timestamp())?, owner_username: playlist.get_owner_username().to_owned(), has_abuse_reporting: playlist.get_abuse_reporting_enabled(), capabilities: playlist.get_capabilities().into(), diff --git a/metadata/src/track.rs b/metadata/src/track.rs index 001590f5..4ab9b2b4 100644 --- a/metadata/src/track.rs +++ b/metadata/src/track.rs @@ -132,7 +132,7 @@ impl TryFrom<&::Message> for Track { sale_periods: track.get_sale_period().try_into()?, previews: track.get_preview().into(), tags: track.get_tags().to_vec(), - earliest_live_timestamp: track.get_earliest_live_timestamp().try_into()?, + earliest_live_timestamp: Date::from_timestamp_ms(track.get_earliest_live_timestamp())?, has_lyrics: track.get_has_lyrics(), availability: track.get_availability().try_into()?, licensor: Uuid::from_slice(track.get_licensor().get_uuid())