From 55e4b5ee09a8bfce868f614eca27ddc3f32ef74d Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Wed, 15 Jan 2025 05:41:44 -0500 Subject: [PATCH 1/2] fix: Use `WeakMap` in `DerivedStateProvider` to separate user state caches (#12866) Bug fix for PM-15914 where switching users would incorrectly share cached derived states. The `DerivedStateProvider` now uses a `WeakMap` to maintain separate caches for each user's state `Observable`. - Modifies `DefaultDerivedStateProvider` to use `WeakMap` for caching - Each user's state `Observable` gets its own definition cache - Added test to verify correct behavior during user switching - Allows proper garbage collection of unused state caches This fixes issues where: - Users would see other users' cached states after switching accounts - Derived states weren't properly isolated between users - Cache keys didn't distinguish between different user states --- .../default-derived-state.provider.ts | 19 +++++++++++--- .../default-derived-state.spec.ts | 26 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/libs/common/src/platform/state/implementations/default-derived-state.provider.ts b/libs/common/src/platform/state/implementations/default-derived-state.provider.ts index 3c8c39e21e8..61f36fa0b75 100644 --- a/libs/common/src/platform/state/implementations/default-derived-state.provider.ts +++ b/libs/common/src/platform/state/implementations/default-derived-state.provider.ts @@ -8,7 +8,14 @@ import { DerivedStateProvider } from "../derived-state.provider"; import { DefaultDerivedState } from "./default-derived-state"; export class DefaultDerivedStateProvider implements DerivedStateProvider { - private cache: Record> = {}; + /** + * The cache uses a WeakMap to maintain separate derived states per user. + * Each user's state Observable acts as a unique key, without needing to + * pass around `userId`. Also, when a user's state Observable is cleaned up + * (like during an account swap) their cache is automatically garbage + * collected. + */ + private cache = new WeakMap, Record>>(); constructor() {} @@ -17,8 +24,14 @@ export class DefaultDerivedStateProvider implements DerivedStateProvider { deriveDefinition: DeriveDefinition, dependencies: TDeps, ): DerivedState { + let stateCache = this.cache.get(parentState$); + if (!stateCache) { + stateCache = {}; + this.cache.set(parentState$, stateCache); + } + const cacheKey = deriveDefinition.buildCacheKey(); - const existingDerivedState = this.cache[cacheKey]; + const existingDerivedState = stateCache[cacheKey]; if (existingDerivedState != null) { // I have to cast out of the unknown generic but this should be safe if rules // around domain token are made @@ -26,7 +39,7 @@ export class DefaultDerivedStateProvider implements DerivedStateProvider { } const newDerivedState = this.buildDerivedState(parentState$, deriveDefinition, dependencies); - this.cache[cacheKey] = newDerivedState; + stateCache[cacheKey] = newDerivedState; return newDerivedState; } diff --git a/libs/common/src/platform/state/implementations/default-derived-state.spec.ts b/libs/common/src/platform/state/implementations/default-derived-state.spec.ts index 7e8d76bd203..6fcc1c408cb 100644 --- a/libs/common/src/platform/state/implementations/default-derived-state.spec.ts +++ b/libs/common/src/platform/state/implementations/default-derived-state.spec.ts @@ -9,6 +9,7 @@ import { DeriveDefinition } from "../derive-definition"; import { StateDefinition } from "../state-definition"; import { DefaultDerivedState } from "./default-derived-state"; +import { DefaultDerivedStateProvider } from "./default-derived-state.provider"; let callCount = 0; const cleanupDelayMs = 10; @@ -182,4 +183,29 @@ describe("DefaultDerivedState", () => { expect(await firstValueFrom(observable)).toEqual(new Date(newDate)); }); }); + + describe("account switching", () => { + let provider: DefaultDerivedStateProvider; + + beforeEach(() => { + provider = new DefaultDerivedStateProvider(); + }); + + it("should provide a dedicated cache for each account", async () => { + const user1State$ = new Subject(); + const user1Derived = provider.get(user1State$, deriveDefinition, deps); + const user1Emissions = trackEmissions(user1Derived.state$); + + const user2State$ = new Subject(); + const user2Derived = provider.get(user2State$, deriveDefinition, deps); + const user2Emissions = trackEmissions(user2Derived.state$); + + user1State$.next("2015-12-30"); + user2State$.next("2020-12-29"); + await awaitAsync(); + + expect(user1Emissions).toEqual([new Date("2015-12-30")]); + expect(user2Emissions).toEqual([new Date("2020-12-29")]); + }); + }); }); From ee6822c00d70c4eace5109b96bb78f2a9fe661ee Mon Sep 17 00:00:00 2001 From: Jonas Hendrickx Date: Wed, 15 Jan 2025 16:05:31 +0100 Subject: [PATCH 2/2] [PM-17064] Prevent error being thrown when taxInformation is undefined. (#12884) --- .../billing/organizations/change-plan-dialog.component.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts b/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts index bc5c7da8db9..73577c7b002 100644 --- a/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts +++ b/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts @@ -1062,7 +1062,11 @@ export class ChangePlanDialogComponent implements OnInit, OnDestroy { } private refreshSalesTax(): void { - if (!this.taxInformation.country || !this.taxInformation.postalCode) { + if ( + this.taxInformation === undefined || + !this.taxInformation.country || + !this.taxInformation.postalCode + ) { return; }