From f8699990112a633ec269a53919bfd85c6b47bb5d Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 21 Aug 2020 14:41:45 +0200 Subject: [PATCH 1/3] Add a helper class for managing low-level config --- src/Foundation/Config.php | 75 +++++++++++++ tests/unit/Foundation/ConfigTest.php | 155 +++++++++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 src/Foundation/Config.php create mode 100644 tests/unit/Foundation/ConfigTest.php diff --git a/src/Foundation/Config.php b/src/Foundation/Config.php new file mode 100644 index 000000000..e540bedfa --- /dev/null +++ b/src/Foundation/Config.php @@ -0,0 +1,75 @@ +data = $data; + + $this->requireKeys('url'); + } + + public function url(): UriInterface + { + return new Uri(rtrim($this->data['url'], '/')); + } + + public function inDebugMode(): bool + { + return $this->data['debug'] ?? false; + } + + public function inMaintenanceMode(): bool + { + return $this->data['offline'] ?? false; + } + + private function requireKeys(...$keys) + { + foreach ($keys as $key) { + if (! array_key_exists($key, $this->data)) { + throw new InvalidArgumentException( + "Configuration is invalid without a $key key" + ); + } + } + } + + public function offsetGet($offset) + { + return Arr::get($this->data, $offset); + } + + public function offsetExists($offset) + { + return Arr::has($this->data, $offset); + } + + public function offsetSet($offset, $value) + { + throw new RuntimeException('The Config is immutable'); + } + + public function offsetUnset($offset) + { + throw new RuntimeException('The Config is immutable'); + } +} diff --git a/tests/unit/Foundation/ConfigTest.php b/tests/unit/Foundation/ConfigTest.php new file mode 100644 index 000000000..a80acf2b7 --- /dev/null +++ b/tests/unit/Foundation/ConfigTest.php @@ -0,0 +1,155 @@ +expectException(InvalidArgumentException::class); + + new Config([]); + } + + /** @test */ + public function it_wraps_base_url_in_value_object() + { + $config = new Config([ + 'url' => 'https://flarum.local/myforum/', + ]); + + $url = $config->url(); + $this->assertEquals('https', $url->getScheme()); + $this->assertEquals('/myforum', $url->getPath()); // Note that trailing slashes are removed + $this->assertEquals('https://flarum.local/myforum', (string) $url); + } + + /** @test */ + public function it_has_a_helper_for_debug_mode() + { + $config = new Config([ + 'url' => 'https://flarum.local', + 'debug' => false, + ]); + + $this->assertFalse($config->inDebugMode()); + + $config = new Config([ + 'url' => 'https://flarum.local', + 'debug' => true, + ]); + + $this->assertTrue($config->inDebugMode()); + } + + /** @test */ + public function it_turns_off_debug_mode_by_default() + { + $config = new Config([ + 'url' => 'https://flarum.local', + ]); + + $this->assertFalse($config->inDebugMode()); + } + + /** @test */ + public function it_has_a_helper_for_maintenance_mode() + { + $config = new Config([ + 'url' => 'https://flarum.local', + 'offline' => false, + ]); + + $this->assertFalse($config->inMaintenanceMode()); + + $config = new Config([ + 'url' => 'https://flarum.local', + 'offline' => true, + ]); + + $this->assertTrue($config->inMaintenanceMode()); + } + + /** @test */ + public function it_turns_off_maintenance_mode_by_default() + { + $config = new Config([ + 'url' => 'https://flarum.local', + ]); + + $this->assertFalse($config->inMaintenanceMode()); + } + + /** @test */ + public function it_exposes_additional_keys_via_array_access() + { + $config = new Config([ + 'url' => 'https://flarum.local', + 'custom_a' => 'b', + ]); + + $this->assertEquals('b', $config['custom_a']); + } + + /** @test */ + public function it_exposes_nested_keys_via_dot_syntax() + { + $config = new Config([ + 'url' => 'https://flarum.local', + 'nested' => [ + 'first' => '1', + 'second' => '2', + ], + ]); + + $this->assertEquals('1', $config['nested.first']); + $this->assertEquals('2', $config['nested.second']); + } + + /** @test */ + public function it_does_not_allow_mutation_via_array_access() + { + $config = new Config([ + 'url' => 'https://flarum.local', + 'custom_a' => 'b', + ]); + + try { + $config['custom_a'] = 'c'; + } catch (RuntimeException $_) { + } + + // Ensure the value was not changed + $this->assertEquals('b', $config['custom_a']); + } + + /** @test */ + public function it_does_not_allow_removal_via_array_access() + { + $config = new Config([ + 'url' => 'https://flarum.local', + 'custom_a' => 'b', + ]); + + try { + unset($config['custom_a']); + } catch (RuntimeException $_) { + } + + // Ensure the value was not changed + $this->assertEquals('b', $config['custom_a']); + } +} From 6639678fb22188b8a492cd960bc0e4d390efd54a Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Sat, 28 Mar 2020 11:03:19 +0100 Subject: [PATCH 2/3] Inject/use new config class where applicable --- src/Admin/AdminServiceProvider.php | 2 +- src/Api/ApiServiceProvider.php | 2 +- src/Api/Serializer/ForumSerializer.php | 13 +++++++------ src/Console/ConsoleServiceProvider.php | 2 ++ src/Forum/ForumServiceProvider.php | 2 +- src/Foundation/Console/InfoCommand.php | 13 +++++++------ src/Foundation/InstalledApp.php | 20 ++++++-------------- src/Frontend/Content/Assets.php | 10 +++++----- src/Http/CookieFactory.php | 21 ++++++++++----------- src/Update/Controller/UpdateController.php | 14 +++++++------- 10 files changed, 47 insertions(+), 52 deletions(-) diff --git a/src/Admin/AdminServiceProvider.php b/src/Admin/AdminServiceProvider.php index 103bd35df..8e8e598f7 100644 --- a/src/Admin/AdminServiceProvider.php +++ b/src/Admin/AdminServiceProvider.php @@ -63,7 +63,7 @@ class AdminServiceProvider extends AbstractServiceProvider $this->app->bind('flarum.admin.error_handler', function () { return new HttpMiddleware\HandleErrors( $this->app->make(Registry::class), - $this->app['flarum']->inDebugMode() ? $this->app->make(WhoopsFormatter::class) : $this->app->make(ViewFormatter::class), + $this->app['flarum.config']->inDebugMode() ? $this->app->make(WhoopsFormatter::class) : $this->app->make(ViewFormatter::class), $this->app->tagged(Reporter::class) ); }); diff --git a/src/Api/ApiServiceProvider.php b/src/Api/ApiServiceProvider.php index 4737d2e8c..83b74f1ce 100644 --- a/src/Api/ApiServiceProvider.php +++ b/src/Api/ApiServiceProvider.php @@ -59,7 +59,7 @@ class ApiServiceProvider extends AbstractServiceProvider $this->app->bind('flarum.api.error_handler', function () { return new HttpMiddleware\HandleErrors( $this->app->make(Registry::class), - new JsonApiFormatter($this->app['flarum']->inDebugMode()), + new JsonApiFormatter($this->app['flarum.config']->inDebugMode()), $this->app->tagged(Reporter::class) ); }); diff --git a/src/Api/Serializer/ForumSerializer.php b/src/Api/Serializer/ForumSerializer.php index 9fb0b3fa0..65d53b3fa 100644 --- a/src/Api/Serializer/ForumSerializer.php +++ b/src/Api/Serializer/ForumSerializer.php @@ -10,6 +10,7 @@ namespace Flarum\Api\Serializer; use Flarum\Foundation\Application; +use Flarum\Foundation\Config; use Flarum\Http\UrlGenerator; use Flarum\Settings\SettingsRepositoryInterface; @@ -21,9 +22,9 @@ class ForumSerializer extends AbstractSerializer protected $type = 'forums'; /** - * @var Application + * @var Config */ - protected $app; + protected $config; /** * @var SettingsRepositoryInterface @@ -36,13 +37,13 @@ class ForumSerializer extends AbstractSerializer protected $url; /** - * @param Application $app + * @param Config $config * @param SettingsRepositoryInterface $settings * @param UrlGenerator $url */ - public function __construct(Application $app, SettingsRepositoryInterface $settings, UrlGenerator $url) + public function __construct(Config $config, SettingsRepositoryInterface $settings, UrlGenerator $url) { - $this->app = $app; + $this->config = $config; $this->settings = $settings; $this->url = $url; } @@ -66,7 +67,7 @@ class ForumSerializer extends AbstractSerializer 'showLanguageSelector' => (bool) $this->settings->get('show_language_selector', true), 'baseUrl' => $url = $this->url->to('forum')->base(), 'basePath' => parse_url($url, PHP_URL_PATH) ?: '', - 'debug' => $this->app->inDebugMode(), + 'debug' => $this->config->inDebugMode(), 'apiUrl' => $this->url->to('api')->base(), 'welcomeTitle' => $this->settings->get('welcome_title'), 'welcomeMessage' => $this->settings->get('welcome_message'), diff --git a/src/Console/ConsoleServiceProvider.php b/src/Console/ConsoleServiceProvider.php index 674927538..0a6ccb5da 100644 --- a/src/Console/ConsoleServiceProvider.php +++ b/src/Console/ConsoleServiceProvider.php @@ -14,6 +14,7 @@ use Flarum\Database\Console\MigrateCommand; use Flarum\Database\Console\ResetCommand; use Flarum\Foundation\AbstractServiceProvider; use Flarum\Foundation\Console\CacheClearCommand; +use Flarum\Foundation\Console\InfoCommand; class ConsoleServiceProvider extends AbstractServiceProvider { @@ -26,6 +27,7 @@ class ConsoleServiceProvider extends AbstractServiceProvider return [ CacheClearCommand::class, GenerateMigrationCommand::class, + InfoCommand::class, MigrateCommand::class, ResetCommand::class, ]; diff --git a/src/Forum/ForumServiceProvider.php b/src/Forum/ForumServiceProvider.php index 1903783d6..bbe609fa5 100644 --- a/src/Forum/ForumServiceProvider.php +++ b/src/Forum/ForumServiceProvider.php @@ -73,7 +73,7 @@ class ForumServiceProvider extends AbstractServiceProvider $this->app->bind('flarum.forum.error_handler', function () { return new HttpMiddleware\HandleErrors( $this->app->make(Registry::class), - $this->app['flarum']->inDebugMode() ? $this->app->make(WhoopsFormatter::class) : $this->app->make(ViewFormatter::class), + $this->app['flarum.config']->inDebugMode() ? $this->app->make(WhoopsFormatter::class) : $this->app->make(ViewFormatter::class), $this->app->tagged(Reporter::class) ); }); diff --git a/src/Foundation/Console/InfoCommand.php b/src/Foundation/Console/InfoCommand.php index cfe7671f8..a0f8d4773 100644 --- a/src/Foundation/Console/InfoCommand.php +++ b/src/Foundation/Console/InfoCommand.php @@ -12,6 +12,7 @@ namespace Flarum\Foundation\Console; use Flarum\Console\AbstractCommand; use Flarum\Extension\ExtensionManager; use Flarum\Foundation\Application; +use Flarum\Foundation\Config; use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Helper\TableStyle; @@ -23,15 +24,15 @@ class InfoCommand extends AbstractCommand protected $extensions; /** - * @var array + * @var Config */ protected $config; /** * @param ExtensionManager $extensions - * @param array $config + * @param Config config */ - public function __construct(ExtensionManager $extensions, array $config) + public function __construct(ExtensionManager $extensions, Config $config) { $this->extensions = $extensions; $this->config = $config; @@ -64,11 +65,11 @@ class InfoCommand extends AbstractCommand $this->getExtensionTable()->render(); - $this->output->writeln('Base URL: '.$this->config['url']); + $this->output->writeln('Base URL: '.$this->config->url()); $this->output->writeln('Installation path: '.getcwd()); - $this->output->writeln('Debug mode: '.($this->config['debug'] ? 'ON' : 'off')); + $this->output->writeln('Debug mode: '.($this->config->inDebugMode() ? 'ON' : 'off')); - if ($this->config['debug']) { + if ($this->config->inDebugMode()) { $this->error( "Don't forget to turn off debug mode! It should never be turned on in a production system." ); diff --git a/src/Foundation/InstalledApp.php b/src/Foundation/InstalledApp.php index bc1e18601..f2a024235 100644 --- a/src/Foundation/InstalledApp.php +++ b/src/Foundation/InstalledApp.php @@ -9,7 +9,6 @@ namespace Flarum\Foundation; -use Flarum\Foundation\Console\InfoCommand; use Flarum\Http\Middleware\DispatchRoute; use Flarum\Settings\SettingsRepositoryInterface; use Illuminate\Console\Command; @@ -112,21 +111,14 @@ class InstalledApp implements AppInterface */ public function getConsoleCommands() { - $commands = []; + return array_map(function ($command) { + $command = $this->container->make($command); - // The info command is a special case, as it requires a config parameter that's only available here. - $commands[] = $this->container->make(InfoCommand::class, ['config' => $this->config]); - - foreach ($this->container->make('flarum.console.commands') as $command) { - $newCommand = $this->container->make($command); - - if ($newCommand instanceof Command) { - $newCommand->setLaravel($this->container); + if ($command instanceof Command) { + $command->setLaravel($this->container); } - $commands[] = $newCommand; - } - - return $commands; + return $command; + }, $this->container->make('flarum.console.commands')); } } diff --git a/src/Frontend/Content/Assets.php b/src/Frontend/Content/Assets.php index 50681f876..009973165 100644 --- a/src/Frontend/Content/Assets.php +++ b/src/Frontend/Content/Assets.php @@ -9,7 +9,7 @@ namespace Flarum\Frontend\Content; -use Flarum\Foundation\Application; +use Flarum\Foundation\Config; use Flarum\Frontend\Compiler\CompilerInterface; use Flarum\Frontend\Document; use Illuminate\Contracts\Container\Container; @@ -19,17 +19,17 @@ use Psr\Http\Message\ServerRequestInterface as Request; class Assets { protected $container; - protected $app; + protected $config; /** * @var \Flarum\Frontend\Assets */ protected $assets; - public function __construct(Container $container, Application $app) + public function __construct(Container $container, Config $config) { $this->container = $container; - $this->app = $app; + $this->config = $config; } public function forFrontend(string $name) @@ -48,7 +48,7 @@ class Assets 'css' => [$this->assets->makeCss(), $this->assets->makeLocaleCss($locale)] ]; - if ($this->app->inDebugMode()) { + if ($this->config->inDebugMode()) { $this->commit(Arr::flatten($compilers)); } diff --git a/src/Http/CookieFactory.php b/src/Http/CookieFactory.php index 4a9eff3a3..5c36cb39b 100644 --- a/src/Http/CookieFactory.php +++ b/src/Http/CookieFactory.php @@ -11,8 +11,7 @@ namespace Flarum\Http; use Dflydev\FigCookies\Modifier\SameSite; use Dflydev\FigCookies\SetCookie; -use Flarum\Foundation\Application; -use Illuminate\Support\Arr; +use Flarum\Foundation\Config; class CookieFactory { @@ -52,19 +51,19 @@ class CookieFactory protected $samesite; /** - * @param Application $app + * @param Config $config */ - public function __construct(Application $app) + public function __construct(Config $config) { - // Parse the forum's base URL so that we can determine the optimal cookie settings - $url = parse_url(rtrim($app->url(), '/')); + // If necessary, we will use the forum's base URL to determine smart defaults for cookie settings + $url = $config->url(); // Get the cookie settings from the config or use the default values - $this->prefix = $app->config('cookie.name', 'flarum'); - $this->path = $app->config('cookie.path', Arr::get($url, 'path') ?: '/'); - $this->domain = $app->config('cookie.domain'); - $this->secure = $app->config('cookie.secure', Arr::get($url, 'scheme') === 'https'); - $this->samesite = $app->config('cookie.samesite'); + $this->prefix = $config['cookie.name'] ?? 'flarum'; + $this->path = $config['cookie.path'] ?? $url->getPath() ?: '/'; + $this->domain = $config['cookie.domain']; + $this->secure = $config['cookie.secure'] ?? $url->getScheme() === 'https'; + $this->samesite = $config['cookie.samesite']; } /** diff --git a/src/Update/Controller/UpdateController.php b/src/Update/Controller/UpdateController.php index 4668fcebb..5b5bf4959 100644 --- a/src/Update/Controller/UpdateController.php +++ b/src/Update/Controller/UpdateController.php @@ -11,7 +11,7 @@ namespace Flarum\Update\Controller; use Exception; use Flarum\Database\Console\MigrateCommand; -use Flarum\Foundation\Application; +use Flarum\Foundation\Config; use Illuminate\Support\Arr; use Laminas\Diactoros\Response; use Laminas\Diactoros\Response\HtmlResponse; @@ -26,18 +26,18 @@ class UpdateController implements RequestHandlerInterface protected $command; /** - * @var Application + * @var Config */ - protected $app; + protected $config; /** * @param MigrateCommand $command - * @param Application $app + * @param Config $config */ - public function __construct(MigrateCommand $command, Application $app) + public function __construct(MigrateCommand $command, Config $config) { $this->command = $command; - $this->app = $app; + $this->config = $config; } /** @@ -48,7 +48,7 @@ class UpdateController implements RequestHandlerInterface { $input = $request->getParsedBody(); - if (Arr::get($input, 'databasePassword') !== $this->app->config('database.password')) { + if (Arr::get($input, 'databasePassword') !== $this->config['database.password']) { return new HtmlResponse('Incorrect database password.', 500); } From 9ea57e6329da9abcd3df482e8904ac790fb9086e Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 21 Aug 2020 18:21:33 +0200 Subject: [PATCH 3/3] Use Config class for data from config.php --- src/Foundation/Application.php | 16 +++++----------- src/Foundation/InstalledApp.php | 13 ++++--------- src/Foundation/InstalledSite.php | 7 ++++--- src/Foundation/Site.php | 16 ++++++++-------- src/Foundation/UninstalledSite.php | 13 ++++++++++--- tests/integration/TestCase.php | 3 ++- 6 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/Foundation/Application.php b/src/Foundation/Application.php index 7463b2f50..ffc350256 100644 --- a/src/Foundation/Application.php +++ b/src/Foundation/Application.php @@ -95,7 +95,9 @@ class Application */ public function config($key, $default = null) { - return Arr::get($this->container->make('flarum.config'), $key, $default); + $config = $this->container->make('flarum.config'); + + return $config[$key] ?? $default; } /** @@ -117,18 +119,10 @@ class Application public function url($path = null) { $config = $this->container->make('flarum.config'); - $url = Arr::get($config, 'url', Arr::get($_SERVER, 'REQUEST_URI')); - - if (is_array($url)) { - if (isset($url[$path])) { - return $url[$path]; - } - - $url = $url['base']; - } + $url = (string) $config->url(); if ($path) { - $url .= '/'.Arr::get($config, "paths.$path", $path); + $url .= '/'.($config["paths.$path"] ?? $path); } return $url; diff --git a/src/Foundation/InstalledApp.php b/src/Foundation/InstalledApp.php index f2a024235..705156292 100644 --- a/src/Foundation/InstalledApp.php +++ b/src/Foundation/InstalledApp.php @@ -27,11 +27,11 @@ class InstalledApp implements AppInterface protected $container; /** - * @var array + * @var Config */ protected $config; - public function __construct(Container $container, array $config) + public function __construct(Container $container, Config $config) { $this->container = $container; $this->config = $config; @@ -47,7 +47,7 @@ class InstalledApp implements AppInterface */ public function getRequestHandler() { - if ($this->inMaintenanceMode()) { + if ($this->config->inMaintenanceMode()) { return new MaintenanceModeHandler(); } elseif ($this->needsUpdate()) { return $this->getUpdaterHandler(); @@ -69,11 +69,6 @@ class InstalledApp implements AppInterface return $pipe; } - protected function inMaintenanceMode(): bool - { - return $this->config['offline'] ?? false; - } - protected function needsUpdate(): bool { $settings = $this->container->make(SettingsRepositoryInterface::class); @@ -98,7 +93,7 @@ class InstalledApp implements AppInterface protected function basePath(): string { - return parse_url($this->config['url'], PHP_URL_PATH) ?: '/'; + return $this->config->url()->getPath() ?: '/'; } protected function subPath($pathName): string diff --git a/src/Foundation/InstalledSite.php b/src/Foundation/InstalledSite.php index 4f9fbb265..8e944e827 100644 --- a/src/Foundation/InstalledSite.php +++ b/src/Foundation/InstalledSite.php @@ -55,7 +55,7 @@ class InstalledSite implements SiteInterface protected $paths; /** - * @var array + * @var Config */ protected $config; @@ -64,7 +64,7 @@ class InstalledSite implements SiteInterface */ protected $extenders = []; - public function __construct(Paths $paths, array $config) + public function __construct(Paths $paths, Config $config) { $this->paths = $paths; $this->config = $config; @@ -101,7 +101,8 @@ class InstalledSite implements SiteInterface $container->instance('env', 'production'); $container->instance('flarum.config', $this->config); - $container->instance('flarum.debug', $laravel->inDebugMode()); + $container->alias('flarum.config', Config::class); + $container->instance('flarum.debug', $this->config->inDebugMode()); $container->instance('config', $config = $this->getIlluminateConfig($laravel)); $this->registerLogger($container); diff --git a/src/Foundation/Site.php b/src/Foundation/Site.php index 8c118196d..c77868467 100644 --- a/src/Foundation/Site.php +++ b/src/Foundation/Site.php @@ -24,13 +24,13 @@ class Site date_default_timezone_set('UTC'); - if (static::hasConfigFile($paths->base)) { - return ( - new InstalledSite($paths, static::loadConfig($paths->base)) - )->extendWith(static::loadExtenders($paths->base)); - } else { - return new UninstalledSite($paths); + if (! static::hasConfigFile($paths->base)) { + return new UninstalledSite($paths, $_SERVER['REQUEST_URI']); } + + return ( + new InstalledSite($paths, static::loadConfig($paths->base)) + )->extendWith(static::loadExtenders($paths->base)); } protected static function hasConfigFile($basePath) @@ -38,7 +38,7 @@ class Site return file_exists("$basePath/config.php"); } - protected static function loadConfig($basePath): array + protected static function loadConfig($basePath): Config { $config = include "$basePath/config.php"; @@ -46,7 +46,7 @@ class Site throw new RuntimeException('config.php should return an array'); } - return $config; + return new Config($config); } protected static function loadExtenders($basePath): array diff --git a/src/Foundation/UninstalledSite.php b/src/Foundation/UninstalledSite.php index ac1ed279b..17ed2be1e 100644 --- a/src/Foundation/UninstalledSite.php +++ b/src/Foundation/UninstalledSite.php @@ -35,9 +35,15 @@ class UninstalledSite implements SiteInterface */ protected $paths; - public function __construct(Paths $paths) + /** + * @var string + */ + private $baseUrl; + + public function __construct(Paths $paths, string $baseUrl) { $this->paths = $paths; + $this->baseUrl = $baseUrl; } /** @@ -58,8 +64,9 @@ class UninstalledSite implements SiteInterface $laravel = new Application($container, $this->paths); $container->instance('env', 'production'); - $container->instance('flarum.config', []); - $container->instance('flarum.debug', $laravel->inDebugMode()); + $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()); $this->registerLogger($container); diff --git a/tests/integration/TestCase.php b/tests/integration/TestCase.php index d7253f45c..16a65f07e 100644 --- a/tests/integration/TestCase.php +++ b/tests/integration/TestCase.php @@ -10,6 +10,7 @@ namespace Flarum\Tests\integration; use Flarum\Extend\ExtenderInterface; +use Flarum\Foundation\Config; use Flarum\Foundation\InstalledSite; use Flarum\Foundation\Paths; use Illuminate\Database\ConnectionInterface; @@ -40,7 +41,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase 'public' => __DIR__.'/tmp/public', 'storage' => __DIR__.'/tmp/storage', ]), - include __DIR__.'/tmp/config.php' + new Config(include __DIR__.'/tmp/config.php') ); $site->extendWith($this->extenders);