From 39574fe6e4e56159e25877e5809cce9a199652e6 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Fri, 29 Sep 2023 10:10:15 -0400 Subject: [PATCH] Transition account status to account info --- .../src/auth/abstractions/account.service.ts | 11 +- .../src/auth/services/account.service.spec.ts | 109 +++++++++++++----- .../src/auth/services/account.service.ts | 43 +++++-- 3 files changed, 124 insertions(+), 39 deletions(-) diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index d943e906459..af9dd77cb95 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -3,11 +3,18 @@ import { Observable } from "rxjs"; import { UserId } from "../../types/guid"; import { AuthenticationStatus } from "../enums/authentication-status"; +export type AccountInfo = { + status: AuthenticationStatus; + email: string; + name: string | undefined; +}; + export abstract class AccountService { - accounts$: Observable>; - activeAccount$: Observable<{ id: UserId | undefined; status: AuthenticationStatus | undefined }>; + accounts$: Observable>; + activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>; accountLock$: Observable; accountLogout$: Observable; + abstract addAccount(userId: UserId, accountData: AccountInfo): void; abstract setAccountStatus(userId: UserId, status: AuthenticationStatus): void; abstract switchAccount(userId: UserId): void; } diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts index 3ded75f59e0..ac801e2e850 100644 --- a/libs/common/src/auth/services/account.service.spec.ts +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -1,10 +1,11 @@ import { MockProxy, mock } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { firstValueFrom } from "rxjs"; import { trackEmissions } from "../../../spec/utils"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; import { UserId } from "../../types/guid"; +import { AccountInfo } from "../abstractions/account.service"; import { AuthenticationStatus } from "../enums/authentication-status"; import { AccountServiceImplementation } from "./account.service"; @@ -14,6 +15,9 @@ describe("accountService", () => { let logService: MockProxy; let sut: AccountServiceImplementation; const userId = "userId" as UserId; + function userInfo(status: AuthenticationStatus): AccountInfo { + return { status, email: "email", name: "name" }; + } beforeEach(() => { messagingService = mock(); @@ -26,41 +30,89 @@ describe("accountService", () => { jest.resetAllMocks(); }); - describe("setAccountStatus", () => { - let sutAccounts: BehaviorSubject>; - let accountsNext: jest.SpyInstance; - let emissions: Record[]; + describe("addAccount", () => { + it("should throw if the account already exists", () => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); - beforeEach(() => { - sutAccounts = sut["accounts"]; - accountsNext = jest.spyOn(sutAccounts, "next"); - emissions = []; - sutAccounts.subscribe((value) => emissions.push(JSON.parse(JSON.stringify(value)))); + expect(() => sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked))).toThrowError( + "Account already exists" + ); }); - it("should not emit if the status is the same", () => { - sut.setAccountStatus(userId, AuthenticationStatus.Locked); - sut.setAccountStatus(userId, AuthenticationStatus.Locked); - - expect(sutAccounts.value).toEqual({ userId: AuthenticationStatus.Locked }); - expect(accountsNext).toHaveBeenCalledTimes(1); - }); - - it("should emit if the status is different", () => { - const emissions = trackEmissions(sutAccounts); - sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); - sut.setAccountStatus(userId, AuthenticationStatus.Locked); + it("should emit the new account", () => { + const emissions = trackEmissions(sut.accounts$); + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); expect(emissions).toEqual([ {}, // initial value - { userId: AuthenticationStatus.Unlocked }, - { userId: AuthenticationStatus.Locked }, + { [userId]: userInfo(AuthenticationStatus.Unlocked) }, + ]); + }); + }); + + describe("setAccountName", () => { + beforeEach(() => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + }); + + it("should emit the updated account", () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountName(userId, "new name"); + + expect(emissions).toEqual([ + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), name: "name" } }, + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), name: "new name" } }, + ]); + }); + }); + + describe("setAccountEmail", () => { + beforeEach(() => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + }); + + it("should emit the updated account", () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountEmail(userId, "new email"); + + expect(emissions).toEqual([ + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), email: "email" } }, + { [userId]: { ...userInfo(AuthenticationStatus.Unlocked), email: "new email" } }, + ]); + }); + }); + + describe("setAccountStatus", () => { + beforeEach(() => { + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); + }); + + it("should not emit if the status is the same", async () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); + sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); + + expect(emissions).toEqual([{ userId: userInfo(AuthenticationStatus.Unlocked) }]); + }); + + it("should maintain an accounts cache", async () => { + expect(await firstValueFrom(sut.accounts$)).toEqual({ + [userId]: userInfo(AuthenticationStatus.Unlocked), + }); + }); + + it("should emit if the status is different", () => { + const emissions = trackEmissions(sut.accounts$); + sut.setAccountStatus(userId, AuthenticationStatus.Locked); + + expect(emissions).toEqual([ + { userId: userInfo(AuthenticationStatus.Unlocked) }, // initial value from beforeEach + { userId: userInfo(AuthenticationStatus.Locked) }, ]); }); it("should emit logout if the status is logged out", () => { const emissions = trackEmissions(sut.accountLogout$); - sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); sut.setAccountStatus(userId, AuthenticationStatus.LoggedOut); expect(emissions).toEqual([userId]); @@ -68,7 +120,6 @@ describe("accountService", () => { it("should emit lock if the status is locked", () => { const emissions = trackEmissions(sut.accountLock$); - sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); sut.setAccountStatus(userId, AuthenticationStatus.Locked); expect(emissions).toEqual([userId]); @@ -90,15 +141,15 @@ describe("accountService", () => { }); it("should emit the active account and status", () => { - sut.setAccountStatus(userId, AuthenticationStatus.Unlocked); + sut.addAccount(userId, userInfo(AuthenticationStatus.Unlocked)); sut.switchAccount(userId); sut.setAccountStatus(userId, AuthenticationStatus.Locked); sut.switchAccount(undefined); sut.switchAccount(undefined); expect(emissions).toEqual([ undefined, // initial value - { id: userId, status: AuthenticationStatus.Unlocked }, - { id: userId, status: AuthenticationStatus.Locked }, + { id: userId, ...userInfo(AuthenticationStatus.Unlocked) }, + { id: userId, ...userInfo(AuthenticationStatus.Locked) }, ]); }); diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index db2ab71c886..b311b29d671 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -7,14 +7,14 @@ import { share, } from "rxjs"; -import { InternalAccountService } from "../../auth/abstractions/account.service"; +import { AccountInfo, InternalAccountService } from "../../auth/abstractions/account.service"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; import { UserId } from "../../types/guid"; import { AuthenticationStatus } from "../enums/authentication-status"; export class AccountServiceImplementation implements InternalAccountService { - private accounts = new BehaviorSubject>({}); + private accounts = new BehaviorSubject>({}); private activeAccountId = new BehaviorSubject(undefined); private lock = new Subject(); private logout = new Subject(); @@ -22,7 +22,7 @@ export class AccountServiceImplementation implements InternalAccountService { accounts$ = this.accounts.asObservable(); activeAccount$ = this.activeAccountId.pipe( combineLatestWith(this.accounts$), - map(([id, accounts]) => (id ? { id, status: accounts[id] } : undefined)), + map(([id, accounts]) => (id ? { id, ...accounts[id] } : undefined)), distinctUntilChanged(), share() ); @@ -30,14 +30,26 @@ export class AccountServiceImplementation implements InternalAccountService { accountLogout$ = this.logout.asObservable(); constructor(private messagingService: MessagingService, private logService: LogService) {} - setAccountStatus(userId: UserId, status: AuthenticationStatus): void { - if (this.accounts.value[userId] === status) { - // Do not emit on no change - return; + addAccount(userId: UserId, accountData: AccountInfo): void { + if (this.accounts.value[userId] != null) { + throw new Error("Account already exists"); } - this.accounts.value[userId] = status; + this.accounts.value[userId] = accountData; this.accounts.next(this.accounts.value); + } + + setAccountName(userId: UserId, name: string): void { + this.setAccountInfo(userId, { ...this.accounts.value[userId], name }); + } + + setAccountEmail(userId: UserId, email: string): void { + this.setAccountInfo(userId, { ...this.accounts.value[userId], email }); + } + + setAccountStatus(userId: UserId, status: AuthenticationStatus): void { + this.setAccountInfo(userId, { ...this.accounts.value[userId], status }); + if (status === AuthenticationStatus.LoggedOut) { this.logout.next(userId); } else if (status === AuthenticationStatus.Locked) { @@ -67,4 +79,19 @@ export class AccountServiceImplementation implements InternalAccountService { throw e; } } + + private setAccountInfo(userId: UserId, accountInfo: AccountInfo) { + if (this.accounts.value[userId] == null) { + throw new Error("Account does not exist"); + } + + // Avoid unnecessary updates + // TODO: Faster comparison, maybe include a hash on the objects? + if (JSON.stringify(this.accounts.value[userId]) === JSON.stringify(accountInfo)) { + return; + } + + this.accounts.value[userId] = accountInfo; + this.accounts.next(this.accounts.value); + } }