Skip to content

FINERACT-2454: Migrate ClientLoanAccountLockIntegrationTest from RestAssured to feign client#5670

Open
Dpk376 wants to merge 1 commit intoapache:developfrom
Dpk376:feature/FINERACT-2441-feign-migration-loanlocktest
Open

FINERACT-2454: Migrate ClientLoanAccountLockIntegrationTest from RestAssured to feign client#5670
Dpk376 wants to merge 1 commit intoapache:developfrom
Dpk376:feature/FINERACT-2441-feign-migration-loanlocktest

Conversation

@Dpk376
Copy link
Copy Markdown
Contributor

@Dpk376 Dpk376 commented Mar 21, 2026

Description

Relates to FINERACT-2454

Migrates ClientLoanAccountLockIntegrationTest from RestAssured to the type-safe feign client.

Changes

  • Replaced RequestSpecification/ResponseSpecification boilerplate with extends BaseLoanIntegrationTest
  • Removed @BeforeEach setup, clientHelper, loanTransactionHelper, loanAccountLockHelper instance fields
  • Replaced ClientHelper.createClient(requestSpec, responseSpec)ClientHelper.addClientAsPerson()
  • Replaced LoanProductTestBuilder + getLoanProductId()createLoanProduct(createOnePeriod30DaysLongNoInterestPeriodicAccrualProduct())
  • Replaced LoanApplicationTestBuilder + getLoanId()applyLoan(applyLoanRequest(...))
  • Replaced approveLoan(String, Integer)approveLoan(Long, PostLoansLoanIdRequest)
  • Replaced disburseLoanWithNetDisbursalAmount()disburseLoan(Long, String, Double)
  • Replaced LoanStatusChecker HashMap assertions → verifyLoanStatus(loanId, LoanStatus.*)
  • Replaced LoanAccountLockHelper.placeSoftLockOnLoanAccount() (RestAssured) → FineractClientHelper.getFineractClient().legacy.placeLockOnLoanAccount() (feign)
  • Replaced clientHelper.retrieveLockedAccounts() instance call → FineractClientHelper.getFineractClient().loanAccountLockApi.retrieveLockedAccounts() (feign)

Result

134 lines → 65 lines. Zero RestAssured imports remaining.

No behavioral changes — the test still verifies that a soft-locked loan account appears in the locked accounts list.

@Dpk376 Dpk376 force-pushed the feature/FINERACT-2441-feign-migration-loanlocktest branch from 4d41ada to 9b56efc Compare March 24, 2026 12:07
@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!

@Dpk376 Dpk376 force-pushed the feature/FINERACT-2441-feign-migration-loanlocktest branch 2 times, most recently from 2f9f7c3 to 0e6b7d0 Compare March 27, 2026 16:44
@Dpk376
Copy link
Copy Markdown
Contributor Author

Dpk376 commented Mar 28, 2026

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!

Sure Adam I will be following for this and subsequent PR's. Thank you for the guidance

@Dpk376 Dpk376 force-pushed the feature/FINERACT-2441-feign-migration-loanlocktest branch from 0e6b7d0 to 1872f06 Compare March 30, 2026 08:31
@adamsaghy
Copy link
Copy Markdown
Contributor

org.apache.fineract.integrationtests.LoanWithdrawnByApplicantIntegrationTest
  
    Test loanWithdrawnByApplicant() FAILED
  
    org.apache.fineract.client.util.CallFailedRuntimeException: HTTP failed: Request{method=POST, url=https://localhost:8443/fineract-provider/api/v1/clients, tags={class retrofit2.Invocation=org.apache.fineract.client.services.ClientApi.createClient() [class PostClientsRequest {
        activationDate: 01 January 2012
        active: true
        address: null
        datatables: null
        dateFormat: dd MMMM yyyy
        dateOfBirth: null
        emailAddress: null
        externalId: null
        firstname: Gage
        fullname: null
        groupId: null
        lastname: Hayes
        legalFormId: null
        locale: en
        middlename: null
        mobileNo: null
        officeId: 1
    }]}}; Response{protocol=http/1.1, code=400, message=, url=https://localhost:8443/fineract-provider/api/v1/clients}; errorBody: {"developerMessage":"The request was invalid. This typically will happen due to validation errors which are provided.","httpStatusCode":"400","defaultUserMessage":"Validation errors exist.","userMessageGlobalisationCode":"validation.msg.validation.errors.exist","errors":[{"defaultUserMessage":"The parameter `legalFormId` is mandatory.","parameterName":"legalFormId","developerMessage":"The parameter `legalFormId` is mandatory.","userMessageGlobalisationCode":"validation.msg.client.legalFormId.cannot.be.blank","args":[{}]}]}
        at app//org.apache.fineract.integrationtests.LoanWithdrawnByApplicantIntegrationTest.loanWithdrawnByApplicant(LoanWithdrawnByApplicantIntegrationTest.java:33)
  

@Dpk376 Dpk376 force-pushed the feature/FINERACT-2441-feign-migration-loanlocktest branch 2 times, most recently from 9c72737 to afe701e Compare March 31, 2026 06:24
@Dpk376
Copy link
Copy Markdown
Contributor Author

Dpk376 commented Apr 2, 2026

Hi @adamsaghy

Regarding my CI check failure

CI OOMs in :fineract-client-feign:buildJavaSdk when it runs concurrently with :fineract-client:buildJavaSdk; I added a SharedBuildService limiter (maxParallelUsages=1) applied to all OpenAPI GenerateTasks so codegen can’t run concurrently.

Is this fine ?

If you say yes i can push the build.gradle with updated changes . Or can you say any alternatives. Since in my local it is green build

@Dpk376 Dpk376 force-pushed the feature/FINERACT-2441-feign-migration-loanlocktest branch 3 times, most recently from a692c54 to 3dbe2bb Compare April 3, 2026 08:28
@Dpk376
Copy link
Copy Markdown
Contributor Author

Dpk376 commented Apr 3, 2026

Hi @adamsaghy

This PR is ready to merge.

@adamsaghy
Copy link
Copy Markdown
Contributor

@Dpk376 Please rebase.

@Dpk376 Dpk376 force-pushed the feature/FINERACT-2441-feign-migration-loanlocktest branch from 3dbe2bb to 0f7ddb9 Compare April 7, 2026 06:14
@Dpk376
Copy link
Copy Markdown
Contributor Author

Dpk376 commented Apr 7, 2026

@Dpk376 Please rebase.

Hi @adamsaghy

Rebased on latest develop and resolved the conflicts. Ran all quality checks (spotless, spotbugs, checkstyle) and full build locally - all passing.

Note: CI may occasionally fail with OOM during concurrent buildJavaSdk tasks (:fineract-client:buildJavaSdk and :fineract-client-feign:buildJavaSdk) - this is a known infrastructure issue unrelated to this change. need to re-trigger if it occurs.

Thanks!

@adamsaghy
Copy link
Copy Markdown
Contributor

@Dpk376 Please rebase this PR with latest develop branch. Many of the checks were skipped due to incorrect dependency in Github Actions CI jobs!

…Assured to fineract-client-feign

- Migrates ClientLoanAccountLockIntegrationTest to use fineract-client-feign
- Replaces RestAssured based API calls with Feign client
- Fixes getObligeeData handling for JSON array responses
- Updates OpenAPI schema for obligeedetails endpoint
- Applies related integration test adjustments
@Dpk376 Dpk376 force-pushed the feature/FINERACT-2441-feign-migration-loanlocktest branch from 0f7ddb9 to e2d4a22 Compare April 8, 2026 11:10
@Dpk376
Copy link
Copy Markdown
Contributor Author

Dpk376 commented Apr 8, 2026

@Dpk376 Please rebase this PR with latest develop branch. Many of the checks were skipped due to incorrect dependency in Github Actions CI jobs!


Hi @adamsaghy,

Rebased onto latest develop (now at 1c59ecb6). The startup_failure on the previous CI run was caused by the gradle/actions bump to v6.1.0 in develop (later reverted in #5746) — my last rebase landed during that broken window. The new CI run should use the restored v5.0.2 and complete fully.

Thanks!


@Dpk376
Copy link
Copy Markdown
Contributor Author

Dpk376 commented Apr 9, 2026

Hi @adamsaghy

This is a memory issue that occurs when the Java Virtual Machine (JVM) runs out of allocated heap memory while building the Java SDK. That's why 1 check failed rerunning will resolve this

@Dpk376
Copy link
Copy Markdown
Contributor Author

Dpk376 commented Apr 12, 2026

Hi @adamsaghy

This is ready for merging . Let me know if anything needed from my side

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