priyam99 opened a new pull request, #174: URL: https://github.com/apache/fineract-loan-origination/pull/174
## Description The mock Fineract adapter keeps its loan ID counter in memory, starting from 100000. Every restart resets it, so the next mock disbursement gets 100001 again even if an application already has that ID. The database accepted the duplicate without complaint, so two applications ended up pointing at the same "Fineract loan" and nothing flagged it. I fixed this in two places. First, before `MockFineractLoanApiClient` creates a loan, it now looks up the highest `fineract_loan_id` already stored and moves its counter past it. After a restart it carries on from where the data left off, so IDs are not reused. I read the value from `loan_application` rather than saving a separate "last ID" somewhere, so there is nothing extra to keep in sync, and it still works if the app was killed rather than shut down cleanly. The lookup is a small `MAX` query on `LoanApplicationRepository`. It returns only a number, so I left it unscoped by tenant and said so in its Javadoc. Second, I added a unique constraint on `(tenant_id, fineract_loan_id)`, both on the entity and as migration `V10`, so a duplicate fails loudly instead of being saved. The issue asks for uniqueness on `fineract_loan_id`. I scoped it by tenant because each Fineract tenant has its own database and numbers its loans independently: loan 5 in one tenant and loan 5 in another are both legitimate. NULLs are still allowed more than once, so applications that haven't reached Fineract are unaffected. One thing to be aware of: if a local database already contains duplicate IDs from the old behaviour, Hibernate's `ddl-auto: update` logs a warning and skips the constraint, and `V10` would fail once Flyway is running again (#158). Those rows need cleaning up first. Fresh databases are fine. I also updated the mock adapter section in `docs/workflows/disbursement.adoc`. Closes #157 ## Verification - [x] `./mvnw spotless:apply` - [x] `./mvnw clean verify`: 207 unit tests and 13 Cucumber scenarios pass, `BUILD SUCCESS` - [x] `./mvnw apache-rat:check`: passes - [ ] Frontend changes: not applicable, this is a backend-only change To check that the tests catch the bug, I removed both parts of the fix and ran the build again. The two restart-related unit tests and the restart Cucumber scenario failed with `expected: 100003L but was: 100001L`, and the duplicate scenario failed because the database accepted the second row. With the fix in place everything passes. I also started the app against an existing local database that had no constraint. Hibernate added it on startup, and inserting a second application with the same tenant and loan ID was rejected: ``` los_db=> INSERT INTO loan_application SELECT * FROM demo_copy; -- tenant 'default', loan 100001 ERROR: duplicate key value violates unique constraint "uk_loan_application_tenant_fineract_loan_id" DETAIL: Key (tenant_id, fineract_loan_id)=(default, 100001) already exists. los_db=> INSERT INTO loan_application SELECT * FROM demo_copy; -- tenant 'other-tenant', loan 100001 INSERT 0 1 ``` I couldn't run a disbursement end to end over HTTP locally, because staff login validates against a real Fineract instance. The Cucumber restart scenario covers that path against PostgreSQL instead. New tests: - with nothing stored yet, the mock still starts at 100001 - with 100002 already stored, a freshly started mock returns 100003 - consecutive loans stay unique even before the previous ID has been saved - (Cucumber) after a restart, the mock does not reuse a stored loan ID - (Cucumber) linking the same Fineract loan ID twice in one tenant is rejected - (Cucumber) the same Fineract loan ID can be used in two different tenants ## AI assistance (optional) - Tool / model: Claude Code (Claude Opus 5.5) - How it was used: helped investigate the issue, draft the fix and tests, and run the build and the before/after checks. I reviewed every line and am responsible for the change. ## Checklist - [x] This PR is one logical change, not a code dump. - [x] I added or updated tests, or explained why tests are not needed. - [x] New files carry the ASF license header. - [x] Docs (and `AGENTS.md`, if commands, paths, or architecture rules changed) are updated - [x] All commits are signed. - [x] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy) and the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html). -- 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]
