Skip to content

Commit 4465ddb

Browse files
chasprowebdevchasprowebdev
committed
fix(api): discover member foreign keys from Postgres catalog instead of a hand-maintained list
1 parent 128d9d1 commit 4465ddb

8 files changed

Lines changed: 878 additions & 417 deletions

‎apps/api/src/trigger/tasks/people/merge-duplicate-user-db-mock.util.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,17 @@ export function createDbProxyMock(): Record<string, unknown> {
4040
const models: Record<string, ModelMock> = {};
4141
let dbProxy: Record<string, unknown>;
4242
const dbMock = {
43-
$transaction: jest.fn(
44-
async (fn: (tx: unknown) => Promise<unknown>) => fn(dbProxy),
43+
$transaction: jest.fn(async (fn: (tx: unknown) => Promise<unknown>) =>
44+
fn(dbProxy),
4545
),
46+
// Raw-query methods (used for catalog-driven FK discovery — see
47+
// merge-duplicate-user-fk-discovery.ts) are explicit callable stubs
48+
// rather than falling into the generic "unknown prop -> model mock"
49+
// branch below, which would produce a non-callable object instead of a
50+
// jest.fn(). Default to harmless empty results; tests that need
51+
// specific rows override these directly.
52+
$queryRaw: jest.fn().mockResolvedValue([]),
53+
$executeRaw: jest.fn().mockResolvedValue(0),
4654
};
4755
dbProxy = new Proxy(dbMock, {
4856
get: (target, prop) => {

‎apps/api/src/trigger/tasks/people/merge-duplicate-user-fk-discovery.spec.ts‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Prisma.sql/Prisma.raw are pure template-string builders — safe to use the
2+
// real ones from @prisma/client without a DB connection. Mocked here only so
3+
// importing '@db' (which re-exports the whole @prisma/client package plus
4+
// the lazily-connecting `db` proxy) doesn't risk touching anything else.
5+
jest.mock('@db', () => {
6+
const { Prisma } = require('@prisma/client');
7+
return { Prisma };
8+
});
9+
10+
import {
11+
assertNoDanglingMemberReferences,
12+
findMemberForeignKeys,
13+
quoteIdentifier,
14+
repointGenericForeignKeys,
15+
} from './merge-duplicate-user-fk-discovery';
16+
17+
function createFakeTx() {
18+
return {
19+
$queryRaw: jest.fn(),
20+
$executeRaw: jest.fn(),
21+
};
22+
}
23+
24+
describe('quoteIdentifier', () => {
25+
it('quotes a safe identifier', () => {
26+
expect(quoteIdentifier('Task')).toBe('"Task"');
27+
expect(quoteIdentifier('background_check_requests')).toBe(
28+
'"background_check_requests"',
29+
);
30+
});
31+
32+
it('rejects identifiers with characters outside [A-Za-z0-9_]', () => {
33+
expect(() => quoteIdentifier('Task"; DROP TABLE "Task')).toThrow(
34+
/Refusing to use unexpected SQL identifier/,
35+
);
36+
expect(() => quoteIdentifier('some table')).toThrow();
37+
expect(() => quoteIdentifier('')).toThrow();
38+
});
39+
});
40+
41+
describe('findMemberForeignKeys', () => {
42+
it('maps information_schema rows to tableName/columnName pairs', async () => {
43+
const tx = createFakeTx();
44+
tx.$queryRaw.mockResolvedValue([
45+
{ table_name: 'Task', column_name: 'assigneeId' },
46+
{ table_name: 'background_check_requests', column_name: 'memberId' },
47+
]);
48+
49+
const result = await findMemberForeignKeys(tx as never);
50+
51+
expect(result).toEqual([
52+
{ tableName: 'Task', columnName: 'assigneeId' },
53+
{ tableName: 'background_check_requests', columnName: 'memberId' },
54+
]);
55+
expect(tx.$queryRaw).toHaveBeenCalledTimes(1);
56+
});
57+
58+
it('returns an empty list when no foreign keys are found', async () => {
59+
const tx = createFakeTx();
60+
tx.$queryRaw.mockResolvedValue([]);
61+
62+
expect(await findMemberForeignKeys(tx as never)).toEqual([]);
63+
});
64+
});
65+
66+
describe('repointGenericForeignKeys', () => {
67+
it('runs an UPDATE for each foreign key not in skipTables', async () => {
68+
const tx = createFakeTx();
69+
tx.$executeRaw.mockResolvedValue(1);
70+
const foreignKeys = [
71+
{ tableName: 'Task', columnName: 'assigneeId' },
72+
{ tableName: 'Risk', columnName: 'assigneeId' },
73+
];
74+
75+
const repointed = await repointGenericForeignKeys(
76+
tx as never,
77+
foreignKeys,
78+
new Set(),
79+
'mem_old',
80+
'mem_new',
81+
);
82+
83+
expect(tx.$executeRaw).toHaveBeenCalledTimes(2);
84+
expect(repointed).toEqual(foreignKeys);
85+
});
86+
87+
it('skips tables in skipTables entirely', async () => {
88+
const tx = createFakeTx();
89+
tx.$executeRaw.mockResolvedValue(1);
90+
const foreignKeys = [
91+
{ tableName: 'Task', columnName: 'assigneeId' },
92+
{ tableName: 'background_check_requests', columnName: 'memberId' },
93+
];
94+
95+
const repointed = await repointGenericForeignKeys(
96+
tx as never,
97+
foreignKeys,
98+
new Set(['background_check_requests']),
99+
'mem_old',
100+
'mem_new',
101+
);
102+
103+
expect(tx.$executeRaw).toHaveBeenCalledTimes(1);
104+
expect(repointed).toEqual([
105+
{ tableName: 'Task', columnName: 'assigneeId' },
106+
]);
107+
});
108+
109+
it('returns an empty list and issues no updates when given no foreign keys', async () => {
110+
const tx = createFakeTx();
111+
112+
const repointed = await repointGenericForeignKeys(
113+
tx as never,
114+
[],
115+
new Set(),
116+
'mem_old',
117+
'mem_new',
118+
);
119+
120+
expect(tx.$executeRaw).not.toHaveBeenCalled();
121+
expect(repointed).toEqual([]);
122+
});
123+
});
124+
125+
describe('assertNoDanglingMemberReferences', () => {
126+
it('resolves without throwing when nothing still references the old member', async () => {
127+
const tx = createFakeTx();
128+
tx.$queryRaw.mockResolvedValue([{ exists: false }]);
129+
130+
await expect(
131+
assertNoDanglingMemberReferences(
132+
tx as never,
133+
[{ tableName: 'Task', columnName: 'assigneeId' }],
134+
'mem_old',
135+
{
136+
'Extra.check': () => Promise.resolve(0),
137+
},
138+
),
139+
).resolves.toBeUndefined();
140+
});
141+
142+
it('throws naming the table.column still pointing at the old member', async () => {
143+
const tx = createFakeTx();
144+
tx.$queryRaw.mockResolvedValue([{ exists: true }]);
145+
146+
await expect(
147+
assertNoDanglingMemberReferences(
148+
tx as never,
149+
[{ tableName: 'Task', columnName: 'assigneeId' }],
150+
'mem_old',
151+
{},
152+
),
153+
).rejects.toThrow('Task.assigneeId');
154+
});
155+
156+
it('throws naming an extra (non-FK) check that still has dangling rows', async () => {
157+
const tx = createFakeTx();
158+
tx.$queryRaw.mockResolvedValue([{ exists: false }]);
159+
160+
await expect(
161+
assertNoDanglingMemberReferences(tx as never, [], 'mem_old', {
162+
'Policy.signedBy': () => Promise.resolve(2),
163+
}),
164+
).rejects.toThrow('Policy.signedBy');
165+
});
166+
167+
it('aggregates every dangling reference into a single error', async () => {
168+
const tx = createFakeTx();
169+
// Two foreign keys checked in order — both dangling.
170+
tx.$queryRaw
171+
.mockResolvedValueOnce([{ exists: true }])
172+
.mockResolvedValueOnce([{ exists: true }]);
173+
174+
await expect(
175+
assertNoDanglingMemberReferences(
176+
tx as never,
177+
[
178+
{ tableName: 'Task', columnName: 'assigneeId' },
179+
{ tableName: 'Risk', columnName: 'assigneeId' },
180+
],
181+
'mem_old',
182+
{ 'Policy.signedBy': () => Promise.resolve(1) },
183+
),
184+
).rejects.toThrow('Task.assigneeId, Risk.assigneeId, Policy.signedBy');
185+
});
186+
});

‎apps/api/src/trigger/tasks/people/merge-duplicate-user-fk-discovery.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { Prisma } from '@db';
2+
3+
/**
4+
* Reads foreign keys pointing at Member.id directly from Postgres's own
5+
* catalog, instead of a hand-maintained list — so a relation added later is
6+
* picked up automatically the next time the merge runs, rather than being
7+
* silently missed (which is what happened previously: a relation was added
8+
* while this merge task's PR was still open, the hand-written list never
9+
* learned about it, and that relation's row for the old member was lost
10+
* instead of re-pointed).
11+
*/
12+
13+
export interface MemberForeignKey {
14+
tableName: string;
15+
columnName: string;
16+
}
17+
18+
const SAFE_IDENTIFIER = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
19+
20+
export function quoteIdentifier(identifier: string): string {
21+
if (!SAFE_IDENTIFIER.test(identifier)) {
22+
throw new Error(
23+
`Refusing to use unexpected SQL identifier from information_schema: ${identifier}`,
24+
);
25+
}
26+
return `"${identifier}"`;
27+
}
28+
29+
/** Every foreign key in the current schema that references Member.id. */
30+
export async function findMemberForeignKeys(
31+
tx: Prisma.TransactionClient,
32+
): Promise<MemberForeignKey[]> {
33+
const rows = await tx.$queryRaw<
34+
Array<{ table_name: string; column_name: string }>
35+
>`
36+
SELECT tc.table_name, kcu.column_name
37+
FROM information_schema.table_constraints tc
38+
JOIN information_schema.key_column_usage kcu
39+
ON tc.constraint_name = kcu.constraint_name
40+
AND tc.table_schema = kcu.table_schema
41+
JOIN information_schema.constraint_column_usage ccu
42+
ON tc.constraint_name = ccu.constraint_name
43+
AND tc.table_schema = ccu.table_schema
44+
WHERE tc.constraint_type = 'FOREIGN KEY'
45+
AND tc.table_schema = current_schema()
46+
AND ccu.table_name = 'Member'
47+
AND ccu.column_name = 'id'
48+
`;
49+
return rows.map((row) => ({
50+
tableName: row.table_name,
51+
columnName: row.column_name,
52+
}));
53+
}
54+
55+
/**
56+
* Re-points every discovered foreign key from `oldMemberId` to
57+
* `newMemberId`, except `skipTables` — tables whose member-id column shares
58+
* a unique constraint with another field, where a blind `UPDATE` could
59+
* violate that constraint. Returns the ones actually updated, for logging.
60+
*/
61+
export async function repointGenericForeignKeys(
62+
tx: Prisma.TransactionClient,
63+
foreignKeys: MemberForeignKey[],
64+
skipTables: ReadonlySet<string>,
65+
oldMemberId: string,
66+
newMemberId: string,
67+
): Promise<MemberForeignKey[]> {
68+
const repointed: MemberForeignKey[] = [];
69+
for (const fk of foreignKeys) {
70+
if (skipTables.has(fk.tableName)) continue;
71+
const table = quoteIdentifier(fk.tableName);
72+
const column = quoteIdentifier(fk.columnName);
73+
await tx.$executeRaw(
74+
Prisma.sql`UPDATE ${Prisma.raw(table)} SET ${Prisma.raw(column)} = ${newMemberId} WHERE ${Prisma.raw(column)} = ${oldMemberId}`,
75+
);
76+
repointed.push(fk);
77+
}
78+
return repointed;
79+
}
80+
81+
/**
82+
* Throws if any discovered foreign key still has a row pointing at
83+
* `oldMemberId` — a safety net so an unhandled or buggy relation fails
84+
* loudly instead of silently losing data when the old member row is deleted
85+
* afterward (some relations cascade-delete or null out on that delete).
86+
* `extraChecks` covers member-id columns with no real foreign key (so
87+
* catalog introspection can never find them) — each returns the count of
88+
* rows still referencing `oldMemberId`.
89+
*/
90+
export async function assertNoDanglingMemberReferences(
91+
tx: Prisma.TransactionClient,
92+
foreignKeys: MemberForeignKey[],
93+
oldMemberId: string,
94+
extraChecks: Record<string, () => Promise<number>>,
95+
): Promise<void> {
96+
const dangling: string[] = [];
97+
98+
for (const fk of foreignKeys) {
99+
const table = quoteIdentifier(fk.tableName);
100+
const column = quoteIdentifier(fk.columnName);
101+
const rows = await tx.$queryRaw<Array<{ exists: boolean }>>(
102+
Prisma.sql`SELECT EXISTS(SELECT 1 FROM ${Prisma.raw(table)} WHERE ${Prisma.raw(column)} = ${oldMemberId}) AS exists`,
103+
);
104+
if (rows[0]?.exists) {
105+
dangling.push(`${fk.tableName}.${fk.columnName}`);
106+
}
107+
}
108+
109+
for (const [label, check] of Object.entries(extraChecks)) {
110+
if ((await check()) > 0) dangling.push(label);
111+
}
112+
113+
if (dangling.length > 0) {
114+
throw new Error(
115+
`Merge safety check failed: member ${oldMemberId} is still referenced by: ${dangling.join(', ')}`,
116+
);
117+
}
118+
}

0 commit comments

Comments
 (0)