From ec5cb98c7705bcef31536442994b9b45b3c51aab Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Fri, 15 Sep 2023 09:30:24 +0100 Subject: [PATCH] chore: merge the app with the container & implement the ApplicationContract (#3862) * chore: merge the app with the container & implement the ApplicationContract Illuminate components always expect the app to be the container, but also expect the app to be implementing the laravel app contract. This means that very often between minor illuminate updates we get a call to a method on the app that doesn't exist in the Flarum app. This fixes the issue once and for all. * chore: improve concern implementation readability * chore: service provider no longer has to change app type * chore: unimplement `terminat(e/ing)` * Apply fixes from StyleCI * chore: recover `container` prop * chore: return types * fix: phpstan errors --- .../Foundation/AbstractServiceProvider.php | 19 +- framework/core/src/Foundation/Application.php | 71 ++---- .../Concerns/InteractsWithLaravel.php | 225 ++++++++++++++++++ framework/core/src/Foundation/Config.php | 5 + framework/core/src/Foundation/Container.php | 27 --- .../core/src/Foundation/InstalledSite.php | 87 ++++--- .../core/src/Foundation/UninstalledSite.php | 43 ++-- framework/core/src/helpers.php | 2 +- .../testing/src/integration/TestCase.php | 2 + 9 files changed, 328 insertions(+), 153 deletions(-) create mode 100644 framework/core/src/Foundation/Concerns/InteractsWithLaravel.php delete mode 100644 framework/core/src/Foundation/Container.php diff --git a/framework/core/src/Foundation/AbstractServiceProvider.php b/framework/core/src/Foundation/AbstractServiceProvider.php index 97f699da9..faf7e1750 100644 --- a/framework/core/src/Foundation/AbstractServiceProvider.php +++ b/framework/core/src/Foundation/AbstractServiceProvider.php @@ -9,24 +9,17 @@ namespace Flarum\Foundation; -use Illuminate\Contracts\Container\Container; +use Illuminate\Contracts\Foundation\Application as ApplicationContract; use Illuminate\Support\ServiceProvider; abstract class AbstractServiceProvider extends ServiceProvider { - /** - * @deprecated perpetually, not removed because Laravel needs it. - * @var Container - */ - protected $app; + protected ApplicationContract $container; - public function __construct( - protected Container $container - ) { - parent::__construct($container); - } - - public function register() + public function __construct(ApplicationContract $app) { + parent::__construct($app); + + $this->container = $app; } } diff --git a/framework/core/src/Foundation/Application.php b/framework/core/src/Foundation/Application.php index eb51c40ab..90b2bb724 100644 --- a/framework/core/src/Foundation/Application.php +++ b/framework/core/src/Foundation/Application.php @@ -9,13 +9,17 @@ namespace Flarum\Foundation; -use Illuminate\Contracts\Container\Container; +use Flarum\Foundation\Concerns\InteractsWithLaravel; +use Illuminate\Container\Container as IlluminateContainer; +use Illuminate\Contracts\Foundation\Application as LaravelApplication; use Illuminate\Events\EventServiceProvider; use Illuminate\Support\Arr; use Illuminate\Support\ServiceProvider; -class Application +class Application extends IlluminateContainer implements LaravelApplication { + use InteractsWithLaravel; + /** * The Flarum version. * @@ -34,7 +38,6 @@ class Application protected array $loadedProviders = []; public function __construct( - private readonly Container $container, protected Paths $paths ) { $this->registerBaseBindings(); @@ -44,19 +47,14 @@ class Application public function config(string $key, mixed $default = null): mixed { - $config = $this->container->make('flarum.config'); + $config = $this->make('flarum.config'); return $config[$key] ?? $default; } - public function inDebugMode(): bool - { - return $this->config('debug', true); - } - public function url(string $path = null): string { - $config = $this->container->make('flarum.config'); + $config = $this->make('flarum.config'); $url = (string) $config->url(); if ($path) { @@ -68,31 +66,27 @@ class Application protected function registerBaseBindings(): void { - \Illuminate\Container\Container::setInstance($this->container); + IlluminateContainer::setInstance($this); - /** - * Needed for the laravel framework code. - * Use container inside flarum instead. - */ - $this->container->instance('app', $this->container); - $this->container->alias('app', \Illuminate\Container\Container::class); + $this->instance('app', $this); + $this->alias('app', IlluminateContainer::class); - $this->container->instance('container', $this->container); - $this->container->alias('container', \Illuminate\Container\Container::class); + $this->instance('container', $this); + $this->alias('container', IlluminateContainer::class); - $this->container->instance('flarum', $this); - $this->container->alias('flarum', self::class); + $this->instance('flarum', $this); + $this->alias('flarum', self::class); - $this->container->instance('flarum.paths', $this->paths); - $this->container->alias('flarum.paths', Paths::class); + $this->instance('flarum.paths', $this->paths); + $this->alias('flarum.paths', Paths::class); } protected function registerBaseServiceProviders(): void { - $this->register(new EventServiceProvider($this->container)); + $this->register(new EventServiceProvider($this)); } - public function register(string|ServiceProvider $provider, array $options = [], bool $force = false): ServiceProvider + public function register($provider, $force = false): ServiceProvider { if (($registered = $this->getProvider($provider)) && ! $force) { return $registered; @@ -102,18 +96,11 @@ class Application // application instance automatically for the developer. This is simply // a more convenient way of specifying your service provider classes. if (is_string($provider)) { - $provider = $this->resolveProviderClass($provider); + $provider = $this->resolveProvider($provider); } $provider->register(); - // Once we have registered the service we will iterate through the options - // and set each of them on the application so they will be available on - // the actual loading of the service objects and for developer usage. - foreach ($options as $key => $value) { - $this[$key] = $value; - } - $this->markAsRegistered($provider); // If the application has already booted, we will call this boot method on @@ -140,14 +127,14 @@ class Application * * @param class-string $provider */ - public function resolveProviderClass(string $provider): ServiceProvider + public function resolveProvider($provider): ServiceProvider { - return new $provider($this->container); + return new $provider($this); } protected function markAsRegistered(ServiceProvider $provider): void { - $this->container['events']->dispatch($class = get_class($provider), [$provider]); + $this['events']->dispatch($class = get_class($provider), [$provider]); $this->serviceProviders[] = $provider; @@ -182,7 +169,7 @@ class Application protected function bootProvider(ServiceProvider $provider): mixed { if (method_exists($provider, 'boot')) { - return $this->container->call([$provider, 'boot']); + return $this->call([$provider, 'boot']); } return null; @@ -232,7 +219,7 @@ class Application foreach ($aliases as $key => $aliasGroup) { foreach ($aliasGroup as $alias) { - $this->container->alias($key, $alias); + $this->alias($key, $alias); } } } @@ -241,12 +228,4 @@ class Application { return static::VERSION; } - - public function terminating(): void - { - } - - public function terminate(): void - { - } } diff --git a/framework/core/src/Foundation/Concerns/InteractsWithLaravel.php b/framework/core/src/Foundation/Concerns/InteractsWithLaravel.php new file mode 100644 index 000000000..58ea69cd7 --- /dev/null +++ b/framework/core/src/Foundation/Concerns/InteractsWithLaravel.php @@ -0,0 +1,225 @@ +joinPaths($this->paths->base, $path); + } + + public function publicPath($path = ''): string + { + return $this->joinPaths($this->paths->public, $path); + } + + public function storagePath($path = ''): string + { + return $this->joinPaths($this->paths->storage, $path); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function bootstrapPath($path = ''): string + { + return $this->joinPaths( + $this->joinPaths($this->paths->base, 'bootstrap'), + $path + ); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function configPath($path = ''): string + { + return $this->joinPaths( + $this->joinPaths($this->paths->storage, 'config'), + $path + ); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function databasePath($path = ''): string + { + return $this->joinPaths( + $this->joinPaths($this->paths->base, 'database'), + $path + ); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function langPath($path = ''): string + { + return $this->joinPaths( + $this->joinPaths($this->paths->base, 'lang'), + $path + ); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function resourcePath($path = ''): string + { + return $this->joinPaths( + $this->joinPaths($this->paths->base, 'resources'), + $path + ); + } + + public function environment(...$environments): bool|string + { + if (count($environments) > 0) { + $patterns = is_array($environments[0]) ? $environments[0] : $environments; + + return Str::is($patterns, $this['env']); + } + + return $this['env']; + } + + public function runningInConsole(): bool + { + return \PHP_SAPI === 'cli' || \PHP_SAPI === 'phpdbg'; + } + + public function runningUnitTests(): bool + { + return $this->bound('env') && $this->environment('testing'); + } + + public function hasDebugModeEnabled(): bool + { + return $this->config('debug', true); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function maintenanceMode() + { + return null; // @phpstan-ignore-line + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function isDownForMaintenance(): bool + { + // TODO: Implement isDownForMaintenance() method. + return false; + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function registerConfiguredProviders(): void + { + // + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function registerDeferredProvider($provider, $service = null): void + { + // + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function bootstrapWith(array $bootstrappers): void + { + // + } + + public function getLocale(): string + { + return $this->make(LocaleManager::class)->getLocale(); + } + + public function getNamespace(): string + { + return 'Flarum'; + } + + public function getProviders($provider): array + { + $name = is_string($provider) ? $provider : get_class($provider); + + return Arr::where($this->serviceProviders, fn ($value) => $value instanceof $name); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function hasBeenBootstrapped(): bool + { + return true; + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function loadDeferredProviders(): void + { + // + } + + public function setLocale($locale): void + { + $this->make(LocaleManager::class)->setLocale($locale); + } + + /** + * @deprecated Not actually used/has no meaning in Flarum. + */ + public function shouldSkipMiddleware(): bool + { + return false; + } + + public function joinPaths(string $basePath, string $path = ''): string + { + return $basePath.($path != '' ? DIRECTORY_SEPARATOR.ltrim($path, DIRECTORY_SEPARATOR) : ''); + } +} diff --git a/framework/core/src/Foundation/Config.php b/framework/core/src/Foundation/Config.php index c81679394..87c1409f8 100644 --- a/framework/core/src/Foundation/Config.php +++ b/framework/core/src/Foundation/Config.php @@ -69,4 +69,9 @@ class Config implements ArrayAccess { throw new RuntimeException('The Config is immutable'); } + + public function environment(): string + { + return $this->data['env'] ?? 'production'; + } } diff --git a/framework/core/src/Foundation/Container.php b/framework/core/src/Foundation/Container.php deleted file mode 100644 index 04ad0be5e..000000000 --- a/framework/core/src/Foundation/Container.php +++ /dev/null @@ -1,27 +0,0 @@ -get('flarum')->$name(...$arguments); - } -} diff --git a/framework/core/src/Foundation/InstalledSite.php b/framework/core/src/Foundation/InstalledSite.php index 2cdb50759..2d52395f7 100644 --- a/framework/core/src/Foundation/InstalledSite.php +++ b/framework/core/src/Foundation/InstalledSite.php @@ -39,7 +39,7 @@ use Illuminate\Cache\Repository as CacheRepository; use Illuminate\Config\Repository as ConfigRepository; use Illuminate\Contracts\Cache\Repository; use Illuminate\Contracts\Cache\Store; -use Illuminate\Contracts\Container\Container as LaravelContainer; +use Illuminate\Contracts\Container\Container; use Illuminate\Filesystem\Filesystem; use Illuminate\Hashing\HashServiceProvider; use Illuminate\Validation\ValidationServiceProvider; @@ -87,62 +87,61 @@ class InstalledSite implements SiteInterface return $this; } - protected function bootLaravel(): LaravelContainer + protected function bootLaravel(): Container { - $container = new Container; - $laravel = new Application($container, $this->paths); + $app = new Application($this->paths); - $container->instance('env', 'production'); - $container->instance('flarum.config', $this->config); - $container->alias('flarum.config', Config::class); - $container->instance('flarum.debug', $this->config->inDebugMode()); - $container->instance('config', $this->getIlluminateConfig()); - $container->instance('flarum.maintenance.handler', new MaintenanceModeHandler); + $app->instance('env', $this->config->environment()); + $app->instance('flarum.config', $this->config); + $app->alias('flarum.config', Config::class); + $app->instance('flarum.debug', $this->config->inDebugMode()); + $app->instance('config', $this->getIlluminateConfig()); + $app->instance('flarum.maintenance.handler', new MaintenanceModeHandler); - $this->registerLogger($container); - $this->registerCache($container); + $this->registerLogger($app); + $this->registerCache($app); - $laravel->register(AdminServiceProvider::class); - $laravel->register(ApiServiceProvider::class); - $laravel->register(BusServiceProvider::class); - $laravel->register(ConsoleServiceProvider::class); - $laravel->register(DatabaseServiceProvider::class); - $laravel->register(DiscussionServiceProvider::class); - $laravel->register(ExtensionServiceProvider::class); - $laravel->register(ErrorServiceProvider::class); - $laravel->register(FilesystemServiceProvider::class); - $laravel->register(FilterServiceProvider::class); - $laravel->register(FormatterServiceProvider::class); - $laravel->register(ForumServiceProvider::class); - $laravel->register(FrontendServiceProvider::class); - $laravel->register(GroupServiceProvider::class); - $laravel->register(HashServiceProvider::class); - $laravel->register(HttpServiceProvider::class); - $laravel->register(LocaleServiceProvider::class); - $laravel->register(MailServiceProvider::class); - $laravel->register(NotificationServiceProvider::class); - $laravel->register(PostServiceProvider::class); - $laravel->register(QueueServiceProvider::class); - $laravel->register(SearchServiceProvider::class); - $laravel->register(SessionServiceProvider::class); - $laravel->register(SettingsServiceProvider::class); - $laravel->register(UpdateServiceProvider::class); - $laravel->register(UserServiceProvider::class); - $laravel->register(ValidationServiceProvider::class); - $laravel->register(ViewServiceProvider::class); + $app->register(AdminServiceProvider::class); + $app->register(ApiServiceProvider::class); + $app->register(BusServiceProvider::class); + $app->register(ConsoleServiceProvider::class); + $app->register(DatabaseServiceProvider::class); + $app->register(DiscussionServiceProvider::class); + $app->register(ExtensionServiceProvider::class); + $app->register(ErrorServiceProvider::class); + $app->register(FilesystemServiceProvider::class); + $app->register(FilterServiceProvider::class); + $app->register(FormatterServiceProvider::class); + $app->register(ForumServiceProvider::class); + $app->register(FrontendServiceProvider::class); + $app->register(GroupServiceProvider::class); + $app->register(HashServiceProvider::class); + $app->register(HttpServiceProvider::class); + $app->register(LocaleServiceProvider::class); + $app->register(MailServiceProvider::class); + $app->register(NotificationServiceProvider::class); + $app->register(PostServiceProvider::class); + $app->register(QueueServiceProvider::class); + $app->register(SearchServiceProvider::class); + $app->register(SessionServiceProvider::class); + $app->register(SettingsServiceProvider::class); + $app->register(UpdateServiceProvider::class); + $app->register(UserServiceProvider::class); + $app->register(ValidationServiceProvider::class); + $app->register(ViewServiceProvider::class); - $laravel->booting(function () use ($container) { + $app->booting(function () use ($app) { // Run all local-site extenders before booting service providers // (but after those from "real" extensions, which have been set up // in a service provider above). foreach ($this->extenders as $extension) { - $extension->extend($container); + $extension->extend($app); } }); - $laravel->boot(); + $app->boot(); - return $container; + return $app; } protected function getIlluminateConfig(): ConfigRepository diff --git a/framework/core/src/Foundation/UninstalledSite.php b/framework/core/src/Foundation/UninstalledSite.php index e933b8d04..3fea1ad7f 100644 --- a/framework/core/src/Foundation/UninstalledSite.php +++ b/framework/core/src/Foundation/UninstalledSite.php @@ -48,37 +48,36 @@ class UninstalledSite implements SiteInterface protected function bootLaravel(): Container { - $container = new \Illuminate\Container\Container; - $laravel = new Application($container, $this->paths); + $app = new Application($this->paths); - $container->instance('env', 'production'); - $container->instance('flarum.config', new Config(['url' => $this->baseUrl])); - $container->alias('flarum.config', Config::class); - $container->instance('flarum.debug', true); - $container->instance('config', $config = $this->getIlluminateConfig()); + $app->instance('env', 'production'); + $app->instance('flarum.config', new Config(['url' => $this->baseUrl])); + $app->alias('flarum.config', Config::class); + $app->instance('flarum.debug', true); + $app->instance('config', $config = $this->getIlluminateConfig()); - $this->registerLogger($container); + $this->registerLogger($app); - $laravel->register(ErrorServiceProvider::class); - $laravel->register(LocaleServiceProvider::class); - $laravel->register(FilesystemServiceProvider::class); - $laravel->register(SessionServiceProvider::class); - $laravel->register(ValidationServiceProvider::class); + $app->register(ErrorServiceProvider::class); + $app->register(LocaleServiceProvider::class); + $app->register(FilesystemServiceProvider::class); + $app->register(SessionServiceProvider::class); + $app->register(ValidationServiceProvider::class); - $laravel->register(InstallServiceProvider::class); + $app->register(InstallServiceProvider::class); - $container->singleton( + $app->singleton( SettingsRepositoryInterface::class, UninstalledSettingsRepository::class ); - $container->singleton('view', function ($container) { + $app->singleton('view', function ($app) { $engines = new EngineResolver(); - $engines->register('php', function () use ($container) { - return $container->make(PhpEngine::class); + $engines->register('php', function () use ($app) { + return $app->make(PhpEngine::class); }); - $finder = new FileViewFinder($container->make('files'), []); - $dispatcher = $container->make(Dispatcher::class); + $finder = new FileViewFinder($app->make('files'), []); + $dispatcher = $app->make(Dispatcher::class); return new \Illuminate\View\Factory( $engines, @@ -87,9 +86,9 @@ class UninstalledSite implements SiteInterface ); }); - $laravel->boot(); + $app->boot(); - return $container; + return $app; } protected function getIlluminateConfig(): ConfigRepository diff --git a/framework/core/src/helpers.php b/framework/core/src/helpers.php index 6df8ffa4c..b45ae2d86 100644 --- a/framework/core/src/helpers.php +++ b/framework/core/src/helpers.php @@ -7,8 +7,8 @@ * LICENSE file that was distributed with this source code. */ -use Flarum\Foundation\Container; use Flarum\Foundation\Paths; +use Illuminate\Container\Container; use Illuminate\Contracts\Config\Repository; if (! function_exists('resolve')) { diff --git a/php-packages/testing/src/integration/TestCase.php b/php-packages/testing/src/integration/TestCase.php index f2bdfb64a..4c773e262 100644 --- a/php-packages/testing/src/integration/TestCase.php +++ b/php-packages/testing/src/integration/TestCase.php @@ -46,6 +46,8 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase protected function app() { if (is_null($this->app)) { + $this->config('env', 'testing'); + $bootstrapper = new Bootstrapper( $this->config, $this->extensions,