[6.2] 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('JPATH_PLATFORM') or die;
@@ -19,15 +20,21 @@
  * Access to plugin specific services.
  *
  * @since  4.0.0
+ *
+ * @todo  In 6.0 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
+     *
+     * @todo  In 6.0 $dispatcher argument will no longer be nullable
      */
-    public function registerListeners();
+    public function registerListeners(?DispatcherInterface $dispatcher = null);
 }

libraries/src/Plugin/CMSPlugin.php

@@ -13,7 +13,6 @@
 use Joomla\CMS\Extension\PluginInterface;
 use Joomla\CMS\Factory;
 use Joomla\Event\AbstractEvent;
-use Joomla\Event\DispatcherAwareInterface;
 use Joomla\Event\DispatcherAwareTrait;
 use Joomla\Event\DispatcherInterface;
 use Joomla\Event\EventInterface;
@@ -29,7 +28,7 @@
  *
  * @since  1.5
  */
-abstract class CMSPlugin implements DispatcherAwareInterface, PluginInterface
+abstract class CMSPlugin implements PluginInterface
 {
     use DispatcherAwareTrait;
 
@@ -91,15 +90,30 @@ abstract class CMSPlugin implements DispatcherAwareInterface, PluginInterface
     /**
      * Constructor
      *
-     * @param   DispatcherInterface  &$subject  The object to observe
-     * @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
      */
-    public function __construct(&$subject, $config = array())
+    public function __construct($config = array())
     {
+        if ($config instanceof DispatcherInterface) {
+            @trigger_error(
+                sprintf(
+                    'Passing an instance of %1$s to %2$s will not be supported in 6.0',
+                    DispatcherInterface::class,
+                    __METHOD__
+                ),
+                \E_USER_DEPRECATED
+            );
+
+             // Set the dispatcher we are to register our listeners with
+            $this->setDispatcher($config);
+
+            $config = func_num_args() > 1 ? func_get_arg(1) : [];
+        }
+
         // Get the parameters.
         if (isset($config['params'])) {
             if ($config['params'] instanceof Registry) {
@@ -143,9 +157,6 @@ public function __construct(&$subject, $config = array())
                 $this->db = Factory::getDbo();
             }
         }
-
-        // Set the dispatcher we are to register our listeners with
-        $this->setDispatcher($subject);
     }
 
     /**
@@ -187,15 +198,28 @@ public function loadLanguage($extension = '', $basePath = JPATH_ADMINISTRATOR)
      * 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
+     *
+     * @todo   In 6.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) {
+            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;
         }
@@ -211,7 +235,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;
             }
@@ -221,7 +245,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;
             }
@@ -232,13 +256,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);
         }
     }
 
@@ -250,15 +274,27 @@ public function registerListeners()
      * 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
+     *
+     * @todo   In 6.0 $dispatcher argument will no longer be nullable.
      */
-    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
@@ -294,15 +330,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
+     *
+     * @todo   In 6.0 $dispatcher argument will no longer be nullable.
      */
-    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

@@ -139,16 +139,17 @@ public static function isEnabled($type, $plugin = null)
      * Loads all the plugin files for a particular type if no specific plugin is specified
      * 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                $type        The plugin type, relates to the subdirectory in the plugins directory.
+     * @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 6.0 this will be required.
      *
      * @return  boolean  True on success.
      *
      * @since   1.5
+     * @todo    Arguments will not be optional in 6.0.
      */
-    public static function importPlugin($type, $plugin = null, $autocreate = true, DispatcherInterface $dispatcher = null)
+    public static function importPlugin($type, $plugin = null, $autocreate = true, ?DispatcherInterface $dispatcher = null)
     {
         static $loaded = [];
 
@@ -160,10 +161,21 @@ public static function importPlugin($type, $plugin = null, $autocreate = true, D
         }
 
         // 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 6.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] = [];
@@ -198,19 +210,19 @@ public static function importPlugin($type, $plugin = null, $autocreate = true, D
      * 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.
      *
      * @return  void
      *
      * @since   3.2
      */
-    protected static function import($plugin, $autocreate = true, DispatcherInterface $dispatcher = null)
+    protected static function import($plugin, $autocreate, DispatcherInterface $dispatcher)
     {
         static $plugins = [];
 
         // 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;
@@ -220,15 +232,16 @@ protected static function import($plugin, $autocreate = true, DispatcherInterfac
 
         $plugin = Factory::getApplication()->bootPlugin($plugin->name, $plugin->type);
 
-        if ($dispatcher && $plugin instanceof DispatcherAwareInterface) {
+        // @todo remove this in 6.0.
+        if ($plugin instanceof DispatcherAwareInterface) {
             $plugin->setDispatcher($dispatcher);
         }
 
         if (!$autocreate) {
             return;
         }
 
-        $plugin->registerListeners();
+        $plugin->registerListeners($dispatcher);
     }
 
     /**

tests/Unit/Libraries/Cms/Plugin/CMSPluginTest.php

@@ -490,4 +490,71 @@ 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'));
+    }
 }