From b346fda03f450ccf9f8adb143f872cfce6ba76a8 Mon Sep 17 00:00:00 2001 From: jrh3 Date: Thu, 20 Jun 2019 14:57:50 -0400 Subject: Allow integer version when using PDP Group Deploy The policies listed in a "PDP Group Deploy" request may not have fully qualified versions. Modified the code to replace the versions in the request with fully qualified versions. Also improved performance by avoiding look-ups of policies that are already in the subgroup. Change-Id: I37899c2b45228b97a80b7ef44f69694ba57e8f4a Issue-ID: POLICY-1784 Signed-off-by: jrh3 --- .../main/rest/depundep/PdpGroupDeployProvider.java | 56 +++++++++++++++++----- .../policy/pap/main/rest/depundep/SessionData.java | 15 +++++- .../rest/depundep/TestPdpGroupDeployProvider.java | 50 +++++++++++++++++-- .../pap/main/rest/depundep/TestSessionData.java | 14 ++++++ .../simpleDeploy/createGroupsNewSubNotFound.json | 56 ++++++++++++++++++++++ .../simpleDeploy/createGroupsVersPrefix.json | 39 +++++++++++++++ .../createGroupsVersPrefixMismatch.json | 39 +++++++++++++++ 7 files changed, 252 insertions(+), 17 deletions(-) create mode 100644 main/src/test/resources/simpleDeploy/createGroupsNewSubNotFound.json create mode 100644 main/src/test/resources/simpleDeploy/createGroupsVersPrefix.json create mode 100644 main/src/test/resources/simpleDeploy/createGroupsVersPrefixMismatch.json (limited to 'main') 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 e861375e..bc3148ef 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 @@ -317,7 +317,7 @@ public class PdpGroupDeployProvider extends ProviderBase { * Adds a new subgroup. * * @param data session data - * @param subgrp the subgroup to be added + * @param subgrp the subgroup to be added, updated to fully qualified versions upon return * @return the validation result * @throws PfModelException if an error occurred */ @@ -339,7 +339,7 @@ public class PdpGroupDeployProvider extends ProviderBase { * @param data session data * @param dbgroup the group, from the DB, containing the subgroup * @param dbsub the subgroup, from the DB - * @param subgrp the subgroup to be updated + * @param subgrp the subgroup to be updated, updated to fully qualified versions upon return * @param container container for additional validation results * @return {@code true} if the subgroup content was changed, {@code false} if there * were no changes @@ -378,7 +378,7 @@ public class PdpGroupDeployProvider extends ProviderBase { * * @param data session data * @param dbsub the subgroup, from the DB - * @param subgrp the subgroup to be validated + * @param subgrp the subgroup to be validated, updated to fully qualified versions upon return * @param container container for additional validation results * @return {@code true} if the subgroup is valid, {@code false} otherwise * @throws PfModelException if an error occurred @@ -447,14 +447,15 @@ public class PdpGroupDeployProvider extends ProviderBase { * * @param data session data * @param dbsub subgroup from the DB, or {@code null} if this is a new subgroup - * @param subgrp the subgroup to be validated + * @param subgrp the subgroup whose policies are to be validated, updated to fully + * qualified versions upon return * @param result the validation result * @throws PfModelException if an error occurred */ private ValidationResult validatePolicies(SessionData data, PdpSubGroup dbsub, PdpSubGroup subgrp) throws PfModelException { - // build a map of the DB data, from policy name to policy version + // build a map of the DB data, from policy name to (fully qualified) policy version Map dbname2vers = new HashMap<>(); if (dbsub != null) { dbsub.getPolicies().forEach(ident -> dbname2vers.put(ident.getName(), ident.getVersion())); @@ -463,7 +464,17 @@ public class PdpGroupDeployProvider extends ProviderBase { BeanValidationResult result = new BeanValidationResult(subgrp.getPdpType(), subgrp); for (ToscaPolicyIdentifier ident : subgrp.getPolicies()) { - String actualVersion; + // note: "ident" may not have a fully qualified version + + String expectedVersion = dbname2vers.get(ident.getName()); + if (expectedVersion != null) { + // policy exists in the DB list - compare the versions + validateVersion(expectedVersion, ident, result); + ident.setVersion(expectedVersion); + continue; + } + + // policy doesn't appear in the DB's policy list - look it up ToscaPolicy policy = data.getPolicy(new ToscaPolicyIdentifierOptVersion(ident)); if (policy == null) { @@ -474,17 +485,40 @@ public class PdpGroupDeployProvider extends ProviderBase { result.addResult(new ObjectValidationResult(POLICY_RESULT_NAME, ident, ValidationStatus.INVALID, "not a supported policy for the subgroup")); - } else if ((actualVersion = dbname2vers.get(ident.getName())) != null - && !actualVersion.equals(ident.getVersion())) { - // policy exists in the DB subgroup, but has the wrong version - result.addResult(new ObjectValidationResult(POLICY_RESULT_NAME, ident, ValidationStatus.INVALID, - "different version already deployed: " + actualVersion)); + } else { + // replace version with the fully qualified version from the policy + ident.setVersion(policy.getVersion()); } } return result; } + /** + * Determines if the new version matches the version in the DB. + * + * @param dbvers fully qualified version from the DB + * @param ident identifier whose version is to be validated; the version need not be + * fully qualified + * @param result the validation result + */ + private void validateVersion(String dbvers, ToscaPolicyIdentifier ident, BeanValidationResult result) { + String idvers = ident.getVersion(); + if (dbvers.equals(idvers)) { + return; + } + + // did not match - see if it's a prefix + + if (SessionData.isVersionPrefix(idvers) && dbvers.startsWith(idvers + ".")) { + // ident has a prefix of this version + return; + } + + result.addResult(new ObjectValidationResult(POLICY_RESULT_NAME, ident, ValidationStatus.INVALID, + "different version already deployed: " + dbvers)); + } + /** * Deploys or updates PDP policies using the simple API. * 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 ee83fb74..437d7a11 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 @@ -54,7 +54,7 @@ public class SessionData { * If a version string matches this, then it is just a prefix (i.e., major or * major.minor). */ - private static final Pattern VERSION_PREFIX_PAT = Pattern.compile("[^.]+(?:[.][^.]*)?"); + private static final Pattern VERSION_PREFIX_PAT = Pattern.compile("[^.]+(?:[.][^.]+)?"); /** * DB provider. @@ -165,7 +165,7 @@ public class SessionData { // no version specified - get the latest filterBuilder.version(ToscaPolicyFilter.LATEST_VERSION); - } else if (VERSION_PREFIX_PAT.matcher(desiredVersion).matches()) { + } else if (isVersionPrefix(desiredVersion)) { // version prefix provided - match the prefix and then pick the latest filterBuilder.versionPrefix(desiredVersion + ".").version(ToscaPolicyFilter.LATEST_VERSION); @@ -175,6 +175,17 @@ public class SessionData { } } + /** + * Determines if a version contains only a prefix. + * + * @param version version to inspect + * @return {@code true} if the version contains only a prefix, {@code false} if it is + * fully qualified + */ + public static boolean isVersionPrefix(String version) { + return VERSION_PREFIX_PAT.matcher(version).matches(); + } + /** * Adds an update and state-change to the sets, replacing any previous entries for the * given PDP. 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 7590c645..90ea1658 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 @@ -316,7 +316,9 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { subgrp.getPolicies().add(new ToscaPolicyIdentifier(POLICY2_NAME, POLICY1_VERSION)); subgrp.getSupportedPolicyTypes().add(new ToscaPolicyTypeIdentifier("typeX", "9.8.7")); - when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("daoPolicyList.json")) + when(dao.getFilteredPolicyList(any())) + .thenReturn(loadPolicies("createGroupNewPolicy.json")) + .thenReturn(loadPolicies("daoPolicyList.json")) .thenReturn(loadPolicies("createGroupNewPolicy.json")); prov.createOrUpdateGroups(groups); @@ -430,7 +432,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { @Test public void testAddSubGroup_ValidationPolicyNotFound() throws Exception { - PdpGroups groups = loadPdpGroups("createGroupsNewSub.json"); + PdpGroups groups = loadPdpGroups("createGroupsNewSubNotFound.json"); PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); @@ -441,7 +443,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { @Test public void testAddSubGroup_ValidationPolicyDaoEx() throws Exception { - PdpGroups groups = loadPdpGroups("createGroupsNewSub.json"); + PdpGroups groups = loadPdpGroups("createGroupsNewSubNotFound.json"); PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); @@ -451,6 +453,45 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isSameAs(exc); } + @Test + public void testAddSubGroup_ValidateVersionPrefixMatch() throws Exception { + PdpGroups groups = loadPdpGroups("createGroups.json"); + PdpGroup newgrp = groups.getGroups().get(0); + PdpGroup dbgroup = new PdpGroup(newgrp); + when(dao.getPdpGroups(dbgroup.getName())).thenReturn(Arrays.asList(dbgroup)); + + when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("createGroupNewPolicy.json")) + .thenReturn(loadPolicies("daoPolicyList.json")) + .thenReturn(loadPolicies("createGroupNewPolicy.json")); + + PdpGroups reqgroups = loadPdpGroups("createGroupsVersPrefix.json"); + + prov.createOrUpdateGroups(reqgroups); + + Collections.sort(newgrp.getPdpSubgroups().get(0).getPolicies()); + Collections.sort(dbgroup.getPdpSubgroups().get(0).getPolicies()); + + assertEquals(newgrp.toString(), dbgroup.toString()); + } + + @Test + public void testAddSubGroup_ValidateVersionPrefixMismatch() throws Exception { + PdpGroups groups = loadPdpGroups("createGroups.json"); + PdpGroup newgrp = groups.getGroups().get(0); + PdpGroup dbgroup = new PdpGroup(newgrp); + when(dao.getPdpGroups(dbgroup.getName())).thenReturn(Arrays.asList(dbgroup)); + + when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("daoPolicyList.json")); + + + PdpGroups reqgroups = loadPdpGroups("createGroupsVersPrefixMismatch.json"); + + assertThatThrownBy(() -> prov.createOrUpdateGroups(reqgroups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("different version already deployed"); + + assertNoGroupAction(); + } + @Test public void testUpdateSubGroup_Invalid() throws Exception { PdpGroups groups = loadPdpGroups("createGroups.json"); @@ -507,7 +548,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpSubGroup subgrp = newgrp.getPdpSubgroups().get(0); subgrp.getPolicies().add(new ToscaPolicyIdentifier(POLICY2_NAME, POLICY1_VERSION)); - when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("daoPolicyList.json")) + when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("createGroupNewPolicy.json")) + .thenReturn(loadPolicies("daoPolicyList.json")) .thenReturn(loadPolicies("createGroupNewPolicy.json")); prov.createOrUpdateGroups(groups); 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 180c0320..d7d9b677 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 @@ -188,6 +188,20 @@ public class TestSessionData extends ProviderSuper { assertThatThrownBy(() -> session.getPolicy(ident)).isSameAs(ex); } + @Test + public void testIsVersionPrefix() { + assertTrue(SessionData.isVersionPrefix("1")); + assertTrue(SessionData.isVersionPrefix("12")); + assertTrue(SessionData.isVersionPrefix("1.2")); + assertTrue(SessionData.isVersionPrefix("1.23")); + + assertFalse(SessionData.isVersionPrefix("1.")); + assertFalse(SessionData.isVersionPrefix("1.2.")); + assertFalse(SessionData.isVersionPrefix("1.2.3")); + assertFalse(SessionData.isVersionPrefix("1.2.3.")); + assertFalse(SessionData.isVersionPrefix("1.2.3.4")); + } + @Test public void testAddRequests_testGetPdpStateChanges_testGetPdpUpdates() { // pre-load with a update and state-change for other PDPs diff --git a/main/src/test/resources/simpleDeploy/createGroupsNewSubNotFound.json b/main/src/test/resources/simpleDeploy/createGroupsNewSubNotFound.json new file mode 100644 index 00000000..f4717008 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/createGroupsNewSubNotFound.json @@ -0,0 +1,56 @@ +{ + "groups": [ + { + "name": "groupA", + "version": "200.2.3", + "description": "my description", + "pdpGroupState": "ACTIVE", + "properties": { + "hello": "world" + }, + "pdpSubgroups": [ + { + "pdpType": "pdpTypeA", + "desiredInstanceCount": 1, + "properties": { + "abc": "def" + }, + "supportedPolicyTypes": [ + { + "name": "typeA", + "version": "100.2.3" + } + ], + "pdpInstances": [ + { + "instanceId": "pdpA" + } + ], + "policies": [ + { + "name": "policy-unknown", + "version": "9.9.9" + } + ] + }, + { + "pdpType": "pdpTypeB", + "desiredInstanceCount": 1, + "currentInstanceCount": 22, + "supportedPolicyTypes": [ + { + "name": "typeA", + "version": "100.2.3" + } + ], + "pdpInstances": [ + { + "instanceId": "pdpB" + } + ], + "policies": [] + } + ] + } + ] +} diff --git a/main/src/test/resources/simpleDeploy/createGroupsVersPrefix.json b/main/src/test/resources/simpleDeploy/createGroupsVersPrefix.json new file mode 100644 index 00000000..e53d927d --- /dev/null +++ b/main/src/test/resources/simpleDeploy/createGroupsVersPrefix.json @@ -0,0 +1,39 @@ +{ + "groups": [ + { + "name": "groupA", + "version": "200.2.3", + "description": "my description", + "pdpGroupState": "ACTIVE", + "properties": { + "hello": "world" + }, + "pdpSubgroups": [ + { + "pdpType": "pdpTypeA", + "desiredInstanceCount": 1, + "properties": { + "abc": "def" + }, + "supportedPolicyTypes": [ + { + "name": "typeA", + "version": "100.2.3" + } + ], + "pdpInstances": [ + { + "instanceId": "pdpA" + } + ], + "policies": [ + { + "name": "policyA", + "version": "1" + } + ] + } + ] + } + ] +} diff --git a/main/src/test/resources/simpleDeploy/createGroupsVersPrefixMismatch.json b/main/src/test/resources/simpleDeploy/createGroupsVersPrefixMismatch.json new file mode 100644 index 00000000..2bf06419 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/createGroupsVersPrefixMismatch.json @@ -0,0 +1,39 @@ +{ + "groups": [ + { + "name": "groupA", + "version": "200.2.3", + "description": "my description", + "pdpGroupState": "ACTIVE", + "properties": { + "hello": "world" + }, + "pdpSubgroups": [ + { + "pdpType": "pdpTypeA", + "desiredInstanceCount": 1, + "properties": { + "abc": "def" + }, + "supportedPolicyTypes": [ + { + "name": "typeA", + "version": "100.2.3" + } + ], + "pdpInstances": [ + { + "instanceId": "pdpA" + } + ], + "policies": [ + { + "name": "policyA", + "version": "9" + } + ] + } + ] + } + ] +} -- cgit 1.2.3-korg