From cce81aa363751b543dd8b6e0a1e8b1bf6e087a15 Mon Sep 17 00:00:00 2001 From: Abhishek Chaudhary Date: Sat, 18 Jul 2026 14:26:40 +0530 Subject: [PATCH 1/2] FINERACT-2694: Return validation errors instead of 500 NPE when type parameters are omitted SmsCampaignValidator, EmailCampaignValidator and StandingInstructionDataValidator queue a notNull validation error for triggerType/campaignType/transferType but dereference the extracted value (intValue() / enum fromInt switch) before the queued errors are thrown. Omitting the parameter therefore crashes with a NullPointerException and the API returns HTTP 500 instead of the intended 400 with the "parameter is mandatory" error. Add null guards at each dereference so the already-queued validation errors surface, and add regression tests reproducing the NPE for all three validators. --- .../email/data/EmailCampaignValidator.java | 4 +- .../serialization/SmsCampaignValidator.java | 8 +- .../StandingInstructionDataValidator.java | 4 +- .../data/EmailCampaignValidatorTest.java | 81 +++++++++++++++++ .../SmsCampaignValidatorTest.java | 81 +++++++++++++++++ .../StandingInstructionDataValidatorTest.java | 90 +++++++++++++++++++ 6 files changed, 260 insertions(+), 8 deletions(-) create mode 100644 fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java create mode 100644 fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java create mode 100644 fineract-provider/src/test/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidatorTest.java diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidator.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidator.java index acbcca26121..8723204e5d8 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidator.java @@ -106,7 +106,7 @@ public void validateCreate(String json) { final Long campaignType = this.fromApiJsonHelper.extractLongNamed(EmailCampaignValidator.campaignType, element); baseDataValidator.reset().parameter(EmailCampaignValidator.campaignType).value(campaignType).notNull().integerGreaterThanZero(); - if (campaignType.intValue() == EmailCampaignType.SCHEDULE.getValue()) { + if (campaignType != null && campaignType.intValue() == EmailCampaignType.SCHEDULE.getValue()) { final String recurrenceParamName = this.fromApiJsonHelper.extractStringNamed(EmailCampaignValidator.recurrenceParamName, element); baseDataValidator.reset().parameter(EmailCampaignValidator.recurrenceParamName).value(recurrenceParamName).notBlank(); @@ -157,7 +157,7 @@ public void validateForUpdate(String json) { final Long campaignType = this.fromApiJsonHelper.extractLongNamed(EmailCampaignValidator.campaignType, element); baseDataValidator.reset().parameter(EmailCampaignValidator.campaignType).value(campaignType).notNull().integerGreaterThanZero(); - if (campaignType.intValue() == EmailCampaignType.SCHEDULE.getValue()) { + if (campaignType != null && campaignType.intValue() == EmailCampaignType.SCHEDULE.getValue()) { final String recurrenceParamName = this.fromApiJsonHelper.extractStringNamed(EmailCampaignValidator.recurrenceParamName, element); baseDataValidator.reset().parameter(EmailCampaignValidator.recurrenceParamName).value(recurrenceParamName).notBlank(); diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidator.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidator.java index 6aab22e61a8..9cef1b2a4c8 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidator.java @@ -117,7 +117,7 @@ public void validateCreate(String json) { final Long triggerType = this.fromApiJsonHelper.extractLongNamed(SmsCampaignValidator.triggerType, element); baseDataValidator.reset().parameter(SmsCampaignValidator.triggerType).value(triggerType).notNull().integerGreaterThanZero(); - if (triggerType.intValue() == SmsCampaignTriggerType.SCHEDULE.getValue()) { + if (triggerType != null && triggerType.intValue() == SmsCampaignTriggerType.SCHEDULE.getValue()) { final Integer frequencyParam = this.fromApiJsonHelper.extractIntegerWithLocaleNamed(SmsCampaignValidator.frequencyParamName, element); @@ -143,7 +143,7 @@ public void validateCreate(String json) { baseDataValidator.reset().parameter(SmsCampaignValidator.message).value(message).notBlank().notExceedingLengthOf(480); final JsonElement paramValueJsonObject = this.fromApiJsonHelper.extractJsonObjectNamed(SmsCampaignValidator.paramValue, element); - if (triggerType.intValue() != SmsCampaignTriggerType.TRIGGERED.getValue()) { + if (triggerType != null && triggerType.intValue() != SmsCampaignTriggerType.TRIGGERED.getValue()) { baseDataValidator.reset().parameter(SmsCampaignValidator.paramValue).value(paramValueJsonObject).notBlank(); if (paramValueJsonObject != null && paramValueJsonObject.isJsonObject()) { for (Map.Entry entry : paramValueJsonObject.getAsJsonObject().entrySet()) { @@ -190,7 +190,7 @@ public void validateForUpdate(String json) { final Long triggerType = this.fromApiJsonHelper.extractLongNamed(SmsCampaignValidator.triggerType, element); baseDataValidator.reset().parameter(SmsCampaignValidator.triggerType).value(triggerType).notNull().integerGreaterThanZero(); - if (triggerType.intValue() == SmsCampaignTriggerType.SCHEDULE.getValue()) { + if (triggerType != null && triggerType.intValue() == SmsCampaignTriggerType.SCHEDULE.getValue()) { if (this.fromApiJsonHelper.parameterExists(SmsCampaignValidator.recurrenceParamName, element)) { final String recurrenceParamName = this.fromApiJsonHelper.extractStringNamed(SmsCampaignValidator.recurrenceParamName, element); @@ -212,7 +212,7 @@ public void validateForUpdate(String json) { baseDataValidator.reset().parameter(SmsCampaignValidator.message).value(message).notBlank().notExceedingLengthOf(480); final JsonElement paramValueJsonObject = this.fromApiJsonHelper.extractJsonObjectNamed(SmsCampaignValidator.paramValue, element); - if (triggerType.intValue() != SmsCampaignTriggerType.TRIGGERED.getValue()) { + if (triggerType != null && triggerType.intValue() != SmsCampaignTriggerType.TRIGGERED.getValue()) { baseDataValidator.reset().parameter(SmsCampaignValidator.paramValue).value(paramValueJsonObject).notBlank(); if (paramValueJsonObject != null && paramValueJsonObject.isJsonObject()) { for (Map.Entry entry : paramValueJsonObject.getAsJsonObject().entrySet()) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidator.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidator.java index 36fbe302c18..0169c4fb760 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidator.java @@ -183,9 +183,9 @@ public void validateForCreate(final JsonCommand command) { } String errorCode = null; - AccountTransferType accountTransferType = AccountTransferType.fromInt(transferType); final Integer fromAccountType = this.fromApiJsonHelper.extractIntegerSansLocaleNamed(fromAccountTypeParamName, element); - if (fromAccountType != null && toAccountType != null) { + if (transferType != null && fromAccountType != null && toAccountType != null) { + AccountTransferType accountTransferType = AccountTransferType.fromInt(transferType); PortfolioAccountType fromPortfolioAccountType = PortfolioAccountType.fromInt(fromAccountType); PortfolioAccountType toPortfolioAccountType = PortfolioAccountType.fromInt(toAccountType); if (accountTransferType.isAccountTransfer() && (PortfolioAccountType.LOAN.equals(fromPortfolioAccountType) diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java new file mode 100644 index 00000000000..1d8c9135de6 --- /dev/null +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java @@ -0,0 +1,81 @@ +/** + * 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.infrastructure.campaigns.email.data; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.fineract.infrastructure.campaigns.email.domain.EmailCampaignType; +import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; +import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Regression tests for the NullPointerException thrown when the mandatory {@code campaignType} parameter is omitted + * from email campaign create/update requests. The validator queued a {@code notNull} validation error but dereferenced + * the value before throwing, turning a client error into an HTTP 500. + */ +class EmailCampaignValidatorTest { + + private EmailCampaignValidator validator; + + @BeforeEach + void setUp() { + validator = new EmailCampaignValidator(new FromJsonHelper()); + } + + @Test + void validateCreateWithoutCampaignTypeReportsValidationErrorInsteadOfNpe() { + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateCreate("{}")); + + assertTrue(ex.getErrors().stream().anyMatch(e -> EmailCampaignValidator.campaignType.equals(e.getParameterName())), + "Expected validation error for parameter 'campaignType'"); + } + + @Test + void validateForUpdateWithoutCampaignTypeReportsValidationErrorInsteadOfNpe() { + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateForUpdate("{}")); + + assertTrue(ex.getErrors().stream().anyMatch(e -> EmailCampaignValidator.campaignType.equals(e.getParameterName())), + "Expected validation error for parameter 'campaignType'"); + } + + @Test + void validateCreateWithScheduleCampaignTypeStillRequiresRecurrenceDetails() { + String json = """ + { + "campaignName": "Loan reminders", + "campaignType": %d, + "businessRuleId": 1, + "emailSubject": "Reminder", + "emailMessage": "Your repayment is due", + "paramValue": "value" + } + """.formatted(EmailCampaignType.SCHEDULE.getValue()); + + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateCreate(json)); + + assertTrue(ex.getErrors().stream().anyMatch(e -> EmailCampaignValidator.recurrenceStartDate.equals(e.getParameterName())), + "Expected validation error for parameter 'recurrenceStartDate'"); + } +} diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java new file mode 100644 index 00000000000..4dace43d136 --- /dev/null +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java @@ -0,0 +1,81 @@ +/** + * 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.infrastructure.campaigns.sms.serialization; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.fineract.infrastructure.campaigns.sms.constants.SmsCampaignTriggerType; +import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; +import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Regression tests for the NullPointerException thrown when the mandatory {@code triggerType} parameter is omitted from + * SMS campaign create/update requests. The validator queued a {@code notNull} validation error but dereferenced the + * value before throwing, turning a client error into an HTTP 500. + */ +class SmsCampaignValidatorTest { + + private SmsCampaignValidator validator; + + @BeforeEach + void setUp() { + validator = new SmsCampaignValidator(new FromJsonHelper()); + } + + @Test + void validateCreateWithoutTriggerTypeReportsValidationErrorInsteadOfNpe() { + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateCreate("{}")); + + assertTrue(ex.getErrors().stream().anyMatch(e -> SmsCampaignValidator.triggerType.equals(e.getParameterName())), + "Expected validation error for parameter 'triggerType'"); + } + + @Test + void validateForUpdateWithoutTriggerTypeReportsValidationErrorInsteadOfNpe() { + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateForUpdate("{}")); + + assertTrue(ex.getErrors().stream().anyMatch(e -> SmsCampaignValidator.triggerType.equals(e.getParameterName())), + "Expected validation error for parameter 'triggerType'"); + } + + @Test + void validateCreateWithScheduleTriggerTypeStillRequiresRecurrenceDetails() { + String json = """ + { + "campaignName": "Repayment reminders", + "campaignType": 1, + "triggerType": %d, + "runReportId": 1, + "message": "Your repayment is due", + "locale": "en" + } + """.formatted(SmsCampaignTriggerType.SCHEDULE.getValue()); + + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateCreate(json)); + + assertTrue(ex.getErrors().stream().anyMatch(e -> SmsCampaignValidator.recurrenceStartDate.equals(e.getParameterName())), + "Expected validation error for parameter 'recurrenceStartDate'"); + } +} diff --git a/fineract-provider/src/test/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidatorTest.java b/fineract-provider/src/test/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidatorTest.java new file mode 100644 index 00000000000..e5fc1f2fb47 --- /dev/null +++ b/fineract-provider/src/test/java/org/apache/fineract/portfolio/account/data/StandingInstructionDataValidatorTest.java @@ -0,0 +1,90 @@ +/** + * 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.account.data; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.fineract.infrastructure.core.api.JsonCommand; +import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; +import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; +import org.apache.fineract.portfolio.account.AccountDetailConstants; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Regression tests for the NullPointerException thrown when the mandatory {@code transferType} parameter is omitted + * from standing instruction create requests. The validator queued a {@code notNull} validation error but passed the + * null value to {@code AccountTransferType.fromInt} before throwing, turning a client error into an HTTP 500. + */ +class StandingInstructionDataValidatorTest { + + private FromJsonHelper fromJsonHelper; + private StandingInstructionDataValidator validator; + + @BeforeEach + void setUp() { + fromJsonHelper = new FromJsonHelper(); + validator = new StandingInstructionDataValidator(fromJsonHelper, new AccountTransfersDetailDataValidator(fromJsonHelper)); + } + + private JsonCommand jsonCommand(String json) { + return new JsonCommand(1L, fromJsonHelper.parse(json), fromJsonHelper); + } + + @Test + void validateForCreateWithoutTransferTypeReportsValidationErrorInsteadOfNpe() { + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateForCreate(jsonCommand("{}"))); + + assertTrue(ex.getErrors().stream().anyMatch(e -> AccountDetailConstants.transferTypeParamName.equals(e.getParameterName())), + "Expected validation error for parameter 'transferType'"); + } + + @Test + void validateForCreateWithLoanAccountForAccountTransferReportsTransferTypeError() { + String json = """ + { + "fromOfficeId": 1, + "fromClientId": 1, + "fromAccountId": 1, + "fromAccountType": 1, + "toOfficeId": 1, + "toClientId": 1, + "toAccountId": 2, + "toAccountType": 2, + "transferType": 1, + "priority": 1, + "status": 1, + "instructionType": 1, + "recurrenceType": 1, + "name": "test instruction", + "validFrom": "01 January 2026", + "locale": "en", + "dateFormat": "dd MMMM yyyy" + } + """; + + PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, + () -> validator.validateForCreate(jsonCommand(json))); + + assertTrue(ex.getErrors().stream().anyMatch(e -> AccountDetailConstants.transferTypeParamName.equals(e.getParameterName())), + "Expected validation error for parameter 'transferType'"); + } +} From 3e828896d33f2fc9ad1a003813dcbdfe824c3812 Mon Sep 17 00:00:00 2001 From: Abhishek Chaudhary Date: Mon, 20 Jul 2026 17:12:35 +0530 Subject: [PATCH 2/2] FINERACT-2694: Fix SpotBugs VA_FORMAT_STRING_USES_NEWLINE in campaign validator tests The recurrence-detail regression tests built their request JSON with String.formatted() on a text block containing newlines, which SpotBugs flags as VA_FORMAT_STRING_USES_NEWLINE and failed the spotbugsTest quality check. Build the JSON with String.replace() instead so no format string is involved; the enum value is still resolved at compile time and the assertions are unchanged. --- .../campaigns/email/data/EmailCampaignValidatorTest.java | 4 ++-- .../campaigns/sms/serialization/SmsCampaignValidatorTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java index 1d8c9135de6..de3e0e7646d 100644 --- a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/email/data/EmailCampaignValidatorTest.java @@ -64,13 +64,13 @@ void validateCreateWithScheduleCampaignTypeStillRequiresRecurrenceDetails() { String json = """ { "campaignName": "Loan reminders", - "campaignType": %d, + "campaignType": CAMPAIGN_TYPE, "businessRuleId": 1, "emailSubject": "Reminder", "emailMessage": "Your repayment is due", "paramValue": "value" } - """.formatted(EmailCampaignType.SCHEDULE.getValue()); + """.replace("CAMPAIGN_TYPE", String.valueOf(EmailCampaignType.SCHEDULE.getValue())); PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, () -> validator.validateCreate(json)); diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java index 4dace43d136..df9e5830421 100644 --- a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/campaigns/sms/serialization/SmsCampaignValidatorTest.java @@ -65,12 +65,12 @@ void validateCreateWithScheduleTriggerTypeStillRequiresRecurrenceDetails() { { "campaignName": "Repayment reminders", "campaignType": 1, - "triggerType": %d, + "triggerType": TRIGGER_TYPE, "runReportId": 1, "message": "Your repayment is due", "locale": "en" } - """.formatted(SmsCampaignTriggerType.SCHEDULE.getValue()); + """.replace("TRIGGER_TYPE", String.valueOf(SmsCampaignTriggerType.SCHEDULE.getValue())); PlatformApiDataValidationException ex = assertThrows(PlatformApiDataValidationException.class, () -> validator.validateCreate(json));