[PM-5434] Create VaultBrowserStateService and migrate components from BrowserStateService (#8017)

* PM-5434 Initial work on migration

* PM-5434 Migration and tests

* PM-5434 Remove unnecessary comments

* PM-5434 Add unit tests

* PM-5434 Reverted last changes

* PM-5434 Added unit test for deserialize

* PM-5434 Minor changes

* PM-5434 Fix pr comments
This commit is contained in:
Carlos Gonçalves 2024-04-02 16:23:05 +01:00 committed by GitHub
parent a201e9cff1
commit af5f45443d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 173 additions and 90 deletions

View File

@ -3,22 +3,9 @@ import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage
import { Account } from "../../../models/account";
import { BrowserComponentState } from "../../../models/browserComponentState";
import { BrowserGroupingsComponentState } from "../../../models/browserGroupingsComponentState";
import { BrowserSendComponentState } from "../../../models/browserSendComponentState";
export abstract class BrowserStateService extends BaseStateServiceAbstraction<Account> {
getBrowserGroupingComponentState: (
options?: StorageOptions,
) => Promise<BrowserGroupingsComponentState>;
setBrowserGroupingComponentState: (
value: BrowserGroupingsComponentState,
options?: StorageOptions,
) => Promise<void>;
getBrowserVaultItemsComponentState: (options?: StorageOptions) => Promise<BrowserComponentState>;
setBrowserVaultItemsComponentState: (
value: BrowserComponentState,
options?: StorageOptions,
) => Promise<void>;
getBrowserSendComponentState: (options?: StorageOptions) => Promise<BrowserSendComponentState>;
setBrowserSendComponentState: (
value: BrowserSendComponentState,

View File

@ -18,7 +18,6 @@ import { UserId } from "@bitwarden/common/types/guid";
import { Account } from "../../models/account";
import { BrowserComponentState } from "../../models/browserComponentState";
import { BrowserGroupingsComponentState } from "../../models/browserGroupingsComponentState";
import { BrowserSendComponentState } from "../../models/browserSendComponentState";
import { BrowserStateService } from "./browser-state.service";
@ -86,27 +85,6 @@ describe("Browser State Service", () => {
);
});
describe("getBrowserGroupingComponentState", () => {
it("should return a BrowserGroupingsComponentState", async () => {
state.accounts[userId].groupings = new BrowserGroupingsComponentState();
const actual = await sut.getBrowserGroupingComponentState();
expect(actual).toBeInstanceOf(BrowserGroupingsComponentState);
});
});
describe("getBrowserVaultItemsComponentState", () => {
it("should return a BrowserComponentState", async () => {
const componentState = new BrowserComponentState();
componentState.scrollY = 0;
componentState.searchText = "test";
state.accounts[userId].ciphers = componentState;
const actual = await sut.getBrowserVaultItemsComponentState();
expect(actual).toStrictEqual(componentState);
});
});
describe("getBrowserSendComponentState", () => {
it("should return a BrowserSendComponentState", async () => {
const sendState = new BrowserSendComponentState();

View File

@ -16,7 +16,6 @@ import { StateService as BaseStateService } from "@bitwarden/common/platform/ser
import { Account } from "../../models/account";
import { BrowserComponentState } from "../../models/browserComponentState";
import { BrowserGroupingsComponentState } from "../../models/browserGroupingsComponentState";
import { BrowserSendComponentState } from "../../models/browserSendComponentState";
import { BrowserApi } from "../browser/browser-api";
import { browserSession, sessionSync } from "../decorators/session-sync-observable";
@ -116,50 +115,6 @@ export class BrowserStateService
);
}
async getBrowserGroupingComponentState(
options?: StorageOptions,
): Promise<BrowserGroupingsComponentState> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions()))
)?.groupings;
}
async setBrowserGroupingComponentState(
value: BrowserGroupingsComponentState,
options?: StorageOptions,
): Promise<void> {
const account = await this.getAccount(
this.reconcileOptions(options, await this.defaultInMemoryOptions()),
);
account.groupings = value;
await this.saveAccount(
account,
this.reconcileOptions(options, await this.defaultInMemoryOptions()),
);
}
async getBrowserVaultItemsComponentState(
options?: StorageOptions,
): Promise<BrowserComponentState> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions()))
)?.ciphers;
}
async setBrowserVaultItemsComponentState(
value: BrowserComponentState,
options?: StorageOptions,
): Promise<void> {
const account = await this.getAccount(
this.reconcileOptions(options, await this.defaultInMemoryOptions()),
);
account.ciphers = value;
await this.saveAccount(
account,
this.reconcileOptions(options, await this.defaultInMemoryOptions()),
);
}
async getBrowserSendComponentState(options?: StorageOptions): Promise<BrowserSendComponentState> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions()))

View File

@ -12,6 +12,7 @@ import { BrowserApi } from "../platform/browser/browser-api";
import { ZonedMessageListenerService } from "../platform/browser/zoned-message-listener.service";
import { BrowserStateService } from "../platform/services/abstractions/browser-state.service";
import { ForegroundPlatformUtilsService } from "../platform/services/platform-utils/foreground-platform-utils.service";
import { VaultBrowserStateService } from "../vault/services/vault-browser-state.service";
import { routerTransition } from "./app-routing.animations";
import { DesktopSyncVerificationDialogComponent } from "./components/desktop-sync-verification-dialog.component";
@ -37,6 +38,7 @@ export class AppComponent implements OnInit, OnDestroy {
private i18nService: I18nService,
private router: Router,
private stateService: BrowserStateService,
private vaultBrowserStateService: VaultBrowserStateService,
private changeDetectorRef: ChangeDetectorRef,
private ngZone: NgZone,
private platformUtilsService: ForegroundPlatformUtilsService,
@ -227,8 +229,8 @@ export class AppComponent implements OnInit, OnDestroy {
}
await Promise.all([
this.stateService.setBrowserGroupingComponentState(null),
this.stateService.setBrowserVaultItemsComponentState(null),
this.vaultBrowserStateService.setBrowserGroupingsComponentState(null),
this.vaultBrowserStateService.setBrowserVaultItemsComponentState(null),
this.stateService.setBrowserSendComponentState(null),
this.stateService.setBrowserSendTypeComponentState(null),
]);

View File

@ -102,6 +102,7 @@ import { ForegroundPlatformUtilsService } from "../../platform/services/platform
import { ForegroundDerivedStateProvider } from "../../platform/state/foreground-derived-state.provider";
import { ForegroundMemoryStorageService } from "../../platform/storage/foreground-memory-storage.service";
import { FilePopoutUtilsService } from "../../tools/popup/services/file-popout-utils.service";
import { VaultBrowserStateService } from "../../vault/services/vault-browser-state.service";
import { VaultFilterService } from "../../vault/services/vault-filter.service";
import { DebounceNavigationService } from "./debounce-navigation.service";
@ -377,6 +378,13 @@ const safeProviders: SafeProvider[] = [
provide: OBSERVABLE_DISK_STORAGE,
useExisting: AbstractStorageService,
}),
safeProvider({
provide: VaultBrowserStateService,
useFactory: (stateProvider: StateProvider) => {
return new VaultBrowserStateService(stateProvider);
},
deps: [StateProvider],
}),
safeProvider({
provide: StateServiceAbstraction,
useFactory: (

View File

@ -20,7 +20,7 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
import { BrowserGroupingsComponentState } from "../../../../models/browserGroupingsComponentState";
import { BrowserApi } from "../../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../../platform/popup/browser-popup-utils";
import { BrowserStateService } from "../../../../platform/services/abstractions/browser-state.service";
import { VaultBrowserStateService } from "../../../services/vault-browser-state.service";
import { VaultFilterService } from "../../../services/vault-filter.service";
const ComponentId = "VaultComponent";
@ -84,8 +84,8 @@ export class VaultFilterComponent implements OnInit, OnDestroy {
private platformUtilsService: PlatformUtilsService,
private searchService: SearchService,
private location: Location,
private browserStateService: BrowserStateService,
private vaultFilterService: VaultFilterService,
private vaultBrowserStateService: VaultBrowserStateService,
) {
this.noFolderListSize = 100;
}
@ -95,7 +95,7 @@ export class VaultFilterComponent implements OnInit, OnDestroy {
this.showLeftHeader = !(
BrowserPopupUtils.inSidebar(window) && this.platformUtilsService.isFirefox()
);
await this.browserStateService.setBrowserVaultItemsComponentState(null);
await this.vaultBrowserStateService.setBrowserVaultItemsComponentState(null);
this.broadcasterService.subscribe(ComponentId, (message: any) => {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
@ -120,7 +120,7 @@ export class VaultFilterComponent implements OnInit, OnDestroy {
const restoredScopeState = await this.restoreState();
// eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe
this.route.queryParams.pipe(first()).subscribe(async (params) => {
this.state = await this.browserStateService.getBrowserGroupingComponentState();
this.state = await this.vaultBrowserStateService.getBrowserGroupingsComponentState();
if (this.state?.searchText) {
this.searchText = this.state.searchText;
} else if (params.searchText) {
@ -413,11 +413,11 @@ export class VaultFilterComponent implements OnInit, OnDestroy {
collections: this.collections,
deletedCount: this.deletedCount,
});
await this.browserStateService.setBrowserGroupingComponentState(this.state);
await this.vaultBrowserStateService.setBrowserGroupingsComponentState(this.state);
}
private async restoreState(): Promise<boolean> {
this.state = await this.browserStateService.getBrowserGroupingComponentState();
this.state = await this.vaultBrowserStateService.getBrowserGroupingsComponentState();
if (this.state == null) {
return false;
}

View File

@ -21,7 +21,7 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
import { BrowserComponentState } from "../../../../models/browserComponentState";
import { BrowserApi } from "../../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../../platform/popup/browser-popup-utils";
import { BrowserStateService } from "../../../../platform/services/abstractions/browser-state.service";
import { VaultBrowserStateService } from "../../../services/vault-browser-state.service";
import { VaultFilterService } from "../../../services/vault-filter.service";
const ComponentId = "VaultItemsComponent";
@ -59,7 +59,7 @@ export class VaultItemsComponent extends BaseVaultItemsComponent implements OnIn
private ngZone: NgZone,
private broadcasterService: BroadcasterService,
private changeDetectorRef: ChangeDetectorRef,
private stateService: BrowserStateService,
private stateService: VaultBrowserStateService,
private i18nService: I18nService,
private collectionService: CollectionService,
private platformUtilsService: PlatformUtilsService,

View File

@ -0,0 +1,87 @@
import {
FakeAccountService,
mockAccountServiceWith,
} from "@bitwarden/common/../spec/fake-account-service";
import { FakeStateProvider } from "@bitwarden/common/../spec/fake-state-provider";
import { Jsonify } from "type-fest";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherType } from "@bitwarden/common/vault/enums";
import { BrowserComponentState } from "../../models/browserComponentState";
import { BrowserGroupingsComponentState } from "../../models/browserGroupingsComponentState";
import {
VAULT_BROWSER_COMPONENT,
VAULT_BROWSER_GROUPINGS_COMPONENT,
VaultBrowserStateService,
} from "./vault-browser-state.service";
describe("Vault Browser State Service", () => {
let stateProvider: FakeStateProvider;
let accountService: FakeAccountService;
let stateService: VaultBrowserStateService;
const mockUserId = Utils.newGuid() as UserId;
beforeEach(() => {
accountService = mockAccountServiceWith(mockUserId);
stateProvider = new FakeStateProvider(accountService);
stateService = new VaultBrowserStateService(stateProvider);
});
describe("getBrowserGroupingsComponentState", () => {
it("should return a BrowserGroupingsComponentState", async () => {
await stateService.setBrowserGroupingsComponentState(new BrowserGroupingsComponentState());
const actual = await stateService.getBrowserGroupingsComponentState();
expect(actual).toBeInstanceOf(BrowserGroupingsComponentState);
});
it("should deserialize BrowserGroupingsComponentState", () => {
const sut = VAULT_BROWSER_GROUPINGS_COMPONENT;
const expectedState = {
deletedCount: 0,
collectionCounts: new Map<string, number>(),
folderCounts: new Map<string, number>(),
typeCounts: new Map<CipherType, number>(),
};
const result = sut.deserializer(
JSON.parse(JSON.stringify(expectedState)) as Jsonify<BrowserGroupingsComponentState>,
);
expect(result).toEqual(expectedState);
});
});
describe("getBrowserVaultItemsComponentState", () => {
it("should deserialize BrowserComponentState", () => {
const sut = VAULT_BROWSER_COMPONENT;
const expectedState = {
scrollY: 0,
searchText: "test",
};
const result = sut.deserializer(JSON.parse(JSON.stringify(expectedState)));
expect(result).toEqual(expectedState);
});
it("should return a BrowserComponentState", async () => {
const componentState = new BrowserComponentState();
componentState.scrollY = 0;
componentState.searchText = "test";
await stateService.setBrowserVaultItemsComponentState(componentState);
const actual = await stateService.getBrowserVaultItemsComponentState();
expect(actual).toStrictEqual(componentState);
});
});
});

View File

@ -0,0 +1,65 @@
import { Observable, firstValueFrom } from "rxjs";
import { Jsonify } from "type-fest";
import {
ActiveUserState,
KeyDefinition,
StateProvider,
VAULT_BROWSER_MEMORY,
} from "@bitwarden/common/platform/state";
import { BrowserComponentState } from "../../models/browserComponentState";
import { BrowserGroupingsComponentState } from "../../models/browserGroupingsComponentState";
export const VAULT_BROWSER_GROUPINGS_COMPONENT = new KeyDefinition<BrowserGroupingsComponentState>(
VAULT_BROWSER_MEMORY,
"vault_browser_groupings_component",
{
deserializer: (obj: Jsonify<BrowserGroupingsComponentState>) =>
BrowserGroupingsComponentState.fromJSON(obj),
},
);
export const VAULT_BROWSER_COMPONENT = new KeyDefinition<BrowserComponentState>(
VAULT_BROWSER_MEMORY,
"vault_browser_component",
{
deserializer: (obj: Jsonify<BrowserComponentState>) => BrowserComponentState.fromJSON(obj),
},
);
export class VaultBrowserStateService {
vaultBrowserGroupingsComponentState$: Observable<BrowserGroupingsComponentState>;
vaultBrowserComponentState$: Observable<BrowserComponentState>;
private activeUserVaultBrowserGroupingsComponentState: ActiveUserState<BrowserGroupingsComponentState>;
private activeUserVaultBrowserComponentState: ActiveUserState<BrowserComponentState>;
constructor(protected stateProvider: StateProvider) {
this.activeUserVaultBrowserGroupingsComponentState = this.stateProvider.getActive(
VAULT_BROWSER_GROUPINGS_COMPONENT,
);
this.activeUserVaultBrowserComponentState =
this.stateProvider.getActive(VAULT_BROWSER_COMPONENT);
this.vaultBrowserGroupingsComponentState$ =
this.activeUserVaultBrowserGroupingsComponentState.state$;
this.vaultBrowserComponentState$ = this.activeUserVaultBrowserComponentState.state$;
}
async getBrowserGroupingsComponentState(): Promise<BrowserGroupingsComponentState> {
return await firstValueFrom(this.vaultBrowserGroupingsComponentState$);
}
async setBrowserGroupingsComponentState(value: BrowserGroupingsComponentState): Promise<void> {
await this.activeUserVaultBrowserGroupingsComponentState.update(() => value);
}
async getBrowserVaultItemsComponentState(): Promise<BrowserComponentState> {
return await firstValueFrom(this.vaultBrowserComponentState$);
}
async setBrowserVaultItemsComponentState(value: BrowserComponentState): Promise<void> {
await this.activeUserVaultBrowserComponentState.update(() => value);
}
}

View File

@ -118,3 +118,4 @@ export const VAULT_ONBOARDING = new StateDefinition("vaultOnboarding", "disk", {
export const VAULT_SETTINGS_DISK = new StateDefinition("vaultSettings", "disk", {
web: "disk-local",
});
export const VAULT_BROWSER_MEMORY = new StateDefinition("vaultBrowser", "memory");