Skip to content

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

Merged
adamsaghy merged 1 commit intoapache:developfrom
awaneetdecoder:FINERACT-1659-fix-optimistic-lock-savings-batch
Apr 17, 2026
Merged

FINERACT-1659: Fix optimistic locking in savings interest posting batch job#5671
adamsaghy merged 1 commit intoapache:developfrom
awaneetdecoder:FINERACT-1659-fix-optimistic-lock-savings-batch

Conversation

@awaneetdecoder
Copy link
Copy Markdown
Contributor

Problem

The savings interest posting batch job could post duplicate interest
when two instances ran concurrently. PR #5550 attempted a fix but
adamsaghy correctly identified there was no rollback mechanism —
version mismatch was silently skipped.

Fix

  • SQL uses WHERE id=? AND version=? to detect concurrent modification
  • version = version + 1 increments on each successful update
  • updateCounts[i] == 0 detected as concurrent modification
  • ConcurrentModificationException thrown on version mismatch
  • @transactional on postInterest() rolls back entire batch

Files Changed

  • SavingsAccountData.java — added version field
  • SavingsAccountReadPlatformServiceImpl.java — reads version from DB
  • SavingsSchedularInterestPoster.java — core fix
  • SavingsSchedularInterestPosterTest.java — unit tests
  • SavingsInterestPostingJobIntegrationTest.java — integration tests

Fixes FINERACT-1659
Supersedes PR #5550

@awaneetdecoder
Copy link
Copy Markdown
Contributor Author

I have force-pushed the fixes for the Lombok duplicate getters/setters and the Checkstyle trailing space. Could a maintainer please approve the workflows so the CI can run against the new commit? Thank you!

@adamsaghy
Copy link
Copy Markdown
Contributor

@awaneetdecoder Please review the failing checks!

@adamsaghy
Copy link
Copy Markdown
Contributor

Please run:
./gradlew spotlessApply spotbugsMain spotbugsTest checkstyleMain checkstyleTest
./gradlew --no-daemon build -x test -x cucumber -x doc

Before any PR or changes, please always run these two commands and make sure there is green build!

@awaneetdecoder awaneetdecoder force-pushed the FINERACT-1659-fix-optimistic-lock-savings-batch branch from 5fc3ca4 to b93060f Compare March 28, 2026 19:00
@awaneetdecoder
Copy link
Copy Markdown
Contributor Author

@adamsaghy I have addressed all the failing checks:

  1. Replaced ConcurrentModificationException with Spring's OptimisticLockingFailureException — this is semantically correct for optimistic locking failures, extends DataAccessException so it is caught by the existing handler in postInterest(), and satisfies SpotBugs.

  2. Removed unused mock infrastructure from the unit test — the 4 tests verify pure logic (updateCounts array checking) and have no dependency on Spring beans, so no mocks are needed.

  3. Ran both commands locallyspotlessApply spotbugsMain spotbugsTest checkstyleMain checkstyleTest and build -x test -x cucumber -x doc both passed cleanly. The :fineract-war:binaryDistTar failure is a pre-existing duplicate JAR issue reproducible on develop with no changes applied.

Thank you for your patience.

@awaneetdecoder awaneetdecoder force-pushed the FINERACT-1659-fix-optimistic-lock-savings-batch branch from b93060f to b5627c5 Compare March 30, 2026 04:57
@awaneetdecoder
Copy link
Copy Markdown
Contributor Author

@adamsaghy — investigated the test-core-2 failure.

testRunningPostInterestJobTwiceDoesNotCreateDuplicateInterest
was failing due to a date logic error in the test itself: with
businessDate = April 12, the scheduler calculates interest up to
yesterday (April 11), so the posting lands on [2022, 4, 11] —
not [2022, 4, 12] as the filter expected.

Fix: changed businessDate to April 13 so yesterday = April 12,
ensuring the interest posting appears on [2022, 4, 12] as the
assertion requires.

The E2E Shard 6 failure is pre-existing — it covers only Currency
and Loan domain features, unrelated to this PR's savings changes.

All other 473 integration tests pass. Updated the commit with the fix.

@awaneetdecoder awaneetdecoder force-pushed the FINERACT-1659-fix-optimistic-lock-savings-batch branch 3 times, most recently from 225b6be to 3ff3f86 Compare March 31, 2026 05:20
@awaneetdecoder
Copy link
Copy Markdown
Contributor Author

@adamsaghy — the previous filter on transactionType.value was incorrectly eliminating all transactions. Simplified to match the pattern used in testDuplicateOverdraftInterestPostingJob — filtering by date [2022, 4, 12] and reversed=false, which is sufficient since no other transaction lands on that date. test-core-2 is now passing on all 3 databases.
The 2 remaining failures are pre-existing and unrelated to this PR:

  • verify (build-core, main) fails on cyclonedxDirectBom due to an unresolvable Eclipse SWT POM (org.eclipse.swt.${osgi.platform}) — a known dependency resolution issue unrelated to savings changes.
  • E2E Shard 7 covers only Client and Loan domain features — no savings involvement.

41 checks passing. Ready for review.

@adamsaghy
Copy link
Copy Markdown
Contributor

SBOM is not a "real" error, it does not fail the build, however spotless is!

Execution failed for task ':integration-tests:spotlessJavaCheck'.
> The following files had format violations:
      src/test/java/org/apache/fineract/integrationtests/SavingsInterestPostingJobIntegrationTest.java
          @@ -247,10 +247,8 @@
           ············Object·transactionObj·=·this.savingsAccountHelper.getSavingsDetails(savingsId,·"transactions");
           ············ArrayList<HashMap<String,·Object>>·transactions·=·(ArrayList<HashMap<String,·Object>>)·transactionObj;
           
          -············long·interestPostingsCount·=·transactions.stream()
          -····················.filter(t·->·t.get("date").toString().equals("[2022,·4,·12]"))
          -····················.filter(t·->·t.get("reversed").toString().equals("false"))
          -····················.count();
          +············long·interestPostingsCount·=·transactions.stream().filter(t·->·t.get("date").toString().equals("[2022,·4,·12]"))
          +····················.filter(t·->·t.get("reversed").toString().equals("false")).count();
           
           ············assertEquals(1,·interestPostingsCount,·"Running·job·twice·must·not·create·duplicate·interest·postings·on·the·same·date");
           ········}·finally·{
          @@ -378,4 +376,4 @@
           ········globalConfigurationHelper.resetAllDefaultGlobalConfigurations();
           ········globalConfigurationHelper.verifyAllDefaultGlobalConfigurations();
           ····}
          -}
          +}
  Run './gradlew :integration-tests:spotlessApply' to fix these violations.

@awaneetdecoder awaneetdecoder force-pushed the FINERACT-1659-fix-optimistic-lock-savings-batch branch 3 times, most recently from 9b356e1 to dfbb9c0 Compare March 31, 2026 09:16
@adamsaghy adamsaghy closed this Mar 31, 2026
@adamsaghy adamsaghy reopened this Mar 31, 2026
@awaneetdecoder
Copy link
Copy Markdown
Contributor Author

@adamsaghy — all 43 checks are now passing on commit dfbb9c0. Spotless formatting has been resolved. The PR is ready for review. Thank you.

@awaneetdecoder
Copy link
Copy Markdown
Contributor Author

Hi @adamsaghy, PR #5646 merged successfully. This PR (#5671) also has all 43 checks passing and no conflicts. Could you please take a look when you get a chance? Thank you.

@awaneetdecoder awaneetdecoder force-pushed the FINERACT-1659-fix-optimistic-lock-savings-batch branch from dfbb9c0 to d469aaa Compare April 8, 2026 17:08
@adamsaghy adamsaghy requested a review from galovics April 17, 2026 10:09
@adamsaghy adamsaghy merged commit 64b21ef into apache:develop Apr 17, 2026
47 of 48 checks passed
@awaneetdecoder awaneetdecoder deleted the FINERACT-1659-fix-optimistic-lock-savings-batch branch April 17, 2026 12:43
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.

2 participants