From 17c2febcac440e872177a8a513e6833aaa39785c Mon Sep 17 00:00:00 2001 From: Ralph Hopman Date: Mon, 16 Feb 2026 21:36:23 +0100 Subject: [PATCH] FINERACT-2492: AprCalculator produces incorrect annual interest rate when DaysInYearType is ACTUAL with WHOLE_TERM frequency --- .../loanschedule/domain/AprCalculator.java | 28 +- .../domain/AprCalculatorTest.java | 334 ++++++++++++++++++ 2 files changed, 360 insertions(+), 2 deletions(-) create mode 100644 fineract-loan/src/test/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculatorTest.java diff --git a/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculator.java b/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculator.java index e22e02f9c..23e71ee9f 100644 --- a/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculator.java +++ b/fineract-loan/src/main/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculator.java @@ -19,21 +19,25 @@ package org.apache.fineract.portfolio.loanaccount.loanschedule.domain; import java.math.BigDecimal; +import lombok.RequiredArgsConstructor; import org.apache.fineract.organisation.monetary.domain.MoneyHelper; import org.apache.fineract.portfolio.common.domain.DaysInYearType; import org.apache.fineract.portfolio.common.domain.PeriodFrequencyType; import org.springframework.stereotype.Component; @Component +@RequiredArgsConstructor public class AprCalculator { + private final PaymentPeriodsInOneYearCalculator paymentPeriodsInOneYearCalculator; + public BigDecimal calculateFrom(final PeriodFrequencyType interestPeriodFrequencyType, final BigDecimal interestRatePerPeriod, final Integer numberOfRepayments, final Integer repaymentEvery, final PeriodFrequencyType repaymentPeriodFrequencyType, final DaysInYearType daysInYearType) { BigDecimal defaultAnnualNominalInterestRate = BigDecimal.ZERO; switch (interestPeriodFrequencyType) { case DAYS: - defaultAnnualNominalInterestRate = interestRatePerPeriod.multiply(BigDecimal.valueOf(daysInYearType.getValue())); + defaultAnnualNominalInterestRate = interestRatePerPeriod.multiply(BigDecimal.valueOf(getDaysInYear(daysInYearType))); break; case WEEKS: defaultAnnualNominalInterestRate = interestRatePerPeriod.multiply(BigDecimal.valueOf(52)); @@ -50,7 +54,7 @@ public class AprCalculator { switch (repaymentPeriodFrequencyType) { case DAYS: - defaultAnnualNominalInterestRate = ratePerPeriod.multiply(BigDecimal.valueOf(daysInYearType.getValue())); + defaultAnnualNominalInterestRate = ratePerPeriod.multiply(BigDecimal.valueOf(getDaysInYear(daysInYearType))); break; case WEEKS: defaultAnnualNominalInterestRate = ratePerPeriod.multiply(BigDecimal.valueOf(52)); @@ -74,4 +78,24 @@ public class AprCalculator { return defaultAnnualNominalInterestRate; } + /** + * Helper method to get the number of days in a year, handling ACTUAL appropriately. + * + * When daysInYearType is ACTUAL, this delegates to the PaymentPeriodsInOneYearCalculator (consistent with how + * Fineract handles ACTUAL elsewhere). For other types (DAYS_360, DAYS_364, DAYS_365), it returns the configured + * value. + * + * @param daysInYearType + * the days in year type configuration + * @return the number of days in a year + */ + private int getDaysInYear(final DaysInYearType daysInYearType) { + // When ACTUAL, delegate to calculator (consistent with LoanApplicationTerms.calculatePeriodsInOneYear) + if (daysInYearType == DaysInYearType.ACTUAL) { + return paymentPeriodsInOneYearCalculator.calculate(PeriodFrequencyType.DAYS); + } + // For DAYS_360, DAYS_364, DAYS_365: use configured value + return daysInYearType.getValue(); + } + } diff --git a/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculatorTest.java b/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculatorTest.java new file mode 100644 index 000000000..f11129dee --- /dev/null +++ b/fineract-loan/src/test/java/org/apache/fineract/portfolio/loanaccount/loanschedule/domain/AprCalculatorTest.java @@ -0,0 +1,334 @@ +/** + * 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. + */ +package org.apache.fineract.portfolio.loanaccount.loanschedule.domain; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.math.BigDecimal; +import java.math.MathContext; +import java.math.RoundingMode; +import org.apache.fineract.organisation.monetary.domain.MoneyHelper; +import org.apache.fineract.portfolio.common.domain.DaysInYearType; +import org.apache.fineract.portfolio.common.domain.PeriodFrequencyType; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class AprCalculatorTest { + + private static final int PRECISION = 19; + private static final MockedStatic MONEY_HELPER = mockStatic(MoneyHelper.class); + private static final MathContext MATH_CONTEXT = new MathContext(PRECISION, RoundingMode.HALF_EVEN); + + @Mock + private PaymentPeriodsInOneYearCalculator paymentPeriodsInOneYearCalculator; + + private AprCalculator aprCalculator; + + @BeforeAll + static void init() { + MONEY_HELPER.when(MoneyHelper::getRoundingMode).thenReturn(RoundingMode.HALF_EVEN); + MONEY_HELPER.when(MoneyHelper::getMathContext).thenReturn(MATH_CONTEXT); + } + + @BeforeEach + void setUp() { + aprCalculator = new AprCalculator(paymentPeriodsInOneYearCalculator); + } + + @AfterAll + static void tearDown() { + MONEY_HELPER.close(); + } + + /** + * Test DAYS frequency with DaysInYearType.ACTUAL + * + * This test verifies the fix for FINERACT-2492 where ACTUAL was incorrectly using value 1 + * instead of delegating to PaymentPeriodsInOneYearCalculator which returns 365. + */ + @Test + void testCalculateFrom_DaysFrequency_WithActualDaysInYear() { + // Given + when(paymentPeriodsInOneYearCalculator.calculate(PeriodFrequencyType.DAYS)).thenReturn(365); + + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(10.0); // 10% per day + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.DAYS; + DaysInYearType daysInYearType = DaysInYearType.ACTUAL; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, 1, 1, + PeriodFrequencyType.DAYS, daysInYearType); + + // Then + // Annual rate should be 10% * 365 = 3650% + assertEquals(0, BigDecimal.valueOf(3650.0).compareTo(annualRate), + "Annual rate should be interestRatePerPeriod * 365 for ACTUAL"); + verify(paymentPeriodsInOneYearCalculator).calculate(PeriodFrequencyType.DAYS); + } + + /** + * Test WHOLE_TERM frequency with DAYS repayment and DaysInYearType.ACTUAL + * + * This is the exact scenario from FINERACT-2492 bug report. + */ + @Test + void testCalculateFrom_WholeTermFrequency_WithDaysRepaymentAndActualDaysInYear() { + // Given + when(paymentPeriodsInOneYearCalculator.calculate(PeriodFrequencyType.DAYS)).thenReturn(365); + + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(10.0); // 10% whole term + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.WHOLE_TERM; + Integer numberOfRepayments = 3; + Integer repaymentEvery = 1; + PeriodFrequencyType repaymentFrequencyType = PeriodFrequencyType.DAYS; + DaysInYearType daysInYearType = DaysInYearType.ACTUAL; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, numberOfRepayments, + repaymentEvery, repaymentFrequencyType, daysInYearType); + + // Then + // ratePerPeriod = 10% / (3 * 1) = 3.33333333% + // annualRate = 3.33333333% * 365 = 1216.66666667% + BigDecimal expectedAnnualRate = BigDecimal.valueOf(10.0).divide(BigDecimal.valueOf(3), 8, java.math.RoundingMode.HALF_EVEN) + .multiply(BigDecimal.valueOf(365)); + + assertEquals(0, expectedAnnualRate.compareTo(annualRate), "Annual rate calculation should use 365 days for ACTUAL"); + verify(paymentPeriodsInOneYearCalculator).calculate(PeriodFrequencyType.DAYS); + } + + /** + * Test DAYS frequency with DaysInYearType.DAYS_360 + * + * Verify that DAYS_360 works correctly and uses value 360 directly. + */ + @Test + void testCalculateFrom_DaysFrequency_WithDays360() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(10.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.DAYS; + DaysInYearType daysInYearType = DaysInYearType.DAYS_360; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, 1, 1, PeriodFrequencyType.DAYS, + daysInYearType); + + // Then + // Annual rate should be 10% * 360 = 3600% + assertEquals(0, BigDecimal.valueOf(3600.0).compareTo(annualRate), "Annual rate should use 360 days for DAYS_360"); + } + + /** + * Test DAYS frequency with DaysInYearType.DAYS_364 + * + * Verify that DAYS_364 works correctly and uses value 364 directly. + */ + @Test + void testCalculateFrom_DaysFrequency_WithDays364() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(10.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.DAYS; + DaysInYearType daysInYearType = DaysInYearType.DAYS_364; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, 1, 1, PeriodFrequencyType.DAYS, + daysInYearType); + + // Then + // Annual rate should be 10% * 364 = 3640% + assertEquals(0, BigDecimal.valueOf(3640.0).compareTo(annualRate), "Annual rate should use 364 days for DAYS_364"); + } + + /** + * Test DAYS frequency with DaysInYearType.DAYS_365 + * + * Verify that DAYS_365 works correctly and uses value 365 directly. + */ + @Test + void testCalculateFrom_DaysFrequency_WithDays365() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(10.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.DAYS; + DaysInYearType daysInYearType = DaysInYearType.DAYS_365; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, 1, 1, PeriodFrequencyType.DAYS, + daysInYearType); + + // Then + // Annual rate should be 10% * 365 = 3650% + assertEquals(0, BigDecimal.valueOf(3650.0).compareTo(annualRate), "Annual rate should use 365 days for DAYS_365"); + } + + /** + * Test WHOLE_TERM frequency with WEEKS repayment and DaysInYearType.ACTUAL + * + * Verify that ACTUAL doesn't affect non-DAYS repayment frequencies. + */ + @Test + void testCalculateFrom_WholeTermFrequency_WithWeeksRepaymentAndActualDaysInYear() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(10.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.WHOLE_TERM; + Integer numberOfRepayments = 4; + Integer repaymentEvery = 1; + PeriodFrequencyType repaymentFrequencyType = PeriodFrequencyType.WEEKS; + DaysInYearType daysInYearType = DaysInYearType.ACTUAL; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, numberOfRepayments, + repaymentEvery, repaymentFrequencyType, daysInYearType); + + // Then + // ratePerPeriod = 10% / (4 * 1) = 2.5% + // annualRate = 2.5% * 52 = 130% + BigDecimal expectedAnnualRate = BigDecimal.valueOf(10.0).divide(BigDecimal.valueOf(4), 8, java.math.RoundingMode.HALF_EVEN) + .multiply(BigDecimal.valueOf(52)); + + assertEquals(0, expectedAnnualRate.compareTo(annualRate), "Annual rate for WEEKS should use 52 weeks"); + } + + /** + * Test WHOLE_TERM frequency with MONTHS repayment and DaysInYearType.ACTUAL + * + * Verify that ACTUAL doesn't affect non-DAYS repayment frequencies. + */ + @Test + void testCalculateFrom_WholeTermFrequency_WithMonthsRepaymentAndActualDaysInYear() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(12.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.WHOLE_TERM; + Integer numberOfRepayments = 6; + Integer repaymentEvery = 1; + PeriodFrequencyType repaymentFrequencyType = PeriodFrequencyType.MONTHS; + DaysInYearType daysInYearType = DaysInYearType.ACTUAL; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, numberOfRepayments, + repaymentEvery, repaymentFrequencyType, daysInYearType); + + // Then + // ratePerPeriod = 12% / (6 * 1) = 2% + // annualRate = 2% * 12 = 24% + BigDecimal expectedAnnualRate = BigDecimal.valueOf(12.0).divide(BigDecimal.valueOf(6), 8, java.math.RoundingMode.HALF_EVEN) + .multiply(BigDecimal.valueOf(12)); + + assertEquals(0, expectedAnnualRate.compareTo(annualRate), "Annual rate for MONTHS should use 12 months"); + } + + /** + * Test WEEKS frequency + */ + @Test + void testCalculateFrom_WeeksFrequency() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(2.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.WEEKS; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, 1, 1, PeriodFrequencyType.WEEKS, + DaysInYearType.ACTUAL); + + // Then + // Annual rate should be 2% * 52 = 104% + assertEquals(0, BigDecimal.valueOf(104.0).compareTo(annualRate), "Annual rate for WEEKS should multiply by 52"); + } + + /** + * Test MONTHS frequency + */ + @Test + void testCalculateFrom_MonthsFrequency() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(2.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.MONTHS; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, 1, 1, PeriodFrequencyType.MONTHS, + DaysInYearType.ACTUAL); + + // Then + // Annual rate should be 2% * 12 = 24% + assertEquals(0, BigDecimal.valueOf(24.0).compareTo(annualRate), "Annual rate for MONTHS should multiply by 12"); + } + + /** + * Test YEARS frequency + */ + @Test + void testCalculateFrom_YearsFrequency() { + // Given + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(5.0); + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.YEARS; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, 1, 1, PeriodFrequencyType.YEARS, + DaysInYearType.ACTUAL); + + // Then + // Annual rate should be 5% * 1 = 5% + assertEquals(0, BigDecimal.valueOf(5.0).compareTo(annualRate), "Annual rate for YEARS should multiply by 1"); + } + + /** + * Test bug scenario with realistic values from FINERACT-2492 + * + * Principal: 3,400 + * Interest rate: 10% WHOLE_TERM + * Repayments: 3 daily + * Expected interest per installment: 113.33 + */ + @Test + void testCalculateFrom_BugReproductionScenario() { + // Given + when(paymentPeriodsInOneYearCalculator.calculate(PeriodFrequencyType.DAYS)).thenReturn(365); + + BigDecimal interestRatePerPeriod = BigDecimal.valueOf(10.0); // 10% whole term + PeriodFrequencyType interestFrequencyType = PeriodFrequencyType.WHOLE_TERM; + Integer numberOfRepayments = 3; + Integer repaymentEvery = 1; + PeriodFrequencyType repaymentFrequencyType = PeriodFrequencyType.DAYS; + DaysInYearType daysInYearType = DaysInYearType.ACTUAL; + + // When + BigDecimal annualRate = aprCalculator.calculateFrom(interestFrequencyType, interestRatePerPeriod, numberOfRepayments, + repaymentEvery, repaymentFrequencyType, daysInYearType); + + // Then + // The bug was causing annual rate to be 3.333% instead of 1216.667% + // Verify it's much greater than 100 (definitely not 3.333) + assertEquals(true, annualRate.compareTo(BigDecimal.valueOf(1000)) > 0, + "Annual rate should be > 1000% (bug was producing 3.333%)"); + + // Verify exact expected value: 10/3 * 365 = 1216.66666667 + BigDecimal expectedRate = BigDecimal.valueOf(10.0).divide(BigDecimal.valueOf(3), 8, java.math.RoundingMode.HALF_EVEN) + .multiply(BigDecimal.valueOf(365)); + assertEquals(0, expectedRate.compareTo(annualRate), "Annual rate should be exactly 1216.67%"); + } +}