From 637fb85de07b6c4438f4b151ab388e83abfb3af3 Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Tue, 10 Dec 2024 03:32:59 +0000 Subject: [PATCH] refactor(handler): delay `store.UserByID` as much as possible In internal/reader/handler/handler.go:RefreshFeed, there is a call to store.UserByID pretty early, which is only used for originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language) Its only other usage is in processor.ProcessFeedEntries(store, originalFeed, user, forceRefresh), which is pretty late in RefreshFeed, and only called if there are new items in the feed. It makes sense to only fetch the user's language if the error localization function is used. Calls to `store.UserByID` take around 10% of the CPU time of RefreshFeed in my profiling. This commit also makes `processor.ProcessFeedEntries` take a `userID` instead of a `user`, to make the code a bit more concise. This should close #2984 --- internal/reader/handler/handler.go | 41 +++++++++++++++----------- internal/reader/processor/processor.go | 8 ++++- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/internal/reader/handler/handler.go b/internal/reader/handler/handler.go index 185c57fa..937d7b78 100644 --- a/internal/reader/handler/handler.go +++ b/internal/reader/handler/handler.go @@ -31,11 +31,6 @@ func CreateFeedFromSubscriptionDiscovery(store *storage.Storage, userID int64, f slog.String("feed_url", feedCreationRequest.FeedURL), ) - user, storeErr := store.UserByID(userID) - if storeErr != nil { - return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) - } - if !store.CategoryIDExists(userID, feedCreationRequest.CategoryID) { return nil, locale.NewLocalizedErrorWrapper(ErrCategoryNotFound, "error.category_not_found") } @@ -71,7 +66,7 @@ func CreateFeedFromSubscriptionDiscovery(store *storage.Storage, userID int64, f subscription.WithCategoryID(feedCreationRequest.CategoryID) subscription.CheckedNow() - processor.ProcessFeedEntries(store, subscription, user, true) + processor.ProcessFeedEntries(store, subscription, userID, true) if storeErr := store.CreateFeed(subscription); storeErr != nil { return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) @@ -105,11 +100,6 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model slog.String("feed_url", feedCreationRequest.FeedURL), ) - user, storeErr := store.UserByID(userID) - if storeErr != nil { - return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) - } - if !store.CategoryIDExists(userID, feedCreationRequest.CategoryID) { return nil, locale.NewLocalizedErrorWrapper(ErrCategoryNotFound, "error.category_not_found") } @@ -170,7 +160,7 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model subscription.WithCategoryID(feedCreationRequest.CategoryID) subscription.CheckedNow() - processor.ProcessFeedEntries(store, subscription, user, true) + processor.ProcessFeedEntries(store, subscription, userID, true) if storeErr := store.CreateFeed(subscription); storeErr != nil { return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) @@ -195,11 +185,6 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool slog.Bool("force_refresh", forceRefresh), ) - user, storeErr := store.UserByID(userID) - if storeErr != nil { - return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) - } - originalFeed, storeErr := store.FeedByID(userID, feedID) if storeErr != nil { return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) @@ -256,6 +241,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool if localizedError := responseHandler.LocalizedError(); localizedError != nil { slog.Warn("Unable to fetch feed", slog.String("feed_url", originalFeed.FeedURL), slog.Any("error", localizedError.Error())) + user, storeErr := store.UserByID(userID) + if storeErr != nil { + return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) + } originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language)) store.UpdateFeedError(originalFeed) return localizedError @@ -263,6 +252,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool if store.AnotherFeedURLExists(userID, originalFeed.ID, responseHandler.EffectiveURL()) { localizedError := locale.NewLocalizedErrorWrapper(ErrDuplicatedFeed, "error.duplicated_feed") + user, storeErr := store.UserByID(userID) + if storeErr != nil { + return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) + } originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language)) store.UpdateFeedError(originalFeed) return localizedError @@ -289,6 +282,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool if errors.Is(parseErr, parser.ErrFeedFormatNotDetected) { localizedError = locale.NewLocalizedErrorWrapper(parseErr, "error.feed_format_not_detected", parseErr) } + user, storeErr := store.UserByID(userID) + if storeErr != nil { + return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) + } originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language)) store.UpdateFeedError(originalFeed) @@ -309,13 +306,17 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool ) originalFeed.Entries = updatedFeed.Entries - processor.ProcessFeedEntries(store, originalFeed, user, forceRefresh) + processor.ProcessFeedEntries(store, originalFeed, userID, forceRefresh) // We don't update existing entries when the crawler is enabled (we crawl only inexisting entries). Unless it is forced to refresh updateExistingEntries := forceRefresh || !originalFeed.Crawler newEntries, storeErr := store.RefreshFeedEntries(originalFeed.UserID, originalFeed.ID, originalFeed.Entries, updateExistingEntries) if storeErr != nil { localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) + user, storeErr := store.UserByID(userID) + if storeErr != nil { + return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) + } originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language)) store.UpdateFeedError(originalFeed) return localizedError @@ -359,6 +360,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool if storeErr := store.UpdateFeed(originalFeed); storeErr != nil { localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) + user, storeErr := store.UserByID(userID) + if storeErr != nil { + return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr) + } originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language)) store.UpdateFeedError(originalFeed) return localizedError diff --git a/internal/reader/processor/processor.go b/internal/reader/processor/processor.go index ceae674c..0bdf0f61 100644 --- a/internal/reader/processor/processor.go +++ b/internal/reader/processor/processor.go @@ -28,9 +28,15 @@ import ( var customReplaceRuleRegex = regexp.MustCompile(`rewrite\("([^"]+)"\|"([^"]+)"\)`) // ProcessFeedEntries downloads original web page for entries and apply filters. -func ProcessFeedEntries(store *storage.Storage, feed *model.Feed, user *model.User, forceRefresh bool) { +func ProcessFeedEntries(store *storage.Storage, feed *model.Feed, userID int64, forceRefresh bool) { var filteredEntries model.Entries + user, storeErr := store.UserByID(userID) + if storeErr != nil { + slog.Error("Database error", slog.Any("error", storeErr)) + return + } + // Process older entries first for i := len(feed.Entries) - 1; i >= 0; i-- { entry := feed.Entries[i]