From 1dbcf2cd9ae26b176a4b90d8eb4ec1dfd2a73dd3 Mon Sep 17 00:00:00 2001 From: CoffeeKnyte <67730400+CoffeeKnyte@users.noreply.github.com> Date: Wed, 1 Jul 2026 21:05:41 +0800 Subject: [PATCH] fix(jellycompat): guard aux search paths and index-back person search (#252) * fix(jellycompat): guard aux search paths and index-back person search Short, recursive type-ahead terms (e.g. a single "a") against the PostgreSQL people index and the in-memory collection/box-set filter produced multi-second scans that pegged the server when a client fired one search per keystroke. Every jellycompat search path except the Meilisearch-backed /Items media search now rejects a provided SearchTerm shorter than 3 runes without touching any backend, and caps results at 20 regardless of the client-requested Limit: - /Persons (PostgreSQL people scan) - /Search/Hints (catalog search) - /Items BoxSet (in-memory collection filter) Shared policy + helpers live in search_guard.go (auxSearchMinTermLen=2, i.e. reject 1-2 runes / allow 3+, and auxSearchMaxResults=20) with unit coverage. The 3-rune floor is deliberate: 3 runes is the point where a pg_trgm trigram index becomes usable, so the gate lines up with the index and still lets legitimate short titles/names ("300", "Saw", 3-letter actors) and 3-char type-ahead hints through. PersonRepository.Search filters with `name ILIKE '%'||$1||'%'` rather than `LOWER(name) LIKE '%'||LOWER($1)||'%'`. The old expression filtered on LOWER(name), which the trigram GIN index idx_people_name_trgm (built on name) could not serve, so every search fell back to an ordered index scan on idx_people_name that walked the whole table for rare terms (~300-400ms on the 889k-row production people table). ILIKE on name lets pg_trgm serve rare 3+ char terms from the trigram index, while the planner still picks the ordered btree scan with early termination for common terms. ILIKE is case-insensitive, so behavior is preserved (including the pre-existing treatment of % and _ in the term as LIKE wildcards). Verified on production silo-postgres (889,302 people), parameterized query under both custom and generic plan cache modes: rare 3-char 'qzx': 304ms -> 1-8ms (trgm bitmap index) rare 4-char 'zzzz': 329ms -> 1-2ms (trgm bitmap index) common 3-char 'ann': 15ms -> 7-74ms (btree early-stop / trgm bitmap) New worst case across all terms is ~83ms (generic-plan common 3-char). * fix(jellycompat): stop gating meilisearch hints, clamp box-set search Two review follow-ups on the aux-search guards: - HandleSearchHints is served by the catalog (Meilisearch-backed) search provider, which already bounds and short-term-handles its own results. Gating it with auxSearchTermTooShort contradicted the guard's own "non-Meilisearch paths only" policy and hid valid 1-2 char titles ("Up", "It") from global type-ahead. Drop the too-short gate there; keep the empty-term check and the result clamp. - handleBoxSetsList gated short terms but passed query.limit straight to slicePage, which treats <=0 as no cap, so a box-set *search* was uncapped unlike Persons/Hints. Clamp the limit when a search term is present; empty-term browse keeps its client paging window. --- internal/catalog/person_repo.go | 9 +++- internal/jellycompat/handlers_collections.go | 17 +++++- internal/jellycompat/handlers_items.go | 7 ++- internal/jellycompat/handlers_persons.go | 11 +++- internal/jellycompat/search_guard.go | 55 ++++++++++++++++++++ internal/jellycompat/search_guard_test.go | 45 ++++++++++++++++ 6 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 internal/jellycompat/search_guard.go create mode 100644 internal/jellycompat/search_guard_test.go diff --git a/internal/catalog/person_repo.go b/internal/catalog/person_repo.go index 7636b3d7..73538785 100644 --- a/internal/catalog/person_repo.go +++ b/internal/catalog/person_repo.go @@ -493,6 +493,13 @@ func (r *PersonRepository) GetByName(ctx context.Context, name string) (*models. } // Search finds persons by name substring (case-insensitive), ordered by name. +// +// The predicate is `name ILIKE '%term%'` rather than `LOWER(name) LIKE ...` so +// the pg_trgm GIN index idx_people_name_trgm can serve it: for a rare 3+ char +// term that index turns a ~300-400ms full name-index scan into a ~10ms bitmap +// scan. ILIKE is itself case-insensitive, so this stays equivalent to the prior +// LOWER(name) comparison (including its existing treatment of % and _ in the +// term as LIKE wildcards). func (r *PersonRepository) Search(ctx context.Context, query string, limit int) ([]models.Person, error) { if limit <= 0 { limit = 20 @@ -500,7 +507,7 @@ func (r *PersonRepository) Search(ctx context.Context, query string, limit int) rows, err := r.pool.Query(ctx, ` SELECT id, name, sort_name, bio, birth_date, death_date, birthplace, homepage, photo_path, photo_source_path, photo_thumbhash, tmdb_id, imdb_id, tvdb_id, plex_guid, created_at, updated_at - FROM people WHERE LOWER(name) LIKE '%' || LOWER($1) || '%' + FROM people WHERE name ILIKE '%' || $1 || '%' ORDER BY name LIMIT $2`, query, limit, ) if err != nil { diff --git a/internal/jellycompat/handlers_collections.go b/internal/jellycompat/handlers_collections.go index dec928ca..397d6a72 100644 --- a/internal/jellycompat/handlers_collections.go +++ b/internal/jellycompat/handlers_collections.go @@ -295,6 +295,14 @@ func (h *ItemsHandler) handleBoxSetsList(w http.ResponseWriter, r *http.Request, return } + // Box-set/collection search is an in-memory filter over every collection + // (not the Meilisearch-backed /Items media search), so short type-ahead + // terms are gated before any rows are loaded. + if auxSearchTermTooShort(query.searchTerm) { + writeJSON(w, http.StatusOK, emptyQueryResult(query.startIndex)) + return + } + visible, err := h.visibleLibraryIDs(r.Context(), session) if err != nil { writeCompatUpstreamError(w, err) @@ -344,7 +352,14 @@ func (h *ItemsHandler) handleBoxSetsList(w http.ResponseWriter, r *http.Request, }) } - page := slicePage(matched, query.startIndex, query.limit) + // A search term makes this a guarded aux search path, so cap results like + // the other guarded handlers; an empty term is a browse/list request and + // keeps the client-requested paging window. + pageLimit := query.limit + if strings.TrimSpace(query.searchTerm) != "" { + pageLimit = clampAuxSearchLimit(query.limit) + } + page := slicePage(matched, query.startIndex, pageLimit) items := make([]baseItemDTO, 0, len(page)) for _, c := range page { items = append(items, h.boxSetFromCollection(r.Context(), c)) diff --git a/internal/jellycompat/handlers_items.go b/internal/jellycompat/handlers_items.go index e137fade..b61ae3fd 100644 --- a/internal/jellycompat/handlers_items.go +++ b/internal/jellycompat/handlers_items.go @@ -1621,12 +1621,17 @@ func (h *ItemsHandler) HandleSearchHints(w http.ResponseWriter, r *http.Request) q := newCaseInsensitiveQuery(r.URL.Query()) query := strings.TrimSpace(q.Get("SearchTerm")) + // Search hints are served by the catalog search provider (the same + // Meilisearch-backed path as /Items media search), which does its own + // short-term handling and result bounding, so the aux short-term gate is + // intentionally NOT applied here — short titles ("Up", "It") stay + // discoverable through type-ahead. The result cap is still clamped below. if query == "" { writeJSON(w, http.StatusOK, searchHintResultDTO{}) return } - limit := parsePositiveInt(q.Get("Limit"), 20) + limit := clampAuxSearchLimit(parsePositiveInt(q.Get("Limit"), auxSearchMaxResults)) result, err := h.content.SearchItems(r.Context(), session, SearchItemsOptions{ Query: query, Limit: limit, diff --git a/internal/jellycompat/handlers_persons.go b/internal/jellycompat/handlers_persons.go index b21d5542..9b7a01b5 100644 --- a/internal/jellycompat/handlers_persons.go +++ b/internal/jellycompat/handlers_persons.go @@ -45,12 +45,21 @@ func (h *PersonsHandler) HandleGetPersons(w http.ResponseWriter, r *http.Request q := newCaseInsensitiveQuery(r.URL.Query()) searchTerm := strings.TrimSpace(q.Get("SearchTerm")) - limit := parsePositiveInt(q.Get("Limit"), 20) + // Person search hits PostgreSQL directly (it is not in the Meilisearch + // index), so it is gated: short terms never run and results are capped. + limit := clampAuxSearchLimit(parsePositiveInt(q.Get("Limit"), auxSearchMaxResults)) var people []models.Person var err error if searchTerm != "" { + if auxSearchTermTooShort(searchTerm) { + writeJSON(w, http.StatusOK, queryResultDTO{ + Items: []baseItemDTO{}, + TotalRecordCount: 0, + }) + return + } if h.shouldSuppressSearchPeople(r.Context(), session, searchTerm) { writeJSON(w, http.StatusOK, queryResultDTO{ Items: []baseItemDTO{}, diff --git a/internal/jellycompat/search_guard.go b/internal/jellycompat/search_guard.go new file mode 100644 index 00000000..3f65029f --- /dev/null +++ b/internal/jellycompat/search_guard.go @@ -0,0 +1,55 @@ +package jellycompat + +import "strings" + +// Search-path guard policy for every jellycompat search EXCEPT the +// Meilisearch-backed /Items media search. +// +// Short, recursive type-ahead terms (e.g. a single "a") against the PostgreSQL +// people index and the in-memory collection/box-set filter produced multi-second +// scans that pegged the server when a client fired one search per keystroke. +// Anything that is not offloaded to Meilisearch is therefore gated: a provided +// term must be longer than auxSearchMinTermLen to run at all, and a guarded path +// never returns more than auxSearchMaxResults rows. +// +// The minimum allowed term length is 3 runes (auxSearchMinTermLen=2 is the +// longest length still rejected, so 1-2 runes are rejected and 3+ are allowed). +// 3 runes is the point where a pg_trgm trigram index becomes usable: a 2-char +// pattern yields no complete trigram, so it cannot be served from the people +// name trigram index and degrades to a ~400ms full scan, whereas any 3+ char +// term is trigram-eligible and stays fast. Aligning the gate with the trigram +// floor lets legitimate short titles/names ("300", "Saw", 3-letter actors) and +// 3-char type-ahead hints through. +const ( + // auxSearchMinTermLen is the maximum SearchTerm length (counted in runes, + // after trimming) that is rejected. A non-empty term of this length or + // shorter returns no results WITHOUT querying any backend; only terms + // strictly longer than this are allowed to run. + auxSearchMinTermLen = 2 + // auxSearchMaxResults caps how many results a guarded search path may return, + // regardless of the client-requested Limit. + auxSearchMaxResults = 20 +) + +// auxSearchTermTooShort reports whether a non-empty SearchTerm is too short to +// run on a guarded (non-/Items) search path. +// +// An empty term is NOT a search — it is a browse/list request — so it is never +// reported as too short; callers keep their existing empty-term behavior (the +// result limit is still clamped separately via clampAuxSearchLimit). +func auxSearchTermTooShort(term string) bool { + trimmed := strings.TrimSpace(term) + if trimmed == "" { + return false + } + return len([]rune(trimmed)) <= auxSearchMinTermLen +} + +// clampAuxSearchLimit caps a client-requested limit to auxSearchMaxResults for +// guarded search paths. A non-positive request collapses to the cap. +func clampAuxSearchLimit(requested int) int { + if requested <= 0 || requested > auxSearchMaxResults { + return auxSearchMaxResults + } + return requested +} diff --git a/internal/jellycompat/search_guard_test.go b/internal/jellycompat/search_guard_test.go new file mode 100644 index 00000000..f3ad1d04 --- /dev/null +++ b/internal/jellycompat/search_guard_test.go @@ -0,0 +1,45 @@ +package jellycompat + +import "testing" + +func TestAuxSearchTermTooShort(t *testing.T) { + cases := []struct { + term string + want bool + }{ + {"", false}, // empty is a browse, not a search + {" ", false}, // whitespace trims to empty -> browse + {"a", true}, // 1 char + {"ab", true}, // 2 chars -> too short (below the trigram floor) + {"abc", false}, // 3 chars -> allowed (trigram-eligible) + {" abc ", false}, // trims to 3 chars -> allowed + {"abcd", false}, // 4 chars -> allowed + {"america", false}, // long term -> allowed + {"日本", true}, // 2 runes -> too short (rune-counted, not bytes) + {"日本語", false}, // 3 runes -> allowed + } + for _, c := range cases { + if got := auxSearchTermTooShort(c.term); got != c.want { + t.Errorf("auxSearchTermTooShort(%q) = %v, want %v", c.term, got, c.want) + } + } +} + +func TestClampAuxSearchLimit(t *testing.T) { + cases := []struct { + in int + want int + }{ + {0, auxSearchMaxResults}, // unset collapses to cap + {-5, auxSearchMaxResults}, // negative collapses to cap + {5, 5}, // under cap preserved + {20, 20}, // exactly cap preserved + {21, auxSearchMaxResults}, // over cap clamped + {1000, auxSearchMaxResults}, // way over cap clamped + } + for _, c := range cases { + if got := clampAuxSearchLimit(c.in); got != c.want { + t.Errorf("clampAuxSearchLimit(%d) = %d, want %d", c.in, got, c.want) + } + } +}