diff --git a/internal/reader/subscription/finder.go b/internal/reader/subscription/finder.go index 02c32150..03f0d2a0 100644 --- a/internal/reader/subscription/finder.go +++ b/internal/reader/subscription/finder.go @@ -70,24 +70,15 @@ func (f *subscriptionFinder) FindSubscriptions(websiteURL, rssBridgeURL string, } // Step 2) Check if the website URL is a YouTube channel. - slog.Debug("Try to detect feeds from YouTube channel page", slog.String("website_url", websiteURL)) - if subscriptions, localizedError := f.findSubscriptionsFromYouTubeChannelPage(websiteURL); localizedError != nil { + slog.Debug("Try to detect feeds for a YouTube page", slog.String("website_url", websiteURL)) + if subscriptions, localizedError := f.findSubscriptionsFromYouTube(websiteURL); localizedError != nil { return nil, localizedError } else if len(subscriptions) > 0 { - slog.Debug("Subscriptions found from YouTube channel page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions)) + slog.Debug("Subscriptions found from YouTube page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions)) return subscriptions, nil } - // Step 3) Check if the website URL is a YouTube playlist. - slog.Debug("Try to detect feeds from YouTube playlist page", slog.String("website_url", websiteURL)) - if subscriptions, localizedError := f.findSubscriptionsFromYouTubePlaylistPage(websiteURL); localizedError != nil { - return nil, localizedError - } else if len(subscriptions) > 0 { - slog.Debug("Subscriptions found from YouTube playlist page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions)) - return subscriptions, nil - } - - // Step 4) Parse web page to find feeds from HTML meta tags. + // Step 3) Parse web page to find feeds from HTML meta tags. slog.Debug("Try to detect feeds from HTML meta tags", slog.String("website_url", websiteURL), slog.String("content_type", responseHandler.ContentType()), @@ -99,7 +90,7 @@ func (f *subscriptionFinder) FindSubscriptions(websiteURL, rssBridgeURL string, return subscriptions, nil } - // Step 5) Check if the website URL can use RSS-Bridge. + // Step 4) Check if the website URL can use RSS-Bridge. if rssBridgeURL != "" { slog.Debug("Try to detect feeds with RSS-Bridge", slog.String("website_url", websiteURL)) if subscriptions, localizedError := f.findSubscriptionsFromRSSBridge(websiteURL, rssBridgeURL, rssBridgeToken); localizedError != nil { @@ -110,7 +101,7 @@ func (f *subscriptionFinder) FindSubscriptions(websiteURL, rssBridgeURL string, } } - // Step 6) Check if the website has a known feed URL. + // Step 5) Check if the website has a known feed URL. slog.Debug("Try to detect feeds from well-known URLs", slog.String("website_url", websiteURL)) if subscriptions, localizedError := f.findSubscriptionsFromWellKnownURLs(websiteURL); localizedError != nil { return nil, localizedError @@ -282,40 +273,26 @@ func (f *subscriptionFinder) findSubscriptionsFromRSSBridge(websiteURL, rssBridg return subscriptions, nil } -func (f *subscriptionFinder) findSubscriptionsFromYouTubeChannelPage(websiteURL string) (Subscriptions, *locale.LocalizedErrorWrapper) { - decodedUrl, err := url.Parse(websiteURL) +func (f *subscriptionFinder) findSubscriptionsFromYouTube(websiteURL string) (Subscriptions, *locale.LocalizedErrorWrapper) { + decodedURL, err := url.Parse(websiteURL) if err != nil { return nil, locale.NewLocalizedErrorWrapper(err, "error.invalid_site_url", err) } - if !strings.HasSuffix(decodedUrl.Host, "youtube.com") { - slog.Debug("This website is not a YouTube page, the regex doesn't match", slog.String("website_url", websiteURL)) + if !strings.HasSuffix(decodedURL.Host, "youtube.com") { + slog.Debug("YouTube feed discovery skipped: not a YouTube domain", slog.String("website_url", websiteURL)) return nil, nil } - - if _, channelID, found := strings.Cut(decodedUrl.Path, "channel/"); found { + if _, channelID, found := strings.Cut(decodedURL.Path, "channel/"); found { feedURL := "https://www.youtube.com/feeds/videos.xml?channel_id=" + channelID - return Subscriptions{NewSubscription(websiteURL, feedURL, parser.FormatAtom)}, nil - } - - return nil, nil -} - -func (f *subscriptionFinder) findSubscriptionsFromYouTubePlaylistPage(websiteURL string) (Subscriptions, *locale.LocalizedErrorWrapper) { - decodedUrl, err := url.Parse(websiteURL) - if err != nil { - return nil, locale.NewLocalizedErrorWrapper(err, "error.invalid_site_url", err) - } - - if !strings.HasSuffix(decodedUrl.Host, "youtube.com") { - slog.Debug("This website is not a YouTube page, the regex doesn't match", slog.String("website_url", websiteURL)) - return nil, nil - } - - if (strings.HasPrefix(decodedUrl.Path, "/watch") && decodedUrl.Query().Has("list")) || strings.HasPrefix(decodedUrl.Path, "/playlist") { - playlistID := decodedUrl.Query().Get("list") - feedURL := "https://www.youtube.com/feeds/videos.xml?playlist_id=" + playlistID - return Subscriptions{NewSubscription(websiteURL, feedURL, parser.FormatAtom)}, nil + return Subscriptions{NewSubscription(decodedURL.String(), feedURL, parser.FormatAtom)}, nil + } + + if strings.HasPrefix(decodedURL.Path, "/watch") || strings.HasPrefix(decodedURL.Path, "/playlist") { + if playlistID := decodedURL.Query().Get("list"); playlistID != "" { + feedURL := "https://www.youtube.com/feeds/videos.xml?playlist_id=" + playlistID + return Subscriptions{NewSubscription(decodedURL.String(), feedURL, parser.FormatAtom)}, nil + } } return nil, nil diff --git a/internal/reader/subscription/finder_test.go b/internal/reader/subscription/finder_test.go index 6b7ae499..324df152 100644 --- a/internal/reader/subscription/finder_test.go +++ b/internal/reader/subscription/finder_test.go @@ -8,96 +8,7 @@ import ( "testing" ) -func TestFindYoutubePlaylistFeed(t *testing.T) { - type testResult struct { - websiteURL string - feedURL string - discoveryError bool - } - - scenarios := []testResult{ - // Video URL - { - websiteURL: "https://www.youtube.com/watch?v=dQw4w9WgXcQ", - feedURL: "", - }, - // Video URL with position argument - { - websiteURL: "https://www.youtube.com/watch?v=dQw4w9WgXcQ&t=1", - feedURL: "", - }, - // Video URL with position argument - { - websiteURL: "https://www.youtube.com/watch?t=1&v=dQw4w9WgXcQ", - feedURL: "", - }, - // Channel URL - { - websiteURL: "https://www.youtube.com/channel/UC-Qj80avWItNRjkZ41rzHyw", - feedURL: "", - }, - // Channel URL with name - { - websiteURL: "https://www.youtube.com/@ABCDEFG", - feedURL: "", - }, - // Playlist URL - { - websiteURL: "https://www.youtube.com/playlist?list=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR", - feedURL: "https://www.youtube.com/feeds/videos.xml?playlist_id=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR", - }, - // Playlist URL with video ID - { - websiteURL: "https://www.youtube.com/watch?v=dQw4w9WgXcQ&list=PLOOwEPgFWm_N42HlCLhqyJ0ZBWr5K1QDM", - feedURL: "https://www.youtube.com/feeds/videos.xml?playlist_id=PLOOwEPgFWm_N42HlCLhqyJ0ZBWr5K1QDM", - }, - // Playlist URL with video ID and index argument - { - websiteURL: "https://www.youtube.com/watch?v=6IutBmRJNLk&list=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR&index=4", - feedURL: "https://www.youtube.com/feeds/videos.xml?playlist_id=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR", - }, - // Non-Youtube URL - { - websiteURL: "https://www.example.com/channel/UC-Qj80avWItNRjkZ41rzHyw", - feedURL: "", - }, - // Invalid URL - { - websiteURL: "https://example|org/", - feedURL: "", - discoveryError: true, - }, - } - - for _, scenario := range scenarios { - subscriptions, localizedError := NewSubscriptionFinder(nil).findSubscriptionsFromYouTubePlaylistPage(scenario.websiteURL) - if scenario.discoveryError { - if localizedError == nil { - t.Fatalf(`Parsing an invalid URL should return an error`) - } - } - - if scenario.feedURL == "" { - if len(subscriptions) > 0 { - t.Fatalf(`Parsing a non-playlist URL should not return any subscription: %q`, scenario.websiteURL) - } - } else { - if localizedError != nil { - t.Fatalf(`Parsing a correctly formatted YouTube playlist page should not return any error: %v`, localizedError) - } - - if len(subscriptions) != 1 { - t.Fatalf(`Incorrect number of subscriptions returned`) - } - - if subscriptions[0].URL != scenario.feedURL { - t.Errorf(`Unexpected Feed, got %s, instead of %s`, subscriptions[0].URL, scenario.feedURL) - } - } - } -} - -func TestFindYoutubeChannelFeed(t *testing.T) { +func TestFindYoutubeFeed(t *testing.T) { type testResult struct { websiteURL string feedURL string @@ -133,16 +44,21 @@ func TestFindYoutubeChannelFeed(t *testing.T) { // Playlist URL { websiteURL: "https://www.youtube.com/playlist?list=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR", - feedURL: "", + feedURL: "https://www.youtube.com/feeds/videos.xml?playlist_id=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR", }, // Playlist URL with video ID { websiteURL: "https://www.youtube.com/watch?v=dQw4w9WgXcQ&list=PLOOwEPgFWm_N42HlCLhqyJ0ZBWr5K1QDM", - feedURL: "", + feedURL: "https://www.youtube.com/feeds/videos.xml?playlist_id=PLOOwEPgFWm_N42HlCLhqyJ0ZBWr5K1QDM", }, // Playlist URL with video ID and index argument { websiteURL: "https://www.youtube.com/watch?v=6IutBmRJNLk&list=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR&index=4", + feedURL: "https://www.youtube.com/feeds/videos.xml?playlist_id=PLOOwEPgFWm_NHcQd9aCi5JXWASHO_n5uR", + }, + // Empty playlist ID parameter + { + websiteURL: "https://www.youtube.com/playlist?list=", feedURL: "", }, // Non-Youtube URL @@ -159,7 +75,7 @@ func TestFindYoutubeChannelFeed(t *testing.T) { } for _, scenario := range scenarios { - subscriptions, localizedError := NewSubscriptionFinder(nil).findSubscriptionsFromYouTubeChannelPage(scenario.websiteURL) + subscriptions, localizedError := NewSubscriptionFinder(nil).findSubscriptionsFromYouTube(scenario.websiteURL) if scenario.discoveryError { if localizedError == nil { t.Fatalf(`Parsing an invalid URL should return an error`) @@ -168,11 +84,11 @@ func TestFindYoutubeChannelFeed(t *testing.T) { if scenario.feedURL == "" { if len(subscriptions) > 0 { - t.Fatalf(`Parsing a non-channel URL should not return any subscription: %q`, scenario.websiteURL) + t.Fatalf(`Parsing an invalid URL should not return any subscription: %q -> %v`, scenario.websiteURL, subscriptions) } } else { if localizedError != nil { - t.Fatalf(`Parsing a correctly formatted YouTube channel page should not return any error: %v`, localizedError) + t.Fatalf(`Parsing a correctly formatted YouTube playlist or channel page should not return any error: %v`, localizedError) } if len(subscriptions) != 1 { @@ -180,7 +96,7 @@ func TestFindYoutubeChannelFeed(t *testing.T) { } if subscriptions[0].URL != scenario.feedURL { - t.Errorf(`Unexpected Feed, got %s, instead of %s`, subscriptions[0].URL, scenario.feedURL) + t.Errorf(`Unexpected feed, got %s, instead of %s`, subscriptions[0].URL, scenario.feedURL) } } }