From b0905f98679919096ee24001b03b225013003b2e Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 22 Sep 2026 12:53:47 +0200 Subject: [PATCH] docs(db): owned-lookup call-site convention; refresh side effect; e2e ORM module spread notes --- server/src/db/repositories/AppSettings.repository.ts | 3 +++ server/src/db/repositories/Users.repository.ts | 3 +++ server/src/db/repositories/_shared/owned-lookup.ts | 12 ++++++++++++ server/tests/helpers/test-orm.ts | 7 +++++-- .../db/repositories/_shared/owned-lookup.test.ts | 3 ++- 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/server/src/db/repositories/AppSettings.repository.ts b/server/src/db/repositories/AppSettings.repository.ts index ed3626e21..ce7cfa2b6 100644 --- a/server/src/db/repositories/AppSettings.repository.ts +++ b/server/src/db/repositories/AppSettings.repository.ts @@ -20,6 +20,9 @@ export class AppSettingsRepository extends EntityRepository { * the same key inside the same request, which is exactly the coexistence * case this plan is built on (raw SQL and the ORM sharing one connection * while callers convert one at a time). `refresh` keeps this a single + * Side effect of `refresh`: an UNFLUSHED in-memory change to the selected + * field on that entity is discarded (the entity reverts to the row). No + * caller mutates these entities before reading, by design (D4: rows out). * query either way; only a stale identity-map hit is avoided. */ async getValue(key: string): Promise { diff --git a/server/src/db/repositories/Users.repository.ts b/server/src/db/repositories/Users.repository.ts index 5e12dc95e..51b0f7c90 100644 --- a/server/src/db/repositories/Users.repository.ts +++ b/server/src/db/repositories/Users.repository.ts @@ -22,6 +22,9 @@ export class UsersRepository extends EntityRepository { * from the identity map on a repeat call, which would hide a raw/native * `UPDATE users SET email = ...` on the same id inside the same request. * `refresh` keeps this a single query and always sees the current row. + * Side effect of `refresh`: an UNFLUSHED in-memory change to the selected + * field on that entity is discarded (the entity reverts to the row). No + * caller mutates these entities before reading, by design (D4: rows out). */ async getEmail(userId: number): Promise { const row = await this.findOne({ id: userId }, { fields: ['email'], refresh: true }); diff --git a/server/src/db/repositories/_shared/owned-lookup.ts b/server/src/db/repositories/_shared/owned-lookup.ts index 4621854d9..d0d7a70de 100644 --- a/server/src/db/repositories/_shared/owned-lookup.ts +++ b/server/src/db/repositories/_shared/owned-lookup.ts @@ -21,6 +21,18 @@ import type { EntityRepository, FilterQuery, FilterValue, OrderDefinition } from * /`TagsRepository`, which stay the actual public API and must run the result * through `toRow` before it leaves the repository. * + * Call-site convention (Task 0 re-review): type arguments are ALWAYS explicit — + * inference from the repository argument alone resolves `T` to the + * `{ id: unknown }` constraint and fails to compile (TS2345) — and the owner + * field is the RELATION property (`'user'`), never the `persist(false)` twin + * (`'user_id'`): both compile and emit the same SQL, but only the relation + * name survives a column rename and reads as the entity's API. `ownerId` is the + * raw FK number. Canonical calls: + * + * listForOwner(tags, 'user', userId, 'name'); + * findOwnedByUser(tags, id, 'user', userId); + * findOwnedOrGlobal(categories, id, 'user', userId); + * * Typing (Task 0 review, I2): `T extends { id: unknown }` and `K extends * keyof T` tie `ownerField` to a real property of `T` and `ownerId` to * `FilterValue` — the same value shape MikroORM's own `FilterObject` diff --git a/server/tests/helpers/test-orm.ts b/server/tests/helpers/test-orm.ts index ffd433cba..bd0cd954d 100644 --- a/server/tests/helpers/test-orm.ts +++ b/server/tests/helpers/test-orm.ts @@ -101,10 +101,13 @@ export async function createTestOrm( * middleware, since D6 needs the per-request EM fork this default provides) * changes what `buildApp()` registers. Building this helper from the same * `mikroOrmConfig` object production uses means that fix reaches both at - * once; if it is ever done as a hand-edit instead, this helper's 15 e2e - * harnesses would silently stop mirroring production and need the same + * once; if it is ever done as a hand-edit instead, the e2e harnesses built + * on this helper would silently stop mirroring production and need the same * `allowGlobalContext`-style re-justification `createTestOrm` above got. */ +// The spread also carries production's `extensions` (Migrator, SeedManager), +// `migrations` and `seeder` settings into the harness; only the driver, the +// database name, the entity list and discovery noise are overridden. export function createTestMikroOrmModule(db: Database.Database): DynamicModule | Promise { return MikroOrmModule.forRoot({ ...mikroOrmConfig, diff --git a/server/tests/unit/db/repositories/_shared/owned-lookup.test.ts b/server/tests/unit/db/repositories/_shared/owned-lookup.test.ts index fcbe870e0..ed8ce9055 100644 --- a/server/tests/unit/db/repositories/_shared/owned-lookup.test.ts +++ b/server/tests/unit/db/repositories/_shared/owned-lookup.test.ts @@ -30,7 +30,8 @@ describe('listForOwner', () => { createTag(testDb, owner.id, { name: 'Apple' }); createTag(testDb, other.id, { name: 'Middle' }); - // T/K/OF all inferred: T from `tags`, K from the 'user' literal, OF from 'name'. + // Type arguments are always explicit: inference from the `tags` argument alone + // resolves T to the `{ id: unknown }` constraint (TS2345), never to the entity. const rows = await listForOwner(tags, 'user', owner.id, 'name'); expect(rows.map((r) => r.name)).toEqual(['Apple', 'Zebra']); expect(rows.every((r) => r.user_id === owner.id)).toBe(true);