Skip to content

Commit

Permalink
[TASK] Inject cache into SiteConfiguration
Browse files Browse the repository at this point in the history 
Do not use global state by looking up the CacheManager from
GeneralUtility, when we can actually inject the required cache.

The core cache is available from early bootstrap, therefore
we do not need to wait for CacheManager to be available, and
can inject it right away.

Releases: master
Resolves: #93440
Change-Id: I5c760acc2a1bdcc740fa3210450167e6bed7e235
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/67647
Tested-by: core-ci <typo3@b13.com>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Richard Haeser <richard@richardhaeser.com>
Reviewed-by: Jörg Bösche <typo3@joergboesche.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Richard Haeser <richard@richardhaeser.com>
  • Loading branch information
@bnf @haassie
bnf authored and haassie committed Feb 11, 2021
1 parent e833d08 commit 081de46
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 33 deletions.
31 changes: 13 additions & 18 deletions typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
View file
Expand Up @@ -19,7 +19,6 @@

use Symfony\Component\Finder\Finder;
use Symfony\Component\Yaml\Yaml;
use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Cache\Frontend\PhpFrontend;
use TYPO3\CMS\Core\Configuration\Loader\YamlFileLoader;
use TYPO3\CMS\Core\Exception\SiteNotFoundException;
Expand All @@ -37,6 +36,8 @@
*/
class SiteConfiguration implements SingletonInterface
{
protected PhpFrontend $cache;

/**
* @var string
*/
Expand Down Expand Up @@ -68,10 +69,15 @@ class SiteConfiguration implements SingletonInterface

/**
* @param string $configPath
* @param PhpFrontend $coreCache
*/
public function __construct(string $configPath)
public function __construct(string $configPath, PhpFrontend $coreCache = null)
{
$this->configPath = $configPath;
// The following fallback to GeneralUtility;:getContainer() is only used in acceptance tests
// @todo: Fix testing-framework/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
// to inject the cache instance
$this->cache = $coreCache ?? GeneralUtility::getContainer()->get('cache.core');
}

/**
Expand Down Expand Up @@ -151,7 +157,7 @@ public function resolveAllExistingSites(bool $useCache = true): array
protected function getAllSiteConfigurationFromFiles(bool $useCache = true): array
{
// Check if the data is already cached
$siteConfiguration = $useCache ? $this->getCache()->require($this->cacheIdentifier) : false;
$siteConfiguration = $useCache ? $this->cache->require($this->cacheIdentifier) : false;
if ($siteConfiguration !== false) {
return $siteConfiguration;
}
Expand All @@ -169,7 +175,7 @@ protected function getAllSiteConfigurationFromFiles(bool $useCache = true): arra
$identifier = basename($fileInfo->getPath());
$siteConfiguration[$identifier] = $configuration;
}
$this->getCache()->set($this->cacheIdentifier, 'return ' . var_export($siteConfiguration, true) . ';');
$this->cache->set($this->cacheIdentifier, 'return ' . var_export($siteConfiguration, true) . ';');

return $siteConfiguration;
}
Expand Down Expand Up @@ -223,7 +229,7 @@ public function write(string $siteIdentifier, array $configuration): void
$yamlFileContents = Yaml::dump($newConfiguration, 99, 2);
GeneralUtility::writeFile($fileName, $yamlFileContents);
$this->firstLevelCache = null;
$this->getCache()->remove($this->cacheIdentifier);
$this->cache->remove($this->cacheIdentifier);
}

/**
Expand All @@ -239,7 +245,7 @@ public function rename(string $currentIdentifier, string $newIdentifier): void
if (!$result) {
throw new \RuntimeException('Unable to rename folder sites/' . $currentIdentifier, 1522491300);
}
$this->getCache()->remove($this->cacheIdentifier);
$this->cache->remove($this->cacheIdentifier);
$this->firstLevelCache = null;
}

Expand All @@ -261,21 +267,10 @@ public function delete(string $siteIdentifier): void
throw new SiteNotFoundException('Site configuration file ' . $this->configFileName . ' within the site ' . $siteIdentifier . ' not found.', 1522866184);
}
@unlink($fileName);
$this->getCache()->remove($this->cacheIdentifier);
$this->cache->remove($this->cacheIdentifier);
$this->firstLevelCache = null;
}

/**
* Short-hand function for the cache
*
* @return PhpFrontend
* @throws \TYPO3\CMS\Core\Cache\Exception\NoSuchCacheException
*/
protected function getCache(): PhpFrontend
{
return GeneralUtility::makeInstance(CacheManager::class)->getCache('core');
}

/**
* @param array $newConfiguration
* @return array
Expand Down
5 changes: 4 additions & 1 deletion typo3/sysext/core/Classes/ServiceProvider.php
View file
Expand Up @@ -128,7 +128,10 @@ public static function getCharsetConverter(ContainerInterface $container): Chars

public static function getSiteConfiguration(ContainerInterface $container): Configuration\SiteConfiguration
{
return self::new($container, Configuration\SiteConfiguration::class, [Environment::getConfigPath() . '/sites']);
return self::new($container, Configuration\SiteConfiguration::class, [
Environment::getConfigPath() . '/sites',
$container->get('cache.core')
]);
}

public static function getListCommand(ContainerInterface $container): Command\ListCommand
Expand Down
1 change: 1 addition & 0 deletions typo3/sysext/core/Configuration/Services.yaml
View file
Expand Up @@ -44,6 +44,7 @@ services:

TYPO3\CMS\Core\Configuration\SiteConfiguration:
arguments:
$coreCache: '@cache.core'
$configPath: "%env(TYPO3:configPath)%/sites"

TYPO3\CMS\Core\Package\UnitTestPackageManager:
Expand Down
3 changes: 2 additions & 1 deletion typo3/sysext/core/Tests/Functional/Routing/Aspect/PersistedAliasMapperTest.php
View file
Expand Up @@ -327,8 +327,9 @@ private function writeSiteConfiguration(Site $site): void
try {
// ensure no previous site configuration influences the test
$path = $this->instancePath . '/typo3conf/sites';
$cache = $this->getContainer()->get('cache.core');
GeneralUtility::rmdir($path . '/' . $site->getIdentifier(), true);
GeneralUtility::makeInstance(SiteConfiguration::class, $path)->write($site->getIdentifier(), $site->getConfiguration());
GeneralUtility::makeInstance(SiteConfiguration::class, $path, $cache)->write($site->getIdentifier(), $site->getConfiguration());
} catch (\Exception $exception) {
self::markTestSkipped($exception->getMessage());
}
Expand Down
3 changes: 2 additions & 1 deletion typo3/sysext/core/Tests/Functional/Routing/Aspect/PersistedPatternMapperTest.php
View file
Expand Up @@ -331,8 +331,9 @@ private function writeSiteConfiguration(Site $site): void
try {
// ensure no previous site configuration influences the test
$path = $this->instancePath . '/typo3conf/sites';
$cache = $this->getContainer()->get('cache.core');
GeneralUtility::rmdir($path . '/' . $site->getIdentifier(), true);
GeneralUtility::makeInstance(SiteConfiguration::class, $path)->write($site->getIdentifier(), $site->getConfiguration());
GeneralUtility::makeInstance(SiteConfiguration::class, $path, $cache)->write($site->getIdentifier(), $site->getConfiguration());
} catch (\Exception $exception) {
self::markTestSkipped($exception->getMessage());
}
Expand Down
6 changes: 4 additions & 2 deletions typo3/sysext/core/Tests/Functional/SiteHandling/SiteBasedTestTrait.php
View file
Expand Up @@ -65,7 +65,8 @@ protected function writeSiteConfiguration(
$configuration['errorHandling'] = $errorHandling;
}
$siteConfiguration = new SiteConfiguration(
$this->instancePath . '/typo3conf/sites/'
$this->instancePath . '/typo3conf/sites/',
$this->getContainer()->get('cache.core')
);

try {
Expand All @@ -86,7 +87,8 @@ protected function mergeSiteConfiguration(
array $overrides
) {
$siteConfiguration = new SiteConfiguration(
$this->instancePath . '/typo3conf/sites/'
$this->instancePath . '/typo3conf/sites/',
$this->getContainer()->get('cache.core')
);
$configuration = $siteConfiguration->load($identifier);
$configuration = array_merge($configuration, $overrides);
Expand Down
9 changes: 4 additions & 5 deletions typo3/sysext/core/Tests/Unit/Configuration/SiteConfigurationTest.php
View file
Expand Up @@ -19,7 +19,6 @@

use Prophecy\Argument;
use Symfony\Component\Yaml\Yaml;
use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Cache\Frontend\PhpFrontend;
use TYPO3\CMS\Core\Configuration\Loader\YamlFileLoader;
use TYPO3\CMS\Core\Configuration\SiteConfiguration;
Expand Down Expand Up @@ -53,15 +52,15 @@ protected function setUp(): void
GeneralUtility::mkdir_deep($this->fixturePath);
}
$this->testFilesToDelete[] = $basePath;
$cacheManager = $this->prophesize(CacheManager::class);
$coreCacheProphecy = $this->prophesize(PhpFrontend::class);
$coreCacheProphecy->require(Argument::any())->willReturn(false);
$coreCacheProphecy->set(Argument::any(), Argument::any())->willReturn(null);
$coreCacheProphecy->remove(Argument::any(), Argument::any())->willReturn(null);
$cacheManager->getCache('core')->willReturn($coreCacheProphecy->reveal());
GeneralUtility::setSingletonInstance(CacheManager::class, $cacheManager->reveal());

$this->siteConfiguration = new SiteConfiguration($this->fixturePath);
$this->siteConfiguration = new SiteConfiguration(
$this->fixturePath,
$coreCacheProphecy->reveal()
);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions typo3/sysext/extensionmanager/Tests/Unit/Utility/InstallUtilityTest.php
View file
Expand Up @@ -302,7 +302,7 @@ public function siteConfigGetsMovedIntoPlace()
GeneralUtility::mkdir_deep($absPath . 'Initialisation/Site/' . $siteIdentifier);
file_put_contents($absPath . 'Initialisation/Site/' . $siteIdentifier . '/config.yaml', $config);

GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites', new NullFrontend('core')));

$subject = new InstallUtility();
$subject->injectEventDispatcher($this->prophesize(EventDispatcherInterface::class)->reveal());
Expand Down Expand Up @@ -370,7 +370,7 @@ public function siteConfigGetsNotOverriddenIfExistsAlready()
GeneralUtility::mkdir_deep($configDir . '/sites/' . $siteIdentifier);
file_put_contents($configDir . '/' . $existingSiteConfig, $config);

GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites', new NullFrontend('core')));

$subject = new InstallUtility();
$subject->injectEventDispatcher($this->prophesize(EventDispatcherInterface::class)->reveal());
Expand Down
6 changes: 3 additions & 3 deletions typo3/sysext/frontend/Tests/Unit/ContentObject/ContentObjectRendererTest.php
View file
Expand Up @@ -2542,9 +2542,9 @@ public function typolinkReturnsCorrectLinksForEmailsAndUrls($linkText, $configur
$GLOBALS['TSFE'] = $typoScriptFrontendControllerMockObject;

$this->cacheManager->getCache('runtime')->willReturn(new NullFrontend('dummy'));
$this->cacheManager->getCache('core')->willReturn(new NullFrontend('dummy'));
$this->cacheManager->getCache('core')->willReturn(new NullFrontend('runtime'));

GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites', new NullFrontend('dummy')));

$this->subject->_set('typoScriptFrontendController', $typoScriptFrontendControllerMockObject);

Expand Down Expand Up @@ -2992,7 +2992,7 @@ public function typolinkOpensInNewWindow()
{
$this->cacheManager->getCache('runtime')->willReturn(new NullFrontend('runtime'));
$this->cacheManager->getCache('core')->willReturn(new NullFrontend('runtime'));
GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites'));
GeneralUtility::setSingletonInstance(SiteConfiguration::class, new SiteConfiguration(Environment::getConfigPath() . '/sites', new NullFrontend('dummy')));
$linkText = 'Nice Text';
$configuration = [
'parameter' => 'https://example.com 13x84:target=myexample'
Expand Down

0 comments on commit 081de46

Please sign in to comment.