[PM-24143] Search performance improvements (#16070)

* [PM-24143] Add perUserCache$ to SearchService index$

* [PM-24143] Cleanup and optimize isSearchable

* [PM-24143] Remove unused search flags and subscription from the vault-items component

* [PM-24143] Add search text for desktop vault items. Consolidate SearchTextDebounceInterval constant to SearchService

* [PM-24143] Ensure cipher search indexing is non-blocking

* [PM-24143] Remove redundant index ciphers operation

* [PM-24143] Add search performance measurements

* [PM-24143] Remove artificial delay

* [PM-24143] Remove startWith from index$ to avoid basic search with lunr queries

* [PM-24143] Re-organize isSearchable to check long lunr queries for index existence

libs/common/src/vault/abstractions/search.service.ts

@@ -9,7 +9,12 @@ export abstract class SearchService {
   abstract indexedEntityId$(userId: UserId): Observable<IndexedEntityId | null>;
 
   abstract clearIndex(userId: UserId): Promise<void>;
-  abstract isSearchable(userId: UserId, query: string): Promise<boolean>;
+
+  /**
+   * Checks if the query is long enough to be searchable.
+   * For short Lunr.js queries (starts with '>'), a valid search index must exist for the user.
+   */
+  abstract isSearchable(userId: UserId, query: string | null): Promise<boolean>;
   abstract indexCiphers(
     userId: UserId,
     ciphersToIndex: CipherView[],

libs/common/src/vault/services/cipher.service.ts

@@ -183,8 +183,7 @@ export class CipherService implements CipherServiceAbstraction {
     ]).pipe(
       filter(([ciphers, _, keys]) => ciphers != null && keys != null), // Skip if ciphers haven't been loaded yor synced yet
       switchMap(() => this.getAllDecrypted(userId)),
-      tap(async (decrypted) => {
-        await this.searchService.indexCiphers(userId, decrypted);
+      tap(() => {
         this.messageSender.send("updateOverlayCiphers");
       }),
     );
@@ -216,7 +215,7 @@ export class CipherService implements CipherServiceAbstraction {
       if (value == null) {
         await this.searchService.clearIndex(userId);
       } else {
-        await this.searchService.indexCiphers(userId, value);
+        void this.searchService.indexCiphers(userId, value);
       }
     }
   }

libs/common/src/vault/services/search.service.spec.ts

@@ -0,0 +1,97 @@
+import { BehaviorSubject } from "rxjs";
+
+import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
+import { UserId } from "@bitwarden/common/types/guid";
+
+import { FakeStateProvider, mockAccountServiceWith } from "../../../spec";
+
+import { SearchService } from "./search.service";
+
+describe("SearchService", () => {
+  let fakeStateProvider: FakeStateProvider;
+  let service: SearchService;
+
+  const userId = "user-id" as UserId;
+  const mockLogService = {
+    error: jest.fn(),
+    measure: jest.fn(),
+  };
+  const mockLocale$ = new BehaviorSubject<string>("en");
+  const mockI18nService = {
+    locale$: mockLocale$.asObservable(),
+  };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+
+    fakeStateProvider = new FakeStateProvider(mockAccountServiceWith(userId));
+    service = new SearchService(
+      mockLogService as unknown as LogService,
+      mockI18nService as unknown as I18nService,
+      fakeStateProvider,
+    );
+  });
+
+  describe("isSearchable", () => {
+    let mockIndex$: jest.Mock;
+    beforeEach(() => {
+      mockIndex$ = jest.fn();
+      service["index$"] = mockIndex$;
+    });
+
+    it("returns false if the query is empty", async () => {
+      const result = await service.isSearchable(userId, "");
+      expect(result).toBe(false);
+      // Ensure we do not call the expensive index$ method
+      expect(mockIndex$).not.toHaveBeenCalled();
+    });
+
+    it("returns false if the query is null", async () => {
+      const result = await service.isSearchable(userId, null as any);
+      expect(result).toBe(false);
+      // Ensure we do not call the expensive index$ method
+      expect(mockIndex$).not.toHaveBeenCalled();
+    });
+
+    it("return true if the query is longer than searchableMinLength", async () => {
+      service["searchableMinLength"] = 3;
+      const result = await service.isSearchable(userId, "test");
+      expect(result).toBe(true);
+      // Ensure we do not call the expensive index$ method
+      expect(mockIndex$).not.toHaveBeenCalled();
+    });
+
+    it("returns false if the query is shorter than searchableMinLength", async () => {
+      service["searchableMinLength"] = 5;
+      const result = await service.isSearchable(userId, "test");
+      expect(result).toBe(false);
+      // Ensure we do not call the expensive index$ method
+      expect(mockIndex$).not.toHaveBeenCalled();
+    });
+
+    it("returns false for short Lunr query with missing index", async () => {
+      mockIndex$.mockReturnValue(new BehaviorSubject(null));
+      service["searchableMinLength"] = 3;
+      const result = await service.isSearchable(userId, ">l");
+      expect(result).toBe(false);
+      expect(mockIndex$).toHaveBeenCalledWith(userId);
+    });
+
+    it("returns false for long Lunr query with missing index", async () => {
+      mockIndex$.mockReturnValue(new BehaviorSubject(null));
+      service["searchableMinLength"] = 3;
+      const result = await service.isSearchable(userId, ">longer");
+      expect(result).toBe(false);
+      expect(mockIndex$).toHaveBeenCalledWith(userId);
+    });
+
+    it("returns true for short Lunr query with index", async () => {
+      mockIndex$.mockReturnValue(new BehaviorSubject(true));
+      service["searchableMinLength"] = 3;
+      const result = await service.isSearchable(userId, ">l");
+      expect(result).toBe(true);
+      expect(mockIndex$).toHaveBeenCalledWith(userId);
+    });
+  });
+});

libs/common/src/vault/services/search.service.ts

@@ -4,6 +4,8 @@ import * as lunr from "lunr";
 import { Observable, firstValueFrom, map } from "rxjs";
 import { Jsonify } from "type-fest";
 
+import { perUserCache$ } from "@bitwarden/common/vault/utils/observable-utilities";
+
 import { UriMatchStrategy } from "../../models/domain/domain-service";
 import { I18nService } from "../../platform/abstractions/i18n.service";
 import { LogService } from "../../platform/abstractions/log.service";
@@ -21,6 +23,9 @@ import { CipherType } from "../enums/cipher-type";
 import { CipherView } from "../models/view/cipher.view";
 import { CipherViewLike, CipherViewLikeUtils } from "../utils/cipher-view-like-utils";
 
+// Time to wait before performing a search after the user stops typing.
+export const SearchTextDebounceInterval = 200; // milliseconds
+
 export type SerializedLunrIndex = {
   version: string;
   fields: string[];
@@ -101,11 +106,19 @@ export class SearchService implements SearchServiceAbstraction {
     return this.stateProvider.getUser(userId, LUNR_SEARCH_INDEX);
   }
 
-  private index$(userId: UserId): Observable<lunr.Index | null> {
+  private index$ = perUserCache$((userId: UserId) => {
     return this.searchIndexState(userId).state$.pipe(
-      map((searchIndex) => (searchIndex ? lunr.Index.load(searchIndex) : null)),
+      map((searchIndex) => {
+        let index: lunr.Index | null = null;
+        if (searchIndex) {
+          const loadTime = performance.now();
+          index = lunr.Index.load(searchIndex);
+          this.logService.measure(loadTime, "Vault", "SearchService", "index load");
+        }
+        return index;
+      }),
     );
-  }
+  });
 
   private searchIndexEntityIdState(userId: UserId): SingleUserState<IndexedEntityId | null> {
     return this.stateProvider.getUser(userId, LUNR_SEARCH_INDEXED_ENTITY_ID);
@@ -129,17 +142,22 @@ export class SearchService implements SearchServiceAbstraction {
     await this.searchIsIndexingState(userId).update(() => null);
   }
 
-  async isSearchable(userId: UserId, query: string): Promise<boolean> {
-    const time = performance.now();
+  async isSearchable(userId: UserId, query: string | null): Promise<boolean> {
     query = SearchService.normalizeSearchQuery(query);
-    const index = await this.getIndexForSearch(userId);
-    const notSearchable =
-      query == null ||
-      (index == null && query.length < this.searchableMinLength) ||
-      (index != null && query.length < this.searchableMinLength && query.indexOf(">") !== 0);
 
-    this.logService.measure(time, "Vault", "SearchService", "isSearchable");
-    return !notSearchable;
+    // Nothing to search if the query is null
+    if (query == null || query === "") {
+      return false;
+    }
+
+    const isLunrQuery = query.indexOf(">") === 0;
+    if (isLunrQuery) {
+      // Lunr queries always require an index
+      return (await this.getIndexForSearch(userId)) != null;
+    }
+
+    // Regular queries only require a minimum length
+    return query.length >= this.searchableMinLength;
   }
 
   async indexCiphers(
@@ -205,6 +223,7 @@ export class SearchService implements SearchServiceAbstraction {
     ciphers: C[],
   ): Promise<C[]> {
     const results: C[] = [];
+    const searchStartTime = performance.now();
     if (query != null) {
       query = SearchService.normalizeSearchQuery(query.trim().toLowerCase());
     }
@@ -236,7 +255,9 @@ export class SearchService implements SearchServiceAbstraction {
     const index = await this.getIndexForSearch(userId);
     if (index == null) {
       // Fall back to basic search if index is not available
-      return this.searchCiphersBasic(ciphers, query);
+      const basicResults = this.searchCiphersBasic(ciphers, query);
+      this.logService.measure(searchStartTime, "Vault", "SearchService", "basic search complete");
+      return basicResults;
     }
 
     const ciphersMap = new Map<string, C>();
@@ -270,6 +291,7 @@ export class SearchService implements SearchServiceAbstraction {
         }
       });
     }
+    this.logService.measure(searchStartTime, "Vault", "SearchService", "search complete");
     return results;
   }