diff --git a/migration/1785460000000-AddLedgerContentChangeScanIndexes.js b/migration/1785460000000-AddLedgerContentChangeScanIndexes.js new file mode 100644 index 0000000000..d7c527e70a --- /dev/null +++ b/migration/1785460000000-AddLedgerContentChangeScanIndexes.js @@ -0,0 +1,164 @@ +/** + * @typedef {import('typeorm').MigrationInterface} MigrationInterface + * @typedef {import('typeorm').QueryRunner} QueryRunner + */ + +/** + * Add composite indexes `(updated, id)` on the nine ledger consumer source tables so the + * per-minute content-change-scan (`runContentChangeScan` in + * `src/subdomains/core/accounting/services/consumers/ledger-watermark.helper.ts:210`) stops doing a + * full sequential scan on each of them. + * + * The scan does: + * `WHERE (updated > :scan OR (updated = :scan AND id > :scanId)) ORDER BY updated ASC, id ASC LIMIT 100`. + * None of the nine tables previously had an index on `updated`. + * + * Column order `(updated, id)` is intentional: the query orders by `updated, id`, so an index on + * `updated` alone would still need an explicit Sort step whenever several rows share the same + * `updated` value — same reasoning as AddFinancialLogQueryIndex1785400000000 for `(created, id)`. + * + * Measured evidence: production `EXPLAIN (ANALYZE, BUFFERS)` for `trading_order` (921 MB, the + * largest of the nine) without this index showed a Parallel Seq Scan, Rows Removed by Filter + * 1,806,023 (x3 workers = 5,418,069 rows), Buffers shared hit=26096 read=63188 (~494 MB from disk + * per call), returning only 4 matching rows, Execution Time 151.765 ms. A 45 s delta measurement + * showed 4,750 MB of disk reads and 32.5 million rows read for `trading_order` alone. + * + * CREATE INDEX CONCURRENTLY is not used: migrations in this codebase run transactionally and + * boot-blockingly (see src/config/config.ts, migrationsRun gated by the SQL_MIGRATE env var). + * CREATE INDEX CONCURRENTLY is not allowed inside a transaction and would crash the migration. + * + * Lock behaviour, stated precisely: all nine `CREATE INDEX` statements run inside a single + * database transaction (TypeORM default `migrationsTransactionMode: "all"`), and PostgreSQL only + * releases locks at COMMIT, not at the end of each statement. Evidence: + * `node_modules/typeorm/data-source/DataSource.js:263-265` (`migrationExecutor.transaction = + * options?.transaction || this.options?.migrationsTransactionMode || "all"` — default `"all"`); + * `src/config/config.ts:265` only sets `migrationsRun` and never overrides + * `migrationsTransactionMode` (corroborated by the comment in + * `src/shared/models/asset/__tests__/add-binance-custody-assets-ondo-ada.migration.spec.ts:348` — + * "no migrationsTransactionMode override → default 'all'"); + * `node_modules/typeorm/migration/MigrationExecutor.js:206` starts one transaction for pending + * migrations and commits only at the end. A plain CREATE INDEX holds a SHARE lock for the entire + * build; reads continue throughout, but writes to the table are blocked while that lock is held. + * Because locks are held until COMMIT, the write-blocking window for `trading_order` (the first + * table built) is the sum of all nine index builds, not just its own, and by the time the + * transaction commits all nine tables — including central transaction tables `bank_tx`, + * `buy_crypto`, `crypto_input`, `payout_order` — are simultaneously write-blocked. If further + * migrations are pending at the same time, those run in the same transaction too and extend the + * window further. Splitting this into multiple separate migration files would NOT change this + * (the same transaction still applies across files run in the same batch). `SET LOCAL lock_timeout` caps only how long we WAIT to acquire a lock, not how long we hold it once + * acquired. That timeout is scoped to each individual lock-acquisition attempt — each of the nine + * `CREATE INDEX` statements (and, in `down()`, each of the nine `DROP INDEX` statements) gets its own + * wait budget, not a single global ceiling shared across the whole migration. An earlier statement can + * succeed well inside its 5s budget while a later one still times out and aborts the transaction. + * Production scan+sort for the biggest table's `(updated, id)` was measured at 1159 ms + * — but that number is a `work_mem`-bound External Merge sort spilling ~116 MB to disk, NOT the + * index build itself, which sorts in `maintenance_work_mem` (256 MB in this instance, in RAM) and + * should be faster, plus the time to write ~150 MB of index pages. That points to a low + * single-digit-second range as a realistic expectation for a single index build, but is NOT a + * measured index-build time and must NOT be asserted as a hard upper bound on total build time or + * on the cumulative lock window across all nine tables (none has been measured). Risk framing: + * this migration runs boot-blockingly at app startup (`migrationsRun`, gated by the `SQL_MIGRATE` + * env var), so the starting instance itself is not yet serving requests and is not itself a + * writer. Concurrent writers would be a still-running predecessor instance during a rolling + * deploy, or external consumers. If a lock conflict occurs, the migration aborts after + * `lock_timeout` and so does the app start — that is fail-closed and intentional, but it is a + * deploy abort and must be named as such. + * + * `down()` reverses this with nine `DROP INDEX` statements and is subject to a stricter lock: PostgreSQL + * takes an ACCESS EXCLUSIVE lock for `DROP INDEX` (vs. the SHARE lock `CREATE INDEX` takes above), and + * ACCESS EXCLUSIVE conflicts with every other lock mode, including the AccessShareLock a plain `SELECT` + * takes — so `down()` blocks reads as well as writes on each table, not writes alone. `down()` runs in + * its own migration transaction (same TypeORM default `migrationsTransactionMode: "all"`), so the same + * cumulative-window reasoning applies: all nine ACCESS EXCLUSIVE locks are held until COMMIT, and the + * first table dropped is blocked — for reads and writes — for the sum of all nine drops. Running + * `migration:revert` against a live table is therefore materially more disruptive than `up()`, not + * merely its mirror image. + * + * Tables and index names: + * trading_order → IDX_47e55a74022f04d725395b9648 + * crypto_input → IDX_37d5dbe4bda6e9e78b0ac08ba1 + * bank_tx → IDX_834c06e67196ac958afc5dccec + * buy_crypto → IDX_398573811cc39fb7ff740459a6 + * exchange_tx → IDX_82c40ae44b9968bf6d2c6acdd0 + * payout_order → IDX_44c2cf65b5554fb61eef1453c5 + * buy_fiat → IDX_934bb0a02ccf36e8ed04bb6bdd + * liquidity_management_order → IDX_6d47b5e8f3e480587a4e3da5a4 + * liquidity_order → IDX_617b110d76b02979c229fbc6be + * + * These are not arbitrary names but the deterministic names TypeORM's DefaultNamingStrategy would + * generate itself, since custom index naming is disallowed by CONTRIBUTING.md. Each name is + * `IDX_` followed by the first 26 hex characters of `sha1( + '_id_updated')` (column names + * `id` and `updated` sorted alphabetically and joined with `_`, per TypeORM's DefaultNamingStrategy). + * + * @class + * @implements {MigrationInterface} + */ +module.exports = class AddLedgerContentChangeScanIndexes1785460000000 { + name = 'AddLedgerContentChangeScanIndexes1785460000000'; + + /** + * @param {QueryRunner} queryRunner + */ + async up(queryRunner) { + // SET LOCAL is scoped to the whole transaction, so set once for all nine CREATE INDEX + // statements below. Bounds WAIT time to acquire the lock, not how long the lock is held. + await queryRunner.query(`SET LOCAL lock_timeout = '5s'`); + await queryRunner.query( + `CREATE INDEX "IDX_47e55a74022f04d725395b9648" ON "trading_order" ("updated", "id")`, + ); + + await queryRunner.query( + `CREATE INDEX "IDX_37d5dbe4bda6e9e78b0ac08ba1" ON "crypto_input" ("updated", "id")`, + ); + + await queryRunner.query(`CREATE INDEX "IDX_834c06e67196ac958afc5dccec" ON "bank_tx" ("updated", "id")`); + + await queryRunner.query( + `CREATE INDEX "IDX_398573811cc39fb7ff740459a6" ON "buy_crypto" ("updated", "id")`, + ); + + await queryRunner.query( + `CREATE INDEX "IDX_82c40ae44b9968bf6d2c6acdd0" ON "exchange_tx" ("updated", "id")`, + ); + + await queryRunner.query( + `CREATE INDEX "IDX_44c2cf65b5554fb61eef1453c5" ON "payout_order" ("updated", "id")`, + ); + + await queryRunner.query(`CREATE INDEX "IDX_934bb0a02ccf36e8ed04bb6bdd" ON "buy_fiat" ("updated", "id")`); + + await queryRunner.query( + `CREATE INDEX "IDX_6d47b5e8f3e480587a4e3da5a4" ON "liquidity_management_order" ("updated", "id")`, + ); + + await queryRunner.query( + `CREATE INDEX "IDX_617b110d76b02979c229fbc6be" ON "liquidity_order" ("updated", "id")`, + ); + } + + /** + * @param {QueryRunner} queryRunner + */ + async down(queryRunner) { + // SET LOCAL is scoped to the whole transaction, so set once for all nine DROP INDEX + // statements below. Bounds WAIT time to acquire the lock, not how long the lock is held. + await queryRunner.query(`SET LOCAL lock_timeout = '5s'`); + await queryRunner.query(`DROP INDEX "public"."IDX_617b110d76b02979c229fbc6be"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_6d47b5e8f3e480587a4e3da5a4"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_934bb0a02ccf36e8ed04bb6bdd"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_44c2cf65b5554fb61eef1453c5"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_82c40ae44b9968bf6d2c6acdd0"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_398573811cc39fb7ff740459a6"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_834c06e67196ac958afc5dccec"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_37d5dbe4bda6e9e78b0ac08ba1"`); + + await queryRunner.query(`DROP INDEX "public"."IDX_47e55a74022f04d725395b9648"`); + } +}; diff --git a/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts b/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts index 72a78be438..cc610c8efc 100644 --- a/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts +++ b/src/subdomains/core/buy-crypto/routes/buy/buy.service.ts @@ -554,8 +554,10 @@ export class BuyService { }; } + // getBank() can return undefined, but this builder dereferences the bank unconditionally - callers + // must resolve that before calling in, so the parameter states the precondition instead of widening. private buildBankResponse( - bank: Awaited>, + bank: NonNullable>>, reference?: string, ): BankInfoDto & { isPersonalIban: boolean; reference?: string } { return { diff --git a/src/subdomains/generic/gs/__tests__/gs.controller.e2e.spec.ts b/src/subdomains/generic/gs/__tests__/gs.controller.e2e.spec.ts index ffd365a6b7..4f0c14f246 100644 --- a/src/subdomains/generic/gs/__tests__/gs.controller.e2e.spec.ts +++ b/src/subdomains/generic/gs/__tests__/gs.controller.e2e.spec.ts @@ -1,6 +1,9 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Body, + CanActivate, Controller, + ExecutionContext, INestApplication, MiddlewareConsumer, Module, @@ -9,12 +12,19 @@ import { ValidationPipe, VersioningType, } from '@nestjs/common'; +import { GUARDS_METADATA } from '@nestjs/common/constants'; import { Test } from '@nestjs/testing'; import * as bodyParser from 'body-parser'; import request from 'supertest'; import { GetConfig } from 'src/config/config'; +import { JwtPayload } from 'src/shared/auth/jwt-payload.interface'; +import { UserRole } from 'src/shared/auth/user-role.enum'; +import { DfxLogger } from 'src/shared/services/dfx-logger'; +import * as processServiceModule from 'src/shared/services/process.service'; import { DbQueryDto, DbReturnData } from 'src/subdomains/generic/gs/dto/db-query.dto'; import { GsTriggerType } from 'src/subdomains/generic/gs/dto/gs-trigger-type.enum'; +import { GsController } from 'src/subdomains/generic/gs/gs.controller'; +import { GsService } from 'src/subdomains/generic/gs/gs.service'; import { DebugQueryDto, DebugQueryResult } from '../dto/debug-query.dto'; import { DebugQueryTreeSizeMiddleware } from '../middleware/debug-query-tree-size.middleware'; @@ -67,20 +77,10 @@ class GsControllerTestModule { } } -// Test-only route for the `/gs/db` request pipeline (production file: `gs.controller.ts`). The -// production `GsController` is NOT bootstrapped here, for the same reason `GsDebugTestController` -// above isn't: `RoleGuard()` and `UserActiveGuard()` return already-instantiated guard objects -// baked into `@UseGuards()` at controller-decoration time in `gs.controller.ts`, so calling -// `RoleGuard()` / `UserActiveGuard()` again in this file creates different instances that -// `Test.overrideGuard()` cannot match. -// -// This controller deliberately does NOT reproduce the trigger-enforcement check. That check is -// exercised against the REAL `GsController` in the unit test `gs.controller.spec.ts`; duplicating it here -// would just be two tests for the same logic. This fixture covers the full DbQueryDto / -// ValidationPipe surface (not only the trigger field) — what only the full NestJS pipeline -// can prove: that the real `DbQueryDto` decorators (`@IsEnum(GsTriggerType)`, -// `@MaxLength(256)` on `table`/`identifier`, control-character rejection, etc.) are actually -// wired into the global `ValidationPipe`. +// Test-only route that isolates the `DbQueryDto` / ValidationPipe surface from controller +// behavior. It proves the real DTO decorators (`@IsEnum(GsTriggerType)`, `@MaxLength(256)` on +// `table`/`identifier`, control-character rejection, etc.) are wired into the global pipe and +// deliberately leaves trigger enforcement to the real-controller HTTP suite below. @Controller('gs') class GsDbQueryDtoTestController { @Post('db') @@ -291,3 +291,111 @@ describe('GsController e2e (db query DTO validation)', () => { .expect(201); }); }); + +describe('GsController e2e (missing trigger enforcement)', () => { + let app: INestApplication; + let service: DeepMocked; + let verboseSpy: jest.SpyInstance; + + const jwt: JwtPayload = { role: UserRole.ADMIN, ip: '1.2.3.4' }; + const allowAdminGuard: CanActivate = { + canActivate(context: ExecutionContext): boolean { + context.switchToHttp().getRequest<{ user?: JwtPayload }>().user = jwt; + return true; + }, + }; + + beforeAll(async () => { + service = createMock(); + verboseSpy = jest.spyOn(DfxLogger.prototype, 'verbose').mockImplementation(); + jest.spyOn(processServiceModule, 'DisabledProcess').mockReturnValue(false); + + const builder = Test.createTestingModule({ + controllers: [GsController], + providers: [{ provide: GsService, useValue: service }], + }); + const handlers = [GsController.prototype.getDbData, GsController.prototype.getExtendedData]; + const guards = handlers.flatMap((handler) => Reflect.getMetadata(GUARDS_METADATA, handler) as CanActivate[]); + + for (const guard of guards) { + if (typeof guard === 'function') { + builder.overrideGuard(guard).useValue(allowAdminGuard); + } else { + jest.spyOn(guard, 'canActivate').mockImplementation(allowAdminGuard.canActivate.bind(allowAdminGuard)); + } + } + + const moduleRef = await builder.compile(); + app = moduleRef.createNestApplication(); + app.enableVersioning({ type: VersioningType.URI, defaultVersion: [GetConfig().defaultVersion] }); + app.use(bodyParser.json({ limit: '20mb' })); + app.useGlobalPipes( + new ValidationPipe({ + whitelist: true, + transformOptions: { exposeUnsetFields: false }, + }), + ); + await app.init(); + }); + + afterAll(async () => { + try { + if (app) await app.close(); + } finally { + jest.restoreAllMocks(); + } + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it.each(['/v1/gs/db', '/v1/gs/db/custom'])( + 'rejects a missing trigger on %s before calling either GS service', + async (path) => { + const response = await request(app.getHttpServer()).post(path).send({ table: 'asset' }).expect(400); + + expect(response.body.message).toBe('Trigger type is required'); + expect(verboseSpy).toHaveBeenCalledTimes(1); + expect(verboseSpy).toHaveBeenCalledWith( + 'GS db call: table=asset, identifier=missing, trigger=missing, role=Admin', + ); + expect(service.getDbData).not.toHaveBeenCalled(); + expect(service.getExtendedDbData).not.toHaveBeenCalled(); + }, + ); + + it('routes a valid /gs/db request to getDbData only', async () => { + const result: DbReturnData = { keys: ['standard'], values: [{ id: 1 }] }; + service.getDbData.mockResolvedValue(result); + + const response = await request(app.getHttpServer()) + .post('/v1/gs/db') + .send({ table: 'asset', trigger: GsTriggerType.MANUAL }) + .expect(201); + + expect(response.body).toEqual(result); + expect(service.getDbData).toHaveBeenCalledWith( + expect.objectContaining({ table: 'asset', trigger: GsTriggerType.MANUAL }), + UserRole.ADMIN, + ); + expect(service.getExtendedDbData).not.toHaveBeenCalled(); + }); + + it('routes a valid /gs/db/custom request to getExtendedDbData only', async () => { + const result: DbReturnData = { keys: ['custom'], values: [{ id: 2 }] }; + service.getExtendedDbData.mockResolvedValue(result); + + const response = await request(app.getHttpServer()) + .post('/v1/gs/db/custom') + .send({ table: 'asset', trigger: GsTriggerType.AUTO }) + .expect(201); + + expect(response.body).toEqual(result); + expect(service.getExtendedDbData).toHaveBeenCalledWith( + expect.objectContaining({ table: 'asset', trigger: GsTriggerType.AUTO }), + UserRole.ADMIN, + ); + expect(service.getDbData).not.toHaveBeenCalled(); + }); +}); diff --git a/src/subdomains/generic/gs/__tests__/gs.controller.spec.ts b/src/subdomains/generic/gs/__tests__/gs.controller.spec.ts index 911211040d..a9fe37e0df 100644 --- a/src/subdomains/generic/gs/__tests__/gs.controller.spec.ts +++ b/src/subdomains/generic/gs/__tests__/gs.controller.spec.ts @@ -9,15 +9,10 @@ import { GsTriggerType } from 'src/subdomains/generic/gs/dto/gs-trigger-type.enu import { GsController } from 'src/subdomains/generic/gs/gs.controller'; import { GsService } from 'src/subdomains/generic/gs/gs.service'; -// Unit-level regression coverage for `GsController`'s private `logAndCheckTrigger` helper -// (called from both `getDbData` and `getExtendedData`), exercised against the REAL controller — -// unlike `gs.controller.e2e.spec.ts`, which cannot bootstrap the real `GsController` through -// NestJS' HTTP pipeline: `RoleGuard()` / `UserActiveGuard()` bake already-instantiated guard -// objects into `@UseGuards()` at controller-decoration time, so a fresh `Test.overrideGuard()` -// call from a test module can't target them. Guards are a framework layer wrapped around the -// controller, not part of the method itself, so a direct `new GsController(service)` plus -// a plain method call sidesteps that problem entirely and exercises the actual production code -// path this feature touches. +// Direct regression coverage for `GsController`'s private `logAndCheckTrigger` helper (called +// from both handlers). Calling the real controller without the NestJS wrapper lets this suite +// assert the synchronous audit/service side effects before the returned Promise settles. The +// E2E suite separately covers both handlers through NestJS routing and exception mapping. describe('GsController', () => { let service: DeepMocked; let controller: GsController; @@ -56,26 +51,17 @@ describe('GsController', () => { for (const { name, call, serviceCall } of handlers) { describe(name, () => { it('rejects a request without trigger, logs first, and never calls the GS service', async () => { - const started = performance.now(); - let caught: unknown; - try { - await call(query({})); - } catch (e) { - caught = e; - } - const elapsed = performance.now() - started; - - expect(caught).toBeInstanceOf(BadRequestException); - expect((caught as BadRequestException).message).toBe('Trigger type is required'); - // Structural invariant: audit line is emitted before rejection, service is never entered. + const promise = call(query({})); + + // Structural invariant: audit line is emitted and service is never entered before rejection settles. expect(verboseSpy).toHaveBeenCalledTimes(1); expect(verboseSpy).toHaveBeenCalledWith( 'GS db call: table=asset, identifier=missing, trigger=missing, role=Admin', ); expect(serviceCall()).not.toHaveBeenCalled(); - // Rejection path is synchronous (no SettingService/DB await). Keep a modest SLA so - // a regression that re-introduces awaited work fails the suite without relying on load. - expect(elapsed).toBeLessThan(1000); + + await expect(promise).rejects.toBeInstanceOf(BadRequestException); + await expect(promise).rejects.toThrow('Trigger type is required'); }); it('accepts trigger=Manual', async () => { diff --git a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts index 8aa15e736b..0a640399ad 100644 --- a/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts +++ b/src/subdomains/supporting/bank/bank/__tests__/bank.service.spec.ts @@ -16,6 +16,7 @@ import { createCustomVirtualIban } from 'src/subdomains/supporting/bank/virtual- import { VirtualIban, VirtualIbanStatus } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.entity'; import { VirtualIbanRepository } from 'src/subdomains/supporting/bank/virtual-iban/virtual-iban.repository'; import { FiatPaymentMethod } from 'src/subdomains/supporting/payment/dto/payment-method.enum'; +import { FindOneOptions, FindOptionsWhere } from 'typeorm'; import { createCustomBank, createDefaultBanks, @@ -24,7 +25,6 @@ import { yapealEUR, olkyEUR, frickEUR, - frickCHF, } from '../__mocks__/bank.entity.mock'; import { Bank } from '../bank.entity'; import { BankRepository } from '../bank.repository'; @@ -46,6 +46,36 @@ function createBankSelectorInput( }; } +function mockFindCachedByForBanks(bankRepo: BankRepository, banks: Bank[]): void { + jest + .spyOn(bankRepo, 'findCachedBy') + .mockImplementation(async (_key: number | string, where: FindOptionsWhere | FindOptionsWhere[]) => { + // getReceiveBanks always supplies one object; fail visibly if that contract changes. + if (Array.isArray(where)) throw new Error('mockFindCachedByForBanks does not support array filters'); + + const receive = where.receive; + if (typeof receive === 'boolean') return banks.filter((bank) => bank.receive === receive); + return banks; + }); +} + +function mockFindCachedForBanks(bankRepo: BankRepository, banks: Bank[]): void { + jest + .spyOn(bankRepo, 'findCached') + .mockImplementation(async (_key: number | string, options?: FindOneOptions) => { + const where = options?.where; + if (!where) return banks; + // getBankInternal always supplies one object; fail visibly if that contract changes, rather + // than casting the array variant away and silently matching nothing. + if (Array.isArray(where)) throw new Error('mockFindCachedForBanks does not support array filters'); + return banks.filter( + (bank) => + (where.name === undefined || bank.name === where.name) && + (where.currency === undefined || bank.currency === where.currency), + ); + }); +} + describe('BankService', () => { let service: BankService; @@ -88,12 +118,8 @@ describe('BankService', () => { .mockResolvedValue(createCustomCountry({ yapealEnable: yapealEnable })); const allBanks = disabledBank ? createDefaultDisabledBanks() : createDefaultBanks(); - jest.spyOn(bankRepo, 'findCachedBy').mockImplementation(async (_key: string, filter?: any) => { - if (filter?.receive !== undefined) { - return allBanks.filter((b) => b.receive === filter.receive); - } - return allBanks; - }); + mockFindCachedByForBanks(bankRepo, allBanks); + mockFindCachedForBanks(bankRepo, []); } it('should be defined', () => { @@ -153,23 +179,191 @@ describe('BankService', () => { expect(result.bic).toBe(yapealEUR.bic); }); - it('never offers Bank Frick as a deposit bank, even when it is the first receive bank for the currency', async () => { - // Frick is placed first so a missing exclusion guard would wrongly select it; the customer must still - // be shown the incumbent bank for each currency. - // A preceding test disables the shared olkyEUR mock in place (createDefaultDisabledBanks mutates it), - // so restore its natural receive state here to exercise Frick exclusion rather than that leaked state. - olkyEUR.receive = true; - const frickFirst = [frickEUR, frickCHF, olkyEUR, yapealEUR, yapealCHF]; - jest.spyOn(bankRepo, 'findCachedBy').mockImplementation(async (_key: string, filter?: any) => { - if (filter?.receive !== undefined) return frickFirst.filter((b) => b.receive === filter.receive); - return frickFirst; + it('routes BANK EUR deposits to Bank Frick regardless of bank order', async () => { + const incumbent = createCustomBank({ ...olkyEUR, receive: true }); + const frick = createCustomBank({ ...frickEUR, receive: true }); + mockFindCachedByForBanks(bankRepo, [incumbent, frick]); + mockFindCachedForBanks(bankRepo, [incumbent, frick]); + + const result = await service.getBank(createBankSelectorInput('EUR', undefined, FiatPaymentMethod.BANK)); + expect(result).toBe(frick); + }); + + it('prefers the older asset-linked Bank Frick EUR row for a BANK EUR deposit', async () => { + // Production shape: the older row owns the custody asset and its IBAN is used by isBankMatching + // and the Financial Log. Returning the newer unbound row would detach the customer IBAN from + // that attribution. The mock order mirrors `order: { id: 'DESC' }` from getBankInternal. + const assetLinkedFrick = Object.assign( + createCustomBank({ + ...frickEUR, + id: 101, + receive: true, + iban: 'LI75088110105923K0101', + }), + { asset: {} }, + ); + const unboundNewerFrick = createCustomBank({ + ...frickEUR, + id: 202, + receive: true, + iban: 'LI75088110105923K0202', + }); + const banks = [unboundNewerFrick, assetLinkedFrick]; + mockFindCachedByForBanks(bankRepo, banks); + mockFindCachedForBanks(bankRepo, banks); + + const result = await service.getBank(createBankSelectorInput('EUR', undefined, FiatPaymentMethod.BANK)); + expect(result).toBe(assetLinkedFrick); + expect(result.iban).toBe('LI75088110105923K0101'); + }); + + it('picks the EUR Bank Frick row, not its CHF row, for an EUR deposit', async () => { + // The CHF row is listed first on purpose: the getBankInternal query matches on bank name AND + // currency. Without the currency check a customer paying in EUR could be handed the franc + // account's IBAN, and no other test in this file would notice. + const frickChfRow = createCustomBank({ + name: IbanBankName.FRICK, + currency: 'CHF', + receive: true, + iban: 'FRICK-CHF-ROW', + bic: 'BFRILI22', + }); + const frickEurRow = createCustomBank({ ...frickEUR, receive: true }); + mockFindCachedByForBanks(bankRepo, [frickChfRow, frickEurRow]); + mockFindCachedForBanks(bankRepo, [frickChfRow, frickEurRow]); + + const result = await service.getBank(createBankSelectorInput('EUR')); + expect(result).toBe(frickEurRow); + expect(result.currency).toBe('EUR'); + }); + + it('falls back to the established EUR receiver when Bank Frick is not receiving', async () => { + const disabledFrick = createCustomBank({ ...frickEUR, receive: false }); + const incumbent = createCustomBank({ ...olkyEUR, receive: true }); + mockFindCachedByForBanks(bankRepo, [disabledFrick, incumbent]); + mockFindCachedForBanks(bankRepo, [disabledFrick, incumbent]); + + const result = await service.getBank(createBankSelectorInput('EUR')); + expect(result).toBe(incumbent); + }); + + it('does not substitute another Bank Frick row when the attributed one is not receiving', async () => { + // The attributed (asset-linked) row is disabled while a second, unbound Frick row still receives. + // The rule must not fall through to that one: attribution stays on the disabled row, so paying + // into the unbound IBAN would book against a row nothing is keyed on. The incumbent wins instead. + const attributedDisabled = createCustomBank({ + ...frickEUR, + id: 19, + receive: false, + asset: createCustomAsset({}), + iban: 'FRICK-ATTRIBUTED-DISABLED', + }); + const unboundReceiving = createCustomBank({ + ...frickEUR, + id: 77, + receive: true, + asset: null, + iban: 'FRICK-UNBOUND-RECEIVING', + }); + const incumbent = createCustomBank({ ...olkyEUR, receive: true }); + mockFindCachedByForBanks(bankRepo, [attributedDisabled, unboundReceiving, incumbent]); + mockFindCachedForBanks(bankRepo, [attributedDisabled, unboundReceiving]); + + const result = await service.getBank(createBankSelectorInput('EUR')); + expect(result).toBe(incumbent); + }); + + it('leaves CHF bank selection unaffected by the Bank Frick EUR rule', async () => { + const frick = createCustomBank({ ...frickEUR, receive: true }); + const chf = createCustomBank({ ...yapealCHF, receive: true }); + mockFindCachedByForBanks(bankRepo, [frick, chf]); + mockFindCachedForBanks(bankRepo, []); + + const result = await service.getBank(createBankSelectorInput('CHF')); + expect(result).toBe(chf); + }); + + it('does not let a Bank Frick CHF row capture a CHF request', async () => { + const frickChf = createCustomBank({ + name: IbanBankName.FRICK, + currency: 'CHF', + receive: true, + iban: 'LI75088110105923K0CHF', + bic: 'BFRILI22', }); + const chf = createCustomBank({ ...yapealCHF, receive: true }); + mockFindCachedByForBanks(bankRepo, [frickChf, chf]); + mockFindCachedForBanks(bankRepo, []); + + const result = await service.getBank(createBankSelectorInput('CHF')); + expect(result).toBe(chf); + }); + + it('uses an instant-capable EUR bank instead of Bank Frick for INSTANT payments', async () => { + const frick = createCustomBank({ ...frickEUR, receive: true, sctInst: false }); + const instantBank = createCustomBank({ ...olkyEUR, receive: true, sctInst: true }); + mockFindCachedByForBanks(bankRepo, [frick, instantBank]); + mockFindCachedForBanks(bankRepo, []); + + const result = await service.getBank(createBankSelectorInput('EUR', undefined, FiatPaymentMethod.INSTANT)); + expect(result).toBe(instantBank); + }); + + it('falls back to an incumbent EUR bank when no EUR bank supports INSTANT', async () => { + const frick = createCustomBank({ ...frickEUR, receive: true, sctInst: false }); + const incumbent = createCustomBank({ ...olkyEUR, receive: true, sctInst: false }); + mockFindCachedByForBanks(bankRepo, [frick, incumbent]); + mockFindCachedForBanks(bankRepo, []); + + const result = await service.getBank(createBankSelectorInput('EUR', undefined, FiatPaymentMethod.INSTANT)); + expect(result).toBe(incumbent); + expect(result).not.toBe(frick); + }); + + it('uses an incumbent EUR bank instead of Bank Frick for CARD payments', async () => { + const frick = createCustomBank({ ...frickEUR, receive: true }); + const incumbent = createCustomBank({ ...olkyEUR, receive: true }); + mockFindCachedByForBanks(bankRepo, [frick, incumbent]); + mockFindCachedForBanks(bankRepo, []); + + const result = await service.getBank(createBankSelectorInput('EUR', undefined, FiatPaymentMethod.CARD)); + expect(result).toBe(incumbent); + expect(result).not.toBe(frick); + }); + + it.each([ + ['Bank Frick first', true], + ['incumbent first', false], + ])('uses the incumbent EUR fallback for unsupported currency with %s', async (_description, frickFirst) => { + const frick = createCustomBank({ ...frickEUR, receive: true }); + const incumbent = createCustomBank({ ...olkyEUR, receive: true }); + const banks = frickFirst ? [frick, incumbent] : [incumbent, frick]; + mockFindCachedByForBanks(bankRepo, banks); + mockFindCachedForBanks(bankRepo, []); + + const result = await service.getBank(createBankSelectorInput('GBP')); + expect(result).toBe(incumbent); + expect(result).not.toBe(frick); + }); + + it('returns undefined when neither the requested currency nor the EUR fallback can receive', async () => { + const disabledGbp = createCustomBank({ currency: 'GBP', receive: false }); + const disabledEur = createCustomBank({ ...olkyEUR, receive: false }); + mockFindCachedByForBanks(bankRepo, [disabledGbp, disabledEur]); + mockFindCachedForBanks(bankRepo, []); + + const result = await service.getBank(createBankSelectorInput('GBP')); + expect(result).toBeUndefined(); + }); - const eur = await service.getBank(createBankSelectorInput('EUR')); - expect(eur.name).toBe(IbanBankName.OLKY); + it('uses an instant-capable EUR fallback when GBP has no instant account', async () => { + const gbpBank = createCustomBank({ currency: 'GBP', receive: true, sctInst: false }); + const eurInstantBank = createCustomBank({ ...olkyEUR, receive: true, sctInst: true }); + mockFindCachedByForBanks(bankRepo, [gbpBank, eurInstantBank]); + mockFindCachedForBanks(bankRepo, []); - const chf = await service.getBank(createBankSelectorInput('CHF', 10000)); - expect(chf.name).toBe(IbanBankName.YAPEAL); + const result = await service.getBank(createBankSelectorInput('GBP', undefined, FiatPaymentMethod.INSTANT)); + expect(result).toBe(eurInstantBank); }); }); diff --git a/src/subdomains/supporting/bank/bank/bank.service.ts b/src/subdomains/supporting/bank/bank/bank.service.ts index 50bb04a34d..a23a8b3d44 100644 --- a/src/subdomains/supporting/bank/bank/bank.service.ts +++ b/src/subdomains/supporting/bank/bank/bank.service.ts @@ -85,19 +85,37 @@ export class BankService implements OnModuleInit { } // --- BANK SELECTOR --- // - async getBank({ currency, paymentMethod }: BankSelectorInput): Promise { + // Returns undefined when no eligible receiving (receive=true) bank exists for either the requested + // currency or the EUR fallback. + async getBank({ currency, paymentMethod }: BankSelectorInput): Promise { const fallBackCurrency = 'EUR'; - // Bank Frick's rows are receive=true so money arriving on its accounts is fully processed, but it - // must never be offered to a customer as a deposit target - customers are always shown the incumbent - // banks (Olkypay/Yapeal). It is deliberately filtered out of this customer-facing selector here; - // inbound crediting runs via BankTxFrickService, not this path, and the outbound payout selector - // applies its own separate Frick handling, so the exclusion affects only the deposit IBAN shown to - // customers. - const banks = (await this.getReceiveBanks()).filter((b) => b.name !== IbanBankName.FRICK); + const receiveBanks = await this.getReceiveBanks(); + + // Product decision, deliberately hardcoded: an EUR bank transfer is routed to Bank Frick. + // Resolved through getBankInternal so this picks the row selectAttributionBank would pick - + // (name, currency) is not unique, and the asset-linked identity is the one isBankMatching and the + // booked bank_tx history are keyed on. Choosing by any other rule here, e.g. the newest row, would + // hand out an IBAN that attribution does not follow. + // This aligns the selection RULE, not the caches: ibanCache is loaded once at module init while + // this read goes through the repository cache, so a bank row edited at runtime can still be seen + // differently by the two until the process restarts. That gap predates this rule and applies to + // every bank; do not read this call as a guarantee that the two can never disagree. + // The rule is scoped to exactly EUR + BANK; receive must still hold, since getBankInternal does + // not filter on it - and if the attributed row is not receiving, no other Frick row stands in for + // it, because the exclusion below then applies to all of them. + if (currency === 'EUR' && paymentMethod === FiatPaymentMethod.BANK) { + const frickEur = await this.getBankInternal(IbanBankName.FRICK, 'EUR'); + if (frickEur?.receive) return frickEur; + } + + // Everything below keeps the categorical exclusion the removed bank-name filter provided: Bank + // Frick must not win a currency it was never routed to, and must not be reachable through the + // instant lookup or the EUR currency fallback either. Only the explicit rule above may return it. + const banks = receiveBanks.filter((bank) => bank.name !== IbanBankName.FRICK); // select the matching bank account - let account: Bank; + let account: Bank | undefined; // instant bank if (!account && paymentMethod === FiatPaymentMethod.INSTANT) { @@ -117,7 +135,7 @@ export class BankService implements OnModuleInit { currencyName: string, fallBackCurrencyName: string, selector?: (bank: Bank) => boolean, - ): Bank { + ): Bank | undefined { const matchingBanks = selector ? banks.filter(selector) : banks; return ( diff --git a/src/subdomains/supporting/dashboard/__tests__/dashboard-financial.service.spec.ts b/src/subdomains/supporting/dashboard/__tests__/dashboard-financial.service.spec.ts index b54ea2fe7e..b9f983e1cd 100644 --- a/src/subdomains/supporting/dashboard/__tests__/dashboard-financial.service.spec.ts +++ b/src/subdomains/supporting/dashboard/__tests__/dashboard-financial.service.spec.ts @@ -3,18 +3,24 @@ import { Test, TestingModule } from '@nestjs/testing'; import { AssetService } from 'src/shared/models/asset/asset.service'; import { RefRewardService } from 'src/subdomains/core/referral/reward/services/ref-reward.service'; import { Log } from '../../log/log.entity'; +import { FinancialLogSummary } from '../../log/log.repository'; import { LogService } from '../../log/log.service'; import { DashboardFinancialService } from '../dashboard-financial.service'; describe('DashboardFinancialService', () => { let service: DashboardFinancialService; + let logService: LogService; + let assetService: AssetService; beforeEach(async () => { + logService = createMock(); + assetService = createMock(); + const module: TestingModule = await Test.createTestingModule({ providers: [ DashboardFinancialService, - { provide: LogService, useValue: createMock() }, - { provide: AssetService, useValue: createMock() }, + { provide: LogService, useValue: logService }, + { provide: AssetService, useValue: assetService }, { provide: RefRewardService, useValue: createMock() }, ], }).compile(); @@ -53,24 +59,205 @@ describe('DashboardFinancialService', () => { expect(entry.minus.binance).toEqual({ total: 7, withdraw: 1, trading: 6 }); }); - describe('mapLogToEntry (fxPnlChf exposure)', () => { - const logWith = (balancesTotal: object): Log => - ({ created: new Date('2026-07-14T00:00:00Z'), message: JSON.stringify({ balancesTotal }) }) as Log; + describe('mapSummaryToEntry (fxPnlChf exposure)', () => { + const summaryWith = (overrides: Partial): FinancialLogSummary => ({ + created: new Date('2026-07-14T00:00:00Z'), + id: 1, + totalBalanceChf: 100, + plusBalanceChf: 100, + minusBalanceChf: 0, + fxPnlChf: null, + btcPriceChf: 0, + balancesByType: {}, + ...overrides, + }); it('exposes the fxPnlChf written into the log entry, preserving a negative value', () => { - const entry = service['mapLogToEntry']( - logWith({ totalBalanceChf: 100, plusBalanceChf: 100, minusBalanceChf: 0, fxPnlChf: -3245 }), + const entry = service['mapSummaryToEntry']( + summaryWith({ totalBalanceChf: 100, plusBalanceChf: 100, minusBalanceChf: 0, fxPnlChf: -3245 }), ); - expect(entry?.fxPnlChf).toBe(-3245); + expect(entry.fxPnlChf).toBe(-3245); }); it('defaults historical entries logged before fxPnlChf existed to 0', () => { - const entry = service['mapLogToEntry']( - logWith({ totalBalanceChf: 100, plusBalanceChf: 100, minusBalanceChf: 0 }), + const entry = service['mapSummaryToEntry']( + summaryWith({ totalBalanceChf: 100, plusBalanceChf: 100, minusBalanceChf: 0, fxPnlChf: null }), + ); + + expect(entry.fxPnlChf).toBe(0); + }); + + it('defaults null totalBalanceChf/plusBalanceChf/minusBalanceChf (as the repository now returns for missing/null source data, F13) to 0 in the response — same as the old mapLogToEntry ?? 0 defaults', () => { + const entry = service['mapSummaryToEntry']( + summaryWith({ totalBalanceChf: null, plusBalanceChf: null, minusBalanceChf: null, fxPnlChf: null }), ); - expect(entry?.fxPnlChf).toBe(0); + expect(entry.totalBalanceChf).toBe(0); + expect(entry.plusBalanceChf).toBe(0); + expect(entry.minusBalanceChf).toBe(0); + expect(entry.fxPnlChf).toBe(0); + }); + + it('produces the same FinancialLogEntryDto the old mapLogToEntry would have for equivalent data', () => { + // Underlying FinanceLog.message JSON that the old mapper would have parsed: + // { + // balancesTotal: { totalBalanceChf: 1000, plusBalanceChf: 1500, minusBalanceChf: 500, fxPnlChf: -12.5 }, + // balancesByFinancialType: { + // Crypto: { plusBalance: 1, plusBalanceChf: 800, minusBalance: 0, minusBalanceChf: 200 }, + // Fiat: { plusBalance: 1, plusBalanceChf: 700, minusBalance: 0, minusBalanceChf: 300 }, + // }, + // assets: { "7": { priceChf: 65000.25 } }, + // } + // Old mapLogToEntry(log, 7) expected output (reconstructed byte-for-byte from that path): + const expectedFromOldMapper = { + timestamp: new Date('2026-07-14T12:00:00Z'), + totalBalanceChf: 1000, + plusBalanceChf: 1500, + minusBalanceChf: 500, + fxPnlChf: -12.5, + btcPriceChf: 65000.25, + balancesByType: { + Crypto: { plusBalanceChf: 800, minusBalanceChf: 200 }, + Fiat: { plusBalanceChf: 700, minusBalanceChf: 300 }, + }, + }; + + const summary: FinancialLogSummary = { + created: expectedFromOldMapper.timestamp, + id: 42, + totalBalanceChf: 1000, + plusBalanceChf: 1500, + minusBalanceChf: 500, + fxPnlChf: -12.5, + btcPriceChf: 65000.25, + balancesByType: { + Crypto: { plusBalanceChf: 800, minusBalanceChf: 200 }, + Fiat: { plusBalanceChf: 700, minusBalanceChf: 300 }, + }, + }; + + expect(service['mapSummaryToEntry'](summary)).toEqual(expectedFromOldMapper); + }); + + it('produces the same FinancialLogEntryDto the old mapLogToEntry/extractBtcPrice pathway would have when btcAssetId is undefined (no BTC asset resolved)', () => { + // Underlying FinanceLog.message JSON (assets present, but no BTC asset id was resolved this call): + // { + // balancesTotal: { totalBalanceChf: 1000, plusBalanceChf: 1500, minusBalanceChf: 500, fxPnlChf: -12.5 }, + // balancesByFinancialType: { Crypto: { plusBalanceChf: 800, minusBalanceChf: 200 } }, + // assets: { "7": { priceChf: 65000.25 } }, + // } + // Old extractBtcPrice(financeLog, undefined): `!btcAssetId` is true for undefined => returns 0, + // regardless of `assets` content. Old mapLogToEntry expected output: + const expectedFromOldMapper = { + timestamp: new Date('2026-07-14T12:00:00Z'), + totalBalanceChf: 1000, + plusBalanceChf: 1500, + minusBalanceChf: 500, + fxPnlChf: -12.5, + btcPriceChf: 0, + balancesByType: { Crypto: { plusBalanceChf: 800, minusBalanceChf: 200 } }, + }; + + // New path: getFinancialLogSummaries projects a SQL literal 0 when btcAssetId is undefined (no + // assets path parameter bound), so the summary already carries btcPriceChf: 0. + const summary: FinancialLogSummary = { + created: expectedFromOldMapper.timestamp, + id: 42, + totalBalanceChf: 1000, + plusBalanceChf: 1500, + minusBalanceChf: 500, + fxPnlChf: -12.5, + btcPriceChf: 0, + balancesByType: { Crypto: { plusBalanceChf: 800, minusBalanceChf: 200 } }, + }; + + expect(service['mapSummaryToEntry'](summary)).toEqual(expectedFromOldMapper); + }); + + it('produces the same FinancialLogEntryDto the old mapLogToEntry/extractBtcPrice pathway would have when btcAssetId is 0 (falsy, same as undefined)', () => { + // Same underlying FinanceLog.message JSON as above, but btcAssetId = 0 this time. + // Old extractBtcPrice(financeLog, 0): `!btcAssetId` is true for 0 (falsy) => returns 0, exactly + // like the undefined case above — this is the behaviour F1 restores (id=0 is unreachable in this + // database today, but the falsy check keeps the two cases byte-identical, as before the projection + // was moved into SQL). + const expectedFromOldMapper = { + timestamp: new Date('2026-07-14T12:00:00Z'), + totalBalanceChf: 1000, + plusBalanceChf: 1500, + minusBalanceChf: 500, + fxPnlChf: -12.5, + btcPriceChf: 0, + balancesByType: { Crypto: { plusBalanceChf: 800, minusBalanceChf: 200 } }, + }; + + // New path: getFinancialLogSummaries(0, ...) also takes the SQL-literal-0 branch (F1 falsy check), + // so the summary carries btcPriceChf: 0 here too — identical to the btcAssetId=undefined case. + const summary: FinancialLogSummary = { + created: expectedFromOldMapper.timestamp, + id: 43, + totalBalanceChf: 1000, + plusBalanceChf: 1500, + minusBalanceChf: 500, + fxPnlChf: -12.5, + btcPriceChf: 0, + balancesByType: { Crypto: { plusBalanceChf: 800, minusBalanceChf: 200 } }, + }; + + expect(service['mapSummaryToEntry'](summary)).toEqual(expectedFromOldMapper); + }); + }); + + describe('getFinancialLog', () => { + it('resolves getBtcCoin before getFinancialLogSummaries (ordering required for SQL btcAssetId)', async () => { + const btcAsset = { id: 7 } as Awaited>; + const summaries: FinancialLogSummary[] = [ + { + created: new Date('2026-07-14T00:00:00Z'), + id: 1, + totalBalanceChf: 100, + plusBalanceChf: 120, + minusBalanceChf: 20, + fxPnlChf: 1.5, + btcPriceChf: 64000, + balancesByType: { Crypto: { plusBalanceChf: 120, minusBalanceChf: 20 } }, + }, + ]; + + const getBtcCoinSpy = jest.spyOn(assetService, 'getBtcCoin').mockResolvedValue(btcAsset); + const getSummariesSpy = jest.spyOn(logService, 'getFinancialLogSummaries').mockResolvedValue(summaries); + + const from = new Date('2026-07-01T00:00:00Z'); + const result = await service.getFinancialLog(from, true); + + expect(getBtcCoinSpy).toHaveBeenCalled(); + expect(getSummariesSpy).toHaveBeenCalledWith(7, from, true); + // Ordering matters: btcAssetId is a SQL projection parameter, so getBtcCoin must finish first. + expect(getBtcCoinSpy.mock.invocationCallOrder[0]).toBeLessThan(getSummariesSpy.mock.invocationCallOrder[0]); + + expect(result.entries).toEqual([ + { + timestamp: summaries[0].created, + totalBalanceChf: 100, + plusBalanceChf: 120, + minusBalanceChf: 20, + fxPnlChf: 1.5, + btcPriceChf: 64000, + balancesByType: { Crypto: { plusBalanceChf: 120, minusBalanceChf: 20 } }, + }, + ]); + // Mutation guard: projected plus/minus and btc price must survive end-to-end. + expect(result.entries[0].plusBalanceChf).not.toBe(result.entries[0].minusBalanceChf); + expect(result.entries[0].btcPriceChf).toBe(64000); + }); + + it('passes undefined btcAssetId when getBtcCoin returns no asset', async () => { + jest.spyOn(assetService, 'getBtcCoin').mockResolvedValue(undefined as never); + const getSummariesSpy = jest.spyOn(logService, 'getFinancialLogSummaries').mockResolvedValue([]); + + await service.getFinancialLog(); + + expect(getSummariesSpy).toHaveBeenCalledWith(undefined, undefined, undefined); }); }); }); diff --git a/src/subdomains/supporting/dashboard/dashboard-financial.service.ts b/src/subdomains/supporting/dashboard/dashboard-financial.service.ts index b3c3e4b8bf..42fa6b397d 100644 --- a/src/subdomains/supporting/dashboard/dashboard-financial.service.ts +++ b/src/subdomains/supporting/dashboard/dashboard-financial.service.ts @@ -2,6 +2,7 @@ import { Injectable } from '@nestjs/common'; import { AssetService } from 'src/shared/models/asset/asset.service'; import { RefRewardService } from '../../core/referral/reward/services/ref-reward.service'; import { Log } from '../log/log.entity'; +import { FinancialLogSummary } from '../log/log.repository'; import { LogService } from '../log/log.service'; import { FinanceLog } from '../log/dto/log.dto'; import { @@ -23,16 +24,13 @@ export class DashboardFinancialService { ) {} async getFinancialLog(from?: Date, dailySample?: boolean): Promise { - const [logs, btcAsset] = await Promise.all([ - this.logService.getFinancialLogs(from, dailySample), - this.assetService.getBtcCoin(), - ]); - - const btcAssetId = btcAsset?.id; - const entries = logs - .map((log) => this.mapLogToEntry(log, btcAssetId)) - .filter((e): e is FinancialLogEntryDto => e != null); + // BTC price is projected in SQL and needs btcAssetId as a parameter, so resolve getBtcCoin first. + // One extra sequential roundtrip vs the previous Promise.all, judged negligible against the + // eliminated transfer volume of the full message JSON. + const btcAsset = await this.assetService.getBtcCoin(); + const summaries = await this.logService.getFinancialLogSummaries(btcAsset?.id, from, dailySample); + const entries = summaries.map((summary) => this.mapSummaryToEntry(summary)); return { entries }; } @@ -234,39 +232,25 @@ export class DashboardFinancialService { return { timestamp: latest.created, byType, byBlockchain }; } - private mapLogToEntry(log: Log, btcAssetId?: number): FinancialLogEntryDto | undefined { - try { - const financeLog: FinanceLog = JSON.parse(log.message); - - const btcPriceChf = this.extractBtcPrice(financeLog, btcAssetId); - - const balancesByType: Record = {}; - if (financeLog.balancesByFinancialType) { - for (const [type, data] of Object.entries(financeLog.balancesByFinancialType)) { - balancesByType[type] = { - plusBalanceChf: data.plusBalanceChf, - minusBalanceChf: data.minusBalanceChf, - }; - } - } - - return { - timestamp: log.created, - totalBalanceChf: financeLog.balancesTotal?.totalBalanceChf ?? 0, - plusBalanceChf: financeLog.balancesTotal?.plusBalanceChf ?? 0, - minusBalanceChf: financeLog.balancesTotal?.minusBalanceChf ?? 0, - fxPnlChf: financeLog.balancesTotal?.fxPnlChf ?? 0, - btcPriceChf, - balancesByType, - }; - } catch { - return undefined; - } - } - - private extractBtcPrice(financeLog: FinanceLog, btcAssetId?: number): number { - if (!financeLog.assets || !btcAssetId) return 0; - - return financeLog.assets[btcAssetId]?.priceChf ?? 0; + // Pure mapping over the SQL projection: for well-formed data, the response matches the previous + // mapLogToEntry path exactly, including the `?? 0` field defaults. One case is intentionally + // different: if a `balancesByFinancialType` entry has the value `null` (e.g. `{"Crypto": null}`), + // the row is now kept (with an empty balancesByType entry) instead of the previous mapLogToEntry's + // per-row try/catch silently dropping the whole log line — a silent gap in a financial curve is + // worse than an empty partial entry, so this was changed on purpose. A malformed `message` document + // still fails loud: the `message::jsonb` cast in SQL throws for that. Individual scalar field + // values, though, are tolerated via `jsonb_typeof` guards (nulled rather than thrown), and + // non-numeric values inside `balancesByFinancialType` are passed through unchanged + // (see log.repository.ts). + private mapSummaryToEntry(summary: FinancialLogSummary): FinancialLogEntryDto { + return { + timestamp: summary.created, + totalBalanceChf: summary.totalBalanceChf ?? 0, + plusBalanceChf: summary.plusBalanceChf ?? 0, + minusBalanceChf: summary.minusBalanceChf ?? 0, + fxPnlChf: summary.fxPnlChf ?? 0, + btcPriceChf: summary.btcPriceChf, + balancesByType: summary.balancesByType, + }; } } diff --git a/src/subdomains/supporting/dashboard/dto/financial-log.dto.ts b/src/subdomains/supporting/dashboard/dto/financial-log.dto.ts index e344a9e69b..b53f4fb8c3 100644 --- a/src/subdomains/supporting/dashboard/dto/financial-log.dto.ts +++ b/src/subdomains/supporting/dashboard/dto/financial-log.dto.ts @@ -7,7 +7,12 @@ export class FinancialLogEntryDto { // logged before this field existed (see BalancesTotal.fxPnlChf). fxPnlChf: number; btcPriceChf: number; - balancesByType: Record; + /** + * plusBalanceChf/minusBalanceChf can be missing per type when the source FinancialDataLog + * snapshot omitted one of the two keys (see FinancialLogSummary.balancesByType); a missing + * value is left out of the JSON response the same way it always was, never defaulted to 0. + */ + balancesByType: Record; } export class FinancialLogResponseDto { diff --git a/src/subdomains/supporting/fiat-output/fiat-output.service.ts b/src/subdomains/supporting/fiat-output/fiat-output.service.ts index 260147d99e..2c3bc47ceb 100644 --- a/src/subdomains/supporting/fiat-output/fiat-output.service.ts +++ b/src/subdomains/supporting/fiat-output/fiat-output.service.ts @@ -59,9 +59,10 @@ export class FiatOutputService { return { accountIban: virtualIban.iban, bank: virtualIban.bank }; } - // Automatic sender-bank selection is incumbent-banks-only. Bank Frick is payout-eligible exclusively - // through explicit per-output assignment (accountIban at creation or manual database assignment), - // mirroring the deliberate exclusion in BankService.getBank() for the customer-facing deposit selector. + // Automatic payout selection excludes Bank Frick by name; it is payout-eligible exclusively through + // explicit per-output assignment (accountIban at creation or manual database assignment). This is + // independent of the customer-facing deposit direction: BankService.getBank() deliberately routes + // EUR deposits to Bank Frick. The payout exclusion and deposit routing therefore do not conflict. const banks = (await this.bankService.getSenderBanks(currency)).filter( (candidate) => candidate.name !== IbanBankName.FRICK, ); diff --git a/src/subdomains/supporting/log/__tests__/log.repository.spec.ts b/src/subdomains/supporting/log/__tests__/log.repository.spec.ts index 6da4f4bbe2..98ba6df5d4 100644 --- a/src/subdomains/supporting/log/__tests__/log.repository.spec.ts +++ b/src/subdomains/supporting/log/__tests__/log.repository.spec.ts @@ -1,5 +1,5 @@ import { EntityManager, UpdateResult } from 'typeorm'; -import { FINANCIAL_LOG_VALIDITY_AUDIT_SUBSYSTEM } from '../log.entity'; +import { FINANCIAL_DATA_LOG_SUBSYSTEM, FINANCIAL_LOG_VALIDITY_AUDIT_SUBSYSTEM, LogSeverity } from '../log.entity'; import { LogRepository } from '../log.repository'; type UpdateQueryBuilderStub = { @@ -187,4 +187,471 @@ describe('LogRepository', () => { expect(stub.getExists).toHaveBeenCalled(); }); }); + + describe('getFinancialLogSummaries', () => { + it('maps the SQL projection into FinancialLogSummary (numbers, balancesByType, btc price)', async () => { + const repo = new LogRepository({} as EntityManager); + const created = new Date('2026-07-14T00:00:00Z'); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([ + { + created, + id: '42', + totalBalanceChf: '100.5', + plusBalanceChf: '200', + minusBalanceChf: '99.5', + fxPnlChf: '-3245', + btcPriceChf: '65000.25', + balancesByFinancialType: { + // Real jsonb-embedded numbers already arrive as JS numbers via the pg driver — no Number() + // coercion happens in this loop anymore (F10), so the mock must reflect that shape. + Crypto: { plusBalanceChf: 10, minusBalanceChf: 5, plusBalance: 1, minusBalance: 1 }, + Fiat: { plusBalanceChf: 90, minusBalanceChf: 40 }, + }, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(7); + + expect(rows).toEqual([ + { + created, + id: 42, + totalBalanceChf: 100.5, + plusBalanceChf: 200, + minusBalanceChf: 99.5, + fxPnlChf: -3245, + btcPriceChf: 65000.25, + balancesByType: { + Crypto: { plusBalanceChf: 10, minusBalanceChf: 5 }, + Fiat: { plusBalanceChf: 90, minusBalanceChf: 40 }, + }, + }, + ]); + // Projection must use distinct plus/minus columns (mutation: swap would fail this assertion). + expect(rows[0].plusBalanceChf).not.toBe(rows[0].minusBalanceChf); + expect(rows[0].balancesByType.Crypto.plusBalanceChf).toBe(10); + expect(rows[0].balancesByType.Crypto.minusBalanceChf).toBe(5); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + // Exact path→alias bindings: swapping plus/minus (or hard-coding btc) in the projection must fail. + // jsonb_typeof guards (F2/F3) wrap each cast; the path→alias pairing is still asserted. + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'totalBalanceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'balancesTotal' ->> 'totalBalanceChf')::float8`); + expect(sql).toContain(`END AS "totalBalanceChf"`); + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'plusBalanceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'balancesTotal' ->> 'plusBalanceChf')::float8`); + expect(sql).toContain(`END AS "plusBalanceChf"`); + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'minusBalanceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'balancesTotal' ->> 'minusBalanceChf')::float8`); + expect(sql).toContain(`END AS "minusBalanceChf"`); + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'assets' -> $5::text -> 'priceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'assets' -> $5::text ->> 'priceChf')::float8`); + expect(sql).toContain(`AS "btcPriceChf"`); + expect(sql).toContain(`message::jsonb -> 'balancesByFinancialType' AS "balancesByFinancialType"`); + expect(sql).not.toContain("-> 'tradings'"); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true, '7']); + }); + + it('projects btcPriceChf as SQL literal 0 when btcAssetId is undefined (no assets path param)', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([ + { + created: new Date('2026-07-14T00:00:00Z'), + id: 1, + totalBalanceChf: 1, + plusBalanceChf: 1, + minusBalanceChf: 0, + fxPnlChf: null, + btcPriceChf: 0, + balancesByFinancialType: null, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(undefined); + + expect(rows[0].btcPriceChf).toBe(0); + expect(rows[0].fxPnlChf).toBeNull(); + expect(rows[0].balancesByType).toEqual({}); + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('0::float8'); + expect(sql).not.toContain("-> 'assets'"); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true]); + }); + + it('defaults absent btc priceChf from SQL null to 0 in the mapping layer', async () => { + const repo = new LogRepository({} as EntityManager); + jest.spyOn(repo, 'query').mockResolvedValue([ + { + created: new Date('2026-07-14T00:00:00Z'), + id: 1, + totalBalanceChf: 1, + plusBalanceChf: 1, + minusBalanceChf: 0, + fxPnlChf: null, + btcPriceChf: null, + balancesByFinancialType: null, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(99); + expect(rows[0].btcPriceChf).toBe(0); + }); + + it('uses the dailySample MAX(id)-per-day subquery when dailySample is true', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + + await repo.getFinancialLogSummaries(undefined, undefined, true); + + const [sql] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('MAX(id)'); + expect(sql).toContain('CAST(created AS DATE)'); + expect(sql).toContain('id IN ('); + }); + + it('fails loud when the keyset cursor id no longer exists (no silent empty main query)', async () => { + const repo = new LogRepository({} as EntityManager); + jest.spyOn(repo, 'query').mockResolvedValue([]); + const stub = financialLogQueryBuilderStub(false); + jest.spyOn(repo, 'createQueryBuilder').mockReturnValue(stub as never); + + await expect( + repo.getFinancialLogSummaries(undefined, undefined, false, undefined, undefined, 999), + ).rejects.toThrow('Financial log cursor row 999 no longer exists'); + + expect(stub.getExists).toHaveBeenCalled(); + }); + + it('returns empty when the cursor still exists (legitimate end-of-data)', async () => { + const repo = new LogRepository({} as EntityManager); + jest.spyOn(repo, 'query').mockResolvedValue([]); + const stub = financialLogQueryBuilderStub(true); + jest.spyOn(repo, 'createQueryBuilder').mockReturnValue(stub as never); + + await expect( + repo.getFinancialLogSummaries(undefined, undefined, false, undefined, undefined, 999), + ).resolves.toEqual([]); + + expect(stub.getExists).toHaveBeenCalled(); + }); + + it('skips the existence check when after is unset', async () => { + const repo = new LogRepository({} as EntityManager); + jest.spyOn(repo, 'query').mockResolvedValue([]); + const stub = financialLogQueryBuilderStub(false); + jest.spyOn(repo, 'createQueryBuilder').mockReturnValue(stub as never); + + await expect(repo.getFinancialLogSummaries()).resolves.toEqual([]); + + expect(stub.getExists).not.toHaveBeenCalled(); + }); + + it('binds from/to/limit/after as incremental parameters after the fixed filters', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + const stub = financialLogQueryBuilderStub(true); + jest.spyOn(repo, 'createQueryBuilder').mockReturnValue(stub as never); + const from = new Date('2026-01-01T00:00:00Z'); + const to = new Date('2026-02-01T00:00:00Z'); + + await repo.getFinancialLogSummaries(3, from, false, to, 50, 10); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('created >='); + expect(sql).toContain('created <='); + expect(sql).toContain('(created, id) >'); + expect(sql).toContain('LIMIT'); + expect(params).toEqual([ + 'LogService', + FINANCIAL_DATA_LOG_SUBSYSTEM, + LogSeverity.INFO, + true, + '3', + from, + to, + 10, + 10, + 50, + ]); + }); + + it('orders by created ASC, id ASC (never DESC) so the chart stays chronological and keyset pagination advances forward', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + + await repo.getFinancialLogSummaries(); + + const [sql] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('ORDER BY created ASC, id ASC'); + }); + + describe('parameter position ($N placeholders match their array index)', () => { + const from = new Date('2026-01-01T00:00:00Z'); + const to = new Date('2026-02-01T00:00:00Z'); + + it('only from, btcAssetId set', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + + await repo.getFinancialLogSummaries(7, from); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('$5::text'); + expect(sql).toContain('created >= $6'); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true, '7', from]); + }); + + it('from + to, btcAssetId set', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + + await repo.getFinancialLogSummaries(7, from, false, to); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('created >= $6'); + expect(sql).toContain('created <= $7'); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true, '7', from, to]); + }); + + it('from + limit, btcAssetId set, dailySample true', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + + await repo.getFinancialLogSummaries(7, from, true, undefined, 50); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('created >= $6'); + expect(sql).toContain('LIMIT $7'); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true, '7', from, 50]); + }); + + it('from + after + limit, btcAssetId set', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + jest.spyOn(repo, 'createQueryBuilder').mockReturnValue(financialLogQueryBuilderStub(true) as never); + + await repo.getFinancialLogSummaries(7, from, false, undefined, 50, 10); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('created >= $6'); + expect(sql).toContain('(created, id) > ((SELECT c.created FROM log c WHERE c.id = $7), $8)'); + expect(sql).toContain('LIMIT $9'); + expect(params).toEqual([ + 'LogService', + FINANCIAL_DATA_LOG_SUBSYSTEM, + LogSeverity.INFO, + true, + '7', + from, + 10, + 10, + 50, + ]); + }); + + it('none of from/to/after/limit, btcAssetId set', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + + await repo.getFinancialLogSummaries(7); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('$5::text'); + expect(sql).not.toContain('created >='); + expect(sql).not.toContain('created <='); + expect(sql).not.toContain('(created, id) >'); + expect(sql).not.toContain('LIMIT'); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true, '7']); + }); + + it('btcAssetId undefined, from + after', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + jest.spyOn(repo, 'createQueryBuilder').mockReturnValue(financialLogQueryBuilderStub(true) as never); + + await repo.getFinancialLogSummaries(undefined, from, false, undefined, undefined, 10); + + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('0::float8'); + expect(sql).toContain('created >= $5'); + expect(sql).toContain('(created, id) > ((SELECT c.created FROM log c WHERE c.id = $6), $7)'); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true, from, 10, 10]); + }); + }); + + it('guards all five projected number fields with jsonb_typeof so one bad value nulls only that field (F2/F3)', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([]); + + await repo.getFinancialLogSummaries(7); + + const [sql] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'totalBalanceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'balancesTotal' ->> 'totalBalanceChf')::float8`); + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'plusBalanceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'balancesTotal' ->> 'plusBalanceChf')::float8`); + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'minusBalanceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'balancesTotal' ->> 'minusBalanceChf')::float8`); + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'fxPnlChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'balancesTotal' ->> 'fxPnlChf')::float8`); + expect(sql).toContain(`WHEN jsonb_typeof(message::jsonb -> 'assets' -> $5::text -> 'priceChf') = 'number'`); + expect(sql).toContain(`THEN (message::jsonb -> 'assets' -> $5::text ->> 'priceChf')::float8`); + }); + + it('projects btcPriceChf as SQL literal 0 when btcAssetId is 0 — same falsy branch as undefined (F1)', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([ + { + created: new Date('2026-07-14T00:00:00Z'), + id: 1, + totalBalanceChf: 1, + plusBalanceChf: 1, + minusBalanceChf: 0, + fxPnlChf: null, + btcPriceChf: 0, + balancesByFinancialType: null, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(0); + + expect(rows[0].btcPriceChf).toBe(0); + const [sql, params] = querySpy.mock.calls[0] as [string, unknown[]]; + expect(sql).toContain('0::float8'); + expect(sql).not.toContain(`-> 'assets'`); + expect(params).toEqual(['LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true]); + }); + + it('maps a raw row where balancesTotal is entirely missing (all guards fire) to null/null/null (not 0) and keeps fxPnlChf null — the ?? 0 default happens only in mapSummaryToEntry, never in the repository (F13)', async () => { + const repo = new LogRepository({} as EntityManager); + const created = new Date('2026-07-14T00:00:00Z'); + jest.spyOn(repo, 'query').mockResolvedValue([ + { + created, + id: 1, + totalBalanceChf: null, + plusBalanceChf: null, + minusBalanceChf: null, + fxPnlChf: null, + btcPriceChf: '65000.25', + balancesByFinancialType: null, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(7); + + expect(rows[0].totalBalanceChf).toBeNull(); + expect(rows[0].plusBalanceChf).toBeNull(); + expect(rows[0].minusBalanceChf).toBeNull(); + expect(rows[0].fxPnlChf).toBeNull(); + expect(rows[0].btcPriceChf).toBe(65000.25); + }); + + it('maps a raw row with JSON null for only totalBalanceChf/plusBalanceChf (minusBalanceChf/fxPnlChf unaffected) to null/null, not 0 (F13)', async () => { + const repo = new LogRepository({} as EntityManager); + const created = new Date('2026-07-14T00:00:00Z'); + jest.spyOn(repo, 'query').mockResolvedValue([ + { + created, + id: 2, + totalBalanceChf: null, + plusBalanceChf: null, + minusBalanceChf: '500', + fxPnlChf: '-12.5', + btcPriceChf: '65000.25', + balancesByFinancialType: null, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(7); + + expect(rows[0].totalBalanceChf).toBeNull(); + expect(rows[0].plusBalanceChf).toBeNull(); + expect(rows[0].minusBalanceChf).toBe(500); + expect(rows[0].fxPnlChf).toBe(-12.5); + expect(rows[0].btcPriceChf).toBe(65000.25); + }); + + it('passes plusBalanceChf through unconverted when the key is missing from one balancesByFinancialType entry, instead of Number(undefined) => NaN => null (F10, matches a real production row)', async () => { + const repo = new LogRepository({} as EntityManager); + const created = new Date('2026-07-14T00:00:00Z'); + jest.spyOn(repo, 'query').mockResolvedValue([ + { + created, + id: 99, + totalBalanceChf: 100, + plusBalanceChf: 100, + minusBalanceChf: 0, + fxPnlChf: null, + btcPriceChf: 0, + balancesByFinancialType: { + Crypto: { minusBalanceChf: 5 }, // plusBalanceChf key missing, as observed in production + }, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(); + + expect(rows[0].balancesByType.Crypto.plusBalanceChf).toBeUndefined(); + expect(rows[0].balancesByType.Crypto.minusBalanceChf).toBe(5); + // The old mapLogToEntry omitted the key entirely for a missing value; JSON serialisation drops an + // undefined-valued key the same way — never NaN, never null. + expect(JSON.parse(JSON.stringify(rows[0].balancesByType.Crypto))).toEqual({ minusBalanceChf: 5 }); + }); + + it('keeps the row and yields undefined fields (not 0, not null, no error) when a balancesByFinancialType entry is null, a number, or a string (F16 reverts the F11 throw)', async () => { + const repo = new LogRepository({} as EntityManager); + const querySpy = jest.spyOn(repo, 'query').mockResolvedValue([ + { + created: new Date('2026-07-14T00:00:00Z'), + id: 77, + totalBalanceChf: 100, + plusBalanceChf: 100, + minusBalanceChf: 0, + fxPnlChf: null, + btcPriceChf: 0, + balancesByFinancialType: { Crypto: null, Fiat: 1, Other: 'bad' }, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(); + + expect(querySpy).toHaveBeenCalledTimes(1); + expect(rows).toHaveLength(1); + expect(rows[0].balancesByType.Crypto.plusBalanceChf).toBeUndefined(); + expect(rows[0].balancesByType.Crypto.minusBalanceChf).toBeUndefined(); + expect(rows[0].balancesByType.Fiat.plusBalanceChf).toBeUndefined(); + expect(rows[0].balancesByType.Fiat.minusBalanceChf).toBeUndefined(); + expect(rows[0].balancesByType.Other.plusBalanceChf).toBeUndefined(); + expect(rows[0].balancesByType.Other.minusBalanceChf).toBeUndefined(); + // Same JSON-serialisation check as the F10 test: undefined values are omitted, never null/0. + expect(JSON.parse(JSON.stringify(rows[0].balancesByType))).toEqual({ Crypto: {}, Fiat: {}, Other: {} }); + }); + + it('yields undefined for both fields (no error, row kept) when a balancesByFinancialType entry is an object but its properties have the wrong type (string/boolean)', async () => { + const repo = new LogRepository({} as EntityManager); + jest.spyOn(repo, 'query').mockResolvedValue([ + { + created: new Date('2026-07-14T00:00:00Z'), + id: 88, + totalBalanceChf: 100, + plusBalanceChf: 100, + minusBalanceChf: 0, + fxPnlChf: null, + btcPriceChf: 0, + balancesByFinancialType: { + Crypto: { plusBalanceChf: 'bad', minusBalanceChf: true }, + }, + }, + ]); + + const rows = await repo.getFinancialLogSummaries(); + + expect(rows).toHaveLength(1); + expect(rows[0].balancesByType.Crypto.plusBalanceChf).toBeUndefined(); + expect(rows[0].balancesByType.Crypto.minusBalanceChf).toBeUndefined(); + // Wrong-typed values become undefined and are dropped on serialisation — never null/0/string/boolean. + expect(JSON.parse(JSON.stringify(rows[0].balancesByType.Crypto))).toEqual({}); + }); + }); }); diff --git a/src/subdomains/supporting/log/__tests__/log.service.spec.ts b/src/subdomains/supporting/log/__tests__/log.service.spec.ts index 748cbd5487..8c06347f83 100644 --- a/src/subdomains/supporting/log/__tests__/log.service.spec.ts +++ b/src/subdomains/supporting/log/__tests__/log.service.spec.ts @@ -12,7 +12,7 @@ import { LogSeverity, MAX_VALIDITY_SWEEP_ROWS, } from '../log.entity'; -import { LogRepository } from '../log.repository'; +import { FinancialLogSummary, LogRepository } from '../log.repository'; import { LogService } from '../log.service'; jest.mock('../log.repository'); @@ -270,6 +270,30 @@ describe('LogService', () => { }); }); + describe('getFinancialLogSummaries', () => { + it('delegates to the repository with the same arguments and returns FinancialLogSummary-shaped rows', async () => { + const from = new Date('2026-07-01T00:00:00Z'); + const to = new Date('2026-07-15T00:00:00Z'); + const summaries: FinancialLogSummary[] = [ + { + created: from, + id: 11, + totalBalanceChf: 100, + plusBalanceChf: 150, + minusBalanceChf: 50, + fxPnlChf: -12, + btcPriceChf: 65000, + balancesByType: { Crypto: { plusBalanceChf: 150, minusBalanceChf: 50 } }, + }, + ]; + const spy = jest.spyOn(logRepo, 'getFinancialLogSummaries').mockResolvedValue(summaries); + + await expect(service.getFinancialLogSummaries(7, from, true, to, 25, 10)).resolves.toEqual(summaries); + + expect(spy).toHaveBeenCalledWith(7, from, true, to, 25, 10); + }); + }); + describe('create', () => { it('should reject fabricated financial log validity audit records', async () => { const saveSpy = jest.spyOn(logRepo, 'save'); diff --git a/src/subdomains/supporting/log/log.repository.ts b/src/subdomains/supporting/log/log.repository.ts index 6be2809ef7..6da5244332 100644 --- a/src/subdomains/supporting/log/log.repository.ts +++ b/src/subdomains/supporting/log/log.repository.ts @@ -40,6 +40,41 @@ export interface FinancialLogAssetPrice { logId: number; } +/** + * Dashboard financial-log chart fields projected from a FinancialDataLog snapshot (no full message JSON). + * Contains exactly what mapSummaryToEntry needs so it never touches log.message. + */ +export interface FinancialLogSummary { + created: Date; + id: number; + /** + * null when absent/non-numeric in the source JSON (missing key, JSON null, or a wrong JSON type) — + * mapSummaryToEntry keeps its existing `?? 0` default at the call site; this method must NOT default + * it itself. + */ + totalBalanceChf: number | null; + /** Same null semantics as totalBalanceChf. */ + plusBalanceChf: number | null; + /** Same null semantics as totalBalanceChf. */ + minusBalanceChf: number | null; + /** + * null when absent in the source JSON (first entry has no previous snapshot to diff against, see + * BalancesTotal.fxPnlChf) — mapSummaryToEntry keeps its existing `?? 0` default at the call site; + * this method must NOT default it itself. + */ + fxPnlChf: number | null; + /** + * 0 when btcAssetId is falsy (undefined or 0) or the asset key/price is unusable — computed in SQL + * only when btcAssetId is truthy. + */ + btcPriceChf: number; + /** + * plusBalanceChf/minusBalanceChf can be `undefined` per type: a real production row had a + * `balancesByFinancialType` entry missing one of the two keys (see getFinancialLogSummaries below). + */ + balancesByType: Record; +} + @Injectable() export class LogRepository extends BaseRepository { constructor(manager: EntityManager) { @@ -324,6 +359,204 @@ ORDER BY l.created ASC, l.id ASC`; return rows; } + /** + * SQL-side projection of the small FinancialDataLog sub-trees needed by the dashboard financial log chart + * (balancesTotal scalars, optional BTC priceChf, balancesByFinancialType). Callers avoid shipping/parsing + * the full ~42 KB `message` JSON per row — `assets` and `tradings` are never selected/transferred. + * + * balancesByFinancialType is selected as a single jsonb sub-object column (not LATERAL-expanded): it is much + * smaller than the parent message and excludes assets/tradings; reduced to plus/minus CHF per type in JS. + * + * Malformed `message` JSON fails loud: `message::jsonb` aborts the whole query — same fail-loud choice as + * getFinancialLogAssetPrices (volume-tested against all 31,925 matching rows in production, zero invalid + * JSON found; re-stated here, not re-verified). + * + * Optional `after` keyset cursor and `dailySample` match getFinancialLogs semantics (see that method). + * + * All five number fields below (totalBalanceChf, plusBalanceChf, minusBalanceChf, btcPriceChf and + * fxPnlChf) are guarded with `jsonb_typeof(...) = 'number'` — same pattern as the priceChf guard in + * getFinancialLogAssetPrices above. A non-numeric value (missing key, JSON null, or a wrong JSON type) + * nulls only that one field instead of aborting the whole query via a failing ::float8 cast; the outer + * message::jsonb cast itself stays fail-loud. SQL NULL is kept as `null` in the mapping below for + * totalBalanceChf/plusBalanceChf/minusBalanceChf/fxPnlChf (btcPriceChf is the only one of the five + * defaulted to 0 here, matching extractBtcPrice's `?.priceChf ?? 0`); mapSummaryToEntry's existing + * `?? 0` default at the call site turns a null field into 0 in the response for total/plus/minus/fxPnl, + * matching the old mapLogToEntry `?? 0` defaults for the same underlying data. That equality holds for + * a missing key or a JSON null value in the source — it does NOT hold for a value that is present but + * wrongly typed (e.g. the JSON string "100" in a number field): the old path passed such a string + * through unchanged, while the jsonb_typeof guard here turns it into SQL NULL and therefore 0 + * downstream. No such value exists in production today (verified: 31,952 of 31,956 balancesTotal rows + * have plusBalanceChf/minusBalanceChf as JSON 'number', the remaining 4 rows have a missing key or JSON + * null — no string observed); the scope limit above is deliberate, not a bug. + */ + async getFinancialLogSummaries( + btcAssetId?: number, + from?: Date, + dailySample?: boolean, + to?: Date, + limit?: number, + after?: number, // id of the last row of the previous page; NEVER a Date/created value + ): Promise { + const params: unknown[] = []; + let i = 1; + + // Fixed filter params shared by the main WHERE and (when dailySample) the MAX(id) subquery. + const systemParam = `$${i++}`; + const subsystemParam = `$${i++}`; + const severityParam = `$${i++}`; + const validParam = `$${i++}`; + params.push('LogService', FINANCIAL_DATA_LOG_SUBSYSTEM, LogSeverity.INFO, true); + + // BTC price: only bind a parameter when btcAssetId is truthy — mirrors the old extractBtcPrice's + // `!btcAssetId` falsy check (0/undefined/null all take the "no BTC asset" path), not merely + // `!== undefined`. btcAssetId=0 is unreachable in this database (asset ids start at 1), so the + // difference is not observable today, but the falsy check preserves byte-identical behaviour with + // extractBtcPrice for any future btcAssetId=0 — do not "fix" this back to `!== undefined`. + let btcPriceSelect: string; + if (btcAssetId) { + const assetPath = `message::jsonb -> 'assets' -> $${i}::text`; + // jsonb_typeof guard (same pattern as the balancesTotal fields below and getFinancialLogAssetPrices' + // priceChf guard above): a missing asset entry or a non-numeric priceChf value nulls only this + // field instead of aborting the whole query via a failing ::float8 cast. Number(null) => 0 in the + // mapping below, matching extractBtcPrice's `?.priceChf ?? 0` and its 0-return paths. + btcPriceSelect = `CASE WHEN jsonb_typeof(${assetPath} -> 'priceChf') = 'number' THEN (${assetPath} ->> 'priceChf')::float8 ELSE NULL END`; + params.push(String(btcAssetId)); + i++; + } else { + btcPriceSelect = '0::float8'; + } + + const conditions: string[] = []; + if (dailySample) { + // Same daily-sample shape as getFinancialLogs: restrict to MAX(id) per calendar day among valid INFO + // FinancialDataLog rows, then apply from/to/after/limit on the outer filtered set. + conditions.push( + `id IN (SELECT MAX(id) FROM log WHERE system = ${systemParam} AND subsystem = ${subsystemParam} AND severity = ${severityParam} AND valid = ${validParam} GROUP BY CAST(created AS DATE))`, + ); + } else { + conditions.push( + `system = ${systemParam}`, + `subsystem = ${subsystemParam}`, + `severity = ${severityParam}`, + `valid = ${validParam}`, + ); + } + + if (from) { + conditions.push(`created >= $${i++}`); + params.push(from); + } + if (to) { + conditions.push(`created <= $${i++}`); + params.push(to); + } + if (after != null) { + // Same row-value keyset as getFinancialLogs: created resolved in-DB at full precision. + conditions.push(`(created, id) > ((SELECT c.created FROM log c WHERE c.id = $${i}), $${i + 1})`); + params.push(after, after); + i += 2; + } + + let limitClause = ''; + if (limit != null) { + limitClause = `LIMIT $${i++}`; + params.push(limit); + } + + const sql = ` +SELECT created AS "created", + id AS "id", + CASE + WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'totalBalanceChf') = 'number' + THEN (message::jsonb -> 'balancesTotal' ->> 'totalBalanceChf')::float8 + ELSE NULL + END AS "totalBalanceChf", + CASE + WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'plusBalanceChf') = 'number' + THEN (message::jsonb -> 'balancesTotal' ->> 'plusBalanceChf')::float8 + ELSE NULL + END AS "plusBalanceChf", + CASE + WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'minusBalanceChf') = 'number' + THEN (message::jsonb -> 'balancesTotal' ->> 'minusBalanceChf')::float8 + ELSE NULL + END AS "minusBalanceChf", + CASE + WHEN jsonb_typeof(message::jsonb -> 'balancesTotal' -> 'fxPnlChf') = 'number' + THEN (message::jsonb -> 'balancesTotal' ->> 'fxPnlChf')::float8 + ELSE NULL + END AS "fxPnlChf", + ${btcPriceSelect} AS "btcPriceChf", + message::jsonb -> 'balancesByFinancialType' AS "balancesByFinancialType" +FROM log +WHERE ${conditions.join(' AND ')} +ORDER BY created ASC, id ASC +${limitClause}`; + + const raw = (await this.query(sql, params)) as { + created: Date | string; + id: number | string; + totalBalanceChf: number | string | null; + plusBalanceChf: number | string | null; + minusBalanceChf: number | string | null; + fxPnlChf: number | string | null; + btcPriceChf: number | string | null; + balancesByFinancialType: unknown; + }[]; + + const rows: FinancialLogSummary[] = raw.map((r) => { + // pg may return numeric columns as strings; coerce with Number(...) like getFinancialLogAssetPrices. + // totalBalanceChf/plusBalanceChf/minusBalanceChf/fxPnlChf all stay null when the SQL projection + // above nulled them (do NOT default any of them to 0 here — that belongs to mapSummaryToEntry's + // `?? 0` at the call site). + // btcPriceChf: absent/unusable path → 0, matching extractBtcPrice's `?.priceChf ?? 0`. + const btcPriceChf = r.btcPriceChf == null ? 0 : Number(r.btcPriceChf); + + const balancesByType: Record = {}; + if (r.balancesByFinancialType != null) { + // Always an already-parsed object/array here, never a JSON string: pg-types registers JSON.parse + // as the type parser for jsonb (OID 3802) and this repo configures no custom type parser, so the + // driver never hands back a raw string for this column. + const byType = r.balancesByFinancialType as Record< + string, + { plusBalanceChf?: number; minusBalanceChf?: number } + >; + for (const [type, data] of Object.entries(byType)) { + // Optional chaining keeps non-object entries (null / number / string / boolean) from throwing: + // property access yields undefined and the row is retained with empty fields, rather than + // failing the whole request. + // Only real numbers are kept for plusBalanceChf / minusBalanceChf; any non-number value + // (string, boolean, null, nested object, or missing key) becomes undefined so the result + // matches the number | undefined contract. On current production data this is a no-op + // (287,989 entries both numbers, one missing plusBalanceChf key — no string/boolean/null), + // and exists only to protect the contract for future/other data. The previous mapLogToEntry + // passed contract-breaking values through unchanged; this closes that hole. Same hardening + // idea as the five scalar fields above (jsonb_typeof = 'number' in SQL), applied in + // TypeScript because balancesByFinancialType is passed through as a raw JSON object. + const asNumber = (v: unknown): number | undefined => (typeof v === 'number' ? v : undefined); + balancesByType[type] = { + plusBalanceChf: asNumber(data?.plusBalanceChf), + minusBalanceChf: asNumber(data?.minusBalanceChf), + }; + } + } + + return { + created: r.created instanceof Date ? r.created : new Date(r.created), + id: Number(r.id), + totalBalanceChf: r.totalBalanceChf == null ? null : Number(r.totalBalanceChf), + plusBalanceChf: r.plusBalanceChf == null ? null : Number(r.plusBalanceChf), + minusBalanceChf: r.minusBalanceChf == null ? null : Number(r.minusBalanceChf), + fxPnlChf: r.fxPnlChf == null ? null : Number(r.fxPnlChf), + btcPriceChf, + balancesByType, + }; + }); + + if (!rows.length && after != null) await this.assertEmptyResultIsEndOfData(after); + return rows; + } + // After an empty main-query result with a keyset cursor, fail loud when the cursor id is gone: the row-value // subquery would return NULL and `(created, id) > (NULL, :afterId)` is NULL in Postgres → WHERE excludes every // row → silent empty result that callers misread as end-of-data. Only invoked when the main query already diff --git a/src/subdomains/supporting/log/log.service.ts b/src/subdomains/supporting/log/log.service.ts index d122a4708e..da29c6fa5f 100644 --- a/src/subdomains/supporting/log/log.service.ts +++ b/src/subdomains/supporting/log/log.service.ts @@ -13,7 +13,7 @@ import { LogSeverity, MAX_VALIDITY_SWEEP_ROWS, } from './log.entity'; -import { FinancialLogAssetPrice, LogRepository } from './log.repository'; +import { FinancialLogAssetPrice, FinancialLogSummary, LogRepository } from './log.repository'; @Injectable() export class LogService { @@ -154,6 +154,17 @@ export class LogService { return this.logRepo.getFinancialLogAssetPrices(from, to, limit, after); } + async getFinancialLogSummaries( + btcAssetId?: number, + from?: Date, + dailySample?: boolean, + to?: Date, + limit?: number, + after?: number, // id of the last row of the previous page; NEVER a Date/created value + ): Promise { + return this.logRepo.getFinancialLogSummaries(btcAssetId, from, dailySample, to, limit, after); + } + async getLatestFinancialLog(): Promise { return this.logRepo.getLatestFinancialLog(); }