Improve perfomance (#170)

* WIP * Fix unit tests * Fix Psalm * Fix rector * Rename method [skip ci] * Fix logic and order * Fix logic with "can" * chore [skip ci] * Add methods for getting items by names' list * Fix coverage * Fix exceptions * Move TODO to issue [skip ci] * chore [skip ci] * Add AssignmentsStorage::getUserIdsByItemNames() * Apply fixes from StyleCI * Add AssignmentsStorage::userHasPermission() * More "exists" methods * Fix Psalm * Fix unit tests, rename * Test get direct children * Fix copy paste * All child permissions + all child roles * has child + has direct child * test for role exists * Add tests for getting items by names * Assignments - test for getting by item names * Assignments - test for exists * Assignments - test for user has item * Test exception for coverage * Ignore order * Sync with rbac-php, minor adjustments * Psalm - type for item names * Psalm - fix unused foreach variable * Psalm - simplify type * Minor improvements regarding recursive methods * Update CHANGELOG [skip ci] * Improve handling and control of `Assignment::$createdAt` * Revert "Improve handling and control of `Assignment::$createdAt`" This reverts commit cd55145. * Test storages (#192) * Test fake storages * Naming (review) * Exclude parent role from `Manager::getAllChildRoles()` (#191) * The rest of renaming * Improve handling and control of `Assignment::$createdAt` (#190) * Improve handling and control of `Assignment::$createdAt` * Apply Rector changes (CI) * Resolve merge conflicts * Update changelog (review) * Fix wording [skip ci] --------- Co-authored-by: arogachev <[email protected]> * Update CHANGELOG.md Co-authored-by: Sergei Predvoditelev <[email protected]> * Update src/Exception/DefaultRolesNotFoundException.php Co-authored-by: Sergei Predvoditelev <[email protected]> * Update CHANGELOG with new methods info [skip ci] * Add info about changing exception [skip ci] --------- Co-authored-by: StyleCI Bot <[email protected]> Co-authored-by: arogachev <[email protected]> Co-authored-by: Sergei Predvoditelev <[email protected]>
yiisoft · Sep 21, 2023 · 5a62e63 · 5a62e63
1 parent 4e46f99
commit 5a62e63
Show file tree

Hide file tree

Showing 23 changed files with 1,063 additions and 439 deletions.
diff --git a/.gitignore b/.gitignore
@@ -28,3 +28,4 @@ phpunit.phar
 /phpunit.xml
 # phpunit cache
 .phpunit.result.cache
+/.phpunit.cache
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,28 @@
 - Chg #161: Allow to reuse manager test code in related packages (@arogachev)
 - New #161: Add `ManagerInterface` (@arogachev)
 - Chg #161: Raise PHP version to 8.0 (@arogachev)
+- Bug #178: Exclude parent role from `Manager::getAllChildRoles()` (@arogachev)
+- Enh #134: Improve handling and control of `Assignment::$createdAt` (@arogachev)
+- Chg #134: Add `$createdAt` parameter to `ManagerInterface::assign()` (@arogachev)
+- Chg #134: Replace parameters with `$assignment` parameter in `AssignmentsStorageInterface::add()` (@arogachev)
+- Enh #165: Improve perfomance (@arogachev)
+- Enh #165: Rename `getChildren` method to `getDirectAchildren()` in `ItemsStorage` (@arogachev)
+- Enh #165: Add methods to `ItemsStorage`:
+    - `roleExists()`;
+    - `getRolesByNames()`;
+    - `getPermissionsByNames()`;
+    - `getAllChildren()`;
+    - `getAllChildRoles()`;
+    - `getAllChildPermissions()`;
+    - `hasChild()`;
+    - `hasDirectChild()`.
+      (@arogachev)
+- Enh #165: Add methods to `AssignmentsStorageInterface`:
+  - `getByItemNames()`;
+  - `exists()`;
+  - `userHasItem()`.
+    (@arogachev)
+- Enh #165: Rename `DefaultRoleNotFoundException` to `DefaultRolesNotFoundException` and finalize it (@arogachev)
 
 ## 1.0.2 April 20, 2023
 

diff --git a/rector.php b/rector.php
@@ -4,7 +4,6 @@
 
 use Rector\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector;
 use Rector\Config\RectorConfig;
-use Rector\Php56\Rector\FunctionLike\AddDefaultValueForUndefinedVariableRector;
 use Rector\Php74\Rector\Closure\ClosureToArrowFunctionRector;
 use Rector\Set\ValueObject\LevelSetList;
 
@@ -24,6 +23,5 @@
 
     $rectorConfig->skip([
         ClosureToArrowFunctionRector::class,
-        AddDefaultValueForUndefinedVariableRector::class,
     ]);
 };
diff --git a/src/AssignmentsStorageInterface.php b/src/AssignmentsStorageInterface.php
@@ -25,11 +25,21 @@ public function getAll(): array;
      *
      * @return Assignment[] The assignments. The array is indexed by the role or the permission names. An empty array
      * will be returned if there is no role or permission assigned to the user.
-     *
      * @psalm-return array<string, Assignment>
      */
     public function getByUserId(string $userId): array;
 
+    /**
+     * Returns all role or permission assignment information by the specified item names' list.
+     *
+     * @param string[] $itemNames List of item names.
+     *
+     * @return Assignment[] The assignments. An empty array will be returned if there are no users assigned to these
+     * item names.
+     * @psalm-return list<Assignment>
+     */
+    public function getByItemNames(array $itemNames): array;
+
     /**
      * Returns role or permission assignment for the specified item name that belongs to user with the specified ID.
      *
@@ -41,12 +51,31 @@ public function getByUserId(string $userId): array;
     public function get(string $itemName, string $userId): ?Assignment;
 
     /**
-     * Adds assignment of the role or permission to the user with ID specified.
+     * Whether assignment with a given item name and user id pair exists.
      *
-     * @param string $itemName Item name to assign.
-     * @param string $userId The user ID.
+     * @param string $itemName Item name.
+     * @param string $userId User id.
+     *
+     * @return bool Whether assignment exists.
+     */
+    public function exists(string $itemName, string $userId): bool;
+
+    /**
+     * Whether at least one item from the given list is assigned to the user.
+     *
+     * @param string $userId User id.
+     * @param string[] $itemNames List of item names.
+     *
+     * @return bool Whether at least one item from the given list is assigned to the user.
+     */
+    public function userHasItem(string $userId, array $itemNames): bool;
+
+    /**
+     * Adds assignment to the storage.
+     *
+     * @param Assignment $assignment Assignment instance.
      */
-    public function add(string $itemName, string $userId): void;
+    public function add(Assignment $assignment): void;
 
     /**
      * Returns whether there is assignment for a named role or permission.

diff --git a/src/Exception/DefaultRoleNotFoundException.php b/src/Exception/DefaultRoleNotFoundException.php
diff --git a/src/Exception/DefaultRolesNotFoundException.php b/src/Exception/DefaultRolesNotFoundException.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Rbac\Exception;
+
+use RuntimeException;
+use Yiisoft\FriendlyException\FriendlyExceptionInterface;
+
+final class DefaultRolesNotFoundException extends RuntimeException implements FriendlyExceptionInterface
+{
+    public function getName(): string
+    {
+        return 'Default roles not found.';
+    }
+
+    public function getSolution(): ?string
+    {
+        return <<<SOLUTION
+You have to add roles with Manager::addRole() before using them as default.
+SOLUTION;
+    }
+}
diff --git a/src/Exception/ItemAlreadyExistsException.php b/src/Exception/ItemAlreadyExistsException.php
@@ -13,12 +13,9 @@ final class ItemAlreadyExistsException extends RuntimeException
     public function __construct(Item $item, int $code = 0, ?Throwable $previous = null)
     {
         parent::__construct(
-            sprintf(
-                'Role or permission with name "%s" already exists.',
-                $item->getName()
-            ),
+            "Role or permission with name \"{$item->getName()}\" already exists.",
             $code,
-            $previous
+            $previous,
         );
     }
 }
diff --git a/src/Exception/RuleInterfaceNotImplementedException.php b/src/Exception/RuleInterfaceNotImplementedException.php
@@ -12,12 +12,10 @@ final class RuleInterfaceNotImplementedException extends Exception
 {
     public function __construct(string $name, int $code = 0, ?Throwable $previous = null)
     {
+        $interfaceName = RuleInterface::class;
+
         parent::__construct(
-            sprintf(
-                'Rule "%s" should implement "%s".',
-                $name,
-                RuleInterface::class,
-            ),
+            "Rule \"$name\" must implement \"$interfaceName\".",
             $code,
             $previous
         );

diff --git a/src/Exception/RuleNotFoundException.php b/src/Exception/RuleNotFoundException.php
@@ -11,6 +11,6 @@ final class RuleNotFoundException extends Exception
 {
     public function __construct(string $name, int $code = 0, ?Throwable $previous = null)
     {
-        parent::__construct(sprintf('Rule "%s" not found.', $name), $code, $previous);
+        parent::__construct("Rule \"$name\" not found.", $code, $previous);
     }
 }
diff --git a/src/ItemsStorageInterface.php b/src/ItemsStorageInterface.php
@@ -7,7 +7,7 @@
 /**
  * A storage for RBAC roles and permissions used in {@see Manager}.
  *
- * @psalm-type ItemsIndexedByName = array<string, Permission|Role>
+ * @psalm-type ItemsIndexedByName = array<string, Item>
  */
 interface ItemsStorageInterface
 {
@@ -34,14 +34,23 @@ public function getAll(): array;
     public function get(string $name): ?Item;
 
     /**
-     * Returns whether named role or permission exists.
+     * Whether named role or permission exists.
      *
      * @param string $name The role or the permission name.
      *
      * @return bool Whether named role or permission exists.
      */
     public function exists(string $name): bool;
 
+    /**
+     * Whether named role exists.
+     *
+     * @param string $name The role name.
+     *
+     * @return bool Whether named role exists.
+     */
+    public function roleExists(string $name): bool;
+
     /**
      * Adds the role or the permission to RBAC system.
      *
@@ -67,10 +76,21 @@ public function remove(string $name): void;
     /**
      * Returns all roles in the system.
      *
-     * @return Role[] All roles in the system.
+     * @return Role[] Array of role instances indexed by role names.
+     * @psalm-return array<string, Role>
      */
     public function getRoles(): array;
 
+    /**
+     * Returns roles by the given names' list.
+     *
+     * @param string[] $names List of role names.
+     *
+     * @return Role[] Array of role instances indexed by role names.
+     * @psalm-return array<string, Role>
+     */
+    public function getRolesByNames(array $names): array;
+
     /**
      * Returns the named role.
      *
@@ -89,17 +109,28 @@ public function clearRoles(): void;
     /**
      * Returns all permissions in the system.
      *
-     * @return Permission[] All permissions in the system.
+     * @return Permission[] Array of permission instances indexed by permission names.
+     * @psalm-return array<string, Permission>
      */
     public function getPermissions(): array;
 
+    /**
+     * Returns permissions by the given names' list.
+     *
+     * @param string[] $names List of permission names.
+     *
+     * @return Permission[] Array of permission instances indexed by permission names.
+     * @psalm-return array<string, Permission>
+     */
+    public function getPermissionsByNames(array $names): array;
+
     /**
      * Returns the named permission.
      *
      * @param string $name The permission name.
      *
-     * @return Permission|null The permission corresponding to the specified name. `null` is returned if no such
-     * permission.
+     * @return Permission|null The permission corresponding to the specified name. `null` is returned if there is no
+     * such permission.
      */
     public function getPermission(string $name): ?Permission;
 
@@ -121,15 +152,45 @@ public function clearPermissions(): void;
     public function getParents(string $name): array;
 
     /**
-     * Returns the child permissions and/or roles.
+     * Returns direct child permissions and/or roles.
      *
      * @param string $name The parent name.
      *
      * @return Item[] The child permissions and/or roles.
      *
      * @psalm-return ItemsIndexedByName
      */
-    public function getChildren(string $name): array;
+    public function getDirectChildren(string $name): array;
+
+    /**
+     * Returns all child permissions and/or roles.
+     *
+     * @param string $name The parent name.
+     *
+     * @return Item[] The child permissions and/or roles.
+     * @psalm-return array<string, Item>
+     */
+    public function getAllChildren(string $name): array;
+
+    /**
+     * Returns all child roles.
+     *
+     * @param string $name The parent name.
+     *
+     * @return Role[] The child roles.
+     * @psalm-return array<string, Role>
+     */
+    public function getAllChildRoles(string $name): array;
+
+    /**
+     * Returns all child permissions.
+     *
+     * @param string $name The parent name.
+     *
+     * @return Permission[] The child permissions.
+     * @psalm-return array<string, Permission>
+     */
+    public function getAllChildPermissions(string $name): array;
 
     /**
      * Returns whether named parent has children.
@@ -140,6 +201,26 @@ public function getChildren(string $name): array;
      */
     public function hasChildren(string $name): bool;
 
+    /**
+     * Returns whether selected parent has a child with a given name.
+     *
+     * @param string $parentName The parent name.
+     * @param string $childName The child name.
+     *
+     * @return bool Whether selected parent has a child with a given name.
+     */
+    public function hasChild(string $parentName, string $childName): bool;
+
+    /**
+     * Returns whether selected parent has a direct child with a given name.
+     *
+     * @param string $parentName The parent name.
+     * @param string $childName The child name.
+     *
+     * @return bool Whether selected parent has a direct child with a given name.
+     */
+    public function hasDirectChild(string $parentName, string $childName): bool;
+
     /**
      * Adds a role or a permission as a child of another role or permission.
      *