From 4455278704a8ca7f3d9c5070b0e2bf30f8b6bca8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= <audrey@audreyality.com>
Date: Fri, 7 Mar 2025 17:36:26 -0500
Subject: [PATCH] additional tests; fix disabled semantic logger panic behavior

---
 .../src/tools/log/disabled-semantic-logger.ts |  8 +++--
 .../generator-metadata-provider.spec.ts       | 24 ++++++++++++--
 .../services/generator-metadata-provider.ts   | 32 +++++++++++--------
 3 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/libs/common/src/tools/log/disabled-semantic-logger.ts b/libs/common/src/tools/log/disabled-semantic-logger.ts
index 054c3ed390b..21ea48bbe51 100644
--- a/libs/common/src/tools/log/disabled-semantic-logger.ts
+++ b/libs/common/src/tools/log/disabled-semantic-logger.ts
@@ -12,7 +12,11 @@ export class DisabledSemanticLogger implements SemanticLogger {
 
   error<T>(_content: Jsonify<T>, _message?: string): void {}
 
-  panic<T>(_content: Jsonify<T>, message?: string): never {
-    throw new Error(message);
+  panic<T>(content: Jsonify<T>, message?: string): never {
+    if (typeof content === "string" && !message) {
+      throw new Error(content);
+    } else {
+      throw new Error(message);
+    }
   }
 }
diff --git a/libs/tools/generator/core/src/services/generator-metadata-provider.spec.ts b/libs/tools/generator/core/src/services/generator-metadata-provider.spec.ts
index 0647eea14d7..8e765adf005 100644
--- a/libs/tools/generator/core/src/services/generator-metadata-provider.spec.ts
+++ b/libs/tools/generator/core/src/services/generator-metadata-provider.spec.ts
@@ -86,14 +86,14 @@ const SomeSite: SiteMetadata = Object.freeze({
 
 const SomePolicyService = mock<PolicyService>();
 
+const SomeExtensionService = mock<ExtensionService>();
+
 const ApplicationProvider = {
   /** Policy configured by the administrative console */
   policy: SomePolicyService,
 
   /** Client extension metadata and profile access */
-  extension: mock<ExtensionService>({
-    site: () => new ExtensionSite(SomeSite, new Map()),
-  }),
+  extension: SomeExtensionService,
 
   /** Event monitoring and diagnostic interfaces */
   log: disabledSemanticLoggerProvider,
@@ -102,6 +102,16 @@ const ApplicationProvider = {
 describe("GeneratorMetadataProvider", () => {
   beforeEach(() => {
     jest.resetAllMocks();
+    SomeExtensionService.site.mockImplementation(() => new ExtensionSite(SomeSite, new Map()));
+  });
+
+  describe("constructor", () => {
+    it("throws when the forwarder site isn't defined by the extension service", () => {
+      SomeExtensionService.site.mockReturnValue(undefined);
+      expect(() => new GeneratorMetadataProvider(SystemProvider, ApplicationProvider, [])).toThrow(
+        "forwarder extension site not found",
+      );
+    });
   });
 
   describe("metadata", () => {
@@ -354,6 +364,14 @@ describe("GeneratorMetadataProvider", () => {
       await expect(firstValueFrom(algorithmResult)).resolves.toEqual([]);
       await expect(firstValueFrom(categoryResult)).resolves.toEqual([password.id]);
     });
+
+    it("panics when neither algorithm nor category are specified", () => {
+      const provider = new GeneratorMetadataProvider(SystemProvider, ApplicationProvider, []);
+
+      expect(() => provider.algorithms$({} as any, { account$: SomeAccount$ })).toThrow(
+        "algorithm or category required",
+      );
+    });
   });
 
   describe("preference$", () => {
diff --git a/libs/tools/generator/core/src/services/generator-metadata-provider.ts b/libs/tools/generator/core/src/services/generator-metadata-provider.ts
index c86f76f38e9..4547448b34a 100644
--- a/libs/tools/generator/core/src/services/generator-metadata-provider.ts
+++ b/libs/tools/generator/core/src/services/generator-metadata-provider.ts
@@ -14,7 +14,7 @@ import { BoundDependency } from "@bitwarden/common/tools/dependencies";
 import { ExtensionSite } from "@bitwarden/common/tools/extension";
 import { SemanticLogger } from "@bitwarden/common/tools/log";
 import { SystemServiceProvider } from "@bitwarden/common/tools/providers";
-import { anyComplete } from "@bitwarden/common/tools/rx";
+import { anyComplete, pin } from "@bitwarden/common/tools/rx";
 import { UserStateSubject } from "@bitwarden/common/tools/state/user-state-subject";
 import { UserStateSubjectDependencyProvider } from "@bitwarden/common/tools/state/user-state-subject-dependency-provider";
 
@@ -134,24 +134,28 @@ export class GeneratorMetadataProvider {
   private isAvailable$(
     dependencies: BoundDependency<"account", Account>,
   ): Observable<(a: CredentialAlgorithm) => boolean> {
-    const account$ = dependencies.account$.pipe(
-      distinctUntilChanged((previous, current) => previous.id === current.id),
+    const id$ = dependencies.account$.pipe(
+      map((account) => account.id),
+      pin(),
       shareReplay({ bufferSize: 1, refCount: true }),
     );
 
-    const available$ = account$.pipe(
-      switchMap((account) => {
-        const policies$ = this.application.policy
-          .getAll$(PolicyType.PasswordGenerator, account.id)
-          .pipe(
-            map((p) => availableAlgorithms_vNext(p).filter((a) => this._metadata.has(a))),
-            map((p) => new Set(p)),
-            // complete policy emissions otherwise `switchMap` holds `available$` open indefinitely
-            takeUntil(anyComplete(account$)),
-          );
+    const available$ = id$.pipe(
+      switchMap((id) => {
+        const policies$ = this.application.policy.getAll$(PolicyType.PasswordGenerator, id).pipe(
+          map((p) => availableAlgorithms_vNext(p).filter((a) => this._metadata.has(a))),
+          map((p) => new Set(p)),
+          // complete policy emissions otherwise `switchMap` holds `available$` open indefinitely
+          takeUntil(anyComplete(id$)),
+        );
         return policies$;
       }),
-      map((available) => (a: CredentialAlgorithm) => isForwarderExtensionId(a) || available.has(a)),
+      map(
+        (available) =>
+          function (a: CredentialAlgorithm) {
+            return isForwarderExtensionId(a) || available.has(a);
+          },
+      ),
     );
 
     return available$;