From a338e4e08e385e741c18ce2d30ae1e5016599cf0 Mon Sep 17 00:00:00 2001 From: Xiang Rong Lin <41164160+XiangRongLin@users.noreply.github.com> Date: Sat, 26 Sep 2020 11:22:24 +0200 Subject: [PATCH] [Youtube] Apply review suggestions and avoid channel mix edge case --- .../youtube/YoutubeParsingHelper.java | 35 +++++++++++++++++-- .../YoutubeMixPlaylistExtractor.java | 17 +++++---- .../YoutubePlaylistLinkHandlerFactory.java | 15 +++++--- .../YoutubeMixPlaylistExtractorTest.java | 34 ++++++++++-------- 4 files changed, 70 insertions(+), 31 deletions(-) diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/YoutubeParsingHelper.java b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/YoutubeParsingHelper.java index ea8743069..06f421a69 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/YoutubeParsingHelper.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/YoutubeParsingHelper.java @@ -214,13 +214,44 @@ public class YoutubeParsingHelper { /** * Checks if the given playlist id is a YouTube Music Mix (auto-generated playlist) - * Ids from a YouTube Music Mix start with "RD" + * Ids from a YouTube Music Mix start with "RDAMVM" * @param playlistId * @return Whether given id belongs to a YouTube Music Mix */ public static boolean isYoutubeMusicMixId(final String playlistId) { return playlistId.startsWith("RDAMVM"); } + /** + * Checks if the given playlist id is a YouTube Channel Mix (auto-generated playlist) + * Ids from a YouTube channel Mix start with "RDCM" + * @return Whether given id belongs to a YouTube Channel Mix + */ + public static boolean isYoutubeChannelMixId(final String playlistId) { + return playlistId.startsWith("RDCM"); + } + + /** + * Extracts the video id from the playlist id for Mixes. + * @throws ParsingException If the playlistId is a Channel Mix or not a mix. + */ + public static String extractVideoIdFromMixId(final String playlistId) throws ParsingException { + if (playlistId.startsWith("RDMM")) { //My Mix + return playlistId.substring(4); + + } else if (playlistId.startsWith("RDAMVM")) { //Music mix + return playlistId.substring(6); + + } else if (playlistId.startsWith("RMCM")) { //Channel mix + //Channel mix are build with RMCM{channelId}, so videoId can't be determined + throw new ParsingException("Video id could not be determined from mix id: " + playlistId); + + } else if (playlistId.startsWith("RD")) { // Normal mix + return playlistId.substring(2); + + } else { //not a mix + throw new ParsingException("Video id could not be determined from mix id: " + playlistId); + } + } public static JsonObject getInitialData(String html) throws ParsingException { try { @@ -362,7 +393,7 @@ public class YoutubeParsingHelper { .end() .value("query", "test") .value("params", "Eg-KAQwIARAAGAAgACgAMABqChAEEAUQAxAKEAk%3D") - .end().done().getBytes(StandardCharsets.UTF_8); + .end().done().getBytes("UTF-8"); // @formatter:on Map> headers = new HashMap<>(); diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeMixPlaylistExtractor.java b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeMixPlaylistExtractor.java index d5e307453..d424129ef 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeMixPlaylistExtractor.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeMixPlaylistExtractor.java @@ -28,6 +28,7 @@ import static org.schabi.newpipe.extractor.services.youtube.YoutubeParsingHelper import static org.schabi.newpipe.extractor.services.youtube.YoutubeParsingHelper.getResponse; import static org.schabi.newpipe.extractor.services.youtube.YoutubeParsingHelper.getUrlFromNavigationEndpoint; import static org.schabi.newpipe.extractor.services.youtube.YoutubeParsingHelper.toJsonArray; +import static org.schabi.newpipe.extractor.utils.Utils.isNullOrEmpty; /** * A {@link YoutubePlaylistExtractor} for a mix (auto-generated playlist). @@ -40,7 +41,7 @@ public class YoutubeMixPlaylistExtractor extends PlaylistExtractor { * YouTube identifies mixes based on this cookie. With this information it can generate * continuations without duplicates. */ - private static final String COOKIE_NAME = "VISITOR_INFO1_LIVE"; + public static final String COOKIE_NAME = "VISITOR_INFO1_LIVE"; private JsonObject initialData; private JsonObject playlistData; @@ -124,11 +125,7 @@ public class YoutubeMixPlaylistExtractor extends PlaylistExtractor { final StreamInfoItemsCollector collector = new StreamInfoItemsCollector(getServiceId()); collectStreamsFrom(collector, playlistData.getArray("contents")); return new InfoItemsPage<>(collector, - new Page(getNextPageUrl(), Collections.singletonMap(COOKIE_NAME, cookieValue))); - } - - private String getNextPageUrl() throws ExtractionException { - return getNextPageUrlFrom(playlistData); + new Page(getNextPageUrlFrom(playlistData), Collections.singletonMap(COOKIE_NAME, cookieValue))); } private String getNextPageUrlFrom(final JsonObject playlistJson) throws ExtractionException { @@ -146,9 +143,11 @@ public class YoutubeMixPlaylistExtractor extends PlaylistExtractor { @Override public InfoItemsPage getPage(final Page page) throws ExtractionException, IOException { - if (page == null || page.getUrl().isEmpty()) { - throw new ExtractionException( - new IllegalArgumentException("Page url is empty or null")); + if (page == null || isNullOrEmpty(page.getUrl())) { + throw new IllegalArgumentException("Page url is empty or null"); + } + if (!page.getCookies().containsKey(COOKIE_NAME)) { + throw new IllegalArgumentException("Cooke '" + COOKIE_NAME + "' is missing"); } final JsonArray ajaxJson = getJsonResponse(page, getExtractorLocalization()); diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/linkHandler/YoutubePlaylistLinkHandlerFactory.java b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/linkHandler/YoutubePlaylistLinkHandlerFactory.java index 096447879..aa2908e64 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/linkHandler/YoutubePlaylistLinkHandlerFactory.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/linkHandler/YoutubePlaylistLinkHandlerFactory.java @@ -1,5 +1,8 @@ package org.schabi.newpipe.extractor.services.youtube.linkHandler; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.List; import org.schabi.newpipe.extractor.exceptions.ContentNotSupportedException; import org.schabi.newpipe.extractor.exceptions.ParsingException; import org.schabi.newpipe.extractor.linkhandler.LinkHandler; @@ -8,10 +11,6 @@ import org.schabi.newpipe.extractor.linkhandler.ListLinkHandlerFactory; import org.schabi.newpipe.extractor.services.youtube.YoutubeParsingHelper; import org.schabi.newpipe.extractor.utils.Utils; -import java.net.MalformedURLException; -import java.net.URL; -import java.util.List; - public class YoutubePlaylistLinkHandlerFactory extends ListLinkHandlerFactory { private static final YoutubePlaylistLinkHandlerFactory INSTANCE = @@ -58,6 +57,12 @@ public class YoutubePlaylistLinkHandlerFactory extends ListLinkHandlerFactory { "YouTube Music Mix playlists are not yet supported"); } + if (YoutubeParsingHelper.isYoutubeChannelMixId(listID) + && Utils.getQueryValue(urlObj, "v") == null) { + //Video id can't be determined from the channel mix id. See YoutubeParsingHelper#extractVideoIdFromMixId + throw new ContentNotSupportedException("Channel Mix without a video id are not supported"); + } + return listID; } catch (final Exception exception) { throw new ParsingException("Error could not parse url :" + exception.getMessage(), @@ -89,7 +94,7 @@ public class YoutubePlaylistLinkHandlerFactory extends ListLinkHandlerFactory { if (listID != null && YoutubeParsingHelper.isYoutubeMixId(listID)) { String videoID = Utils.getQueryValue(urlObj, "v"); if (videoID == null) { - videoID = listID.substring(2); + videoID = YoutubeParsingHelper.extractVideoIdFromMixId(listID); } final String newUrl = "https://www.youtube.com/watch?v=" + videoID + "&list=" + listID; diff --git a/extractor/src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeMixPlaylistExtractorTest.java b/extractor/src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeMixPlaylistExtractorTest.java index 8e41f12d8..49fb3fe04 100644 --- a/extractor/src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeMixPlaylistExtractorTest.java +++ b/extractor/src/test/java/org/schabi/newpipe/extractor/services/youtube/YoutubeMixPlaylistExtractorTest.java @@ -1,15 +1,8 @@ package org.schabi.newpipe.extractor.services.youtube; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.startsWith; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.schabi.newpipe.extractor.ExtractorAsserts.assertIsSecureUrl; -import static org.schabi.newpipe.extractor.ServiceList.YouTube; - +import java.util.Collections; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.hamcrest.MatcherAssert; import org.junit.BeforeClass; @@ -31,6 +24,15 @@ import org.schabi.newpipe.extractor.services.youtube.YoutubeMixPlaylistExtractor import org.schabi.newpipe.extractor.services.youtube.extractors.YoutubeMixPlaylistExtractor; import org.schabi.newpipe.extractor.stream.StreamInfoItem; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.schabi.newpipe.extractor.ExtractorAsserts.assertIsSecureUrl; +import static org.schabi.newpipe.extractor.ServiceList.YouTube; + @RunWith(Suite.class) @SuiteClasses({Mix.class, MixWithIndex.class, MyMix.class, Invalid.class, ChannelMix.class}) public class YoutubeMixPlaylistExtractorTest { @@ -41,6 +43,8 @@ public class YoutubeMixPlaylistExtractorTest { "Most Beautiful And Emotional Piano: Anime Music Shigatsu wa Kimi no Uso OST IMO"; private static YoutubeMixPlaylistExtractor extractor; + private static Map dummyCookie + = Collections.singletonMap(YoutubeMixPlaylistExtractor.COOKIE_NAME, "whatever"); public static class Mix { @@ -83,8 +87,8 @@ public class YoutubeMixPlaylistExtractorTest { @Test public void getPage() throws Exception { final InfoItemsPage streams = extractor.getPage( - new Page("https://www.youtube.com/watch?v=" + VIDEO_ID + "&list=RD" + VIDEO_ID - + PBJ)); + new Page("https://www.youtube.com/watch?v=" + VIDEO_ID + "&list=RD" + VIDEO_ID + + PBJ, dummyCookie)); assertFalse(streams.getItems().isEmpty()); assertTrue(streams.hasNextPage()); } @@ -157,7 +161,7 @@ public class YoutubeMixPlaylistExtractorTest { public void getPage() throws Exception { final InfoItemsPage streams = extractor.getPage( new Page("https://www.youtube.com/watch?v=" + VIDEO_ID_NUMBER_13 + "&list=RD" - + VIDEO_ID + INDEX + PBJ)); + + VIDEO_ID + INDEX + PBJ, dummyCookie)); assertFalse(streams.getItems().isEmpty()); assertTrue(streams.hasNextPage()); } @@ -229,7 +233,7 @@ public class YoutubeMixPlaylistExtractorTest { public void getPage() throws Exception { final InfoItemsPage streams = extractor.getPage(new Page("https://www.youtube.com/watch?v=" + VIDEO_ID - + "&list=RDMM" + VIDEO_ID + PBJ)); + + "&list=RDMM" + VIDEO_ID + PBJ, dummyCookie)); assertFalse(streams.getItems().isEmpty()); assertTrue(streams.hasNextPage()); } @@ -267,7 +271,7 @@ public class YoutubeMixPlaylistExtractorTest { NewPipe.init(DownloaderTestImpl.getInstance()); } - @Test(expected = ExtractionException.class) + @Test(expected = IllegalArgumentException.class) public void getPageEmptyUrl() throws Exception { extractor = (YoutubeMixPlaylistExtractor) YouTube .getPlaylistExtractor( @@ -328,7 +332,7 @@ public class YoutubeMixPlaylistExtractorTest { public void getPage() throws Exception { final InfoItemsPage streams = extractor.getPage( new Page("https://www.youtube.com/watch?v=" + VIDEO_ID_OF_CHANNEL - + "&list=RDCM" + CHANNEL_ID + PBJ)); + + "&list=RDCM" + CHANNEL_ID + PBJ, dummyCookie)); assertFalse(streams.getItems().isEmpty()); assertTrue(streams.hasNextPage()); }