From ad3f2e4eb927022c0da3883227da03c406bff2aa Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Tue, 30 Apr 2024 16:00:03 -0500 Subject: [PATCH] [PM-6426] Working through jest tests and implementing a method to guard against setting a task without having a registered callback --- ...rate-password-to-clipboard-command.spec.ts | 2 +- .../browser/src/background/main.background.ts | 5 +- .../task-scheduler-service.factory.ts | 10 +- .../browser-task-scheduler.service.ts | 9 +- .../browser-task-scheduler.service.spec.ts | 361 +++--------------- .../browser-task-scheduler.service.ts | 68 +++- .../src/popup/services/services.module.ts | 6 +- 7 files changed, 120 insertions(+), 341 deletions(-) diff --git a/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts b/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts index e74292563e..b56cb4d5e3 100644 --- a/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts +++ b/apps/browser/src/autofill/clipboard/generate-password-to-clipboard-command.spec.ts @@ -6,7 +6,7 @@ import { ScheduledTaskNames } from "@bitwarden/common/platform/enums/scheduled-t import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password"; import { BrowserApi } from "../../platform/browser/browser-api"; -import { BrowserTaskSchedulerService } from "../../platform/services/browser-task-scheduler.service"; +import { BrowserTaskSchedulerService } from "../../platform/services/abstractions/browser-task-scheduler.service"; import { ClearClipboard } from "./clear-clipboard"; import { GeneratePasswordToClipboardCommand } from "./generate-password-to-clipboard-command"; diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 31a56a1a3c..1c2655e8f7 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -215,12 +215,13 @@ import { UpdateBadge } from "../platform/listeners/update-badge"; import { ChromeMessageSender } from "../platform/messaging/chrome-message.sender"; /* eslint-enable no-restricted-imports */ import { BrowserStateService as StateServiceAbstraction } from "../platform/services/abstractions/browser-state.service"; +import { BrowserTaskSchedulerService } from "../platform/services/abstractions/browser-task-scheduler.service"; import { BrowserCryptoService } from "../platform/services/browser-crypto.service"; import { BrowserEnvironmentService } from "../platform/services/browser-environment.service"; import BrowserLocalStorageService from "../platform/services/browser-local-storage.service"; import BrowserMemoryStorageService from "../platform/services/browser-memory-storage.service"; import { BrowserScriptInjectorService } from "../platform/services/browser-script-injector.service"; -import { BrowserTaskSchedulerService } from "../platform/services/browser-task-scheduler.service"; +import { BrowserTaskSchedulerServiceImplementation } from "../platform/services/browser-task-scheduler.service"; import { DefaultBrowserStateService } from "../platform/services/default-browser-state.service"; import I18nService from "../platform/services/i18n.service"; import { LocalBackedSessionStorageService } from "../platform/services/local-backed-session-storage.service"; @@ -505,7 +506,7 @@ export default class MainBackground { // The taskSchedulerService needs to be instantiated a single time in a potential context. // Since the popup creates a new instance of the main background in mv3, we need to guard against a duplicate registration. if (!this.popupOnlyContext) { - this.taskSchedulerService = new BrowserTaskSchedulerService( + this.taskSchedulerService = new BrowserTaskSchedulerServiceImplementation( this.logService, this.stateProvider, ); diff --git a/apps/browser/src/platform/background/service-factories/task-scheduler-service.factory.ts b/apps/browser/src/platform/background/service-factories/task-scheduler-service.factory.ts index 218566a32b..640f4fdae5 100644 --- a/apps/browser/src/platform/background/service-factories/task-scheduler-service.factory.ts +++ b/apps/browser/src/platform/background/service-factories/task-scheduler-service.factory.ts @@ -1,4 +1,4 @@ -import { BrowserTaskSchedulerService } from "../../services/browser-task-scheduler.service"; +import { BrowserTaskSchedulerServiceImplementation } from "../../services/browser-task-scheduler.service"; import { CachedServices, factory, FactoryOptions } from "./factory-options"; import { logServiceFactory, LogServiceInitOptions } from "./log-service.factory"; @@ -11,15 +11,17 @@ export type TaskSchedulerServiceInitOptions = TaskSchedulerServiceFactoryOptions StateProviderInitOptions; export function taskSchedulerServiceFactory( - cache: { browserTaskSchedulerService?: BrowserTaskSchedulerService } & CachedServices, + cache: { + browserTaskSchedulerService?: BrowserTaskSchedulerServiceImplementation; + } & CachedServices, opts: TaskSchedulerServiceInitOptions, -): Promise { +): Promise { return factory( cache, "browserTaskSchedulerService", opts, async () => - new BrowserTaskSchedulerService( + new BrowserTaskSchedulerServiceImplementation( await logServiceFactory(cache, opts), await stateProviderFactory(cache, opts), ), diff --git a/apps/browser/src/platform/services/abstractions/browser-task-scheduler.service.ts b/apps/browser/src/platform/services/abstractions/browser-task-scheduler.service.ts index 7a8c72bef2..63ceb400fc 100644 --- a/apps/browser/src/platform/services/abstractions/browser-task-scheduler.service.ts +++ b/apps/browser/src/platform/services/abstractions/browser-task-scheduler.service.ts @@ -1,3 +1,5 @@ +import { Observable } from "rxjs"; + import { TaskSchedulerService } from "@bitwarden/common/platform/abstractions/task-scheduler.service"; export type ActiveAlarm = { @@ -6,7 +8,8 @@ export type ActiveAlarm = { createInfo: chrome.alarms.AlarmCreateInfo; }; -export interface BrowserTaskSchedulerService extends TaskSchedulerService { - clearAllScheduledTasks(): Promise; - verifyAlarmsState(): Promise; +export abstract class BrowserTaskSchedulerService extends TaskSchedulerService { + activeAlarms$: Observable; + abstract clearAllScheduledTasks(): Promise; + abstract verifyAlarmsState(): Promise; } diff --git a/apps/browser/src/platform/services/browser-task-scheduler.service.spec.ts b/apps/browser/src/platform/services/browser-task-scheduler.service.spec.ts index 05f6bace7d..8a080ba14a 100644 --- a/apps/browser/src/platform/services/browser-task-scheduler.service.spec.ts +++ b/apps/browser/src/platform/services/browser-task-scheduler.service.spec.ts @@ -1,28 +1,35 @@ import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject, Observable } from "rxjs"; -import { TaskIdentifier } from "@bitwarden/common/platform/abstractions/task-scheduler.service"; import { ScheduledTaskNames } from "@bitwarden/common/platform/enums/scheduled-task-name.enum"; import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import { GlobalState, StateProvider } from "@bitwarden/common/platform/state"; import { UserId } from "@bitwarden/common/types/guid"; -import { ActiveAlarm } from "./abstractions/browser-task-scheduler.service"; -import { BrowserTaskSchedulerService } from "./browser-task-scheduler.service"; +import { + ActiveAlarm, + BrowserTaskSchedulerService, +} from "./abstractions/browser-task-scheduler.service"; +import { BrowserTaskSchedulerServiceImplementation } from "./browser-task-scheduler.service"; -let activeAlarms: ActiveAlarm[] = []; -jest.mock("rxjs", () => ({ - firstValueFrom: jest.fn(() => Promise.resolve(activeAlarms)), - map: jest.fn(), - Observable: jest.fn(), -})); +jest.mock("rxjs", () => { + const actualModule = jest.requireActual("rxjs"); + return { + ...actualModule, + firstValueFrom: jest.fn((state$: BehaviorSubject) => state$.value), + }; +}); -// TODO CG - Likely need to rethink how to test this service a bit more carefully. describe("BrowserTaskSchedulerService", () => { + const callback = jest.fn(); + const delayInMinutes = 2; + const userUuid = "user-uuid" as UserId; let activeUserIdMock$: BehaviorSubject; + let activeAlarmsMock$: BehaviorSubject; let logService: MockProxy; let stateProvider: MockProxy; let browserTaskSchedulerService: BrowserTaskSchedulerService; + let activeAlarms: ActiveAlarm[] = []; const eventUploadsIntervalCreateInfo = { periodInMinutes: 5, delayInMinutes: 5 }; const scheduleNextSyncIntervalCreateInfo = { periodInMinutes: 5, delayInMinutes: 5 }; @@ -43,7 +50,8 @@ describe("BrowserTaskSchedulerService", () => { createInfo: { delayInMinutes: 1, periodInMinutes: undefined }, }), ]; - activeUserIdMock$ = new BehaviorSubject("user-uuid" as UserId); + activeAlarmsMock$ = new BehaviorSubject(activeAlarms); + activeUserIdMock$ = new BehaviorSubject(userUuid); logService = mock(); stateProvider = mock({ activeUserId$: activeUserIdMock$, @@ -54,50 +62,27 @@ describe("BrowserTaskSchedulerService", () => { }), ), }); - browserTaskSchedulerService = new BrowserTaskSchedulerService(logService, stateProvider); - jest.spyOn(browserTaskSchedulerService as any, "getAlarm").mockImplementation((alarmName) => { - if (alarmName === ScheduledTaskNames.scheduleNextSyncInterval) { - return Promise.resolve(mock({ name: alarmName })); - } - }); + browserTaskSchedulerService = new BrowserTaskSchedulerServiceImplementation( + logService, + stateProvider, + ); + browserTaskSchedulerService.activeAlarms$ = activeAlarmsMock$; + browserTaskSchedulerService.registerTaskHandler( + ScheduledTaskNames.loginStrategySessionTimeout, + callback, + ); }); afterEach(() => { jest.clearAllMocks(); jest.clearAllTimers(); jest.useRealTimers(); - // eslint-disable-next-line - // @ts-ignore - globalThis.browser = {}; - }); - - describe("verifyAlarmsState", () => { - it("verifies the status of potentially existing alarms referenced from state on initialization", () => { - expect(chrome.alarms.create).toHaveBeenCalledWith( - ScheduledTaskNames.eventUploadsInterval, - eventUploadsIntervalCreateInfo, - expect.any(Function), - ); - }); - - it("skips creating an alarm if the alarm already exists", () => { - expect(chrome.alarms.create).not.toHaveBeenCalledWith( - ScheduledTaskNames.scheduleNextSyncInterval, - scheduleNextSyncIntervalCreateInfo, - expect.any(Function), - ); - }); }); describe("setTimeout", () => { it("uses the global setTimeout API if the delay is less than 1000ms", async () => { - const callback = jest.fn(); const delayInMs = 999; jest.spyOn(globalThis, "setTimeout"); - await browserTaskSchedulerService.registerTaskHandler( - ScheduledTaskNames.loginStrategySessionTimeout, - callback, - ); await browserTaskSchedulerService.setTimeout( ScheduledTaskNames.loginStrategySessionTimeout, @@ -113,299 +98,49 @@ describe("BrowserTaskSchedulerService", () => { }); it("creates a timeout alarm", async () => { - const callback = jest.fn(); - const delayInMinutes = 2; - await browserTaskSchedulerService.registerTaskHandler( - ScheduledTaskNames.loginStrategySessionTimeout, - callback, - ); - await browserTaskSchedulerService.setTimeout( ScheduledTaskNames.loginStrategySessionTimeout, delayInMinutes * 60 * 1000, ); expect(chrome.alarms.create).toHaveBeenCalledWith( - ScheduledTaskNames.loginStrategySessionTimeout, + `${userUuid}__${ScheduledTaskNames.loginStrategySessionTimeout}`, { delayInMinutes }, expect.any(Function), ); }); - // it("skips creating a duplicate timeout alarm", async () => { - // const callback = jest.fn(); - // const delayInMinutes = 2; - // jest.spyOn(browserTaskSchedulerService as any, "getAlarm").mockResolvedValue( - // mock({ - // name: ScheduledTaskNames.loginStrategySessionTimeout, - // }), - // ); - // jest.spyOn(browserTaskSchedulerService, "createAlarm"); - // await browserTaskSchedulerService.registerTaskHandler( - // ScheduledTaskNames.loginStrategySessionTimeout, - // callback, - // ); - // - // await browserTaskSchedulerService.setTimeout( - // ScheduledTaskNames.loginStrategySessionTimeout, - // delayInMinutes * 60 * 1000, - // ); - // - // expect(browserTaskSchedulerService.createAlarm).not.toHaveBeenCalled(); - // }); + it("skips creating a duplicate timeout alarm", async () => { + const mockAlarm = mock(); + chrome.alarms.get = jest.fn().mockImplementation((_name, callback) => callback(mockAlarm)); - // it("logs a warning if a duplicate handler is registered when creating an alarm", () => { - // const callback = jest.fn(); - // const name = ScheduledTaskNames.loginStrategySessionTimeout; - // browserTaskSchedulerService["onAlarmHandlers"][name] = jest.fn(); - // - // browserTaskSchedulerService["registerAlarmHandler"](name, callback); - // - // expect(logService.warning).toHaveBeenCalledWith( - // `Alarm handler for ${name} already exists. Overwriting.`, - // ); - // }); - }); - - describe("setInterval", () => { - it("uses the global setInterval API if the interval is less than 1000ms", async () => { - const callback = jest.fn(); - const intervalInMs = 999; - jest.spyOn(globalThis, "setInterval"); - await browserTaskSchedulerService.registerTaskHandler( + await browserTaskSchedulerService.setTimeout( ScheduledTaskNames.loginStrategySessionTimeout, - callback, + delayInMinutes * 60 * 1000, ); - await browserTaskSchedulerService.setInterval( - ScheduledTaskNames.loginStrategySessionTimeout, - intervalInMs, - ); - - expect(globalThis.setInterval).toHaveBeenCalledWith(expect.any(Function), intervalInMs); - expect(chrome.alarms.create).not.toHaveBeenCalledWith( - ScheduledTaskNames.loginStrategySessionTimeout, - { periodInMinutes: 1, delayInMinutes: 1 }, - expect.any(Function), - ); + expect(chrome.alarms.create).not.toHaveBeenCalled(); }); - it("creates an interval alarm", async () => { - const callback = jest.fn(); - const periodInMinutes = 2; - const initialDelayInMs = 1000; - await browserTaskSchedulerService.registerTaskHandler( - ScheduledTaskNames.loginStrategySessionTimeout, - callback, - ); - - await browserTaskSchedulerService.setInterval( - ScheduledTaskNames.loginStrategySessionTimeout, - periodInMinutes * 60 * 1000, - initialDelayInMs, - ); - - expect(chrome.alarms.create).toHaveBeenCalledWith( - ScheduledTaskNames.loginStrategySessionTimeout, - { periodInMinutes, delayInMinutes: initialDelayInMs / 1000 / 60 }, - expect.any(Function), - ); - }); - }); - - describe("clearScheduledTask", () => { - afterEach(() => { - chrome.alarms.clear = jest.fn().mockImplementation((_name, callback) => callback(true)); - }); - - it("skips clearing the alarm if the alarm name is not provided", async () => { - await browserTaskSchedulerService.clearScheduledTask({ - timeoutId: 1, - intervalId: 2, + it("clears a scheduled alarm if a user-specific alarm for the same task is being registered", async () => { + const mockAlarm = mock({ + name: ScheduledTaskNames.loginStrategySessionTimeout, }); + chrome.alarms.get = jest + .fn() + .mockImplementation((name, callback) => + callback(name === ScheduledTaskNames.loginStrategySessionTimeout ? mockAlarm : undefined), + ); - expect(chrome.alarms.clear).not.toHaveBeenCalled(); - }); - - it("skips deleting the active alarm if the alarm was not cleared", async () => { - const taskIdentifier: TaskIdentifier = { taskName: ScheduledTaskNames.eventUploadsInterval }; - chrome.alarms.clear = jest.fn().mockImplementation((_name, callback) => callback(false)); - jest.spyOn(browserTaskSchedulerService as any, "deleteActiveAlarm"); - - await browserTaskSchedulerService.clearScheduledTask(taskIdentifier); - - expect(browserTaskSchedulerService["deleteActiveAlarm"]).not.toHaveBeenCalled(); - }); - - it("clears a named alarm", async () => { - const taskIdentifier: TaskIdentifier = { taskName: ScheduledTaskNames.eventUploadsInterval }; - jest.spyOn(browserTaskSchedulerService as any, "deleteActiveAlarm"); - - await browserTaskSchedulerService.clearScheduledTask(taskIdentifier); + await browserTaskSchedulerService.setTimeout( + ScheduledTaskNames.loginStrategySessionTimeout, + delayInMinutes * 60 * 1000, + ); expect(chrome.alarms.clear).toHaveBeenCalledWith( - ScheduledTaskNames.eventUploadsInterval, + ScheduledTaskNames.loginStrategySessionTimeout, expect.any(Function), ); - expect(browserTaskSchedulerService["deleteActiveAlarm"]).toHaveBeenCalledWith( - ScheduledTaskNames.eventUploadsInterval, - ); }); }); - - // describe("clearAllScheduledTasks", () => { - // it("clears all scheduled tasks and extension alarms", async () => { - // jest.spyOn(browserTaskSchedulerService, "clearAllAlarms"); - // jest.spyOn(browserTaskSchedulerService as any, "updateActiveAlarms"); - // - // await browserTaskSchedulerService.clearAllScheduledTasks(); - // - // expect(browserTaskSchedulerService.clearAllAlarms).toHaveBeenCalled(); - // expect(browserTaskSchedulerService["updateActiveAlarms"]).toHaveBeenCalledWith([]); - // // expect(browserTaskSchedulerService["onAlarmHandlers"]).toEqual({}); - // expect(browserTaskSchedulerService["recoveredAlarms"].size).toBe(0); - // }); - // }); - - // describe("handleOnAlarm", () => { - // it("triggers the alarm", async () => { - // const alarm = mock({ name: ScheduledTaskNames.eventUploadsInterval }); - // const callback = jest.fn(); - // browserTaskSchedulerService["onAlarmHandlers"][alarm.name] = callback; - // - // await browserTaskSchedulerService["handleOnAlarm"](alarm); - // - // expect(callback).toHaveBeenCalled(); - // }); - // }); - - // describe("clearAlarm", () => { - // it("uses the browser.alarms API if it is available", async () => { - // const alarmName = "alarm-name"; - // globalThis.browser = { - // // eslint-disable-next-line - // // @ts-ignore - // alarms: { - // clear: jest.fn(), - // }, - // }; - // - // await browserTaskSchedulerService.clearAlarm(alarmName); - // - // expect(browser.alarms.clear).toHaveBeenCalledWith(alarmName); - // }); - // - // it("clears the alarm with the provided name", async () => { - // const alarmName = "alarm-name"; - // - // const wasCleared = await browserTaskSchedulerService.clearAlarm(alarmName); - // - // expect(chrome.alarms.clear).toHaveBeenCalledWith(alarmName, expect.any(Function)); - // expect(wasCleared).toBe(true); - // }); - // }); - // - // describe("clearAllAlarms", () => { - // it("uses the browser.alarms API if it is available", async () => { - // globalThis.browser = { - // // eslint-disable-next-line - // // @ts-ignore - // alarms: { - // clearAll: jest.fn(), - // }, - // }; - // - // await browserTaskSchedulerService.clearAllAlarms(); - // - // expect(browser.alarms.clearAll).toHaveBeenCalled(); - // }); - // - // it("clears all alarms", async () => { - // const wasCleared = await browserTaskSchedulerService.clearAllAlarms(); - // - // expect(chrome.alarms.clearAll).toHaveBeenCalledWith(expect.any(Function)); - // expect(wasCleared).toBe(true); - // }); - // }); - // - // describe("createAlarm", () => { - // it("uses the browser.alarms API if it is available", async () => { - // const alarmName = "alarm-name"; - // const alarmInfo = { when: 1000 }; - // globalThis.browser = { - // // eslint-disable-next-line - // // @ts-ignore - // alarms: { - // create: jest.fn(), - // }, - // }; - // - // await browserTaskSchedulerService.createAlarm(alarmName, alarmInfo); - // - // expect(browser.alarms.create).toHaveBeenCalledWith(alarmName, alarmInfo); - // }); - // - // it("creates an alarm", async () => { - // const alarmName = "alarm-name"; - // const alarmInfo = { when: 1000 }; - // - // await browserTaskSchedulerService.createAlarm(alarmName, alarmInfo); - // - // expect(chrome.alarms.create).toHaveBeenCalledWith(alarmName, alarmInfo, expect.any(Function)); - // }); - // }); - // - // describe.skip("getAlarm", () => { - // it("uses the browser.alarms API if it is available", async () => { - // const alarmName = "alarm-name"; - // globalThis.browser = { - // // eslint-disable-next-line - // // @ts-ignore - // alarms: { - // get: jest.fn(), - // }, - // }; - // - // await browserTaskSchedulerService.getAlarm(alarmName); - // - // expect(browser.alarms.get).toHaveBeenCalledWith(alarmName); - // }); - // - // it("gets the alarm by name", async () => { - // const alarmName = "alarm-name"; - // const alarmMock = mock(); - // chrome.alarms.get = jest.fn().mockImplementation((_name, callback) => callback(alarmMock)); - // - // const receivedAlarm = await browserTaskSchedulerService.getAlarm(alarmName); - // - // expect(chrome.alarms.get).toHaveBeenCalledWith(alarmName, expect.any(Function)); - // expect(receivedAlarm).toBe(alarmMock); - // }); - // }); - // - // describe("getAllAlarms", () => { - // it("uses the browser.alarms API if it is available", async () => { - // globalThis.browser = { - // // eslint-disable-next-line - // // @ts-ignore - // alarms: { - // getAll: jest.fn(), - // }, - // }; - // - // await browserTaskSchedulerService.getAllAlarms(); - // - // expect(browser.alarms.getAll).toHaveBeenCalled(); - // }); - // - // it("gets all registered alarms", async () => { - // const alarms = [mock(), mock()]; - // chrome.alarms.getAll = jest.fn().mockImplementation((callback) => callback(alarms)); - // - // const receivedAlarms = await browserTaskSchedulerService.getAllAlarms(); - // - // expect(chrome.alarms.getAll).toHaveBeenCalledWith(expect.any(Function)); - // expect(receivedAlarms).toBe(alarms); - // }); - // }); }); diff --git a/apps/browser/src/platform/services/browser-task-scheduler.service.ts b/apps/browser/src/platform/services/browser-task-scheduler.service.ts index 5edefbe2eb..34e424dc63 100644 --- a/apps/browser/src/platform/services/browser-task-scheduler.service.ts +++ b/apps/browser/src/platform/services/browser-task-scheduler.service.ts @@ -15,16 +15,16 @@ import { BrowserApi } from "../browser/browser-api"; import { ActiveAlarm, - BrowserTaskSchedulerService as BrowserTaskSchedulerServiceInterface, + BrowserTaskSchedulerService, } from "./abstractions/browser-task-scheduler.service"; const ACTIVE_ALARMS = new KeyDefinition(TASK_SCHEDULER_DISK, "activeAlarms", { deserializer: (value: ActiveAlarm[]) => value ?? [], }); -export class BrowserTaskSchedulerService +export class BrowserTaskSchedulerServiceImplementation extends DefaultTaskSchedulerService - implements BrowserTaskSchedulerServiceInterface + implements BrowserTaskSchedulerService { private activeAlarmsState: GlobalState; readonly activeAlarms$: Observable; @@ -57,6 +57,8 @@ export class BrowserTaskSchedulerService return super.setTimeout(taskName, delayInMs); } + this.validateRegisteredTask(taskName); + const alarmName = await this.getActiveUserAlarmName(taskName); await this.scheduleAlarm(alarmName, { delayInMinutes }); } @@ -80,6 +82,8 @@ export class BrowserTaskSchedulerService return super.setInterval(taskName, intervalInMs); } + this.validateRegisteredTask(taskName); + const alarmName = await this.getActiveUserAlarmName(taskName); const initialDelayInMinutes = initialDelayInMs ? initialDelayInMs / 1000 / 60 : undefined; await this.scheduleAlarm(alarmName, { @@ -122,7 +126,7 @@ export class BrowserTaskSchedulerService */ async verifyAlarmsState(): Promise { const currentTime = Date.now(); - const activeAlarms = await firstValueFrom(this.activeAlarms$); + const activeAlarms = await this.getActiveAlarms(); for (const alarm of activeAlarms) { const { alarmName, startTime, createInfo } = alarm; @@ -159,10 +163,6 @@ export class BrowserTaskSchedulerService alarmName: string, createInfo: chrome.alarms.AlarmCreateInfo, ): Promise { - if (!alarmName) { - return; - } - const existingAlarm = await this.getAlarm(alarmName); if (existingAlarm) { this.logService.warning(`Alarm ${alarmName} already exists. Skipping creation.`); @@ -181,6 +181,13 @@ export class BrowserTaskSchedulerService await this.setActiveAlarm(alarmName, createInfo); } + /** + * Gets the active alarms from state. + */ + private async getActiveAlarms(): Promise { + return await firstValueFrom(this.activeAlarms$); + } + /** * Sets an active alarm in state. * @@ -191,7 +198,7 @@ export class BrowserTaskSchedulerService alarmName: string, createInfo: chrome.alarms.AlarmCreateInfo, ): Promise { - const activeAlarms = await firstValueFrom(this.activeAlarms$); + const activeAlarms = await this.getActiveAlarms(); const filteredAlarms = activeAlarms.filter((alarm) => alarm.alarmName !== alarmName); filteredAlarms.push({ alarmName, @@ -207,11 +214,16 @@ export class BrowserTaskSchedulerService * @param alarmName - The name of the active alarm to delete. */ private async deleteActiveAlarm(alarmName: string): Promise { - const activeAlarms = await firstValueFrom(this.activeAlarms$); + const activeAlarms = await this.getActiveAlarms(); const filteredAlarms = activeAlarms.filter((alarm) => alarm.alarmName !== alarmName); await this.updateActiveAlarms(filteredAlarms || []); } + /** + * Clears a scheduled alarm by its name and deletes it from the active alarms state. + * + * @param alarmName - The name of the alarm to clear. + */ private async clearScheduledAlarm(alarmName: string): Promise { const wasCleared = await this.clearAlarm(alarmName); if (wasCleared) { @@ -285,8 +297,20 @@ export class BrowserTaskSchedulerService } } + /** + * Gets the active user id from state. + */ + private async getActiveUserId(): Promise { + return await firstValueFrom(this.stateProvider.activeUserId$); + } + + /** + * Gets the active user alarm name by appending the active user id to the task name. + * + * @param taskName - The task name to append the active user id to. + */ private async getActiveUserAlarmName(taskName: ScheduledTaskName): Promise { - const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$); + const activeUserId = await this.getActiveUserId(); if (!activeUserId) { return taskName; } @@ -294,6 +318,12 @@ export class BrowserTaskSchedulerService return `${activeUserId}__${taskName}`; } + /** + * Parses and returns the task name from an alarm name. If the alarm name + * contains a user id, it will return the task name without the user id. + * + * @param alarmName - The alarm name to parse. + */ private getTaskFromAlarmName(alarmName: string): ScheduledTaskName { const activeUserTask = alarmName.split("__")[1] as ScheduledTaskName; if (activeUserTask) { @@ -310,7 +340,7 @@ export class BrowserTaskSchedulerService * @param alarmName - The name of the alarm to create. */ private async clearAlarm(alarmName: string): Promise { - if (typeof browser !== "undefined" && browser.alarms) { + if (this.isNonChromeEnvironment()) { return browser.alarms.clear(alarmName); } @@ -322,7 +352,7 @@ export class BrowserTaskSchedulerService * that indicates when all alarms have been cleared successfully. */ private clearAllAlarms(): Promise { - if (typeof browser !== "undefined" && browser.alarms) { + if (this.isNonChromeEnvironment()) { return browser.alarms.clearAll(); } @@ -339,7 +369,7 @@ export class BrowserTaskSchedulerService alarmName: string, createInfo: chrome.alarms.AlarmCreateInfo, ): Promise { - if (typeof browser !== "undefined" && browser.alarms) { + if (this.isNonChromeEnvironment()) { return browser.alarms.create(alarmName, createInfo); } @@ -352,10 +382,18 @@ export class BrowserTaskSchedulerService * @param alarmName - The name of the alarm to get. */ private async getAlarm(alarmName: string): Promise { - if (typeof browser !== "undefined" && browser.alarms) { + if (this.isNonChromeEnvironment()) { return browser.alarms.get(alarmName); } return new Promise((resolve) => chrome.alarms.get(alarmName, resolve)); } + + /** + * Checks if the environment is a non-Chrome environment. This is used to determine + * if the browser alarms API should be used in place of the chrome alarms API. + */ + private isNonChromeEnvironment(): boolean { + return typeof browser !== "undefined" && !!browser.alarms; + } } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index a7d631824c..6195203128 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -104,7 +104,7 @@ import { ScriptInjectorService } from "../../platform/services/abstractions/scri import { BrowserEnvironmentService } from "../../platform/services/browser-environment.service"; import BrowserLocalStorageService from "../../platform/services/browser-local-storage.service"; import { BrowserScriptInjectorService } from "../../platform/services/browser-script-injector.service"; -import { BrowserTaskSchedulerService } from "../../platform/services/browser-task-scheduler.service"; +import { BrowserTaskSchedulerServiceImplementation } from "../../platform/services/browser-task-scheduler.service"; import { DefaultBrowserStateService } from "../../platform/services/default-browser-state.service"; import I18nService from "../../platform/services/i18n.service"; import { ForegroundPlatformUtilsService } from "../../platform/services/platform-utils/foreground-platform-utils.service"; @@ -568,10 +568,10 @@ const safeProviders: SafeProvider[] = [ }), safeProvider({ provide: TaskSchedulerService, - useExisting: BrowserTaskSchedulerService, + useExisting: BrowserTaskSchedulerServiceImplementation, }), safeProvider({ - provide: BrowserTaskSchedulerService, + provide: BrowserTaskSchedulerServiceImplementation, deps: [LogService, StateProvider], }), ];