From a634b2a26bb119c7e2feebdff6f8b55302907498 Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:29:53 +0200 Subject: [PATCH] feat(server): give the scheduler a contract with the container (#1827) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scheduler runs outside the Nest container but needs four things inside it: backupService, trekPhotoCache, placePhotoCache and airtrailSync. It reaches all four with a require() or await import() at call time. This settles how that should work before four separate folds each invent their own answer, and wires one consumer as proof. It takes the dependencies from the container rather than growing four more bridges, for a reason that showed up while writing the previous PRs: adding one constructor parameter to a service meant hand-editing five bridge files that rebuild their services with `new`. app.get() has no such cost, and hands back the container singleton instead of a second instance. The scheduler declares what it needs structurally — a `backups` with a createBackup — and never names BackupService. That is not fastidiousness: an import of the class would pull in src/nest/backup → src/services/backupService → src/scheduler, which is the exact cycle the lazy `await import()` in runBackup existed to dodge. With the dependency handed in, the scheduler stops importing backupService entirely and the cycle is gone; backupService still imports the scheduler for VALID_INTERVALS, but now only in one direction. index.ts fills the contract from the app buildApp() already initialised, right before the first cron starts, the way bootstrap.ts hands the /mcp handler its registry. Two smaller things fall out. The Nest BackupService was swallowing createBackup's prefix parameter, so it could only ever produce 'backup' names; it forwards it now. And the scheduler can be started without a container at all — its unit tests do exactly that — so runBackup says so in the log instead of throwing, with a case covering it. The other three consumers stay on their require() calls; they move with their own folds. --- server/src/index.ts | 5 ++++ server/src/nest/backup/backup.service.ts | 2 +- server/src/scheduler.ts | 35 +++++++++++++++++++++--- server/tests/unit/auto-backup.test.ts | 25 ++++++++++++++++- 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/server/src/index.ts b/server/src/index.ts index e1c4c3bc6..91a7764af 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -9,6 +9,7 @@ import fs from 'node:fs'; import http from 'node:http'; import type { INestApplication } from '@nestjs/common'; import { buildApp } from './bootstrap'; +import { BackupService } from './nest/backup/backup.service'; // Create upload and data directories on startup. // Every uploads subdir the app writes to must be listed here (#1762): a dir @@ -87,6 +88,10 @@ const onListen = () => { if (env.demo.enabled && env.app.isProduction) { sLogWarn('SECURITY WARNING: DEMO_MODE is enabled in production!'); } + // The scheduler runs outside the container but needs things inside it. It is + // handed them here, from the app buildApp() already initialised, rather than + // reaching for src/services at call time. + scheduler.setSchedulerDeps({ backups: nestApp.get(BackupService) }); scheduler.start(); scheduler.startTripReminders(); scheduler.startTodoReminders(); diff --git a/server/src/nest/backup/backup.service.ts b/server/src/nest/backup/backup.service.ts index ea9fddfa4..f2c66429c 100644 --- a/server/src/nest/backup/backup.service.ts +++ b/server/src/nest/backup/backup.service.ts @@ -9,7 +9,7 @@ import * as svc from '../../services/backupService'; @Injectable() export class BackupService { listBackups() { return svc.listBackups(); } - createBackup() { return svc.createBackup(); } + createBackup(prefix?: 'backup' | 'auto-backup') { return svc.createBackup(prefix); } restoreFromZip(zipPath: string) { return svc.restoreFromZip(zipPath); } getAutoSettings() { return svc.getAutoSettings(); } updateAutoSettings(body: Record) { return svc.updateAutoSettings(body); } diff --git a/server/src/scheduler.ts b/server/src/scheduler.ts index affdd225f..4b05cb5b2 100644 --- a/server/src/scheduler.ts +++ b/server/src/scheduler.ts @@ -57,6 +57,31 @@ function saveSettings(settings: BackupSettings): void { fs.writeFileSync(settingsFile, JSON.stringify(settings, null, 2)); } +/** + * What the scheduler needs from the Nest container. + * + * The shapes are structural on purpose: the scheduler names the capability it + * wants, never the provider class. Importing BackupService here would drag in + * src/nest/backup → src/services/backupService → src/scheduler and put the + * cycle straight back — the same cycle the lazy `await import()` in runBackup + * existed to dodge. + * + * index.ts fills this in from the container after buildApp(), the way + * bootstrap.ts hands the /mcp handler its registry. Nothing here is resolved at + * import time, so the scheduler stays loadable without a container (which is + * what its unit tests rely on). + */ +export interface SchedulerDeps { + backups: { createBackup(prefix?: 'backup' | 'auto-backup'): Promise<{ filename: string }> }; +} + +let deps: SchedulerDeps | undefined; + +/** Hand the scheduler its container-resolved dependencies. Call after app.init(). */ +export function setSchedulerDeps(next: SchedulerDeps): void { + deps = next; +} + async function runBackup(): Promise { try { // Same archive the admin panel builds, only under the auto-backup name: it @@ -64,10 +89,12 @@ async function runBackup(): Promise { // (the archiver reads its entries during finalize, so a WAL auto-checkpoint // landing in between tears the copy), and it carries .encryption_key and the // plugin trees — without the key a scheduled backup cannot be decrypted on - // another install. Imported lazily because backupService imports this module - // for the schedule settings. - const { createBackup } = await import('./services/backupService'); - const { filename } = await createBackup('auto-backup'); + // another install. + if (!deps) { + logError('Auto-Backup: skipped, the scheduler was started without its container dependencies'); + return; + } + const { filename } = await deps.backups.createBackup('auto-backup'); logInfo(`Auto-Backup created: ${filename}`); } catch (err: unknown) { // createBackup removes its own half-written zip before rethrowing. diff --git a/server/tests/unit/auto-backup.test.ts b/server/tests/unit/auto-backup.test.ts index cdf2c03b2..6994d509b 100644 --- a/server/tests/unit/auto-backup.test.ts +++ b/server/tests/unit/auto-backup.test.ts @@ -62,7 +62,8 @@ vi.mock('../../src/config', () => ({ })); import path from 'node:path'; -import { start } from '../../src/scheduler'; +import { start, setSchedulerDeps } from '../../src/scheduler'; +import { createBackup } from '../../src/services/backupService'; const liveDb = path.join(__dirname, '../../data', 'travel.db'); @@ -82,6 +83,9 @@ function stubArchiver(): Record void> { /** Starts the scheduler with auto-backup enabled and hands back the cron callback. */ function scheduledRun(): () => Promise { fsMock.readFileSync.mockReturnValue(JSON.stringify({ enabled: true, interval: 'daily', keep_days: 7 })); + // The same wiring index.ts does from the container, with the real service + // function behind it — the run below is still the production code path. + setSchedulerDeps({ backups: { createBackup } }); start(); return cronMock.schedule.mock.calls.at(-1)?.[1] as () => Promise; } @@ -154,3 +158,22 @@ describe('auto-backup run', () => { expect(fsMock.readdirSync).not.toHaveBeenCalled(); }); }); + +describe('auto-backup without container dependencies', () => { + it('logs and skips instead of throwing when setSchedulerDeps was never called', async () => { + // A fresh module registry, so the deps this file's other cases install are + // not still sitting in the scheduler's module state. + vi.resetModules(); + const { start: freshStart } = await import('../../src/scheduler'); + + vi.clearAllMocks(); + fsMock.readFileSync.mockReturnValue(JSON.stringify({ enabled: true, interval: 'daily', keep_days: 7 })); + freshStart(); + await (cronMock.schedule.mock.calls.at(-1)?.[1] as () => Promise)(); + + expect(logMock.logError).toHaveBeenCalledWith( + 'Auto-Backup: skipped, the scheduler was started without its container dependencies', + ); + expect(archiverMock).not.toHaveBeenCalled(); + }); +});