adamsaghy opened a new pull request, #6847: URL: https://github.com/apache/fineract/pull/6847
## Description [FINERACT-3004](https://issues.apache.org/jira/browse/FINERACT-3004): after the Spring Batch 6 upgrade, one loan failing a COB business step loses its whole chunk in Loan COB and Working Capital Loan COB. This takes the approach discussed on #6753: one transaction per chunk normally, and one per loan only when a chunk fails. ### Why it fails today - The business steps run in the item processor and join the chunk transaction, so a failure marks that transaction rollback-only. - Batch 6 skips a failed process inside the chunk transaction and never replays the chunk. The healthy loans processed with the failing one are rolled back with it, and their locks stay. - Saving the error on the lock fails as well. `requiresNewTransactionJdbcTemplate` uses the JDBC transaction manager, which does not see the JPA chunk transaction as an existing one. Its `REQUIRES_NEW` runs on the chunk's own connection, so the update is rolled back and the commit throws `UnexpectedRollbackException`. - Batch 6 calls `onProcessError` before the skip decision, so that exception fails the step: the job ends FAILED with process_skip_count 0. ### What changes - **Business steps run in the writer** (`AbstractCOBItemWriter`, which replaces `AbstractItemProcessor`). A failed write rolls the chunk back and enters scan mode, which writes each loan again in a transaction of its own. The failing loan is skipped and stays locked with its error. Every other loan commits its business steps, last closed business date and lock release together. A chunk with no failure still commits once. - **Each loan is loaded again** before its business steps. In scan mode the chunk items are detached copies that the rolled-back attempt may have changed; without the reload the steps are applied twice. - **The writer flushes before releasing the locks**, so an optimistic lock conflict or constraint violation fails the write, where scan mode handles it, instead of failing the commit and the step. - **The lock listener** saves errors through the JPA `requiresNewTransactionTemplate`, never throws, and saves the loan's own error on a skip in write. - **No retries on the COB worker steps.** Batch 6 retries inside the doomed chunk transaction, so a retry cannot succeed. `LOAN_COB_RETRY_LIMIT` and `WC_LOAN_COB_RETRY_LIMIT` are removed. - The COB processors are removed (Loan, inline Loan, Working Capital, inline Working Capital). The inline writers become `@JobScope`, as the inline processors were. A skipped loan is now counted in `write_skip_count` instead of `process_skip_count`. ### Tests - `COBWriterScanModeTest` (Testcontainers, PostgreSQL + EclipseLink) runs the real writer and listener in a fault-tolerant Batch 6 step configured like the COB worker steps: - one failing loan of three: the step completes with one write skip, the other two are closed with their steps applied once, and the failing loan keeps its lock with its own error and stack trace; - an optimistic lock conflict raised at flush is recovered when the loan is written again on its own; - the JDBC `REQUIRES_NEW` template inside a rollback-only JPA transaction throws `UnexpectedRollbackException` and loses the error, while the JPA one keeps it. - Removing the flush makes the optimistic-lock test end FAILED; removing the reload makes both scan tests see the business steps applied twice. - New `LoanItemWriterTest` and `ChunkProcessingLoanItemListenerTest`; `CobStepListenerRegistrationTest` and `WorkingCapitalLoanCOBWorkerStepScopeTest` updated; the processor and writer Cucumber features are replaced by the JUnit tests. - Run locally: the tests above, `BatchJpaTransactionTest`, `CobSequentialProcessingTest`, `AbstractLockingServiceTest`, the `fineract-provider` `@cob` Cucumber features (22 scenarios), Spotless and Checkstyle. Integration and E2E suites were not run locally. ## Checklist - [x] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests) - [ ] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers. - [x] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [x] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions). - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes - [ ] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). - [ ] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately. - [x] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
