[ 
https://issues.apache.org/jira/browse/FINERACT-2609?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18082247#comment-18082247
 ] 

Yvana Kengne commented on FINERACT-2609:
----------------------------------------

[~krishnamewara]  please can i work on PR 1?

>From the ticket’s migration tiers:

SavingsAccountHelper (~1,500 lines, ~97% RestAssured) has no Feign equivalent
It has 37 importers
BaseSavingsIntegrationTest still extends legacy IntegrationTest, initializes 
RestAssured, and uses legacy helpers; it should be retired only after a Feign 
savings base and helper exist (per the ticket decommission plan)
Savings is listed as the first core PR because no partial Feign helper exists 
yet, and later savings test migration does not depend on loan or client work 
(PR 2–4)

> Migrate integration test helpers from RestAssured to Feign client
> -----------------------------------------------------------------
>
>                 Key: FINERACT-2609
>                 URL: https://issues.apache.org/jira/browse/FINERACT-2609
>             Project: Apache Fineract
>          Issue Type: Improvement
>            Reporter: Krishna Mewara
>            Priority: Trivial
>              Labels: feign-migration, test-infrastructure
>
> h2. Overview
> Migrate Fineract's integration test infrastructure from raw RestAssured HTTP 
> calls to the
> type-safe Feign client ({{{}fineract-client-feign{}}}). Following the 
> Strangler Fig pattern —
> new Feign helpers coexist alongside legacy RestAssured helpers, tests 
> migrated incrementally.
> No big bang rewrite.
> To close: FINERACT-2454
> h2. Why migrate?
>  * RestAssured tests use raw JSON strings and untyped HTTP — brittle, no 
> compile-time safety
>  * Feign client is type-safe, auto-generated from the OpenAPI spec
>  * Catch API contract drift at compile time, not at runtime in CI
> h2. Current State
> ||Metric||Count||
> |Total helper files in {{common/}}|123|
> |Helpers with RestAssured only (unmigrated)|53|
> |Helpers with both RestAssured + Retrofit (mixed)|18|
> |Helpers already using Retrofit ({{{}Calls.ok{}}})|35|
> |Helpers with NO RestAssured (pure utility)|51|
> |Total test files (excluding {{common/}} and Feign)|312|
> |Test files directly importing RestAssured|197 of 312 (63%)|
> |Existing Feign test files|10|
> |Existing Feign helpers|17|
> |Existing Feign modules|7|
> |Existing wrapper interfaces|4|
> h2. Existing Feign Infrastructure
> These are already merged and in {{{}upstream/develop{}}}. Listed here so the 
> scope of remaining
> work is clear and we don't accidentally duplicate what already exists.
> h3. Base Classes
> ||Class||Role||Replaces||
> |{{FeignIntegrationTest}}|Root base for all Feign tests|{{IntegrationTest}} 
> (Retrofit-based)|
> |{{FeignLoanTestBase}}|Wires all loan helpers, ready for loan test 
> migration|{{BaseLoanIntegrationTest}} (decommission target)|
> h3. Helpers (under {{{}client/feign/helpers/{}}})
> ||Helper||Replaces||Coverage||
> |{{FeignLoanHelper}}|{{LoanTransactionHelper}} (3,250 lines)|~16% — core 
> lifecycle only|
> |{{FeignTransactionHelper}}|Extracted from 
> {{LoanTransactionHelper}}|repayment, chargeOff, reAge, inline COB|
> |{{FeignClientHelper}}|{{ClientHelper}} (1,207 lines)|~6 of ~50 methods|
> |{{FeignAccountHelper}}|{{AccountHelper}}|GL account lookup|
> |{{FeignBusinessDateHelper}}|{{BusinessDateHelper}}|Full functional 
> replacement|
> |{{FeignJournalEntryHelper}}|{{JournalEntryHelper}}|Journal verification|
> |{{FeignOfficeHelper}}|{{OfficeHelper}}|Full functional replacement|
> |{{FeignSchedulerHelper}}|{{SchedulerJobHelper}}|Full functional replacement|
> |{{FeignGlobalConfigurationHelper}}|{{GlobalConfigurationHelper}}|Full 
> functional replacement|
> |{{FeignWorkingCapitalLoanHelper}}|{{WorkingCapitalLoanHelper}}|WC loan ops|
> |{{{}FeignSearchHelper{}}}, {{{}FeignTaxComponentHelper{}}}, 
> {{FeignTaxGroupHelper}}|No legacy equivalent|New capability|
> |{{{}FeignNotificationHelper{}}}, {{{}FeignLoanOriginatorHelper{}}}, 
> {{FeignExternalEventHelper}}|No legacy equivalent|New capability|
> h2. Migration Tiers
> h3. Tier 1 — High-Impact
> ||Helper||Importers||RestAssured %||Feign existence||
> |{{Utils.java}}|309|100%|None — migrate last|
> |{{ClientHelper.java}}|216|~80%|{{FeignClientHelper}} (6 methods — extend in 
> PR 2)|
> |{{BaseLoanIntegrationTest.java}}|121|~95%|{{FeignLoanTestBase}} — exists, 
> decommission after charge coverage added|
> |{{LoanTransactionHelper.java}}|118|~55%|{{FeignLoanHelper}} + 
> {{FeignTransactionHelper}} — extend in PR 4|
> |{{SavingsAccountHelper.java}}|37|~97%|None — greenfield target for PR 1|
> h3. Tier 2 — Medium-Impact
> ||Helper||Importers||Feign Beachhead||
> |{{BusinessDateHelper}}|61|{{FeignBusinessDateHelper}} (done)|
> |{{AccountHelper}}|60|{{FeignAccountHelper}} (done)|
> |{{ChargesHelper}}|55|None — PR 3|
> |{{DelinquencyBucketsHelper}}|36|None — PR 3|
> |{{SchedulerJobHelper}}|35|{{FeignSchedulerHelper}} (done)|
> |{{GlobalConfigurationHelper}}|26|{{FeignGlobalConfigurationHelper}} (done)|
> |{{JournalEntryHelper}}|24|{{FeignJournalEntryHelper}} (done)|
> |{{OfficeHelper}}|19|{{FeignOfficeHelper}} (done)|
> |{{GroupHelper}}|17|None — PR 5|
> h3. Tier 3 — Low-Impact / Niche
> {{BatchHelper}} (7 importers), {{{}FixedDepositAccountHelper{}}}, 
> {{{}RecurringDepositAccountHelper{}}},
> and domain-specific helpers with <10 importers each.
> h3. Foundation Layer
> {{Utils.java}} (309 importers) is the RestAssured gateway that nearly all 
> helpers delegate to.
> {*}Deprecate last{*}, only after all helpers above are migrated. The 
> RA-specific methods
> ({{{}initializeRESTAssured(){}}}, HTTP request wrappers) can be 
> {{@Deprecated}} progressively —
> but {{uniqueRandomStringGenerator()}} and 
> {{loginIntoServerAndGetBase64EncodedAuthenticationKey()}}
> are used widely even in non-RA code, so those stay until their callers are 
> migrated.
> h2. Architecture Pattern
> New {{FeignXxxHelper}} classes live under {{{}client/feign/helpers/{}}}. They:
>  * Take {{FineractFeignClient}} via constructor injection
>  * Use {{ok(() -> ...)}} for API calls (via {{FeignCalls}} utility)
>  * Tests extend {{FeignIntegrationTest}} base class
>  * Share the same model classes ({{{}org.apache.fineract.client.models.*{}}}) 
> as Retrofit
> Legacy and Feign helpers coexist — both {{fineract-client}} (Retrofit) and
> {{fineract-client-feign}} are on the classpath simultaneously. The 
> {{exclude}} in
> {{dependencies.gradle}} prevents duplicate model classes.
> h2. Key Design Decisions
>  # *Separate Feign helper classes* — not dual methods in existing helpers
>  # Both clients share model classes — no conflicts
>  # Thin wrapper interfaces only for APIs with unreadable generated names 
> (e.g. {{create6()}} → {{{}createClient(){}}})
>  # Coexistence over replacement — 197 test files still use RestAssured, 
> helper migration enables but doesn't force test-level migration
> h2. Decommission Plan
> These classes will be retired once their Feign replacement has full method 
> coverage.
> Both old and new coexist until importer counts reach zero.
> ||Class||Importers||Depends On||
> |{{BaseLoanIntegrationTest}}|121|PR 4 (charge management added to 
> {{{}FeignLoanHelper{}}})|
> |{{BaseSavingsIntegrationTest}}|—|PR 1 ({{{}FeignSavingsHelper{}}} + 
> {{{}FeignSavingsTestBase{}}})|
> h2. PR Roadmap
> h3. Core PRs
> ||PR||Scope||Status||
> |PR 1|{{FeignSavingsHelper}} + {{FeignSavingsTestBase}} + first savings 
> test|Not Started|
> |PR 2|Extend {{FeignClientHelper}} (update, delete, search, status 
> transitions)|Not Started|
> |PR 3|{{FeignChargesHelper}} + {{FeignDelinquencyHelper}}|Not Started|
> |PR 4|Extend {{FeignLoanHelper}} — charge management + remaining disbursal 
> methods|Not Started|
> |PR 5|{{FeignGroupHelper}} + {{FeignCenterHelper}}|Not Started|
> |PR 6|Migrate 5–10 simple test files end-to-end from RestAssured to Feign|Not 
> Started|
> h3. Stretch PRs
> ||PR||Scope||
> |PR S1|Batch migrate {{BaseLoanIntegrationTest}} importers to 
> {{FeignLoanTestBase}} (after PR 4)|
> |PR S2|{{FeignFixedDepositHelper}} + {{FeignRecurringDepositHelper}}|
> |PR S3|{{@Deprecated}} on RestAssured-specific methods in {{Utils.java}}|
> h2. Progress Log
> _Updated after each PR merge._
> ||Date||PR||Ticket||Notes||
> | | | | |



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to