FINERACT-2657: Fix NPE updating a provisioning criteria by matching definitions on categoryId

fineract-provider/src/main/java/org/apache/fineract/organisation/provisioning/domain/ProvisioningCriteria.java

@@ -26,14 +26,17 @@
 import jakarta.persistence.Table;
 import jakarta.persistence.UniqueConstraint;
 import java.time.LocalDateTime;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import org.apache.fineract.accounting.glaccount.domain.GLAccount;
 import org.apache.fineract.infrastructure.core.api.JsonCommand;
+import org.apache.fineract.infrastructure.core.data.ApiParameterError;
 import org.apache.fineract.infrastructure.core.domain.AbstractAuditableCustom;
+import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
 import org.apache.fineract.organisation.provisioning.constants.ProvisioningCriteriaConstants;
 import org.apache.fineract.organisation.provisioning.data.ProvisioningCriteriaDefinitionData;
 import org.apache.fineract.portfolio.loanproduct.domain.LoanProduct;
@@ -116,10 +119,19 @@ public Map<String, Object> update(JsonCommand command, List<LoanProduct> loanPro
 
     public void update(ProvisioningCriteriaDefinitionData data, GLAccount liability, GLAccount expense) {
         for (ProvisioningCriteriaDefinition def : provisioningCriteriaDefinition) {
-            if (data.getId().equals(def.getId())) {
+            // The public UPDATE payload keys each definition by categoryId (assumed unique per criteria; not
+            // DB-enforced), not by the surrogate id, which it never carries. Match on categoryId so the
+            // always-null data.getId() is no longer dereferenced (which threw a NullPointerException / HTTP 500
+            // on every update).
+            if (data.getCategoryId() != null && data.getCategoryId().equals(def.getCategoryId())) {
                 def.update(data.getMinAge(), data.getMaxAge(), data.getProvisioningPercentage(), liability, expense);
-                break;
+                return;
             }
         }
+        final List<ApiParameterError> errors = new ArrayList<>();
+        errors.add(ApiParameterError.parameterError("error.msg.provisioningcriteria.definition.category.not.found",
+                "Provisioning criteria has no definition for the given category", ProvisioningCriteriaConstants.JSON_CATEOGRYID_PARAM,
+                data.getCategoryId()));
+        throw new PlatformApiDataValidationException(errors);
     }
 }

fineract-provider/src/main/java/org/apache/fineract/organisation/provisioning/domain/ProvisioningCriteriaDefinition.java

@@ -79,6 +79,10 @@ public static ProvisioningCriteriaDefinition newPrivisioningCriteria(Provisionin
                 liabilityAccount, expenseAccount);
     }
 
+    public Long getCategoryId() {
+        return this.provisioningCategory == null ? null : this.provisioningCategory.getId();
+    }
+
     public void update(Long minAge, Long maxAge, BigDecimal percentage, GLAccount lia, GLAccount exp) {
         this.minimumAge = minAge;
         this.maximumAge = maxAge;

fineract-provider/src/test/java/org/apache/fineract/organisation/provisioning/domain/ProvisioningCriteriaTest.java

@@ -0,0 +1,98 @@
+/**
+ * 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.organisation.provisioning.domain;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.math.BigDecimal;
+import java.util.Set;
+import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
+import org.apache.fineract.organisation.provisioning.data.ProvisioningCriteriaDefinitionData;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression coverage for
+ * {@link ProvisioningCriteria#update(ProvisioningCriteriaDefinitionData, org.apache.fineract.accounting.glaccount.domain.GLAccount, org.apache.fineract.accounting.glaccount.domain.GLAccount)}.
+ *
+ * <p>
+ * The public UPDATE payload keys each definition only by {@code categoryId}; it never carries the surrogate {@code id}.
+ * The method used to match definitions by {@code data.getId().equals(def.getId())}, so {@code data.getId()} was null
+ * and every update threw a {@link NullPointerException} (HTTP 500). These tests assert it now matches by
+ * {@code categoryId} and rejects an unknown category with a clean validation exception instead.
+ */
+class ProvisioningCriteriaTest {
+
+    @Test
+    void update_matchesExistingDefinitionByCategoryId() {
+        final ProvisioningCriteriaDefinition def = mock(ProvisioningCriteriaDefinition.class);
+        when(def.getCategoryId()).thenReturn(7L);
+
+        final ProvisioningCriteria criteria = new ProvisioningCriteria();
+        criteria.setProvisioningCriteriaDefinitions(Set.of(def));
+
+        final ProvisioningCriteriaDefinitionData data = new ProvisioningCriteriaDefinitionData().setCategoryId(7L).setMinAge(0L)
+                .setMaxAge(30L).setProvisioningPercentage(BigDecimal.valueOf(25));
+
+        criteria.update(data, null, null);
+
+        verify(def, times(1)).update(0L, 30L, BigDecimal.valueOf(25), null, null);
+    }
+
+    @Test
+    void update_withMultipleDefinitions_updatesOnlyTheMatchingCategory() {
+        final ProvisioningCriteriaDefinition matched = mock(ProvisioningCriteriaDefinition.class);
+        when(matched.getCategoryId()).thenReturn(2L);
+        final ProvisioningCriteriaDefinition other = mock(ProvisioningCriteriaDefinition.class);
+        when(other.getCategoryId()).thenReturn(5L);
+
+        final ProvisioningCriteria criteria = new ProvisioningCriteria();
+        criteria.setProvisioningCriteriaDefinitions(Set.of(matched, other));
+
+        final ProvisioningCriteriaDefinitionData data = new ProvisioningCriteriaDefinitionData().setCategoryId(2L).setMinAge(31L)
+                .setMaxAge(60L).setProvisioningPercentage(BigDecimal.valueOf(50));
+
+        criteria.update(data, null, null);
+
+        verify(matched, times(1)).update(31L, 60L, BigDecimal.valueOf(50), null, null);
+        verify(other, never()).update(any(), any(), any(), any(), any());
+    }
+
+    @Test
+    void update_withUnknownCategoryId_throwsValidationExceptionNotNpe() {
+        final ProvisioningCriteriaDefinition def = mock(ProvisioningCriteriaDefinition.class);
+        when(def.getCategoryId()).thenReturn(7L);
+
+        final ProvisioningCriteria criteria = new ProvisioningCriteria();
+        criteria.setProvisioningCriteriaDefinitions(Set.of(def));
+
+        // categoryId 99 is not present on the criteria, and the data carries no surrogate id (as the real payload
+        // does).
+        final ProvisioningCriteriaDefinitionData data = new ProvisioningCriteriaDefinitionData().setCategoryId(99L).setMinAge(0L)
+                .setMaxAge(30L).setProvisioningPercentage(BigDecimal.valueOf(25));
+
+        assertThrows(PlatformApiDataValidationException.class, () -> criteria.update(data, null, null));
+        verify(def, never()).update(any(), any(), any(), any(), any());
+    }
+}