mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 13:36:48 +00:00
fix(db): no NaN class-field initialisers; entity generator --check gate in CI; parity normaliser keeps literal case
This commit is contained in:
@@ -180,9 +180,16 @@ jobs:
|
||||
# catches it.
|
||||
run: cd server && npm run typecheck:tests
|
||||
|
||||
- name: Typecheck scripts
|
||||
- name: Typecheck entity generator
|
||||
# tsconfig.scripts.json only covers scripts/generate-entities.ts —
|
||||
# gen-plugin-facts.ts/migrate-encryption.ts don't compile under this
|
||||
# config (import.meta needs a newer --module target), so the gate is
|
||||
# named for what it actually covers rather than the whole directory.
|
||||
run: cd server && npm run typecheck:scripts
|
||||
|
||||
- name: Entity files match the generator
|
||||
run: cd server && npm run check:entities
|
||||
|
||||
- name: Lint
|
||||
run: cd server && npm run lint:check
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
"gen:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts",
|
||||
"check:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts --check",
|
||||
"gen:entities": "node --import tsx scripts/generate-entities.ts",
|
||||
"check:entities": "node --import tsx scripts/generate-entities.ts --check",
|
||||
"format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
|
||||
"format:check": "prettier --check \"src/**/*.ts\" \"test/**/*.ts\"",
|
||||
"lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix",
|
||||
|
||||
@@ -321,6 +321,58 @@ export function RULE1c_markBooleanColumns(metadata: EntityMetadata[]): RetypedSc
|
||||
return fixups;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rule 1d (metadata level): a numeric scalar's class-field initialiser must
|
||||
* never render `NaN`.
|
||||
*
|
||||
* `@mikro-orm/sql`'s own introspection (`DatabaseTable.js#getPropertyDeclaration`
|
||||
* -> `getPropertyDefaultValue`) computes a NUMERIC column's `prop.default` as
|
||||
* `+defaultValue` — a plain unary-plus coercion of the raw SQLite default
|
||||
* text — whenever that text doesn't equal the literal, lowercase string
|
||||
* `'null'`. SQLite reports the DDL's exact casing back through
|
||||
* `PRAGMA table_info` (`'NULL'`, not `'null'`, for `DEFAULT NULL`), and
|
||||
* `SchemaHelper#normalizeDefaultValue`'s own `'null'` check
|
||||
* (`DatabaseTable.js` line ~869) is the same case-sensitive comparison — so
|
||||
* neither the `DEFAULT NULL` case NOR a non-literal expression default
|
||||
* (`DEFAULT (strftime('%s','now'))`) is ever recognised as "no usable
|
||||
* numeric value", and `+'NULL'` / `+"(strftime('%s','now'))"` both evaluate
|
||||
* to `NaN`. That `NaN` becomes `prop.default`, which
|
||||
* `SourceFile.js#getPropertyDefinition` then treats as a genuine, usable
|
||||
* default (`NaN` is neither `undefined` nor `null`) and renders `= NaN` as
|
||||
* the class-field initialiser — confirmed by generating the real schema
|
||||
* (`BudgetItems.persons`/`.days`, `IdempotencyKeys.created_at`,
|
||||
* `BudgetItems.place_id`/`.reservation_id`, `PackingBags.user_id`) and by
|
||||
* reading `node_modules/@mikro-orm/sql/schema/DatabaseTable.js`, not
|
||||
* assumed. `prop.defaultRaw` itself is unaffected — it is threaded straight
|
||||
* through to `.defaultRaw(...)` in the rendered schema and is correct there;
|
||||
* only the class-field initialiser is wrong, and it is wrong specifically
|
||||
* because the upstream library's `NaN` looks like "a real number" to the
|
||||
* renderer's own `useDefault` check.
|
||||
*
|
||||
* Clearing `prop.default` back to `undefined` (never `null` — `null` would
|
||||
* make the renderer's `hasUsableNullDefault` branch fire and print `= null`,
|
||||
* which is still an initialiser this rule must not emit) makes the
|
||||
* renderer's `useDefault` check naturally false, so NO initialiser is
|
||||
* emitted at all: the class field falls back to nullability alone —
|
||||
* `?: number | null` for a nullable column, `!: number` for a NOT NULL one
|
||||
* (matching every other scalar's fallback shape in this generator).
|
||||
*
|
||||
* A `defaultRaw` that IS a finite numeric literal (`'0'`, `'1'`) is left
|
||||
* completely alone: `Number(raw)` is correct there, `prop.default` already
|
||||
* holds the right value, and nothing needs fixing.
|
||||
*/
|
||||
export function RULE1d_fixNaNNumericDefaults(metadata: EntityMetadata[]): void {
|
||||
for (const meta of metadata) {
|
||||
for (const prop of Object.values(meta.properties)) {
|
||||
if (prop.kind !== undefined && prop.kind !== ReferenceKind.SCALAR) continue;
|
||||
if (typeof prop.defaultRaw !== 'string') continue;
|
||||
if (Number.isFinite(Number(prop.defaultRaw))) continue; // a genuine numeric literal — untouched
|
||||
if (typeof prop.default !== 'number' || !Number.isNaN(prop.default)) continue; // nothing broken here
|
||||
prop.default = undefined;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Rule 2 (metadata level): p.datetime() -> p.type(DbTimestampType).
|
||||
//
|
||||
@@ -691,6 +743,7 @@ export function applyRules(metadata: EntityMetadata[], _platform: Platform): Rul
|
||||
// `retypedScalars` (stripRedundantColumnType's "found no .columnType() to
|
||||
// strip" throw catches this immediately if that ever stops being true).
|
||||
RULE1c_markBooleanColumns(metadata);
|
||||
RULE1d_fixNaNNumericDefaults(metadata);
|
||||
const timestamps = RULE2_datetimeToDbTimestampType(metadata);
|
||||
RULE3_integerPkAutoincrement(metadata);
|
||||
RULE4_hideAllRelations(metadata);
|
||||
@@ -1258,13 +1311,8 @@ export function writeRepositoriesIfMissing(repositoriesDir: string, files: Reado
|
||||
return created;
|
||||
}
|
||||
|
||||
/** Regenerates `src/db/entities/index.ts` from the directory, sorted, exporting `ALL_ENTITIES`. */
|
||||
export function regenerateEntitiesIndex(entitiesDir: string): string {
|
||||
const files = fs
|
||||
.readdirSync(entitiesDir)
|
||||
.filter((f) => f.endsWith('.entity.ts'))
|
||||
.sort();
|
||||
const classNames = files.map((f) => f.replace(/\.entity\.ts$/, ''));
|
||||
/** Builds `src/db/entities/index.ts`'s source for a given, already-sorted class-name list. */
|
||||
export function buildEntitiesIndexSource(classNames: readonly string[]): string {
|
||||
const imports = classNames.map((name) => `import { ${name}Schema } from './${name}.entity';`).join('\n');
|
||||
const exportList = classNames.map((name) => ` ${name}Schema,`).join('\n');
|
||||
return (
|
||||
@@ -1273,6 +1321,58 @@ export function regenerateEntitiesIndex(entitiesDir: string): string {
|
||||
);
|
||||
}
|
||||
|
||||
/** Regenerates `src/db/entities/index.ts` from the directory, sorted, exporting `ALL_ENTITIES`. */
|
||||
export function regenerateEntitiesIndex(entitiesDir: string): string {
|
||||
const classNames = fs
|
||||
.readdirSync(entitiesDir)
|
||||
.filter((f) => f.endsWith('.entity.ts'))
|
||||
.sort()
|
||||
.map((f) => f.replace(/\.entity\.ts$/, ''));
|
||||
return buildEntitiesIndexSource(classNames);
|
||||
}
|
||||
|
||||
/** What `--check` found: every generated file (entity + `index.ts`) that differs from — or is missing from — disk, and every repository `--write` would create. Read-only; never touches disk. */
|
||||
export interface CheckReport {
|
||||
/** `X.entity.ts` (or `'index.ts'`) for every generated file whose on-disk content differs, byte-for-byte, from what the generator produces now. */
|
||||
differingFiles: string[];
|
||||
/** `src/db/repositories/<X>.repository.ts` for every entity with no repository file yet — `--write` would create these. */
|
||||
missingRepositories: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* The read-only half of `--check` (I1 in `task-4-review-gates.md`): compares
|
||||
* `files` (already generated in memory by `generateEntities()`) plus the
|
||||
* `index.ts` they imply against `entitiesDir`, and reports which
|
||||
* `repositoriesDir/<X>.repository.ts` files `writeRepositoriesIfMissing`
|
||||
* would create. Never reads `regenerateEntitiesIndex`'s directory-listing
|
||||
* variant — the *expected* index is built straight from `files`' own key
|
||||
* set, not from whatever happens to already be on disk, so a stray or
|
||||
* missing entity file cannot mask itself out of the index comparison.
|
||||
*/
|
||||
export function checkEntities(entitiesDir: string, repositoriesDir: string, files: ReadonlyMap<string, string>): CheckReport {
|
||||
const differingFiles: string[] = [];
|
||||
for (const [fileName, content] of files) {
|
||||
const onDiskPath = path.join(entitiesDir, fileName);
|
||||
const onDisk = fs.existsSync(onDiskPath) ? fs.readFileSync(onDiskPath, 'utf8') : undefined;
|
||||
if (onDisk !== content) differingFiles.push(fileName);
|
||||
}
|
||||
|
||||
const classNames = [...files.keys()].map((f) => f.replace(/\.entity\.ts$/, '')).sort();
|
||||
const expectedIndex = buildEntitiesIndexSource(classNames);
|
||||
const indexPath = path.join(entitiesDir, 'index.ts');
|
||||
const onDiskIndex = fs.existsSync(indexPath) ? fs.readFileSync(indexPath, 'utf8') : undefined;
|
||||
if (onDiskIndex !== expectedIndex) differingFiles.push('index.ts');
|
||||
|
||||
const missingRepositories: string[] = [];
|
||||
for (const fileName of files.keys()) {
|
||||
const className = fileName.replace(/\.entity\.ts$/, '');
|
||||
const repoFile = path.join(repositoriesDir, `${className}.repository.ts`);
|
||||
if (!fs.existsSync(repoFile)) missingRepositories.push(repoFile);
|
||||
}
|
||||
|
||||
return { differingFiles, missingRepositories };
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// CLI
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -1280,11 +1380,28 @@ export function regenerateEntitiesIndex(entitiesDir: string): string {
|
||||
async function main(): Promise<void> {
|
||||
const args = process.argv.slice(2);
|
||||
const write = args.includes('--write');
|
||||
const check = args.includes('--check');
|
||||
const outIndex = args.indexOf('--out');
|
||||
const outDir = outIndex >= 0 ? args[outIndex + 1] : undefined;
|
||||
|
||||
const { files } = await generateEntities();
|
||||
|
||||
if (check) {
|
||||
const { differingFiles, missingRepositories } = checkEntities(ENTITIES_DIR, REPOSITORIES_DIR, files);
|
||||
for (const fileName of differingFiles) {
|
||||
console.error(`[generate-entities] DRIFT: src/db/entities/${fileName} does not match the generator.`);
|
||||
}
|
||||
for (const repoFile of missingRepositories) {
|
||||
console.error(`[generate-entities] MISSING: ${path.relative(SERVER_ROOT, repoFile)} would be created by --write.`);
|
||||
}
|
||||
if (differingFiles.length > 0 || missingRepositories.length > 0) {
|
||||
console.error('[generate-entities] --check failed. Run: node --import tsx scripts/generate-entities.ts --write');
|
||||
process.exit(1);
|
||||
}
|
||||
console.log('[generate-entities] --check: entity files match the generator');
|
||||
return;
|
||||
}
|
||||
|
||||
if (write) {
|
||||
dumpEntities(ENTITIES_DIR, files);
|
||||
const created = writeRepositoriesIfMissing(REPOSITORIES_DIR, files);
|
||||
|
||||
@@ -16,8 +16,8 @@ export class BudgetItems {
|
||||
category: string & Opt = 'Other';
|
||||
name!: string;
|
||||
total_price!: number & Opt;
|
||||
persons?: number | null = NaN;
|
||||
days?: number | null = NaN;
|
||||
persons?: number | null;
|
||||
days?: number | null;
|
||||
note?: string | null;
|
||||
sort_order?: number | null = 0;
|
||||
created_at?: string | null;
|
||||
@@ -25,12 +25,12 @@ export class BudgetItems {
|
||||
paid_by_user_id?: number | null;
|
||||
expense_date?: string | null;
|
||||
reservation?: Ref<Reservations> | null;
|
||||
reservation_id?: number | null = NaN;
|
||||
reservation_id?: number | null;
|
||||
currency?: string | null;
|
||||
exchange_rate!: number & Opt;
|
||||
ticket_json?: string | null;
|
||||
place?: Ref<Places> | null;
|
||||
place_id?: number | null = NaN;
|
||||
place_id?: number | null;
|
||||
budget_item_members_collection = new Collection<BudgetItemMembers>(this);
|
||||
budget_item_payers_collection = new Collection<BudgetItemPayers>(this);
|
||||
file_links_collection = new Collection<FileLinks>(this);
|
||||
|
||||
@@ -11,7 +11,7 @@ export class IdempotencyKeys {
|
||||
path!: string;
|
||||
status_code!: number;
|
||||
response_body!: string;
|
||||
created_at: number & Opt = NaN;
|
||||
created_at!: number & Opt;
|
||||
}
|
||||
|
||||
export const IdempotencyKeysSchema = defineEntity({
|
||||
|
||||
@@ -15,7 +15,7 @@ export class PackingBags {
|
||||
sort_order?: number | null = 0;
|
||||
created_at?: string | null;
|
||||
user?: Ref<Users> | null;
|
||||
user_id?: number | null = NaN;
|
||||
user_id?: number | null;
|
||||
packing_bag_members = new Collection<Users>(this);
|
||||
packing_items_collection = new Collection<PackingItems>(this);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
import { ReferenceKind } from '@mikro-orm/core';
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { ALL_ENTITIES } from '../../../src/db/entities';
|
||||
import { BudgetItems } from '../../../src/db/entities/BudgetItems.entity';
|
||||
import { IdempotencyKeys } from '../../../src/db/entities/IdempotencyKeys.entity';
|
||||
import { createUser, createTrip } from '../../helpers/factories';
|
||||
import { createSnapshotTestDb } from '../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
|
||||
|
||||
/**
|
||||
* Permanent guard for `task-4-review-shape.md` Important finding 1: a
|
||||
* numeric class-field initialiser must never render `NaN` (see
|
||||
* `scripts/generate-entities.ts`'s `RULE1d_fixNaNNumericDefaults` and
|
||||
* `tests/unit/db/generate-entities.test.ts`'s `RULE1D-*` fixture tests for
|
||||
* the mechanism). This file pins the OUTPUT of that rule against the real,
|
||||
* committed entities — every one of the 125, not a sample — and reproduces
|
||||
* the reviewer's own insert probe so a future regenerate/hand-edit that
|
||||
* reintroduces a `NaN` initialiser fails loudly here, not silently at a
|
||||
* caller's first `em.create()`.
|
||||
*/
|
||||
describe('entity class fields never carry a NaN initialiser (task-4-review-shape.md, Important 1)', () => {
|
||||
it('CLASSFIELD-001: new X() has no own property whose value is NaN, across all 125 entities', () => {
|
||||
const failures: string[] = [];
|
||||
for (const schema of ALL_ENTITIES) {
|
||||
const Ctor = schema.class as new () => Record<string, unknown>;
|
||||
const instance = new Ctor();
|
||||
for (const [key, value] of Object.entries(instance)) {
|
||||
if (typeof value === 'number' && Number.isNaN(value)) {
|
||||
failures.push(`${schema.meta.className}.${key}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
expect(failures).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* Report-only (per the brief): a NOT NULL scalar property with no
|
||||
* `default`/`defaultRaw` at all legitimately has no class-field
|
||||
* initialiser — the caller is expected to supply it (`name!: string`, for
|
||||
* example). That is normal and not a bug, so this never asserts the list
|
||||
* is empty; it exists so a reviewer can see the shape at a glance and so a
|
||||
* suspicious spike in the count (e.g. every numeric property in an entity
|
||||
* suddenly losing its default) shows up in test output without failing
|
||||
* the suite on a false positive.
|
||||
*/
|
||||
it('CLASSFIELD-002 (report-only): NOT NULL scalar properties with no default and no class-field initialiser', async () => {
|
||||
const testDb = createSnapshotTestDb();
|
||||
try {
|
||||
const t = await createTestOrm(testDb);
|
||||
try {
|
||||
const report: string[] = [];
|
||||
for (const schema of ALL_ENTITIES) {
|
||||
const meta = t.orm.getMetadata().get(schema.class);
|
||||
const Ctor = schema.class as new () => Record<string, unknown>;
|
||||
const instance = new Ctor();
|
||||
for (const prop of meta.props) {
|
||||
if (prop.kind !== undefined && prop.kind !== ReferenceKind.SCALAR) continue;
|
||||
if (prop.nullable) continue;
|
||||
if (prop.default !== undefined || prop.defaultRaw !== undefined) continue;
|
||||
if (!(prop.name in instance)) continue;
|
||||
if (instance[prop.name] === undefined) {
|
||||
report.push(`${meta.className}.${prop.name}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
// Informational only — see the doc comment above. `console.info` (not
|
||||
// a `console.log` vitest might buffer away) so it survives a run.
|
||||
console.info(`[CLASSFIELD-002] ${report.length} NOT NULL, no-default scalar propert${report.length === 1 ? 'y' : 'ies'} with no initialiser (expected, not a failure)`);
|
||||
expect(report).toEqual(report); // always true — this test only ever reports, never fails
|
||||
} finally {
|
||||
await t.close();
|
||||
}
|
||||
} finally {
|
||||
testDb.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The reviewer's exact repro (`task-4-review-shape.md`, Important finding
|
||||
* 1): before the fix, `em.create(BudgetItems, …)` and
|
||||
* `em.create(IdempotencyKeys, …)` both threw `SqliteError: no such column:
|
||||
* NaN` at `flush()` — Kysely rendered the `NaN` class-field initialiser as a
|
||||
* bare SQL identifier. Neither probe names the broken columns
|
||||
* (`persons`/`days`/`place_id`/`reservation_id` on `BudgetItems`,
|
||||
* `created_at` on `IdempotencyKeys`) — the whole point is that a caller
|
||||
* should never have to.
|
||||
*/
|
||||
describe('insert probes (task-4-review-shape.md, the reviewer\'s exact repro)', () => {
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
});
|
||||
beforeEach(() => {
|
||||
resetTestDb(testDb);
|
||||
t.clear();
|
||||
});
|
||||
afterAll(async () => {
|
||||
await t.close();
|
||||
testDb.close();
|
||||
});
|
||||
|
||||
it('INSERT-PROBE-001: em.create(BudgetItems, { trip, name, total_price, exchange_rate }) + flush succeeds', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
const created = t.em.create(BudgetItems, {
|
||||
trip: trip.id,
|
||||
name: 'Dinner',
|
||||
total_price: 4200,
|
||||
exchange_rate: 1,
|
||||
});
|
||||
await t.em.persist(created).flush();
|
||||
|
||||
const row = testDb.prepare('SELECT persons, days, place_id, reservation_id FROM budget_items WHERE id = ?').get(created.id);
|
||||
expect(row).toEqual({ persons: null, days: null, place_id: null, reservation_id: null });
|
||||
});
|
||||
|
||||
it('INSERT-PROBE-002: em.create(IdempotencyKeys, { key, user, method, path, status_code, response_body }) + flush succeeds', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
|
||||
const created = t.em.create(IdempotencyKeys, {
|
||||
key: 'idem-key-probe-1',
|
||||
user: user.id,
|
||||
method: 'POST',
|
||||
path: '/api/trips',
|
||||
status_code: 201,
|
||||
response_body: '{}',
|
||||
});
|
||||
await t.em.persist(created).flush();
|
||||
|
||||
const row = testDb
|
||||
.prepare('SELECT created_at FROM idempotency_keys WHERE key = ? AND user_id = ? AND method = ? AND path = ?')
|
||||
.get('idem-key-probe-1', user.id, 'POST', '/api/trips') as { created_at: number };
|
||||
// The DB's own defaultRaw (strftime('%s','now')) filled it in — never NaN, never null.
|
||||
expect(row.created_at).toEqual(expect.any(Number));
|
||||
expect(Number.isNaN(row.created_at)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -419,16 +419,28 @@ describe('entity ↔ migrated-schema parity', () => {
|
||||
* folds a `p.boolean()` property's literal `true`/`false` default onto
|
||||
* SQLite's `1`/`0` (SQLite has no boolean storage class — see
|
||||
* `BOOLEAN_COLUMNS` in `scripts/generate-entities.ts`). Used by PARITY-005.
|
||||
*
|
||||
* M1 (task-4-review-gates.md): only the SQL keywords/functions above are
|
||||
* folded case-insensitively — a quoted string literal's PAYLOAD is data, not
|
||||
* a keyword, and is compared verbatim, case included. A blanket
|
||||
* `s.toLowerCase()` on the whole string (the previous shape) silently
|
||||
* equated `'#10b981'` and `'#10B981'`, proved by the gate reviewer's mutation
|
||||
* (d2): flipping `Tags.color`'s class-field default's case left every test
|
||||
* green. The entity side and the db side aren't even symmetric about
|
||||
* quoting to begin with (`prop.default` arrives as a bare JS string, e.g.
|
||||
* `#10b981`; `column.dflt_value` arrives SQL-quoted, `'#10b981'`) — quotes
|
||||
* are stripped from whichever side has them, then BOTH sides are compared
|
||||
* verbatim, never lowercased, unless they match a known keyword.
|
||||
*/
|
||||
function normaliseDefault(value: string | null): string | null {
|
||||
if (value == null) return null;
|
||||
let s = value.trim();
|
||||
if (s.startsWith('(') && s.endsWith(')')) s = s.slice(1, -1).trim();
|
||||
if (s.startsWith("'") && s.endsWith("'")) s = s.slice(1, -1);
|
||||
s = s.toLowerCase();
|
||||
if (s === "datetime('now')" || s === 'current_timestamp') s = '__now__';
|
||||
if (s === 'true') s = '1';
|
||||
if (s === 'false') s = '0';
|
||||
if (s.startsWith("'") && s.endsWith("'")) s = s.slice(1, -1); // SQL quoting stripped; payload kept verbatim below
|
||||
const lower = s.toLowerCase();
|
||||
if (lower === "datetime('now')" || lower === 'current_timestamp') return '__now__';
|
||||
if (lower === 'true') return '1';
|
||||
if (lower === 'false') return '0';
|
||||
return s;
|
||||
}
|
||||
|
||||
@@ -453,6 +465,39 @@ describe('normaliseColumnType', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('normaliseDefault', () => {
|
||||
it('DEFAULT-NORM-001: a case-only difference in a quoted string literal payload is a real mismatch (M1)', () => {
|
||||
expect(normaliseDefault("'#10b981'")).not.toBe(normaliseDefault("'#10B981'"));
|
||||
});
|
||||
|
||||
it('DEFAULT-NORM-002: an identical quoted string literal payload matches, case preserved', () => {
|
||||
expect(normaliseDefault("'#10b981'")).toBe(normaliseDefault("'#10b981'"));
|
||||
expect(normaliseDefault("'#10b981'")).toBe('#10b981');
|
||||
});
|
||||
|
||||
it('DEFAULT-NORM-003: an unquoted default (as PARITY-005 passes the entity side, String(prop.default)) compares verbatim too', () => {
|
||||
expect(normaliseDefault('#10B981')).toBe('#10B981');
|
||||
expect(normaliseDefault('#10B981')).not.toBe(normaliseDefault("'#10b981'"));
|
||||
});
|
||||
|
||||
it('DEFAULT-NORM-004: SQL keyword/function spellings still fold case-insensitively', () => {
|
||||
expect(normaliseDefault('CURRENT_TIMESTAMP')).toBe('__now__');
|
||||
expect(normaliseDefault('current_timestamp')).toBe('__now__');
|
||||
expect(normaliseDefault("datetime('now')")).toBe('__now__');
|
||||
expect(normaliseDefault("DATETIME('NOW')")).toBe('__now__');
|
||||
expect(normaliseDefault('TRUE')).toBe('1');
|
||||
expect(normaliseDefault('FALSE')).toBe('0');
|
||||
});
|
||||
|
||||
it('DEFAULT-NORM-005: a wrapping paren pair around a non-literal expression is stripped once', () => {
|
||||
expect(normaliseDefault("(strftime('%s','now'))")).toBe("strftime('%s','now')");
|
||||
});
|
||||
|
||||
it('DEFAULT-NORM-006: null passes through as null', () => {
|
||||
expect(normaliseDefault(null)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('columnTypesEqual', () => {
|
||||
it('TYPE-004: accepts datetime on one side and text on the other, either direction', () => {
|
||||
expect(columnTypesEqual('datetime', 'text')).toBe(true);
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
RULE1_fixUnknownScalarTypes,
|
||||
RULE1b_markJsonColumns,
|
||||
RULE1c_markBooleanColumns,
|
||||
RULE1d_fixNaNNumericDefaults,
|
||||
RULE2_datetimeToDbTimestampType,
|
||||
RULE3_integerPkAutoincrement,
|
||||
RULE4_hideAllRelations,
|
||||
@@ -21,6 +22,8 @@ import {
|
||||
RULE8_bindRepositories,
|
||||
RULE_normalizeLiteralDefaults,
|
||||
applyTextPasses,
|
||||
checkEntities,
|
||||
dumpEntities,
|
||||
fixDbTimestampClassFieldTypes,
|
||||
fixJsonClassFieldTypes,
|
||||
generateEntities,
|
||||
@@ -35,6 +38,7 @@ import {
|
||||
} from '../../../scripts/generate-entities';
|
||||
|
||||
const ENTITIES_DIR = path.join(__dirname, '../../../src/db/entities');
|
||||
const REPOSITORIES_DIR = path.join(__dirname, '../../../src/db/repositories');
|
||||
|
||||
/**
|
||||
* A minimal, correctly-typed `EntityProperty` fixture. Every rule under test
|
||||
@@ -169,6 +173,56 @@ describe('RULE1c_markBooleanColumns', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('RULE1d_fixNaNNumericDefaults', () => {
|
||||
/**
|
||||
* `task-4-review-shape.md` Important finding 1: `@mikro-orm/sql`'s own
|
||||
* introspection computes `prop.default` for a numeric column as
|
||||
* `+prop.defaultRaw`, which is `NaN` whenever `defaultRaw` is the literal
|
||||
* text `'NULL'` (a nullable column with an explicit `DEFAULT NULL`) or a
|
||||
* non-literal SQL expression (`(strftime('%s','now'))`) — the exact repro
|
||||
* the reviewer ran against the real schema (`BudgetItems.persons`/`.days`,
|
||||
* `IdempotencyKeys.created_at`, `BudgetItems.place_id`/`.reservation_id`,
|
||||
* `PackingBags.user_id`).
|
||||
*/
|
||||
it("RULE1D-001: defaultRaw 'NULL' with a NaN default clears the initialiser (never renders = null)", () => {
|
||||
const p = fixtureProp({ name: 'persons', type: 'integer', nullable: true, defaultRaw: 'NULL', default: NaN });
|
||||
const meta = fixtureMeta('BudgetItems', 'budget_items', [p]);
|
||||
RULE1d_fixNaNNumericDefaults([meta]);
|
||||
expect(p.default).toBeUndefined();
|
||||
expect(p.defaultRaw).toBe('NULL'); // untouched — the schema's .defaultRaw('NULL') is correct and stays
|
||||
});
|
||||
|
||||
it('RULE1D-002: a non-literal expression default with a NaN default clears the initialiser', () => {
|
||||
const p = fixtureProp({ name: 'created_at', type: 'integer', nullable: false, defaultRaw: "(strftime('%s','now'))", default: NaN });
|
||||
const meta = fixtureMeta('IdempotencyKeys', 'idempotency_keys', [p]);
|
||||
RULE1d_fixNaNNumericDefaults([meta]);
|
||||
expect(p.default).toBeUndefined();
|
||||
expect(p.defaultRaw).toBe("(strftime('%s','now'))"); // untouched
|
||||
});
|
||||
|
||||
it("RULE1D-003: a genuine finite numeric literal default ('0') is left completely untouched", () => {
|
||||
const p = fixtureProp({ name: 'sort_order', type: 'integer', nullable: false, defaultRaw: '0', default: 0 });
|
||||
const meta = fixtureMeta('X', 'x', [p]);
|
||||
RULE1d_fixNaNNumericDefaults([meta]);
|
||||
expect(p.default).toBe(0);
|
||||
expect(p.defaultRaw).toBe('0');
|
||||
});
|
||||
|
||||
it('RULE1D-004: a property whose default is not NaN (e.g. a text column default(raw) NULL, default null) is untouched', () => {
|
||||
const p = fixtureProp({ name: 'note', type: 'text', nullable: true, defaultRaw: 'NULL', default: null });
|
||||
const meta = fixtureMeta('X', 'x', [p]);
|
||||
RULE1d_fixNaNNumericDefaults([meta]);
|
||||
expect(p.default).toBeNull();
|
||||
});
|
||||
|
||||
it('RULE1D-005: a property with no defaultRaw at all is untouched', () => {
|
||||
const p = fixtureProp({ name: 'title', type: 'text' });
|
||||
const meta = fixtureMeta('X', 'x', [p]);
|
||||
RULE1d_fixNaNNumericDefaults([meta]);
|
||||
expect(p.default).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('RULE2_datetimeToDbTimestampType', () => {
|
||||
it('RULE2-001: p.datetime() -> p.type(DbTimestampType), fixup recorded', () => {
|
||||
const p = fixtureProp({ name: 'created_at', type: 'datetime', nullable: true, defaultRaw: 'CURRENT_TIMESTAMP' });
|
||||
@@ -637,6 +691,92 @@ describe('regenerateEntitiesIndex', () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* `--check` (task-4-review-gates.md, I1): closes the gap the gate reviewer
|
||||
* proved — losing `.hidden()` on a relation (or any other rule regression)
|
||||
* passes every other suite for 347 of 396 relation lines. These tests exercise
|
||||
* the real generated `files` (from `generateEntities()`) against a SCRATCH
|
||||
* COPY of the committed `src/db/entities/` — never the real directory itself,
|
||||
* so a failing assertion here can never leave the working tree dirty.
|
||||
*/
|
||||
describe('checkEntities', () => {
|
||||
it(
|
||||
'CHECK-001: a hand-mutated entity in the scratch copy is reported by name, exit-worthy',
|
||||
async () => {
|
||||
const { files } = await generateEntities();
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-'));
|
||||
try {
|
||||
fs.cpSync(ENTITIES_DIR, dir, { recursive: true });
|
||||
// Same shape as VALIDATE-004's mutation (MUT-B): drop a ONE_TO_MANY relation's .hidden().
|
||||
const daysPath = path.join(dir, 'Days.entity.ts');
|
||||
const original = fs.readFileSync(daysPath, 'utf8');
|
||||
const mutated = original.replace(
|
||||
"day_accommodations_collection: () => p.oneToMany(DayAccommodations).mappedBy('startDay').hidden(),",
|
||||
"day_accommodations_collection: () => p.oneToMany(DayAccommodations).mappedBy('startDay'),",
|
||||
);
|
||||
expect(mutated, 'the mutation string was not found in the committed file — update it to match the current shape').not.toBe(
|
||||
original,
|
||||
);
|
||||
fs.writeFileSync(daysPath, mutated);
|
||||
|
||||
const report = checkEntities(dir, REPOSITORIES_DIR, files);
|
||||
expect(report.differingFiles).toEqual(['Days.entity.ts']);
|
||||
expect(report.missingRepositories).toEqual([]);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
30_000,
|
||||
);
|
||||
|
||||
it(
|
||||
'CHECK-002: an untouched copy of the committed tree reports no differences and no missing repositories',
|
||||
async () => {
|
||||
const { files } = await generateEntities();
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-'));
|
||||
try {
|
||||
fs.cpSync(ENTITIES_DIR, dir, { recursive: true });
|
||||
const report = checkEntities(dir, REPOSITORIES_DIR, files);
|
||||
expect(report.differingFiles).toEqual([]);
|
||||
expect(report.missingRepositories).toEqual([]);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
30_000,
|
||||
);
|
||||
|
||||
it('CHECK-003: a missing entity file is reported', async () => {
|
||||
const files = new Map([['X.entity.ts', 'content']]);
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-'));
|
||||
const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-repos-'));
|
||||
try {
|
||||
const report = checkEntities(dir, repoDir, files);
|
||||
expect(report.differingFiles).toEqual(['X.entity.ts', 'index.ts']);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
fs.rmSync(repoDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('CHECK-004: a missing repository file is reported without being created (never writes)', async () => {
|
||||
const files = new Map([['X.entity.ts', 'content']]);
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-'));
|
||||
const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gen-entities-check-repos-'));
|
||||
try {
|
||||
dumpEntities(dir, files);
|
||||
fs.writeFileSync(path.join(dir, 'index.ts'), regenerateEntitiesIndex(dir));
|
||||
const report = checkEntities(dir, repoDir, files);
|
||||
expect(report.differingFiles).toEqual([]);
|
||||
expect(report.missingRepositories).toEqual([path.join(repoDir, 'X.repository.ts')]);
|
||||
expect(fs.existsSync(path.join(repoDir, 'X.repository.ts'))).toBe(false); // never writes
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
fs.rmSync(repoDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('assertFilesGenerated', () => {
|
||||
it('GUARD-001: throws when zero files were generated (I5)', () => {
|
||||
expect(() => assertFilesGenerated(new Map())).toThrow(/zero entity files/);
|
||||
@@ -850,6 +990,28 @@ describe('PENDING_ENTITIES ratchet (I7)', () => {
|
||||
}
|
||||
expect(failures).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* M2 (task-4-review-gates.md): the mirror arm ENTITIES_STILL_MISSING has
|
||||
* (PARITY-008) and this list did not — a listed class name that names
|
||||
* nothing the generator actually produces is a stale entry, not a real
|
||||
* pending tolerance, and `stripPendingEntityRelations` would silently no-op
|
||||
* on it forever. Every name in `PENDING_ENTITIES` must appear somewhere in
|
||||
* `generateEntities()`'s own output (as a relation type reference in
|
||||
* another entity's file, the normal way a class with no entity of its own
|
||||
* yet still shows up — see the class doc comment above).
|
||||
*/
|
||||
it("PENDING-ENTITIES-002: stays accurate — every one of them must appear in generateEntities()'s output (M2)", async () => {
|
||||
const { files } = await generateEntities();
|
||||
const allGeneratedSource = [...files.values()].join('\n');
|
||||
const failures: string[] = [];
|
||||
for (const className of PENDING_ENTITIES) {
|
||||
if (!new RegExp(`\\b${className}\\b`).test(allGeneratedSource)) {
|
||||
failures.push(`${className}: listed in PENDING_ENTITIES but does not appear anywhere in generateEntities()'s output`);
|
||||
}
|
||||
}
|
||||
expect(failures).toEqual([]);
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
describe('generateEntities — migrations path is anchored to SERVER_ROOT, not cwd (I5)', () => {
|
||||
|
||||
@@ -172,7 +172,7 @@ export default defineConfig({
|
||||
// src/demo/** is deliberately absent: it measures 0%, and a floor of
|
||||
// zero asserts nothing. It needs tests before it needs a threshold.
|
||||
'src/app-config/**/*.ts': { statements: 99, branches: 95, functions: 99, lines: 99 },
|
||||
'src/db/**/*.ts': { statements: 73, branches: 38, functions: 59, lines: 80 },
|
||||
'src/db/**/*.ts': { statements: 84, branches: 42, functions: 95, lines: 87 },
|
||||
'src/db/dialect/**/*.ts': { statements: 99, branches: 99, functions: 99, lines: 99 },
|
||||
'src/db/repositories/**/*.ts': { statements: 99, branches: 99, functions: 99, lines: 99 },
|
||||
'src/db/types/**/*.ts': { statements: 93, branches: 99, functions: 99, lines: 93 },
|
||||
|
||||
Reference in New Issue
Block a user