From a8765f7d8a9be082c281d46592307f5056f32fcd Mon Sep 17 00:00:00 2001 From: liamfallon Date: Wed, 19 Feb 2020 16:56:59 +0000 Subject: [PATCH] Changes for cascaded get in policy models This review adapts policy API for changes introduced for cascaded returns on filtered gets in policy-models. Issue-ID: POLICY-1402 Change-Id: I7593eb64cc497809f8485f6add40b6a64291a060 Signed-off-by: liamfallon --- .../main/rest/provider/CommonModelProvider.java | 136 +++------------------ .../api/main/rest/provider/PolicyProvider.java | 48 +------- .../api/main/rest/provider/PolicyTypeProvider.java | 46 +------ .../policy/api/main/rest/TestApiRestServer.java | 58 ++++++++- .../main/rest/provider/TestPolicyTypeProvider.java | 6 +- 5 files changed, 76 insertions(+), 218 deletions(-) diff --git a/main/src/main/java/org/onap/policy/api/main/rest/provider/CommonModelProvider.java b/main/src/main/java/org/onap/policy/api/main/rest/provider/CommonModelProvider.java index 4a490896..db864d7a 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/provider/CommonModelProvider.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/provider/CommonModelProvider.java @@ -3,7 +3,7 @@ * ONAP Policy API * ================================================================================ * Copyright (C) 2019-2020 AT&T Intellectual Property. All rights reserved. - * Modifications Copyright (C) 2019 Nordix Foundation. + * Modifications 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. @@ -28,6 +28,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.BiConsumer; + import javax.ws.rs.core.Response; import org.apache.commons.lang3.tuple.Pair; @@ -42,10 +43,8 @@ import org.onap.policy.models.pdp.enums.PdpState; import org.onap.policy.models.provider.PolicyModelsProvider; import org.onap.policy.models.provider.PolicyModelsProviderFactory; import org.onap.policy.models.provider.PolicyModelsProviderParameters; -import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyTypeIdentifier; -import org.onap.policy.models.tosca.authorative.concepts.ToscaServiceTemplate; /** * Super class for providers that use a model provider. @@ -77,113 +76,6 @@ public class CommonModelProvider implements AutoCloseable { modelsProvider.close(); } - /** - * Checks if service template contains any policy. - * - * @param serviceTemplate the service template to check against - * - * @return boolean whether service template contains any policy - */ - protected boolean hasPolicy(ToscaServiceTemplate serviceTemplate) { - - return hasData(serviceTemplate.getToscaTopologyTemplate().getPolicies()); - } - - /** - * Checks if service template contains any policy type. - * - * @param serviceTemplate the service template to check against - * - * @return boolean whether service template contains any policy type - */ - protected boolean hasPolicyType(ToscaServiceTemplate serviceTemplate) { - - return hasData(serviceTemplate.getPolicyTypes()); - } - - /** - * Checks if the first element of a list of maps contains data. - * - * @param listOfMapsToCheck list of maps to be examined - * @return {@code true} if the list contains data, {@code false} otherwise - */ - protected boolean hasData(List> listOfMapsToCheck) { - - return (listOfMapsToCheck != null && !listOfMapsToCheck.isEmpty() && !listOfMapsToCheck.get(0).isEmpty()); - } - - - /** - * Checks if a maps contains data. - * - * @param mapToCheck map to be examined - * @return {@code true} if the list contains data, {@code false} otherwise - */ - protected boolean hasData(Map mapToCheck) { - - // We don't allow a null or empty map as well as a map entry with a valid key but null value - return (mapToCheck != null && !mapToCheck.isEmpty() && !mapToCheck.containsValue(null)); - } - - /** - * Validates that some text represents a number. - * - * @param text text to be validated - * @param errorMsg error message included in the exception, if the text is not a valid - * number - * @throws PfModelException if the text is not a valid number - */ - protected void validNumber(String text, String errorMsg) throws PfModelException { - try { - Integer.parseInt(text); - - } catch (NumberFormatException exc) { - throw new PfModelException(Response.Status.BAD_REQUEST, errorMsg, exc); - } - } - - /** - * Constructs returned message for policy delete rule violation. - * - * @param policyId the ID of policy - * @param policyVersion the version of policy - * @param pdpGroups the list of pdp groups - * - * @return the constructed message - */ - protected String constructDeletePolicyViolationMessage(String policyId, String policyVersion, - List pdpGroups) { - - List pdpGroupNameVersionList = new ArrayList<>(pdpGroups.size()); - for (PdpGroup pdpGroup : pdpGroups) { - pdpGroupNameVersionList.add(pdpGroup.getName() + ":" + pdpGroup.getVersion()); - } - String deployedPdpGroups = String.join(",", pdpGroupNameVersionList); - return "policy with ID " + policyId + ":" + policyVersion - + " cannot be deleted as it is deployed in pdp groups " + deployedPdpGroups; - } - - /** - * Constructs returned message for policy type delete rule violation. - * - * @param policyTypeId the ID of policy type - * @param policyTypeVersion the version of policy type - * @param policies the list of policies that parameterizes specified policy type - * - * @return the constructed message - */ - protected String constructDeletePolicyTypeViolationMessage(String policyTypeId, String policyTypeVersion, - List policies) { - - List policyNameVersionList = new ArrayList<>(policies.size()); - for (ToscaPolicy policy : policies) { - policyNameVersionList.add(policy.getName() + ":" + policy.getVersion()); - } - String parameterizedPolicies = String.join(",", policyNameVersionList); - return "policy type with ID " + policyTypeId + ":" + policyTypeVersion - + " cannot be deleted as it is parameterized by policies " + parameterizedPolicies; - } - /** * Collects all deployed versions of specified policy in all pdp groups. * @@ -206,13 +98,13 @@ public class CommonModelProvider implements AutoCloseable { } @FunctionalInterface - protected interface BiFunctionWithEx { + protected interface BiFunctionWithEx { public R apply(T value1, U value2) throws PfModelException; } /** - * Checks if the list of pdp groups is empty. - * If so, throws exception saying specified policy deployment is not found in all existing pdp groups. + * Checks if the list of pdp groups is empty. If so, throws exception saying specified policy deployment is not + * found in all existing pdp groups. * * @param pdpGroups the list of pdp groups to check against * @param policyType the concept key of policy type @@ -239,13 +131,12 @@ public class CommonModelProvider implements AutoCloseable { * * @throws PfModelException the PfModel parsing exception */ - private List getPolicyTypeFilteredPdpGroups(PfConceptKey policyType) - throws PfModelException { + private List getPolicyTypeFilteredPdpGroups(PfConceptKey policyType) throws PfModelException { List policyTypes = new ArrayList<>(); policyTypes.add(new ToscaPolicyTypeIdentifier(policyType.getName(), policyType.getVersion())); - PdpGroupFilter pdpGroupFilter = PdpGroupFilter.builder().policyTypeList(policyTypes) - .groupState(PdpState.ACTIVE).pdpState(PdpState.ACTIVE).build(); + PdpGroupFilter pdpGroupFilter = PdpGroupFilter.builder().policyTypeList(policyTypes).groupState(PdpState.ACTIVE) + .pdpState(PdpState.ACTIVE).build(); return modelsProvider.getFilteredPdpGroups(pdpGroupFilter); } @@ -265,7 +156,7 @@ public class CommonModelProvider implements AutoCloseable { */ private Map, T> constructDeployedPolicyMap(List pdpGroups, String policyId, PfConceptKey policyType, BiFunctionWithEx getter, BiConsumer consumer, T data) - throws PfModelException { + throws PfModelException { Map, T> deployedPolicyMap = new HashMap<>(); for (PdpGroup pdpGroup : pdpGroups) { @@ -338,8 +229,9 @@ public class CommonModelProvider implements AutoCloseable { * @return the trimmed version */ private String getTrimedVersionForLegacyType(String fullVersion, PfConceptKey policyType) { - return (policyType.getName().contains("guard") - || policyType.getName().contains("Operational")) ? fullVersion.split("\\.")[0] : fullVersion; + return (policyType.getName().contains("guard") || policyType.getName().contains("Operational")) + ? fullVersion.split("\\.")[0] + : fullVersion; } /** @@ -352,7 +244,7 @@ public class CommonModelProvider implements AutoCloseable { */ private String constructDeploymentNotFoundMessage(PfConceptKey policyType, String policyId) { - return "could not find policy with ID " + policyId + " and type " - + policyType.getName() + ":" + policyType.getVersion() + " deployed in any pdp group"; + return "could not find policy with ID " + policyId + " and type " + policyType.getName() + ":" + + policyType.getVersion() + " deployed in any pdp group"; } } diff --git a/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyProvider.java b/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyProvider.java index 6b123952..afed3165 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyProvider.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyProvider.java @@ -27,8 +27,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import javax.ws.rs.core.Response; - import org.apache.commons.lang3.tuple.Pair; import org.onap.policy.models.base.PfConceptKey; import org.onap.policy.models.base.PfModelException; @@ -65,15 +63,7 @@ public class PolicyProvider extends CommonModelProvider { public ToscaServiceTemplate fetchPolicies(String policyTypeId, String policyTypeVersion, String policyId, String policyVersion) throws PfModelException { - ToscaServiceTemplate serviceTemplate = - getFilteredPolicies(policyTypeId, policyTypeVersion, policyId, policyVersion); - - if (!hasPolicy(serviceTemplate)) { - throw new PfModelException(Response.Status.NOT_FOUND, - constructResourceNotFoundMessage(policyTypeId, policyTypeVersion, policyId, policyVersion)); - } - - return serviceTemplate; + return getFilteredPolicies(policyTypeId, policyTypeVersion, policyId, policyVersion); } /** @@ -90,15 +80,7 @@ public class PolicyProvider extends CommonModelProvider { public ToscaServiceTemplate fetchLatestPolicies(String policyTypeId, String policyTypeVersion, String policyId) throws PfModelException { - ToscaServiceTemplate serviceTemplate = - getFilteredPolicies(policyTypeId, policyTypeVersion, policyId, ToscaPolicyFilter.LATEST_VERSION); - - if (!hasPolicy(serviceTemplate)) { - throw new PfModelException(Response.Status.NOT_FOUND, - constructResourceNotFoundMessage(policyTypeId, policyTypeVersion, policyId, null)); - } - - return serviceTemplate; + return getFilteredPolicies(policyTypeId, policyTypeVersion, policyId, ToscaPolicyFilter.LATEST_VERSION); } /** @@ -164,14 +146,7 @@ public class PolicyProvider extends CommonModelProvider { public ToscaServiceTemplate deletePolicy(String policyTypeId, String policyTypeVersion, String policyId, String policyVersion) throws PfModelException { - ToscaServiceTemplate serviceTemplate = modelsProvider.deletePolicy(policyId, policyVersion); - - if (!hasPolicy(serviceTemplate)) { - throw new PfModelException(Response.Status.NOT_FOUND, - constructResourceNotFoundMessage(policyTypeId, policyTypeVersion, policyId, policyVersion)); - } - - return serviceTemplate; + return modelsProvider.deletePolicy(policyId, policyVersion); } /** @@ -193,21 +168,4 @@ public class PolicyProvider extends CommonModelProvider { .type(policyTypeName).typeVersion(policyTypeVersion).build(); return modelsProvider.getFilteredPolicies(policyFilter); } - - /** - * Constructs returned message for not found resource. - * - * @param policyTypeId the ID of policy type - * @param policyTypeVersion the version of policy type - * @param policyId the ID of policy - * @param policyVersion the version of policy - * - * @return constructed message - */ - private String constructResourceNotFoundMessage(String policyTypeId, String policyTypeVersion, String policyId, - String policyVersion) { - - return "policy with ID " + policyId + ":" + policyVersion + " and type " + policyTypeId + ":" - + policyTypeVersion + " does not exist"; - } } diff --git a/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyTypeProvider.java b/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyTypeProvider.java index 6c8e73e6..9c9c41cc 100644 --- a/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyTypeProvider.java +++ b/main/src/main/java/org/onap/policy/api/main/rest/provider/PolicyTypeProvider.java @@ -23,8 +23,6 @@ package org.onap.policy.api.main.rest.provider; -import javax.ws.rs.core.Response; - import org.onap.policy.models.base.PfModelException; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyTypeFilter; import org.onap.policy.models.tosca.authorative.concepts.ToscaServiceTemplate; @@ -56,14 +54,7 @@ public class PolicyTypeProvider extends CommonModelProvider { public ToscaServiceTemplate fetchPolicyTypes(String policyTypeId, String policyTypeVersion) throws PfModelException { - ToscaServiceTemplate serviceTemplate = getFilteredPolicyTypes(policyTypeId, policyTypeVersion); - - if (policyTypeId != null && !hasPolicyType(serviceTemplate)) { - throw new PfModelException(Response.Status.NOT_FOUND, - constructResourceNotFoundMessage(policyTypeId, policyTypeVersion)); - } - - return serviceTemplate; + return getFilteredPolicyTypes(policyTypeId, policyTypeVersion); } /** @@ -77,13 +68,7 @@ public class PolicyTypeProvider extends CommonModelProvider { */ public ToscaServiceTemplate fetchLatestPolicyTypes(String policyTypeId) throws PfModelException { - ToscaServiceTemplate serviceTemplate = - getFilteredPolicyTypes(policyTypeId, ToscaPolicyTypeFilter.LATEST_VERSION); - if (!hasPolicyType(serviceTemplate)) { - throw new PfModelException(Response.Status.NOT_FOUND, constructResourceNotFoundMessage(policyTypeId, null)); - } - - return serviceTemplate; + return getFilteredPolicyTypes(policyTypeId, ToscaPolicyTypeFilter.LATEST_VERSION); } /** @@ -96,11 +81,6 @@ public class PolicyTypeProvider extends CommonModelProvider { */ public ToscaServiceTemplate createPolicyType(ToscaServiceTemplate body) throws PfModelException { - if (!hasPolicyType(body)) { - throw new PfModelException(Response.Status.BAD_REQUEST, - "no policy types specified in the service template"); - } - return modelsProvider.createPolicyTypes(body); } @@ -117,14 +97,7 @@ public class PolicyTypeProvider extends CommonModelProvider { public ToscaServiceTemplate deletePolicyType(String policyTypeId, String policyTypeVersion) throws PfModelException { - ToscaServiceTemplate serviceTemplate = modelsProvider.deletePolicyType(policyTypeId, policyTypeVersion); - - if (!hasPolicyType(serviceTemplate)) { - throw new PfModelException(Response.Status.NOT_FOUND, - constructResourceNotFoundMessage(policyTypeId, policyTypeVersion)); - } - - return serviceTemplate; + return modelsProvider.deletePolicyType(policyTypeId, policyTypeVersion); } /** @@ -144,17 +117,4 @@ public class PolicyTypeProvider extends CommonModelProvider { ToscaPolicyTypeFilter.builder().name(policyTypeName).version(policyTypeVersion).build(); return modelsProvider.getFilteredPolicyTypes(policyTypeFilter); } - - /** - * Constructs returned message for not found resource. - * - * @param policyTypeId the ID of policy type - * @param policyTypeVersion the version of policy type - * - * @return constructed message - */ - private String constructResourceNotFoundMessage(String policyTypeId, String policyTypeVersion) { - - return "policy type with ID " + policyTypeId + ":" + policyTypeVersion + " does not exist"; - } } diff --git a/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java b/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java index d531a0ad..4af601a2 100644 --- a/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java +++ b/main/src/test/java/org/onap/policy/api/main/rest/TestApiRestServer.java @@ -317,9 +317,9 @@ public class TestApiRestServer { String firstPolicyType = response.getPolicyTypes().keySet().iterator().next(); response.getPolicyTypes().put(firstPolicyType, null); Response rawResponse2 = createResource(POLICYTYPES, standardCoder.encode(response)); - assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), rawResponse2.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawResponse2.getStatus()); ErrorResponse errorResponse = rawResponse2.readEntity(ErrorResponse.class); - assertEquals("no policy types specified in the service template", errorResponse.getErrorMessage()); + assertEquals("no policy types specified on service template", errorResponse.getErrorMessage()); } @Test @@ -504,7 +504,14 @@ public class TestApiRestServer { } private void testReadPolicyTypes(String mediaType) throws Exception { - Response rawResponse = readResource(POLICYTYPES, mediaType); + Response rawResponse = readResource("policytypes/onap.policies.optimization.resource.HpaPolicy", mediaType); + assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); + ToscaServiceTemplate namingServiceTemplate = rawResponse.readEntity(ToscaServiceTemplate.class); + assertNotNull(namingServiceTemplate); + assertEquals(3, namingServiceTemplate.getPolicyTypesAsMap().size()); + assertEquals(5, namingServiceTemplate.getDataTypesAsMap().size()); + + rawResponse = readResource(POLICYTYPES, mediaType); assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); ToscaServiceTemplate response = rawResponse.readEntity(ToscaServiceTemplate.class); assertFalse(response.getPolicyTypes().isEmpty()); @@ -602,6 +609,47 @@ public class TestApiRestServer { assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); } + @Test + public void testNamingPolicyGet() throws Exception { + Response rawResponse = createResource("policies", "policies/sdnc.policy.naming.input.tosca.yaml"); + assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); + + rawResponse = readResource("policytypes/" + + "onap.policies.Naming/versions/1.0.0/policies/SDNC_Policy.ONAP_VNF_NAMING_TIMESTAMP/versions/1.0.0", + APP_JSON); + assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); + + ToscaServiceTemplate namingServiceTemplate = rawResponse.readEntity(ToscaServiceTemplate.class); + assertEquals(1, namingServiceTemplate.getToscaTopologyTemplate().getPoliciesAsMap().size()); + assertEquals(1, namingServiceTemplate.getPolicyTypesAsMap().size()); + assertEquals(3, namingServiceTemplate.getDataTypesAsMap().size()); + + rawResponse = readResource("policytypes/" + + "onap.policies.Naming/versions/1.0.0/policies/SDNC_Policy.ONAP_VNF_NAMING_TIMESTAMP/versions/latest", + APP_JSON); + assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); + + namingServiceTemplate = rawResponse.readEntity(ToscaServiceTemplate.class); + assertEquals(1, namingServiceTemplate.getToscaTopologyTemplate().getPoliciesAsMap().size()); + assertEquals(1, namingServiceTemplate.getPolicyTypesAsMap().size()); + assertEquals(3, namingServiceTemplate.getDataTypesAsMap().size()); + + rawResponse = readResource( + "policytypes/" + "onap.policies.Naming/versions/1.0.0/policies/SDNC_Policy.ONAP_VNF_NAMING_TIMESTAMP", + APP_JSON); + assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); + + namingServiceTemplate = rawResponse.readEntity(ToscaServiceTemplate.class); + assertEquals(1, namingServiceTemplate.getToscaTopologyTemplate().getPoliciesAsMap().size()); + assertEquals(1, namingServiceTemplate.getPolicyTypesAsMap().size()); + assertEquals(3, namingServiceTemplate.getDataTypesAsMap().size()); + + rawResponse = deleteResource("policytypes/" + + "onap.policies.Naming/versions/1.0.0/policies/SDNC_Policy.ONAP_VNF_NAMING_TIMESTAMP/versions/1.0.0", + APP_JSON); + assertEquals(Response.Status.OK.getStatusCode(), rawResponse.getStatus()); + } + @Test public void testDeletePoliciesJson() throws Exception { testDeletePolicies(APP_JSON); @@ -648,8 +696,8 @@ public class TestApiRestServer { assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawResponse.getStatus()); ErrorResponse errorResponse = rawResponse.readEntity(ErrorResponse.class); assertEquals( - "policy with ID onap.restart.tca:1.0.0 and type " - + "onap.policies.monitoring.cdap.tca.hi.lo.app:1.0.0 does not exist", + "policies for filter ToscaPolicyFilter(name=onap.restart.tca, version=1.0.0, versionPrefix=null, " + + "type=onap.policies.monitoring.cdap.tca.hi.lo.app, typeVersion=1.0.0) do not exist", errorResponse.getErrorMessage()); rawResponse = deleteResource(POLICYTYPES_TCA_POLICIES_VCPE_VERSION2, mediaType); diff --git a/main/src/test/java/org/onap/policy/api/main/rest/provider/TestPolicyTypeProvider.java b/main/src/test/java/org/onap/policy/api/main/rest/provider/TestPolicyTypeProvider.java index 82d25245..58576abc 100644 --- a/main/src/test/java/org/onap/policy/api/main/rest/provider/TestPolicyTypeProvider.java +++ b/main/src/test/java/org/onap/policy/api/main/rest/provider/TestPolicyTypeProvider.java @@ -118,11 +118,11 @@ public class TestPolicyTypeProvider { assertThatThrownBy(() -> { policyTypeProvider.fetchPolicyTypes("dummy", null); - }).hasMessage("policy type with ID dummy:null does not exist"); + }).hasMessage("policy types for filter ToscaPolicyTypeFilter(name=dummy, version=null) do not exist"); assertThatThrownBy(() -> { policyTypeProvider.fetchPolicyTypes("dummy", "dummy"); - }).hasMessage("policy type with ID dummy:dummy does not exist"); + }).hasMessage("policy types for filter ToscaPolicyTypeFilter(name=dummy, version=dummy) do not exist"); } @Test @@ -130,7 +130,7 @@ public class TestPolicyTypeProvider { assertThatThrownBy(() -> { policyTypeProvider.fetchLatestPolicyTypes("dummy"); - }).hasMessage("policy type with ID dummy:null does not exist"); + }).hasMessage("policy types for filter ToscaPolicyTypeFilter(name=dummy, version=LATEST) do not exist"); } @Test -- 2.16.6