From cba656383eb3af306fcd62752620c241a76960f1 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 21 Jul 2023 22:41:36 +0200 Subject: [PATCH] Adhere feedback - rename hooks.config.php to strategies.config.php - change all corresponding classes and tests --- doc/StrategyHooks.md | 2 +- .../Capabilities/ICanCreateInstances.php | 4 +- ...stances.php => ICanRegisterStrategies.php} | 4 +- src/Core/Hooks/Model/DiceInstanceManager.php | 20 ++--- ...eManager.php => StrategiesFileManager.php} | 46 ++++------- src/Core/Logger/Factory/Logger.php | 4 +- src/DI.php | 2 +- src/Module/Admin/Summary.php | 4 +- static/dependencies.config.php | 9 ++- ...hooks.config.php => strategies.config.php} | 10 +-- .../Core/Hooks/Model/InstanceManagerTest.php | 8 +- ...Test.php => StrategiesFileManagerTest.php} | 80 +++++-------------- 12 files changed, 72 insertions(+), 121 deletions(-) rename src/Core/Hooks/Capabilities/{ICanRegisterInstances.php => ICanRegisterStrategies.php} (93%) rename src/Core/Hooks/Util/{HookFileManager.php => StrategiesFileManager.php} (62%) rename static/{hooks.config.php => strategies.config.php} (83%) rename tests/src/Core/Hooks/Util/{HookFileManagerTest.php => StrategiesFileManagerTest.php} (64%) diff --git a/doc/StrategyHooks.md b/doc/StrategyHooks.md index cb355f93f0..39fc1bd8df 100644 --- a/doc/StrategyHooks.md +++ b/doc/StrategyHooks.md @@ -43,7 +43,7 @@ public class ConcreteClassB implements ExampleInterface } } -/** @var \Friendica\Core\Hooks\Capabilities\ICanRegisterInstances $instanceRegister */ +/** @var \Friendica\Core\Hooks\Capabilities\ICanRegisterStrategies $instanceRegister */ $instanceRegister->registerStrategy(ExampleInterface::class, ConcreteClassA::class, 'A'); $instanceRegister->registerStrategy(ExampleInterface::class, ConcreteClassB::class, 'B'); diff --git a/src/Core/Hooks/Capabilities/ICanCreateInstances.php b/src/Core/Hooks/Capabilities/ICanCreateInstances.php index f2e4b8b0a1..77d4c4b366 100644 --- a/src/Core/Hooks/Capabilities/ICanCreateInstances.php +++ b/src/Core/Hooks/Capabilities/ICanCreateInstances.php @@ -32,10 +32,10 @@ interface ICanCreateInstances * The instance will be build based on the registered strategy and the (unique) name * * @param string $class The fully-qualified name of the given class or interface which will get returned - * @param string $name An arbitrary identifier to find a concrete instance strategy. + * @param string $strategy An arbitrary identifier to find a concrete instance strategy. * @param array $arguments Additional arguments, which can be passed to the constructor of "$class" at runtime * * @return object The concrete instance of the type "$class" */ - public function create(string $class, string $name, array $arguments = []): object; + public function create(string $class, string $strategy, array $arguments = []): object; } diff --git a/src/Core/Hooks/Capabilities/ICanRegisterInstances.php b/src/Core/Hooks/Capabilities/ICanRegisterStrategies.php similarity index 93% rename from src/Core/Hooks/Capabilities/ICanRegisterInstances.php rename to src/Core/Hooks/Capabilities/ICanRegisterStrategies.php index f7689bbee7..911eb3499f 100644 --- a/src/Core/Hooks/Capabilities/ICanRegisterInstances.php +++ b/src/Core/Hooks/Capabilities/ICanRegisterStrategies.php @@ -26,7 +26,7 @@ use Friendica\Core\Hooks\Exceptions\HookRegisterArgumentException; /** * Register strategies for given classes */ -interface ICanRegisterInstances +interface ICanRegisterStrategies { /** * Register a class(strategy) for a given interface with a unique name. @@ -36,7 +36,7 @@ interface ICanRegisterInstances * @param string $interface The interface, which the given class implements * @param string $class The fully-qualified given class name * A placeholder for dependencies is possible as well - * @param ?string $name An arbitrary identifier for the given class, which will be used for factories, dependency injections etc. + * @param ?string $name An arbitrary identifier for the given strategy, which will be used for factories, dependency injections etc. * * @return $this This interface for chain-calls * diff --git a/src/Core/Hooks/Model/DiceInstanceManager.php b/src/Core/Hooks/Model/DiceInstanceManager.php index 77b9c4d495..a8b0b540ce 100644 --- a/src/Core/Hooks/Model/DiceInstanceManager.php +++ b/src/Core/Hooks/Model/DiceInstanceManager.php @@ -23,31 +23,31 @@ namespace Friendica\Core\Hooks\Model; use Dice\Dice; use Friendica\Core\Hooks\Capabilities\ICanCreateInstances; -use Friendica\Core\Hooks\Capabilities\ICanRegisterInstances; +use Friendica\Core\Hooks\Capabilities\ICanRegisterStrategies; use Friendica\Core\Hooks\Exceptions\HookInstanceException; use Friendica\Core\Hooks\Exceptions\HookRegisterArgumentException; -use Friendica\Core\Hooks\Util\HookFileManager; +use Friendica\Core\Hooks\Util\StrategiesFileManager; /** * This class represents an instance register, which uses Dice for creation * * @see Dice */ -class DiceInstanceManager implements ICanCreateInstances, ICanRegisterInstances +class DiceInstanceManager implements ICanCreateInstances, ICanRegisterStrategies { protected $instance = []; /** @var Dice */ protected $dice; - public function __construct(Dice $dice, HookFileManager $hookFileManager) + public function __construct(Dice $dice, StrategiesFileManager $strategiesFileManager) { $this->dice = $dice; - $hookFileManager->setupHooks($this); + $strategiesFileManager->setupStrategies($this); } /** {@inheritDoc} */ - public function registerStrategy(string $interface, string $class, ?string $name = null): ICanRegisterInstances + public function registerStrategy(string $interface, string $class, ?string $name = null): ICanRegisterStrategies { if (!empty($this->instance[$interface][$name])) { throw new HookRegisterArgumentException(sprintf('A class with the name %s is already set for the interface %s', $name, $interface)); @@ -59,12 +59,12 @@ class DiceInstanceManager implements ICanCreateInstances, ICanRegisterInstances } /** {@inheritDoc} */ - public function create(string $class, string $name, array $arguments = []): object + public function create(string $class, string $strategy, array $arguments = []): object { - if (empty($this->instance[$class][$name])) { - throw new HookInstanceException(sprintf('The class with the name %s isn\'t registered for the class or interface %s', $name, $class)); + if (empty($this->instance[$class][$strategy])) { + throw new HookInstanceException(sprintf('The class with the name %s isn\'t registered for the class or interface %s', $strategy, $class)); } - return $this->dice->create($this->instance[$class][$name], $arguments); + return $this->dice->create($this->instance[$class][$strategy], $arguments); } } diff --git a/src/Core/Hooks/Util/HookFileManager.php b/src/Core/Hooks/Util/StrategiesFileManager.php similarity index 62% rename from src/Core/Hooks/Util/HookFileManager.php rename to src/Core/Hooks/Util/StrategiesFileManager.php index b83f2b49c6..700c401f2b 100644 --- a/src/Core/Hooks/Util/HookFileManager.php +++ b/src/Core/Hooks/Util/StrategiesFileManager.php @@ -22,22 +22,21 @@ namespace Friendica\Core\Hooks\Util; use Friendica\Core\Addon\Capabilities\ICanLoadAddons; -use Friendica\Core\Hooks\Capabilities\BehavioralHookType; -use Friendica\Core\Hooks\Capabilities\ICanRegisterInstances; +use Friendica\Core\Hooks\Capabilities\ICanRegisterStrategies; use Friendica\Core\Hooks\Exceptions\HookConfigException; /** - * Manage all hooks.config.php files + * Manage all strategies.config.php files */ -class HookFileManager +class StrategiesFileManager { const STATIC_DIR = 'static'; - const CONFIG_NAME = 'hooks'; + const CONFIG_NAME = 'strategies'; /** @var ICanLoadAddons */ protected $addonLoader; /** @var array */ - protected $hookConfig = []; + protected $config = []; /** @var string */ protected $basePath; @@ -50,32 +49,21 @@ class HookFileManager /** * Loads all kinds of hooks and registers the corresponding instances * - * @param ICanRegisterInstances $instanceRegister The instance register + * @param ICanRegisterStrategies $instanceRegister The instance register * * @return void */ - public function setupHooks(ICanRegisterInstances $instanceRegister) + public function setupStrategies(ICanRegisterStrategies $instanceRegister) { - // In case it wasn't used before, reload the whole hook config - if (empty($this->hookConfig)) { - $this->reloadHookConfig(); - } - - foreach ($this->hookConfig as $hookType => $classList) { - switch ($hookType) { - case BehavioralHookType::STRATEGY: - foreach ($classList as $interface => $strategy) { - foreach ($strategy as $dependencyName => $names) { - if (is_array($names)) { - foreach ($names as $name) { - $instanceRegister->registerStrategy($interface, $dependencyName, $name); - } - } else { - $instanceRegister->registerStrategy($interface, $dependencyName, $names); - } - } + foreach ($this->config as $interface => $strategy) { + foreach ($strategy as $dependencyName => $names) { + if (is_array($names)) { + foreach ($names as $name) { + $instanceRegister->registerStrategy($interface, $dependencyName, $name); } - break; + } else { + $instanceRegister->registerStrategy($interface, $dependencyName, $names); + } } } } @@ -87,7 +75,7 @@ class HookFileManager * * @return void */ - protected function reloadHookConfig() + public function loadConfig() { // load core hook config $configFile = $this->basePath . '/' . static::STATIC_DIR . '/' . static::CONFIG_NAME . '.config.php'; @@ -102,6 +90,6 @@ class HookFileManager throw new HookConfigException(sprintf('Error loading config file %s.', $configFile)); } - $this->hookConfig = array_merge_recursive($config, $this->addonLoader->getActiveAddonConfig(static::CONFIG_NAME)); + $this->config = array_merge_recursive($config, $this->addonLoader->getActiveAddonConfig(static::CONFIG_NAME)); } } diff --git a/src/Core/Logger/Factory/Logger.php b/src/Core/Logger/Factory/Logger.php index 81f8887efc..c370cc4dd7 100644 --- a/src/Core/Logger/Factory/Logger.php +++ b/src/Core/Logger/Factory/Logger.php @@ -43,7 +43,7 @@ class Logger $this->channel = $channel; } - public function create(ICanCreateInstances $createInstances, IManageConfigValues $config, Profiler $profiler): LoggerInterface + public function create(ICanCreateInstances $instanceCreator, IManageConfigValues $config, Profiler $profiler): LoggerInterface { if (empty($config->get('system', 'debugging') ?? false)) { return new NullLogger(); @@ -53,7 +53,7 @@ class Logger try { /** @var LoggerInterface $logger */ - $logger = $createInstances->create(LoggerInterface::class, $name, [$this->channel]); + $logger = $instanceCreator->create(LoggerInterface::class, $name, [$this->channel]); if ($config->get('system', 'profiling') ?? false) { return new ProfilerLoggerClass($logger, $profiler); } else { diff --git a/src/DI.php b/src/DI.php index c62b5e8d79..1917f710d3 100644 --- a/src/DI.php +++ b/src/DI.php @@ -297,7 +297,7 @@ abstract class DI static::init($flushDice); } - public static function loggCheck(): ICheckLoggerSettings + public static function logCheck(): ICheckLoggerSettings { return self::$dice->create(LoggerSettingsCheck::class); } diff --git a/src/Module/Admin/Summary.php b/src/Module/Admin/Summary.php index 80437305dc..99d0e3325b 100644 --- a/src/Module/Admin/Summary.php +++ b/src/Module/Admin/Summary.php @@ -126,10 +126,10 @@ class Summary extends BaseAdmin } // Check logfile permission - if (($return = DI::loggCheck()->checkLogfile()) !== null) { + if (($return = DI::logCheck()->checkLogfile()) !== null) { $warningtext[] = $return; } - if (($return = DI::loggCheck()->checkDebugLogfile()) !== null) { + if (($return = DI::logCheck()->checkDebugLogfile()) !== null) { $warningtext[] = $return; } diff --git a/static/dependencies.config.php b/static/dependencies.config.php index ce8a5bd9df..98a9f3077d 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -38,7 +38,7 @@ use Friendica\App; use Friendica\Core\Cache; use Friendica\Core\Config; use Friendica\Core\Hooks\Capabilities\ICanCreateInstances; -use Friendica\Core\Hooks\Capabilities\ICanRegisterInstances; +use Friendica\Core\Hooks\Capabilities\ICanRegisterStrategies; use Friendica\Core\Hooks\Model\DiceInstanceManager; use Friendica\Core\PConfig; use Friendica\Core\L10n; @@ -91,12 +91,15 @@ return [ [Dice::INSTANCE => Dice::SELF], ] ], - \Friendica\Core\Hooks\Util\HookFileManager::class => [ + \Friendica\Core\Hooks\Util\StrategiesFileManager::class => [ 'constructParams' => [ [Dice::INSTANCE => '$basepath'], ], + 'call' => [ + ['loadConfig'], + ], ], - ICanRegisterInstances::class => [ + ICanRegisterStrategies::class => [ 'instanceOf' => DiceInstanceManager::class, 'constructParams' => [ [Dice::INSTANCE => Dice::SELF], diff --git a/static/hooks.config.php b/static/strategies.config.php similarity index 83% rename from static/hooks.config.php rename to static/strategies.config.php index d46762f871..bb3598c369 100644 --- a/static/hooks.config.php +++ b/static/strategies.config.php @@ -24,11 +24,9 @@ use Friendica\Core\Logger\Type; use Psr\Log; return [ - H::STRATEGY => [ - Log\LoggerInterface::class => [ - Log\NullLogger::class => [''], - Type\SyslogLogger::class => ['syslog'], - Type\StreamLogger::class => ['stream'], - ], + Log\LoggerInterface::class => [ + Log\NullLogger::class => [''], + Type\SyslogLogger::class => ['syslog'], + Type\StreamLogger::class => ['stream'], ], ]; diff --git a/tests/src/Core/Hooks/Model/InstanceManagerTest.php b/tests/src/Core/Hooks/Model/InstanceManagerTest.php index de2bdc6b83..44459eb2ee 100644 --- a/tests/src/Core/Hooks/Model/InstanceManagerTest.php +++ b/tests/src/Core/Hooks/Model/InstanceManagerTest.php @@ -25,7 +25,7 @@ use Dice\Dice; use Friendica\Core\Hooks\Exceptions\HookInstanceException; use Friendica\Core\Hooks\Exceptions\HookRegisterArgumentException; use Friendica\Core\Hooks\Model\DiceInstanceManager; -use Friendica\Core\Hooks\Util\HookFileManager; +use Friendica\Core\Hooks\Util\StrategiesFileManager; use Friendica\Test\MockedTest; use Friendica\Test\Util\Hooks\InstanceMocks\FakeInstance; use Friendica\Test\Util\Hooks\InstanceMocks\FakeInstanceDecorator; @@ -34,15 +34,15 @@ use Mockery\MockInterface; class InstanceManagerTest extends MockedTest { - /** @var HookFileManager|MockInterface */ + /** @var StrategiesFileManager|MockInterface */ protected $hookFileManager; protected function setUp(): void { parent::setUp(); - $this->hookFileManager = \Mockery::mock(HookFileManager::class); - $this->hookFileManager->shouldReceive('setupHooks')->withAnyArgs(); + $this->hookFileManager = \Mockery::mock(StrategiesFileManager::class); + $this->hookFileManager->shouldReceive('setupStrategies')->withAnyArgs(); } protected function tearDown(): void diff --git a/tests/src/Core/Hooks/Util/HookFileManagerTest.php b/tests/src/Core/Hooks/Util/StrategiesFileManagerTest.php similarity index 64% rename from tests/src/Core/Hooks/Util/HookFileManagerTest.php rename to tests/src/Core/Hooks/Util/StrategiesFileManagerTest.php index f718e8a92f..9e1855da14 100644 --- a/tests/src/Core/Hooks/Util/HookFileManagerTest.php +++ b/tests/src/Core/Hooks/Util/StrategiesFileManagerTest.php @@ -22,16 +22,16 @@ namespace Friendica\Test\src\Core\Hooks\Util; use Friendica\Core\Addon\Capabilities\ICanLoadAddons; -use Friendica\Core\Hooks\Capabilities\ICanRegisterInstances; +use Friendica\Core\Hooks\Capabilities\ICanRegisterStrategies; use Friendica\Core\Hooks\Exceptions\HookConfigException; -use Friendica\Core\Hooks\Util\HookFileManager; +use Friendica\Core\Hooks\Util\StrategiesFileManager; use Friendica\Test\MockedTest; use Friendica\Test\Util\VFSTrait; use org\bovigo\vfs\vfsStream; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; -class HookFileManagerTest extends MockedTest +class StrategiesFileManagerTest extends MockedTest { use VFSTrait; @@ -50,11 +50,9 @@ class HookFileManagerTest extends MockedTest [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => [''], ], - ], ]; EOF, 'addonsArray' => [], @@ -67,11 +65,9 @@ EOF, [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => '', ], - ], ]; EOF, 'addonsArray' => [], @@ -84,19 +80,15 @@ EOF, [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => [''], ], - ], ]; EOF, 'addonsArray' => [ - \Friendica\Core\Hooks\Capabilities\BehavioralHookType::STRATEGY => [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => ['null'], ], - ], ], 'assertStrategies' => [ [LoggerInterface::class, NullLogger::class, ''], @@ -108,19 +100,15 @@ EOF, [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => [''], ], - ], ]; EOF, 'addonsArray' => [ - \Friendica\Core\Hooks\Capabilities\BehavioralHookType::STRATEGY => [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => 'null', ], - ], ], 'assertStrategies' => [ [LoggerInterface::class, NullLogger::class, ''], @@ -133,48 +121,21 @@ EOF, [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => [''], ], - ], ]; EOF, 'addonsArray' => [ - \Friendica\Core\Hooks\Capabilities\BehavioralHookType::STRATEGY => [ \Psr\Log\LoggerInterface::class => [ \Psr\Log\NullLogger::class => [''], ], - ], ], 'assertStrategies' => [ [LoggerInterface::class, NullLogger::class, ''], [LoggerInterface::class, NullLogger::class, ''], ], ], - 'withWrongContentButAddons' => [ - 'content' => << [ - \Psr\Log\LoggerInterface::class => [ - \Psr\Log\NullLogger::class => [''], - ], - ], -]; -EOF, - 'addonsArray' => [ - \Friendica\Core\Hooks\Capabilities\BehavioralHookType::STRATEGY => [ - \Psr\Log\LoggerInterface::class => [ - \Psr\Log\NullLogger::class => [''], - ], - ], - ], - 'assertStrategies' => [ - [LoggerInterface::class, NullLogger::class, ''], - ], - ], ]; } @@ -183,58 +144,59 @@ EOF, */ public function testSetupHooks(string $content, array $addonsArray, array $assertStrategies) { - vfsStream::newFile('static/hooks.config.php') + vfsStream::newFile(StrategiesFileManager::STATIC_DIR . '/' . StrategiesFileManager::CONFIG_NAME . '.config.php') ->withContent($content) ->at($this->root); $addonLoader = \Mockery::mock(ICanLoadAddons::class); $addonLoader->shouldReceive('getActiveAddonConfig')->andReturn($addonsArray)->once(); - $hookFileManager = new HookFileManager($this->root->url(), $addonLoader); + $hookFileManager = new StrategiesFileManager($this->root->url(), $addonLoader); - $instanceManager = \Mockery::mock(ICanRegisterInstances::class); + $instanceManager = \Mockery::mock(ICanRegisterStrategies::class); foreach ($assertStrategies as $assertStrategy) { $instanceManager->shouldReceive('registerStrategy')->withArgs($assertStrategy)->once(); } - $hookFileManager->setupHooks($instanceManager); + $hookFileManager->loadConfig(); + $hookFileManager->setupStrategies($instanceManager); self::expectNotToPerformAssertions(); } /** - * Test the exception in case the hooks.config.php file is missing + * Test the exception in case the strategies.config.php file is missing */ - public function testMissingHooksFile() + public function testMissingStrategiesFile() { $addonLoader = \Mockery::mock(ICanLoadAddons::class); - $instanceManager = \Mockery::mock(ICanRegisterInstances::class); - $hookFileManager = new HookFileManager($this->root->url(), $addonLoader); + $instanceManager = \Mockery::mock(ICanRegisterStrategies::class); + $hookFileManager = new StrategiesFileManager($this->root->url(), $addonLoader); self::expectException(HookConfigException::class); self::expectExceptionMessage(sprintf('config file %s does not exist.', - $this->root->url() . '/' . HookFileManager::STATIC_DIR . '/' . HookFileManager::CONFIG_NAME . '.config.php')); + $this->root->url() . '/' . StrategiesFileManager::STATIC_DIR . '/' . StrategiesFileManager::CONFIG_NAME . '.config.php')); - $hookFileManager->setupHooks($instanceManager); + $hookFileManager->loadConfig(); } /** - * Test the exception in case the hooks.config.php file is wrong + * Test the exception in case the strategies.config.php file is wrong */ - public function testWrongHooksFile() + public function testWrongStrategiesFile() { $addonLoader = \Mockery::mock(ICanLoadAddons::class); - $instanceManager = \Mockery::mock(ICanRegisterInstances::class); - $hookFileManager = new HookFileManager($this->root->url(), $addonLoader); + $instanceManager = \Mockery::mock(ICanRegisterStrategies::class); + $hookFileManager = new StrategiesFileManager($this->root->url(), $addonLoader); - vfsStream::newFile('static/hooks.config.php') + vfsStream::newFile(StrategiesFileManager::STATIC_DIR . '/' . StrategiesFileManager::CONFIG_NAME . '.config.php') ->withContent("at($this->root); self::expectException(HookConfigException::class); self::expectExceptionMessage(sprintf('Error loading config file %s.', - $this->root->url() . '/' . HookFileManager::STATIC_DIR . '/' . HookFileManager::CONFIG_NAME . '.config.php')); + $this->root->url() . '/' . StrategiesFileManager::STATIC_DIR . '/' . StrategiesFileManager::CONFIG_NAME . '.config.php')); - $hookFileManager->setupHooks($instanceManager); + $hookFileManager->loadConfig(); } }