perf: project the read paths, with the test harness the four levels need

.github/workflows/api-pr.yaml

@@ -228,3 +228,46 @@ jobs:
         run: npm test -- --shard=${{ matrix.shard }}/3
         env:
           MIGRATION_TEST_PG: postgres://postgres:postgres@localhost:5432/postgres
+
+  projection:
+    name: Read-path projections
+    runs-on: ubuntu-latest
+    services:
+      # These specs build the schema from the entity metadata and assert against real rows: a mocked
+      # repository cannot observe which columns a query asked for, so none of the four levels in
+      # docs/read-path-projections.md can be tested without a database.
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_PASSWORD: postgres
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v5
+
+      - name: Use Node.js ${{ env.NODE_VERSION }}
+        uses: actions/setup-node@v5
+        with:
+          node-version: ${{ env.NODE_VERSION }}
+          cache: 'npm'
+
+      - name: Install packages
+        uses: nick-fields/retry@v4
+        with:
+          timeout_minutes: 10
+          max_attempts: 3
+          retry_on: any
+          command: npm ci
+
+      # Separate from the sharded run because it compiles with full type information; see
+      # jest.projection.config.js for why the main suite cannot host these.
+      - name: Run projection tests
+        run: npm run test:projection
+        env:
+          MIGRATION_TEST_PG: postgres://postgres:postgres@localhost:5432/postgres

jest-projection.setup.ts

@@ -0,0 +1,9 @@
+// Turns the projection guard on for every spec in this configuration.
+//
+// A projected query that answers with a column it did not select is the defect this suite exists
+// to catch, and it is silent by nature. The guard makes it throw. Installing it here rather than
+// per spec is deliberate: a spec written later would otherwise lose the protection without anything
+// saying so.
+import { installProjectionGuard } from 'src/shared/utils/projection-test.util';
+
+installProjectionGuard();

jest.coverage-gate.config.js

@@ -69,6 +69,7 @@ const PINNED_LOGIC = [
   'src/shared/decorators/log-rejected-value.decorator.ts',
   'src/shared/models/fiat/fiat.controller.ts',
   'src/shared/pipes/detailed-validation.pipe.ts',
+  'src/shared/models/read-projection.ts',
   'src/shared/services/typeorm-logger.ts',
   'src/shared/utils/bitbox-ascii.util.ts',
   'src/shared/utils/cron.ts',

jest.projection.config.js

@@ -0,0 +1,27 @@
+// Read-path projection tests (docs/read-path-projections.md).
+//
+// Kept out of the main suite for one reason, and it is a hard one: these specs build a TypeORM data
+// source from the entity sources, and that needs the decorator metadata to carry real types. The
+// main suite runs ts-jest transpile-only (tsconfig.json sets isolatedModules: true), which emits
+// `design:type` as `Object` for any imported type — an enum column then fails metadata validation
+// with `Data type "Object" ... is not supported`, and no projection spec could run at all. The same
+// reasoning is why the Frick and coverage gates compile with tsconfig.coverage.json.
+//
+// The database gate is shared with the migration specs on purpose: without MIGRATION_TEST_PG these
+// suites skip, so a machine with no database still runs a green suite.
+const base = require('./package.json').jest;
+
+module.exports = {
+  ...base,
+  transform: { '^.+\\.(t|j)s$': ['ts-jest', { tsconfig: 'tsconfig.coverage.json' }] },
+  testRegex: '.*\\.projection\\.spec\\.ts$',
+  // The guard is installed once for the whole configuration; see jest-projection.setup.ts.
+  setupFilesAfterEnv: ['<rootDir>/../jest-projection.setup.ts'],
+  // The base config excludes these specs so the main suite does not pick them up. Spreading it in
+  // would exclude them here too — this run is the one that must find them.
+  testPathIgnorePatterns: ['/node_modules/'],
+  // Each spec file creates and drops its own Postgres schema, which is what allows them to run in
+  // parallel — but the schema is built by `synchronize` over 113 entities and costs about half a
+  // minute, so the wall clock is dominated by how many files there are, not by how many assertions.
+  testTimeout: 300000,
+};

package.json

@@ -23,6 +23,7 @@
     "test:frick:cov": "jest --config jest.frick.config.js integration/bank/services/__tests__/frick.service.spec.ts integration/bank/services/__tests__/iso20022.service.spec.ts config/__tests__/frick.config.spec.ts config/__tests__/bank-frick-config.spec.ts subdomains/supporting/bank-tx/bank-tx/services/__tests__/bank-tx-frick.service.spec.ts subdomains/supporting/bank-tx/bank-tx/services/__tests__/bank-tx-outgoing-match.service.spec.ts subdomains/supporting/fiat-output/__tests__/fiat-output-frick.service.spec.ts subdomains/supporting/bank/virtual-iban/__tests__/virtual-iban-frick-issuance-reconciliation.service.spec.ts subdomains/supporting/bank/virtual-iban/__tests__/virtual-iban.service.spec.ts subdomains/supporting/bank/virtual-iban/providers/__tests__/frick-viban.provider.spec.ts --coverage --runInBand --collectCoverageFrom=integration/bank/dto/frick.dto.ts --collectCoverageFrom=integration/bank/services/frick.service.ts --collectCoverageFrom=integration/bank/services/iso20022.service.ts --collectCoverageFrom=config/frick.config.ts --collectCoverageFrom=subdomains/supporting/bank-tx/bank-tx/services/bank-tx-frick.service.ts --collectCoverageFrom=subdomains/supporting/bank-tx/bank-tx/services/bank-tx-outgoing-match.service.ts --collectCoverageFrom=subdomains/supporting/fiat-output/fiat-output-frick.service.ts --collectCoverageFrom=subdomains/supporting/bank/virtual-iban/virtual-iban-frick-issuance-reconciliation.service.ts --collectCoverageFrom=subdomains/supporting/bank/virtual-iban/virtual-iban.service.ts --collectCoverageFrom=subdomains/supporting/bank/virtual-iban/providers/frick-viban.provider.ts",
     "test:gate:cov": "jest --config jest.coverage-gate.config.js --coverage --silent",
     "test:staff-gate:cov": "jest --config jest.staff-gate.config.js shared/auth/__tests__/role.guard.spec.ts shared/auth/__tests__/staff-kyc-clearance.spec.ts subdomains/generic/user/models/user/__tests__/staff-kyc-clearance.service.spec.ts --coverage --runInBand --collectCoverageFrom=shared/auth/role.guard.ts --collectCoverageFrom=shared/auth/staff-kyc-clearance.ts --collectCoverageFrom=shared/auth/exceptions/staff-kyc-required.exception.ts --collectCoverageFrom=subdomains/generic/user/models/user/staff-kyc-clearance.service.ts",
+    "test:projection": "jest --config jest.projection.config.js --maxWorkers=2",
     "type-check": "tsc --noEmit",
     "format:check": "prettier --check \"src/**/*.ts\" \"test/**/*.ts\"",
     "check": "npm run lint && npm run test",
@@ -193,6 +194,9 @@
       "^@arkade-os/sdk$": "<rootDir>/integration/blockchain/arkade/__mocks__/arkade-sdk.mock.ts"
     },
     "testRegex": ".*\\.spec\\.ts$",
+    "testPathIgnorePatterns": [
+      "\\.projection\\.spec\\.ts$"
+    ],
     "transform": {
       "^.+\\.(t|j)s$": "ts-jest"
     },

src/shared/models/__tests__/eager-relations.projection.spec.ts

@@ -0,0 +1,272 @@
+import { readdirSync, readFileSync } from 'fs';
+import { join } from 'path';
+import {
+  createProjectionDataSource,
+  describeProjection,
+  destroyProjectionDataSource,
+} from 'src/shared/utils/projection-test.util';
+import { DataSource, EntityMetadata } from 'typeorm';
+
+const SCHEMA = 'eager_relations_spec';
+const SRC = join(__dirname, '../../..');
+
+/**
+ * The entities that leave through a controller as themselves — for those, the eager relations are
+ * the response rather than a loading detail.
+ *
+ * Read out of the source rather than listed, so that a controller added later is covered without
+ * anyone remembering a list. Deliberately generous: any method in a controller file whose return
+ * type names an entity counts, including through a renamed import. An over-match costs precision
+ * in the message, a miss costs the guarantee.
+ */
+function entitiesReturnedWhole(entities: Set<string>): Map<string, string[]> {
+  const controllers: string[] = [];
+  const walk = (directory: string): void => {
+    for (const item of readdirSync(directory, { withFileTypes: true })) {
+      const path = join(directory, item.name);
+      if (item.isDirectory()) walk(path);
+      else if (item.name.endsWith('.controller.ts')) controllers.push(path);
+    }
+  };
+  walk(SRC);
+
+  const found = new Map<string, string[]>();
+  for (const path of controllers) {
+    for (const name of entitiesReturnedBy(readFileSync(path, 'utf8'), entities)) {
+      const file = path.slice(SRC.length + 1);
+      const where = found.get(name) ?? [];
+      if (!where.includes(file)) where.push(file);
+      found.set(name, where);
+    }
+  }
+
+  return found;
+}
+
+/**
+ * The entities one controller file answers with, by their own names.
+ *
+ * Separate from the walk above so it can be exercised on source text rather than on whatever the
+ * repository happens to contain today — the alias case has no instance here yet, and a guard whose
+ * hardest branch never runs is the thing this suite exists to argue against.
+ */
+export function entitiesReturnedBy(source: string, entities: Set<string>): Set<string> {
+  // `import { SupportIssue as Issue }` — the handler then names `Issue`, which is not an entity
+  // name and would drop out of the scan. Mapped back, so renaming an import cannot quietly remove
+  // a controller from the closure.
+  const renamed = new Map<string, string>();
+  // The other direction of the same clause: a local name that happens to match an entity while
+  // standing for something else. Counting it would attribute relations to a response that never
+  // carries them.
+  const shadowed = new Set<string>();
+  for (const clause of source.matchAll(/import\s*\{([^}]*)\}/g))
+    for (const part of clause[1].split(',')) {
+      const [original, alias] = part.split(/\s+as\s+/).map((piece) => piece.trim());
+      if (!alias) continue;
+      if (entities.has(original)) renamed.set(alias, original);
+      else shadowed.add(alias);
+    }
+
+  const found = new Set<string>();
+  // The whole return type, then every entity name in it: `Promise<Issue | null>`, `Promise<Issue[]>`
+  // and any wrapper around them all have to count. Matching the first identifier after the colon
+  // reads more simply and silently misses the union forms — the expensive direction, because a
+  // handler it misses is one whose answer the closure below then fails to cover.
+  for (const match of source.matchAll(/\)\s*:\s*([^;{]+?)\s*\{/g))
+    for (const identifier of match[1].match(/[A-Za-z0-9_]+/g) ?? []) {
+      if (shadowed.has(identifier)) continue;
+      const name = renamed.get(identifier) ?? identifier;
+      if (entities.has(name)) found.add(name);
+    }
+
+  return found;
+}
+
+/**
+ * Every eager relation those responses contain, reached recursively.
+ *
+ * A relation on this list cannot be removed without changing an answer, and one added to any entity
+ * on it becomes part of an answer. That is the decision this list exists to force: when it fails,
+ * the question is not how to make the test pass but whether the endpoints above should carry the
+ * relation.
+ */
+const IN_A_PAYLOAD = [
+  'BankData.preferredCurrency',
+  'BankTxRepeat.transaction',
+  'BankTxReturn.transaction',
+  'BankTxReturn.userData',
+  'BuyCrypto.batch',
+  'BuyCrypto.fee',
+  'BuyCrypto.outputAsset',
+  'BuyCrypto.outputReferenceAsset',
+  'BuyCrypto.transaction',
+  'BuyCryptoBatch.outputAsset',
+  'BuyCryptoBatch.outputReferenceAsset',
+  'BuyCryptoFee.feeReferenceAsset',
+  'BuyFiat.outputAsset',
+  'BuyFiat.outputReferenceAsset',
+  'BuyFiat.transaction',
+  'CryptoInput.asset',
+  'CryptoInput.route',
+  // The deposit routes share one table, and the metadata of the parent carries the relations of
+  // every child. A query on the parent loads all of them, which is why they are on this list.
+  'DepositRoute.asset',
+  'DepositRoute.deposit',
+  'DepositRoute.fiat',
+  'DepositRoute.paybackAsset',
+  'DepositRoute.paybackDeposit',
+  'DepositRoute.rewardAsset',
+  'DepositRoute.rewardDeposit',
+  'DepositRoute.route',
+  'DepositRoute.targetDeposit',
+  'Fee.bank',
+  'Fee.wallet',
+  'FiatOutput.bank',
+  'LimitRequest.supportIssue',
+  'LiquidityBalance.asset',
+  'LiquidityManagementOrder.action',
+  'LiquidityManagementOrder.pipeline',
+  'LiquidityManagementPipeline.currentAction',
+  'LiquidityManagementPipeline.previousAction',
+  'LiquidityManagementPipeline.rule',
+  'LiquidityManagementRule.deficitStartAction',
+  'LiquidityManagementRule.redundancyStartAction',
+  'LiquidityManagementRule.targetAsset',
+  'LiquidityManagementRule.targetFiat',
+  'Organization.country',
+  'PaymentLinkPayment.currency',
+  'RefReward.outputAsset',
+  'RefReward.transaction',
+  'SupportIssue.transaction',
+  'SupportIssue.transactionRequest',
+  'SupportIssue.userData',
+  'SupportIssue.wallet',
+  'Transaction.user',
+  'User.refAsset',
+  'UserData.country',
+  'UserData.currency',
+  'UserData.language',
+  'UserData.nationality',
+  'UserData.organization',
+  'UserData.organizationCountry',
+  'UserData.verifiedCountry',
+];
+
+/**
+ * Eager relations in this repository, counted per entity — an inherited one counts once for each
+ * entity that carries it, because that is how often it is loaded.
+ */
+const EAGER_RELATIONS = 103;
+
+describeProjection('eager relations', () => {
+  let dataSource: DataSource;
+  let byName: Map<string, EntityMetadata>;
+
+  beforeAll(async () => {
+    dataSource = await createProjectionDataSource(SCHEMA);
+    byName = new Map(dataSource.entityMetadatas.map((metadata) => [metadata.name, metadata]));
+  }, 300000);
+
+  afterAll(async () => {
+    await destroyProjectionDataSource(dataSource, SCHEMA);
+  });
+
+  /**
+   * Every eager relation reachable from `roots`, with every root that reaches it.
+   *
+   * All of them, not the first: `UserData.wallet` is in the answer of the account endpoints and of
+   * everything that carries an account, and a message naming one of those sends the reader to the
+   * wrong controller.
+   */
+  function closureOf(roots: string[]): Map<string, Set<string>> {
+    const found = new Map<string, Set<string>>();
+
+    for (const root of roots) {
+      const visited = new Set<string>();
+      const pending = [root];
+
+      while (pending.length) {
+        const name = pending.pop();
+        if (visited.has(name)) continue;
+        visited.add(name);
+
+        for (const relation of byName.get(name)?.eagerRelations ?? []) {
+          const path = `${name}.${relation.propertyName}`;
+          if (!found.has(path)) found.set(path, new Set());
+          found.get(path).add(root);
+          pending.push(relation.inverseEntityMetadata.name);
+        }
+      }
+    }
+
+    return found;
+  }
+
+  it('reads an entity out of a return type, through a union, an array and a renamed import', () => {
+    const entities = new Set(['SupportIssue', 'UserData']);
+    const source = `
+      import { SupportIssue as Issue } from './support-issue.entity';
+      import { UserData } from './user-data.entity';
+      import { Something } from './elsewhere';
+
+      class C {
+        async one(id: number): Promise<Issue | null> { return null; }
+        async many(): Promise<UserData[]> { return []; }
+        async neither(): Promise<Something> { return null; }
+      }`;
+
+    // The renamed one is the case with no instance in this repository today, which is why it is
+    // asserted here rather than left to the walk over the real controllers.
+    expect(entitiesReturnedBy(source, entities)).toEqual(new Set(['SupportIssue', 'UserData']));
+  });
+
+  it('takes a name that is not an entity for nothing, renamed or not', () => {
+    const entities = new Set(['SupportIssue']);
+    const source = `
+      import { Helper as SupportIssue } from './helper';
+
+      class C {
+        async one(): Promise<SupportIssue> { return null; }
+      }`;
+
+    // `SupportIssue` here is a local name for something else entirely. Reading the alias map in the
+    // other direction would report the entity and put a relation in a closure it is not part of.
+    expect(entitiesReturnedBy(source, entities)).toEqual(new Set());
+  });
+
+  it('finds the controllers that answer with an entity', () => {
+    // If this reads zero, the search above stopped matching and every assertion below would pass
+    // for the wrong reason.
+    expect(entitiesReturnedWhole(new Set(byName.keys())).size).toBeGreaterThan(20);
+  });
+
+  it('the responses that are entities contain exactly the eager relations recorded here', () => {
+    const returned = entitiesReturnedWhole(new Set(byName.keys()));
+    const closure = closureOf([...returned.keys()]);
+
+    // Reported with the controllers, because that is the part a diff of relation names does not
+    // show: an added relation is a changed response, and this says whose.
+    const withOrigin = (paths: string[]): string[] =>
+      paths.map((path) => {
+        const controllers = [...closure.get(path)].flatMap((origin) => returned.get(origin) ?? []);
+        return `${path} — in the answer of ${[...new Set(controllers)].sort().join(', ')}`;
+      });
+
+    const added = [...closure.keys()].filter((path) => !IN_A_PAYLOAD.includes(path)).sort();
+    expect(withOrigin(added)).toEqual([]);
+
+    const removed = IN_A_PAYLOAD.filter((path) => !closure.has(path)).sort();
+    expect(removed).toEqual([]);
+  });
+
+  it('has no more eager relations than are recorded', () => {
+    const all = dataSource.entityMetadatas.flatMap((metadata) =>
+      metadata.eagerRelations.map((relation) => `${metadata.name}.${relation.propertyName}`),
+    );
+
+    // Not a limit, a count. Every one of these makes some query load a table it was not asked for,
+    // and the point of writing the number down is that adding one is a decision rather than a
+    // detail of an unrelated change.
+    expect(all.length).toEqual(EAGER_RELATIONS);
+  });
+});