From 81020faa519c9328511603116f2abc6e3fa31c01 Mon Sep 17 00:00:00 2001 From: Felix Prillwitz Date: Tue, 26 Nov 2024 23:38:04 +0100 Subject: [PATCH] core/context: adjust context requests and update - search should now return the expected context - removed workaround for single track playback - move local playback check into update_context - check track uri for invalid characters - early return with `?` --- connect/src/spirc.rs | 7 +++--- connect/src/state.rs | 5 ++++- connect/src/state/context.rs | 41 +++++++++++++----------------------- connect/src/state/options.rs | 5 ++--- core/src/spclient.rs | 23 ++++++++++++++++++-- 5 files changed, 46 insertions(+), 35 deletions(-) diff --git a/connect/src/spirc.rs b/connect/src/spirc.rs index 2cac273c6..c218109eb 100644 --- a/connect/src/spirc.rs +++ b/connect/src/spirc.rs @@ -546,9 +546,10 @@ impl SpircTask { if context_uri.contains("spotify:show:") || context_uri.contains("spotify:episode:") { // autoplay is not supported for podcasts - return Err( - SpircError::NotAllowedContext(ResolveContext::from_uri(context_uri, true)).into(), - ); + Err(SpircError::NotAllowedContext(ResolveContext::from_uri( + context_uri, + true, + )))? } let previous_tracks = self.connect_state.prev_autoplay_track_uris(); diff --git a/connect/src/state.rs b/connect/src/state.rs index 94095cc5a..27c69ef98 100644 --- a/connect/src/state.rs +++ b/connect/src/state.rs @@ -50,6 +50,8 @@ pub enum StateError { ContextHasNoTracks, #[error("playback of local files is not supported")] UnsupportedLocalPlayBack, + #[error("track uri <{0}> contains invalid characters")] + InvalidTrackUri(String), } impl From for Error { @@ -60,7 +62,8 @@ impl From for Error { | MessageFieldNone(_) | NoContext(_) | CanNotFindTrackInContext(_, _) - | ContextHasNoTracks => Error::failed_precondition(err), + | ContextHasNoTracks + | InvalidTrackUri(_) => Error::failed_precondition(err), CurrentlyDisallowed { .. } | UnsupportedLocalPlayBack => Error::unavailable(err), } } diff --git a/connect/src/state/context.rs b/connect/src/state/context.rs index 825dca157..0b71fc781 100644 --- a/connect/src/state/context.rs +++ b/connect/src/state/context.rs @@ -6,6 +6,8 @@ use librespot_protocol::player::{Context, ContextIndex, ContextPage, ContextTrac use std::collections::HashMap; use uuid::Uuid; +const LOCAL_FILES_IDENTIFIER: &str = "spotify:local-files"; + #[derive(Debug, Clone)] pub struct StateContext { pub tracks: Vec, @@ -84,6 +86,8 @@ impl ConnectState { if context.pages.iter().all(|p| p.tracks.is_empty()) { error!("context didn't have any tracks: {context:#?}"); return Err(StateError::ContextHasNoTracks.into()); + } else if context.uri.starts_with(LOCAL_FILES_IDENTIFIER) { + return Err(StateError::UnsupportedLocalPlayBack.into()); } self.next_contexts.clear(); @@ -98,7 +102,7 @@ impl ConnectState { } let page = match first_page { - None => return Err(StateError::ContextHasNoTracks.into()), + None => Err(StateError::ContextHasNoTracks)?, Some(p) => p, }; @@ -244,39 +248,24 @@ impl ConnectState { context_uri: Option<&str>, provider: Option, ) -> Result { - // completely ignore local playback. - if matches!(context_uri, Some(context_uri) if context_uri.starts_with("spotify:local-files")) - { - return Err(StateError::UnsupportedLocalPlayBack.into()); - } - - let question_mark_idx = ctx_track - .uri - .contains('?') - .then(|| ctx_track.uri.find('?')) - .flatten(); + let id = if !ctx_track.uri.is_empty() { + if ctx_track.uri.contains(['?', '%']) { + Err(StateError::InvalidTrackUri(ctx_track.uri.clone()))? + } - let ctx_track_uri = if let Some(idx) = question_mark_idx { - &ctx_track.uri[..idx] + SpotifyId::from_uri(&ctx_track.uri)? + } else if !ctx_track.gid.is_empty() { + SpotifyId::from_raw(&ctx_track.gid)? } else { - &ctx_track.uri - } - .to_string(); + Err(StateError::InvalidTrackUri(String::new()))? + }; - let provider = if self.unavailable_uri.contains(&ctx_track_uri) { + let provider = if self.unavailable_uri.contains(&ctx_track.uri) { Provider::Unavailable } else { provider.unwrap_or(Provider::Context) }; - let id = if !ctx_track_uri.is_empty() { - SpotifyId::from_uri(&ctx_track_uri) - } else if !ctx_track.gid.is_empty() { - SpotifyId::from_raw(&ctx_track.gid) - } else { - return Err(Error::unavailable("track not available")); - }?; - // assumption: the uid is used as unique-id of any item // - queue resorting is done by each client and orients itself by the given uid // - if no uid is present, resorting doesn't work or behaves not as intended diff --git a/connect/src/state/options.rs b/connect/src/state/options.rs index 7a32a2531..534869df1 100644 --- a/connect/src/state/options.rs +++ b/connect/src/state/options.rs @@ -40,11 +40,10 @@ impl ConnectState { .disallow_toggling_shuffle_reasons .first() { - return Err(StateError::CurrentlyDisallowed { + Err(StateError::CurrentlyDisallowed { action: "shuffle".to_string(), reason: reason.clone(), - } - .into()); + })? } self.prev_tracks.clear(); diff --git a/core/src/spclient.rs b/core/src/spclient.rs index da55b7007..e04ae304e 100644 --- a/core/src/spclient.rs +++ b/core/src/spclient.rs @@ -802,10 +802,28 @@ impl SpClient { self.request_url(&url).await } + /// Request the context for an uri + /// + /// ## Query entry found in the wild: + /// - include_video=true + /// ## Remarks: + /// - track + /// - returns a single page with a single track + /// - when requesting a single track with a query in the request, the returned track uri + /// **will** contain the query + /// - artists + /// - returns 2 pages with tracks: 10 most popular tracks and latest/popular album + /// - remaining pages are albums of the artists and are only provided as page_url + /// - search + /// - is massively influenced by the provided query + /// - the query result shown by the search expects no query at all + /// - uri looks like "spotify:search:never+gonna" pub async fn get_context(&self, uri: &str) -> Result { let uri = format!("/context-resolve/v1/{uri}"); - let res = self.request(&Method::GET, &uri, None, None).await?; + let res = self + .request_with_options(&Method::GET, &uri, None, None, &NO_METRICS_AND_SALT) + .await?; let ctx_json = String::from_utf8(res.to_vec())?; let ctx = protobuf_json_mapping::parse_from_str::(&ctx_json)?; @@ -817,11 +835,12 @@ impl SpClient { context_request: &AutoplayContextRequest, ) -> Result { let res = self - .request_with_protobuf( + .request_with_protobuf_and_options( &Method::POST, "/context-resolve/v1/autoplay", None, context_request, + &NO_METRICS_AND_SALT, ) .await?;