[upstream #4930] fix(core): Prevent stock oversell under concurrent checkout - #25
Open
ayim wants to merge 33 commits into
Open
[upstream #4930] fix(core): Prevent stock oversell under concurrent checkout#25ayim wants to merge 33 commits into
ayim wants to merge 33 commits into
Conversation
…tfallEvent Fixes #OSS-94
…l-be-bought-when-inside-an-active
…of-stock-items-can-still-be-bought-when-inside-an-active
…rder shortfall, transaction wrap)
…n-transactional callers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirrored from vendurehq#4930 for the Overwatch review demo.
Original author: @grolmus
Fixes vendurehq#1738
Problem
Stock can be oversold under concurrency. The saleable-stock check (
AddingItems → ArrangingPayment) and stock allocation (ArrangingPayment → PaymentSettled/Authorized) run in different transitions / transactions with unlocked reads. Between them, other orders can deplete stock; the later allocation then oversells — or, with the default multi-location strategy, silently under-allocates a paid order. Additionally,StockLevelServicewrites were non-atomic read-modify-write with no clamp, allowing lost updates and negative stock values.Changes
Atomic, clamped stock writes
StockLevelService.updateStockOnHandForLocation/updateStockAllocatedForLocationnow perform the read-modify-write under a pessimistic row lock and clampstockAllocatedat ≥ 0 (and never create aStockLevelwith a negative value). This removes the lost-update race and prevents negative stock.Locked re-check at allocation
StockMovementService.createAllocationsForOrderLinesacquires apessimistic_writelock on the variant'sStockLevelrows before computing allocation, so concurrent settlements can no longer consume the same units. The default location strategy already caps allocation to availability; the lock makes that re-check race-free (the locking read is also the freshness re-check, MySQL REPEATABLE-READ-safe).StockShortfallEvent(new)When allocation cannot fully satisfy a line because stock was depleted since checkout, allocation is capped to what is available and a new
StockShortfallEvent(order + per-linerequested/allocated) is published. Because the payment may already be captured (async gateways), allocation is not hard-failed — plugins react to the event (refund, backorder, notify).Notes
SQLITE_BUSY, not a silent lost update).StockLevelrow yet is not covered by the row lock; concurrent inserts would collide on the unique(productVariantId, stockLocationId)index — a safe rollback, not an oversell.Testing
stock-control.e2e-spec.ts: sequential depletion (order A passes the checkout stock check, order B depletes and settles the stock, order A then settles) → allocation capped, no negative/oversold stock,StockShortfallEventemitted — with exact-value assertions. Plus atomic/clamp coverage.stock-controlandstock-control-multi-locationsuites pass.Relates to real-time cart recalculation (vendurehq#3510), handled separately.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.