Skip to content

Commit 7c8eec1

Browse files
committed
fix: address code review feedback on codeManager
isCodeRedeemedByUser: add qualifying status filter (Success / Already Redeemed / Code Expired) so transient failures like Invalid Parameters do not permanently block retries. addNewPendingCodes: replace pre-read + N+1 loop with a single bulk INSERT ... ON CONFLICT DO NOTHING RETURNING to avoid concurrency races and excess round-trips. getRedeemedCodesByUsers: rebase the codes.length guard on the combined parameter budget (999 - codes - 3 statuses) and chunk discordIds to stay within SQLite's 999-variable limit for large fan-outs. Signed-off-by: Michael Cramer <michael@bigmichi1.de>
1 parent a43c4d5 commit 7c8eec1

1 file changed

Lines changed: 40 additions & 28 deletions

File tree

src/bot/database/codeManager.ts

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,17 @@ class CodeManager {
117117
}
118118

119119
async isCodeRedeemedByUser(code: string, discordId: string): Promise<boolean> {
120+
const qualifyingStatuses = ['Success', 'Already Redeemed', 'Code Expired'] as const;
120121
const result = db
121122
.select({ code: redeemedCodes.code })
122123
.from(redeemedCodes)
123-
.where(and(eq(redeemedCodes.code, code), eq(redeemedCodes.discordId, discordId)))
124+
.where(
125+
and(
126+
eq(redeemedCodes.code, code),
127+
eq(redeemedCodes.discordId, discordId),
128+
inArray(redeemedCodes.status, qualifyingStatuses as unknown as string[])
129+
)
130+
)
124131
.get();
125132
return result !== undefined;
126133
}
@@ -271,20 +278,18 @@ class CodeManager {
271278
/**
272279
* Bulk-insert pending codes, skipping any that are already present.
273280
* Returns only the codes that were newly inserted.
281+
* Uses a single INSERT … ON CONFLICT DO NOTHING RETURNING to avoid N+1
282+
* queries and race conditions between pre-read and insert.
274283
*/
275284
async addNewPendingCodes(codes: string[]): Promise<string[]> {
276285
if (codes.length === 0) return [];
277-
const existing = new Set(
278-
db.select({ code: pendingCodes.code }).from(pendingCodes)
279-
.where(inArray(pendingCodes.code, codes))
280-
.all()
281-
.map((r) => r.code)
282-
);
283-
const fresh = codes.filter((c) => !existing.has(c));
284-
for (const code of fresh) {
285-
db.insert(pendingCodes).values({ code, discordId: null }).onConflictDoNothing().run();
286-
}
287-
return fresh;
286+
const inserted = db
287+
.insert(pendingCodes)
288+
.values(codes.map((code) => ({ code, discordId: null })))
289+
.onConflictDoNothing()
290+
.returning({ code: pendingCodes.code })
291+
.all();
292+
return inserted.map((r) => r.code);
288293
}
289294

290295
async deleteUserRedeemedCodes(discordId: string): Promise<number> {
@@ -301,34 +306,41 @@ class CodeManager {
301306
* discordId → Set<code> containing codes that user has any qualifying
302307
* redemption record for (Success, Already Redeemed, Code Expired).
303308
*
304-
* Throws if codes.length exceeds the safe SQLite parameter limit.
309+
* Throws if codes.length would exceed the safe SQLite parameter budget.
310+
* Chunks discordIds automatically to stay within the 999-variable limit.
305311
*/
306312
async getRedeemedCodesByUsers(
307313
codes: string[],
308314
discordIds: string[]
309315
): Promise<Map<string, Set<string>>> {
310-
if (codes.length > 995) {
316+
const SQLITE_PARAM_LIMIT = 999;
317+
const STATUS_COUNT = 3; // 'Success', 'Already Redeemed', 'Code Expired'
318+
// codes + status params are fixed per query; remaining budget is for discordIds
319+
const discordIdChunkSize = SQLITE_PARAM_LIMIT - codes.length - STATUS_COUNT;
320+
if (discordIdChunkSize <= 0) {
311321
throw new Error('Too many codes provided to getRedeemedCodesByUsers');
312322
}
313323
const result = new Map<string, Set<string>>();
314324
if (codes.length === 0 || discordIds.length === 0) return result;
315325

316326
const qualifyingStatuses = ['Success', 'Already Redeemed', 'Code Expired'] as const;
317-
const rows = db
318-
.select({ code: redeemedCodes.code, discordId: redeemedCodes.discordId })
319-
.from(redeemedCodes)
320-
.where(
321-
and(
322-
inArray(redeemedCodes.code, codes),
323-
inArray(redeemedCodes.discordId, discordIds),
324-
inArray(redeemedCodes.status, qualifyingStatuses as unknown as string[])
327+
for (let i = 0; i < discordIds.length; i += discordIdChunkSize) {
328+
const chunk = discordIds.slice(i, i + discordIdChunkSize);
329+
const rows = db
330+
.select({ code: redeemedCodes.code, discordId: redeemedCodes.discordId })
331+
.from(redeemedCodes)
332+
.where(
333+
and(
334+
inArray(redeemedCodes.code, codes),
335+
inArray(redeemedCodes.discordId, chunk),
336+
inArray(redeemedCodes.status, qualifyingStatuses as unknown as string[])
337+
)
325338
)
326-
)
327-
.all();
328-
329-
for (const row of rows) {
330-
if (!result.has(row.discordId)) result.set(row.discordId, new Set());
331-
result.get(row.discordId)!.add(row.code);
339+
.all();
340+
for (const row of rows) {
341+
if (!result.has(row.discordId)) result.set(row.discordId, new Set());
342+
result.get(row.discordId)!.add(row.code);
343+
}
332344
}
333345
return result;
334346
}

0 commit comments

Comments
 (0)