From 5d9d0b2652b85b4151816407d815283eda1c8c44 Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Tue, 12 Aug 2025 04:48:36 +0200 Subject: [PATCH] refactor(ui): standardize user variable naming and avoid a SQL query when only `userID` is used - Use `user` everywhere, instead of sometimes `loggedUser` - Delay the instantiation of some variables: no need to perform SQL queries for nothing. - Remove a SQL query getting the whole user struct when only user.ID is used. --- internal/ui/api_key_save.go | 14 +++++++------- internal/ui/category_refresh.go | 2 +- internal/ui/category_save.go | 14 +++++++------- internal/ui/category_update.go | 12 ++++++------ internal/ui/integration_update.go | 12 ++++-------- internal/ui/oauth2_callback.go | 8 ++++---- internal/ui/oauth2_redirect.go | 3 +-- internal/ui/oauth2_unlink.go | 4 ++-- internal/ui/opml_upload.go | 12 +++++------- internal/ui/proxy.go | 2 +- internal/ui/settings_update.go | 24 ++++++++++++------------ 11 files changed, 50 insertions(+), 57 deletions(-) diff --git a/internal/ui/api_key_save.go b/internal/ui/api_key_save.go index c28c70a4..43364a51 100644 --- a/internal/ui/api_key_save.go +++ b/internal/ui/api_key_save.go @@ -17,7 +17,7 @@ import ( ) func (h *handler) saveAPIKey(w http.ResponseWriter, r *http.Request) { - loggedUser, err := h.store.UserByID(request.UserID(r)) + user, err := h.store.UserByID(request.UserID(r)) if err != nil { html.ServerError(w, r, err) return @@ -28,20 +28,20 @@ func (h *handler) saveAPIKey(w http.ResponseWriter, r *http.Request) { Description: apiKeyForm.Description, } - if validationErr := validator.ValidateAPIKeyCreation(h.store, loggedUser.ID, apiKeyCreationRequest); validationErr != nil { + if validationErr := validator.ValidateAPIKeyCreation(h.store, user.ID, apiKeyCreationRequest); validationErr != nil { sess := session.New(h.store, request.SessionID(r)) view := view.New(h.tpl, r, sess) view.Set("form", apiKeyForm) view.Set("menu", "settings") - view.Set("user", loggedUser) - view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID)) - view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID)) - view.Set("errorMessage", validationErr.Translate(loggedUser.Language)) + view.Set("user", user) + view.Set("countUnread", h.store.CountUnreadEntries(user.ID)) + view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(user.ID)) + view.Set("errorMessage", validationErr.Translate(user.Language)) html.OK(w, r, view.Render("create_api_key")) return } - if _, err = h.store.CreateAPIKey(loggedUser.ID, apiKeyCreationRequest.Description); err != nil { + if _, err = h.store.CreateAPIKey(user.ID, apiKeyCreationRequest.Description); err != nil { html.ServerError(w, r, err) return } diff --git a/internal/ui/category_refresh.go b/internal/ui/category_refresh.go index 6eae71ab..c0016784 100644 --- a/internal/ui/category_refresh.go +++ b/internal/ui/category_refresh.go @@ -27,7 +27,6 @@ func (h *handler) refreshCategoryFeedsPage(w http.ResponseWriter, r *http.Reques } func (h *handler) refreshCategory(w http.ResponseWriter, r *http.Request) int64 { - userID := request.UserID(r) categoryID := request.RouteInt64Param(r, "categoryID") printer := locale.NewPrinter(request.UserLanguage(r)) sess := session.New(h.store, request.SessionID(r)) @@ -37,6 +36,7 @@ func (h *handler) refreshCategory(w http.ResponseWriter, r *http.Request) int64 time := config.Opts.ForceRefreshInterval() sess.NewFlashErrorMessage(printer.Plural("alert.too_many_feeds_refresh", time, time)) } else { + userID := request.UserID(r) // We allow the end-user to force refresh all its feeds in this category // without taking into consideration the number of errors. batchBuilder := h.store.NewBatchBuilder() diff --git a/internal/ui/category_save.go b/internal/ui/category_save.go index e23c2d39..d8144887 100644 --- a/internal/ui/category_save.go +++ b/internal/ui/category_save.go @@ -17,7 +17,7 @@ import ( ) func (h *handler) saveCategory(w http.ResponseWriter, r *http.Request) { - loggedUser, err := h.store.UserByID(request.UserID(r)) + user, err := h.store.UserByID(request.UserID(r)) if err != nil { html.ServerError(w, r, err) return @@ -29,19 +29,19 @@ func (h *handler) saveCategory(w http.ResponseWriter, r *http.Request) { view := view.New(h.tpl, r, sess) view.Set("form", categoryForm) view.Set("menu", "categories") - view.Set("user", loggedUser) - view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID)) - view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID)) + view.Set("user", user) + view.Set("countUnread", h.store.CountUnreadEntries(user.ID)) + view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(user.ID)) categoryCreationRequest := &model.CategoryCreationRequest{Title: categoryForm.Title} - if validationErr := validator.ValidateCategoryCreation(h.store, loggedUser.ID, categoryCreationRequest); validationErr != nil { - view.Set("errorMessage", validationErr.Translate(loggedUser.Language)) + if validationErr := validator.ValidateCategoryCreation(h.store, user.ID, categoryCreationRequest); validationErr != nil { + view.Set("errorMessage", validationErr.Translate(user.Language)) html.OK(w, r, view.Render("create_category")) return } - if _, err = h.store.CreateCategory(loggedUser.ID, categoryCreationRequest); err != nil { + if _, err = h.store.CreateCategory(user.ID, categoryCreationRequest); err != nil { html.ServerError(w, r, err) return } diff --git a/internal/ui/category_update.go b/internal/ui/category_update.go index 7403bd63..0edbd76c 100644 --- a/internal/ui/category_update.go +++ b/internal/ui/category_update.go @@ -17,7 +17,7 @@ import ( ) func (h *handler) updateCategory(w http.ResponseWriter, r *http.Request) { - loggedUser, err := h.store.UserByID(request.UserID(r)) + user, err := h.store.UserByID(request.UserID(r)) if err != nil { html.ServerError(w, r, err) return @@ -42,17 +42,17 @@ func (h *handler) updateCategory(w http.ResponseWriter, r *http.Request) { view.Set("form", categoryForm) view.Set("category", category) view.Set("menu", "categories") - view.Set("user", loggedUser) - view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID)) - view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID)) + view.Set("user", user) + view.Set("countUnread", h.store.CountUnreadEntries(user.ID)) + view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(user.ID)) categoryRequest := &model.CategoryModificationRequest{ Title: model.SetOptionalField(categoryForm.Title), HideGlobally: model.SetOptionalField(categoryForm.HideGlobally), } - if validationErr := validator.ValidateCategoryModification(h.store, loggedUser.ID, category.ID, categoryRequest); validationErr != nil { - view.Set("errorMessage", validationErr.Translate(loggedUser.Language)) + if validationErr := validator.ValidateCategoryModification(h.store, user.ID, category.ID, categoryRequest); validationErr != nil { + view.Set("errorMessage", validationErr.Translate(user.Language)) html.OK(w, r, view.Render("create_category")) return } diff --git a/internal/ui/integration_update.go b/internal/ui/integration_update.go index c54ec625..a06568cc 100644 --- a/internal/ui/integration_update.go +++ b/internal/ui/integration_update.go @@ -20,13 +20,9 @@ import ( func (h *handler) updateIntegration(w http.ResponseWriter, r *http.Request) { printer := locale.NewPrinter(request.UserLanguage(r)) sess := session.New(h.store, request.SessionID(r)) - user, err := h.store.UserByID(request.UserID(r)) - if err != nil { - html.ServerError(w, r, err) - return - } + userID := request.UserID(r) - integration, err := h.store.Integration(user.ID) + integration, err := h.store.Integration(userID) if err != nil { html.ServerError(w, r, err) return @@ -35,7 +31,7 @@ func (h *handler) updateIntegration(w http.ResponseWriter, r *http.Request) { integrationForm := form.NewIntegrationForm(r) integrationForm.Merge(integration) - if integration.FeverUsername != "" && h.store.HasDuplicateFeverUsername(user.ID, integration.FeverUsername) { + if integration.FeverUsername != "" && h.store.HasDuplicateFeverUsername(userID, integration.FeverUsername) { sess.NewFlashErrorMessage(printer.Print("error.duplicate_fever_username")) html.Redirect(w, r, route.Path(h.router, "integrations")) return @@ -49,7 +45,7 @@ func (h *handler) updateIntegration(w http.ResponseWriter, r *http.Request) { integration.FeverToken = "" } - if integration.GoogleReaderUsername != "" && h.store.HasDuplicateGoogleReaderUsername(user.ID, integration.GoogleReaderUsername) { + if integration.GoogleReaderUsername != "" && h.store.HasDuplicateGoogleReaderUsername(userID, integration.GoogleReaderUsername) { sess.NewFlashErrorMessage(printer.Print("error.duplicate_googlereader_username")) html.Redirect(w, r, route.Path(h.router, "integrations")) return diff --git a/internal/ui/oauth2_callback.go b/internal/ui/oauth2_callback.go index 09c03b80..3d649853 100644 --- a/internal/ui/oauth2_callback.go +++ b/internal/ui/oauth2_callback.go @@ -20,10 +20,6 @@ import ( ) func (h *handler) oauth2Callback(w http.ResponseWriter, r *http.Request) { - clientIP := request.ClientIP(r) - printer := locale.NewPrinter(request.UserLanguage(r)) - sess := session.New(h.store, request.SessionID(r)) - provider := request.RouteStringParam(r, "provider") if provider == "" { slog.Warn("Invalid or missing OAuth2 provider") @@ -68,6 +64,9 @@ func (h *handler) oauth2Callback(w http.ResponseWriter, r *http.Request) { return } + printer := locale.NewPrinter(request.UserLanguage(r)) + sess := session.New(h.store, request.SessionID(r)) + if request.IsAuthenticated(r) { loggedUser, err := h.store.UserByID(request.UserID(r)) if err != nil { @@ -124,6 +123,7 @@ func (h *handler) oauth2Callback(w http.ResponseWriter, r *http.Request) { } } + clientIP := request.ClientIP(r) sessionToken, _, err := h.store.CreateUserSessionFromUsername(user.Username, r.UserAgent(), clientIP) if err != nil { html.ServerError(w, r, err) diff --git a/internal/ui/oauth2_redirect.go b/internal/ui/oauth2_redirect.go index 78f9fd1a..71f23867 100644 --- a/internal/ui/oauth2_redirect.go +++ b/internal/ui/oauth2_redirect.go @@ -15,8 +15,6 @@ import ( ) func (h *handler) oauth2Redirect(w http.ResponseWriter, r *http.Request) { - sess := session.New(h.store, request.SessionID(r)) - provider := request.RouteStringParam(r, "provider") if provider == "" { slog.Warn("Invalid or missing OAuth2 provider") @@ -36,6 +34,7 @@ func (h *handler) oauth2Redirect(w http.ResponseWriter, r *http.Request) { auth := oauth2.GenerateAuthorization(authProvider.GetConfig()) + sess := session.New(h.store, request.SessionID(r)) sess.SetOAuth2State(auth.State()) sess.SetOAuth2CodeVerifier(auth.CodeVerifier()) diff --git a/internal/ui/oauth2_unlink.go b/internal/ui/oauth2_unlink.go index e10cb5a4..d12022a2 100644 --- a/internal/ui/oauth2_unlink.go +++ b/internal/ui/oauth2_unlink.go @@ -24,7 +24,6 @@ func (h *handler) oauth2Unlink(w http.ResponseWriter, r *http.Request) { return } - printer := locale.NewPrinter(request.UserLanguage(r)) provider := request.RouteStringParam(r, "provider") if provider == "" { slog.Warn("Invalid or missing OAuth2 provider") @@ -42,7 +41,6 @@ func (h *handler) oauth2Unlink(w http.ResponseWriter, r *http.Request) { return } - sess := session.New(h.store, request.SessionID(r)) user, err := h.store.UserByID(request.UserID(r)) if err != nil { html.ServerError(w, r, err) @@ -55,6 +53,8 @@ func (h *handler) oauth2Unlink(w http.ResponseWriter, r *http.Request) { return } + sess := session.New(h.store, request.SessionID(r)) + printer := locale.NewPrinter(request.UserLanguage(r)) if !hasPassword { sess.NewFlashErrorMessage(printer.Print("error.unlink_account_without_password")) html.Redirect(w, r, route.Path(h.router, "settings")) diff --git a/internal/ui/opml_upload.go b/internal/ui/opml_upload.go index c31b6e88..87af8bf1 100644 --- a/internal/ui/opml_upload.go +++ b/internal/ui/opml_upload.go @@ -21,8 +21,7 @@ import ( ) func (h *handler) uploadOPML(w http.ResponseWriter, r *http.Request) { - loggedUserID := request.UserID(r) - user, err := h.store.UserByID(loggedUserID) + user, err := h.store.UserByID(request.UserID(r)) if err != nil { html.ServerError(w, r, err) return @@ -31,7 +30,7 @@ func (h *handler) uploadOPML(w http.ResponseWriter, r *http.Request) { file, fileHeader, err := r.FormFile("file") if err != nil { slog.Error("OPML file upload error", - slog.Int64("user_id", loggedUserID), + slog.Int64("user_id", user.ID), slog.Any("error", err), ) @@ -41,7 +40,7 @@ func (h *handler) uploadOPML(w http.ResponseWriter, r *http.Request) { defer file.Close() slog.Info("OPML file uploaded", - slog.Int64("user_id", loggedUserID), + slog.Int64("user_id", user.ID), slog.String("file_name", fileHeader.Filename), slog.Int64("file_size", fileHeader.Size), ) @@ -69,8 +68,7 @@ func (h *handler) uploadOPML(w http.ResponseWriter, r *http.Request) { } func (h *handler) fetchOPML(w http.ResponseWriter, r *http.Request) { - loggedUserID := request.UserID(r) - user, err := h.store.UserByID(loggedUserID) + user, err := h.store.UserByID(request.UserID(r)) if err != nil { html.ServerError(w, r, err) return @@ -83,7 +81,7 @@ func (h *handler) fetchOPML(w http.ResponseWriter, r *http.Request) { } slog.Info("Fetching OPML file remotely", - slog.Int64("user_id", loggedUserID), + slog.Int64("user_id", user.ID), slog.String("opml_file_url", opmlFileURL), ) diff --git a/internal/ui/proxy.go b/internal/ui/proxy.go index a18447a2..2486db59 100644 --- a/internal/ui/proxy.go +++ b/internal/ui/proxy.go @@ -30,13 +30,13 @@ func (h *handler) mediaProxy(w http.ResponseWriter, r *http.Request) { return } - encodedDigest := request.RouteStringParam(r, "encodedDigest") encodedURL := request.RouteStringParam(r, "encodedURL") if encodedURL == "" { html.BadRequest(w, r, errors.New("no URL provided")) return } + encodedDigest := request.RouteStringParam(r, "encodedDigest") decodedDigest, err := base64.URLEncoding.DecodeString(encodedDigest) if err != nil { html.BadRequest(w, r, errors.New("unable to decode this digest")) diff --git a/internal/ui/settings_update.go b/internal/ui/settings_update.go index 682eada2..89ef0327 100644 --- a/internal/ui/settings_update.go +++ b/internal/ui/settings_update.go @@ -18,7 +18,7 @@ import ( ) func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) { - loggedUser, err := h.store.UserByID(request.UserID(r)) + user, err := h.store.UserByID(request.UserID(r)) if err != nil { html.ServerError(w, r, err) return @@ -30,7 +30,7 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) { return } - creds, err := h.store.WebAuthnCredentialsByUserID(loggedUser.ID) + creds, err := h.store.WebAuthnCredentialsByUserID(user.ID) if err != nil { html.ServerError(w, r, err) return @@ -51,16 +51,16 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) { view.Set("languages", locale.AvailableLanguages) view.Set("timezones", timezones) view.Set("menu", "settings") - view.Set("user", loggedUser) - view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID)) - view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID)) + view.Set("user", user) + view.Set("countUnread", h.store.CountUnreadEntries(user.ID)) + view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(user.ID)) view.Set("default_home_pages", model.HomePages()) view.Set("categories_sorting_options", model.CategoriesSortingOptions()) - view.Set("countWebAuthnCerts", h.store.CountWebAuthnCredentialsByUserID(loggedUser.ID)) + view.Set("countWebAuthnCerts", h.store.CountWebAuthnCredentialsByUserID(user.ID)) view.Set("webAuthnCerts", creds) if validationErr := settingsForm.Validate(); validationErr != nil { - view.Set("errorMessage", validationErr.Translate(loggedUser.Language)) + view.Set("errorMessage", validationErr.Translate(user.Language)) html.OK(w, r, view.Render("settings")) return } @@ -86,20 +86,20 @@ func (h *handler) updateSettings(w http.ResponseWriter, r *http.Request) { ExternalFontHosts: model.OptionalString(settingsForm.ExternalFontHosts), } - if validationErr := validator.ValidateUserModification(h.store, loggedUser.ID, userModificationRequest); validationErr != nil { - view.Set("errorMessage", validationErr.Translate(loggedUser.Language)) + if validationErr := validator.ValidateUserModification(h.store, user.ID, userModificationRequest); validationErr != nil { + view.Set("errorMessage", validationErr.Translate(user.Language)) html.OK(w, r, view.Render("settings")) return } - err = h.store.UpdateUser(settingsForm.Merge(loggedUser)) + err = h.store.UpdateUser(settingsForm.Merge(user)) if err != nil { html.ServerError(w, r, err) return } - sess.SetLanguage(loggedUser.Language) - sess.SetTheme(loggedUser.Theme) + sess.SetLanguage(user.Language) + sess.SetTheme(user.Theme) sess.NewFlashMessage(locale.NewPrinter(request.UserLanguage(r)).Printf("alert.prefs_saved")) html.Redirect(w, r, route.Path(h.router, "settings")) }