Skip to content

fix: BE-51 — allocate invoice numbers safely under concurrency (#640) - #824

Merged
Qoder-Voidd merged 3 commits into
Akazajan:mainfrom
VeronicDev:fix/issue-640-be-51-invoice-number-allocation
Oct 20, 2026
Merged

Qoder-Voidd merged 3 commits into
Akazajan:mainfrom
VeronicDev:fix/issue-640-be-51-invoice-number-allocation

Conversation

@VeronicDev

Copy link
Copy Markdown
Contributor

Closes #640

Problem

Invoice generation had two concurrency holes.

1. Number allocation was not actually guaranteed unique. The number came straight from nextval() in GenerateInvoiceProvider, and a collision was detected by substring-matching the driver error message for "duplicate" — brittle across pg versions. Worse, the sequence was only ever CREATEd, never advanced: on a database restored from a dump, nextval() re-issued numbers that already existed and every attempt collided.

2. One payment could produce two invoices. generateForPayment opens with a "does an invoice already exist for this payment?" lookup and then inserts. That is a check-then-act, so two concurrent calls for the same payment both see nothing and both insert — double-billing the customer with two perfectly valid, non-duplicate invoice numbers.

Fix

  • InvoiceSequenceProvider now owns allocation end to end. It ensures the sequence, advances it past the highest invoiceNumber already stored, hands out numbers through formatInvoiceNumber, and classifies failures with isInvoiceNumberCollision.
    • The alignment is a single setval(..., GREATEST(last_value, MAX(existing invoice number))) statement. It only ever moves forward, so concurrent app boots converge on the same value and a healthy database is untouched.
    • isInvoiceNumberCollision matches the 23505 SQLSTATE (and optionally the constraint name) instead of the driver message.
  • GenerateInvoiceProvider injects the sequence provider, retries a number collision with a freshly allocated number, and catches the unique violation on paymentId to return the invoice the winning transaction created.
  • Invoice gains a partial unique index on paymentId (WHERE "paymentId" IS NOT NULL). This is what makes the second concurrent insert fail at the database level instead of silently duplicating the invoice.
  • Only the transaction that actually inserted the row emails the customer, so the caller that lost the race does not send a second "invoice ready" notification.

The insert stays inside its own transaction, so a failed generation rolls back completely and never surfaces a half-written invoice; a burned sequence value is a gap, not a visible invoice.

Tests

  • invoice-sequence.provider.spec.ts — number formatting, collision classification, bootstrap alignment, and that 25 concurrent callers each receive a distinct number.
  • generate-invoice.provider.spec.ts — the allocation transaction, distinct numbers under concurrency, the same-payment race returning a single row, the collision retry and its give-up path, propagation of unrelated errors without retrying, no visible invoice on failure, and the single-notification guarantee.

Notes

The issue listed a 3-file scope; this touches those three plus generate-invoice.provider.ts (which is where the race actually lives and had to call the new provider) and the two additive spec files the acceptance criteria require.

fix(contact): improve rate-limit key normalization for IPv4 and IPv4-mapped IPv6
…jan#640)

Invoice generation had two concurrency holes.

**Numbers were only "unique until the database noticed".** The number
came straight from `nextval()` in `GenerateInvoiceProvider`, and a
collision was detected by substring-matching the driver's error
message for `"duplicate"` — brittle across `pg` versions. Worse, the
sequence was only ever `CREATE`d: on a database restored from a dump,
`nextval()` happily re-issued numbers that already existed and every
attempt collided.

**One payment could produce two invoices.** `generateForPayment` opens
with a "does an invoice already exist for this payment?" lookup and
then inserts. That is a check-then-act, so two concurrent calls for the
same payment both see nothing and both insert — double-billing the
customer with two perfectly valid, non-duplicate invoice numbers.

- `InvoiceSequenceProvider` now owns allocation end to end: it ensures
  the sequence, advances it past the highest `invoiceNumber` already
  stored (`setval` in a single statement that only ever moves forward,
  so concurrent boots converge), hands out numbers through
  `formatInvoiceNumber`, and classifies failures with
  `isInvoiceNumberCollision`, which matches the `23505` SQLSTATE (and
  optionally the constraint name) instead of the driver message.
- `GenerateInvoiceProvider` injects the sequence provider, retries a
  number collision with a freshly allocated number, and — the actual
  fix for the race — catches the unique violation on `paymentId` and
  returns the invoice the winning transaction created.
- `Invoice` gains a partial unique index on `paymentId`
  (`WHERE "paymentId" IS NOT NULL`), which is what makes the second
  concurrent insert fail at the database level instead of silently
  duplicating the invoice.
- Only the transaction that actually inserted the row now emails the
  customer, so the caller that lost the race does not send a second
  "invoice ready" notification for the same invoice.

The insert stays inside its own transaction, so a failed generation
rolls back completely and never surfaces a half-written invoice; a
burned sequence value is a gap, not a visible invoice.

Adds `invoice-sequence.provider.spec.ts` and
`generate-invoice.provider.spec.ts` covering the allocation, the
collision retry, the same-payment race, the failure path and the
single-notification guarantee.
@drips-wave

drips-wave Bot commented Oct 19, 2026

Copy link
Copy Markdown

@VeronicDev Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Qoder-Voidd Qoder-Voidd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Qoder-Voidd
Qoder-Voidd merged commit e4cf762 into Akazajan:main Oct 20, 2026
0 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BE-51 — Allocate invoice numbers safely under concurrency

2 participants