Explorar el Código

refactor(icon): simplify findIconURLsFromHTMLDocument

- Don't define the queries before possible early returns
- Check for the presence of the href attribute in the queries, instead of later
  on iterating on the selection
- Add two edge-cases to the tests
- Use EachIter instead of Each, if only to avoid the lambda
jvoisin hace 11 meses
padre
commit
0e9da3a090
Se han modificado 2 ficheros con 16 adiciones y 17 borrados
  1. 15 16
      internal/reader/icon/finder.go
  2. 1 1
      internal/reader/icon/finder_test.go

+ 15 - 16
internal/reader/icon/finder.go

@@ -241,13 +241,6 @@ func resizeIcon(icon *model.Icon) *model.Icon {
 }
 
 func findIconURLsFromHTMLDocument(body io.Reader, contentType string) ([]string, error) {
-	queries := []string{
-		"link[rel='icon' i]",
-		"link[rel='shortcut icon' i]",
-		"link[rel='icon shortcut' i]",
-		"link[rel='apple-touch-icon-precomposed.png']",
-	}
-
 	htmlDocumentReader, err := encoding.NewCharsetReader(body, contentType)
 	if err != nil {
 		return nil, fmt.Errorf("icon: unable to create charset reader: %w", err)
@@ -258,20 +251,26 @@ func findIconURLsFromHTMLDocument(body io.Reader, contentType string) ([]string,
 		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-precomposed.png'][href]",
+	}
+
 	var iconURLs []string
 	for _, query := range queries {
 		slog.Debug("Searching icon URL in HTML document", slog.String("query", query))
 
-		doc.Find(query).Each(func(i int, s *goquery.Selection) {
-			if href, exists := s.Attr("href"); exists {
-				if iconURL := strings.TrimSpace(href); iconURL != "" {
-					iconURLs = append(iconURLs, iconURL)
-					slog.Debug("Found icon URL in HTML document",
-						slog.String("query", query),
-						slog.String("icon_url", iconURL))
-				}
+		for _, s := range doc.Find(query).EachIter() {
+			href, _ := s.Attr("href")
+			if iconURL := strings.TrimSpace(href); iconURL != "" {
+				iconURLs = append(iconURLs, iconURL)
+				slog.Debug("Found icon URL in HTML document",
+					slog.String("query", query),
+					slog.String("icon_url", iconURL))
 			}
-		})
+		}
 	}
 
 	return iconURLs, nil

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

@@ -115,7 +115,7 @@ func TestParseInvalidImageDataURLWithWrongPrefix(t *testing.T) {
 func TestParseDocumentWithWhitespaceIconURL(t *testing.T) {
 	html := `<link rel="shortcut icon" href="
 		/static/img/favicon.ico
-	">`
+	"><link rel='shortcut icon'><link rel='shortcut icon' href="  ">`
 
 	iconURLs, err := findIconURLsFromHTMLDocument(strings.NewReader(html), "text/html")
 	if err != nil {