From 454938c31fcd5a491739b62215901dc79e7dc846 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sat, 17 Oct 2026 17:35:43 -0400 Subject: [PATCH] =?UTF-8?q?refactor(vis):=20H4.5=20steps=204-5=20=E2=80=94?= =?UTF-8?q?=20statistics=20and=20the=20three=20reader=20gates=20on=20the?= =?UTF-8?q?=20canonical=20resolver;=20core=20goes=20package-level?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VisibleLibrarySet becomes a package-level function taking the queries (db-only services like statistics reach it without new wiring); the LibraryService method stays as the ergonomic entry point. Statistics' ids-shaper and the reader pages' three hand-rolled membership loops (empty-rows fallback then linear scans) now delegate; the gates keep their 403-on-invisible semantics (>null set membership, never silent pass). Census addendum from this pass: two OPDS sites call GetUserVisibleLibraries directly but consume FULL ROWS (per-library folder enumeration) — a third legitimate pattern alongside set/ids and resolveLibrary's active-selection; documented, untouched. The library LIST endpoint and its SSR data helper are row consumers too, by design. --- internal/handlers/reader.go | 39 +++++++++---------------- internal/services/library_service.go | 13 +++++++-- internal/services/statistics_service.go | 14 ++------- 3 files changed, 27 insertions(+), 39 deletions(-) diff --git a/internal/handlers/reader.go b/internal/handlers/reader.go index 299090ac..400b38b4 100644 --- a/internal/handlers/reader.go +++ b/internal/handlers/reader.go @@ -78,19 +78,14 @@ func (h *ReaderHandler) ShowReader(c *echo.Context) error { } userData := user.(database.Users) - // Get user's visible libraries - libraries, err := h.libraryService.GetUserVisibleLibraries(c.Request().Context(), userData.ID) + // Family-lens gate (H4.5 canonical resolver): the media item's + // library must be in the requester's visible set — 403, never a + // silent pass. + visibleSet, _, err := h.libraryService.VisibleLibrarySet(c.Request().Context(), userData.ID) if err != nil { return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to check library access"}) } - // Check if media item's library is in visible list - visible := false - for _, lib := range libraries { - if lib.ID == mediaItem.LibraryID { - visible = true - break - } - } + visible := visibleSet[uuid.UUID(mediaItem.LibraryID.Bytes).String()] if !visible { return c.JSON(http.StatusForbidden, map[string]string{"error": "Access denied to this library"}) } @@ -172,19 +167,14 @@ func (h *ReaderHandler) GetPage(c *echo.Context) error { } userData := user.(database.Users) - // Get user's visible libraries - libraries, err := h.libraryService.GetUserVisibleLibraries(c.Request().Context(), userData.ID) + // Family-lens gate (H4.5 canonical resolver): the media item's + // library must be in the requester's visible set — 403, never a + // silent pass. + visibleSet, _, err := h.libraryService.VisibleLibrarySet(c.Request().Context(), userData.ID) if err != nil { return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to check library access"}) } - // Check if media item's library is in visible list - visible := false - for _, lib := range libraries { - if lib.ID == mediaItem.LibraryID { - visible = true - break - } - } + visible := visibleSet[uuid.UUID(mediaItem.LibraryID.Bytes).String()] if !visible { return c.JSON(http.StatusForbidden, map[string]string{"error": "Access denied to this library"}) } @@ -225,14 +215,13 @@ func (h *ReaderHandler) ensureItemVisible(c *echo.Context, mediaItem *database.M return c.JSON(http.StatusUnauthorized, map[string]string{"error": "User not authenticated"}) } userData := user.(database.Users) - libraries, err := h.libraryService.GetUserVisibleLibraries(c.Request().Context(), userData.ID) + // Family-lens gate (H4.5 canonical resolver). + visibleSet, _, err := h.libraryService.VisibleLibrarySet(c.Request().Context(), userData.ID) if err != nil { return c.JSON(http.StatusInternalServerError, map[string]string{"error": "Failed to check library access"}) } - for _, lib := range libraries { - if lib.ID == mediaItem.LibraryID { - return nil - } + if visibleSet[uuid.UUID(mediaItem.LibraryID.Bytes).String()] { + return nil } return c.JSON(http.StatusForbidden, map[string]string{"error": "Access denied to this library"}) } diff --git a/internal/services/library_service.go b/internal/services/library_service.go index 1fb93799..45daa21b 100644 --- a/internal/services/library_service.go +++ b/internal/services/library_service.go @@ -174,7 +174,8 @@ func (s *LibraryService) SetLibraryVisibility(ctx context.Context, userID, libra return &visibility, nil } -// GetUserVisibleLibraries retrieves all libraries visible to a user +// GetUserVisibleLibraries retrieves all libraries visible to a user. + // VisibleLibrarySet is THE canonical library-visibility resolver (H4.5): // every endpoint that asks "which libraries can this user see" funnels // here. The family-lens rule itself lives in the @@ -182,8 +183,8 @@ func (s *LibraryService) SetLibraryVisibility(ctx context.Context, userID, libra // default-allow: an admin hides a library, everyone else still sees it). // Callers adapt their own identity source (JWT user object, device // token, SSR session); this core takes the resolved user id. -func (s *LibraryService) VisibleLibrarySet(ctx context.Context, userID pgtype.UUID) (map[string]bool, []pgtype.UUID, error) { - rows, err := s.GetUserVisibleLibraries(ctx, userID) +func VisibleLibrarySet(ctx context.Context, db *database.Queries, userID pgtype.UUID) (map[string]bool, []pgtype.UUID, error) { + rows, err := db.GetUserVisibleLibraries(ctx, userID) if err != nil { return nil, nil, fmt.Errorf("failed to check library access: %w", err) } @@ -199,6 +200,12 @@ func (s *LibraryService) VisibleLibrarySet(ctx context.Context, userID pgtype.UU return set, ids, nil } +// VisibleLibrarySet is the LibraryService entry point for the +// package-level core (see the core's doc). +func (s *LibraryService) VisibleLibrarySet(ctx context.Context, userID pgtype.UUID) (map[string]bool, []pgtype.UUID, error) { + return VisibleLibrarySet(ctx, s.db, userID) +} + func (s *LibraryService) GetUserVisibleLibraries(ctx context.Context, userID pgtype.UUID) ([]database.GetUserVisibleLibrariesRow, error) { return s.db.GetUserVisibleLibraries(ctx, userID) } diff --git a/internal/services/statistics_service.go b/internal/services/statistics_service.go index f713519f..64088969 100644 --- a/internal/services/statistics_service.go +++ b/internal/services/statistics_service.go @@ -188,18 +188,10 @@ type SessionSummary struct { // userVisibleLibraryIDs loads the family-lens set for this user (empty → // no visible-library filter, i.e. everything is visible by default). +// userVisibleLibraryIDs delegates to the canonical resolver (H4.5). func (s *StatisticsService) userVisibleLibraryIDs(ctx context.Context, userID pgtype.UUID) ([]pgtype.UUID, error) { - rows, err := s.db.GetUserVisibleLibraries(ctx, userID) - if err != nil { - return nil, fmt.Errorf("failed to check library access: %w", err) - } - ids := make([]pgtype.UUID, 0, len(rows)) - for _, lib := range rows { - if lib.ID.Valid { - ids = append(ids, lib.ID) - } - } - return ids, nil + _, ids, err := VisibleLibrarySet(ctx, s.db, userID) + return ids, err } // ErrLibraryNotVisible is returned by LibraryUsage when the caller asked