Add subscribe method to event extender (#2535)

Historically, extensions using subscribers has caused problems because subscribers were constructed/applied at extension boot. This caused some classes (e.g. UrlGenerator) to be resolved early, breaking parts of Flarum. For this reason, subscriber support wasn't included in the initial version of the Event extender.

However, updating extensions has shown that there is a legitimate use case for subscribers in organizing clean code; for instance, core's own `DiscussionMetadataUpdater`.

This commit introduces support for subscribers, but only applies them after the app has booted, which avoids the early resolution issues. Since event listeners/subscribers are only intended to be used with domain events, which would never be dispatched during app boot, the late activation of subscribers should not cause issue.

src/Extend/Event.php

@@ -16,13 +16,16 @@ use Illuminate\Contracts\Events\Dispatcher;
 class Event implements ExtenderInterface
 {
     private $listeners = [];
+    private $subscribers = [];
 
     /**
      * Add a listener to a domain event dispatched by flarum or a flarum extension.
      *
      * The listener can either be:
-     *  - a callback function or
+     *  - a callback function
      *  - the class attribute of a class with a public `handle` method, which accepts an instance of the event as a parameter
+     *  - an array, where the first argument is an object or class name, and the second argument is the method on the
+     *    first argument that should be executed as the listener
      *
      * @param string $event
      * @param callable|string $listener
@@ -34,6 +37,22 @@ class Event implements ExtenderInterface
         return $this;
     }
 
+    /**
+     * Add a subscriber for a set of domain events dispatched by flarum or a flarum extension.
+     * Event subscribers are classes that may subscribe to multiple events from within the subscriber class itself,
+     * allowing you to define several event handlers within a single class.
+     *
+     * @see https://laravel.com/docs/6.x/events#writing-event-subscribers
+     *
+     * @param string $subscriber: The class attribute of the subscriber class
+     */
+    public function subscribe(string $subscriber)
+    {
+        $this->subscribers[] = $subscriber;
+
+        return $this;
+    }
+
     public function extend(Container $container, Extension $extension = null)
     {
         $events = $container->make(Dispatcher::class);
@@ -41,5 +60,13 @@ class Event implements ExtenderInterface
         foreach ($this->listeners as $listener) {
             $events->listen($listener[0], $listener[1]);
         }
+
+        $app = $container->make('flarum');
+
+        $app->booted(function () use ($events) {
+            foreach ($this->subscribers as $subscriber) {
+                $events->subscribe($subscriber);
+            }
+        });
     }
 }

tests/integration/extenders/EventTest.php

@@ -10,12 +10,14 @@
 namespace Flarum\Tests\integration\extenders;
 
 use Flarum\Extend;
+use Flarum\Foundation\Application;
 use Flarum\Group\Command\CreateGroup;
 use Flarum\Group\Event\Created;
 use Flarum\Tests\integration\RetrievesAuthorizedUsers;
 use Flarum\Tests\integration\TestCase;
 use Flarum\User\User;
-use Illuminate\Contracts\Bus\Dispatcher;
+use Illuminate\Contracts\Bus\Dispatcher as BusDispatcher;
+use Illuminate\Contracts\Events\Dispatcher;
 use Symfony\Component\Translation\TranslatorInterface;
 
 class EventTest extends TestCase
@@ -24,7 +26,7 @@ class EventTest extends TestCase
 
     protected function buildGroup()
     {
-        $bus = $this->app()->getContainer()->make(Dispatcher::class);
+        $bus = $this->app()->getContainer()->make(BusDispatcher::class);
 
         return $bus->dispatch(
             new CreateGroup(User::find(1), ['attributes' => [
@@ -72,6 +74,32 @@ class EventTest extends TestCase
 
         $this->assertEquals('core.group.admin', $group->name_singular);
     }
+
+    /**
+     * @test
+     */
+    public function custom_subscriber_works()
+    {
+        // Because it injects a translator, this also tests that stuff can be injected into this callback.
+        $this->extend((new Extend\Event)->subscribe(CustomSubscriber::class));
+
+        $group = $this->buildGroup();
+
+        $this->assertEquals('core.group.admin', $group->name_singular);
+    }
+
+    /**
+     * @test
+     */
+    public function custom_subscriber_applied_after_app_booted()
+    {
+        // Because it injects a translator, this also tests that stuff can be injected into this callback.
+        $this->extend((new Extend\Event)->subscribe(CustomSubscriber::class));
+
+        $group = $this->buildGroup();
+
+        $this->assertEquals('booted', $group->name_plural);
+    }
 }
 
 class CustomListener
@@ -88,3 +116,26 @@ class CustomListener
         $event->group->name_singular = $this->translator->trans('core.group.admin');
     }
 }
+
+class CustomSubscriber
+{
+    protected $bootedAtConstruct;
+    protected $translator;
+
+    public function __construct(Application $app, TranslatorInterface $translator)
+    {
+        $this->bootedAtConstruct = $app->isBooted();
+        $this->translator = $translator;
+    }
+
+    public function subscribe(Dispatcher $events)
+    {
+        $events->listen(Created::class, [$this, 'whenGroupCreated']);
+    }
+
+    public function whenGroupCreated(Created $event)
+    {
+        $event->group->name_singular = $this->translator->trans('core.group.admin');
+        $event->group->name_plural = $this->bootedAtConstruct ? 'booted' : 'not booted';
+    }
+}