fix: isolate per-account state snapshots from leakage and mutation
account-state-manager had two latent correctness issues: 1. Shared references: snapshotAccount stored the live store arrays/objects directly, so a later in-place mutation (array push/splice, or a shared email object being stamped) retroactively corrupted an earlier snapshot. Now copies the captured collections. 2. Incomplete restore: the snapshot only captures a subset of each store's fields, but restoreAccount applied it with a merge, leaving every other field (email selection, loading flags, tag counts, …) at the previously active account's values. It only worked because every caller happened to call clearAllStores() first. restoreAccount now resets the stores to baseline itself before layering the snapshot back on, so it is correct standalone and can't leak state across accounts. Adds tests pinning the isolation guarantees.
This commit is contained in:
committed by
Linus Rath
parent
bfc8ba851a
commit
ddb596affc
@@ -58,7 +58,7 @@ describe('snapshotAccount / restoreAccount', () => {
|
||||
expect(useVacationStore.getState().isEnabled).toBe(true);
|
||||
});
|
||||
|
||||
it('CHARACTERISATION: only snapshotted fields are restored; others survive', () => {
|
||||
it('resets fields outside the snapshot subset to their defaults (no cross-account leak)', () => {
|
||||
// isLoading is NOT part of the email snapshot subset.
|
||||
useEmailStore.setState({ selectedMailbox: 'a-in', isLoading: false });
|
||||
snapshotAccount('A');
|
||||
@@ -66,11 +66,11 @@ describe('snapshotAccount / restoreAccount', () => {
|
||||
|
||||
restoreAccount('A');
|
||||
|
||||
expect(useEmailStore.getState().selectedMailbox).toBe('a-in'); // restored
|
||||
expect(useEmailStore.getState().isLoading).toBe(true); // NOT restored (merge)
|
||||
expect(useEmailStore.getState().selectedMailbox).toBe('a-in'); // captured → restored
|
||||
expect(useEmailStore.getState().isLoading).toBe(false); // uncaptured → reset, not leaked
|
||||
});
|
||||
|
||||
it('CHARACTERISATION: snapshot stores array references, not deep clones', () => {
|
||||
it('decouples the snapshot from later in-place mutation of the source array', () => {
|
||||
const arr = [makeEmail({ id: '1' })];
|
||||
useEmailStore.setState({ emails: arr });
|
||||
snapshotAccount('A');
|
||||
@@ -78,8 +78,8 @@ describe('snapshotAccount / restoreAccount', () => {
|
||||
useEmailStore.setState({ emails: [] });
|
||||
|
||||
restoreAccount('A');
|
||||
// The post-snapshot mutation leaked into the snapshot.
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['1', '2']);
|
||||
// The post-snapshot mutation did NOT leak into the snapshot.
|
||||
expect(useEmailStore.getState().emails.map((e) => e.id)).toEqual(['1']);
|
||||
});
|
||||
|
||||
it('returns false and leaves stores untouched for an unknown account', () => {
|
||||
|
||||
@@ -37,33 +37,37 @@ export function snapshotAccount(accountId: string): void {
|
||||
const identityState = useIdentityStore.getState();
|
||||
const vacationState = useVacationStore.getState();
|
||||
|
||||
// Copy the captured collections so the snapshot is decoupled from the live
|
||||
// store: a later in-place mutation (e.g. an array push/splice, or stamping
|
||||
// fields onto a shared email object) must not retroactively corrupt a
|
||||
// snapshot taken earlier.
|
||||
cache.set(accountId, {
|
||||
email: {
|
||||
emails: emailState.emails,
|
||||
mailboxes: emailState.mailboxes,
|
||||
emails: [...emailState.emails],
|
||||
mailboxes: [...emailState.mailboxes],
|
||||
selectedEmail: emailState.selectedEmail,
|
||||
selectedMailbox: emailState.selectedMailbox,
|
||||
searchQuery: emailState.searchQuery,
|
||||
quota: emailState.quota,
|
||||
quota: emailState.quota ? { ...emailState.quota } : emailState.quota,
|
||||
},
|
||||
contact: {
|
||||
contacts: contactState.contacts,
|
||||
addressBooks: contactState.addressBooks,
|
||||
contacts: [...contactState.contacts],
|
||||
addressBooks: [...contactState.addressBooks],
|
||||
supportsSync: contactState.supportsSync,
|
||||
},
|
||||
calendar: {
|
||||
calendars: calendarState.calendars,
|
||||
events: calendarState.events,
|
||||
selectedCalendarIds: calendarState.selectedCalendarIds,
|
||||
calendars: [...calendarState.calendars],
|
||||
events: [...calendarState.events],
|
||||
selectedCalendarIds: [...calendarState.selectedCalendarIds],
|
||||
viewMode: calendarState.viewMode,
|
||||
supportsCalendar: calendarState.supportsCalendar,
|
||||
},
|
||||
filter: {
|
||||
rules: filterState.rules,
|
||||
rules: [...filterState.rules],
|
||||
isSupported: filterState.isSupported,
|
||||
},
|
||||
identity: {
|
||||
identities: identityState.identities,
|
||||
identities: [...identityState.identities],
|
||||
preferredPrimaryId: identityState.preferredPrimaryId,
|
||||
},
|
||||
vacation: {
|
||||
@@ -73,11 +77,22 @@ export function snapshotAccount(accountId: string): void {
|
||||
});
|
||||
}
|
||||
|
||||
/** Restore cached store states for the given account. Returns false if no cache exists. */
|
||||
/**
|
||||
* Restore cached store states for the given account. Returns false if no cache
|
||||
* exists.
|
||||
*
|
||||
* The snapshot only captures a subset of each store's fields (the loaded data),
|
||||
* so we reset every store to its baseline first. Without this, fields outside
|
||||
* the captured subset (e.g. email selection, loading flags, tag counts) would
|
||||
* carry over from whatever account was active, leaking state across accounts.
|
||||
* `setState` merges, so the captured fields are then layered back on top.
|
||||
*/
|
||||
export function restoreAccount(accountId: string): boolean {
|
||||
const snapshot = cache.get(accountId);
|
||||
if (!snapshot) return false;
|
||||
|
||||
clearAllStores();
|
||||
|
||||
useEmailStore.setState(snapshot.email);
|
||||
useContactStore.setState(snapshot.contact);
|
||||
useCalendarStore.setState(snapshot.calendar);
|
||||
|
||||
Reference in New Issue
Block a user