From f6af357f542587322f7c739f5fd4795f4388b77e Mon Sep 17 00:00:00 2001 From: Jose Alberto Hernandez Date: Tue, 27 Aug 2024 07:59:43 -0500 Subject: [PATCH] FINERACT-2081: fix NPE Loan product creation with null values in Interest rate or Number of repayments --- .../LoanProductDataValidator.java | 60 ++++++++++--------- ...ncedPaymentAllocationIntegrationTests.java | 44 ++++++++++++++ 2 files changed, 77 insertions(+), 27 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java index b55f40c6834..2b9a876eeeb 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanproduct/serialization/LoanProductDataValidator.java @@ -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; @@ -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 @@ -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)) { @@ -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 @@ -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"); + } } } diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java index a3303169ad4..40edf8ac3cc 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/LoanProductWithAdvancedPaymentAllocationIntegrationTests.java @@ -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> 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> loanProductError = validationErrorHelper.getLoanProductError(loanProduct, "errors"); + Assertions.assertEquals("The parameter interestRatePerPeriod is mandatory.", + loanProductError.get(0).get("defaultUserMessage").replace('`', ' ')); + } + private String loanProductTestBuilder(Consumer customization) { LoanProductTestBuilder builder = new LoanProductTestBuilder().withPrincipal("15,000.00").withNumberOfRepayments("4") .withRepaymentAfterEvery("1").withRepaymentTypeAsMonth().withinterestRatePerPeriod("1")