fix: resolve concurrency, data integrity, and correctness bugs in stock management#126
Conversation
…ck management 1. StockAdjustmentServiceImpl - Replace parallelStream with sequential forEach parallelStream on a non-thread-safe ArrayList caused lost updates and ArrayIndexOutOfBoundsException under concurrent load. JPA @Modifying queries called from parallel threads also bypass the @transactional boundary, making rollback unreliable and causing partial stock adjustments. 2. StockExitServiceImpl - Throw InventoryException on partial stock validation failure When getItemStockAndValidate filtered out items due to insufficient stock, all three exit flows (patient issue, self-consumption, store transfer) silently returned 0 with no indication of which items failed. Now throws InventoryException naming each failed item with its requested vs. available quantities. 3. ItemStockEntryRepo - Fix updateStock query to match on itemStockEntryID not vanSerialNo The WHERE clause used vanSerialNo (a mutable sync field) while the caller passed itemStockEntryID. In any record where the two diverge, stock was deducted from the wrong batch or not deducted at all. Fixed to use the stable primary key. 4. PatientReturnServiceImpl - Add @transactional and quantity guard to updateQuantityReturned Two separate DB updates (stock restore + exit quantity decrement) ran non-atomically with no upper-bound check. A crash after the first update permanently inflated stock. A caller could also return more than was dispensed. Now @transactional ensures both updates roll back together, and the guard rejects returnQuantity > issuedQuantity. 5. StockEntryServiceImpl - Replace deprecated Date mutation with java.time Date.setDate/setMonth are deprecated since Java 1.1 and do not handle month-boundary overflow (e.g. Jan 31 + 1 month wraps incorrectly) or DST transitions, producing a wrong expiry threshold that could serve expired stock or reject valid batches. Replaced with LocalDate.plusDays/plusMonths/plusWeeks via java.time.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
1 participant
Summary
This PR fixes 5 real bugs across the inventory and stock management services:
1. Race condition in
StockAdjustmentServiceImpl—parallelStreamon non-thread-safe collectionsparallelStreamwas used with a sharedArrayList(not thread-safe) and with JPA@Modifyingqueries. Parallel threads bypass the@Transactionalcoordinator, making rollback unreliable and causing lost updates or corrupted stock counts under concurrent load. Fixed by converting to sequentialforEach.2. Silent dispense failure in
StockExitServiceImplWhen
getItemStockAndValidatefiltered out items due to insufficient stock, all three exit flows (patient issue, self-consumption, store transfer) silently returned0with no indication of which items failed. Pharmacists had no way to know what went wrong. Fixed by throwingInventoryExceptionnaming each failed item with requested vs. available quantities.3. Wrong column in
updateStockquery —ItemStockEntryRepoThe
WHEREclause matched onvanSerialNo(a mutable sync field) while the caller always passeditemStockEntryID. In any record where the two diverge, stock was deducted from the wrong batch or not deducted at all. Fixed by changing theWHEREclause to use the stable primary keyitemStockEntryID.4. No atomicity or quantity guard in
PatientReturnServiceImplTwo separate DB updates (stock restore + exit quantity decrement) ran non-atomically with no upper-bound validation. A crash after the first update permanently inflated stock. A caller could also return more drugs than were dispensed. Fixed by adding
@Transactional(rollbackFor = Exception.class)and a guard that rejectsreturnQuantity > issuedQuantity.5. Deprecated
Datemutation causing wrong expiry window —StockEntryServiceImplDate.setDate()andDate.setMonth()are deprecated since Java 1.1 and do not handle month-boundary overflow (e.g. Jan 31 + 1 month → Mar 3) or DST transitions, producing an incorrect expiry cutoff that could serve expired stock or reject valid batches. Fixed by replacing withjava.time.LocalDate.plusDays/plusMonths/plusWeeks.Test plan
updateStock