diff --git a/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManager.java b/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManager.java index 8d1a4cc632cf..dcb8b6d0de7e 100644 --- a/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManager.java +++ b/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManager.java @@ -18,6 +18,7 @@ package org.wso2.carbon.identity.entitlement.persistence; +import org.apache.commons.lang.StringUtils; import org.wso2.carbon.identity.entitlement.EntitlementException; import org.wso2.carbon.identity.entitlement.EntitlementUtil; import org.wso2.carbon.identity.entitlement.dto.AttributeDTO; @@ -310,6 +311,9 @@ public void removePolicy(String policyId) throws EntitlementException { @Override public void addPolicy(PolicyStoreDTO policy) throws EntitlementException { + if (policy == null || StringUtils.isBlank(policy.getPolicyId())) { + throw new EntitlementException("Policy and policy id can not be null"); + } if (jdbcPolicyPersistenceManager.isPolicyExistsInPap(policy.getPolicyId())) { jdbcPolicyPersistenceManager.addPolicy(policy); } else { @@ -326,6 +330,9 @@ public void addPolicy(PolicyStoreDTO policy) throws EntitlementException { @Override public void updatePolicy(PolicyStoreDTO policy) throws EntitlementException { + if (policy == null || StringUtils.isBlank(policy.getPolicyId())) { + throw new EntitlementException("Policy and policy id can not be null"); + } if (jdbcPolicyPersistenceManager.isPolicyExistsInPap(policy.getPolicyId())) { jdbcPolicyPersistenceManager.updatePolicy(policy); } else { diff --git a/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManager.java b/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManager.java index 7a20d0c69f4e..a5eb8344050d 100644 --- a/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManager.java +++ b/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManager.java @@ -481,14 +481,14 @@ public void addPolicy(PolicyStoreDTO policy) throws EntitlementException { @Override public void updatePolicy(PolicyStoreDTO policy) throws EntitlementException { - if (LOG.isDebugEnabled()) { - LOG.debug(String.format("Updating policy %s", policy.getPolicyId())); - } int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); - if (policy == null || StringUtils.isBlank(policy.getPolicyId())) { throw new EntitlementException("Policy and policy id can not be null"); } + + if (LOG.isDebugEnabled()) { + LOG.debug(String.format("Updating policy %s", policy.getPolicyId())); + } if (policy.isSetActive() != policy.isSetOrder()) { if (StringUtils.isBlank(policy.getVersion())) { // Get published version diff --git a/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManager.java b/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManager.java index 94938b1a1106..0c58374e15d0 100644 --- a/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManager.java +++ b/components/entitlement/org.wso2.carbon.identity.entitlement/src/main/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManager.java @@ -552,7 +552,7 @@ public void addPolicy(PolicyStoreDTO policy) throws EntitlementException { @Override public void updatePolicy(PolicyStoreDTO policy) throws EntitlementException { - if (LOG.isDebugEnabled()) { + if (LOG.isDebugEnabled() && policy != null) { LOG.debug(String.format("Updating policy %s", policy.getPolicyId())); } addPolicy(policy); diff --git a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManagerTest.java b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManagerTest.java index 39e031c0fdef..60a8be880e0b 100644 --- a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManagerTest.java +++ b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManagerTest.java @@ -216,7 +216,7 @@ public void testDeletePDPPolicyInDatabase() throws Exception { } @Test(priority = 22) - public void testDeletePDPPolicyInRegistry() throws Exception { + public void testDeletePDPPolicy() throws Exception { registryPolicyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); registryPolicyPersistenceManager.addPolicy(samplePDPPolicy1); @@ -323,7 +323,6 @@ public void testListPDPPolicyInDatabase() throws Exception { String[] orderedPolicyIdentifiersFromDb = jdbcPolicyPersistenceManager.getOrderedPolicyIdentifiers(); assertEquals(orderedPolicyIdentifiersFromDb.length, 3); - // Verify the number of active policies. String[] activePolicies = policyPersistenceManager.getActivePolicies(); assertEquals(activePolicies.length, 2); diff --git a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManagerTest.java b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManagerTest.java index 308874874df6..e59306e52415 100644 --- a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManagerTest.java +++ b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManagerTest.java @@ -26,7 +26,8 @@ import org.wso2.carbon.identity.common.testng.WithRegistry; import org.wso2.carbon.identity.entitlement.internal.EntitlementConfigHolder; -import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; /** @@ -47,9 +48,20 @@ public void setUp() throws Exception { @Test public void testIsPolicyExistsInPap() throws Exception { + assertFalse(((JDBCPolicyPersistenceManager) policyPersistenceManager).isPolicyExistsInPap(null)); + assertFalse(((JDBCPolicyPersistenceManager) policyPersistenceManager).isPolicyExistsInPap(" ")); + assertFalse(((JDBCPolicyPersistenceManager) policyPersistenceManager).isPolicyExistsInPap( + samplePAPPolicy1.getPolicyId())); + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); assertTrue(((JDBCPolicyPersistenceManager) policyPersistenceManager). isPolicyExistsInPap(samplePAPPolicy1.getPolicyId())); - policyPersistenceManager.removePolicy(samplePAPPolicy1.getPolicyId()); + } + + @Test(priority = 3) + public void testAddPAPPolicyNotFromPAP() throws Exception { + + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, false); + assertNull(policyPersistenceManager.getPAPPolicy(samplePAPPolicy1.getPolicyId())); } } diff --git a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/PolicyPersistenceManagerTest.java b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/PolicyPersistenceManagerTest.java index 4c5b0c2e6854..e1750c91998c 100644 --- a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/PolicyPersistenceManagerTest.java +++ b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/PolicyPersistenceManagerTest.java @@ -23,6 +23,7 @@ import org.testng.annotations.Test; import org.wso2.carbon.identity.entitlement.EntitlementException; import org.wso2.carbon.identity.entitlement.PDPConstants; +import org.wso2.carbon.identity.entitlement.dto.AttributeDTO; import org.wso2.carbon.identity.entitlement.dto.PolicyDTO; import org.wso2.carbon.identity.entitlement.dto.PolicyStoreDTO; import org.wso2.carbon.identity.entitlement.internal.EntitlementConfigHolder; @@ -30,7 +31,9 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Properties; +import java.util.Set; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; @@ -74,7 +77,7 @@ public abstract class PolicyPersistenceManagerTest { public void setUpClass() { Properties engineProperties = new Properties(); - engineProperties.put(PDPConstants.MAX_NO_OF_POLICY_VERSIONS, "0"); + engineProperties.put(PDPConstants.MAX_NO_OF_POLICY_VERSIONS, "4"); EntitlementConfigHolder.getInstance().setEngineProperties(engineProperties); samplePAPPolicy1 = new PolicyDTO(SAMPLE_POLICY_ID_1); @@ -148,6 +151,23 @@ public void testAddInvalidPolicy() { addOrUpdatePolicy(papPolicyWithEmptyPolicyId, true)); } + @Test(priority = 3) + public void testAddPolicyMoreThanMaxVersions() throws Exception { + + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + + String[] policyVersions = policyPersistenceManager.getVersions(samplePAPPolicy1.getPolicyId()); + assertEquals(policyVersions.length, 5); + + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + String[] policyVersionsAfterMax = policyPersistenceManager.getVersions(samplePAPPolicy1.getPolicyId()); + assertEquals(policyVersionsAfterMax.length, 5); + } + @Test(priority = 3) public void testGetPolicyForInvalidScenarios() throws EntitlementException { @@ -169,10 +189,15 @@ public void testDeletePAPPolicy() throws Exception { @Test(priority = 5) public void testListPAPPolicy() throws Exception { + List policyIds = new ArrayList<>(); + List papPolicies = policyPersistenceManager.getPAPPolicies(policyIds); + assertEquals(papPolicies.size(), 0); + papPolicies = policyPersistenceManager.getPAPPolicies(null); + assertEquals(papPolicies.size(), 0); + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy2, true); - List policyIds = new ArrayList<>(); policyIds.add(samplePAPPolicy1.getPolicyId()); policyIds.add(samplePAPPolicy2.getPolicyId()); List papPoliciesFromStorage = policyPersistenceManager.getPAPPolicies(policyIds); @@ -206,6 +231,20 @@ public void testUpdatePAPPolicy() throws Exception { assertEquals(policyVersions.length, 2); } + @Test(priority = 6) + public void testGetPolicyWithoutVersion() throws Exception { + + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + policyPersistenceManager.addOrUpdatePolicy(sampleUpdatedPAPPolicy1, true); + + // Verify the policy version without defining the version. + PolicyDTO latestPolicy = policyPersistenceManager.getPolicy(samplePAPPolicy1.getPolicyId(), " "); + assertEquals(latestPolicy.getPolicy(), sampleUpdatedPAPPolicy1.getPolicy()); + + latestPolicy = policyPersistenceManager.getPolicy(samplePAPPolicy1.getPolicyId(), null); + assertEquals(latestPolicy.getPolicy(), sampleUpdatedPAPPolicy1.getPolicy()); + } + @Test(priority = 7) public void testAddPDPPolicy() throws Exception { @@ -219,15 +258,28 @@ public void testAddPDPPolicy() throws Exception { assertEquals(publishedPolicyFromStorage.getPolicyId(), samplePDPPolicy1.getPolicyId()); } + @Test(priority = 7) + public void testIsPolicyExists() throws Exception { + + assertFalse(policyPersistenceManager.isPolicyExist(null)); + assertFalse(policyPersistenceManager.isPolicyExist("")); + assertFalse(policyPersistenceManager.isPolicyExist("sample_policy1")); + + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + policyPersistenceManager.addPolicy(samplePDPPolicy1); + assertTrue(policyPersistenceManager.isPolicyExist(samplePDPPolicy1.getPolicyId())); + } + @Test(priority = 7) public void testAddInvalidPDPPolicy() throws Exception { assertThrows(EntitlementException.class, () -> policyPersistenceManager.addPolicy(pdpPolicyWithEmptyId)); assertThrows(EntitlementException.class, () -> policyPersistenceManager.addPolicy(pdpPolicyWithEmptyVersion)); + assertThrows(EntitlementException.class, () -> policyPersistenceManager.addPolicy(null)); } @Test(priority = 8) - public void testDeletePDPPolicyInRegistry() throws Exception { + public void testDeletePDPPolicy() throws Exception { policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); policyPersistenceManager.addPolicy(samplePDPPolicy1); @@ -236,6 +288,13 @@ public void testDeletePDPPolicyInRegistry() throws Exception { assertFalse(policyPersistenceManager.isPolicyExist(samplePDPPolicy1.getPolicyId())); } + @Test(priority = 8) + public void testDeletePDPPolicyUsingBlankID() throws Exception { + + assertFalse(policyPersistenceManager.deletePolicy(null)); + assertFalse(policyPersistenceManager.deletePolicy("")); + } + @Test(priority = 9) public void testGetReferencedPolicy() throws Exception { @@ -316,11 +375,32 @@ public void testUpdatePDPPolicy() throws Exception { @Test(priority = 12) public void testUpdateInvalidPDPPolicy() throws Exception { + assertThrows(EntitlementException.class, () -> policyPersistenceManager.updatePolicy(null)); assertThrows(EntitlementException.class, () -> policyPersistenceManager.updatePolicy(pdpPolicyWithEmptyId)); assertThrows(EntitlementException.class, () -> policyPersistenceManager. updatePolicy(pdpPolicyWithEmptyVersion)); } + @Test(priority = 13) + public void testGetSearchAttributes() throws Exception { + + Map> attributes = policyPersistenceManager.getSearchAttributes("identifier", null); + assertEquals(attributes.size(), 0); + + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy1, true); + policyPersistenceManager.addPolicy(samplePDPPolicy1); + attributes = policyPersistenceManager.getSearchAttributes(null, null); + assertEquals(attributes.size(), 1); + assertEquals(attributes.get(samplePDPPolicy1.getPolicyId()).size(), 4); + + policyPersistenceManager.addOrUpdatePolicy(samplePAPPolicy3, true); + policyPersistenceManager.addPolicy(samplePDPPolicy3); + attributes = policyPersistenceManager.getSearchAttributes(null, null); + assertEquals(attributes.size(), 2); + assertEquals(attributes.get(samplePDPPolicy1.getPolicyId()).size(), 4); + assertEquals(attributes.get(samplePDPPolicy3.getPolicyId()).size(), 4); + } + private PolicyStoreDTO getPDPPolicy(String id, String policy, String version, boolean active, boolean setActive, int order, boolean setOrder) { diff --git a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManagerTest.java b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManagerTest.java index dc5a41a6ab7d..6662a4864bc1 100644 --- a/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManagerTest.java +++ b/components/entitlement/org.wso2.carbon.identity.entitlement/src/test/java/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManagerTest.java @@ -29,7 +29,7 @@ import java.util.Properties; -import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; /** @@ -56,5 +56,7 @@ public void testIsPolicyExistsInPap() throws Exception { assertTrue(((RegistryPolicyPersistenceManager) policyPersistenceManager). isPolicyExistsInPap(samplePAPPolicy1.getPolicyId())); policyPersistenceManager.removePolicy(samplePAPPolicy1.getPolicyId()); + + assertFalse(((RegistryPolicyPersistenceManager) policyPersistenceManager).isPolicyExistsInPap(null)); } }