bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 15:53:27 +00:00
Code Issues Releases Wiki Activity

[PM-24280] Remove account service from state (#15828)

Browse Source 
* Introduce ActiveUserAccessor

* Use ActiveUserAccessor over AccountService

* Updates tests and testing utils to support ActiveUserAccessor

* Update all injection points

* Fix types test

* Use ternary instead
This commit is contained in:
Justin Baur
2025-07-31 09:09:14 -04:00
committed by GitHub
parent 9c8188875a
commit 4f9b2b618f
14 changed files with 118 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
libs/common/src/auth/services/default-active-user.accessor.ts Normal file
View File

@@ -0,0 +1,19 @@
import { map, Observable } from "rxjs";
import { UserId } from "@bitwarden/user-core";
import { ActiveUserAccessor } from "../../platform/state";
import { AccountService } from "../abstractions/account.service";
/**
* Implementation for Platform so they can avoid a direct dependency on AccountService. Not for general consumption.
*/
export class DefaultActiveUserAccessor implements ActiveUserAccessor {
constructor(private readonly accountService: AccountService) {
this.activeUserId$ = this.accountService.activeAccount$.pipe(
map((a) => (a != null ? a.id : null)),
);
}
activeUserId$: Observable<UserId | null>;
}

11
libs/common/src/platform/state/active-user.accessor.ts Normal file
View File

@@ -0,0 +1,11 @@
import { Observable } from "rxjs";
import { UserId } from "@bitwarden/user-core";
export abstract class ActiveUserAccessor {
/**
* Returns a stream of the current active user for the application. The stream either emits the user id for that account
* or returns null if there is no current active user.
*/
abstract activeUserId$: Observable<UserId | null>;
}

37
libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts
View File

@@ -1,37 +0,0 @@
import { mock } from "jest-mock-extended";
import { mockAccountServiceWith, trackEmissions } from "../../../../spec";
import { UserId } from "../../../types/guid";
import { SingleUserStateProvider } from "../user-state.provider";
import { DefaultActiveUserStateProvider } from "./default-active-user-state.provider";
describe("DefaultActiveUserStateProvider", () => {
const singleUserStateProvider = mock<SingleUserStateProvider>();
const userId = "userId" as UserId;
const accountInfo = {
id: userId,
name: "name",
email: "email",
emailVerified: false,
};
const accountService = mockAccountServiceWith(userId, accountInfo);
let sut: DefaultActiveUserStateProvider;
beforeEach(() => {
sut = new DefaultActiveUserStateProvider(accountService, singleUserStateProvider);
});
afterEach(() => {
jest.resetAllMocks();
});
it("should track the active User id from account service", () => {
const emissions = trackEmissions(sut.activeUserId$);
accountService.activeAccountSubject.next(undefined);
accountService.activeAccountSubject.next(accountInfo);
expect(emissions).toEqual([userId, undefined, userId]);
});
});

9
libs/common/src/platform/state/implementations/default-active-user-state.provider.ts
View File

@@ -1,9 +1,9 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { Observable, distinctUntilChanged, map } from "rxjs";
import { Observable, distinctUntilChanged } from "rxjs";
import { AccountService } from "../../../auth/abstractions/account.service";
import { UserId } from "../../../types/guid";
import { ActiveUserAccessor } from "../active-user.accessor";
import { UserKeyDefinition } from "../user-key-definition";
import { ActiveUserState } from "../user-state";
import { ActiveUserStateProvider, SingleUserStateProvider } from "../user-state.provider";
@@ -14,11 +14,10 @@ export class DefaultActiveUserStateProvider implements ActiveUserStateProvider {
activeUserId$: Observable<UserId | undefined>;
constructor(
private readonly accountService: AccountService,
private readonly activeAccountAccessor: ActiveUserAccessor,
private readonly singleUserStateProvider: SingleUserStateProvider,
) {
this.activeUserId$ = this.accountService.activeAccount$.pipe(
map((account) => account?.id),
this.activeUserId$ = this.activeAccountAccessor.activeUserId$.pipe(
// To avoid going to storage when we don't need to, only get updates when there is a true change.
distinctUntilChanged((a, b) => (a == null || b == null ? a == b : a === b)), // Treat null and undefined as equal
);

18
libs/common/src/platform/state/implementations/default-active-user-state.spec.ts
View File

@@ -3,14 +3,13 @@
* @jest-environment ../shared/test.environment.ts
*/
import { any, mock } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom, map, of, timeout } from "rxjs";
import { BehaviorSubject, firstValueFrom, of, timeout } from "rxjs";
import { Jsonify } from "type-fest";
import { StorageServiceProvider } from "@bitwarden/storage-core";
import { awaitAsync, trackEmissions } from "../../../../spec";
import { FakeStorageService } from "../../../../spec/fake-storage.service";
import { Account } from "../../../auth/abstractions/account.service";
import { UserId } from "../../../types/guid";
import { LogService } from "../../abstractions/log.service";
import { StateDefinition } from "../state-definition";
@@ -48,7 +47,7 @@ describe("DefaultActiveUserState", () => {
const storageServiceProvider = mock<StorageServiceProvider>();
const stateEventRegistrarService = mock<StateEventRegistrarService>();
const logService = mock<LogService>();
let activeAccountSubject: BehaviorSubject<Account | null>;
let activeAccountSubject: BehaviorSubject<UserId | null>;
let singleUserStateProvider: DefaultSingleUserStateProvider;
@@ -64,11 +63,11 @@ describe("DefaultActiveUserState", () => {
logService,
);
activeAccountSubject = new BehaviorSubject<Account | null>(null);
activeAccountSubject = new BehaviorSubject<UserId | null>(null);
userState = new DefaultActiveUserState(
testKeyDefinition,
activeAccountSubject.asObservable().pipe(map((a) => a?.id)),
activeAccountSubject.asObservable(),
singleUserStateProvider,
);
});
@@ -83,12 +82,7 @@ describe("DefaultActiveUserState", () => {
const changeActiveUser = async (id: string) => {
const userId = makeUserId(id);
activeAccountSubject.next({
id: userId,
email: `test${id}@example.com`,
emailVerified: false,
name: `Test User ${id}`,
});
activeAccountSubject.next(userId);
await awaitAsync();
};
@@ -588,7 +582,7 @@ describe("DefaultActiveUserState", () => {
});
it("does not await updates if the active user changes", async () => {
const initialUserId = (await firstValueFrom(activeAccountSubject)).id;
const initialUserId = activeAccountSubject.value;
expect(initialUserId).toBe(userId);
trackEmissions(userState.state$);
await awaitAsync(); // storage updates are behind a promise

34
libs/common/src/platform/state/implementations/default-state.provider.spec.ts
View File

@@ -4,16 +4,16 @@
*/
import { Observable, of } from "rxjs";
import { UserId } from "@bitwarden/user-core";
import { awaitAsync, trackEmissions } from "../../../../spec";
import { FakeAccountService, mockAccountServiceWith } from "../../../../spec/fake-account-service";
import {
FakeActiveUserAccessor,
FakeActiveUserStateProvider,
FakeDerivedStateProvider,
FakeGlobalStateProvider,
FakeSingleUserStateProvider,
} from "../../../../spec/fake-state-provider";
import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
import { UserId } from "../../../types/guid";
import { DeriveDefinition } from "../derive-definition";
import { KeyDefinition } from "../key-definition";
import { StateDefinition } from "../state-definition";
@@ -27,12 +27,12 @@ describe("DefaultStateProvider", () => {
let singleUserStateProvider: FakeSingleUserStateProvider;
let globalStateProvider: FakeGlobalStateProvider;
let derivedStateProvider: FakeDerivedStateProvider;
let accountService: FakeAccountService;
let activeAccountAccessor: FakeActiveUserAccessor;
const userId = "fakeUserId" as UserId;
beforeEach(() => {
accountService = mockAccountServiceWith(userId);
activeUserStateProvider = new FakeActiveUserStateProvider(accountService);
activeAccountAccessor = new FakeActiveUserAccessor(userId);
activeUserStateProvider = new FakeActiveUserStateProvider(activeAccountAccessor);
singleUserStateProvider = new FakeSingleUserStateProvider();
globalStateProvider = new FakeGlobalStateProvider();
derivedStateProvider = new FakeDerivedStateProvider();
@@ -70,12 +70,6 @@ describe("DefaultStateProvider", () => {
userId?: UserId,
) => Observable<string>,
) => {
const accountInfo = {
email: "email",
emailVerified: false,
name: "name",
status: AuthenticationStatus.LoggedOut,
};
const keyDefinition = new UserKeyDefinition<string>(
new StateDefinition("test", "disk"),
"test",
@@ -97,7 +91,7 @@ describe("DefaultStateProvider", () => {
});
it("should follow the current active user if no userId is provided", async () => {
accountService.activeAccountSubject.next({ id: userId, ...accountInfo });
activeAccountAccessor.switch(userId);
const state = singleUserStateProvider.getFake(userId, keyDefinition);
state.nextState("value");
const emissions = trackEmissions(methodUnderTest(keyDefinition));
@@ -113,7 +107,7 @@ describe("DefaultStateProvider", () => {
state.nextState("value");
const emissions = trackEmissions(methodUnderTest(keyDefinition));
accountService.activeAccountSubject.next({ id: "newUserId" as UserId, ...accountInfo });
activeAccountAccessor.switch("newUserId" as UserId);
const newUserEmissions = trackEmissions(sut.getUserState$(keyDefinition));
state.nextState("value2");
state.nextState("value3");
@@ -125,12 +119,6 @@ describe("DefaultStateProvider", () => {
);
describe("getUserState$", () => {
const accountInfo = {
email: "email",
emailVerified: false,
name: "name",
status: AuthenticationStatus.LoggedOut,
};
const keyDefinition = new UserKeyDefinition<string>(
new StateDefinition("test", "disk"),
"test",
@@ -141,7 +129,7 @@ describe("DefaultStateProvider", () => {
);
it("should not emit any values until a truthy user id is supplied", async () => {
accountService.activeAccountSubject.next(null);
activeAccountAccessor.switch(null);
const state = singleUserStateProvider.getFake(userId, keyDefinition);
state.nextState("value");
@@ -151,7 +139,7 @@ describe("DefaultStateProvider", () => {
expect(emissions).toHaveLength(0);
accountService.activeAccountSubject.next({ id: userId, ...accountInfo });
activeAccountAccessor.switch(userId);
await awaitAsync();
@@ -170,7 +158,7 @@ describe("DefaultStateProvider", () => {
);
it("should emit default value if no userId supplied and first active user id emission in falsy", async () => {
accountService.activeAccountSubject.next(null);
activeAccountAccessor.switch(null);
const emissions = trackEmissions(
sut.getUserStateOrDefault$(keyDefinition, {

4
libs/common/src/platform/state/implementations/specific-state.provider.spec.ts
View File

@@ -2,7 +2,7 @@ import { mock } from "jest-mock-extended";
import { StorageServiceProvider } from "@bitwarden/storage-core";
import { mockAccountServiceWith } from "../../../../spec/fake-account-service";
import { FakeActiveUserAccessor } from "../../../../spec";
import { FakeStorageService } from "../../../../spec/fake-storage.service";
import { UserId } from "../../../types/guid";
import { LogService } from "../../abstractions/log.service";
@@ -39,7 +39,7 @@ describe("Specific State Providers", () => {
stateEventRegistrarService,
logService,
);
activeSut = new DefaultActiveUserStateProvider(mockAccountServiceWith(null), singleSut);
activeSut = new DefaultActiveUserStateProvider(new FakeActiveUserAccessor(null), singleSut);
globalSut = new DefaultGlobalStateProvider(storageServiceProvider, logService);
});

1
libs/common/src/platform/state/index.ts
View File

@@ -10,5 +10,6 @@ export { KeyDefinition, KeyDefinitionOptions } from "./key-definition";
export { StateUpdateOptions } from "./state-update-options";
export { UserKeyDefinitionOptions, UserKeyDefinition } from "./user-key-definition";
export { StateEventRunnerService } from "./state-event-runner.service";
export { ActiveUserAccessor } from "./active-user.accessor";
export * from "./state-definitions";