Skip to content

Commit

Permalink
Permissions: Updated generation querying to be more efficient
Browse files Browse the repository at this point in the history
Query of existing entity permissions during view permission generation
could cause timeouts or SQL placeholder limits due to massive whereOr
query generation, where an "or where" clause would be created for each
entity type/id combo involved, which could be all within 20 books.

This updates the query handling to use a query per type involved, with
no "or where"s, and to be chunked at large entity counts.

Also tweaked role-specific permission regen to chunk books at
half-previous rate to prevent such a large scope being involved on each
chunk.

For #4695
  • Loading branch information
ssddanbrown committed Dec 23, 2023
1 parent 88ee33e commit 02d94c8
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 24 deletions.
56 changes: 38 additions & 18 deletions app/Permissions/EntityPermissionEvaluator.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@

class EntityPermissionEvaluator
{
protected string $action;

public function __construct(string $action)
{
$this->action = $action;
public function __construct(
protected string $action
) {
}

public function evaluateEntityForUser(Entity $entity, array $userRoleIds): ?bool
Expand Down Expand Up @@ -82,23 +80,25 @@ protected function collapseAndCategorisePermissions(array $typeIdChain, array $p
*/
protected function getPermissionsMapByTypeId(array $typeIdChain, array $filterRoleIds): array
{
$query = EntityPermission::query()->where(function (Builder $query) use ($typeIdChain) {
foreach ($typeIdChain as $typeId) {
$query->orWhere(function (Builder $query) use ($typeId) {
[$type, $id] = explode(':', $typeId);
$query->where('entity_type', '=', $type)
->where('entity_id', '=', $id);
});
$idsByType = [];
foreach ($typeIdChain as $typeId) {
[$type, $id] = explode(':', $typeId);
if (!isset($idsByType[$type])) {
$idsByType[$type] = [];
}
});

if (!empty($filterRoleIds)) {
$query->where(function (Builder $query) use ($filterRoleIds) {
$query->whereIn('role_id', [...$filterRoleIds, 0]);
});
$idsByType[$type][] = $id;
}

$relevantPermissions = $query->get(['entity_id', 'entity_type', 'role_id', $this->action])->all();
$relevantPermissions = [];

foreach ($idsByType as $type => $ids) {
$idsChunked = array_chunk($ids, 10000);
foreach ($idsChunked as $idChunk) {
$permissions = $this->getPermissionsForEntityIdsOfType($type, $idChunk, $filterRoleIds);
array_push($relevantPermissions, ...$permissions);
}
}

$map = [];
foreach ($relevantPermissions as $permission) {
Expand All @@ -113,6 +113,26 @@ protected function getPermissionsMapByTypeId(array $typeIdChain, array $filterRo
return $map;
}

/**
* @param string[] $ids
* @param int[] $filterRoleIds
* @return EntityPermission[]
*/
protected function getPermissionsForEntityIdsOfType(string $type, array $ids, array $filterRoleIds): array
{
$query = EntityPermission::query()
->where('entity_type', '=', $type)
->whereIn('entity_id', $ids);

if (!empty($filterRoleIds)) {
$query->where(function (Builder $query) use ($filterRoleIds) {
$query->whereIn('role_id', [...$filterRoleIds, 0]);
});
}

return $query->get(['entity_id', 'entity_type', 'role_id', $this->action])->all();
}

/**
* @return string[]
*/
Expand Down
4 changes: 2 additions & 2 deletions app/Permissions/JointPermissionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ public function rebuildForRole(Role $role)
$role->load('permissions');

// Chunk through all books
$this->bookFetchQuery()->chunk(20, function ($books) use ($roles) {
$this->bookFetchQuery()->chunk(10, function ($books) use ($roles) {
$this->buildJointPermissionsForBooks($books, $roles);
});

// Chunk through all bookshelves
Bookshelf::query()->select(['id', 'owned_by'])
->chunk(50, function ($shelves) use ($roles) {
->chunk(100, function ($shelves) use ($roles) {
$this->createManyJointPermissions($shelves->all(), $roles);
});
}
Expand Down
14 changes: 10 additions & 4 deletions database/seeders/LargeContentSeeder.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,18 @@ public function run()

/** @var Book $largeBook */
$largeBook = Book::factory()->create(['name' => 'Large book' . Str::random(10), 'created_by' => $editorUser->id, 'updated_by' => $editorUser->id]);
$pages = Page::factory()->count(200)->make(['created_by' => $editorUser->id, 'updated_by' => $editorUser->id]);
$chapters = Chapter::factory()->count(50)->make(['created_by' => $editorUser->id, 'updated_by' => $editorUser->id]);

$largeBook->pages()->saveMany($pages);
$largeBook->chapters()->saveMany($chapters);
$all = array_merge([$largeBook], array_values($pages->all()), array_values($chapters->all()));

$allPages = [];

foreach ($chapters as $chapter) {
$pages = Page::factory()->count(100)->make(['created_by' => $editorUser->id, 'updated_by' => $editorUser->id, 'chapter_id' => $chapter->id]);
$largeBook->pages()->saveMany($pages);
array_push($allPages, ...$pages->all());
}

$all = array_merge([$largeBook], $allPages, array_values($chapters->all()));

app()->make(JointPermissionBuilder::class)->rebuildForEntity($largeBook);
app()->make(SearchIndex::class)->indexEntities($all);
Expand Down

0 comments on commit 02d94c8

Please sign in to comment.