Refactor feed discovery and avoid an extra HTTP request if the url provided is the feed

internal/reader/subscription/finder.go

@@ -13,6 +13,7 @@ import (
 	"miniflux.app/v2/internal/config"
 	"miniflux.app/v2/internal/integration/rssbridge"
 	"miniflux.app/v2/internal/locale"
+	"miniflux.app/v2/internal/model"
 	"miniflux.app/v2/internal/reader/fetcher"
 	"miniflux.app/v2/internal/reader/parser"
 	"miniflux.app/v2/internal/urllib"
@@ -25,20 +26,28 @@ var (
 	youtubeVideoRegex   = regexp.MustCompile(`youtube\.com/watch\?v=(.*)`)
 )
 
-func FindSubscriptions(websiteURL, userAgent, cookie, username, password string, fetchViaProxy, allowSelfSignedCertificates bool, rssbridgeURL string) (Subscriptions, *locale.LocalizedErrorWrapper) {
-	websiteURL = findYoutubeChannelFeed(websiteURL)
-	websiteURL = parseYoutubeVideoPage(websiteURL)
+type SubscriptionFinder struct {
+	requestBuilder   *fetcher.RequestBuilder
+	feedDownloaded   bool
+	feedResponseInfo *model.FeedCreationRequestFromSubscriptionDiscovery
+}
 
-	requestBuilder := fetcher.NewRequestBuilder()
-	requestBuilder.WithUsernameAndPassword(username, password)
-	requestBuilder.WithUserAgent(userAgent)
-	requestBuilder.WithCookie(cookie)
-	requestBuilder.WithTimeout(config.Opts.HTTPClientTimeout())
-	requestBuilder.WithProxy(config.Opts.HTTPClientProxy())
-	requestBuilder.UseProxy(fetchViaProxy)
-	requestBuilder.IgnoreTLSErrors(allowSelfSignedCertificates)
+func NewSubscriptionFinder(requestBuilder *fetcher.RequestBuilder) *SubscriptionFinder {
+	return &SubscriptionFinder{
+		requestBuilder: requestBuilder,
+	}
+}
 
-	responseHandler := fetcher.NewResponseHandler(requestBuilder.ExecuteRequest(websiteURL))
+func (f *SubscriptionFinder) IsFeedAlreadyDownloaded() bool {
+	return f.feedDownloaded
+}
+
+func (f *SubscriptionFinder) FeedResponseInfo() *model.FeedCreationRequestFromSubscriptionDiscovery {
+	return f.feedResponseInfo
+}
+
+func (f *SubscriptionFinder) FindSubscriptions(websiteURL, rssBridgeURL string) (Subscriptions, *locale.LocalizedErrorWrapper) {
+	responseHandler := fetcher.NewResponseHandler(f.requestBuilder.ExecuteRequest(websiteURL))
 	defer responseHandler.Close()
 
 	if localizedError := responseHandler.LocalizedError(); localizedError != nil {
@@ -52,69 +61,97 @@ func FindSubscriptions(websiteURL, userAgent, cookie, username, password string,
 		return nil, localizedError
 	}
 
-	if format := parser.DetectFeedFormat(string(responseBody)); format != parser.FormatUnknown {
-		var subscriptions Subscriptions
-		subscriptions = append(subscriptions, &Subscription{
-			Title: responseHandler.EffectiveURL(),
-			URL:   responseHandler.EffectiveURL(),
-			Type:  format,
-		})
+	f.feedResponseInfo = &model.FeedCreationRequestFromSubscriptionDiscovery{
+		Content:      bytes.NewReader(responseBody),
+		ETag:         responseHandler.ETag(),
+		LastModified: responseHandler.LastModified(),
+	}
 
+	// Step 1) Check if the website URL is a feed.
+	if feedFormat := parser.DetectFeedFormat(f.feedResponseInfo.Content); feedFormat != parser.FormatUnknown {
+		f.feedDownloaded = true
+		return Subscriptions{NewSubscription(responseHandler.EffectiveURL(), responseHandler.EffectiveURL(), feedFormat)}, nil
+	}
+
+	// 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))
+	subscriptions, localizedError := f.FindSubscriptionsFromYouTubeChannelPage(websiteURL)
+	if localizedError != nil {
+		return nil, localizedError
+	}
+
+	if len(subscriptions) > 0 {
+		slog.Debug("Subscriptions found from YouTube channel page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions))
 		return subscriptions, nil
 	}
 
-	subscriptions, localizedError := parseWebPage(responseHandler.EffectiveURL(), bytes.NewReader(responseBody))
-	if localizedError != nil || subscriptions != nil {
-		return subscriptions, localizedError
+	// Step 3) Check if the website URL is a YouTube video.
+	slog.Debug("Try to detect feeds from YouTube video page", slog.String("website_url", websiteURL))
+	subscriptions, localizedError = f.FindSubscriptionsFromYouTubeVideoPage(websiteURL)
+	if localizedError != nil {
+		return nil, localizedError
 	}
 
-	if rssbridgeURL != "" {
-		slog.Debug("Trying to detect feeds using RSS-Bridge",
-			slog.String("website_url", websiteURL),
-			slog.String("rssbridge_url", rssbridgeURL),
-		)
+	if len(subscriptions) > 0 {
+		slog.Debug("Subscriptions found from YouTube video page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions))
+		return subscriptions, nil
+	}
 
-		bridges, err := rssbridge.DetectBridges(rssbridgeURL, websiteURL)
-		if err != nil {
-			return nil, locale.NewLocalizedErrorWrapper(err, "error.unable_to_detect_rssbridge", err)
+	// Step 4) 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))
+	subscriptions, localizedError = f.FindSubscriptionsFromWebPage(websiteURL, bytes.NewReader(responseBody))
+	if localizedError != nil {
+		return nil, localizedError
+	}
+
+	if len(subscriptions) > 0 {
+		slog.Debug("Subscriptions found from web page", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions))
+		return subscriptions, nil
+	}
+
+	// Step 5) 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))
+		subscriptions, localizedError := f.FindSubscriptionsFromRSSBridge(websiteURL, rssBridgeURL)
+		if localizedError != nil {
+			return nil, localizedError
 		}
 
-		slog.Debug("RSS-Bridge results",
-			slog.String("website_url", websiteURL),
-			slog.String("rssbridge_url", rssbridgeURL),
-			slog.Int("nb_bridges", len(bridges)),
-		)
-
-		if len(bridges) > 0 {
-			var subscriptions Subscriptions
-			for _, bridge := range bridges {
-				subscriptions = append(subscriptions, &Subscription{
-					Title: bridge.BridgeMeta.Name,
-					URL:   bridge.URL,
-					Type:  "atom",
-				})
-			}
+		if len(subscriptions) > 0 {
+			slog.Debug("Subscriptions found from RSS-Bridge", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions))
 			return subscriptions, nil
 		}
 	}
 
-	return tryWellKnownUrls(websiteURL, userAgent, cookie, username, password, fetchViaProxy, allowSelfSignedCertificates)
-}
-
-func parseWebPage(websiteURL string, data io.Reader) (Subscriptions, *locale.LocalizedErrorWrapper) {
-	var subscriptions Subscriptions
-	queries := map[string]string{
-		"link[type='application/rss+xml']":   "rss",
-		"link[type='application/atom+xml']":  "atom",
-		"link[type='application/json']":      "json",
-		"link[type='application/feed+json']": "json",
+	// Step 6) Check if the website has a known feed URL.
+	slog.Debug("Try to detect feeds from well-known URLs", slog.String("website_url", websiteURL))
+	subscriptions, localizedError = f.FindSubscriptionsFromWellKnownURLs(websiteURL)
+	if localizedError != nil {
+		return nil, localizedError
 	}
 
-	doc, err := goquery.NewDocumentFromReader(data)
+	if len(subscriptions) > 0 {
+		slog.Debug("Subscriptions found with well-known URLs", slog.String("website_url", websiteURL), slog.Any("subscriptions", subscriptions))
+		return subscriptions, nil
+	}
+
+	return nil, nil
+}
+
+func (f *SubscriptionFinder) FindSubscriptionsFromWebPage(websiteURL string, body io.Reader) (Subscriptions, *locale.LocalizedErrorWrapper) {
+	queries := map[string]string{
+		"link[type='application/rss+xml']":   parser.FormatRSS,
+		"link[type='application/atom+xml']":  parser.FormatAtom,
+		"link[type='application/json']":      parser.FormatJSON,
+		"link[type='application/feed+json']": parser.FormatJSON,
+	}
+
+	doc, err := goquery.NewDocumentFromReader(body)
 	if err != nil {
 		return nil, locale.NewLocalizedErrorWrapper(err, "error.unable_to_parse_html_document", err)
 	}
 
+	var subscriptions Subscriptions
 	for query, kind := range queries {
 		doc.Find(query).Each(func(i int, s *goquery.Selection) {
 			subscription := new(Subscription)
@@ -143,52 +180,13 @@ func parseWebPage(websiteURL string, data io.Reader) (Subscriptions, *locale.Loc
 	return subscriptions, nil
 }
 
-func findYoutubeChannelFeed(websiteURL string) string {
-	matches := youtubeChannelRegex.FindStringSubmatch(websiteURL)
-
-	if len(matches) == 2 {
-		return fmt.Sprintf(`https://www.youtube.com/feeds/videos.xml?channel_id=%s`, matches[1])
-	}
-	return websiteURL
-}
-
-func parseYoutubeVideoPage(websiteURL string) string {
-	if !youtubeVideoRegex.MatchString(websiteURL) {
-		return websiteURL
-	}
-
-	requestBuilder := fetcher.NewRequestBuilder()
-	requestBuilder.WithTimeout(config.Opts.HTTPClientTimeout())
-	requestBuilder.WithProxy(config.Opts.HTTPClientProxy())
-
-	responseHandler := fetcher.NewResponseHandler(requestBuilder.ExecuteRequest(websiteURL))
-	defer responseHandler.Close()
-
-	if localizedError := responseHandler.LocalizedError(); localizedError != nil {
-		slog.Warn("Unable to find subscriptions", slog.String("website_url", websiteURL), slog.Any("error", localizedError.Error()))
-		return websiteURL
-	}
-
-	doc, docErr := goquery.NewDocumentFromReader(responseHandler.Body(config.Opts.HTTPClientMaxBodySize()))
-	if docErr != nil {
-		return websiteURL
-	}
-
-	if channelID, exists := doc.Find(`meta[itemprop="channelId"]`).First().Attr("content"); exists {
-		return fmt.Sprintf(`https://www.youtube.com/feeds/videos.xml?channel_id=%s`, channelID)
-	}
-
-	return websiteURL
-}
-
-func tryWellKnownUrls(websiteURL, userAgent, cookie, username, password string, fetchViaProxy, allowSelfSignedCertificates bool) (Subscriptions, *locale.LocalizedErrorWrapper) {
-	var subscriptions Subscriptions
+func (f *SubscriptionFinder) FindSubscriptionsFromWellKnownURLs(websiteURL string) (Subscriptions, *locale.LocalizedErrorWrapper) {
 	knownURLs := map[string]string{
-		"atom.xml": "atom",
-		"feed.xml": "atom",
-		"feed/":    "atom",
-		"rss.xml":  "rss",
-		"rss/":     "rss",
+		"atom.xml": parser.FormatAtom,
+		"feed.xml": parser.FormatAtom,
+		"feed/":    parser.FormatAtom,
+		"rss.xml":  parser.FormatRSS,
+		"rss/":     parser.FormatRSS,
 	}
 
 	websiteURLRoot := urllib.RootURL(websiteURL)
@@ -203,6 +201,7 @@ func tryWellKnownUrls(websiteURL, userAgent, cookie, username, password string,
 		baseURLs = append(baseURLs, websiteURL)
 	}
 
+	var subscriptions Subscriptions
 	for _, baseURL := range baseURLs {
 		for knownURL, kind := range knownURLs {
 			fullURL, err := urllib.AbsoluteURL(baseURL, knownURL)
@@ -210,21 +209,12 @@ func tryWellKnownUrls(websiteURL, userAgent, cookie, username, password string,
 				continue
 			}
 
-			requestBuilder := fetcher.NewRequestBuilder()
-			requestBuilder.WithUsernameAndPassword(username, password)
-			requestBuilder.WithUserAgent(userAgent)
-			requestBuilder.WithCookie(cookie)
-			requestBuilder.WithTimeout(config.Opts.HTTPClientTimeout())
-			requestBuilder.WithProxy(config.Opts.HTTPClientProxy())
-			requestBuilder.UseProxy(fetchViaProxy)
-			requestBuilder.IgnoreTLSErrors(allowSelfSignedCertificates)
-
 			// Some websites redirects unknown URLs to the home page.
 			// As result, the list of known URLs is returned to the subscription list.
 			// We don't want the user to choose between invalid feed URLs.
-			requestBuilder.WithoutRedirects()
+			f.requestBuilder.WithoutRedirects()
 
-			responseHandler := fetcher.NewResponseHandler(requestBuilder.ExecuteRequest(fullURL))
+			responseHandler := fetcher.NewResponseHandler(f.requestBuilder.ExecuteRequest(fullURL))
 			defer responseHandler.Close()
 
 			if localizedError := responseHandler.LocalizedError(); localizedError != nil {
@@ -241,3 +231,75 @@ func tryWellKnownUrls(websiteURL, userAgent, cookie, username, password string,
 
 	return subscriptions, nil
 }
+
+func (f *SubscriptionFinder) FindSubscriptionsFromRSSBridge(websiteURL, rssBridgeURL string) (Subscriptions, *locale.LocalizedErrorWrapper) {
+	slog.Debug("Trying to detect feeds using RSS-Bridge",
+		slog.String("website_url", websiteURL),
+		slog.String("rssbridge_url", rssBridgeURL),
+	)
+
+	bridges, err := rssbridge.DetectBridges(rssBridgeURL, websiteURL)
+	if err != nil {
+		return nil, locale.NewLocalizedErrorWrapper(err, "error.unable_to_detect_rssbridge", err)
+	}
+
+	slog.Debug("RSS-Bridge results",
+		slog.String("website_url", websiteURL),
+		slog.String("rssbridge_url", rssBridgeURL),
+		slog.Int("nb_bridges", len(bridges)),
+	)
+
+	if len(bridges) == 0 {
+		return nil, nil
+	}
+
+	var subscriptions Subscriptions
+	for _, bridge := range bridges {
+		subscriptions = append(subscriptions, &Subscription{
+			Title: bridge.BridgeMeta.Name,
+			URL:   bridge.URL,
+			Type:  parser.FormatAtom,
+		})
+	}
+
+	return subscriptions, nil
+}
+
+func (f *SubscriptionFinder) FindSubscriptionsFromYouTubeChannelPage(websiteURL string) (Subscriptions, *locale.LocalizedErrorWrapper) {
+	matches := youtubeChannelRegex.FindStringSubmatch(websiteURL)
+
+	if len(matches) == 2 {
+		feedURL := fmt.Sprintf(`https://www.youtube.com/feeds/videos.xml?channel_id=%s`, matches[1])
+		return Subscriptions{NewSubscription(websiteURL, feedURL, parser.FormatAtom)}, nil
+	}
+
+	slog.Debug("This website is not a YouTube channel page, the regex doesn't match", slog.String("website_url", websiteURL))
+
+	return nil, nil
+}
+
+func (f *SubscriptionFinder) FindSubscriptionsFromYouTubeVideoPage(websiteURL string) (Subscriptions, *locale.LocalizedErrorWrapper) {
+	if !youtubeVideoRegex.MatchString(websiteURL) {
+		slog.Debug("This website is not a YouTube video page, the regex doesn't match", slog.String("website_url", websiteURL))
+		return nil, nil
+	}
+
+	responseHandler := fetcher.NewResponseHandler(f.requestBuilder.ExecuteRequest(websiteURL))
+	defer responseHandler.Close()
+
+	if localizedError := responseHandler.LocalizedError(); localizedError != nil {
+		return nil, localizedError
+	}
+
+	doc, docErr := goquery.NewDocumentFromReader(responseHandler.Body(config.Opts.HTTPClientMaxBodySize()))
+	if docErr != nil {
+		return nil, locale.NewLocalizedErrorWrapper(docErr, "error.unable_to_parse_html_document", docErr)
+	}
+
+	if channelID, exists := doc.Find(`meta[itemprop="channelId"]`).First().Attr("content"); exists {
+		feedURL := fmt.Sprintf(`https://www.youtube.com/feeds/videos.xml?channel_id=%s`, channelID)
+		return Subscriptions{NewSubscription(websiteURL, feedURL, parser.FormatAtom)}, nil
+	}
+
+	return nil, nil
+}

internal/reader/subscription/finder_test.go

@@ -11,13 +11,20 @@ import (
 func TestFindYoutubeChannelFeed(t *testing.T) {
 	scenarios := map[string]string{
 		"https://www.youtube.com/channel/UC-Qj80avWItNRjkZ41rzHyw": "https://www.youtube.com/feeds/videos.xml?channel_id=UC-Qj80avWItNRjkZ41rzHyw",
-		"http://example.org/feed":                                  "http://example.org/feed",
 	}
 
 	for websiteURL, expectedFeedURL := range scenarios {
-		result := findYoutubeChannelFeed(websiteURL)
-		if result != expectedFeedURL {
-			t.Errorf(`Unexpected Feed, got %s, instead of %s`, result, expectedFeedURL)
+		subscriptions, localizedError := NewSubscriptionFinder(nil).FindSubscriptionsFromYouTubeChannelPage(websiteURL)
+		if localizedError != nil {
+			t.Fatalf(`Parsing a correctly formatted YouTube channel page should not return any error: %v`, localizedError)
+		}
+
+		if len(subscriptions) != 1 {
+			t.Fatal(`Incorrect number of subscriptions returned`)
+		}
+
+		if subscriptions[0].URL != expectedFeedURL {
+			t.Errorf(`Unexpected Feed, got %s, instead of %s`, subscriptions[0].URL, expectedFeedURL)
 		}
 	}
 }
@@ -33,7 +40,7 @@ func TestParseWebPageWithRssFeed(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -66,7 +73,7 @@ func TestParseWebPageWithAtomFeed(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -99,7 +106,7 @@ func TestParseWebPageWithJSONFeed(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -132,7 +139,7 @@ func TestParseWebPageWithOldJSONFeedMimeType(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -165,7 +172,7 @@ func TestParseWebPageWithRelativeFeedURL(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -198,7 +205,7 @@ func TestParseWebPageWithEmptyTitle(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -232,7 +239,7 @@ func TestParseWebPageWithMultipleFeeds(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -253,7 +260,7 @@ func TestParseWebPageWithEmptyFeedURL(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}
@@ -274,7 +281,7 @@ func TestParseWebPageWithNoHref(t *testing.T) {
 		</body>
 	</html>`
 
-	subscriptions, err := parseWebPage("http://example.org/", strings.NewReader(htmlPage))
+	subscriptions, err := NewSubscriptionFinder(nil).FindSubscriptionsFromWebPage("http://example.org/", strings.NewReader(htmlPage))
 	if err != nil {
 		t.Fatalf(`Parsing a correctly formatted HTML page should not return any error: %v`, err)
 	}

internal/reader/subscription/subscription.go

@@ -12,6 +12,10 @@ type Subscription struct {
 	Type  string `json:"type"`
 }
 
+func NewSubscription(title, url, kind string) *Subscription {
+	return &Subscription{Title: title, URL: url, Type: kind}
+}
+
 func (s Subscription) String() string {
 	return fmt.Sprintf(`Title="%s", URL="%s", Type="%s"`, s.Title, s.URL, s.Type)
 }