Browse Source

refactor(subscription): combine all JSON feed mime types in one query

- Look for JSON feeds in one pass instead of two
- Move conditions around to reduce the amount of comparisons
- Edit an existing test to exercise this commit's changes
Julien Voisin 7 months ago
parent
commit
8f2dd02f3f
2 changed files with 14 additions and 15 deletions
  1. 13 14
      internal/reader/subscription/finder.go
  2. 1 1
      internal/reader/subscription/finder_test.go

+ 13 - 14
internal/reader/subscription/finder.go

@@ -124,10 +124,9 @@ func (f *subscriptionFinder) FindSubscriptions(websiteURL, rssBridgeURL string,
 
 func (f *subscriptionFinder) findSubscriptionsFromWebPage(websiteURL, contentType 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,
+		"link[type='application/rss+xml']":                                  parser.FormatRSS,
+		"link[type='application/atom+xml']":                                 parser.FormatAtom,
+		"link[type='application/json'], link[type='application/feed+json']": parser.FormatJSON,
 	}
 
 	htmlDocumentReader, err := encoding.NewCharsetReader(body, contentType)
@@ -154,24 +153,24 @@ func (f *subscriptionFinder) findSubscriptionsFromWebPage(websiteURL, contentTyp
 			subscription := new(subscription)
 			subscription.Type = kind
 
-			if title, exists := s.Attr("title"); exists {
-				subscription.Title = title
+			if feedURL, exists := s.Attr("href"); exists && feedURL != "" {
+				subscription.URL, err = urllib.AbsoluteURL(websiteURL, feedURL)
+				if err != nil {
+					return
+				}
+			} else {
+				return // without an url, there can be no subscription.
 			}
 
-			if feedURL, exists := s.Attr("href"); exists {
-				if feedURL != "" {
-					subscription.URL, err = urllib.AbsoluteURL(websiteURL, feedURL)
-					if err != nil {
-						return
-					}
-				}
+			if title, exists := s.Attr("title"); exists {
+				subscription.Title = title
 			}
 
 			if subscription.Title == "" {
 				subscription.Title = subscription.URL
 			}
 
-			if subscription.URL != "" && !subscriptionURLs[subscription.URL] {
+			if !subscriptionURLs[subscription.URL] {
 				subscriptionURLs[subscription.URL] = true
 				subscriptions = append(subscriptions, subscription)
 			}

+ 1 - 1
internal/reader/subscription/finder_test.go

@@ -390,7 +390,7 @@ func TestParseWebPageWithMultipleFeeds(t *testing.T) {
 	<html>
 		<head>
 			<link href="http://example.org/atom.xml" rel="alternate" type="application/atom+xml" title="Atom Feed">
-			<link href="http://example.org/feed.json" rel="alternate" type="application/feed+json" title="JSON Feed">
+			<link href="http://example.org/feed.json" rel="alternate" type="application/json" title="JSON Feed">
 		</head>
 		<body>
 		</body>