Improved event discovery (Laravel 10/11 compatible) (#88)

* Config switch added for event autodiscovery

* WIP

* WIP

* Working Laravel 10

* Remove old test

* Working Laravel 11

* Fix code style

---------

Co-authored-by: Alex Wulf <alex.f.wulf@gmail.com>

config.php

@@ -78,4 +78,17 @@
 	*/
 
 	'stubs' => null,
+
+	/*
+	|--------------------------------------------------------------------------
+	| Custom override of event discovery
+	|--------------------------------------------------------------------------
+	|
+	| This is a custom override of the event discovery feature. If you want to
+	| disable event discovery, set this to false. If you want to enable event
+	| discovery, set this to true. We will still check the app namespace for
+	| the presence of event discovery.
+	*/
+
+	'should_discover_events' => null,
 ];

src/Support/ModularEventServiceProvider.php

@@ -3,34 +3,72 @@
 namespace InterNACHI\Modular\Support;
 
 use Illuminate\Foundation\Support\Providers\EventServiceProvider;
+use Illuminate\Support\Arr;
+use Illuminate\Support\ServiceProvider;
+use ReflectionProperty;
 use Symfony\Component\Finder\SplFileInfo;
 
-class ModularEventServiceProvider extends EventServiceProvider
+class ModularEventServiceProvider extends ServiceProvider
 {
-	public function discoverEvents()
+	public function register()
 	{
-		return collect($this->discoverEventsWithin())
-			->reject(fn($directory) => ! is_dir($directory))
-			->reduce(fn($discovered, $directory) => array_merge_recursive(
-				$discovered,
-				DiscoverEvents::within($directory, $this->eventDiscoveryBasePath())
-			), []);
+		// We need to do this in the App::booting hook to ensure that it registers
+		// events before the EventServiceProvider::booting callback triggers. It's
+		// necessary to modify the existing EventServiceProvider's $listen array,
+		// rather than just register our own EventServiceProvider subclass, because
+		// Laravel behaves differently if the non-default provider is registered.
+		$this->app->booting(function() {
+			$events = $this->getEvents();
+			$provider = Arr::first($this->app->getProviders(EventServiceProvider::class));
+
+			if (! $provider || empty($events)) {
+				return;
+			}
+
+			$listen = new ReflectionProperty($provider, 'listen');
+			$listen->setAccessible(true);
+			$listen->setValue($provider, array_merge_recursive($listen->getValue($provider), $events));
+		});
 	}
 
-	public function shouldDiscoverEvents()
+	public function getEvents(): array
 	{
-		// We'll enable event discovery if it's enabled in the app namespace
-		return collect($this->app->getProviders(EventServiceProvider::class))
-			->filter(fn(EventServiceProvider $provider) => str_starts_with(get_class($provider), $this->app->getNamespace()))
-			->contains(fn(EventServiceProvider $provider) => $provider->shouldDiscoverEvents());
+		// If events are cached, or Modular event discovery is disabled, then we'll
+		// just let the normal event service provider handle all the event loading.
+		if ($this->app->eventsAreCached() || ! $this->shouldDiscoverEvents()) {
+			return [];
+		}
+
+		return $this->discoverEvents();
+	}
+
+	public function shouldDiscoverEvents(): bool
+	{
+		return config('app-modules.should_discover_events')
+			?? $this->appIsConfiguredToDiscoverEvents();
 	}
 
-	protected function discoverEventsWithin()
+	public function discoverEvents()
 	{
+		$modules = $this->app->make(ModuleRegistry::class);
+
 		return $this->app->make(AutoDiscoveryHelper::class)
 			->listenerDirectoryFinder()
 			->map(fn(SplFileInfo $directory) => $directory->getPathname())
-			->values()
-			->all();
+			->reduce(function($discovered, string $directory) use ($modules) {
+				$module = $modules->moduleForPath($directory);
+				return array_merge_recursive(
+					$discovered,
+					DiscoverEvents::within($directory, $module->path('src'))
+				);
+			}, []);
+	}
+
+	public function appIsConfiguredToDiscoverEvents(): bool
+	{
+		return collect($this->app->getProviders(EventServiceProvider::class))
+			->filter(fn(EventServiceProvider $provider) => $provider::class === EventServiceProvider::class
+				|| str_starts_with(get_class($provider), $this->app->getNamespace()))
+			->contains(fn(EventServiceProvider $provider) => $provider->shouldDiscoverEvents());
 	}
 }

tests/Concerns/PreloadsAppModules.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace InterNACHI\Modular\Tests\Concerns;
+
+use Illuminate\Filesystem\Filesystem;
+
+trait PreloadsAppModules
+{
+	protected static $autoloader_registered = false;
+
+	/** @before */
+	public function prepareTestModule(): void
+	{
+		$src = __DIR__.'/../testbench-core/app-modules';
+		$dest = static::applicationBasePath().'/app-modules';
+
+		$fs = new Filesystem();
+		$fs->deleteDirectory($dest);
+		$fs->copyDirectory($src, $dest);
+	}
+
+	/** @before */
+	public function prepareModuleAutoloader(): void
+	{
+		if (! static::$autoloader_registered) {
+			spl_autoload_register(function($fqcn) {
+				if (str_starts_with($fqcn, 'Modules\\TestModule\\')) {
+					$path = str_replace(
+						['Modules\\TestModule\\', '\\'],
+						['', DIRECTORY_SEPARATOR],
+						$fqcn
+					);
+					$path = static::applicationBasePath().'/app-modules/test-module/src/'.$path.'.php';
+					if (file_exists($path)) {
+						include_once $path;
+					}
+				}
+			});
+		}
+
+		static::$autoloader_registered = true;
+	}
+}

tests/EventDiscovery/EventDiscoveryExplicitlyDisabledTest.php

@@ -0,0 +1,67 @@
+<?php
+
+// Because we need to preload files before TestBench boots the app,
+// this needs to be its own isolated test file.
+
+namespace InterNACHI\Modular\Tests\EventDiscovery {
+	use App\EventDiscoveryExplicitlyDisabledTestProvider;
+	use Illuminate\Support\Facades\Event;
+	use InterNACHI\Modular\Support\Facades\Modules;
+	use InterNACHI\Modular\Tests\Concerns\PreloadsAppModules;
+	use InterNACHI\Modular\Tests\TestCase;
+
+	class EventDiscoveryExplicitlyDisabledTest extends TestCase
+	{
+		use PreloadsAppModules;
+
+		protected function setUp(): void
+		{
+			parent::setUp();
+
+			$this->beforeApplicationDestroyed(fn() => $this->artisan('event:clear'));
+		}
+
+		public function test_it_does_not_auto_discover_event_listeners(): void
+		{
+			$module = Modules::module('test-module');
+
+			$this->assertEmpty(Event::getListeners($module->qualify('Events\\TestEvent')));
+
+			// Also check that the events are cached correctly
+
+			$this->artisan('event:cache');
+
+			$cache = require $this->app->getCachedEventsPath();
+
+			$this->assertEmpty($cache[EventDiscoveryExplicitlyDisabledTestProvider::class]);
+
+			$this->artisan('event:clear');
+		}
+
+		protected function getPackageProviders($app)
+		{
+			return array_merge([EventDiscoveryExplicitlyDisabledTestProvider::class], parent::getPackageProviders($app));
+		}
+
+		protected function resolveApplicationConfiguration($app)
+		{
+			parent::resolveApplicationConfiguration($app);
+
+			$app['config']['app-modules.should_discover_events'] = false;
+		}
+	}
+}
+
+// We need to use an "App" namespace to tell modular that this provider should be deferred to
+
+namespace App {
+	use Illuminate\Foundation\Support\Providers\EventServiceProvider;
+
+	class EventDiscoveryExplicitlyDisabledTestProvider extends EventServiceProvider
+	{
+		public function shouldDiscoverEvents()
+		{
+			return true;
+		}
+	}
+}

tests/EventDiscovery/EventDiscoveryExplicitlyEnabledTest.php

@@ -0,0 +1,72 @@
+<?php
+
+// Because we need to preload files before TestBench boots the app,
+// this needs to be its own isolated test file.
+
+namespace InterNACHI\Modular\Tests\EventDiscovery {
+	use App\EventDiscoveryExplicitlyEnabledTestProvider;
+	use Illuminate\Support\Facades\Event;
+	use InterNACHI\Modular\Support\Facades\Modules;
+	use InterNACHI\Modular\Tests\Concerns\PreloadsAppModules;
+	use InterNACHI\Modular\Tests\TestCase;
+
+	class EventDiscoveryExplicitlyEnabledTest extends TestCase
+	{
+		use PreloadsAppModules;
+
+		protected function setUp(): void
+		{
+			parent::setUp();
+
+			$this->beforeApplicationDestroyed(fn() => $this->artisan('event:clear'));
+		}
+
+		public function test_it_auto_discovers_event_listeners(): void
+		{
+			$module = Modules::module('test-module');
+
+			$this->assertNotEmpty(Event::getListeners($module->qualify('Events\\TestEvent')));
+
+			// Also check that the events are cached correctly
+
+			$this->artisan('event:cache');
+
+			$cache = require $this->app->getCachedEventsPath();
+
+			$this->assertArrayHasKey($module->qualify('Events\\TestEvent'), $cache[EventDiscoveryExplicitlyEnabledTestProvider::class]);
+
+			$this->assertContains(
+				$module->qualify('Listeners\\TestEventListener@handle'),
+				$cache[EventDiscoveryExplicitlyEnabledTestProvider::class][$module->qualify('Events\\TestEvent')]
+			);
+
+			$this->artisan('event:clear');
+		}
+
+		protected function getPackageProviders($app)
+		{
+			return array_merge([EventDiscoveryExplicitlyEnabledTestProvider::class], parent::getPackageProviders($app));
+		}
+
+		protected function resolveApplicationConfiguration($app)
+		{
+			parent::resolveApplicationConfiguration($app);
+
+			$app['config']['app-modules.should_discover_events'] = true;
+		}
+	}
+}
+
+// We need to use an "App" namespace to tell modular that this provider should be deferred to
+
+namespace App {
+	use Illuminate\Foundation\Support\Providers\EventServiceProvider;
+
+	class EventDiscoveryExplicitlyEnabledTestProvider extends EventServiceProvider
+	{
+		public function shouldDiscoverEvents()
+		{
+			return false;
+		}
+	}
+}

tests/EventDiscovery/EventDiscoveryImplicitlyDisabledTest.php

@@ -0,0 +1,60 @@
+<?php
+
+// Because we need to preload files before TestBench boots the app,
+// this needs to be its own isolated test file.
+
+namespace InterNACHI\Modular\Tests\EventDiscovery {
+	use App\EventDiscoveryImplicitlyDisabledTestProvider;
+	use Illuminate\Support\Facades\Event;
+	use InterNACHI\Modular\Support\Facades\Modules;
+	use InterNACHI\Modular\Tests\Concerns\PreloadsAppModules;
+	use InterNACHI\Modular\Tests\TestCase;
+
+	class EventDiscoveryImplicitlyDisabledTest extends TestCase
+	{
+		use PreloadsAppModules;
+
+		protected function setUp(): void
+		{
+			parent::setUp();
+
+			$this->beforeApplicationDestroyed(fn() => $this->artisan('event:clear'));
+		}
+
+		public function test_it_does_not_auto_discover_event_listeners(): void
+		{
+			$module = Modules::module('test-module');
+
+			$this->assertEmpty(Event::getListeners($module->qualify('Events\\TestEvent')));
+
+			// Also check that the events are cached correctly
+
+			$this->artisan('event:cache');
+
+			$cache = require $this->app->getCachedEventsPath();
+
+			$this->assertEmpty($cache[EventDiscoveryImplicitlyDisabledTestProvider::class]);
+
+			$this->artisan('event:clear');
+		}
+
+		protected function getPackageProviders($app)
+		{
+			return array_merge([EventDiscoveryImplicitlyDisabledTestProvider::class], parent::getPackageProviders($app));
+		}
+	}
+}
+
+// We need to use an "App" namespace to tell modular that this provider should be deferred to
+
+namespace App {
+	use Illuminate\Foundation\Support\Providers\EventServiceProvider;
+
+	class EventDiscoveryImplicitlyDisabledTestProvider extends EventServiceProvider
+	{
+		public function shouldDiscoverEvents()
+		{
+			return false;
+		}
+	}
+}