From a25527ef8d90b3e9cf252ba9c8cdd01286f58142 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Fri, 3 Jan 2025 01:36:42 +0000 Subject: [PATCH] Refactor loadAndCheck() Signed-off-by: Kamil Tekiela --- bin/console | 2 +- phpstan-baseline.neon | 6 ++--- src/Command/CacheWarmupCommand.php | 2 +- src/Config.php | 30 +++++++--------------- src/Http/Middleware/ConfigLoading.php | 2 +- tests/unit/AbstractTestCase.php | 4 +-- tests/unit/ApplicationTest.php | 2 +- tests/unit/ConfigTest.php | 36 ++++++--------------------- 8 files changed, 25 insertions(+), 59 deletions(-) diff --git a/bin/console b/bin/console index 0a8f57d1ff..6c2c325be2 100755 --- a/bin/console +++ b/bin/console @@ -28,7 +28,7 @@ if (! class_exists(Application::class)) { $containerBuilder = ContainerBuilder::getContainer(); $cfg['environment'] = 'production'; $config = new Config(); -$config->loadAndCheck(CONFIG_FILE); +$config->loadFromFile(CONFIG_FILE); $config->set('environment', $cfg['environment']); $dbi = DatabaseInterface::getInstanceForTest(new DbiDummy()); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 9fd1f8f2b9..9395488af6 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -355,7 +355,7 @@ parameters: path: src/Command/CacheWarmupCommand.php - - message: '#^Parameter \#1 \$source of method PhpMyAdmin\\Config\:\:loadAndCheck\(\) expects string\|null, mixed given\.$#' + message: '#^Parameter \#1 \$source of method PhpMyAdmin\\Config\:\:loadFromFile\(\) expects string\|null, mixed given\.$#' identifier: argument.type count: 1 path: src/Command/CacheWarmupCommand.php @@ -8470,7 +8470,7 @@ parameters: path: src/Http/Middleware/Authentication.php - - message: '#^Parameter \#1 \$source of method PhpMyAdmin\\Config\:\:loadAndCheck\(\) expects string\|null, mixed given\.$#' + message: '#^Parameter \#1 \$source of method PhpMyAdmin\\Config\:\:loadFromFile\(\) expects string\|null, mixed given\.$#' identifier: argument.type count: 1 path: src/Http/Middleware/ConfigLoading.php @@ -17740,7 +17740,7 @@ parameters: path: tests/unit/ConfigTest.php - - message: '#^Parameter \#1 \$source of method PhpMyAdmin\\Config\:\:loadAndCheck\(\) expects string\|null, mixed given\.$#' + message: '#^Parameter \#1 \$source of method PhpMyAdmin\\Config\:\:loadFromFile\(\) expects string\|null, mixed given\.$#' identifier: argument.type count: 1 path: tests/unit/ConfigTest.php diff --git a/src/Command/CacheWarmupCommand.php b/src/Command/CacheWarmupCommand.php index 4fd3160ade..584ed69c13 100644 --- a/src/Command/CacheWarmupCommand.php +++ b/src/Command/CacheWarmupCommand.php @@ -119,7 +119,7 @@ final class CacheWarmupCommand extends Command ): int { $output->writeln('Warming up the twig cache', OutputInterface::VERBOSITY_VERBOSE); $config = Config::getInstance(); - $config->loadAndCheck(CONFIG_FILE); + $config->loadFromFile(CONFIG_FILE); $config->settings['environment'] = $environment; $config->set('environment', $config->settings['environment']); DatabaseInterface::$instance = DatabaseInterface::getInstanceForTest(new DbiDummy(), $config); diff --git a/src/Config.php b/src/Config.php index f227ef9abe..7e7aac3d68 100644 --- a/src/Config.php +++ b/src/Config.php @@ -140,27 +140,19 @@ class Config * * @throws ConfigException */ - public function loadAndCheck(string|null $source = null): void + public function loadFromFile(string|null $source = null): void { if ($source !== null) { $this->setSource($source); } - $this->load(); - - // other settings, independent of config file, comes in - $this->checkSystem(); + if ($this->configFileExists()) { + $this->load(); + } $this->baseSettings = $this->settings; } - /** - * sets system and application settings - */ - public function checkSystem(): void - { - } - public function isGd2Available(): bool { if ($this->config->GD2Available === 'yes') { @@ -201,12 +193,8 @@ class Config * * @throws ConfigException */ - public function load(): bool + private function load(): void { - if (! $this->configFileExists()) { - return false; - } - /** @var mixed $cfg */ $cfg = []; @@ -226,12 +214,12 @@ class Config $this->sourceMtime = (int) filemtime($this->source); - if (is_array($cfg)) { - $this->config = new Settings($cfg); - $this->settings = array_replace_recursive($this->settings, $this->config->asArray()); + if (! is_array($cfg)) { + return; } - return true; + $this->config = new Settings($cfg); + $this->settings = array_replace_recursive($this->settings, $this->config->asArray()); } /** diff --git a/src/Http/Middleware/ConfigLoading.php b/src/Http/Middleware/ConfigLoading.php index e27531b064..0174565df4 100644 --- a/src/Http/Middleware/ConfigLoading.php +++ b/src/Http/Middleware/ConfigLoading.php @@ -28,7 +28,7 @@ final class ConfigLoading implements MiddlewareInterface public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { try { - $this->config->loadAndCheck(CONFIG_FILE); + $this->config->loadFromFile(CONFIG_FILE); } catch (ConfigException $exception) { // Disables template caching because the cache directory is not known yet. $this->template->disableCache(); diff --git a/tests/unit/AbstractTestCase.php b/tests/unit/AbstractTestCase.php index 942d1cea9a..b1d87e606f 100644 --- a/tests/unit/AbstractTestCase.php +++ b/tests/unit/AbstractTestCase.php @@ -86,7 +86,7 @@ abstract class AbstractTestCase extends TestCase protected function createConfig(): Config { $config = new Config(); - $config->loadAndCheck(); + $config->loadFromFile(); return $config; } @@ -95,7 +95,7 @@ abstract class AbstractTestCase extends TestCase { Config::$instance = null; $config = Config::getInstance(); - $config->loadAndCheck(); + $config->loadFromFile(); $config->set('environment', 'development'); } diff --git a/tests/unit/ApplicationTest.php b/tests/unit/ApplicationTest.php index a13f6daec3..d1a6cc5cb9 100644 --- a/tests/unit/ApplicationTest.php +++ b/tests/unit/ApplicationTest.php @@ -30,7 +30,7 @@ final class ApplicationTest extends AbstractTestCase $errorHandler = self::createStub(ErrorHandler::class); $config = self::createMock(Config::class); - $config->expects(self::once())->method('loadAndCheck') + $config->expects(self::once())->method('loadFromFile') ->willThrowException(new ConfigException('Failed to load phpMyAdmin configuration.')); $config->config = new Config\Settings([]); diff --git a/tests/unit/ConfigTest.php b/tests/unit/ConfigTest.php index 43043a6b0a..894350d9ed 100644 --- a/tests/unit/ConfigTest.php +++ b/tests/unit/ConfigTest.php @@ -62,12 +62,12 @@ class ConfigTest extends AbstractTestCase $_SESSION['is_git_revision'] = true; Config::$instance = null; $config = Config::getInstance(); - $config->loadAndCheck(CONFIG_FILE); + $config->loadFromFile(CONFIG_FILE); $config->settings['ProxyUrl'] = ''; //for testing file permissions $this->permTestObj = new Config(); - $this->permTestObj->loadAndCheck(ROOT_PATH . 'config.sample.inc.php'); + $this->permTestObj->loadFromFile(ROOT_PATH . 'config.sample.inc.php'); } /** @@ -96,7 +96,7 @@ class ConfigTest extends AbstractTestCase // Test loading an empty file does not change the default config $config = new Config(); - $config->loadAndCheck($tmpConfig); + $config->loadFromFile($tmpConfig); self::assertSame($defaultConfig->settings, $config->settings); self::assertEquals($defaultConfig->getSettings(), $config->getSettings()); @@ -109,7 +109,7 @@ PHP; // Test loading a config changes the setup $config = new Config(); - $config->loadAndCheck($tmpConfig); + $config->loadFromFile($tmpConfig); $defaultConfig->set('environment', 'development'); self::assertSame($defaultConfig->settings, $config->settings); self::assertArrayHasKey('environment', $config->settings); @@ -200,18 +200,6 @@ PHP; self::assertSame($config, $object->baseSettings); } - /** - * test for CheckConfigSource - */ - public function testConfigFileExists(): void - { - $this->object->setSource('unexisted.config.php'); - self::assertFalse($this->object->configFileExists()); - - $this->object->setSource(__DIR__ . '/../test_data/config.inc.php'); - self::assertTrue($this->object->configFileExists()); - } - /** * Test getting and setting config values */ @@ -360,24 +348,14 @@ PHP; ]; } - /** - * Tests loading of config file - * - * @param string $source File name of config to load - * @param bool $result Expected result of loading - */ #[DataProvider('configPaths')] - public function testLoad(string $source, bool $result): void + public function testConfigFileExists(string $source, bool $result): void { $this->object->setSource($source); - self::assertSame($result, $this->object->load()); + self::assertSame($result, $this->object->configFileExists()); } - /** - * return of config Paths - * - * @return array - */ + /** @return array */ public static function configPaths(): array { return [