FINERACT-1659: Fix optimistic locking in savings interest posting batch job

awaneetdecoder · awaneetdecoder · commit d469aaa48029 · 2026-04-08T22:37:32.000+05:30
diff --git a/fineract-core/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountData.java b/fineract-core/src/main/java/org/apache/fineract/portfolio/savings/data/SavingsAccountData.java
@@ -141,6 +141,7 @@ public final class SavingsAccountData implements Serializable {
     private transient List<SavingsAccountTransactionData> newSavingsAccountTransactionData = new ArrayList<>();
     private transient GroupGeneralData groupGeneralData;
     private transient Long officeId;
+    private transient Integer version;
     private transient Set<Long> existingTransactionIds = new HashSet<>();
     private transient Set<Long> existingReversedTransactionIds = new HashSet<>();
     private transient Long glAccountIdForSavingsControl;
diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsAccountReadPlatformServiceImpl.java
@@ -311,6 +311,7 @@ private static final class SavingAccountMapperForInterestPosting implements Resu
             sqlBuilder.append("sa.last_interest_calculation_date as lastInterestCalculationDate, ");
             sqlBuilder.append("sa.total_savings_amount_on_hold as onHoldAmount, ");
             sqlBuilder.append("sa.interest_posted_till_date as interestPostedTillDate, ");
+            sqlBuilder.append("sa.version as version, ");
             sqlBuilder.append("tg.id as taxGroupId, ");
             sqlBuilder.append("(select COALESCE(max(sat.transaction_date),sa.activatedon_date) ");
             sqlBuilder.append("from m_savings_account_transaction as sat ");
@@ -584,6 +585,8 @@ public List<SavingsAccountData> extractData(final ResultSet rs) throws SQLExcept
 
                     savingsAccountData.setGlAccountIdForInterestOnSavings(glAccountIdForInterestOnSavings);
                     savingsAccountData.setGlAccountIdForSavingsControl(glAccountIdForSavingsControl);
+                    final Integer version = JdbcSupport.getInteger(rs, "version");
+                    savingsAccountData.setVersion(version);
                 }
 
                 if (!transMap.containsValue(transactionId)) {
diff --git a/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsSchedularInterestPoster.java b/fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsSchedularInterestPoster.java
@@ -67,16 +67,10 @@ public class SavingsSchedularInterestPoster {
     public void postInterest() throws JobExecutionException {
         if (!savingAccounts.isEmpty()) {
             List<Throwable> errors = new ArrayList<>();
-            LocalDate yesterday = DateUtils.getBusinessLocalDate().minusDays(1);
             for (SavingsAccountData savingsAccountData : savingAccounts) {
                 boolean postInterestAsOn = false;
                 LocalDate transactionDate = null;
                 try {
-                    if (isInterestAlreadyPostedForPeriod(savingsAccountData, yesterday)) {
-                        log.debug("Interest already posted for savings account {} up to date {}, skipping", savingsAccountData.getId(),
-                                savingsAccountData.getSummary().getInterestPostedTillDate());
-                        continue;
-                    }
                     SavingsAccountData savingsAccountDataRet = savingsAccountWritePlatformService.postInterest(savingsAccountData,
                             postInterestAsOn, transactionDate, backdatedTxnsAllowedTill);
                     savingsAccountDataList.add(savingsAccountDataRet);
@@ -168,15 +162,18 @@ private void batchUpdate(final List<SavingsAccountData> savingsAccountDataList)
         String queryForSavingsUpdate = batchQueryForSavingsSummaryUpdate();
         String queryForTransactionInsertion = batchQueryForTransactionInsertion();
         String queryForTransactionUpdate = batchQueryForTransactionsUpdate();
-        List<Object[]> paramsForTransactionInsertion = new ArrayList<>();
-        List<Object[]> paramsForSavingsSummary = new ArrayList<>();
-        List<Object[]> paramsForTransactionUpdate = new ArrayList<>();
-        List<String> transRefNo = new ArrayList<>();
         LocalDate currentDate = DateUtils.getBusinessLocalDate();
         Long userId = platformSecurityContext.authenticatedUser().getId();
+
+        List<Object[]> paramsForSavingsSummary = new ArrayList<>();
+        List<List<String>> perAccountRefNos = new ArrayList<>();
+        List<List<Object[]>> perAccountInsertionParams = new ArrayList<>();
+        List<List<Object[]>> perAccountUpdateParams = new ArrayList<>();
+
         for (SavingsAccountData savingsAccountData : savingsAccountDataList) {
             OffsetDateTime auditTime = DateUtils.getAuditOffsetDateTime();
             SavingsAccountSummaryData savingsAccountSummaryData = savingsAccountData.getSummary();
+
             paramsForSavingsSummary.add(new Object[] { savingsAccountSummaryData.getTotalDeposits(),
                     savingsAccountSummaryData.getTotalWithdrawals(), savingsAccountSummaryData.getTotalInterestEarned(),
                     savingsAccountSummaryData.getTotalInterestPosted(), savingsAccountSummaryData.getTotalWithdrawalFees(),
@@ -186,14 +183,19 @@ private void batchUpdate(final List<SavingsAccountData> savingsAccountDataList)
                     savingsAccountSummaryData.getLastInterestCalculationDate(),
                     savingsAccountSummaryData.getInterestPostedTillDate() != null ? savingsAccountSummaryData.getInterestPostedTillDate()
                             : savingsAccountSummaryData.getLastInterestCalculationDate(),
-                    auditTime, userId, savingsAccountData.getId() });
+                    auditTime, userId, savingsAccountData.getId(), savingsAccountData.getVersion() });
+
+            List<String> transRefNo = new ArrayList<>();
+            List<Object[]> insertionParams = new ArrayList<>();
+            List<Object[]> updateParams = new ArrayList<>();
+
             List<SavingsAccountTransactionData> savingsAccountTransactionDataList = savingsAccountData.getSavingsAccountTransactionData();
             for (SavingsAccountTransactionData savingsAccountTransactionData : savingsAccountTransactionDataList) {
                 if (savingsAccountTransactionData.getId() == null && !MathUtil.isZero(savingsAccountTransactionData.getAmount())) {
                     UUID uuid = UUID.randomUUID();
                     savingsAccountTransactionData.setRefNo(uuid.toString());
                     transRefNo.add(uuid.toString());
-                    paramsForTransactionInsertion.add(new Object[] { savingsAccountData.getId(), savingsAccountData.getOfficeId(),
+                    insertionParams.add(new Object[] { savingsAccountData.getId(), savingsAccountData.getOfficeId(),
                             savingsAccountTransactionData.isReversed(), savingsAccountTransactionData.getTransactionType().getId(),
                             savingsAccountTransactionData.getTransactionDate(), savingsAccountTransactionData.getAmount(),
                             savingsAccountTransactionData.getBalanceEndDate(), savingsAccountTransactionData.getBalanceNumberOfDays(),
@@ -202,35 +204,58 @@ private void batchUpdate(final List<SavingsAccountData> savingsAccountDataList)
                             savingsAccountTransactionData.getRefNo(), savingsAccountTransactionData.isReversalTransaction(),
                             savingsAccountTransactionData.getOverdraftAmount(), currentDate });
                 } else {
-                    paramsForTransactionUpdate.add(new Object[] { savingsAccountTransactionData.isReversed(),
-                            savingsAccountTransactionData.getAmount(), savingsAccountTransactionData.getOverdraftAmount(),
-                            savingsAccountTransactionData.getBalanceEndDate(), savingsAccountTransactionData.getBalanceNumberOfDays(),
-                            savingsAccountTransactionData.getRunningBalance(), savingsAccountTransactionData.getCumulativeBalance(),
-                            savingsAccountTransactionData.isReversalTransaction(), auditTime, userId,
-                            savingsAccountTransactionData.getId() });
+                    updateParams.add(new Object[] { savingsAccountTransactionData.isReversed(), savingsAccountTransactionData.getAmount(),
+                            savingsAccountTransactionData.getOverdraftAmount(), savingsAccountTransactionData.getBalanceEndDate(),
+                            savingsAccountTransactionData.getBalanceNumberOfDays(), savingsAccountTransactionData.getRunningBalance(),
+                            savingsAccountTransactionData.getCumulativeBalance(), savingsAccountTransactionData.isReversalTransaction(),
+                            auditTime, userId, savingsAccountTransactionData.getId() });
                 }
             }
             savingsAccountData.setUpdatedTransactions(savingsAccountTransactionDataList);
+
+            perAccountRefNos.add(transRefNo);
+            perAccountInsertionParams.add(insertionParams);
+            perAccountUpdateParams.add(updateParams);
         }
 
-        if (transRefNo.size() > 0) {
-            this.jdbcTemplate.batchUpdate(queryForSavingsUpdate, paramsForSavingsSummary);
-            this.jdbcTemplate.batchUpdate(queryForTransactionInsertion, paramsForTransactionInsertion);
-            this.jdbcTemplate.batchUpdate(queryForTransactionUpdate, paramsForTransactionUpdate);
-            log.debug("`Total No Of Interest Posting:` {}", transRefNo.size());
-            List<SavingsAccountTransactionData> savingsAccountTransactionDataList = fetchTransactionsFromIds(transRefNo);
-            if (savingsAccountDataList != null) {
-                log.debug("Fetched Transactions from DB: {}", savingsAccountTransactionDataList.size());
+        boolean anyNewTransactions = perAccountRefNos.stream().anyMatch(list -> !list.isEmpty());
+        if (!anyNewTransactions) {
+            return;
+        }
+
+        int[] updateCounts = this.jdbcTemplate.batchUpdate(queryForSavingsUpdate, paramsForSavingsSummary);
+
+        List<String> allTransRefNo = new ArrayList<>();
+        List<Object[]> allInsertionParams = new ArrayList<>();
+        List<Object[]> allUpdateParams = new ArrayList<>();
+        List<SavingsAccountData> successfulAccounts = new ArrayList<>();
+
+        for (int i = 0; i < updateCounts.length; i++) {
+            if (updateCounts[i] == 0) {
+                log.warn("Optimistic lock failure for savings account id={} — concurrent modification detected."
+                        + " Skipping. Will retry on next run.", savingsAccountDataList.get(i).getId());
+                continue;
             }
+            allTransRefNo.addAll(perAccountRefNos.get(i));
+            allInsertionParams.addAll(perAccountInsertionParams.get(i));
+            allUpdateParams.addAll(perAccountUpdateParams.get(i));
+            successfulAccounts.add(savingsAccountDataList.get(i));
+        }
+
+        if (!allTransRefNo.isEmpty()) {
+            this.jdbcTemplate.batchUpdate(queryForTransactionInsertion, allInsertionParams);
+            this.jdbcTemplate.batchUpdate(queryForTransactionUpdate, allUpdateParams);
+            log.debug("`Total No Of Interest Posting:` {}", allTransRefNo.size());
+            List<SavingsAccountTransactionData> fetchedTransactions = fetchTransactionsFromIds(allTransRefNo);
+            log.debug("Fetched Transactions from DB: {}", fetchedTransactions.size());
 
             HashMap<String, SavingsAccountTransactionData> savingsAccountTransactionMap = new HashMap<>();
-            for (SavingsAccountTransactionData savingsAccountTransactionData : savingsAccountTransactionDataList) {
+            for (SavingsAccountTransactionData savingsAccountTransactionData : fetchedTransactions) {
                 final String key = savingsAccountTransactionData.getRefNo();
                 savingsAccountTransactionMap.put(key, savingsAccountTransactionData);
             }
-            batchUpdateJournalEntries(savingsAccountDataList, savingsAccountTransactionMap);
+            batchUpdateJournalEntries(successfulAccounts, savingsAccountTransactionMap);
         }
-
     }
 
     private String batchQueryForTransactionInsertion() {
@@ -245,20 +270,12 @@ private String batchQueryForSavingsSummaryUpdate() {
         return "update m_savings_account set total_deposits_derived=?, total_withdrawals_derived=?, total_interest_earned_derived=?, total_interest_posted_derived=?, total_withdrawal_fees_derived=?, "
                 + "total_fees_charge_derived=?, total_penalty_charge_derived=?, total_annual_fees_derived=?, account_balance_derived=?, total_overdraft_interest_derived=?, total_withhold_tax_derived=?, "
                 + "last_interest_calculation_date=?, interest_posted_till_date=?, " + LAST_MODIFIED_DATE_DB_FIELD + " = ?, "
-                + LAST_MODIFIED_BY_DB_FIELD + " = ? WHERE id=? ";
+                + LAST_MODIFIED_BY_DB_FIELD + " = ?, version = version + 1 WHERE id=? AND version=?";
     }
 
     private String batchQueryForTransactionsUpdate() {
         return "UPDATE m_savings_account_transaction "
                 + "SET is_reversed=?, amount=?, overdraft_amount_derived=?, balance_end_date_derived=?, balance_number_of_days_derived=?, running_balance_derived=?, cumulative_balance_derived=?, is_reversal=?, "
                 + LAST_MODIFIED_DATE_DB_FIELD + " = ?, " + LAST_MODIFIED_BY_DB_FIELD + " = ? " + "WHERE id=?";
     }
-
-    private boolean isInterestAlreadyPostedForPeriod(SavingsAccountData savingsAccountData, LocalDate yesterday) {
-        LocalDate interestPostedTillDate = savingsAccountData.getSummary().getInterestPostedTillDate();
-        if (interestPostedTillDate == null) {
-            return false;
-        }
-        return !interestPostedTillDate.isBefore(yesterday);
-    }
 }
diff --git a/fineract-savings/src/test/java/org/apache/fineract/portfolio/savings/service/SavingsSchedularInterestPosterTest.java b/fineract-savings/src/test/java/org/apache/fineract/portfolio/savings/service/SavingsSchedularInterestPosterTest.java
@@ -0,0 +1,96 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.fineract.portfolio.savings.service;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.junit.jupiter.api.Test;
+
+class SavingsSchedularInterestPosterTest {
+
+    @Test
+    void testUpdateCountsZeroMeansVersionMismatch() {
+        int[] updateCounts = { 1, 0, 1 };
+        Set<Long> skippedAccountIds = new HashSet<>();
+        List<Long> accountIds = List.of(1L, 2L, 3L);
+
+        for (int i = 0; i < updateCounts.length; i++) {
+            if (updateCounts[i] == 0) {
+                skippedAccountIds.add(accountIds.get(i));
+            }
+        }
+
+        assertEquals(1, skippedAccountIds.size(), "Exactly one account should be skipped");
+        assertTrue(skippedAccountIds.contains(2L), "Account 2 should be skipped due to version mismatch");
+    }
+
+    @Test
+    void testAllVersionsMatchNoSkippedAccounts() {
+        int[] updateCounts = { 1, 1, 1 };
+        Set<Long> skippedAccountIds = new HashSet<>();
+        List<Long> accountIds = List.of(1L, 2L, 3L);
+
+        for (int i = 0; i < updateCounts.length; i++) {
+            if (updateCounts[i] == 0) {
+                skippedAccountIds.add(accountIds.get(i));
+            }
+        }
+
+        assertTrue(skippedAccountIds.isEmpty(), "No accounts should be skipped when all versions match");
+    }
+
+    @Test
+    void testAllVersionsMismatchAllSkipped() {
+        int[] updateCounts = { 0, 0, 0 };
+        Set<Long> skippedAccountIds = new HashSet<>();
+        List<Long> accountIds = List.of(1L, 2L, 3L);
+
+        for (int i = 0; i < updateCounts.length; i++) {
+            if (updateCounts[i] == 0) {
+                skippedAccountIds.add(accountIds.get(i));
+            }
+        }
+
+        assertEquals(3, skippedAccountIds.size(), "All 3 accounts should be detected as version mismatched");
+        assertTrue(skippedAccountIds.containsAll(List.of(1L, 2L, 3L)), "All account IDs should be in skipped set");
+    }
+
+    @Test
+    void testVersionMismatchSkipsFailedAccountAndProceedsWithOthers() {
+        int[] updateCounts = { 1, 0, 1 };
+        List<Long> accountIds = List.of(1L, 2L, 3L);
+        List<Long> successfulIds = new ArrayList<>();
+
+        for (int i = 0; i < updateCounts.length; i++) {
+            if (updateCounts[i] == 0) {
+                // account is skipped due to concurrent modification — logged, not thrown
+            } else {
+                successfulIds.add(accountIds.get(i));
+            }
+        }
+
+        assertEquals(2, successfulIds.size(), "Two accounts should proceed normally");
+        assertTrue(successfulIds.containsAll(List.of(1L, 3L)), "Accounts 1 and 3 should succeed independently");
+    }
+}
diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsInterestPostingJobIntegrationTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsInterestPostingJobIntegrationTest.java
