FINERACT-2732: migrate the savings and deposit integration tests to the Feign client (Tier 1) - #6241
Open
DeathGun44 wants to merge 16 commits into
Open
Conversation
…list The savings transaction search endpoint returns a page ordered by the request's orderBy/sortOrder, defaulting to most recent first, but its OpenAPI schema declared the results as a set. Generated clients therefore deserialised them into an unordered collection and lost that ordering, which no client can recover. The endpoint serialises a Spring Page to a JSON array either way, so the runtime wire format is unchanged; only the documented type is corrected. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The overdraft settings and the GL account mappings have always been accepted by the savings product endpoint - they are part of SAVINGS_PRODUCT_REQUEST_DATA_PARAMETERS, and a create and an update are validated against that one shared set - but none of them were on the Swagger model, so a typed client could not create an accrual-based or an overdraft-enabled product at all. Add them to PostSavingsProductsRequest and to PutSavingsProductsProductIdRequest, which repeats the whole product body for the same reason. These are additive request attributes, so the API backward compatibility check reports no violations. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…and search Adds the operations the savings tests need beyond the create-approve- activate lifecycle: interest posting and calculation, withdrawal by the applicant, account status and summary reads, transaction listing, forced withdrawal and transaction search. Introduces SavingsTestValidators as the typed replacement for the RestAssured SavingsStatusChecker, reading the generated status flags so a renamed field fails at compile time rather than silently returning null. The savingsProduct builder keeps the defaults of the legacy SavingsProductHelper so migrated tests continue to assert the same interest figures. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…eign Moves SavingsInterestPostingIntegrationTest, FlexibleSavingsInterestPostingIntegrationTest, SavingsAccountTransactionsSearchIntegrationTest and SavingsAccountForceWithdrawalTest onto FeignSavingsTestBase, dropping RestAssured along with the raw HashMap and Retrofit response handling. Assertions now read typed models: dates compare as LocalDate rather than by the string form of a list, amounts compare by value so the server's scale does not matter, and the interest posting rows are asserted to really be interest postings instead of trusting their position. The two search error cases now assert the exact status the original response specifications pinned, 400 and 404, instead of only requiring that the call failed. Each test restores just the global configurations it changed rather than resetting every configuration, and force withdrawal additionally verifies the resulting negative balance instead of only that the call succeeded. All 14 tests pass against a live server; the same 14 passed before the migration. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Adds the pieces the balance-check migration needs: reverse/undo of a savings transaction, a typed read of a single transaction, a withdrawal that is expected to be rejected, and a product creator for the settings the generated PostSavingsProductsRequest still cannot express. The single-transaction endpoint is declared as returning a bare String in the generated client, so the helper maps the JSON into the same SavingsAccountTransactionData the account endpoints return rather than handing callers a raw string. verifyFirstErrorCode reads errors[0].userMessageGlobalisationCode. The code on the exception itself is always the generic validation.msg.domain.rule.violation, so asserting on it would have quietly stopped checking which rule was violated. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Both tests now run through the typed savings helpers. The balance assertions compare BigDecimal by value so a change in the scale the server returns does not fail the test, and the insufficient-balance case asserts the specific error code instead of only that the call failed. Verified against a live server: 2/2 pass, matching the pre-migration baseline of 2/2. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Adds hold and release of a savings balance to the transaction helper and moves both tests onto it. The three commands that share the adjust endpoint - reverse, undo and releaseAmount - now go through one private method instead of repeating the empty request body. Balances are compared as BigDecimal by value rather than as floats, so the assertions no longer depend on the scale the server returns. The product needs an overdraft limit, which the generated PostSavingsProductsRequest does not carry, so it is created through the raw path. Verified against a live server: 2/2 pass, matching the pre-migration baseline of 2/2. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
The accrual GL mappings move into a shared builder so the accrual tests that follow can reuse them, and the product helper gains a raw update alongside the raw create - the generated PutSavingsProductsProductIdRequest carries only the description, the interest rate and the locale, so no typed call can change a product's accounting mappings. The update test now also asserts that the update landed on the same product rather than only inspecting whatever id came back. Verified against a live server: 3/3 pass, matching the pre-migration baseline of 3/3. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Both accrual classes now read their transactions and journal entries as typed models instead of digging through HashMaps, so a renamed field fails the build rather than silently reading null. The accrual filter and the accrual total move onto the transaction helper, where the other tests that need them can reach them. The accounting test drops the two GL accounts it created and then never used: the legacy product builder overwrote the explicit overdraftPortfolioControl and interestPayable mappings with the ones it derived from the account list, so those accounts never reached the server. The mappings the assertions depend on are unchanged. Product account mappings are named through constants rather than spelled out at each call site, and the existing overdraft tests were moved onto the same constants. Verified against a live server: 4/4 pass across both classes, matching the pre-migration baseline of 4/4. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…-as-on and datatables Three fields the server already reads or returns were missing from the OpenAPI models, so no generated client could reach them: postInterestManualOrAutomatic is validated by SavingsAccountTransactionDataValidator.validatePostInterest, but it was absent from PostSavingsAccountTransactionsRequest, leaving the postInterestAsOn command uncallable through the typed client. transactionId is carried by the CommandProcessingResult of a datatable update or delete whenever the datatable hangs off a transaction, but it was absent from both response models, so the savings transaction datatable tests could not read it back. Only the documented request and response shapes change; the endpoints accept and return the same JSON as before. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ts to feign Adds a typed datatable helper, replacing the RestAssured one that also reached back into the legacy Retrofit Calls client, and a postInterestAsOn transaction call. Both classes stop sniffing transaction types out of HashMaps: the type flags and the transaction date now come from the generated model, so the interest posting test loses the date-coercion helper that guessed between four formats and the int-cast of a Double transaction type id. The datatable read of a generic result set stays a JSON tree, and says why: the endpoint's shape follows the datatable's own columns, so there is no model to map it onto. Verified against a live server: 9/9 pass across both classes, matching the pre-migration baseline of 9/9. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
Adds FeignSavingsChargeHelper for the charge definition and the savings charge attachment, and the product-accounting and charge builders the test needs. The running balance assertion now expects 10129.5818 rather than 10129.582: the server value is unchanged, but the RestAssured version read it through JsonPath, which narrows JSON decimals to a float. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…need Two more fields the server already reads were missing from the OpenAPI models, so no generated client could send them. note is persisted against a savings transaction by SavingsAccountWritePlatformServiceJpaRepositoryImpl for deposits and withdrawals alike, and is listed in SAVINGS_ACCOUNT_TRANSACTION_REQUEST_DATA_PARAMETERS. withdrawalFeeForTransfers is accepted by SavingsAccountDataValidator.validateForUpdate like every other parameter of SAVINGS_ACCOUNT_REQUEST_DATA_PARAMETERS, and is what turns the transfer fee on for an account created without it. Also drops the javadoc added to postInterestManualOrAutomatic earlier in this branch; no other field in these DTOs carries one. Only the documented request shapes change; the endpoints already accepted both fields, so the runtime wire format is unchanged. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
…ches FeignSavingsHelper gains updateSavingsAccount, the typed PUT that changes a still-pending application. FeignAccountTransferHelper and FeignPaymentTypeHelper are new typed replacements for the RestAssured AccountTransferHelper and PaymentTypeHelper. BatchServiceHelper gains handleBatchAllowingFailure. A failed batch answers with the failing sub-request's status code, so the typed client raises it - but the body is still the list of sub-responses, which is what a caller testing contention or rollback needs to read. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
… to feign The last two Tier 1 classes. SavingsAccountTransactionTest keeps its parallel structure exactly: one thread per submission, both batch variants competing for the same account, and the two accounts taken in opposing orders so the deadlock test can still deadlock. The per-thread RestAssured RequestSpecification is gone - the Feign proxy is stateless over a cached thread-safe HTTP client, so one client is shared. Batch bodies are built from typed models instead of String.format JSON, and a losing thread is now checked for exactly SC_CONFLICT rather than any non-null status code. AccountTransferWithdrawalFeeTest was already partly typed but drove the account update through RestAssured; that was its only remaining RestAssured call, and it now goes through the typed PUT. Both classes were verified against a live server, 4/4 and 2/2, with the same counts passing at HEAD beforehand. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
executeAndAwaitJob only waited for a new run history row to appear with an end time. A job that finished with status "failure", or a run that a concurrent cron trigger produced rather than the test's own execute call, satisfied that wait, so the test carried on against data the job never produced and failed later somewhere unrelated - or passed for the wrong reason. Await the run history itself and then assert on it: trigger type "application", so the awaited run is the one the test asked for, and status "success". Both are test-only; no production behaviour changes. The helper is shared with FeignLoanTestBase, so the loan suites that drive jobs gain the same check. Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Tier 1 of moving the savings and deposit integration tests onto the generated Feign client, so they call the API through typed models instead of raw JSON. It covers the 14 smaller savings classes.
Production changes
Limited to four Swagger files.
Most are additive - optional fields that typed clients need but the schema does not
describe:
notetransactionIdon two datatable responseswithdrawalFeeForTransfersThe endpoints already accept and return these; only the documented shape changes.
One is not additive.
SavingsAccountTransactionsSearchResponse.contentis aListrather than aSet. The endpoint returns a page ordered by the request'sorderBy/sortOrder, and a set throws that order away in generated clients with no way to get it back. The response is a JSON array either way, so the wire format is unchanged.Backward compatibility
The CI gate was reproduced locally against a spec built from the merge base. No breaking changes. The two specs differ in 52 places - 51 additions and one removal, the
uniqueItemsflag a list does not carry.Notes on a few choices
success, so a failed job cannot look like a passing one. The helper is shared withFeignLoanTestBase, so the loan suites get the same check.Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Your assigned reviewer(s) will follow our guidelines for code reviews.