From a3e6eb52a4cc3904828a5825243a0d69a6a3dc07 Mon Sep 17 00:00:00 2001 From: jubnl Date: Mon, 27 Jul 2026 22:58:09 +0200 Subject: [PATCH] chore: fold exchangeRateService into the nest budget domain --- server/src/mcp/tools/budget.ts | 2 +- server/src/nest/README.md | 14 +- server/src/nest/budget/budget.module.ts | 5 +- server/src/nest/budget/budget.service.ts | 5 +- .../src/nest/budget/exchange-rates.bridge.ts | 32 ++++ .../src/nest/budget/exchange-rates.service.ts | 87 +++++++++ server/src/nest/plugins/DI-MIGRATION.md | 8 +- .../plugins/host/plugin-host-deps.factory.ts | 11 +- server/src/services/budgetService.ts | 2 +- server/src/services/exchangeRateService.ts | 75 -------- server/tests/helpers/plugin-host.ts | 5 +- server/tests/unit/nest/budget.service.test.ts | 9 +- .../unit/nest/exchange-rates.service.test.ts | 170 ++++++++++++++++++ .../plugins/plugin-host-deps.factory.test.ts | 14 +- .../tests/unit/services/budgetService.test.ts | 2 +- .../unit/services/budgetServiceDb.test.ts | 2 +- 16 files changed, 340 insertions(+), 103 deletions(-) create mode 100644 server/src/nest/budget/exchange-rates.bridge.ts create mode 100644 server/src/nest/budget/exchange-rates.service.ts delete mode 100644 server/src/services/exchangeRateService.ts create mode 100644 server/tests/unit/nest/exchange-rates.service.test.ts diff --git a/server/src/mcp/tools/budget.ts b/server/src/mcp/tools/budget.ts index 6352ddc1a..7c4a4da4f 100644 --- a/server/src/mcp/tools/budget.ts +++ b/server/src/mcp/tools/budget.ts @@ -8,7 +8,7 @@ import { toggleMemberPaid, getBudgetItem, freezeForeignRate, calculateSettlement, listSettlements, createSettlement, updateSettlement, deleteSettlement, } from '../../services/budgetService'; -import { getRates } from '../../services/exchangeRateService'; +import { getRates } from '../../nest/budget/exchange-rates.bridge'; import { getTripOwner, listMembers } from '../../services/tripService'; import { safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_READONLY, diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 0fe9cf316..821d42765 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -184,10 +184,18 @@ require strings, scheduler, globalMiddleware, notifications) point at the logger directly; 8 controllers + `PluginRuntimeService` inject `AuditService`, and the domain e2e suites dropped the audit mock entirely — writeAudit runs for real against an `audit_log` table in their temp DBs). +The exchangeRateService fold followed (the pure-infra FX helper — Frankfurter +fetch + module-scoped 6h cache, no SQL, no controller, no MCP registrar of its +own — folded into `nest/budget/` as a dep-free `ExchangeRatesService`, injected +by `BudgetService` and `PluginHostDepsFactory` (its 17th constructor dep), with +an `exchange-rates.bridge.ts` for the legacy `services/budgetService.ts` FX +seams and the `mcp/tools/budget.ts` registrar; the rate **cache stays +module-scoped** like the permissions cache, so the bridge instance and the +container singleton share one cached upstream feed). Repeat these steps per -service (next up: the exchangeRateService fold → budgetService per the -dependency-honest order in `migration-graph.md`, now that the Wave-2 -permissions + auditLog pair is folded). This is a +service (next up: **budgetService** — frontier-ready now that its +exchangeRateService dependency is folded, per the dependency-honest order in +`migration-graph.md`). This is a **pure relocation** — byte-identical SQL, statuses, bodies, and error strings. The plugin RPC host is **no longer a bridge consumer**: since Option A of `src/nest/plugins/DI-MIGRATION.md` it diff --git a/server/src/nest/budget/budget.module.ts b/server/src/nest/budget/budget.module.ts index e0b31ad94..d9d006f7f 100644 --- a/server/src/nest/budget/budget.module.ts +++ b/server/src/nest/budget/budget.module.ts @@ -1,14 +1,15 @@ import { Module } from '@nestjs/common'; import { BudgetController } from './budget.controller'; import { BudgetService } from './budget.service'; +import { ExchangeRatesService } from './exchange-rates.service'; import { PermissionsModule } from '../permissions/permissions.module'; /** Budget domain (S4 — Phase 2 trip sub-domain). Registered in AppModule. */ @Module({ imports: [PermissionsModule], controllers: [BudgetController], - providers: [BudgetService], + providers: [BudgetService, ExchangeRatesService], // For in-container consumers (PluginHostDepsFactory). - exports: [BudgetService], + exports: [BudgetService, ExchangeRatesService], }) export class BudgetModule {} diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index e7693c306..6559a68ad 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -4,7 +4,7 @@ import { broadcast } from '../../websocket'; import { PermissionsService } from '../permissions/permissions.service'; import type { User } from '../../types'; import * as svc from '../../services/budgetService'; -import { getRates } from '../../services/exchangeRateService'; +import { ExchangeRatesService } from './exchange-rates.service'; type Trip = NonNullable>; @@ -18,6 +18,7 @@ export class BudgetService { constructor( private readonly dbs: DatabaseService, private readonly permissions: PermissionsService, + private readonly exchangeRates: ExchangeRatesService, ) {} private get db() { @@ -46,7 +47,7 @@ export class BudgetService { async settlement(tripId: string, base: string | undefined, tripCurrency: string) { const effectiveBase = (base || tripCurrency || 'EUR').toUpperCase(); - const rates = await getRates(effectiveBase); + const rates = await this.exchangeRates.getRates(effectiveBase); return svc.calculateSettlement(tripId, { base: effectiveBase, rates, tripCurrency }); } diff --git a/server/src/nest/budget/exchange-rates.bridge.ts b/server/src/nest/budget/exchange-rates.bridge.ts new file mode 100644 index 000000000..db66ddf5a --- /dev/null +++ b/server/src/nest/budget/exchange-rates.bridge.ts @@ -0,0 +1,32 @@ +import { ExchangeRatesService } from './exchange-rates.service'; + +/** + * Non-Nest entry point for the exchange-rates fold — for code running OUTSIDE + * the Nest container (the legacy `services/budgetService.ts` FX-freeze/rebase + * seams and the legacy `get_settlement_summary` registrar in + * src/mcp/tools/budget.ts; the plugin RPC host injects ExchangeRatesService via + * PluginHostDepsFactory). Exports the legacy services/exchangeRateService names + * 1:1, so repointing a consumer is an import-path-only diff. Inside the + * container, inject ExchangeRatesService instead. + * Delete this file when the budgetService migration lands. + * + * Module-level construction is safe: the service has no dependencies and no + * side effects at construction; its rate cache is module-scoped in + * exchange-rates.service.ts, so this instance and the DI singleton share one + * cached upstream feed. + */ +const fx = new ExchangeRatesService(); + +export function getRates(base: string) { + return fx.getRates(base); +} + +/** Unconsumed today (no call sites) — re-exported for 1:1 legacy surface parity. */ +export function convertWithRates( + amount: number, + from: string | null | undefined, + base: string, + rates: Record | null, +): number { + return fx.convertWithRates(amount, from, base, rates); +} diff --git a/server/src/nest/budget/exchange-rates.service.ts b/server/src/nest/budget/exchange-rates.service.ts new file mode 100644 index 000000000..01079671d --- /dev/null +++ b/server/src/nest/budget/exchange-rates.service.ts @@ -0,0 +1,87 @@ +import { Injectable } from '@nestjs/common'; + +/** + * Live exchange rates for the Costs/Budget money conversion. + * + * Fetches from api.frankfurter.dev (no key, already CSP-allowlisted for the + * dashboard widget) and caches per base currency in-memory for a few hours so a + * settlement request never hammers the upstream. Rates are "units of X per 1 + * base", so an amount in currency C converts to base as `amount / rates[C]`. + * + * Everything degrades gracefully: if the fetch fails (offline, upstream down), + * callers get `null`/identity conversion and amounts are treated as already in + * the base currency rather than throwing. + * + * Moved 1:1 from the legacy `services/exchangeRateService.ts` (the budget-domain + * fold): identical fetch, cache/coalescing behavior, `||` falsy-coercion + * defaults and null degradation. The cache and inflight maps are deliberately + * MODULE-scoped, not instance fields — the DI singleton and the + * exchange-rates.bridge instance wrap the same upstream feed, so they must + * share one cache (same reasoning as the permissions.service.ts cache). + */ + +const TTL_MS = 6 * 60 * 60 * 1000; // 6h +const cache = new Map; ts: number }>(); +const inflight = new Map | null>>(); + +async function fetchRates(base: string): Promise | null> { + try { + const res = await fetch(`https://api.frankfurter.dev/v2/rates?base=${encodeURIComponent(base)}`); + if (!res.ok) return null; + // Frankfurter returns an array of { date, base, quote, rate } and omits the + // base's own self-rate, so seed the map with `base = 1` then index by quote. + const data = (await res.json()) as Array<{ quote?: string; rate?: number }>; + if (!Array.isArray(data)) return null; + const rates: Record = { [base.toUpperCase()]: 1 }; + for (const r of data) { + if (r && typeof r.quote === 'string' && typeof r.rate === 'number') rates[r.quote] = r.rate; + } + return Object.keys(rates).length > 1 ? rates : null; + } catch { + return null; + } +} + +@Injectable() +export class ExchangeRatesService { + /** Rates map for `base` (cached). Returns null if unavailable. */ + async getRates(base: string): Promise | null> { + const key = (base || 'EUR').toUpperCase(); + const hit = cache.get(key); + const now = Date.now(); + if (hit && now - hit.ts < TTL_MS) return hit.rates; + + // Coalesce concurrent fetches for the same base. + let p = inflight.get(key); + if (!p) { + p = fetchRates(key).then(rates => { + if (rates) cache.set(key, { rates, ts: Date.now() }); + inflight.delete(key); + return rates; + }); + inflight.set(key, p); + } + const rates = await p; + // On failure fall back to the last cached value if we have one. + if (!rates && hit) return hit.rates; + return rates; + } + + /** + * Convert `amount` from `from` currency into `base` using a rates map obtained + * from getRates(base). Identity when same currency or the rate is missing. + */ + convertWithRates( + amount: number, + from: string | null | undefined, + base: string, + rates: Record | null, + ): number { + const fromCur = (from || base).toUpperCase(); + const baseCur = base.toUpperCase(); + if (fromCur === baseCur || !rates) return amount; + const r = rates[fromCur]; + if (!r || r <= 0) return amount; + return amount / r; + } +} diff --git a/server/src/nest/plugins/DI-MIGRATION.md b/server/src/nest/plugins/DI-MIGRATION.md index 27655eb22..277fedf99 100644 --- a/server/src/nest/plugins/DI-MIGRATION.md +++ b/server/src/nest/plugins/DI-MIGRATION.md @@ -19,7 +19,7 @@ The plugin runtime has three layers. Only the third is "un-Nest". |---|---|---|---| | Sandbox / transport | `supervisor/plugin-supervisor.ts`, `runtime/plugin-host-entry.ts`, `protocol/envelope.ts` | Forked child per plugin, IPC envelopes, permission tables, actor resolution, rate limiting, RSS policing | Intentionally not — this is the security boundary | | Enforcement | `host/rpc-host.ts` (~1330 lines) | `PluginRpcHost`: builds a per-plugin method map at spawn — a handler is registered **only if the plugin holds the unlocking permission** (registration = authorization). Depends only on the `HostDeps` interface | Neutral — decoupled by design | -| **Wiring** | `host/plugin-host-deps.factory.ts` (was `create-rpc-host.ts`) | Fills the ~130-member `HostDeps` object. Since Option A: an `@Injectable() PluginHostDepsFactory` with the DI-native domain services (`BudgetService`, `ReservationsService`, `TagsService`, `CategoriesService`, `TodoService`, `PluginOAuthService`) constructor-injected, fifteen domains as of 2026-07 (permissions joined with the Wave-2 pair); legacy `services/*` domains stay plain function imports until their own migration. Raw `db`/`broadcast` also stay until injectable equivalents exist | **Yes (Option A)** — drains further per §3 | +| **Wiring** | `host/plugin-host-deps.factory.ts` (was `create-rpc-host.ts`) | Fills the ~130-member `HostDeps` object. Since Option A: an `@Injectable() PluginHostDepsFactory` with the DI-native domain services (`BudgetService`, `ReservationsService`, `TagsService`, `CategoriesService`, `TodoService`, `PluginOAuthService`) constructor-injected, sixteen domains as of 2026-07 (exchange-rates joined with the budget-domain fold, after the Wave-2 permissions pair); legacy `services/*` domains stay plain function imports until their own migration. Raw `db`/`broadcast` also stay until injectable equivalents exist | **Yes (Option A)** — drains further per §3 | | Host state | `host/plugin-host-state.ts` | Deliberately module-level (NOT a provider): the per-plugin data-DB map (`getPluginDataDb`/`closePluginDataDb`) and daily broker budgets (`budgetFor`/`pluginBudgetUsage`). Must be one shared instance across host recreations, and is read by `PluginsService`, which the factory imports from — folding it into the injectable would create a provider cycle | Intentionally not | Key fact that made Option A cheap: the wiring is **called from inside a Nest @@ -114,7 +114,9 @@ swapped in 2026-07 — its eleven imported symbols (the heaviest single import) became the injected `DaysService`; permissions swapped in 2026-07 with the Wave-2 pair — its `checkPermission` import became the injected `PermissionsService` (the auditLog half never touched the factory: the plugin -host's audit trail is the separate `plugin-audit.ts`); the remaining factory +host's audit trail is the separate `plugin-audit.ts`); exchangeRateService +swapped in 2026-07 with the budget-domain fold — its `getRates` import became +the injected `ExchangeRatesService`; the remaining factory imports are all Wave-4+ domains: trips, places, journeys, atlas, collections). ### Test impact (as landed) @@ -123,7 +125,7 @@ imports are all Wave-4+ domains: trips, places, journeys, atlas, collections). `PluginRpcHost` directly with hand-built `HostDeps`). - `tests/unit/plugins/plugin-host-deps.factory.test.ts` (was `create-rpc-host.test.ts`) — the six DI-domain path mocks became constructor - stubs (fifteen stubs as of the 2026-07 permissions migration); the ~22 + stubs (sixteen stubs as of the 2026-07 exchange-rates fold); the ~22 legacy-service path mocks remain; a file-local shim keeps the historical `createRealRpcHost(id, granted)` call sites and supplies a default no-op router. diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index abb3b3c53..02e3a99b3 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -5,7 +5,6 @@ import { listBudgetItems } from '../../../services/budgetService'; import { isUpdateConflict } from '../../../services/conflictResult'; import { getWeather } from '../../../services/weatherService'; import { BLOCKED_EXTENSIONS, filesDir } from '../../files/files.constants'; -import { getRates as getExchangeRates } from '../../../services/exchangeRateService'; import { joinTripAsMember } from '../../../services/tripMembership'; import { send as sendNotification } from '../../../services/notificationService'; import { createLlmClient } from '../../llm-parse/llm-client.factory'; @@ -24,6 +23,7 @@ import { listJourneys, listEntries as listJournalEntriesSvc, createEntry as crea import { listVisitedCountries, listManuallyVisitedRegions, listBucketList, markCountryVisited, unmarkCountryVisited, markRegionVisited, unmarkRegionVisited, createBucketItem as createBucketItemSvc, deleteBucketItem as deleteBucketItemSvc } from '../../../services/atlasService'; import { listCollections, getCollection, createCollection, updateCollection, savePlace as saveCollectionPlaceSvc, copyToTrip as copyCollectionToTripSvc, deletePlace as deleteCollectionPlaceSvc } from '../../../services/collectionsService'; import { BudgetService } from '../../budget/budget.service'; +import { ExchangeRatesService } from '../../budget/exchange-rates.service'; import { ReservationsService } from '../../reservations/reservations.service'; import { TagsService } from '../../tags/tags.service'; import { CategoriesService } from '../../categories/categories.service'; @@ -129,9 +129,9 @@ export interface PluginCallRouter { * host (parent), never in the child. Broadcasts are force-namespaced to * `plugin:{id}:{event}` so a plugin can't forge a core event. * - * DI-native domain services (budget, reservations, tags, categories, todo, - * packing, day-notes, assignments, oauth, files, collab, vacay, the LLM - * config resolver) and + * DI-native domain services (budget, exchange-rates, reservations, tags, + * categories, todo, packing, day-notes, assignments, oauth, files, collab, + * vacay, the LLM config resolver) and * the DatabaseService (all inline SQL + the trip-access helper) are * constructor-injected; legacy `services/*` domains stay plain function * imports until their own migration lands (DI-MIGRATION.md), at which point @@ -156,6 +156,7 @@ export class PluginHostDepsFactory { private readonly vacay: VacayService, private readonly days: DaysService, private readonly permissions: PermissionsService, + private readonly exchangeRates: ExchangeRatesService, ) {} /** @@ -534,7 +535,7 @@ export class PluginHostDepsFactory { } }, // --- Exchange rates: the same cached upstream feed the budget uses (tenant-free). --- - getRates: (base) => getExchangeRates(base), + getRates: (base) => this.exchangeRates.getRates(base), // --- Trip (trip_edit). Only the schema-writable fields reach updateTrip; its // NotFound/Validation errors are mapped to clean RPC codes. --- canEditTrip: (tripId, userId) => this.canEditTripAs('trip_edit', tripId, userId), diff --git a/server/src/services/budgetService.ts b/server/src/services/budgetService.ts index 7156aaddf..935eceead 100644 --- a/server/src/services/budgetService.ts +++ b/server/src/services/budgetService.ts @@ -1,7 +1,7 @@ import { db } from '../db/database'; import { BudgetItem, BudgetItemMember, BudgetItemPayer } from '../types'; import { avatarUrl } from './avatarUrl'; -import { getRates } from './exchangeRateService'; +import { getRates } from '../nest/budget/exchange-rates.bridge'; // --------------------------------------------------------------------------- // Helpers diff --git a/server/src/services/exchangeRateService.ts b/server/src/services/exchangeRateService.ts deleted file mode 100644 index 184431332..000000000 --- a/server/src/services/exchangeRateService.ts +++ /dev/null @@ -1,75 +0,0 @@ -/** - * Live exchange rates for the Costs/Budget money conversion. - * - * Fetches from api.frankfurter.dev (no key, already CSP-allowlisted for the - * dashboard widget) and caches per base currency in-memory for a few hours so a - * settlement request never hammers the upstream. Rates are "units of X per 1 - * base", so an amount in currency C converts to base as `amount / rates[C]`. - * - * Everything degrades gracefully: if the fetch fails (offline, upstream down), - * callers get `null`/identity conversion and amounts are treated as already in - * the base currency rather than throwing. - */ - -const TTL_MS = 6 * 60 * 60 * 1000; // 6h -const cache = new Map; ts: number }>(); -const inflight = new Map | null>>(); - -async function fetchRates(base: string): Promise | null> { - try { - const res = await fetch(`https://api.frankfurter.dev/v2/rates?base=${encodeURIComponent(base)}`); - if (!res.ok) return null; - // Frankfurter returns an array of { date, base, quote, rate } and omits the - // base's own self-rate, so seed the map with `base = 1` then index by quote. - const data = (await res.json()) as Array<{ quote?: string; rate?: number }>; - if (!Array.isArray(data)) return null; - const rates: Record = { [base.toUpperCase()]: 1 }; - for (const r of data) { - if (r && typeof r.quote === 'string' && typeof r.rate === 'number') rates[r.quote] = r.rate; - } - return Object.keys(rates).length > 1 ? rates : null; - } catch { - return null; - } -} - -/** Rates map for `base` (cached). Returns null if unavailable. */ -export async function getRates(base: string): Promise | null> { - const key = (base || 'EUR').toUpperCase(); - const hit = cache.get(key); - const now = Date.now(); - if (hit && now - hit.ts < TTL_MS) return hit.rates; - - // Coalesce concurrent fetches for the same base. - let p = inflight.get(key); - if (!p) { - p = fetchRates(key).then(rates => { - if (rates) cache.set(key, { rates, ts: Date.now() }); - inflight.delete(key); - return rates; - }); - inflight.set(key, p); - } - const rates = await p; - // On failure fall back to the last cached value if we have one. - if (!rates && hit) return hit.rates; - return rates; -} - -/** - * Convert `amount` from `from` currency into `base` using a rates map obtained - * from getRates(base). Identity when same currency or the rate is missing. - */ -export function convertWithRates( - amount: number, - from: string | null | undefined, - base: string, - rates: Record | null, -): number { - const fromCur = (from || base).toUpperCase(); - const baseCur = base.toUpperCase(); - if (fromCur === baseCur || !rates) return amount; - const r = rates[fromCur]; - if (!r || r <= 0) return amount; - return amount / r; -} diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 5b7501d2b..8246cf489 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -4,6 +4,7 @@ import type { PluginRegistryService } from '../../src/nest/plugins/registry/regi import { PluginHostDepsFactory } from '../../src/nest/plugins/host/plugin-host-deps.factory'; import { PluginOAuthService } from '../../src/nest/plugins/plugin-oauth.service'; import { BudgetService } from '../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../src/nest/budget/exchange-rates.service'; import { ReservationsService } from '../../src/nest/reservations/reservations.service'; import { TagsService } from '../../src/nest/tags/tags.service'; import { CategoriesService } from '../../src/nest/categories/categories.service'; @@ -28,8 +29,9 @@ import { AuditService } from '../../src/nest/audit/audit.service'; */ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFactory { const permissions = new PermissionsService(dbs); + const exchangeRates = new ExchangeRatesService(); return new PluginHostDepsFactory( - new BudgetService(dbs, permissions), + new BudgetService(dbs, permissions, exchangeRates), new ReservationsService(dbs, permissions), new TagsService(dbs), new CategoriesService(dbs), @@ -45,6 +47,7 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto new VacayService(dbs), new DaysService(dbs, permissions), permissions, + exchangeRates, ); } diff --git a/server/tests/unit/nest/budget.service.test.ts b/server/tests/unit/nest/budget.service.test.ts index 79e7f9ba7..abc638f4e 100644 --- a/server/tests/unit/nest/budget.service.test.ts +++ b/server/tests/unit/nest/budget.service.test.ts @@ -16,8 +16,11 @@ vi.mock('../../../src/websocket', () => ({ broadcast })); const checkPermission = vi.fn(() => true); const permissionsStub = { checkPermission } as unknown as PermissionsService; -const { getRates } = vi.hoisted(() => ({ getRates: vi.fn() })); -vi.mock('../../../src/services/exchangeRateService', () => ({ getRates })); +// Constructor-injected stub since the exchange-rates fold (was a path mock of +// the deleted services/exchangeRateService). +import type { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +const getRates = vi.fn(); +const exchangeRatesStub = { getRates } as unknown as ExchangeRatesService; const { budget } = vi.hoisted(() => ({ budget: { @@ -45,7 +48,7 @@ vi.mock('../../../src/services/budgetService', () => budget); import { BudgetService } from '../../../src/nest/budget/budget.service'; function svc() { - return new BudgetService(new DatabaseService(dbConn), permissionsStub); + return new BudgetService(new DatabaseService(dbConn), permissionsStub, exchangeRatesStub); } beforeEach(() => { diff --git a/server/tests/unit/nest/exchange-rates.service.test.ts b/server/tests/unit/nest/exchange-rates.service.test.ts new file mode 100644 index 000000000..e7b8f042b --- /dev/null +++ b/server/tests/unit/nest/exchange-rates.service.test.ts @@ -0,0 +1,170 @@ +/** + * Unit tests for the DI-native ExchangeRatesService — FX-SVC-001 through + * FX-SVC-020. New suite: the legacy services/exchangeRateService.ts had no + * dedicated tests; the budget-domain fold moves it inside the src/nest/** + * coverage gate. 001–018 pin the fetch/cache/convert behavior byte-for-byte + * (including the parity quirks: `|| 'EUR'` falsy coercion, stale-cache + * fallback, the `>1 keys` failure heuristic, silent catch → null); 019–020 pin + * the exchange-rates.bridge delegation and the module-scoped cache shared + * between the DI instance and the bridge instance. + * + * The rate cache is deliberately MODULE-scoped, so it persists across tests in + * this file — every case uses its own base currency to stay isolated. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { getRates as bridgeGetRates, convertWithRates as bridgeConvertWithRates } from '../../../src/nest/budget/exchange-rates.bridge'; + +const TTL_MS = 6 * 60 * 60 * 1000; // mirrors the service's 6h TTL + +// A minimal Frankfurter-shaped success response (array of { quote, rate }). +const okResponse = (data: unknown) => ({ ok: true, json: async () => data }); + +const svc = new ExchangeRatesService(); + +let fetchMock: ReturnType; + +beforeEach(() => { + fetchMock = vi.fn(async () => okResponse([{ quote: 'USD', rate: 1.08 }])); + vi.stubGlobal('fetch', fetchMock); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); +}); + +describe('ExchangeRatesService.getRates', () => { + it('FX-SVC-001: falls back to EUR for a falsy base (|| coercion, not ??)', async () => { + const rates = await svc.getRates(''); + expect(fetchMock).toHaveBeenCalledWith('https://api.frankfurter.dev/v2/rates?base=EUR'); + expect(rates).toEqual({ EUR: 1, USD: 1.08 }); + }); + + it('FX-SVC-002: upper-cases the base for the request and the self-rate seed', async () => { + fetchMock.mockResolvedValueOnce(okResponse([{ quote: 'GBP', rate: 0.85 }])); + const rates = await svc.getRates('usd'); + expect(fetchMock).toHaveBeenCalledWith('https://api.frankfurter.dev/v2/rates?base=USD'); + expect(rates).toEqual({ USD: 1, GBP: 0.85 }); + }); + + it('FX-SVC-003: seeds base = 1, indexes by quote and skips malformed entries', async () => { + fetchMock.mockResolvedValueOnce(okResponse([ + { quote: 'USD', rate: 1.08 }, + { quote: 'GBP', rate: 0.85 }, + { quote: 42, rate: 1 }, // non-string quote → skipped + { quote: 'JPY' }, // missing rate → skipped + null, // null entry → skipped + ])); + const rates = await svc.getRates('CHF'); + expect(rates).toEqual({ CHF: 1, USD: 1.08, GBP: 0.85 }); + }); + + it('FX-SVC-004: returns null on a non-ok upstream response', async () => { + fetchMock.mockResolvedValueOnce({ ok: false, json: async () => [] }); + expect(await svc.getRates('NOK')).toBeNull(); + }); + + it('FX-SVC-005: returns null when the response body is not an array', async () => { + fetchMock.mockResolvedValueOnce(okResponse({ USD: 1.08 })); + expect(await svc.getRates('SEK')).toBeNull(); + }); + + it('FX-SVC-006: returns null when fetch throws (silent catch, no logging)', async () => { + fetchMock.mockRejectedValueOnce(new Error('offline')); + expect(await svc.getRates('DKK')).toBeNull(); + }); + + it('FX-SVC-007: treats a response that yields only the self-rate as failure (>1 keys heuristic)', async () => { + fetchMock.mockResolvedValueOnce(okResponse([])); + expect(await svc.getRates('CZK')).toBeNull(); + }); + + it('FX-SVC-008: serves the cached rates within the TTL without refetching', async () => { + const first = await svc.getRates('PLN'); + const second = await svc.getRates('PLN'); + expect(second).toBe(first); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('FX-SVC-009: refetches once the TTL has elapsed', async () => { + vi.useFakeTimers(); + await svc.getRates('HUF'); + vi.advanceTimersByTime(TTL_MS + 1); + fetchMock.mockResolvedValueOnce(okResponse([{ quote: 'USD', rate: 1.2 }])); + const rates = await svc.getRates('HUF'); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(rates).toEqual({ HUF: 1, USD: 1.2 }); + }); + + it('FX-SVC-010: quirk — falls back to the stale cache (beyond the TTL) when the upstream fails', async () => { + vi.useFakeTimers(); + const first = await svc.getRates('RON'); + vi.advanceTimersByTime(TTL_MS + 1); + fetchMock.mockRejectedValueOnce(new Error('down')); + expect(await svc.getRates('RON')).toBe(first); + }); + + it('FX-SVC-011: coalesces concurrent fetches for the same base into one request', async () => { + let release!: (v: unknown) => void; + fetchMock.mockReturnValueOnce(new Promise((resolve) => { release = resolve; })); + const a = svc.getRates('ISK'); + const b = svc.getRates('ISK'); + release(okResponse([{ quote: 'USD', rate: 1.08 }])); + const [ra, rb] = await Promise.all([a, b]); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(rb).toBe(ra); + }); + + it('FX-SVC-012: returns null on failure when nothing is cached', async () => { + fetchMock.mockResolvedValueOnce({ ok: false, json: async () => [] }); + expect(await svc.getRates('BGN')).toBeNull(); + }); +}); + +describe('ExchangeRatesService.convertWithRates', () => { + const rates = { EUR: 1, USD: 1.08, BAD: 0, NEG: -2 }; + + it('FX-SVC-013: identity when from equals base (case-insensitive)', () => { + expect(svc.convertWithRates(10, 'eur', 'EUR', rates)).toBe(10); + }); + + it('FX-SVC-014: identity when the rates map is null', () => { + expect(svc.convertWithRates(10, 'USD', 'EUR', null)).toBe(10); + }); + + it('FX-SVC-015: a falsy from means "already in base" (|| coercion) → identity', () => { + expect(svc.convertWithRates(10, null, 'EUR', rates)).toBe(10); + expect(svc.convertWithRates(10, undefined, 'EUR', rates)).toBe(10); + expect(svc.convertWithRates(10, '', 'EUR', rates)).toBe(10); + }); + + it('FX-SVC-016: identity when the from-currency has no rate', () => { + expect(svc.convertWithRates(10, 'JPY', 'EUR', rates)).toBe(10); + }); + + it('FX-SVC-017: identity for a zero or negative rate', () => { + expect(svc.convertWithRates(10, 'BAD', 'EUR', rates)).toBe(10); + expect(svc.convertWithRates(10, 'NEG', 'EUR', rates)).toBe(10); + }); + + it('FX-SVC-018: divides by the rate ("units of X per 1 base")', () => { + expect(svc.convertWithRates(10.8, 'USD', 'EUR', rates)).toBeCloseTo(10); + }); +}); + +describe('exchange-rates.bridge', () => { + it('FX-SVC-019: getRates delegates and shares the module-scoped cache with the DI instance', async () => { + // Prime the cache through the DI-style instance… + const primed = await svc.getRates('AUD'); + expect(fetchMock).toHaveBeenCalledTimes(1); + // …then the bridge (its own instance) must serve the same cached feed. + expect(await bridgeGetRates('AUD')).toBe(primed); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('FX-SVC-020: convertWithRates delegates 1:1', () => { + expect(bridgeConvertWithRates(21.6, 'USD', 'EUR', { EUR: 1, USD: 1.08 })).toBeCloseTo(20); + expect(bridgeConvertWithRates(5, 'EUR', 'EUR', null)).toBe(5); + }); +}); diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index eae25510b..cb212e1fd 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -3,8 +3,9 @@ * capability host to the real privileged modules (#plugins, M1). Verifies the * per-plugin data db is cached, a granted db:own call works through the wired * host, and trip broadcasts are force-namespaced to plugin:{id}:{event}. - * DI-native domains (budget/reservations/tags/categories/todo/packing/ - * day-notes/days/assignments/oauth/llm-config/files/collab/vacay/permissions) + * DI-native domains (budget/exchange-rates/reservations/tags/categories/todo/ + * packing/day-notes/days/assignments/oauth/llm-config/files/collab/vacay/ + * permissions) * are constructor-injected stubs; legacy services/* domains stay path-mocked * until their own DI migration lands. */ @@ -84,9 +85,11 @@ vi.mock('../../../src/services/tripService', () => { NotFoundError, ValidationError, }; }); -vi.mock('../../../src/services/exchangeRateService', () => ({ +// Exchange rates are a constructor-injected stub since the budget-domain fold +// (was a path mock of the deleted services/exchangeRateService — same behavior). +const exchangeRatesStub = { getRates: vi.fn(async (base: string) => ({ [base]: 1, USD: 1.08, GBP: 0.85 })), -})); +} as unknown as ExchangeRatesService; vi.mock('../../../src/services/placeService', () => ({ createPlace: vi.fn((tid: string, body: Record) => ({ id: 10, trip_id: Number(tid), ...body })), updatePlace: vi.fn((_tid: string, pid: string) => (pid === '99' ? null : { id: Number(pid) })), @@ -308,6 +311,7 @@ import { PluginHostDepsFactory, type PluginCallRouter } from '../../../src/nest/ import { getPluginDataDb, closePluginDataDb } from '../../../src/nest/plugins/host/plugin-host-state'; import { db as mockDb } from '../../../src/db/database'; import type { BudgetService } from '../../../src/nest/budget/budget.service'; +import type { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; import type { ReservationsService } from '../../../src/nest/reservations/reservations.service'; import type { TagsService } from '../../../src/nest/tags/tags.service'; import type { CategoriesService } from '../../../src/nest/categories/categories.service'; @@ -327,7 +331,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; // The factory under test, wired exactly like PluginsModule does — but with the // DI-native domain services replaced by the stubs above. The shim keeps the // ~45 historical call sites unchanged and supplies a default no-op router. -const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub, permissionsStub); +const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub, permissionsStub, exchangeRatesStub); const stubRouter: PluginCallRouter = { callPlugin: async () => undefined, emitPluginEvent: () => {} }; const createRealRpcHost = (id: string, granted: ReadonlySet, router: PluginCallRouter = stubRouter) => factory.create(id, granted, router); diff --git a/server/tests/unit/services/budgetService.test.ts b/server/tests/unit/services/budgetService.test.ts index 47e654c6a..99f6647b5 100644 --- a/server/tests/unit/services/budgetService.test.ts +++ b/server/tests/unit/services/budgetService.test.ts @@ -18,7 +18,7 @@ const mockDb = vi.hoisted(() => { vi.mock('../../../src/db/database', () => mockDb); const mockRates = vi.hoisted(() => ({ getRates: vi.fn() })); -vi.mock('../../../src/services/exchangeRateService', () => mockRates); +vi.mock('../../../src/nest/budget/exchange-rates.bridge', () => mockRates); import { calculateSettlement, updateSettlement, freezeForeignRate } from '../../../src/services/budgetService'; import type { BudgetItem, BudgetItemMember, BudgetItemPayer } from '../../../src/types'; diff --git a/server/tests/unit/services/budgetServiceDb.test.ts b/server/tests/unit/services/budgetServiceDb.test.ts index 741e52db7..8ac97530d 100644 --- a/server/tests/unit/services/budgetServiceDb.test.ts +++ b/server/tests/unit/services/budgetServiceDb.test.ts @@ -33,7 +33,7 @@ const RATES: Record> = { RUB: { RUB: 1, USD: 0.013042, EUR: 0.011412 }, EUR: { EUR: 1, USD: 1.1429, RUB: 87.63 }, }; -vi.mock('../../../src/services/exchangeRateService', () => ({ +vi.mock('../../../src/nest/budget/exchange-rates.bridge', () => ({ getRates: vi.fn(async (base: string) => RATES[base.toUpperCase()] ?? null), }));