[BUGFIX] Prevent repopulating TYPO3_CONF_VARS 03/55703/15
authorNicole Cordes <typo3@cordes.co>
Tue, 13 Feb 2018 17:25:59 +0000 (18:25 +0100)
committerAlexander Opitz <opitz.alexander@googlemail.com>
Thu, 22 Feb 2018 10:56:50 +0000 (11:56 +0100)
Instead of using the native API when changing a TYPO3 Extension
configuration through the new ExtensionConfiguration API, the
method is now solely overwriting the global $TYPO3_CONF_VARS[EXTENSIONS]
and $TYPO3_CONF_VARS[EXT][extConf] options during runtime
to avoid deadlocks and to avoid the removal of existing configuration
of extensions.

The second parameter of ExtensionConfiguration->set() can be removed
separately once EXT:bootstrap_package is adapted to the new functionaliy.

Resolves: #83958
Resolves: #83954
Releases: master
Change-Id: Icc8a3482edaef1ea329e68638d5ef467548062fc
Reviewed-on: https://review.typo3.org/55703
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Nicole Cordes <typo3@cordes.co>
Tested-by: Nicole Cordes <typo3@cordes.co>
Reviewed-by: Jigal van Hemert <jigal.van.hemert@typo3.org>
Tested-by: Jigal van Hemert <jigal.van.hemert@typo3.org>
Reviewed-by: Alexander Opitz <opitz.alexander@googlemail.com>
Tested-by: Alexander Opitz <opitz.alexander@googlemail.com>
typo3/sysext/core/Classes/Configuration/ExtensionConfiguration.php
typo3/sysext/core/Tests/Unit/Configuration/ExtensionConfigurationTest.php

index bed8c24..c8565dc 100644 (file)
@@ -17,7 +17,6 @@ namespace TYPO3\CMS\Core\Configuration;
 
 use TYPO3\CMS\Core\Configuration\Exception\ExtensionConfigurationExtensionNotConfiguredException;
 use TYPO3\CMS\Core\Configuration\Exception\ExtensionConfigurationPathDoesNotExistException;
-use TYPO3\CMS\Core\Core\Bootstrap;
 use TYPO3\CMS\Core\Package\PackageManager;
 use TYPO3\CMS\Core\Utility\ArrayUtility;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -170,29 +169,16 @@ class ExtensionConfiguration
      * on the fly for whatever reason.
      *
      * Examples:
-     * // Enable a single feature
-     * ->set('myExtension', 'myFeature', true)
-     *
      * // Set a full extension configuration ($value could be a nested array, too)
-     * ->set('myExtension', '', ['aFeature' => 'true', 'aCustomClass' => 'css-foo'])
-     *
-     * // Set a full sub path
-     * ->set('myExtension', 'myFeatureCategory', ['aFeature' => 'true', 'aCustomClass' => 'css-foo'])
+     * ->set('myExtension', ['aFeature' => 'true', 'aCustomClass' => 'css-foo'])
      *
      * // Unset a whole extension configuration
      * ->set('myExtension')
      *
-     * // Unset a single value or sub path
-     * ->set('myExtension', 'myFeature')
-     *
      * Notes:
      * - Do NOT call this at arbitrary places during runtime (eg. NOT in ext_localconf.php or
      *   similar). ->set() is not supposed to be called each request since it writes LocalConfiguration
      *   each time. This API is however OK to be called from extension manager hooks.
-     * - $path is NOT validated. It is up to an ext author to also define them in
-     *   ext_conf_template.txt to have an interface in install tool reflecting these settings.
-     * - If $path is currently an array, $value overrides the whole thing. Merging existing values
-     *   is up to the extension author
      * - Values are not type safe, if the install tool wrote them,
      *   boolean true could become string 1 on ->get()
      * - It is not possible to store 'null' as value, giving $value=null
@@ -211,31 +197,34 @@ class ExtensionConfiguration
         if (empty($extension)) {
             throw new \RuntimeException('extension name must not be empty', 1509715852);
         }
+        if (!empty($path)) {
+            // @todo: this functionality can be removed once EXT:bootstrap_package is adapted to the new API.
+            $extensionConfiguration = $this->get($extension);
+            $value = ArrayUtility::setValueByPath($extensionConfiguration, $path, $value);
+        }
         $configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
-        if ($path === '' && $value === null) {
+        if ($value === null) {
             // Remove whole extension config
             $configurationManager->removeLocalConfigurationKeysByPath(['EXTENSIONS/' . $extension]);
-        } elseif ($path !== '' && $value === null) {
-            // Remove a single value or sub path
-            $configurationManager->removeLocalConfigurationKeysByPath(['EXTENSIONS/' . $extension . '/' . $path]);
-        } elseif ($path === '' && $value !== null) {
+            if (isset($GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS'][$extension])) {
+                unset($GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS'][$extension]);
+            }
+        } else {
             // Set full extension config
             $configurationManager->setLocalConfigurationValueByPath('EXTENSIONS/' . $extension, $value);
-        } else {
-            // Set single path
-            $configurationManager->setLocalConfigurationValueByPath('EXTENSIONS/' . $extension . '/' . $path, $value);
+            $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS'][$extension] = $value;
         }
 
         // After TYPO3_CONF_VARS['EXTENSIONS'] has been written, update legacy layer TYPO3_CONF_VARS['EXTENSIONS']['extConf']
         // @deprecated since TYPO3 v9, will be removed in v10 with removal of old serialized 'extConf' layer
-        $extensionsConfigs = $configurationManager->getConfigurationValueByPath('EXTENSIONS');
-        foreach ($extensionsConfigs as $extensionName => $extensionConfig) {
-            $extensionConfig = $this->addDotsToArrayKeysRecursiveForLegacyExtConf($extensionConfig);
-            $configurationManager->setLocalConfigurationValueByPath('EXT/extConf/' . $extensionName, serialize($extensionConfig));
+        if (!empty($GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS'])) {
+            $extConfArray = [];
+            foreach ($GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS'] as $extensionName => $extensionConfig) {
+                $extConfArray[$extensionName] = serialize($this->addDotsToArrayKeysRecursiveForLegacyExtConf($extensionConfig));
+            }
+            $configurationManager->setLocalConfigurationValueByPath('EXT/extConf', $extConfArray);
+            $GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf'] = $extConfArray;
         }
-
-        // Reload TYPO3_CONF_VARS from LocalConfiguration & AdditionalConfiguration
-        Bootstrap::getInstance()->populateLocalConfiguration();
     }
 
     /**
@@ -250,6 +239,7 @@ class ExtensionConfiguration
     {
         $configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
         $configurationManager->setLocalConfigurationValueByPath('EXTENSIONS', $configuration);
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS'] = $configuration;
 
         // After TYPO3_CONF_VARS['EXTENSIONS'] has been written, update legacy layer TYPO3_CONF_VARS['EXTENSIONS']['extConf']
         // @deprecated since TYPO3 v9, will be removed in v10 with removal of old serialized 'extConf' layer
@@ -258,9 +248,7 @@ class ExtensionConfiguration
             $extConfArray[$extensionName] = serialize($this->addDotsToArrayKeysRecursiveForLegacyExtConf($extensionConfig));
         }
         $configurationManager->setLocalConfigurationValueByPath('EXT/extConf', $extConfArray);
-
-        // Reload TYPO3_CONF_VARS from LocalConfiguration & AdditionalConfiguration
-        Bootstrap::getInstance()->populateLocalConfiguration();
+        $GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf'] = $extConfArray;
     }
 
     /**
index 08c3ac8..2c6b6e4 100644 (file)
@@ -86,7 +86,6 @@ class ExtensionConfigurationTest extends UnitTestCase
     {
         $configurationManagerProphecy = $this->prophesize(ConfigurationManager::class);
         GeneralUtility::addInstance(ConfigurationManager::class, $configurationManagerProphecy->reveal());
-        $configurationManagerProphecy->getConfigurationValueByPath(Argument::cetera())->willReturn([]);
         $configurationManagerProphecy->removeLocalConfigurationKeysByPath(['EXTENSIONS/foo'])->shouldBeCalled();
         (new ExtensionConfiguration())->set('foo');
     }
@@ -94,23 +93,11 @@ class ExtensionConfigurationTest extends UnitTestCase
     /**
      * @test
      */
-    public function setRemovesPath()
-    {
-        $configurationManagerProphecy = $this->prophesize(ConfigurationManager::class);
-        GeneralUtility::addInstance(ConfigurationManager::class, $configurationManagerProphecy->reveal());
-        $configurationManagerProphecy->getConfigurationValueByPath(Argument::cetera())->willReturn([]);
-        $configurationManagerProphecy->removeLocalConfigurationKeysByPath(['EXTENSIONS/foo/bar'])->shouldBeCalled();
-        (new ExtensionConfiguration())->set('foo', 'bar');
-    }
-
-    /**
-     * @test
-     */
     public function setWritesFullExtensionConfig()
     {
         $configurationManagerProphecy = $this->prophesize(ConfigurationManager::class);
         GeneralUtility::addInstance(ConfigurationManager::class, $configurationManagerProphecy->reveal());
-        $configurationManagerProphecy->getConfigurationValueByPath(Argument::cetera())->willReturn([]);
+        $configurationManagerProphecy->setLocalConfigurationValueByPath(Argument::cetera())->shouldBeCalled();
         $configurationManagerProphecy->setLocalConfigurationValueByPath('EXTENSIONS/foo', ['bar' => 'baz'])->shouldBeCalled();
         (new ExtensionConfiguration())->set('foo', '', ['bar' => 'baz']);
     }
@@ -118,25 +105,12 @@ class ExtensionConfigurationTest extends UnitTestCase
     /**
      * @test
      */
-    public function setWritesPath()
-    {
-        $configurationManagerProphecy = $this->prophesize(ConfigurationManager::class);
-        GeneralUtility::addInstance(ConfigurationManager::class, $configurationManagerProphecy->reveal());
-        $configurationManagerProphecy->getConfigurationValueByPath(Argument::cetera())->willReturn([]);
-        $configurationManagerProphecy->setLocalConfigurationValueByPath('EXTENSIONS/foo/aPath', ['bar' => 'baz'])->shouldBeCalled();
-        (new ExtensionConfiguration())->set('foo', 'aPath', ['bar' => 'baz']);
-    }
-
-    /**
-     * @test
-     */
     public function setUpdatesLegacyExtConfToNewValues()
     {
         $configurationManagerProphecy = $this->prophesize(ConfigurationManager::class);
         GeneralUtility::addInstance(ConfigurationManager::class, $configurationManagerProphecy->reveal());
         $configurationManagerProphecy->setLocalConfigurationValueByPath(Argument::cetera())->shouldBeCalled();
-        $configurationManagerProphecy->getConfigurationValueByPath('EXTENSIONS')->willReturn(['foo' => ['bar' => 'baz']]);
-        $configurationManagerProphecy->setLocalConfigurationValueByPath('EXT/extConf/foo', serialize(['bar' => 'baz']))->shouldBeCalled();
+        $configurationManagerProphecy->setLocalConfigurationValueByPath('EXT/extConf', ['foo' => serialize(['bar' => 'baz'])])->shouldBeCalled();
         (new ExtensionConfiguration())->set('foo', '', ['bar' => 'baz']);
     }
 
@@ -147,7 +121,6 @@ class ExtensionConfigurationTest extends UnitTestCase
     {
         $configurationManagerProphecy = $this->prophesize(ConfigurationManager::class);
         GeneralUtility::addInstance(ConfigurationManager::class, $configurationManagerProphecy->reveal());
-        $configurationManagerProphecy->setLocalConfigurationValueByPath(Argument::cetera())->shouldBeCalled();
         $nestedInput = [
             'FE' => [
                 'forceSalted' => true,
@@ -158,8 +131,8 @@ class ExtensionConfigurationTest extends UnitTestCase
                 'forceSalted' => true,
             ]
         ];
-        $configurationManagerProphecy->getConfigurationValueByPath('EXTENSIONS')->willReturn(['saltedPasswords' => $nestedInput]);
-        $configurationManagerProphecy->setLocalConfigurationValueByPath('EXT/extConf/saltedPasswords', serialize($expectedLegacyExtConf))->shouldBeCalled();
+        $configurationManagerProphecy->setLocalConfigurationValueByPath(Argument::cetera())->shouldBeCalled();
+        $configurationManagerProphecy->setLocalConfigurationValueByPath('EXT/extConf', ['saltedPasswords' => serialize($expectedLegacyExtConf)])->shouldBeCalled();
         (new ExtensionConfiguration())->set('saltedPasswords', '', $nestedInput);
     }
 
@@ -170,7 +143,6 @@ class ExtensionConfigurationTest extends UnitTestCase
     {
         $configurationManagerProphecy = $this->prophesize(ConfigurationManager::class);
         GeneralUtility::addInstance(ConfigurationManager::class, $configurationManagerProphecy->reveal());
-        $configurationManagerProphecy->setLocalConfigurationValueByPath(Argument::cetera())->shouldBeCalled();
         $nestedInput = [
             'aCategory' => [
                 'aSubCategory' => [
@@ -185,8 +157,8 @@ class ExtensionConfigurationTest extends UnitTestCase
                 ],
             ],
         ];
-        $configurationManagerProphecy->getConfigurationValueByPath('EXTENSIONS')->willReturn(['someExtension' => $nestedInput]);
-        $configurationManagerProphecy->setLocalConfigurationValueByPath('EXT/extConf/someExtension', serialize($expectedLegacyExtConf))->shouldBeCalled();
+        $configurationManagerProphecy->setLocalConfigurationValueByPath(Argument::cetera())->shouldBeCalled();
+        $configurationManagerProphecy->setLocalConfigurationValueByPath('EXT/extConf', ['someExtension' => serialize($expectedLegacyExtConf)])->shouldBeCalled();
         (new ExtensionConfiguration())->set('someExtension', '', $nestedInput);
     }
 }