From ea16a3e74fc5abe2cef3978cefe0de5a4d4ae654 Mon Sep 17 00:00:00 2001 From: ThetaDev Date: Wed, 9 Nov 2022 12:48:44 +0100 Subject: [PATCH 1/2] fix: ignore radio items from search --- src/client/response/music_item.rs | 53 ++++++++++++++++++++----------- tests/youtube.rs | 7 ++++ 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/client/response/music_item.rs b/src/client/response/music_item.rs index cded154..d552771 100644 --- a/src/client/response/music_item.rs +++ b/src/client/response/music_item.rs @@ -345,8 +345,9 @@ impl MusicListMapper { } } - fn map_item(&mut self, item: MusicResponseItem) -> Result { + fn map_item(&mut self, item: MusicResponseItem) -> Result, String> { match item { + // List item MusicResponseItem::MusicResponsiveListItemRenderer(item) => { let mut columns = item.flex_columns.into_iter(); let title = columns.next().map(|col| col.renderer.text.to_string()); @@ -356,19 +357,27 @@ impl MusicListMapper { match item.navigation_endpoint { // Artist / Album / Playlist Some(ne) => { - let (page_type, id) = ne - .music_page() - .ok_or_else(|| "could not get navigation endpoint".to_owned())?; - - let title = - title.ok_or_else(|| format!("track {}: could not get title", id))?; - let mut subtitle_parts = c2 - .ok_or_else(|| format!("item {}: could not get subtitle", id))? + .ok_or_else(|| "could not get subtitle".to_owned())? .renderer .text .split(util::DOT_SEPARATOR) .into_iter(); + + let (page_type, id) = match ne.music_page() { + Some(music_page) => music_page, + None => { + // Ignore radio items + if subtitle_parts.len() == 1 { + return Ok(None); + } + return Err("invalid navigation endpoint".to_string()); + } + }; + + let title = + title.ok_or_else(|| format!("track {}: could not get title", id))?; + let subtitle_p1 = subtitle_parts.next(); let subtitle_p2 = subtitle_parts.next(); let subtitle_p3 = subtitle_parts.next(); @@ -385,7 +394,7 @@ impl MusicListMapper { avatar: item.thumbnail.into(), subscriber_count, })); - Ok(MusicEntityType::Artist) + Ok(Some(MusicEntityType::Artist)) } PageType::Album => { let album_type = subtitle_p1 @@ -406,7 +415,7 @@ impl MusicListMapper { year, by_va, })); - Ok(MusicEntityType::Album) + Ok(Some(MusicEntityType::Album)) } PageType::Playlist => { // Part 1 may be the "Playlist" label @@ -433,11 +442,11 @@ impl MusicListMapper { track_count, from_ytm, })); - Ok(MusicEntityType::Playlist) + Ok(Some(MusicEntityType::Playlist)) } PageType::Channel => { // There may be broken YT channels from the artist search. They can be skipped. - Ok(MusicEntityType::Artist) + Ok(None) } } } @@ -553,10 +562,11 @@ impl MusicListMapper { is_video, track_nr, })); - Ok(MusicEntityType::Track) + Ok(Some(MusicEntityType::Track)) } } } + // Tile MusicResponseItem::MusicTwoRowItemRenderer(item) => { let mut subtitle_parts = item.subtitle.split(util::DOT_SEPARATOR).into_iter(); let subtitle_p1 = subtitle_parts.next(); @@ -581,7 +591,7 @@ impl MusicListMapper { is_video: true, track_nr: None, })); - Ok(MusicEntityType::Track) + Ok(Some(MusicEntityType::Track)) } // Artist / Album / Playlist None => { @@ -636,7 +646,7 @@ impl MusicListMapper { year, by_va, })); - Ok(MusicEntityType::Album) + Ok(Some(MusicEntityType::Album)) } PageType::Playlist => { let from_ytm = subtitle_p2 @@ -657,7 +667,7 @@ impl MusicListMapper { track_count, from_ytm, })); - Ok(MusicEntityType::Playlist) + Ok(Some(MusicEntityType::Playlist)) } PageType::Artist => { let subscriber_count = subtitle_p1.and_then(|p| { @@ -670,7 +680,7 @@ impl MusicListMapper { avatar: item.thumbnail_renderer.into(), subscriber_count, })); - Ok(MusicEntityType::Artist) + Ok(Some(MusicEntityType::Artist)) } PageType::Channel => { Err(format!("channel items unsupported. id: {}", id)) @@ -691,7 +701,12 @@ impl MusicListMapper { res.c .into_iter() .for_each(|item| match self.map_item(item) { - Ok(t) => etype = Some(t), + Ok(Some(et)) => { + if etype.is_none() { + etype = Some(et); + } + } + Ok(None) => {} Err(e) => self.warnings.push(e), }); etype diff --git a/tests/youtube.rs b/tests/youtube.rs index b119402..365fa9a 100644 --- a/tests/youtube.rs +++ b/tests/youtube.rs @@ -1746,6 +1746,13 @@ async fn music_search_playlists_community() { assert!(!playlist.from_ytm); } +/// The YouTube Music search sometimes shows genre radio items. They should be skipped. +#[tokio::test] +async fn music_search_genre_radio() { + let rp = RustyPipe::builder().strict().build(); + rp.query().music_search("pop radio").await.unwrap(); +} + //#TESTUTIL /// Assert equality within 10% margin From 9b6c952fd3eaf0771636b137cd0d82cec3ebb08e Mon Sep 17 00:00:00 2001 From: ThetaDev Date: Wed, 9 Nov 2022 21:02:21 +0100 Subject: [PATCH 2/2] fix: support YTM browse urls --- src/client/url_resolver.rs | 13 +++++++++++++ src/model/mod.rs | 4 +--- tests/youtube.rs | 2 ++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/client/url_resolver.rs b/src/client/url_resolver.rs index 9ced71a..4446eb1 100644 --- a/src/client/url_resolver.rs +++ b/src/client/url_resolver.rs @@ -72,6 +72,19 @@ impl RustyPipeQuery { Ok(UrlTarget::Playlist { id }) } } + // Album or channel + Some("browse") => match path_split.next() { + Some(id) => { + if util::CHANNEL_ID_REGEX.is_match(id).unwrap_or_default() { + Ok(UrlTarget::Channel { id: id.to_owned() }) + } else if util::ALBUM_ID_REGEX.is_match(id).unwrap_or_default() { + Ok(UrlTarget::Album { id: id.to_owned() }) + } else { + Err(Error::Other("invalid url: no browse id".into())) + } + } + None => Err(Error::Other("invalid url: invalid browse id".into())), + }, // Channel vanity URL or youtu.be shortlink Some(mut id) => { if id == "c" || id == "user" { diff --git a/src/model/mod.rs b/src/model/mod.rs index b5ed216..a77328e 100644 --- a/src/model/mod.rs +++ b/src/model/mod.rs @@ -68,9 +68,7 @@ impl UrlTarget { format!("{}/playlist?list={}", yt_host, id) } UrlTarget::Album { id } => { - // The official album URLs use the playlist ID - // This looks weird, but it works - format!("{}/channel/{}", yt_host, id) + format!("https://music.youtube.com/browse/{}", id) } } } diff --git a/tests/youtube.rs b/tests/youtube.rs index 365fa9a..efce4e2 100644 --- a/tests/youtube.rs +++ b/tests/youtube.rs @@ -1189,6 +1189,8 @@ async fn search_suggestion_empty() { // Both a video ID and a channel name + video time param => returns video #[case("https://piped.mha.fi/dQw4w9WgXcQ?t=0", UrlTarget::Video {id: "dQw4w9WgXcQ".to_owned(), start_time: 0})] #[case("https://music.youtube.com/playlist?list=OLAK5uy_k0yFrZlFRgCf3rLPza-lkRmCrtLPbK9pE", UrlTarget::Album {id: "MPREb_GyH43gCvdM5".to_owned()})] +#[case("https://music.youtube.com/browse/MPREb_GyH43gCvdM5", UrlTarget::Album {id: "MPREb_GyH43gCvdM5".to_owned()})] +#[case("https://music.youtube.com/browse/UC5I2hjZYiW9gZPVkvzM8_Cw", UrlTarget::Channel {id: "UC5I2hjZYiW9gZPVkvzM8_Cw".to_owned()})] #[tokio::test] async fn resolve_url(#[case] url: &str, #[case] expect: UrlTarget) { let rp = RustyPipe::builder().strict().build();