Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACL Cache #883

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 12 additions & 28 deletions lib/ACL/ACLManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class ACLManager {
private $rootStorageId = null;
private $rootFolderProvider;

public function __construct(RuleManager $ruleManager, IUser $user, callable $rootFolderProvider) {
public function __construct(RuleManager $ruleManager, ACLRuleCache $ruleCache, IUser $user, callable $rootFolderProvider) {
$this->ruleManager = $ruleManager;
$this->ruleCache = new CappedMemoryCache();
$this->ruleCache = $ruleCache;
$this->user = $user;
$this->rootFolderProvider = $rootFolderProvider;
}
Expand All @@ -53,40 +53,24 @@ private function getRootStorageId() {
return $this->rootStorageId;
}

private function pathsAreCached(array $paths): bool {
foreach ($paths as $path) {
if (!$this->ruleCache->hasKey($path)) {
return false;
}
}
return true;
}

/**
* @param int $folderId
* @param array $paths
* @return (Rule[])[]
*/
private function getRules(array $paths): array {
if ($this->pathsAreCached($paths)) {
$rules = array_combine($paths, array_map(function (string $path) {
return $this->ruleCache->get($path);
}, $paths));
} else {
$rules = $this->ruleManager->getRulesForFilesByPath($this->user, $this->getRootStorageId(), $paths);
foreach ($rules as $path => $rulesForPath) {
$this->ruleCache->set($path, $rulesForPath);
}
$rules = $this->ruleCache->getByPaths($this->user, $this->getRootStorageId(), $paths);

if (count($paths) > 2) {
// also cache the direct sibling since it's likely that we'll be needing those later
$directParent = $paths[1];
$siblingRules = $this->ruleManager->getRulesForFilesByParent($this->user, $this->getRootStorageId(), $directParent);
foreach ($siblingRules as $path => $rulesForPath) {
$this->ruleCache->set($path, $rulesForPath);
}
}
$uncachedPaths = array_values(array_diff($paths, array_keys($rules)));

if (count($uncachedPaths) > 0) {
$this->ruleCache->cachePath(
$this->getRootStorageId(),
$this->ruleManager->getAllRulesForFilesByPath($this->getRootStorageId(), $uncachedPaths)
);
$rules = $this->ruleCache->getByPaths($this->user, $this->getRootStorageId(), $paths);
}

ksort($rules);

return $rules;
Expand Down
6 changes: 4 additions & 2 deletions lib/ACL/ACLManagerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@
class ACLManagerFactory {
private $ruleManager;
private $rootFolderProvider;
private $ruleCache;

public function __construct(RuleManager $ruleManager, callable $rootFolderProvider) {
public function __construct(RuleManager $ruleManager, ACLRuleCache $ruleCache, callable $rootFolderProvider) {
$this->ruleManager = $ruleManager;
$this->ruleCache = $ruleCache;
$this->rootFolderProvider = $rootFolderProvider;
}

public function getACLManager(IUser $user): ACLManager {
return new ACLManager($this->ruleManager, $user, $this->rootFolderProvider);
return new ACLManager($this->ruleManager, $this->ruleCache, $user, $this->rootFolderProvider);
}
}
259 changes: 259 additions & 0 deletions lib/ACL/ACLRuleCache.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
<?php

namespace OCA\GroupFolders\ACL;

use OCA\GroupFolders\ACL\UserMapping\IUserMapping;
use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
use OCA\GroupFolders\ACL\UserMapping\UserMapping;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IUser;

class ACLRuleCache
{
const DERIVATION = 0.1;

const DEFAULT_TTL = 3600 * 24;

const INDEX_TTL = (3600 * 24) + 3600;

/**
* @var ICache
*/
private $ruleCache;

/**
* @var IUserMappingManager
*/
protected $userMappingManager;

public function __construct(ICacheFactory $cacheFactory, IUserMappingManager $userMappingManager) {
$this->userMappingManager = $userMappingManager;
$this->ruleCache = $cacheFactory->createDistributed('acl');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->ruleCache = $cacheFactory->createDistributed('acl');
$this->ruleCache = $cacheFactory->createDistributed('groupfolders-acl');

Maybe a bit safer in terms of naming collisions.

}

protected function set($key, $value, $ttl = null) : void
{
if ($ttl === null) {
$ttl = self::DEFAULT_TTL;
}

$ttl = rand($ttl, round($ttl * self::DERIVATION));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong, this currently select a ttl between 0.1 days and 1 days, while I would expect a derivation of 0.1 to mean something like 0.9 days to 1 day


$this->ruleCache->set($key, $value, $ttl);
}

protected function has($key) : bool
{
return $this->ruleCache->hasKey($key);
}

protected function get($key)
{
return $this->ruleCache->get($key);
}

protected function remove($key) : void
{
$this->ruleCache->remove($key);
}

protected function keyPath(int $storageId, string $path) : string
{
return 'path_' . $storageId . '_' . $path;
}

protected function keyFileId(int $fileId) : string
{
return 'file_' . $fileId;
}

protected function keyPathMapping(int $storageId, string $path, IUserMapping $mapping) : string
{
return $this->keyPath($storageId, $path) . '_' . $mapping->getType() . '_' . $mapping->getId();
}

public function clearByPathKey(string $pathKey) : void
{
$keys = $this->get($pathKey);
if (!isset($keys['rule_path_keys'])) {
$keys['rule_path_keys'] = [];
}
if (!isset($keys['rule_id_keys'])) {
$keys['rule_id_keys'] = [];
}

foreach (array_merge($keys['rule_path_keys'], $keys['rule_id_keys']) as $key) {
$this->remove($key);
}
}

public function clearByPath(int $storageId, string $path) : void
{
$pathKey = $this->keyPath($storageId, $path);
if (!$this->has($pathKey)) {
return;
}

$this->clearByPathKey($pathKey);
}

public function clearByFileId(int $fileId) : void
{
$fileIdKey = $this->keyFileId($fileId);
if (!$this->has($fileIdKey)) {
return;
}

$this->clearByPathKey($this->get($fileIdKey));
}

/**
* @return Rule[]|null
*/
public function getByPath(IUser $user, int $storageId, string $path) : ?array
{
return $this->getByPathKey($user, $this->keyPath($storageId, $path));
}

/**
* @param string[] $paths
* @return (Rule[]|null)[]
*/
public function getByPaths(IUser $user, int $storageId, array $paths) : array
{
$rulePaths = [];

foreach ($paths as $path) {
$cached = $this->getByPath($user, $storageId, $path);
if ($cached !== null) {
$rulePaths[$path] = $cached;
}
}

return $rulePaths;
}

/**
* @return Rule[]|null
*/
public function getByFileId(IUser $user, int $fileId) : ?array
{
$fileIdKey = $this->keyFileId($fileId);
if (!$this->has($fileIdKey)) {
return null;
}

$cached = $this->getByPathKey($user, $this->get($fileIdKey));
if ($cached === null) {
return null;
}

$rules = [];

foreach ($cached as $rule) {
if ($rule->getFileId() == $fileIdKey) {
$rules[] = $rule;
}
}

return $rules;
}

/**
* @param (Rule[])[] $paths
*/

public function cachePath(int $storageId, array $paths) : void
{
foreach ($paths as $path => $rules) {
$this->cacheRules($storageId, $path, $rules);
}
}

/**
* @param Rule[] $rules
*/
public function cacheRules(int $storageId, string $path, array $rules) : void
{
$rulePathKeys = [];
$fileIdKeys = [];

foreach ($rules as $rule) {
$rulePathKeys[] = $rulePathKey = $this->keyPathMapping($storageId, $path, $rule->getUserMapping());
$fileIdKeys[] = $ruleFileIdKey = $this->keyFileId($rule->getFileId());
$this->set($rulePathKey, array_merge($rule->jsonSerialize(), [
'file_id' => $rule->getFileId()
]));
}


$pathKey = $this->keyPath($storageId, $path);
$this->set($pathKey, [
'rule_path_keys' => $rulePathKeys,
'rule_id_keys' => $fileIdKeys
], self::INDEX_TTL);
foreach ($fileIdKeys as $fileIdKey) {
$this->set($fileIdKey, $pathKey, self::INDEX_TTL);
}
}

/**
* @param IUserMapping[] $userMappings
* @param Rule $rule
* @return bool
*/
protected function checkMappingInRule(array $userMappings, Rule $rule) : bool
{
foreach ($userMappings as $userMapping) {
if ($userMapping->getId() == $rule->getUserMapping()->getId() &&
$userMapping->getType() == $rule->getUserMapping()->getType()) {
return true;
}
}

return false;
}

/**
* @param IUser $user
* @param string $pathKey
* @return Rule[]|null
*/
protected function getByPathKey(IUser $user, string $pathKey) : ?array
{
if (!$this->has($pathKey)) {
return null;
}

$keys = $this->get($pathKey);
if (!isset($keys['rule_path_keys'])) {
return null;
}

$userMappings = $this->userMappingManager->getMappingsForUser($user);

$rules = [];

foreach ($keys['rule_path_keys'] as $key) {
if (!$this->has($key)) {
return null;
}

$ruleData = $this->get($key);

$rule = new Rule(
new UserMapping($ruleData['mapping']['type'], $ruleData['mapping']['id']),
$ruleData['file_id'],
$ruleData['mask'],
$ruleData['permissions']
);

if ($this->checkMappingInRule($userMappings, $rule)) {
$rules[] = $rule;
}
}

return $rules;
}
}
28 changes: 27 additions & 1 deletion lib/ACL/RuleManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@

class RuleManager {
private $connection;
private $ruleCache;
private $userMappingManager;

public function __construct(IDBConnection $connection, IUserMappingManager $userMappingManager) {
public function __construct(IDBConnection $connection, ACLRuleCache $ruleCache, IUserMappingManager $userMappingManager) {
$this->connection = $connection;
$this->ruleCache = $ruleCache;
$this->userMappingManager = $userMappingManager;
}

Expand Down Expand Up @@ -261,6 +263,28 @@ public function getRulesForPrefix(IUser $user, int $storageId, string $prefix):
return $this->rulesByPath($rows);
}

/**
* @param int $storageId
* @param string[] $filePaths
* @return (Rule[])[] [$path => Rule[]]
*/
public function getAllRulesForFilesByPath(int $storageId, array $filePaths): array {
$query = $this->connection->getQueryBuilder();
$query->select(['f.fileid', 'mapping_type', 'mapping_id', 'mask', 'a.permissions', 'path'])
->from('group_folders_acl', 'a')
->innerJoin('a', 'filecache', 'f', $query->expr()->eq('f.fileid', 'a.fileid'))
->where($query->expr()->in('path_hash', $query->createNamedParameter(array_map('md5', $filePaths), IQueryBuilder::PARAM_STR_ARRAY)))
->andWhere($query->expr()->eq('storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)));

$rows = $query->execute()->fetchAll();

$result = [];
foreach ($filePaths as $path) {
$result[$path] = [];
}
return $this->rulesByPath($rows, $result);
}

private function hasRule(IUserMapping $mapping, int $fileId): bool {
$query = $this->connection->getQueryBuilder();
$query->select('fileid')
Expand Down Expand Up @@ -293,6 +317,7 @@ public function saveRule(Rule $rule) {
]);
$query->execute();
}
$this->ruleCache->clearByFileId($rule->getFileId());
}

public function deleteRule(Rule $rule) {
Expand All @@ -302,5 +327,6 @@ public function deleteRule(Rule $rule) {
->andWhere($query->expr()->eq('mapping_type', $query->createNamedParameter($rule->getUserMapping()->getType())))
->andWhere($query->expr()->eq('mapping_id', $query->createNamedParameter($rule->getUserMapping()->getId())));
$query->execute();
$this->ruleCache->clearByFileId($rule->getFileId());
}
}
Loading