refactor: simplify PWA handlers and fix review findings
Address code review findings from reuse, quality, and efficiency agents: - Cache manifest JSON and service worker JS at init (was rebuilt per request with allocations and JSON encoding on every hit) - Add ImagePathsByUser store method for targeted image cleanup (was loading 100k full fave objects just to read image_path) - Add missing aria-label on privacy toggle in fave_list.html (inline copy had drifted from the partial — accessibility bug) - Fix comment/function name mismatch in pwa.go - Remove redundant user nil-check in handleShare (requireLogin guards) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
254573316a
commit
e379039fe8
5 changed files with 54 additions and 37 deletions
|
|
@ -218,12 +218,10 @@ func (h *Handler) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Delete user's images from disk before database deletion.
|
// Delete user's images from disk before database deletion.
|
||||||
faves, _, _ := h.deps.Faves.ListByUser(id, 100000, 0)
|
imagePaths, _ := h.deps.Faves.ImagePathsByUser(id)
|
||||||
for _, f := range faves {
|
for _, p := range imagePaths {
|
||||||
if f.ImagePath != "" {
|
if delErr := image.Delete(h.deps.Config.UploadDir, p); delErr != nil {
|
||||||
if delErr := image.Delete(h.deps.Config.UploadDir, f.ImagePath); delErr != nil {
|
slog.Error("image delete error", "error", delErr)
|
||||||
slog.Error("image delete error", "fave_id", f.ID, "error", delErr)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if user.AvatarPath != "" {
|
if user.AvatarPath != "" {
|
||||||
|
|
|
||||||
|
|
@ -32,14 +32,20 @@ type Deps struct {
|
||||||
type Handler struct {
|
type Handler struct {
|
||||||
deps Deps
|
deps Deps
|
||||||
rateLimiter *middleware.RateLimiter
|
rateLimiter *middleware.RateLimiter
|
||||||
|
|
||||||
|
// Cached PWA responses (computed once at init, never change).
|
||||||
|
manifestJSON []byte
|
||||||
|
swJS []byte
|
||||||
}
|
}
|
||||||
|
|
||||||
// New creates a new Handler with the given dependencies.
|
// New creates a new Handler with the given dependencies.
|
||||||
func New(deps Deps) *Handler {
|
func New(deps Deps) *Handler {
|
||||||
return &Handler{
|
h := &Handler{
|
||||||
deps: deps,
|
deps: deps,
|
||||||
rateLimiter: middleware.NewRateLimiter(deps.Config.RateLimit),
|
rateLimiter: middleware.NewRateLimiter(deps.Config.RateLimit),
|
||||||
}
|
}
|
||||||
|
h.initPWACache()
|
||||||
|
return h
|
||||||
}
|
}
|
||||||
|
|
||||||
// RateLimiterCleanupLoop periodically evicts stale rate limiter entries.
|
// RateLimiterCleanupLoop periodically evicts stale rate limiter entries.
|
||||||
|
|
|
||||||
|
|
@ -14,8 +14,9 @@ import (
|
||||||
"kode.naiv.no/olemd/favoritter/web"
|
"kode.naiv.no/olemd/favoritter/web"
|
||||||
)
|
)
|
||||||
|
|
||||||
// handleManifest serves the Web App Manifest with dynamic BasePath injection.
|
// initPWACache pre-computes the manifest JSON and service worker JS
|
||||||
func (h *Handler) handleManifest(w http.ResponseWriter, r *http.Request) {
|
// so they can be served without per-request allocations.
|
||||||
|
func (h *Handler) initPWACache() {
|
||||||
bp := h.deps.Config.BasePath
|
bp := h.deps.Config.BasePath
|
||||||
|
|
||||||
manifest := map[string]any{
|
manifest := map[string]any{
|
||||||
|
|
@ -42,44 +43,33 @@ func (h *Handler) handleManifest(w http.ResponseWriter, r *http.Request) {
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
h.manifestJSON, _ = json.Marshal(manifest)
|
||||||
|
|
||||||
w.Header().Set("Content-Type", "application/manifest+json")
|
|
||||||
json.NewEncoder(w).Encode(manifest)
|
|
||||||
}
|
|
||||||
|
|
||||||
// handleServiceWorker serves the service worker JS from root scope.
|
|
||||||
// BasePath is injected via placeholder replacement so the SW can
|
|
||||||
// cache the correct static asset paths.
|
|
||||||
func (h *Handler) handleServiceWorker(w http.ResponseWriter, r *http.Request) {
|
|
||||||
staticFS, err := fs.Sub(web.StaticFS, "static")
|
staticFS, err := fs.Sub(web.StaticFS, "static")
|
||||||
if err != nil {
|
if err == nil {
|
||||||
http.Error(w, "Not found", http.StatusNotFound)
|
if data, err := fs.ReadFile(staticFS, "sw.js"); err == nil {
|
||||||
return
|
h.swJS = []byte(strings.ReplaceAll(string(data), "{{BASE_PATH}}", bp))
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
data, err := fs.ReadFile(staticFS, "sw.js")
|
// handleManifest serves the pre-computed Web App Manifest.
|
||||||
if err != nil {
|
func (h *Handler) handleManifest(w http.ResponseWriter, r *http.Request) {
|
||||||
http.Error(w, "Not found", http.StatusNotFound)
|
w.Header().Set("Content-Type", "application/manifest+json")
|
||||||
return
|
w.Write(h.manifestJSON)
|
||||||
}
|
}
|
||||||
|
|
||||||
content := strings.ReplaceAll(string(data), "{{BASE_PATH}}", h.deps.Config.BasePath)
|
// handleServiceWorker serves the pre-computed service worker JS.
|
||||||
|
func (h *Handler) handleServiceWorker(w http.ResponseWriter, r *http.Request) {
|
||||||
w.Header().Set("Content-Type", "application/javascript")
|
w.Header().Set("Content-Type", "application/javascript")
|
||||||
w.Header().Set("Cache-Control", "no-cache")
|
w.Header().Set("Cache-Control", "no-cache")
|
||||||
w.Write([]byte(content))
|
w.Write(h.swJS)
|
||||||
}
|
}
|
||||||
|
|
||||||
// handleShare receives Android share intents via the Web Share Target API
|
// handleShare receives Android share intents via the Web Share Target API
|
||||||
// and redirects to the new-fave form with pre-filled values.
|
// and redirects to the new-fave form with pre-filled values.
|
||||||
// Uses GET to avoid CSRF issues (Android cannot provide CSRF tokens).
|
// Uses GET to avoid CSRF issues (Android cannot provide CSRF tokens).
|
||||||
func (h *Handler) handleShare(w http.ResponseWriter, r *http.Request) {
|
func (h *Handler) handleShare(w http.ResponseWriter, r *http.Request) {
|
||||||
user := middleware.UserFromContext(r.Context())
|
|
||||||
if user == nil {
|
|
||||||
http.Redirect(w, r, h.deps.Config.BasePath+"/login", http.StatusSeeOther)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
sharedURL := r.URL.Query().Get("url")
|
sharedURL := r.URL.Query().Get("url")
|
||||||
sharedTitle := r.URL.Query().Get("title")
|
sharedTitle := r.URL.Query().Get("title")
|
||||||
sharedText := r.URL.Query().Get("text")
|
sharedText := r.URL.Query().Get("text")
|
||||||
|
|
@ -88,7 +78,6 @@ func (h *Handler) handleShare(w http.ResponseWriter, r *http.Request) {
|
||||||
if sharedURL == "" && sharedText != "" {
|
if sharedURL == "" && sharedText != "" {
|
||||||
sharedURL = extractURL(sharedText)
|
sharedURL = extractURL(sharedText)
|
||||||
if sharedURL != "" {
|
if sharedURL != "" {
|
||||||
// Remove the URL from text so it's not duplicated.
|
|
||||||
sharedText = strings.TrimSpace(strings.Replace(sharedText, sharedURL, "", 1))
|
sharedText = strings.TrimSpace(strings.Replace(sharedText, sharedURL, "", 1))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -96,7 +85,7 @@ func (h *Handler) handleShare(w http.ResponseWriter, r *http.Request) {
|
||||||
description := sharedTitle
|
description := sharedTitle
|
||||||
if description == "" && sharedText != "" {
|
if description == "" && sharedText != "" {
|
||||||
description = sharedText
|
description = sharedText
|
||||||
sharedText = "" // Don't duplicate into notes.
|
sharedText = ""
|
||||||
}
|
}
|
||||||
|
|
||||||
target := h.deps.Config.BasePath + "/faves/new?"
|
target := h.deps.Config.BasePath + "/faves/new?"
|
||||||
|
|
@ -114,7 +103,7 @@ func (h *Handler) handleShare(w http.ResponseWriter, r *http.Request) {
|
||||||
http.Redirect(w, r, target+params.Encode(), http.StatusSeeOther)
|
http.Redirect(w, r, target+params.Encode(), http.StatusSeeOther)
|
||||||
}
|
}
|
||||||
|
|
||||||
// handleFaveNewWithPreFill shows the new fave form, optionally pre-filled
|
// handleFaveNewPreFill shows the new fave form, optionally pre-filled
|
||||||
// from query parameters (used by share target and bookmarklets).
|
// from query parameters (used by share target and bookmarklets).
|
||||||
func (h *Handler) handleFaveNewPreFill(w http.ResponseWriter, r *http.Request) {
|
func (h *Handler) handleFaveNewPreFill(w http.ResponseWriter, r *http.Request) {
|
||||||
user := middleware.UserFromContext(r.Context())
|
user := middleware.UserFromContext(r.Context())
|
||||||
|
|
|
||||||
|
|
@ -97,6 +97,29 @@ func (s *FaveStore) Delete(id int64) error {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ImagePathsByUser returns all non-empty image paths for a user's faves.
|
||||||
|
// Used for cleanup before user deletion.
|
||||||
|
func (s *FaveStore) ImagePathsByUser(userID int64) ([]string, error) {
|
||||||
|
rows, err := s.db.Query(
|
||||||
|
"SELECT image_path FROM faves WHERE user_id = ? AND image_path != ''",
|
||||||
|
userID,
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
defer rows.Close()
|
||||||
|
|
||||||
|
var paths []string
|
||||||
|
for rows.Next() {
|
||||||
|
var p string
|
||||||
|
if err := rows.Scan(&p); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
paths = append(paths, p)
|
||||||
|
}
|
||||||
|
return paths, rows.Err()
|
||||||
|
}
|
||||||
|
|
||||||
// ListByUser returns all faves for a user (both public and private),
|
// ListByUser returns all faves for a user (both public and private),
|
||||||
// ordered by newest first, with pagination.
|
// ordered by newest first, with pagination.
|
||||||
func (s *FaveStore) ListByUser(userID int64, limit, offset int) ([]*model.Fave, int, error) {
|
func (s *FaveStore) ListByUser(userID int64, limit, offset int) ([]*model.Fave, int, error) {
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,7 @@
|
||||||
hx-target="#privacy-{{.ID}}"
|
hx-target="#privacy-{{.ID}}"
|
||||||
hx-swap="outerHTML"
|
hx-swap="outerHTML"
|
||||||
class="fave-action-btn {{if eq .Privacy "private"}}secondary{{end}}"
|
class="fave-action-btn {{if eq .Privacy "private"}}secondary{{end}}"
|
||||||
|
aria-label="{{if eq .Privacy "public"}}Gjør privat{{else}}Gjør offentlig{{end}}"
|
||||||
title="{{if eq .Privacy "public"}}Gjør privat{{else}}Gjør offentlig{{end}}"
|
title="{{if eq .Privacy "public"}}Gjør privat{{else}}Gjør offentlig{{end}}"
|
||||||
>{{if eq .Privacy "public"}}Offentlig{{else}}Privat{{end}}</button>
|
>{{if eq .Privacy "public"}}Offentlig{{else}}Privat{{end}}</button>
|
||||||
</span>
|
</span>
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue