From 727fb0e53f902c83bd0d55fcedbd36388f5715f8 Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Mon, 28 Aug 2023 17:51:14 +0200
Subject: [PATCH] [BUGFIX] Avoid static state in LocalizationUtility
The extbase ConfigurationManager is (unfortunately)
a stateful singleton that we can not get rid of
without a bigger rewrite.
While stateful singletons are bad enough, the
extbase LocalizationUtility makes this worse by
parking an instance of ConfigurationManager in
a static property, re-using it as a "cached"
singleton.
LocalizationUtility does this since it in itself
is static, which makes this service just so
convenient to use. When it comes to sub requests
and similar, static state is doomed and we need
to get rid of it, we've had a couple of patches
in v12 dealing with similar things.
Mid-term, extbase LocalizationUtility needs to
vanish anyways, but in the meantime, we have to
get rid of static state that kills sub request scope.
The patch removes the static $configurationManager
property and adapts functional tests that already
showed the current solution was a hack. There are
various upper and lower cache layers that ensure
removing this "cache layer" won't make things more
expensive in practice, which allows us to remove
this static state without further fallback.
In main, this needs a TF update:
> composer u typo3/testing-framework
In 12.4, this need a TF raise:
> composer req --dev typo3/testing-framework:^8.0.3
Resolves: #101779
Releases: main, 12.4
Change-Id: Ie5db07b0475f612a996d369ab3417672b33fbb2d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/80737
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
---
composer.json | 2 +-
composer.lock | 14 ++---
.../Classes/Utility/LocalizationUtility.php | 19 +------
.../Utility/LocalizationUtilityTest.php | 53 +++++++------------
.../Utility/LocalizationUtilityTest.php | 29 +++-------
5 files changed, 34 insertions(+), 83 deletions(-)
diff --git a/composer.json b/composer.json
index 1d35e0910027..1e772ed36119 100644
--- a/composer.json
+++ b/composer.json
@@ -118,7 +118,7 @@
"sokil/php-isocodes-db-i18n": "^4.0.13",
"symfony/translation": "^6.2",
"typo3/cms-styleguide": "^12.0.2",
- "typo3/testing-framework": "^8.0.2",
+ "typo3/testing-framework": "^8.0.3",
"webmozart/assert": "^1.11.0"
},
"suggest": {
diff --git a/composer.lock b/composer.lock
index 6509d1dc34b0..7f235b310498 100644
--- a/composer.lock
+++ b/composer.lock
@@ -4,7 +4,7 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
"This file is @generated automatically"
],
- "content-hash": "8080136c7d55f58568c412c63b32ba78",
+ "content-hash": "ac3ccb1566155180ebadeb00f1d5ae34",
"packages": [
{
"name": "bacon/bacon-qr-code",
@@ -8825,16 +8825,16 @@
},
{
"name": "typo3/testing-framework",
- "version": "8.0.2",
+ "version": "8.0.3",
"source": {
"type": "git",
"url": "https://github.com/TYPO3/testing-framework.git",
- "reference": "0f64df79ad145ee193f692a2e84e7d6c993d7b5f"
+ "reference": "82d8a12ccf186b919c080a10276ed30590cfb7f3"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/TYPO3/testing-framework/zipball/0f64df79ad145ee193f692a2e84e7d6c993d7b5f",
- "reference": "0f64df79ad145ee193f692a2e84e7d6c993d7b5f",
+ "url": "https://api.github.com/repos/TYPO3/testing-framework/zipball/82d8a12ccf186b919c080a10276ed30590cfb7f3",
+ "reference": "82d8a12ccf186b919c080a10276ed30590cfb7f3",
"shasum": ""
},
"require": {
@@ -8893,9 +8893,9 @@
"support": {
"general": "https://typo3.org/support/",
"issues": "https://github.com/TYPO3/testing-framework/issues",
- "source": "https://github.com/TYPO3/testing-framework/tree/8.0.2"
+ "source": "https://github.com/TYPO3/testing-framework/tree/8.0.3"
},
- "time": "2023-08-02T16:10:26+00:00"
+ "time": "2023-08-28T17:20:23+00:00"
}
],
"aliases": [],
diff --git a/typo3/sysext/extbase/Classes/Utility/LocalizationUtility.php b/typo3/sysext/extbase/Classes/Utility/LocalizationUtility.php
index 5ac791568d99..438f71cdb587 100644
--- a/typo3/sysext/extbase/Classes/Utility/LocalizationUtility.php
+++ b/typo3/sysext/extbase/Classes/Utility/LocalizationUtility.php
@@ -36,11 +36,6 @@ class LocalizationUtility
{
protected static string $locallangPath = 'Resources/Private/Language/';
- /**
- * @var \TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface
- */
- protected static $configurationManager;
-
/**
* Returns the localized label of the LOCAL_LANG key, $key.
*
@@ -177,7 +172,7 @@ class LocalizationUtility
*/
protected static function loadTypoScriptLabels(string $extensionName): array
{
- $configurationManager = static::getConfigurationManager();
+ $configurationManager = GeneralUtility::makeInstance(ConfigurationManagerInterface::class);
$frameworkConfiguration = $configurationManager->getConfiguration(ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK, $extensionName);
if (!is_array($frameworkConfiguration['_LOCAL_LANG'] ?? false)) {
return [];
@@ -233,18 +228,6 @@ class LocalizationUtility
return $result;
}
- /**
- * Returns instance of the configuration manager
- */
- protected static function getConfigurationManager(): ConfigurationManagerInterface
- {
- if (static::$configurationManager !== null) {
- return static::$configurationManager;
- }
- static::$configurationManager = GeneralUtility::makeInstance(ConfigurationManagerInterface::class);
- return static::$configurationManager;
- }
-
/**
* Returns the currently configured "site language" if a site is configured (= resolved)
* in the current request.
diff --git a/typo3/sysext/extbase/Tests/Functional/Utility/LocalizationUtilityTest.php b/typo3/sysext/extbase/Tests/Functional/Utility/LocalizationUtilityTest.php
index 493c078d21d1..70cebf0ce641 100644
--- a/typo3/sysext/extbase/Tests/Functional/Utility/LocalizationUtilityTest.php
+++ b/typo3/sysext/extbase/Tests/Functional/Utility/LocalizationUtilityTest.php
@@ -17,38 +17,16 @@ declare(strict_types=1);
namespace TYPO3\CMS\Extbase\Tests\Functional\Utility;
-use PHPUnit\Framework\MockObject\MockObject;
use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
use TYPO3\CMS\Extbase\Utility\LocalizationUtility;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
final class LocalizationUtilityTest extends FunctionalTestCase
{
- protected ConfigurationManagerInterface&MockObject $configurationManagerInterfaceMock;
-
protected array $testExtensionsToLoad = ['typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/label_test'];
- protected function setUp(): void
- {
- parent::setUp();
- $reflectionClass = new \ReflectionClass(LocalizationUtility::class);
- $this->configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
- $property = $reflectionClass->getProperty('configurationManager');
- $property->setValue(null, $this->configurationManagerInterfaceMock);
- }
-
- /**
- * Reset static properties
- */
- protected function tearDown(): void
- {
- $reflectionClass = new \ReflectionClass(LocalizationUtility::class);
- $property = $reflectionClass->getProperty('configurationManager');
- $property->setValue(null, null);
- parent::tearDown();
- }
-
/**
* @test
*/
@@ -122,18 +100,15 @@ final class LocalizationUtilityTest extends FunctionalTestCase
* @dataProvider translateDataProvider
* @test
*/
- public function translateTestWithBackendUserLanguage(
- string $key,
- string $languageKey,
- string $expected,
- array $altLanguageKeys = [],
- array $arguments = null
- ): void {
+ public function translateTestWithBackendUserLanguage(string $key, string $languageKey, string $expected, array $altLanguageKeys = [], array $arguments = null): void
+ {
// No TypoScript overrides
- $this->configurationManagerInterfaceMock
+ $configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
+ $configurationManagerInterfaceMock
->method('getConfiguration')
->with('Framework', 'label_test', null)
->willReturn([]);
+ GeneralUtility::setSingletonInstance(ConfigurationManagerInterface::class, $configurationManagerInterfaceMock);
$GLOBALS['BE_USER'] = new BackendUserAuthentication();
$GLOBALS['BE_USER']->user = ['lang' => $languageKey];
@@ -152,10 +127,12 @@ final class LocalizationUtilityTest extends FunctionalTestCase
array $arguments = null
): void {
// No TypoScript overrides
- $this->configurationManagerInterfaceMock
+ $configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
+ $configurationManagerInterfaceMock
->method('getConfiguration')
->with('Framework', 'label_test', null)
->willReturn([]);
+ GeneralUtility::setSingletonInstance(ConfigurationManagerInterface::class, $configurationManagerInterfaceMock);
self::assertSame($expected, LocalizationUtility::translate($key, 'label_test', $arguments, $languageKey, $altLanguageKeys));
}
@@ -168,7 +145,8 @@ final class LocalizationUtilityTest extends FunctionalTestCase
public function loadTypoScriptLabels(): void
{
$configurationType = ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK;
- $this->configurationManagerInterfaceMock
+ $configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
+ $configurationManagerInterfaceMock
->expects(self::atLeastOnce())
->method('getConfiguration')
->with($configurationType, 'label_test', null)
@@ -188,6 +166,7 @@ final class LocalizationUtilityTest extends FunctionalTestCase
],
],
]);
+ GeneralUtility::setSingletonInstance(ConfigurationManagerInterface::class, $configurationManagerInterfaceMock);
self::assertSame('key1 value from TS core', LocalizationUtility::translate('key1', 'label_test', languageKey: 'da'));
// Label from XLF file, no override
@@ -203,7 +182,8 @@ final class LocalizationUtilityTest extends FunctionalTestCase
public function clearLabelWithTypoScript(): void
{
$configurationType = ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK;
- $this->configurationManagerInterfaceMock
+ $configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
+ $configurationManagerInterfaceMock
->expects(self::atLeastOnce())
->method('getConfiguration')
->with($configurationType, 'label_test', null)
@@ -214,6 +194,7 @@ final class LocalizationUtilityTest extends FunctionalTestCase
],
],
]);
+ GeneralUtility::setSingletonInstance(ConfigurationManagerInterface::class, $configurationManagerInterfaceMock);
$result = LocalizationUtility::translate('key1', 'label_test', languageKey: 'da');
@@ -244,11 +225,13 @@ final class LocalizationUtilityTest extends FunctionalTestCase
];
$configurationType = ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK;
- $this->configurationManagerInterfaceMock
+ $configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
+ $configurationManagerInterfaceMock
->expects(self::atLeastOnce())
->method('getConfiguration')
->with($configurationType, 'core', null)
->willReturn($typoScriptLocalLang);
+ GeneralUtility::setSingletonInstance(ConfigurationManagerInterface::class, $configurationManagerInterfaceMock);
$result = LocalizationUtility::translate('key1', 'core', languageKey: 'da');
diff --git a/typo3/sysext/extbase/Tests/FunctionalDeprecated/Utility/LocalizationUtilityTest.php b/typo3/sysext/extbase/Tests/FunctionalDeprecated/Utility/LocalizationUtilityTest.php
index dfde867cdd82..a4765ffd5791 100644
--- a/typo3/sysext/extbase/Tests/FunctionalDeprecated/Utility/LocalizationUtilityTest.php
+++ b/typo3/sysext/extbase/Tests/FunctionalDeprecated/Utility/LocalizationUtilityTest.php
@@ -19,6 +19,7 @@ namespace TYPO3\CMS\Extbase\Tests\FunctionalDeprecated\Utility;
use PHPUnit\Framework\MockObject\MockObject;
use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
use TYPO3\CMS\Extbase\Utility\LocalizationUtility;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
@@ -29,26 +30,6 @@ final class LocalizationUtilityTest extends FunctionalTestCase
protected array $testExtensionsToLoad = ['typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/label_test'];
- protected function setUp(): void
- {
- parent::setUp();
- $reflectionClass = new \ReflectionClass(LocalizationUtility::class);
- $this->configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
- $property = $reflectionClass->getProperty('configurationManager');
- $property->setValue($this->configurationManagerInterfaceMock);
- }
-
- /**
- * Reset static properties
- */
- protected function tearDown(): void
- {
- $reflectionClass = new \ReflectionClass(LocalizationUtility::class);
- $property = $reflectionClass->getProperty('configurationManager');
- $property->setValue(null);
- parent::tearDown();
- }
-
public static function translateDataProvider(): array
{
return [
@@ -75,10 +56,12 @@ final class LocalizationUtilityTest extends FunctionalTestCase
array $arguments = null
): void {
// No TypoScript overrides
- $this->configurationManagerInterfaceMock
+ $configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
+ $configurationManagerInterfaceMock
->method('getConfiguration')
->with('Framework', 'label_test', null)
->willReturn([]);
+ GeneralUtility::setSingletonInstance(ConfigurationManagerInterface::class, $configurationManagerInterfaceMock);
$GLOBALS['BE_USER'] = new BackendUserAuthentication();
$GLOBALS['BE_USER']->user = ['lang' => $languageKey];
@@ -97,10 +80,12 @@ final class LocalizationUtilityTest extends FunctionalTestCase
array $arguments = null
): void {
// No TypoScript overrides
- $this->configurationManagerInterfaceMock
+ $configurationManagerInterfaceMock = $this->createMock(ConfigurationManagerInterface::class);
+ $configurationManagerInterfaceMock
->method('getConfiguration')
->with('Framework', 'label_test', null)
->willReturn([]);
+ GeneralUtility::setSingletonInstance(ConfigurationManagerInterface::class, $configurationManagerInterfaceMock);
self::assertSame($expected, LocalizationUtility::translate($key, 'label_test', $arguments, $languageKey, $altLanguageKeys));
}
--
GitLab