From ce28259c5f21620db2a61f79fd6479be60a2d8c4 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 26 Nov 2015 15:19:43 +1300 Subject: [PATCH 1/2] 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 | 4 +- docs/en/04_Changelogs/4.0.0.md | 26 +-- .../FlysystemGeneratedAssetHandler.php | 107 +++++++++++ .../storage/CacheGeneratedAssetHandler.php | 177 ------------------ filesystem/storage/GeneratedAssetHandler.php | 32 ++-- tests/filesystem/AssetStoreTest.php | 10 +- tests/forms/RequirementsTest.php | 13 +- tests/view/SSViewerTest.php | 2 +- view/Requirements.php | 85 ++++++--- 9 files changed, 207 insertions(+), 249 deletions(-) create mode 100644 filesystem/flysystem/FlysystemGeneratedAssetHandler.php delete mode 100644 filesystem/storage/CacheGeneratedAssetHandler.php diff --git a/_config/asset.yml b/_config/asset.yml index 3ff54fd29..2adc2688f 100644 --- a/_config/asset.yml +++ b/_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 diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 178b5e77a..a0e051664 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/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. diff --git a/filesystem/flysystem/FlysystemGeneratedAssetHandler.php b/filesystem/flysystem/FlysystemGeneratedAssetHandler.php new file mode 100644 index 000000000..b4735f7a0 --- /dev/null +++ b/filesystem/flysystem/FlysystemGeneratedAssetHandler.php @@ -0,0 +1,107 @@ +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(); + } + + } + + +} diff --git a/filesystem/storage/CacheGeneratedAssetHandler.php b/filesystem/storage/CacheGeneratedAssetHandler.php deleted file mode 100644 index f97718699..000000000 --- a/filesystem/storage/CacheGeneratedAssetHandler.php +++ /dev/null @@ -1,177 +0,0 @@ -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']); - } - -} diff --git a/filesystem/storage/GeneratedAssetHandler.php b/filesystem/storage/GeneratedAssetHandler.php index 3cbc64192..0087b0d35 100644 --- a/filesystem/storage/GeneratedAssetHandler.php +++ b/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 the generated file + * Returns a URL to 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, 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); } diff --git a/tests/filesystem/AssetStoreTest.php b/tests/filesystem/AssetStoreTest.php index 1d11606cf..6f941a62f 100644 --- a/tests/filesystem/AssetStoreTest.php +++ b/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(); diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index a07654276..2d22a5583 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -1,6 +1,5 @@ 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); diff --git a/tests/view/SSViewerTest.php b/tests/view/SSViewerTest.php index 5a7b7e1db..1e33528da 100644 --- a/tests/view/SSViewerTest.php +++ b/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)) { diff --git a/view/Requirements.php b/view/Requirements.php index e1239c281..f62a5b3c4 100644 --- a/view/Requirements.php +++ b/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()} */ @@ -619,8 +634,8 @@ class Requirements_Backend { /** * Retrieve the combined files folder prefix - * - * @return string + * + * @return string */ public function getCombinedFilesFolder() { if($this->combinedFilesFolder) { @@ -931,7 +946,7 @@ class Requirements_Backend { $this->customCSS = $this->disabled['customCSS']; $this->customHeadTags = $this->disabled['customHeadTags']; } - + /** * Block inclusion of a specific file * @@ -1229,7 +1244,7 @@ class Requirements_Backend { if(isset($this->combinedFiles[$combinedFileName])) { return; } - + // Add all files to necessary type list $paths = array(); $combinedType = null; @@ -1270,7 +1285,7 @@ class Requirements_Backend { )); } } - + $this->combinedFiles[$combinedFileName] = array( 'files' => $paths, 'type' => $combinedType, @@ -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,31 +1429,31 @@ 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) { $fileContent = $minifier->minify($fileContent, $type, $file); } - + if ($this->writeHeaderComment) { // Write a header comment for each file for easier identification and debugging. $combinedData .= "/****** FILE: $file *****/\n"; @@ -1439,15 +1464,23 @@ 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; } + /** + * 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; + } + /** * Check if combined files are enabled * @@ -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); } /** From c13b5d989f4267e457fb08e0ff0ccdfdd77aa7fe Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 30 Nov 2015 15:03:46 +1300 Subject: [PATCH 2/2] API Enable advanced configuration options for requirements combined files API Enable relative root paths for the default Flysystem AssetAdapter --- _config/asset.yml | 4 + control/Director.php | 2 +- .../01_Templates/03_Requirements.md | 90 ++++++++++++++++--- filesystem/flysystem/AssetAdapter.php | 15 +++- .../FlysystemGeneratedAssetHandler.php | 3 +- tests/filesystem/AssetStoreTest.php | 1 + tests/forms/RequirementsTest.php | 28 +++--- tests/view/SSViewerTest.php | 4 +- view/Requirements.php | 82 +++++++++++++++-- 9 files changed, 188 insertions(+), 41 deletions(-) diff --git a/_config/asset.yml b/_config/asset.yml index 2adc2688f..1b4249277 100644 --- a/_config/asset.yml +++ b/_config/asset.yml @@ -25,9 +25,13 @@ Injector: type: prototype # Image mechanism Image_Backend: GDBackend + # Requirements config GeneratedAssetHandler: class: SilverStripe\Filesystem\Storage\FlysystemGeneratedAssetHandler properties: Filesystem: '%$FlysystemBackend' Requirements_Minifier: class: SilverStripe\View\JSMinifier + Requirements_Backend: + properties: + AssetHandler: '%$GeneratedAssetHandler' diff --git a/control/Director.php b/control/Director.php index f50e423cf..48fce1515 100644 --- a/control/Director.php +++ b/control/Director.php @@ -289,7 +289,7 @@ class Director implements TemplateGlobalProvider { $existingRequirementsBackend = Requirements::backend(); Config::inst()->update('Cookie', 'report_errors', false); - Requirements::set_backend(new Requirements_Backend()); + Requirements::set_backend(Injector::inst()->create('Requirements_Backend')); // Set callback to invoke prior to return $onCleanup = function() use( diff --git a/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md b/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md index 51bea33ab..4f1226433 100644 --- a/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md +++ b/docs/en/02_Developer_Guides/01_Templates/03_Requirements.md @@ -102,22 +102,83 @@ by reducing HTTP requests.
To make debugging easier in your local environment, combined files is disabled when running your application in `dev` -mode. +mode. You can re-enable dev combination by setting `Requirements_Backend.combine_in_dev` to true.
-By default it stores the generated file in the assets/ folder, but you can configure this by pointing the -`Requirements.combined_files_folder` configuration setting to a specific folder. +### Configuring combined file storage -**mysite/_config/app.yml** - - :::yml - Requirements: - combined_files_folder: '_combined' +In some situations or server configurations, it may be necessary to customise the behaviour of generated javascript +files in order to ensure that current files are served in requests. -
-If SilverStripe doesn't have permissions on your server to write these files it will default back to including them -individually. SilverStripe **will not** rewrite your paths within the file. -
+By default, files will be generated on demand in the format `assets/_combinedfiles/name-.js`, +where `` represents the hash of the source files used to generate that content. The default flysystem backend, +as used by the `[api:AssetStore]` backend, is used for this storage, but it can be substituted for any +other backend. + +You can also use any of the below options in order to tweak this behaviour: + + * `Requirements.disable_flush_combined` - By default all combined files are deleted on flush. + If combined files are stored in source control, and thus updated manually, you might want to + turn this on to disable this behaviour. + * `Requirements_Backend.combine_hash_querystring` - By default the `` of the source files is appended to + the end of the combined file (prior to the file extension). If combined files are versioned in source control, + or running in a distributed environment (such as one where the newest version of a file may not always be + immediately available) then it may sometimes be necessary to disable this. When this is set to true, the hash + will instead be appended via a querystring parameter to enable cache busting, but not in the + filename itself. I.e. `assets/_combinedfiles/name.js?m=` + * `Requirements_Backend.default_combined_files_folder` - This defaults to `_combinedfiles`, and is the folder + within the configured asset backend that combined files will be stored in. If using a backend shared with + other systems, it is usually necessary to distinguish combined files from other assets. + * `Requirements_Backend.combine_in_dev` - By default combined files will not be combined except in test + or live environments. Turning this on will allow for pre-combining of files in development mode. + +In some cases it may be necessary to create a new storage backend for combined files, if the default location +is not appropriate. Normally a single backend is used for all site assets, so a number of objects must be +replaced. For instance, the below will set a new set of dependencies to write to `mysite/javascript/combined` + + + :::yaml + --- + Name: myrequirements + --- + Requirements: + disable_flush_combined: true + Requirements_Backend: + combine_in_dev: true + combine_hash_querystring: true + default_combined_files_folder: 'combined' + Injector: + MySiteAdapter: + class: 'SilverStripe\Filesystem\Flysystem\AssetAdapter' + constructor: + Root: ./mysite/javascript + # Define the default filesystem + MySiteBackend: + class: 'League\Flysystem\Filesystem' + constructor: + Adapter: '%$MySiteAdapter' + calls: + PublicURLPlugin: [ addPlugin, [ %$FlysystemUrlPlugin ] ] + # Requirements config + MySiteAssetHandler: + class: SilverStripe\Filesystem\Storage\FlysystemGeneratedAssetHandler + properties: + Filesystem: '%$MySiteBackend' + Requirements_Backend: + properties: + AssetHandler: '%$MySiteAssetHandler' + +In the above configuration, automatic expiry of generated files has been disabled, and it is necessary for +the developer to maintain these files manually. This may be useful in environments where assets must +be pre-cached, where scripts must be served alongside static files, or where no framework php request is +guaranteed. Alternatively, files may be served from instances other than the one which generated the +page response, and file synchronisation might not occur fast enough to propagate combined files to +mirrored filesystems. + +In any case, care should be taken to determine the mechanism appropriate for your development +and production environments. + +### Combined CSS Files You can also combine CSS files into a media-specific stylesheets as you would with the `Requirements::css` call - use the third paramter of the `combine_files` function: @@ -130,6 +191,11 @@ the third paramter of the `combine_files` function: Requirements::combine_files('print.css', $printStylesheets, 'print'); +
+When combining CSS files, take care of relative urls, as these will not be re-written to match +the destination location of the resulting combined CSS. +
+ ## Clearing assets :::php diff --git a/filesystem/flysystem/AssetAdapter.php b/filesystem/flysystem/AssetAdapter.php index 98cc00642..ac9fadd31 100644 --- a/filesystem/flysystem/AssetAdapter.php +++ b/filesystem/flysystem/AssetAdapter.php @@ -32,9 +32,22 @@ class AssetAdapter extends Local { ); public function __construct($root = null, $writeFlags = LOCK_EX, $linkHandling = self::DISALLOW_LINKS) { + // Get root path + if (!$root) { + // Empty root will set the path to assets + $root = ASSETS_PATH; + } elseif(strpos($root, './') === 0) { + // Substitute leading ./ with BASE_PATH + $root = BASE_PATH . substr($root, 1); + } elseif(strpos($root, '../') === 0) { + // Substitute leading ./ with parent of BASE_PATH, in case storage is outside of the webroot. + $root = dirname(BASE_PATH) . substr($root, 2); + } + // Override permissions with config $permissions = \Config::inst()->get(get_class($this), 'file_permissions'); - parent::__construct($root ?: ASSETS_PATH, $writeFlags, $linkHandling, $permissions); + + parent::__construct($root, $writeFlags, $linkHandling, $permissions); } /** diff --git a/filesystem/flysystem/FlysystemGeneratedAssetHandler.php b/filesystem/flysystem/FlysystemGeneratedAssetHandler.php index b4735f7a0..5606e072d 100644 --- a/filesystem/flysystem/FlysystemGeneratedAssetHandler.php +++ b/filesystem/flysystem/FlysystemGeneratedAssetHandler.php @@ -67,8 +67,7 @@ class FlysystemGeneratedAssetHandler implements GeneratedAssetHandler { * @return bool Whether or not the file exists * @throws Exception If an error has occurred during save */ - protected function checkOrCreate($filename, $callback = null) - { + protected function checkOrCreate($filename, $callback = null) { // Check if there is an existing asset if ($this->getFilesystem()->has($filename)) { return true; diff --git a/tests/filesystem/AssetStoreTest.php b/tests/filesystem/AssetStoreTest.php index 6f941a62f..c000cf175 100644 --- a/tests/filesystem/AssetStoreTest.php +++ b/tests/filesystem/AssetStoreTest.php @@ -428,6 +428,7 @@ class AssetStoreTest_SpyStore extends FlysystemAssetStore { $generated = new FlysystemGeneratedAssetHandler(); $generated->setFilesystem($filesystem); Injector::inst()->registerService($generated, 'GeneratedAssetHandler'); + Requirements::backend()->setAssetHandler($generated); // Disable legacy and set defaults Config::inst()->remove(get_class(new FlysystemAssetStore()), 'legacy_filenames'); diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index 2d22a5583..29b1d4270 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -22,7 +22,7 @@ class RequirementsTest extends SapphireTest { } public function testExternalUrls() { - $backend = new Requirements_Backend; + $backend = Injector::inst()->create('Requirements_Backend'); $backend->setCombinedFilesEnabled(true); $backend->javascript('http://www.mydomain.com/test.js'); @@ -118,7 +118,7 @@ class RequirementsTest extends SapphireTest { } public function testCombinedJavascript() { - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupCombinedRequirements($backend); $combinedFileName = '/_combinedfiles/RequirementsTest_bc-51622b5.js'; @@ -166,7 +166,7 @@ class RequirementsTest extends SapphireTest { // Then do it again, this time not requiring the files beforehand unlink($combinedFilePath); - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupCombinedNonrequiredRequirements($backend); $html = $backend->includeInHTML(false, self::$html_template); @@ -204,7 +204,7 @@ class RequirementsTest extends SapphireTest { public function testCombinedCss() { $basePath = $this->getCurrentRelativePath(); - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->combineFiles( @@ -230,7 +230,7 @@ class RequirementsTest extends SapphireTest { ); // Test that combining a file multiple times doesn't trigger an error - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->combineFiles( 'style.css', @@ -257,7 +257,7 @@ class RequirementsTest extends SapphireTest { public function testBlockedCombinedJavascript() { $basePath = $this->getCurrentRelativePath(); - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupCombinedRequirements($backend); $combinedFileName = '/_combinedfiles/RequirementsTest_bc-51622b5.js'; $combinedFilePath = AssetStoreTest_SpyStore::base_path() . $combinedFileName; @@ -314,7 +314,7 @@ class RequirementsTest extends SapphireTest { public function testArgsInUrls() { $basePath = $this->getCurrentRelativePath(); - $backend = new Requirements_Backend; + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript($basePath . '/RequirementsTest_a.js?test=1&test=2&test=3'); @@ -339,7 +339,7 @@ class RequirementsTest extends SapphireTest { public function testRequirementsBackend() { $basePath = $this->getCurrentRelativePath(); - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript($basePath . '/a.js'); @@ -377,7 +377,7 @@ class RequirementsTest extends SapphireTest { // to something else $basePath = 'framework' . substr($basePath, strlen(FRAMEWORK_DIR)); - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $holder = Requirements::backend(); Requirements::set_backend($backend); @@ -406,7 +406,7 @@ class RequirementsTest extends SapphireTest { } public function testJsWriteToBody() { - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript('http://www.mydomain.com/test.js'); @@ -425,7 +425,7 @@ class RequirementsTest extends SapphireTest { public function testIncludedJsIsNotCommentedOut() { $template = ''; - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript($this->getCurrentRelativePath() . '/RequirementsTest_a.js'); $html = $backend->includeInHTML(false, $template); @@ -437,7 +437,7 @@ class RequirementsTest extends SapphireTest { public function testCommentedOutScriptTagIsIgnored() { $template = '' . '

more content

'; - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->setSuffixRequirements(false); $src = $this->getCurrentRelativePath() . '/RequirementsTest_a.js'; @@ -449,7 +449,7 @@ class RequirementsTest extends SapphireTest { } public function testForceJsToBottom() { - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript('http://www.mydomain.com/test.js'); @@ -505,7 +505,7 @@ class RequirementsTest extends SapphireTest { $template = '
My header

Body

'; $basePath = $this->getCurrentRelativePath(); - $backend = new Requirements_Backend(); + $backend = Injector::inst()->create('Requirements_Backend'); $this->setupRequirements($backend); $backend->javascript($basePath .'/RequirementsTest_a.js'); diff --git a/tests/view/SSViewerTest.php b/tests/view/SSViewerTest.php index 1e33528da..74a8c135c 100644 --- a/tests/view/SSViewerTest.php +++ b/tests/view/SSViewerTest.php @@ -146,7 +146,7 @@ class SSViewerTest extends SapphireTest { } public function testRequirementsCombine(){ - $testBackend = new Requirements_Backend(); + $testBackend = Injector::inst()->create('Requirements_Backend'); $testBackend->setSuffixRequirements(false); //$combinedTestFilePath = BASE_PATH . '/' . $testBackend->getCombinedFilesFolder() . '/testRequirementsCombine.js'; @@ -1348,7 +1348,7 @@ after') $template = new SSViewer(array('SSViewerTestProcess')); $basePath = dirname($this->getCurrentRelativePath()) . '/forms'; - $backend = new Requirements_Backend; + $backend = Injector::inst()->create('Requirements_Backend'); $backend->setCombinedFilesEnabled(false); $backend->combineFiles( 'RequirementsTest_ab.css', diff --git a/view/Requirements.php b/view/Requirements.php index f62a5b3c4..af9db8aa2 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -10,11 +10,25 @@ use SilverStripe\Filesystem\Storage\GeneratedAssetHandler; */ class Requirements implements Flushable { + /** + * Flag whether combined files should be deleted on flush. + * + * By default all combined files are deleted on flush. If combined files are stored in source control, + * and thus updated manually, you might want to turn this on to disable this behaviour. + * + * @config + * @var bool + */ + private static $disable_flush_combined = false; + /** * Triggered early in the request when a flush is requested */ public static function flush() { - self::delete_all_combined_files(); + $disabled = Config::inst()->get(static::class, 'disable_flush_combined'); + if(!$disabled) { + self::delete_all_combined_files(); + } } /** @@ -73,6 +87,9 @@ class Requirements implements Flushable { */ private static $backend = null; + /** + * @return Requirements_Backend + */ public static function backend() { if(!self::$backend) { self::$backend = Injector::inst()->create('Requirements_Backend'); @@ -442,7 +459,8 @@ class Requirements implements Flushable { * @package framework * @subpackage view */ -class Requirements_Backend { +class Requirements_Backend +{ /** * Whether to add caching query params to the requests for file-based requirements. @@ -462,8 +480,10 @@ class Requirements_Backend { protected $combinedFilesEnabled = true; /** - * Determine if files should be combined automatically on dev mode - * By default, files will be left uncombined when developing. + * Determine if files should be combined automatically on dev mode. + * + * By default combined files will not be combined except in test or + * live environments. Turning this on will allow for pre-combining of files in development mode. * * @config * @var bool @@ -574,20 +594,53 @@ class Requirements_Backend { protected $forceJSToBottom = false; /** - * Configures the default prefix for comined files + * Configures the default prefix for combined files. + * + * This defaults to `_combinedfiles`, and is the folder within the configured asset backend that + * combined files will be stored in. If using a backend shared with other systems, it is usually + * necessary to distinguish combined files from other assets. * * @config * @var string */ private static $default_combined_files_folder = '_combinedfiles'; + /** + * Flag to include the hash in the querystring instead of the filename for combined files. + * + * By default the `` of the source files is appended to the end of the combined file + * (prior to the file extension). If combined files are versioned in source control or running + * in a distributed environment (such as one where the newest version of a file may not always be + * immediately available) then it may sometimes be necessary to disable this. When this is set to true, + * the hash will instead be appended via a querystring parameter to enable cache busting, but not in + * the filename itself. I.e. `assets/_combinedfiles/name.js?m=` + * + * @config + * @var bool + */ + private static $combine_hash_querystring = false; + + /** + * @var GeneratedAssetHandler + */ + protected $assetHandler = null; + /** * Gets the backend storage for generated files * * @return GeneratedAssetHandler */ - protected function getAssetHandler() { - return Injector::inst()->get('GeneratedAssetHandler'); + public function getAssetHandler() { + return $this->assetHandler; + } + + /** + * Set a new asset handler for this backend + * + * @param GeneratedAssetHandler $handler + */ + public function setAssetHandler(GeneratedAssetHandler $handler) { + $this->assetHandler = $handler; } /** @@ -1433,8 +1486,11 @@ class Requirements_Backend { $fileList = array_diff($fileList, $this->getBlocked()); // Generate path (Filename) - $filename = $this->hashedCombinedFilename($combinedFile, $fileList); - $combinedFileID = File::join_paths($this->getCombinedFilesFolder(), $filename); + $hashQuerystring = Config::inst()->get(static::class, 'combine_hash_querystring'); + if(!$hashQuerystring) { + $combinedFile = $this->hashedCombinedFilename($combinedFile, $fileList); + } + $combinedFileID = File::join_paths($this->getCombinedFilesFolder(), $combinedFile); // Send file combination request to the backend, with an optional callback to perform regeneration $minify = $this->getMinifyCombinedJSFiles(); @@ -1464,6 +1520,14 @@ class Requirements_Backend { } ); + // If the name isn't hashed, we will need to append the querystring m= parameter instead + // Since url won't be automatically suffixed, add it in here + if($hashQuerystring && $this->getSuffixRequirements()) { + $hash = $this->hashOfFiles($fileList); + $q = stripos($combinedURL, '?') === false ? '?' : '&'; + $combinedURL .= "{$q}m={$hash}"; + } + return $combinedURL; }