1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-17 08:43:33 +00:00

ActiveUserState Update should return the userId of the impacted user. (#7869)

This allows us to ensure that linked updates all go to the same user without risking active account changes in the middle of an operation.
This commit is contained in:
Matt Gibson
2024-02-08 14:54:15 -05:00
committed by GitHub
parent 78730ff18a
commit 4c051f8d7f
7 changed files with 63 additions and 31 deletions

View File

@@ -266,12 +266,13 @@ describe("DefaultActiveUserState", () => {
});
it("should save on update", async () => {
const result = await userState.update((state, dependencies) => {
const [setUserId, result] = await userState.update((state, dependencies) => {
return newData;
});
expect(diskStorageService.mock.save).toHaveBeenCalledTimes(1);
expect(result).toEqual(newData);
expect(setUserId).toEqual("00000000-0000-1000-a000-000000000001");
});
it("should emit once per update", async () => {
@@ -316,7 +317,7 @@ describe("DefaultActiveUserState", () => {
const emissions = trackEmissions(userState.state$);
await awaitAsync(); // Need to await for the initial value to be emitted
const result = await userState.update(
const [userIdResult, result] = await userState.update(
(state, dependencies) => {
return newData;
},
@@ -328,6 +329,7 @@ describe("DefaultActiveUserState", () => {
await awaitAsync();
expect(diskStorageService.mock.save).not.toHaveBeenCalled();
expect(userIdResult).toEqual("00000000-0000-1000-a000-000000000001");
expect(result).toBeNull();
expect(emissions).toEqual([null]);
});
@@ -422,12 +424,13 @@ describe("DefaultActiveUserState", () => {
await originalSave(key, obj);
});
const val = await userState.update(() => {
const [userIdResult, val] = await userState.update(() => {
return newData;
});
await awaitAsync(10);
expect(userIdResult).toEqual(userId);
expect(val).toEqual(newData);
expect(emissions).toEqual([initialData, newData]);
expect(emissions2).toEqual([initialData, newData]);
@@ -447,7 +450,7 @@ describe("DefaultActiveUserState", () => {
expect(emissions).toEqual([initialData]);
let emissions2: TestState[];
const val = await userState.update(
const [userIdResult, val] = await userState.update(
(state) => {
return newData;
},
@@ -461,6 +464,7 @@ describe("DefaultActiveUserState", () => {
await awaitAsync();
expect(userIdResult).toEqual(userId);
expect(val).toEqual(initialData);
expect(emissions).toEqual([initialData]);
@@ -497,10 +501,11 @@ describe("DefaultActiveUserState", () => {
test("updates with FAKE_DEFAULT initial value should resolve correctly", async () => {
expect(diskStorageService["updatesSubject"]["observers"]).toHaveLength(0);
const val = await userState.update((state) => {
const [userIdResult, val] = await userState.update((state) => {
return newData;
});
expect(userIdResult).toEqual(userId);
expect(val).toEqual(newData);
const call = diskStorageService.mock.save.mock.calls[0];
expect(call[0]).toEqual(`user_${userId}_fake_fake`);

View File

@@ -31,7 +31,7 @@ const FAKE = Symbol("fake");
export class DefaultActiveUserState<T> implements ActiveUserState<T> {
[activeMarker]: true;
private updatePromise: Promise<T> | null = null;
private updatePromise: Promise<[UserId, T]> | null = null;
private activeUserId$: Observable<UserId | null>;
@@ -120,15 +120,15 @@ export class DefaultActiveUserState<T> implements ActiveUserState<T> {
async update<TCombine>(
configureState: (state: T, dependency: TCombine) => T,
options: StateUpdateOptions<T, TCombine> = {},
): Promise<T> {
): Promise<[UserId, T]> {
options = populateOptionsWithDefault(options);
try {
if (this.updatePromise != null) {
await this.updatePromise;
}
this.updatePromise = this.internalUpdate(configureState, options);
const newState = await this.updatePromise;
return newState;
const [userId, newState] = await this.updatePromise;
return [userId, newState];
} finally {
this.updatePromise = null;
}
@@ -137,20 +137,20 @@ export class DefaultActiveUserState<T> implements ActiveUserState<T> {
private async internalUpdate<TCombine>(
configureState: (state: T, dependency: TCombine) => T,
options: StateUpdateOptions<T, TCombine>,
) {
const [key, currentState] = await this.getStateForUpdate();
): Promise<[UserId, T]> {
const [userId, key, currentState] = await this.getStateForUpdate();
const combinedDependencies =
options.combineLatestWith != null
? await firstValueFrom(options.combineLatestWith.pipe(timeout(options.msTimeout)))
: null;
if (!options.shouldUpdate(currentState, combinedDependencies)) {
return currentState;
return [userId, currentState];
}
const newState = configureState(currentState, combinedDependencies);
await this.saveToStorage(key, newState);
return newState;
return [userId, newState];
}
/** For use in update methods, does not wait for update to complete before yielding state.
@@ -170,6 +170,7 @@ export class DefaultActiveUserState<T> implements ActiveUserState<T> {
}
const fullKey = userKeyBuilder(userId, this.keyDefinition);
return [
userId,
fullKey,
await getStoredValue(fullKey, this.chosenStorageLocation, this.keyDefinition.deserializer),
] as const;

View File

@@ -32,11 +32,15 @@ export class DefaultStateProvider implements StateProvider {
}
}
async setUserState<T>(keyDefinition: KeyDefinition<T>, value: T, userId?: UserId): Promise<void> {
async setUserState<T>(
keyDefinition: KeyDefinition<T>,
value: T,
userId?: UserId,
): Promise<[UserId, T]> {
if (userId) {
await this.getUser<T>(userId, keyDefinition).update(() => value);
return [userId, await this.getUser<T>(userId, keyDefinition).update(() => value)];
} else {
await this.getActive<T>(keyDefinition).update(() => value);
return await this.getActive<T>(keyDefinition).update(() => value);
}
}