[jira] [Resolved] (FINERACT-2668) Bulk-import download templates load the entire tenant (every client and every account) into the .xls — O(tenant) memory and file size for an O(rows-entered) operation; can OOM the server

["Farooq Ayoade (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/FINERACT-2668?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Farooq Ayoade resolved FINERACT-2668.
-------------------------------------
    Resolution: Fixed

> Bulk-import download templates load the entire tenant (every client and every account) into the .xls — O(tenant) memory and file size for an O(rows-entered) operation; can OOM the server
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: FINERACT-2668
>                 URL: https://issues.apache.org/jira/browse/FINERACT-2668
>             Project: Apache Fineract
>          Issue Type: Improvement
>          Components: DataImportTool
>            Reporter: Farooq Ayoade
>            Priority: Major
>
> h3. Observed behavior
> Requesting a bulk-import *download template* for a transaction-type entity (e.g. loan repayment) builds an Excel workbook that embeds the {*}entire tenant{*}:
>  * a hidden *{{Clients}}* sheet containing *every client* in the tenant (one row each),
>  * a hidden *{{Offices}}* sheet with every office,
>  * the data sheet itself carries lookup columns holding *every active account* in the tenant, plus a {{VLOOKUP}} array formula pre-written into a *hard-coded 3000 rows × 5 columns* block.
> The underlying fetches are {*}unbounded{*}. For loan repayment ({{{}BulkImportWorkbookPopulatorServiceImpl.populateLoanRepaymentWorkbook{}}}):
> {{List<ClientData>      clients = fetchClients(officeId);        // officeId null ⇒ retrieveAll(null), no LIMIT
> List<LoanAccountData> loans   = fetchLoanAccounts(officeId);   // officeId null ⇒ retrieveAll(null), no LIMIT}}
> {{ClientReadPlatformServiceImpl.retrieveAll(SearchParameters)}} only appends a {{LIMIT}} clause when search parameters request one; called with {{null}} it returns {*}all rows{*}. The result set is then materialised as POI {{HSSFCell}} objects in heap.
> On a tenant with, say, 100k clients an HQ-scoped user downloading a "simple repayment" template pulls 100k client rows + every active loan into memory and into a multi-megabyte {{{}.xls{}}}. Multiple concurrent downloads multiply the heap cost and can OOM the instance.
> h3. Why this is wasteful — the importer never reads the loaded data
> The {{Clients}} and {{Offices}} sheets exist *only* to drive Excel's cascading data-validation dropdowns (Office → Client → Account) during manual entry. The {*}import handler does not read them{*}. {{LoanRepaymentImportHandler.readLoanRepayment(...)}} resolves the loan purely from the typed account number:
> {{String loanaccountInfo = ImportHandlerUtils.readAsString(LoanRepaymentConstants.LOAN_ACCOUNT_NO_COL, row);
> loanAccountId = loanReadPlatformService.retrieveLoanIdByAccountNumber(loanAccountAr.get(0));}}
> So the server already does an authoritative server-side lookup on upload. The entire embedded client/office dataset is a data-entry convenience that costs O(tenant) to build and is discarded at import time. (Only the small {{Extras}} sheet — payment types — is read back, via {{{}getIdByName{}}}.)
> h3. Expected behavior
>  * A download template must not be O(tenant size). Lookup data must be *bounded* and/or *scoped* (by office hierarchy), and {*}omittable{*}.
>  * When a tenant exceeds a configurable lookup threshold, the template should *gracefully degrade* to a lean form (plain typed columns, no embedded client/office dataset, no 3000-row VLOOKUP block), relying on the server-side validation that already runs on upload.
>  * Existing callers that depend on the dropdown UX keep working (backward compatible by default).
> h3. Steps to reproduce
>  # On a tenant with a large client/loan base (or seed several thousand clients), authenticate as a user whose office is the head office.
>  # {{GET /fineract-provider/api/v1/loans/repayments/downloadtemplate}} (no {{{}officeId{}}}).
>  # Observe: response is a multi-MB {{{}.xls{}}}; the {{Clients}} sheet contains every client in the tenant; opening the file in Excel is slow (15,000 pre-baked VLOOKUP formulas recalc). Under concurrent requests, watch server heap.
> h3. Root cause
>  # *Unbounded fetch.* {{fetchClients(null)}} / {{fetchLoanAccounts(null)}} call {{{}retrieveAll(null){}}}, which applies no {{{}LIMIT{}}}.
>  # *Lookup sheets are mandatory and tenant-wide.* {{ClientSheetPopulator}} / {{OfficeSheetPopulator}} are always built and always span the whole (office-hierarchy-visible) tenant, even though the importer ignores them.
>  # *Hard-coded 3000-row formula block.* Each transaction populator's {{setDefaults(...)}} writes {{VLOOKUP}} formulas into rows 1..3000 regardless of how many rows the user will actually enter.
> This pattern is shared across the transaction-type templates (loan repayment, guarantor, savings/recurring/ fixed-deposit transactions, shared accounts, …), so the fix belongs in the shared populator path, not in any single handler.
> h3. Proposed fix
> Backward-compatible, opt-out-of-bloat design (default behaviour unchanged for small tenants):
>  # *Bound every lookup fetch.* Introduce a configurable cap (e.g. {{{}fineract.bulkimport.template.max-lookup-rows{}}}, default ~10,000). {{fetchClients}} / {{fetchLoanAccounts}} request at most that many rows; honour {{officeId}} scoping that already exists on the endpoint.
>  # *Make lookup sheets optional.* Add {{?includeLookups=\{true|false}}} to the download endpoints (default {{{}true{}}}). When {{{}false{}}}, skip {{ClientSheetPopulator}} / {{OfficeSheetPopulator}} and the in-sheet VLOOKUP cascade; emit a lean sheet of plain typed columns ({{{}Loan Account No.*{}}}, {{{}Amount Repaid*{}}}, {{{}Date*{}}}, {{{}Type*{}}}, payment-detail). Upload already validates the account number server-side.
>  # *Auto-degrade.* If the bounded count would exceed the cap, automatically fall back to the lean template and write a note cell explaining that dropdowns were omitted and the account number must be typed.
>  # *Stop hard-coding 3000 rows.* Replace the fixed 1..3000 VLOOKUP block with a small configurable default (or column/table references), and omit it entirely in lean mode.
> Pilot the change on the *loan-repayment* template (smallest, highest-traffic reproducer); the shared-path changes then extend to the other transaction templates in follow-ups.



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