Skip to content

Commit 055fbbf

Browse files
solracsficewind1991
authored andcommitted
fix(acl): make preloadRulesForFolder actually warm the rule cache
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
1 parent 3d2e642 commit 055fbbf

2 files changed

Lines changed: 36 additions & 1 deletion

File tree

lib/ACL/ACLManager.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,18 @@ public function getPermissionsForTree(int $storageId, string $path): int {
242242
}
243243
}
244244

245+
/**
246+
* Warm the rule cache for the direct children of a folder.
247+
*
248+
* Used before listing a folder with many children (e.g. trash listings),
249+
* so the subsequent per-child `getACLPermissionsForPath` lookups are
250+
* served from the cache instead of querying the rules for each child path.
251+
*/
245252
public function preloadRulesForFolder(int $storageId, int $parentId): void {
246-
$this->ruleManager->getRulesForFilesByParent($this->user, $storageId, $parentId);
253+
$rules = $this->ruleManager->getRulesForFilesByParent($this->user, $storageId, $parentId);
254+
foreach ($rules as $path => $rulesForPath) {
255+
$this->ruleCache->set($path, $rulesForPath);
256+
}
247257
}
248258

249259
/**

tests/ACL/ACLManagerTest.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ class ACLManagerTest extends TestCase {
3030
private IUserMapping&MockObject $dummyMapping;
3131
/** @var array<string, list<Rule>> */
3232
private array $rules = [];
33+
/** @var list<string> paths that were resolved through a query rather than the cache */
34+
private array $requestedPaths = [];
3335

3436
protected function setUp(): void {
3537
parent::setUp();
@@ -45,6 +47,8 @@ protected function setUp(): void {
4547
$this->ruleManager->method('getRulesForFilesByPath')
4648
->willReturnCallback(function (IUser $user, int $storageId, array $paths): array {
4749
// fill with empty in case no rule was found
50+
/** @var string[] $paths */
51+
$this->requestedPaths = array_values(array_merge($this->requestedPaths, $paths));
4852
$rules = array_fill_keys($paths, []);
4953
$actualRules = array_filter($this->rules, fn (string $path): bool => array_search($path, $paths) !== false, ARRAY_FILTER_USE_KEY);
5054

@@ -122,6 +126,27 @@ public function testGetACLPermissionsForPathInTrashbin(): void {
122126

123127

124128

129+
public function testPreloadRulesForFolderPopulatesCache(): void {
130+
// no per-path rules are available: anything resolved by a query returns empty
131+
$this->rules = [];
132+
$childPath = '__groupfolders/trash/1/subfolder2.d1700752274';
133+
$rule = new Rule($this->dummyMapping, 10, Constants::PERMISSION_SHARE, 0); // deny share
134+
135+
$this->ruleManager->expects($this->once())
136+
->method('getRulesForFilesByParent')
137+
->willReturn([$childPath => [$rule]]);
138+
139+
$this->requestedPaths = [];
140+
$this->aclManager->preloadRulesForFolder(0, 1);
141+
142+
$permissions = $this->aclManager->getACLPermissionsForPath(0, 0, $childPath);
143+
144+
// the rule supplied through preload must have been applied straight from the cache ...
145+
$this->assertEquals(Constants::PERMISSION_ALL - Constants::PERMISSION_SHARE, $permissions);
146+
// ... without re-querying the leaf path that was preloaded
147+
$this->assertNotContains($childPath, $this->requestedPaths);
148+
}
149+
125150
public function testGetACLPermissionsForPathPerUserMerge(): void {
126151
$aclManager = $this->getAclManager(true);
127152
$this->rules = [

0 commit comments

Comments
 (0)