Skip to content

Commit 637fb85

Browse files
authored
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
1 parent 02c6d14 commit 637fb85

File tree

2 files changed

+30
-19
lines changed

2 files changed

+30
-19
lines changed

internal/reader/handler/handler.go

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,6 @@ func CreateFeedFromSubscriptionDiscovery(store *storage.Storage, userID int64, f
3131
slog.String("feed_url", feedCreationRequest.FeedURL),
3232
)
3333

34-
user, storeErr := store.UserByID(userID)
35-
if storeErr != nil {
36-
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
37-
}
38-
3934
if !store.CategoryIDExists(userID, feedCreationRequest.CategoryID) {
4035
return nil, locale.NewLocalizedErrorWrapper(ErrCategoryNotFound, "error.category_not_found")
4136
}
@@ -71,7 +66,7 @@ func CreateFeedFromSubscriptionDiscovery(store *storage.Storage, userID int64, f
7166
subscription.WithCategoryID(feedCreationRequest.CategoryID)
7267
subscription.CheckedNow()
7368

74-
processor.ProcessFeedEntries(store, subscription, user, true)
69+
processor.ProcessFeedEntries(store, subscription, userID, true)
7570

7671
if storeErr := store.CreateFeed(subscription); storeErr != nil {
7772
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
@@ -105,11 +100,6 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model
105100
slog.String("feed_url", feedCreationRequest.FeedURL),
106101
)
107102

108-
user, storeErr := store.UserByID(userID)
109-
if storeErr != nil {
110-
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
111-
}
112-
113103
if !store.CategoryIDExists(userID, feedCreationRequest.CategoryID) {
114104
return nil, locale.NewLocalizedErrorWrapper(ErrCategoryNotFound, "error.category_not_found")
115105
}
@@ -170,7 +160,7 @@ func CreateFeed(store *storage.Storage, userID int64, feedCreationRequest *model
170160
subscription.WithCategoryID(feedCreationRequest.CategoryID)
171161
subscription.CheckedNow()
172162

173-
processor.ProcessFeedEntries(store, subscription, user, true)
163+
processor.ProcessFeedEntries(store, subscription, userID, true)
174164

175165
if storeErr := store.CreateFeed(subscription); storeErr != nil {
176166
return nil, locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
@@ -195,11 +185,6 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
195185
slog.Bool("force_refresh", forceRefresh),
196186
)
197187

198-
user, storeErr := store.UserByID(userID)
199-
if storeErr != nil {
200-
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
201-
}
202-
203188
originalFeed, storeErr := store.FeedByID(userID, feedID)
204189
if storeErr != nil {
205190
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
@@ -256,13 +241,21 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
256241

257242
if localizedError := responseHandler.LocalizedError(); localizedError != nil {
258243
slog.Warn("Unable to fetch feed", slog.String("feed_url", originalFeed.FeedURL), slog.Any("error", localizedError.Error()))
244+
user, storeErr := store.UserByID(userID)
245+
if storeErr != nil {
246+
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
247+
}
259248
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
260249
store.UpdateFeedError(originalFeed)
261250
return localizedError
262251
}
263252

264253
if store.AnotherFeedURLExists(userID, originalFeed.ID, responseHandler.EffectiveURL()) {
265254
localizedError := locale.NewLocalizedErrorWrapper(ErrDuplicatedFeed, "error.duplicated_feed")
255+
user, storeErr := store.UserByID(userID)
256+
if storeErr != nil {
257+
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
258+
}
266259
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
267260
store.UpdateFeedError(originalFeed)
268261
return localizedError
@@ -289,6 +282,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
289282
if errors.Is(parseErr, parser.ErrFeedFormatNotDetected) {
290283
localizedError = locale.NewLocalizedErrorWrapper(parseErr, "error.feed_format_not_detected", parseErr)
291284
}
285+
user, storeErr := store.UserByID(userID)
286+
if storeErr != nil {
287+
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
288+
}
292289

293290
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
294291
store.UpdateFeedError(originalFeed)
@@ -309,13 +306,17 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
309306
)
310307

311308
originalFeed.Entries = updatedFeed.Entries
312-
processor.ProcessFeedEntries(store, originalFeed, user, forceRefresh)
309+
processor.ProcessFeedEntries(store, originalFeed, userID, forceRefresh)
313310

314311
// We don't update existing entries when the crawler is enabled (we crawl only inexisting entries). Unless it is forced to refresh
315312
updateExistingEntries := forceRefresh || !originalFeed.Crawler
316313
newEntries, storeErr := store.RefreshFeedEntries(originalFeed.UserID, originalFeed.ID, originalFeed.Entries, updateExistingEntries)
317314
if storeErr != nil {
318315
localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
316+
user, storeErr := store.UserByID(userID)
317+
if storeErr != nil {
318+
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
319+
}
319320
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
320321
store.UpdateFeedError(originalFeed)
321322
return localizedError
@@ -359,6 +360,10 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
359360

360361
if storeErr := store.UpdateFeed(originalFeed); storeErr != nil {
361362
localizedError := locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
363+
user, storeErr := store.UserByID(userID)
364+
if storeErr != nil {
365+
return locale.NewLocalizedErrorWrapper(storeErr, "error.database_error", storeErr)
366+
}
362367
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language))
363368
store.UpdateFeedError(originalFeed)
364369
return localizedError

internal/reader/processor/processor.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,15 @@ import (
2828
var customReplaceRuleRegex = regexp.MustCompile(`rewrite\("([^"]+)"\|"([^"]+)"\)`)
2929

3030
// ProcessFeedEntries downloads original web page for entries and apply filters.
31-
func ProcessFeedEntries(store *storage.Storage, feed *model.Feed, user *model.User, forceRefresh bool) {
31+
func ProcessFeedEntries(store *storage.Storage, feed *model.Feed, userID int64, forceRefresh bool) {
3232
var filteredEntries model.Entries
3333

34+
user, storeErr := store.UserByID(userID)
35+
if storeErr != nil {
36+
slog.Error("Database error", slog.Any("error", storeErr))
37+
return
38+
}
39+
3440
// Process older entries first
3541
for i := len(feed.Entries) - 1; i >= 0; i-- {
3642
entry := feed.Entries[i]

0 commit comments

Comments
 (0)