mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 06:26:26 +00:00
fix(db): PK reads refresh past the identity map; typed owned-lookup generics; e2e ORM module mirrors production config
This commit is contained in:
@@ -11,13 +11,28 @@ export interface AppSettingsRow {
|
||||
const _appSettingsRowKeys: AssertRowKeys<AppSettingsRow, AppSettings> = true;
|
||||
|
||||
export class AppSettingsRepository extends EntityRepository<AppSettings> {
|
||||
/** `SELECT value FROM app_settings WHERE key = ?` */
|
||||
/**
|
||||
* `SELECT value FROM app_settings WHERE key = ?`
|
||||
*
|
||||
* `refresh: true` (Task 0 review, I1): this is a primary-key lookup, so
|
||||
* without it MikroORM answers a repeat call from the identity map rather
|
||||
* than re-querying — invisible to a raw/native write or `deleteValue` on
|
||||
* 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
|
||||
* query either way; only a stale identity-map hit is avoided.
|
||||
*/
|
||||
async getValue(key: string): Promise<string | null> {
|
||||
const row = await this.findOne({ key }, { fields: ['value'] });
|
||||
const row = await this.findOne({ key }, { fields: ['value'], refresh: true });
|
||||
return row?.value ?? null;
|
||||
}
|
||||
|
||||
/** `SELECT key, value FROM app_settings WHERE key IN (?, ?, ...)` */
|
||||
/**
|
||||
* `SELECT key, value FROM app_settings WHERE key IN (?, ?, ...)` — a
|
||||
* NULL-valued row is silently dropped from the returned Map, matching the
|
||||
* legacy callers, which never distinguished "row present with a NULL
|
||||
* value" from "no row" (both read as absent).
|
||||
*/
|
||||
async getValues(keys: string[]): Promise<Map<string, string>> {
|
||||
const rows = await this.find({ key: { $in: keys } });
|
||||
const values = new Map<string, string>();
|
||||
@@ -38,6 +53,15 @@ export class AppSettingsRepository extends EntityRepository<AppSettings> {
|
||||
* with no other column whose reset by REPLACE could be observable, both
|
||||
* spellings are exactly `em.upsert`'s job, with the sole unique field
|
||||
* (`key`, the primary key) as the conflict target.
|
||||
*
|
||||
* The one difference from `INSERT OR REPLACE` — verified, unobservable
|
||||
* here — is the row's implicit SQLite rowid: REPLACE deletes and
|
||||
* reinserts (rowid moves), `em.upsert`'s `ON CONFLICT ... DO UPDATE`
|
||||
* keeps the original row (rowid stays). `app_settings` has no `INTEGER
|
||||
* PRIMARY KEY`, no `AUTOINCREMENT`, no trigger and no foreign key
|
||||
* referencing it, so nothing here reads the rowid — but the next table
|
||||
* converted off `INSERT OR REPLACE` that does have one of those needs its
|
||||
* own check, not an assumption from this comment.
|
||||
*/
|
||||
async setValue(key: string, value: string): Promise<void> {
|
||||
await this.upsert({ key, value }, { onConflictFields: ['key'], onConflictAction: 'merge' });
|
||||
|
||||
@@ -9,14 +9,22 @@ import { EntityRepository } from '@mikro-orm/sql';
|
||||
* name outside this union fails `tsc` rather than reaching a query.
|
||||
*
|
||||
* This repository is the union's source of truth; `instance-api-keys.ts`
|
||||
* imports it rather than keeping its own copy (Task 5 wires that import).
|
||||
* will import it rather than keeping its own copy (Task 5 wires that import
|
||||
* — it still declares the identical union itself today).
|
||||
*/
|
||||
export type InstanceApiKeyName = 'maps_api_key' | 'unsplash_api_key' | 'amap_api_key';
|
||||
|
||||
export class UsersRepository extends EntityRepository<Users> {
|
||||
/** `SELECT email FROM users WHERE id = ?` */
|
||||
/**
|
||||
* `SELECT email FROM users WHERE id = ?`
|
||||
*
|
||||
* `refresh: true` (Task 0 review, I1): a primary-key `findOne` is answered
|
||||
* 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.
|
||||
*/
|
||||
async getEmail(userId: number): Promise<string | null> {
|
||||
const row = await this.findOne({ id: userId }, { fields: ['email'] });
|
||||
const row = await this.findOne({ id: userId }, { fields: ['email'], refresh: true });
|
||||
return row?.email ?? null;
|
||||
}
|
||||
|
||||
@@ -25,9 +33,11 @@ export class UsersRepository extends EntityRepository<Users> {
|
||||
* `SELECT <name> FROM users WHERE id = ?`. `name` is the typed union above,
|
||||
* not an interpolated column — MikroORM's `fields` option only accepts a
|
||||
* real property of `Users`, so this can never select an arbitrary column.
|
||||
*
|
||||
* `refresh: true` for the same identity-map reason as `getEmail` above.
|
||||
*/
|
||||
async getApiKeyColumn(userId: number, name: InstanceApiKeyName): Promise<string | null> {
|
||||
const row = await this.findOne({ id: userId }, { fields: [name] });
|
||||
const row = await this.findOne({ id: userId }, { fields: [name], refresh: true });
|
||||
return row ? (row[name] ?? null) : null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { EntityRepository, FilterQuery, OrderDefinition } from '@mikro-orm/core';
|
||||
import type { EntityRepository, FilterQuery, FilterValue, OrderDefinition } from '@mikro-orm/core';
|
||||
|
||||
/**
|
||||
* Shared lookup shapes for entities scoped to a user, some of which also
|
||||
@@ -15,23 +15,35 @@ import type { EntityRepository, FilterQuery, OrderDefinition } from '@mikro-orm/
|
||||
* = ?`), so it stays a plain `findOne({ id })` rather than either helper
|
||||
* here; changing that would be a behaviour change, not a refactor.
|
||||
*
|
||||
* `ownerField`/`orderField` are taken as parameters (`keyof T`) rather than
|
||||
* hard-coded so one helper serves every entity shaped this way; the `as
|
||||
* FilterQuery<T>`/`as Record<string, 'asc'>` casts are the one place that
|
||||
* type is asserted rather than inferred; the entity's own properties still
|
||||
* decide, at the call site, which literal `ownerField`/`orderField` values
|
||||
* are legal (`keyof T`).
|
||||
* These helpers return live MikroORM entities, not rows — unlike every other
|
||||
* repository method in this codebase (D4: "repositories return rows, never a
|
||||
* live entity"). They are an internal building block for `CategoriesRepository`
|
||||
* /`TagsRepository`, which stay the actual public API and must run the result
|
||||
* through `toRow` before it leaves the repository.
|
||||
*
|
||||
* 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<T[K]>` — the same value shape MikroORM's own `FilterObject`
|
||||
* accepts for that property (`T[K]` directly is too strict for a relation
|
||||
* property: `Tags['user']` is `Ref<Users>`, but every call site here filters
|
||||
* it with the raw foreign-key id, which `FilterValue` allows and a bare
|
||||
* `T[K]` would not). `id`/`ownerId` are no longer `unknown` — a call for an
|
||||
* entity without an `id` column, or with an `ownerId` of the wrong shape, now
|
||||
* fails `tsc` instead of only failing at runtime; the `as FilterQuery<T>`/
|
||||
* `as OrderDefinition<T>` casts are the one place the shape is still
|
||||
* asserted rather than inferred, exactly at the computed-property-key
|
||||
* boundary TypeScript cannot check on its own.
|
||||
*/
|
||||
|
||||
/**
|
||||
* List every row owned by `ownerId`, ordered by `orderField` ascending:
|
||||
* `SELECT * FROM <table> WHERE <ownerField> = ? ORDER BY <orderField> ASC`.
|
||||
*/
|
||||
export async function listForOwner<T extends object>(
|
||||
export async function listForOwner<T extends { id: unknown }, K extends keyof T, OF extends keyof T>(
|
||||
repo: EntityRepository<T>,
|
||||
ownerField: keyof T,
|
||||
ownerId: unknown,
|
||||
orderField: keyof T,
|
||||
ownerField: K,
|
||||
ownerId: FilterValue<T[K]>,
|
||||
orderField: OF,
|
||||
): Promise<T[]> {
|
||||
return repo.find({ [ownerField]: ownerId } as FilterQuery<T>, {
|
||||
orderBy: { [orderField]: 'asc' } as OrderDefinition<T>,
|
||||
@@ -42,11 +54,11 @@ export async function listForOwner<T extends object>(
|
||||
* Find one row by id, strictly scoped to an owner:
|
||||
* `SELECT * FROM <table> WHERE id = ? AND <ownerField> = ?`.
|
||||
*/
|
||||
export async function findOwnedByUser<T extends object>(
|
||||
export async function findOwnedByUser<T extends { id: unknown }, K extends keyof T>(
|
||||
repo: EntityRepository<T>,
|
||||
id: unknown,
|
||||
ownerField: keyof T,
|
||||
ownerId: unknown,
|
||||
id: T['id'],
|
||||
ownerField: K,
|
||||
ownerId: FilterValue<T[K]>,
|
||||
): Promise<T | null> {
|
||||
return repo.findOne({ id, [ownerField]: ownerId } as FilterQuery<T>);
|
||||
}
|
||||
@@ -57,11 +69,11 @@ export async function findOwnedByUser<T extends object>(
|
||||
* `SELECT * FROM <table> WHERE id = ? AND (<ownerField> = ? OR <ownerField>
|
||||
* IS NULL)`.
|
||||
*/
|
||||
export async function findOwnedOrGlobal<T extends object>(
|
||||
export async function findOwnedOrGlobal<T extends { id: unknown }, K extends keyof T>(
|
||||
repo: EntityRepository<T>,
|
||||
id: unknown,
|
||||
ownerField: keyof T,
|
||||
ownerId: unknown,
|
||||
id: T['id'],
|
||||
ownerField: K,
|
||||
ownerId: FilterValue<T[K]>,
|
||||
): Promise<T | null> {
|
||||
return repo.findOne({
|
||||
id,
|
||||
|
||||
@@ -169,7 +169,13 @@ export class SettingsService {
|
||||
if (ENCRYPTED_SETTING_KEYS.has(settingKey)) {
|
||||
defaults[settingKey] = row.value ? (decrypt_api_key(row.value) ?? '') : '';
|
||||
} else {
|
||||
defaults[settingKey] = parseValue(row.value ?? '');
|
||||
// Parity with the legacy `parseValue(row.value)` call this replaced:
|
||||
// `JSON.parse(null)` coerces its argument to the string "null" and
|
||||
// returns the JS value `null`, so a NULL-valued row parsed to `null`.
|
||||
// `row.value ?? ''` would instead call `parseValue('')`, which throws
|
||||
// inside JSON.parse and falls back to the empty string — a silent
|
||||
// divergence for a case parseValue never actually threw on before.
|
||||
defaults[settingKey] = row.value === null ? null : parseValue(row.value);
|
||||
}
|
||||
}
|
||||
return defaults;
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { DynamicModule } from '@nestjs/common';
|
||||
import type Database from 'better-sqlite3';
|
||||
import { ALL_ENTITIES } from '../../src/db/entities';
|
||||
import { createBoundSqliteDriver } from '../../src/db/orm-driver';
|
||||
import mikroOrmConfig from '../../src/mikro-orm.config';
|
||||
|
||||
export interface TestOrm {
|
||||
orm: MikroORM;
|
||||
@@ -83,9 +84,30 @@ export async function createTestOrm(
|
||||
* `createNestApplication()`, so `registerRequestContext` (the NestJS
|
||||
* integration's default) forks a context-resolving EntityManager per request
|
||||
* the same way `buildApp()` does — no test-only global context needed here.
|
||||
*
|
||||
* Built from `src/mikro-orm.config.ts` (spread, then overridden only on
|
||||
* `driver`/`dbName`/`discovery`) rather than a hand-rolled options object, so
|
||||
* this stays in lockstep with production instead of being a second copy of
|
||||
* the config that can silently drift.
|
||||
*
|
||||
* UPLOADS-P16 (Phase 1 ledger, entries 102–103): `registerRequestContext` is
|
||||
* left at its NestJS-integration default here — the `@mikro-orm/nestjs` auto
|
||||
* middleware that provides it is *also* the confirmed root cause of
|
||||
* UPLOADS-P16 (its Nest-11 wildcard route throws on a malformed `%`-encoded
|
||||
* upload path before any TREK handler runs). That coupling is faithful to
|
||||
* production as production is broken right now, not a weakened test — but
|
||||
* the recommended fix for UPLOADS-P16 (`registerRequestContext: false` on
|
||||
* `MikroOrmModule.forRoot`, paired with our own keyless `RequestContext.create`
|
||||
* 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
|
||||
* `allowGlobalContext`-style re-justification `createTestOrm` above got.
|
||||
*/
|
||||
export function createTestMikroOrmModule(db: Database.Database): DynamicModule | Promise<DynamicModule> {
|
||||
return MikroOrmModule.forRoot({
|
||||
...mikroOrmConfig,
|
||||
entities: [...ALL_ENTITIES],
|
||||
driver: createBoundSqliteDriver(() => db),
|
||||
dbName: ':memory:',
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
@@ -24,6 +24,24 @@ function insertRaw(key: string, value: string | null): void {
|
||||
testDb.prepare('INSERT INTO app_settings (key, value) VALUES (?, ?)').run(key, value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs `fn` while counting queries MikroORM actually issues over the shared
|
||||
* connection, so a regression test can assert "one query, with this value"
|
||||
* rather than only the value — the identity-map bug I1 fixed (Task 0 review)
|
||||
* returned the *right* value from a stale cache with *zero* queries, which a
|
||||
* value-only assertion would never catch.
|
||||
*/
|
||||
async function withQueryCount<T>(fn: () => Promise<T>): Promise<{ value: T; queries: number }> {
|
||||
const connection = t.orm.em.getConnection();
|
||||
const spy = vi.spyOn(connection, 'execute');
|
||||
try {
|
||||
const value = await fn();
|
||||
return { value, queries: spy.mock.calls.length };
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
}
|
||||
|
||||
describe('AppSettingsRepository', () => {
|
||||
it('APPSETREPO-001: getValue reads the stored value', async () => {
|
||||
insertRaw('bag_tracking_enabled', 'true');
|
||||
@@ -41,6 +59,39 @@ describe('AppSettingsRepository', () => {
|
||||
expect(values).toEqual(new Map([['collab_chat_enabled', 'false'], ['collab_notes_enabled', 'true']]));
|
||||
});
|
||||
|
||||
it('APPSETREPO-012 (M1): getValues also drops a key whose row exists but whose value is NULL, matching the "row present with a NULL value" case as absent, same as a missing row', async () => {
|
||||
insertRaw('collab_chat_enabled', 'false');
|
||||
insertRaw('collab_links_enabled', null); // present row, NULL value — legacy treated this as absent too
|
||||
const values = await appSettings.getValues(['collab_chat_enabled', 'collab_links_enabled', 'collab_notes_enabled']);
|
||||
expect(values).toEqual(new Map([['collab_chat_enabled', 'false']]));
|
||||
expect(values.has('collab_links_enabled')).toBe(false);
|
||||
});
|
||||
|
||||
// I1 (Task 0 review): getValue is a primary-key findOne, which MikroORM
|
||||
// answers from the identity map on a repeat call unless `refresh: true` is
|
||||
// set — invisible to a raw write or delete on the same key inside the same
|
||||
// request. These reproduce the review's two failing probes and pin the
|
||||
// fix at one query each.
|
||||
describe('getValue sees a raw write/delete on the same key in the same request (I1, identity-map regression)', () => {
|
||||
it('APPSETREPO-013: deleteValue then getValue reads null, not the deleted row, in one query', async () => {
|
||||
insertRaw('bag_tracking_enabled', 'v');
|
||||
expect(await appSettings.getValue('bag_tracking_enabled')).toBe('v'); // populate the identity map
|
||||
await appSettings.deleteValue('bag_tracking_enabled');
|
||||
const { value, queries } = await withQueryCount(() => appSettings.getValue('bag_tracking_enabled'));
|
||||
expect(value).toBeNull();
|
||||
expect(queries).toBe(1);
|
||||
});
|
||||
|
||||
it('APPSETREPO-014: a raw UPDATE on the same key then getValue reads the new value, in one query', async () => {
|
||||
insertRaw('bag_tracking_enabled', 'old');
|
||||
expect(await appSettings.getValue('bag_tracking_enabled')).toBe('old'); // populate the identity map
|
||||
testDb.prepare('UPDATE app_settings SET value = ? WHERE key = ?').run('new', 'bag_tracking_enabled');
|
||||
const { value, queries } = await withQueryCount(() => appSettings.getValue('bag_tracking_enabled'));
|
||||
expect(value).toBe('new');
|
||||
expect(queries).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
it('APPSETREPO-004: setValue inserts a new row exactly as INSERT OR REPLACE would', async () => {
|
||||
await appSettings.setValue('bag_tracking_enabled', 'true');
|
||||
expect(rawRow('bag_tracking_enabled')).toStrictEqual({ key: 'bag_tracking_enabled', value: 'true' });
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
@@ -17,6 +17,22 @@ beforeAll(async () => {
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
/**
|
||||
* Runs `fn` while counting queries MikroORM actually issues over the shared
|
||||
* connection — see `AppSettings.repository.test.ts` for why a value-only
|
||||
* assertion can't catch the identity-map bug I1 fixed (Task 0 review).
|
||||
*/
|
||||
async function withQueryCount<T>(fn: () => Promise<T>): Promise<{ value: T; queries: number }> {
|
||||
const connection = t.orm.em.getConnection();
|
||||
const spy = vi.spyOn(connection, 'execute');
|
||||
try {
|
||||
const value = await fn();
|
||||
return { value, queries: spy.mock.calls.length };
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
}
|
||||
|
||||
describe('UsersRepository', () => {
|
||||
it('USERSREPO-001: getEmail reads the stored email, matching SELECT email FROM users WHERE id = ?', async () => {
|
||||
const { user } = createUser(testDb, { email: 'someone@example.com' });
|
||||
@@ -53,4 +69,29 @@ describe('UsersRepository', () => {
|
||||
expect(await users.getApiKeyColumn(user.id, 'maps_api_key')).toBeNull();
|
||||
expect(await users.getApiKeyColumn(999999, 'maps_api_key')).toBeNull();
|
||||
});
|
||||
|
||||
// I1 (Task 0 review): getEmail/getApiKeyColumn are primary-key findOne
|
||||
// calls, which MikroORM answers from the identity map on a repeat call
|
||||
// unless `refresh: true` is set — invisible to a raw UPDATE on the same
|
||||
// user id inside the same request. Pins the fix at one query each.
|
||||
describe('getEmail/getApiKeyColumn see a raw UPDATE on the same user in the same request (I1, identity-map regression)', () => {
|
||||
it('USERSREPO-007: a raw UPDATE users SET email then getEmail reads the new email, in one query', async () => {
|
||||
const { user } = createUser(testDb, { email: 'old@example.com' });
|
||||
expect(await users.getEmail(user.id)).toBe('old@example.com'); // populate the identity map
|
||||
testDb.prepare('UPDATE users SET email = ? WHERE id = ?').run('new@example.com', user.id);
|
||||
const { value, queries } = await withQueryCount(() => users.getEmail(user.id));
|
||||
expect(value).toBe('new@example.com');
|
||||
expect(queries).toBe(1);
|
||||
});
|
||||
|
||||
it('USERSREPO-008: a raw UPDATE users SET maps_api_key then getApiKeyColumn reads the new value, in one query', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET maps_api_key = ? WHERE id = ?').run('old-key', user.id);
|
||||
expect(await users.getApiKeyColumn(user.id, 'maps_api_key')).toBe('old-key'); // populate the identity map
|
||||
testDb.prepare('UPDATE users SET maps_api_key = ? WHERE id = ?').run('new-key', user.id);
|
||||
const { value, queries } = await withQueryCount(() => users.getApiKeyColumn(user.id, 'maps_api_key'));
|
||||
expect(value).toBe('new-key');
|
||||
expect(queries).toBe(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -30,14 +30,15 @@ describe('listForOwner', () => {
|
||||
createTag(testDb, owner.id, { name: 'Apple' });
|
||||
createTag(testDb, other.id, { name: 'Middle' });
|
||||
|
||||
const rows = await listForOwner<Tags>(tags, 'user', owner.id, 'name');
|
||||
// T/K/OF all inferred: T from `tags`, K from the 'user' literal, OF from 'name'.
|
||||
const rows = await listForOwner<Tags, 'user', 'name'>(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);
|
||||
});
|
||||
|
||||
it('OWNEDLOOKUP-002: an owner with no rows gets an empty list', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
expect(await listForOwner<Tags>(tags, 'user', owner.id, 'name')).toEqual([]);
|
||||
expect(await listForOwner<Tags, 'user', 'name'>(tags, 'user', owner.id, 'name')).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -45,7 +46,7 @@ describe('findOwnedByUser', () => {
|
||||
it('OWNEDLOOKUP-003: finds a row the given user owns', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const tag = createTag(testDb, owner.id, { name: 'Mine' });
|
||||
const found = await findOwnedByUser<Tags>(tags, tag.id, 'user', owner.id);
|
||||
const found = await findOwnedByUser<Tags, 'user'>(tags, tag.id, 'user', owner.id);
|
||||
expect(found?.id).toBe(tag.id);
|
||||
expect(found?.name).toBe('Mine');
|
||||
});
|
||||
@@ -54,12 +55,12 @@ describe('findOwnedByUser', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: stranger } = createUser(testDb);
|
||||
const tag = createTag(testDb, owner.id, { name: 'Mine' });
|
||||
expect(await findOwnedByUser<Tags>(tags, tag.id, 'user', stranger.id)).toBeNull();
|
||||
expect(await findOwnedByUser<Tags, 'user'>(tags, tag.id, 'user', stranger.id)).toBeNull();
|
||||
});
|
||||
|
||||
it('OWNEDLOOKUP-005: returns null for an id that does not exist', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
expect(await findOwnedByUser<Tags>(tags, 999999, 'user', owner.id)).toBeNull();
|
||||
expect(await findOwnedByUser<Tags, 'user'>(tags, 999999, 'user', owner.id)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -67,14 +68,14 @@ describe('findOwnedOrGlobal', () => {
|
||||
it('OWNEDLOOKUP-006: finds a row the given user owns — Categories (a permissively-owned entity, user_id nullable)', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const category = createCategory(testDb, { name: 'Mine', user_id: owner.id });
|
||||
const found = await findOwnedOrGlobal<Categories>(categories, category.id, 'user', owner.id);
|
||||
const found = await findOwnedOrGlobal<Categories, 'user'>(categories, category.id, 'user', owner.id);
|
||||
expect(found?.id).toBe(category.id);
|
||||
});
|
||||
|
||||
it('OWNEDLOOKUP-007: finds a row with no owner at all (a globally-shared row)', async () => {
|
||||
const { user: someone } = createUser(testDb);
|
||||
const global = createCategory(testDb, { name: 'Global', user_id: null });
|
||||
const found = await findOwnedOrGlobal<Categories>(categories, global.id, 'user', someone.id);
|
||||
const found = await findOwnedOrGlobal<Categories, 'user'>(categories, global.id, 'user', someone.id);
|
||||
expect(found?.id).toBe(global.id);
|
||||
});
|
||||
|
||||
@@ -82,11 +83,53 @@ describe('findOwnedOrGlobal', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: stranger } = createUser(testDb);
|
||||
const category = createCategory(testDb, { name: 'Theirs', user_id: owner.id });
|
||||
expect(await findOwnedOrGlobal<Categories>(categories, category.id, 'user', stranger.id)).toBeNull();
|
||||
expect(await findOwnedOrGlobal<Categories, 'user'>(categories, category.id, 'user', stranger.id)).toBeNull();
|
||||
});
|
||||
|
||||
it('OWNEDLOOKUP-009: returns null for an id that does not exist', async () => {
|
||||
const { user: someone } = createUser(testDb);
|
||||
expect(await findOwnedOrGlobal<Categories>(categories, 999999, 'user', someone.id)).toBeNull();
|
||||
expect(await findOwnedOrGlobal<Categories, 'user'>(categories, 999999, 'user', someone.id)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Type-only probes (Task 0 review, I2): each of these would have compiled
|
||||
* clean under the old `id: unknown` / `ownerId: unknown` / `T extends
|
||||
* object` signatures — that was the finding. They must fail `tsc` now, and
|
||||
* `typecheck:tests` (which runs over this file) is the gate that proves it;
|
||||
* nothing here executes at runtime. `@ts-expect-error` itself fails the
|
||||
* build if the line stops erroring (a silent regression back to `unknown`),
|
||||
* so this function doubles as the regression test.
|
||||
*/
|
||||
function typeProbes(): void {
|
||||
// AppSettings has no `id` column — `T extends { id: unknown }` rejects it
|
||||
// as a type argument before `ownerField`/`ownerId` are even considered.
|
||||
// @ts-expect-error AppSettings has no `id` property
|
||||
void findOwnedByUser<AppSettings, 'value'>(appSettingsRepoForProbe, 1, 'value', 'whatever');
|
||||
|
||||
// `id` must be a Tags id (number), not an arbitrary string.
|
||||
// @ts-expect-error id must be Tags['id'] (number), not a string
|
||||
void findOwnedByUser<Tags, 'user'>(tags, 'not-a-number', 'user', 1);
|
||||
|
||||
// `ownerId` must be a value `FilterValue<Tags['user']>` accepts (the FK id
|
||||
// or the related entity) — an unrelated object literal is not one.
|
||||
// @ts-expect-error ownerId is not a value `Tags['user']` can be filtered by
|
||||
void findOwnedByUser<Tags, 'user'>(tags, 1, 'user', { nonsense: true });
|
||||
|
||||
// Same two shapes for listForOwner's ownerField/ownerId pair.
|
||||
// @ts-expect-error ownerId must be Tags['user']-filterable, not a string
|
||||
void listForOwner<Tags, 'user', 'name'>(tags, 'user', 'a string owner id', 'name');
|
||||
|
||||
// orderField must be a real property of Tags.
|
||||
// @ts-expect-error 'not_a_column' is not a key of Tags
|
||||
void listForOwner<Tags, 'user', 'not_a_column'>(tags, 'user', 1, 'not_a_column');
|
||||
|
||||
// Same two shapes for findOwnedOrGlobal.
|
||||
// @ts-expect-error AppSettings has no `id` property
|
||||
void findOwnedOrGlobal<AppSettings, 'value'>(appSettingsRepoForProbe, 1, 'value', 'whatever');
|
||||
// @ts-expect-error ownerId is not a value `Categories['user']` can be filtered by
|
||||
void findOwnedOrGlobal<Categories, 'user'>(categories, 1, 'user', { nonsense: true });
|
||||
}
|
||||
void typeProbes;
|
||||
declare const appSettingsRepoForProbe: import('../../../../../src/db/repositories/AppSettings.repository').AppSettingsRepository;
|
||||
type AppSettings = import('../../../../../src/db/entities/AppSettings.entity').AppSettings;
|
||||
|
||||
Reference in New Issue
Block a user