Farooq Ayoade created FINERACT-2668:
---------------------------------------
Summary: 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
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)
