Merge branch '165-improve-perfomance' into 184-assignment-created-at

# Conflicts: # tests/Common/AssignmentsStorageTestTrait.php
yiisoft · Sep 15, 2023 · 7243d88 · 7243d88
2 parents 8cfa9a1 + bd01b15
commit 7243d88
Show file tree

Hide file tree

Showing 9 changed files with 185 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 - Chg #161: Raise PHP version to 8.0 (@arogachev)
 - Enh #165: Improve perfomance (@arogachev)
 - Enh #134: Improve handling and control of `Assignment::$createdAt` (@arogachev)
+- Bug #178: Exclude parent role from `Manager::getAllChildRoles()` (@arogachev)
 
 ## 1.0.2 April 20, 2023
 

diff --git a/src/Manager.php b/src/Manager.php
@@ -158,12 +158,11 @@ public function getRolesByUserId(int|Stringable|string $userId): array
 
     public function getChildRoles(string $roleName): array
     {
-        $role = $this->itemsStorage->getRole($roleName);
-        if ($role === null) {
+        if (!$this->itemsStorage->roleExists($roleName)) {
             throw new InvalidArgumentException("Role \"$roleName\" not found.");
         }
 
-        return array_merge([$roleName => $role], $this->itemsStorage->getAllChildRoles($roleName));
+        return $this->itemsStorage->getAllChildRoles($roleName);
     }
 
     public function getPermissionsByRoleName(string $roleName): array

diff --git a/src/ManagerInterface.php b/src/ManagerInterface.php
@@ -120,8 +120,7 @@ public function getRolesByUserId(int|Stringable|string $userId): array;
      *
      * @throws InvalidArgumentException If role was not found by `$roleName`.
      *
-     * @return Role[] Child roles. The array is indexed by the role names. First element is an instance of the parent
-     * role itself.
+     * @return Role[] Child roles. The array is indexed by the role names.
      * @psalm-return array<string, Role>
      */
     public function getChildRoles(string $roleName): array;

diff --git a/tests/AssignmentsStorageTest.php b/tests/AssignmentsStorageTest.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Rbac\Tests;
+
+use PHPUnit\Framework\TestCase;
+use Yiisoft\Rbac\Tests\Common\AssignmentsStorageTestTrait;
+
+final class AssignmentsStorageTest extends TestCase
+{
+    use AssignmentsStorageTestTrait;
+}
diff --git a/tests/Common/AssignmentsStorageTestTrait.php b/tests/Common/AssignmentsStorageTestTrait.php
@@ -6,34 +6,49 @@
 
 use SlopeIt\ClockMock\ClockMock;
 use Yiisoft\Rbac\Assignment;
+use Yiisoft\Rbac\AssignmentsStorageInterface;
 use Yiisoft\Rbac\Item;
+use Yiisoft\Rbac\ItemsStorageInterface;
+use Yiisoft\Rbac\Permission;
+use Yiisoft\Rbac\Role;
+use Yiisoft\Rbac\Tests\Support\FakeAssignmentsStorage;
+use Yiisoft\Rbac\Tests\Support\FakeItemsStorage;
 
 trait AssignmentsStorageTestTrait
 {
+    private ?ItemsStorageInterface $itemsStorage = null;
+    private ?AssignmentsStorageInterface $storage = null;
+
     protected function setUp(): void
     {
         if ($this->getName() === 'testAdd') {
             ClockMock::freeze(new DateTime('2023-05-10 08:24:39'));
         }
+
+        $this->populateItemsStorage();
+        $this->populateAssignmentsStorage();
     }
 
     protected function tearDown(): void
     {
         if ($this->getName() === 'testAdd') {
             ClockMock::reset();
         }
+
+        $this->getItemsStorage()->clear();
+        $this->getAssignmentsStorage()->clear();
     }
 
     public function testHasItem(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
 
         $this->assertTrue($storage->hasItem('Accountant'));
     }
 
     public function testRenameItem(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $storage->renameItem('Accountant', 'Senior accountant');
 
         $this->assertFalse($storage->hasItem('Accountant'));
@@ -42,7 +57,7 @@ public function testRenameItem(): void
 
     public function testGetAll(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $all = $storage->getAll();
 
         $this->assertCount(3, $all);
@@ -56,7 +71,7 @@ public function testGetAll(): void
 
     public function testRemoveByItemName(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $storage->removeByItemName('Manager');
 
         $this->assertFalse($storage->hasItem('Manager'));
@@ -66,7 +81,7 @@ public function testRemoveByItemName(): void
 
     public function testGetByUserId(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $assignments = $storage->getByUserId('john');
 
         $this->assertCount(3, $assignments);
@@ -93,7 +108,7 @@ public function dataGetByItemNames(): array
      */
     public function testGetByItemNames(array $itemNames, array $expectedAssignments): void
     {
-        $assignments = $this->getStorage()->getByItemNames($itemNames);
+        $assignments = $this->getAssignmentsStorage()->getByItemNames($itemNames);
         $this->assertCount(count($expectedAssignments), $assignments);
 
         $assignmentFound = false;
@@ -115,7 +130,7 @@ public function testGetByItemNames(array $itemNames, array $expectedAssignments)
 
     public function testRemoveByUserId(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $storage->removeByUserId('jack');
 
         $this->assertEmpty($storage->getByUserId('jack'));
@@ -124,7 +139,7 @@ public function testRemoveByUserId(): void
 
     public function testRemove(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $storage->remove('Accountant', 'john');
 
         $this->assertEmpty($storage->get('Accountant', 'john'));
@@ -133,15 +148,15 @@ public function testRemove(): void
 
     public function testClear(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $storage->clear();
 
         $this->assertEmpty($storage->getAll());
     }
 
     public function testGet(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $assignment = $storage->get('Manager', 'jack');
 
         $this->assertSame('Manager', $assignment->getItemName());
@@ -151,7 +166,7 @@ public function testGet(): void
 
     public function testGetNonExisting(): void
     {
-        $this->assertNull($this->getStorage()->get('Researcher', 'jeff'));
+        $this->assertNull($this->getAssignmentsStorage()->get('Researcher', 'jeff'));
     }
 
     public function dataExists(): array
@@ -170,7 +185,7 @@ public function dataExists(): array
      */
     public function testExists(string $itemName, string $userId, bool $expectedExists): void
     {
-        $this->assertSame($expectedExists, $this->getStorage()->exists($itemName, $userId));
+        $this->assertSame($expectedExists, $this->getAssignmentsStorage()->exists($itemName, $userId));
     }
 
     public function dataUserHasItem(): array
@@ -190,12 +205,12 @@ public function dataUserHasItem(): array
      */
     public function testUserHasItem(string $userId, array $itemNames, bool $expectedUserHasItem): void
     {
-        $this->assertSame($expectedUserHasItem, $this->getStorage()->userHasItem($userId, $itemNames));
+        $this->assertSame($expectedUserHasItem, $this->getAssignmentsStorage()->userHasItem($userId, $itemNames));
     }
 
     public function testAdd(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $storage->add(userId: 'john', itemName: 'Operator');
 
         $this->assertEquals(
@@ -206,7 +221,7 @@ public function testAdd(): void
 
     public function testAddWithCreatedAt(): void
     {
-        $storage = $this->getStorage();
+        $storage = $this->getAssignmentsStorage();
         $storage->add(userId: 'john', itemName: 'Operator', createdAt: 1_694_508_008);
 
         $this->assertEquals(
@@ -256,4 +271,51 @@ static function (array $item) use ($time): array {
 
         return ['items' => $items, 'assignments' => $assignments];
     }
+
+    protected function populateItemsStorage(): void
+    {
+        foreach ($this->getFixtures()['items'] as $itemData) {
+            $name = $itemData['name'];
+            $item = $itemData['type'] === Item::TYPE_PERMISSION ? new Permission($name) : new Role($name);
+            $item = $item
+                ->withCreatedAt($itemData['createdAt'])
+                ->withUpdatedAt($itemData['updatedAt']);
+            $this->getItemsStorage()->add($item);
+        }
+    }
+
+    protected function populateAssignmentsStorage(): void
+    {
+        foreach ($this->getFixtures()['assignments'] as $assignmentData) {
+            $this->getAssignmentsStorage()->add($assignmentData['itemName'], $assignmentData['userId']);
+        }
+    }
+
+    protected function getItemsStorage(): ItemsStorageInterface
+    {
+        if ($this->itemsStorage === null) {
+            $this->itemsStorage = $this->createItemsStorage();
+        }
+
+        return $this->itemsStorage;
+    }
+
+    protected function getAssignmentsStorage(): AssignmentsStorageInterface
+    {
+        if ($this->storage === null) {
+            $this->storage = $this->createAssignmentsStorage();
+        }
+
+        return $this->storage;
+    }
+
+    protected function createItemsStorage(): ItemsStorageInterface
+    {
+        return new FakeItemsStorage();
+    }
+
+    protected function createAssignmentsStorage(): AssignmentsStorageInterface
+    {
+        return new FakeAssignmentsStorage();
+    }
 }