feat: update feed icon during force refresh

5 files changed, 138 insertions, 98 deletions

diff --git a/internal/api/icon.go b/internal/api/icon.go
index 84db8652..dff76b1d 100644
--- a/internal/api/icon.go
+++ b/internal/api/icon.go
@@ -13,7 +13,7 @@ import (
 func (h *handler) getIconByFeedID(w http.ResponseWriter, r *http.Request) {
 	feedID := request.RouteInt64Param(r, "feedID")
 
-	if !h.store.HasIcon(feedID) {
+	if !h.store.HasFeedIcon(feedID) {
 		json.NotFound(w, r)
 		return
 	}
diff --git a/internal/model/feed.go b/internal/model/feed.go
index 91c582bd..141abd41 100644
--- a/internal/model/feed.go
+++ b/internal/model/feed.go
@@ -56,11 +56,12 @@ type Feed struct {
 	NtfyEnabled                 bool      `json:"ntfy_enabled"`
 	NtfyPriority                int       `json:"ntfy_priority"`
 
-	// Non persisted attributes
+	// Non-persisted attributes
 	Category *Category `json:"category,omitempty"`
 	Icon     *FeedIcon `json:"icon"`
 	Entries  Entries   `json:"entries,omitempty"`
 
+	// Internal attributes (not exposed in the API and not persisted in the database)
 	TTL                    int    `json:"-"`
 	IconURL                string `json:"-"`
 	UnreadCount            int    `json:"-"`
diff --git a/internal/reader/handler/handler.go b/internal/reader/handler/handler.go
index 04275598..aa9d9704 100644
--- a/internal/reader/handler/handler.go
+++ b/internal/reader/handler/handler.go
@@ -93,13 +93,7 @@ func CreateFeedFromSubscriptionDiscovery(store *storage.Storage, userID int64, f
 	requestBuilder.IgnoreTLSErrors(feedCreationRequest.AllowSelfSignedCertificates)
 	requestBuilder.DisableHTTP2(feedCreationRequest.DisableHTTP2)
 
-	checkFeedIcon(
-		store,
-		requestBuilder,
-		subscription.ID,
-		subscription.SiteURL,
-		subscription.IconURL,
-	)
+	icon.NewIconChecker(store, subscription).UpdateOrCreateFeedIcon()
 
 	return subscription, nil
 }
@@ -188,13 +182,8 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model
 		slog.String("feed_url", subscription.FeedURL),
 	)
 
-	checkFeedIcon(
-		store,
-		requestBuilder,
-		subscription.ID,
-		subscription.SiteURL,
-		subscription.IconURL,
-	)
+	icon.NewIconChecker(store, subscription).UpdateOrCreateFeedIcon()
+
 	return subscription, nil
 }
 
@@ -270,6 +259,8 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 		slog.Debug("Feed modified",
 			slog.Int64("user_id", userID),
 			slog.Int64("feed_id", feedID),
+			slog.String("etag_header", originalFeed.EtagHeader),
+			slog.String("last_modified_header", originalFeed.LastModifiedHeader),
 		)
 
 		responseBody, localizedError := responseHandler.ReadBody(config.Opts.HTTPClientMaxBodySize())
@@ -293,8 +284,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 
 		// If the feed has a TTL defined, we use it to make sure we don't check it too often.
 		newTTL = updatedFeed.TTL
+
 		// Set the next check at with updated arguments.
 		originalFeed.ScheduleNextCheck(weeklyEntryCount, newTTL)
+
 		slog.Debug("Updated next check date",
 			slog.Int64("user_id", userID),
 			slog.Int64("feed_id", feedID),
@@ -329,18 +322,18 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 		originalFeed.EtagHeader = responseHandler.ETag()
 		originalFeed.LastModifiedHeader = responseHandler.LastModified()
 
-		checkFeedIcon(
-			store,
-			requestBuilder,
-			originalFeed.ID,
-			originalFeed.SiteURL,
-			updatedFeed.IconURL,
-		)
+		iconChecker := icon.NewIconChecker(store, originalFeed)
+		if forceRefresh {
+			iconChecker.UpdateOrCreateFeedIcon()
+		} else {
+			iconChecker.CreateFeedIconIfMissing()
+		}
 	} else {
 		slog.Debug("Feed not modified",
 			slog.Int64("user_id", userID),
 			slog.Int64("feed_id", feedID),
 		)
+
 		// Last-Modified may be updated even if ETag is not. In this case, per
 		// RFC9111 sections 3.2 and 4.3.4, the stored response must be updated.
 		if responseHandler.LastModified() != "" {
@@ -359,32 +352,3 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 
 	return nil
 }
-
-func checkFeedIcon(store *storage.Storage, requestBuilder *fetcher.RequestBuilder, feedID int64, websiteURL, feedIconURL string) {
-	if !store.HasIcon(feedID) {
-		iconFinder := icon.NewIconFinder(requestBuilder, websiteURL, feedIconURL)
-		if icon, err := iconFinder.FindIcon(); err != nil {
-			slog.Debug("Unable to find feed icon",
-				slog.Int64("feed_id", feedID),
-				slog.String("website_url", websiteURL),
-				slog.String("feed_icon_url", feedIconURL),
-				slog.Any("error", err),
-			)
-		} else if icon == nil {
-			slog.Debug("No icon found",
-				slog.Int64("feed_id", feedID),
-				slog.String("website_url", websiteURL),
-				slog.String("feed_icon_url", feedIconURL),
-			)
-		} else {
-			if err := store.CreateFeedIcon(feedID, icon); err != nil {
-				slog.Error("Unable to store feed icon",
-					slog.Int64("feed_id", feedID),
-					slog.String("website_url", websiteURL),
-					slog.String("feed_icon_url", feedIconURL),
-					slog.Any("error", err),
-				)
-			}
-		}
-	}
-}
diff --git a/internal/reader/icon/checker.go b/internal/reader/icon/checker.go
new file mode 100644
index 00000000..4ec63635
--- /dev/null
+++ b/internal/reader/icon/checker.go
@@ -0,0 +1,84 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package icon // import "miniflux.app/v2/internal/reader/icon"
+
+import (
+	"log/slog"
+
+	"miniflux.app/v2/internal/config"
+	"miniflux.app/v2/internal/model"
+	"miniflux.app/v2/internal/reader/fetcher"
+	"miniflux.app/v2/internal/storage"
+)
+
+type IconChecker struct {
+	store *storage.Storage
+	feed  *model.Feed
+}
+
+func NewIconChecker(store *storage.Storage, feed *model.Feed) *IconChecker {
+	return &IconChecker{
+		store: store,
+		feed:  feed,
+	}
+}
+
+func (c *IconChecker) fetchAndStoreIcon() {
+	requestBuilder := fetcher.NewRequestBuilder()
+	requestBuilder.WithUserAgent(c.feed.UserAgent, config.Opts.HTTPClientUserAgent())
+	requestBuilder.WithCookie(c.feed.Cookie)
+	requestBuilder.WithTimeout(config.Opts.HTTPClientTimeout())
+	requestBuilder.WithProxy(config.Opts.HTTPClientProxy())
+	requestBuilder.UseProxy(c.feed.FetchViaProxy)
+	requestBuilder.IgnoreTLSErrors(c.feed.AllowSelfSignedCertificates)
+	requestBuilder.DisableHTTP2(c.feed.DisableHTTP2)
+
+	iconFinder := NewIconFinder(requestBuilder, c.feed.FeedURL, c.feed.IconURL)
+	if icon, err := iconFinder.FindIcon(); err != nil {
+		slog.Debug("Unable to find feed icon",
+			slog.Int64("feed_id", c.feed.ID),
+			slog.String("website_url", c.feed.FeedURL),
+			slog.String("feed_icon_url", c.feed.IconURL),
+			slog.Any("error", err),
+		)
+	} else if icon == nil {
+		slog.Debug("No icon found",
+			slog.Int64("feed_id", c.feed.ID),
+			slog.String("website_url", c.feed.FeedURL),
+			slog.String("feed_icon_url", c.feed.IconURL),
+		)
+	} else {
+		if err := c.store.StoreFeedIcon(c.feed.ID, icon); err != nil {
+			slog.Error("Unable to store feed icon",
+				slog.Int64("feed_id", c.feed.ID),
+				slog.String("website_url", c.feed.FeedURL),
+				slog.String("feed_icon_url", c.feed.IconURL),
+				slog.Any("error", err),
+			)
+		} else {
+			slog.Debug("Feed icon stored",
+				slog.Int64("feed_id", c.feed.ID),
+				slog.String("website_url", c.feed.FeedURL),
+				slog.String("feed_icon_url", c.feed.IconURL),
+				slog.Int64("icon_id", icon.ID),
+				slog.String("icon_hash", icon.Hash),
+			)
+		}
+	}
+}
+
+func (c *IconChecker) CreateFeedIconIfMissing() {
+	if c.store.HasFeedIcon(c.feed.ID) {
+		slog.Debug("Feed icon already exists",
+			slog.Int64("feed_id", c.feed.ID),
+		)
+		return
+	}
+
+	c.fetchAndStoreIcon()
+}
+
+func (c *IconChecker) UpdateOrCreateFeedIcon() {
+	c.fetchAndStoreIcon()
+}
diff --git a/internal/storage/icon.go b/internal/storage/icon.go
index bd407d92..c99d3b19 100644
--- a/internal/storage/icon.go
+++ b/internal/storage/icon.go
@@ -11,8 +11,8 @@ import (
 	"miniflux.app/v2/internal/model"
 )
 
-// HasIcon checks if the given feed has an icon.
-func (s *Storage) HasIcon(feedID int64) bool {
+// HasFeedIcon checks if the given feed has an icon.
+func (s *Storage) HasFeedIcon(feedID int64) bool {
 	var result bool
 	query := `SELECT true FROM feed_icons WHERE feed_id=$1`
 	s.db.QueryRow(query, feedID).Scan(&result)
@@ -57,59 +57,50 @@ func (s *Storage) IconByFeedID(userID, feedID int64) (*model.Icon, error) {
 	return &icon, nil
 }
 
-// IconByHash returns an icon by the hash (checksum).
-func (s *Storage) IconByHash(icon *model.Icon) error {
-	err := s.db.QueryRow(`SELECT id FROM icons WHERE hash=$1`, icon.Hash).Scan(&icon.ID)
-	if err == sql.ErrNoRows {
-		return nil
-	} else if err != nil {
-		return fmt.Errorf(`store: unable to fetch icon by hash %q: %v`, icon.Hash, err)
+// StoreFeedIcon creates or updates a feed icon.
+func (s *Storage) StoreFeedIcon(feedID int64, icon *model.Icon) error {
+	tx, err := s.db.Begin()
+	if err != nil {
+		return fmt.Errorf(`store: unable to start transaction: %v`, err)
 	}
 
-	return nil
-}
+	if err := tx.QueryRow(`SELECT id FROM icons WHERE hash=$1`, icon.Hash).Scan(&icon.ID); err == sql.ErrNoRows {
+		query := `
+			INSERT INTO icons
+				(hash, mime_type, content)
+			VALUES
+				($1, $2, $3)
+			RETURNING
+				id
+		`
+		err := tx.QueryRow(
+			query,
+			icon.Hash,
+			normalizeMimeType(icon.MimeType),
+			icon.Content,
+		).Scan(&icon.ID)
 
-// CreateIcon creates a new icon.
-func (s *Storage) CreateIcon(icon *model.Icon) error {
-	query := `
-		INSERT INTO icons
-			(hash, mime_type, content)
-		VALUES
-			($1, $2, $3)
-		RETURNING
-			id
-	`
-	err := s.db.QueryRow(
-		query,
-		icon.Hash,
-		normalizeMimeType(icon.MimeType),
-		icon.Content,
-	).Scan(&icon.ID)
-
-	if err != nil {
-		return fmt.Errorf(`store: unable to create icon: %v`, err)
+		if err != nil {
+			tx.Rollback()
+			return fmt.Errorf(`store: unable to create icon: %v`, err)
+		}
+	} else if err != nil {
+		tx.Rollback()
+		return fmt.Errorf(`store: unable to fetch icon by hash %q: %v`, icon.Hash, err)
 	}
 
-	return nil
-}
-
-// CreateFeedIcon creates an icon and associate the icon to the given feed.
-func (s *Storage) CreateFeedIcon(feedID int64, icon *model.Icon) error {
-	err := s.IconByHash(icon)
-	if err != nil {
-		return err
+	if _, err := tx.Exec(`DELETE FROM feed_icons WHERE feed_id=$1`, feedID); err != nil {
+		tx.Rollback()
+		return fmt.Errorf(`store: unable to delete feed icon: %v`, err)
 	}
 
-	if icon.ID == 0 {
-		err := s.CreateIcon(icon)
-		if err != nil {
-			return err
-		}
+	if _, err := tx.Exec(`INSERT INTO feed_icons (feed_id, icon_id) VALUES ($1, $2)`, feedID, icon.ID); err != nil {
+		tx.Rollback()
+		return fmt.Errorf(`store: unable to associate feed and icon: %v`, err)
 	}
 
-	_, err = s.db.Exec(`INSERT INTO feed_icons (feed_id, icon_id) VALUES ($1, $2)`, feedID, icon.ID)
-	if err != nil {
-		return fmt.Errorf(`store: unable to create feed icon: %v`, err)
+	if err := tx.Commit(); err != nil {
+		return fmt.Errorf(`store: unable to commit transaction: %v`, err)
 	}
 
 	return nil