core: prevent db.lookup.notesWithHighlighting from mutating the passed in selector

The FilteredSelector passed into notesWithHighlighting was being mutated. This wasn't an issue in the apps/web because a fresh selector was passed in for every query. But on apps/mobile, the same selector is reused for every query on the search page.

The bug only became apparent when passing in a query like `tag:one`, then after the results came in, continuing the query: `tag:one tag:two`. This only showed results from tag:one since the selector was mutated by the previous lookup.

The mutation happens in FilteredSelector's where method. So, the bug is fixed by cloning the selector before the where method call in notesWithHighlighting.

Signed-off-by: 01zulfi <85733202+01zulfi@users.noreply.github.com>

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]);
+    }));
 });

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

packages/core/src/database/sql-collection.ts

@@ -388,6 +388,17 @@ export class FilteredSelector<T extends Item> {
     this.filter = filter;
   }
 
+  clone() {
+    const selector = new FilteredSelector<T>(
+      this.type,
+      this.filter,
+      this.batchSize
+    );
+    selector._fields = this._fields.slice();
+    selector._limit = this._limit;
+    return selector;
+  }
+
   fields(fields: AnyColumnWithTable<DatabaseSchema, keyof DatabaseSchema>[]) {
     this._fields = fields;
     return this;