diff --git a/packages/core/__tests__/lookup.test.js b/packages/core/__tests__/lookup.test.js index 0c646d8be..30d593c56 100644 --- a/packages/core/__tests__/lookup.test.js +++ b/packages/core/__tests__/lookup.test.js @@ -209,4 +209,39 @@ describe("notesWithHighlighting", () => { ); expect(searchWithDiacritics.length).toBe(4); })); + + test("should not reuse filters (aka mutate selector) between searches", () => + databaseTest().then(async (db) => { + const note1Id = await db.notes.add({ title: "note 1" }); + const note2Id = await db.notes.add({ title: "note 2" }); + const note3Id = await db.notes.add({ title: "note 3" }); + const tag1Id = await db.tags.add({ title: "daily" }); + const tag2Id = await db.tags.add({ title: "academia" }); + await db.relations.add( + { id: tag1Id, type: "tag" }, + { id: note1Id, type: "note" } + ); + await db.relations.add( + { id: tag2Id, type: "tag" }, + { id: note2Id, type: "note" } + ); + + /** + * this test ensures that the selector passed to `notesWithHighlighting` is not mutated between searches + */ + const selector = db.notes.all; + + const tag1SearchResults = await db.lookup.notesWithHighlighting( + "tag:daily", + selector + ); + expect(await tag1SearchResults.ids()).toEqual([note1Id]); + expect(await selector.ids()).toEqual([note1Id, note2Id, note3Id]); + const bothTagResults = await db.lookup.notesWithHighlighting( + "tag:daily tag:academia", + selector + ); + expect(await bothTagResults.ids()).toEqual([note1Id, note2Id]); + expect(await selector.ids()).toEqual([note1Id, note2Id, note3Id]); + })); }); diff --git a/packages/core/src/api/lookup.ts b/packages/core/src/api/lookup.ts index 2096d10b8..71130041e 100644 --- a/packages/core/src/api/lookup.ts +++ b/packages/core/src/api/lookup.ts @@ -155,6 +155,13 @@ export default class Lookup { : []; const defaultVault = await this.db.vaults.default(); + + /** + * `notes` is a FilteredSelector whose `where` method mutates the selector. + * Thus the selector passed into `notesWithHighlighting` is mutated, which can cause unexpected results. + * To avoid this, we clone the selector before `where`. + */ + notes = notes.clone(); notes = notes.where((eb) => { const exprs = []; const tagsFilter = this.db.relations diff --git a/packages/core/src/database/sql-collection.ts b/packages/core/src/database/sql-collection.ts index 4c3e11da1..f6fcff074 100644 --- a/packages/core/src/database/sql-collection.ts +++ b/packages/core/src/database/sql-collection.ts @@ -388,6 +388,17 @@ export class FilteredSelector { this.filter = filter; } + clone() { + const selector = new FilteredSelector( + this.type, + this.filter, + this.batchSize + ); + selector._fields = this._fields.slice(); + selector._limit = this._limit; + return selector; + } + fields(fields: AnyColumnWithTable[]) { this._fields = fields; return this; diff --git a/packages/core/src/utils/__tests__/query-transformer.test.ts b/packages/core/src/utils/__tests__/query-transformer.test.ts index 8e1dc76d7..5e76fbe6d 100644 --- a/packages/core/src/utils/__tests__/query-transformer.test.ts +++ b/packages/core/src/utils/__tests__/query-transformer.test.ts @@ -77,3 +77,37 @@ for (const [input, expectedOutput] of TRANSFORM_QUERY_TESTS) { expect(transformQuery(input).content?.query).toBe(expectedOutput); }); } + +test("should remove syntax quotes from multi-word tag filters", () => { + expect(transformQuery('tag:"the diary"').tag).toEqual(["the diary"]); +}); + +test("should preserve multiple exact tag filter values", () => { + expect(transformQuery('tag:"the diary" title:journal tag:work').tag).toEqual([ + "the diary", + "work" + ]); +}); + +test("should remove syntax quotes from multi-word color filters", () => { + expect(transformQuery('color:"deep blue"').color).toEqual(["deep blue"]); +}); + +test("should remove syntax quotes added for punctuation in exact filters", () => { + expect(transformQuery("tag:foo-bar").tag).toEqual(["foo-bar"]); +}); + +test("should separate repeated tag filters", () => { + expect(transformQuery("tag:one tag:two").tag).toEqual(["one", "two"]); +}); + +test("should separate repeated color filters", () => { + expect(transformQuery("color:red color:blue").color).toEqual(["red", "blue"]); +}); + +test("should separate repeated quoted tag filters", () => { + expect(transformQuery('tag:"one two" tag:"three four"').tag).toEqual([ + "one two", + "three four" + ]); +}); diff --git a/packages/core/src/utils/query-transformer.ts b/packages/core/src/utils/query-transformer.ts index b97c64b82..489a00844 100644 --- a/packages/core/src/utils/query-transformer.ts +++ b/packages/core/src/utils/query-transformer.ts @@ -38,6 +38,12 @@ type FieldPhraseNode = { value: QueryNode; }; +type FieldToken = { + field?: string; + fieldOccurrence?: number; + token: string; +}; + type OperatorNode = { type: "AND" | "OR" | "NOT"; }; @@ -77,6 +83,8 @@ const SUPPORTED_FIELDS = { in_notebook: (ast) => parseBooleanField("in_notebook", ast) } satisfies Record unknown>; +const ARRAY_FIELDS = ["tag", "color"]; + function isFieldSupported(field: string) { return field in SUPPORTED_FIELDS; } @@ -101,10 +109,15 @@ function parseArrayField( (a): a is FieldPhraseNode => a.type === "field_phrase" && a.field === field ) - .map((a) => generateSQL(a.value)); + .map((a) => unquoteExactValue(generateSQL(a.value))); return values.length > 0 ? values : null; } +function unquoteExactValue(value: string) { + if (!value.startsWith('"') || !value.endsWith('"')) return value; + return value.slice(1, -1).replace(/""/g, '"'); +} + function parseDateField( field: string, ast: (QueryNode | FieldPhraseNode)[] @@ -146,13 +159,23 @@ function escapeSQLString(str: string): string { return str.replace(/"/g, '""'); } -function tokenizeWithFields( - query: string -): Array<{ field?: string; token: string }> { - const tokens: Array<{ field?: string; token: string }> = []; +function tokenizeWithFields(query: string): FieldToken[] { + const tokens: FieldToken[] = []; let buffer = ""; let isQuoted = false; let currentField: string | undefined = undefined; + let fieldOccurrence = 0; + + const pushToken = () => { + if (buffer.length > 0) { + tokens.push({ + field: currentField, + fieldOccurrence: currentField ? fieldOccurrence : undefined, + token: buffer + }); + buffer = ""; + } + }; for (let i = 0; i < query.length; ++i) { const char = query[i]; @@ -160,15 +183,13 @@ function tokenizeWithFields( isQuoted = !isQuoted; } if (char === " " && !isQuoted) { - if (buffer.length > 0) { - tokens.push({ field: currentField, token: buffer }); - buffer = ""; - } + pushToken(); } else if (char === ":" && !isQuoted) { // Check for field const maybeField = buffer.trim().toLowerCase(); if (isFieldSupported(maybeField)) { currentField = maybeField; + fieldOccurrence++; buffer = ""; } else { buffer += char; @@ -177,24 +198,31 @@ function tokenizeWithFields( buffer += char; } } - if (buffer.length > 0) tokens.push({ field: currentField, token: buffer }); + pushToken(); return tokens; } // Helper: group tokens by field -function groupTokensByField(tokens: Array<{ field?: string; token: string }>) { +function groupTokensByField(tokens: FieldToken[]) { const groups: Array<{ field?: string; tokens: string[] }> = []; let currentField: string | undefined = undefined; + let currentOccurrence: number | undefined = undefined; let currentTokens: string[] = []; - for (const { field, token } of tokens) { - if (field !== currentField) { + for (const { field, fieldOccurrence, token } of tokens) { + const isArrayField = ARRAY_FIELDS.includes(field || ""); + const shouldStartNewGroup = + field !== currentField || + (isArrayField && fieldOccurrence !== currentOccurrence); + + if (shouldStartNewGroup) { if (currentTokens.length > 0) { groups.push({ field: currentField, tokens: currentTokens }); currentTokens = []; } currentField = field; + currentOccurrence = fieldOccurrence; } currentTokens.push(token); }