review comments

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.changeset/dirty-grapes-vanish.md

@@ -1,5 +1,7 @@
 ---
-'@backstage/backend-defaults': patch
+'@backstage/backend-defaults': minor
 ---
 
-The `DefaultSchedulerService` now accepts `HttpRouterService` and `PluginMetadataService` arguments. If you supply a router, the scheduler will register a REST API for listing and triggering tasks. Please see [the scheduler documentation](https://backstage.io/docs/backend-system/core-services/scheduler) for more details about this API.
+**BREAKING**: The `DefaultSchedulerService` constructor options now requires `RootLifecycleService`, `HttpRouterService`, and `PluginMetadataService` fields.
+
+The scheduler will register a REST API for listing and triggering tasks. Please see [the scheduler documentation](https://backstage.io/docs/backend-system/core-services/scheduler) for more details about this API.

packages/backend-defaults/report-scheduler.api.md

@@ -17,9 +17,9 @@ export class DefaultSchedulerService {
   static create(options: {
     database: DatabaseService;
     logger: LoggerService;
-    rootLifecycle?: RootLifecycleService;
-    httpRouter?: HttpRouterService;
-    pluginMetadata?: PluginMetadataService;
+    rootLifecycle: RootLifecycleService;
+    httpRouter: HttpRouterService;
+    pluginMetadata: PluginMetadataService;
   }): SchedulerService;
 }
 

packages/backend-defaults/src/entrypoints/scheduler/lib/DefaultSchedulerService.test.ts

@@ -19,12 +19,18 @@ import { Duration } from 'luxon';
 import waitForExpect from 'wait-for-expect';
 import { DefaultSchedulerService } from './DefaultSchedulerService';
 import { createTestScopedSignal } from './__testUtils__/createTestScopedSignal';
+import { PluginMetadataService } from '@backstage/backend-plugin-api';
 
 jest.setTimeout(60_000);
 
 describe('TaskScheduler', () => {
   const logger = mockServices.logger.mock();
   const databases = TestDatabases.create();
+  const rootLifecycle = mockServices.rootLifecycle.mock();
+  const httpRouter = mockServices.httpRouter.mock();
+  const pluginMetadata = {
+    getId: () => 'test',
+  } satisfies PluginMetadataService;
   const testScopedSignal = createTestScopedSignal();
 
   it.each(databases.eachSupportedId())(
@@ -33,7 +39,13 @@ describe('TaskScheduler', () => {
       const knex = await databases.init(databaseId);
       const database = mockServices.database({ knex });
 
-      const manager = DefaultSchedulerService.create({ database, logger });
+      const manager = DefaultSchedulerService.create({
+        database,
+        logger,
+        rootLifecycle,
+        httpRouter,
+        pluginMetadata,
+      });
       const fn = jest.fn();
 
       await manager.scheduleTask({
@@ -56,7 +68,13 @@ describe('TaskScheduler', () => {
       const knex = await databases.init(databaseId);
       const database = mockServices.database({ knex });
 
-      const manager = DefaultSchedulerService.create({ database, logger });
+      const manager = DefaultSchedulerService.create({
+        database,
+        logger,
+        rootLifecycle,
+        httpRouter,
+        pluginMetadata,
+      });
       const fn = jest.fn();
 
       await manager.scheduleTask({

packages/backend-defaults/src/entrypoints/scheduler/lib/DefaultSchedulerService.ts

@@ -37,9 +37,9 @@ export class DefaultSchedulerService {
   static create(options: {
     database: DatabaseService;
     logger: LoggerService;
-    rootLifecycle?: RootLifecycleService;
-    httpRouter?: HttpRouterService;
-    pluginMetadata?: PluginMetadataService;
+    rootLifecycle: RootLifecycleService;
+    httpRouter: HttpRouterService;
+    pluginMetadata: PluginMetadataService;
   }): SchedulerService {
     const databaseFactory = once(async () => {
       const knex = await options.database.getClient();
@@ -56,7 +56,7 @@ export class DefaultSchedulerService {
           logger: options.logger,
         });
 
-        options.rootLifecycle?.addShutdownHook(() => abortController.abort());
+        options.rootLifecycle.addShutdownHook(() => abortController.abort());
         janitor.start(abortController.signal);
       }
 
@@ -64,13 +64,13 @@ export class DefaultSchedulerService {
     });
 
     const scheduler = new PluginTaskSchedulerImpl(
-      options.pluginMetadata?.getId() ?? 'unknown',
+      options.pluginMetadata.getId(),
       databaseFactory,
       options.logger,
       options.rootLifecycle,
     );
 
-    options.httpRouter?.use(scheduler.getRouter());
+    options.httpRouter.use(scheduler.getRouter());
 
     return scheduler;
   }

packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.ts

@@ -54,7 +54,7 @@ export class PluginTaskSchedulerImpl implements SchedulerService {
     private readonly pluginId: string,
     private readonly databaseFactory: () => Promise<Knex>,
     private readonly logger: LoggerService,
-    rootLifecycle?: RootLifecycleService,
+    rootLifecycle: RootLifecycleService,
   ) {
     const meter = metrics.getMeter('default');
     this.counter = meter.createCounter('backend_tasks.task.runs.count', {
@@ -76,7 +76,7 @@ export class PluginTaskSchedulerImpl implements SchedulerService {
       },
     );
     this.shutdownInitiated = new Promise(shutdownInitiated => {
-      rootLifecycle?.addShutdownHook(() => shutdownInitiated(true));
+      rootLifecycle.addShutdownHook(() => shutdownInitiated(true));
     });
   }