diff --git a/libraries/src/Extension/PluginInterface.php b/libraries/src/Extension/PluginInterface.php index 24387920e33d0..c80b2eda222e9 100644 --- a/libraries/src/Extension/PluginInterface.php +++ b/libraries/src/Extension/PluginInterface.php @@ -10,6 +10,7 @@ namespace Joomla\CMS\Extension; use Joomla\Event\DispatcherAwareInterface; +use Joomla\Event\DispatcherInterface; // phpcs:disable PSR1.Files.SideEffects \defined('_JEXEC') or die; @@ -20,20 +21,20 @@ * * @since 4.0.0 * - * @TODO Starting from 7.0 the class will no longer extend DispatcherAwareInterface + * @todo Starting from 7.0 the class will no longer extend DispatcherAwareInterface */ interface PluginInterface extends DispatcherAwareInterface { /** * Registers its listeners. * + * @param ?DispatcherInterface Dispatcher instance to register listeners with + * * @return void * * @since 4.0.0 * - * @deprecated 5.4.0 will be removed in 7.0 - * Plugin should implement SubscriberInterface. - * These plugins will be added to dispatcher in PluginHelper::import(). + * @todo In 7.0 $dispatcher argument will no longer be nullable */ - public function registerListeners(); + public function registerListeners(?DispatcherInterface $dispatcher = null); } diff --git a/libraries/src/Plugin/CMSPlugin.php b/libraries/src/Plugin/CMSPlugin.php index e489948acd55a..af7d8b881b6f1 100644 --- a/libraries/src/Plugin/CMSPlugin.php +++ b/libraries/src/Plugin/CMSPlugin.php @@ -18,7 +18,6 @@ use Joomla\CMS\Language\LanguageAwareInterface; use Joomla\CMS\Language\LanguageAwareTrait; use Joomla\Event\AbstractEvent; -use Joomla\Event\DispatcherAwareInterface; use Joomla\Event\DispatcherAwareTrait; use Joomla\Event\DispatcherInterface; use Joomla\Event\EventInterface; @@ -34,9 +33,9 @@ * * @since 1.5 * - * @TODO Starting from 7.0 the class will no longer implement DispatcherAwareInterface and LanguageAwareInterface + * @todo Starting from 7.0 the class will no longer implement DispatcherAwareInterface and LanguageAwareInterface */ -abstract class CMSPlugin implements DispatcherAwareInterface, PluginInterface, LanguageAwareInterface +abstract class CMSPlugin implements PluginInterface, LanguageAwareInterface { use DispatcherAwareTrait { setDispatcher as traitSetDispatcher; @@ -122,9 +121,9 @@ abstract class CMSPlugin implements DispatcherAwareInterface, PluginInterface, L /** * Constructor * - * @param array $config An optional associative array of configuration settings. - * Recognized key values include 'name', 'group', 'params', 'language' - * (this list is not meant to be comprehensive). + * @param array $config An optional associative array of configuration settings. + * Recognized key values include 'name', 'group', 'params', 'language' + * (this list is not meant to be comprehensive). * * @since 1.5 */ @@ -133,8 +132,7 @@ public function __construct($config = []) if ($config instanceof DispatcherInterface) { @trigger_error( \sprintf( - 'Passing an instance of %1$s to %2$s() will not be supported in 7.0. ' - . 'Starting from 7.0 CMSPlugin class will no longer implement DispatcherAwareInterface.', + 'Passing an instance of %1$s to %2$s() will not be supported in 7.0.', DispatcherInterface::class, __METHOD__ ), @@ -285,19 +283,37 @@ final protected function autoloadLanguage(): void * This method additionally supports Joomla\Event\SubscriberInterface and plugins implementing this will be * registered to the dispatcher as a subscriber. * + * @param ?DispatcherInterface $dispatcher Dispatcher instance to register listeners with + * * @return void * * @since 4.0.0 * - * @deprecated 5.4.0 will be removed in 7.0 - * Plugin should implement SubscriberInterface. - * These plugins will be added to dispatcher in PluginHelper::import(). + * @todo In 7.0 $dispatcher argument will no longer be nullable. */ - public function registerListeners() + public function registerListeners(?DispatcherInterface $dispatcher = null) { + // Make sure we have a dispatcher. + if ($dispatcher === null) { + @trigger_error( + \sprintf( + 'Passing an instance of %1$s to %2$s() will be required in 7.0.', + DispatcherInterface::class, + __METHOD__ + ), + \E_USER_DEPRECATED + ); + + try { + $dispatcher = $this->getDispatcher(); + } catch (\UnexpectedValueException) { + $dispatcher = Factory::getContainer()->get(DispatcherInterface::class); + } + } + // Plugins which are SubscriberInterface implementations are handled without legacy layer support if ($this instanceof SubscriberInterface) { - $this->getDispatcher()->addSubscriber($this); + $dispatcher->addSubscriber($this); return; } @@ -315,7 +331,7 @@ public function registerListeners() // Save time if I'm not to detect legacy listeners if (!$this->allowLegacyListeners) { - $this->registerListener($method->name); + $this->registerListener($method->name, $dispatcher); continue; } @@ -325,7 +341,7 @@ public function registerListeners() // If the parameter count is not 1 it is by definition a legacy listener if (\count($parameters) !== 1) { - $this->registerLegacyListener($method->name); + $this->registerLegacyListener($method->name, $dispatcher); continue; } @@ -336,13 +352,13 @@ public function registerListeners() // No type hint / type hint class not an event or parameter name is not "event"? It's a legacy listener. if ($paramName !== 'event' || !$this->parameterImplementsEventInterface($param)) { - $this->registerLegacyListener($method->name); + $this->registerLegacyListener($method->name, $dispatcher); continue; } // Everything checks out, this is a proper listener. - $this->registerListener($method->name); + $this->registerListener($method->name, $dispatcher); } } @@ -354,18 +370,27 @@ public function registerListeners() * into old style method arguments and call your on method with them. The result will be passed back to * the Event, as an element into an array argument called 'result'. * - * @param string $methodName The method name to register + * @param string $methodName The method name to register + * @param ?DispatcherInterface $dispatcher Dispatcher instance to register listeners with * * @return void * * @since 4.0.0 * - * @deprecated 5.4.0 will be removed in 7.0 - * Plugin should implement SubscriberInterface. + * @deprecated 6.0 Without replacement */ - final protected function registerLegacyListener(string $methodName) + final protected function registerLegacyListener(string $methodName, ?DispatcherInterface $dispatcher = null) { - $this->getDispatcher()->addListener( + // Make sure we have a dispatcher. + if ($dispatcher === null) { + try { + $dispatcher = $this->getDispatcher(); + } catch (\UnexpectedValueException) { + $dispatcher = Factory::getContainer()->get(DispatcherInterface::class); + } + } + + $dispatcher->addListener( $methodName, function (AbstractEvent $event) use ($methodName) { // Get the event arguments @@ -405,18 +430,27 @@ function (AbstractEvent $event) use ($methodName) { * Registers a proper event listener, i.e. a method which accepts an AbstractEvent as its sole argument. This is the * preferred way to implement plugins in Joomla! 4.x and will be the only possible method with Joomla! 5.x onwards. * - * @param string $methodName The method name to register + * @param string $methodName The method name to register + * @param ?DispatcherInterface $dispatcher Dispatcher instance to register listeners with * * @return void * * @since 4.0.0 * - * @deprecated 5.4.0 will be removed in 7.0 - * Plugin should implement SubscriberInterface. + * @deprecated 6.0 Use \Joomla\Event\DispatcherInterface::addListener() directly */ - final protected function registerListener(string $methodName) + final protected function registerListener(string $methodName, ?DispatcherInterface $dispatcher = null) { - $this->getDispatcher()->addListener($methodName, [$this, $methodName]); + // Make sure we have a dispatcher. + if ($dispatcher === null) { + try { + $dispatcher = $this->getDispatcher(); + } catch (\UnexpectedValueException) { + $dispatcher = Factory::getContainer()->get(DispatcherInterface::class); + } + } + + $dispatcher->addListener($methodName, [$this, $methodName]); } /** diff --git a/libraries/src/Plugin/PluginHelper.php b/libraries/src/Plugin/PluginHelper.php index 08ac384b26d83..f01eaa8fe0d35 100644 --- a/libraries/src/Plugin/PluginHelper.php +++ b/libraries/src/Plugin/PluginHelper.php @@ -151,13 +151,14 @@ public static function isEnabled($type, $plugin = null) * otherwise only the specific plugin is loaded. * * @param string $type The plugin type, relates to the subdirectory in the plugins directory. - * @param string $plugin The plugin name. - * @param boolean $autocreate Autocreate the plugin. - * @param ?DispatcherInterface $dispatcher Optionally allows the plugin to use a different dispatcher. + * @param ?string $plugin The plugin name to import a specific plugin or null to import all plugins in group. + * @param boolean $autocreate Whether to register listeners with the event dispatcher. + * @param ?DispatcherInterface $dispatcher The event dispatcher. In 7.0 this will be required. * * @return boolean True on success. * * @since 1.5 + * @todo Arguments will not be optional in 7.0. */ public static function importPlugin($type, $plugin = null, $autocreate = true, ?DispatcherInterface $dispatcher = null) { @@ -171,10 +172,21 @@ public static function importPlugin($type, $plugin = null, $autocreate = true, ? } // Ensure we have a dispatcher now so we can correctly track the loaded plugins - $dispatcher = $dispatcher ?: Factory::getApplication()->getDispatcher(); + if ($dispatcher === null) { + @trigger_error( + \sprintf('Passing an instance of %1$s to %2$s() will be required in 7.0', DispatcherInterface::class, __METHOD__), + \E_USER_DEPRECATED + ); + + try { + $dispatcher = Factory::getApplication()->getDispatcher(); + } catch (\UnexpectedValueException) { + $dispatcher = Factory::getContainer()->get(DispatcherInterface::class); + } + } // Get the dispatcher's hash to allow plugins to be registered to unique dispatchers - $dispatcherHash = spl_object_hash($dispatcher); + $dispatcherHash = spl_object_id($dispatcher); if (!isset($loaded[$dispatcherHash])) { $loaded[$dispatcherHash] = []; @@ -209,19 +221,33 @@ public static function importPlugin($type, $plugin = null, $autocreate = true, ? * Loads the plugin file. * * @param object $plugin The plugin. - * @param boolean $autocreate True to autocreate. - * @param ?DispatcherInterface $dispatcher Optionally allows the plugin to use a different dispatcher. + * @param boolean $autocreate Whether to register listeners with the event dispatcher. + * @param ?DispatcherInterface $dispatcher The event dispatcher. In 7.0 this will be required. * * @return void * * @since 3.2 + * @todo Arguments will not be optional in 7.0. */ protected static function import($plugin, $autocreate = true, ?DispatcherInterface $dispatcher = null) { static $plugins = []; + if ($dispatcher === null) { + @trigger_error( + \sprintf('Passing an instance of %1$s to %2$s() will be required in 7.0', DispatcherInterface::class, __METHOD__), + \E_USER_DEPRECATED + ); + + try { + $dispatcher = Factory::getApplication()->getDispatcher(); + } catch (\UnexpectedValueException) { + $dispatcher = Factory::getContainer()->get(DispatcherInterface::class); + } + } + // Get the dispatcher's hash to allow paths to be tracked against unique dispatchers - $hash = spl_object_hash($dispatcher) . $plugin->type . $plugin->name; + $hash = spl_object_id($dispatcher) . $plugin->type . $plugin->name; if (\array_key_exists($hash, $plugins)) { return; @@ -231,7 +257,8 @@ protected static function import($plugin, $autocreate = true, ?DispatcherInterfa $plugin = Factory::getApplication()->bootPlugin($plugin->name, $plugin->type); - if ($dispatcher && $plugin instanceof DispatcherAwareInterface) { + // @todo remove this in 7.0. + if ($plugin instanceof DispatcherAwareInterface) { $plugin->setDispatcher($dispatcher); } @@ -239,8 +266,7 @@ protected static function import($plugin, $autocreate = true, ?DispatcherInterfa return; } - // @TODO: Starting from 7.0 it should use $dispatcher->addSubscriber($plugin); for plugins which implement SubscriberInterface. - $plugin->registerListeners(); + $plugin->registerListeners($dispatcher); } /** diff --git a/tests/Unit/Libraries/Cms/Plugin/CMSPluginTest.php b/tests/Unit/Libraries/Cms/Plugin/CMSPluginTest.php index 44ad0633a1614..5926abbe3db9d 100644 --- a/tests/Unit/Libraries/Cms/Plugin/CMSPluginTest.php +++ b/tests/Unit/Libraries/Cms/Plugin/CMSPluginTest.php @@ -469,4 +469,69 @@ public function onTest() $this->assertEquals(['test', 'unit'], $event->getArgument('result')); } + + /** + * @testdox listeners registered with dispatcher passed to registerListeners() + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testRegisterListenersDispatcherUsed() + { + $constructorDispatcher = new Dispatcher(); + $setterDispatcher = new Dispatcher(); + $interfaceDispatcher = new Dispatcher(); + + $plugin = new class ($constructorDispatcher, []) extends CMSPlugin { + public function onLegacyEvent(stdClass $argument) + { + } + + public function onEvent(EventInterface $event) + { + } + }; + + $plugin->setDispatcher($setterDispatcher); + $plugin->registerListeners($interfaceDispatcher); + + $this->assertSame(0, $constructorDispatcher->countListeners('onLegacyEvent')); + $this->assertSame(0, $constructorDispatcher->countListeners('onEvent')); + $this->assertSame(0, $setterDispatcher->countListeners('onLegacyEvent')); + $this->assertSame(0, $setterDispatcher->countListeners('onEvent')); + $this->assertSame(1, $interfaceDispatcher->countListeners('onLegacyEvent')); + $this->assertSame(1, $interfaceDispatcher->countListeners('onEvent')); + } + + /** + * @testdox listeners registered with dispatcher set with setDispatcher() + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testSetterDispatcherUsed() + { + $constructorDispatcher = new Dispatcher(); + $setterDispatcher = new Dispatcher(); + + $plugin = new class ($constructorDispatcher, []) extends CMSPlugin { + public function onLegacyEvent(stdClass $argument) + { + } + + public function onEvent(EventInterface $event) + { + } + }; + + $plugin->setDispatcher($setterDispatcher); + $plugin->registerListeners(null); + + $this->assertSame(0, $constructorDispatcher->countListeners('onLegacyEvent')); + $this->assertSame(0, $constructorDispatcher->countListeners('onEvent')); + $this->assertSame(1, $setterDispatcher->countListeners('onLegacyEvent')); + $this->assertSame(1, $setterDispatcher->countListeners('onEvent')); + } }