ACP2E-1958: avoid reloading configs for each requested key; added unit tests

Author: alzota

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

@@ -51,6 +51,11 @@ class DeploymentConfig
      */
     private $overrideData;
 
+    /**
+     * @var array
+     */
+    private $envOverrides = [];
+
     /**
      * Constructor
      *
@@ -76,6 +81,7 @@ public function __construct(DeploymentConfig\Reader $reader, $overrideData = [])
      */
     public function get($key = null, $defaultValue = null)
     {
+        //if no key is requested, then all config is returned.
         if ($key === null) {
             if (empty($this->flatData)) {
                 $this->reloadData();
@@ -84,7 +90,7 @@ public function get($key = null, $defaultValue = null)
         }
         $result = $this->getByKey($key);
         if ($result === null) {
-            if (empty($this->flatData)) {
+            if (empty($this->flatData) || count($this->getAllEnvOverrides())) {
                 $this->reloadData();
             }
             $result = $this->getByKey($key);
@@ -181,19 +187,29 @@ private function reloadData(): void
         );
         // flatten data for config retrieval using get()
         $this->flatData = $this->flattenParams($this->data);
+        $this->flatData = $this->getAllEnvOverrides() + $this->flatData;
+    }
 
-        // allow reading values from env variables by convention
-        // MAGENTO_DC_{path}, like db/connection/default/host =>
-        // can be overwritten by MAGENTO_DC_DB__CONNECTION__DEFAULT__HOST
-        foreach (getenv() as $key => $value) {
-            if (false !== \strpos($key, self::MAGENTO_ENV_PREFIX)
-                && $key !== self::OVERRIDE_KEY
-            ) {
-                // convert MAGENTO_DC_DB__CONNECTION__DEFAULT__HOST into db/connection/default/host
-                $flatKey = strtolower(str_replace([self::MAGENTO_ENV_PREFIX, '__'], ['', '/'], $key));
-                $this->flatData[$flatKey] = $value;
+    /**
+     * @return array
+     */
+    private function getAllEnvOverrides(): array
+    {
+        if (empty($this->envOverrides)) {
+            // allow reading values from env variables by convention
+            // MAGENTO_DC_{path}, like db/connection/default/host =>
+            // can be overwritten by MAGENTO_DC_DB__CONNECTION__DEFAULT__HOST
+            foreach (getenv() as $key => $value) {
+                if (false !== \strpos($key, self::MAGENTO_ENV_PREFIX)
+                    && $key !== self::OVERRIDE_KEY
+                ) {
+                    // convert MAGENTO_DC_DB__CONNECTION__DEFAULT__HOST into db/connection/default/host
+                    $flatKey = strtolower(str_replace([self::MAGENTO_ENV_PREFIX, '__'], ['', '/'], $key));
+                    $this->envOverrides[$flatKey] = $value;
+                }
             }
         }
+        return $this->envOverrides;
     }
 
     /**

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

@@ -50,6 +50,14 @@ class DeploymentConfigTest extends TestCase
             'test_override' => 'overridden',
         ];
 
+    /**
+     * @var array
+     */
+    private static $flattenedFixtureSecond
+        = [
+            'test_override' => 'overridden2'
+        ];
+
     /**
      * @var array
      */
@@ -112,6 +120,25 @@ public function testGetters(): void
         $this->assertSame('overridden', $this->deploymentConfig->get('test_override'));
     }
 
+    /**
+     * @return void
+     * @throws FileSystemException
+     * @throws RuntimeException
+     */
+    public function testGettersReloadConfig(): void
+    {
+        $this->readerMock->expects($this->any())->method('load')->willReturn(self::$flattenedFixtureSecond);
+        $this->deploymentConfig = new DeploymentConfig(
+            $this->readerMock,
+            ['test_override' => 'overridden2']
+        );
+        $this->assertNull($this->deploymentConfig->get('invalid_key'));
+        $this->assertNull($this->deploymentConfig->getConfigData('invalid_key'));
+        putenv('MAGENTO_DC_A=abc');
+        $this->assertSame('abc', $this->deploymentConfig->get('a'));
+        $this->assertSame('overridden2', $this->deploymentConfig->get('test_override'));
+    }
+
     /**
      * @return void
      * @throws FileSystemException
@@ -149,7 +176,7 @@ public function testNotAvailable(): void
      */
     public function testNotAvailableThenAvailable(): void
     {
-        $this->readerMock->expects($this->exactly(2))->method('load')->willReturn(['Test']);
+        $this->readerMock->expects($this->exactly(1))->method('load')->willReturn(['Test']);
         $object = new DeploymentConfig($this->readerMock);
         $this->assertFalse($object->isAvailable());
         $this->assertFalse($object->isAvailable());