[PM-329] Detangle SearchService & CipherService (#4838)

* Remove Circular Dependency

* Fix Vault Searching

* Remove Unused cipherServiceOptions

* Add searchService Parameter to CipherService

* Fix instantiation of CipherService in test

apps/browser/src/autofill/browser/cipher-context-menu-handler.ts

@@ -1,4 +1,3 @@
-import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { StateFactory } from "@bitwarden/common/factories/stateFactory";
@@ -14,7 +13,6 @@ import {
   AuthServiceInitOptions,
 } from "../../auth/background/service-factories/auth-service.factory";
 import { CachedServices } from "../../background/service_factories/factory-options";
-import { searchServiceFactory } from "../../background/service_factories/search-service.factory";
 import { BrowserApi } from "../../browser/browserApi";
 import { Account } from "../../models/account";
 import {
@@ -45,14 +43,10 @@ export class CipherContextMenuHandler {
 
   static async create(cachedServices: CachedServices) {
     const stateFactory = new StateFactory(GlobalState, Account);
-    let searchService: SearchService | null = null;
     const serviceOptions: AuthServiceInitOptions & CipherServiceInitOptions = {
       apiServiceOptions: {
         logoutCallback: NOT_IMPLEMENTED,
       },
-      cipherServiceOptions: {
-        searchServiceFactory: () => searchService,
-      },
       cryptoFunctionServiceOptions: {
         win: self,
       },
@@ -80,7 +74,6 @@ export class CipherContextMenuHandler {
         stateFactory: stateFactory,
       },
     };
-    searchService = await searchServiceFactory(cachedServices, serviceOptions);
     return new CipherContextMenuHandler(
       await MainContextMenuHandler.mv3Create(cachedServices),
       await authServiceFactory(cachedServices, serviceOptions),

apps/browser/src/autofill/browser/context-menu-clicked-handler.ts

@@ -1,5 +1,4 @@
 import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
-import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { TotpService } from "@bitwarden/common/abstractions/totp.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
@@ -19,7 +18,6 @@ import LockedVaultPendingNotificationsItem from "../../background/models/lockedV
 import { eventCollectionServiceFactory } from "../../background/service_factories/event-collection-service.factory";
 import { CachedServices } from "../../background/service_factories/factory-options";
 import { passwordGenerationServiceFactory } from "../../background/service_factories/password-generation-service.factory";
-import { searchServiceFactory } from "../../background/service_factories/search-service.factory";
 import { stateServiceFactory } from "../../background/service_factories/state-service.factory";
 import { BrowserApi } from "../../browser/browserApi";
 import { Account } from "../../models/account";
@@ -63,14 +61,10 @@ export class ContextMenuClickedHandler {
 
   static async mv3Create(cachedServices: CachedServices) {
     const stateFactory = new StateFactory(GlobalState, Account);
-    let searchService: SearchService | null = null;
     const serviceOptions: AuthServiceInitOptions & CipherServiceInitOptions = {
       apiServiceOptions: {
         logoutCallback: NOT_IMPLEMENTED,
       },
-      cipherServiceOptions: {
-        searchServiceFactory: () => searchService,
-      },
       cryptoFunctionServiceOptions: {
         win: self,
       },
@@ -98,7 +92,6 @@ export class ContextMenuClickedHandler {
         stateFactory: stateFactory,
       },
     };
-    searchService = await searchServiceFactory(cachedServices, serviceOptions);
 
     const generatePasswordToClipboardCommand = new GeneratePasswordToClipboardCommand(
       await passwordGenerationServiceFactory(cachedServices, serviceOptions),

apps/browser/src/background/main.background.ts

@@ -303,12 +303,14 @@ export default class MainBackground {
       this.apiService,
       this.fileUploadService
     );
+    this.searchService = new SearchService(this.logService, this.i18nService);
+
     this.cipherService = new CipherService(
       this.cryptoService,
       this.settingsService,
       this.apiService,
       this.i18nService,
-      () => this.searchService,
+      this.searchService,
       this.stateService,
       this.encryptService,
       this.cipherFileUploadService
@@ -325,7 +327,6 @@ export default class MainBackground {
       this.i18nService,
       this.stateService
     );
-    this.searchService = new SearchService(this.cipherService, this.logService, this.i18nService);
     this.syncNotifierService = new SyncNotifierService();
     this.organizationService = new BrowserOrganizationService(this.stateService);
     this.policyService = new BrowserPolicyService(this.stateService, this.organizationService);

apps/browser/src/background/service_factories/search-service.factory.ts

@@ -1,11 +1,6 @@
 import { SearchService as AbstractSearchService } from "@bitwarden/common/abstractions/search.service";
 import { SearchService } from "@bitwarden/common/services/search.service";
 
-import {
-  cipherServiceFactory,
-  CipherServiceInitOptions,
-} from "../../vault/background/service_factories/cipher-service.factory";
-
 import { CachedServices, factory, FactoryOptions } from "./factory-options";
 import { i18nServiceFactory, I18nServiceInitOptions } from "./i18n-service.factory";
 import { logServiceFactory, LogServiceInitOptions } from "./log-service.factory";
@@ -13,7 +8,6 @@ import { logServiceFactory, LogServiceInitOptions } from "./log-service.factory"
 type SearchServiceFactoryOptions = FactoryOptions;
 
 export type SearchServiceInitOptions = SearchServiceFactoryOptions &
-  CipherServiceInitOptions &
   LogServiceInitOptions &
   I18nServiceInitOptions;
 
@@ -26,10 +20,6 @@ export function searchServiceFactory(
     "searchService",
     opts,
     async () =>
-      new SearchService(
+      new SearchService(await logServiceFactory(cache, opts), await i18nServiceFactory(cache, opts))
-        await cipherServiceFactory(cache, opts),
-        await logServiceFactory(cache, opts),
-        await i18nServiceFactory(cache, opts)
-      )
   );
 }

apps/browser/src/listeners/onCommandListener.ts

@@ -1,4 +1,3 @@
-import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { StateFactory } from "@bitwarden/common/factories/stateFactory";
 import { GlobalState } from "@bitwarden/common/models/domain/global-state";
@@ -60,9 +59,6 @@ const doAutoFillLogin = async (tab: chrome.tabs.Tab): Promise<void> => {
     i18nServiceOptions: {
       systemLanguage: BrowserApi.getUILanguage(self),
     },
-    cipherServiceOptions: {
-      searchServiceFactory: null as () => SearchService, // No dependence on search service
-    },
   };
   const logService = await logServiceFactory(cachedServices, opts);
   const authService = await authServiceFactory(cachedServices, opts);

apps/browser/src/listeners/update-badge.ts

@@ -9,7 +9,6 @@ import { ContainerService } from "@bitwarden/common/services/container.service";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 
 import { authServiceFactory } from "../auth/background/service-factories/auth-service.factory";
-import { searchServiceFactory } from "../background/service_factories/search-service.factory";
 import { stateServiceFactory } from "../background/service_factories/state-service.factory";
 import { BrowserApi } from "../browser/browserApi";
 import { Account } from "../models/account";
@@ -279,12 +278,7 @@ export class UpdateBadge {
     };
     this.stateService = await stateServiceFactory(serviceCache, opts);
     this.authService = await authServiceFactory(serviceCache, opts);
-    const searchService = await searchServiceFactory(serviceCache, opts);
+    this.cipherService = await cipherServiceFactory(serviceCache, opts);
-
-    this.cipherService = await cipherServiceFactory(serviceCache, {
-      ...opts,
-      cipherServiceOptions: { searchServiceFactory: () => searchService },
-    });
 
     // Needed for cipher decryption
     if (!self.bitwardenContainerService) {

apps/browser/src/popup/services/popup-search.service.ts

@@ -1,16 +1,14 @@
 import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { ConsoleLogService } from "@bitwarden/common/services/consoleLog.service";
 import { SearchService } from "@bitwarden/common/services/search.service";
-import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 
 export class PopupSearchService extends SearchService {
   constructor(
     private mainSearchService: SearchService,
-    cipherService: CipherService,
     consoleLogService: ConsoleLogService,
     i18nService: I18nService
   ) {
-    super(cipherService, consoleLogService, i18nService);
+    super(consoleLogService, i18nService);
   }
 
   clearIndex() {

apps/browser/src/popup/services/services.module.ts

@@ -164,19 +164,14 @@ function getBgService<T>(service: keyof MainBackground) {
     },
     {
       provide: SearchServiceAbstraction,
-      useFactory: (
+      useFactory: (logService: ConsoleLogService, i18nService: I18nServiceAbstraction) => {
-        cipherService: CipherService,
-        logService: ConsoleLogService,
-        i18nService: I18nServiceAbstraction
-      ) => {
         return new PopupSearchService(
           getBgService<SearchService>("searchService")(),
-          cipherService,
           logService,
           i18nService
         );
       },
-      deps: [CipherService, LogServiceAbstraction, I18nServiceAbstraction],
+      deps: [LogServiceAbstraction, I18nServiceAbstraction],
     },
     { provide: AuditService, useFactory: getBgService<AuditService>("auditService"), deps: [] },
     {

apps/browser/src/vault/background/service_factories/cipher-service.factory.ts

@@ -1,4 +1,3 @@
-import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { CipherService as AbstractCipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherService } from "@bitwarden/common/vault/services/cipher.service";
 
@@ -27,6 +26,10 @@ import {
   i18nServiceFactory,
   I18nServiceInitOptions,
 } from "../../../background/service_factories/i18n-service.factory";
+import {
+  searchServiceFactory,
+  SearchServiceInitOptions,
+} from "../../../background/service_factories/search-service.factory";
 import {
   SettingsServiceInitOptions,
   settingsServiceFactory,
@@ -36,11 +39,7 @@ import {
   StateServiceInitOptions,
 } from "../../../background/service_factories/state-service.factory";
 
-type CipherServiceFactoryOptions = FactoryOptions & {
+type CipherServiceFactoryOptions = FactoryOptions;
-  cipherServiceOptions?: {
-    searchServiceFactory?: () => SearchService;
-  };
-};
 
 export type CipherServiceInitOptions = CipherServiceFactoryOptions &
   CryptoServiceInitOptions &
@@ -48,6 +47,7 @@ export type CipherServiceInitOptions = CipherServiceFactoryOptions &
   ApiServiceInitOptions &
   CipherFileUploadServiceInitOptions &
   I18nServiceInitOptions &
+  SearchServiceInitOptions &
   StateServiceInitOptions &
   EncryptServiceInitOptions;
 
@@ -65,9 +65,7 @@ export function cipherServiceFactory(
         await settingsServiceFactory(cache, opts),
         await apiServiceFactory(cache, opts),
         await i18nServiceFactory(cache, opts),
-        opts.cipherServiceOptions?.searchServiceFactory === undefined
+        await searchServiceFactory(cache, opts),
-          ? () => cache.searchService as SearchService
-          : opts.cipherServiceOptions.searchServiceFactory,
         await stateServiceFactory(cache, opts),
         await encryptServiceFactory(cache, opts),
         await cipherFileUploadServiceFactory(cache, opts)

apps/browser/src/vault/popup/components/vault/vault-items.component.ts

@@ -66,10 +66,10 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnIn
     private folderService: FolderService,
     private collectionService: CollectionService,
     private platformUtilsService: PlatformUtilsService,
-    private cipherService: CipherService,
+    cipherService: CipherService,
     private vaultFilterService: VaultFilterService
   ) {
-    super(searchService);
+    super(searchService, cipherService);
     this.applySavedState =
       (window as any).previousPopupUrl != null &&
       !(window as any).previousPopupUrl.startsWith("/ciphers");

apps/cli/src/bw.ts

@@ -239,12 +239,14 @@ export class Main {
       this.sendService
     );
 
+    this.searchService = new SearchService(this.logService, this.i18nService);
+
     this.cipherService = new CipherService(
       this.cryptoService,
       this.settingsService,
       this.apiService,
       this.i18nService,
-      null,
+      this.searchService,
       this.stateService,
       this.encryptService,
       this.cipherFileUploadService
@@ -267,8 +269,6 @@ export class Main {
       this.stateService
     );
 
-    this.searchService = new SearchService(this.cipherService, this.logService, this.i18nService);
-
     this.providerService = new ProviderService(this.stateService);
 
     this.organizationService = new OrganizationService(this.stateService);

apps/desktop/src/vault/app/vault/vault-items.component.ts

@@ -2,6 +2,7 @@ import { Component } from "@angular/core";
 
 import { VaultItemsComponent as BaseVaultItemsComponent } from "@bitwarden/angular/vault/components/vault-items.component";
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
+import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
 import { SearchBarService } from "../../../app/layout/search/search-bar.service";
@@ -12,8 +13,12 @@ import { SearchBarService } from "../../../app/layout/search/search-bar.service"
 })
 // eslint-disable-next-line rxjs-angular/prefer-takeuntil
 export class VaultItemsComponent extends BaseVaultItemsComponent {
-  constructor(searchService: SearchService, searchBarService: SearchBarService) {
+  constructor(
-    super(searchService);
+    searchService: SearchService,
+    searchBarService: SearchBarService,
+    cipherService: CipherService
+  ) {
+    super(searchService, cipherService);
 
     // eslint-disable-next-line rxjs-angular/prefer-takeuntil
     searchBarService.searchText$.subscribe((searchText) => {

apps/web/src/app/vault/individual-vault/vault-items.component.ts

@@ -118,7 +118,7 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnDe
     protected i18nService: I18nService,
     protected platformUtilsService: PlatformUtilsService,
     protected vaultFilterService: VaultFilterService,
-    protected cipherService: CipherService,
+    cipherService: CipherService,
     protected eventCollectionService: EventCollectionService,
     protected totpService: TotpService,
     protected stateService: StateService,
@@ -129,7 +129,7 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnDe
     private organizationService: OrganizationService,
     private tokenService: TokenService
   ) {
-    super(searchService);
+    super(searchService, cipherService);
   }
 
   ngOnDestroy() {
@@ -223,6 +223,7 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnDe
   }
 
   async doSearch(indexedCiphers?: CipherView[]) {
+    indexedCiphers = indexedCiphers ?? (await this.cipherService.getAllDecrypted());
     this.ciphers = await this.searchService.searchCiphers(
       this.searchText,
       [this.filter, this.deletedFilter],

apps/web/src/app/vault/org-vault/vault-items.component.ts

@@ -110,7 +110,8 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnDe
         (c) => c.organizationId === this.organization?.id
       );
     }
-    await this.searchService.indexCiphers(this.organization?.id, this.allCiphers);
+
+    this.searchService.indexCiphers(this.allCiphers, this.organization?.id);
   }
 
   async refreshCollections(): Promise<void> {

libs/angular/src/services/jslib-services.module.ts

@@ -1,4 +1,4 @@
-import { Injector, LOCALE_ID, NgModule } from "@angular/core";
+import { LOCALE_ID, NgModule } from "@angular/core";
 
 import { AvatarUpdateService as AccountUpdateServiceAbstraction } from "@bitwarden/common/abstractions/account/avatar-update.service";
 import { AnonymousHubService as AnonymousHubServiceAbstraction } from "@bitwarden/common/abstractions/anonymousHub.service";
@@ -251,7 +251,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
         settingsService: SettingsServiceAbstraction,
         apiService: ApiServiceAbstraction,
         i18nService: I18nServiceAbstraction,
-        injector: Injector,
+        searchService: SearchServiceAbstraction,
         stateService: StateServiceAbstraction,
         encryptService: EncryptService,
         fileUploadService: CipherFileUploadServiceAbstraction
@@ -261,7 +261,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
           settingsService,
           apiService,
           i18nService,
-          () => injector.get(SearchServiceAbstraction),
+          searchService,
           stateService,
           encryptService,
           fileUploadService
@@ -271,7 +271,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
         SettingsServiceAbstraction,
         ApiServiceAbstraction,
         I18nServiceAbstraction,
-        Injector, // TODO: Get rid of this circular dependency!
+        SearchServiceAbstraction,
         StateServiceAbstraction,
         EncryptService,
         CipherFileUploadServiceAbstraction,
@@ -475,7 +475,7 @@ import { AbstractThemingService } from "./theming/theming.service.abstraction";
     {
       provide: SearchServiceAbstraction,
       useClass: SearchService,
-      deps: [CipherServiceAbstraction, LogService, I18nServiceAbstraction],
+      deps: [LogService, I18nServiceAbstraction],
     },
     {
       provide: NotificationsServiceAbstraction,

libs/angular/src/vault/components/vault-items.component.ts

@@ -2,6 +2,7 @@ import { Directive, EventEmitter, Input, Output } from "@angular/core";
 
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
 @Directive()
@@ -31,7 +32,7 @@ export class VaultItemsComponent {
     this._searchText = value;
   }
 
-  constructor(protected searchService: SearchService) {}
+  constructor(protected searchService: SearchService, protected cipherService: CipherService) {}
 
   async load(filter: (cipher: CipherView) => boolean = null, deleted = false) {
     this.deleted = deleted ?? false;
@@ -92,6 +93,7 @@ export class VaultItemsComponent {
   protected deletedFilter: (cipher: CipherView) => boolean = (c) => c.isDeleted === this.deleted;
 
   protected async doSearch(indexedCiphers?: CipherView[]) {
+    indexedCiphers = indexedCiphers ?? (await this.cipherService.getAllDecrypted());
     this.ciphers = await this.searchService.searchCiphers(
       this.searchText,
       [this.filter, this.deletedFilter],

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

@@ -5,7 +5,7 @@ export abstract class SearchService {
   indexedEntityId?: string = null;
   clearIndex: () => void;
   isSearchable: (query: string) => boolean;
-  indexCiphers: (indexedEntityGuid?: string, ciphersToIndex?: CipherView[]) => Promise<void>;
+  indexCiphers: (ciphersToIndex: CipherView[], indexedEntityGuid?: string) => void;
   searchCiphers: (
     query: string,
     filter?: ((cipher: CipherView) => boolean) | ((cipher: CipherView) => boolean)[],

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

@@ -5,7 +5,6 @@ import { LogService } from "../abstractions/log.service";
 import { SearchService as SearchServiceAbstraction } from "../abstractions/search.service";
 import { FieldType, UriMatchType } from "../enums";
 import { SendView } from "../tools/send/models/view/send.view";
-import { CipherService } from "../vault/abstractions/cipher.service";
 import { CipherType } from "../vault/enums/cipher-type";
 import { CipherView } from "../vault/models/view/cipher.view";
 
@@ -19,11 +18,7 @@ export class SearchService implements SearchServiceAbstraction {
   private readonly defaultSearchableMinLength: number = 2;
   private searchableMinLength: number = this.defaultSearchableMinLength;
 
-  constructor(
+  constructor(private logService: LogService, private i18nService: I18nService) {
-    private cipherService: CipherService,
-    private logService: LogService,
-    private i18nService: I18nService
-  ) {
     this.i18nService.locale$.subscribe((locale) => {
       if (this.immediateSearchLocales.indexOf(locale) !== -1) {
         this.searchableMinLength = 1;
@@ -55,7 +50,7 @@ export class SearchService implements SearchServiceAbstraction {
     return !notSearchable;
   }
 
-  async indexCiphers(indexedEntityId?: string, ciphers?: CipherView[]): Promise<void> {
+  indexCiphers(ciphers: CipherView[], indexedEntityId?: string): void {
     if (this.indexing) {
       return;
     }
@@ -94,7 +89,7 @@ export class SearchService implements SearchServiceAbstraction {
       extractor: (c: CipherView) => this.attachmentExtractor(c, true),
     });
     builder.field("organizationid", { extractor: (c: CipherView) => c.organizationId });
-    ciphers = ciphers || (await this.cipherService.getAllDecrypted());
+    ciphers = ciphers || [];
     ciphers.forEach((c) => builder.add(c));
     this.index = builder.build();
 
@@ -106,7 +101,7 @@ export class SearchService implements SearchServiceAbstraction {
   async searchCiphers(
     query: string,
     filter: ((cipher: CipherView) => boolean) | ((cipher: CipherView) => boolean)[] = null,
-    ciphers: CipherView[] = null
+    ciphers: CipherView[]
   ): Promise<CipherView[]> {
     const results: CipherView[] = [];
     if (query != null) {
@@ -117,7 +112,7 @@ export class SearchService implements SearchServiceAbstraction {
     }
 
     if (ciphers == null) {
-      ciphers = await this.cipherService.getAllDecrypted();
+      ciphers = [];
     }
 
     if (filter != null && Array.isArray(filter) && filter.length > 0) {

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

@@ -49,7 +49,7 @@ describe("Cipher Service", () => {
       settingsService,
       apiService,
       i18nService,
-      () => searchService,
+      searchService,
       stateService,
       encryptService,
       cipherFileUploadService

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

@@ -53,7 +53,7 @@ export class CipherService implements CipherServiceAbstraction {
     private settingsService: SettingsService,
     private apiService: ApiService,
     private i18nService: I18nService,
-    private searchService: () => SearchService,
+    private searchService: SearchService,
     private stateService: StateService,
     private encryptService: EncryptService,
     private cipherFileUploadService: CipherFileUploadService
@@ -68,9 +68,9 @@ export class CipherService implements CipherServiceAbstraction {
     await this.stateService.setDecryptedCiphers(value);
     if (this.searchService != null) {
       if (value == null) {
-        this.searchService().clearIndex();
+        this.searchService.clearIndex();
       } else {
-        this.searchService().indexCiphers();
+        this.searchService.indexCiphers(value);
       }
     }
   }
@@ -358,9 +358,9 @@ export class CipherService implements CipherServiceAbstraction {
   private async reindexCiphers() {
     const userId = await this.stateService.getUserId();
     const reindexRequired =
-      this.searchService != null && (this.searchService().indexedEntityId ?? userId) !== userId;
+      this.searchService != null && (this.searchService.indexedEntityId ?? userId) !== userId;
     if (reindexRequired) {
-      await this.searchService().indexCiphers(userId, await this.getDecryptedCipherCache());
+      this.searchService.indexCiphers(await this.getDecryptedCipherCache(), userId);
     }
   }