From ebbd97f849e9457152f8aa1bb1446f25f693e63a Mon Sep 17 00:00:00 2001 From: "Eric Richer eric.richer@vistoconsulting.com" Date: Fri, 16 Aug 2024 17:18:54 -0400 Subject: [PATCH] Beautify code --- src/Collector/RbacCollector.php | 17 +- src/ConfigProvider.php | 19 +- src/Module.php | 7 +- test/Asset/MockRoleWithPermissionMethod.php | 10 +- test/Asset/MockRoleWithPermissionProperty.php | 20 ++- .../MockRoleWithPermissionTraversable.php | 24 +-- test/Bootstrap.php | 7 +- test/Collector/RbacCollectorTest.php | 170 +++++++++--------- test/ConfigProviderTest.php | 24 ++- test/ModuleTest.php | 8 +- test/TestConfiguration.php | 10 +- test/Util/ServiceManagerFactory.php | 49 +++-- 12 files changed, 199 insertions(+), 166 deletions(-) diff --git a/src/Collector/RbacCollector.php b/src/Collector/RbacCollector.php index bdee314..6a08d2e 100644 --- a/src/Collector/RbacCollector.php +++ b/src/Collector/RbacCollector.php @@ -97,7 +97,10 @@ public function collect(MvcEvent $mvcEvent): void */ private function collectOptions(ModuleOptions $moduleOptions): void { - $this->collectedOptions = ['guest_role' => $moduleOptions->getGuestRole(), 'protection_policy' => $moduleOptions->getProtectionPolicy()]; + $this->collectedOptions = [ + 'guest_role' => $moduleOptions->getGuestRole(), + 'protection_policy' => $moduleOptions->getProtectionPolicy(), + ]; } /** @@ -126,7 +129,10 @@ private function collectIdentityRolesAndPermissions(RoleService $roleService): v if (empty($role->getChildren())) { $this->collectedRoles[] = $roleName; } else { - $iteratorIterator = new RecursiveIteratorIterator(new RecursiveRoleIterator($role->getChildren()), RecursiveIteratorIterator::SELF_FIRST); + $iteratorIterator = new RecursiveIteratorIterator( + new RecursiveRoleIterator($role->getChildren()), + RecursiveIteratorIterator::SELF_FIRST + ); foreach ($iteratorIterator as $childRole) { $this->collectedRoles[$roleName][] = $childRole->getName(); $this->collectPermissions($childRole); @@ -147,8 +153,8 @@ private function collectPermissions(RoleInterface $role): void if (method_exists($role, 'getPermissions')) { $permissions = $role->getPermissions(); } else { - $reflectionProperty = new ReflectionProperty($role, 'permissions'); - $permissions = $reflectionProperty->getValue($role); + $reflectionProperty = new ReflectionProperty($role, 'permissions'); + $permissions = $reflectionProperty->getValue($role); } if ($permissions instanceof Traversable) { @@ -166,13 +172,14 @@ private function collectPermissions(RoleInterface $role): void */ public function getCollection(): array { + // Start collect all the data we need! return [ 'guards' => $this->collectedGuards, 'roles' => $this->collectedRoles, 'permissions' => $this->collectedPermissions, 'options' => $this->collectedOptions, ]; - } // Start collect all the data we need! + } // Gather the permissions for the given role. We have to use reflection as // the RoleInterface does not have "getPermissions" method diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php index aef7c12..0618332 100644 --- a/src/ConfigProvider.php +++ b/src/ConfigProvider.php @@ -1,14 +1,19 @@ $this->getDependencies(), - 'view_manager' => $this->getViewManagerConfig(), + 'dependencies' => $this->getDependencies(), + 'view_manager' => $this->getViewManagerConfig(), 'laminas-developer-tools' => $this->getLaminasDeveloperToolsConfig(), ]; } @@ -17,7 +22,7 @@ public function getDependencies(): array { return [ 'factories' => [ - \LmcRbac\Mvc\DevTools\Collector\RbacCollector::class => \Laminas\ServiceManager\Factory\InvokableFactory::class, + RbacCollector::class => InvokableFactory::class, ], ]; } @@ -26,8 +31,8 @@ public function getViewManagerConfig(): array { return [ 'template_map' => [ - 'laminas-developer-tools/toolbar/lmc-rbac' => __DIR__ . '/../view/laminas-developer-tools/toolbar/lmc-rbac.phtml' - ] + 'laminas-developer-tools/toolbar/lmc-rbac' => __DIR__ . '/../view/laminas-developer-tools/toolbar/lmc-rbac.phtml', + ], ]; } @@ -36,10 +41,10 @@ public function getLaminasDeveloperToolsConfig(): array return [ 'profiler' => [ 'collectors' => [ - 'lmc_rbac' => \LmcRbac\Mvc\DevTools\Collector\RbacCollector::class, + 'lmc_rbac' => RbacCollector::class, ], ], - 'toolbar' => [ + 'toolbar' => [ 'entries' => [ 'lmc_rbac' => 'laminas-developer-tools/toolbar/lmc-rbac', ], diff --git a/src/Module.php b/src/Module.php index 5fc76a5..7bf4743 100644 --- a/src/Module.php +++ b/src/Module.php @@ -1,16 +1,17 @@ $configProvider->getDependencies(), - 'view_manager' => $configProvider->getViewManagerConfig(), + 'service_manager' => $configProvider->getDependencies(), + 'view_manager' => $configProvider->getViewManagerConfig(), 'laminas-developer-tools' => $configProvider->getLaminasDeveloperToolsConfig(), ]; } diff --git a/test/Asset/MockRoleWithPermissionMethod.php b/test/Asset/MockRoleWithPermissionMethod.php index cc2abc8..b0426b6 100644 --- a/test/Asset/MockRoleWithPermissionMethod.php +++ b/test/Asset/MockRoleWithPermissionMethod.php @@ -1,7 +1,10 @@ add('LmcRbac\\Mvc\\DevToolsTest\\', __DIR__); - $config = require __DIR__ . '/TestConfiguration.php'; ServiceManagerFactory::setApplicationConfig($config); -unset($files, $file, $loader, $configFiles, $configFile, $config); +unset($files, $file, $loader, $config); diff --git a/test/Collector/RbacCollectorTest.php b/test/Collector/RbacCollectorTest.php index d751a8f..a0629fc 100644 --- a/test/Collector/RbacCollectorTest.php +++ b/test/Collector/RbacCollectorTest.php @@ -1,4 +1,5 @@ assertSame(-100, $collector->getPriority()); $this->assertSame('lmc_rbac', $collector->getName()); } - public function testSerialize() + public function testSerialize(): void { $collector = new RbacCollector(); $serialized = $collector->serialize(); - $this->assertIsString($serialized); - $unserialized = unserialize($serialized); - $this->assertSame([], $unserialized['guards']); $this->assertSame([], $unserialized['roles']); $this->assertSame([], $unserialized['options']); } - public function testUnserialize() + public function testUnserialize(): vois { $collector = new RbacCollector(); $unserialized = [ - 'guards' => ['foo' => 'bar'], - 'roles' => ['foo' => 'bar'], - 'permissions' => ['foo' => 'bar'], - 'options' => ['foo' => 'bar'] + 'guards' => [ + 'foo' => 'bar', + ], + 'roles' => [ + 'foo' => 'bar', + ], + 'permissions' => [ + 'foo' => 'bar', + ], + 'options' => [ + 'foo' => 'bar', + ], ]; $serialized = serialize($unserialized); - $collector->unserialize($serialized); - $collection = $collector->getCollection(); - $this->assertIsArray($collection); $this->assertSame(['foo' => 'bar'], $collection['guards']); $this->assertSame(['foo' => 'bar'], $collection['roles']); @@ -82,58 +95,54 @@ public function testUnserialize() $this->assertSame(['foo' => 'bar'], $collection['permissions']); } - public function testUnserializeThrowsInvalidArgumentException() + public function testUnserializeThrowsInvalidArgumentException(): void { $this->expectException('InvalidArgumentException'); $collector = new RbacCollector(); $unserialized = 'not_an_array'; $serialized = serialize($unserialized); - $collector->unserialize($serialized); } - - public function testCollectNothingIfNoApplicationIsSet() + public function testCollectNothingIfNoApplicationIsSet(): void { $mvcEvent = new MvcEvent(); $collector = new RbacCollector(); - $this->assertNull($collector->collect($mvcEvent)); } - public function testCanCollect() + public function testCanCollect(): void { $dataToCollect = [ 'module_options' => [ 'guest_role' => 'guest', 'protection_policy' => GuardInterface::POLICY_ALLOW, 'guards' => [ - 'Lmc\Rbac\Mvc\Guard\RouteGuard' => [ - 'admin*' => ['*'] + RouteGuard::class => [ + 'admin*' => ['*'], ], - 'Lmc\Rbac\Mvc\Guard\ControllerGuard' => [ + ControllerGuard::class => [ [ 'controller' => 'Foo', - 'roles' => ['*'] - ] - ] - ] + 'roles' => ['*'], + ], + ], + ], ], - 'role_config' => [ + 'role_config' => [ 'member' => [ 'children' => ['guest'], - 'permissions' => ['write', 'delete'] + 'permissions' => ['write', 'delete'], + ], + 'guest' => [ + 'permissions' => ['read'], ], - 'guest' => [ - 'permissions' => ['read'] - ] ], - 'identity_role' => ['member'] + 'identity_role' => ['member'], ]; - //$serviceManager = $this->getMockBuilder('Laminas\ServiceManager\ServiceLocatorInterface')->getMock(); $serviceManager = new ServiceManager(); - $application = $this->getMockBuilder('Laminas\Mvc\Application') + $application = $this->getMockBuilder(Application::class) ->disableOriginalConstructor() ->getMock(); @@ -145,14 +154,14 @@ public function testCanCollect() $identity = $this->createMock(IdentityInterface::class); $identity->expects($this->once())->method('getRoles')->willReturn($dataToCollect['identity_role']); - $identityProvider = $this->createMock(\Lmc\Rbac\Mvc\Identity\IdentityProviderInterface::class); + $identityProvider = $this->createMock(IdentityProviderInterface::class); $identityProvider->expects($this->once())->method('getIdentity')->willReturn($identity); $baseRoleService = new BaseRoleService(new InMemoryRoleProvider($dataToCollect['role_config']), 'guest'); - $roleService = new RoleService($identityProvider, $baseRoleService, new RecursiveRoleIteratorStrategy()); + $roleService = new RoleService($identityProvider, $baseRoleService, new RecursiveRoleIteratorStrategy()); - $serviceManager->setService('Lmc\Rbac\Mvc\Service\RoleService', $roleService); - $serviceManager->setService('Lmc\Rbac\Mvc\Options\ModuleOptions', new ModuleOptions($dataToCollect['module_options'])); + $serviceManager->setService(RoleService::class, $roleService); + $serviceManager->setService(ModuleOptions::class, new ModuleOptions($dataToCollect['module_options'])); $collector = new RbacCollector(); $collector->collect($mvcEvent); @@ -160,27 +169,27 @@ public function testCanCollect() $collection = $collector->getCollection(); $expectedCollection = [ - 'guards' => [ - 'Lmc\Rbac\Mvc\Guard\RouteGuard' => [ - 'admin*' => ['*'] + 'guards' => [ + RouteGuard::class => [ + 'admin*' => ['*'], ], - 'Lmc\Rbac\Mvc\Guard\ControllerGuard' => [ + ControllerGuard::class => [ [ 'controller' => 'Foo', - 'roles' => ['*'] - ] - ] + 'roles' => ['*'], + ], + ], ], - 'roles' => [ - 'member' => ['guest'] + 'roles' => [ + 'member' => ['guest'], ], 'permissions' => [ 'member' => ['write', 'delete'], - 'guest' => ['read'] + 'guest' => ['read'], ], - 'options' => [ + 'options' => [ 'guest_role' => 'guest', - 'protection_policy' => 'allow' + 'protection_policy' => 'allow', ], ]; @@ -190,16 +199,16 @@ public function testCanCollect() /** * Tests the collectPermissions method when the role has a $permissions Property */ - public function testCollectPermissionsProperty() + public function testCollectPermissionsProperty(): void { $expectedCollection = [ - 'guards' => [], - 'roles' => ['role-with-permission-property'], + 'guards' => [], + 'roles' => ['role-with-permission-property'], 'permissions' => [ 'role-with-permission-property' => ['permission-property-a', 'permission-property-b'], ], - 'options' => [ - 'guest_role' => 'guest', + 'options' => [ + 'guest_role' => 'guest', 'protection_policy' => GuardInterface::POLICY_ALLOW, ], ]; @@ -211,16 +220,16 @@ public function testCollectPermissionsProperty() /** * Tests the collectPermissions method when the role has a getPermissions() method */ - public function testCollectPermissionsMethod() + public function testCollectPermissionsMethod(): void { $expectedCollection = [ - 'guards' => [], - 'roles' => ['role-with-permission-method'], + 'guards' => [], + 'roles' => ['role-with-permission-method'], 'permissions' => [ 'role-with-permission-method' => ['permission-method-a', 'permission-method-b'], ], - 'options' => [ - 'guest_role' => 'guest', + 'options' => [ + 'guest_role' => 'guest', 'protection_policy' => GuardInterface::POLICY_ALLOW, ], ]; @@ -232,16 +241,16 @@ public function testCollectPermissionsMethod() /** * Tests the collectPermissions method when the role implements Traversable */ - public function testCollectPermissionsTraversable() + public function testCollectPermissionsTraversable(): void { $expectedCollection = [ - 'guards' => [], - 'roles' => ['role-with-permission-traversable'], + 'guards' => [], + 'roles' => ['role-with-permission-traversable'], 'permissions' => [ 'role-with-permission-traversable' => ['permission-method-a', 'permission-method-b'], ], - 'options' => [ - 'guest_role' => 'guest', + 'options' => [ + 'guest_role' => 'guest', 'protection_policy' => GuardInterface::POLICY_ALLOW, ], ]; @@ -250,17 +259,16 @@ public function testCollectPermissionsTraversable() $this->assertEquals($expectedCollection, $collection); } - /** * Base method for the *collectPermissionProperty tests - * @param RoleInterface $role + * * @return array|string[] */ private function collectPermissionsPropertyTestBase(RoleInterface $role): array { $serviceManager = new ServiceManager(); - $application = $this->getMockBuilder(\Laminas\Mvc\Application::class) + $application = $this->getMockBuilder(Application::class) ->disableOriginalConstructor() ->getMock(); $application->expects($this->once())->method('getServiceManager')->willReturn($serviceManager); @@ -268,17 +276,17 @@ private function collectPermissionsPropertyTestBase(RoleInterface $role): array $mvcEvent = new MvcEvent(); $mvcEvent->setApplication($application); - $identity = $this->createMock(\Lmc\Rbac\Identity\IdentityInterface::class); + $identity = $this->createMock(IdentityInterface::class); $identity->expects($this->once()) ->method('getRoles') ->willReturn([$role]); - $identityProvider = $this->createMock(\Lmc\Rbac\Mvc\Identity\IdentityProviderInterface::class); + $identityProvider = $this->createMock(IdentityProviderInterface::class); $identityProvider->expects($this->once()) ->method('getIdentity') ->will($this->returnValue($identity)); - $roleProvider = $this->createMock(\Lmc\Rbac\Role\RoleProviderInterface::class); + $roleProvider = $this->createMock(RoleProviderInterface::class); $baseRoleService = new BaseRoleService($roleProvider, ''); @@ -287,8 +295,8 @@ private function collectPermissionsPropertyTestBase(RoleInterface $role): array $baseRoleService, new RecursiveRoleIteratorStrategy() ); - $serviceManager->setService('Lmc\Rbac\Mvc\Service\RoleService', $roleService); - $serviceManager->setService('Lmc\Rbac\Mvc\Options\ModuleOptions', new ModuleOptions()); + $serviceManager->setService(RoleService::class, $roleService); + $serviceManager->setService(ModuleOptions::class, new ModuleOptions()); $collector = new RbacCollector(); $collector->collect($mvcEvent); diff --git a/test/ConfigProviderTest.php b/test/ConfigProviderTest.php index fbc17b5..040616c 100644 --- a/test/ConfigProviderTest.php +++ b/test/ConfigProviderTest.php @@ -1,10 +1,16 @@ [ - \LmcRbac\Mvc\DevTools\Collector\RbacCollector::class => \Laminas\ServiceManager\Factory\InvokableFactory::class, + RbacCollector::class => InvokableFactory::class, ], ]; $expectedLaminasDevtoolsConfig = [ 'profiler' => [ 'collectors' => [ - 'lmc_rbac' => \LmcRbac\Mvc\DevTools\Collector\RbacCollector::class, + 'lmc_rbac' => RbacCollector::class, ], ], - 'toolbar' => [ + 'toolbar' => [ 'entries' => [ 'lmc_rbac' => 'laminas-developer-tools/toolbar/lmc-rbac', ], ], ]; - $expectedViewManagerConfig = [ + $expectedViewManagerConfig = [ 'template_map' => [ 'laminas-developer-tools/toolbar/lmc-rbac' => realpath(__DIR__ . '/../view/laminas-developer-tools/toolbar/lmc-rbac.phtml'), ], @@ -43,14 +49,14 @@ public function testProvidesExpectedConfig() 'laminas-developer-tools/toolbar/lmc-rbac' => realpath(__DIR__ . '/../view/laminas-developer-tools/toolbar/lmc-rbac.phtml'), ], ]; - $result = $provider->getViewManagerConfig(); + $result = $provider->getViewManagerConfig(); // substitute path $result['template_map']['laminas-developer-tools/toolbar/lmc-rbac'] = realpath($result['template_map']['laminas-developer-tools/toolbar/lmc-rbac']); $this->assertEquals($expectedViewManagerConfig, $result); $expectedConfig = [ - 'dependencies' => $expectedDependencyConfig, - 'view_manager' => $expectedViewManagerConfig, + 'dependencies' => $expectedDependencyConfig, + 'view_manager' => $expectedViewManagerConfig, 'laminas-developer-tools' => $expectedLaminasDevtoolsConfig, ]; diff --git a/test/ModuleTest.php b/test/ModuleTest.php index 0b13c1e..67cfd19 100644 --- a/test/ModuleTest.php +++ b/test/ModuleTest.php @@ -1,5 +1,7 @@ $provider->getDependencies(), - 'view_manager' => $provider->getViewManagerConfig(), + 'service_manager' => $provider->getDependencies(), + 'view_manager' => $provider->getViewManagerConfig(), 'laminas-developer-tools' => $provider->getLaminasDeveloperToolsConfig(), ]; $this->assertEquals($expected, $module->getConfig()); diff --git a/test/TestConfiguration.php b/test/TestConfiguration.php index 6cb5e6a..7d336fd 100644 --- a/test/TestConfiguration.php +++ b/test/TestConfiguration.php @@ -1,4 +1,7 @@ [ - 'LmcRbacMvc', + 'modules' => [ + 'Lmc\Rbac\Mvc', ], 'module_listener_options' => [ 'config_glob_paths' => [ __DIR__ . '/testing.config.php', ], - 'module_paths' => [ - ], + 'module_paths' => [], ], ]; diff --git a/test/Util/ServiceManagerFactory.php b/test/Util/ServiceManagerFactory.php index 17823f7..4a9b8df 100644 --- a/test/Util/ServiceManagerFactory.php +++ b/test/Util/ServiceManagerFactory.php @@ -1,20 +1,23 @@ */ abstract class ServiceManagerFactory { - /** - * @var array - */ + /** @var array */ private static array $config = []; /** @@ -55,20 +53,19 @@ public static function getApplicationConfig(): array /** * @param array|null $config - * @return ServiceManager */ - public static function getServiceManager(array $config = null): ServiceManager + public static function getServiceManager(?array $config = null): ServiceManager { - $config = $config ?: static::getApplicationConfig(); + $config = $config ?: static::getApplicationConfig(); $serviceManagerConfig = new ServiceManagerConfig( $config['service_manager'] ?? [] ); - $serviceManager = new ServiceManager(); + $serviceManager = new ServiceManager(); $serviceManagerConfig->configureServiceManager($serviceManager); $serviceManager->setService('ApplicationConfig', $config); $serviceManager->setAllowOverride(true); - /* @var $moduleManager ModuleManagerInterface */ + /** @var ModuleManagerInterface $moduleManager */ $moduleManager = $serviceManager->get('ModuleManager'); $moduleManager->loadModules();