refactoring
* made cache entries reflect the number of protected resource type
* optimised cache queries
diff --git a/pom.xml b/pom.xml
index 521b55f..2ab24de 100644
--- a/pom.xml
+++ b/pom.xml
@@ -46,6 +46,7 @@
<properties>
<site.jira.version.id>12314287</site.jira.version.id>
<site.javadoc.exclude>**.impl.**</site.javadoc.exclude>
+ <sling.java.version>8</sling.java.version>
</properties>
<build>
diff --git a/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypeExecutionCache.java b/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypeExecutionCache.java
index 4aa6472..290c8ee 100644
--- a/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypeExecutionCache.java
+++ b/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypeExecutionCache.java
@@ -18,9 +18,12 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
package org.apache.sling.engine.impl.request.permissions;
+import java.util.Collections;
+import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -37,24 +40,47 @@
)
public class ResourceTypeExecutionCache implements ResourceChangeListener, ExternalResourceChangeListener {
- private Map<String, Boolean> preEvaluatedPermissions = new CachingMap(65536);
+ private Map<String, CacheEntry> preEvaluatedPermissions = new CachingMap(65536);
private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
private final Lock readLock = rwl.readLock();
private final Lock writeLock = rwl.writeLock();
+ private static final Set<String> EMPTY = Collections.emptySet();
- public Boolean getCachedPermission(String key) {
+ Set<String> getAllowedUsers(String permission) {
readLock.lock();
try {
- return preEvaluatedPermissions.get(key);
+ CacheEntry entry = preEvaluatedPermissions.get(permission);
+ if (entry != null) {
+ return entry.allowedUsers;
+ }
+ return EMPTY;
} finally {
readLock.unlock();
}
}
- public void setCachedPermission(String key, Boolean value) {
+ Set<String> getDisallowedUsers(String permission) {
+ readLock.lock();
+ try {
+ CacheEntry entry = preEvaluatedPermissions.get(permission);
+ if (entry != null) {
+ return entry.disallowedUsers;
+ }
+ return EMPTY;
+ } finally {
+ readLock.unlock();
+ }
+ }
+
+ void updateResourceType(String permission, String user, boolean allow) {
writeLock.lock();
try {
- preEvaluatedPermissions.put(key, value);
+ CacheEntry entry = preEvaluatedPermissions.computeIfAbsent(permission, k -> new CacheEntry());
+ if (allow) {
+ entry.allowedUsers.add(user);
+ } else {
+ entry.disallowedUsers.add(user);
+ }
} finally {
writeLock.unlock();
}
@@ -70,7 +96,7 @@
}
}
- private class CachingMap extends LinkedHashMap<String, Boolean> {
+ private class CachingMap extends LinkedHashMap<String, CacheEntry> {
private final int capacity;
@@ -79,8 +105,13 @@
}
@Override
- protected boolean removeEldestEntry(Map.Entry<String, Boolean> eldest) {
+ protected boolean removeEldestEntry(Map.Entry<String, CacheEntry> eldest) {
return size() > capacity;
}
}
+
+ private class CacheEntry {
+ private Set<String> allowedUsers = new HashSet<>();
+ private Set<String> disallowedUsers = new HashSet<>();
+ }
}
diff --git a/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypePermissionEvaluator.java b/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypePermissionEvaluator.java
index 9b33783..9a66fa9 100644
--- a/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypePermissionEvaluator.java
+++ b/src/main/java/org/apache/sling/engine/impl/request/permissions/ResourceTypePermissionEvaluator.java
@@ -18,6 +18,8 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
package org.apache.sling.engine.impl.request.permissions;
+import java.util.Set;
+
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.request.SlingRequestEvent;
import org.apache.sling.api.request.SlingRequestListener;
@@ -55,32 +57,56 @@
final String userId = request.getResourceResolver().getUserID();
if (userId != null) {
String permissionPath = resourceType.startsWith("/") ? RESOURCE_TYPE_PERMISSIONS + resourceType :
- RESOURCE_TYPE_PERMISSIONS + "/" + resourceType + "/http." + request.getMethod();
- while (!RESOURCE_TYPE_PERMISSIONS.equals(permissionPath)) {
- String cacheKey = userId + ":" + permissionPath;
- Boolean cached = resourceTypeExecutionCache.getCachedPermission(cacheKey);
- if (cached != null) {
- return cached;
- }
- Resource permission = accessedResource.getResourceResolver().getResource(permissionPath);
- if (permission != null) {
- resourceTypeExecutionCache.setCachedPermission(cacheKey, true);
+ RESOURCE_TYPE_PERMISSIONS + "/" + resourceType;
+ String permissionPathWithHttpMethod = permissionPath + "/http." + request.getMethod();
+ while (permissionPath != null && !RESOURCE_TYPE_PERMISSIONS.equals(permissionPath)) {
+
+ Set<String> allowedUsers = resourceTypeExecutionCache.getAllowedUsers(permissionPathWithHttpMethod);
+ if (allowedUsers.contains(userId)) {
return true;
- } else {
- ResourceResolver elevatedResourceResolver = threadLocalResourceResolver.get();
- if (elevatedResourceResolver != null) {
- permission = elevatedResourceResolver.getResource(permissionPath);
- if (permission != null) {
- resourceTypeExecutionCache.setCachedPermission(cacheKey, false);
- return false;
- }
+ }
+ allowedUsers = resourceTypeExecutionCache.getAllowedUsers(permissionPath);
+ if (allowedUsers.contains(userId)) {
+ return true;
+ }
+
+ Set<String> disallowedUsers = resourceTypeExecutionCache.getDisallowedUsers(permissionPathWithHttpMethod);
+ if (disallowedUsers.contains(userId)) {
+ return false;
+ }
+ disallowedUsers = resourceTypeExecutionCache.getDisallowedUsers(permissionPath);
+ if (disallowedUsers.contains(userId)) {
+ return false;
+ }
+
+ Resource permission = accessedResource.getResourceResolver().getResource(permissionPathWithHttpMethod);
+ if (permission != null) {
+ resourceTypeExecutionCache.updateResourceType(permissionPathWithHttpMethod, userId, true);
+ return true;
+ }
+
+ permission = accessedResource.getResourceResolver().getResource(permissionPath);
+ if (permission != null) {
+ resourceTypeExecutionCache.updateResourceType(permissionPath, userId, true);
+ return true;
+ }
+
+ ResourceResolver elevatedResourceResolver = threadLocalResourceResolver.get();
+ if (elevatedResourceResolver != null) {
+ permission = elevatedResourceResolver.getResource(permissionPathWithHttpMethod);
+ if (permission != null) {
+ resourceTypeExecutionCache.updateResourceType(permissionPathWithHttpMethod, userId, false);
+ return false;
+ }
+ permission = elevatedResourceResolver.getResource(permissionPath);
+ if (permission != null) {
+ resourceTypeExecutionCache.updateResourceType(permissionPath, userId, false);
+ return false;
}
}
permissionPath = ResourceUtil.getParent(permissionPath);
}
- resourceTypeExecutionCache.setCachedPermission(userId + ":" + (resourceType.startsWith("/") ?
- RESOURCE_TYPE_PERMISSIONS + resourceType :
- RESOURCE_TYPE_PERMISSIONS + "/" + resourceType + "/http." + request.getMethod()), true);
+ resourceTypeExecutionCache.updateResourceType(permissionPathWithHttpMethod, userId, true);
return true;
}
}
