From d7f252ff01ed6411218167f490e0438bd5b3ac81 Mon Sep 17 00:00:00 2001 From: Alexey Rogachev Date: Tue, 12 Dec 2023 19:36:16 +0600 Subject: [PATCH 1/4] Use simple storages, PHP 8.1 and PHPUnit 10.5.2 --- CHANGELOG.md | 6 +- composer.json | 7 +- phpunit.xml.dist | 36 +- src/AssignmentsStorage.php | 125 +----- ...CommonStorage.php => FileStorageTrait.php} | 2 +- src/ItemsStorage.php | 387 +----------------- tests/ItemsStorageTest.php | 4 +- tests/bootstrap.php | 10 + 8 files changed, 72 insertions(+), 505 deletions(-) rename src/{CommonStorage.php => FileStorageTrait.php} (98%) create mode 100644 tests/bootstrap.php diff --git a/CHANGELOG.md b/CHANGELOG.md index a43e090..994fd65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,9 @@ - Chg #63: Raise PHP version to 8.0 (@arogachev) - Enh #63: Improve performance (@arogachev) -- Enh #?: Implement `getByNames()` and `getAccessTree()` methods in `ItemsStorage` (@arogachev) -- Enh #?: Implement `filterUserItemNames()` method in `AssignmentsStorage` (@arogachev) -- Chg #?: Rename `$name` argument to `$names` and allow array type for it in `getAllChildren()`, `getAllChildRoles()`, +- Enh #70: Implement `getByNames()` and `getAccessTree()` methods in `ItemsStorage` (@arogachev) +- Enh #70: Implement `filterUserItemNames()` method in `AssignmentsStorage` (@arogachev) +- Chg #70: Rename `$name` argument to `$names` and allow array type for it in `getAllChildren()`, `getAllChildRoles()`, `getAllChildPermissions()` methods in `ItemsStorage` (@arogachev) ## 1.0.0 April 08, 2022 diff --git a/composer.json b/composer.json index 145b56c..e8866dd 100644 --- a/composer.json +++ b/composer.json @@ -29,13 +29,13 @@ ], "minimum-stability": "dev", "require": { - "php": "^8.0", + "php": "^8.1", "yiisoft/rbac": "dev-master", "yiisoft/var-dumper": "^1.0" }, "require-dev": { "ext-uopz": "*", - "phpunit/phpunit": "^9.5", + "phpunit/phpunit": "^10.5.2", "psr/log": "^1.1.3", "roave/infection-static-analysis-plugin": "^1.18", "slope-it/clock-mock": "0.4.0", @@ -46,7 +46,8 @@ "autoload": { "psr-4": { "Yiisoft\\Rbac\\Php\\": "src" - } + }, + "files": ["tests/bootstrap.php"] }, "autoload-dev": { "psr-4": { diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 35cc0ef..da2f32a 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,31 +1,27 @@ - - - - - - - - ./tests - - - - - - ./src - - + + + + + + ./tests + + + + + ./src + + diff --git a/src/AssignmentsStorage.php b/src/AssignmentsStorage.php index 37c4db0..ffed31e 100644 --- a/src/AssignmentsStorage.php +++ b/src/AssignmentsStorage.php @@ -5,19 +5,18 @@ namespace Yiisoft\Rbac\Php; use Yiisoft\Rbac\Assignment; -use Yiisoft\Rbac\AssignmentsStorageInterface; - -use function array_key_exists; +use Yiisoft\Rbac\SimpleAssignmentsStorage; /** - * Storage stores authorization data in three PHP files specified by {@see Storage::itemFile}, - * {@see Storage::assignmentFile} and {@see Storage::ruleFile}. + * Storage stores roles and permissions in PHP file specified in {@see AssignmentsStorage::$assignmentFile}. * * It is suitable for authorization data that is not too big (for example, the authorization data for a personal blog * system). */ -final class AssignmentsStorage extends CommonStorage implements AssignmentsStorageInterface +final class AssignmentsStorage extends SimpleAssignmentsStorage { + use FileStorageTrait; + /** * @var string The path of the PHP script that contains the authorization assignments. Make sure this file is * writable by the web server process if the authorization needs to be changed online. @@ -27,12 +26,6 @@ final class AssignmentsStorage extends CommonStorage implements AssignmentsStora */ private string $assignmentFile; - /** - * @var array Array in format is `[userId => [itemName => assignment]]`. - * @psalm-var array> - */ - private array $assignments = []; - public function __construct( string $directory, string $assignmentFile = 'assignments.php' @@ -41,88 +34,11 @@ public function __construct( $this->loadAssignments(); } - public function getAll(): array - { - return $this->assignments; - } - - public function getByUserId(string $userId): array - { - return $this->assignments[$userId] ?? []; - } - - public function getByItemNames(array $itemNames): array - { - $result = []; - - foreach ($this->assignments as $assignments) { - foreach ($assignments as $userAssignment) { - if (in_array($userAssignment->getItemName(), $itemNames, true)) { - $result[] = $userAssignment; - } - } - } - - return $result; - } - - public function get(string $itemName, string $userId): ?Assignment - { - return $this->getByUserId($userId)[$itemName] ?? null; - } - - public function exists(string $itemName, string $userId): bool - { - return isset($this->getByUserId($userId)[$itemName]); - } - - public function userHasItem(string $userId, array $itemNames): bool - { - $assignments = $this->getByUserId($userId); - if (empty($assignments)) { - return false; - } - - foreach ($itemNames as $itemName) { - if (array_key_exists($itemName, $assignments)) { - return true; - } - } - - return false; - } - - public function filterUserItemNames(string $userId, array $itemNames): array - { - $assignments = $this->getByUserId($userId); - if (empty($assignments)) { - return []; - } - - $userItemNames = []; - foreach ($itemNames as $itemName) { - if (array_key_exists($itemName, $assignments)) { - $userItemNames[] = $itemName; - } - } - - return $userItemNames; - } - public function add(Assignment $assignment): void { - $this->assignments[$assignment->getUserId()][$assignment->getItemName()] = $assignment; - $this->saveAssignments(); - } + parent::add($assignment); - public function hasItem(string $name): bool - { - foreach ($this->getAll() as $assignmentInfo) { - if (array_key_exists($name, $assignmentInfo)) { - return true; - } - } - return false; + $this->saveAssignments(); } public function renameItem(string $oldName, string $newName): void @@ -131,12 +47,7 @@ public function renameItem(string $oldName, string $newName): void return; } - foreach ($this->assignments as &$assignments) { - if (isset($assignments[$oldName])) { - $assignments[$newName] = $assignments[$oldName]->withItemName($newName); - unset($assignments[$oldName]); - } - } + parent::renameItem($oldName, $newName); $this->saveAssignments(); } @@ -147,33 +58,32 @@ public function remove(string $itemName, string $userId): void return; } - unset($this->assignments[$userId][$itemName]); + parent::remove($itemName, $userId); + $this->saveAssignments(); } public function removeByUserId(string $userId): void { - $this->assignments[$userId] = []; + parent::removeByUserId($userId); + $this->saveAssignments(); } public function removeByItemName(string $itemName): void { - foreach ($this->assignments as &$assignments) { - unset($assignments[$itemName]); - } + parent::removeByItemName($itemName); + $this->saveAssignments(); } public function clear(): void { - $this->assignments = []; + parent::clear(); + $this->saveAssignments(); } - /** - * Loads authorization data from persistent storage. - */ private function loadAssignments(): void { /** @psalm-var array $assignments */ @@ -187,9 +97,6 @@ private function loadAssignments(): void } } - /** - * Saves assignments data into persistent storage. - */ private function saveAssignments(): void { $assignmentData = []; diff --git a/src/CommonStorage.php b/src/FileStorageTrait.php similarity index 98% rename from src/CommonStorage.php rename to src/FileStorageTrait.php index 6417cd3..96b4667 100644 --- a/src/CommonStorage.php +++ b/src/FileStorageTrait.php @@ -10,7 +10,7 @@ use function dirname; use function function_exists; -abstract class CommonStorage +trait FileStorageTrait { /** * Loads the authorization data from a PHP script file. diff --git a/src/ItemsStorage.php b/src/ItemsStorage.php index 87abe77..08a9233 100644 --- a/src/ItemsStorage.php +++ b/src/ItemsStorage.php @@ -7,27 +7,20 @@ use Yiisoft\Rbac\Item; use Yiisoft\Rbac\Permission; use Yiisoft\Rbac\Role; -use Yiisoft\Rbac\ItemsStorageInterface; +use Yiisoft\Rbac\SimpleItemsStorage; /** - * Storage stores roles and permissions in PHP file specified by `itemFile`. + * Storage stores roles and permissions in PHP file specified in {@see ItemsStorage::$itemFile}. * * It is suitable for authorization data that is not too big (for example, the authorization data for a personal blog * system). * - * @psalm-type RawItem = array{ - * type: string, - * name: string, - * description?: string, - * ruleName?: string, - * children?: string[] - * } - * - * @psalm-import-type ItemsIndexedByName from ItemsStorageInterface - * @psalm-import-type AccessTree from ItemsStorageInterface + * @psalm-import-type RawItem from SimpleItemsStorage */ -final class ItemsStorage extends CommonStorage implements ItemsStorageInterface +final class ItemsStorage extends SimpleItemsStorage { + use FileStorageTrait; + /** * @var string The path of the PHP script that contains the authorization items. * @@ -36,19 +29,6 @@ final class ItemsStorage extends CommonStorage implements ItemsStorageInterface */ private string $itemFile; - /** - * @var Permission[]|Role[] - * @psalm-var ItemsIndexedByName - * Format is [itemName => item]. - */ - private array $items = []; - - /** - * @psalm-var array - * Format is [itemName => [childName => child]]. - */ - private array $children = []; - /** * @param string $directory Base directory to append to `$itemFile`. * @param string $itemFile The path of the PHP script that contains the authorization items. Make sure this file is @@ -60,154 +40,18 @@ public function __construct(string $directory, string $itemFile = 'items.php') $this->load(); } - public function getAll(): array - { - return $this->items; - } - - public function getByNames(array $names): array - { - return array_filter($this->getAll(), static fn (Item $item): bool => in_array($item->getName(), $names)); - } - - public function get(string $name): Permission|Role|null - { - return $this->items[$name] ?? null; - } - - public function exists(string $name): bool - { - return array_key_exists($name, $this->items); - } - - public function roleExists(string $name): bool - { - return isset($this->getItemsByType(Item::TYPE_ROLE)[$name]); - } - public function add(Permission|Role $item): void { - $this->items[$item->getName()] = $item; - $this->save(); - } - - public function getRole(string $name): ?Role - { - return $this->getItemsByType(Item::TYPE_ROLE)[$name] ?? null; - } - - public function getRoles(): array - { - return $this->getItemsByType(Item::TYPE_ROLE); - } - - public function getRolesByNames(array $names): array - { - return array_filter( - $this->getAll(), - static fn (Permission|Role $item): bool => $item instanceof Role && in_array($item->getName(), $names), - ); - } - - public function getPermission(string $name): ?Permission - { - return $this->getItemsByType(Item::TYPE_PERMISSION)[$name] ?? null; - } - - public function getPermissions(): array - { - return $this->getItemsByType(Item::TYPE_PERMISSION); - } - - public function getPermissionsByNames(array $names): array - { - return array_filter( - $this->getAll(), - static function (Permission|Role $item) use ($names): bool { - return $item instanceof Permission && in_array($item->getName(), $names); - }, - ); - } - - public function getParents(string $name): array - { - $result = []; - $this->fillParentsRecursive($name, $result); - - return $result; - } + parent::add($item); - public function getAccessTree(string $name): array - { - $result = [$name => ['item' => $this->items[$name], 'children' => []]]; - $this->fillAccessTreeRecursive($name, $result); - - return $result; - } - - public function getDirectChildren(string $name): array - { - return $this->children[$name] ?? []; - } - - public function getAllChildren(string|array $names): array - { - $result = []; - $this->getAllChildrenInternal($names, $result); - - return $result; - } - - public function getAllChildRoles(string|array $names): array - { - $result = []; - $this->getAllChildrenInternal($names, $result); - - return $this->filterRoles($result); - } - - public function getAllChildPermissions(string|array $names): array - { - $result = []; - $this->getAllChildrenInternal($names, $result); - - return $this->filterPermissions($result); - } - - public function addChild(string $parentName, string $childName): void - { - $this->children[$parentName][$childName] = $this->items[$childName]; $this->save(); } - public function hasChildren(string $name): bool - { - return isset($this->children[$name]); - } - - public function hasChild(string $parentName, string $childName): bool + public function addChild(string $parentName, string $childName): void { - if ($parentName === $childName) { - return true; - } - - $children = $this->getDirectChildren($parentName); - if (empty($children)) { - return false; - } - - foreach ($children as $groupChild) { - if ($this->hasChild($groupChild->getName(), $childName)) { - return true; - } - } - - return false; - } + parent::addChild($parentName, $childName); - public function hasDirectChild(string $parentName, string $childName): bool - { - return isset($this->children[$parentName][$childName]); + $this->save(); } public function removeChild(string $parentName, string $childName): void @@ -216,7 +60,8 @@ public function removeChild(string $parentName, string $childName): void return; } - unset($this->children[$parentName][$childName]); + parent::removeChild($parentName, $childName); + $this->save(); } @@ -226,51 +71,25 @@ public function removeChildren(string $parentName): void return; } - unset($this->children[$parentName]); + parent::removeChildren($parentName); + $this->save(); } public function remove(string $name): void { - $this->clearChildrenFromItem($name); - $this->removeItemByName($name); - $this->save(); - } + parent::remove($name); - public function update(string $name, Permission|Role $item): void - { - if ($item->getName() !== $name) { - $this->updateItemName($name, $item); - $this->removeItemByName($name); - } - - $this->add($item); - } - - public function clear(): void - { - $this->clearLoadedData(); $this->save(); } - public function clearPermissions(): void - { - $this->removeAllItems(Item::TYPE_PERMISSION); - } - - public function clearRoles(): void + public function clear(): void { - $this->removeAllItems(Item::TYPE_ROLE); - } + parent::clear(); - private function updateItemName(string $name, Item $item): void - { - $this->updateChildrenForItemName($name, $item); + $this->save(); } - /** - * Saves authorization data into persistent storage. - */ private function save(): void { $items = []; @@ -285,12 +104,10 @@ private function save(): void $this->saveToFile($items, $this->itemFile); } - /** - * Loads authorization data from persistent storage. - */ private function load(): void { - $this->clearLoadedData(); + parent::clear(); + $this->loadItems(); } @@ -317,48 +134,11 @@ private function loadItems(): void } } - private function clearLoadedData(): void - { - $this->children = []; - $this->items = []; - } - private function hasItem(string $name): bool { return isset($this->items[$name]); } - /** - * @psalm-param Item::TYPE_* $type - * - * @psalm-return ($type is Item::TYPE_PERMISSION ? array : array) - */ - private function getItemsByType(string $type): array - { - return array_filter( - $this->getAll(), - static fn (Permission|Role $item): bool => $item->getType() === $type, - ); - } - - /** - * Removes all auth items of the specified type. - * - * @param string $type The auth item type (either {@see Item::TYPE_PERMISSION} or {@see Item::TYPE_ROLE}). - * @psalm-param Item::TYPE_* $type - */ - private function removeAllItems(string $type): void - { - foreach ($this->getItemsByType($type) as $item) { - $this->remove($item->getName()); - } - } - - private function clearChildrenFromItem(string $itemName): void - { - unset($this->children[$itemName]); - } - private function getInstanceByTypeAndName(string $type, string $name): Permission|Role { return $type === Item::TYPE_PERMISSION ? new Permission($name) : new Role($name); @@ -383,131 +163,4 @@ private function getInstanceFromAttributes(array $attributes): Permission|Role return $item; } - - private function updateChildrenForItemName(string $name, Item $item): void - { - if ($this->hasChildren($name)) { - $this->children[$item->getName()] = $this->children[$name]; - unset($this->children[$name]); - } - foreach ($this->children as &$children) { - if (isset($children[$name])) { - $children[$item->getName()] = $item; - unset($children[$name]); - } - } - } - - private function removeItemByName(string $name): void - { - unset($this->items[$name]); - } - - /** - * @psalm-param array $result - * @psalm-param-out array $result - */ - private function fillParentsRecursive(string $name, array &$result): void - { - foreach ($this->children as $parentName => $childItems) { - foreach ($childItems as $childItem) { - if ($childItem->getName() !== $name) { - continue; - } - - $parent = $this->get($parentName); - if ($parent !== null) { - $result[$parentName] = $parent; - } - - $this->fillParentsRecursive($parentName, $result); - - break; - } - } - } - - /** - * @psalm-param AccessTree $result - * @psalm-param-out AccessTree $result - * - * @psalm-param ItemsIndexedByName $addedChildItems - */ - private function fillAccessTreeRecursive(string $name, array &$result, array $addedChildItems = []): void - { - foreach ($this->children as $parentName => $childItems) { - foreach ($childItems as $childItem) { - if ($childItem->getName() !== $name) { - continue; - } - - $parent = $this->get($parentName); - if ($parent !== null) { - /** @psalm-var AccessTree $result Imported type in `psalm-param-out` is not resolved. */ - $result[$parentName]['item'] = $this->items[$parentName]; - - $addedChildItems[$childItem->getName()] = $childItem; - $result[$parentName]['children'] = $addedChildItems; - } - - $this->fillAccessTreeRecursive($parentName, $result, $addedChildItems); - - break; - } - } - } - - /** - * @param string|string[] $names - * - * @psalm-param ItemsIndexedByName $result - * @psalm-param-out ItemsIndexedByName $result - */ - private function getAllChildrenInternal(string|array $names, array &$result): void - { - $names = (array) $names; - foreach ($names as $name) { - $this->fillChildrenRecursive($name, $result); - } - } - - /** - * @psalm-param ItemsIndexedByName $result - * @psalm-param-out ItemsIndexedByName $result - */ - private function fillChildrenRecursive(string $name, array &$result): void - { - $children = $this->children[$name] ?? []; - foreach ($children as $childName => $_childItem) { - $child = $this->get($childName); - if ($child !== null) { - /** @psalm-var ItemsIndexedByName $result Imported type in `psalm-param-out` is not resolved. */ - $result[$childName] = $child; - } - - $this->fillChildrenRecursive($childName, $result); - } - } - - /** - * @psalm-param ItemsIndexedByName $items - * - * @return Role[] - * @psalm-return array - */ - private function filterRoles(array $items): array - { - return array_filter($items, static fn (Permission|Role $item): bool => $item instanceof Role); - } - - /** - * @psalm-param ItemsIndexedByName $items - * - * @return Permission[] - * @psalm-return array - */ - private function filterPermissions(array $items): array - { - return array_filter($items, static fn (Permission|Role $item): bool => $item instanceof Permission); - } } diff --git a/tests/ItemsStorageTest.php b/tests/ItemsStorageTest.php index 6194179..bc90565 100644 --- a/tests/ItemsStorageTest.php +++ b/tests/ItemsStorageTest.php @@ -21,7 +21,7 @@ final class ItemsStorageTest extends TestCase protected function setUp(): void { - if ($this->getName() === 'testFailCreateDirectory' || $this->getName() === 'testCreateNestedDirectory') { + if ($this->name() === 'testFailCreateDirectory' || $this->name() === 'testCreateNestedDirectory') { FileHelper::ensureDirectory($this->getTempDirectory()); FileHelper::clearDirectory($this->getTempDirectory()); } @@ -31,7 +31,7 @@ protected function setUp(): void protected function tearDown(): void { - if ($this->getName() === 'testFailCreateDirectory' || $this->getName() === 'testCreateNestedDirectory') { + if ($this->name() === 'testFailCreateDirectory' || $this->name() === 'testCreateNestedDirectory') { FileHelper::removeDirectory($this->getTempDirectory()); } diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..f7cb39e --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,10 @@ + Date: Tue, 12 Dec 2023 19:39:08 +0600 Subject: [PATCH 2/4] Update CHANGELOG [skip ci] --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 994fd65..1add85e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ - Enh #70: Implement `filterUserItemNames()` method in `AssignmentsStorage` (@arogachev) - Chg #70: Rename `$name` argument to `$names` and allow array type for it in `getAllChildren()`, `getAllChildRoles()`, `getAllChildPermissions()` methods in `ItemsStorage` (@arogachev) +- Enh #76: Use simple storages for items and assignments from the base `rbac` package (@arogachev) +- Chg #76: Raise PHP version to 8.1 (@arogachev) ## 1.0.0 April 08, 2022 From bb0d2e5fd19fd23480a07bc13a8765c15bbe4e7f Mon Sep 17 00:00:00 2001 From: Alexey Rogachev Date: Tue, 12 Dec 2023 19:42:38 +0600 Subject: [PATCH 3/4] Fix CI and improve Psalm --- .github/workflows/build.yml | 1 - .github/workflows/static.yml | 2 +- .scrutinizer.yml | 2 +- src/ItemsStorage.php | 5 +++++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index fc9c00a..27aedcb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -34,7 +34,6 @@ jobs: - windows-latest php: - - 8.0 - 8.1 - 8.2 diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index 2344a41..ae5a02c 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -28,5 +28,5 @@ jobs: os: >- ['ubuntu-latest'] php: >- - ['8.0', '8.1', '8.2'] + ['8.1', '8.2'] extensions: uopz diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 29e9b30..d5c6ac3 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -10,7 +10,7 @@ build: environment: php: - version: 8.0.18 + version: 8.1 pecl_extensions: - uopz ini: diff --git a/src/ItemsStorage.php b/src/ItemsStorage.php index 08a9233..ef275c3 100644 --- a/src/ItemsStorage.php +++ b/src/ItemsStorage.php @@ -139,6 +139,11 @@ private function hasItem(string $name): bool return isset($this->items[$name]); } + /** + * @psalm-param Item::TYPE_* $type + * + * @psalm-return ($type is Item::TYPE_PERMISSION ? array : array) + */ private function getInstanceByTypeAndName(string $type, string $name): Permission|Role { return $type === Item::TYPE_PERMISSION ? new Permission($name) : new Role($name); From 06df6f2eb9b3252624e589327be726715e18fadb Mon Sep 17 00:00:00 2001 From: Alexey Rogachev Date: Tue, 12 Dec 2023 19:55:13 +0600 Subject: [PATCH 4/4] Fix Psalm --- src/ItemsStorage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ItemsStorage.php b/src/ItemsStorage.php index ef275c3..e4b1822 100644 --- a/src/ItemsStorage.php +++ b/src/ItemsStorage.php @@ -142,7 +142,7 @@ private function hasItem(string $name): bool /** * @psalm-param Item::TYPE_* $type * - * @psalm-return ($type is Item::TYPE_PERMISSION ? array : array) + * @psalm-return ($type is Item::TYPE_PERMISSION ? Permission : Role) */ private function getInstanceByTypeAndName(string $type, string $name): Permission|Role {