Add SimpleRuleFactory (#249)

* Add `SimpleRuleFactory` * WIP * Apply fixes from StyleCI * Fix failed workflows * Apply fixes from StyleCI * Add separate test * Review fix * Parameters are not optional too (review fix) --------- Co-authored-by: StyleCI Bot <[email protected]>
yiisoft · Feb 16, 2024 · 2204ce4 · 2204ce4
1 parent 98cf8d7
commit 2204ce4
Show file tree

Hide file tree

Showing 17 changed files with 166 additions and 131 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -57,6 +57,7 @@
 - Enh #245: Handle same names during renaming item in `AssignmentsStorage` (@arogachev)
 - Chg #208: Rename `getAccessTree()` to `getHierarchy()` in `ItemsStorageInterface` (@arogachev)
 - Enh #252: Return `$this` instead of throwing "already assigned" exception in `Manager::assign()` (@arogachev)
+- Enh #248: Add `SimpleRuleFactory` (@arogachev)
 
 ## 1.0.2 April 20, 2023
 

diff --git a/README.md b/README.md
@@ -70,38 +70,15 @@ use Yiisoft\Rbac\RuleFactoryInterface;
 $manager = new Manager($itemsStorage, $assignmentsStorage, $ruleFactory);
 ```
 
-It requires specifying the following dependencies:
+It requires the following dependencies:
 
 - Items storage (hierarchy itself).
 - Assignments storage where user IDs are mapped to roles.
-- Rule factory. Given a rule name stored in item storage it can create an instance of `Rule`.
+- Rule factory. Creates a rule instance by a given name.
 
-If you don't want to use [Rules Container](https://github.com/yiisoft/rbac-rules-container), here is an example of 
-simple self-contained rule factory:
-
-```php
-use Yiisoft\Rbac\Exception\RuleNotFoundException;
-use Yiisoft\Rbac\RuleFactoryInterface;
-use Yiisoft\Rbac\RuleInterface;
-
-use function array_key_exists;
-
-final class SimpleRuleFactory implements RuleFactoryInterface
-{    
-    public function __construct(private array $rules = [])
-    {
-    }
-
-    public function create(string $name): RuleInterface
-    {
-        if (!array_key_exists($name, $this->rules)) {
-            throw new RuleNotFoundException($name);
-        }
-
-        return $this->rules[$name];
-    }
-}
-```
+While storages are required, rule factory is optional and, when omitted, `SimpleRuleFactory` will be used. For more 
+advanced usage, such as resolving rules by aliases and passing arguments in rules constructor, install 
+[Rules Container](https://github.com/yiisoft/rbac-rules-container) additionally or write your own implementation.
 
 A few tips for choosing storage backend:
 
@@ -198,13 +175,13 @@ use Yiisoft\Rbac\Permission;
 
 /** @var ManagerInterface $manager */
 $manager->addPermission( 
-    (new Permission('viewList'))->withRuleName('action_rule'),
+    (new Permission('viewList'))->withRuleName(ActionRule::class),
 );
 
 // or
 
 $manager->addRole(
-    (new Role('NewYearMaintainer'))->withRuleName('new_year_only_rule')
+    (new Role('NewYearMaintainer'))->withRuleName(NewYearOnlyRule::class)
 );
 ```
 

diff --git a/src/Manager.php b/src/Manager.php
@@ -19,6 +19,7 @@
  */
 final class Manager implements ManagerInterface
 {
+    private readonly RuleFactoryInterface $ruleFactory;
     /**
      * @var string[] A list of role names that are assigned to every user automatically without calling {@see assign()}.
      * Note that these roles are applied to users, regardless of their state of authentication.
@@ -36,9 +37,10 @@ final class Manager implements ManagerInterface
     public function __construct(
         private readonly ItemsStorageInterface $itemsStorage,
         private readonly AssignmentsStorageInterface $assignmentsStorage,
-        private readonly RuleFactoryInterface $ruleFactory,
+        ?RuleFactoryInterface $ruleFactory = null,
         private readonly bool $enableDirectPermissions = false,
     ) {
+        $this->ruleFactory = $ruleFactory ?? new SimpleRuleFactory();
     }
 
     public function userHasPermission(

diff --git a/src/SimpleRuleFactory.php b/src/SimpleRuleFactory.php
@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Rbac;
+
+use Yiisoft\Rbac\Exception\RuleInterfaceNotImplementedException;
+use Yiisoft\Rbac\Exception\RuleNotFoundException;
+
+final class SimpleRuleFactory implements RuleFactoryInterface
+{
+    public function create(string $name): RuleInterface
+    {
+        if (!class_exists($name)) {
+            throw new RuleNotFoundException($name);
+        }
+
+        if (!is_a($name, RuleInterface::class, allow_string: true)) {
+            throw new RuleInterfaceNotImplementedException($name);
+        }
+
+        return new $name();
+    }
+}
diff --git a/tests/Common/ItemsStorageTestTrait.php b/tests/Common/ItemsStorageTestTrait.php
@@ -11,6 +11,7 @@
 use Yiisoft\Rbac\Permission;
 use Yiisoft\Rbac\Role;
 use Yiisoft\Rbac\Tests\Support\FakeItemsStorage;
+use Yiisoft\Rbac\Tests\Support\TrueRule;
 
 trait ItemsStorageTestTrait
 {
@@ -66,7 +67,7 @@ public function testUpdate(string $itemName, string $parentNameForChildrenCheck,
 
         $item = $item
             ->withName('Super Admin')
-            ->withRuleName('super admin');
+            ->withRuleName(TrueRule::class);
         $actionStorage->update($itemName, $item);
 
         $this->assertNull($testStorage->get($itemName));
@@ -75,7 +76,7 @@ public function testUpdate(string $itemName, string $parentNameForChildrenCheck,
         $this->assertNotNull($item);
 
         $this->assertSame('Super Admin', $item->getName());
-        $this->assertSame('super admin', $item->getRuleName());
+        $this->assertSame(TrueRule::class, $item->getRuleName());
 
         $this->assertSame($expectedHasChildren, $testStorage->hasChildren($parentNameForChildrenCheck));
     }

diff --git a/tests/Common/ManagerConfigurationTestTrait.php b/tests/Common/ManagerConfigurationTestTrait.php
@@ -10,28 +10,23 @@
 use Yiisoft\Rbac\ManagerInterface;
 use Yiisoft\Rbac\Permission;
 use Yiisoft\Rbac\Role;
-use Yiisoft\Rbac\RuleFactoryInterface;
 use Yiisoft\Rbac\Tests\Support\AuthorRule;
-use Yiisoft\Rbac\Tests\Support\EasyRule;
 use Yiisoft\Rbac\Tests\Support\FakeAssignmentsStorage;
 use Yiisoft\Rbac\Tests\Support\FakeItemsStorage;
-use Yiisoft\Rbac\Tests\Support\SimpleRuleFactory;
 
 trait ManagerConfigurationTestTrait
 {
     protected function createManager(
         ?ItemsStorageInterface $itemsStorage = null,
         ?AssignmentsStorageInterface $assignmentsStorage = null,
-        ?RuleFactoryInterface $ruleFactory = null,
-        ?bool $enableDirectPermissions = false
+        ?bool $enableDirectPermissions = false,
     ): ManagerInterface {
         $arguments = [
-            $itemsStorage ?? $this->createItemsStorage(),
-            $assignmentsStorage ?? $this->createAssignmentsStorage(),
-            $ruleFactory ?? new SimpleRuleFactory(),
+            'itemsStorage' => $itemsStorage ?? $this->createItemsStorage(),
+            'assignmentsStorage' => $assignmentsStorage ?? $this->createAssignmentsStorage(),
         ];
         if ($enableDirectPermissions !== null) {
-            $arguments[] = $enableDirectPermissions;
+            $arguments['enableDirectPermissions'] = $enableDirectPermissions;
         }
 
         return new Manager(...$arguments);
@@ -50,25 +45,19 @@ protected function createAssignmentsStorage(): AssignmentsStorageInterface
     protected function createFilledManager(
         ?ItemsStorageInterface $itemsStorage = null,
         ?AssignmentsStorageInterface $assignmentsStorage = null,
-        ?RuleFactoryInterface $ruleFactory = null,
     ): ManagerInterface {
         return $this
             ->createManager(
                 $itemsStorage ?? $this->createItemsStorage(),
                 $assignmentsStorage ?? $this->createAssignmentsStorage(),
-                $ruleFactory ?? new SimpleRuleFactory([
-                    'isAuthor' => new AuthorRule(),
-                    'easyTrue' => new EasyRule(true),
-                    'easyFalse' => new EasyRule(false),
-                ]),
                 enableDirectPermissions: true,
             )
             ->addPermission(new Permission('Fast Metabolism'))
             ->addPermission(new Permission('createPost'))
             ->addPermission(new Permission('publishPost'))
             ->addPermission(new Permission('readPost'))
             ->addPermission(new Permission('deletePost'))
-            ->addPermission((new Permission('updatePost'))->withRuleName('isAuthor'))
+            ->addPermission((new Permission('updatePost'))->withRuleName(AuthorRule::class))
             ->addPermission(new Permission('updateAnyPost'))
             ->addRole(new Role('reader'))
             ->addRole(new Role('author'))

diff --git a/tests/Common/ManagerLogicTestTrait.php b/tests/Common/ManagerLogicTestTrait.php
@@ -12,18 +12,20 @@
 use Yiisoft\Rbac\Assignment;
 use Yiisoft\Rbac\Exception\DefaultRolesNotFoundException;
 use Yiisoft\Rbac\Exception\ItemAlreadyExistsException;
+use Yiisoft\Rbac\Exception\RuleInterfaceNotImplementedException;
 use Yiisoft\Rbac\Exception\RuleNotFoundException;
 use Yiisoft\Rbac\Permission;
 use Yiisoft\Rbac\Role;
+use Yiisoft\Rbac\RuleInterface;
 use Yiisoft\Rbac\Tests\Support\AdsRule;
-use Yiisoft\Rbac\Tests\Support\GuestRule;
 use Yiisoft\Rbac\Tests\Support\AuthorRule;
 use Yiisoft\Rbac\Tests\Support\BanRule;
-use Yiisoft\Rbac\Tests\Support\EasyRule;
 use Yiisoft\Rbac\Tests\Support\FakeAssignmentsStorage;
 use Yiisoft\Rbac\Tests\Support\FakeItemsStorage;
-use Yiisoft\Rbac\Tests\Support\SimpleRuleFactory;
+use Yiisoft\Rbac\Tests\Support\GuestRule;
 use Yiisoft\Rbac\Tests\Support\SubscriptionRule;
+use Yiisoft\Rbac\Tests\Support\TrueRule;
+use Yiisoft\Rbac\Tests\Support\WannabeRule;
 
 trait ManagerLogicTestTrait
 {
@@ -175,30 +177,23 @@ public function testUserHasPermissionGuestOriented(
             ->createManager(
                 $this->createItemsStorage(),
                 $this->createAssignmentsStorage(),
-                new SimpleRuleFactory([
-                    'subscription' => new SubscriptionRule(),
-                    'ads' => new AdsRule(),
-                    'author' => new AuthorRule(),
-                    'ban' => new BanRule(),
-                    'guest' => new GuestRule(),
-                ]),
                 enableDirectPermissions: true,
             )
-            ->addRole((new Role('guest'))->withRuleName('guest'))
+            ->addRole((new Role('guest'))->withRuleName(GuestRule::class))
             ->setGuestRoleName('guest')
             ->addRole(new Role('news comment manager'))
             ->addRole(new Role('warned user'))
             ->addRole(new Role('trial user'))
-            ->addRole((new Role('subscribed user'))->withRuleName('subscription'))
-            ->addPermission((new Permission('view ads'))->withRuleName('ads'))
-            ->addPermission((new Permission('view ban warning'))->withRuleName('ban'))
+            ->addRole((new Role('subscribed user'))->withRuleName(SubscriptionRule::class))
+            ->addPermission((new Permission('view ads'))->withRuleName(AdsRule::class))
+            ->addPermission((new Permission('view ban warning'))->withRuleName(BanRule::class))
             ->addPermission(new Permission('view content'))
             ->addPermission(new Permission('view regular content'))
             ->addPermission(new Permission('view news'))
             ->addPermission(new Permission('add news comment'))
             ->addPermission(new Permission('view news comment'))
-            ->addPermission((new Permission('edit news comment'))->withRuleName('author'))
-            ->addPermission((new Permission('remove news comment'))->withRuleName('author'))
+            ->addPermission((new Permission('edit news comment'))->withRuleName(AuthorRule::class))
+            ->addPermission((new Permission('remove news comment'))->withRuleName(AuthorRule::class))
             ->addPermission(new Permission('view wiki'))
             ->addPermission(new Permission('view exclusive content'))
             ->addChild('view content', 'view regular content')
@@ -262,6 +257,22 @@ public function testUserHasPermissionWithNonExistingRule(): void
         $manager->userHasPermission('reader A', 'test-permission');
     }
 
+    public function testUserHasPermissionWithRuleMissingImplements(): void
+    {
+        $className = WannabeRule::class;
+        $interfaceName = RuleInterface::class;
+        $manager = $this
+            ->createFilledManager()
+            ->addPermission((new Permission('test-permission'))->withRuleName($className))
+            ->addRole(new Role('test'))
+            ->addChild('test', 'test-permission')
+            ->assign('test-permission', 'reader A');
+
+        $this->expectException(RuleInterfaceNotImplementedException::class);
+        $this->expectExceptionMessage("Rule \"$className\" must implement \"$interfaceName\".");
+        $manager->userHasPermission('reader A', 'test-permission');
+    }
+
     public function testCanAddExistingChild(): void
     {
         $manager = $this->createFilledManager();
@@ -601,11 +612,11 @@ public function testAddRole(): void
     {
         $manager = $this->createFilledManager();
 
-        $rule = new EasyRule();
+        $rule = new TrueRule();
 
         $role = (new Role('new role'))
             ->withDescription('new role description')
-            ->withRuleName($rule->getName())
+            ->withRuleName(TrueRule::class)
             ->withCreatedAt(1_642_026_147)
             ->withUpdatedAt(1_642_026_148);
 
@@ -621,7 +632,7 @@ public function testAddRole(): void
             [
                 'name' => 'new role',
                 'description' => 'new role description',
-                'rule_name' => EasyRule::class,
+                'rule_name' => TrueRule::class,
                 'type' => 'role',
                 'updated_at' => 1_642_026_148,
                 'created_at' => 1_642_026_147,

diff --git a/tests/CompositeRuleTest.php b/tests/CompositeRuleTest.php
@@ -9,21 +9,22 @@
 use Yiisoft\Rbac\CompositeRule;
 use Yiisoft\Rbac\Permission;
 use Yiisoft\Rbac\RuleContext;
-use Yiisoft\Rbac\Tests\Support\EasyRule;
-use Yiisoft\Rbac\Tests\Support\SimpleRuleFactory;
+use Yiisoft\Rbac\SimpleRuleFactory;
+use Yiisoft\Rbac\Tests\Support\FalseRule;
+use Yiisoft\Rbac\Tests\Support\TrueRule;
 
 final class CompositeRuleTest extends TestCase
 {
     public static function dataCompositeRule(): array
     {
         return [
             'AND empty' => [CompositeRule::AND, [], true],
-            'AND all true' => [CompositeRule::AND, ['easy_rule_true', 'easy_rule_true'], true],
-            'AND last false' => [CompositeRule::AND, ['easy_rule_true', 'easy_rule_false'], false],
+            'AND all true' => [CompositeRule::AND, [TrueRule::class, TrueRule::class], true],
+            'AND last false' => [CompositeRule::AND, [TrueRule::class, FalseRule::class], false],
 
             'OR empty' => [CompositeRule::OR, [], true],
-            'OR all false' => [CompositeRule::OR, ['easy_rule_false', 'easy_rule_false'], false],
-            'OR last true' => [CompositeRule::OR, ['easy_rule_false', 'easy_rule_true'], true],
+            'OR all false' => [CompositeRule::OR, [FalseRule::class, FalseRule::class], false],
+            'OR last true' => [CompositeRule::OR, [FalseRule::class, TrueRule::class], true],
         ];
     }
 
@@ -33,11 +34,7 @@ public static function dataCompositeRule(): array
     public function testCompositeRule(string $operator, array $rules, bool $expected): void
     {
         $rule = new CompositeRule($operator, $rules);
-        $ruleFactory = new SimpleRuleFactory([
-            'easy_rule_false' => new EasyRule(false),
-            'easy_rule_true' => new EasyRule(true),
-        ]);
-        $result = $rule->execute('user', new Permission('permission'), new RuleContext($ruleFactory, []));
+        $result = $rule->execute('user', new Permission('permission'), new RuleContext(new SimpleRuleFactory(), []));
         $this->assertSame($expected, $result);
     }
 

diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php
@@ -13,9 +13,9 @@
 use Yiisoft\Rbac\Manager;
 use Yiisoft\Rbac\ManagerInterface;
 use Yiisoft\Rbac\RuleFactoryInterface;
+use Yiisoft\Rbac\SimpleRuleFactory;
 use Yiisoft\Rbac\Tests\Support\FakeAssignmentsStorage;
 use Yiisoft\Rbac\Tests\Support\FakeItemsStorage;
-use Yiisoft\Rbac\Tests\Support\SimpleRuleFactory;
 
 final class ConfigTest extends TestCase
 {

diff --git a/tests/PermissionTest.php b/tests/PermissionTest.php
@@ -7,6 +7,7 @@
 use PHPUnit\Framework\TestCase;
 use Yiisoft\Rbac\Item;
 use Yiisoft\Rbac\Permission;
+use Yiisoft\Rbac\Tests\Support\TrueRule;
 
 final class PermissionTest extends TestCase
 {
@@ -17,7 +18,7 @@ public function testImmutability(): void
         $new2 = $original->withDescription('new description');
         $new3 = $original->withUpdatedAt(1_642_029_084);
         $new4 = $original->withCreatedAt(1_642_029_084);
-        $new5 = $original->withRuleName('test');
+        $new5 = $original->withRuleName(TrueRule::class);
 
         $this->assertNotSame($original, $new1);
         $this->assertNotSame($original, $new2);

diff --git a/tests/RoleTest.php b/tests/RoleTest.php
@@ -7,6 +7,7 @@
 use PHPUnit\Framework\TestCase;
 use Yiisoft\Rbac\Item;
 use Yiisoft\Rbac\Role;
+use Yiisoft\Rbac\Tests\Support\TrueRule;
 
 final class RoleTest extends TestCase
 {
@@ -17,8 +18,7 @@ public function testImmutability(): void
         $new2 = $original->withDescription('new description');
         $new3 = $original->withUpdatedAt(1_642_029_084);
         $new4 = $original->withCreatedAt(1_642_029_084);
-        $new5 = $original->withRuleName('test');
-
+        $new5 = $original->withRuleName(TrueRule::class);
 
         $this->assertNotSame($original, $new1);
         $this->assertNotSame($original, $new2);