Move Config::loadDefaults() to constructor

Makes sure that Config's properties are always initialized and that the config property is in sync with the settings property. Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
phpmyadmin · Apr 28, 2023 · 56926e8 · 56926e8
1 parent fd88454
commit 56926e8
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 58 deletions.
diff --git a/libraries/classes/Config.php b/libraries/classes/Config.php
@@ -75,13 +75,13 @@
 class Config
 {
     /** @var mixed[]   default configuration settings */
-    public array $default = [];
+    public array $default;
 
     /** @var mixed[]   configuration settings, without user preferences applied */
-    public array $baseSettings = [];
+    public array $baseSettings;
 
     /** @var mixed[]   configuration settings */
-    public array $settings = [];
+    public array $settings;
 
     /** @var string  config source */
     public string $source = '';
@@ -92,11 +92,22 @@ class Config
     public bool $errorConfigFile = false;
 
     /** @var mixed[] */
-    public array $defaultServer = [];
+    public array $defaultServer;
 
     private bool $isHttps = false;
 
-    private Settings|null $config = null;
+    private Settings $config;
+
+    public function __construct()
+    {
+        $this->config = new Settings([]);
+        $this->defaultServer = $this->config->Servers[1]->asArray();
+        $config = $this->config->asArray();
+        unset($config['Servers']);
+        $this->default = $config;
+        $this->settings = $config;
+        $this->baseSettings = $config;
+    }
 
     /**
      * @param string|null $source source to read config from
@@ -105,7 +116,7 @@ class Config
      */
     public function loadAndCheck(string|null $source = null): void
     {
-        $this->settings = ['is_setup' => false];
+        $this->settings['is_setup'] = false;
 
         // functions need to refresh in case of config file changed goes in PhpMyAdmin\Config::load()
         $this->load($source);
@@ -317,22 +328,6 @@ public function checkWebServerOs(): void
         }
     }
 
-    /**
-     * loads default values from default source
-     */
-    public function loadDefaults(): void
-    {
-        $settings = new Settings([]);
-        $cfg = $settings->asArray();
-
-        // phpcs:ignore Squiz.NamingConventions.ValidVariableName.MemberNotCamelCaps
-        $this->defaultServer = $settings->Servers[1]->asArray();
-        unset($cfg['Servers']);
-
-        $this->default = $cfg;
-        $this->settings = array_replace_recursive($this->settings, $cfg);
-    }
-
     /**
      * loads configuration from $source, usually the config file
      * should be called on object creation
@@ -343,8 +338,6 @@ public function loadDefaults(): void
      */
     public function load(string|null $source = null): bool
     {
-        $this->loadDefaults();
-
         if ($source !== null) {
             $this->setSource($source);
         }
@@ -404,6 +397,7 @@ public function load(string|null $source = null): bool
         );
 
         $this->settings = array_replace_recursive($this->settings, $cfg);
+        $this->config = new Settings($cfg);
 
         return true;
     }
@@ -460,6 +454,7 @@ public function loadUserPreferences(bool $isMinimumCommon = false): void
         // load config array
         $this->settings = array_replace_recursive($this->settings, $configData);
         $GLOBALS['cfg'] = array_replace_recursive($GLOBALS['cfg'], $configData);
+        $this->config = new Settings($this->settings);
 
         if ($isMinimumCommon) {
             return;
@@ -716,6 +711,7 @@ public function set(string $setting, mixed $value): void
         }
 
         $this->settings[$setting] = $value;
+        $this->config = new Settings($this->settings);
     }
 
     /**
@@ -1151,6 +1147,7 @@ public function checkServers(): void
         if (! isset($this->settings['Servers']) || count($this->settings['Servers']) === 0) {
             // No server => create one with defaults
             $this->settings['Servers'] = [1 => $this->defaultServer];
+            $this->config = new Settings($this->settings);
 
             return;
         }
@@ -1184,6 +1181,7 @@ public function checkServers(): void
         }
 
         $this->settings['Servers'] = $newServers;
+        $this->config = new Settings($this->settings);
     }
 
     /**
@@ -1317,10 +1315,6 @@ public function getLoginCookieValidityFromCache(int $server): void
 
     public function getSettings(): Settings
     {
-        if ($this->config === null) {
-            $this->config = new Settings($this->settings);
-        }
-
         return $this->config;
     }
 }
diff --git a/test/classes/BrowseForeignersTest.php b/test/classes/BrowseForeignersTest.php
@@ -47,7 +47,7 @@ public function testGetForeignLimit(): void
         );
 
         $config = new Config();
-        $config->settings['MaxRows'] = 50;
+        $config->set('MaxRows', 50);
         $browseForeigners = new BrowseForeigners(new Template(), $config);
 
         $this->assertEquals(
@@ -128,7 +128,7 @@ public function testGetDescriptionAndTitle(): void
         );
 
         $config = new Config();
-        $config->settings['LimitChars'] = 5;
+        $config->set('LimitChars', 5);
         $browseForeigners = new BrowseForeigners(new Template(), $config);
 
         $this->assertEquals(

diff --git a/test/classes/ConfigTest.php b/test/classes/ConfigTest.php
@@ -9,7 +9,6 @@
 use PhpMyAdmin\Dbal\Connection;
 
 use function array_merge;
-use function array_replace_recursive;
 use function define;
 use function defined;
 use function file_exists;
@@ -420,30 +419,18 @@ public function testCheckWebServerOs(): void
         }
     }
 
-    /**
-     * Tests loading of default values
-     *
-     * @group large
-     */
-    public function testLoadDefaults(): void
+    public function testConstructor(): void
     {
-        $this->object->defaultServer = [];
-        $this->object->default = [];
-        $this->object->settings = ['is_setup' => false, 'AvailableCharsets' => ['test']];
-
-        $this->object->loadDefaults();
-
+        $object = new Config();
         $settings = new Settings([]);
         $config = $settings->asArray();
-
         $this->assertIsArray($config['Servers']);
-        $this->assertEquals($config['Servers'][1], $this->object->defaultServer);
+        $this->assertEquals($settings, $object->getSettings());
+        $this->assertSame($config['Servers'][1], $object->defaultServer);
         unset($config['Servers']);
-        $this->assertEquals($config, $this->object->default);
-        $this->assertEquals(
-            array_replace_recursive(['is_setup' => false, 'AvailableCharsets' => ['test']], $config),
-            $this->object->settings,
-        );
+        $this->assertSame($config, $object->default);
+        $this->assertSame($config, $object->settings);
+        $this->assertSame($config, $object->baseSettings);
     }
 
     /**
@@ -1056,12 +1043,4 @@ public static function connectionParams(): array
             ],
         ];
     }
-
-    public function testGetSettings(): void
-    {
-        $config = new Config();
-        $firstCall = $config->getSettings();
-        $secondCall = $config->getSettings();
-        $this->assertSame($firstCall, $secondCall);
-    }
 }