Fix name/version regexp in model keys

The regular expressions for checking names and versions
in policy key names and versions were not expressive enough
to restrict all error names and versions.

Issue-ID: POLICY-1777
Change-Id: I037eca051f6c7a9f1e7182150d40d8b8d906a75c
Signed-off-by: liamfallon <liam.fallon@est.tech>

diff --git a/models-base/src/main/java/org/onap/policy/models/base/PfKey.java b/models-base/src/main/java/org/onap/policy/models/base/PfKey.java
index 72b8b58..f38689a 100644 (file)
--- a/models-base/src/main/java/org/onap/policy/models/base/PfKey.java
+++ b/models-base/src/main/java/org/onap/policy/models/base/PfKey.java
@@ -30,13 +30,13 @@ public abstract class PfKey extends PfConcept {
     private static final long serialVersionUID = 6281159885962014041L;
 
     /** Regular expression to specify the structure of key names. */
-    public static final String NAME_REGEXP = "[A-Za-z0-9\\-_\\.]+";
+    public static final String NAME_REGEXP = "^[A-Za-z0-9\\-_\\.]+$";
 
     /** Regular expression to specify the structure of key versions. */
-    public static final String VERSION_REGEXP = "[0-9.]+";
+    public static final String VERSION_REGEXP = "^(\\d+.){2}\\d+$";
 
     /** Regular expression to specify the structure of key IDs. */
-    public static final String KEY_ID_REGEXP = "[A-Za-z0-9\\-_\\.]+:[0-9].[0-9].[0-9]";
+    public static final String KEY_ID_REGEXP = "^[A-Za-z0-9\\-_\\.]+:(\\d+.){2}\\d+$";
 
     /** Specifies the value for names in NULL keys. */
     public static final String NULL_KEY_NAME = "NULL";

diff --git a/models-base/src/test/java/org/onap/policy/models/base/PfKeyTest.java b/models-base/src/test/java/org/onap/policy/models/base/PfKeyTest.java
index 7e7a409..13541b8 100644 (file)
--- a/models-base/src/test/java/org/onap/policy/models/base/PfKeyTest.java
+++ b/models-base/src/test/java/org/onap/policy/models/base/PfKeyTest.java
@@ -44,7 +44,7 @@ public class PfKeyTest {
         } catch (IllegalArgumentException e) {
             assertEquals(
                     "parameter \"id\": value \"some bad key id\", "
-                            + "does not match regular expression \"[A-Za-z0-9\\-_\\.]+:[0-9].[0-9].[0-9]\"",
+                            + "does not match regular expression \"" + PfKey.KEY_ID_REGEXP + "\"",
                     e.getMessage());
         }
 
@@ -77,13 +77,13 @@ public class PfKeyTest {
         someKey4.setVersion("0.1.2");
 
         PfConceptKey someKey4a = new PfConceptKey(someKey1);
-        someKey4a.setVersion("0");
+        someKey4a.setVersion("0.0.0");
 
         PfConceptKey someKey5 = new PfConceptKey(someKey1);
         someKey5.setVersion("1.2.2");
 
         PfConceptKey someKey6 = new PfConceptKey(someKey1);
-        someKey6.setVersion("3");
+        someKey6.setVersion("3.0.0");
 
         assertEquals("name:0.1.2", someKey4.getId());
 
@@ -217,7 +217,7 @@ public class PfKeyTest {
             nameField.setAccessible(false);
             assertEquals(
                     "name invalid-parameter name with value Key Name "
-                            + "does not match regular expression [A-Za-z0-9\\-_\\.]+",
+                            + "does not match regular expression " + PfKey.NAME_REGEXP,
                     validationResult.getMessageList().get(0).getMessage());
         } catch (Exception validationException) {
             fail("test should not throw an exception");
@@ -233,7 +233,7 @@ public class PfKeyTest {
             versionField.setAccessible(false);
             assertEquals(
                     "version invalid-parameter version with value Key Version "
-                            + "does not match regular expression [0-9.]+",
+                            + "does not match regular expression " + PfKey.VERSION_REGEXP,
                     validationResult.getMessageList().get(0).getMessage());
         } catch (Exception validationException) {
             fail("test should not throw an exception");
@@ -291,9 +291,9 @@ public class PfKeyTest {
         key1a.setVersion("1.2.3");
         assertFalse(key1.isNewerThan(key1a));
 
-        key1.setVersion("1");
+        key1.setVersion("1.0.0");
         assertFalse(key1.isNewerThan(key1a));
-        key1a.setVersion("1");
+        key1a.setVersion("1.0.0");
         assertFalse(key1.isNewerThan(key1a));
 
         PfReferenceKey refKey = new PfReferenceKey();
@@ -318,12 +318,12 @@ public class PfKeyTest {
 
     @Test
     public void testmajorMinorPatch() {
-        PfConceptKey key = new PfConceptKey("Key", "1");
+        PfConceptKey key = new PfConceptKey("Key", "1.0.0");
         assertEquals(1, key.getMajorVersion());
         assertEquals(0, key.getMinorVersion());
         assertEquals(0, key.getPatchVersion());
 
-        key = new PfConceptKey("Key", "1.2");
+        key = new PfConceptKey("Key", "1.2.0");
         assertEquals(1, key.getMajorVersion());
         assertEquals(2, key.getMinorVersion());
         assertEquals(0, key.getPatchVersion());

diff --git a/models-base/src/test/java/org/onap/policy/models/base/PfReferenceKeyTest.java b/models-base/src/test/java/org/onap/policy/models/base/PfReferenceKeyTest.java
index 494e2a1..f1d1810 100644 (file)
--- a/models-base/src/test/java/org/onap/policy/models/base/PfReferenceKeyTest.java
+++ b/models-base/src/test/java/org/onap/policy/models/base/PfReferenceKeyTest.java
@@ -162,7 +162,7 @@ public class PfReferenceKeyTest {
             parentNameField.setAccessible(false);
             assertEquals(
                     "parentKeyName invalid-parameter parentKeyName with value Parent Name "
-                            + "does not match regular expression [A-Za-z0-9\\-_\\.]+",
+                            + "does not match regular expression " + PfKey.NAME_REGEXP,
                     validationResult.getMessageList().get(0).getMessage());
         } catch (Exception validationException) {
             fail("test should not throw an exception");
@@ -178,7 +178,7 @@ public class PfReferenceKeyTest {
             parentVersionField.setAccessible(false);
             assertEquals(
                     "parentKeyVersion invalid-parameter parentKeyVersion with value Parent Version "
-                            + "does not match regular expression [0-9.]+",
+                            + "does not match regular expression " + PfKey.VERSION_REGEXP,
                     validationResult.getMessageList().get(0).getMessage());
         } catch (Exception validationException) {
             fail("test should not throw an exception");

diff --git a/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java b/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java
index 9b494d1..bd28c32 100644 (file)
--- a/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java
+++ b/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProvider.java
@@ -232,7 +232,7 @@ public interface PolicyModelsProvider extends AutoCloseable {
      * Delete legacy operational policy.
      *
      * @param policyId ID of the policy.
-     * @param policyVersion version of the policy, set to null to get the latest policy
+     * @param policyVersion version of the policy
      * @return the deleted policy
      * @throws PfModelException on errors deleting policies
      */
@@ -274,7 +274,7 @@ public interface PolicyModelsProvider extends AutoCloseable {
      * Delete legacy guard policy.
      *
      * @param policyId ID of the policy.
-     * @param policyVersion version of the policy, set to null to get the latest policy
+     * @param policyVersion version of the policy
      * @return the deleted policy
      * @throws PfModelException on errors deleting policies
      */

diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java
index 6fac587..456f88d 100644 (file)
--- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java
+++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java
@@ -440,8 +440,8 @@ public class DatabasePolicyModelsProviderTest {
             }).hasMessage("no policy found for policy: policy_id:null");
 
             assertThatThrownBy(() -> {
-                databaseProvider.getOperationalPolicy("policy_id", "10.9.8");
-            }).hasMessage("no policy found for policy: policy_id:10.9.8");
+                databaseProvider.getOperationalPolicy("policy_id", "10");
+            }).hasMessage("no policy found for policy: policy_id:10");
 
             assertThatThrownBy(() -> {
                 databaseProvider.createOperationalPolicy(new LegacyOperationalPolicy());
@@ -452,16 +452,16 @@ public class DatabasePolicyModelsProviderTest {
             }).hasMessage("name is marked @NonNull but is null");
 
             assertThatThrownBy(() -> {
-                databaseProvider.deleteOperationalPolicy("policy_id", "55.44.33");
-            }).hasMessage("no policy found for policy: policy_id:55.44.33");
+                databaseProvider.deleteOperationalPolicy("policy_id", "55");
+            }).hasMessage("no policy found for policy: policy_id:55");
 
             assertThatThrownBy(() -> {
                 databaseProvider.getGuardPolicy("policy_id", null);
             }).hasMessage("no policy found for policy: policy_id:null");
 
             assertThatThrownBy(() -> {
-                databaseProvider.getGuardPolicy("policy_id", "6.7.5");
-            }).hasMessage("no policy found for policy: policy_id:6.7.5");
+                databaseProvider.getGuardPolicy("policy_id", "6");
+            }).hasMessage("no policy found for policy: policy_id:6");
 
             assertThatThrownBy(() -> {
                 databaseProvider.createGuardPolicy(new LegacyGuardPolicyInput());
@@ -472,8 +472,8 @@ public class DatabasePolicyModelsProviderTest {
             }).hasMessage("policy type for guard policy \"null\" unknown");
 
             assertThatThrownBy(() -> {
-                databaseProvider.deleteGuardPolicy("policy_id", "33.22.11");
-            }).hasMessage("no policy found for policy: policy_id:33.22.11");
+                databaseProvider.deleteGuardPolicy("policy_id", "33");
+            }).hasMessage("no policy found for policy: policy_id:33");
 
             assertEquals(0, databaseProvider.getPdpGroups("name").size());
             assertEquals(0, databaseProvider.getFilteredPdpGroups(PdpGroupFilter.builder().build()).size());

diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java
index 4f4c1c3..07f4ed7 100644 (file)
--- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java
+++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DummyBadProviderImpl.java
@@ -121,8 +121,8 @@ public class DummyBadProviderImpl implements PolicyModelsProvider {
     }
 
     @Override
-    public LegacyOperationalPolicy deleteOperationalPolicy(@NonNull String policyId, final String policyVersion)
-            throws PfModelException {
+    public LegacyOperationalPolicy deleteOperationalPolicy(@NonNull String policyId,
+            @NonNull final String policyVersion) throws PfModelException {
         return null;
     }
 
@@ -145,8 +145,8 @@ public class DummyBadProviderImpl implements PolicyModelsProvider {
     }
 
     @Override
-    public Map<String, LegacyGuardPolicyOutput> deleteGuardPolicy(@NonNull String policyId, final String policyVersion)
-            throws PfModelException {
+    public Map<String, LegacyGuardPolicyOutput> deleteGuardPolicy(@NonNull String policyId,
+            @NonNull final String policyVersion) throws PfModelException {
         return null;
     }
 

diff --git a/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyGuardTest.java b/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyGuardTest.java
index 9487ed8..be5fa5d 100644 (file)
--- a/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyGuardTest.java
+++ b/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyGuardTest.java
@@ -311,8 +311,8 @@ public class LegacyProvider4LegacyGuardTest {
         }).hasMessage("policyVersion is marked @NonNull but is null");
 
         assertThatThrownBy(() -> {
-            new LegacyProvider().deleteGuardPolicy(pfDao, "IDontExist", "");
-        }).hasMessage("no policy found for policy: IDontExist:");
+            new LegacyProvider().deleteGuardPolicy(pfDao, "IDontExist", "0");
+        }).hasMessage("no policy found for policy: IDontExist:0");
 
         createPolicyTypes();
 

diff --git a/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyOperationalTest.java b/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyOperationalTest.java
index 17b9128..6360636 100644 (file)
--- a/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyOperationalTest.java
+++ b/models-tosca/src/test/java/org/onap/policy/models/tosca/legacy/provider/LegacyProvider4LegacyOperationalTest.java
@@ -250,8 +250,8 @@ public class LegacyProvider4LegacyOperationalTest {
         }).hasMessage("policyVersion is marked @NonNull but is null");
 
         assertThatThrownBy(() -> {
-            new LegacyProvider().deleteOperationalPolicy(pfDao, "IDontExist", "");
-        }).hasMessage("no policy found for policy: IDontExist:");
+            new LegacyProvider().deleteOperationalPolicy(pfDao, "IDontExist", "0");
+        }).hasMessage("no policy found for policy: IDontExist:0");
 
         createPolicyTypes();