diff options
author | liamfallon <liam.fallon@est.tech> | 2019-05-22 02:38:32 +0000 |
---|---|---|
committer | liamfallon <liam.fallon@est.tech> | 2019-05-22 02:38:32 +0000 |
commit | 3a228ca91c313990741ef6a537d9f9615a0e0215 (patch) | |
tree | 77049c4ea0d2892730656bceb259d7110f8282c7 | |
parent | d5ed712cf50bcf270fed8cd597d78ff4ff9370a0 (diff) |
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>
8 files changed, 32 insertions, 32 deletions
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 72b8b5844..f38689a04 100644 --- 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 7e7a40998..13541b84e 100644 --- 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 494e2a1e2..f1d181040 100644 --- 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 9b494d1ab..bd28c3238 100644 --- 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 6fac58706..456f88d9b 100644 --- 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 4f4c1c3fb..07f4ed72d 100644 --- 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 9487ed8aa..be5fa5dc3 100644 --- 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 17b912826..636063641 100644 --- 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(); |