mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(server): prevent broad fuzzy search matches
Restore containment as the search eligibility rule so common trigrams cannot admit unrelated books, authors, or series. Fixes #1490
This commit is contained in:
@@ -223,6 +223,30 @@ describe('AuthorsRepository', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('uses containment rather than trigram expansion for multi-word searches', async () => {
|
||||
const { db, selectBuilder } = makeDb();
|
||||
selectBuilder.where.mockReturnValueOnce(selectBuilder).mockResolvedValueOnce([{ total: '0' }]);
|
||||
selectBuilder.offset.mockResolvedValueOnce([]);
|
||||
vi.mocked(sql).mockClear();
|
||||
const repo = new AuthorsRepository(db as never);
|
||||
|
||||
await repo.findPage({
|
||||
page: 0,
|
||||
size: 20,
|
||||
sort: 'relevance',
|
||||
order: 'desc',
|
||||
libraryIds: [1],
|
||||
q: 'The Wax Child',
|
||||
});
|
||||
|
||||
const sqlText = vi
|
||||
.mocked(sql)
|
||||
.mock.calls.map(([strings]) => strings.join(''))
|
||||
.join(' ');
|
||||
expect(sqlText).toContain(' ILIKE ');
|
||||
expect(sqlText).not.toContain(' % ');
|
||||
});
|
||||
|
||||
it('findPage uses grouped subquery count when minBookCount is provided', async () => {
|
||||
const { db, selectBuilder } = makeDb();
|
||||
selectBuilder.from
|
||||
|
||||
@@ -572,10 +572,7 @@ export class AuthorsRepository {
|
||||
}
|
||||
const query = params.q?.trim();
|
||||
if (query) {
|
||||
const contains = accentInsensitiveIlike(authors.name, buildSearchPattern(query));
|
||||
clauses.push(
|
||||
query.length >= 3 ? or(contains, sql`public.bookorbit_unaccent(${authors.name}) % public.bookorbit_unaccent(${query})`)! : contains,
|
||||
);
|
||||
clauses.push(accentInsensitiveIlike(authors.name, buildSearchPattern(query)));
|
||||
}
|
||||
if (params.hasPhoto !== undefined) {
|
||||
clauses.push(eq(authors.hasPhoto, params.hasPhoto));
|
||||
|
||||
@@ -786,26 +786,31 @@ describe('buildQuickSearch', () => {
|
||||
|
||||
expect(result).toMatchObject({ type: 'or' });
|
||||
expect(result.clauses).toHaveLength(5);
|
||||
expect(result.clauses[0]).toMatchObject({
|
||||
type: 'or',
|
||||
clauses: [expect.objectContaining({ type: 'accentInsensitiveIlike', pattern: '%tolkien%' }), expect.objectContaining({ type: 'sql' })],
|
||||
});
|
||||
expect(result.clauses[0]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%tolkien%' });
|
||||
expect(result.clauses[1]).toMatchObject({ type: 'sql' });
|
||||
expect(result.clauses[2]).toMatchObject({
|
||||
type: 'or',
|
||||
clauses: [expect.objectContaining({ type: 'accentInsensitiveIlike', pattern: '%tolkien%' }), expect.objectContaining({ type: 'sql' })],
|
||||
});
|
||||
expect(result.clauses[2]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%tolkien%' });
|
||||
expect(result.clauses[3]).toMatchObject({ type: 'sql' });
|
||||
expect(result.clauses[4]).toMatchObject({ type: 'sql' });
|
||||
});
|
||||
|
||||
it('uses phrase containment without trigram expansion for multi-word searches', () => {
|
||||
const { builder } = makeBuilder();
|
||||
vi.mocked(accentInsensitiveIlike).mockClear();
|
||||
|
||||
const result = builder.buildQuickSearch('The Wax Child');
|
||||
|
||||
expect(accentInsensitiveIlike).toHaveBeenCalledTimes(5);
|
||||
expect(accentInsensitiveIlike).toHaveBeenCalledWith(expect.anything(), '%The Wax Child%');
|
||||
expect(collectSqlText(result).join(' ')).not.toContain(' % ');
|
||||
});
|
||||
|
||||
it('escapes LIKE special characters in q', () => {
|
||||
const { builder } = makeBuilder();
|
||||
|
||||
const result = builder.buildQuickSearch('50% off') as any;
|
||||
|
||||
expect(result.clauses[0].clauses[0]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%50\\% off%' });
|
||||
expect(result.clauses[2].clauses[0]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%50\\% off%' });
|
||||
expect(result.clauses[0]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%50\\% off%' });
|
||||
expect(result.clauses[2]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%50\\% off%' });
|
||||
});
|
||||
|
||||
it('escapes underscore in q', () => {
|
||||
@@ -813,10 +818,10 @@ describe('buildQuickSearch', () => {
|
||||
|
||||
const result = builder.buildQuickSearch('book_one') as any;
|
||||
|
||||
expect(result.clauses[0].clauses[0]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%book\\_one%' });
|
||||
expect(result.clauses[0]).toMatchObject({ type: 'accentInsensitiveIlike', pattern: '%book\\_one%' });
|
||||
});
|
||||
|
||||
it('does not add fuzzy predicates for two-character queries', () => {
|
||||
it('uses the same containment predicate for two-character queries', () => {
|
||||
const { builder } = makeBuilder();
|
||||
|
||||
const result = builder.buildQuickSearch('du') as any;
|
||||
|
||||
@@ -99,7 +99,7 @@ export class BookQueryBuilder {
|
||||
.select({ one: sql`1` })
|
||||
.from(bookAuthors)
|
||||
.innerJoin(authors, eq(bookAuthors.authorId, authors.id))
|
||||
.where(and(eq(bookAuthors.bookId, books.id), this.buildSearchTextMatch(authors.name, pattern, q))!);
|
||||
.where(and(eq(bookAuthors.bookId, books.id), accentInsensitiveIlike(authors.name, pattern))!);
|
||||
return sql`exists (${sq})`;
|
||||
})();
|
||||
|
||||
@@ -108,7 +108,7 @@ export class BookQueryBuilder {
|
||||
.select({ one: sql`1` })
|
||||
.from(bookNarrators)
|
||||
.innerJoin(narrators, eq(bookNarrators.narratorId, narrators.id))
|
||||
.where(and(eq(bookNarrators.bookId, books.id), this.buildSearchTextMatch(narrators.name, pattern, q))!);
|
||||
.where(and(eq(bookNarrators.bookId, books.id), accentInsensitiveIlike(narrators.name, pattern))!);
|
||||
return sql`exists (${sq})`;
|
||||
})();
|
||||
|
||||
@@ -117,14 +117,14 @@ export class BookQueryBuilder {
|
||||
.select({ one: sql`1` })
|
||||
.from(bookSeriesMemberships)
|
||||
.innerJoin(bookSeries, eq(bookSeries.id, bookSeriesMemberships.seriesId))
|
||||
.where(and(eq(bookSeriesMemberships.bookId, books.id), this.buildSearchTextMatch(bookSeries.name, pattern, q))!);
|
||||
.where(and(eq(bookSeriesMemberships.bookId, books.id), accentInsensitiveIlike(bookSeries.name, pattern))!);
|
||||
return sql`exists (${sq})`;
|
||||
})();
|
||||
|
||||
return or(
|
||||
this.buildSearchTextMatch(bookMetadata.title, pattern, q),
|
||||
accentInsensitiveIlike(bookMetadata.title, pattern),
|
||||
existsAuthor,
|
||||
this.buildSearchTextMatch(bookMetadata.seriesName, pattern, q),
|
||||
accentInsensitiveIlike(bookMetadata.seriesName, pattern),
|
||||
existsSeries,
|
||||
existsNarrator,
|
||||
)!;
|
||||
@@ -134,12 +134,6 @@ export class BookQueryBuilder {
|
||||
return this.sortBuilder.build(sort, userId, customFieldTypes, context);
|
||||
}
|
||||
|
||||
private buildSearchTextMatch(column: AnyColumn, pattern: string, query: string): SQL {
|
||||
const contains = accentInsensitiveIlike(column, pattern);
|
||||
if (query.trim().length < 3) return contains;
|
||||
return or(contains, sql`public.bookorbit_unaccent(${column}) % public.bookorbit_unaccent(${query})`)!;
|
||||
}
|
||||
|
||||
private groupToSql(node: GroupRule, depth: number, userId?: number, timeZone = 'UTC'): SQL {
|
||||
if (depth > 5) throw new BadRequestException('Filter nesting exceeds maximum depth of 5');
|
||||
const clauses = node.rules.map((r) =>
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type { SQL } from 'drizzle-orm';
|
||||
import { PgDialect } from 'drizzle-orm/pg-core';
|
||||
|
||||
import { SeriesRepository } from './series.repository';
|
||||
|
||||
@@ -231,6 +233,20 @@ describe('SeriesRepository', () => {
|
||||
expect(result.page).toBe(3);
|
||||
expect(result.size).toBe(15);
|
||||
});
|
||||
|
||||
it('uses containment rather than trigram expansion for multi-word searches', async () => {
|
||||
const { select, baseChain } = makeFindPageDb([{ ...NO_FACETS }], []);
|
||||
db.select = select;
|
||||
stubPageHelpers(repo);
|
||||
|
||||
await repo.findPage({ ...BASE_PARAMS, q: 'The Wax Child', sort: 'relevance', order: 'desc' });
|
||||
|
||||
const where = (baseChain.where as ReturnType<typeof vi.fn>).mock.calls[0]![0] as SQL;
|
||||
const query = new PgDialect().sqlToQuery(where);
|
||||
expect(query.sql).toContain(' ILIKE ');
|
||||
expect(query.sql).not.toContain(' % ');
|
||||
expect(query.params).toContain('%The Wax Child%');
|
||||
});
|
||||
});
|
||||
|
||||
describe('countSeries', () => {
|
||||
|
||||
@@ -79,14 +79,11 @@ export class SeriesRepository {
|
||||
return inArray(books.libraryId, libraryIds);
|
||||
}
|
||||
|
||||
private buildAuthorNameMatchCondition(pattern: string, query?: string): SQL {
|
||||
const contains = accentInsensitiveIlike(authors.name, pattern);
|
||||
const nameMatch =
|
||||
query && query.length >= 3 ? or(contains, sql`public.bookorbit_unaccent(${authors.name}) % public.bookorbit_unaccent(${query})`)! : contains;
|
||||
private buildAuthorNameMatchCondition(pattern: string): SQL {
|
||||
return sql`${books.id} IN (
|
||||
SELECT ${bookAuthors.bookId} FROM ${bookAuthors}
|
||||
INNER JOIN ${authors} ON ${authors.id} = ${bookAuthors.authorId}
|
||||
WHERE ${nameMatch}
|
||||
WHERE ${accentInsensitiveIlike(authors.name, pattern)}
|
||||
)`;
|
||||
}
|
||||
|
||||
@@ -109,18 +106,13 @@ export class SeriesRepository {
|
||||
|
||||
if (params.q) {
|
||||
const qPattern = buildSearchPattern(params.q);
|
||||
const authorNameMatch = this.buildAuthorNameMatchCondition(qPattern, params.q);
|
||||
const nameContains = accentInsensitiveIlike(bookSeries.name, qPattern);
|
||||
const nameMatch =
|
||||
params.q.length >= 3
|
||||
? or(nameContains, sql`public.bookorbit_unaccent(${bookSeries.name}) % public.bookorbit_unaccent(${params.q})`)!
|
||||
: nameContains;
|
||||
conditions.push(sql`(${nameMatch} OR ${authorNameMatch})`);
|
||||
const authorNameMatch = this.buildAuthorNameMatchCondition(qPattern);
|
||||
conditions.push(sql`(${accentInsensitiveIlike(bookSeries.name, qPattern)} OR ${authorNameMatch})`);
|
||||
}
|
||||
|
||||
if (params.author) {
|
||||
const authorPattern = buildSearchPattern(params.author);
|
||||
conditions.push(this.buildAuthorNameMatchCondition(authorPattern, params.author));
|
||||
conditions.push(this.buildAuthorNameMatchCondition(authorPattern));
|
||||
}
|
||||
|
||||
const baseWhere = and(...conditions)!;
|
||||
|
||||
Reference in New Issue
Block a user