From 2b2dfa098c8d0f9f740ca78574d3ec18f0638ece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Fri, 4 Oct 2024 19:50:54 -0700 Subject: [PATCH] feat: update feed icon during force refresh --- internal/api/icon.go | 2 +- internal/model/feed.go | 3 +- internal/reader/handler/handler.go | 64 +++++------------------ internal/reader/icon/checker.go | 84 ++++++++++++++++++++++++++++++ internal/storage/icon.go | 83 +++++++++++++---------------- 5 files changed, 138 insertions(+), 98 deletions(-) create mode 100644 internal/reader/icon/checker.go diff --git a/internal/api/icon.go b/internal/api/icon.go index 84db8652964..dff76b1d128 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 91c582bd64e..141abd41387 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 04275598c21..aa9d9704538 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 00000000000..4ec63635a9e --- /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 bd407d92a26..c99d3b199c1 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