diff --git a/cmd/server/tests/filter_builder_test.go b/cmd/server/tests/filter_builder_test.go index 24a641de..9f168872 100644 --- a/cmd/server/tests/filter_builder_test.go +++ b/cmd/server/tests/filter_builder_test.go @@ -547,6 +547,58 @@ func TestFilterSuggestionsUnderRules(t *testing.T) { } } assert.True(t, found, "the visible library is offered") + + // BROWSE the library field UNDER rules (the blank-typed click): the + // id-source query must not choke on the unused typed parameter and + // must list exactly the library holding the surviving books. + status, results = suggest("library", " ", qfAll(qfRule("author", "contains", "Voss"))) + require.Equal(t, http.StatusOK, status, "library browse under rules (blank typed)") + require.Len(t, results, 1, "exactly the fixture library survives the rules") + assert.Equal(t, libUUID.String(), results[0]["value"]) + assert.InDelta(t, float64(2), results[0]["count"], 0.001, "both Voss books live there") + + // Series browse (blank typed, under rules): full-membership source. + status, results = suggest("series", " ", qfAll(qfRule("author", "contains", "Voss"))) + require.Equal(t, http.StatusOK, status) + assert.Contains(t, valuesOf(results), "Voss Saga") + + // Added-by browse (blank typed): books without added_by → an empty + // menu, but NO error (the parameter-discipline regression net). + status, results = suggest("added_by", " ", "") + require.Equal(t, http.StatusOK, status) + + // Collection browse (blank typed): the container sources ride the + // same id-source query — a collection holding a surviving book is + // offered with the surviving count; zero-match containers never are. + userA := getRegularUserID(t, setup.DB) + var collID pgtype.UUID + err = setup.DBPool.QueryRow(ctx, + `INSERT INTO collections (user_id, name) VALUES ($1, $2) RETURNING id`, + pgtype.UUID{Bytes: [16]byte(userA), Valid: true}, + fmt.Sprintf("Test Builder Sug Coll %d", time.Now().UnixNano())).Scan(&collID) + require.NoError(t, err) + t.Cleanup(func() { _ = setup.DB.DeleteCollection(ctx, collID) }) + var tideID pgtype.UUID + err = setup.DBPool.QueryRow(ctx, `SELECT id FROM media_items WHERE title = $1`, "Sug Tide").Scan(&tideID) + require.NoError(t, err) + _, err = setup.DBPool.Exec(ctx, + `INSERT INTO collection_items (collection_id, media_item_id) VALUES ($1, $2)`, collID, tideID) + require.NoError(t, err) + + status, results = suggest("collection", " ", "") + require.Equal(t, http.StatusOK, status) + assert.True(t, func() bool { + for _, r := range results { + if r["value"] == collID.String() { + return true + } + } + return false + }(), "the collection holding a book is offered on browse") + + status, results = suggest("collection", " ", qfAll(qfRule("genre", "is", "spacewestern"))) + require.Equal(t, http.StatusOK, status) + assert.Empty(t, results, "zero-match containers never offered") } func TestFilterFamilyLens(t *testing.T) { diff --git a/internal/services/filter_query.go b/internal/services/filter_query.go index 91593f71..7a071880 100644 --- a/internal/services/filter_query.go +++ b/internal/services/filter_query.go @@ -869,15 +869,18 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge p.Typed = strings.TrimSpace(p.Typed) b := newFilterBuilder(p.UserID) - typedBind := b.bind(p.Typed) limitBind := b.bind(p.Limit) libBind := b.bind(p.LibraryID) // The typed filter differs per source family: substring for the // viewer's own words (note/highlight), name-filter for the id // sources (library/collection/added_by — you find them by name), - // word_similarity for metadata text. userBind is bound ONLY for the - // viewer-scoped sources — an unused parameter is a 42P18 error. + // word_similarity for metadata text. + // + // PARAMETER DISCIPLINE: every bind must be USED by the query it + // lands in — an unused parameter is a 42P18 error. The typed query + // is bound only in the branches that reference it, and only when + // non-blank (blank typed = browse: no typed filter, score 0). var userBind string var valueSource, typedFilter string switch field.Suggest { @@ -893,7 +896,7 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge WHERE t.media_item_id = mi.id AND t.user_id = %s ) AS src`, col, table, userBind) if p.Typed != "" { - typedFilter = " AND src.value ILIKE '%' || " + typedBind + " || '%'" + typedFilter = " AND src.value ILIKE '%' || " + b.bind(p.Typed) + " || '%'" } case suggestSeries: valueSource = `CROSS JOIN LATERAL ( @@ -901,20 +904,22 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge FROM unnest(ARRAY[mi.series, mi.alternate_info->>'alternate_series']) AS s WHERE s IS NOT NULL AND s <> '' ) AS src` - typedFilter = " AND word_similarity(" + typedBind + ", src.value) > 0.3" + if p.Typed != "" { + typedFilter = " AND word_similarity(" + b.bind(p.Typed) + ", src.value) > 0.3" + } case suggestLibrary: return s.composedIDSourceValues(ctx, b, p, "libraries l2", "l2.id::text", "l2.name", - "JOIN media_items mi ON mi.library_id = l2.id", typedBind, limitBind, libBind) + "JOIN media_items mi ON mi.library_id = l2.id", limitBind, libBind) case suggestCollection: return s.composedIDSourceValues(ctx, b, p, "collections c JOIN collection_items ci ON ci.collection_id = c.id", "c.id::text", "c.name", "JOIN media_items mi ON ci.media_item_id = mi.id AND ci.excluded IS NOT TRUE", - typedBind, limitBind, libBind) + limitBind, libBind) case suggestAddedBy: return s.composedIDSourceValues(ctx, b, p, "users u JOIN media_items mi ON mi.added_by_admin_id = u.id", - "u.id::text", "u.username", "", typedBind, limitBind, libBind) + "u.id::text", "u.username", "", limitBind, libBind) default: // Column / array sources: the value expr mirrors the field's // match column (tags via the *_search column, like the flat @@ -937,10 +942,17 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge } valueSource = "CROSS JOIN LATERAL (SELECT " + col + " AS value WHERE " + col + " IS NOT NULL AND " + col + " <> '') AS src" } - typedFilter = " AND word_similarity(" + typedBind + ", src.value) > 0.3" + if p.Typed != "" { + typedFilter = " AND word_similarity(" + b.bind(p.Typed) + ", src.value) > 0.3" + } } - if p.Typed == "" { - typedFilter = "" // no typing = the field's world, under the other rules + + // Score: similarity ranking only when something is typed; browse + // orders by count alone (and binds nothing extra). + scoreExpr := "0 AS score" + if p.Typed != "" { + tb := b.bind(p.Typed) + scoreExpr = fmt.Sprintf("CASE WHEN %s = '' THEN 0 ELSE word_similarity(%s, src.value) END AS score", tb, tb) } whereSQL := b.headWhere(p.LibraryID) + ` @@ -948,7 +960,7 @@ func (s *SearchService) ComposedFieldValues(ctx context.Context, p ComposedSugge treeWhere(p.Tree, b) + ` AND src.value IS NOT NULL AND src.value <> ''` + typedFilter sqlStr := `SELECT src.value, COUNT(*) AS count, - CASE WHEN ` + typedBind + ` = '' THEN 0 ELSE word_similarity(` + typedBind + `, src.value) END AS score + ` + scoreExpr + ` ` + filterFrom + b.userIDBind + ` ` + valueSource + b.joinBlock() + ` WHERE ` + whereSQL + ` @@ -985,10 +997,12 @@ LIMIT ` + limitBind // added_by): value = id, display = name, typing filters the NAME, and // counts are books-in-scope. Zero-count sources drop out (INNER JOIN). func (s *SearchService) composedIDSourceValues(ctx context.Context, b *filterBuilder, p ComposedSuggestParams, - fromTables, valueExpr, displayExpr, joinMedia, typedBind, limitBind, libBind string) ([]FieldValue, error) { + fromTables, valueExpr, displayExpr, joinMedia, limitBind, libBind string) ([]FieldValue, error) { + // The typed bind happens HERE and only when non-blank — an unused + // parameter is a 42P18 error (the blank-browse case binds nothing). nameFilter := "" if p.Typed != "" { - nameFilter = " AND " + displayExpr + " ILIKE '%' || " + typedBind + " || '%'" + nameFilter = " AND " + displayExpr + " ILIKE '%' || " + b.bind(p.Typed) + " || '%'" } sqlStr := `SELECT ` + valueExpr + ` AS value, ` + displayExpr + ` AS display, COUNT(mi.id) AS count FROM ` + fromTables + `