Use SilverStripe cache API instead of memory cache, add tests and fix cache config indentation

_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
+    factory: SilverStripe\Core\Cache\CacheFactory
-      constructor:
+    constructor:
-        namespace: "VersionProvider_composerlock"
+      namespace: "VersionProvider_composerlock"
-        args:
+      args:
-          disable-container: true
+        disable-container: true
   Psr\SimpleCache\CacheInterface.RateLimiter:
-      factory: SilverStripe\Core\Cache\CacheFactory
+    factory: SilverStripe\Core\Cache\CacheFactory
-      constructor:
+    constructor:
-        namespace: 'ratelimiter'
+      namespace: 'ratelimiter'
-        args:
+      args:
-          disable-container: true
+        disable-container: true
   Psr\SimpleCache\CacheInterface.InheritedPermissions:
     factory: SilverStripe\Core\Cache\CacheFactory
     constructor:
       namespace: "InheritedPermissions"
       args:
         disable-container: true
+  Psr\SimpleCache\CacheInterface.ThemeResourceLoader:
+    factory: SilverStripe\Core\Cache\CacheFactory
+    constructor:
+      namespace: 'ThemeResourceLoader'

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);
+        $cacheKey = md5(json_encode($template) . json_encode($themes));
-        if (isset(static::$cacheData[$cacheKey])) {
+        if ($this->getCache()->has($cacheKey)) {
-            return static::$cacheData[$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;
     }
 }

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
+        // Ensure the cache is flushed between tests
-        ThemeResourceLoader::flushCache();
+        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']));
+    }
 }