Skip to content

Commit

Permalink
Merge pull request #5953 from UdeshAthukorala/more-policy-tests
Browse files Browse the repository at this point in the history 
Improve Policy Persistence Managers and related test cases
  • Loading branch information
@UdeshAthukorala
UdeshAthukorala committed Sep 25, 2024
2 parents 4d49706 + ec02bc0 commit 17ecd1a
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 13 deletions.
7 changes: 7 additions & 0 deletions ...java/org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions ...n/java/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ...va/org/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions .../org/wso2/carbon/identity/entitlement/persistence/HybridPolicyPersistenceManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 14 additions & 2 deletions ...va/org/wso2/carbon/identity/entitlement/persistence/JDBCPolicyPersistenceManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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()));
}
}
86 changes: 83 additions & 3 deletions ...t/java/org/wso2/carbon/identity/entitlement/persistence/PolicyPersistenceManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@
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;
import org.wso2.carbon.identity.entitlement.policy.finder.PolicyFinderModule;

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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {

Expand All @@ -169,10 +189,15 @@ public void testDeletePAPPolicy() throws Exception {
@Test(priority = 5)
public void testListPAPPolicy() throws Exception {

List<String> policyIds = new ArrayList<>();
List<PolicyDTO> 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<String> policyIds = new ArrayList<>();
policyIds.add(samplePAPPolicy1.getPolicyId());
policyIds.add(samplePAPPolicy2.getPolicyId());
List<PolicyDTO> papPoliciesFromStorage = policyPersistenceManager.getPAPPolicies(policyIds);
Expand Down Expand Up @@ -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 {

Expand All @@ -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);
Expand All @@ -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 {

Expand Down Expand Up @@ -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<String, Set<AttributeDTO>> 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) {

Expand Down
4 changes: 3 additions & 1 deletion ...rg/wso2/carbon/identity/entitlement/persistence/RegistryPolicyPersistenceManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -56,5 +56,7 @@ public void testIsPolicyExistsInPap() throws Exception {
assertTrue(((RegistryPolicyPersistenceManager) policyPersistenceManager).
isPolicyExistsInPap(samplePAPPolicy1.getPolicyId()));
policyPersistenceManager.removePolicy(samplePAPPolicy1.getPolicyId());

assertFalse(((RegistryPolicyPersistenceManager) policyPersistenceManager).isPolicyExistsInPap(null));
}
}

0 comments on commit 17ecd1a

Please sign in to comment.