API Replace CacheGeneratedAssetHandler with FlysystemGeneratedAssetHandler

API Reduce GeneratedAssetHandler API
API Re-introduce Requirements::delete_all_combined_files();
API Re-introduce Requirements::flush()
API Combined files now uses new filenames distinguished by sha1 of sources

_config/asset.yml

@@ -26,8 +26,8 @@ Injector:
   # Image mechanism
   Image_Backend: GDBackend
   GeneratedAssetHandler:
-    class: SilverStripe\Filesystem\Storage\CacheGeneratedAssetHandler
+    class: SilverStripe\Filesystem\Storage\FlysystemGeneratedAssetHandler
     properties:
-      AssetStore: '%$AssetStore'
+      Filesystem: '%$FlysystemBackend'
   Requirements_Minifier:
     class: SilverStripe\View\JSMinifier

docs/en/04_Changelogs/4.0.0.md

@@ -38,6 +38,7 @@
 
 ### ErrorPage
 
+ * `ErrorPage.static_filepath` config has been removed.
  * `ErrorPage::get_filepath_for_errorcode` has been removed
  * `ErrorPage::alternateFilepathForErrorcode` extension point has been removed
 
@@ -123,7 +124,6 @@ New methods on `Requirements` are added to access these:
 And some methods on `Requirements` and `Requirements_Backend` have been removed as they are obsolete.
 
  * `delete_combined_files` (both classes)
- * `delete_all_combined_files` (both classes)
 
 A new config `Requirements_Backend.combine_in_dev` has been added in order to allow combined files to be
 forced on during development. If this is off, combined files is only enabled in live environments.
@@ -418,27 +418,13 @@ After:
 
 ### Update code that modifies the behaviour of ErrorPage
 
-Since ErrorPage writes statically cached files for each dataobject, in order to integrate it with both the
-new asset backend, and ensure that .htaccess static references still works, these files are now cached
-potentially in two places:
+ErrorPage has been updated to use a configurable asset backend, similar to the `AssetStore` described above.
+This replaces the `ErrorPage.static_filepath` config that was used to write local files.
 
- * When an error is generated within the live environment, by default the error handler will query the 
-`GeneratedAssetHandler` for cached content, which is then served up in the same PHP request. This is the
-primary cache for error content, which does not involve database access, but is processed within the
-PHP process. Although, the file path for this cache is not predictable, as it uses the configured asset backend,
-and is not necessarily present on the same filesystem as the site code.
+As a result, error pages may be cached either to a local filesystem, or an external Flysystem store
+(which is configured via setting a new Flysystem backend with YAML).
 
- * In order to ensure that the webserver has direct access an available cached error page, it can be necessary
-to ensure a physical file is present locally. By setting the `ErrorPage.enable_static_file` config,
-the generation of this file can be controlled. When this disabled, the static file will only be cached
-via the configured backend. When this is enabled (as it is by default) then the error page will be generated
-in the same location as it were in framework version 3.x. E.g. `/assets/error-404.html`
-
-If your webserver relies on static paths encoded in `.htaccess` to these files, then it's preferable to leave
-this option on. If using a non-local filesystem, and another mechanism for intercepting webserver errors,
-then it may be preferable to leave this off, meaning that the local assets folder is unnecessary.
-
-`ErrorPage::get_filepath_for_errorcode` has been removed, because the local path for a specific code is
+`ErrorPage::get_filepath_for_errorcode()` has been removed, because the local path for a specific code is
 no longer assumed. Instead you should use `ErrorPage::get_content_for_errorcode` which retrieves the
 appropriate content for that error using one of the methods above.
 

filesystem/flysystem/FlysystemGeneratedAssetHandler.php

@@ -0,0 +1,107 @@
+<?php
+
+namespace SilverStripe\Filesystem\Storage;
+
+use Config;
+use Exception;
+use League\Flysystem\Filesystem;
+
+/**
+ * Simple Flysystem implementation of GeneratedAssetHandler for storing generated content
+ * 
+ * @package framework
+ * @subpackage filesystem
+ */
+class FlysystemGeneratedAssetHandler implements GeneratedAssetHandler {
+
+	/**
+	 * Flysystem store for files
+	 *
+	 * @var Filesystem
+	 */
+	protected $assetStore = null;
+
+	/**
+	 * Assign the asset backend
+	 *
+	 * @param Filesystem $store
+	 * @return $this
+	 */
+	public function setFilesystem(Filesystem $store) {
+		$this->assetStore = $store;
+		return $this;
+	}
+
+	/**
+	 * Get the asset backend
+	 *
+	 * @return Filesystem
+	 */
+	public function getFilesystem() {
+		return $this->assetStore;
+	}
+
+    public function getContentURL($filename, $callback = null) {
+		$result = $this->checkOrCreate($filename, $callback);
+		if($result) {
+			return $this
+				->getFilesystem()
+				->getPublicUrl($filename);
+		}
+	}
+
+	public function getContent($filename, $callback = null) {
+		$result = $this->checkOrCreate($filename, $callback);
+		if($result) {
+			return $this
+				->getFilesystem()
+				->read($filename);
+		}
+	}
+
+	/**
+	 * Check if the file exists or that the $callback provided was able to regenerate it.
+	 *
+	 * @param string $filename
+	 * @param callable $callback
+	 * @return bool Whether or not the file exists
+	 * @throws Exception If an error has occurred during save
+	 */
+	protected function checkOrCreate($filename, $callback = null)
+	{
+		// Check if there is an existing asset
+		if ($this->getFilesystem()->has($filename)) {
+			return true;
+		}
+
+		if (!$callback) {
+			return false;
+		}
+
+		// Invoke regeneration and save
+		$content = call_user_func($callback);
+		$this->setContent($filename, $content);
+		return true;
+	}
+
+	public function setContent($filename, $content) {
+		// Store content
+		$result = $this
+				->getFilesystem()
+				->put($filename, $content);
+
+		if(!$result) {
+			throw new Exception("Error regenerating file \"{$filename}\"");
+		}
+	}
+
+	public function removeContent($filename) {
+		if($this->getFilesystem()->has($filename)) {
+			$handler = $this->getFilesystem()->get($filename);
+			$handler->delete();
+		}
+
+	}
+
+
+}

filesystem/storage/CacheGeneratedAssetHandler.php

@@ -1,177 +0,0 @@
-<?php
-
-namespace SilverStripe\Filesystem\Storage;
-
-use Config;
-use Exception;
-use Flushable;
-use SS_Cache;
-use Zend_Cache_Core;
-
-/**
- * Handle references to generated files via cached tuples
- *
- * Important: If you are using the default FlysystemStore with legacy_filenames, you will need to ?flush
- * in order to refresh combined files.
- * 
- * @package framework
- * @subpackage filesystem
- */
-class CacheGeneratedAssetHandler implements GeneratedAssetHandler, Flushable {
-
-	/**
-	 * Lifetime of cache
-	 *
-	 * @config
-	 * @var int
-	 */
-	private static $lifetime = null;
-
-	/**
-	 * Backend for generated files
-	 *
-	 * @var AssetStore
-	 */
-	protected $assetStore = null;
-
-	/**
-	 * Assign the asset backend
-	 *
-	 * @param AssetStore $store
-	 * @return $this
-	 */
-	public function setAssetStore(AssetStore $store) {
-		$this->assetStore = $store;
-		return $this;
-	}
-
-	/**
-	 * Get the asset backend
-	 *
-	 * @return AssetStore
-	 */
-	public function getAssetStore() {
-		return $this->assetStore;
-	}
-
-	/**
-	 * @return Zend_Cache_Core
-	 */
-	protected static function get_cache() {
-		$cache = SS_Cache::factory('CacheGeneratedAssetHandler');
-		$lifetime = Config::inst()->get(__CLASS__, 'lifetime') ?: null; // map falsey to null (indefinite)
-		$cache->setLifetime($lifetime);
-		return $cache;
-	}
-
-	/**
-	 * Flush the cache
-	 */
-	public static function flush() {
-		self::get_cache()->clean();
-	}
-
-	public function getGeneratedURL($filename, $entropy = 0, $callback = null) {
-		$result = $this->getGeneratedFile($filename, $entropy, $callback);
-		if($result) {
-			return $this
-				->getAssetStore()
-				->getAsURL($result['Filename'], $result['Hash'], $result['Variant']);
-		}
-	}
-
-	public function getGeneratedContent($filename, $entropy = 0, $callback = null) {
-		$result = $this->getGeneratedFile($filename, $entropy, $callback);
-		if($result) {
-			return $this
-				->getAssetStore()
-				->getAsString($result['Filename'], $result['Hash'], $result['Variant']);
-		}
-	}
-
-	/**
-	 * Generate or return the tuple for the given file, optionally regenerating it if it
-	 * doesn't exist
-	 *
-	 * @param string $filename
-	 * @param mixed $entropy
-	 * @param callable $callback
-	 * @return array tuple array if available
-	 * @throws Exception If the file isn't available and $callback fails to regenerate content
-	 */
-	protected function getGeneratedFile($filename, $entropy = 0, $callback = null) {
-		// Check if there is an existing asset
-		$cache = self::get_cache();
-		$cacheID = $this->getCacheKey($filename, $entropy);
-		$data = $cache->load($cacheID);
-		if($data) {
-			$result = unserialize($data);
-			$valid = $this->validateResult($result, $filename);
-			if($valid) {
-				return $result;
-			}
-		}
-
-		// Regenerate
-		if($callback) {
-			// Invoke regeneration and save
-			$content = call_user_func($callback);
-			return $this->updateContent($filename, $entropy, $content);
-		}
-	}
-
-	public function updateContent($filename, $entropy, $content) {
-		$cache = self::get_cache();
-		$cacheID = $this->getCacheKey($filename, $entropy);
-
-		// Store content
-		$result = $this
-			->getAssetStore()
-			->setFromString($content, $filename);
-		if($result) {
-			$cache->save(serialize($result), $cacheID);
-		}
-
-		// Ensure this result is successfully saved
-		$valid = $this->validateResult($result, $filename);
-		if($valid) {
-			return $result;
-		}
-
-		throw new Exception("Error regenerating file \"{$filename}\"");
-	}
-
-
-	/**
-	 * Get cache key for the given generated asset
-	 *
-	 * @param string $filename
-	 * @param mixed $entropy
-	 * @return string
-	 */
-	protected function getCacheKey($filename, $entropy = 0) {
-		$cacheID = sha1($filename);
-		if($entropy) {
-			$cacheID .= '_' . sha1($entropy);
-		}
-		return $cacheID;
-	}
-
-	/**
-	 * Validate that the given result is valid
-	 *
-	 * @param mixed $result
-	 * @param string $filename
-	 * @return bool True if this $result is valid
-	 */
-	protected function validateResult($result, $filename) {
-		if(!$result) {
-			return false;
-		}
-		
-		// Retrieve URL from tuple
-		$store = $this->getAssetStore();
-		return $store->exists($result['Filename'], $result['Hash'], $result['Variant']);
-	}
-
-}

filesystem/storage/GeneratedAssetHandler.php

@@ -11,37 +11,45 @@ namespace SilverStripe\Filesystem\Storage;
 interface GeneratedAssetHandler {
 
 	/**
-	 * Given a filename and entropy, determine if a pre-generated file is valid. If this file is invalid
-	 * or expired, invoke $callback to regenerate the content.
+     * Returns a URL to a generated asset, if one is available.
      * 
-	 * Returns a URL to the generated file
+	 * Given a filename, determine if a file is available. If the file is unavailable,
+     * and a callback is supplied, invoke it to regenerate the content.
 	 *
 	 * @param string $filename
-	 * @param mixed $entropy
 	 * @param callable $callback To generate content. If none provided, url will only be returned
 	 * if there is valid content.
 	 * @return string URL to generated file
 	 */
-	public function getGeneratedURL($filename, $entropy = 0, $callback = null);
+	public function getContentURL($filename, $callback = null);
 
 	/**
-	 * Given a filename and entropy, determine if a pre-generated file is valid. If this file is invalid
-	 * or expired, invoke $callback to regenerate the content.
+	 * Returns the content for a generated asset, if one is available.
+     * 
+	 * Given a filename, determine if a file is available. If the file is unavailable,
+     * and a callback is supplied, invoke it to regenerate the content.
 	 *
 	 * @param string $filename
-	 * @param mixed $entropy
 	 * @param callable $callback To generate content. If none provided, content will only be returned
 	 * if there is valid content.
 	 * @return string Content for this generated file
 	 */
-	public function getGeneratedContent($filename, $entropy = 0, $callback = null);
+	public function getContent($filename, $callback = null);
 
 	/**
 	 * Update content with new value
 	 *
 	 * @param string $filename
-	 * @param mixed $entropy
 	 * @param string $content Content to write to the backend
 	 */
-	public function updateContent($filename, $entropy, $content);
+	public function setContent($filename, $content);
+    
+    /**
+     * Remove any content under the given file.
+     * 
+     * If $filename is a folder, it should delete all files underneath it also.
+     * 
+     * @param string $filename
+     */
+    public function removeContent($filename);
 }

tests/filesystem/AssetStoreTest.php

@@ -7,7 +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;
+use SilverStripe\Filesystem\Storage\FlysystemGeneratedAssetHandler;
 
 class AssetStoreTest extends SapphireTest {
 
@@ -424,6 +424,11 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore {
 		$backend->setFilesystem($filesystem);
 		Injector::inst()->registerService($backend, 'AssetStore');
 
+		// Assign flysystem backend to generated asset handler at the same time
+		$generated = new FlysystemGeneratedAssetHandler();
+		$generated->setFilesystem($filesystem);
+		Injector::inst()->registerService($generated, 'GeneratedAssetHandler');
+
 		// Disable legacy and set defaults
 		Config::inst()->remove(get_class(new FlysystemAssetStore()), 'legacy_filenames');
 		Config::inst()->update('Director', 'alternate_base_url', '/');
@@ -451,9 +456,6 @@ 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();

tests/forms/RequirementsTest.php

@@ -1,6 +1,5 @@
 <?php
 
-use SilverStripe\Filesystem\Storage\CacheGeneratedAssetHandler;
 /**
  * @package framework
  * @subpackage tests
@@ -72,7 +71,7 @@ class RequirementsTest extends SapphireTest {
 		$backend->clearCombinedFiles();
 		$backend->setCombinedFilesFolder('_combinedfiles');
 		$backend->setMinifyCombinedJSFiles(false);
-		CacheGeneratedAssetHandler::flush();
+		Requirements::flush();
 	}
 
 	/**
@@ -122,7 +121,7 @@ class RequirementsTest extends SapphireTest {
 		$backend = new Requirements_Backend();
 		$this->setupCombinedRequirements($backend);
 
-		$combinedFileName = '/_combinedfiles/b2a28d2463/RequirementsTest_bc.js';
+		$combinedFileName = '/_combinedfiles/RequirementsTest_bc-51622b5.js';
 		$combinedFilePath = AssetStoreTest_SpyStore::base_path() . $combinedFileName;
 
 		$html = $backend->includeInHTML(false, self::$html_template);
@@ -220,7 +219,7 @@ class RequirementsTest extends SapphireTest {
 		$html = $backend->includeInHTML(false, self::$html_template);
 
 		$this->assertRegExp(
-			'/href=".*\/print\.css/',
+			'/href=".*\/print\-94e723d\.css/',
 			$html,
 			'Print stylesheets have been combined.'
 		);
@@ -250,7 +249,7 @@ class RequirementsTest extends SapphireTest {
 
 		$html = $backend->includeInHTML(false, self::$html_template);
 		$this->assertRegExp(
-			'/href=".*\/style\.css/',
+			'/href=".*\/style\-2b3e4c9\.css/',
 			$html,
 			'Stylesheets have been combined.'
 		);
@@ -260,7 +259,7 @@ class RequirementsTest extends SapphireTest {
 		$basePath = $this->getCurrentRelativePath();
 		$backend = new Requirements_Backend();
 		$this->setupCombinedRequirements($backend);
-		$combinedFileName = '/_combinedfiles/b2a28d2463/RequirementsTest_bc.js';
+		$combinedFileName = '/_combinedfiles/RequirementsTest_bc-51622b5.js';
 		$combinedFilePath = AssetStoreTest_SpyStore::base_path() . $combinedFileName;
 
 		/* BLOCKED COMBINED FILES ARE NOT INCLUDED */
@@ -279,7 +278,7 @@ class RequirementsTest extends SapphireTest {
 		/* BLOCKED UNCOMBINED FILES ARE NOT INCLUDED */
 		$this->setupCombinedRequirements($backend);
 		$backend->block($basePath .'/RequirementsTest_b.js');
-		$combinedFileName2 = '/_combinedfiles/37bd2d9dcb/RequirementsTest_bc.js'; // SHA1 without file c included
+		$combinedFileName2 = '/_combinedfiles/RequirementsTest_bc-fc7468e.js'; // SHA1 without file c included
 		$combinedFilePath2 = AssetStoreTest_SpyStore::base_path() . $combinedFileName2;
 		clearstatcache(); // needed to get accurate file_exists() results
 		$html = $backend->includeInHTML(false, self::$html_template);

tests/view/SSViewerTest.php

@@ -167,7 +167,7 @@ class SSViewerTest extends SapphireTest {
 		$testBackend->processCombinedFiles();
 		$js = $testBackend->getJavascript();
 		$combinedTestFilePath = BASE_PATH . reset($js);
-		$this->assertContains('testRequirementsCombine.js', $combinedTestFilePath);
+		$this->assertContains('_combinedfiles/testRequirementsCombine-7c20750.js', $combinedTestFilePath);
 
 		// and make sure the combined content matches the input content, i.e. no loss of functionality
 		if(!file_exists($combinedTestFilePath)) {

view/Requirements.php

@@ -8,7 +8,14 @@ use SilverStripe\Filesystem\Storage\GeneratedAssetHandler;
  * @package framework
  * @subpackage view
  */
-class Requirements {
+class Requirements implements Flushable {
+
+	/**
+	 * Triggered early in the request when a flush is requested
+	 */
+	public static function flush() {
+		self::delete_all_combined_files();
+	}
 
 	/**
 	 * Enable combining of css/javascript files.
@@ -323,6 +330,14 @@ class Requirements {
 		return self::backend()->getCombinedFiles();
 	}
 
+	/**
+	 * Deletes all generated combined files in the configured combined files directory,
+	 * but doesn't delete the directory itself
+	 */
+	public static function delete_all_combined_files() {
+		return self::backend()->deleteAllCombinedFiles();
+	}
+
 	/**
 	 * Re-sets the combined files definition. See {@link Requirements_Backend::clear_combined_files()}
 	 */
@@ -1328,6 +1343,16 @@ class Requirements_Backend {
 		return $this->combinedFiles;
 	}
 
+	/**
+	 * Clears all combined files
+	 */
+	public function deleteAllCombinedFiles() {
+		$combinedFolder = $this->getCombinedFilesFolder();
+		if($combinedFolder) {
+			$this->getAssetHandler()->removeContent($combinedFolder);
+		}
+	}
+
 	/**
 	 * Clear all registered CSS and JavaScript file combinations
 	 */
@@ -1404,25 +1429,25 @@ class Requirements_Backend {
 	 * @return string URL to this resource
 	 */
 	protected function getCombinedFileURL($combinedFile, $fileList, $type) {
-		// Generate path (Filename)
-		$combinedFileID = File::join_paths($this->getCombinedFilesFolder(), $combinedFile);
+		// Filter blocked files
+		$fileList = array_diff($fileList, $this->getBlocked());
 
-		// Get entropy for this combined file (last modified date of most recent file)
-		$entropy = $this->getEntropyOfFiles($fileList);
+		// Generate path (Filename)
+		$filename = $this->hashedCombinedFilename($combinedFile, $fileList);
+		$combinedFileID =  File::join_paths($this->getCombinedFilesFolder(), $filename);
 
 		// Send file combination request to the backend, with an optional callback to perform regeneration
 		$minify = $this->getMinifyCombinedJSFiles();
 		$combinedURL = $this
 			->getAssetHandler()
-			->getGeneratedURL(
+			->getContentURL(
 				$combinedFileID,
-				$entropy,
 				function() use ($fileList, $minify, $type) {
 					// Physically combine all file content
 					$combinedData = '';
 					$base = Director::baseFolder() . '/';
 					$minifier = Injector::inst()->get('Requirements_Minifier');
-					foreach(array_diff($fileList, $this->getBlocked()) as $file) {
+					foreach($fileList as $file) {
 						$fileContent = file_get_contents($base . $file);
 						// Use configured minifier
 						if($minify) {
@@ -1439,13 +1464,21 @@ class Requirements_Backend {
 				}
 			);
 
-		// Since url won't be automatically suffixed, add it in here
-		if($this->getSuffixRequirements()) {
-			$q = stripos($combinedURL, '?') === false ? '?' : '&';
-			$combinedURL .= "{$q}m={$entropy}";
+		return $combinedURL;
 	}
 
-		return $combinedURL;
+	/**
+	 * Given a filename and list of files, generate a new filename unique to these files
+	 *
+	 * @param string $name
+	 * @param array $files
+	 * @return string
+	 */
+	protected function hashedCombinedFilename($combinedFile, $fileList) {
+		$name = pathinfo($combinedFile, PATHINFO_FILENAME);
+		$hash = $this->hashOfFiles($fileList);
+		$extension = File::get_file_extension($combinedFile);
+		return $name . '-' . substr($hash, 0, 7) . '.' . $extension;
 	}
 
 	/**
@@ -1482,20 +1515,20 @@ class Requirements_Backend {
 	 * any of these files have changed.
 	 *
 	 * @param array $fileList List of files
-	 * @return int Last modified timestamp of these files
+	 * @return string SHA1 bashed file hash
 	 */
-	protected function getEntropyOfFiles($fileList) {
-		// file exists, check modification date of every contained file
+	protected function hashOfFiles($fileList) {
+		// Get hash based on hash of each file
 		$base = Director::baseFolder() . '/';
-		$srcLastMod = 0;
+		$hash = '';
 		foreach($fileList as $file) {
 			if(file_exists($base . $file)) {
-				$srcLastMod = max(filemtime($base . $file), $srcLastMod);
+				$hash .= sha1_file($base . $file);
 			} else {
 				throw new InvalidArgumentException("Combined file {$file} does not exist");
 			}
 		}
-		return $srcLastMod;
+		return sha1($hash);
 	}
 
 	/**