Skip to content

Commit

Permalink
DRY permission caching
Browse files Browse the repository at this point in the history
  • Loading branch information
lukasbestle committed Dec 23, 2024
1 parent 13ea6fc commit 9905934
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 24 deletions.
14 changes: 2 additions & 12 deletions src/Cms/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,7 @@ public function isAccessible(): bool
return false;
}

static $accessible = [];
$role = $this->kirby()->role()?->id() ?? '__none__';
$template = $this->template() ?? '__none__';
$accessible[$role] ??= [];

return $accessible[$role][$template] ??= $this->permissions()->access();
return $this->permissions()->access();
}

/**
Expand All @@ -342,12 +337,7 @@ public function isListable(): bool
return false;
}

static $listable = [];
$role = $this->kirby()->role()?->id() ?? '__none__';
$template = $this->template() ?? '__none__';
$listable[$role] ??= [];

return $listable[$role][$template] ??= $this->permissions()->list();
return $this->permissions()->list();
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/Cms/FilePermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ class FilePermissions extends ModelPermissions
{
protected string $category = 'files';

/**
* Used to cache once determined permissions in memory
*/
protected function cacheKey(): string
{
return $this->model->template() ?? '__none__';
}

protected function canChangeTemplate(): bool
{
if (count($this->model->blueprints()) <= 1) {
Expand Down
39 changes: 39 additions & 0 deletions src/Cms/ModelPermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ abstract class ModelPermissions
protected string $category;
protected array $options;

protected static array $cache = [];

public function __construct(protected ModelWithContent|Language $model)
{
$this->options = match (true) {
Expand All @@ -40,6 +42,16 @@ public function __debugInfo(): array
return $this->toArray();
}

/**
* Can be overridden by specific child classes
* to return a model-specific value used to
* cache a once determined permission in memory
*/
protected function cacheKey(): string
{
return '';
}

/**
* Returns whether the current user is allowed to do
* a certain action on the model
Expand Down Expand Up @@ -75,6 +87,33 @@ public function can(
return true;
}

// cache the often-used read permissions;
// note that the custom `can` method check happens before,
// so dynamic code still has the chance to be run each time
if ($action === 'access' || $action === 'list') {
$category = $this->category();
$cacheKey = $category . '.' . $action . '/' . $this->cacheKey() . '/' . $role;

if (isset(static::$cache[$cacheKey]) === true) {
return static::$cache[$cacheKey];
}

return static::$cache[$cacheKey] = $this->canFromOptionsAndRole($action, $role, $default);
}

// determine all other permissions dynamically
// TODO: caching these is generally possible, but currently makes many unit tests break
return $this->canFromOptionsAndRole($action, $role, $default);
}

/**
* Main logic for `can()` that can be cached
*/
protected function canFromOptionsAndRole(
string $action,
string $role,
bool $default = false
): bool {
// evaluate the blueprint options block
if (isset($this->options[$action]) === true) {
$options = $this->options[$action];
Expand Down
14 changes: 2 additions & 12 deletions src/Cms/Page.php
Original file line number Diff line number Diff line change
Expand Up @@ -522,12 +522,7 @@ public function isAccessible(): bool
return false;
}

static $accessible = [];
$role = $this->kirby()->role()?->id() ?? '__none__';
$template = $this->intendedTemplate()->name();
$accessible[$role] ??= [];

return $accessible[$role][$template] ??= $this->permissions()->access();
return $this->permissions()->access();
}

/**
Expand Down Expand Up @@ -694,12 +689,7 @@ public function isListable(): bool
return false;
}

static $listable = [];
$role = $this->kirby()->role()?->id() ?? '__none__';
$template = $this->intendedTemplate()->name();
$listable[$role] ??= [];

return $listable[$role][$template] ??= $this->permissions()->list();
return $this->permissions()->list();
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/Cms/PagePermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ class PagePermissions extends ModelPermissions
{
protected string $category = 'pages';

/**
* Used to cache once determined permissions in memory
*/
protected function cacheKey(): string
{
return $this->model->intendedTemplate()->name();
}

protected function canChangeSlug(): bool
{
return $this->model->isHomeOrErrorPage() !== true;
Expand Down
8 changes: 8 additions & 0 deletions src/Cms/UserPermissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@
*/
class UserPermissions extends ModelPermissions
{
/**
* Used to cache once determined permissions in memory
*/
protected function cacheKey(): string
{
return $this->model->role()->id();
}

protected function canChangeRole(): bool
{
// protect admin from role changes by non-admin
Expand Down
30 changes: 30 additions & 0 deletions tests/Cms/Files/FilePermissionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ public function setUp(): void
$this->app = new App([
'roots' => [
'index' => '/dev/null'
],
'users' => [
['id' => 'bastian', 'role' => 'admin']
]
]);
}
Expand Down Expand Up @@ -60,6 +63,33 @@ public function testWithNobody($action)
$this->assertFalse($perms->can($action));
}

/**
* @covers \Kirby\Cms\ModelPermissions::can
*/
public function testCaching()
{
$this->app->impersonate('bastian');

$page = new Page(['slug' => 'test']);
$file = new File([
'filename' => 'test.jpg',
'parent' => $page,
'template' => 'some-template',
'blueprint' => [
'name' => 'files/some-template',
'options' => [
'access' => false,
'list' => false
]
]
]);

$this->assertFalse($file->permissions()->can('access'));
$this->assertFalse($file->permissions()->can('access'));
$this->assertFalse($file->permissions()->can('list'));
$this->assertFalse($file->permissions()->can('list'));
}

/**
* @covers ::canChangeTemplate
*/
Expand Down
26 changes: 26 additions & 0 deletions tests/Cms/Pages/PagePermissionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,32 @@ public function testWithNobody($action)
$this->assertFalse($perms->can($action));
}

/**
* @covers \Kirby\Cms\ModelPermissions::can
*/
public function testCaching()
{
$this->app->impersonate('bastian');

$page = new Page([
'slug' => 'test',
'num' => 1,
'template' => 'some-template',
'blueprint' => [
'name' => 'some-template',
'options' => [
'access' => false,
'list' => false
]
]
]);

$this->assertFalse($page->permissions()->can('access'));
$this->assertFalse($page->permissions()->can('access'));
$this->assertFalse($page->permissions()->can('list'));
$this->assertFalse($page->permissions()->can('list'));
}

/**
* @covers ::canChangeTemplate
*/
Expand Down

0 comments on commit 9905934

Please sign in to comment.