From 5ce4bff8c624a558631c2fe5509e73bf3f608712 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Mon, 1 Apr 2024 17:43:04 -0500 Subject: [PATCH] [PM-6426] Implementing jest tests for the BrowserTaskSchedulerService --- .../browser/src/background/main.background.ts | 1 + .../browser-task-scheduler.service.spec.ts | 113 +++++++++++++++++- .../browser-task-scheduler.service.ts | 17 ++- 3 files changed, 122 insertions(+), 9 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index a3fef17799..0aafb400a7 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1137,6 +1137,7 @@ export default class MainBackground { userId ??= (await firstValueFrom(this.accountService.activeAccount$))?.id; await this.eventUploadService.uploadEvents(userId as UserId); + await this.taskSchedulerService.clearAllScheduledTasks(); await Promise.all([ this.syncService.setLastSync(new Date(0), userId), 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 2d8222ff00..17ceb3057c 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 @@ -5,6 +5,8 @@ import { ScheduledTaskNames } from "@bitwarden/common/platform/enums/scheduled-t import { ConsoleLogService } from "@bitwarden/common/platform/services/console-log.service"; import { GlobalState, StateProvider } from "@bitwarden/common/platform/state"; +import { BrowserApi } from "../browser/browser-api"; + import { ActiveAlarm } from "./abstractions/browser-task-scheduler.service"; import { BrowserTaskSchedulerService } from "./browser-task-scheduler.service"; @@ -19,9 +21,31 @@ describe("BrowserTaskSchedulerService", () => { let logService: MockProxy; let stateProvider: MockProxy; let browserTaskSchedulerService: BrowserTaskSchedulerService; + const eventUploadsIntervalCreateInfo = { periodInMinutes: 5, delayInMinutes: 5 }; + const scheduleNextSyncIntervalCreateInfo = { periodInMinutes: 5, delayInMinutes: 5 }; beforeEach(() => { - activeAlarms = []; + jest.useFakeTimers(); + jest.spyOn(BrowserApi, "getAlarm").mockImplementation((alarmName) => { + if (alarmName === ScheduledTaskNames.scheduleNextSyncInterval) { + return Promise.resolve(mock({ name: alarmName })); + } + }); + activeAlarms = [ + mock({ + name: ScheduledTaskNames.eventUploadsInterval, + createInfo: eventUploadsIntervalCreateInfo, + }), + mock({ + name: ScheduledTaskNames.scheduleNextSyncInterval, + createInfo: scheduleNextSyncIntervalCreateInfo, + }), + mock({ + name: ScheduledTaskNames.fido2ClientAbortTimeout, + startTime: Date.now() - 60001, + createInfo: { delayInMinutes: 1, periodInMinutes: undefined }, + }), + ]; logService = mock(); stateProvider = mock({ getGlobal: jest.fn(() => @@ -35,6 +59,44 @@ describe("BrowserTaskSchedulerService", () => { afterEach(() => { jest.clearAllMocks(); + jest.clearAllTimers(); + jest.useRealTimers(); + }); + + 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), + ); + }); + + it("adds the alarm name to the set of recovered alarms if the alarm create info indicates it has expired", () => { + expect( + browserTaskSchedulerService["recoveredAlarms"].has( + ScheduledTaskNames.fido2ClientAbortTimeout, + ), + ).toBe(true); + }); + + it("clears the list of recovered alarms after 10 seconds", () => { + jest.advanceTimersByTime(10 * 1000); + + expect( + browserTaskSchedulerService["recoveredAlarms"].has( + ScheduledTaskNames.fido2ClientAbortTimeout, + ), + ).toBe(false); + }); }); describe("setTimeout", () => { @@ -50,7 +112,11 @@ describe("BrowserTaskSchedulerService", () => { ); expect(globalThis.setTimeout).toHaveBeenCalledWith(expect.any(Function), delayInMs); - expect(chrome.alarms.create).not.toHaveBeenCalled(); + expect(chrome.alarms.create).not.toHaveBeenCalledWith( + ScheduledTaskNames.loginStrategySessionTimeout, + { delayInMinutes: 1 }, + expect.any(Function), + ); }); it("triggers a recovered alarm immediately and skips creating the alarm", async () => { @@ -67,7 +133,11 @@ describe("BrowserTaskSchedulerService", () => { ); expect(callback).toHaveBeenCalled(); - expect(chrome.alarms.create).not.toHaveBeenCalled(); + expect(chrome.alarms.create).not.toHaveBeenCalledWith( + ScheduledTaskNames.loginStrategySessionTimeout, + { delayInMinutes: 1 }, + expect.any(Function), + ); }); it("creates a timeout alarm", async () => { @@ -86,6 +156,37 @@ describe("BrowserTaskSchedulerService", () => { expect.any(Function), ); }); + + it("skips creating a duplicate timeout alarm", async () => { + const callback = jest.fn(); + const delayInMinutes = 2; + jest.spyOn(BrowserApi, "getAlarm").mockResolvedValue( + mock({ + name: ScheduledTaskNames.loginStrategySessionTimeout, + }), + ); + jest.spyOn(BrowserApi, "createAlarm"); + + await browserTaskSchedulerService.setTimeout( + callback, + delayInMinutes * 60 * 1000, + ScheduledTaskNames.loginStrategySessionTimeout, + ); + + expect(BrowserApi.createAlarm).not.toHaveBeenCalled(); + }); + + 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", () => { @@ -101,7 +202,11 @@ describe("BrowserTaskSchedulerService", () => { ); expect(globalThis.setInterval).toHaveBeenCalledWith(expect.any(Function), intervalInMs); - expect(chrome.alarms.create).not.toHaveBeenCalled(); + expect(chrome.alarms.create).not.toHaveBeenCalledWith( + ScheduledTaskNames.loginStrategySessionTimeout, + { periodInMinutes: 1, delayInMinutes: 1 }, + expect.any(Function), + ); }); it("triggers a recovered alarm before creating the interval alarm", async () => { 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 b9ede29c7d..a3daca0d5d 100644 --- a/apps/browser/src/platform/services/browser-task-scheduler.service.ts +++ b/apps/browser/src/platform/services/browser-task-scheduler.service.ts @@ -79,10 +79,10 @@ export class BrowserTaskSchedulerService * less than 1 minute, it will use the global setInterval. Otherwise, it will * create a browser extension alarm to handle the interval. * - * @param callback - * @param intervalInMs - * @param taskName - * @param initialDelayInMs + * @param callback - The function to be called at each interval. + * @param intervalInMs - The interval in milliseconds. + * @param taskName - The name of the task, used in defining the alarm. + * @param initialDelayInMs - The initial delay in milliseconds. */ async setInterval( callback: () => void, @@ -107,6 +107,13 @@ export class BrowserTaskSchedulerService }); } + /** + * Clears a scheduled task by its task identifier. If the task identifier + * contains a task name, it will clear the browser extension alarm with that + * name. + * + * @param taskIdentifier - The task identifier containing the task name. + */ async clearScheduledTask(taskIdentifier: TaskIdentifier): Promise { void super.clearScheduledTask(taskIdentifier); @@ -176,7 +183,7 @@ export class BrowserTaskSchedulerService } // 10 seconds after verifying the alarm state, we should treat any newly created alarms as non-recovered alarms. - setTimeout(() => this.recoveredAlarms.clear(), 10 * 1000); + globalThis.setTimeout(() => this.recoveredAlarms.clear(), 10 * 1000); } private async setActiveAlarm(alarm: ActiveAlarm): Promise {