From 1e921eb4f6dea93753a1e928f95f643e6c464743 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 15 Mar 2024 08:05:04 -0500 Subject: [PATCH] `getUserState$` Helper Improvements (#8267) * Block Sending Null to `getUser` * Update Comments & Tests Co-authored-by: Matt Gibson * Update Comment * Update Fake --------- Co-authored-by: Matt Gibson --- libs/common/spec/fake-state-provider.ts | 27 +++- .../default-state.provider.spec.ts | 116 ++++++++++++++---- .../implementations/default-state.provider.ts | 20 ++- .../src/platform/state/state.provider.ts | 42 ++++++- 4 files changed, 174 insertions(+), 31 deletions(-) diff --git a/libs/common/spec/fake-state-provider.ts b/libs/common/spec/fake-state-provider.ts index 78140ba4af..2078fe3abd 100644 --- a/libs/common/spec/fake-state-provider.ts +++ b/libs/common/spec/fake-state-provider.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { Observable, map } from "rxjs"; +import { Observable, map, of, switchMap, take } from "rxjs"; import { GlobalState, @@ -171,7 +171,30 @@ export class FakeStateProvider implements StateProvider { if (userId) { return this.getUser(userId, keyDefinition).state$; } - return this.getActive(keyDefinition).state$; + + return this.getActive(keyDefinition).state$; + } + + getUserStateOrDefault$( + keyDefinition: KeyDefinition | UserKeyDefinition, + config: { userId: UserId | undefined; defaultValue?: T }, + ): Observable { + const { userId, defaultValue = null } = config; + if (isUserKeyDefinition(keyDefinition)) { + this.mock.getUserStateOrDefault$(keyDefinition, config); + } else { + this.mock.getUserStateOrDefault$(keyDefinition, config); + } + if (userId) { + return this.getUser(userId, keyDefinition).state$; + } + + return this.activeUserId$.pipe( + take(1), + switchMap((userId) => + userId != null ? this.getUser(userId, keyDefinition).state$ : of(defaultValue), + ), + ); } async setUserState( diff --git a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts index e7228192f1..3243b53d67 100644 --- a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts @@ -2,9 +2,9 @@ * need to update test environment so structuredClone works appropriately * @jest-environment ../shared/test.environment.ts */ -import { of } from "rxjs"; +import { Observable, of } from "rxjs"; -import { trackEmissions } from "../../../../spec"; +import { awaitAsync, trackEmissions } from "../../../../spec"; import { FakeAccountService, mockAccountServiceWith } from "../../../../spec/fake-account-service"; import { FakeActiveUserStateProvider, @@ -49,47 +49,111 @@ describe("DefaultStateProvider", () => { }); }); + describe.each([ + [ + "getUserState$", + (keyDefinition: KeyDefinition, userId?: UserId) => + sut.getUserState$(keyDefinition, userId), + ], + [ + "getUserStateOrDefault$", + (keyDefinition: KeyDefinition, userId?: UserId) => + sut.getUserStateOrDefault$(keyDefinition, { userId: userId }), + ], + ])( + "Shared behavior for %s", + ( + _testName: string, + methodUnderTest: ( + keyDefinition: KeyDefinition, + userId?: UserId, + ) => Observable, + ) => { + const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; + const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { + deserializer: (s) => s, + }); + + it("should follow the specified user if userId is provided", async () => { + const state = singleUserStateProvider.getFake(userId, keyDefinition); + state.nextState("value"); + const emissions = trackEmissions(methodUnderTest(keyDefinition, userId)); + + state.nextState("value2"); + state.nextState("value3"); + + expect(emissions).toEqual(["value", "value2", "value3"]); + }); + + it("should follow the current active user if no userId is provided", async () => { + accountService.activeAccountSubject.next({ id: userId, ...accountInfo }); + const state = singleUserStateProvider.getFake(userId, keyDefinition); + state.nextState("value"); + const emissions = trackEmissions(methodUnderTest(keyDefinition)); + + state.nextState("value2"); + state.nextState("value3"); + + expect(emissions).toEqual(["value", "value2", "value3"]); + }); + + it("should continue to follow the state of the user that was active when called, even if active user changes", async () => { + const state = singleUserStateProvider.getFake(userId, keyDefinition); + state.nextState("value"); + const emissions = trackEmissions(methodUnderTest(keyDefinition)); + + accountService.activeAccountSubject.next({ id: "newUserId" as UserId, ...accountInfo }); + const newUserEmissions = trackEmissions(sut.getUserState$(keyDefinition)); + state.nextState("value2"); + state.nextState("value3"); + + expect(emissions).toEqual(["value", "value2", "value3"]); + expect(newUserEmissions).toEqual([null]); + }); + }, + ); + describe("getUserState$", () => { const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { deserializer: (s) => s, }); - it("should follow the specified user if userId is provided", async () => { + it("should not emit any values until a truthy user id is supplied", async () => { + accountService.activeAccountSubject.next(null); const state = singleUserStateProvider.getFake(userId, keyDefinition); - state.nextState("value"); - const emissions = trackEmissions(sut.getUserState$(keyDefinition, userId)); + state.stateSubject.next([userId, "value"]); - state.nextState("value2"); - state.nextState("value3"); + const emissions = trackEmissions(sut.getUserState$(keyDefinition)); - expect(emissions).toEqual(["value", "value2", "value3"]); - }); + await awaitAsync(); + + expect(emissions).toHaveLength(0); - it("should follow the current active user if no userId is provided", async () => { accountService.activeAccountSubject.next({ id: userId, ...accountInfo }); - const state = singleUserStateProvider.getFake(userId, keyDefinition); - state.nextState("value"); - const emissions = trackEmissions(sut.getUserState$(keyDefinition)); - state.nextState("value2"); - state.nextState("value3"); + await awaitAsync(); - expect(emissions).toEqual(["value", "value2", "value3"]); + expect(emissions).toEqual(["value"]); + }); + }); + + describe("getUserStateOrDefault$", () => { + const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { + deserializer: (s) => s, }); - it("should continue to follow the state of the user that was active when called, even if active user changes", async () => { - const state = singleUserStateProvider.getFake(userId, keyDefinition); - state.nextState("value"); - const emissions = trackEmissions(sut.getUserState$(keyDefinition)); + it("should emit default value if no userId supplied and first active user id emission in falsy", async () => { + accountService.activeAccountSubject.next(null); - accountService.activeAccountSubject.next({ id: "newUserId" as UserId, ...accountInfo }); - const newUserEmissions = trackEmissions(sut.getUserState$(keyDefinition)); - state.nextState("value2"); - state.nextState("value3"); + const emissions = trackEmissions( + sut.getUserStateOrDefault$(keyDefinition, { + userId: undefined, + defaultValue: "I'm default!", + }), + ); - expect(emissions).toEqual(["value", "value2", "value3"]); - expect(newUserEmissions).toEqual([null]); + expect(emissions).toEqual(["I'm default!"]); }); }); diff --git a/libs/common/src/platform/state/implementations/default-state.provider.ts b/libs/common/src/platform/state/implementations/default-state.provider.ts index 6a35e0fabb..1413d26ce5 100644 --- a/libs/common/src/platform/state/implementations/default-state.provider.ts +++ b/libs/common/src/platform/state/implementations/default-state.provider.ts @@ -1,4 +1,4 @@ -import { Observable, switchMap, take } from "rxjs"; +import { Observable, filter, of, switchMap, take } from "rxjs"; import { UserId } from "../../../types/guid"; import { DerivedStateDependencies } from "../../../types/state"; @@ -30,12 +30,30 @@ export class DefaultStateProvider implements StateProvider { return this.getUser(userId, keyDefinition).state$; } else { return this.activeUserId$.pipe( + filter((userId) => userId != null), // Filter out null-ish user ids since we can't get state for a null user id take(1), switchMap((userId) => this.getUser(userId, keyDefinition).state$), ); } } + getUserStateOrDefault$( + keyDefinition: KeyDefinition | UserKeyDefinition, + config: { userId: UserId | undefined; defaultValue?: T }, + ): Observable { + const { userId, defaultValue = null } = config; + if (userId) { + return this.getUser(userId, keyDefinition).state$; + } else { + return this.activeUserId$.pipe( + take(1), + switchMap((userId) => + userId != null ? this.getUser(userId, keyDefinition).state$ : of(defaultValue), + ), + ); + } + } + async setUserState( keyDefinition: KeyDefinition | UserKeyDefinition, value: T, diff --git a/libs/common/src/platform/state/state.provider.ts b/libs/common/src/platform/state/state.provider.ts index f2b008eb99..ddbb6a7c87 100644 --- a/libs/common/src/platform/state/state.provider.ts +++ b/libs/common/src/platform/state/state.provider.ts @@ -24,8 +24,11 @@ export abstract class StateProvider { /** * Gets a state observable for a given key and userId. * - * @remarks If userId is falsy the observable returned will point to the currently active user _and not update if the active user changes_. + * @remarks If userId is falsy the observable returned will attempt to point to the currently active user _and not update if the active user changes_. * This is different to how `getActive` works and more similar to `getUser` for whatever user happens to be active at the time of the call. + * If no user happens to be active at the time this method is called with a falsy userId then this observable will not emit a value until + * a user becomes active. If you are not confident a user is active at the time this method is called, you may want to pipe a call to `timeout` + * or instead call {@link getUserStateOrDefault$} and supply a value you would rather have given in the case of no passed in userId and no active user. * * @note consider converting your {@link KeyDefinition} to a {@link UserKeyDefinition} for additional features. * @@ -37,14 +40,49 @@ export abstract class StateProvider { /** * Gets a state observable for a given key and userId. * - * @remarks If userId is falsy the observable returned will point to the currently active user _and not update if the active user changes_. + * @remarks If userId is falsy the observable returned will attempt to point to the currently active user _and not update if the active user changes_. * This is different to how `getActive` works and more similar to `getUser` for whatever user happens to be active at the time of the call. + * If no user happens to be active at the time this method is called with a falsy userId then this observable will not emit a value until + * a user becomes active. If you are not confident a user is active at the time this method is called, you may want to pipe a call to `timeout` + * or instead call {@link getUserStateOrDefault$} and supply a value you would rather have given in the case of no passed in userId and no active user. * * @param keyDefinition - The key definition for the state you want to get. * @param userId - The userId for which you want the state for. If not provided, the state for the currently active user will be returned. */ abstract getUserState$(keyDefinition: UserKeyDefinition, userId?: UserId): Observable; + /** + * Gets a state observable for a given key and userId + * + * @remarks If userId is falsy the observable return will first attempt to point to the currently active user but will not follow subsequent active user changes, + * if there is no immediately available active user, then it will fallback to returning a default value in an observable that immediately completes. + * + * @note consider converting your {@link KeyDefinition} to a {@link UserKeyDefinition} for additional features. + * + * @param keyDefinition - The key definition for the state you want to get. + * @param config.userId - The userId for which you want the state for. If not provided, the state for the currently active user will be returned. + * @param config.defaultValue - The default value that should be wrapped in an observable if no active user is immediately available and no truthy userId is passed in. + */ + abstract getUserStateOrDefault$( + keyDefinition: KeyDefinition, + config: { userId: UserId | undefined; defaultValue?: T }, + ): Observable; + + /** + * Gets a state observable for a given key and userId + * + * @remarks If userId is falsy the observable return will first attempt to point to the currently active user but will not follow subsequent active user changes, + * if there is no immediately available active user, then it will fallback to returning a default value in an observable that immediately completes. + * + * @param keyDefinition - The key definition for the state you want to get. + * @param config.userId - The userId for which you want the state for. If not provided, the state for the currently active user will be returned. + * @param config.defaultValue - The default value that should be wrapped in an observable if no active user is immediately available and no truthy userId is passed in. + */ + abstract getUserStateOrDefault$( + keyDefinition: UserKeyDefinition, + config: { userId: UserId | undefined; defaultValue?: T }, + ): Observable; + /** * Sets the state for a given key and userId. *