From 6beab4fbd2aecf0899cf52a3d7fd8ec641aaf69b Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Wed, 17 Apr 2024 16:16:13 -0500 Subject: [PATCH] [PM-6426] Attempting to re-work implementation to facilitate userId-spcific alarms --- .../browser-task-scheduler.service.ts | 2 +- .../browser-task-scheduler.service.spec.ts | 14 ++-- .../browser-task-scheduler.service.ts | 79 ++++++++++++------- .../abstractions/task-scheduler.service.ts | 4 +- .../default-task-scheduler.service.ts | 30 +++---- 5 files changed, 72 insertions(+), 57 deletions(-) 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 9a91a0119c..8a1b49f456 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 @@ -2,7 +2,7 @@ import { TaskSchedulerService } from "@bitwarden/common/platform/abstractions/ta import { ScheduledTaskName } from "@bitwarden/common/platform/enums/scheduled-task-name.enum"; export type ActiveAlarm = { - name: ScheduledTaskName; + taskName: ScheduledTaskName; startTime: number; createInfo: chrome.alarms.AlarmCreateInfo; }; 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 42509b6b00..08560e5296 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 @@ -30,15 +30,15 @@ describe("BrowserTaskSchedulerService", () => { jest.useFakeTimers(); activeAlarms = [ mock({ - name: ScheduledTaskNames.eventUploadsInterval, + taskName: ScheduledTaskNames.eventUploadsInterval, createInfo: eventUploadsIntervalCreateInfo, }), mock({ - name: ScheduledTaskNames.scheduleNextSyncInterval, + taskName: ScheduledTaskNames.scheduleNextSyncInterval, createInfo: scheduleNextSyncIntervalCreateInfo, }), mock({ - name: ScheduledTaskNames.fido2ClientAbortTimeout, + taskName: ScheduledTaskNames.fido2ClientAbortTimeout, startTime: Date.now() - 60001, createInfo: { delayInMinutes: 1, periodInMinutes: undefined }, }), @@ -131,7 +131,9 @@ describe("BrowserTaskSchedulerService", () => { }); it("triggers a recovered alarm immediately and skips creating the alarm", async () => { - activeAlarms = [mock({ name: ScheduledTaskNames.loginStrategySessionTimeout })]; + activeAlarms = [ + mock({ taskName: ScheduledTaskNames.loginStrategySessionTimeout }), + ]; browserTaskSchedulerService["recoveredAlarms"].add( ScheduledTaskNames.loginStrategySessionTimeout, ); @@ -234,7 +236,9 @@ describe("BrowserTaskSchedulerService", () => { it("triggers a recovered alarm before creating the interval alarm", async () => { const periodInMinutes = 4; - activeAlarms = [mock({ name: ScheduledTaskNames.loginStrategySessionTimeout })]; + activeAlarms = [ + mock({ taskName: ScheduledTaskNames.loginStrategySessionTimeout }), + ]; browserTaskSchedulerService["recoveredAlarms"].add( ScheduledTaskNames.loginStrategySessionTimeout, ); 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 6152c8a78d..baf7ba891e 100644 --- a/apps/browser/src/platform/services/browser-task-scheduler.service.ts +++ b/apps/browser/src/platform/services/browser-task-scheduler.service.ts @@ -132,21 +132,26 @@ export class BrowserTaskSchedulerService /** * Creates a browser extension alarm with the given name and create info. * - * @param name - The name of the alarm. + * @param taskName - The name of the alarm. * @param createInfo - The alarm create info. */ private async scheduleAlarm( - name: ScheduledTaskName, + taskName: ScheduledTaskName, createInfo: chrome.alarms.AlarmCreateInfo, ): Promise { - const existingAlarm = await this.getAlarm(name); + const existingAlarm = await this.getAlarm(taskName); if (existingAlarm) { - this.logService.warning(`Alarm ${name} already exists. Skipping creation.`); + this.logService.warning(`Alarm ${taskName} already exists. Skipping creation.`); return; } - await this.createAlarm(name, createInfo); - await this.setActiveAlarm({ name, startTime: Date.now(), createInfo }); + await this.createAlarm(taskName, createInfo); + + await this.setActiveAlarm({ + taskName, + startTime: Date.now(), + createInfo, + }); } /** @@ -158,8 +163,8 @@ export class BrowserTaskSchedulerService const activeAlarms = await firstValueFrom(this.activeAlarms$); for (const alarm of activeAlarms) { - const { name, startTime, createInfo } = alarm; - const existingAlarm = await this.getAlarm(name); + const { taskName, startTime, createInfo } = alarm; + const existingAlarm = await this.getAlarm(taskName); if (existingAlarm) { continue; } @@ -170,11 +175,11 @@ export class BrowserTaskSchedulerService createInfo.delayInMinutes && startTime + createInfo.delayInMinutes * 60 * 1000 < currentTime; if (shouldAlarmHaveBeenTriggered || hasSetTimeoutAlarmExceededDelay) { - this.recoveredAlarms.add(name); + this.recoveredAlarms.add(taskName); continue; } - void this.scheduleAlarm(name, createInfo); + void this.scheduleAlarm(taskName, createInfo); } // 10 seconds after verifying the alarm state, we should treat any newly created alarms as non-recovered alarms. @@ -195,11 +200,11 @@ export class BrowserTaskSchedulerService /** * Deletes an active alarm from state. * - * @param name - The name of the active alarm to delete. + * @param taskName - The name of the active alarm to delete. */ - private async deleteActiveAlarm(name: ScheduledTaskName): Promise { + private async deleteActiveAlarm(taskName: ScheduledTaskName): Promise { const activeAlarms = await firstValueFrom(this.activeAlarms$); - const filteredAlarms = activeAlarms?.filter((alarm) => alarm.name !== name); + const filteredAlarms = activeAlarms?.filter((alarm) => alarm.taskName !== taskName); await this.updateActiveAlarms(filteredAlarms || []); } @@ -247,13 +252,17 @@ export class BrowserTaskSchedulerService * Triggers an alarm by calling its handler and * deleting it if it is a one-time alarm. * - * @param name - The name of the alarm to trigger. + * @param alarmName - The name of the alarm to trigger. * @param periodInMinutes - The period in minutes of an interval alarm. */ - protected async triggerTask(name: ScheduledTaskName, periodInMinutes?: number): Promise { - const handler = this.taskHandlers.get(name); + protected async triggerTask( + alarmName: ScheduledTaskName, + periodInMinutes?: number, + ): Promise { + const activeUserAlarmName = await this.getActiveUserAlarmName(alarmName); + const handler = this.taskHandlers.get(activeUserAlarmName); if (!periodInMinutes) { - await this.deleteActiveAlarm(name); + await this.deleteActiveAlarm(alarmName); } if (handler) { @@ -265,14 +274,15 @@ export class BrowserTaskSchedulerService * Clears a new alarm with the given name and create info. Returns a promise * that indicates when the alarm has been cleared successfully. * - * @param alarmName - The name of the alarm to create. + * @param taskName - The name of the alarm to create. */ - clearAlarm(alarmName: string): Promise { + async clearAlarm(taskName: string): Promise { + const activeUserAlarmName = await this.getActiveUserAlarmName(taskName); if (typeof browser !== "undefined" && browser.alarms) { - return browser.alarms.clear(alarmName); + return browser.alarms.clear(activeUserAlarmName); } - return new Promise((resolve) => chrome.alarms.clear(alarmName, resolve)); + return new Promise((resolve) => chrome.alarms.clear(activeUserAlarmName, resolve)); } /** @@ -290,28 +300,30 @@ export class BrowserTaskSchedulerService /** * Creates a new alarm with the given name and create info. * - * @param name - The name of the alarm to create. + * @param taskName - The name of the alarm to create. * @param createInfo - The creation info for the alarm. */ - async createAlarm(name: string, createInfo: chrome.alarms.AlarmCreateInfo): Promise { + async createAlarm(taskName: string, createInfo: chrome.alarms.AlarmCreateInfo): Promise { + const activeUserAlarmName = await this.getActiveUserAlarmName(taskName); if (typeof browser !== "undefined" && browser.alarms) { - return browser.alarms.create(name, createInfo); + return browser.alarms.create(activeUserAlarmName, createInfo); } - return new Promise((resolve) => chrome.alarms.create(name, createInfo, resolve)); + return new Promise((resolve) => chrome.alarms.create(activeUserAlarmName, createInfo, resolve)); } /** * Gets the alarm with the given name. * - * @param alarmName - The name of the alarm to get. + * @param taskName - The name of the alarm to get. */ - getAlarm(alarmName: string): Promise { + async getAlarm(taskName: string): Promise { + const activeUserAlarmName = await this.getActiveUserAlarmName(taskName); if (typeof browser !== "undefined" && browser.alarms) { - return browser.alarms.get(alarmName); + return browser.alarms.get(activeUserAlarmName); } - return new Promise((resolve) => chrome.alarms.get(alarmName, resolve)); + return new Promise((resolve) => chrome.alarms.get(activeUserAlarmName, resolve)); } /** @@ -324,4 +336,13 @@ export class BrowserTaskSchedulerService return new Promise((resolve) => chrome.alarms.getAll(resolve)); } + + protected async getActiveUserAlarmName(taskName: string): Promise { + const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$); + if (!activeUserId) { + return taskName; + } + + return `${activeUserId}_${taskName}`; + } } diff --git a/libs/common/src/platform/abstractions/task-scheduler.service.ts b/libs/common/src/platform/abstractions/task-scheduler.service.ts index 1e68d190f1..fc6d901f28 100644 --- a/libs/common/src/platform/abstractions/task-scheduler.service.ts +++ b/libs/common/src/platform/abstractions/task-scheduler.service.ts @@ -17,9 +17,9 @@ export abstract class TaskSchedulerService { protected stateProvider: StateProvider, ) {} - abstract registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): Promise; + abstract registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): void; - abstract unregisterTaskHandler(taskName: ScheduledTaskName): Promise; + abstract unregisterTaskHandler(taskName: ScheduledTaskName): void; abstract setTimeout( taskName: ScheduledTaskName, diff --git a/libs/common/src/platform/services/default-task-scheduler.service.ts b/libs/common/src/platform/services/default-task-scheduler.service.ts index 75c5db84c7..a77b75ee98 100644 --- a/libs/common/src/platform/services/default-task-scheduler.service.ts +++ b/libs/common/src/platform/services/default-task-scheduler.service.ts @@ -1,5 +1,3 @@ -import { firstValueFrom } from "rxjs"; - import { LogService } from "../abstractions/log.service"; import { TaskIdentifier, TaskSchedulerService } from "../abstractions/task-scheduler.service"; import { ScheduledTaskName } from "../enums/scheduled-task-name.enum"; @@ -12,23 +10,24 @@ export class DefaultTaskSchedulerService extends TaskSchedulerService { this.taskHandlers = new Map(); } - async registerTaskHandler(taskName: ScheduledTaskName, handler: () => void): Promise { - const activeUserTaskName = await this.getActiveUserTaskName(taskName); - const existingHandler = this.taskHandlers.get(activeUserTaskName); + registerTaskHandler(taskName: ScheduledTaskName, handler: () => void) { + const existingHandler = this.taskHandlers.get(taskName); if (existingHandler) { this.logService.warning(`Task handler for ${taskName} already exists. Overwriting.`); - await this.unregisterTaskHandler(taskName); + this.unregisterTaskHandler(taskName); } - this.taskHandlers.set(activeUserTaskName, handler); + this.taskHandlers.set(taskName, handler); } - async unregisterTaskHandler(taskName: ScheduledTaskName): Promise { - const activeUserTaskName = await this.getActiveUserTaskName(taskName); - this.taskHandlers.delete(activeUserTaskName); + unregisterTaskHandler(taskName: ScheduledTaskName) { + this.taskHandlers.delete(taskName); } - protected triggerTask(taskName: ScheduledTaskName, _periodInMinutes?: number): void { + protected async triggerTask( + taskName: ScheduledTaskName, + _periodInMinutes?: number, + ): Promise { const handler = this.taskHandlers.get(taskName); if (handler) { handler(); @@ -77,13 +76,4 @@ export class DefaultTaskSchedulerService extends TaskSchedulerService { globalThis.clearInterval(taskIdentifier.intervalId); } } - - private async getActiveUserTaskName(taskName: ScheduledTaskName): Promise { - const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$); - if (!activeUserId) { - return taskName; - } - - return `${activeUserId}_${taskName}`; - } }