[6.1] Remove class-level event dispatcher dependency in plugins

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);
 }

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 @@
     /**
      * 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 @@
         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 @@
      * 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;
         }
@@ -314,8 +330,8 @@
             }
 
             // 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 @@
 
             // 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;
             }
@@ -335,14 +351,14 @@
             $paramName = $param->getName();
 
             // 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 @@
      * into old style method arguments and call your on<Something> 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 @@
      * 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]);
     }
 
     /**

libraries/src/Plugin/PluginHelper.php

@@ -151,13 +151,14 @@
      * 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 @@
         }
 
         // 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 @@
      * 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,16 +257,16 @@
 
         $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);
         }
 
         if (!$autocreate) {
             return;
         }
 
-        // @TODO: Starting from 7.0 it should use $dispatcher->addSubscriber($plugin); for plugins which implement SubscriberInterface.
-        $plugin->registerListeners();
+        $plugin->registerListeners($dispatcher);
     }
 
     /**

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'));
+    }
 }