Handling supported policy type during PdpGroup Update

Change-Id: I14daaa3d56d3293095227e0e3121e4fd82425b68
Issue-ID: POLICY-2023
Signed-off-by: a.sreekumar <ajith.sreekumar@est.tech>

3 files changed, 38 insertions, 41 deletions

diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupCreateOrUpdateProvider.java b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupCreateOrUpdateProvider.java
index 37f19c4e..ee6cbe4c 100644
--- a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupCreateOrUpdateProvider.java
+++ b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupCreateOrUpdateProvider.java
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP PAP
  * ================================================================================
- * Copyright (C) 2019 Nordix Foundation.
+ * Copyright (C) 2019-2020 Nordix Foundation.
  * Modifications Copyright (C) 2019 AT&T Intellectual Property.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -21,7 +21,6 @@
 
 package org.onap.policy.pap.main.rest;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -78,8 +77,8 @@ public class PdpGroupCreateOrUpdateProvider extends ProviderBase {
      * @throws PfModelException if an error occurred
      */
     public void createOrUpdateGroups(PdpGroups groups) throws PfModelException {
-        ValidationResult result = groups.validatePapRest();
-
+        BeanValidationResult result = new BeanValidationResult("groups", groups);
+        groups.checkForDuplicateGroups(result);
         if (!result.isValid()) {
             String msg = result.getResult().trim();
             logger.warn(msg);
@@ -89,7 +88,8 @@ public class PdpGroupCreateOrUpdateProvider extends ProviderBase {
         // There is a separate API for this.
         List<PdpSubGroup> subGroupsWithPolicies =
             groups.getGroups().parallelStream().flatMap(group -> group.getPdpSubgroups().parallelStream())
-                .filter(subGroup -> !subGroup.getPolicies().isEmpty()).collect(Collectors.toList());
+                .filter(subGroup -> null != subGroup.getPolicies() && !subGroup.getPolicies().isEmpty())
+                .collect(Collectors.toList());
         if (!subGroupsWithPolicies.isEmpty()) {
             logger.warn(
                 "Policies cannot be deployed during PdpGroup Create/Update operation. Ignoring the list of policies");
@@ -110,12 +110,24 @@ public class PdpGroupCreateOrUpdateProvider extends ProviderBase {
 
         for (PdpGroup group : groups.getGroups()) {
             PdpGroup dbgroup = data.getGroup(group.getName());
-
+            ValidationResult groupValidationResult;
             if (dbgroup == null) {
-                result.addResult(addGroup(data, group));
+                // create group flow
+                groupValidationResult = group.validatePapRest(false);
+                if (groupValidationResult.isValid()) {
+                    result.addResult(addGroup(data, group));
+                } else {
+                    result.addResult(groupValidationResult);
+                }
 
             } else {
-                result.addResult(updateGroup(data, dbgroup, group));
+                // update group flow
+                groupValidationResult = group.validatePapRest(true);
+                if (groupValidationResult.isValid()) {
+                    result.addResult(updateGroup(data, dbgroup, group));
+                } else {
+                    result.addResult(groupValidationResult);
+                }
             }
         }
 
@@ -376,11 +388,18 @@ public class PdpGroupCreateOrUpdateProvider extends ProviderBase {
             return false;
         }
 
-        boolean updated = updateList(dbsub.getSupportedPolicyTypes(), subgrp.getSupportedPolicyTypes(),
-            dbsub::setSupportedPolicyTypes);
+        if (null != subgrp.getSupportedPolicyTypes() && !new HashSet<>(dbsub.getSupportedPolicyTypes())
+            .equals(new HashSet<>(subgrp.getSupportedPolicyTypes()))) {
+            logger.warn("Supported policy types cannot be updated while updating PdpGroup. "
+                + "Hence, ignoring the new set of supported policy types.");
+        }
 
+        // while updating PdpGroup, list of policies (already deployed ones) and supported policies (the ones provided
+        // during PdpGroup creation) has to be retained
+        subgrp.setSupportedPolicyTypes(dbsub.getSupportedPolicyTypes());
+        subgrp.setPolicies(dbsub.getPolicies());
         return updateField(dbsub.getDesiredInstanceCount(), subgrp.getDesiredInstanceCount(),
-            dbsub::setDesiredInstanceCount) || updated;
+            dbsub::setDesiredInstanceCount);
     }
 
     /**
@@ -404,36 +423,13 @@ public class PdpGroupCreateOrUpdateProvider extends ProviderBase {
                 new ObjectValidationResult("properties", "", ValidationStatus.INVALID, "cannot change properties"));
         }
 
-        result.addResult(validateSupportedTypes(data, subgrp));
         container.addResult(result);
 
         return result.isValid();
     }
 
     /**
-     * Updates a DB list with items from a new list.
-     *
-     * @param dblist the list from the DB
-     * @param newList the new list
-     * @param setter function to set the new list
-     * @return {@code true} if the list changed, {@code false} if the lists were the same
-     */
-    private <T> boolean updateList(List<T> dblist, List<T> newList, Consumer<List<T>> setter) {
-
-        Set<T> dbTypes = new HashSet<>(dblist);
-        Set<T> newTypes = new HashSet<>(newList);
-
-        if (dbTypes.equals(newTypes)) {
-            return false;
-        }
-
-        setter.accept(new ArrayList<>(newTypes));
-
-        return true;
-    }
-
-    /**
-     * Performs additional validations of the supported policy types within a subgroup.
+     * Performs validations of the supported policy types within a subgroup.
      *
      * @param data session data
      * @param subgrp the subgroup to be validated
@@ -442,7 +438,6 @@ public class PdpGroupCreateOrUpdateProvider extends ProviderBase {
      */
     private ValidationResult validateSupportedTypes(SessionData data, PdpSubGroup subgrp) throws PfModelException {
         BeanValidationResult result = new BeanValidationResult(subgrp.getPdpType(), subgrp);
-
         for (ToscaPolicyTypeIdentifier type : subgrp.getSupportedPolicyTypes()) {
             if (!type.getName().endsWith(".*") && data.getPolicyType(type) == null) {
                 result.addResult(
diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupCreateOrUpdateProvider.java b/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupCreateOrUpdateProvider.java
index 1ac97c77..ddf24293 100644
--- a/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupCreateOrUpdateProvider.java
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupCreateOrUpdateProvider.java
@@ -2,7 +2,7 @@
  * ============LICENSE_START=======================================================
  * ONAP PAP
  * ================================================================================
- * Copyright (C) 2019 Nordix Foundation.
+ * Copyright (C) 2019-2020 Nordix Foundation.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,7 +25,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -479,10 +479,13 @@ public class TestPdpGroupCreateOrUpdateProvider extends ProviderSuper {
         newgrp.getPdpSubgroups().get(0).getSupportedPolicyTypes()
             .add(new ToscaPolicyTypeIdentifier("typeX.*", "9.8.7"));
 
+        // the group is updated with a new supported policy type in subgroup
+        assertEquals(2, newgrp.getPdpSubgroups().get(0).getSupportedPolicyTypes().size());
         prov.createOrUpdateGroups(groups);
-
+        // PdpGroup update doesn't allow supported policy type modifications
+        // during pdp group update, the ones in db is maintained
+        assertEquals(1, newgrp.getPdpSubgroups().get(0).getSupportedPolicyTypes().size());
         assertEquals(newgrp.toString(), group.toString());
-        assertGroupUpdateOnly(group);
     }
 
     @Test
diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/End2EndBase.java b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/End2EndBase.java
index abec6d72..3f4015b1 100644
--- a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/End2EndBase.java
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/End2EndBase.java
@@ -26,7 +26,6 @@ import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.List;
-
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;