[PM-19032] Live Sync on Desktop (#13851)

* migrate the vault-items to an observables rather than async/promises

- this helps keep data in sync with the service state and avoids race conditions

* migrate the view component to an observables rather than async/promises

- this helps keep data in sync with the service state and avoids race conditions

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

@@ -28,9 +28,6 @@ export class VaultItemsComponent extends BaseVaultItemsComponent {
     // eslint-disable-next-line rxjs-angular/prefer-takeuntil
     searchBarService.searchText$.pipe(distinctUntilChanged()).subscribe((searchText) => {
       this.searchText = searchText;
-      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      this.search(200);
     });
   }
 

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

@@ -497,7 +497,6 @@ export class VaultComponent implements OnInit, OnDestroy {
     this.action = "view";
     await this.vaultItemsComponent.refresh();
     await this.cipherService.clearCache(this.activeUserId);
-    await this.viewComponent.load();
     this.go();
   }
 

apps/desktop/src/vault/app/vault/view.component.ts

@@ -120,9 +120,7 @@ export class ViewComponent extends BaseViewComponent implements OnInit, OnDestro
   }
 
   async ngOnChanges() {
-    await super.load();
+    if (this.cipher?.decryptionFailure) {
-
-    if (this.cipher.decryptionFailure) {
       DecryptionFailureDialogComponent.open(this.dialogService, {
         cipherIds: [this.cipherId as CipherId],
       });

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

@@ -1,13 +1,13 @@
 // FIXME: Update this file to be type safe and remove this and next line
 // @ts-strict-ignore
 import { Directive, EventEmitter, Input, OnDestroy, OnInit, Output } from "@angular/core";
-import { BehaviorSubject, Subject, firstValueFrom, from, switchMap, takeUntil } from "rxjs";
+import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
+import { BehaviorSubject, Subject, combineLatest, filter, from, switchMap, takeUntil } from "rxjs";
 
 import { SearchService } from "@bitwarden/common/abstractions/search.service";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
-import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 
@@ -21,16 +21,17 @@ export class VaultItemsComponent implements OnInit, OnDestroy {
 
   loaded = false;
   ciphers: CipherView[] = [];
-  filter: (cipher: CipherView) => boolean = null;
   deleted = false;
   organization: Organization;
 
   protected searchPending = false;
 
+  /** Construct filters as an observable so it can be appended to the cipher stream. */
+  private _filter$ = new BehaviorSubject<(cipher: CipherView) => boolean | null>(null);
   private destroy$ = new Subject<void>();
-  private searchTimeout: any = null;
   private isSearchable: boolean = false;
   private _searchText$ = new BehaviorSubject<string>("");
+
   get searchText() {
     return this._searchText$.value;
   }
@@ -38,11 +39,21 @@ export class VaultItemsComponent implements OnInit, OnDestroy {
     this._searchText$.next(value);
   }
 
+  get filter() {
+    return this._filter$.value;
+  }
+
+  set filter(value: (cipher: CipherView) => boolean | null) {
+    this._filter$.next(value);
+  }
+
   constructor(
     protected searchService: SearchService,
     protected cipherService: CipherService,
     protected accountService: AccountService,
-  ) {}
+  ) {
+    this.subscribeToCiphers();
+  }
 
   ngOnInit(): void {
     this._searchText$
@@ -77,23 +88,6 @@ export class VaultItemsComponent implements OnInit, OnDestroy {
 
   async applyFilter(filter: (cipher: CipherView) => boolean = null) {
     this.filter = filter;
-    await this.search(null);
-  }
-
-  async search(timeout: number = null, indexedCiphers?: CipherView[]) {
-    this.searchPending = false;
-    if (this.searchTimeout != null) {
-      clearTimeout(this.searchTimeout);
-    }
-    if (timeout == null) {
-      await this.doSearch(indexedCiphers);
-      return;
-    }
-    this.searchPending = true;
-    this.searchTimeout = setTimeout(async () => {
-      await this.doSearch(indexedCiphers);
-      this.searchPending = false;
-    }, timeout);
   }
 
   selectCipher(cipher: CipherView) {
@@ -118,24 +112,42 @@ export class VaultItemsComponent implements OnInit, OnDestroy {
 
   protected deletedFilter: (cipher: CipherView) => boolean = (c) => c.isDeleted === this.deleted;
 
-  protected async doSearch(indexedCiphers?: CipherView[], userId?: UserId) {
+  /**
-    // Get userId from activeAccount if not provided from parent stream
+   * Creates stream of dependencies that results in the list of ciphers to display
-    if (!userId) {
+   * within the vault list.
-      userId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
+   *
-    }
+   * Note: This previously used promises but race conditions with how the ciphers were
+   * stored in electron. Using observables is more reliable as fresh values will always
+   * cascade through the components.
+   */
+  private subscribeToCiphers() {
+    getUserId(this.accountService.activeAccount$)
+      .pipe(
+        switchMap((userId) =>
+          combineLatest([
+            this.cipherService.cipherViews$(userId).pipe(filter((ciphers) => ciphers != null)),
+            this.cipherService.failedToDecryptCiphers$(userId),
+            this._searchText$,
+            this._filter$,
+          ]),
+        ),
+        switchMap(([indexedCiphers, failedCiphers, searchText, filter]) => {
+          let allCiphers = indexedCiphers ?? [];
+          const _failedCiphers = failedCiphers ?? [];
 
-    indexedCiphers =
+          allCiphers = [..._failedCiphers, ...allCiphers];
-      indexedCiphers ?? (await firstValueFrom(this.cipherService.cipherViews$(userId)));
 
-    const failedCiphers = await firstValueFrom(this.cipherService.failedToDecryptCiphers$(userId));
+          return this.searchService.searchCiphers(
-    if (failedCiphers != null && failedCiphers.length > 0) {
+            searchText,
-      indexedCiphers = [...failedCiphers, ...indexedCiphers];
+            [filter, this.deletedFilter],
-    }
+            allCiphers,
-
-    this.ciphers = await this.searchService.searchCiphers(
-      this.searchText,
-      [this.filter, this.deletedFilter],
-      indexedCiphers,
           );
+        }),
+        takeUntilDestroyed(),
+      )
+      .subscribe((ciphers) => {
+        this.ciphers = ciphers;
+        this.loaded = true;
+      });
   }
 }

libs/angular/src/vault/components/view.component.ts

@@ -11,7 +11,17 @@ import {
   OnInit,
   Output,
 } from "@angular/core";
-import { filter, firstValueFrom, map, Observable } from "rxjs";
+import {
+  BehaviorSubject,
+  combineLatest,
+  filter,
+  firstValueFrom,
+  map,
+  Observable,
+  of,
+  switchMap,
+  tap,
+} from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
@@ -49,7 +59,18 @@ const BroadcasterSubscriptionId = "BaseViewComponent";
 
 @Directive()
 export class ViewComponent implements OnDestroy, OnInit {
-  @Input() cipherId: string;
+  /** Observable of cipherId$ that will update each time the `Input` updates */
+  private _cipherId$ = new BehaviorSubject<string>(null);
+
+  @Input()
+  set cipherId(value: string) {
+    this._cipherId$.next(value);
+  }
+
+  get cipherId(): string {
+    return this._cipherId$.getValue();
+  }
+
   @Input() collectionId: string;
   @Output() onEditCipher = new EventEmitter<CipherView>();
   @Output() onCloneCipher = new EventEmitter<CipherView>();
@@ -125,13 +146,30 @@ export class ViewComponent implements OnDestroy, OnInit {
         switch (message.command) {
           case "syncCompleted":
             if (message.successfully) {
-              await this.load();
               this.changeDetectorRef.detectChanges();
             }
             break;
         }
       });
     });
+
+    // Set up the subscription to the activeAccount$ and cipherId$ observables
+    combineLatest([this.accountService.activeAccount$.pipe(getUserId), this._cipherId$])
+      .pipe(
+        tap(() => this.cleanUp()),
+        switchMap(([userId, cipherId]) => {
+          const cipher$ = this.cipherService.cipherViews$(userId).pipe(
+            map((ciphers) => ciphers?.find((c) => c.id === cipherId)),
+            filter((cipher) => !!cipher),
+          );
+          return combineLatest([of(userId), cipher$]);
+        }),
+      )
+      .subscribe(([userId, cipher]) => {
+        this.cipher = cipher;
+
+        void this.constructCipherDetails(userId);
+      });
   }
 
   ngOnDestroy() {
@@ -139,55 +177,6 @@ export class ViewComponent implements OnDestroy, OnInit {
     this.cleanUp();
   }
 
-  async load() {
-    this.cleanUp();
-
-    // Grab individual cipher from `cipherViews$` for the most up-to-date information
-    const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
-    this.cipher = await firstValueFrom(
-      this.cipherService.cipherViews$(activeUserId).pipe(
-        map((ciphers) => ciphers?.find((c) => c.id === this.cipherId)),
-        filter((cipher) => !!cipher),
-      ),
-    );
-
-    this.canAccessPremium = await firstValueFrom(
-      this.billingAccountProfileStateService.hasPremiumFromAnySource$(activeUserId),
-    );
-    this.showPremiumRequiredTotp =
-      this.cipher.login.totp && !this.canAccessPremium && !this.cipher.organizationUseTotp;
-    this.canDeleteCipher$ = this.cipherAuthorizationService.canDeleteCipher$(this.cipher, [
-      this.collectionId as CollectionId,
-    ]);
-
-    if (this.cipher.folderId) {
-      this.folder = await (
-        await firstValueFrom(this.folderService.folderViews$(activeUserId))
-      ).find((f) => f.id == this.cipher.folderId);
-    }
-
-    if (
-      this.cipher.type === CipherType.Login &&
-      this.cipher.login.totp &&
-      (this.cipher.organizationUseTotp || this.canAccessPremium)
-    ) {
-      await this.totpUpdateCode();
-      const interval = this.totpService.getTimeInterval(this.cipher.login.totp);
-      await this.totpTick(interval);
-
-      this.totpInterval = setInterval(async () => {
-        await this.totpTick(interval);
-      }, 1000);
-    }
-
-    if (this.previousCipherId !== this.cipherId) {
-      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
-      // eslint-disable-next-line @typescript-eslint/no-floating-promises
-      this.eventCollectionService.collect(EventType.Cipher_ClientViewed, this.cipherId);
-    }
-    this.previousCipherId = this.cipherId;
-  }
-
   async edit() {
     if (await this.promptPassword()) {
       this.onEditCipher.emit(this.cipher);
@@ -567,4 +556,46 @@ export class ViewComponent implements OnDestroy, OnInit {
       await this.totpUpdateCode();
     }
   }
+
+  /**
+   * When a cipher is viewed, construct all details for the view that are not directly
+   * available from the cipher object itself.
+   */
+  private async constructCipherDetails(userId: UserId) {
+    this.canAccessPremium = await firstValueFrom(
+      this.billingAccountProfileStateService.hasPremiumFromAnySource$(userId),
+    );
+    this.showPremiumRequiredTotp =
+      this.cipher.login.totp && !this.canAccessPremium && !this.cipher.organizationUseTotp;
+    this.canDeleteCipher$ = this.cipherAuthorizationService.canDeleteCipher$(this.cipher, [
+      this.collectionId as CollectionId,
+    ]);
+
+    if (this.cipher.folderId) {
+      this.folder = await (
+        await firstValueFrom(this.folderService.folderViews$(userId))
+      ).find((f) => f.id == this.cipher.folderId);
+    }
+
+    if (
+      this.cipher.type === CipherType.Login &&
+      this.cipher.login.totp &&
+      (this.cipher.organizationUseTotp || this.canAccessPremium)
+    ) {
+      await this.totpUpdateCode();
+      const interval = this.totpService.getTimeInterval(this.cipher.login.totp);
+      await this.totpTick(interval);
+
+      this.totpInterval = setInterval(async () => {
+        await this.totpTick(interval);
+      }, 1000);
+    }
+
+    if (this.previousCipherId !== this.cipherId) {
+      // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
+      // eslint-disable-next-line @typescript-eslint/no-floating-promises
+      this.eventCollectionService.collect(EventType.Cipher_ClientViewed, this.cipherId);
+    }
+    this.previousCipherId = this.cipherId;
+  }
 }