ISS-33802: refactor DeploymentConfig class in order to ensure that all methods follow the same reload logic

Author: sergeynezbritskiy

lib/internal/Magento/Framework/App/DeploymentConfig.php

@@ -38,7 +38,7 @@ class DeploymentConfig
      *
      * @var array
      */
-    private $flatData = [];
+    private $flatData;
 
     /**
      * Injected configuration data
@@ -55,7 +55,7 @@ class DeploymentConfig
      * @param DeploymentConfig\Reader $reader
      * @param array $overrideData
      */
-    public function __construct(DeploymentConfig\Reader $reader, $overrideData = [])
+    public function __construct(DeploymentConfig\Reader $reader, array $overrideData = [])
     {
         $this->reader = $reader;
         $this->overrideData = $overrideData;
@@ -64,63 +64,50 @@ public function __construct(DeploymentConfig\Reader $reader, $overrideData = [])
     /**
      * Gets data from flattened data
      *
-     * @param string $key
+     * @param string|null $key
      * @param mixed $defaultValue
      * @return mixed|null
      * @throws FileSystemException
      * @throws RuntimeException
      */
-    public function get($key = null, $defaultValue = null)
+    public function get(string $key = null, $defaultValue = null)
     {
         $result = $this->getByKey($key);
         if ($result === null) {
-            $this->resetData();
-            $this->load();
+            $this->reloadData();
             $result = $this->getByKey($key);
         }
         return $result ?? $defaultValue;
     }
 
-    /**
-     * Checks if data available
-     *
-     * @return bool
-     * @throws FileSystemException
-     * @throws RuntimeException
-     */
-    public function isAvailable()
-    {
-        $this->load();
-        return isset($this->flatData[ConfigOptionsListConstants::CONFIG_PATH_INSTALL_DATE]);
-    }
-
     /**
      * Gets a value specified key from config data
      *
-     * @param string $key
+     * @param string|null $key
      * @return null|mixed
      * @throws FileSystemException
      * @throws RuntimeException
      */
-    public function getConfigData($key = null)
+    public function getConfigData(string $key = null)
     {
-        $this->load();
-
-        if ($key !== null && !isset($this->data[$key])) {
-            return null;
+        $result = $this->getConfigDataByKey($key);
+        if ($result === null) {
+            $this->reloadData();
+            $result = $this->getConfigDataByKey($key);
         }
-
-        return $this->data[$key] ?? $this->data;
+        return $result;
     }
 
     /**
-     * Resets config data
+     * Checks if data available
      *
-     * @return void
+     * @return bool
+     * @throws FileSystemException
+     * @throws RuntimeException
      */
-    public function resetData()
+    public function isAvailable(): bool
     {
-        $this->data = null;
+        return $this->get(ConfigOptionsListConstants::CONFIG_PATH_INSTALL_DATE) !== null;
     }
 
     /**
@@ -131,10 +118,20 @@ public function resetData()
      * @throws RuntimeException
      * @since 100.1.3
      */
-    public function isDbAvailable()
+    public function isDbAvailable(): bool
     {
-        $this->load();
-        return isset($this->data['db']);
+        return $this->getConfigData('db') !== null;
+    }
+
+    /**
+     * Resets config data
+     *
+     * @return void
+     */
+    public function resetData(): void
+    {
+        $this->data = null;
+        $this->flatData = null;
     }
 
     /**
@@ -144,16 +141,14 @@ public function isDbAvailable()
      * @throws FileSystemException
      * @throws RuntimeException
      */
-    private function load()
+    private function reloadData(): void
     {
-        if (empty($this->data)) {
-            $this->data = $this->reader->load();
-            if ($this->overrideData) {
-                $this->data = array_replace($this->data, $this->overrideData);
-            }
-            // flatten data for config retrieval using get()
-            $this->flatData = $this->flattenParams($this->data);
+        $this->data = $this->reader->load();
+        if ($this->overrideData) {
+            $this->data = array_replace($this->data, $this->overrideData);
         }
+        // flatten data for config retrieval using get()
+        $this->flatData = $this->flattenParams($this->data);
     }
 
     /**
@@ -163,12 +158,12 @@ private function load()
      * each level of array is accessible by path key
      *
      * @param array $params
-     * @param string $path
-     * @param array $flattenResult
+     * @param string|null $path
+     * @param array|null $flattenResult
      * @return array
      * @throws RuntimeException
      */
-    private function flattenParams(array $params, $path = null, array &$flattenResult = null): array
+    private function flattenParams(array $params, ?string $path = null, array &$flattenResult = null): array
     {
         if (null === $flattenResult) {
             $flattenResult = [];
@@ -195,17 +190,29 @@ private function flattenParams(array $params, $path = null, array &$flattenResul
 
     /**
      * @param string|null $key
-     * @return array|mixed|string
+     * @return mixed|null
      */
-    protected function getByKey($key)
+    private function getByKey(?string $key)
     {
         if ($key === null) {
             return $this->flatData ?: null;
         }
-        if (array_key_exists($key, $this->flatData) && $this->flatData[$key] === null) {
+        if (is_array($this->flatData) && array_key_exists($key, $this->flatData) && $this->flatData[$key] === null) {
             return '';
         }
 
         return $this->flatData[$key] ?? null;
     }
+
+    /**
+     * @param string|null $key
+     * @return mixed|null
+     */
+    private function getConfigDataByKey(?string $key)
+    {
+        if ($key === null) {
+            return $this->data;
+        }
+        return $this->data[$key] ?? null;
+    }
 }

lib/internal/Magento/Framework/App/Test/Unit/DeploymentConfigTest.php

@@ -149,7 +149,7 @@ public function testNotAvailable(): void
      */
     public function testNotAvailableThenAvailable(): void
     {
-        $this->readerMock->expects($this->once())->method('load')->willReturn(['Test']);
+        $this->readerMock->expects($this->exactly(2))->method('load')->willReturn(['Test']);
         $object = new DeploymentConfig($this->readerMock);
         $this->assertFalse($object->isAvailable());
         $this->assertFalse($object->isAvailable());
@@ -191,7 +191,7 @@ public function keyCollisionDataProvider(): array
      */
     public function testResetData(): void
     {
-        $this->readerMock->expects($this->once())->method('load')->willReturn(self::$fixture);
+        $this->readerMock->expects($this->exactly(2))->method('load')->willReturn(self::$fixture);
         $this->assertSame(self::$flattenedFixture, $this->deploymentConfig->get());
         $this->deploymentConfig->resetData();
         // second time to ensure loader will be invoked only once after reset
@@ -208,7 +208,6 @@ public function testIsDbAvailable(): void
     {
         $this->readerMock->expects($this->exactly(2))->method('load')->willReturnOnConsecutiveCalls([], ['db' => []]);
         $this->assertFalse($this->deploymentConfig->isDbAvailable());
-        $this->deploymentConfig->resetData();
         $this->assertTrue($this->deploymentConfig->isDbAvailable());
     }