From 7602b9ad62e7085878ce173d3cb62966446c533b Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 1 May 2020 14:23:44 +0200 Subject: [PATCH 1/3] Extract a class to hold / determine paths --- .../core/src/Foundation/InstalledSite.php | 25 +++-- framework/core/src/Foundation/Paths.php | 44 +++++++++ framework/core/src/Foundation/Site.php | 13 +-- .../core/src/Foundation/UninstalledSite.php | 17 ++-- .../core/tests/unit/Foundation/PathsTest.php | 94 +++++++++++++++++++ 5 files changed, 160 insertions(+), 33 deletions(-) create mode 100644 framework/core/src/Foundation/Paths.php create mode 100644 framework/core/tests/unit/Foundation/PathsTest.php diff --git a/framework/core/src/Foundation/InstalledSite.php b/framework/core/src/Foundation/InstalledSite.php index d61037767..2afb716c7 100644 --- a/framework/core/src/Foundation/InstalledSite.php +++ b/framework/core/src/Foundation/InstalledSite.php @@ -51,7 +51,7 @@ use Psr\Log\LoggerInterface; class InstalledSite implements SiteInterface { /** - * @var array + * @var Paths */ private $paths; @@ -65,7 +65,7 @@ class InstalledSite implements SiteInterface */ private $extenders = []; - public function __construct(array $paths, array $config) + public function __construct(Paths $paths, array $config) { $this->paths = $paths; $this->config = $config; @@ -97,13 +97,10 @@ class InstalledSite implements SiteInterface private function bootLaravel(): Application { - $laravel = new Application($this->paths['base'], $this->paths['public']); + $laravel = new Application($this->paths->base, $this->paths->public); - $laravel->useStoragePath($this->paths['storage']); - - if (isset($this->paths['vendor'])) { - $laravel->useVendorPath($this->paths['vendor']); - } + $laravel->useStoragePath($this->paths->storage); + $laravel->useVendorPath($this->paths->vendor); $laravel->instance('env', 'production'); $laravel->instance('flarum.config', $this->config); @@ -164,7 +161,7 @@ class InstalledSite implements SiteInterface return new ConfigRepository([ 'view' => [ 'paths' => [], - 'compiled' => $this->paths['storage'].'/views', + 'compiled' => $this->paths->storage.'/views', ], 'mail' => [ 'driver' => 'mail', @@ -175,18 +172,18 @@ class InstalledSite implements SiteInterface 'disks' => [ 'flarum-assets' => [ 'driver' => 'local', - 'root' => $this->paths['public'].'/assets', + 'root' => $this->paths->public.'/assets', 'url' => $app->url('assets') ], 'flarum-avatars' => [ 'driver' => 'local', - 'root' => $this->paths['public'].'/assets/avatars' + 'root' => $this->paths->public.'/assets/avatars' ] ] ], 'session' => [ 'lifetime' => 120, - 'files' => $this->paths['storage'].'/sessions', + 'files' => $this->paths->storage.'/sessions', 'cookie' => 'session' ] ]); @@ -194,7 +191,7 @@ class InstalledSite implements SiteInterface private function registerLogger(Application $app) { - $logPath = $this->paths['storage'].'/logs/flarum.log'; + $logPath = $this->paths->storage.'/logs/flarum.log'; $handler = new RotatingFileHandler($logPath, Logger::INFO); $handler->setFormatter(new LineFormatter(null, null, true, true)); @@ -210,7 +207,7 @@ class InstalledSite implements SiteInterface $app->alias('cache.store', Repository::class); $app->singleton('cache.filestore', function () { - return new FileStore(new Filesystem, $this->paths['storage'].'/cache'); + return new FileStore(new Filesystem, $this->paths->storage.'/cache'); }); $app->alias('cache.filestore', Store::class); } diff --git a/framework/core/src/Foundation/Paths.php b/framework/core/src/Foundation/Paths.php new file mode 100644 index 000000000..9f7186a4e --- /dev/null +++ b/framework/core/src/Foundation/Paths.php @@ -0,0 +1,44 @@ +paths = array_map(function ($path) { + return rtrim($path, '\/'); + }, $paths); + + // Assume a standard Composer directory structure unless specified + $this->paths['vendor'] = $this->vendor ?? $this->base.DIRECTORY_SEPARATOR.'vendor'; + } + + public function __get($name): ?string + { + return $this->paths[$name] ?? null; + } +} diff --git a/framework/core/src/Foundation/Site.php b/framework/core/src/Foundation/Site.php index ba44d459c..13b715b2c 100644 --- a/framework/core/src/Foundation/Site.php +++ b/framework/core/src/Foundation/Site.php @@ -9,7 +9,6 @@ namespace Flarum\Foundation; -use InvalidArgumentException; use RuntimeException; class Site @@ -20,18 +19,14 @@ class Site */ public static function fromPaths(array $paths) { - if (! isset($paths['base'], $paths['public'], $paths['storage'])) { - throw new InvalidArgumentException( - 'Paths array requires keys base, public and storage' - ); - } + $paths = new Paths($paths); date_default_timezone_set('UTC'); - if (static::hasConfigFile($paths['base'])) { + if (static::hasConfigFile($paths->base)) { return ( - new InstalledSite($paths, static::loadConfig($paths['base'])) - )->extendWith(static::loadExtenders($paths['base'])); + new InstalledSite($paths, static::loadConfig($paths->base)) + )->extendWith(static::loadExtenders($paths->base)); } else { return new UninstalledSite($paths); } diff --git a/framework/core/src/Foundation/UninstalledSite.php b/framework/core/src/Foundation/UninstalledSite.php index 64c1b329c..5e8f61f58 100644 --- a/framework/core/src/Foundation/UninstalledSite.php +++ b/framework/core/src/Foundation/UninstalledSite.php @@ -30,11 +30,11 @@ use Psr\Log\LoggerInterface; class UninstalledSite implements SiteInterface { /** - * @var array + * @var Paths */ private $paths; - public function __construct(array $paths) + public function __construct(Paths $paths) { $this->paths = $paths; } @@ -53,13 +53,10 @@ class UninstalledSite implements SiteInterface private function bootLaravel(): Application { - $laravel = new Application($this->paths['base'], $this->paths['public']); + $laravel = new Application($this->paths->base, $this->paths->public); - $laravel->useStoragePath($this->paths['storage']); - - if (isset($this->paths['vendor'])) { - $laravel->useVendorPath($this->paths['vendor']); - } + $laravel->useStoragePath($this->paths->storage); + $laravel->useVendorPath($this->paths->vendor); $laravel->instance('env', 'production'); $laravel->instance('flarum.config', []); @@ -108,7 +105,7 @@ class UninstalledSite implements SiteInterface return new ConfigRepository([ 'session' => [ 'lifetime' => 120, - 'files' => $this->paths['storage'].'/sessions', + 'files' => $this->paths->storage.'/sessions', 'cookie' => 'session' ], 'view' => [ @@ -119,7 +116,7 @@ class UninstalledSite implements SiteInterface private function registerLogger(Application $app) { - $logPath = $this->paths['storage'].'/logs/flarum-installer.log'; + $logPath = $this->paths->storage.'/logs/flarum-installer.log'; $handler = new StreamHandler($logPath, Logger::DEBUG); $handler->setFormatter(new LineFormatter(null, null, true, true)); diff --git a/framework/core/tests/unit/Foundation/PathsTest.php b/framework/core/tests/unit/Foundation/PathsTest.php new file mode 100644 index 000000000..7e6faada5 --- /dev/null +++ b/framework/core/tests/unit/Foundation/PathsTest.php @@ -0,0 +1,94 @@ +expectException(InvalidArgumentException::class); + + new Paths([ + 'base' => '/var/www/flarum', + ]); + } + + /** @test */ + public function it_makes_paths_available_as_properties() + { + $paths = new Paths([ + 'base' => '/var/www/flarum', + 'public' => '/var/www/flarum/public', + 'storage' => '/var/www/flarum/storage', + ]); + + $this->assertEquals('/var/www/flarum', $paths->base); + $this->assertEquals('/var/www/flarum/public', $paths->public); + $this->assertEquals('/var/www/flarum/storage', $paths->storage); + } + + /** @test */ + public function it_derives_the_vendor_dir_from_the_base_path() + { + $paths = new Paths([ + 'base' => '/var/www/flarum', + 'public' => '/var/www/flarum/public', + 'storage' => '/var/www/flarum/storage', + ]); + + $this->assertEquals('/var/www/flarum/vendor', $paths->vendor); + } + + /** @test */ + public function it_allows_setting_a_custom_vendor_dir() + { + $paths = new Paths([ + 'base' => '/var/www/flarum', + 'public' => '/var/www/flarum/public', + 'storage' => '/var/www/flarum/storage', + 'vendor' => '/share/composer-vendor', + ]); + + $this->assertEquals('/share/composer-vendor', $paths->vendor); + } + + /** @test */ + public function it_strips_trailing_forward_slashes_from_paths() + { + $paths = new Paths([ + 'base' => '/var/www/flarum/', + 'public' => '/var/www/flarum/public/', + 'storage' => '/var/www/flarum/storage/', + ]); + + $this->assertEquals('/var/www/flarum', $paths->base); + $this->assertEquals('/var/www/flarum/public', $paths->public); + $this->assertEquals('/var/www/flarum/storage', $paths->storage); + } + + /** @test */ + public function it_strips_trailing_backslashes_from_paths() + { + $paths = new Paths([ + 'base' => 'C:\\flarum\\', + 'public' => 'C:\\flarum\\public\\', + 'storage' => 'C:\\flarum\\storage\\', + ]); + + $this->assertEquals('C:\\flarum', $paths->base); + $this->assertEquals('C:\\flarum\\public', $paths->public); + $this->assertEquals('C:\\flarum\\storage', $paths->storage); + } +} From bf526125eba5b9a8d89277e21cc6dfa3c7c7f559 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 1 May 2020 09:53:55 +0000 Subject: [PATCH 2/3] Split up Application and Container - Stop trying to implement Laravel's Application contract, which has no value for us. - Stop inheriting from the Container, injecting one works equally well and does not clutter up the interfaces. - Inject the Paths collection instead of unwrapping it again, for better encapsulation. This brings us one step closer toward upgrading our Laravel components (#2055), because we no longer need to adopt the changes to the Application contract. --- .../core/src/Admin/AdminServiceProvider.php | 9 +- framework/core/src/Api/ApiServiceProvider.php | 9 +- .../src/Database/DatabaseServiceProvider.php | 2 +- .../src/Database/MigrationServiceProvider.php | 8 +- .../Extension/ExtensionServiceProvider.php | 5 +- .../Formatter/FormatterServiceProvider.php | 2 +- .../core/src/Forum/ForumServiceProvider.php | 9 +- .../Foundation/AbstractServiceProvider.php | 9 +- framework/core/src/Foundation/Application.php | 421 ++---------------- .../core/src/Foundation/InstalledSite.php | 43 +- .../core/src/Foundation/UninstalledSite.php | 26 +- .../src/Frontend/FrontendServiceProvider.php | 4 +- framework/core/src/Http/Server.php | 2 + framework/core/src/Http/UrlGenerator.php | 5 + .../core/src/Locale/LocaleServiceProvider.php | 4 +- framework/core/tests/integration/TestCase.php | 5 +- 16 files changed, 110 insertions(+), 453 deletions(-) diff --git a/framework/core/src/Admin/AdminServiceProvider.php b/framework/core/src/Admin/AdminServiceProvider.php index 496ca5473..291144cee 100644 --- a/framework/core/src/Admin/AdminServiceProvider.php +++ b/framework/core/src/Admin/AdminServiceProvider.php @@ -12,7 +12,6 @@ namespace Flarum\Admin; use Flarum\Extension\Event\Disabled; use Flarum\Extension\Event\Enabled; use Flarum\Foundation\AbstractServiceProvider; -use Flarum\Foundation\Application; use Flarum\Foundation\ErrorHandling\Registry; use Flarum\Foundation\ErrorHandling\Reporter; use Flarum\Foundation\ErrorHandling\ViewFormatter; @@ -60,14 +59,14 @@ class AdminServiceProvider extends AbstractServiceProvider ]; }); - $this->app->singleton('flarum.admin.handler', function (Application $app) { + $this->app->singleton('flarum.admin.handler', function () { $pipe = new MiddlewarePipe; // All requests should first be piped through our global error handler $pipe->pipe(new HttpMiddleware\HandleErrors( - $app->make(Registry::class), - $app->inDebugMode() ? $app->make(WhoopsFormatter::class) : $app->make(ViewFormatter::class), - $app->tagged(Reporter::class) + $this->app->make(Registry::class), + $this->app['flarum']->inDebugMode() ? $this->app->make(WhoopsFormatter::class) : $this->app->make(ViewFormatter::class), + $this->app->tagged(Reporter::class) )); foreach ($this->app->make('flarum.admin.middleware') as $middleware) { diff --git a/framework/core/src/Api/ApiServiceProvider.php b/framework/core/src/Api/ApiServiceProvider.php index 69c839561..18fb23ab2 100644 --- a/framework/core/src/Api/ApiServiceProvider.php +++ b/framework/core/src/Api/ApiServiceProvider.php @@ -15,7 +15,6 @@ use Flarum\Api\Serializer\BasicDiscussionSerializer; use Flarum\Api\Serializer\NotificationSerializer; use Flarum\Event\ConfigureNotificationTypes; use Flarum\Foundation\AbstractServiceProvider; -use Flarum\Foundation\Application; use Flarum\Foundation\ErrorHandling\JsonApiFormatter; use Flarum\Foundation\ErrorHandling\Registry; use Flarum\Foundation\ErrorHandling\Reporter; @@ -56,13 +55,13 @@ class ApiServiceProvider extends AbstractServiceProvider ]; }); - $this->app->singleton('flarum.api.handler', function (Application $app) { + $this->app->singleton('flarum.api.handler', function () { $pipe = new MiddlewarePipe; $pipe->pipe(new HttpMiddleware\HandleErrors( - $app->make(Registry::class), - new JsonApiFormatter($app->inDebugMode()), - $app->tagged(Reporter::class) + $this->app->make(Registry::class), + new JsonApiFormatter($this->app['flarum']->inDebugMode()), + $this->app->tagged(Reporter::class) )); foreach ($this->app->make('flarum.api.middleware') as $middleware) { diff --git a/framework/core/src/Database/DatabaseServiceProvider.php b/framework/core/src/Database/DatabaseServiceProvider.php index d4cd418a7..6d32601f9 100644 --- a/framework/core/src/Database/DatabaseServiceProvider.php +++ b/framework/core/src/Database/DatabaseServiceProvider.php @@ -24,7 +24,7 @@ class DatabaseServiceProvider extends AbstractServiceProvider $this->app->singleton(Manager::class, function ($app) { $manager = new Manager($app); - $config = $app->config('database'); + $config = $this->app['flarum']->config('database'); $config['engine'] = 'InnoDB'; $config['prefix_indexes'] = true; diff --git a/framework/core/src/Database/MigrationServiceProvider.php b/framework/core/src/Database/MigrationServiceProvider.php index a3e55ecf6..fb536c9f9 100644 --- a/framework/core/src/Database/MigrationServiceProvider.php +++ b/framework/core/src/Database/MigrationServiceProvider.php @@ -10,7 +10,6 @@ namespace Flarum\Database; use Flarum\Foundation\AbstractServiceProvider; -use Flarum\Foundation\Application; use Illuminate\Filesystem\Filesystem; class MigrationServiceProvider extends AbstractServiceProvider @@ -24,8 +23,11 @@ class MigrationServiceProvider extends AbstractServiceProvider return new DatabaseMigrationRepository($app['flarum.db'], 'migrations'); }); - $this->app->bind(MigrationCreator::class, function (Application $app) { - return new MigrationCreator($app->make(Filesystem::class), $app->basePath()); + $this->app->bind(MigrationCreator::class, function () { + return new MigrationCreator( + $this->app->make(Filesystem::class), + $this->app->basePath() + ); }); } } diff --git a/framework/core/src/Extension/ExtensionServiceProvider.php b/framework/core/src/Extension/ExtensionServiceProvider.php index b8f45a531..582cc1098 100644 --- a/framework/core/src/Extension/ExtensionServiceProvider.php +++ b/framework/core/src/Extension/ExtensionServiceProvider.php @@ -11,7 +11,6 @@ namespace Flarum\Extension; use Flarum\Extension\Event\Disabling; use Flarum\Foundation\AbstractServiceProvider; -use Illuminate\Contracts\Container\Container; class ExtensionServiceProvider extends AbstractServiceProvider { @@ -27,8 +26,8 @@ class ExtensionServiceProvider extends AbstractServiceProvider // listener on the app rather than in the service provider's boot method // below, so that extensions have a chance to register things on the // container before the core boots up (and starts resolving services). - $this->app->booting(function (Container $app) { - $app->make('flarum.extensions')->extend($app); + $this->app['flarum']->booting(function () { + $this->app->make('flarum.extensions')->extend($this->app); }); } diff --git a/framework/core/src/Formatter/FormatterServiceProvider.php b/framework/core/src/Formatter/FormatterServiceProvider.php index 31428c3b2..31d48051d 100644 --- a/framework/core/src/Formatter/FormatterServiceProvider.php +++ b/framework/core/src/Formatter/FormatterServiceProvider.php @@ -24,7 +24,7 @@ class FormatterServiceProvider extends AbstractServiceProvider return new Formatter( new Repository($container->make('cache.filestore')), $container->make('events'), - $this->app->storagePath().'/formatter' + $this->app['flarum']->storagePath().'/formatter' ); }); diff --git a/framework/core/src/Forum/ForumServiceProvider.php b/framework/core/src/Forum/ForumServiceProvider.php index 5866a7289..4640df804 100644 --- a/framework/core/src/Forum/ForumServiceProvider.php +++ b/framework/core/src/Forum/ForumServiceProvider.php @@ -13,7 +13,6 @@ 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\ErrorHandling\Registry; use Flarum\Foundation\ErrorHandling\Reporter; use Flarum\Foundation\ErrorHandling\ViewFormatter; @@ -70,14 +69,14 @@ class ForumServiceProvider extends AbstractServiceProvider ]; }); - $this->app->singleton('flarum.forum.handler', function (Application $app) { + $this->app->singleton('flarum.forum.handler', function () { $pipe = new MiddlewarePipe; // All requests should first be piped through our global error handler $pipe->pipe(new HttpMiddleware\HandleErrors( - $app->make(Registry::class), - $app->inDebugMode() ? $app->make(WhoopsFormatter::class) : $app->make(ViewFormatter::class), - $app->tagged(Reporter::class) + $this->app->make(Registry::class), + $this->app['flarum']->inDebugMode() ? $this->app->make(WhoopsFormatter::class) : $this->app->make(ViewFormatter::class), + $this->app->tagged(Reporter::class) )); foreach ($this->app->make('flarum.forum.middleware') as $middleware) { diff --git a/framework/core/src/Foundation/AbstractServiceProvider.php b/framework/core/src/Foundation/AbstractServiceProvider.php index 48efb4c8c..59f2980a7 100644 --- a/framework/core/src/Foundation/AbstractServiceProvider.php +++ b/framework/core/src/Foundation/AbstractServiceProvider.php @@ -9,21 +9,22 @@ namespace Flarum\Foundation; +use Illuminate\Contracts\Container\Container; use Illuminate\Support\ServiceProvider; abstract class AbstractServiceProvider extends ServiceProvider { /** - * @var Application + * @var Container */ protected $app; /** - * @param Application $app + * @param Container $container */ - public function __construct(Application $app) + public function __construct(Container $container) { - parent::__construct($app); + $this->app = $container; } /** diff --git a/framework/core/src/Foundation/Application.php b/framework/core/src/Foundation/Application.php index fe9372b61..597235131 100644 --- a/framework/core/src/Foundation/Application.php +++ b/framework/core/src/Foundation/Application.php @@ -9,14 +9,12 @@ namespace Flarum\Foundation; -use Illuminate\Container\Container; -use Illuminate\Contracts\Foundation\Application as ApplicationContract; +use Illuminate\Contracts\Container\Container; use Illuminate\Events\EventServiceProvider; use Illuminate\Support\Arr; use Illuminate\Support\ServiceProvider; -use Illuminate\Support\Str; -class Application extends Container implements ApplicationContract +class Application { /** * The Flarum version. @@ -26,32 +24,18 @@ class Application extends Container implements ApplicationContract const VERSION = '0.1.0-beta.13-dev'; /** - * The base path for the Flarum installation. + * The IoC container for the Flarum application. * - * @var string + * @var Container */ - protected $basePath; + private $container; /** - * The public path for the Flarum installation. + * The paths for the Flarum installation. * - * @var string + * @var Paths */ - protected $publicPath; - - /** - * The custom storage path defined by the developer. - * - * @var string - */ - protected $storagePath; - - /** - * A custom vendor path to find dependencies in non-standard environments. - * - * @var string - */ - protected $vendorPath; + protected $paths; /** * Indicates if the application has "booted". @@ -88,34 +72,22 @@ class Application extends Container implements ApplicationContract */ protected $loadedProviders = []; - /** - * The deferred services and their providers. - * - * @var array - */ - protected $deferredServices = []; - /** * Create a new Flarum application instance. * - * @param string|null $basePath - * @param string|null $publicPath + * @param Container $container + * @param Paths $paths */ - public function __construct($basePath = null, $publicPath = null) + public function __construct(Container $container, Paths $paths) { + $this->container = $container; + $this->paths = $paths; + $this->registerBaseBindings(); - $this->registerBaseServiceProviders(); - $this->registerCoreContainerAliases(); - if ($basePath) { - $this->setBasePath($basePath); - } - - if ($publicPath) { - $this->setPublicPath($publicPath); - } + $this->bindPathsInContainer(); } /** @@ -125,7 +97,7 @@ class Application extends Container implements ApplicationContract */ public function config($key, $default = null) { - return Arr::get($this->make('flarum.config'), $key, $default); + return Arr::get($this->container->make('flarum.config'), $key, $default); } /** @@ -146,7 +118,7 @@ class Application extends Container implements ApplicationContract */ public function url($path = null) { - $config = $this->make('flarum.config'); + $config = $this->container->make('flarum.config'); $url = Arr::get($config, 'url', Arr::get($_SERVER, 'REQUEST_URI')); if (is_array($url)) { @@ -164,26 +136,21 @@ class Application extends Container implements ApplicationContract return $url; } - /** - * Get the version number of the application. - * - * @return string - */ - public function version() - { - return static::VERSION; - } - /** * Register the basic bindings into the container. */ protected function registerBaseBindings() { - static::setInstance($this); + \Illuminate\Container\Container::setInstance($this->container); - $this->instance('app', $this); + $this->container->instance('app', $this->container); + $this->container->alias('app', \Illluminate\Container\Container::class); - $this->instance(Container::class, $this); + $this->container->instance('flarum', $this); + $this->container->alias('flarum', self::class); + + $this->container->instance('flarum.paths', $this->paths); + $this->container->alias('flarum.paths', Paths::class); } /** @@ -191,37 +158,7 @@ class Application extends Container implements ApplicationContract */ protected function registerBaseServiceProviders() { - $this->register(new EventServiceProvider($this)); - } - - /** - * Set the base path for the application. - * - * @param string $basePath - * @return $this - */ - public function setBasePath($basePath) - { - $this->basePath = rtrim($basePath, '\/'); - - $this->bindPathsInContainer(); - - return $this; - } - - /** - * Set the public path for the application. - * - * @param string $publicPath - * @return $this - */ - public function setPublicPath($publicPath) - { - $this->publicPath = rtrim($publicPath, '\/'); - - $this->bindPathsInContainer(); - - return $this; + $this->register(new EventServiceProvider($this->container)); } /** @@ -232,7 +169,7 @@ class Application extends Container implements ApplicationContract protected function bindPathsInContainer() { foreach (['base', 'public', 'storage', 'vendor'] as $path) { - $this->instance('path.'.$path, $this->{$path.'Path'}()); + $this->container->instance('path.'.$path, $this->paths->$path); } } @@ -243,7 +180,7 @@ class Application extends Container implements ApplicationContract */ public function basePath() { - return $this->basePath; + return $this->paths->base; } /** @@ -253,7 +190,7 @@ class Application extends Container implements ApplicationContract */ public function publicPath() { - return $this->publicPath; + return $this->paths->public; } /** @@ -263,7 +200,7 @@ class Application extends Container implements ApplicationContract */ public function storagePath() { - return $this->storagePath ?: $this->basePath.DIRECTORY_SEPARATOR.'storage'; + return $this->paths->storage; } /** @@ -273,89 +210,7 @@ class Application extends Container implements ApplicationContract */ public function vendorPath() { - return $this->vendorPath ?: $this->basePath.DIRECTORY_SEPARATOR.'vendor'; - } - - /** - * Set the storage directory. - * - * @param string $path - * @return $this - */ - public function useStoragePath($path) - { - $this->storagePath = $path; - - $this->instance('path.storage', $path); - - return $this; - } - - /** - * Set the vendor directory. - * - * @param string $path - * @return $this - */ - public function useVendorPath($path) - { - $this->vendorPath = $path; - - $this->instance('path.vendor', $path); - - return $this; - } - - /** - * Get or check the current application environment. - * - * @param mixed - * @return string - */ - public function environment() - { - if (func_num_args() > 0) { - $patterns = is_array(func_get_arg(0)) ? func_get_arg(0) : func_get_args(); - - foreach ($patterns as $pattern) { - if (Str::is($pattern, $this['env'])) { - return true; - } - } - - return false; - } - - return $this['env']; - } - - /** - * Determine if we are running in the console. - * - * @return bool - */ - public function runningInConsole() - { - return php_sapi_name() == 'cli'; - } - - /** - * Determine if we are running unit tests. - * - * @return bool - */ - public function runningUnitTests() - { - return $this['env'] == 'testing'; - } - - /** - * Register all of the configured providers. - * - * @return void - */ - public function registerConfiguredProviders() - { + return $this->paths->vendor; } /** @@ -423,7 +278,7 @@ class Application extends Container implements ApplicationContract */ public function resolveProviderClass($provider) { - return new $provider($this); + return new $provider($this->container); } /** @@ -434,106 +289,13 @@ class Application extends Container implements ApplicationContract */ protected function markAsRegistered($provider) { - $this['events']->dispatch($class = get_class($provider), [$provider]); + $this->container['events']->dispatch($class = get_class($provider), [$provider]); $this->serviceProviders[] = $provider; $this->loadedProviders[$class] = true; } - /** - * Load and boot all of the remaining deferred providers. - */ - public function loadDeferredProviders() - { - // We will simply spin through each of the deferred providers and register each - // one and boot them if the application has booted. This should make each of - // the remaining services available to this application for immediate use. - foreach ($this->deferredServices as $service => $provider) { - $this->loadDeferredProvider($service); - } - - $this->deferredServices = []; - } - - /** - * Load the provider for a deferred service. - * - * @param string $service - */ - public function loadDeferredProvider($service) - { - if (! isset($this->deferredServices[$service])) { - return; - } - - $provider = $this->deferredServices[$service]; - - // If the service provider has not already been loaded and registered we can - // register it with the application and remove the service from this list - // of deferred services, since it will already be loaded on subsequent. - if (! isset($this->loadedProviders[$provider])) { - $this->registerDeferredProvider($provider, $service); - } - } - - /** - * Register a deferred provider and service. - * - * @param string $provider - * @param string $service - */ - public function registerDeferredProvider($provider, $service = null) - { - // Once the provider that provides the deferred service has been registered we - // will remove it from our local list of the deferred services with related - // providers so that this container does not try to resolve it out again. - if ($service) { - unset($this->deferredServices[$service]); - } - - $this->register($instance = new $provider($this)); - - if (! $this->booted) { - $this->booting(function () use ($instance) { - $this->bootProvider($instance); - }); - } - } - - /** - * Resolve the given type from the container. - * - * (Overriding Container::make) - * - * @param string $abstract - * @param array $parameters - * @return mixed - */ - public function make($abstract, array $parameters = []) - { - $abstract = $this->getAlias($abstract); - - if (isset($this->deferredServices[$abstract])) { - $this->loadDeferredProvider($abstract); - } - - return parent::make($abstract, $parameters); - } - - /** - * Determine if the given abstract type has been bound. - * - * (Overriding Container::bound) - * - * @param string $abstract - * @return bool - */ - public function bound($abstract) - { - return isset($this->deferredServices[$abstract]) || parent::bound($abstract); - } - /** * Determine if the application has booted. * @@ -578,7 +340,7 @@ class Application extends Container implements ApplicationContract protected function bootProvider(ServiceProvider $provider) { if (method_exists($provider, 'boot')) { - return $this->call([$provider, 'boot']); + return $this->container->call([$provider, 'boot']); } } @@ -621,96 +383,13 @@ class Application extends Container implements ApplicationContract } } - /** - * Get the path to the cached "compiled.php" file. - * - * @return string - */ - public function getCachedCompilePath() - { - return $this->basePath().'/bootstrap/cache/compiled.php'; - } - - /** - * Get the path to the cached services.json file. - * - * @return string - */ - public function getCachedServicesPath() - { - return $this->basePath().'/bootstrap/cache/services.json'; - } - - /** - * Determine if the application is currently down for maintenance. - * - * @return bool - */ - public function isDownForMaintenance() - { - return $this->config('offline'); - } - - /** - * Get the service providers that have been loaded. - * - * @return array - */ - public function getLoadedProviders() - { - return $this->loadedProviders; - } - - /** - * Get the application's deferred services. - * - * @return array - */ - public function getDeferredServices() - { - return $this->deferredServices; - } - - /** - * Set the application's deferred services. - * - * @param array $services - * @return void - */ - public function setDeferredServices(array $services) - { - $this->deferredServices = $services; - } - - /** - * Add an array of services to the application's deferred services. - * - * @param array $services - * @return void - */ - public function addDeferredServices(array $services) - { - $this->deferredServices = array_merge($this->deferredServices, $services); - } - - /** - * Determine if the given service is a deferred service. - * - * @param string $service - * @return bool - */ - public function isDeferredService($service) - { - return isset($this->deferredServices[$service]); - } - /** * Register the core class aliases in the container. */ public function registerCoreContainerAliases() { $aliases = [ - 'app' => [self::class, \Illuminate\Contracts\Container\Container::class, \Illuminate\Contracts\Foundation\Application::class, \Psr\Container\ContainerInterface::class], + 'app' => [\Illuminate\Contracts\Container\Container::class, \Illuminate\Contracts\Foundation\Application::class, \Psr\Container\ContainerInterface::class], 'blade.compiler' => [\Illuminate\View\Compilers\BladeCompiler::class], 'cache' => [\Illuminate\Cache\CacheManager::class, \Illuminate\Contracts\Cache\Factory::class], 'cache.store' => [\Illuminate\Cache\Repository::class, \Illuminate\Contracts\Cache\Repository::class], @@ -730,36 +409,8 @@ class Application extends Container implements ApplicationContract foreach ($aliases as $key => $aliases) { foreach ((array) $aliases as $alias) { - $this->alias($key, $alias); + $this->container->alias($key, $alias); } } } - - /** - * Flush the container of all bindings and resolved instances. - */ - public function flush() - { - parent::flush(); - - $this->loadedProviders = []; - } - - /** - * Get the path to the cached packages.php file. - * - * @return string - */ - public function getCachedPackagesPath() - { - return storage_path('app/cache/packages.php'); - } - - /** - * @return string - */ - public function resourcePath() - { - return storage_path('resources'); - } } diff --git a/framework/core/src/Foundation/InstalledSite.php b/framework/core/src/Foundation/InstalledSite.php index 2afb716c7..fd54a6488 100644 --- a/framework/core/src/Foundation/InstalledSite.php +++ b/framework/core/src/Foundation/InstalledSite.php @@ -95,19 +95,18 @@ class InstalledSite implements SiteInterface return $this; } - private function bootLaravel(): Application + private function bootLaravel(): Container { - $laravel = new Application($this->paths->base, $this->paths->public); + $container = new \Illuminate\Container\Container; + $laravel = new Application($container, $this->paths); - $laravel->useStoragePath($this->paths->storage); - $laravel->useVendorPath($this->paths->vendor); + $container->instance('env', 'production'); + $container->instance('flarum.config', $this->config); + $container->instance('flarum.debug', $laravel->inDebugMode()); + $container->instance('config', $config = $this->getIlluminateConfig($laravel)); - $laravel->instance('env', 'production'); - $laravel->instance('flarum.config', $this->config); - $laravel->instance('config', $config = $this->getIlluminateConfig($laravel)); - - $this->registerLogger($laravel); - $this->registerCache($laravel); + $this->registerLogger($container); + $this->registerCache($container); $laravel->register(AdminServiceProvider::class); $laravel->register(ApiServiceProvider::class); @@ -138,18 +137,18 @@ class InstalledSite implements SiteInterface $laravel->register(ValidationServiceProvider::class); $laravel->register(ViewServiceProvider::class); - $laravel->booting(function (Container $app) { + $laravel->booting(function () use ($container) { // 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($app); + $extension->extend($container); } }); $laravel->boot(); - return $laravel; + return $container; } /** @@ -189,26 +188,26 @@ class InstalledSite implements SiteInterface ]); } - private function registerLogger(Application $app) + private function registerLogger(Container $container) { $logPath = $this->paths->storage.'/logs/flarum.log'; $handler = new RotatingFileHandler($logPath, Logger::INFO); $handler->setFormatter(new LineFormatter(null, null, true, true)); - $app->instance('log', new Logger($app->environment(), [$handler])); - $app->alias('log', LoggerInterface::class); + $container->instance('log', new Logger('flarum', [$handler])); + $container->alias('log', LoggerInterface::class); } - private function registerCache(Application $app) + private function registerCache(Container $container) { - $app->singleton('cache.store', function ($app) { - return new CacheRepository($app->make('cache.filestore')); + $container->singleton('cache.store', function ($container) { + return new CacheRepository($container->make('cache.filestore')); }); - $app->alias('cache.store', Repository::class); + $container->alias('cache.store', Repository::class); - $app->singleton('cache.filestore', function () { + $container->singleton('cache.filestore', function () { return new FileStore(new Filesystem, $this->paths->storage.'/cache'); }); - $app->alias('cache.filestore', Store::class); + $container->alias('cache.filestore', Store::class); } } diff --git a/framework/core/src/Foundation/UninstalledSite.php b/framework/core/src/Foundation/UninstalledSite.php index 5e8f61f58..9b863c849 100644 --- a/framework/core/src/Foundation/UninstalledSite.php +++ b/framework/core/src/Foundation/UninstalledSite.php @@ -16,6 +16,7 @@ use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Settings\UninstalledSettingsRepository; use Flarum\User\SessionServiceProvider; use Illuminate\Config\Repository as ConfigRepository; +use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Filesystem\FilesystemServiceProvider; use Illuminate\Validation\ValidationServiceProvider; @@ -51,18 +52,17 @@ class UninstalledSite implements SiteInterface ); } - private function bootLaravel(): Application + private function bootLaravel(): Container { - $laravel = new Application($this->paths->base, $this->paths->public); + $container = new \Illuminate\Container\Container; + $laravel = new Application($container, $this->paths); - $laravel->useStoragePath($this->paths->storage); - $laravel->useVendorPath($this->paths->vendor); + $container->instance('env', 'production'); + $container->instance('flarum.config', []); + $container->instance('flarum.debug', $laravel->inDebugMode()); + $container->instance('config', $config = $this->getIlluminateConfig()); - $laravel->instance('env', 'production'); - $laravel->instance('flarum.config', []); - $laravel->instance('config', $config = $this->getIlluminateConfig()); - - $this->registerLogger($laravel); + $this->registerLogger($container); $laravel->register(ErrorServiceProvider::class); $laravel->register(LocaleServiceProvider::class); @@ -94,7 +94,7 @@ class UninstalledSite implements SiteInterface $laravel->boot(); - return $laravel; + return $container; } /** @@ -114,13 +114,13 @@ class UninstalledSite implements SiteInterface ]); } - private function registerLogger(Application $app) + private function registerLogger(Container $container) { $logPath = $this->paths->storage.'/logs/flarum-installer.log'; $handler = new StreamHandler($logPath, Logger::DEBUG); $handler->setFormatter(new LineFormatter(null, null, true, true)); - $app->instance('log', new Logger('Flarum Installer', [$handler])); - $app->alias('log', LoggerInterface::class); + $container->instance('log', new Logger('Flarum Installer', [$handler])); + $container->alias('log', LoggerInterface::class); } } diff --git a/framework/core/src/Frontend/FrontendServiceProvider.php b/framework/core/src/Frontend/FrontendServiceProvider.php index fd47ca738..6ba2980c3 100644 --- a/framework/core/src/Frontend/FrontendServiceProvider.php +++ b/framework/core/src/Frontend/FrontendServiceProvider.php @@ -24,11 +24,11 @@ class FrontendServiceProvider extends AbstractServiceProvider $assets = new Assets( $name, $this->app->make('filesystem')->disk('flarum-assets'), - $this->app->storagePath() + $this->app['flarum']->storagePath() ); $assets->setLessImportDirs([ - $this->app->vendorPath().'/components/font-awesome/less' => '' + $this->app['flarum']->vendorPath().'/components/font-awesome/less' => '' ]); $assets->css([$this, 'addBaseCss']); diff --git a/framework/core/src/Http/Server.php b/framework/core/src/Http/Server.php index 3ddb14e8c..88697f01d 100644 --- a/framework/core/src/Http/Server.php +++ b/framework/core/src/Http/Server.php @@ -73,6 +73,8 @@ class Server Flarum encountered a boot error ($type)
$message
thrown in $file on line $line + +
$error
ERROR; } } diff --git a/framework/core/src/Http/UrlGenerator.php b/framework/core/src/Http/UrlGenerator.php index 8da304fb4..4be39ab7d 100644 --- a/framework/core/src/Http/UrlGenerator.php +++ b/framework/core/src/Http/UrlGenerator.php @@ -18,6 +18,11 @@ class UrlGenerator */ protected $routes = []; + /** + * @var Application + */ + protected $app; + /** * @param Application $app */ diff --git a/framework/core/src/Locale/LocaleServiceProvider.php b/framework/core/src/Locale/LocaleServiceProvider.php index 1f8fed457..0ff5360b4 100644 --- a/framework/core/src/Locale/LocaleServiceProvider.php +++ b/framework/core/src/Locale/LocaleServiceProvider.php @@ -51,7 +51,7 @@ class LocaleServiceProvider extends AbstractServiceProvider $this->getDefaultLocale(), null, $this->getCacheDir(), - $this->app->inDebugMode() + $this->app['flarum.debug'] ); $translator->setFallbackLocales(['en']); @@ -73,6 +73,6 @@ class LocaleServiceProvider extends AbstractServiceProvider private function getCacheDir(): string { - return $this->app->storagePath().'/locale'; + return $this->app['flarum']->storagePath().'/locale'; } } diff --git a/framework/core/tests/integration/TestCase.php b/framework/core/tests/integration/TestCase.php index ea22a5a42..cf35bbcf1 100644 --- a/framework/core/tests/integration/TestCase.php +++ b/framework/core/tests/integration/TestCase.php @@ -11,6 +11,7 @@ namespace Flarum\Tests\integration; use Flarum\Extend\ExtenderInterface; use Flarum\Foundation\InstalledSite; +use Flarum\Foundation\Paths; use Illuminate\Database\ConnectionInterface; use Laminas\Diactoros\ServerRequest; use Psr\Http\Message\ResponseInterface; @@ -33,12 +34,12 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { if (is_null($this->app)) { $site = new InstalledSite( - [ + new Paths([ 'base' => __DIR__.'/tmp', 'vendor' => __DIR__.'/../../vendor', 'public' => __DIR__.'/tmp/public', 'storage' => __DIR__.'/tmp/storage', - ], + ]), include __DIR__.'/tmp/config.php' ); From 244f61f5d3f9eef4973af456ee17f425c2f28e7f Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 1 May 2020 14:54:08 +0200 Subject: [PATCH 3/3] Inject new Paths class instead of Application This (and similar work in other areas) will allow us to further reduce the API surface of the Application class. Separation of concerns etc. --- .../src/Database/Console/MigrateCommand.php | 15 ++++---- .../src/Database/MigrationServiceProvider.php | 3 +- .../core/src/Extension/ExtensionManager.php | 35 ++++++++----------- .../Foundation/Console/CacheClearCommand.php | 14 ++++---- 4 files changed, 31 insertions(+), 36 deletions(-) diff --git a/framework/core/src/Database/Console/MigrateCommand.php b/framework/core/src/Database/Console/MigrateCommand.php index 8e1357c3c..18631fac1 100644 --- a/framework/core/src/Database/Console/MigrateCommand.php +++ b/framework/core/src/Database/Console/MigrateCommand.php @@ -13,6 +13,7 @@ use Flarum\Console\AbstractCommand; use Flarum\Database\Migrator; use Flarum\Extension\ExtensionManager; use Flarum\Foundation\Application; +use Flarum\Foundation\Paths; use Flarum\Settings\SettingsRepositoryInterface; use Illuminate\Contracts\Container\Container; use Illuminate\Database\ConnectionInterface; @@ -26,18 +27,18 @@ class MigrateCommand extends AbstractCommand protected $container; /** - * @var Application + * @var Paths */ - protected $app; + protected $paths; /** * @param Container $container - * @param Application $application + * @param Paths $paths */ - public function __construct(Container $container, Application $application) + public function __construct(Container $container, Paths $paths) { $this->container = $container; - $this->app = $application; + $this->paths = $paths; parent::__construct(); } @@ -91,8 +92,8 @@ class MigrateCommand extends AbstractCommand $this->info('Publishing assets...'); $this->container->make('files')->copyDirectory( - $this->app->vendorPath().'/components/font-awesome/webfonts', - $this->app->publicPath().'/assets/fonts' + $this->paths->vendor.'/components/font-awesome/webfonts', + $this->paths->public.'/assets/fonts' ); } } diff --git a/framework/core/src/Database/MigrationServiceProvider.php b/framework/core/src/Database/MigrationServiceProvider.php index fb536c9f9..131522975 100644 --- a/framework/core/src/Database/MigrationServiceProvider.php +++ b/framework/core/src/Database/MigrationServiceProvider.php @@ -10,6 +10,7 @@ namespace Flarum\Database; use Flarum\Foundation\AbstractServiceProvider; +use Flarum\Foundation\Paths; use Illuminate\Filesystem\Filesystem; class MigrationServiceProvider extends AbstractServiceProvider @@ -26,7 +27,7 @@ class MigrationServiceProvider extends AbstractServiceProvider $this->app->bind(MigrationCreator::class, function () { return new MigrationCreator( $this->app->make(Filesystem::class), - $this->app->basePath() + $this->app->make(Paths::class)->base ); }); } diff --git a/framework/core/src/Extension/ExtensionManager.php b/framework/core/src/Extension/ExtensionManager.php index ac5dcfd8a..40136e348 100644 --- a/framework/core/src/Extension/ExtensionManager.php +++ b/framework/core/src/Extension/ExtensionManager.php @@ -15,7 +15,7 @@ use Flarum\Extension\Event\Disabling; use Flarum\Extension\Event\Enabled; use Flarum\Extension\Event\Enabling; use Flarum\Extension\Event\Uninstalled; -use Flarum\Foundation\Application; +use Flarum\Foundation\Paths; use Flarum\Settings\SettingsRepositoryInterface; use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Events\Dispatcher; @@ -29,7 +29,10 @@ class ExtensionManager { protected $config; - protected $app; + /** + * @var Paths + */ + protected $paths; protected $container; @@ -52,14 +55,14 @@ class ExtensionManager public function __construct( SettingsRepositoryInterface $config, - Application $app, + Paths $paths, Container $container, Migrator $migrator, Dispatcher $dispatcher, Filesystem $filesystem ) { $this->config = $config; - $this->app = $app; + $this->paths = $paths; $this->container = $container; $this->migrator = $migrator; $this->dispatcher = $dispatcher; @@ -71,11 +74,11 @@ class ExtensionManager */ public function getExtensions() { - if (is_null($this->extensions) && $this->filesystem->exists($this->app->vendorPath().'/composer/installed.json')) { + if (is_null($this->extensions) && $this->filesystem->exists($this->paths->vendor.'/composer/installed.json')) { $extensions = new Collection(); // Load all packages installed by composer. - $installed = json_decode($this->filesystem->get($this->app->vendorPath().'/composer/installed.json'), true); + $installed = json_decode($this->filesystem->get($this->paths->vendor.'/composer/installed.json'), true); // Composer 2.0 changes the structure of the installed.json manifest $installed = $installed['packages'] ?? $installed; @@ -86,8 +89,8 @@ class ExtensionManager } $path = isset($package['install-path']) - ? $this->getExtensionsDir().'/composer/'.$package['install-path'] - : $this->getExtensionsDir().'/'.Arr::get($package, 'name'); + ? $this->paths->vendor.'/composer/'.$package['install-path'] + : $this->paths->vendor.'/'.Arr::get($package, 'name'); // Instantiates an Extension object using the package path and composer.json file. $extension = new Extension($path, $package); @@ -203,7 +206,7 @@ class ExtensionManager if ($extension->hasAssets()) { $this->filesystem->copyDirectory( $extension->getPath().'/assets', - $this->app->publicPath().'/assets/extensions/'.$extension->getId() + $this->paths->public.'/assets/extensions/'.$extension->getId() ); } } @@ -215,7 +218,7 @@ class ExtensionManager */ protected function unpublishAssets(Extension $extension) { - $this->filesystem->deleteDirectory($this->app->publicPath().'/assets/extensions/'.$extension->getId()); + $this->filesystem->deleteDirectory($this->paths->public.'/assets/extensions/'.$extension->getId()); } /** @@ -227,7 +230,7 @@ class ExtensionManager */ public function getAsset(Extension $extension, $path) { - return $this->app->publicPath().'/assets/extensions/'.$extension->getId().$path; + return $this->paths->public.'/assets/extensions/'.$extension->getId().$path; } /** @@ -332,14 +335,4 @@ class ExtensionManager return isset($enabled[$extension]); } - - /** - * The extensions path. - * - * @return string - */ - protected function getExtensionsDir() - { - return $this->app->vendorPath(); - } } diff --git a/framework/core/src/Foundation/Console/CacheClearCommand.php b/framework/core/src/Foundation/Console/CacheClearCommand.php index 15e049724..1229f74fa 100644 --- a/framework/core/src/Foundation/Console/CacheClearCommand.php +++ b/framework/core/src/Foundation/Console/CacheClearCommand.php @@ -10,8 +10,8 @@ namespace Flarum\Foundation\Console; use Flarum\Console\AbstractCommand; -use Flarum\Foundation\Application; use Flarum\Foundation\Event\ClearingCache; +use Flarum\Foundation\Paths; use Illuminate\Contracts\Cache\Store; class CacheClearCommand extends AbstractCommand @@ -22,18 +22,18 @@ class CacheClearCommand extends AbstractCommand protected $cache; /** - * @var Application + * @var Paths */ - protected $app; + protected $paths; /** * @param Store $cache - * @param Application $app + * @param Paths $paths */ - public function __construct(Store $cache, Application $app) + public function __construct(Store $cache, Paths $paths) { $this->cache = $cache; - $this->app = $app; + $this->paths = $paths; parent::__construct(); } @@ -57,7 +57,7 @@ class CacheClearCommand extends AbstractCommand $this->cache->flush(); - $storagePath = $this->app->storagePath(); + $storagePath = $this->paths->storage; array_map('unlink', glob($storagePath.'/formatter/*')); array_map('unlink', glob($storagePath.'/locale/*'));