Prechádzať zdrojové kódy

refactor(icon): remove two loops

1. If the websiteURL is equal to the RootURL, there is no need to check them
   both.
2. Group the icon-looking-queries into a single one.
jvoisin 4 mesiacov pred
rodič
commit
223d29e5d9

+ 27 - 27
internal/reader/icon/finder.go

@@ -66,7 +66,11 @@ func (f *iconFinder) findIcon() (*model.Icon, error) {
 
 	// Try the website URL first, then fall back to the root URL if no icon is found.
 	// The website URL may include a subdirectory (e.g., https://example.org/subfolder/), and icons can be referenced relative to that path.
-	for _, documentURL := range []string{f.websiteURL, urllib.RootURL(f.websiteURL)} {
+	urls := []string{f.websiteURL}
+	if rootURL := urllib.RootURL(f.websiteURL); rootURL != urls[0] {
+		urls = []string{f.websiteURL, rootURL}
+	}
+	for _, documentURL := range urls {
 		if icon, err := f.fetchIconsFromHTMLDocument(documentURL); err != nil {
 			slog.Debug("Unable to fetch icons from HTML document",
 				slog.String("document_url", documentURL),
@@ -257,34 +261,30 @@ func findIconURLsFromHTMLDocument(documentURL string, body io.Reader, contentTyp
 		return nil, fmt.Errorf("icon: unable to read document: %v", err)
 	}
 
-	queries := [...]string{
-		"link[rel='icon' i][href]",
-		"link[rel='shortcut icon' i][href]",
-		"link[rel='icon shortcut' i][href]",
-		"link[rel='apple-touch-icon'][href]",
-	}
+	query := `link[rel='icon' i][href],
+		link[rel='shortcut icon' i][href],
+		link[rel='icon shortcut' i][href],
+		link[rel='apple-touch-icon'][href]`
 
 	var iconURLs []string
-	for _, query := range queries {
-		slog.Debug("Searching icon URL in HTML document", slog.String("query", query))
-
-		for _, s := range doc.Find(query).EachIter() {
-			href, _ := s.Attr("href")
-			href = strings.TrimSpace(href)
-			if href == "" {
-				continue
-			}
-
-			if absoluteIconURL, err := urllib.ResolveToAbsoluteURL(documentURL, href); err != nil {
-				slog.Warn("Unable to convert icon URL to absolute URL", slog.Any("error", err), slog.String("icon_href", href))
-			} else {
-				iconURLs = append(iconURLs, absoluteIconURL)
-				slog.Debug("Found icon URL in HTML document",
-					slog.String("query", query),
-					slog.String("icon_href", href),
-					slog.String("absolute_icon_url", absoluteIconURL),
-				)
-			}
+	slog.Debug("Searching icon URL in HTML document", slog.String("query", query))
+
+	for _, s := range doc.Find(query).EachIter() {
+		href, _ := s.Attr("href")
+		href = strings.TrimSpace(href)
+		if href == "" {
+			continue
+		}
+
+		if absoluteIconURL, err := urllib.ResolveToAbsoluteURL(documentURL, href); err != nil {
+			slog.Warn("Unable to convert icon URL to absolute URL", slog.Any("error", err), slog.String("icon_href", href))
+		} else {
+			iconURLs = append(iconURLs, absoluteIconURL)
+			slog.Debug("Found icon URL in HTML document",
+				slog.String("query", query),
+				slog.String("icon_href", href),
+				slog.String("absolute_icon_url", absoluteIconURL),
+			)
 		}
 	}
 

+ 1 - 3
internal/reader/icon/finder_test.go

@@ -246,12 +246,10 @@ func TestFindIconURLsFromHTMLDocument_DataURLs(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// The function processes queries in order: rel="icon", then rel="shortcut icon", etc.
-	// So both rel="icon" links are found first, then the rel="shortcut icon" link
 	expected := []string{
 		"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChAGAhGAQ+QAAAABJRU5ErkJggg==",
-		"https://example.org/regular-icon.ico",
 		"data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg'></svg>",
+		"https://example.org/regular-icon.ico",
 	}
 
 	if len(iconURLs) != len(expected) {