Merge pull request #654 from hydephp/refactor-bootstrappers-and-impro…

…ve-yaml-environment-configuration-loading

Refactor bootstrappers and improve Yaml environment configuration loading

src/Foundation/ConsoleKernel.php

@@ -18,9 +18,14 @@ protected function bootstrappers(): array
         // We do this by swapping out the LoadConfiguration class with our own.
         // We also inject our Yaml configuration loading bootstrapper.
 
+        // First, we need to register our Yaml configuration repository,
+        // as this code executes before service providers are registered.
+        $this->app->singleton(Internal\YamlConfigurationRepository::class);
+
         return [
             \LaravelZero\Framework\Bootstrap\CoreBindings::class,
             \LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables::class,
+            \Hyde\Foundation\Internal\LoadYamlEnvironmentVariables::class,
             \Hyde\Foundation\Internal\LoadConfiguration::class,
             \Illuminate\Foundation\Bootstrap\HandleExceptions::class,
             \LaravelZero\Framework\Bootstrap\RegisterFacades::class,

src/Foundation/Internal/LoadYamlConfiguration.php

@@ -4,125 +4,51 @@
 
 namespace Hyde\Foundation\Internal;
 
-use Hyde\Hyde;
-use Hyde\Facades\Config;
 use Illuminate\Support\Arr;
-use Symfony\Component\Yaml\Yaml;
+use Hyde\Foundation\Application;
+use Illuminate\Config\Repository;
 
-use function array_key_first;
-use function file_get_contents;
 use function array_merge;
-use function file_exists;
 
 /**
  * @internal Bootstrap service that loads the YAML configuration file.
  *
- * @implements \LaravelZero\Framework\Contracts\BoostrapperContract [sic]
- *
  * @see docs/digging-deeper/customization.md#yaml-configuration
  *
  * It also supports loading multiple configuration namespaces, where a configuration namespace is defined
  * as a firs level entry in the service container configuration repository array, and corresponds
  * one-to-one with a file in the config directory, and a root-level key in the YAML file.
  *
- * This feature, by design, requires a top-level configuration entry to be present as 'hyde' in the YAML file.
+ * The namespace feature by design, requires a top-level configuration entry to be present as 'hyde' in the YAML file.
  * Existing config files will be parsed as normal, but can be migrated by indenting all entries by one level,
  * and adding a top-level 'hyde' key. Then additional namespaces can be added underneath as needed.
  */
 class LoadYamlConfiguration
 {
+    protected YamlConfigurationRepository $yaml;
     protected array $config;
-    protected array $yaml;
 
-    /**
-     * Performs a core task that needs to be performed on
-     * early stages of the framework.
-     */
-    public function bootstrap(): void
+    public function bootstrap(Application $app): void
     {
-        if ($this->hasYamlConfigFile()) {
-            $this->config = Config::all();
-            $this->yaml = $this->parseYamlFile();
-
-            $this->supportSettingSidebarHeaderFromSiteName();
-            $this->supportSettingRssFeedTitleFromSiteName();
+        $this->yaml = $app->make(YamlConfigurationRepository::class);
 
-            $this->mergeParsedConfiguration();
-
-            Config::set($this->config);
+        if ($this->yaml->hasYamlConfigFile()) {
+            tap($app->make('config'), function (Repository $config): void {
+                $this->config = $config->all();
+                $this->mergeParsedConfiguration();
+            })->set($this->config);
         }
     }
 
-    protected function hasYamlConfigFile(): bool
-    {
-        return file_exists(Hyde::path('hyde.yml'))
-            || file_exists(Hyde::path('hyde.yaml'));
-    }
-
-    /** @return array<string, scalar|array> */
-    protected function parseYamlFile(): array
-    {
-        return Arr::undot((array) Yaml::parse(file_get_contents($this->getFile())));
-    }
-
-    protected function getFile(): string
-    {
-        return file_exists(Hyde::path('hyde.yml'))
-            ? Hyde::path('hyde.yml')
-            : Hyde::path('hyde.yaml');
-    }
-
     protected function mergeParsedConfiguration(): void
     {
-        $yaml = $this->yaml;
-
-        // If the Yaml file contains namespaces, we merge those using more granular logic
-        // that only applies the namespace data to each configuration namespace.
-        if ($this->configurationContainsNamespaces()) {
-            /** @var array<string, array<string, scalar>> $yaml */
-            foreach ($yaml as $namespace => $data) {
-                $this->mergeConfiguration($namespace, Arr::undot((array) $data));
-            }
-        } else {
-            // Otherwise, we can merge using the default strategy, which is simply applying all the data to the hyde namespace.
-            $this->mergeConfiguration('hyde', $yaml);
-        }
-    }
-
-    protected function mergeConfiguration(string $namespace, array $yamlData): void
-    {
-        $this->config[$namespace] = array_merge(
-            $this->config[$namespace] ?? [],
-            $yamlData
-        );
-    }
-
-    protected function configurationContainsNamespaces(): bool
-    {
-        return array_key_first($this->yaml) === 'hyde';
-    }
-
-    private function supportSettingSidebarHeaderFromSiteName(): void
-    {
-        $sidebarHeaderIsNotSetInPhpConfig = ($this->config['docs']['sidebar']['header'] ?? null) === 'HydePHP Docs';
-        $siteNameFromYaml = $this->configurationContainsNamespaces() ? ($this->yaml['hyde']['name'] ?? null) : ($this->yaml['name'] ?? null);
-
-        if ($sidebarHeaderIsNotSetInPhpConfig) {
-            if ($siteNameFromYaml !== null) {
-                $this->config['docs']['sidebar']['header'] = $siteNameFromYaml.' Docs';
-            }
+        foreach ($this->yaml->getData() as $namespace => $data) {
+            $this->mergeConfiguration($namespace, Arr::undot($data ?: []));
         }
     }
 
-    private function supportSettingRssFeedTitleFromSiteName(): void
+    protected function mergeConfiguration(string $namespace, array $yaml): void
     {
-        $rssFeedTitleIsNotSetInPhpConfig = ($this->config['hyde']['rss']['description'] ?? null) === 'HydePHP RSS Feed';
-        $siteNameFromYaml = $this->configurationContainsNamespaces() ? ($this->yaml['hyde']['name'] ?? null) : ($this->yaml['name'] ?? null);
-
-        if ($rssFeedTitleIsNotSetInPhpConfig) {
-            if ($siteNameFromYaml !== null) {
-                $this->config['hyde']['rss']['description'] = $siteNameFromYaml.' RSS Feed';
-            }
-        }
+        $this->config[$namespace] = array_merge($this->config[$namespace] ?? [], $yaml);
     }
 }

src/Foundation/Internal/LoadYamlEnvironmentVariables.php

@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Hyde\Foundation\Internal;
+
+use Illuminate\Support\Env;
+use Hyde\Foundation\Application;
+
+use function filled;
+
+/**
+ * @internal Inject environment variables parsed from the YAML configuration file.
+ */
+class LoadYamlEnvironmentVariables
+{
+    protected YamlConfigurationRepository $yaml;
+
+    public function bootstrap(Application $app): void
+    {
+        $this->yaml = $app->make(YamlConfigurationRepository::class);
+
+        if ($this->yaml->hasYamlConfigFile()) {
+            $this->injectEnvironmentVariables();
+        }
+    }
+
+    protected function injectEnvironmentVariables(): void
+    {
+        if ($this->canInjectSiteNameEnvironmentVariable()) {
+            $this->injectSiteNameEnvironmentVariable();
+        }
+    }
+
+    protected function canInjectSiteNameEnvironmentVariable(): bool
+    {
+        return $this->yamlHasSiteNameSet() && ! $this->alreadyHasEnvironmentVariable();
+    }
+
+    protected function alreadyHasEnvironmentVariable(): bool
+    {
+        return filled(Env::get('SITE_NAME'));
+    }
+
+    protected function injectSiteNameEnvironmentVariable(): void
+    {
+        $name = $this->getSiteNameFromYaml();
+
+        Env::getRepository()->set('SITE_NAME', $name);
+    }
+
+    protected function yamlHasSiteNameSet(): bool
+    {
+        return isset($this->yaml->getData()['hyde']['name']);
+    }
+
+    protected function getSiteNameFromYaml(): string
+    {
+        return $this->yaml->getData()['hyde']['name'];
+    }
+}

src/Foundation/Internal/YamlConfigurationRepository.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Hyde\Foundation\Internal;
+
+use Hyde\Hyde;
+use Illuminate\Support\Arr;
+use Symfony\Component\Yaml\Yaml;
+
+use function file_exists;
+use function file_get_contents;
+use function array_key_first;
+
+/**
+ * @internal Contains shared logic for loading and parsing the YAML configuration file.
+ *
+ * @see LoadYamlEnvironmentVariables Which uses this repository to inject environment variables from the YAML configuration file.
+ * @see LoadYamlConfiguration Which uses this repository to merge the YAML configuration data with the existing configuration.
+ */
+class YamlConfigurationRepository
+{
+    protected false|string $file;
+    protected array $data;
+
+    public function __construct()
+    {
+        $this->file = $this->getFilePath();
+
+        if ($this->hasYamlConfigFile()) {
+            $data = $this->parseYamlFile();
+
+            if (! self::configurationContainsNamespaces($data)) {
+                $data = ['hyde' => $data];
+            }
+
+            $this->data = $data;
+        }
+    }
+
+    /** @return array<string, array<string, null|scalar|array>> */
+    public function getData(): array
+    {
+        return $this->data;
+    }
+
+    public function hasYamlConfigFile(): bool
+    {
+        return $this->file !== false;
+    }
+
+    protected function parseYamlFile(): array
+    {
+        return Arr::undot((array) Yaml::parse(file_get_contents($this->file)));
+    }
+
+    protected function getFilePath(): string|false
+    {
+        return match (true) {
+            file_exists(Hyde::path('hyde.yml')) => Hyde::path('hyde.yml'),
+            file_exists(Hyde::path('hyde.yaml')) => Hyde::path('hyde.yaml'),
+            default => false,
+        };
+    }
+
+    protected static function configurationContainsNamespaces(array $config): bool
+    {
+        return array_key_first($config) === 'hyde';
+    }
+}

tests/Feature/ConsoleKernelTest.php

@@ -55,6 +55,7 @@ public function testHydeBootstrapperInjections()
         $this->assertSame([
             \LaravelZero\Framework\Bootstrap\CoreBindings::class,
             \LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables::class,
+            \Hyde\Foundation\Internal\LoadYamlEnvironmentVariables::class,
             \Hyde\Foundation\Internal\LoadConfiguration::class,
             \Illuminate\Foundation\Bootstrap\HandleExceptions::class,
             \LaravelZero\Framework\Bootstrap\RegisterFacades::class,

tests/Feature/HighLevelYamlConfigurationFeatureTest.php

@@ -0,0 +1,85 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Hyde\Framework\Testing\Feature;
+
+use Hyde\Testing\TestCase;
+
+/**
+ * High level test for the Yaml configuration feature.
+ *
+ * @see \Hyde\Framework\Testing\Feature\YamlConfigurationFeatureTest
+ *
+ * @covers \Hyde\Foundation\Internal\LoadYamlConfiguration
+ * @covers \Hyde\Foundation\Internal\LoadYamlEnvironmentVariables
+ * @covers \Hyde\Foundation\Internal\YamlConfigurationRepository
+ */
+class HighLevelYamlConfigurationFeatureTest extends TestCase
+{
+    protected function setUp(): void
+    {
+        $this->setUpConfigurationBeforeApplicationBoots();
+
+        parent::setUp();
+    }
+
+    protected function tearDown(): void
+    {
+        unlink('hyde.yml');
+        unlink('config/custom.php');
+
+        $this->clearEnvVars();
+
+        parent::tearDown();
+    }
+
+    protected function setUpConfigurationBeforeApplicationBoots(): void
+    {
+        file_put_contents('hyde.yml', <<<'YAML'
+        hyde:
+            name: Yaml Site Name
+        docs:
+            sidebar_order:
+                # Reversed compared to the default order
+                - getting-started
+                - installation
+                - readme
+        custom:
+            setting_one: Override
+            setting_two: Added
+        YAML);
+
+        file_put_contents('config/custom.php', <<<'PHP'
+        <?php   
+        return [
+            'setting_one' => 'Default',
+            'setting_three' => 'Inherited'
+        ];
+        PHP);
+    }
+
+    public function testTestTheYamlConfigurationFeature()
+    {
+        $config = config()->all();
+
+        $this->assertSame('Yaml Site Name', env('SITE_NAME'));
+        $this->assertSame('Yaml Site Name', $_ENV['SITE_NAME']);
+        $this->assertSame('Yaml Site Name', $_SERVER['SITE_NAME']);
+
+        $this->assertSame('Yaml Site Name', $config['hyde']['name']);
+        $this->assertSame('Yaml Site Name Docs', $config['docs']['sidebar']['header']);
+        $this->assertSame(['getting-started', 'installation', 'readme'], $config['docs']['sidebar_order']);
+
+        $this->assertSame('Override', $config['custom']['setting_one']);
+        $this->assertSame('Added', $config['custom']['setting_two']);
+        $this->assertSame('Inherited', $config['custom']['setting_three']);
+    }
+
+    protected function clearEnvVars(): void
+    {
+        // Todo: Can we access loader? https://github.com/vlucas/phpdotenv/pull/107/files
+        putenv('SITE_NAME');
+        unset($_ENV['SITE_NAME'], $_SERVER['SITE_NAME']);
+    }
+}