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 <jrh3@att.com>
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 e861375..bc3148e 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 @@
* 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 @@
* @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 @@
*
* @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 @@
*
* @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<String, String> dbname2vers = new HashMap<>();
if (dbsub != null) {
dbsub.getPolicies().forEach(ident -> dbname2vers.put(ident.getName(), ident.getVersion()));
@@ -463,7 +464,17 @@
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,11 +485,9 @@
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());
}
}
@@ -486,6 +495,31 @@
}
/**
+ * 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.
*
* @param policies PDP policies
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 ee83fb7..437d7a1 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 @@
* 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 @@
// 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);
@@ -176,6 +176,17 @@
}
/**
+ * 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 7590c64..90ea165 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 @@
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 @@
@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 @@
@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));
@@ -452,6 +454,45 @@
}
@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");
PdpGroup newgrp = groups.getGroups().get(0);
@@ -507,7 +548,8 @@
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 180c032..d7d9b67 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
@@ -189,6 +189,20 @@
}
@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
PdpUpdate update2 = makeUpdate(PDP2);
diff --git a/main/src/test/resources/simpleDeploy/createGroupsNewSubNotFound.json b/main/src/test/resources/simpleDeploy/createGroupsNewSubNotFound.json
new file mode 100644
index 0000000..f471700
--- /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 0000000..e53d927
--- /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 0000000..2bf0641
--- /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"
+ }
+ ]
+ }
+ ]
+ }
+ ]
+}