From e000fd77d405652a6a2c8954ea3de34a7e0108fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Wed, 23 Apr 2025 15:02:32 -0400 Subject: [PATCH] fix engine-settings desync error --- libs/common/src/tools/rx.rxjs.ts | 13 +++++++++ libs/common/src/tools/rx.ts | 29 ++++++++++++------- .../src/credential-generator.component.html | 10 +++---- .../src/credential-generator.component.ts | 14 ++++++--- ...fault-credential-generator.service.spec.ts | 2 +- .../default-credential-generator.service.ts | 12 ++++---- 6 files changed, 54 insertions(+), 26 deletions(-) create mode 100644 libs/common/src/tools/rx.rxjs.ts diff --git a/libs/common/src/tools/rx.rxjs.ts b/libs/common/src/tools/rx.rxjs.ts new file mode 100644 index 00000000000..7c11d658f19 --- /dev/null +++ b/libs/common/src/tools/rx.rxjs.ts @@ -0,0 +1,13 @@ +import { Observable } from "rxjs"; + +/** + * Used to infer types from arguments to functions like {@link withLatestReady}. + * So that you can have `forkJoin([Observable, PromiseLike]): Observable<[A, B]>` + * et al. + * @remarks this type definition is derived from rxjs' {@link ObservableInputTuple}. + * The difference is it *only* works with observables, while the rx version works + * with any thing that can become an observable. + */ +export type ObservableTuple = { + [K in keyof T]: Observable; +}; diff --git a/libs/common/src/tools/rx.ts b/libs/common/src/tools/rx.ts index ea397135581..8a801cb9798 100644 --- a/libs/common/src/tools/rx.ts +++ b/libs/common/src/tools/rx.ts @@ -20,8 +20,11 @@ import { startWith, pairwise, MonoTypeOperatorFunction, + Cons, } from "rxjs"; +import { ObservableTuple } from "./rx.rxjs"; + /** Returns its input. */ function identity(value: any): any { return value; @@ -164,26 +167,30 @@ export function ready(watch$: Observable | Observable[]) { ); } -export function withLatestReady( - watch$: Observable, -): OperatorFunction { +export function withLatestReady( + ...watches$: [...ObservableTuple] +): OperatorFunction> { return connect((source$) => { // these subscriptions are safe because `source$` connects only after there // is an external subscriber. const source = new ReplaySubject(1); source$.subscribe(source); - const watch = new ReplaySubject(1); - watch$.subscribe(watch); + + const watches = watches$.map((w) => { + const watch$ = new ReplaySubject(1); + w.subscribe(watch$); + return watch$; + }) as [...ObservableTuple]; // `concat` is subscribed immediately after it's returned, at which point - // `zip` blocks until all items in `watching$` are ready. If that occurs + // `zip` blocks until all items in `watches` are ready. If that occurs // after `source$` is hot, then the replay subject sends the last-captured - // emission through immediately. Otherwise, `ready` waits for the next - // emission - return concat(zip(watch).pipe(first(), ignoreElements()), source).pipe( - withLatestFrom(watch), + // emission through immediately. Otherwise, `withLatestFrom` waits for the + // next emission + return concat(zip(watches).pipe(first(), ignoreElements()), source).pipe( + withLatestFrom(...watches), takeUntil(anyComplete(source)), - ); + ) as Observable>; }); } diff --git a/libs/tools/generator/components/src/credential-generator.component.html b/libs/tools/generator/components/src/credential-generator.component.html index e95df388f39..0153da2c4a4 100644 --- a/libs/tools/generator/components/src/credential-generator.component.html +++ b/libs/tools/generator/components/src/credential-generator.component.html @@ -40,13 +40,13 @@ @@ -82,7 +82,7 @@ @@ -92,12 +92,12 @@ [forwarder]="forwarderId$ | async" /> diff --git a/libs/tools/generator/components/src/credential-generator.component.ts b/libs/tools/generator/components/src/credential-generator.component.ts index 53a33a0c90e..c9bce3086ca 100644 --- a/libs/tools/generator/components/src/credential-generator.component.ts +++ b/libs/tools/generator/components/src/credential-generator.component.ts @@ -23,6 +23,7 @@ import { ReplaySubject, Subject, takeUntil, + tap, withLatestFrom, } from "rxjs"; @@ -49,6 +50,7 @@ import { isPasswordAlgorithm, CredentialAlgorithm, AlgorithmMetadata, + Algorithm, } from "@bitwarden/generator-core"; import { GeneratorHistoryService } from "@bitwarden/generator-history"; @@ -79,6 +81,8 @@ export class CredentialGeneratorComponent implements OnInit, OnChanges, OnDestro private ariaLive: LiveAnnouncer, ) {} + protected readonly Algorithm = Algorithm; + /** Binds the component to a specific user's settings. When this input is not provided, * the form binds to the active user */ @@ -91,7 +95,7 @@ export class CredentialGeneratorComponent implements OnInit, OnChanges, OnDestro * @warning this may reveal sensitive information in plaintext. */ @Input() - debug: boolean = false; + debug: boolean = true; // this `log` initializer is overridden in `ngOnInit` private log: SemanticLogger = disabledSemanticLoggerProvider({}); @@ -230,7 +234,9 @@ export class CredentialGeneratorComponent implements OnInit, OnChanges, OnDestro // wire up the generator this.generatorService .generator$({ - on$: this.generate$, + on$: this.generate$.pipe( + tap((g) => this.log.debug(g, "generate request issued by component")), + ), account$: this.account$, }) .pipe( @@ -290,7 +296,7 @@ export class CredentialGeneratorComponent implements OnInit, OnChanges, OnDestro } else if (root.nav) { return { nav: root.nav, algorithm: JSON.parse(root.nav) }; } else { - this.log.panic(root, "unknown navigation value."); + return { nav: IDENTIFIER }; } }), takeUntil(this.destroyed), @@ -305,7 +311,7 @@ export class CredentialGeneratorComponent implements OnInit, OnChanges, OnDestro } else if (username.nav) { return { nav: username.nav, algorithm: JSON.parse(username.nav) }; } else { - this.log.panic(username, "unknown navigation value."); + return { nav: FORWARDER }; } }), takeUntil(this.destroyed), diff --git a/libs/tools/generator/core/src/services/default-credential-generator.service.spec.ts b/libs/tools/generator/core/src/services/default-credential-generator.service.spec.ts index 02e72beaa47..847d4d80c45 100644 --- a/libs/tools/generator/core/src/services/default-credential-generator.service.spec.ts +++ b/libs/tools/generator/core/src/services/default-credential-generator.service.spec.ts @@ -32,7 +32,7 @@ type MockTwoLevelPartial = { : T[K]; }; -describe("CredentialGeneratorService", () => { +describe("DefaultCredentialGeneratorService", () => { let service: DefaultCredentialGeneratorService; let providers: MockTwoLevelPartial; let system: any; diff --git a/libs/tools/generator/core/src/services/default-credential-generator.service.ts b/libs/tools/generator/core/src/services/default-credential-generator.service.ts index 4afb2d2d2d9..5080a0dd4e6 100644 --- a/libs/tools/generator/core/src/services/default-credential-generator.service.ts +++ b/libs/tools/generator/core/src/services/default-credential-generator.service.ts @@ -1,4 +1,5 @@ import { + EMPTY, concatMap, distinctUntilChanged, filter, @@ -60,12 +61,14 @@ export class DefaultCredentialGeneratorService implements CredentialGeneratorSer } else if (isTypeRequest(requested)) { return this.provide.metadata.preference$(requested.type, { account$ }); } else { - this.log.panic(requested, "algorithm or category required"); + this.log.warn(requested, "algorithm or category required"); + return EMPTY; } }), filter((algorithm): algorithm is CredentialAlgorithm => !!algorithm), + distinctUntilChanged(), map((algorithm) => this.provide.metadata.metadata(algorithm)), - distinctUntilChanged((previous, current) => previous.id === current.id), + shareReplay({ refCount: true, bufferSize: 1 }), ); // load the active profile's algorithm settings @@ -84,9 +87,8 @@ export class DefaultCredentialGeneratorService implements CredentialGeneratorSer // generation proper const generate$ = on$.pipe( - withLatestReady(engine$), - withLatestReady(settings$), - concatMap(([[request, engine], settings]) => engine.generate(request, settings)), + withLatestReady(settings$, engine$), + concatMap(([request, settings, engine]) => engine.generate(request, settings)), takeUntil(anyComplete([settings$])), );