From 6db91c8bf3c92c40ea846d298e035123dde2497c Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 22 Sep 2026 14:06:57 +0200 Subject: [PATCH] refactor(addons): addon catalog and photo providers through repositories --- .../src/db/repositories/Addons.repository.ts | 64 +- .../PhotoProviderFields.repository.ts | 34 +- .../repositories/PhotoProviders.repository.ts | 27 +- server/src/nest/addons/addons.module.ts | 11 + server/src/nest/addons/addons.service.ts | 165 ++--- .../nest/platform/mcp-metadata.middleware.ts | 21 +- server/tests/e2e/addons.e2e.test.ts | 12 +- server/tests/e2e/system-notices.e2e.test.ts | 7 +- server/tests/helpers/mcp-test-controllers.ts | 10 +- server/tests/helpers/plugin-host.ts | 12 +- server/tests/helpers/test-addons.ts | 67 ++ server/tests/integration/mcp.test.ts | 4 +- .../tests/integration/oauth-authorize.test.ts | 4 +- server/tests/integration/oauth.test.ts | 4 +- .../plugins/boot-registry-order.test.ts | 5 +- .../db/repositories/Addons.repository.test.ts | 145 ++++ .../PhotoProviderFields.repository.test.ts | 111 ++++ .../PhotoProviders.repository.test.ts | 81 +++ .../tests/unit/mcp/tools-help-addons.test.ts | 5 +- server/tests/unit/nest/addons.service.test.ts | 627 +++++++++--------- server/tests/unit/nest/admin.service.test.ts | 4 +- .../nest/admin.version-notification.test.ts | 4 +- .../unit/nest/dawarich-sync.service.test.ts | 6 +- .../unit/nest/discovery.controller.test.ts | 24 +- .../nest/unified-memories.service.test.ts | 4 +- .../unit/plugins/operator-egress.test.ts | 4 +- .../plugins/plugin-instance-settings.test.ts | 12 +- .../unit/plugins/plugin-user-settings.test.ts | 14 +- .../unit/plugins/plugins-service.test.ts | 49 +- .../unit/plugins/request-context.test.ts | 4 +- .../unit/plugins/settings-isolation.test.ts | 4 +- 31 files changed, 1083 insertions(+), 462 deletions(-) create mode 100644 server/tests/helpers/test-addons.ts create mode 100644 server/tests/unit/db/repositories/Addons.repository.test.ts create mode 100644 server/tests/unit/db/repositories/PhotoProviderFields.repository.test.ts create mode 100644 server/tests/unit/db/repositories/PhotoProviders.repository.test.ts diff --git a/server/src/db/repositories/Addons.repository.ts b/server/src/db/repositories/Addons.repository.ts index 8ca0d698d..0987ac1d7 100644 --- a/server/src/db/repositories/Addons.repository.ts +++ b/server/src/db/repositories/Addons.repository.ts @@ -1,4 +1,64 @@ -import type { Addons } from '../entities/Addons.entity'; +import type { Addons, AddonConfig } from '../entities/Addons.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { EntityRepository } from '@mikro-orm/sql'; -export class AddonsRepository extends EntityRepository {} +/** An `addons` row as the API emits it. */ +export interface AddonRow { + id: string | null; + name: string; + description: string | null; + type: string; + icon: string | null; + enabled: boolean | null; + config: AddonConfig | null; + sort_order: number | null; +} + +const _addonRowKeys: AssertRowKeys = true; + +export class AddonsRepository extends EntityRepository { + /** + * `SELECT enabled FROM addons WHERE id = ?` + * + * `refresh: true` (Task 0 review, I1 — carried forward for every PK read in + * this plan): a primary-key `findOne` answers a repeat call from the + * identity map, invisible to a write on the same id inside the same + * request (`setEnabled`, below, or `admin.service.ts`'s still-raw + * `UPDATE addons SET enabled = ?` on the same connection). + */ + async isEnabled(id: string): Promise { + const row = await this.findOne({ id }, { fields: ['enabled'], refresh: true }); + return !!row?.enabled; + } + + /** + * `SELECT id, name, type, icon, enabled FROM addons WHERE enabled = 1 + * ORDER BY sort_order` — this reads every scalar column (not just the + * legacy statement's five), because `AddonsService.list()` already + * explicitly picks the five fields it forwards to the client; narrowing the + * query itself would need a second, `AssertRowKeys`-incompatible row type + * for no parity benefit. + */ + async listEnabled(): Promise { + const rows = await this.find({ enabled: true }, { orderBy: { sort_order: 'asc' } }); + return rows.map((row) => toRow(row) as AddonRow); + } + + /** + * `UPDATE addons SET enabled = ? WHERE id = ?` — the write half of the + * enablement toggle `admin.service.ts` still issues as raw SQL + * (`updateAddon`, outside this plan's six domains). Declared here, on the + * repository that owns the `addons` table, so that conversion has a + * ready-made method rather than inventing its own; nothing in this domain + * calls it yet. + * + * `enabled` is `p.boolean()` on the entity, so `nativeUpdate` stores the + * legacy `1`/`0` through MikroORM's own boolean type mapper — the + * repository test asserts the raw stored value directly, and that a + * following read (with `refresh: true`) comes back as a JS boolean, not + * the raw int. + */ + async setEnabled(id: string, enabled: boolean): Promise { + await this.nativeUpdate({ id }, { enabled }); + } +} diff --git a/server/src/db/repositories/PhotoProviderFields.repository.ts b/server/src/db/repositories/PhotoProviderFields.repository.ts index 5e0467960..87c55b52c 100644 --- a/server/src/db/repositories/PhotoProviderFields.repository.ts +++ b/server/src/db/repositories/PhotoProviderFields.repository.ts @@ -1,4 +1,36 @@ import type { PhotoProviderFields } from '../entities/PhotoProviderFields.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { EntityRepository } from '@mikro-orm/sql'; -export class PhotoProviderFieldsRepository extends EntityRepository {} +/** A `photo_provider_fields` row as the API emits it. */ +export interface PhotoProviderFieldRow { + id: number; + provider_id: string; + field_key: string; + label: string; + input_type: string; + placeholder: string | null; + hint: string | null; + required: number | null; + secret: number | null; + settings_key: string | null; + payload_key: string | null; + sort_order: number | null; +} + +const _photoProviderFieldRowKeys: AssertRowKeys = true; + +export class PhotoProviderFieldsRepository extends EntityRepository { + /** + * `SELECT provider_id, field_key, label, input_type, placeholder, hint, + * required, secret, settings_key, payload_key, sort_order FROM + * photo_provider_fields ORDER BY sort_order, id` — `provider_id` is the + * `persist(false)` twin of the `provider` relation (Days.trip_id + * precedent): reading it back after `find()` needs no extra populate, only + * a `create()`/twin write would. + */ + async listAllOrdered(): Promise { + const rows = await this.find({}, { orderBy: { sort_order: 'asc', id: 'asc' } }); + return rows.map((row) => toRow(row) as PhotoProviderFieldRow); + } +} diff --git a/server/src/db/repositories/PhotoProviders.repository.ts b/server/src/db/repositories/PhotoProviders.repository.ts index 8a800814f..2b129eb73 100644 --- a/server/src/db/repositories/PhotoProviders.repository.ts +++ b/server/src/db/repositories/PhotoProviders.repository.ts @@ -1,4 +1,29 @@ import type { PhotoProviders } from '../entities/PhotoProviders.entity'; +import { toRow, type AssertRowKeys } from './_shared/rows'; import { EntityRepository } from '@mikro-orm/sql'; -export class PhotoProvidersRepository extends EntityRepository {} +/** A `photo_providers` row as the API emits it. */ +export interface PhotoProviderRow { + id: string | null; + name: string; + description: string | null; + icon: string | null; + enabled: number | null; + sort_order: number | null; +} + +const _photoProviderRowKeys: AssertRowKeys = true; + +export class PhotoProvidersRepository extends EntityRepository { + /** + * `SELECT id, name, icon, enabled, sort_order FROM photo_providers WHERE + * enabled = 1 ORDER BY sort_order, id` — `enabled` is `p.integer()` on this + * entity (unlike `Addons.enabled`, which is `p.boolean()`), so it comes + * back as the raw stored `0`/`1`; `AddonsService.list()` does the `!!` + * coercion itself, matching the legacy `!!p.enabled`. + */ + async listEnabled(): Promise { + const rows = await this.find({ enabled: 1 }, { orderBy: { sort_order: 'asc', id: 'asc' } }); + return rows.map((row) => toRow(row) as PhotoProviderRow); + } +} diff --git a/server/src/nest/addons/addons.module.ts b/server/src/nest/addons/addons.module.ts index 28eb2dd4f..960298dde 100644 --- a/server/src/nest/addons/addons.module.ts +++ b/server/src/nest/addons/addons.module.ts @@ -1,8 +1,13 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { AddonsController } from './addons.controller'; import { AddonsService } from './addons.service'; import { AddonGuard } from './addon.guard'; import { AddonsMcp } from './addons.mcp'; +import { Addons } from '../../db/entities/Addons.entity'; +import { PhotoProviders } from '../../db/entities/PhotoProviders.entity'; +import { PhotoProviderFields } from '../../db/entities/PhotoProviderFields.entity'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; /** * GET /api/addons — enabled add-ons + photo providers (was an inline handler in @@ -18,8 +23,14 @@ import { AddonsMcp } from './addons.mcp'; * AddonsMcp puts the same listing on the MCP surface, so a model can read which * add-ons this instance has switched on instead of guessing from the tools it * was handed. + * + * MikroOrmModule.forFeature registers AddonsRepository/PhotoProvidersRepository/ + * PhotoProviderFieldsRepository/AppSettingsRepository for AddonsService's + * @InjectRepository constructor (Plan 3a Task 4) — the forFeature + + * @InjectRepository wiring pattern Task 0 set up on SettingsModule. */ @Module({ + imports: [MikroOrmModule.forFeature([Addons, PhotoProviders, PhotoProviderFields, AppSettings])], controllers: [AddonsController], providers: [AddonsService, AddonGuard, AddonsMcp], exports: [AddonsService, AddonGuard], diff --git a/server/src/nest/addons/addons.service.ts b/server/src/nest/addons/addons.service.ts index 1376907e0..c97c8786f 100644 --- a/server/src/nest/addons/addons.service.ts +++ b/server/src/nest/addons/addons.service.ts @@ -1,16 +1,24 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { ADDON_IDS } from '../../addons'; import { DatabaseService } from '../database/database.service'; -import type { Addon } from '../../types'; import { getPhotoProviderConfig } from '../memories/memories.helpers'; import { readTransitProvider, writeTransitProvider } from '../transit/transit-provider'; import { resolveApiKey, type ApiKeySource } from '../settings/instance-api-keys'; import { readEnv } from '../../app-config'; import type { TransitProvider } from '@trek/shared'; +import { Addons } from '../../db/entities/Addons.entity'; +import type { AddonsRepository } from '../../db/repositories/Addons.repository'; +import { PhotoProviders } from '../../db/entities/PhotoProviders.entity'; +import type { PhotoProvidersRepository, PhotoProviderRow } from '../../db/repositories/PhotoProviders.repository'; +import { PhotoProviderFields } from '../../db/entities/PhotoProviderFields.entity'; +import type { PhotoProviderFieldsRepository, PhotoProviderFieldRow } from '../../db/repositories/PhotoProviderFields.repository'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; +import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; /** * Thin wrapper around the enabled-addons + photo-provider read that the legacy - * inline `GET /api/addons` handler performed (server/src/app.ts). The SQL, + * inline `GET /api/addons` handler performed (server/src/app.ts). The * ordering, boolean coercions and the merged photo-provider entries are * reproduced 1:1 so the body is byte-identical for the client. * @@ -19,54 +27,77 @@ import type { TransitProvider } from '@trek/shared'; * bag-tracking and collab-features flags. The boolean polarities differ on * purpose — bag tracking is opt-in (`=== 'true'`, default OFF), the collab * sub-features are opt-out (`!== 'false'`, default ON). Reads are uncached - * per-call queries so admin toggles stay immediately visible. + * per-call repository calls so admin toggles stay immediately visible. + * + * `DatabaseService` stays injected (Plan 3a Task 4) purely as a passthrough + * for two cross-domain functions this service still calls: `transit-provider.ts`'s + * readTransitProvider/writeTransitProvider (nest/transit, not one of this + * plan's six domains) and `instance-api-keys.ts`'s resolveApiKey (nest/settings, + * Task 5's own conversion). Neither reads/writes through `this.db` in THIS + * file — every `app_settings`/`addons`/`photo_providers`/`photo_provider_fields` + * site this service itself used to touch is repository-backed below — so this + * is the same kind of carve-out `permissions`' two guard delegations are: `grep + * DatabaseService src/nest/addons` still finds this constructor parameter and + * its import, not a raw query. */ @Injectable() export class AddonsService { - constructor(private readonly dbs: DatabaseService) {} - - private get db() { - return this.dbs.connection; - } + constructor( + @InjectRepository(Addons) private readonly addons: AddonsRepository, + @InjectRepository(PhotoProviders) private readonly photoProviders: PhotoProvidersRepository, + @InjectRepository(PhotoProviderFields) private readonly photoProviderFields: PhotoProviderFieldsRepository, + @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository, + private readonly dbs: DatabaseService, + ) {} async isAddonEnabled(addonId: string): Promise { - const addon = this.db.prepare('SELECT enabled FROM addons WHERE id = ?').get(addonId) as - | { enabled: number } - | undefined; - return !!addon?.enabled; + return this.addons.isEnabled(addonId); } async getBagTracking() { - const row = this.db.prepare("SELECT value FROM app_settings WHERE key = 'bag_tracking_enabled'").get() as - | { value: string } - | undefined; - return { enabled: row?.value === 'true' }; + const value = await this.appSettings.getValue('bag_tracking_enabled'); + return { enabled: value === 'true' }; } async updateBagTracking(enabled: boolean) { - this.db.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('bag_tracking_enabled', ?)").run( - enabled ? 'true' : 'false', - ); + await this.appSettings.setValue('bag_tracking_enabled', enabled ? 'true' : 'false'); return { enabled: !!enabled }; } + /** + * `AppSettingsRepository.getValues` drops a key from the returned `Map` + * both when no row exists AND when the row's `value` is `NULL` (Task 0 + * concern #4) — this method's polarity (`!== 'false'`, fail-open) makes + * that safe: a dropped key reads `map.get(key) === undefined`, and + * `undefined !== 'false'` is `true`, the exact same outcome the legacy + * `Record` produced for a `NULL` value (`null !== 'false'` + * is also `true`). Confirmed, not assumed — see the repository test + * ADDONSREPO/APPSETREPO parity note and this service's own test file. + */ async getCollabFeatures() { - const rows = this.db - .prepare( - "SELECT key, value FROM app_settings WHERE key IN ('collab_chat_enabled', 'collab_notes_enabled', 'collab_links_enabled', 'collab_polls_enabled', 'collab_whatsnext_enabled')", - ) - .all() as { key: string; value: string }[]; - const map: Record = {}; - for (const r of rows) map[r.key] = r.value; + const map = await this.appSettings.getValues([ + 'collab_chat_enabled', + 'collab_notes_enabled', + 'collab_links_enabled', + 'collab_polls_enabled', + 'collab_whatsnext_enabled', + ]); return { - chat: map['collab_chat_enabled'] !== 'false', - notes: map['collab_notes_enabled'] !== 'false', - links: map['collab_links_enabled'] !== 'false', - polls: map['collab_polls_enabled'] !== 'false', - whatsnext: map['collab_whatsnext_enabled'] !== 'false', + chat: map.get('collab_chat_enabled') !== 'false', + notes: map.get('collab_notes_enabled') !== 'false', + links: map.get('collab_links_enabled') !== 'false', + polls: map.get('collab_polls_enabled') !== 'false', + whatsnext: map.get('collab_whatsnext_enabled') !== 'false', }; } + /** + * Ruling (plan §"Do not fix legacy behaviour"): the legacy handler wrote up + * to 5 `app_settings` rows one at a time, with no transaction around them — + * this stays exactly that way. `AppSettingsRepository.setValue` imposes no + * transaction of its own (Task 0 concern #5), so the loop below is still 0 + * to 5 independent, individually-committed upserts. + */ async updateCollabFeatures(features: { chat?: boolean; notes?: boolean; links?: boolean; polls?: boolean; whatsnext?: boolean }) { const mapping: Record = { chat: 'collab_chat_enabled', @@ -76,9 +107,9 @@ export class AddonsService { whatsnext: 'collab_whatsnext_enabled', }; const before = await this.getCollabFeatures(); - const stmt = this.db.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)'); for (const [feat, key] of Object.entries(mapping)) { - if (features[feat] !== undefined) stmt.run(key, features[feat] ? 'true' : 'false'); + const value = features[feat as keyof typeof features]; + if (value !== undefined) await this.appSettings.setValue(key, value ? 'true' : 'false'); } const after = await this.getCollabFeatures(); // Collab flags gate MCP tool/resource registration, so callers must know @@ -89,46 +120,19 @@ export class AddonsService { } async list() { - const addons = this.db - .prepare('SELECT id, name, type, icon, enabled FROM addons WHERE enabled = 1 ORDER BY sort_order') - .all() as Pick[]; + const addonRows = await this.addons.listEnabled(); // Photo providers surface only inside journeys, so with the journey addon // off they are unavailable no matter what their own rows say. Deriving that // here (instead of a migration) also covers installs that still hold an // enabled provider under a disabled journey from before updateAddon // cascaded the disable. - const providers = !(await this.isAddonEnabled(ADDON_IDS.JOURNEY)) + const providerRows: PhotoProviderRow[] = !(await this.isAddonEnabled(ADDON_IDS.JOURNEY)) ? [] - : (this.db - .prepare( - `SELECT id, name, icon, enabled, sort_order - FROM photo_providers - WHERE enabled = 1 - ORDER BY sort_order, id`, - ) - .all() as Array<{ id: string; name: string; icon: string; enabled: number; sort_order: number }>); - const fields = this.db - .prepare( - `SELECT provider_id, field_key, label, input_type, placeholder, hint, required, secret, settings_key, payload_key, sort_order - FROM photo_provider_fields - ORDER BY sort_order, id`, - ) - .all() as Array<{ - provider_id: string; - field_key: string; - label: string; - input_type: string; - placeholder?: string | null; - hint?: string | null; - required: number; - secret: number; - settings_key?: string | null; - payload_key?: string | null; - sort_order: number; - }>; + : await this.photoProviders.listEnabled(); + const fieldRows: PhotoProviderFieldRow[] = await this.photoProviderFields.listAllOrdered(); - const fieldsByProvider = new Map(); - for (const field of fields) { + const fieldsByProvider = new Map(); + for (const field of fieldRows) { const arr = fieldsByProvider.get(field.provider_id) || []; arr.push(field); fieldsByProvider.set(field.provider_id, arr); @@ -138,15 +142,19 @@ export class AddonsService { collabFeatures: await this.getCollabFeatures(), bagTracking: (await this.getBagTracking()).enabled, addons: [ - ...addons.map((a) => ({ ...a, enabled: !!a.enabled })), - ...providers.map((p) => ({ + // The repository row carries every scalar column (description, config, + // sort_order included); the legacy statement selected only these five, + // so the client-facing shape is picked explicitly here rather than + // spread from the row. + ...addonRows.map((a) => ({ id: a.id, name: a.name, type: a.type, icon: a.icon, enabled: !!a.enabled })), + ...providerRows.map((p) => ({ id: p.id, name: p.name, type: 'photo_provider', icon: p.icon, enabled: !!p.enabled, - config: getPhotoProviderConfig(p.id), - fields: (fieldsByProvider.get(p.id) || []).map((f) => ({ + config: getPhotoProviderConfig(p.id ?? ''), + fields: (fieldsByProvider.get(p.id ?? '') || []).map((f) => ({ key: f.field_key, label: f.label, input_type: f.input_type, @@ -172,12 +180,12 @@ export class AddonsService { // loses a feature on upgrade. private async readFlag(key: string) { - const row = this.db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined; - return { enabled: row?.value === 'true' }; + const value = await this.appSettings.getValue(key); + return { enabled: value === 'true' }; } private async writeFlag(key: string, enabled: boolean) { - this.db.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(key, enabled ? 'true' : 'false'); + await this.appSettings.setValue(key, enabled ? 'true' : 'false'); return { enabled: !!enabled }; } @@ -212,10 +220,8 @@ export class AddonsService { * "off" while the feature runs, which is worse than either default. */ async getPlacesEnrich() { - const row = this.db.prepare("SELECT value FROM app_settings WHERE key = 'places_enrich_enabled'").get() as - | { value: string } - | undefined; - return { enabled: row?.value !== 'false' }; + const value = await this.appSettings.getValue('places_enrich_enabled'); + return { enabled: value !== 'false' }; } async updatePlacesEnrich(enabled: boolean) { return this.writeFlag('places_enrich_enabled', enabled); } @@ -224,7 +230,10 @@ export class AddonsService { // Not a flag: two named backends, so it stores the name rather than a // boolean. The read/write pair lives in transit/transit-provider.ts because // TransitService reads the same row on every request — one key, one reader, - // one writer. + // one writer. Neither that module nor instance-api-keys.ts (below) is one of + // this plan's six domains, so they stay on DatabaseService until their own + // conversion (transit is outside Plan 3a entirely; instance-api-keys.ts is + // Task 5's). /** * Where the Google key would come from for this caller, or null if nowhere. diff --git a/server/src/nest/platform/mcp-metadata.middleware.ts b/server/src/nest/platform/mcp-metadata.middleware.ts index a9e77b0e5..4a164ba0b 100644 --- a/server/src/nest/platform/mcp-metadata.middleware.ts +++ b/server/src/nest/platform/mcp-metadata.middleware.ts @@ -1,7 +1,9 @@ import type { RequestHandler } from 'express'; +import { MikroORM } from '@mikro-orm/core'; import { DiscoveryMetadataService } from './discovery-metadata.service'; import { AddonsService } from '../addons/addons.service'; import { ADDON_IDS } from '../../addons'; +import { withRequestContext } from '../database/request-context'; /** * The SDK discovery router plus its addon gate: 404 (empty body) on every @@ -16,29 +18,42 @@ import { ADDON_IDS } from '../../addons'; * from the container by token (the httpConfig.KEY precedent). A factory * provider rather than a NestMiddleware class because nothing consumer-mounts * it: the deliverable is the bare Express RequestHandler itself. + * + * D6 boot rule (Plan 3a Task 4): a pathless pre-init `app.use` runs on the raw + * Express instance, before `@mikro-orm/nestjs`'s per-request EntityManager + * fork middleware — unlike `createMcpAddonGate`'s OTHER mount in + * `oauth.module.ts`, which goes through a normal `MiddlewareConsumer.forRoutes()` + * and so is already inside that fork. Once `AddonsService.isAddonEnabled` + * became repository-backed, a request here threw MikroORM's + * `cannotUseGlobalContext` (proven: every `/.well-known/*` integration test + * failed with a 500 until this wrap). `withRequestContext` is this file's own + * choke point — every request through this middleware, gated or not, gets a + * context the same way the boot-time `getAdminUserDefaults()` read in + * `bootstrap.ts` already does. */ export const MCP_METADATA_MIDDLEWARE = Symbol('MCP_METADATA_MIDDLEWARE'); export function createMcpMetadataMiddleware( meta: DiscoveryMetadataService, addons: AddonsService, + orm: MikroORM, ): RequestHandler { // Express cannot await a middleware, so the now-async addon check runs in a // helper and its rejection is handed to next() — the same error path a // synchronous throw took before (recipe R1.5). return (req, res, next) => { - void (async () => { + void withRequestContext(orm, async () => { if (req.path.startsWith('/.well-known/') && !(await addons.isAddonEnabled(ADDON_IDS.MCP))) { res.status(404).end(); return; } meta.getMetaRouter()(req, res, next); - })().catch(next); + }).catch(next); }; } export const mcpMetadataMiddlewareProvider = { provide: MCP_METADATA_MIDDLEWARE, useFactory: createMcpMetadataMiddleware, - inject: [DiscoveryMetadataService, AddonsService], + inject: [DiscoveryMetadataService, AddonsService, MikroORM], }; diff --git a/server/tests/e2e/addons.e2e.test.ts b/server/tests/e2e/addons.e2e.test.ts index 1c848a5f9..badc2b476 100644 --- a/server/tests/e2e/addons.e2e.test.ts +++ b/server/tests/e2e/addons.e2e.test.ts @@ -20,8 +20,13 @@ const { db } = vi.hoisted(() => { tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); - tmp.exec(`CREATE TABLE addons (id TEXT PRIMARY KEY, name TEXT, type TEXT, icon TEXT, enabled INTEGER, sort_order INTEGER);`); - tmp.exec(`CREATE TABLE photo_providers (id TEXT PRIMARY KEY, name TEXT, icon TEXT, enabled INTEGER, sort_order INTEGER);`); + // description/config (addons) and description (photo_providers) are absent + // from the legacy SELECTs this suite exercises, but the Addons/PhotoProviders + // entities declare them as columns (Plan 3a Task 4) — MikroORM's generated + // SELECT names every scalar column, so they must exist on this hand-rolled + // schema even though no test here reads them off addons/photo_providers rows. + tmp.exec(`CREATE TABLE addons (id TEXT PRIMARY KEY, name TEXT, description TEXT, type TEXT, icon TEXT, enabled INTEGER, config TEXT DEFAULT '{}', sort_order INTEGER);`); + tmp.exec(`CREATE TABLE photo_providers (id TEXT PRIMARY KEY, name TEXT, description TEXT, icon TEXT, enabled INTEGER, sort_order INTEGER);`); tmp.exec(`CREATE TABLE photo_provider_fields (id INTEGER PRIMARY KEY AUTOINCREMENT, provider_id TEXT, field_key TEXT, label TEXT, input_type TEXT, placeholder TEXT, hint TEXT, required INTEGER, secret INTEGER, settings_key TEXT, payload_key TEXT, sort_order INTEGER);`); @@ -41,13 +46,14 @@ vi.mock('../../src/nest/memories/memories.helpers', () => ({ getPhotoProviderCon import { AddonsModule } from '../../src/nest/addons/addons.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; import { TestUnitOfWorkModule } from '../helpers/test-uow'; +import { createTestMikroOrmModule } from '../helpers/test-orm'; describe('GET /api/addons e2e (real auth guard + temp SQLite)', () => { let server: Server; let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [await TestUnitOfWorkModule.forRoot(db), DatabaseModule, AddonsModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [await TestUnitOfWorkModule.forRoot(db), await createTestMikroOrmModule(db), DatabaseModule, AddonsModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); diff --git a/server/tests/e2e/system-notices.e2e.test.ts b/server/tests/e2e/system-notices.e2e.test.ts index 991e2933d..3269f2309 100644 --- a/server/tests/e2e/system-notices.e2e.test.ts +++ b/server/tests/e2e/system-notices.e2e.test.ts @@ -34,6 +34,7 @@ import { SystemNoticesModule } from '../../src/nest/system-notices/system-notice import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; import { TestUnitOfWorkModule } from '../helpers/test-uow'; +import { createTestMikroOrmModule } from '../helpers/test-orm'; const notice = { id: 'welcome', display: 'modal', severity: 'info', @@ -47,8 +48,10 @@ describe('System-notices e2e (real auth guard + temp SQLite)', () => { async function build() { // DatabaseModule is @Global in the real app; a partial graph has to // provide it for SystemNoticesModule's AddonsModule import (the - // addons.e2e precedent). - const moduleRef = await Test.createTestingModule({ imports: [await TestUnitOfWorkModule.forRoot(db), DatabaseModule, SystemNoticesModule] }).compile(); + // addons.e2e precedent). createTestMikroOrmModule for the same import's + // MikroOrmModule.forFeature (Plan 3a Task 4) — no case here calls an + // AddonsService method, so the minimal `users`-only schema above is enough. + const moduleRef = await Test.createTestingModule({ imports: [await TestUnitOfWorkModule.forRoot(db), await createTestMikroOrmModule(db), DatabaseModule, SystemNoticesModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 9272fadc1..92e1f87b3 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -81,7 +81,7 @@ import { TripPromptsMcp } from '../../src/nest/trips/trip-prompts.mcp'; import { PlacePhotoCacheService } from '../../src/nest/place-photos/place-photo-cache.service'; import { RuntimeEnvService } from '../../src/nest/app-config/runtime-env.service'; import { makeNotificationsService, makeNotificationPreferencesService } from './notifications'; -import { AddonsService } from '../../src/nest/addons/addons.service'; +import { createTestAddonsService } from './test-addons'; import { RoadtripMcp } from '../../src/nest/roadtrip/roadtrip.mcp'; import { RoadtripService } from '../../src/nest/roadtrip/roadtrip.service'; import { notificationsStub } from './notifications'; @@ -114,7 +114,7 @@ import { AirtrailImportService } from '../../src/nest/integrations/airtrail-impo import { ReservationImportMcp } from '../../src/nest/reservation-import/reservation-import.mcp'; import { HelpMcp } from '../../src/nest/help/help.mcp'; import { AddonsMcp } from '../../src/nest/addons/addons.mcp'; -import { createTestUnitOfWork, createTestAppSettingsRepo } from './test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo } from './test-uow'; import { createTestOrm } from './test-orm'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; import type { AppSettingsRepository } from '../../src/db/repositories/AppSettings.repository'; @@ -210,7 +210,7 @@ export async function createMcpTestRegistry(): Promise { // rather than off addons.bridge's own instance, so the harness has to supply // one — against the same test DB, which is what makes the `when:` gates // answer truthfully here instead of against the process-wide singleton. - const addonsService = new AddonsService(dbService); + const addonsService = await createTestAddonsService(dbService.connection, dbService); // The two photo providers, shared by MemoriesMcp (which browses them) and by // the capture backfill JourneyMcp schedules after a provider photo is attached // (which asks them when and where it was taken). Built for real rather than @@ -222,8 +222,8 @@ export async function createMcpTestRegistry(): Promise { const captureBackfill = new PhotoCaptureBackfillService(new PhotoResolverService(trekPhotos, new ThumbnailService(addonsService, generalStorage, dbService), new TrekPhotoCacheService(dbService, generalStorage), new PhotoProviderRegistry([new ImmichPhotoProvider(immichService), new SynologyPhotoProvider(synologyService)]), generalStorage), trekPhotos, generalStorage); return createTestRegistry( [ - new TagsMcp(new TagsService(dbService), authService), - new CategoriesMcp(new CategoriesService(dbService), dbService, new RuntimeEnvService(), guards), + new TagsMcp(new TagsService(await createTestTagsRepo(dbService.connection)), authService), + new CategoriesMcp(new CategoriesService(await createTestCategoriesRepo(dbService.connection)), dbService, new RuntimeEnvService(), guards), // The weather and airport tools left the legacy mapsWeather registrar. new WeatherMcp(new WeatherService()), new AirportsMcp(), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index e9a82e282..2906fc607 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -35,7 +35,7 @@ import { AtlasService } from '../../src/nest/atlas/atlas.service'; import { MapsService } from '../../src/nest/maps/maps.service'; import { PermissionsService } from '../../src/nest/permissions/permissions.service'; import { AuditService } from '../../src/nest/audit/audit.service'; -import { AddonsService } from '../../src/nest/addons/addons.service'; +import { createTestAddonsService } from './test-addons'; import { RealtimeService } from '../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service'; import { TripMembershipService } from '../../src/nest/trip-membership/trip-membership.service'; @@ -72,7 +72,7 @@ import { PlacePhotoCacheService } from '../../src/nest/place-photos/place-photo- import { TrekPhotosRepository } from '../../src/nest/photos/trek-photos.repository'; import { RuntimeEnvService } from '../../src/nest/app-config/runtime-env.service'; import { makeStorageFixture } from './storage-fixture'; -import { createTestUnitOfWork, createTestAppSettingsRepo, sharedTestOrm } from './test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo, sharedTestOrm } from './test-uow'; import { createTestOrm } from './test-orm'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; import type { AppSettingsRepository } from '../../src/db/repositories/AppSettings.repository'; @@ -98,7 +98,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< const exchangeRates = new ExchangeRatesService(); const realtime = new RealtimeService(); const budget = new BudgetService(dbs, permissions, exchangeRates, realtime, await createTestUnitOfWork(dbs.connection)); - const addons = new AddonsService(dbs); + const addons = await createTestAddonsService(dbs.connection, dbs); const queryHelpers = new QueryHelpersService(dbs); const todos = new TodoService(dbs, permissions, realtime, await createTestUnitOfWork(dbs.connection)); const packing = new PackingService(dbs, permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection)); @@ -127,8 +127,8 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< const guards = new PluginGuards(dbs, permissions, addons); const registry = createTestPluginRegistry([ - new TagsRpc(new TagsService(dbs)), - new CategoriesRpc(new CategoriesService(dbs)), + new TagsRpc(new TagsService(await createTestTagsRepo(dbs.connection))), + new CategoriesRpc(new CategoriesService(await createTestCategoriesRepo(dbs.connection))), new WeatherRpc(new WeatherService()), new ExchangeRatesRpc(exchangeRates), new TodoRpc(todos, realtime, guards), @@ -170,7 +170,7 @@ export async function createPluginRuntime(dbs: DatabaseService, registry?: Plugi return new PluginRuntimeService( dbs, new AuditService(auditOrm.repo(AuditLog) as AuditLogRepository, auditOrm.repo(Users) as UsersRepository), - new AddonsService(dbs), + await createTestAddonsService(dbs.connection, dbs), new PluginUserSettingsService(dbs), registry, await createPluginRpcHostFactory(dbs), diff --git a/server/tests/helpers/test-addons.ts b/server/tests/helpers/test-addons.ts new file mode 100644 index 000000000..dcfc2f107 --- /dev/null +++ b/server/tests/helpers/test-addons.ts @@ -0,0 +1,67 @@ +import type Database from 'better-sqlite3'; +import { AddonsService } from '../../src/nest/addons/addons.service'; +import { DatabaseService } from '../../src/nest/database/database.service'; +import { sharedTestOrm } from './test-uow'; +import { Addons } from '../../src/db/entities/Addons.entity'; +import type { AddonsRepository } from '../../src/db/repositories/Addons.repository'; +import { PhotoProviders } from '../../src/db/entities/PhotoProviders.entity'; +import type { PhotoProvidersRepository } from '../../src/db/repositories/PhotoProviders.repository'; +import { PhotoProviderFields } from '../../src/db/entities/PhotoProviderFields.entity'; +import type { PhotoProviderFieldsRepository } from '../../src/db/repositories/PhotoProviderFields.repository'; +import { AppSettings } from '../../src/db/entities/AppSettings.entity'; +import type { AppSettingsRepository } from '../../src/db/repositories/AppSettings.repository'; + +/** + * AddonsService's constructor grew from one `DatabaseService` to four + * injected repositories plus `DatabaseService` (Plan 3a Task 4, addons.md — + * the `AppSettingsRepository`/`AddonsRepository`/`PhotoProvidersRepository`/ + * `PhotoProviderFieldsRepository` wiring). Every hand-constructed + * `new AddonsService(dbs)` in the test tree needs the same four repositories + * a real `@InjectRepository` graph would hand it — this file is that + * factory, named `test-addons.ts` per the Task 4 brief (owned by Task 4; + * `tests/helpers/test-uow.ts` stays Task 3's file to extend). + * + * Built on the shared per-handle ORM (`test-uow.ts`'s `sharedTestOrm`, not a + * fresh `createTestOrm` here) — the SAME one `createTestAppSettingsRepo`/ + * `createTestUnitOfWork` draw from, so a repository built here and a + * `UnitOfWork`/repository built elsewhere for the same handle resolve + * through the identical context-resolving `EntityManager`, exactly like + * Nest's real DI graph (task-2-review.md I2's fix, carried forward for this + * domain). + */ +export async function createTestAddonsRepo(db: Database.Database): Promise { + const t = await sharedTestOrm(db); + return t.repo(Addons) as AddonsRepository; +} + +export async function createTestPhotoProvidersRepo(db: Database.Database): Promise { + const t = await sharedTestOrm(db); + return t.repo(PhotoProviders) as PhotoProvidersRepository; +} + +export async function createTestPhotoProviderFieldsRepo(db: Database.Database): Promise { + const t = await sharedTestOrm(db); + return t.repo(PhotoProviderFields) as PhotoProviderFieldsRepository; +} + +/** + * The full `AddonsService` a hand-constructed test collaborator needs, bound + * to the suite's own better-sqlite3 handle. + * + * `dbs` defaults to a fresh `DatabaseService` over the same handle when the + * caller has no existing one to pass through — AddonsService still needs one + * (see `addons.service.ts`'s constructor doc) purely to forward to + * `transit-provider.ts`'s readTransitProvider/writeTransitProvider and + * `instance-api-keys.ts`'s resolveApiKey, neither of which is one of this + * plan's six domains. + */ +export async function createTestAddonsService(db: Database.Database, dbs: DatabaseService = new DatabaseService(db)): Promise { + const t = await sharedTestOrm(db); + return new AddonsService( + t.repo(Addons) as AddonsRepository, + t.repo(PhotoProviders) as PhotoProvidersRepository, + t.repo(PhotoProviderFields) as PhotoProviderFieldsRepository, + t.repo(AppSettings) as AppSettingsRepository, + dbs, + ); +} diff --git a/server/tests/integration/mcp.test.ts b/server/tests/integration/mcp.test.ts index 7670cb16c..97da3c1fd 100644 --- a/server/tests/integration/mcp.test.ts +++ b/server/tests/integration/mcp.test.ts @@ -38,7 +38,7 @@ import type { McpDynamicTool } from '../../src/nest-mcp'; import { getMcpSafeUrl } from '../../src/app-config'; import { OauthService } from '../../src/nest/oauth/oauth.service'; import { DatabaseService } from '../../src/nest/database/database.service'; -import { AddonsService } from '../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../helpers/test-addons'; import { AuditService } from '../../src/nest/audit/audit.service'; import { createTestOrm, type TestOrm } from '../helpers/test-orm'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; @@ -76,7 +76,7 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); t = await createTestOrm(testDb); - oauthSvc = new OauthService(oauthDbs, new AddonsService(oauthDbs), new AuditService(t.repo(AuditLog) as AuditLogRepository, t.repo(Users) as UsersRepository)); + oauthSvc = new OauthService(oauthDbs, await createTestAddonsService(testDb, oauthDbs), new AuditService(t.repo(AuditLog) as AuditLogRepository, t.repo(Users) as UsersRepository)); }); beforeEach(() => { diff --git a/server/tests/integration/oauth-authorize.test.ts b/server/tests/integration/oauth-authorize.test.ts index 9b0065704..9b39d5d98 100644 --- a/server/tests/integration/oauth-authorize.test.ts +++ b/server/tests/integration/oauth-authorize.test.ts @@ -42,7 +42,7 @@ import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser } from '../helpers/factories'; import { OauthService } from '../../src/nest/oauth/oauth.service'; import { DatabaseService } from '../../src/nest/database/database.service'; -import { AddonsService } from '../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../helpers/test-addons'; import { AuditService } from '../../src/nest/audit/audit.service'; import { createTestOrm, type TestOrm } from '../helpers/test-orm'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; @@ -87,7 +87,7 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); t = await createTestOrm(testDb); - containerSideOauth = new OauthService(oauthDbs, new AddonsService(oauthDbs), new AuditService(t.repo(AuditLog) as AuditLogRepository, t.repo(Users) as UsersRepository)); + containerSideOauth = new OauthService(oauthDbs, await createTestAddonsService(testDb, oauthDbs), new AuditService(t.repo(AuditLog) as AuditLogRepository, t.repo(Users) as UsersRepository)); }); beforeEach(() => { diff --git a/server/tests/integration/oauth.test.ts b/server/tests/integration/oauth.test.ts index 3fbac0e69..19a179e3e 100644 --- a/server/tests/integration/oauth.test.ts +++ b/server/tests/integration/oauth.test.ts @@ -39,7 +39,7 @@ import { authCookie } from '../helpers/auth'; import { ALL_SCOPES } from '../../src/mcp/scopes'; import { OauthService } from '../../src/nest/oauth/oauth.service'; import { DatabaseService } from '../../src/nest/database/database.service'; -import { AddonsService } from '../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../helpers/test-addons'; import { AuditService } from '../../src/nest/audit/audit.service'; import { createTestOrm, type TestOrm } from '../helpers/test-orm'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; @@ -85,7 +85,7 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); t = await createTestOrm(testDb); - containerSideOauth = new OauthService(oauthDbs, new AddonsService(oauthDbs), new AuditService(t.repo(AuditLog) as AuditLogRepository, t.repo(Users) as UsersRepository)); + containerSideOauth = new OauthService(oauthDbs, await createTestAddonsService(testDb, oauthDbs), new AuditService(t.repo(AuditLog) as AuditLogRepository, t.repo(Users) as UsersRepository)); }); beforeEach(() => { diff --git a/server/tests/integration/plugins/boot-registry-order.test.ts b/server/tests/integration/plugins/boot-registry-order.test.ts index 4439a619d..c7dd38e6a 100644 --- a/server/tests/integration/plugins/boot-registry-order.test.ts +++ b/server/tests/integration/plugins/boot-registry-order.test.ts @@ -52,7 +52,7 @@ vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { AuditService } from '../../../src/nest/audit/audit.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service'; import { PluginUserSettingsService } from '../../../src/nest/plugins/plugin-user-settings.service'; import { PluginRpcHostFactory } from '../../../src/nest/plugins/host/plugin-rpc-host.factory'; @@ -114,6 +114,7 @@ describe('plugin boot vs registry scan ordering', () => { t = await createTestOrm(dbConn); const auditLogRepo = t.repo(AuditLog) as AuditLogRepository; const usersRepo = t.repo(Users) as UsersRepository; + const addonsService = await createTestAddonsService(dbConn, dbs); mod = await Test.createTestingModule({ providers: [ @@ -122,7 +123,7 @@ describe('plugin boot vs registry scan ordering', () => { // (if it had one) first. The boot reconcile must therefore not live there. { provide: PluginRuntimeService, - useFactory: () => new PluginRuntimeService(dbs, new AuditService(auditLogRepo, usersRepo), new AddonsService(dbs), userSettings, undefined, hostFactory), + useFactory: () => new PluginRuntimeService(dbs, new AuditService(auditLogRepo, usersRepo), addonsService, userSettings, undefined, hostFactory), }, { provide: 'REGISTRY_SCAN', diff --git a/server/tests/unit/db/repositories/Addons.repository.test.ts b/server/tests/unit/db/repositories/Addons.repository.test.ts new file mode 100644 index 000000000..2f997f0fd --- /dev/null +++ b/server/tests/unit/db/repositories/Addons.repository.test.ts @@ -0,0 +1,145 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { Addons } from '../../../../src/db/entities/Addons.entity'; +import type { AddonsRepository } from '../../../../src/db/repositories/Addons.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let addons: AddonsRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + addons = t.repo(Addons) as AddonsRepository; +}); +beforeEach(() => { + resetTestDb(testDb); + // resetTestDb deliberately keeps the seeded addons catalogue (test-db.ts's + // KEEP_TABLES) — these tests want a known, empty table so every case + // controls its own rows. + testDb.exec('DELETE FROM addons'); + t.clear(); +}); +afterAll(async () => { + await t.close(); + testDb.close(); +}); + +function rawRow(id: string): unknown { + return testDb.prepare('SELECT * FROM addons WHERE id = ?').get(id); +} + +function insertAddon(row: { + id: string; + name: string; + description?: string | null; + type?: string; + icon?: string | null; + enabled: 0 | 1; + sort_order?: number; +}): void { + testDb + .prepare('INSERT INTO addons (id, name, description, type, icon, enabled, sort_order) VALUES (?, ?, ?, ?, ?, ?, ?)') + .run(row.id, row.name, row.description ?? null, row.type ?? 'global', row.icon ?? null, row.enabled, row.sort_order ?? 0); +} + +describe('AddonsRepository', () => { + describe('isEnabled', () => { + it('ADDONSREPO-001: reads a stored 1 as true', async () => { + insertAddon({ id: 'budget', name: 'Budget', enabled: 1 }); + expect(await addons.isEnabled('budget')).toBe(true); + }); + + it('ADDONSREPO-002: reads a stored 0 as false', async () => { + insertAddon({ id: 'budget', name: 'Budget', enabled: 0 }); + expect(await addons.isEnabled('budget')).toBe(false); + }); + + it('ADDONSREPO-003: a missing row reads false', async () => { + expect(await addons.isEnabled('does-not-exist')).toBe(false); + }); + + // I1 (Task 0 review, carried forward): a primary-key findOne answers a + // repeat call from the identity map unless refresh: true is set — + // invisible to a write on the same id in the same request. + describe('sees a write on the same id in the same request (I1, identity-map regression)', () => { + it('ADDONSREPO-004: setEnabled then isEnabled reads the new value, not the stale one', async () => { + insertAddon({ id: 'budget', name: 'Budget', enabled: 0 }); + expect(await addons.isEnabled('budget')).toBe(false); // populate the identity map + await addons.setEnabled('budget', true); + expect(await addons.isEnabled('budget')).toBe(true); + }); + + it('ADDONSREPO-005: a raw UPDATE on the same id then isEnabled reads the new value', async () => { + insertAddon({ id: 'budget', name: 'Budget', enabled: 0 }); + expect(await addons.isEnabled('budget')).toBe(false); // populate the identity map + testDb.prepare('UPDATE addons SET enabled = 1 WHERE id = ?').run('budget'); + expect(await addons.isEnabled('budget')).toBe(true); + }); + }); + }); + + describe('listEnabled', () => { + it('ADDONSREPO-006: returns only enabled addons, ordered by sort_order', async () => { + insertAddon({ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: 1, sort_order: 2 }); + insertAddon({ id: 'budget', name: 'Costs', type: 'trip', icon: 'wallet', enabled: 1, sort_order: 1 }); + insertAddon({ id: 'vacay', name: 'Vacay', type: 'page', icon: 'sun', enabled: 0, sort_order: 0 }); + + const rows = await addons.listEnabled(); + expect(rows.map((r) => r.id)).toEqual(['budget', 'atlas']); + }); + + it('ADDONSREPO-007: the enabled column comes back as a JS boolean, not the stored int', async () => { + insertAddon({ id: 'atlas', name: 'Atlas', enabled: 1 }); + const [row] = await addons.listEnabled(); + expect(row.enabled).toBe(true); + expect(rawRow('atlas')).toMatchObject({ enabled: 1 }); + }); + + it('ADDONSREPO-008: an empty table returns an empty array', async () => { + expect(await addons.listEnabled()).toEqual([]); + }); + + it('ADDONSREPO-009: carries the full row shape (id, name, description, type, icon, enabled, config, sort_order)', async () => { + insertAddon({ id: 'atlas', name: 'Atlas', description: 'Visited countries map', type: 'page', icon: 'globe', enabled: 1, sort_order: 3 }); + const [row] = await addons.listEnabled(); + expect(row).toEqual({ + id: 'atlas', + name: 'Atlas', + description: 'Visited countries map', + type: 'page', + icon: 'globe', + enabled: true, + config: {}, + sort_order: 3, + }); + }); + }); + + describe('setEnabled', () => { + it('ADDONSREPO-010: stores the boolean as the legacy 0/1', async () => { + insertAddon({ id: 'budget', name: 'Budget', enabled: 0 }); + await addons.setEnabled('budget', true); + expect(rawRow('budget')).toMatchObject({ enabled: 1 }); + + await addons.setEnabled('budget', false); + expect(rawRow('budget')).toMatchObject({ enabled: 0 }); + }); + + it('ADDONSREPO-011: does not touch any other column', async () => { + insertAddon({ id: 'budget', name: 'Costs', description: 'Track spend', type: 'trip', icon: 'wallet', enabled: 0, sort_order: 5 }); + await addons.setEnabled('budget', true); + expect(rawRow('budget')).toEqual({ + id: 'budget', + name: 'Costs', + description: 'Track spend', + type: 'trip', + icon: 'wallet', + enabled: 1, + config: '{}', + sort_order: 5, + }); + }); + }); +}); diff --git a/server/tests/unit/db/repositories/PhotoProviderFields.repository.test.ts b/server/tests/unit/db/repositories/PhotoProviderFields.repository.test.ts new file mode 100644 index 000000000..654a3619e --- /dev/null +++ b/server/tests/unit/db/repositories/PhotoProviderFields.repository.test.ts @@ -0,0 +1,111 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { PhotoProviderFields } from '../../../../src/db/entities/PhotoProviderFields.entity'; +import type { PhotoProviderFieldsRepository } from '../../../../src/db/repositories/PhotoProviderFields.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let fields: PhotoProviderFieldsRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + fields = t.repo(PhotoProviderFields) as PhotoProviderFieldsRepository; +}); +beforeEach(() => { + resetTestDb(testDb); + testDb.exec('DELETE FROM photo_provider_fields'); + testDb.exec('DELETE FROM photo_providers'); + t.clear(); +}); +afterAll(async () => { + await t.close(); + testDb.close(); +}); + +function insertProvider(id: string): void { + testDb.prepare("INSERT INTO photo_providers (id, name, icon, enabled, sort_order) VALUES (?, ?, 'Image', 1, 0)").run(id, id); +} + +function insertField(row: { + provider_id: string; + field_key: string; + label: string; + input_type?: string; + placeholder?: string | null; + hint?: string | null; + required?: 0 | 1; + secret?: 0 | 1; + settings_key?: string | null; + payload_key?: string | null; + sort_order?: number; +}): void { + testDb + .prepare( + `INSERT INTO photo_provider_fields + (provider_id, field_key, label, input_type, placeholder, hint, required, secret, settings_key, payload_key, sort_order) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ) + .run( + row.provider_id, + row.field_key, + row.label, + row.input_type ?? 'text', + row.placeholder ?? null, + row.hint ?? null, + row.required ?? 0, + row.secret ?? 0, + row.settings_key ?? null, + row.payload_key ?? null, + row.sort_order ?? 0, + ); +} + +describe('PhotoProviderFieldsRepository.listAllOrdered', () => { + it('ADDONSPPFREPO-001: returns every field, ordered by sort_order then id, across all providers', async () => { + insertProvider('immich'); + insertProvider('synology'); + insertField({ provider_id: 'synology', field_key: 'url', label: 'URL', sort_order: 1 }); + insertField({ provider_id: 'immich', field_key: 'url', label: 'URL', sort_order: 0 }); + insertField({ provider_id: 'immich', field_key: 'token', label: 'Token', sort_order: 0 }); + + const rows = await fields.listAllOrdered(); + // Two rows share sort_order 0 — insertion order (ascending id) breaks the tie. + expect(rows.map((r) => [r.provider_id, r.field_key])).toEqual([ + ['immich', 'url'], + ['immich', 'token'], + ['synology', 'url'], + ]); + }); + + it('ADDONSPPFREPO-002: the provider_id twin reads back from find(), same as trip_id on Days', async () => { + insertProvider('immich'); + insertField({ provider_id: 'immich', field_key: 'url', label: 'URL' }); + const [row] = await fields.listAllOrdered(); + expect(row.provider_id).toBe('immich'); + }); + + it('ADDONSPPFREPO-003: required/secret come back as the raw stored integer, matching the entity type (not a coerced boolean)', async () => { + insertProvider('immich'); + insertField({ provider_id: 'immich', field_key: 'token', label: 'Token', required: 1, secret: 1 }); + const [row] = await fields.listAllOrdered(); + expect(row.required).toBe(1); + expect(row.secret).toBe(1); + expect(typeof row.required).toBe('number'); + }); + + it('ADDONSPPFREPO-004: nullable columns (placeholder, hint, settings_key, payload_key) come back null when unset', async () => { + insertProvider('immich'); + insertField({ provider_id: 'immich', field_key: 'k', label: 'L' }); + const [row] = await fields.listAllOrdered(); + expect(row.placeholder).toBeNull(); + expect(row.hint).toBeNull(); + expect(row.settings_key).toBeNull(); + expect(row.payload_key).toBeNull(); + }); + + it('ADDONSPPFREPO-005: an empty table returns an empty array', async () => { + expect(await fields.listAllOrdered()).toEqual([]); + }); +}); diff --git a/server/tests/unit/db/repositories/PhotoProviders.repository.test.ts b/server/tests/unit/db/repositories/PhotoProviders.repository.test.ts new file mode 100644 index 000000000..23753853c --- /dev/null +++ b/server/tests/unit/db/repositories/PhotoProviders.repository.test.ts @@ -0,0 +1,81 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { PhotoProviders } from '../../../../src/db/entities/PhotoProviders.entity'; +import type { PhotoProvidersRepository } from '../../../../src/db/repositories/PhotoProviders.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let photoProviders: PhotoProvidersRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + photoProviders = t.repo(PhotoProviders) as PhotoProvidersRepository; +}); +beforeEach(() => { + resetTestDb(testDb); + // photo_provider_fields FK-references photo_providers, so clear the child + // first — resetTestDb keeps both tables (test-db.ts's KEEP_TABLES). + testDb.exec('DELETE FROM photo_provider_fields'); + testDb.exec('DELETE FROM photo_providers'); + t.clear(); +}); +afterAll(async () => { + await t.close(); + testDb.close(); +}); + +function rawRow(id: string): unknown { + return testDb.prepare('SELECT * FROM photo_providers WHERE id = ?').get(id); +} + +function insertProvider(row: { + id: string; + name: string; + description?: string | null; + icon?: string | null; + enabled: 0 | 1; + sort_order?: number; +}): void { + testDb + .prepare('INSERT INTO photo_providers (id, name, description, icon, enabled, sort_order) VALUES (?, ?, ?, ?, ?, ?)') + .run(row.id, row.name, row.description ?? null, row.icon ?? null, row.enabled, row.sort_order ?? 0); +} + +describe('PhotoProvidersRepository.listEnabled', () => { + it('ADDONSPPREPO-001: returns only enabled providers, ordered by sort_order then id', async () => { + insertProvider({ id: 'synology', name: 'Synology', enabled: 1, sort_order: 0 }); + insertProvider({ id: 'immich', name: 'Immich', enabled: 1, sort_order: 0 }); + insertProvider({ id: 'off', name: 'Off', enabled: 0, sort_order: -1 }); + + const rows = await photoProviders.listEnabled(); + // Same sort_order (0) for both enabled rows — id breaks the tie. + expect(rows.map((r) => r.id)).toEqual(['immich', 'synology']); + }); + + it('ADDONSPPREPO-002: the enabled column comes back as the raw stored integer, not a coerced boolean', async () => { + insertProvider({ id: 'immich', name: 'Immich', enabled: 1 }); + const [row] = await photoProviders.listEnabled(); + expect(row.enabled).toBe(1); + expect(typeof row.enabled).toBe('number'); + }); + + it('ADDONSPPREPO-003: an empty table returns an empty array', async () => { + expect(await photoProviders.listEnabled()).toEqual([]); + }); + + it('ADDONSPPREPO-004: carries the full row shape (id, name, description, icon, enabled, sort_order)', async () => { + insertProvider({ id: 'immich', name: 'Immich', description: 'Self-hosted photos', icon: 'image', enabled: 1, sort_order: 4 }); + const [row] = await photoProviders.listEnabled(); + expect(row).toEqual({ + id: 'immich', + name: 'Immich', + description: 'Self-hosted photos', + icon: 'image', + enabled: 1, + sort_order: 4, + }); + expect(rawRow('immich')).toMatchObject({ enabled: 1 }); + }); +}); diff --git a/server/tests/unit/mcp/tools-help-addons.test.ts b/server/tests/unit/mcp/tools-help-addons.test.ts index 1c27cab82..8b2ee117f 100644 --- a/server/tests/unit/mcp/tools-help-addons.test.ts +++ b/server/tests/unit/mcp/tools-help-addons.test.ts @@ -59,8 +59,7 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb, setAddonEnabled } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; -import { DatabaseService } from '../../../src/nest/database/database.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { ADDON_IDS } from '../../../src/addons'; const SECTIONS = [ @@ -360,7 +359,7 @@ describe('Tool: list_addons', () => { // Written through the same service the admin panel writes through rather // than through a hand-rolled app_settings row, so the tool is checked // against the real writer and not against a restatement of it. - const addonsService = new AddonsService(new DatabaseService(testDb)); + const addonsService = await createTestAddonsService(testDb); const { user } = createUser(testDb); await withHarness(user.id, async (h) => { await addonsService.updateCollabFeatures({ polls: false }); diff --git a/server/tests/unit/nest/addons.service.test.ts b/server/tests/unit/nest/addons.service.test.ts index de34a4418..6fd152d13 100644 --- a/server/tests/unit/nest/addons.service.test.ts +++ b/server/tests/unit/nest/addons.service.test.ts @@ -1,27 +1,53 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; - -// Three distinct prepare(...).all() reads (addons, photo_providers, photo_provider_fields). -// A single shared statement is reused, so .all() is fed result sets in call order. -const { dbMock } = vi.hoisted(() => { - const stmt = { get: vi.fn(), all: vi.fn(() => []), run: vi.fn() }; - // Takes the statement text, as the real one does: a reader that inlines its - // key in the SQL is identified by that string and by nothing else. - return { dbMock: { prepare: vi.fn((_sql?: string) => stmt), _stmt: stmt } }; -}); -vi.mock('../../../src/db/database', () => ({ db: dbMock, closeDb: () => {}, reinitialize: () => {} })); -import { db as dbConn } from '../../../src/db/database'; +/** + * Unit tests for AddonsService — rebuilt on real rows (Plan 3a Task 4). + * + * The legacy version of this file fully stubbed DatabaseService at the + * `db.prepare(sql).{get,all,run}` level: a single shared fake `stmt` fed + * canned rows *in call order* (three `.all()` reads for addons/providers/ + * fields, a `.get()` for the journey gate, and so on). That coupling cannot + * survive AddonsService now calling four different repositories instead of + * one shared prepared statement, so every case here runs against a real + * in-memory SQLite DB (createTestOrm) and inserts the exact rows it needs. + * + * Two cases needed their INTENT re-expressed, not just their mechanics + * (noted at each): the legacy `list()` SQL filters `WHERE enabled = 1` on + * both `addons` and `photo_providers`, so a disabled row is never returned by + * the real query — the old mocked tests fed a disabled row through anyway + * (mocks don't enforce a WHERE clause) to prove the service's `!!enabled` + * coercion handled `0`. That coercion is still real (and still asserted + * directly, on `isAddonEnabled` and on an enabled row's boolean in `list()`), + * but "list() returns a disabled addon/provider with enabled: false" is not a + * reachable production behaviour, so those two cases now assert the WHERE + * filter itself (the disabled row is excluded) alongside the boolean coercion + * of the enabled row that remains. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser } from '../../helpers/factories'; +import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { PlaceShadowService } from '../../../src/nest/place-shadow/place-shadow.service'; +import { Addons } from '../../../src/db/entities/Addons.entity'; +import type { AddonsRepository } from '../../../src/db/repositories/Addons.repository'; +import { PhotoProviders } from '../../../src/db/entities/PhotoProviders.entity'; +import type { PhotoProvidersRepository } from '../../../src/db/repositories/PhotoProviders.repository'; +import { PhotoProviderFields } from '../../../src/db/entities/PhotoProviderFields.entity'; +import type { PhotoProviderFieldsRepository } from '../../../src/db/repositories/PhotoProviderFields.repository'; +import { AppSettings } from '../../../src/db/entities/AppSettings.entity'; +import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository'; const { getPhotoProviderConfig } = vi.hoisted(() => ({ getPhotoProviderConfig: vi.fn(() => ({})) })); vi.mock('../../../src/nest/memories/memories.helpers', () => ({ getPhotoProviderConfig })); -import { AddonsService } from '../../../src/nest/addons/addons.service'; -import { PlaceShadowService } from '../../../src/nest/place-shadow/place-shadow.service'; -import { MapsService } from '../../../src/nest/maps/maps.service'; - -function svc() { - return new AddonsService(new DatabaseService(dbConn)); -} +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let addonsRepo: AddonsRepository; +let photoProvidersRepo: PhotoProvidersRepository; +let photoProviderFieldsRepo: PhotoProviderFieldsRepository; +let appSettingsRepo: AppSettingsRepository; +let svc: AddonsService; type ListAddon = Awaited>['addons'][number]; @@ -47,68 +73,143 @@ function providerFields(addon: ListAddon): PhotoProviderField[] { return (addon as ListAddon & { fields: PhotoProviderField[] }).fields; } -// Feed list()'s reads in call order: addons, providers, fields (.all), then the -// collab-features rows (.all) and the bag-tracking row (.get) from app_settings. -// The provider query now sits behind an isAddonEnabled(journey) read (.get), fed -// first as enabled so every case below sees its providers. -function feedReads( - addons: unknown[], - providers: unknown[], - fields: unknown[], - collabRows: unknown[] = [], - bagRow: { value: string } | undefined = undefined, -) { - dbMock._stmt.all - .mockReturnValueOnce(addons) - .mockReturnValueOnce(providers) - .mockReturnValueOnce(fields) - .mockReturnValueOnce(collabRows); - dbMock._stmt.get.mockReturnValueOnce({ enabled: 1 }).mockReturnValueOnce(bagRow); +function insertAddon(row: { id: string; name: string; type?: string; icon?: string | null; enabled: 0 | 1; sort_order?: number }): void { + testDb + .prepare('INSERT INTO addons (id, name, description, type, icon, enabled, sort_order) VALUES (?, ?, NULL, ?, ?, ?, ?)') + .run(row.id, row.name, row.type ?? 'global', row.icon ?? null, row.enabled, row.sort_order ?? 0); } +/** + * Stubs the journey gate `list()` checks before reading photo providers, + * WITHOUT inserting a real 'journey' addons row. A real row is what the + * migration seeds (`journey_addon.ts`) and is exactly what `isAddonEnabled` + * should read in production — but inserting one here would also make it a + * genuine, `enabled: true` member of `listEnabled()`'s own result (the same + * WHERE-clause reality the file header's two re-expressed cases describe), + * which would pollute every one of these provider/field-shape assertions + * with an unrelated 'journey' entry. Spying on the one repository call the + * gate makes keeps these tests about provider/field shaping, not about + * `list()`'s addons array — that array is already covered by the addons- + * focused cases above. + */ +function stubJourneyEnabled(): ReturnType { + return vi.spyOn(addonsRepo, 'isEnabled').mockImplementation(async (id: string) => id === 'journey'); +} + +function insertProvider(row: { id: string; name: string; icon?: string | null; enabled: 0 | 1; sort_order?: number }): void { + testDb + .prepare('INSERT INTO photo_providers (id, name, description, icon, enabled, sort_order) VALUES (?, ?, NULL, ?, ?, ?)') + .run(row.id, row.name, row.icon ?? null, row.enabled, row.sort_order ?? 0); +} + +function insertField(row: { + provider_id: string; + field_key: string; + label: string; + input_type?: string; + placeholder?: string | null; + hint?: string | null; + required?: 0 | 1; + secret?: 0 | 1; + settings_key?: string | null; + payload_key?: string | null; + sort_order?: number; +}): void { + testDb + .prepare( + `INSERT INTO photo_provider_fields + (provider_id, field_key, label, input_type, placeholder, hint, required, secret, settings_key, payload_key, sort_order) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ) + .run( + row.provider_id, + row.field_key, + row.label, + row.input_type ?? 'text', + row.placeholder ?? null, + row.hint ?? null, + row.required ?? 0, + row.secret ?? 0, + row.settings_key ?? null, + row.payload_key ?? null, + row.sort_order ?? 0, + ); +} + +function setAppSetting(key: string, value: string | null): void { + testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(key, value); +} + +function rawAppSetting(key: string): { key: string; value: string | null } | undefined { + return testDb.prepare('SELECT key, value FROM app_settings WHERE key = ?').get(key) as + | { key: string; value: string | null } + | undefined; +} + +beforeAll(async () => { + t = await createTestOrm(testDb); + addonsRepo = t.repo(Addons) as AddonsRepository; + photoProvidersRepo = t.repo(PhotoProviders) as PhotoProvidersRepository; + photoProviderFieldsRepo = t.repo(PhotoProviderFields) as PhotoProviderFieldsRepository; + appSettingsRepo = t.repo(AppSettings) as AppSettingsRepository; +}); + beforeEach(() => { - vi.clearAllMocks(); - dbMock._stmt.all.mockReturnValue([]); - dbMock._stmt.get.mockReturnValue(undefined); + resetTestDb(testDb); + // resetTestDb keeps the seeded addons/photo_providers/photo_provider_fields + // catalogue (test-db.ts's KEEP_TABLES) — every case here wants a known, + // empty starting point and inserts exactly the rows it needs. + testDb.exec('DELETE FROM photo_provider_fields'); + testDb.exec('DELETE FROM photo_providers'); + testDb.exec('DELETE FROM addons'); + t.clear(); + getPhotoProviderConfig.mockReset(); getPhotoProviderConfig.mockReturnValue({}); + svc = new AddonsService(addonsRepo, photoProvidersRepo, photoProviderFieldsRepo, appSettingsRepo, new DatabaseService(testDb)); +}); + +afterEach(() => { + // Restores stubJourneyEnabled's spy on addonsRepo.isEnabled (and the "drops + // the photo providers" case's spy on photoProvidersRepo.listEnabled) so no + // stub outlives the test that set it up. + vi.restoreAllMocks(); +}); + +afterAll(async () => { + await t.close(); + testDb.close(); }); describe('AddonsService.list', () => { it('returns the collab features and the bag-tracking flag from app_settings', async () => { - feedReads([], [], [], [{ key: 'collab_chat_enabled', value: 'false' }], { value: 'true' }); + setAppSetting('collab_chat_enabled', 'false'); + setAppSetting('bag_tracking_enabled', 'true'); - const res = await svc().list(); + const res = await svc.list(); expect(res.collabFeatures).toEqual({ chat: false, notes: true, links: true, polls: true, whatsnext: true }); expect(res.bagTracking).toBe(true); expect(res.addons).toEqual([]); }); - it('coerces the addon enabled column to a boolean (both 1 and 0)', async () => { - feedReads( - [ - { id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: 1 }, - { id: 'vacay', name: 'Vacay', type: 'page', icon: 'sun', enabled: 0 }, - ], - [], - [], - ); + // Re-expressed (see file header): the legacy WHERE enabled = 1 makes a + // disabled row in list()'s output unreachable, so this now proves the + // filter (the disabled row never surfaces) and the boolean coercion of the + // enabled row that does (the entity's `enabled` is p.boolean(), so the + // stored int comes back as a real JS boolean, not just truthy). + it('only returns enabled addons, with the stored int coerced to a JS boolean', async () => { + insertAddon({ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: 1 }); + insertAddon({ id: 'vacay', name: 'Vacay', type: 'page', icon: 'sun', enabled: 0 }); - const res = await svc().list(); - expect(res.addons).toEqual([ - { id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: true }, - { id: 'vacay', name: 'Vacay', type: 'page', icon: 'sun', enabled: false }, - ]); + const res = await svc.list(); + expect(res.addons).toEqual([{ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: true }]); }); it('maps a photo provider with no fields to an empty fields array (the || [] fallback)', async () => { - feedReads( - [], - [{ id: 'immich', name: 'Immich', icon: 'image', enabled: 1, sort_order: 0 }], - [], - ); + stubJourneyEnabled(); + insertProvider({ id: 'immich', name: 'Immich', icon: 'image', enabled: 1 }); getPhotoProviderConfig.mockReturnValue({ baseUrl: 'http://x' }); - const res = await svc().list(); + const res = await svc.list(); expect(res.addons).toEqual([ { id: 'immich', @@ -123,53 +224,49 @@ describe('AddonsService.list', () => { expect(getPhotoProviderConfig).toHaveBeenCalledWith('immich'); }); - it('coerces a disabled photo provider enabled flag to false', async () => { - feedReads( - [], - [{ id: 'synology', name: 'Synology', icon: 'image', enabled: 0, sort_order: 1 }], - [], - ); + // Re-expressed for the same reason as the addons case above: a disabled + // photo_providers row is filtered out by listEnabled()'s WHERE clause, so + // this now proves that filter alongside the enabled row's integer→boolean + // coercion (photo_providers.enabled is p.integer(), not p.boolean() — + // AddonsService does the `!!` itself, unlike the Addons entity above). + it('only returns enabled photo providers, with the stored integer coerced to a JS boolean', async () => { + stubJourneyEnabled(); + insertProvider({ id: 'synology', name: 'Synology', icon: 'image', enabled: 1, sort_order: 1 }); + insertProvider({ id: 'off-provider', name: 'Off', icon: 'image', enabled: 0, sort_order: 2 }); - const res = await svc().list(); - expect((res.addons[0] as { enabled: boolean }).enabled).toBe(false); + const res = await svc.list(); + expect(res.addons.map((a) => (a as { id: string }).id)).toEqual(['synology']); + expect((res.addons[0] as { enabled: boolean }).enabled).toBe(true); }); it('groups multiple fields under their provider and keeps insertion order', async () => { - feedReads( - [], - [{ id: 'immich', name: 'Immich', icon: 'image', enabled: 1, sort_order: 0 }], - [ - { - provider_id: 'immich', - field_key: 'url', - label: 'URL', - input_type: 'text', - placeholder: 'https://', - hint: 'Base URL', - required: 1, - secret: 0, - settings_key: 'immich_url', - payload_key: 'url', - sort_order: 0, - }, - // Second field for the SAME provider exercises the `get(...) || []` truthy branch. - { - provider_id: 'immich', - field_key: 'token', - label: 'Token', - input_type: 'password', - placeholder: null, - hint: null, - required: 0, - secret: 1, - settings_key: null, - payload_key: null, - sort_order: 1, - }, - ], - ); + stubJourneyEnabled(); + insertProvider({ id: 'immich', name: 'Immich', icon: 'image', enabled: 1 }); + insertField({ + provider_id: 'immich', + field_key: 'url', + label: 'URL', + input_type: 'text', + placeholder: 'https://', + hint: 'Base URL', + required: 1, + secret: 0, + settings_key: 'immich_url', + payload_key: 'url', + sort_order: 0, + }); + // Second field for the SAME provider exercises the `get(...) || []` truthy branch. + insertField({ + provider_id: 'immich', + field_key: 'token', + label: 'Token', + input_type: 'password', + required: 0, + secret: 1, + sort_order: 1, + }); - const res = await svc().list(); + const res = await svc.list(); expect(providerFields(res.addons[0])).toEqual([ { key: 'url', @@ -199,24 +296,11 @@ describe('AddonsService.list', () => { }); it('falls back placeholder→"", hint→null, settings/payload keys→null when columns are missing/empty', async () => { - feedReads( - [], - [{ id: 'p', name: 'P', icon: 'i', enabled: 1, sort_order: 0 }], - [ - { - provider_id: 'p', - field_key: 'k', - label: 'L', - input_type: 'text', - // placeholder/hint/settings_key/payload_key omitted entirely (undefined) - required: 0, - secret: 0, - sort_order: 0, - }, - ], - ); + stubJourneyEnabled(); + insertProvider({ id: 'p', name: 'P', icon: 'i', enabled: 1 }); + insertField({ provider_id: 'p', field_key: 'k', label: 'L' }); // placeholder/hint/settings_key/payload_key all null - const res = await svc().list(); + const res = await svc.list(); const field = providerFields(res.addons[0])[0]; expect(field).toMatchObject({ placeholder: '', @@ -227,53 +311,38 @@ describe('AddonsService.list', () => { }); it('keeps fields belonging to other providers out of a provider with none of its own', async () => { + stubJourneyEnabled(); // A field exists, but for a DIFFERENT provider than the one returned — exercises // the `fieldsByProvider.get(p.id) || []` fallback while the map is non-empty. - feedReads( - [], - [{ id: 'has-none', name: 'X', icon: 'i', enabled: 1, sort_order: 0 }], - [ - { - provider_id: 'other', - field_key: 'k', - label: 'L', - input_type: 'text', - required: 0, - secret: 0, - sort_order: 0, - }, - ], - ); + insertProvider({ id: 'has-none', name: 'X', icon: 'i', enabled: 1 }); + insertProvider({ id: 'other', name: 'Other', icon: 'i', enabled: 0 }); // FK target for the field below + insertField({ provider_id: 'other', field_key: 'k', label: 'L' }); - const res = await svc().list(); + const res = await svc.list(); expect(providerFields(res.addons[0])).toEqual([]); }); it('concatenates regular addons before the photo providers', async () => { - feedReads( - [{ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: 1 }], - [{ id: 'immich', name: 'Immich', icon: 'image', enabled: 1, sort_order: 0 }], - [], - ); + stubJourneyEnabled(); + insertAddon({ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: 1 }); + insertProvider({ id: 'immich', name: 'Immich', icon: 'image', enabled: 1 }); - const res = await svc().list(); + const res = await svc.list(); expect(res.addons.map((a) => (a as { id: string }).id)).toEqual(['atlas', 'immich']); expect((res.addons[1] as { type: string }).type).toBe('photo_provider'); }); it('drops the photo providers while the journey addon is off, whatever their rows say', async () => { - // The journey gate (.get) answers disabled, so the provider query is never - // made and the .all chain shortens to addons, fields, collab. - dbMock._stmt.get.mockReturnValueOnce({ enabled: 0 }); - dbMock._stmt.all - .mockReturnValueOnce([{ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: 1 }]) - .mockReturnValueOnce([]) - .mockReturnValueOnce([]); + // No 'journey' row at all — isAddonEnabled('journey') reads false, same as + // the legacy `!!undefined`. + insertAddon({ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: 1 }); + insertProvider({ id: 'immich', name: 'Immich', icon: 'image', enabled: 1 }); + const listSpy = vi.spyOn(photoProvidersRepo, 'listEnabled'); - const res = await svc().list(); + const res = await svc.list(); expect(res.addons).toEqual([{ id: 'atlas', name: 'Atlas', type: 'page', icon: 'globe', enabled: true }]); - const providerReads = (dbMock.prepare.mock.calls as unknown[][]).filter((call) => String(call[0]).includes('FROM photo_providers')); - expect(providerReads).toEqual([]); + expect(listSpy).not.toHaveBeenCalled(); + listSpy.mockRestore(); }); }); @@ -282,62 +351,65 @@ describe('AddonsService.list', () => { // purpose: bag tracking is opt-in (=== 'true'), collab flags opt-out (!== 'false'). describe('AddonsService addon/feature flags', () => { it('isAddonEnabled coerces the enabled column (1/0/missing row)', async () => { - dbMock._stmt.get.mockReturnValueOnce({ enabled: 1 }); - expect(await svc().isAddonEnabled('budget')).toBe(true); - dbMock._stmt.get.mockReturnValueOnce({ enabled: 0 }); - expect(await svc().isAddonEnabled('budget')).toBe(false); - dbMock._stmt.get.mockReturnValueOnce(undefined); - expect(await svc().isAddonEnabled('nope')).toBe(false); + insertAddon({ id: 'budget', name: 'Budget', enabled: 1 }); + expect(await svc.isAddonEnabled('budget')).toBe(true); + + testDb.prepare('UPDATE addons SET enabled = 0 WHERE id = ?').run('budget'); + expect(await svc.isAddonEnabled('budget')).toBe(false); + + expect(await svc.isAddonEnabled('nope')).toBe(false); }); it('getBagTracking is opt-in: only the literal string true enables it', async () => { - dbMock._stmt.get.mockReturnValueOnce({ value: 'true' }); - expect(await svc().getBagTracking()).toEqual({ enabled: true }); - dbMock._stmt.get.mockReturnValueOnce({ value: 'false' }); - expect(await svc().getBagTracking()).toEqual({ enabled: false }); - dbMock._stmt.get.mockReturnValueOnce(undefined); // absent row → OFF - expect(await svc().getBagTracking()).toEqual({ enabled: false }); + setAppSetting('bag_tracking_enabled', 'true'); + expect(await svc.getBagTracking()).toEqual({ enabled: true }); + + setAppSetting('bag_tracking_enabled', 'false'); + expect(await svc.getBagTracking()).toEqual({ enabled: false }); + + testDb.prepare("DELETE FROM app_settings WHERE key = 'bag_tracking_enabled'").run(); + expect(await svc.getBagTracking()).toEqual({ enabled: false }); // absent row → OFF }); it('updateBagTracking persists true/false strings and echoes the flag (ADMIN-SVC-030)', async () => { - expect(await svc().updateBagTracking(true)).toEqual({ enabled: true }); - expect(dbMock._stmt.run).toHaveBeenCalledWith('true'); - expect(await svc().updateBagTracking(false)).toEqual({ enabled: false }); - expect(dbMock._stmt.run).toHaveBeenCalledWith('false'); + expect(await svc.updateBagTracking(true)).toEqual({ enabled: true }); + expect(rawAppSetting('bag_tracking_enabled')).toEqual({ key: 'bag_tracking_enabled', value: 'true' }); + + expect(await svc.updateBagTracking(false)).toEqual({ enabled: false }); + expect(rawAppSetting('bag_tracking_enabled')).toEqual({ key: 'bag_tracking_enabled', value: 'false' }); }); it('getCollabFeatures is opt-out: absent rows default ON, only false disables', async () => { - dbMock._stmt.all.mockReturnValueOnce([ - { key: 'collab_chat_enabled', value: 'false' }, - { key: 'collab_polls_enabled', value: 'true' }, - ]); - expect(await svc().getCollabFeatures()).toEqual({ chat: false, notes: true, links: true, polls: true, whatsnext: true }); + setAppSetting('collab_chat_enabled', 'false'); + setAppSetting('collab_polls_enabled', 'true'); + + expect(await svc.getCollabFeatures()).toEqual({ chat: false, notes: true, links: true, polls: true, whatsnext: true }); }); it('updateCollabFeatures writes only the provided flags and reports changed (#1414, ADMIN-SVC-070)', async () => { + const setValueSpy = vi.spyOn(appSettingsRepo, 'setValue'); + // before-read: all default ON; after-read: chat flipped off → changed - dbMock._stmt.all.mockReturnValueOnce([]).mockReturnValueOnce([{ key: 'collab_chat_enabled', value: 'false' }]); - const first = await svc().updateCollabFeatures({ chat: false }); + const first = await svc.updateCollabFeatures({ chat: false }); expect(first.changed).toBe(true); expect(first.features.chat).toBe(false); - expect(dbMock._stmt.run).toHaveBeenCalledTimes(1); - expect(dbMock._stmt.run).toHaveBeenCalledWith('collab_chat_enabled', 'false'); + expect(setValueSpy).toHaveBeenCalledTimes(1); + expect(setValueSpy).toHaveBeenCalledWith('collab_chat_enabled', 'false'); + expect(rawAppSetting('collab_chat_enabled')).toEqual({ key: 'collab_chat_enabled', value: 'false' }); // identical save → before and after read the same → no change, MCP sessions must survive - dbMock._stmt.run.mockClear(); - dbMock._stmt.all - .mockReturnValueOnce([{ key: 'collab_chat_enabled', value: 'false' }]) - .mockReturnValueOnce([{ key: 'collab_chat_enabled', value: 'false' }]); - const second = await svc().updateCollabFeatures({ chat: false }); + setValueSpy.mockClear(); + const second = await svc.updateCollabFeatures({ chat: false }); expect(second.changed).toBe(false); - expect(dbMock._stmt.run).toHaveBeenCalledWith('collab_chat_enabled', 'false'); + expect(setValueSpy).toHaveBeenCalledWith('collab_chat_enabled', 'false'); // undefined flags are not written - dbMock._stmt.run.mockClear(); - dbMock._stmt.all.mockReturnValueOnce([]).mockReturnValueOnce([]); - const third = await svc().updateCollabFeatures({}); + setValueSpy.mockClear(); + const third = await svc.updateCollabFeatures({}); expect(third.changed).toBe(false); - expect(dbMock._stmt.run).not.toHaveBeenCalled(); + expect(setValueSpy).not.toHaveBeenCalled(); + + setValueSpy.mockRestore(); }); }); @@ -349,10 +421,6 @@ describe('AddonsService addon/feature flags', () => { * nobody loses a feature on upgrade. */ describe('AddonsService places flags', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - const cases = [ ['getPlacesPhotos', 'updatePlacesPhotos', 'places_photos_enabled'], ['getPlacesAutocomplete', 'updatePlacesAutocomplete', 'places_autocomplete_enabled'], @@ -361,29 +429,28 @@ describe('AddonsService places flags', () => { it('ADDONS-SVC-080 an unset flag reads as OFF, and anything but the literal true does too', async () => { for (const [getter, , key] of cases) { - dbMock._stmt.get.mockReturnValueOnce(undefined); - expect(await svc()[getter]()).toEqual({ enabled: false }); - expect(dbMock.prepare).toHaveBeenLastCalledWith('SELECT value FROM app_settings WHERE key = ?'); - expect(dbMock._stmt.get).toHaveBeenLastCalledWith(key); + testDb.prepare('DELETE FROM app_settings WHERE key = ?').run(key); + expect(await svc[getter]()).toEqual({ enabled: false }); - dbMock._stmt.get.mockReturnValueOnce({ value: 'garbage' }); - expect(await svc()[getter]()).toEqual({ enabled: false }); + setAppSetting(key, 'garbage'); + expect(await svc[getter]()).toEqual({ enabled: false }); } }); it('ADDONS-SVC-081 a stored "true" reads as ON', async () => { - for (const [getter] of cases) { - dbMock._stmt.get.mockReturnValueOnce({ value: 'true' }); - expect(await svc()[getter]()).toEqual({ enabled: true }); + for (const [getter, , key] of cases) { + setAppSetting(key, 'true'); + expect(await svc[getter]()).toEqual({ enabled: true }); } }); it('ADDONS-SVC-082 the setters persist the literal string and echo the boolean back', async () => { for (const [, setter, key] of cases) { - expect(await svc()[setter](true)).toEqual({ enabled: true }); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith(key, 'true'); - expect(await svc()[setter](false)).toEqual({ enabled: false }); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith(key, 'false'); + expect(await svc[setter](true)).toEqual({ enabled: true }); + expect(rawAppSetting(key)).toEqual({ key, value: 'true' }); + + expect(await svc[setter](false)).toEqual({ enabled: false }); + expect(rawAppSetting(key)).toEqual({ key, value: 'false' }); } }); }); @@ -399,30 +466,26 @@ describe('AddonsService places flags', () => { * feature runs. */ describe('AddonsService places enrichment flag', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - it('ADDONS-SVC-083 an unset flag reads as ON', async () => { - dbMock._stmt.get.mockReturnValueOnce(undefined); - expect(await svc().getPlacesEnrich()).toEqual({ enabled: true }); + expect(await svc.getPlacesEnrich()).toEqual({ enabled: true }); }); it('ADDONS-SVC-084 only the literal "false" switches it off', async () => { - dbMock._stmt.get.mockReturnValueOnce({ value: 'false' }); - expect(await svc().getPlacesEnrich()).toEqual({ enabled: false }); + setAppSetting('places_enrich_enabled', 'false'); + expect(await svc.getPlacesEnrich()).toEqual({ enabled: false }); for (const value of ['true', 'garbage', '']) { - dbMock._stmt.get.mockReturnValueOnce({ value }); - expect(await svc().getPlacesEnrich()).toEqual({ enabled: true }); + setAppSetting('places_enrich_enabled', value); + expect(await svc.getPlacesEnrich()).toEqual({ enabled: true }); } }); it('ADDONS-SVC-085 the setter persists the literal string and echoes the boolean back', async () => { - expect(await svc().updatePlacesEnrich(false)).toEqual({ enabled: false }); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith('places_enrich_enabled', 'false'); - expect(await svc().updatePlacesEnrich(true)).toEqual({ enabled: true }); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith('places_enrich_enabled', 'true'); + expect(await svc.updatePlacesEnrich(false)).toEqual({ enabled: false }); + expect(rawAppSetting('places_enrich_enabled')).toEqual({ key: 'places_enrich_enabled', value: 'false' }); + + expect(await svc.updatePlacesEnrich(true)).toEqual({ enabled: true }); + expect(rawAppSetting('places_enrich_enabled')).toEqual({ key: 'places_enrich_enabled', value: 'true' }); }); }); @@ -431,34 +494,33 @@ describe('AddonsService places enrichment flag', () => { * unrecognised-value case the booleans get for free: anything that is not a * known provider must read as Transitous rather than silently billing the * install's Google key. + * + * NOT one of this task's 11 app_settings/addons/photo_provider(_fields) sites + * (transit-provider.ts and instance-api-keys.ts are other domains, out of this + * plan's six) — still on DatabaseService, exercised here over the same real + * connection every other case uses. */ describe('AddonsService transit provider', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - it('ADDONS-SVC-086 an unset provider reads as Transitous, with no key anywhere', async () => { - dbMock._stmt.get.mockReturnValue(undefined); - expect(await svc().getTransitProvider()).toEqual({ provider: 'transitous', googleKeySource: null }); + expect(await svc.getTransitProvider()).toEqual({ provider: 'transitous', googleKeySource: null }); }); it('ADDONS-SVC-087 only a known provider name is honoured', async () => { - dbMock._stmt.get.mockReturnValueOnce({ value: 'google' }).mockReturnValue(undefined); - expect((await svc().getTransitProvider()).provider).toBe('google'); + setAppSetting('transit_provider', 'google'); + expect((await svc.getTransitProvider()).provider).toBe('google'); for (const value of ['someday-maps', '', 'GOOGLE']) { - dbMock._stmt.get.mockReset(); - dbMock._stmt.get.mockReturnValueOnce({ value }).mockReturnValue(undefined); - expect((await svc().getTransitProvider()).provider).toBe('transitous'); + setAppSetting('transit_provider', value); + expect((await svc.getTransitProvider()).provider).toBe('transitous'); } }); it('ADDONS-SVC-088 the setter persists the name and echoes it back', async () => { - dbMock._stmt.get.mockReturnValue(undefined); - expect((await svc().updateTransitProvider('google')).provider).toBe('google'); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith('transit_provider', 'google'); - expect((await svc().updateTransitProvider('transitous')).provider).toBe('transitous'); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith('transit_provider', 'transitous'); + expect((await svc.updateTransitProvider('google')).provider).toBe('google'); + expect(rawAppSetting('transit_provider')).toEqual({ key: 'transit_provider', value: 'google' }); + + expect((await svc.updateTransitProvider('transitous')).provider).toBe('transitous'); + expect(rawAppSetting('transit_provider')).toEqual({ key: 'transit_provider', value: 'transitous' }); }); /** @@ -467,23 +529,19 @@ describe('AddonsService transit provider', () => { * means "works for this admin, Transitous for everybody else". */ it('ADDONS-SVC-089 reports where the Google key resolved from', async () => { - // provider row, then the instance maps_api_key row. - dbMock._stmt.get.mockReset(); - dbMock._stmt.get.mockReturnValueOnce({ value: 'google' }).mockReturnValueOnce({ value: 'instance-key' }); - expect((await svc().getTransitProvider(7)).googleKeySource).toBe('instance'); + setAppSetting('transit_provider', 'google'); + setAppSetting('maps_api_key', 'instance-key'); + expect((await svc.getTransitProvider(7)).googleKeySource).toBe('instance'); // No instance row, but the caller's own users column has one. - dbMock._stmt.get.mockReset(); - dbMock._stmt.get - .mockReturnValueOnce({ value: 'google' }) - .mockReturnValueOnce(undefined) - .mockReturnValueOnce({ maps_api_key: 'personal-key' }); - expect((await svc().getTransitProvider(7)).googleKeySource).toBe('user-row'); + testDb.prepare("DELETE FROM app_settings WHERE key = 'maps_api_key'").run(); + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET maps_api_key = ? WHERE id = ?').run('personal-key', user.id); + expect((await svc.getTransitProvider(user.id)).googleKeySource).toBe('user-row'); // Nothing anywhere. - dbMock._stmt.get.mockReset(); - dbMock._stmt.get.mockReturnValue(undefined); - expect((await svc().getTransitProvider(7)).googleKeySource).toBeNull(); + const { user: bareUser } = createUser(testDb); + expect((await svc.getTransitProvider(bareUser.id)).googleKeySource).toBeNull(); }); }); @@ -496,76 +554,49 @@ describe('AddonsService transit provider', () => { * diverge the admin panel shows "off" while the log keeps collecting picks. */ describe('AddonsService place shadow flag', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - it('ADDONS-SVC-090 an unset flag reads as OFF, and so does every value but the literal "true"', async () => { - dbMock._stmt.get.mockReturnValueOnce(undefined); - expect(await svc().getPlaceShadow()).toEqual({ enabled: false }); - expect(dbMock.prepare).toHaveBeenLastCalledWith('SELECT value FROM app_settings WHERE key = ?'); - expect(dbMock._stmt.get).toHaveBeenLastCalledWith('place_shadow_enabled'); + expect(await svc.getPlaceShadow()).toEqual({ enabled: false }); for (const value of ['false', 'TRUE', '1', '']) { - dbMock._stmt.get.mockReturnValueOnce({ value }); - expect(await svc().getPlaceShadow()).toEqual({ enabled: false }); + setAppSetting('place_shadow_enabled', value); + expect(await svc.getPlaceShadow()).toEqual({ enabled: false }); } }); it('ADDONS-SVC-091 a stored "true" reads as ON', async () => { - dbMock._stmt.get.mockReturnValueOnce({ value: 'true' }); - expect(await svc().getPlaceShadow()).toEqual({ enabled: true }); + setAppSetting('place_shadow_enabled', 'true'); + expect(await svc.getPlaceShadow()).toEqual({ enabled: true }); }); it('ADDONS-SVC-092 the setter round-trips through the getter under its own key', async () => { - // Keyed store instead of an echo assertion: the write has to produce the - // exact string the read compares against, so a setter persisting '1' fails - // here rather than silently reading back OFF in production. - const stored = new Map(); - dbMock._stmt.run.mockImplementation((key: string, value: string) => { - stored.set(key, value); - }); - dbMock._stmt.get.mockImplementation((key: string) => { - const value = stored.get(key); - return value === undefined ? undefined : { value }; - }); + expect(await svc.updatePlaceShadow(true)).toEqual({ enabled: true }); + expect(rawAppSetting('place_shadow_enabled')).toEqual({ key: 'place_shadow_enabled', value: 'true' }); + expect(await svc.getPlaceShadow()).toEqual({ enabled: true }); + // a sibling switch must not ride along + expect(await svc.getPlacesDetails()).toEqual({ enabled: false }); - expect(await svc().updatePlaceShadow(true)).toEqual({ enabled: true }); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith('place_shadow_enabled', 'true'); - expect(await svc().getPlaceShadow()).toEqual({ enabled: true }); - // a sibling switch must not ride along on the shared statement - expect(await svc().getPlacesDetails()).toEqual({ enabled: false }); - - expect(await svc().updatePlaceShadow(false)).toEqual({ enabled: false }); - expect(dbMock._stmt.run).toHaveBeenLastCalledWith('place_shadow_enabled', 'false'); - expect(await svc().getPlaceShadow()).toEqual({ enabled: false }); - expect([...stored.keys()]).toEqual(['place_shadow_enabled']); + expect(await svc.updatePlaceShadow(false)).toEqual({ enabled: false }); + expect(rawAppSetting('place_shadow_enabled')).toEqual({ key: 'place_shadow_enabled', value: 'false' }); + expect(await svc.getPlaceShadow()).toEqual({ enabled: false }); + // no sibling key was ever written + expect(rawAppSetting('places_details_enabled')).toBeUndefined(); }); it('ADDONS-SVC-093 answers the same as PlaceShadowService.enabled() for every stored value', async () => { - const shadow = new PlaceShadowService(new DatabaseService(dbConn)); - const rows: Array<[{ value: string } | undefined, boolean]> = [ + const shadow = new PlaceShadowService(new DatabaseService(testDb)); + const cases: Array<[string | undefined, boolean]> = [ [undefined, false], - [{ value: 'true' }, true], - [{ value: 'false' }, false], - [{ value: 'garbage' }, false], + ['true', true], + ['false', false], + ['garbage', false], ]; - for (const [row, expected] of rows) { - // one read for the admin getter, one for the service that gates the log - dbMock._stmt.get.mockReturnValueOnce(row).mockReturnValueOnce(row); - - // The shared statement hands both readers this row whatever they ask for, - // so the key each one names has to be asserted too: the getter binds it, - // the gate inlines it, and a divergence there would still look like - // agreement on the value alone. - expect(await svc().getPlaceShadow()).toEqual({ enabled: expected }); - expect(dbMock._stmt.get).toHaveBeenLastCalledWith('place_shadow_enabled'); + for (const [value, expected] of cases) { + if (value === undefined) testDb.prepare("DELETE FROM app_settings WHERE key = 'place_shadow_enabled'").run(); + else setAppSetting('place_shadow_enabled', value); + expect(await svc.getPlaceShadow()).toEqual({ enabled: expected }); expect(await shadow.enabled()).toBe(expected); - expect(dbMock.prepare).toHaveBeenLastCalledWith( - "SELECT value FROM app_settings WHERE key = 'place_shadow_enabled'", - ); } }); }); diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 84869e75a..feb91c4df 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -60,7 +60,7 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser, createUserWithMfa, createAdmin, createInviteToken } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { SettingsService } from '../../../src/nest/settings/settings.service'; import { AtlasService } from '../../../src/nest/atlas/atlas.service'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; @@ -104,7 +104,7 @@ beforeAll(async () => { auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection)); svc = new AdminService( dbs, - new AddonsService(dbs), + await createTestAddonsService(testDb, dbs), new PasskeyService(dbs, auth, webauthn, await createTestUnitOfWork(dbs.connection)), auth, permissions, diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index 2423c270f..b4b97be07 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -43,7 +43,7 @@ vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn(), import { DatabaseService } from '../../../src/nest/database/database.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { SettingsService } from '../../../src/nest/settings/settings.service'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; @@ -82,7 +82,7 @@ beforeAll(async () => { auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs), new EphemeralTokenService(), new AllowedFileTypesService(dbs), await createTestUnitOfWork(dbs.connection)); svc = new AdminService( dbs, - new AddonsService(dbs), + await createTestAddonsService(testDb, dbs), new PasskeyService(dbs, auth, webauthn, await createTestUnitOfWork(dbs.connection)), auth, permissions, diff --git a/server/tests/unit/nest/dawarich-sync.service.test.ts b/server/tests/unit/nest/dawarich-sync.service.test.ts index 98849549f..3110a520f 100644 --- a/server/tests/unit/nest/dawarich-sync.service.test.ts +++ b/server/tests/unit/nest/dawarich-sync.service.test.ts @@ -45,7 +45,8 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb, setAddonEnabled } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { DawarichSyncService } from '../../../src/nest/integrations/dawarich-sync.service'; import { createTestUnitOfWork } from '../../helpers/test-uow'; import { DawarichError } from '../../../src/nest/integrations/dawarich.client'; @@ -112,7 +113,7 @@ const dawarich = { // Direct construction over the shared test connection — no TestingModule // (repo convention for DI-native service unit tests). const dbs = new DatabaseService(testDb); -const addons = new AddonsService(dbs); +let addons: AddonsService; let svc: DawarichSyncService; // ── Fixtures ───────────────────────────────────────────────────────────────── @@ -226,6 +227,7 @@ function withVisits(...visits: DawarichVisitRaw[]): void { beforeAll(async () => { createTables(testDb); runMigrations(testDb); + addons = await createTestAddonsService(testDb, dbs); svc = new DawarichSyncService(dbs, addons, client, dawarich, await createTestUnitOfWork(dbs.connection)); }); diff --git a/server/tests/unit/nest/discovery.controller.test.ts b/server/tests/unit/nest/discovery.controller.test.ts index 3bf18465a..0e86818d0 100644 --- a/server/tests/unit/nest/discovery.controller.test.ts +++ b/server/tests/unit/nest/discovery.controller.test.ts @@ -28,6 +28,7 @@ import { DiscoveryController } from '../../../src/nest/platform/discovery.contro import { PlatformModule } from '../../../src/nest/platform/platform.module'; import { ALL_SCOPES } from '../../../src/mcp/scopes'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import type { MikroORM } from '@mikro-orm/core'; function makeRes() { const res = { @@ -47,6 +48,19 @@ function addons(enabled: boolean) { return { isAddonEnabled: vi.fn(() => enabled) } as unknown as AddonsService; } +/** + * createMcpMetadataMiddleware wraps the addon check in withRequestContext + * (Plan 3a Task 4: this pathless pre-init mount runs before + * @mikro-orm/nestjs's per-request EM fork, so once isAddonEnabled became + * repository-backed it needed its own context). The mocked AddonsService + * above never touches the ORM, so this fake only needs to satisfy + * RequestContext.create's em.fork()/em.name contract. + */ +function fakeOrm(): MikroORM { + const em = { name: 'default', fork: () => ({ name: 'default' }) }; + return { em } as unknown as MikroORM; +} + beforeEach(() => { vi.clearAllMocks(); h.getMcpSafeUrl.mockReturnValue('https://trek.example.test'); @@ -105,7 +119,7 @@ async function flushMicrotasks(): Promise { describe('createMcpMetadataMiddleware', () => { it('DISC-010: 404s a /.well-known path with an empty body when MCP is disabled', async () => { - const mw = createMcpMetadataMiddleware(new DiscoveryMetadataService(), addons(false)); + const mw = createMcpMetadataMiddleware(new DiscoveryMetadataService(), addons(false), fakeOrm()); const res = makeRes(); const next = vi.fn(); mw({ path: '/.well-known/oauth-authorization-server' } as never, res as never, next); @@ -117,7 +131,7 @@ describe('createMcpMetadataMiddleware', () => { }); it('DISC-011: delegates a /.well-known path to the SDK router when MCP is enabled', async () => { - const mw = createMcpMetadataMiddleware(new DiscoveryMetadataService(), addons(true)); + const mw = createMcpMetadataMiddleware(new DiscoveryMetadataService(), addons(true), fakeOrm()); const next = vi.fn(); mw({ path: '/.well-known/oauth-authorization-server' } as never, makeRes() as never, next); await flushMicrotasks(); @@ -126,16 +140,16 @@ describe('createMcpMetadataMiddleware', () => { it('DISC-012: delegates non-well-known paths without consulting the addon gate', () => { const gate = addons(false); - const mw = createMcpMetadataMiddleware(new DiscoveryMetadataService(), gate); + const mw = createMcpMetadataMiddleware(new DiscoveryMetadataService(), gate, fakeOrm()); mw({ path: '/api/trips' } as never, makeRes() as never, vi.fn()); expect(gate.isAddonEnabled).not.toHaveBeenCalled(); expect(h.metaRouter).toHaveBeenCalled(); }); - it('DISC-013: the factory provider wires the token to the factory with its two deps', () => { + it('DISC-013: the factory provider wires the token to the factory with its three deps (MikroORM added — Plan 3a Task 4)', () => { expect(mcpMetadataMiddlewareProvider.provide).toBe(MCP_METADATA_MIDDLEWARE); expect(mcpMetadataMiddlewareProvider.useFactory).toBe(createMcpMetadataMiddleware); - expect(mcpMetadataMiddlewareProvider.inject.length).toBe(2); + expect(mcpMetadataMiddlewareProvider.inject.length).toBe(3); }); }); diff --git a/server/tests/unit/nest/unified-memories.service.test.ts b/server/tests/unit/nest/unified-memories.service.test.ts index 19a86b6d2..7acde1e21 100644 --- a/server/tests/unit/nest/unified-memories.service.test.ts +++ b/server/tests/unit/nest/unified-memories.service.test.ts @@ -43,7 +43,7 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb, setAddonEnabled } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { ADDON_IDS } from '../../../src/addons'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { UnifiedMemoriesService } from '../../../src/nest/memories/unified-memories.service'; import { MemoriesAccessService } from '../../../src/nest/memories/memories-access.service'; import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; @@ -81,7 +81,7 @@ beforeAll(async () => { {} as SynologyService, new MemoriesAccessService(dbs), notificationsStub(), - new AddonsService(dbs), + await createTestAddonsService(testDb, dbs), await createTestUnitOfWork(testDb), ); }); diff --git a/server/tests/unit/plugins/operator-egress.test.ts b/server/tests/unit/plugins/operator-egress.test.ts index 1d00df3a7..3e8627470 100644 --- a/server/tests/unit/plugins/operator-egress.test.ts +++ b/server/tests/unit/plugins/operator-egress.test.ts @@ -30,7 +30,7 @@ import { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.s import { createPluginRuntime } from '../../helpers/plugin-host'; import { parseManifest, ManifestError } from '../../../src/nest/plugins/install/manifest'; import { makeHostAllow } from '../../../src/nest/plugins/runtime/egress-policy'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; function install(id: string, operatorEgress: boolean, perms: string[] = ['http:outbound:gotify.net']) { testDb.prepare( @@ -174,7 +174,7 @@ describe('the admin list surfaces operator egress (so the chip can be shown)', ( // one over the same DB. These fixtures declare no dependencies, so it is never consulted. const listPlugins = async () => { const dbs = new DatabaseService(dbConn); - return (await new PluginsService(dbs, new AddonsService(dbs)).list()).plugins; + return (await new PluginsService(dbs, await createTestAddonsService(testDb, dbs)).list()).plugins; }; const before = await listPlugins(); diff --git a/server/tests/unit/plugins/plugin-instance-settings.test.ts b/server/tests/unit/plugins/plugin-instance-settings.test.ts index 2067f145d..b8c8f900e 100644 --- a/server/tests/unit/plugins/plugin-instance-settings.test.ts +++ b/server/tests/unit/plugins/plugin-instance-settings.test.ts @@ -30,7 +30,8 @@ import { PluginsController } from '../../../src/nest/plugins/plugins.controller' import { PluginConsentRequired, type PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service'; import type { PluginRegistryService } from '../../../src/nest/plugins/registry/registry.service'; import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { createPluginRuntime } from '../../helpers/plugin-host'; import { discoverPlugins } from '../../../src/nest/plugins/install/discovery'; @@ -52,10 +53,8 @@ function declareField(pluginId: string, key: string, scope: 'instance' | 'user', .run(pluginId, key, key, opts.required ? 1 : 0, opts.secret ? 1 : 0, scope, opts.options ?? null, opts.sortOrder ?? 0); } -const svc = () => { - const dbs = new DatabaseService(dbConn); - return new PluginsService(dbs, new AddonsService(dbs)); -}; +let addonsService: AddonsService; +const svc = () => new PluginsService(new DatabaseService(dbConn), addonsService); /** * Installs a real plugin through the manifest -> discovery pipeline (not a direct @@ -74,9 +73,10 @@ async function installFixturePlugin(opts: { settings: Array { +beforeAll(async () => { createTables(testDb); runMigrations(testDb); + addonsService = await createTestAddonsService(testDb, new DatabaseService(dbConn)); }); beforeEach(() => { testDb.prepare('DELETE FROM plugins').run(); diff --git a/server/tests/unit/plugins/plugin-user-settings.test.ts b/server/tests/unit/plugins/plugin-user-settings.test.ts index 05aff958e..db12fa8db 100644 --- a/server/tests/unit/plugins/plugin-user-settings.test.ts +++ b/server/tests/unit/plugins/plugin-user-settings.test.ts @@ -17,7 +17,7 @@ const { getDb } = vi.hoisted(() => ({ getDb: { current: null as unknown } })); vi.mock('../../../src/db/database', () => ({ get db() { return getDb.current; } })); import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import Database from 'better-sqlite3'; import { PluginsService } from '../../../src/nest/plugins/plugins.service'; @@ -43,10 +43,10 @@ describe('per-user plugin settings', () => { let svc: PluginsService; // AddonsService only feeds PluginsService.list(), which no case here calls, but it is a // real collaborator on the same connection rather than a stand-in. - beforeEach(() => { + beforeEach(async () => { getDb.current = freshDb(); const dbs = new DatabaseService(dbConn); - svc = new PluginsService(dbs, new AddonsService(dbs)); + svc = new PluginsService(dbs, await createTestAddonsService(getDb.current as Database.Database, dbs)); }); it('lists only the user-scope fields, in order', async () => { @@ -95,10 +95,10 @@ describe('manifest defaults reach the runtime reads', () => { // through ctx.settings.get() / the channel dispatch's readAll — otherwise a plugin // that ships a sensible default serves nobody until every user opens the form. let svc: PluginsService; - beforeEach(() => { + beforeEach(async () => { getDb.current = freshDb(); const dbs = new DatabaseService(dbConn); - svc = new PluginsService(dbs, new AddonsService(dbs)); + svc = new PluginsService(dbs, await createTestAddonsService(getDb.current as Database.Database, dbs)); const ins = (getDb.current as import('better-sqlite3').Database).prepare( 'INSERT INTO plugin_settings_fields (plugin_id, field_key, input_type, required, secret, scope, sort_order, default_value) VALUES (?,?,?,?,?,?,?,?)', ); @@ -138,10 +138,10 @@ describe('hasRequired applies the same "filled" rule as the save gate', () => { // it disagreed with assertRequiredFilled() a save the form accepted could still leave // the user "not configured" (or the reverse). let svc: PluginsService; - beforeEach(() => { + beforeEach(async () => { getDb.current = freshDb(); const dbs = new DatabaseService(dbConn); - svc = new PluginsService(dbs, new AddonsService(dbs)); + svc = new PluginsService(dbs, await createTestAddonsService(getDb.current as Database.Database, dbs)); (getDb.current as import('better-sqlite3').Database) .prepare('INSERT INTO plugin_settings_fields (plugin_id, field_key, input_type, required, secret, scope, sort_order) VALUES (?,?,?,?,?,?,?)') .run('p', 'consent', 'checkbox', 1, 0, 'user', 9); diff --git a/server/tests/unit/plugins/plugins-service.test.ts b/server/tests/unit/plugins/plugins-service.test.ts index 717839d94..1ae3317a7 100644 --- a/server/tests/unit/plugins/plugins-service.test.ts +++ b/server/tests/unit/plugins/plugins-service.test.ts @@ -2,7 +2,7 @@ * The read-side plugin service + controller (#plugins, M0). Lists installed * plugins and reports whether the runtime is enabled (TREK_PLUGINS_ENABLED). */ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest'; const { testDb } = vi.hoisted(() => { const Database = require('better-sqlite3'); @@ -20,12 +20,21 @@ vi.mock('../../../src/db/database', () => ({ db: testDb })); import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { PluginsService } from '../../../src/nest/plugins/plugins.service'; import { PluginsController } from '../../../src/nest/plugins/plugins.controller'; import { PluginsFeedController } from '../../../src/nest/plugins/plugins-feed.controller'; import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; +// AddonsService only feeds PluginsService.list()'s required-addon-dependency +// resolution; none of the fixtures below declare any, so it is built once +// (over the same connection) rather than reconstructed at every call site. +let addonsService: AddonsService; +beforeAll(async () => { + addonsService = await createTestAddonsService(testDb, new DatabaseService(dbConn)); +}); + beforeEach(() => { testDb.exec('DELETE FROM plugins'); testDb.exec('DELETE FROM plugin_settings_fields'); @@ -42,7 +51,7 @@ describe('PluginsService.list', () => { .run('flight', 'Flight', 'desc', 'widget', 'inactive', '1.0.0'); process.env.TREK_PLUGINS_ENABLED = 'true'; - const out = await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); + const out = await new PluginsService(new DatabaseService(dbConn), addonsService).list(); expect(out.enabled).toBe(true); expect(out.plugins).toHaveLength(1); expect(out.plugins[0]).toMatchObject({ id: 'flight', name: 'Flight', status: 'inactive' }); @@ -63,7 +72,7 @@ describe('PluginsService.list', () => { it('reports the switch off and an outgrown plugin as hostIncompatible by default', async () => { process.env.APP_VERSION = '4.1.0'; seed(); - const out = await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); + const out = await new PluginsService(new DatabaseService(dbConn), addonsService).list(); expect(out.ignoreTrekRange).toBe(false); expect(out.plugins[0]).toMatchObject({ dependencyStatus: 'hostIncompatible', trekRangeBypassed: null }); }); @@ -72,7 +81,7 @@ describe('PluginsService.list', () => { process.env.APP_VERSION = '4.1.0'; process.env.TREK_PLUGINS_IGNORE_TREK_RANGE = '1'; seed(); - const out = await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); + const out = await new PluginsService(new DatabaseService(dbConn), addonsService).list(); expect(out.ignoreTrekRange).toBe(true); expect(out.plugins[0]).toMatchObject({ dependencyStatus: 'ok', @@ -84,7 +93,7 @@ describe('PluginsService.list', () => { process.env.APP_VERSION = '3.5.0'; process.env.TREK_PLUGINS_IGNORE_TREK_RANGE = '1'; seed(); - const out = await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); + const out = await new PluginsService(new DatabaseService(dbConn), addonsService).list(); expect(out.plugins[0]).toMatchObject({ dependencyStatus: 'ok', trekRangeBypassed: null }); }); }); @@ -97,7 +106,7 @@ describe('PluginsService.list', () => { .prepare("INSERT INTO plugins (id, name, type, status, version) VALUES ('free','Free','widget','inactive','1.0.0')") .run(); - const out = await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); + const out = await new PluginsService(new DatabaseService(dbConn), addonsService).list(); expect(out.plugins.find((p) => p.id === 'held')).toMatchObject({ updateHold: true }); expect(out.plugins.find((p) => p.id === 'free')).toMatchObject({ updateHold: false }); }); @@ -106,7 +115,7 @@ describe('PluginsService.list', () => { testDb .prepare("INSERT INTO plugins (id, name, type, status, version, update_hold) VALUES ('held','Held','widget','inactive','1.0.0',1)") .run(); - const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))); + const svc = new PluginsService(new DatabaseService(dbConn), addonsService); expect(await svc.resumeUpdates('held')).toBe(true); expect(testDb.prepare("SELECT update_hold FROM plugins WHERE id='held'").get()).toMatchObject({ update_hold: 0 }); @@ -118,14 +127,14 @@ describe('PluginsService.list', () => { .prepare('INSERT INTO plugins (id, name, description, type, status, version) VALUES (?,?,?,?,?,?)') .run('flight', 'Flight', 'desc', 'widget', 'inactive', '1.0.0'); - const out = await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); + const out = await new PluginsService(new DatabaseService(dbConn), addonsService).list(); expect(out.enabled).toBe(true); expect(out.plugins).toHaveLength(1); }); it('reports disabled when the kill switch is off (TREK_PLUGINS_ENABLED=false)', async () => { process.env.TREK_PLUGINS_ENABLED = 'false'; - const out = await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); + const out = await new PluginsService(new DatabaseService(dbConn), addonsService).list(); expect(out.enabled).toBe(false); expect(out.plugins).toEqual([]); }); @@ -145,7 +154,7 @@ describe('PluginsService.list', () => { const key = 'RWTvBn0aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789abcd'; insert('signed-one', 'acme/signed-one', key); - const p = (await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list()).plugins[0]; + const p = (await new PluginsService(new DatabaseService(dbConn), addonsService).list()).plugins[0]; expect(p.signed).toBe(true); // Short head…tail, for eyeballing against what the author reads out over the // phone. NOT a confidentiality measure — the key is public, and the re-trust @@ -155,7 +164,7 @@ describe('PluginsService.list', () => { it('reports unsigned for a registry plugin with no pinned key', async () => { insert('plain', 'acme/plain', null); - const p = (await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list()).plugins[0]; + const p = (await new PluginsService(new DatabaseService(dbConn), addonsService).list()).plugins[0]; expect(p.signed).toBe(false); expect(p.keyFingerprint).toBeNull(); }); @@ -163,7 +172,7 @@ describe('PluginsService.list', () => { it('reports unsigned for a sideloaded and a dev-linked plugin (they carry no key)', async () => { insert('uploaded', 'local:upload', null); insert('linked', 'local:link', null); - const plugins = (await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list()).plugins; + const plugins = (await new PluginsService(new DatabaseService(dbConn), addonsService).list()).plugins; expect(plugins.map((p) => [p.id, p.signed, p.source_repo])).toEqual([ ['linked', false, 'local:link'], ['uploaded', false, 'local:upload'], @@ -177,7 +186,7 @@ describe('PluginsService.list', () => { .prepare('UPDATE plugins SET update_block_code = ?, update_block_detail = ?, update_block_version = ? WHERE id = ?') .run('SIGNATURE_KEY_CHANGED', 'the key changed', '2.0.0', 'blocked'); - const byId = Object.fromEntries((await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list()).plugins.map((p) => [p.id, p])); + const byId = Object.fromEntries((await new PluginsService(new DatabaseService(dbConn), addonsService).list()).plugins.map((p) => [p.id, p])); expect(byId.blocked.updateBlock).toEqual({ code: 'SIGNATURE_KEY_CHANGED', detail: 'the key changed', version: '2.0.0' }); expect(byId.fine.updateBlock).toBeNull(); }); @@ -187,7 +196,7 @@ describe('PluginsService.list', () => { // PluginListItem is an interface, so it has no implicit index signature and cannot // be narrowed to a record directly. Widening through unknown is what lets this case // probe for a key the contract deliberately does not declare. - const p = (await new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list()) + const p = (await new PluginsService(new DatabaseService(dbConn), addonsService).list()) .plugins[0] as unknown as Record; expect(p.author_pubkey).toBeUndefined(); }); @@ -314,7 +323,7 @@ describe('PluginsService instance config', () => { testDb.prepare("INSERT INTO plugin_settings_fields (plugin_id, field_key, scope, secret) VALUES ('x','api_key','instance',1)").run(); testDb.prepare("INSERT INTO plugin_settings_fields (plugin_id, field_key, scope, secret) VALUES ('x','server','instance',0)").run(); - const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))); + const svc = new PluginsService(new DatabaseService(dbConn), addonsService); const masked = await svc.updateInstanceConfig('x', { api_key: 'super-secret', server: 'https://h' }); // client gets the masked view expect(masked.api_key).toBe('••••••••'); @@ -338,7 +347,7 @@ describe('PluginsService instance config', () => { testDb.prepare("INSERT INTO plugins (id, name, status, config) VALUES ('y','Y','inactive','{}')").run(); testDb.prepare("INSERT INTO plugin_settings_fields (plugin_id, field_key, scope, secret) VALUES ('y','server','instance',0)").run(); - const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))); + const svc = new PluginsService(new DatabaseService(dbConn), addonsService); const masked = await svc.updateInstanceConfig('y', { server: 'https://h', smuggled: 'nope' }); expect(masked.server).toBe('https://h'); @@ -348,8 +357,8 @@ describe('PluginsService instance config', () => { }); it('throws for an unknown plugin', async () => { - await expect(new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).updateInstanceConfig('nope', {})).rejects.toThrow(/not found/); - await expect(new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).getInstanceConfig('nope')).rejects.toThrow(/not found/); + await expect(new PluginsService(new DatabaseService(dbConn), addonsService).updateInstanceConfig('nope', {})).rejects.toThrow(/not found/); + await expect(new PluginsService(new DatabaseService(dbConn), addonsService).getInstanceConfig('nope')).rejects.toThrow(/not found/); }); }); @@ -357,7 +366,7 @@ describe('PluginsService error log', () => { beforeEach(() => testDb.exec('DELETE FROM plugin_error_log')); it('lists and clears a plugin error log', async () => { testDb.prepare("INSERT INTO plugin_error_log (plugin_id, level, message) VALUES ('p','error','boom')").run(); - const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))); + const svc = new PluginsService(new DatabaseService(dbConn), addonsService); expect(await svc.errors('p')).toEqual([{ ts: '2026-01-01', level: 'error', message: 'boom' }]); await svc.clearErrors('p'); expect(await svc.errors('p')).toEqual([]); diff --git a/server/tests/unit/plugins/request-context.test.ts b/server/tests/unit/plugins/request-context.test.ts index 50c0067e2..128f15338 100644 --- a/server/tests/unit/plugins/request-context.test.ts +++ b/server/tests/unit/plugins/request-context.test.ts @@ -27,7 +27,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { createSnapshotTestDb } from '../../helpers/db-mock'; import { createUser } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service'; import { PluginSupervisor } from '../../../src/nest/plugins/supervisor/plugin-supervisor'; @@ -64,7 +64,7 @@ beforeAll(async () => { t = await createTestOrm(testDb, { allowGlobalContext: false }); const dbs = new DatabaseService(testDb); permissions = new PermissionsService(t.repo(AppSettings) as AppSettingsRepository, new UnitOfWork(t.em)); - guards = new PluginGuards(dbs, permissions, new AddonsService(dbs)); + guards = new PluginGuards(dbs, permissions, await createTestAddonsService(testDb, dbs)); userId = createUser(testDb, { role: 'user' }).user.id; // An admin has tightened trip_create from its 'everybody' default. testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_trip_create', 'admin')").run(); diff --git a/server/tests/unit/plugins/settings-isolation.test.ts b/server/tests/unit/plugins/settings-isolation.test.ts index 0817c01e4..954d9e51e 100644 --- a/server/tests/unit/plugins/settings-isolation.test.ts +++ b/server/tests/unit/plugins/settings-isolation.test.ts @@ -20,7 +20,7 @@ vi.mock('../../../src/db/database', () => dbMock); import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { AuditService } from '../../../src/nest/audit/audit.service'; -import { AddonsService } from '../../../src/nest/addons/addons.service'; +import { createTestAddonsService } from '../../helpers/test-addons'; vi.mock('../../../src/config', () => ({ JWT_SECRET: 'x'.repeat(40), ENCRYPTION_KEY: 'a'.repeat(64), updateJwtSecret: () => {} })); import { createTables } from '../../../src/db/schema'; @@ -140,7 +140,7 @@ describe('a plugin channel label is bounded by the host', () => { const rt = new PluginRuntimeService( new DatabaseService(dbConn), new AuditService(t.repo(AuditLog) as AuditLogRepository, t.repo(Users) as UsersRepository), - new AddonsService(new DatabaseService(dbConn)), + await createTestAddonsService(dbConn), userSettings(), ); // Stand the plugin up as a granted, active notificationChannel provider.