From da9301f241085a6359d36a88f28df4d5dc1ec529 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 5 Oct 2018 14:58:48 +0200 Subject: [PATCH] Use SilverStripe cache API instead of memory cache, add tests and fix cache config indentation --- _config/cache.yml | 26 +++++---- src/View/ThemeResourceLoader.php | 57 +++++++++++++++---- .../Core/Manifest/ThemeResourceLoaderTest.php | 29 +++++++++- 3 files changed, 88 insertions(+), 24 deletions(-) diff --git a/_config/cache.yml b/_config/cache.yml index 48715eb4f..7719370f3 100644 --- a/_config/cache.yml +++ b/_config/cache.yml @@ -15,20 +15,24 @@ SilverStripe\Core\Injector\Injector: namespace: "cacheblock" defaultLifetime: 600 Psr\SimpleCache\CacheInterface.VersionProvider_composerlock: - factory: SilverStripe\Core\Cache\CacheFactory - constructor: - namespace: "VersionProvider_composerlock" - args: - disable-container: true + factory: SilverStripe\Core\Cache\CacheFactory + constructor: + namespace: "VersionProvider_composerlock" + args: + disable-container: true Psr\SimpleCache\CacheInterface.RateLimiter: - factory: SilverStripe\Core\Cache\CacheFactory - constructor: - namespace: 'ratelimiter' - args: - disable-container: true + factory: SilverStripe\Core\Cache\CacheFactory + constructor: + namespace: 'ratelimiter' + args: + disable-container: true Psr\SimpleCache\CacheInterface.InheritedPermissions: factory: SilverStripe\Core\Cache\CacheFactory constructor: namespace: "InheritedPermissions" args: - disable-container: true \ No newline at end of file + disable-container: true + Psr\SimpleCache\CacheInterface.ThemeResourceLoader: + factory: SilverStripe\Core\Cache\CacheFactory + constructor: + namespace: 'ThemeResourceLoader' diff --git a/src/View/ThemeResourceLoader.php b/src/View/ThemeResourceLoader.php index 9a742489b..a0e3b07a4 100644 --- a/src/View/ThemeResourceLoader.php +++ b/src/View/ThemeResourceLoader.php @@ -3,13 +3,16 @@ namespace SilverStripe\View; use InvalidArgumentException; +use Psr\SimpleCache\CacheInterface; +use SilverStripe\Core\Flushable; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\Core\Path; /** * Handles finding templates from a stack of template manifest objects. */ -class ThemeResourceLoader +class ThemeResourceLoader implements Flushable { /** @@ -39,6 +42,11 @@ class ThemeResourceLoader */ protected $sets = []; + /** + * @var CacheInterface + */ + protected $cache; + /** * @return ThemeResourceLoader */ @@ -168,6 +176,8 @@ class ThemeResourceLoader * format "type/name", where type is the type of template to search for * (e.g. Includes, Layout). * + * The results of this method will be cached for future use. + * * @param string|array $template Template name, or template spec in array format with the keys * 'type' (type string) and 'templates' (template hierarchy in order of precedence). * If 'templates' is ommitted then any other item in the array will be treated as the template @@ -182,9 +192,9 @@ class ThemeResourceLoader public function findTemplate($template, $themes = null) { // Look for a cached result for this data set - $cacheKey = json_encode($template) . json_encode($themes); - if (isset(static::$cacheData[$cacheKey])) { - return static::$cacheData[$cacheKey]; + $cacheKey = md5(json_encode($template) . json_encode($themes)); + if ($this->getCache()->has($cacheKey)) { + return $this->getCache()->get($cacheKey); } if ($themes === null) { @@ -209,7 +219,8 @@ class ThemeResourceLoader if (is_array($template)) { $path = $this->findTemplate($template, $themes); if ($path) { - return static::$cacheData[$cacheKey] = $path; + $this->getCache()->set($cacheKey, $path); + return $path; } continue; } @@ -218,7 +229,8 @@ class ThemeResourceLoader // pass in templates without extensions in order for template manifest to find // files dynamically. if (substr($template, -3) == '.ss' && file_exists($template)) { - return static::$cacheData[$cacheKey] = $template; + $this->getCache()->set($cacheKey, $template); + return $template; } // Check string template identifier @@ -233,13 +245,15 @@ class ThemeResourceLoader $pathParts = [ $this->base, $themePath, 'templates', $head, $type, $tail ]; $path = Path::join($pathParts) . '.ss'; if (file_exists($path)) { - return static::$cacheData[$cacheKey] = $path; + $this->getCache()->set($cacheKey, $path); + return $path; } } } // No template found - return static::$cacheData[$cacheKey] = null; + $this->getCache()->set($cacheKey, null); + return null; } /** @@ -352,10 +366,31 @@ class ThemeResourceLoader } /** - * Clear any internally cached data from memory + * Flush any cached data */ - public static function flushCache() + public static function flush() { - static::$cacheData = []; + self::inst()->getCache()->clear(); + } + + /** + * @return CacheInterface + */ + public function getCache() + { + if (!$this->cache) { + $this->setCache(Injector::inst()->get(CacheInterface::class . '.ThemeResourceLoader')); + } + return $this->cache; + } + + /** + * @param CacheInterface $cache + * @return ThemeResourceLoader + */ + public function setCache(CacheInterface $cache) + { + $this->cache = $cache; + return $this; } } diff --git a/tests/php/Core/Manifest/ThemeResourceLoaderTest.php b/tests/php/Core/Manifest/ThemeResourceLoaderTest.php index aaf8ff27e..42616ef99 100644 --- a/tests/php/Core/Manifest/ThemeResourceLoaderTest.php +++ b/tests/php/Core/Manifest/ThemeResourceLoaderTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Core\Tests\Manifest; +use Psr\SimpleCache\CacheInterface; use SilverStripe\Control\Director; use SilverStripe\Core\Manifest\ModuleLoader; use SilverStripe\View\ThemeResourceLoader; @@ -55,8 +56,8 @@ class ThemeResourceLoaderTest extends SapphireTest $this->loader = new ThemeResourceLoader($this->base); $this->loader->addSet('$default', $this->manifest); - // Flush any cached values - ThemeResourceLoader::flushCache(); + // Ensure the cache is flushed between tests + ThemeResourceLoader::flush(); } protected function tearDown() @@ -380,4 +381,28 @@ class ThemeResourceLoaderTest extends SapphireTest { $this->assertEquals($path, $this->loader->getPath($name)); } + + public function testFindTemplateWithCacheMiss() + { + $mockCache = $this->createMock(CacheInterface::class); + $mockCache->expects($this->once())->method('has')->willReturn(false); + $mockCache->expects($this->never())->method('get'); + $mockCache->expects($this->once())->method('set'); + + $loader = new ThemeResourceLoader(); + $loader->setCache($mockCache); + $loader->findTemplate('Page', ['$default']); + } + + public function testFindTemplateWithCacheHit() + { + $mockCache = $this->createMock(CacheInterface::class); + $mockCache->expects($this->once())->method('has')->willReturn(true);; + $mockCache->expects($this->never())->method('set'); + $mockCache->expects($this->once())->method('get')->willReturn('mock_template.ss'); + + $loader = new ThemeResourceLoader(); + $loader->setCache($mockCache); + $this->assertSame('mock_template.ss', $loader->findTemplate('Page', ['$default'])); + } }