Skip to content

Commit 78f99ba

Browse files
simeng-ligao-sun
andauthored
refactor(core): refactor findPayloadByPayloadField query (#7650)
* refactor(core): refactor findPayloadByPayloadField query refactor findPayloadByPayloadField query * chore: update comments Co-authored-by: Gao Sun <[email protected]> --------- Co-authored-by: Gao Sun <[email protected]>
1 parent f58ec73 commit 78f99ba

File tree

2 files changed

+63
-15
lines changed

2 files changed

+63
-15
lines changed

packages/core/src/queries/oidc-model-instance.test.ts

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ describe('oidc-model-instance query', () => {
4040
payload: JSON.stringify(instance.payload),
4141
};
4242

43+
beforeEach(() => {
44+
jest.clearAllMocks();
45+
});
46+
4347
it('upsertInstance', async () => {
4448
const expectSql = sql`
4549
insert into ${table} ("model_name", "id", "payload", "expires_at")
@@ -83,21 +87,13 @@ describe('oidc-model-instance query', () => {
8387
});
8488
});
8589

86-
it('findPayloadByPayloadField', async () => {
90+
it('findPayloadByPayloadField - single result', async () => {
8791
const uid_key = 'uid';
8892
const uid_value = 'foo';
8993

90-
const expectSql = sql`
91-
select ${fields.payload}, ${fields.consumedAt}
92-
from ${table}
93-
where ${fields.modelName}=$1
94-
and ${fields.payload}->>$2=$3
95-
`;
96-
94+
// Mock a single result
9795
mockQuery.mockImplementationOnce(async (sql, values) => {
98-
expectSqlAssert(sql, expectSql.sql);
99-
expect(values).toEqual([instance.modelName, uid_key, uid_value]);
100-
96+
// Simulate pool.any
10197
return createMockQueryResult([{ consumedAt: 10 }]);
10298
});
10399

@@ -108,6 +104,44 @@ describe('oidc-model-instance query', () => {
108104
});
109105
});
110106

107+
it('findPayloadByPayloadField - multiple results (should delete and return null)', async () => {
108+
const uid_key = 'uid';
109+
const uid_value = 'foo';
110+
111+
// Mock multiple results
112+
mockQuery
113+
.mockImplementationOnce(async () => {
114+
// Simulate pool.any
115+
return createMockQueryResult([{ uid: uid_value }, { uid: uid_value }]);
116+
})
117+
// @ts-expect-error - mock delete query
118+
.mockImplementationOnce(async () => {
119+
// Simulate delete query
120+
return { rowCount: 2 };
121+
});
122+
123+
await expect(
124+
findPayloadByPayloadField(instance.modelName, uid_key, uid_value)
125+
).resolves.toBeUndefined();
126+
127+
expect(mockQuery).toHaveBeenCalledTimes(2);
128+
});
129+
130+
it('findPayloadByPayloadField - no result', async () => {
131+
const uid_key = 'uid';
132+
const uid_value = 'foo';
133+
134+
// Mock no result
135+
mockQuery.mockImplementationOnce(async () => {
136+
// Simulate pool.any
137+
return createMockQueryResult([]);
138+
});
139+
140+
await expect(
141+
findPayloadByPayloadField(instance.modelName, uid_key, uid_value)
142+
).resolves.toBeUndefined();
143+
});
144+
111145
it('consumeInstanceById', async () => {
112146
jest.useFakeTimers().setSystemTime(100_000);
113147

packages/core/src/queries/oidc-model-instance.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,26 @@ export const createOidcModelInstanceQueries = (pool: CommonQueryMethods) => {
7979
field: Field,
8080
value: T
8181
) => {
82-
const result = await pool.maybeOne<QueryResult>(sql`
83-
${findByModel(modelName)}
84-
and ${fields.payload}->>${field}=${value}
82+
// Fetch all matching records
83+
const results = await pool.any<QueryResult>(sql`
84+
${findByModel(modelName)}
85+
and ${fields.payload}->>${field}=${value}
86+
`);
87+
88+
// Rarely, duplicate UIDs can exist for different sessions.
89+
// This query may throw `DataIntegrityError`.
90+
// If that happens, delete all duplicates and return `null`.
91+
if (results.length > 1) {
92+
// Delete all duplicates
93+
await pool.query(sql`
94+
delete from ${sql.identifier([modelName])}
95+
where ${fields.payload}->>${field}=${value}
8596
`);
97+
return;
98+
}
8699

87-
return convertResult(result, modelName);
100+
// If there is only one record, return the result.
101+
return results[0] ? convertResult(results[0], modelName) : undefined;
88102
};
89103

90104
const consumeInstanceById = async (modelName: string, id: string) => {

0 commit comments

Comments
 (0)