diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e3e957..dd90dd68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,9 @@ - Enh #203: Add `getGuestRoleName()` and `getGuestRole()` methods to `Manager` (@arogachev) - Chg #203: Throw `RuntimeException` in the case with implicit guest and non-existing guest role in `Manager::userHasPermission()` (@arogachev) +- Enh #206: Optimize calls for getting child items within the loops (@arogachev) +- Chg #206: Rename `$name` argument to `$names` and allow array type for it in `getAllChildren()`, `getAllChildRoles()`, + `getAllChildPermissions()` methods of `ItemsStorageInterface` (@arogachev) ## 1.0.2 April 20, 2023 diff --git a/src/ItemsStorageInterface.php b/src/ItemsStorageInterface.php index 07bfaccb..9b64dd34 100644 --- a/src/ItemsStorageInterface.php +++ b/src/ItemsStorageInterface.php @@ -190,32 +190,32 @@ public function getDirectChildren(string $name): array; /** * Returns all child permissions and/or roles. * - * @param string $name The parent name. + * @param array|string $names The parent name / names. * * @return array The child permissions and/or roles. * @psalm-return ItemsIndexedByName */ - public function getAllChildren(string $name): array; + public function getAllChildren(string|array $names): array; /** * Returns all child roles. * - * @param string $name The parent name. + * @param array|string $names The parent name / names. * * @return Role[] The child roles. * @psalm-return array */ - public function getAllChildRoles(string $name): array; + public function getAllChildRoles(string|array $names): array; /** * Returns all child permissions. * - * @param string $name The parent name. + * @param array|string $names The parent name / names. * * @return Permission[] The child permissions. * @psalm-return array */ - public function getAllChildPermissions(string $name): array; + public function getAllChildPermissions(string|array $names): array; /** * Returns whether named parent has children. diff --git a/src/Manager.php b/src/Manager.php index aa8faff7..cb8bc6c1 100644 --- a/src/Manager.php +++ b/src/Manager.php @@ -182,32 +182,26 @@ public function getItemsByUserId(int|Stringable|string $userId): array { $userId = (string) $userId; $assignments = $this->assignmentsStorage->getByUserId($userId); - $items = array_merge( + $assignmentNames = array_keys($assignments); + + return array_merge( $this->getDefaultRoles(), - $this->itemsStorage->getByNames(array_keys($assignments)), + $this->itemsStorage->getByNames($assignmentNames), + $this->itemsStorage->getAllChildren($assignmentNames), ); - - foreach ($assignments as $assignment) { - $items = array_merge($items, $this->itemsStorage->getAllChildren($assignment->getItemName())); - } - - return $items; } public function getRolesByUserId(int|Stringable|string $userId): array { $userId = (string) $userId; $assignments = $this->assignmentsStorage->getByUserId($userId); - $roles = array_merge( + $assignmentNames = array_keys($assignments); + + return array_merge( $this->getDefaultRoles(), - $this->itemsStorage->getRolesByNames(array_keys($assignments)), + $this->itemsStorage->getRolesByNames($assignmentNames), + $this->itemsStorage->getAllChildRoles($assignmentNames) ); - - foreach ($assignments as $assignment) { - $roles = array_merge($roles, $this->itemsStorage->getAllChildRoles($assignment->getItemName())); - } - - return $roles; } public function getChildRoles(string $roleName): array @@ -228,16 +222,12 @@ public function getPermissionsByUserId(int|Stringable|string $userId): array { $userId = (string) $userId; $assignments = $this->assignmentsStorage->getByUserId($userId); - $permissions = $this->itemsStorage->getPermissionsByNames(array_keys($assignments)); - - foreach ($assignments as $assignment) { - $permissions = array_merge( - $permissions, - $this->itemsStorage->getAllChildPermissions($assignment->getItemName()), - ); - } + $assignmentNames = array_keys($assignments); - return $permissions; + return array_merge( + $this->itemsStorage->getPermissionsByNames($assignmentNames), + $this->itemsStorage->getAllChildPermissions($assignmentNames), + ); } public function getUserIdsByRoleName(string $roleName): array diff --git a/tests/Common/ItemsStorageTestTrait.php b/tests/Common/ItemsStorageTestTrait.php index af0ff46d..4b6536f4 100644 --- a/tests/Common/ItemsStorageTestTrait.php +++ b/tests/Common/ItemsStorageTestTrait.php @@ -190,6 +190,11 @@ public function dataGetAllChildren(): array 'posts.admin', ['posts.redactor', 'posts.viewer', 'posts.view', 'posts.create', 'posts.update', 'posts.delete'], ], + [['Parent 1', 'Parent 2'], ['Child 1', 'Child 2', 'Child 3']], + [ + ['posts.viewer', 'posts.redactor', 'posts.admin'], + ['posts.redactor', 'posts.viewer', 'posts.view', 'posts.create', 'posts.update', 'posts.delete'], + ], ['non-existing', []], ]; } @@ -197,9 +202,9 @@ public function dataGetAllChildren(): array /** * @dataProvider dataGetAllChildren */ - public function testGetAllChildren(string $parentName, array $expectedChildren): void + public function testGetAllChildren(string|array $parentNames, array $expectedChildren): void { - $children = $this->getItemsStorage()->getAllChildren($parentName); + $children = $this->getItemsStorage()->getAllChildren($parentNames); $this->assertChildren($children, $expectedChildren); } @@ -215,6 +220,11 @@ public function dataGetAllChildPermissions(): array ['posts.viewer', ['posts.view']], ['posts.redactor', ['posts.view', 'posts.create', 'posts.update']], ['posts.admin', ['posts.view', 'posts.create', 'posts.update', 'posts.delete']], + [['Parent 1', 'Parent 5'], ['Child 1', 'Child 5']], + [ + ['posts.viewer', 'posts.redactor', 'posts.admin'], + ['posts.view', 'posts.create', 'posts.update', 'posts.delete'], + ], ['non-existing', []], ]; } @@ -222,9 +232,9 @@ public function dataGetAllChildPermissions(): array /** * @dataProvider dataGetAllChildPermissions */ - public function testGetAllChildPermissions(string $parentName, array $expectedChildren): void + public function testGetAllChildPermissions(string|array $parentNames, array $expectedChildren): void { - $children = $this->getItemsStorage()->getAllChildPermissions($parentName); + $children = $this->getItemsStorage()->getAllChildPermissions($parentNames); $this->assertChildren($children, $expectedChildren); } @@ -240,6 +250,8 @@ public function dataGetAllChildRoles(): array ['posts.viewer', []], ['posts.redactor', ['posts.viewer']], ['posts.admin', ['posts.redactor', 'posts.viewer']], + [['Parent 2', 'Parent 4'], ['Child 2', 'Child 3', 'Child 4']], + [['posts.viewer', 'posts.redactor', 'posts.admin'], ['posts.redactor', 'posts.viewer']], ['non-existing', []], ]; } @@ -247,9 +259,9 @@ public function dataGetAllChildRoles(): array /** * @dataProvider dataGetAllChildRoles */ - public function testGetAllChildRoles(string $parentName, array $expectedChildren): void + public function testGetAllChildRoles(string|array $parentNames, array $expectedChildren): void { - $children = $this->getItemsStorage()->getAllChildRoles($parentName); + $children = $this->getItemsStorage()->getAllChildRoles($parentNames); $this->assertChildren($children, $expectedChildren); } diff --git a/tests/Support/FakeItemsStorage.php b/tests/Support/FakeItemsStorage.php index 3a0ee2b2..04ef563b 100644 --- a/tests/Support/FakeItemsStorage.php +++ b/tests/Support/FakeItemsStorage.php @@ -102,26 +102,26 @@ public function getDirectChildren(string $name): array return $this->children[$name] ?? []; } - public function getAllChildren(string $name): array + public function getAllChildren(string|array $names): array { $result = []; - $this->fillChildrenRecursive($name, $result); + $this->getAllChildrenInternal($names, $result); return $result; } - public function getAllChildRoles(string $name): array + public function getAllChildRoles(string|array $names): array { $result = []; - $this->fillChildrenRecursive($name, $result); + $this->getAllChildrenInternal($names, $result); return $this->filterRoles($result); } - public function getAllChildPermissions(string $name): array + public function getAllChildPermissions(string|array $names): array { $result = []; - $this->fillChildrenRecursive($name, $result); + $this->getAllChildrenInternal($names, $result); return $this->filterPermissions($result); } @@ -290,6 +290,14 @@ private function fillAccessTreeRecursive(string $name, array &$result, array $ad } } + private function getAllChildrenInternal(string|array $names, array &$result): void + { + $names = (array) $names; + foreach ($names as $name) { + $this->fillChildrenRecursive($name, $result); + } + } + private function fillChildrenRecursive(string $name, array &$result): void { $children = $this->children[$name] ?? []; @@ -327,14 +335,9 @@ private function filterRoles(array $array): array */ private function filterPermissions(array $items): array { - $permissions = []; - foreach (array_keys($items) as $permissionName) { - $permission = $this->getPermission($permissionName); - if ($permission !== null) { - $permissions[$permissionName] = $permission; - } - } - - return $permissions; + return array_filter( + $this->getPermissions(), + static fn (Permission $permissionItem): bool => array_key_exists($permissionItem->getName(), $items), + ); } }