Skip to content

FINERACT-2668: Stop bulk-import loan-repayment template loading the entire tenant#6071

Open
oluexpert99 wants to merge 1 commit into
apache:developfrom
TECHSERVICES-LIMITED:bugfix/FINERACT-2668
Open

FINERACT-2668: Stop bulk-import loan-repayment template loading the entire tenant#6071
oluexpert99 wants to merge 1 commit into
apache:developfrom
TECHSERVICES-LIMITED:bugfix/FINERACT-2668

Conversation

@oluexpert99

Copy link
Copy Markdown
Contributor
  • Downloading the loan-repayment template (GET /v1/loans/repayments/downloadtemplate) eagerly fetched every
    client (ClientReadPlatformService.retrieveAll(null) applies no LIMIT) and every active loan, embedding them
    as a tenant-wide Clients sheet, a per-loan lookup table and a 3000-row x 5 VLOOKUP block. The cost is
    O(tenant size) -- multi-MB .xls, slow Excel open, and server heap pressure / OOM risk under concurrent
    downloads -- for an operation that is conceptually O(rows entered). The import handler never reads the
    Clients/Offices sheets; it resolves the loan from the typed account number
    (LoanRepaymentImportHandler -> retrieveLoanIdByAccountNumber), so the embedded dataset is discarded at import.
    - Add an optional lean template, selected by a new LookupMode {AUTO, INCLUDE, EXCLUDE}.
    BulkImportWorkbookPopulatorService gains a 5-arg getTemplate(..., LookupMode); the existing 4-arg signature
    becomes a default delegating with AUTO, so all other callers and templates are unchanged.
    - LoansApiResource#getLoanRepaymentTemplate exposes ?includeLookups={true|false}, mapped to a LookupMode
    (null -> AUTO, true -> INCLUDE, false -> EXCLUDE). INCLUDE keeps the previous unbounded behaviour (the caller
    accepts the full cost). EXCLUDE always produces a lean template. AUTO bounds the client/loan fetch with a SQL
    LIMIT and degrades to lean when either set exceeds fineract.bulk-import.template.max-lookup-rows
    (default 10000), surfaced through FineractProperties.
    - LoanRepaymentWorkbookPopulator gains a lean mode (static lean()/full() factories) that omits the
    client/office sheets, the per-loan lookup table and the 3000-row VLOOKUP block, while keeping the data-entry
    column layout intact (the import handler parses by fixed column index) and retaining the payment-type
    dropdown sourced from the small Extras sheet (extracted to a shared addPaymentTypeValidation()).
    - Add unit tests: LookupMode mapping; the max-lookup-rows cap boundary; and lean-mode output asserting the
    Clients/Offices sheets and per-loan lookup rows are omitted while the data-column headers are preserved.

Description

Describe the changes made and why they were made. (Ignore if these details are present on the associated Apache Fineract JIRA ticket.)

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per our guidelines
  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
  • Create/update unit or integration tests for verifying the changes made.
  • Follow our coding conventions.
  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes
  • This PR must not be a "code dump". Large changes can be made in a branch, with assistance. Ask for help on the developer mailing list.

Your assigned reviewer(s) will follow our guidelines for code reviews.

…ntire tenant

- Downloading the loan-repayment template (GET /v1/loans/repayments/downloadtemplate) eagerly fetched every
  client (ClientReadPlatformService.retrieveAll(null) applies no LIMIT) and every active loan, embedding them
  as a tenant-wide Clients sheet, a per-loan lookup table and a 3000-row x 5 VLOOKUP block. The cost is
  O(tenant size) -- multi-MB .xls, slow Excel open, and server heap pressure / OOM risk under concurrent
  downloads -- for an operation that is conceptually O(rows entered). The import handler never reads the
  Clients/Offices sheets; it resolves the loan from the typed account number
  (LoanRepaymentImportHandler -> retrieveLoanIdByAccountNumber), so the embedded dataset is discarded at import.
- Add an optional lean template, selected by a new LookupMode {AUTO, INCLUDE, EXCLUDE}.
  BulkImportWorkbookPopulatorService gains a 5-arg getTemplate(..., LookupMode); the existing 4-arg signature
  becomes a default delegating with AUTO, so all other callers and templates are unchanged.
- LoansApiResource#getLoanRepaymentTemplate exposes ?includeLookups={true|false}, mapped to a LookupMode
  (null -> AUTO, true -> INCLUDE, false -> EXCLUDE). INCLUDE keeps the previous unbounded behaviour (the caller
  accepts the full cost). EXCLUDE always produces a lean template. AUTO bounds the client/loan fetch with a SQL
  LIMIT and degrades to lean when either set exceeds fineract.bulk-import.template.max-lookup-rows
  (default 10000), surfaced through FineractProperties.
- LoanRepaymentWorkbookPopulator gains a lean mode (static lean()/full() factories) that omits the
  client/office sheets, the per-loan lookup table and the 3000-row VLOOKUP block, while keeping the data-entry
  column layout intact (the import handler parses by fixed column index) and retaining the payment-type
  dropdown sourced from the small Extras sheet (extracted to a shared addPaymentTypeValidation()).
- Add unit tests: LookupMode mapping; the max-lookup-rows cap boundary; and lean-mode output asserting the
  Clients/Offices sheets and per-loan lookup rows are omitted while the data-column headers are preserved.

Signed-off-by: oluexpert99 <farooq@techservicehub.io>

// FINERACT-2668: max tenant-wide lookup rows (clients/loans) embedded in a download template before it
// auto-degrades to a lean form (typed keys only). Honoured by templates that opt in (loan repayment).
private int maxLookupRows = 10000;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for hardcoded value

@adamsaghy

Copy link
Copy Markdown
Contributor

I am having some concerns... limiting the number of fetched data, limiting the data you got... What if you need all 20k clients or loan? Maybe i dont fully understand this functionality... can you please help me to understand what these templates, downloadTemplates, bulk-import is doing exactly and used for what purpose?

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