Skip to content

Commit

Permalink
FINERACT-2081: fix NPE Loan product creation with null values in Inte…
Browse files Browse the repository at this point in the history 
…rest rate or Number of repayments
  • Loading branch information
Jose Alberto Hernandez committed Aug 28, 2024
1 parent 6ca0268 commit f6af357
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 27 deletions.
60 changes: 33 additions & 27 deletions ...ava/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.fineract.infrastructure.core.exception.InvalidJsonException;
import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
import org.apache.fineract.infrastructure.core.service.MathUtil;
import org.apache.fineract.portfolio.calendar.service.CalendarUtils;
import org.apache.fineract.portfolio.common.domain.PeriodFrequencyType;
import org.apache.fineract.portfolio.loanaccount.api.LoanApiConstants;
Expand Down Expand Up @@ -569,7 +570,7 @@ public void validateForCreate(final JsonCommand command) {
Locale.getDefault());
baseDataValidator.reset().parameter(INTEREST_RATE_FREQUENCY_TYPE).value(interestRateFrequencyType).notNull().inMinMaxRange(0,
4);
isInterestBearing = interestRatePerPeriod.compareTo(BigDecimal.ZERO) > 0;
isInterestBearing = MathUtil.isGreaterThanZero(interestRatePerPeriod);
}

// Fixed Length validation
Expand Down Expand Up @@ -1296,11 +1297,14 @@ public void validateForUpdate(final JsonCommand command, final LoanProduct loanP
.integerGreaterThanZero();
}

Integer numberOfRepayments = loanProduct.getNumberOfRepayments();
Integer numberOfRepayments = null;
if (this.fromApiJsonHelper.parameterExists(NUMBER_OF_REPAYMENTS, element)) {
numberOfRepayments = this.fromApiJsonHelper.extractIntegerWithLocaleNamed(NUMBER_OF_REPAYMENTS, element);
baseDataValidator.reset().parameter(NUMBER_OF_REPAYMENTS).value(numberOfRepayments).notNull().integerGreaterThanZero();
}
if (numberOfRepayments == null) {
numberOfRepayments = loanProduct.getNumberOfRepayments();
}

Integer repaymentEvery = loanProduct.getLoanProductRelatedDetail().getRepayEvery();
if (this.fromApiJsonHelper.parameterExists(REPAYMENT_EVERY, element)) {
Expand Down Expand Up @@ -1608,7 +1612,7 @@ public void validateForUpdate(final JsonCommand command, final LoanProduct loanP
}
baseDataValidator.reset().parameter(INTEREST_RATE_FREQUENCY_TYPE).value(interestRateFrequencyType).notNull().inMinMaxRange(0,
4);
isInterestBearing = interestRatePerPeriod.compareTo(BigDecimal.ZERO) > 0;
isInterestBearing = MathUtil.isGreaterThanZero(interestRatePerPeriod);
}

// Fixed Length validation
Expand Down Expand Up @@ -2541,34 +2545,36 @@ private void validateLoanScheduleType(final String transactionProcessingStrategy
public void validateRepaymentPeriodWithGraceSettings(final Integer numberOfRepayments, final Integer graceOnPrincipalPayment,
final Integer graceOnInterestPayment, final Integer graceOnInterestCharged, final Integer recurringMoratoriumOnPrincipalPeriods,
DataValidatorBuilder baseDataValidator) {
if (numberOfRepayments <= defaultToZeroIfNull(graceOnPrincipalPayment)) {
baseDataValidator.reset().parameter("graceOnPrincipalPayment").value(graceOnPrincipalPayment)
.failWithCode(".mustBeLessThan.numberOfRepayments");
}
if (numberOfRepayments != null) {
if (numberOfRepayments <= defaultToZeroIfNull(graceOnPrincipalPayment)) {
baseDataValidator.reset().parameter("graceOnPrincipalPayment").value(graceOnPrincipalPayment)
.failWithCode(".mustBeLessThan.numberOfRepayments");
}

if (numberOfRepayments <= defaultToZeroIfNull(graceOnInterestPayment)) {
baseDataValidator.reset().parameter("graceOnInterestPayment").value(graceOnInterestPayment)
.failWithCode(".mustBeLessThan.numberOfRepayments");
}
if (numberOfRepayments <= defaultToZeroIfNull(graceOnInterestPayment)) {
baseDataValidator.reset().parameter("graceOnInterestPayment").value(graceOnInterestPayment)
.failWithCode(".mustBeLessThan.numberOfRepayments");
}

if (numberOfRepayments < defaultToZeroIfNull(graceOnInterestCharged)) {
baseDataValidator.reset().parameter("graceOnInterestCharged").value(graceOnInterestCharged)
.failWithCode(".mustBeLessThan.numberOfRepayments");
}
if (numberOfRepayments < defaultToZeroIfNull(graceOnInterestCharged)) {
baseDataValidator.reset().parameter("graceOnInterestCharged").value(graceOnInterestCharged)
.failWithCode(".mustBeLessThan.numberOfRepayments");
}

int graceOnPrincipal = 0;
if (graceOnPrincipalPayment != null) {
graceOnPrincipal = graceOnPrincipalPayment;
}
int recurMoratoriumOnPrincipal = 0;
if (recurringMoratoriumOnPrincipalPeriods != null) {
recurMoratoriumOnPrincipal = recurringMoratoriumOnPrincipalPeriods;
}
int graceOnPrincipal = 0;
if (graceOnPrincipalPayment != null) {
graceOnPrincipal = graceOnPrincipalPayment;
}
int recurMoratoriumOnPrincipal = 0;
if (recurringMoratoriumOnPrincipalPeriods != null) {
recurMoratoriumOnPrincipal = recurringMoratoriumOnPrincipalPeriods;
}

if ((recurMoratoriumOnPrincipal > 0) && ((numberOfRepayments - graceOnPrincipal) % (recurMoratoriumOnPrincipal + 1) != 1)) {
baseDataValidator.reset().parameter("graceOnPrincipalPayments.and.recurringMoratoriumOnPrincipalPeriods")
.value(graceOnPrincipal).value(recurMoratoriumOnPrincipal)
.failWithCode("causes.principal.moratorium.for.last.installment");
if ((recurMoratoriumOnPrincipal > 0) && ((numberOfRepayments - graceOnPrincipal) % (recurMoratoriumOnPrincipal + 1) != 1)) {
baseDataValidator.reset().parameter("graceOnPrincipalPayments.and.recurringMoratoriumOnPrincipalPeriods")
.value(graceOnPrincipal).value(recurMoratoriumOnPrincipal)
.failWithCode("causes.principal.moratorium.for.last.installment");
}
}
}

Expand Down
44 changes: 44 additions & 0 deletions ...e/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,50 @@ public void testCreateCumulativeLoanProductWithInterestRefund() {
loanProductError.get(0).get("userMessageGlobalisationCode"));
}

@Test
public void testCreateShouldFailWhenNoNumberOfRepaymentsIsProvided() {
// given
ResponseSpecification errorResponse = new ResponseSpecBuilder().expectStatusCode(400).build();
LoanTransactionHelper validationErrorHelper = new LoanTransactionHelper(REQUEST_SPEC, errorResponse);
AdvancedPaymentData defaultAllocation = createDefaultPaymentAllocation();
AdvancedPaymentData repaymentPaymentAllocation = createRepaymentPaymentAllocation();

// when
String loanProduct = loanProductTestBuilder(customization -> customization
.addAdvancedPaymentAllocation(defaultAllocation, repaymentPaymentAllocation).withPrincipal("15,000.00")
.withNumberOfRepayments(null).withRepaymentAfterEvery("1").withRepaymentTypeAsMonth().withinterestRatePerPeriod("1")
.withAccountingRulePeriodicAccrual(new Account[] { ASSET_ACCOUNT, EXPENSE_ACCOUNT, INCOME_ACCOUNT, OVERPAYMENT_ACCOUNT })
.withInterestRateFrequencyTypeAsMonths().withAmortizationTypeAsEqualInstallments().withInterestTypeAsDecliningBalance()
.withFeeAndPenaltyAssetAccount(FEE_PENALTY_ACCOUNT).build());

// when
List<Map<String, String>> loanProductError = validationErrorHelper.getLoanProductError(loanProduct, "errors");
Assertions.assertEquals("The parameter numberOfRepayments is mandatory.",
loanProductError.get(0).get("defaultUserMessage").replace('`', ' '));
}

@Test
public void testCreateShouldFailWhenNoInterestRateIsProvided() {
// given
ResponseSpecification errorResponse = new ResponseSpecBuilder().expectStatusCode(400).build();
LoanTransactionHelper validationErrorHelper = new LoanTransactionHelper(REQUEST_SPEC, errorResponse);
AdvancedPaymentData defaultAllocation = createDefaultPaymentAllocation();
AdvancedPaymentData repaymentPaymentAllocation = createRepaymentPaymentAllocation();

// when
String loanProduct = loanProductTestBuilder(customization -> customization
.addAdvancedPaymentAllocation(defaultAllocation, repaymentPaymentAllocation).withPrincipal("15,000.00")
.withNumberOfRepayments("4").withRepaymentAfterEvery("1").withRepaymentTypeAsMonth().withinterestRatePerPeriod(null)
.withAccountingRulePeriodicAccrual(new Account[] { ASSET_ACCOUNT, EXPENSE_ACCOUNT, INCOME_ACCOUNT, OVERPAYMENT_ACCOUNT })
.withInterestRateFrequencyTypeAsMonths().withAmortizationTypeAsEqualInstallments().withInterestTypeAsDecliningBalance()
.withFeeAndPenaltyAssetAccount(FEE_PENALTY_ACCOUNT).build());

// when
List<Map<String, String>> loanProductError = validationErrorHelper.getLoanProductError(loanProduct, "errors");
Assertions.assertEquals("The parameter interestRatePerPeriod is mandatory.",
loanProductError.get(0).get("defaultUserMessage").replace('`', ' '));
}

private String loanProductTestBuilder(Consumer<LoanProductTestBuilder> customization) {
LoanProductTestBuilder builder = new LoanProductTestBuilder().withPrincipal("15,000.00").withNumberOfRepayments("4")
.withRepaymentAfterEvery("1").withRepaymentTypeAsMonth().withinterestRatePerPeriod("1")
Expand Down

0 comments on commit f6af357

Please sign in to comment.