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>
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 386add0..9dc2fc4 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 @@
 
 	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 @@
 
 		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 d0db725..f594725 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 @@
 
 	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 @@
 			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 6a93c53..a084b88 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 @@
 		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;
 				}