Bug #53417
closed
ConfigurationManager->getConfigurationValueByPath() ignores AdditionalConfiguration.php
0%
Description
Hi.
Recently I've discovered an issue in ConfigurationManager. If I override something in AdditionalConfiguration.php, e.g. $GLOBALS['TYPO3_CONF_VARS']['BE']['compressionLevel'] = 4;, while in my LocalConfiguration.php I have this value set to 5, I'll get 4 in Install Tool, but following code will return 5:
$cm = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Core\\Configuration\\ConfigurationManager');
return $cm->getLocalConfigurationValueByPath('BE/compressionLevel');
Same result will be for ConfigurationManager->getConfigurationValueByPath(), which is used by Extension Manager and therefore I'm not able to override $GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf'] for any extension, which makes it impossible to have different configurations for dev-stage-live environments.
Updated by Viktor Livakivskyi over 10 years ago
Small update.
getLocalConfigurationValueByPath() may ignore AdditionalConfiguration.php, becasue it is explicitly mentioned in method's name, that it takes values form LocalConfiguration.php
but
getConfigurationValueByPath() should include defaultConfiguration merged with localConfiguration and merged with additionalConfiguration
Updated by Markus Klein over 10 years ago
@Christian: Can you maybe say something about this?
Updated by Gerrit Code Review over 9 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/38041
Updated by Gerrit Code Review over 9 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/38041
Updated by Mathias Brodala over 9 years ago
IIRC this is on purpose since AdditionalConfiguration.php can contain arbitrary PHP code. This can break the install tool which is supposed to be the last resort in case of breakage and basically unbreakable.
Updated by Viktor Livakivskyi over 9 years ago
Mathias, you're right. But then ConfigurationManager->getConfigurationValueByPath() makes no sense to use, because AdditionalConfiguration.php is ignored. AdditionalConfiguration is intended to be used to override configuration from LocalConfiguration, and we need to rely on this in client code.
E.g. we have following in AdditionalConfiguration:
switch (\TYPO3\CMS\Core\Utility\GeneralUtility::getApplicationContext()) {
case 'Development':
$GLOBALS['TYPO3_CONF_VARS']['SYS']['displayErrors'] = 1;
/* skip */
break;
case 'Production/Staging':
$GLOBALS['TYPO3_CONF_VARS']['SYS']['displayErrors'] = -1;
/* skip */
break;
case 'Production':
default:
$GLOBALS['TYPO3_CONF_VARS']['SYS']['displayErrors'] = 0;
/* skip */
}
And if I get ConfigurationManager->getConfigurationValueByPath('SYS/displayErrors') I'll not get the value, dependant on context, but one, defined in LocalConfiguration.php
So, I can't rely on this value.
Updated by Markus Klein over 9 years ago
ConfigurationManager is internal API, see class header
- IMPORTANT:
- This class is intended for internal core use ONLY.
Updated by Viktor Livakivskyi over 9 years ago
Why is it then mentioned in "What's New" slides of TYPO3 6.0 on page 35 in "Coding Examples"?
http://forge.typo3.org/attachments/download/24568/TYPO3-v6-0-whats-new.pdf
Maybe, makes sense to add remark in that PDF, that developes shouldn't use it or completely remove that page?
UPDATE: also checked code for 6.0 and 6.1 - there were no remark about @internal, therefore I suggested it's usage in client code at the moment of creating this issue
Updated by Markus Klein over 9 years ago
Ok that is really bad. Although the title of the slide talks about "LocalConfiguration".
I doubt anybody will touch the slides, since 6.0 is not supported for a long time now.
Updated by Viktor Livakivskyi over 9 years ago
Markus, you're right.
Since 6.2 marks this class as internal, then it makes no sense to use it in client code.
So, the issue may be closed.
Updated by Christian Kuhn over 9 years ago
- Status changed from Under Review to Rejected
Hey.
It was unfortunate that the internal low level class to handle LocalConfiguration was mentioned in the slides of former core versions. We clarified with the @internal annotation only later and now in result stumble upon the one or other one complaining about the situation. Sorry about that.
The issue however is bogus since AdditionalConfiguration is not handled at this point by intention - otherwise settings from additionalconfiguration could end up in localconfiguration on writing, which is not wanted.
All "using / 3rd party code" should operate (reading) on $GLOBALS['TYPO3_CONF_VARS'] directly and will then have the "final" array.
This issue will be closed as rejected now.