From f5ac0e05aca26f049f17b0c3d0c427a4962fcf62 Mon Sep 17 00:00:00 2001 From: PragyaTripathi990 <16pragyatripathi@gmail.com> Date: Fri, 8 May 2026 03:17:24 +0530 Subject: [PATCH] fix: resolve concurrency, data integrity, and correctness bugs in stock 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. --- .../repo/stockEntry/ItemStockEntryRepo.java | 4 +- .../patientreturn/PatientReturnService.java | 2 +- .../PatientReturnServiceImpl.java | 23 +++- .../stockEntry/StockEntryServiceImpl.java | 15 ++- .../service/stockExit/StockExitService.java | 6 +- .../stockExit/StockExitServiceImpl.java | 116 +++++++++++------- .../StockAdjustmentServiceImpl.java | 16 ++- 7 files changed, 120 insertions(+), 62 deletions(-) diff --git a/src/main/java/com/iemr/inventory/repo/stockEntry/ItemStockEntryRepo.java b/src/main/java/com/iemr/inventory/repo/stockEntry/ItemStockEntryRepo.java index 94671b2c..71683f9b 100644 --- a/src/main/java/com/iemr/inventory/repo/stockEntry/ItemStockEntryRepo.java +++ b/src/main/java/com/iemr/inventory/repo/stockEntry/ItemStockEntryRepo.java @@ -54,8 +54,10 @@ List findByFacilityIDAndItemIDAndQuantityInHandGreaterThanAndDel @Transactional @Modifying + // Previously matched on vanSerialNo which is a mutable sync field; using the + // stable primary key itemStockEntryID ensures the correct batch is always updated. @Query("UPDATE ItemStockEntry c SET c.quantityInHand = c.quantityInHand - :dispQuant " - + " WHERE c.vanSerialNo = :itemStockEntryId and c.facilityID = :facilityID") + + " WHERE c.itemStockEntryID = :itemStockEntryId and c.facilityID = :facilityID") Integer updateStock(@Param("facilityID") Integer facilityID, @Param("itemStockEntryId") Long itemStockEntryId, @Param("dispQuant") Integer dispQuant); diff --git a/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnService.java b/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnService.java index 9fbf33a1..1eb3c751 100644 --- a/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnService.java +++ b/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnService.java @@ -35,7 +35,7 @@ public interface PatientReturnService { List getItemDetailByBen(ItemDetailModel itemDetailModel); - String updateQuantityReturned(ItemDetailModel[] itemDetailModel); + String updateQuantityReturned(ItemDetailModel[] itemDetailModel) throws Exception; List getBenReturnHistory(ItemReturnEntry itemReturnEntry); } diff --git a/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnServiceImpl.java b/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnServiceImpl.java index e5fcc014..eb42ec88 100644 --- a/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnServiceImpl.java +++ b/src/main/java/com/iemr/inventory/service/patientreturn/PatientReturnServiceImpl.java @@ -32,6 +32,7 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import com.iemr.inventory.data.patientreturn.ItemDetailModel; import com.iemr.inventory.data.patientreturn.PatientReturnModel; @@ -104,15 +105,26 @@ public List getItemDetailByBen(ItemDetailModel itemDetailModel) return list; } @Override - public String updateQuantityReturned(ItemDetailModel[] itemDetailModel) + @Transactional(rollbackFor = Exception.class) + public String updateQuantityReturned(ItemDetailModel[] itemDetailModel) throws Exception { logger.info("updateQuantityReturned - Start"); List list = Arrays.asList(itemDetailModel); - List returnList=null; - returnList = new ArrayList(); - for(ItemDetailModel itemDetail : list) + List returnList = new ArrayList(); + for (ItemDetailModel itemDetail : list) { - int result = patientReturnRepo.updateQuantityReturned(itemDetail.getReturnQuantity(), itemDetail.getItemStockEntryID()); + // Guard: returnQuantity must not exceed the originally issued quantity. + // Without this check a caller could inflate stock beyond what was dispensed. + if (itemDetail.getReturnQuantity() == null || itemDetail.getReturnQuantity() <= 0) { + throw new Exception("Return quantity must be greater than zero for ItemID: " + itemDetail.getItemID()); + } + if (itemDetail.getIssuedQuantity() != null + && itemDetail.getReturnQuantity() > itemDetail.getIssuedQuantity()) { + throw new Exception("Return quantity (" + itemDetail.getReturnQuantity() + + ") exceeds issued quantity (" + itemDetail.getIssuedQuantity() + + ") for ItemID: " + itemDetail.getItemID()); + } + patientReturnRepo.updateQuantityReturned(itemDetail.getReturnQuantity(), itemDetail.getItemStockEntryID()); patientReturnRepo.updateIssuedQuantity(itemDetail.getReturnQuantity(), itemDetail.getItemStockExitID()); ItemReturnEntry itemReturnEntry = new ItemReturnEntry(); itemReturnEntry.setCreatedBy(itemDetail.getCreatedBy()); @@ -123,7 +135,6 @@ public String updateQuantityReturned(ItemDetailModel[] itemDetailModel) itemReturnEntry.setProviderServiceMapID(itemDetail.getProviderServiceMapID()); itemReturnEntry.setVisitID(itemDetail.getVisitID()); itemReturnEntry.setVisitCode(itemDetail.getVisitCode()); - returnList.add(itemReturnEntry); } itemReturnEntryRepo.saveAll(returnList); diff --git a/src/main/java/com/iemr/inventory/service/stockEntry/StockEntryServiceImpl.java b/src/main/java/com/iemr/inventory/service/stockEntry/StockEntryServiceImpl.java index ae5247ad..4e3b3f07 100644 --- a/src/main/java/com/iemr/inventory/service/stockEntry/StockEntryServiceImpl.java +++ b/src/main/java/com/iemr/inventory/service/stockEntry/StockEntryServiceImpl.java @@ -22,6 +22,8 @@ package com.iemr.inventory.service.stockEntry; import java.sql.Timestamp; +import java.time.LocalDate; +import java.time.ZoneId; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -162,24 +164,29 @@ public List getItemStockFromItemID(Integer facilityID, List stockList = new ArrayList(); String method = item.getItemCategory().getIssueType(); Integer itemID = itemStockExit.getItemID(); - Date nowdate = new Date(); + // Use java.time to compute the expiry threshold. + // The deprecated Date.setDate/setMonth do not handle month-boundary overflow + // (e.g. Jan 31 + 1 month wraps to Mar 3) and ignore DST, producing an + // incorrect cutoff that can serve expired stock or reject valid batches. + LocalDate nowLocal = LocalDate.now(); if (itemStockExit.getDuration() != null && itemStockExit.getDuration() > 0 && itemStockExit.getDurationUnit() != null) { switch (itemStockExit.getDurationUnit()) { case "Day(s)": - nowdate.setDate(nowdate.getDate() + itemStockExit.getDuration()); + nowLocal = nowLocal.plusDays(itemStockExit.getDuration()); break; case "Month(s)": - nowdate.setMonth(nowdate.getMonth() + itemStockExit.getDuration()); + nowLocal = nowLocal.plusMonths(itemStockExit.getDuration()); break; case "Week(s)": - nowdate.setDate(nowdate.getDate() + (7 * itemStockExit.getDuration())); + nowLocal = nowLocal.plusWeeks(itemStockExit.getDuration()); break; default: break; } } + Date nowdate = Date.from(nowLocal.atStartOfDay(ZoneId.systemDefault()).toInstant()); if (method == null) { stockList = getItemStockForStoreIDOrderByEntryDateAsc(facilityID, itemID, nowdate); diff --git a/src/main/java/com/iemr/inventory/service/stockExit/StockExitService.java b/src/main/java/com/iemr/inventory/service/stockExit/StockExitService.java index 22c66b50..6bbbdc91 100644 --- a/src/main/java/com/iemr/inventory/service/stockExit/StockExitService.java +++ b/src/main/java/com/iemr/inventory/service/stockExit/StockExitService.java @@ -43,9 +43,9 @@ public interface StockExitService { List getItemStockAndValidate(List itemissueList, Integer facilityID, String createdBy,Long vanID,Long ppID); - Integer storeSelfConsumption(StoreSelfConsumption storeSelfConsumption); - - Integer storeTransfer(T_StockTransfer stockTransfer); + Integer storeSelfConsumption(StoreSelfConsumption storeSelfConsumption) throws InventoryException; + + Integer storeTransfer(T_StockTransfer stockTransfer) throws InventoryException; // List getItemStockFromItemID(Integer facilityID,List itemStockExit); diff --git a/src/main/java/com/iemr/inventory/service/stockExit/StockExitServiceImpl.java b/src/main/java/com/iemr/inventory/service/stockExit/StockExitServiceImpl.java index 469bd6e6..ea8ddac9 100644 --- a/src/main/java/com/iemr/inventory/service/stockExit/StockExitServiceImpl.java +++ b/src/main/java/com/iemr/inventory/service/stockExit/StockExitServiceImpl.java @@ -113,23 +113,36 @@ public Integer issuePatientDrugs(T_PatientIssue patientIssue) throws InventoryEx + itemissueListUpdated.size()); logger.info("itemissueList " + itemissueList.toString()); logger.info("itemissueListUpdated " + itemissueListUpdated.toString()); - if (itemissueList.size() == itemissueListUpdated.size()) { - patientIssue.setSyncFacilityID(patientIssue.getFacilityID()); - patientIssueRepo.save(patientIssue); - patientIssueRepo.updateVanSerialNo(); - - returnvalue = saveItemExit(itemissueListUpdated, patientIssue.getPatientIssueID(), "T_PatientIssue"); - if (returnvalue == 1) { - if (patientIssue != null && patientIssue.getBenRegID() != null - && patientIssue.getVisitCode() != null) { - int i = updateBenFlowAfterPharmaTransaction(patientIssue); - if (i > 0) - returnvalue = 1; - else - returnvalue = 0; - } else { + if (itemissueList.size() != itemissueListUpdated.size()) { + // Identify which items failed validation (insufficient stock) so the caller + // gets actionable feedback instead of a silent 0 return. + List validatedItemIDs = itemissueListUpdated.stream() + .map(ItemStockExit::getItemID) + .collect(Collectors.toList()); + List failedItems = itemissueList.stream() + .filter(item -> !validatedItemIDs.contains(item.getItemID())) + .map(item -> "ItemID " + item.getItemID() + + " (requested: " + item.getQuantity() + + ", available: " + item.getQuantityInHand() + ")") + .collect(Collectors.toList()); + throw new InventoryException( + "Insufficient stock for the following item(s): " + String.join(", ", failedItems)); + } + patientIssue.setSyncFacilityID(patientIssue.getFacilityID()); + patientIssueRepo.save(patientIssue); + patientIssueRepo.updateVanSerialNo(); + + returnvalue = saveItemExit(itemissueListUpdated, patientIssue.getPatientIssueID(), "T_PatientIssue"); + if (returnvalue == 1) { + if (patientIssue != null && patientIssue.getBenRegID() != null + && patientIssue.getVisitCode() != null) { + int i = updateBenFlowAfterPharmaTransaction(patientIssue); + if (i > 0) + returnvalue = 1; + else returnvalue = 0; - } + } else { + returnvalue = 0; } } @@ -194,31 +207,39 @@ public List getItemStockAndValidate(List itemissue @Transactional(propagation = Propagation.REQUIRES_NEW, rollbackFor = Exception.class) @Override - public Integer storeSelfConsumption(StoreSelfConsumption storeSelfConsumption) { + public Integer storeSelfConsumption(StoreSelfConsumption storeSelfConsumption) throws InventoryException { Integer returnvalue = 0; - if (true) { - List itemissueList = storeSelfConsumption.getItemStockExit(); - - List itemissueListUpdated = getItemStockAndValidate(itemissueList, - storeSelfConsumption.getFacilityID(), storeSelfConsumption.getCreatedBy(), - storeSelfConsumption.getVanID(), storeSelfConsumption.getParkingPlaceID()); - if (itemissueList.size() == itemissueListUpdated.size()) { - storeSelfConsumption.setSyncFacilityID(storeSelfConsumption.getFacilityID()); - storeSelfConsumptionRepo.save(storeSelfConsumption); - storeSelfConsumptionRepo.updateVanSerialNo(); - - returnvalue = saveItemExit(itemissueListUpdated, storeSelfConsumption.getConsumptionID(), - "StoreSelfConsumption"); - } + List itemissueList = storeSelfConsumption.getItemStockExit(); + List itemissueListUpdated = getItemStockAndValidate(itemissueList, + storeSelfConsumption.getFacilityID(), storeSelfConsumption.getCreatedBy(), + storeSelfConsumption.getVanID(), storeSelfConsumption.getParkingPlaceID()); + if (itemissueList.size() != itemissueListUpdated.size()) { + List validatedItemIDs = itemissueListUpdated.stream() + .map(ItemStockExit::getItemID) + .collect(Collectors.toList()); + List failedItems = itemissueList.stream() + .filter(item -> !validatedItemIDs.contains(item.getItemID())) + .map(item -> "ItemID " + item.getItemID() + + " (requested: " + item.getQuantity() + + ", available: " + item.getQuantityInHand() + ")") + .collect(Collectors.toList()); + throw new InventoryException( + "Insufficient stock for the following item(s): " + String.join(", ", failedItems)); } + storeSelfConsumption.setSyncFacilityID(storeSelfConsumption.getFacilityID()); + storeSelfConsumptionRepo.save(storeSelfConsumption); + storeSelfConsumptionRepo.updateVanSerialNo(); + + returnvalue = saveItemExit(itemissueListUpdated, storeSelfConsumption.getConsumptionID(), + "StoreSelfConsumption"); return returnvalue; } @Transactional(propagation = Propagation.REQUIRES_NEW, rollbackFor = Exception.class) @Override - public Integer storeTransfer(T_StockTransfer stockTransfer) { + public Integer storeTransfer(T_StockTransfer stockTransfer) throws InventoryException { Integer returnvalue = 0; Long toVanID = stockTransferRepo.findVanIDByFacID(stockTransfer.getTransferToFacilityID()); stockTransfer.setToVanID(toVanID); @@ -228,18 +249,29 @@ public Integer storeTransfer(T_StockTransfer stockTransfer) { stockTransfer.getTransferFromFacilityID(), stockTransfer.getCreatedBy(), stockTransfer.getVanID(), null); - if (itemissueList.size() == itemissueListUpdated.size()) { - stockTransfer.setSyncFacilityID(stockTransfer.getTransferFromFacilityID()); - stockTransferRepo.save(stockTransfer); - stockTransferRepo.updateVanSerialNo(); + if (itemissueList.size() != itemissueListUpdated.size()) { + List validatedItemIDs = itemissueListUpdated.stream() + .map(ItemStockExit::getItemID) + .collect(Collectors.toList()); + List failedItems = itemissueList.stream() + .filter(item -> !validatedItemIDs.contains(item.getItemID())) + .map(item -> "ItemID " + item.getItemID() + + " (requested: " + item.getQuantity() + + ", available: " + item.getQuantityInHand() + ")") + .collect(Collectors.toList()); + throw new InventoryException( + "Insufficient stock for the following item(s): " + String.join(", ", failedItems)); + } + stockTransfer.setSyncFacilityID(stockTransfer.getTransferFromFacilityID()); + stockTransferRepo.save(stockTransfer); + stockTransferRepo.updateVanSerialNo(); - saveItemExit(itemissueListUpdated, stockTransfer.getStockTransferID(), "T_StockTransfer"); + saveItemExit(itemissueListUpdated, stockTransfer.getStockTransferID(), "T_StockTransfer"); - stockEntryService.saveItemStockFromStockTransfer(itemissueListUpdated, stockTransfer.getStockTransferID(), - "T_StockTransfer", stockTransfer.getTransferFromFacilityID(), - stockTransfer.getTransferToFacilityID(), toVanID); - returnvalue = 1; - } + stockEntryService.saveItemStockFromStockTransfer(itemissueListUpdated, stockTransfer.getStockTransferID(), + "T_StockTransfer", stockTransfer.getTransferFromFacilityID(), + stockTransfer.getTransferToFacilityID(), toVanID); + returnvalue = 1; return returnvalue; } diff --git a/src/main/java/com/iemr/inventory/service/stockadjustment/StockAdjustmentServiceImpl.java b/src/main/java/com/iemr/inventory/service/stockadjustment/StockAdjustmentServiceImpl.java index fe002032..f620715a 100644 --- a/src/main/java/com/iemr/inventory/service/stockadjustment/StockAdjustmentServiceImpl.java +++ b/src/main/java/com/iemr/inventory/service/stockadjustment/StockAdjustmentServiceImpl.java @@ -93,7 +93,9 @@ public StockAdjustmentDraft saveDraft(StockAdjustmentDraft stockAdjustmentDraft) } Long stockdraftid = stockdraft.getStockAdjustmentDraftID(); - itemdraft.parallelStream().forEach(action -> { + // Sequential: repo lookups inside a parallel stream run on separate threads, + // bypassing the @Transactional context and risking inconsistent reads. + itemdraft.forEach(action -> { if (action.getSADraftItemMapID() != null) { StockAdjustmentItemDraft stockAdjustmentItemDraft = stockAdjustmentItemDraftRepo .findById(action.getSADraftItemMapID()).get(); @@ -103,7 +105,6 @@ public StockAdjustmentDraft saveDraft(StockAdjustmentDraft stockAdjustmentDraft) action.setProcessed(stockAdjustmentItemDraft.getProcessed()); } action.setStockAdjustmentDraftID(stockdraftid); - }); itemdraft = (List) stockAdjustmentItemDraftRepo.saveAll(itemdraft); stockdraft.setStockAdjustmentItemDraft(itemdraft); @@ -150,7 +151,10 @@ public StockAdjustment savetransaction(StockAdjustment stockAdjustment) throws I List comapreid = new ArrayList<>(); final Integer facID = stockAdjustment.getFacilityID(); - sd.parallelStream().forEach(action -> { + // Sequential stream: ArrayList is not thread-safe, and mutating shared objects + // or calling @Modifying JPA queries from a parallel stream bypasses the + // @Transactional boundary, risking lost updates and corrupted stock counts. + sd.forEach(action -> { comapreid.add(action.getItemStockEntryID()); action.setFacilityID(facID); }); @@ -177,14 +181,16 @@ public StockAdjustment savetransaction(StockAdjustment stockAdjustment) throws I stockAdjustmentRepo.updateVanSerialNo(); Long saID = stockAdjustment.getStockAdjustmentID(); - sd.parallelStream().forEach(action -> { + // Sequential: @Modifying JPA queries must run within the active transaction. + // A parallel stream spawns threads that execute outside the transaction + // coordinator, defeating rollback guarantees and causing partial stock updates. + sd.forEach(action -> { action.setStockAdjustmentID(saID); if (action.getIsAdded()) { itemStockEntryRepo.addStock(action.getItemStockEntryID(), action.getAdjustedQuantity()); } else { itemStockEntryRepo.subtractStock(action.getItemStockEntryID(), action.getAdjustedQuantity()); } - }); stockAdjustmentItemRepo.saveAll(sd);