From ed02eed88fbe65f0a9ba83e36da88224345c3817 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Thu, 13 Dec 2018 01:57:31 +0100 Subject: [PATCH 1/2] Do not resolve services when extending them Refs #1578. --- src/Extend/Frontend.php | 22 +++++++++++++-------- src/Extend/LanguagePack.php | 12 ++++++++++-- src/Extend/Locales.php | 38 +++++++++++++++++++------------------ src/Extend/Routes.php | 29 +++++++++++++++------------- 4 files changed, 60 insertions(+), 41 deletions(-) diff --git a/src/Extend/Frontend.php b/src/Extend/Frontend.php index e4927696a..cd3d0abc0 100644 --- a/src/Extend/Frontend.php +++ b/src/Extend/Frontend.php @@ -16,6 +16,7 @@ use Flarum\Frontend\Assets; use Flarum\Frontend\Compiler\Source\SourceCollector; use Flarum\Frontend\Frontend as ActualFrontend; use Flarum\Frontend\RecompileFrontendAssets; +use Flarum\Http\RouteCollection; use Flarum\Http\RouteHandlerFactory; use Illuminate\Contracts\Container\Container; @@ -122,15 +123,20 @@ class Frontend implements ExtenderInterface return; } - $routes = $container->make("flarum.$this->frontend.routes"); - $factory = $container->make(RouteHandlerFactory::class); + $container->resolving( + "flarum.{$this->frontend}.routes", + function (RouteCollection $collection, Container $container) { + /** @var RouteHandlerFactory $factory */ + $factory = $container->make(RouteHandlerFactory::class); - foreach ($this->routes as $route) { - $routes->get( - $route['path'], $route['name'], - $factory->toFrontend($this->frontend, $route['content']) - ); - } + foreach ($this->routes as $route) { + $collection->get( + $route['path'], $route['name'], + $factory->toFrontend($this->frontend, $route['content']) + ); + } + } + ); } private function registerContent(Container $container) diff --git a/src/Extend/LanguagePack.php b/src/Extend/LanguagePack.php index d51c228e7..b956a4746 100644 --- a/src/Extend/LanguagePack.php +++ b/src/Extend/LanguagePack.php @@ -37,8 +37,16 @@ class LanguagePack implements ExtenderInterface, LifecycleInterface ); } - /** @var LocaleManager $locales */ - $locales = $container->make(LocaleManager::class); + $container->resolving( + LocaleManager::class, + function (LocaleManager $locales) use ($extension, $locale, $title) { + $this->registerLocale($locales, $extension, $locale, $title); + } + ); + } + + private function registerLocale(LocaleManager $locales, Extension $extension, $locale, $title) + { $locales->addLocale($locale, $title); $directory = $extension->getPath().'/locale'; diff --git a/src/Extend/Locales.php b/src/Extend/Locales.php index fab28553a..f68892b14 100644 --- a/src/Extend/Locales.php +++ b/src/Extend/Locales.php @@ -27,33 +27,35 @@ class Locales implements ExtenderInterface, LifecycleInterface public function extend(Container $container, Extension $extension = null) { - /** @var LocaleManager $locales */ - $locales = $container->make(LocaleManager::class); + $container->resolving( + LocaleManager::class, + function (LocaleManager $locales) { + foreach (new DirectoryIterator($this->directory) as $file) { + if (! $file->isFile()) { + continue; + } - foreach (new DirectoryIterator($this->directory) as $file) { - if (! $file->isFile()) { - continue; + $extension = $file->getExtension(); + if (! in_array($extension, ['yml', 'yaml'])) { + continue; + } + + $locales->addTranslations( + $file->getBasename(".$extension"), + $file->getPathname() + ); + } } - - $extension = $file->getExtension(); - if (! in_array($extension, ['yml', 'yaml'])) { - continue; - } - - $locales->addTranslations( - $file->getBasename(".$extension"), - $file->getPathname() - ); - } + ); } public function onEnable(Container $container, Extension $extension) { - $container->make('flarum.locales')->clearCache(); + $container->make(LocaleManager::class)->clearCache(); } public function onDisable(Container $container, Extension $extension) { - $container->make('flarum.locales')->clearCache(); + $container->make(LocaleManager::class)->clearCache(); } } diff --git a/src/Extend/Routes.php b/src/Extend/Routes.php index 2d3cc71ab..dae7d17d4 100644 --- a/src/Extend/Routes.php +++ b/src/Extend/Routes.php @@ -12,6 +12,7 @@ namespace Flarum\Extend; use Flarum\Extension\Extension; +use Flarum\Http\RouteCollection; use Flarum\Http\RouteHandlerFactory; use Illuminate\Contracts\Container\Container; @@ -69,19 +70,21 @@ class Routes implements ExtenderInterface return; } - /** @var \Flarum\Http\RouteCollection $collection */ - $collection = $container->make("flarum.{$this->appName}.routes"); + $container->resolving( + "flarum.{$this->appName}.routes", + function (RouteCollection $collection, Container $container) { + /** @var RouteHandlerFactory $factory */ + $factory = $container->make(RouteHandlerFactory::class); - /** @var RouteHandlerFactory $factory */ - $factory = $container->make(RouteHandlerFactory::class); - - foreach ($this->routes as $route) { - $collection->addRoute( - $route['method'], - $route['path'], - $route['name'], - $factory->toController($route['handler']) - ); - } + foreach ($this->routes as $route) { + $collection->addRoute( + $route['method'], + $route['path'], + $route['name'], + $factory->toController($route['handler']) + ); + } + } + ); } } From b41d9fb0e71b2107617196ee61e6889b2ff2df8d Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Thu, 13 Dec 2018 02:01:50 +0100 Subject: [PATCH 2/2] Inject dependencies when firing events, not before The event subscriber approach means that dependencies have to be injected (and thus instantiated, along with all *their* dependencies) at the time of registering event listeners - even when events are never fired within a request's lifecycle. This is unnecessary and causes more classes than necessary to be loaded. In this case, we can explicitly register event listeners that will resolve their dependencies when the event is fired, not before. Refs #1578. --- src/Admin/AdminServiceProvider.php | 32 +++++++++++++++++++---- src/Extend/Frontend.php | 33 ++++++++++++++++++++---- src/Forum/ForumServiceProvider.php | 30 +++++++++++++++++---- src/Frontend/RecompileFrontendAssets.php | 15 ----------- 4 files changed, 80 insertions(+), 30 deletions(-) diff --git a/src/Admin/AdminServiceProvider.php b/src/Admin/AdminServiceProvider.php index 08c9348e1..ea727c2f0 100644 --- a/src/Admin/AdminServiceProvider.php +++ b/src/Admin/AdminServiceProvider.php @@ -12,8 +12,11 @@ namespace Flarum\Admin; use Flarum\Event\ConfigureMiddleware; +use Flarum\Extension\Event\Disabled; +use Flarum\Extension\Event\Enabled; use Flarum\Foundation\AbstractServiceProvider; use Flarum\Foundation\Application; +use Flarum\Foundation\Event\ClearingCache; use Flarum\Frontend\AddLocaleAssets; use Flarum\Frontend\AddTranslations; use Flarum\Frontend\Compiler\Source\SourceCollector; @@ -22,6 +25,8 @@ use Flarum\Http\Middleware as HttpMiddleware; use Flarum\Http\RouteCollection; use Flarum\Http\RouteHandlerFactory; use Flarum\Http\UrlGenerator; +use Flarum\Locale\LocaleManager; +use Flarum\Settings\Event\Saved; use Zend\Stratigility\MiddlewarePipe; class AdminServiceProvider extends AbstractServiceProvider @@ -102,11 +107,28 @@ class AdminServiceProvider extends AbstractServiceProvider $this->loadViewsFrom(__DIR__.'/../../views', 'flarum.admin'); - $this->app->make('events')->subscribe( - new RecompileFrontendAssets( - $this->app->make('flarum.assets.admin'), - $this->app->make('flarum.locales') - ) + $events = $this->app->make('events'); + + $events->listen( + [Enabled::class, Disabled::class, ClearingCache::class], + function () { + $recompile = new RecompileFrontendAssets( + $this->app->make('flarum.assets.admin'), + $this->app->make(LocaleManager::class) + ); + $recompile->flush(); + } + ); + + $events->listen( + Saved::class, + function (Saved $event) { + $recompile = new RecompileFrontendAssets( + $this->app->make('flarum.assets.admin'), + $this->app->make(LocaleManager::class) + ); + $recompile->whenSettingsSaved($event); + } ); } diff --git a/src/Extend/Frontend.php b/src/Extend/Frontend.php index cd3d0abc0..90ca00559 100644 --- a/src/Extend/Frontend.php +++ b/src/Extend/Frontend.php @@ -11,13 +11,18 @@ namespace Flarum\Extend; +use Flarum\Extension\Event\Disabled; +use Flarum\Extension\Event\Enabled; use Flarum\Extension\Extension; +use Flarum\Foundation\Event\ClearingCache; use Flarum\Frontend\Assets; use Flarum\Frontend\Compiler\Source\SourceCollector; use Flarum\Frontend\Frontend as ActualFrontend; use Flarum\Frontend\RecompileFrontendAssets; use Flarum\Http\RouteCollection; use Flarum\Http\RouteHandlerFactory; +use Flarum\Locale\LocaleManager; +use Flarum\Settings\Event\Saved; use Illuminate\Contracts\Container\Container; class Frontend implements ExtenderInterface @@ -108,11 +113,29 @@ class Frontend implements ExtenderInterface return $container->make('flarum.assets.factory')($this->frontend); }); - $container->make('events')->subscribe( - new RecompileFrontendAssets( - $container->make($abstract), - $container->make('flarum.locales') - ) + /** @var \Illuminate\Contracts\Events\Dispatcher $events */ + $events = $container->make('events'); + + $events->listen( + [Enabled::class, Disabled::class, ClearingCache::class], + function () use ($container, $abstract) { + $recompile = new RecompileFrontendAssets( + $container->make($abstract), + $container->make(LocaleManager::class) + ); + $recompile->flush(); + } + ); + + $events->listen( + Saved::class, + function (Saved $event) use ($container, $abstract) { + $recompile = new RecompileFrontendAssets( + $container->make($abstract), + $container->make(LocaleManager::class) + ); + $recompile->whenSettingsSaved($event); + } ); } } diff --git a/src/Forum/ForumServiceProvider.php b/src/Forum/ForumServiceProvider.php index 2d40259d9..94ee49d5b 100644 --- a/src/Forum/ForumServiceProvider.php +++ b/src/Forum/ForumServiceProvider.php @@ -13,9 +13,12 @@ namespace Flarum\Forum; use Flarum\Event\ConfigureForumRoutes; use Flarum\Event\ConfigureMiddleware; +use Flarum\Extension\Event\Disabled; +use Flarum\Extension\Event\Enabled; use Flarum\Formatter\Formatter; use Flarum\Foundation\AbstractServiceProvider; use Flarum\Foundation\Application; +use Flarum\Foundation\Event\ClearingCache; use Flarum\Frontend\AddLocaleAssets; use Flarum\Frontend\AddTranslations; use Flarum\Frontend\Assets; @@ -25,6 +28,8 @@ use Flarum\Http\Middleware as HttpMiddleware; use Flarum\Http\RouteCollection; use Flarum\Http\RouteHandlerFactory; use Flarum\Http\UrlGenerator; +use Flarum\Locale\LocaleManager; +use Flarum\Settings\Event\Saved; use Flarum\Settings\SettingsRepositoryInterface; use Symfony\Component\Translation\TranslatorInterface; use Zend\Stratigility\MiddlewarePipe; @@ -116,11 +121,26 @@ class ForumServiceProvider extends AbstractServiceProvider $events = $this->app->make('events'); - $events->subscribe( - new RecompileFrontendAssets( - $this->app->make('flarum.assets.forum'), - $this->app->make('flarum.locales') - ) + $events->listen( + [Enabled::class, Disabled::class, ClearingCache::class], + function () { + $recompile = new RecompileFrontendAssets( + $this->app->make('flarum.assets.forum'), + $this->app->make(LocaleManager::class) + ); + $recompile->flush(); + } + ); + + $events->listen( + Saved::class, + function (Saved $event) { + $recompile = new RecompileFrontendAssets( + $this->app->make('flarum.assets.forum'), + $this->app->make(LocaleManager::class) + ); + $recompile->whenSettingsSaved($event); + } ); $events->subscribe( diff --git a/src/Frontend/RecompileFrontendAssets.php b/src/Frontend/RecompileFrontendAssets.php index 9351da9bc..8fb372695 100644 --- a/src/Frontend/RecompileFrontendAssets.php +++ b/src/Frontend/RecompileFrontendAssets.php @@ -11,12 +11,8 @@ namespace Flarum\Frontend; -use Flarum\Extension\Event\Disabled; -use Flarum\Extension\Event\Enabled; -use Flarum\Foundation\Event\ClearingCache; use Flarum\Locale\LocaleManager; use Flarum\Settings\Event\Saved; -use Illuminate\Contracts\Events\Dispatcher; class RecompileFrontendAssets { @@ -40,17 +36,6 @@ class RecompileFrontendAssets $this->locales = $locales; } - /** - * @param Dispatcher $events - */ - public function subscribe(Dispatcher $events) - { - $events->listen(Saved::class, [$this, 'whenSettingsSaved']); - $events->listen(Enabled::class, [$this, 'flush']); - $events->listen(Disabled::class, [$this, 'flush']); - $events->listen(ClearingCache::class, [$this, 'flush']); - } - public function whenSettingsSaved(Saved $event) { if (preg_grep('/^theme_/i', array_keys($event->settings))) {