FINERACT-2460: Add Client Performance API for real-time financial metrics#5410
FINERACT-2460: Add Client Performance API for real-time financial metrics#5410nidhiii128 wants to merge 1 commit intoapache:developfrom
Conversation
nidhiii128
changed the title
|
@nidhiii128 Apache Fineract != Mifos please join the Apache Fineract list, involve with the Apache Fineract community. If you are interested please open an account in Jira, then review if the item has not been previously reported if not then you can open a new ticket https://selfserve.apache.org/jira-account.html Read and understand how to submit contibutions to Apache Fineract |
Really sorry to misunderstand things, will understand more about the fineract project and help to contribute. Thank you for your the information. |
|
Don worry, your effort can be applied using the proper Jira ticket also Apache Fineract will be a GSOC participant. |
nidhiii128
changed the title
There was a problem hiding this comment.
@nidhiii128 the PR and the commit must follow the title conventions, please use "Fineract-2460: Add Client Performance API for real-time financial metrics"
Squash and commit your changes because only 1 commit per PR is required.
Why not to use the https://{URL_BASE}/fineract-provider/api/v1/clients/{CLIENT_ID}/accounts endpoint ?
nidhiii128
changed the title
nidhiii128
changed the title
nidhiii128
changed the title
/accounts returns a large array of objects (full account summaries for every loan). my peformance endpoint returns exactly two numbers. Calculating the total on the database side (via SUM and COALESCE query) is significantly faster than fetching 50 loan objects and summing them in the Java frontend or mobile app. By returning exactly two numbers instead of a large object tree, we reduce the time-to-first-render for the Client Profile summary, which is critical for performance in low-bandwidth environments. 
The endpoint returns groupLoanIndividualMonitoringAccounts and guarantorAccounts. If the user just wants to see their "Total Balance," they don't need to know about guarantor status. Even if these arrays were full of loans, the user would still be seeing 0.00$ for "Performance" because this endpoint doesn't sum up the totals for them. |
| @Consumes({ MediaType.APPLICATION_JSON }) | ||
| @Produces({ MediaType.APPLICATION_JSON }) | ||
| @Operation(summary = "Retrieve client performance metrics", description = "Returns a summary of active loans and outstanding balances for a specific client.") | ||
| @ApiResponses({ |
There was a problem hiding this comment.
Remove ApiResponses anotation, It is not needed in newer java versions
| final String sql = "SELECT " + "(SELECT COUNT(*) FROM m_loan WHERE client_id = c.id AND loan_status_id = 300) as activeLoans, " | ||
| + "(SELECT COALESCE(SUM(principal_outstanding_derived + interest_outstanding_derived + fee_charges_outstanding_derived + penalty_charges_outstanding_derived), 0) " | ||
| + " FROM m_loan WHERE client_id = c.id AND loan_status_id = 300) as totalOutstanding " | ||
| + "FROM m_client c WHERE c.id = ?"; |
There was a problem hiding this comment.
Its a pure string concatenaton java 15 + can use textblock as it willl make SQL more readable
There was a problem hiding this comment.
What about the use of StringBuilder?
There was a problem hiding this comment.
StringBuilder will help if the sql written here was dynamic but i can see there is a simple static string. In that case there is no performance gain using this. This will make it harder to review lateron
There was a problem hiding this comment.
like this will be much better as it will compile only once
`
final String sql = """
SELECT
(SELECT COUNT(*)
FROM m_loan
WHERE client_id = c.id
AND loan_status_id = 300) AS activeLoans,
(SELECT COALESCE(
SUM(principal_outstanding_derived
+ interest_outstanding_derived
+ fee_charges_outstanding_derived
+ penalty_charges_outstanding_derived), 0)
FROM m_loan
WHERE client_id = c.id
AND loan_status_id = 300) AS totalOutstanding
FROM m_client c
WHERE c.id = ?
""";
`
| final String sql = "SELECT " + "(SELECT COUNT(*) FROM m_loan WHERE client_id = c.id AND loan_status_id = 300) as activeLoans, " | ||
| + "(SELECT COALESCE(SUM(principal_outstanding_derived + interest_outstanding_derived + fee_charges_outstanding_derived + penalty_charges_outstanding_derived), 0) " | ||
| + " FROM m_loan WHERE client_id = c.id AND loan_status_id = 300) as totalOutstanding " | ||
| + "FROM m_client c WHERE c.id = ?"; |
There was a problem hiding this comment.
What about the use of StringBuilder?
|
@IOhacker, regarding the StringBuilder, as it is traditionally used in Fineract to manage multi-line SQL. However, I believe Java 15 Text Blocks are the more technically sound choice here. |
|
@nidhiii128 why use Sql and why not JPQL/JQL? |
|
|
@Aman-Mittal I have updated the ClientsApiResource.java to remove the redundant @ApiResponses wrappers for methods with a single response code. I've also converted the SQL query in ClientReadPlatformServiceImpl.java into a Java 15 Text Block for better readability and verified the output locally via Swagger/curl. Ready for review! |
nidhiii128
changed the title
|
@nidhiii128 fyi https://issues.apache.org/jira/browse/FINERACT-2022 |
I have verified that the SQL query uses only standard ANSI SQL functions (like SUM and JOIN) that are fully compatible with both MariaDB and PostgreSQL. I have avoided any database-specific functions (like date formatting) that caused issues in the past. I am also monitoring the PostgreSQL integration tests in GitHub Actions to ensure 100% compliance. Regarding FINERACT-2022, I understand that JdbcTemplate is being phased out for QueryDSL, and I will keep this architectural goal in mind for future contributions |
| } catch (EmptyResultDataAccessException e) { | ||
| throw new ClientNotFoundException(clientId,e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Current implementation performs two queries and scans m_loan twice via correlated subqueries. This can be optimized into a single LEFT JOIN + aggregate, avoiding redundant DB round trips and duplicate table scans.
Why this is important? because it can degrade the performance if client have huge amount of data,
It is also recommended that indexes are maintained before using this type of aggregation.
There was a problem hiding this comment.
@Aman-Mittal I have updated the implementation to use a single LEFT JOIN with GROUP BY as suggested. This has eliminated the redundant database round trips and the duplicate scans of the m_loan table. I have also verified the fix locally and ensured the exception chaining correctly preserves the root cause.
There was a problem hiding this comment.
can you add appopriate index for this too? with compound index.
this will make this query faster.
you can check existing implementations in codebase
There was a problem hiding this comment.
@Aman-Mittal , thank you for the feedback. I have updated the implementation with the following optimizations:
- Rewrote the logic to use a single LEFT JOIN with GROUP BY. This avoids redundant database round trips and eliminates the duplicate scans of the m_loan table.
- Added a Liquibase migration (0209_add_index_for_client_performance_api.xml) to create a compound index on m_loan(client_id, loan_status_id). This ensures the query remains performant even for clients with a large number of loan accounts. ( taking reference from the existing implementations in codebase)
- Refined the exception handling to use EmptyResultDataAccessException for detecting missing clients while preserving the ClientNotFoundException for API consistency.
Verified the changes locally with Checkstyle and compilation.
nidhiii128
force-pushed
the
develop
branch
2 times, most recently
from
27837c2 to
abd31cc
Compare
| @@ -0,0 +1,12 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
add this here
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.3.xsd"> | ||
|
|
||
| <changeSet id="1" author="nidhiii128"> |
There was a problem hiding this comment.
author will be fineract here
nidhiii128
force-pushed
the
develop
branch
2 times, most recently
from
1c26881 to
60b8f4e
Compare
|
@IOhacker I have updated with all requested changes. |
|
@nidhiii128 Github Actions are running now |
3 participants
Description
This project implements a robust API endpoint and service layer to retrieve and display performance-related data for a client within the Apache Fineract ecosystem. The implementation focuses on aggregating real-time financial metrics, including active loan counts and a comprehensive "Total Outstanding Balance" (Principal + Interest + Fees + Penalties) for active loan accounts.
Acceptance Criteria & Implementation Proof
What I Did: Developed a RESTful endpoint GET /clients/{clientId}/performance within the ClientsApiResource class.
What I Did: Implemented the retrieveClientPerformance method in the service layer to fetch real-time data from the database using an optimized SQL query.
What I Did: Utilized Fineract's JdbcTemplate to perform direct reads on the m_loan and m_client tables, ensuring the metrics reflect the current state of the database immediately upon request.
What I Did: Implemented error handling to detect non-existent client IDs, throwing a ClientNotFoundException which results in a standard Fineract 404 error response.
What I Did: Created ClientPerformanceApiIntegrationTest.java using the RestAssured framework to automate the verification of the API.
Proof: The test suite executed with a 100% success rate.
What I Did: Defined a dedicated Data Transfer Object (DTO), ClientPerformanceData, to ensure the response structure is consistent and types match the expected financial precision.
Supporting Evidence:
[Image 1: DTO Structure]: ClientPerformanceData.java showing the defined fields.
Technical Summary of Financial Logic
The balance is aggregated using the following formula for all loans with Status 300 (Active):
TotalOutstanding = Principal_{derived} + Interest_{derived} + Fees_{derived} + Penalties_{derived}
Using the _derived columns from the m_loan table ensures that we are using pre-calculated, high-performance data rather than re-calculating the entire schedule on every API call.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Your assigned reviewer(s) will follow our guidelines for code reviews.