Merge pull request #8552 from magento-performance/ACPT-1599-2.4.7-beta2-develop

ACPT-1599: Fixing DeploymentConfig resets & reloads every time new key that wasn't previously set

Author: andimov

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

@@ -61,11 +61,6 @@ class DeploymentConfig
      */
     private $readerLoad = [];
 
-    /**
-     * @var array
-     */
-    private $noConfigData = [];
-
     /**
      * Constructor
      *
@@ -99,17 +94,9 @@ public function get($key = null, $defaultValue = null)
         }
         $result = $this->getByKey($key);
         if ($result === null) {
-            if (empty($this->flatData)
-                || !isset($this->flatData[$key]) && !isset($this->noConfigData[$key])
-                || count($this->getAllEnvOverrides())
-            ) {
-                $this->resetData();
+            if (empty($this->flatData) || count($this->getAllEnvOverrides())) {
                 $this->reloadData();
             }
-
-            if (!isset($this->flatData[$key])) {
-                $this->noConfigData[$key] = $key;
-            }
             $result = $this->getByKey($key);
         }
         return $result ?? $defaultValue;

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

@@ -306,15 +306,11 @@ public function testEnvVariablesSubstitution(): void
      * @throws FileSystemException
      * @throws RuntimeException
      */
-    public function testReloadDataOnMissingConfig(): void
+    public function testShouldntReloadDataOnMissingConfig(): void
     {
-        $this->readerMock->expects($this->exactly(2))
+        $this->readerMock->expects($this->once())
             ->method('load')
-            ->willReturnOnConsecutiveCalls(
-                ['db' => ['connection' => ['default' => ['host' => 'localhost']]]],
-                [],
-                []
-            );
+            ->willReturn(['db' => ['connection' => ['default' => ['host' => 'localhost']]]]);
         $connectionConfig1 = $this->deploymentConfig->get(
             ConfigOptionsListConstants::CONFIG_PATH_DB_CONNECTIONS . '/' . 'default'
         );
@@ -330,4 +326,42 @@ public function testReloadDataOnMissingConfig(): void
         $result3 = $this->deploymentConfig->get('missing/key');
         $this->assertNull($result3);
     }
+
+    /**
+     * @return void
+     */
+    public function testShouldntLoadMultipleTimes() : void
+    {
+        $this->readerMock->expects($this->once())->method('load')
+            ->willReturn(['a' => ['a' => ['a' => 1]]]);
+        $this->deploymentConfig->get('a/a/a');
+        $this->deploymentConfig->get('a/a/b');
+        $this->deploymentConfig->get('a/a/c');
+        $this->deploymentConfig->get('a/b/a');
+        $this->deploymentConfig->get('a/b/b');
+        $this->deploymentConfig->get('a/b/c');
+    }
+
+    /**
+     * @return void
+     */
+    public function testShouldReloadPreviouslyUnsetKeysAfterReset() : void
+    {
+        $testValue = 42;
+        $loadReturn = ['a' => ['a' => ['a' => 1]]];
+        $this->readerMock->expects($this->any())->method('load')
+            ->will($this->returnCallback(
+                function () use (&$loadReturn) {
+                    return $loadReturn;
+                }
+            ));
+        $this->deploymentConfig->get('a/a/a');
+        $abcReturnValue1 = $this->deploymentConfig->get('a/b/c');
+        $this->assertNull($abcReturnValue1); // first try, it isn't set yet.
+        $loadReturn = ['a' => ['a' => ['a' => 1], 'b' => ['c' => $testValue]]];
+        $this->deploymentConfig->resetData();
+        $this->deploymentConfig->get('a/a/a');
+        $abcReturnValue2 = $this->deploymentConfig->get('a/b/c');
+        $this->assertEquals($testValue, $abcReturnValue2); // second try, it should load the newly set value
+    }
 }

setup/src/Magento/Setup/Model/Installer.php

@@ -12,10 +12,12 @@
 use Magento\Framework\App\Cache\Type\Block as BlockCache;
 use Magento\Framework\App\Cache\Type\Config as ConfigCache;
 use Magento\Framework\App\Cache\Type\Layout as LayoutCache;
+use Magento\Framework\App\DeploymentConfig;
 use Magento\Framework\App\DeploymentConfig\Reader;
 use Magento\Framework\App\DeploymentConfig\Writer;
 use Magento\Framework\App\Filesystem\DirectoryList;
 use Magento\Framework\App\MaintenanceMode;
+use Magento\Framework\App\ObjectManager;
 use Magento\Framework\App\State\CleanupFiles;
 use Magento\Framework\Component\ComponentRegistrar;
 use Magento\Framework\Config\ConfigOptionsListConstants;
@@ -178,10 +180,15 @@ class Installer
     private $installInfo = [];
 
     /**
-     * @var \Magento\Framework\App\DeploymentConfig
+     * @var DeploymentConfig
      */
     private $deploymentConfig;
 
+    /**
+     * @var DeploymentConfig
+     */
+    private $firstDeploymentConfig;
+
     /**
      * @var ObjectManagerProvider
      */
@@ -330,6 +337,11 @@ public function __construct(
         $this->phpReadinessCheck = $phpReadinessCheck;
         $this->schemaPersistor = $this->objectManagerProvider->get()->get(SchemaPersistor::class);
         $this->triggerCleaner = $this->objectManagerProvider->get()->get(TriggerCleaner::class);
+        /* Note: Because this class is dependency injected with Laminas ServiceManager, but our plugins, and some
+         * other classes also use the App\ObjectManager instead, we have to make sure that the DeploymentConfig object
+         * from that ObjectManager gets reset as different steps in the installer will write to the deployment config.
+         */
+        $this->firstDeploymentConfig = ObjectManager::getInstance()->get(DeploymentConfig::class);
     }
 
     /**
@@ -389,6 +401,9 @@ public function install($request)
         $this->log->log('Starting Magento installation:');
 
         foreach ($script as $item) {
+            /* Note: Because the $this->DeploymentConfig gets written to, but plugins use $this->firstDeploymentConfig,
+             * we have to reset this one after each item of $script so the plugins will see the config updates. */
+            $this->firstDeploymentConfig->resetData();
             list($message, $method, $params) = $item;
             $this->log->log($message);
             try {