[
https://issues.apache.org/jira/browse/FINERACT-2708?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Farooq Ayoade updated FINERACT-2708:
------------------------------------
Description:
Observed behavior
{{GET /v1/savingsaccounts/transactions/downloadtemplate}} returns HTTP 500 on a
tenant where a client has more than one savings account:
{noformat}
java.lang.IllegalArgumentException: The workbook already contains this name:
Account_maryam_yusuf_18_
at
org.apache.poi.hssf.usermodel.HSSFWorkbook.createName(HSSFWorkbook.java)
at
org.apache.fineract.infrastructure.bulkimport.populator.savings.SavingsTransactionsWorkbookPopulator.setNames(SavingsTransactionsWorkbookPopulator.java:240)
at ...SavingsTransactionsWorkbookPopulator.setRules(...)
at ...SavingsTransactionsWorkbookPopulator.populate(...)
{noformat}
The same defect is present in the recurring-deposit and fixed-deposit
transaction templates, which carry identical copies of the loop.
h3. Expected behavior
The template downloads successfully (HTTP 200). Each client contributes exactly
one {{Account_<client><id>}} Excel defined name (the dropdown source of that
client's accounts), regardless of how many accounts the client has.
h3. Steps to reproduce
Create client A with two or more savings accounts (e.g. "maryam yusuf").
Create client B whose display name differs from A's only in letter case (e.g.
"MARYAM YUSUF"), with at least one savings account. This is what triggers the
interleaving described below — two accounts on a single client are not
sufficient on their
own.
{{GET
/fineract-provider/api/v1/savingsaccounts/transactions/downloadtemplate?dateFormat=dd
MMMM yyyy}}
Observe HTTP 500 with {{{}The workbook already contains this name:
Account_<client><id>{}}}.
h3. Root cause
{{setNames()}} emits one {{Account_<client><id>}} defined name per client. It
collects the clients by walking the account lookup table and starting a new
"run" whenever the client name changes:
{code:java}
if (!clientName.equals(savingsAccounts.get(i).getClientName())) { //
case-SENSITIVE
...
clientsWithActiveSavings.add(clientName);
clientIdsWithActiveSavings.add(clientId);
}
{code}
That table was sorted immediately beforehand, in
{{{}populateSavingsTable(){}}}, with
{{SavingsAccountData.ClientNameComparator}} — which compares the names
case-insensitively:
{code:java}
String clientOfSavings1 =
savings1.getClientName().toUpperCase(Locale.ENGLISH);
String clientOfSavings2 =
savings2.getClientName().toUpperCase(Locale.ENGLISH);
return clientOfSavings1.compareTo(clientOfSavings2);
{code}
The two disagree. Names differing only in case compare equal to the comparator,
so the sort may interleave two such clients. The case-sensitive run detection
then sees the name change, change back, and starts a second run for the same
client,
appending it to {{clientsWithActiveSavings}} twice. The per-client loop that
follows asks POI to create a defined name that already exists:
{code:java}
for (int j = 0; j < clientsWithActiveSavings.size(); j++) {
Name name = workbook.createName();
setSanitized(name, "Account_" + clientsWithActiveSavings.get(j) + "" +
clientIdsWithActiveSavings.get(j) + "");
...
}
{code}
POI rejects the duplicate and the whole template download fails with HTTP 500.
This is why the symptom presents as "a client with more than one account" —
that is the necessary condition, but the trigger is the second client whose
name differs
only in case.
{{LoanRepaymentWorkbookPopulator.setNames()}} does not have this bug because it
guards the append:
{code:java}
if (!clientsWithActiveLoans.contains(clientName)) {
clientsWithActiveLoans.add(clientName);
clientIdsWithActiveLoans.add(clientId);
}
{code}
The guard is missing from the savings-transaction,
recurring-deposit-transaction and fixed-deposit-transaction populators.
h3. Proposed fix
Add the same guard to all three transaction populators, so a client name
already recorded is not added again and each client produces exactly one
{{Account_<client><id>}} defined name whatever its account count. Tenants that
never hit the
collision are unaffected — the guard only ever suppresses a duplicate.
Patch and tests: PR [#6180|https://github.com/apache/fineract/pull/6180].
was:
h3. Observed behavior
{{GET /v1/savingsaccounts/transactions/downloadtemplate}} returns HTTP 500 on a
tenant where a client has more than one savings account:
java.lang.IllegalArgumentException: The workbook already contains this name:
Account_maryam_yusuf_18_
at org.apache.poi.hssf.usermodel.HSSFWorkbook.createName(HSSFWorkbook.java)
at
org.apache.fineract.infrastructure.bulkimport.populator.savings.SavingsTransactionsWorkbookPopulator.setNames(SavingsTransactionsWorkbookPopulator.java:240)
at ...SavingsTransactionsWorkbookPopulator.setRules(...)
at ...SavingsTransactionsWorkbookPopulator.populate(...)
h3. Expected behavior
The template downloads successfully (HTTP 200). Each client contributes exactly
one {{Account_<client>_<id>_}} Excel defined name (the dropdown source of that
client's accounts), regardless of how many accounts the client has.
h3. Steps to reproduce
# Create a client with *two or more* savings accounts (e.g. client "Maryam
Yusuf", two active accounts).
# {{{}GET
/fineract-provider/api/v1/savingsaccounts/transactions/downloadtemplate?dateFormat=dd%20MMMM%20yyyy{}}}.
# Observe HTTP 500 with the {{The workbook already contains this name:
Account_<client>_<id>_}} exception.
h3. Root cause
{{SavingsTransactionsWorkbookPopulator.setNames()}} builds the per-client
account dropdown named ranges by walking the (client-name-sorted) account list
and, on each client-name transition, appending to {{clientsWithActiveSavings}}
{*}without a de-duplication guard{*}:
clientsWithActiveSavings.add(clientName); // no "already added?" check
clientIdsWithActiveSavings.add(clientId);
It then creates one Excel defined name per entry:
for (int j = 0; j < clientsWithActiveSavings.size(); j++) {
Name name = workbook.createName();
setSanitized(name, "Account_" + clientsWithActiveSavings.get(j) + "_" +
clientIdsWithActiveSavings.get(j) + "_");
...
}
When the same client name recurs in the account list, the client is added twice
and {{createName}} is called with the same {{Account_<client>_<id>_}} twice →
POI rejects the duplicate defined name → HTTP 500.
The *loan-repayment* template does not have this bug because
{{LoanRepaymentWorkbookPopulator.setNames()}} guards the append:
{{if (!clientsWithActiveLoans.contains(clientName)) \{
clientsWithActiveLoans.add(clientName);
clientIdsWithActiveLoans.add(clientId);
}}}
The same guard is missing from *savings-transaction,
recurring-deposit-transaction and fixed-deposit-transaction* populators.
h3. Proposed fix
Add the same de-duplication guard the loan-repayment populator already uses to
{{SavingsTransactionsWorkbookPopulator.setNames()}} (and the RD/FD-transaction
populators), so each client produces a single {{Account_<client>_<id>_}}
defined name.
> Bulk-import savings/recurring/fixed-deposit transaction download templates
> throw HTTP 500 (IllegalArgumentException: The workbook already contains this
> name) when a client has more than one account
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: FINERACT-2708
> URL: https://issues.apache.org/jira/browse/FINERACT-2708
> Project: Apache Fineract
> Issue Type: Bug
> Components: DataImportTool
> Reporter: Farooq Ayoade
> Priority: Minor
>
> Observed behavior
> {{GET /v1/savingsaccounts/transactions/downloadtemplate}} returns HTTP 500 on
> a tenant where a client has more than one savings account:
> {noformat}
> java.lang.IllegalArgumentException: The workbook already contains this
> name: Account_maryam_yusuf_18_
> at
> org.apache.poi.hssf.usermodel.HSSFWorkbook.createName(HSSFWorkbook.java)
> at
> org.apache.fineract.infrastructure.bulkimport.populator.savings.SavingsTransactionsWorkbookPopulator.setNames(SavingsTransactionsWorkbookPopulator.java:240)
> at ...SavingsTransactionsWorkbookPopulator.setRules(...)
> at ...SavingsTransactionsWorkbookPopulator.populate(...)
> {noformat}
> The same defect is present in the recurring-deposit and fixed-deposit
> transaction templates, which carry identical copies of the loop.
> h3. Expected behavior
> The template downloads successfully (HTTP 200). Each client contributes
> exactly one {{Account_<client><id>}} Excel defined name (the dropdown source
> of that client's accounts), regardless of how many accounts the client has.
> h3. Steps to reproduce
> Create client A with two or more savings accounts (e.g. "maryam yusuf").
> Create client B whose display name differs from A's only in letter case (e.g.
> "MARYAM YUSUF"), with at least one savings account. This is what triggers the
> interleaving described below — two accounts on a single client are not
> sufficient on their
> own.
> {{GET
> /fineract-provider/api/v1/savingsaccounts/transactions/downloadtemplate?dateFormat=dd
> MMMM yyyy}}
> Observe HTTP 500 with {{{}The workbook already contains this name:
> Account_<client><id>{}}}.
> h3. Root cause
> {{setNames()}} emits one {{Account_<client><id>}} defined name per client. It
> collects the clients by walking the account lookup table and starting a new
> "run" whenever the client name changes:
> {code:java}
> if (!clientName.equals(savingsAccounts.get(i).getClientName())) { //
> case-SENSITIVE
> ...
> clientsWithActiveSavings.add(clientName);
> clientIdsWithActiveSavings.add(clientId);
> }
> {code}
> That table was sorted immediately beforehand, in
> {{{}populateSavingsTable(){}}}, with
> {{SavingsAccountData.ClientNameComparator}} — which compares the names
> case-insensitively:
> {code:java}
> String clientOfSavings1 =
> savings1.getClientName().toUpperCase(Locale.ENGLISH);
> String clientOfSavings2 =
> savings2.getClientName().toUpperCase(Locale.ENGLISH);
> return clientOfSavings1.compareTo(clientOfSavings2);
> {code}
> The two disagree. Names differing only in case compare equal to the
> comparator, so the sort may interleave two such clients. The case-sensitive
> run detection then sees the name change, change back, and starts a second run
> for the same client,
> appending it to {{clientsWithActiveSavings}} twice. The per-client loop that
> follows asks POI to create a defined name that already exists:
> {code:java}
> for (int j = 0; j < clientsWithActiveSavings.size(); j++) {
> Name name = workbook.createName();
> setSanitized(name, "Account_" + clientsWithActiveSavings.get(j) + "" +
> clientIdsWithActiveSavings.get(j) + "");
> ...
> }
> {code}
> POI rejects the duplicate and the whole template download fails with HTTP
> 500. This is why the symptom presents as "a client with more than one
> account" — that is the necessary condition, but the trigger is the second
> client whose name differs
> only in case.
> {{LoanRepaymentWorkbookPopulator.setNames()}} does not have this bug because
> it guards the append:
> {code:java}
> if (!clientsWithActiveLoans.contains(clientName)) {
> clientsWithActiveLoans.add(clientName);
> clientIdsWithActiveLoans.add(clientId);
> }
> {code}
> The guard is missing from the savings-transaction,
> recurring-deposit-transaction and fixed-deposit-transaction populators.
> h3. Proposed fix
> Add the same guard to all three transaction populators, so a client name
> already recorded is not added again and each client produces exactly one
> {{Account_<client><id>}} defined name whatever its account count. Tenants
> that never hit the
> collision are unaffected — the guard only ever suppresses a duplicate.
> Patch and tests: PR [#6180|https://github.com/apache/fineract/pull/6180].
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
