API Enable advanced configuration options for requirements combined files

API Enable relative root paths for the default Flysystem AssetAdapter

_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'

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(

docs/en/02_Developer_Guides/01_Templates/03_Requirements.md

@@ -102,22 +102,83 @@ by reducing HTTP requests.
 
 <div class="alert" markdown='1'>
 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.
 </div>
 
-By default it stores the generated file in the assets/ folder, but you can configure this by pointing the 
+### Configuring combined file storage
-`Requirements.combined_files_folder` configuration setting to a specific folder.
 
-**mysite/_config/app.yml**
+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.
 
-	:::yml
+By default, files will be generated on demand in the format `assets/_combinedfiles/name-<hash>.js`,
+where `<hash>` 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 `<hash>` 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=<hash>`
+ * `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:
-	  combined_files_folder: '_combined'
+      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'
 
-<div class="info" markdown='1'>
+In the above configuration, automatic expiry of generated files has been disabled, and it is necessary for
-If SilverStripe doesn't have permissions on your server to write these files it will default back to including them
+the developer to maintain these files manually. This may be useful in environments where assets must
-individually. SilverStripe **will not** rewrite your paths within the file.
+be pre-cached, where scripts must be served alongside static files, or where no framework php request is
-</div>
+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');
 
+<div class="alert" markdown='1'>
+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.
+</div>
+
 ## Clearing assets
 
 	:::php

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);
 	}
 
 	/**

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;

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');

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 = '<html><head></head><body><!--<script>alert("commented out");</script>--></body></html>';
-		$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 = '<html><head></head><body><!--<script>alert("commented out");</script>-->'
 			. '<h1>more content</h1></body></html>';
-		$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 = '<html><head></head><body><header>My header</header><p>Body</p></body></html>';
 		$basePath = $this->getCurrentRelativePath();
 
-		$backend = new Requirements_Backend();
+		$backend = Injector::inst()->create('Requirements_Backend');
 		$this->setupRequirements($backend);
 
 		$backend->javascript($basePath .'/RequirementsTest_a.js');

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',

view/Requirements.php

@@ -10,12 +10,26 @@ 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() {
+		$disabled = Config::inst()->get(static::class, 'disable_flush_combined');
+		if(!$disabled) {
 			self::delete_all_combined_files();
 		}
+	}
 
 	/**
 	 * Enable combining of css/javascript 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
+	 * Determine if files should be combined automatically on dev mode.
-	 * By default, files will be left uncombined when developing.
+	 *
+	 * 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 `<hash>` 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=<hash>`
+	 *
+	 * @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() {
+	public function getAssetHandler() {
-		return Injector::inst()->get('GeneratedAssetHandler');
+		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);
+		$hashQuerystring = Config::inst()->get(static::class, 'combine_hash_querystring');
-		$combinedFileID =  File::join_paths($this->getCombinedFilesFolder(), $filename);
+		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;
 	}