From cccfb11b59becaaf86adc4c88600bd70f2519b0d Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 23 Apr 2019 13:27:55 -0400 Subject: Validate supported types When a group is created, PAP should verify that the "supported types" exist in the DB. Address potential sonar issue. Address potential sonar issue in similar block of code. Change-Id: Ib830550bc37d4ebe42c8782f3f874e463f3f51c2 Issue-ID: POLICY-1688 Signed-off-by: Jim Hahn --- .../main/rest/depundep/PdpGroupDeployProvider.java | 41 +++++++++++++++------- .../pap/main/rest/depundep/ProviderBase.java | 8 ++++- .../policy/pap/main/rest/depundep/SessionData.java | 35 ++++++++++++++++-- .../pap/main/rest/depundep/ProviderSuper.java | 11 ++++++ .../rest/depundep/TestPdpGroupDeployProvider.java | 27 ++++++++++++-- .../pap/main/rest/depundep/TestProviderBase.java | 11 ++++++ .../pap/main/rest/depundep/TestSessionData.java | 40 +++++++++++++++++++-- .../test/resources/e2e/monitoring.policy-type.yaml | 2 +- .../test/resources/simpleDeploy/daoPolicyType.json | 4 +++ 9 files changed, 157 insertions(+), 22 deletions(-) create mode 100644 main/src/test/resources/simpleDeploy/daoPolicyType.json diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java index ee66b35c..fa721054 100644 --- a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java +++ b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java @@ -251,6 +251,7 @@ public class PdpGroupDeployProvider extends ProviderBase { BeanValidationResult result = new BeanValidationResult(subgrp.getPdpType(), subgrp); + result.addResult(validateSupportedTypes(data, subgrp)); result.addResult(validatePolicies(data, subgrp)); return result; @@ -345,32 +346,46 @@ public class PdpGroupDeployProvider extends ProviderBase { } /** - * Performs additional validations of the policies within a subgroup. + * Performs additional validations of the supported policy types within a subgroup. * * @param data session data * @param subgrp the subgroup to be validated * @param result the validation result * @throws PfModelException if an error occurred */ - private ValidationResult validatePolicies(SessionData data, PdpSubGroup subgrp) throws PfModelException { + private ValidationResult validateSupportedTypes(SessionData data, PdpSubGroup subgrp) throws PfModelException { BeanValidationResult result = new BeanValidationResult(subgrp.getPdpType(), subgrp); - for (ToscaPolicyIdentifier ident : subgrp.getPolicies()) { - try { - ToscaPolicy policy = data.getPolicy(new ToscaPolicyIdentifierOptVersion(ident)); + for (ToscaPolicyTypeIdentifier type : subgrp.getSupportedPolicyTypes()) { + if (data.getPolicyType(type) == null) { + result.addResult(new ObjectValidationResult("policy type", type, ValidationStatus.INVALID, + "unknown policy type")); + } + } - if (!subgrp.getSupportedPolicyTypes().contains(policy.getTypeIdentifier())) { - result.addResult(new ObjectValidationResult("policy", ident, ValidationStatus.INVALID, - "not a supported policy for the subgroup")); - } + return result; + } - } catch (PfModelException e) { - if (e.getErrorResponse().getResponseCode() != Status.NOT_FOUND) { - throw e; - } + /** + * Performs additional validations of the policies within a subgroup. + * + * @param data session data + * @param subgrp the subgroup to be validated + * @param result the validation result + * @throws PfModelException if an error occurred + */ + private ValidationResult validatePolicies(SessionData data, PdpSubGroup subgrp) throws PfModelException { + BeanValidationResult result = new BeanValidationResult(subgrp.getPdpType(), subgrp); + for (ToscaPolicyIdentifier ident : subgrp.getPolicies()) { + ToscaPolicy policy = data.getPolicy(new ToscaPolicyIdentifierOptVersion(ident)); + if (policy == null) { result.addResult(new ObjectValidationResult("policy", ident, ValidationStatus.INVALID, "unknown policy")); + + } else if (!subgrp.getSupportedPolicyTypes().contains(policy.getTypeIdentifier())) { + result.addResult(new ObjectValidationResult("policy", ident, ValidationStatus.INVALID, + "not a supported policy for the subgroup")); } } diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/ProviderBase.java b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/ProviderBase.java index a66b03d2..d3f0d13e 100644 --- a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/ProviderBase.java +++ b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/ProviderBase.java @@ -248,7 +248,13 @@ public abstract class ProviderBase { */ private ToscaPolicy getPolicy(SessionData data, ToscaPolicyIdentifierOptVersion ident) { try { - return data.getPolicy(ident); + ToscaPolicy policy = data.getPolicy(ident); + if (policy == null) { + throw new PfModelRuntimeException(Status.NOT_FOUND, + "cannot find policy: " + ident.getName() + " " + ident.getVersion()); + } + + return policy; } catch (PfModelException e) { throw new PfModelRuntimeException(e.getErrorResponse().getResponseCode(), diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java index 98a5e675..a76d6e13 100644 --- a/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java +++ b/main/src/main/java/org/onap/policy/pap/main/rest/depundep/SessionData.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; import java.util.stream.Collectors; -import javax.ws.rs.core.Response.Status; import org.apache.commons.lang3.tuple.Pair; import org.onap.policy.models.base.PfModelException; import org.onap.policy.models.pdp.concepts.PdpGroup; @@ -40,6 +39,7 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyFilter; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyFilter.ToscaPolicyFilterBuilder; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifierOptVersion; +import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyType; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyTypeIdentifier; /** @@ -79,6 +79,11 @@ public class SessionData { */ private final Map policyCache = new HashMap<>(); + /** + * Maps a policy type's identifier to the policy. + */ + private final Map typeCache = new HashMap<>(); + /** * Constructs the object. @@ -89,6 +94,31 @@ public class SessionData { this.dao = dao; } + /** + * Gets the policy type, referenced by an identifier. Loads it from the cache, if possible. + * Otherwise, gets it from the DB. + * + * @param desiredType policy type identifier + * @return the specified policy type + * @throws PfModelException if an error occurred + */ + public ToscaPolicyType getPolicyType(ToscaPolicyTypeIdentifier desiredType) throws PfModelException { + + ToscaPolicyType type = typeCache.get(desiredType); + if (type == null) { + + List lst = dao.getPolicyTypeList(desiredType.getName(), desiredType.getVersion()); + if (lst.isEmpty()) { + return null; + } + + type = lst.get(0); + typeCache.put(desiredType, type); + } + + return type; + } + /** * Gets the policy, referenced by an identifier. Loads it from the cache, if possible. * Otherwise, gets it from the DB. @@ -106,8 +136,7 @@ public class SessionData { List lst = dao.getFilteredPolicyList(filterBuilder.build()); if (lst.isEmpty()) { - throw new PfModelException(Status.NOT_FOUND, - "cannot find policy: " + desiredPolicy.getName() + " " + desiredPolicy.getVersion()); + return null; } policy = lst.get(0); diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java index 256d3af0..2fca6848 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java @@ -48,6 +48,7 @@ import org.onap.policy.models.pdp.concepts.PdpStateChange; import org.onap.policy.models.pdp.concepts.PdpUpdate; import org.onap.policy.models.provider.PolicyModelsProvider; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; +import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyType; import org.onap.policy.pap.main.PapConstants; import org.onap.policy.pap.main.PolicyModelsProviderFactoryWrapper; import org.onap.policy.pap.main.comm.PdpModifyRequestMap; @@ -233,6 +234,16 @@ public class ProviderSuper { return loadFile(fileName, ToscaPolicy.class); } + /** + * Loads a policy type. + * + * @param fileName name of the file from which to load + * @return a policy type + */ + protected ToscaPolicyType loadPolicyType(String fileName) { + return loadFile(fileName, ToscaPolicyType.class); + } + /** * Loads an object from a JSON file. * diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java index 406345c6..81664176 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java @@ -83,6 +83,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { super.setUp(); when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("daoPolicyList.json")); + when(dao.getPolicyTypeList("typeA", "100.2.3")).thenReturn(Arrays.asList(loadPolicyType("daoPolicyType.json"))); prov = new PdpGroupDeployProvider(); } @@ -356,14 +357,36 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { assertGroupUpdateOnly(group); } + @Test + public void testAddSubGroup_ValidationPolicyTypeNotFound() throws Exception { + PdpGroups groups = loadPdpGroups("createGroupsNewSub.json"); + PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); + when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); + + when(dao.getPolicyTypeList(any(), any())).thenReturn(Collections.emptyList()); + + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).hasMessageContaining("unknown policy type"); + } + + @Test + public void testAddSubGroup_ValidationPolicyTypeDaoEx() throws Exception { + PdpGroups groups = loadPdpGroups("createGroupsNewSub.json"); + PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); + when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); + + PfModelException exc = new PfModelException(Status.CONFLICT, EXPECTED_EXCEPTION); + when(dao.getPolicyTypeList(any(), any())).thenThrow(exc); + + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isSameAs(exc); + } + @Test public void testAddSubGroup_ValidationPolicyNotFound() throws Exception { PdpGroups groups = loadPdpGroups("createGroupsNewSub.json"); PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); - PfModelException exc = new PfModelException(Status.NOT_FOUND, EXPECTED_EXCEPTION); - when(dao.getFilteredPolicyList(any())).thenThrow(exc); + when(dao.getFilteredPolicyList(any())).thenReturn(Collections.emptyList()); assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).hasMessageContaining("unknown policy"); } diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java index e8ddd821..55c6f3d4 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java @@ -150,6 +150,17 @@ public class TestProviderBase extends ProviderSuper { .hasCause(exc); } + @Test + public void testGetPolicy_NotFound() throws Exception { + when(dao.getFilteredPolicyList(any())).thenReturn(Collections.emptyList()); + + assertThatThrownBy(() -> prov.process(loadRequest(), this::handle)).isInstanceOf(PfModelRuntimeException.class) + .hasMessage("cannot find policy: policyA 1.2.3").matches(thr -> { + PfModelRuntimeException exc = (PfModelRuntimeException) thr; + return (exc.getErrorResponse().getResponseCode() == Status.NOT_FOUND); + }); + } + @Test public void testGetGroup() throws Exception { when(dao.getFilteredPdpGroups(any())).thenReturn(loadGroups("getGroupDao.json")) diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java index 17acd5a2..f586d167 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java @@ -51,6 +51,7 @@ import org.onap.policy.models.pdp.concepts.PdpUpdate; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyFilter; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifierOptVersion; +import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyType; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyTypeIdentifier; public class TestSessionData extends ProviderSuper { @@ -90,6 +91,32 @@ public class TestSessionData extends ProviderSuper { session = new SessionData(dao); } + @Test + public void testGetPolicyType() throws Exception { + ToscaPolicyType policy1 = makePolicyType(POLICY_TYPE, POLICY_TYPE_VERSION); + when(dao.getPolicyTypeList(POLICY_TYPE, POLICY_TYPE_VERSION)).thenReturn(Arrays.asList(policy1)); + + assertSame(policy1, session.getPolicyType(type)); + + // retrieve a second time - should use cache + assertSame(policy1, session.getPolicyType(type)); + } + + @Test + public void testGetPolicyType_NotFound() throws Exception { + when(dao.getPolicyTypeList(any(), any())).thenReturn(Collections.emptyList()); + + assertNull(session.getPolicyType(type)); + } + + @Test + public void testGetPolicyType_DaoEx() throws Exception { + PfModelException ex = new PfModelException(Status.INTERNAL_SERVER_ERROR, EXPECTED_EXCEPTION); + when(dao.getPolicyTypeList(POLICY_TYPE, POLICY_TYPE_VERSION)).thenThrow(ex); + + assertThatThrownBy(() -> session.getPolicyType(type)).isSameAs(ex); + } + @Test public void testGetPolicy_NullVersion() throws Exception { ToscaPolicy policy1 = makePolicy(POLICY_NAME, POLICY_VERSION); @@ -148,12 +175,12 @@ public class TestSessionData extends ProviderSuper { public void testGetPolicy_NotFound() throws Exception { when(dao.getFilteredPolicyList(any())).thenReturn(Collections.emptyList()); - assertThatThrownBy(() -> session.getPolicy(ident)).hasMessage("cannot find policy: myPolicy 1.2.3"); + assertNull(session.getPolicy(ident)); } @Test public void testGetPolicy_DaoEx() throws Exception { - PfModelException ex = new PfModelException(Status.INTERNAL_SERVER_ERROR, "expected exception"); + PfModelException ex = new PfModelException(Status.INTERNAL_SERVER_ERROR, EXPECTED_EXCEPTION); when(dao.getFilteredPolicyList(any())).thenThrow(ex); assertThatThrownBy(() -> session.getPolicy(ident)).isSameAs(ex); @@ -270,6 +297,15 @@ public class TestSessionData extends ProviderSuper { assertEquals(Arrays.asList(change1, change2, change3).toString(), lst.toString()); } + private ToscaPolicyType makePolicyType(String name, String version) { + ToscaPolicyType type = new ToscaPolicyType(); + + type.setName(name); + type.setVersion(version); + + return type; + } + private ToscaPolicy makePolicy(String name, String version) { ToscaPolicy policy = new ToscaPolicy(); diff --git a/main/src/test/resources/e2e/monitoring.policy-type.yaml b/main/src/test/resources/e2e/monitoring.policy-type.yaml index 98b64522..28e5d6e5 100644 --- a/main/src/test/resources/e2e/monitoring.policy-type.yaml +++ b/main/src/test/resources/e2e/monitoring.policy-type.yaml @@ -6,7 +6,7 @@ policy_types: description: a base policy type for all policies that governs monitoring provisioning version: 1.0.0 - - onap.policy.monitoring.cdap.tca.hi.lo.app: + onap.policies.monitoring.cdap.tca.hi.lo.app: derived_from: onap.policies.Monitoring version: 1.0.0 properties: diff --git a/main/src/test/resources/simpleDeploy/daoPolicyType.json b/main/src/test/resources/simpleDeploy/daoPolicyType.json new file mode 100644 index 00000000..e71bf980 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/daoPolicyType.json @@ -0,0 +1,4 @@ +{ + "name": "typeA", + "version": "100.2.3" +} -- cgit 1.2.3-korg