From fe3d23f0d4c381040f47d24c4dcc9b27768a06b8 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 23 Oct 2015 11:46:58 +1300 Subject: [PATCH] BUG Fix GeneratedAssetHandler crashing on expired resources --- .../storage/CacheGeneratedAssetHandler.php | 28 ++++++++++++------- tests/filesystem/AssetStoreTest.php | 11 ++++++++ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/filesystem/storage/CacheGeneratedAssetHandler.php b/filesystem/storage/CacheGeneratedAssetHandler.php index 439cab2a7..2c6c47989 100644 --- a/filesystem/storage/CacheGeneratedAssetHandler.php +++ b/filesystem/storage/CacheGeneratedAssetHandler.php @@ -105,7 +105,10 @@ class CacheGeneratedAssetHandler implements GeneratedAssetHandler, Flushable { $data = $cache->load($cacheID); if($data) { $result = unserialize($data); - return $this->validateResult($result, $filename); + $valid = $this->validateResult($result, $filename); + if($valid) { + return $result; + } } // Regenerate @@ -127,8 +130,14 @@ class CacheGeneratedAssetHandler implements GeneratedAssetHandler, Flushable { if($result) { $cache->save(serialize($result), $cacheID); } - - return $this->validateResult($result, $filename); + + // Ensure this result is successfully saved + $valid = $this->validateResult($result, $filename); + if($valid) { + return $result; + } + + throw new Exception("Error regenerating file \"{$filename}\""); } /** @@ -151,17 +160,16 @@ class CacheGeneratedAssetHandler implements GeneratedAssetHandler, Flushable { * * @param mixed $result * @param string $filename - * @return array The result - * @throws Exception + * @return bool True if this $result is valid */ protected function validateResult($result, $filename) { + if(!$result) { + return false; + } + // Retrieve URL from tuple $store = $this->getAssetStore(); - if($result && $store->exists($result['Filename'], $result['Hash'], $result['Variant'])) { - return $result; - } - - throw new Exception("Error regenerating file \"{$filename}\""); + return $store->exists($result['Filename'], $result['Hash'], $result['Variant']); } } diff --git a/tests/filesystem/AssetStoreTest.php b/tests/filesystem/AssetStoreTest.php index bd4454a84..1d11606cf 100644 --- a/tests/filesystem/AssetStoreTest.php +++ b/tests/filesystem/AssetStoreTest.php @@ -7,6 +7,7 @@ use SilverStripe\Filesystem\Flysystem\FlysystemAssetStore; use SilverStripe\Filesystem\Flysystem\FlysystemUrlPlugin; use SilverStripe\Filesystem\Storage\AssetContainer; use SilverStripe\Filesystem\Storage\AssetStore; +use SilverStripe\Filesystem\Storage\CacheGeneratedAssetHandler; class AssetStoreTest extends SapphireTest { @@ -450,6 +451,10 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { * Reset defaults for this store */ public static function reset() { + // Need flushing since it won't have any files left + CacheGeneratedAssetHandler::flush(); + + // Remove all files in this store if(self::$basedir) { $path = self::base_path(); if(file_exists($path)) { @@ -469,6 +474,12 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { if($asset instanceof Folder) { return self::base_path() . '/' . $asset->getFilename(); } + if($asset instanceof File) { + $asset = $asset->File; + } + if($asset instanceof DBFile) { + return BASE_PATH . $asset->getSourceURL(); + } return BASE_PATH . $asset->getUrl(); }