mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 21:46:49 +00:00
fix(server): plan 4 — un-skip PARITY-009 (FK deleteRule and index-name parity)
This commit is contained in:
@@ -968,6 +968,125 @@ export function RULE12_fixGarbledCheckExpressions(metadata: EntityMetadata[]): C
|
||||
return fixups;
|
||||
}
|
||||
|
||||
/** A relation whose `deleteRule` RULE13 had to pin explicitly because MikroORM's own runtime default would otherwise disagree with `PRAGMA foreign_key_list`. */
|
||||
export interface DeleteRuleFixup {
|
||||
className: string;
|
||||
propName: string;
|
||||
from: string | undefined;
|
||||
to: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* RULE13 (metadata level; DB-dependent — see `collectForeignKeyDeleteRules`
|
||||
* below for the half that queries the schema): every owning to-one
|
||||
* relation's EFFECTIVE `deleteRule` — explicit if the entity sets one,
|
||||
* otherwise whatever `@mikro-orm/core`'s `MetadataDiscovery` infers by
|
||||
* default at runtime — must equal `PRAGMA foreign_key_list`'s `on_delete`
|
||||
* for its column. Plan 4 Task 7's own measurement
|
||||
* (`tests/unit/db/entity-schema-parity.test.ts`, PARITY-009b) found exactly
|
||||
* 10 owning relations across 120 entities where it does not.
|
||||
*
|
||||
* The root cause, read directly out of
|
||||
* `node_modules/@mikro-orm/core/metadata/MetadataDiscovery.js` (not
|
||||
* assumed): a nullable owning relation defaults to `deleteRule ??= 'set
|
||||
* null'` (`initManyToOneFields`/`initOneToOneFields`); separately, when
|
||||
* EVERY one of an entity's primary keys is itself a relation (composite or
|
||||
* single FK-as-PK), each of those relations defaults to `deleteRule ??=
|
||||
* 'cascade'` (`processEntity`'s `fkPks` block) — and the nullable check runs
|
||||
* FIRST, so a relation that is both nullable and part of an all-FK PK still
|
||||
* gets `'set null'`, never `'cascade'`. Neither default is announced by the
|
||||
* generated source when the introspected value happens to equal it — Rule 7
|
||||
* above strips the redundant `deleteRule('no action')` noise, and
|
||||
* `@mikro-orm/entity-generator`'s OWN `cleanUpReferentialIntegrityRules`
|
||||
* (`EntityGenerator.js`) strips a redundant `'cascade'`/`'set null'` the
|
||||
* same way for the FK-as-PK, fixed-order-pivot and nullable-relation shapes
|
||||
* — but neither cleanup step re-derives the SAME default this rule computes
|
||||
* here, so when the physical schema's real `on_delete` differs from what
|
||||
* MikroORM would infer left implicit, the generated entity silently carries
|
||||
* the wrong effective rule. Concretely, of the 10: 5 are a nullable relation
|
||||
* whose column has no `ON DELETE` clause at all in its migration (SQLite ⇒
|
||||
* `NO ACTION`) — `trip_members.invited_by`, `oauth_tokens.parent_token_id`,
|
||||
* `budget_settlements.created_by_user_id`, `budget_items.paid_by_user_id`,
|
||||
* and `roadtrip_day_tracks.day_id` (nullable AND the table's sole PK — its
|
||||
* migration DOES say `ON DELETE CASCADE`, so this one goes the other way:
|
||||
* physical is `cascade`, the nullable default silently gives `set null`);
|
||||
* `journey_contributors.user_id` is a composite-PK member whose migration
|
||||
* has no `ON DELETE` (physical `no action`) while its FK-as-PK sibling
|
||||
* relation, `journey_id`, DOES say `ON DELETE CASCADE` (so its own implicit
|
||||
* `cascade` default is correct — only `user_id` disagrees); the remaining 4
|
||||
* (`reservation_travelers.reservation_id`/`.user_id`,
|
||||
* `assignment_participants.assignment_id`/`.user_id`) are plain non-nullable
|
||||
* relations on a table with its own `id INTEGER PRIMARY KEY AUTOINCREMENT`
|
||||
* — no implicit default applies to them at all (MikroORM has none for that
|
||||
* shape) — but `@mikro-orm/entity-generator`'s pivot-shaped-table cleanup
|
||||
* (Case 2: a single autoincrement PK + exactly 2 many-to-one relations and
|
||||
* no other columns) strips their introspected `'cascade'` on the mistaken
|
||||
* assumption that MikroORM would restore it as a default, which it does not
|
||||
* for a non-composite-PK entity; their migrations DO say `ON DELETE CASCADE`.
|
||||
*
|
||||
* Every one of the 10 is, per the task's own ruling, an entity that never
|
||||
* matched its migration's physical FK to begin with — not a migration
|
||||
* defect — so this rule always fixes the ENTITY side: it pins an explicit
|
||||
* `deleteRule` equal to the physical value whenever leaving the property
|
||||
* alone (explicit-if-set, else the same implicit default computed above)
|
||||
* would produce the wrong effective rule, and otherwise leaves the property
|
||||
* untouched — matching Rule 7's own "no noise where the implicit default is
|
||||
* already correct" intent instead of stamping every relation explicitly.
|
||||
*
|
||||
* One more wrinkle, found empirically (probed `t.orm.getMetadata()` against
|
||||
* the real `ALL_ENTITIES`, not assumed): a relation that is BOTH nullable
|
||||
* AND its entity's sole/composite FK-as-PK member — `RoadtripDayTracks.day`,
|
||||
* `DawarichConnections.user`, `PlaceRegions.place`, `VacayUserSettings.user`,
|
||||
* every one of them shaped exactly like `.primary().ref().nullable()` with
|
||||
* no explicit `deleteRule` — is where BOTH candidate defaults above apply at
|
||||
* once, and the doc comment's "nullable runs first" precedence is only true
|
||||
* WITHIN a single `processEntity(meta)` call. Across entities, `Days`'/
|
||||
* `Users`'/`Places`' own inverse `.mappedBy(...)` side of the same relation
|
||||
* also runs `initManyToOneFields`, and depending on `ALL_ENTITIES`
|
||||
* discovery order that can re-touch the owning prop before ITS OWN
|
||||
* `processEntity` runs — measured outcome: `RoadtripDayTracks.day` really
|
||||
* does resolve to `'set null'` at runtime (matching this task's 10), while
|
||||
* the other three resolve to `'cascade'` (NOT in the 10) despite identical
|
||||
* source. That order dependency is exactly the kind of fact a future entity
|
||||
* addition could silently flip, so this rule does not trust "already
|
||||
* correct" for this one shape — `isAmbiguousNullablePrimaryFk` below always
|
||||
* pins an explicit `deleteRule` on it, matching physical, even on the three
|
||||
* that happen to already be correct today.
|
||||
*/
|
||||
export function RULE13_pinDeleteRuleDrift(
|
||||
metadata: EntityMetadata[],
|
||||
fkDeleteRules: ReadonlyMap<string, ReadonlyMap<string, string>>,
|
||||
): DeleteRuleFixup[] {
|
||||
const fixups: DeleteRuleFixup[] = [];
|
||||
for (const meta of metadata) {
|
||||
const byColumn = fkDeleteRules.get(meta.tableName);
|
||||
if (!byColumn) continue;
|
||||
const pks = meta.getPrimaryProps().filter((pk): pk is EntityProperty => pk !== undefined);
|
||||
const allPksAreFk = pks.length > 0 && pks.every((pk) => pk.kind !== undefined && pk.kind !== ReferenceKind.SCALAR);
|
||||
for (const prop of meta.relations) {
|
||||
if (!isOwningToOne(prop) || prop.fieldNames.length !== 1) continue;
|
||||
const physical = byColumn.get(prop.fieldNames[0]);
|
||||
if (physical === undefined) continue; // no db FK on this column at all — PARITY-009 proper's job, not this rule's
|
||||
// The two candidate implicit defaults collide on this one shape — see
|
||||
// the doc comment's "one more wrinkle" — so never trust "already
|
||||
// correct" here; always pin it explicitly.
|
||||
const isAmbiguousNullablePrimaryFk = !!prop.nullable && !!prop.primary && allPksAreFk;
|
||||
if (!isAmbiguousNullablePrimaryFk) {
|
||||
// The same precedence MetadataDiscovery applies at runtime within a single
|
||||
// entity's own processEntity() call: nullable wins over FK-as-PK.
|
||||
const implicitDefault = prop.nullable ? 'set null' : prop.primary && allPksAreFk ? 'cascade' : 'no action';
|
||||
const effectiveIfLeftAsIs = (prop.deleteRule ?? implicitDefault).toLowerCase();
|
||||
if (effectiveIfLeftAsIs === physical) continue; // already correct, explicit or by the implicit default alike
|
||||
} else if ((prop.deleteRule ?? '').toLowerCase() === physical) {
|
||||
continue; // already pinned explicitly to the right value — nothing to change
|
||||
}
|
||||
fixups.push({ className: meta.className, propName: prop.name, from: prop.deleteRule, to: physical });
|
||||
prop.deleteRule = physical;
|
||||
}
|
||||
}
|
||||
return fixups;
|
||||
}
|
||||
|
||||
/** A scalar property whose literal default the renderer's own heuristic drops or mis-renders — see below. */
|
||||
export interface DefaultFixup {
|
||||
className: string;
|
||||
@@ -1048,17 +1167,21 @@ export interface RuleFixups {
|
||||
referencedColumns: ReferencedColumnsFixup[];
|
||||
/** Rule 12's garbled-CHECK-expression repairs. */
|
||||
checkExpressions: CheckExpressionFixup[];
|
||||
/** Rule 13's deleteRule-drift pins — reporting only, no text pass depends on it (the renderer already emits an explicit `.deleteRule(...)` from `prop.deleteRule` alone). */
|
||||
deleteRules: DeleteRuleFixup[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Every rule above, composed into the single hook MikroORM calls. Order
|
||||
* matters (see comments). `implicitUniques` defaults to empty so a caller
|
||||
* (the fixture tests) that doesn't care about Rule 9 need not pass it.
|
||||
* matters (see comments). `implicitUniques`/`fkDeleteRules` default to empty
|
||||
* so a caller (the fixture tests) that doesn't care about Rule 9/Rule 13
|
||||
* need not pass them.
|
||||
*/
|
||||
export function applyRules(
|
||||
metadata: EntityMetadata[],
|
||||
_platform: Platform,
|
||||
implicitUniques: ReadonlyMap<string, string[][]> = new Map(),
|
||||
fkDeleteRules: ReadonlyMap<string, ReadonlyMap<string, string>> = new Map(),
|
||||
): RuleFixups {
|
||||
const retypedByRule1 = RULE1_fixUnknownScalarTypes(metadata);
|
||||
const jsonColumns = RULE1b_markJsonColumns(metadata);
|
||||
@@ -1089,6 +1212,11 @@ export function applyRules(
|
||||
// the renderer dumps verbatim — no text pass depends on it, same as
|
||||
// RULE7/RULE9); placed last only by convention (highest rule number).
|
||||
const checkExpressions = RULE12_fixGarbledCheckExpressions(metadata);
|
||||
// Must run after RULE7 (which already dropped the genuinely redundant
|
||||
// `deleteRule('no action')` noise) so `prop.deleteRule` reflects what the
|
||||
// generated source would actually carry before this rule decides whether
|
||||
// that's still wrong against the live schema.
|
||||
const deleteRules = RULE13_pinDeleteRuleDrift(metadata, fkDeleteRules);
|
||||
return {
|
||||
joinColumns,
|
||||
defaults,
|
||||
@@ -1098,6 +1226,7 @@ export function applyRules(
|
||||
repositoryMarkers,
|
||||
referencedColumns,
|
||||
checkExpressions,
|
||||
deleteRules,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1139,6 +1268,31 @@ export async function collectImplicitUniqueIndexes(
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* DB-dependent half of Rule 13: for every table name given, `PRAGMA
|
||||
* foreign_key_list`'s `on_delete` per `from` (owning) column, lowercased —
|
||||
* the same physical fact `tests/unit/db/entity-schema-parity.test.ts`'s
|
||||
* PARITY-009b reads at test time, queried once here so `RULE13_pinDeleteRuleDrift`
|
||||
* stays a pure function over plain data, same shape as Rule 9's split.
|
||||
*/
|
||||
export async function collectForeignKeyDeleteRules(
|
||||
connection: Connection,
|
||||
tableNames: readonly string[],
|
||||
): Promise<Map<string, Map<string, string>>> {
|
||||
const result = new Map<string, Map<string, string>>();
|
||||
for (const tableName of tableNames) {
|
||||
const fkRows = (await connection.execute(`pragma foreign_key_list(\`${tableName}\`)`, [], 'all')) as {
|
||||
from: string;
|
||||
on_delete: string;
|
||||
}[];
|
||||
if (fkRows.length === 0) continue;
|
||||
const byColumn = new Map<string, string>();
|
||||
for (const row of fkRows) byColumn.set(row.from, row.on_delete.toLowerCase());
|
||||
result.set(tableName, byColumn);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// onImport — resolves the two identifiers the rules above add to a file's
|
||||
// entityImports that are NOT sibling `*.entity.ts` classes: `DbTimestampType`
|
||||
@@ -1749,6 +1903,7 @@ export async function generateEntities(): Promise<GenerateResult> {
|
||||
repositoryMarkers: [],
|
||||
referencedColumns: [],
|
||||
checkExpressions: [],
|
||||
deleteRules: [],
|
||||
};
|
||||
const rawFiles = await generator.generate({
|
||||
entityDefinition: 'defineEntity',
|
||||
@@ -1762,13 +1917,12 @@ export async function generateEntities(): Promise<GenerateResult> {
|
||||
onImport,
|
||||
onProcessedMetadata: async (metadata, platform) => {
|
||||
// Same connection the generator itself just introspected the schema
|
||||
// through (`orm`, in scope from just above) — Rule 9 needs no
|
||||
// second connection or a second migrated temp DB.
|
||||
const implicitUniques = await collectImplicitUniqueIndexes(
|
||||
orm.em.getConnection(),
|
||||
metadata.map((meta) => meta.tableName),
|
||||
);
|
||||
fixups = applyRules(metadata, platform, implicitUniques);
|
||||
// through (`orm`, in scope from just above) — Rule 9/Rule 13 need
|
||||
// no second connection or a second migrated temp DB.
|
||||
const tableNames = metadata.map((meta) => meta.tableName);
|
||||
const implicitUniques = await collectImplicitUniqueIndexes(orm.em.getConnection(), tableNames);
|
||||
const fkDeleteRules = await collectForeignKeyDeleteRules(orm.em.getConnection(), tableNames);
|
||||
fixups = applyRules(metadata, platform, implicitUniques, fkDeleteRules);
|
||||
},
|
||||
});
|
||||
|
||||
@@ -1786,6 +1940,7 @@ export async function generateEntities(): Promise<GenerateResult> {
|
||||
repositoryMarkers: fixups.repositoryMarkers.filter((f) => f.className === className),
|
||||
referencedColumns: fixups.referencedColumns.filter((f) => f.className === className),
|
||||
checkExpressions: fixups.checkExpressions.filter((f) => f.className === className),
|
||||
deleteRules: fixups.deleteRules.filter((f) => f.className === className),
|
||||
};
|
||||
files.set(`${className}.entity.ts`, applyTextPasses(raw, relevantFixups));
|
||||
}
|
||||
|
||||
@@ -18,9 +18,9 @@ export const AssignmentParticipantsSchema = defineEntity({
|
||||
uniques: [{ properties: ['assignment', 'user'] }],
|
||||
properties: {
|
||||
id: p.integer().primary(),
|
||||
assignment: () => p.manyToOne(DayAssignments).ref().hidden().index('idx_assignment_participants_assignment'),
|
||||
assignment: () => p.manyToOne(DayAssignments).ref().deleteRule('cascade').hidden().index('idx_assignment_participants_assignment'),
|
||||
assignment_id: p.integer().persist(false).index('idx_assignment_participants_assignment'),
|
||||
user: () => p.manyToOne(Users).ref().hidden(),
|
||||
user: () => p.manyToOne(Users).ref().deleteRule('cascade').hidden(),
|
||||
user_id: p.integer().persist(false),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -52,7 +52,7 @@ export const BudgetItemsSchema = defineEntity({
|
||||
note: p.text().nullable(),
|
||||
sort_order: p.integer().nullable(),
|
||||
created_at: p.type(DbTimestampType).nullable().defaultRaw(`CURRENT_TIMESTAMP`),
|
||||
paidByUser: () => p.manyToOne(Users).ref().nullable().hidden(),
|
||||
paidByUser: () => p.manyToOne(Users).ref().deleteRule('no action').nullable().hidden(),
|
||||
paid_by_user_id: p.integer().nullable().persist(false),
|
||||
expense_date: p.text().nullable().defaultRaw(`NULL`),
|
||||
reservation: () => p.manyToOne(Reservations).ref().nullable().hidden().defaultRaw(`NULL`),
|
||||
|
||||
@@ -35,7 +35,7 @@ export const BudgetSettlementsSchema = defineEntity({
|
||||
to_user_id: p.integer().persist(false),
|
||||
amount: p.double().defaultRaw(`0`),
|
||||
created_at: p.type(DbTimestampType).nullable().defaultRaw(`CURRENT_TIMESTAMP`),
|
||||
createdByUser: () => p.manyToOne(Users).ref().nullable().hidden(),
|
||||
createdByUser: () => p.manyToOne(Users).ref().deleteRule('no action').nullable().hidden(),
|
||||
created_by_user_id: p.integer().nullable().persist(false),
|
||||
currency: p.text().nullable(),
|
||||
exchange_rate: p.double().defaultRaw(`1`),
|
||||
|
||||
@@ -24,7 +24,7 @@ export const DawarichConnectionsSchema = defineEntity({
|
||||
class: DawarichConnections,
|
||||
repository: () => DawarichConnectionsRepository,
|
||||
properties: {
|
||||
user: () => p.oneToOne(Users).primary().ref().nullable().hidden(),
|
||||
user: () => p.oneToOne(Users).primary().ref().deleteRule('cascade').nullable().hidden(),
|
||||
user_id: p.integer().nullable().persist(false),
|
||||
url: p.text().nullable(),
|
||||
api_key: p.text().nullable(),
|
||||
|
||||
@@ -22,7 +22,7 @@ export const JourneyContributorsSchema = defineEntity({
|
||||
properties: {
|
||||
journey: () => p.manyToOne(Journeys).primary().ref().hidden(),
|
||||
journey_id: p.integer().persist(false),
|
||||
user: () => p.manyToOne(Users).primary().ref().hidden().index('idx_journey_contributors_user'),
|
||||
user: () => p.manyToOne(Users).primary().ref().deleteRule('no action').hidden().index('idx_journey_contributors_user'),
|
||||
user_id: p.integer().persist(false).index('idx_journey_contributors_user'),
|
||||
role: p.text(),
|
||||
added_at: p.integer(),
|
||||
|
||||
@@ -44,7 +44,7 @@ export const OauthTokensSchema = defineEntity({
|
||||
refresh_token_expires_at: p.type(DbTimestampType),
|
||||
revoked_at: p.type(DbTimestampType).nullable(),
|
||||
created_at: p.type(DbTimestampType).nullable().defaultRaw(`CURRENT_TIMESTAMP`),
|
||||
parentToken: () => p.manyToOne(OauthTokens).ref().nullable().hidden().index('idx_oauth_tokens_parent'),
|
||||
parentToken: () => p.manyToOne(OauthTokens).ref().deleteRule('no action').nullable().hidden().index('idx_oauth_tokens_parent'),
|
||||
parent_token_id: p.integer().nullable().persist(false).index('idx_oauth_tokens_parent'),
|
||||
audience: p.text().nullable(),
|
||||
oauth_tokens_collection: () => p.oneToMany(OauthTokens).mappedBy('parentToken').hidden(),
|
||||
|
||||
@@ -16,7 +16,7 @@ export const PlaceRegionsSchema = defineEntity({
|
||||
class: PlaceRegions,
|
||||
repository: () => PlaceRegionsRepository,
|
||||
properties: {
|
||||
place: () => p.oneToOne(Places).primary().ref().nullable().hidden(),
|
||||
place: () => p.oneToOne(Places).primary().ref().deleteRule('cascade').nullable().hidden(),
|
||||
place_id: p.integer().nullable().persist(false),
|
||||
country_code: p.text().index('idx_place_regions_country'),
|
||||
region_code: p.text().index('idx_place_regions_region'),
|
||||
|
||||
@@ -18,9 +18,9 @@ export const ReservationTravelersSchema = defineEntity({
|
||||
uniques: [{ properties: ['reservation', 'user'] }],
|
||||
properties: {
|
||||
id: p.integer().primary(),
|
||||
reservation: () => p.manyToOne(Reservations).ref().hidden().index('idx_reservation_travelers_res'),
|
||||
reservation: () => p.manyToOne(Reservations).ref().deleteRule('cascade').hidden().index('idx_reservation_travelers_res'),
|
||||
reservation_id: p.integer().persist(false).index('idx_reservation_travelers_res'),
|
||||
user: () => p.manyToOne(Users).ref().hidden().index('idx_reservation_travelers_user'),
|
||||
user: () => p.manyToOne(Users).ref().deleteRule('cascade').hidden().index('idx_reservation_travelers_user'),
|
||||
user_id: p.integer().persist(false).index('idx_reservation_travelers_user'),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -18,7 +18,7 @@ export const RoadtripDayTracksSchema = defineEntity({
|
||||
class: RoadtripDayTracks,
|
||||
repository: () => RoadtripDayTracksRepository,
|
||||
properties: {
|
||||
day: () => p.oneToOne(Days).primary().ref().nullable().hidden(),
|
||||
day: () => p.oneToOne(Days).primary().ref().deleteRule('cascade').nullable().hidden(),
|
||||
day_id: p.integer().nullable().persist(false),
|
||||
place: () => p.manyToOne(Places).ref().deleteRule('cascade').hidden().index('idx_roadtrip_day_tracks_place'),
|
||||
place_id: p.integer().persist(false).index('idx_roadtrip_day_tracks_place'),
|
||||
|
||||
@@ -28,6 +28,6 @@ export const TripMembersSchema = defineEntity({
|
||||
user_id: p.integer().persist(false).index('idx_trip_members_user_id'),
|
||||
invited_by: p.integer().nullable().persist(false),
|
||||
added_at: p.type(DbTimestampType).nullable().defaultRaw(`CURRENT_TIMESTAMP`),
|
||||
invitedByRef: () => p.manyToOne(Users).ref().joinColumn('invited_by').nullable().hidden(),
|
||||
invitedByRef: () => p.manyToOne(Users).ref().joinColumn('invited_by').deleteRule('no action').nullable().hidden(),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -17,7 +17,7 @@ export const VacayUserSettingsSchema = defineEntity({
|
||||
class: VacayUserSettings,
|
||||
repository: () => VacayUserSettingsRepository,
|
||||
properties: {
|
||||
user: () => p.oneToOne(Users).primary().ref().nullable().hidden(),
|
||||
user: () => p.oneToOne(Users).primary().ref().deleteRule('cascade').nullable().hidden(),
|
||||
user_id: p.integer().nullable().persist(false),
|
||||
year_type: p.text().default('calendar'),
|
||||
year_start_month: p.integer().default(1),
|
||||
|
||||
@@ -317,8 +317,17 @@ describe('entity ↔ migrated-schema parity', () => {
|
||||
* `school_holiday_regions.country -> school_holiday_countries.code`
|
||||
* included (`code` IS that table's primary key, so the generator's default
|
||||
* assumption was already correct there — unchanged by Rule 11, confirmed
|
||||
* by this same test). The deleteRule half stays skipped — see
|
||||
* `PARITY-009b` immediately below for why and by how many rows.
|
||||
* by this same test). The deleteRule half is un-skipped by Plan 4 Task 7 —
|
||||
* see `PARITY-009b` immediately below.
|
||||
*
|
||||
* `school_holiday_regions.country` re-verified one more time at Task 7's
|
||||
* own HEAD, directly against the live migration DDL (not the entity, not
|
||||
* an earlier report): `Migration20200101033600_create_school_holiday_countries.ts`
|
||||
* declares `country TEXT NOT NULL REFERENCES school_holiday_countries(code)`
|
||||
* — a real, DB-enforced FK, confirmed here by this very test passing for
|
||||
* that column too (`SchoolHolidayRegions.countryRef`'s `.joinColumn('country')`,
|
||||
* Plan 2 Task 3). 3f's inventory was right; there is no schema gap here to
|
||||
* document for the user.
|
||||
*/
|
||||
it('PARITY-009: FK referenced columns agree with PRAGMA foreign_key_list', () => {
|
||||
const failures: string[] = [];
|
||||
@@ -350,36 +359,43 @@ describe('entity ↔ migrated-schema parity', () => {
|
||||
});
|
||||
|
||||
/**
|
||||
* The deleteRule half of the same check (Task 0, Step 3). Task 0 found
|
||||
* deleteRule disagreeing on 11 of 180 owning relations; Task 3's rewrite
|
||||
* fixed one of those eleven as a side effect (`school_holiday_regions.
|
||||
* country` now has a real, explicit `joinColumn`, so it reports a genuine
|
||||
* FK row) and re-ran this check against the other 124 rewritten/added
|
||||
* entities, composite-PK relations included (`packing_item_contributors`,
|
||||
* `vacay_user_settings`, `document_connections`, `trip_document_links` —
|
||||
* their `.entity.ts` source never spells `.deleteRule(...)` on those
|
||||
* relations, but MikroORM discovery still resolves the correct rule at
|
||||
* runtime, confirmed by running this check, so they are not drift): the
|
||||
* count was 10. Plan 3b interlude A's `RULE11_referencedColumns` only
|
||||
* touches the referenced COLUMN, never `deleteRule` (`OauthTokens.client`/
|
||||
* `OauthConsents.client` already agreed on `deleteRule('cascade')` —
|
||||
* unaffected either way), so re-running this exact query still finds
|
||||
* **10** disagreements, unchanged: `trip_members.invited_by` entity=set
|
||||
* null vs db=no action, `roadtrip_day_tracks.day_id` entity=set null vs
|
||||
* db=cascade, `reservation_travelers.reservation_id`/`.user_id` and
|
||||
* `assignment_participants.assignment_id`/`.user_id` entity=no action vs
|
||||
* db=cascade, `oauth_tokens.parent_token_id` entity=set null vs db=no
|
||||
* action, `journey_contributors.user_id` entity=cascade vs db=no action,
|
||||
* `budget_settlements.created_by_user_id`/`budget_items.paid_by_user_id`
|
||||
* entity=set null vs db=no action. None of these are in this task's scope
|
||||
* (or Task 0's/Task 3's); an allow-list would need 10 one-line entries for
|
||||
* drift no task so far was asked to fix. Index-name parity looked clean on
|
||||
* inspection but was not exhaustively checked either, and stays out of
|
||||
* scope here too. Left skipped, with the count pinned above (Plan 4 owns
|
||||
* turning this into either fixes or a real allow-list — the number to
|
||||
* reconcile against is exactly 10, not "some").
|
||||
* The deleteRule half of the same check (Task 0, Step 3), un-skipped by
|
||||
* Plan 4 Task 7. Task 0 found deleteRule disagreeing on 11 of 180 owning
|
||||
* relations; Task 3's rewrite fixed one of those eleven as a side effect
|
||||
* (`school_holiday_regions.country`) and re-measured the other 124
|
||||
* rewritten/added entities at 10. Task 7 re-measured again at its own HEAD
|
||||
* (program rule: re-measure, never assume a prior count) and found the
|
||||
* same 10, unchanged: `trip_members.invited_by`, `oauth_tokens.
|
||||
* parent_token_id`, `budget_settlements.created_by_user_id`, `budget_items.
|
||||
* paid_by_user_id` (a nullable relation whose column has no `ON DELETE` at
|
||||
* all in its migration — SQLite ⇒ `no action` — while the entity's bare
|
||||
* `.nullable()` defaults to MikroORM's own `'set null'`), `roadtrip_day_
|
||||
* tracks.day_id` (nullable AND the table's sole PK — migration says
|
||||
* `ON DELETE CASCADE`, entity defaulted to `'set null'` the same way),
|
||||
* `journey_contributors.user_id` (a composite-PK member whose migration
|
||||
* has no `ON DELETE`, while the entity's FK-as-PK shape defaults to
|
||||
* `'cascade'`), and `reservation_travelers.reservation_id`/`.user_id` +
|
||||
* `assignment_participants.assignment_id`/`.user_id` (migrations say
|
||||
* `ON DELETE CASCADE`; the entity had no default at all for that shape,
|
||||
* landing on `'no action'`).
|
||||
*
|
||||
* All 10 are the ENTITY side never having matched its migration's physical
|
||||
* FK to begin with (never a migration defect — program rule: migrations
|
||||
* are append-only, and every one of the 10 migrations above is verified
|
||||
* correct against its own table's real production behavior). Fixed via a
|
||||
* new generator rule, `RULE13_pinDeleteRuleDrift` (`scripts/generate-
|
||||
* entities.ts`), which pins an explicit `.deleteRule(...)` wherever
|
||||
* leaving the relation implicit would produce the wrong effective rule —
|
||||
* see that rule's own doc comment for the full mechanism, including one
|
||||
* more wrinkle it found empirically: `DawarichConnections.user`,
|
||||
* `PlaceRegions.place` and `VacayUserSettings.user` share the exact same
|
||||
* "nullable AND sole FK-as-PK" shape as `roadtrip_day_tracks.day_id` but
|
||||
* were NOT among the 10 — their runtime default already happens to land on
|
||||
* `'cascade'` (matching physical) purely because of `ALL_ENTITIES`
|
||||
* discovery order, not because the shape is safe. `RULE13` pins all four
|
||||
* explicitly rather than trust that order to hold.
|
||||
*/
|
||||
it.skip('PARITY-009b (report-only, deferred to Plan 4): FK deleteRule agrees — 10 known disagreements, see doc comment', () => {
|
||||
it('PARITY-009b: FK deleteRule agrees with PRAGMA foreign_key_list', () => {
|
||||
const failures: string[] = [];
|
||||
for (const meta of tableBackedMetas()) {
|
||||
const table = meta.tableName;
|
||||
@@ -409,6 +425,56 @@ describe('entity ↔ migrated-schema parity', () => {
|
||||
expect(failures).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* The index-name half of the same PRAGMA-allow-listed check (Task 0, Step
|
||||
* 3) — per 3a's own ledger note, "PARITY-009's index half was never
|
||||
* implemented (only deleteRule)." Task 7 writes it fresh.
|
||||
*
|
||||
* `PRAGMA index_list(<table>)` names every index SQLite actually built —
|
||||
* a plain `CREATE INDEX <name> ON ...` or `CREATE UNIQUE INDEX <name>
|
||||
* ON ...`, one row each — MINUS the unnamed `sqlite_autoindex_<table>_<n>`
|
||||
* entries a bare `PRIMARY KEY`/inline `UNIQUE(...)` constraint gets for
|
||||
* free (excluded the same way `collectImplicitUniqueIndexes` in
|
||||
* `scripts/generate-entities.ts` excludes them — those are RULE9's
|
||||
* `uniques:` territory, a DIFFERENT, already-covered check, not a named
|
||||
* index this one can compare against). The entity side collects every
|
||||
* NAMED index a `defineEntity` call can declare: a property-level
|
||||
* `.index('name')`/`.unique('name')` (`prop.index`/`prop.unique`, a string
|
||||
* when named — the codebase never uses the unnamed `boolean` form, see the
|
||||
* `grep` a future author can re-run: `grep -rn '\.index()\|\.unique()' src/db/entities/*.entity.ts`
|
||||
* finds nothing), and every entity-level `indexes:`/`uniques:` block's own
|
||||
* `.name` (composite indexes, e.g. `Notifications.idx_notifications_target_scope`).
|
||||
* The two sets must be equal — a name in one and not the other means
|
||||
* either a migration added/renamed/dropped an index without the entity
|
||||
* following, or the reverse. Measured clean on today's schema (the
|
||||
* generator's own introspection already names every entity index after
|
||||
* its real DB index, RULE9's unnamed `uniques:` entries excluded by
|
||||
* construction from both sides) — this is a drift GUARD for a future
|
||||
* migration, not a fix for a found gap.
|
||||
*/
|
||||
it('PARITY-009c: index names agree with PRAGMA index_list', () => {
|
||||
const failures: string[] = [];
|
||||
for (const meta of tableBackedMetas()) {
|
||||
const table = meta.tableName;
|
||||
const dbIndexRows = testDb.prepare(`PRAGMA index_list("${table}")`).all() as { name: string; unique: number }[];
|
||||
const dbNames = new Set(dbIndexRows.filter((row) => !row.name.startsWith('sqlite_autoindex_')).map((row) => row.name));
|
||||
const entityNames = new Set<string>();
|
||||
for (const prop of meta.props) {
|
||||
if (typeof prop.index === 'string') entityNames.add(prop.index);
|
||||
if (typeof prop.unique === 'string') entityNames.add(prop.unique);
|
||||
}
|
||||
for (const idx of meta.indexes) if (idx.name) entityNames.add(idx.name);
|
||||
for (const uniq of meta.uniques) if (uniq.name) entityNames.add(uniq.name);
|
||||
for (const name of dbNames) {
|
||||
if (!entityNames.has(name)) failures.push(`${table}: db index "${name}" has no matching entity index declaration`);
|
||||
}
|
||||
for (const name of entityNames) {
|
||||
if (!dbNames.has(name)) failures.push(`${table}: entity declares index "${name}" with no matching db index`);
|
||||
}
|
||||
}
|
||||
expect(failures).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* D1's own rule (the naming strategy derives the same column from a
|
||||
* camelCase or a snake_case property, so a column-level check can never
|
||||
|
||||
@@ -11,6 +11,8 @@ import {
|
||||
assertFilesGenerated,
|
||||
BOOLEAN_COLUMNS,
|
||||
CheckExpressionFixup,
|
||||
collectForeignKeyDeleteRules,
|
||||
DeleteRuleFixup,
|
||||
JoinColumnFixup,
|
||||
JsonColumnFixup,
|
||||
KNOWN_DIFFS,
|
||||
@@ -31,6 +33,7 @@ import {
|
||||
RULE10_repositoryTypeMarker,
|
||||
RULE11_referencedColumns,
|
||||
RULE12_fixGarbledCheckExpressions,
|
||||
RULE13_pinDeleteRuleDrift,
|
||||
RULE_normalizeLiteralDefaults,
|
||||
applyTextPasses,
|
||||
checkEntities,
|
||||
@@ -548,6 +551,137 @@ describe('RULE12_fixGarbledCheckExpressions', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('RULE13_pinDeleteRuleDrift', () => {
|
||||
it('RULE13-001: a plain relation (not nullable, not part of an FK-as-PK) whose physical rule is "cascade" gets pinned — no implicit default covers this shape (the real reservation_travelers/assignment_participants case)', () => {
|
||||
const id = fixtureProp({ name: 'id', primary: true, autoincrement: true });
|
||||
const rel = fixtureProp({ name: 'reservation', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['reservation_id'] });
|
||||
const meta = fixtureMeta('ReservationTravelers', 'reservation_travelers', [id, rel]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['reservation_travelers', new Map([['reservation_id', 'cascade']])]]));
|
||||
expect(fixups).toEqual<DeleteRuleFixup[]>([
|
||||
{ className: 'ReservationTravelers', propName: 'reservation', from: undefined, to: 'cascade' },
|
||||
]);
|
||||
expect(rel.deleteRule).toBe('cascade');
|
||||
});
|
||||
|
||||
it('RULE13-002: a nullable relation whose physical rule is "no action" (no ON DELETE at all in its migration) gets pinned — the implicit "set null" default would be wrong (the real trip_members.invited_by case)', () => {
|
||||
const rel = fixtureProp({ name: 'invitedByRef', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['invited_by'], nullable: true });
|
||||
const meta = fixtureMeta('TripMembers', 'trip_members', [rel]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['trip_members', new Map([['invited_by', 'no action']])]]));
|
||||
expect(fixups).toEqual<DeleteRuleFixup[]>([{ className: 'TripMembers', propName: 'invitedByRef', from: undefined, to: 'no action' }]);
|
||||
expect(rel.deleteRule).toBe('no action');
|
||||
});
|
||||
|
||||
it('RULE13-003: a plain relation whose physical rule already agrees with the "no action" fallback is left untouched (the common case — no noise)', () => {
|
||||
const rel = fixtureProp({ name: 'trip', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['trip_id'] });
|
||||
const meta = fixtureMeta('X', 'x', [rel]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['x', new Map([['trip_id', 'no action']])]]));
|
||||
expect(fixups).toEqual([]);
|
||||
expect(rel.deleteRule).toBeUndefined();
|
||||
});
|
||||
|
||||
it('RULE13-004: a composite-PK member (every PK on the entity is itself a relation) whose physical rule is "no action" gets pinned — the implicit "cascade" default for FK-as-PK would be wrong (the real journey_contributors.user_id case)', () => {
|
||||
const journey = fixtureProp({ name: 'journey', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['journey_id'], primary: true });
|
||||
const user = fixtureProp({ name: 'user', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['user_id'], primary: true });
|
||||
const meta = fixtureMeta('JourneyContributors', 'journey_contributors', [journey, user]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift(
|
||||
[meta],
|
||||
new Map([
|
||||
[
|
||||
'journey_contributors',
|
||||
new Map([
|
||||
['journey_id', 'cascade'],
|
||||
['user_id', 'no action'],
|
||||
]),
|
||||
],
|
||||
]),
|
||||
);
|
||||
expect(fixups).toEqual<DeleteRuleFixup[]>([{ className: 'JourneyContributors', propName: 'user', from: undefined, to: 'no action' }]);
|
||||
expect(journey.deleteRule).toBeUndefined(); // already correct via the implicit cascade default — left alone
|
||||
expect(user.deleteRule).toBe('no action');
|
||||
});
|
||||
|
||||
it('RULE13-005: a relation that is BOTH nullable AND its entity\'s sole FK-as-PK is always pinned explicitly, even when the implicit default happens to already agree with physical (the DawarichConnections/PlaceRegions/VacayUserSettings wrinkle — order-dependent at runtime, never trusted)', () => {
|
||||
const rel = fixtureProp({ name: 'user', kind: ReferenceKind.ONE_TO_ONE, owner: true, fieldNames: ['user_id'], primary: true, nullable: true });
|
||||
const meta = fixtureMeta('DawarichConnections', 'dawarich_connections', [rel]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['dawarich_connections', new Map([['user_id', 'cascade']])]]));
|
||||
expect(fixups).toEqual<DeleteRuleFixup[]>([{ className: 'DawarichConnections', propName: 'user', from: undefined, to: 'cascade' }]);
|
||||
expect(rel.deleteRule).toBe('cascade');
|
||||
});
|
||||
|
||||
it('RULE13-006: the same ambiguous nullable-FK-as-PK shape is left alone once it already carries the correct explicit deleteRule (idempotent — a second generator run produces no fixup)', () => {
|
||||
const rel = fixtureProp({
|
||||
name: 'day',
|
||||
kind: ReferenceKind.ONE_TO_ONE,
|
||||
owner: true,
|
||||
fieldNames: ['day_id'],
|
||||
primary: true,
|
||||
nullable: true,
|
||||
deleteRule: 'cascade',
|
||||
});
|
||||
const meta = fixtureMeta('RoadtripDayTracks', 'roadtrip_day_tracks', [rel]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['roadtrip_day_tracks', new Map([['day_id', 'cascade']])]]));
|
||||
expect(fixups).toEqual([]);
|
||||
expect(rel.deleteRule).toBe('cascade');
|
||||
});
|
||||
|
||||
it('RULE13-007: a table absent from the fkDeleteRules map (no FK columns queried, or a routine/enum entity) is left untouched, never thrown on', () => {
|
||||
const rel = fixtureProp({ name: 'trip', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['trip_id'] });
|
||||
const meta = fixtureMeta('X', 'x', [rel]);
|
||||
expect(() => RULE13_pinDeleteRuleDrift([meta], new Map())).not.toThrow();
|
||||
expect(RULE13_pinDeleteRuleDrift([meta], new Map())).toEqual([]);
|
||||
});
|
||||
|
||||
it('RULE13-008: a column the map has no entry for (no db FK on it at all) is skipped — PARITY-009 proper\'s job, not this rule\'s', () => {
|
||||
const rel = fixtureProp({ name: 'trip', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['trip_id'] });
|
||||
const meta = fixtureMeta('X', 'x', [rel]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['x', new Map([['other_id', 'cascade']])]]));
|
||||
expect(fixups).toEqual([]);
|
||||
expect(rel.deleteRule).toBeUndefined();
|
||||
});
|
||||
|
||||
it('RULE13-009: an inverse relation (mappedBy set, not owning) is never a candidate, even when its column name collides with a physical FK entry', () => {
|
||||
const inverse = fixtureProp({ name: 'trips_collection', kind: ReferenceKind.ONE_TO_MANY, mappedBy: 'x', fieldNames: ['trip_id'] });
|
||||
const meta = fixtureMeta('X', 'x', [inverse]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['x', new Map([['trip_id', 'cascade']])]]));
|
||||
expect(fixups).toEqual([]);
|
||||
});
|
||||
|
||||
it('RULE13-010: a multi-column owning relation (fieldNames.length !== 1) is never a candidate', () => {
|
||||
const rel = fixtureProp({ name: 'composite', kind: ReferenceKind.MANY_TO_ONE, fieldNames: ['a', 'b'] });
|
||||
const meta = fixtureMeta('X', 'x', [rel]);
|
||||
const fixups = RULE13_pinDeleteRuleDrift([meta], new Map([['x', new Map([['a', 'cascade']])]]));
|
||||
expect(fixups).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('collectForeignKeyDeleteRules', () => {
|
||||
it('COLLECTFKDR-001: reads PRAGMA foreign_key_list(table).on_delete per column, lowercased, against the real migrated schema', async () => {
|
||||
const db = createSnapshotTestDb();
|
||||
const t = await createTestOrm(db);
|
||||
try {
|
||||
const result = await collectForeignKeyDeleteRules(t.orm.em.getConnection(), ['trip_members', 'reservation_travelers']);
|
||||
expect(result.get('trip_members')?.get('trip_id')).toBe('cascade');
|
||||
expect(result.get('trip_members')?.get('invited_by')).toBe('no action');
|
||||
expect(result.get('reservation_travelers')?.get('reservation_id')).toBe('cascade');
|
||||
} finally {
|
||||
await t.close();
|
||||
db.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('COLLECTFKDR-002: a table with no foreign keys at all is absent from the result map, not present with an empty one', async () => {
|
||||
const db = createSnapshotTestDb();
|
||||
const t = await createTestOrm(db);
|
||||
try {
|
||||
const result = await collectForeignKeyDeleteRules(t.orm.em.getConnection(), ['users']);
|
||||
expect(result.has('users')).toBe(false);
|
||||
} finally {
|
||||
await t.close();
|
||||
db.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('RULE9_addImplicitUniqueConstraints', () => {
|
||||
it('RULE9-001: a table with a matching implicit-unique entry gets a uniques: block referencing the columns as properties — a plain (non-FK) column keeps its own name', () => {
|
||||
const userId = fixtureProp({ name: 'user_id', primary: false });
|
||||
@@ -979,6 +1113,7 @@ describe('applyTextPasses', () => {
|
||||
repositoryMarkers: [{ className: 'X', repositoryClassName: 'XRepository' }],
|
||||
referencedColumns: [{ className: 'X', propName: 'countryRef', referencedColumnNames: ['code'] }],
|
||||
checkExpressions: [],
|
||||
deleteRules: [],
|
||||
});
|
||||
expect(out).toContain(".joinColumn('country')");
|
||||
expect(out).toContain(".referencedColumnNames('code')");
|
||||
|
||||
Reference in New Issue
Block a user