Skip to content

Commit 1f7baae

Browse files
committed
fix(cards): make last-card deletion protection concurrency-safe
The DELETE /api/cards/:id handler performed an ownership lookup, a card count check, and the delete as three separate non-transactional operations. Under concurrent requests both could observe count > 1 and both proceed to delete, leaving the user with zero cards. Move the count guard, optional default-card promotion, and the delete into a single Prisma $transaction with Serializable isolation. The database now serializes concurrent count reads against the write, rolling back the second conflicting transaction so the invariant (user always retains at least one card) cannot be violated even under load. Adds a focused test suite covering normal deletion, last-card rejection, default-card promotion, and four concurrency-guard scenarios.
1 parent 4c7437f commit 1f7baae

2 files changed

Lines changed: 126 additions & 19 deletions

File tree

apps/backend/src/__tests__/cards.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,3 +438,97 @@ describe('PUT /api/cards/:id/default', () => {
438438
expect(mockPrisma.card.update).toHaveBeenCalled();
439439
});
440440
});
441+
442+
// ─────────────────────────────────────────────────────────────────────────────
443+
// DELETE /api/cards/:id — concurrency guard
444+
// ─────────────────────────────────────────────────────────────────────────────
445+
446+
describe('DELETE /api/cards/:id — concurrency guard', () => {
447+
beforeEach(() => {
448+
vi.clearAllMocks();
449+
wireTransaction();
450+
});
451+
452+
it('runs the count check and deletion inside a single transaction', async () => {
453+
// Verifies that $transaction wraps both the last-card guard and the delete,
454+
// closing the TOCTOU window where two concurrent requests could both see
455+
// count > 1 and both proceed to delete.
456+
mockPrisma.card.findFirst.mockResolvedValue({ ...mockCard, isDefault: false });
457+
mockPrisma.card.count.mockResolvedValue(2);
458+
mockPrisma.card.delete.mockResolvedValue(mockCard);
459+
460+
const app = await buildApp();
461+
await app.inject({ method: 'DELETE', url: `/api/cards/${CARD_ID}` });
462+
463+
expect(mockPrisma.$transaction).toHaveBeenCalledOnce();
464+
// Both the guard and the mutation run through the transaction client.
465+
expect(mockPrisma.card.count).toHaveBeenCalledWith({ where: { userId: USER_ID } });
466+
expect(mockPrisma.card.delete).toHaveBeenCalledWith({ where: { id: CARD_ID } });
467+
});
468+
469+
it('blocks a concurrent request whose transaction-internal count reflects a prior committed delete', async () => {
470+
// Simulates: another request committed its delete first so the count is
471+
// now 1 by the time this transaction's count query executes.
472+
// The last-card guard must fire and prevent deletion.
473+
mockPrisma.card.findFirst.mockResolvedValue({ ...mockCard, isDefault: false });
474+
mockPrisma.card.count.mockResolvedValue(1);
475+
476+
const app = await buildApp();
477+
const res = await app.inject({ method: 'DELETE', url: `/api/cards/${CARD_ID}` });
478+
479+
expect(res.statusCode).toBe(400);
480+
expect(res.json().error).toBe('Cannot delete the last remaining card. A user must have at least one card.');
481+
expect(mockPrisma.card.delete).not.toHaveBeenCalled();
482+
});
483+
484+
it('allows exactly one of two racing deletes (user has 2 cards) to succeed', async () => {
485+
// User starts with 2 cards. Two delete requests arrive.
486+
// The first request's transaction sees count=2 and commits.
487+
// The second request's transaction sees count=1 (after the first committed)
488+
// and is correctly rejected — the user is never left with zero cards.
489+
const CARD_B = 'card-xyz';
490+
const app = await buildApp();
491+
492+
// Request A: ownership passes, count inside tx = 2, delete succeeds
493+
mockPrisma.card.findFirst.mockResolvedValueOnce({ ...mockCard, isDefault: false });
494+
mockPrisma.card.count.mockResolvedValueOnce(2);
495+
mockPrisma.card.delete.mockResolvedValueOnce(mockCard);
496+
497+
// Request B: ownership passes, but count inside tx = 1 (A already committed)
498+
mockPrisma.card.findFirst.mockResolvedValueOnce({ ...mockCard, id: CARD_B, isDefault: false });
499+
mockPrisma.card.count.mockResolvedValueOnce(1);
500+
501+
const resA = await app.inject({ method: 'DELETE', url: `/api/cards/${CARD_ID}` });
502+
const resB = await app.inject({ method: 'DELETE', url: `/api/cards/${CARD_B}` });
503+
504+
expect(resA.statusCode).toBe(204); // first request succeeds
505+
expect(resB.statusCode).toBe(400); // second is blocked by the guard
506+
// Exactly one card deleted — user retains at least one.
507+
expect(mockPrisma.card.delete).toHaveBeenCalledOnce();
508+
});
509+
510+
it('user always retains at least one card regardless of which concurrent request wins', async () => {
511+
// Invariant assertion: one request must succeed and one must be blocked,
512+
// with the database having been written exactly once.
513+
const CARD_B = 'card-xyz';
514+
const app = await buildApp();
515+
516+
mockPrisma.card.findFirst
517+
.mockResolvedValueOnce({ ...mockCard, isDefault: false })
518+
.mockResolvedValueOnce({ ...mockCard, id: CARD_B, isDefault: false });
519+
mockPrisma.card.count
520+
.mockResolvedValueOnce(2) // Tx A's count inside the transaction
521+
.mockResolvedValueOnce(1); // Tx B's count inside the transaction (after A committed)
522+
mockPrisma.card.delete.mockResolvedValueOnce(mockCard);
523+
524+
const resA = await app.inject({ method: 'DELETE', url: `/api/cards/${CARD_ID}` });
525+
const resB = await app.inject({ method: 'DELETE', url: `/api/cards/${CARD_B}` });
526+
527+
const succeeded = [resA, resB].filter((r) => r.statusCode === 204).length;
528+
const blocked = [resA, resB].filter((r) => r.statusCode === 400).length;
529+
expect(succeeded).toBe(1);
530+
expect(blocked).toBe(1);
531+
// Exactly one delete reached the database — invariant preserved.
532+
expect(mockPrisma.card.delete).toHaveBeenCalledOnce();
533+
});
534+
});

apps/backend/src/routes/cards.ts

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -239,30 +239,43 @@ export async function cardRoutes(app: FastifyInstance): Promise<void> {
239239
return;
240240
}
241241

242-
// Prevent deleting the last card — every user must retain at least one.
243-
const userCardCount = await app.prisma.card.count({ where: { userId } });
244-
if (userCardCount <= 1) {
245-
reply.status(400).send({ error: 'Cannot delete the last remaining card. A user must have at least one card.' });
246-
return;
247-
}
248-
249-
// If the card being deleted is the default, promote the next-oldest card
250-
// before deletion so the user always has an active default.
251-
if (existing.isDefault) {
252-
const oldestRemainingCard = await app.prisma.card.findFirst({
253-
where: { userId, id: { not: id } },
254-
orderBy: { createdAt: 'asc' },
255-
});
242+
// The last-card check and the delete run inside a single serializable
243+
// transaction. Without this, two concurrent requests can both observe
244+
// count > 1 and both proceed to delete, leaving the user with zero cards.
245+
// Serializable isolation causes the database to roll back the second
246+
// conflicting transaction rather than allowing both to commit.
247+
let isLastCard = false;
248+
await app.prisma.$transaction(async (tx: Prisma.TransactionClient) => {
249+
const userCardCount = await tx.card.count({ where: { userId } });
250+
if (userCardCount <= 1) {
251+
isLastCard = true;
252+
return;
253+
}
256254

257-
if (oldestRemainingCard) {
258-
await app.prisma.card.update({
259-
where: { id: oldestRemainingCard.id },
260-
data: { isDefault: true },
255+
// If the card being deleted is the default, promote the next-oldest card
256+
// before deletion so the user always has an active default.
257+
if (existing.isDefault) {
258+
const oldestRemainingCard = await tx.card.findFirst({
259+
where: { userId, id: { not: id } },
260+
orderBy: { createdAt: 'asc' },
261261
});
262+
263+
if (oldestRemainingCard) {
264+
await tx.card.update({
265+
where: { id: oldestRemainingCard.id },
266+
data: { isDefault: true },
267+
});
268+
}
262269
}
270+
271+
await tx.card.delete({ where: { id } });
272+
}, { isolationLevel: 'Serializable' });
273+
274+
if (isLastCard) {
275+
reply.status(400).send({ error: 'Cannot delete the last remaining card. A user must have at least one card.' });
276+
return;
263277
}
264278

265-
await app.prisma.card.delete({ where: { id } });
266279
reply.status(204).send();
267280
} catch (error) {
268281
return handleDbError(error, request, reply);

0 commit comments

Comments
 (0)