Bug 532194 - Use BundlePermissions for the EvaluationCacheKey

Also check for null Conditions from a row and update testcase to handle
when multiple conditions are used in a single row. If there is a
mix of mutable and immutable conditions then the satisfied conditions
are nulled out to avoid calling them again on future permission
checkes.  The method checkMutable needs to handle the null values.

Change-Id: I9adf9cf54bfe571d53049d48ffb44e76bc78f121
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>

3 files changed, 24 insertions, 5 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java
index 386add007..9dc2fc42f 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java
@@ -57,6 +57,7 @@ public class SecurityAdminUnitTests extends AbstractBundleTests {
 
 	private static final ConditionInfo[] ALLLOCATION_CONDS = new ConditionInfo[] {new ConditionInfo("org.osgi.service.condpermadmin.BundleLocationCondition", new String[] {"*"})}; //$NON-NLS-1$ //$NON-NLS-2$
 	private static final ConditionInfo MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"MUT_SAT", "true", "false", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
+	private static final ConditionInfo NOT_MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"NOT_MUT_SAT", "false", "false", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
 	private static final ConditionInfo POST_MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"POST_MUT_SAT", "true", "true", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
 	private static final ConditionInfo POST_MUT_UNSAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"POST_MUT_UNSAT", "true", "true", "false"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$
 
@@ -687,6 +688,24 @@ public class SecurityAdminUnitTests extends AbstractBundleTests {
 
 		tc1sat.setSatisfied(false);
 		testSMPermission(pds, new FilePermission("test", "read"), true); //$NON-NLS-1$ //$NON-NLS-2$
+
+		tc1sat.setSatisfied(true);
+		update = cpa.newConditionalPermissionUpdate();
+		rows = update.getConditionalPermissionInfos();
+		rows.clear();
+		rows.add(cpa.newConditionalPermissionInfo(null, new ConditionInfo[] {NOT_MUT_SAT, MUT_SAT}, READONLY_INFOS, ConditionalPermissionInfo.DENY));
+		rows.add(cpa.newConditionalPermissionInfo(null, ALLLOCATION_CONDS, READONLY_INFOS, ConditionalPermissionInfo.ALLOW));
+		assertTrue("failed to commit", update.commit()); //$NON-NLS-1$);
+
+		testSMPermission(pds, new FilePermission("test", "read"), false); //$NON-NLS-1$ //$NON-NLS-2$
+		// test again to make sure we get the same result
+		testSMPermission(pds, new FilePermission("test", "read"), false); //$NON-NLS-1$ //$NON-NLS-2$
+		// test with different file name
+		testSMPermission(pds, new FilePermission("test2", "read"), false); //$NON-NLS-1$ //$NON-NLS-2$
+
+		TestCondition tc2sat = TestCondition.getTestCondition("NOT_MUT_SAT_" + test1.getBundleId()); //$NON-NLS-1$
+		assertNotNull("tc2sat", tc2sat); //$NON-NLS-1$
+
 	}
 
 	public void testAccessControlContext01() {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java
index d0db72521..f59472545 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java
@@ -17,11 +17,11 @@ class EvaluationCacheKey {
 
 	private final Permission permission;
 
-	private final Long bundleId;
+	private final BundlePermissions bundlePermissions;
 
 	EvaluationCacheKey(BundlePermissions bundlePermissions, Permission permission) {
 		this.permission = permission;
-		this.bundleId = bundlePermissions.getBundle().getBundleId();
+		this.bundlePermissions = bundlePermissions;
 	}
 
 	@Override
@@ -33,11 +33,11 @@ class EvaluationCacheKey {
 			return false;
 		}
 		EvaluationCacheKey that = (EvaluationCacheKey) o;
-		return Objects.equals(bundleId, that.bundleId) && Objects.equals(permission, that.permission);
+		return bundlePermissions == that.bundlePermissions && Objects.equals(permission, that.permission);
 	}
 
 	@Override
 	public int hashCode() {
-		return Objects.hash(bundleId, permission);
+		return Objects.hash(bundlePermissions, permission);
 	}
 }
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java
index 6a93c53d2..a084b889b 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java
@@ -111,7 +111,7 @@ public class SecurityTable extends PermissionCollection {
 		Condition[] conditions = row.getConditions(bundlePermissions);
 		if (conditions != null) {
 			for (Condition condition : conditions) {
-				if (condition.isMutable()) {
+				if (condition != null && condition.isMutable()) {
 					evaluationCache.put(evaluationCacheKey, MUTABLE);
 					return true;
 				}