oluexpert99 opened a new pull request, #6181:
URL: https://github.com/apache/fineract/pull/6181
- ChartOfAccountsWorkbook.setNames builds a Tags named range per
account type from the
lookup-index entry, but null-checks the map it read the entry from
rather than the
entry itself: it reads Integer[] tagValueBeginEndIndexes from
accountTypeToBeginEndIndexesofAccountNames, then tests the map for
null and
dereferences tagValueBeginEndIndexes[0] and [1] inside the block.
- The map is an instance field that was already dereferenced on the
line above (.get(i)),
so it is never null at that point: the guard is always true and
protects nothing, while
reading as though it protects the array access. The value that can
be null is the
per-iteration array, from a Map.get miss.
- The sibling block immediately below, which builds the AccountName
named range, shows the
intended pattern and guards the array it dereferences. The Tags
block appears to be a
copy of it with the guarded variable left unchanged.
- Guard tagValueBeginEndIndexes instead, so a missing lookup-index
entry skips the Tags
named range rather than dereferencing null, matching the sibling
block.
- Note on reachability and on the test: with well-formed GL-account
data the null branch
cannot be reached. accountTypesNoDuplicatesList and the index map
are both derived from
the same glAccounts list, and setLookupTable only skips a put --
leaving a gap -- for an
account type whose account-name list is empty, which cannot happen
for a type that came
from an actual account. This is therefore a dead guard and a latent
NPE rather than a
reproducible failure, and the added ChartOfAccountsWorkbookTest is a
characterisation
test: it pins the working path (one Tags_<type> range per account
type, alongside the
AccountName_<type> range) and would catch an incorrect fix, but it
passes both with and
without this change, since no input reachable through the public API
can produce the
null entry. Reaching the null branch would require reflecting into
the populator's
internal map, which would test an implementation detail rather than
behaviour.
## 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](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests)
- [ ] 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](https://fineract.apache.org/docs/current/#_testing) for verifying the
changes made.
- [ ] Follow our [coding
conventions](https://cwiki.apache.org/confluence/display/FINERACT/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"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit).
Large changes can be made in a branch, with assistance. Ask for help on the
[developer mailing list](https://fineract.apache.org/#contribute).
- [ ] If merging this PR resolves a JIRA issue, I will mark that issue as
resolved and set "Fix Version/s" appropriately.
Your assigned reviewer(s) will follow our [guidelines for code
reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
