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; }