NEW Provide a consistent way of triggering flush

Provides an interface for classes to implement their own flush()
functionality. This function gets called early in a request on
all implementations of Flushable when flush=1|all is requested in the
URL.

This fix came out of an issue where Requirements combined files were not
being cleaned up after dev/build?flush=1, due to the fact that flush
would only occur when you called it while on a page that used those
combined files, but not in any other contexts. This will now call flush
on any implementors of Flushable regardless of the context of where
flush was called.

_config/requestprocessors.yml

@@ -5,4 +5,5 @@ Injector:
   RequestProcessor:
     properties:
       filters:
+        - '%$FlushRequestFilter'
         - '%$VersionedRequestFilter'

api/RestfulService.php

@@ -9,7 +9,7 @@
  * @package framework
  * @subpackage integration
  */
-class RestfulService extends ViewableData {
+class RestfulService extends ViewableData implements Flushable {
 
 	protected $baseURL;
 	protected $queryString;
@@ -33,6 +33,19 @@ class RestfulService extends ViewableData {
 	 */
 	private static $default_curl_options = array();
 
+	/**
+	 * @config
+	 * @var bool Flushes caches if set to true. This is set by {@link flush()}
+	 */
+	private static $flush = false;
+
+	/**
+	 * Triggered early in the request when someone requests a flush.
+	 */
+	public static function flush() {
+		self::$flush = true;
+	}
+
 	/**
 	 * set a curl option that will be applied to all requests as default
 	 * {@see http://php.net/manual/en/function.curl-setopt.php#refsect1-function.curl-setopt-parameters}
@@ -171,7 +184,7 @@ class RestfulService extends ViewableData {
 		// Check for unexpired cached feed (unless flush is set)
 		//assume any cache_expire that is 0 or less means that we dont want to
 		// cache
-		if($this->cache_expire > 0 && !isset($_GET['flush'])
+		if($this->cache_expire > 0 && self::$flush
 				&& @file_exists($cache_path)
 				&& @filemtime($cache_path) + $this->cache_expire > time()) {
 			

control/FlushRequestFilter.php

@@ -0,0 +1,24 @@
+<?php
+/**
+ * Triggers a call to flush() on all implementors of Flushable.
+ *
+ * @package framework
+ * @subpackage control
+ */
+class FlushRequestFilter implements RequestFilter {
+
+	public function preRequest(SS_HTTPRequest $request, Session $session, DataModel $model) {
+		if($request->getVar('flush')) {
+			foreach(ClassInfo::implementorsOf('Flushable') as $class) {
+				$class::flush();
+			}
+		}
+
+		return true;
+	}	
+
+	public function postRequest(SS_HTTPRequest $request, SS_HTTPResponse $response, DataModel $model) {
+		return true;
+	}
+
+}

control/RequestProcessor.php

@@ -45,4 +45,4 @@ class RequestProcessor implements RequestFilter {
 			}
 		}
 	}
-}
+}

core/Flushable.php

@@ -0,0 +1,20 @@
+<?php
+/**
+ * Provides an interface for classes to implement their own flushing functionality
+ * whenever flush=1 is requested.
+ *
+ * @package framework
+ * @subpackage core
+ */
+interface Flushable {
+
+	/**
+	 * This function is triggered early in the request if the "flush" query
+	 * parameter has been set. Each class that implements Flushable implements
+	 * this function which looks after it's own specific flushing functionality.
+	 *
+	 * @see FlushRequestFilter
+	 */
+	public static function flush();
+
+}

dev/TestRunner.php

@@ -49,7 +49,7 @@ class TestRunner extends Controller {
 		'build',
 		'only'
 	);
-	
+
 	/**
 	 * @var Array Blacklist certain directories for the coverage report.
 	 * Filepaths are relative to the webroot, without leading slash.
@@ -62,7 +62,7 @@ class TestRunner extends Controller {
 		'*/tests',
 		'*/lang',
 	);
-	
+
 	/**
 	 * Override the default reporter with a custom configured subclass.
 	 *
@@ -78,21 +78,26 @@ class TestRunner extends Controller {
 	 * top of the loader stacks.
 	 */
 	public static function use_test_manifest() {
+		$flush = true;
+		if(isset($_GET['flush']) && $_GET['flush'] === '0') {
+			$flush = false;
+		}
+
 		$classManifest = new SS_ClassManifest(
-			BASE_PATH, true, isset($_GET['flush'])
+			BASE_PATH, true, $flush
 		);
-		
+
 		SS_ClassLoader::instance()->pushManifest($classManifest, false);
 		SapphireTest::set_test_class_manifest($classManifest);
 
 		SS_TemplateLoader::instance()->pushManifest(new SS_TemplateManifest(
-			BASE_PATH, project(), true, isset($_GET['flush'])
+			BASE_PATH, project(), true, $flush
 		));
 
 		Config::inst()->pushConfigStaticManifest(new SS_ConfigStaticManifest(
-			BASE_PATH, true, isset($_GET['flush'])
+			BASE_PATH, true, $flush
 		));
-		
+
 		// Invalidate classname spec since the test manifest will now pull out new subclasses for each internal class
 		// (e.g. Member will now have various subclasses of DataObjects that implement TestOnly)
 		DataObject::clear_classname_spec_cache();
@@ -109,14 +114,6 @@ class TestRunner extends Controller {
 		if(!PhpUnitWrapper::has_php_unit()) {
 			die("Please install PHPUnit using pear");
 		}
-
-		if(!isset($_GET['flush']) || !$_GET['flush']) {
-			Debug::message(
-				"WARNING: Manifest not flushed. " .
-				"Add flush=1 as an argument to discover new classes or files.\n",
-				false
-			);
-		}
 	}
 	
 	public function Link() {

docs/en/howto/phpunit-configuration.md

@@ -76,17 +76,31 @@ Example `mysite/_config.php`:
 			if($db == 'sqlite3') $databaseConfig['type'] = 'SQLite3Database';
 		}
 	}
-	
+
 You can either use the database on a single invocation:
 
 	phpunit framework/tests "" db=sqlite3
-	
+
 or through a `<php>` flag in your `phpunit.xml` (see [Appenix C: "Setting PHP INI settings"](http://www.phpunit.de/manual/current/en/appendixes.configuration.html)):
 
 	<phpunit>
 		<!-- ... -->
 		<php>
-			<var name="db" value="sqlite3"/>
+			<get name="db" value="sqlite3"/>
+		</php>
+	</phpunit>
+
+Note that on every test run, the manifest is flushed to avoid any bugs where a test doesn't clean up after
+itself properly. You can override that behaviour by passing `flush=0` to the test command:
+
+	phpunit framework/tests flush=0
+
+Alternatively, you can set the var in your `phpunit.xml` file:
+
+	<phpunit>
+		<!-- ... -->
+		<php>
+			<get name="flush" value="0"/>
 		</php>
 	</phpunit>
 

docs/en/reference/execution-pipeline.md

@@ -91,3 +91,12 @@ You can access the following controller-method with /team/signup
 ## SSViewer template rendering
 
 See [templates](/reference/templates) for information on the SSViewer template system.
+
+## Flush requests
+
+If `?flush=1` is requested in the URL, e.g. http://mysite.com?flush=1, this will trigger a call to `flush()` on
+any classes that implement the `Flushable` interface.
+
+See [reference documentation on Flushable](/reference/flushable) for more details.
+
+

docs/en/reference/flushable.md

@@ -0,0 +1,56 @@
+# Flushable
+
+## Introduction
+
+Allows a class to define it's own flush functionality, which is triggered when `flush=1` is requested in the URL.
+
+`[api:FlushRequestFilter]` is run before a request is made, calling `flush()` statically on all
+implementors of `[api:Flushable]`.
+
+## Usage
+
+To use this API, you need to make your class implement `[api:Flushable]`, and define a `flush()` static function,
+this defines the actions that need to be executed on a flush request.
+
+### Using with SS_Cache
+
+This example uses `[api:SS_Cache]` in some custom code, and the same cache is cleaned on flush:
+
+	:::php
+	<?php
+	class MyClass extends DataObject implements Flushable {
+	
+		public static function flush() {
+			SS_Cache::factory('mycache')->clean(Zend_Cache::CLEANING_MODE_ALL);
+		}
+	
+		public function MyCachedContent() {
+			$cache = SS_Cache::factory('mycache')
+			$something = $cache->get('mykey');
+			if(!$something) {
+				$something = 'value to be cached';
+				$cache->save($something);
+			}
+			return $something;
+		}
+	
+	}
+
+### Using with filesystem
+
+Another example, some temporary files are created in a directory in assets, and are deleted on flush. This would be
+useful in an example like `GD` or `Imagick` generating resampled images, but we want to delete any cached images on
+flush so they are re-created on demand.
+
+	:::php
+	<?php
+	class MyClass extends DataObject implements Flushable {
+	
+		public static function flush() {
+			foreach(glob(ASSETS_PATH . '/_tempfiles/*.jpg') as $file) {
+				unlink($file);
+			}
+		}
+	
+	}
+

docs/en/topics/caching.md

@@ -98,6 +98,13 @@ this is reasonably quick, but still requires access to slow disk I/O.
 The `Zend_Cache` API supports various caching backends ([list](http://framework.zend.com/manual/1.12/en/zend.cache.backends.html))
 which can provide better performance, including APC, Xcache, ZendServer, Memcached and SQLite.
 
+## Cleaning caches on flush=1 requests
+
+If `?flush=1` is requested in the URL, e.g. http://mysite.com?flush=1, this will trigger a call to `flush()` on
+any classes that implement the `Flushable` interface. Using this, you can trigger your caches to clean.
+
+See [reference documentation on Flushable](/reference/flushable) for implementation details.
+
 ### Memcached
 
 This backends stores cache records into a [memcached](http://www.danga.com/memcached/) 
@@ -144,4 +151,4 @@ a fast one (but limited) like Apc, Memcache... and a "slow" one like File or Sql
 				'cache_dir' => TEMP_FOLDER . DIRECTORY_SEPARATOR . 'cache'
 		)
 	));
-	SS_Cache::pick_backend('two_level', 'any', 10); 
+	SS_Cache::pick_backend('two_level', 'any', 10); 

i18n/i18n.php

@@ -61,7 +61,7 @@ require_once 'i18nSSLegacyAdapter.php';
  * @package framework
  * @subpackage misc
  */
-class i18n extends Object implements TemplateGlobalProvider {
+class i18n extends Object implements TemplateGlobalProvider, Flushable {
 
 	/**
 	 * This static variable is used to store the current defined locale.
@@ -97,6 +97,21 @@ class i18n extends Object implements TemplateGlobalProvider {
 	 */
 	protected static $translators;
 
+	/**
+	 * Triggered early in the request when someone requests a flush.
+	 */
+	public static function flush() {
+		self::get_cache()->clean(Zend_Cache::CLEANING_MODE_ALL);
+	}
+
+	/**
+	 * Return an instance of the cache used for i18n data.
+	 * @return Zend_Cache
+	 */
+	public static function get_cache() {
+		return SS_Cache::factory('i18n', 'Output', array('lifetime' => null, 'automatic_serialization' => true));
+	}
+
 	/**
 	 * Use javascript i18n through the ss.i18n class (enabled by default).
 	 * If set to TRUE, includes javascript requirements for the base library
@@ -2039,11 +2054,11 @@ class i18n extends Object implements TemplateGlobalProvider {
 				// which is instanciated by core with a $clean instance variable.
 
 				if(!$adapter->isAvailable($lang)) {
-					i18n::include_by_locale($lang, (isset($_GET['flush'])));
+					i18n::include_by_locale($lang);
 				}
 
 				if(!$adapter->isAvailable($locale)) {
-					i18n::include_by_locale($locale, (isset($_GET['flush'])));
+					i18n::include_by_locale($locale);
 				}
 
 				$translation = $adapter->translate($entity, $locale);
@@ -2107,9 +2122,7 @@ class i18n extends Object implements TemplateGlobalProvider {
 	 */
 	public static function get_translators() {
 		if(!Zend_Translate::getCache()) {
-			Zend_Translate::setCache(
-				SS_Cache::factory('i18n', 'Output', array('lifetime' => null, 'automatic_serialization' => true))
-			);
+			Zend_Translate::setCache(self::get_cache());
 		}
 
 		if(!self::$translators) {
@@ -2122,8 +2135,8 @@ class i18n extends Object implements TemplateGlobalProvider {
 				))
 			);
 
-			i18n::include_by_locale('en', isset($_GET['flush']));
-			i18n::include_by_locale('en_US', isset($_GET['flush']));
+			i18n::include_by_locale('en');
+			i18n::include_by_locale('en_US');
 		}
 
 		return self::$translators;
@@ -2515,7 +2528,7 @@ class i18n extends Object implements TemplateGlobalProvider {
 	 */
 	public static function include_by_locale($locale, $clean = false) {
 		if($clean) {
-			Zend_Translate::clearCache();
+			self::flush();
 		}
 
 		// Get list of module => path pairs, and then just the names

model/Image.php

@@ -6,7 +6,7 @@
  * @package framework
  * @subpackage filesystem
  */
-class Image extends File {
+class Image extends File implements Flushable {
 	
 	const ORIENTATION_SQUARE = 0;
 	const ORIENTATION_PORTRAIT = 1;
@@ -71,11 +71,24 @@ class Image extends File {
 	 * @var bool Force all images to resample in all cases
 	 */
 	private static $force_resample = false;
-	
+
+	/**
+	 * @config
+	 * @var bool Regenerates images if set to true. This is set by {@link flush()}
+	 */
+	private static $flush = false;
+
+	/**
+	 * Triggered early in the request when someone requests a flush.
+	 */
+	public static function flush() {
+		self::$flush = true;
+	}
+
 	public static function set_backend($backend) {
 		self::config()->backend = $backend;
 	}
-	
+
 	public static function get_backend() {
 		return self::config()->backend;
 	}
@@ -424,7 +437,7 @@ class Image extends File {
 		if($this->ID && $this->Filename && Director::fileExists($this->Filename)) {
 			$cacheFile = call_user_func_array(array($this, "cacheFilename"), $args);
 			
-			if(!file_exists(Director::baseFolder()."/".$cacheFile) || isset($_GET['flush'])) {
+			if(!file_exists(Director::baseFolder()."/".$cacheFile) || self::$flush) {
 				call_user_func_array(array($this, "generateFormattedImage"), $args);
 			}
 			

tests/bootstrap.php

@@ -66,10 +66,3 @@ SapphireTest::set_is_running_test(true);
 // Remove the error handler so that PHPUnit can add its own
 restore_error_handler();
 
-if(!isset($_GET['flush']) || !$_GET['flush']) {
-	Debug::message(
-		"WARNING: Manifest not flushed. " .
-		"Add flush=1 as an argument to discover new classes or files.\n",
-		false
-	);
-}

view/Requirements.php

@@ -7,7 +7,14 @@
  * @package framework
  * @subpackage view
  */
-class Requirements {
+class Requirements implements Flushable {
+
+	/**
+	 * Triggered early in the request when someone requests a flush.
+	 */
+	public static function flush() {
+		self::delete_all_combined_files();
+	}
 
 	/**
 	 * Enable combining of css/javascript files.
@@ -272,6 +279,13 @@ class Requirements {
 		return self::backend()->delete_combined_files($combinedFileName);
 	}
 
+	/**
+	 * 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()->delete_all_combined_files();
+	}
 
 	/**
 	 * Re-sets the combined files definition. See {@link Requirements_Backend::clear_combined_files()}
@@ -455,7 +469,7 @@ class Requirements_Backend {
 	 * @var boolean
 	 */
 	protected $force_js_to_bottom = false;
-	
+
 	public function set_combined_files_enabled($enable) {
 		$this->combined_files_enabled = (bool) $enable;
 	}
@@ -1005,6 +1019,20 @@ class Requirements_Backend {
 		}
 	}
 
+	/**
+	 * Deletes all generated combined files in the configured combined files directory,
+	 * but doesn't delete the directory itself.
+	 */
+	public function delete_all_combined_files() {
+		$combinedFolder = $this->getCombinedFilesFolder();
+		if(!$combinedFolder) return false;
+
+		$path = Director::baseFolder() . '/' . $combinedFolder;
+		if(file_exists($path)) {
+			Filesystem::removeFolder($path, true);
+		}
+	}
+
 	public function clear_combined_files() {
 		$this->combine_files = array();
 	}
@@ -1085,7 +1113,7 @@ class Requirements_Backend {
 			}
 
 			// Determine if we need to build the combined include
-			if(file_exists($combinedFilePath) && !isset($_GET['flush'])) {
+			if(file_exists($combinedFilePath)) {
 				// file exists, check modification date of every contained file
 				$srcLastMod = 0;
 				foreach($fileList as $file) {

view/SSViewer.php

@@ -555,7 +555,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope {
  * <b>Caching</b>
  *
  * Compiled templates are cached via {@link SS_Cache}, usually on the filesystem.  
- * If you put ?flush=all on your URL, it will force the template to be recompiled.  
+ * If you put ?flush=1 on your URL, it will force the template to be recompiled.
  *
  * @see http://doc.silverstripe.org/themes
  * @see http://doc.silverstripe.org/themes:developing
@@ -563,7 +563,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope {
  * @package framework
  * @subpackage view
  */
-class SSViewer {
+class SSViewer implements Flushable {
 	
 	/**
 	 * @config
@@ -638,6 +638,13 @@ class SSViewer {
 	 */
 	private static $global_key = '$CurrentReadingMode, $CurrentUser.ID';
 
+	/**
+	 * Triggered early in the request when someone requests a flush.
+	 */
+	public static function flush() {
+		self::flush_template_cache();
+	}
+
 	/**
 	 * Create a template from a string instead of a .ss file
 	 * 
@@ -744,18 +751,6 @@ class SSViewer {
 	public function __construct($templateList, TemplateParser $parser = null) {
 		$this->setParser($parser ?: Injector::inst()->get('SSTemplateParser'));
 
-		// flush template manifest cache if requested
-		if (isset($_GET['flush']) && $_GET['flush'] == 'all') {
-			if(Director::isDev() || Director::is_cli() || Permission::check('ADMIN')) {
-				self::flush_template_cache();
-			} else {
-				if(!Security::ignore_disallowed_actions()) {
-					return Security::permissionFailure(null, 'Please log in as an administrator to flush ' .
-						'the template cache.');
-				}
-			}
-		}
-		
 		if(!is_array($templateList) && substr((string) $templateList,-3) == '.ss') {
 			$this->chosenTemplates['main'] = $templateList;
 		} else {
@@ -927,7 +922,7 @@ class SSViewer {
 		if (!self::$flushed) {
 			$dir = dir(TEMP_FOLDER);
 			while (false !== ($file = $dir->read())) {
-				if (strstr($file, '.cache')) { unlink(TEMP_FOLDER.'/'.$file); }
+				if (strstr($file, '.cache')) unlink(TEMP_FOLDER . '/' . $file);
 			}
 			self::$flushed = true;
 		}
@@ -1038,7 +1033,7 @@ class SSViewer {
 			. str_replace(array('\\','/',':'), '.', Director::makeRelative(realpath($template)));
 		$lastEdited = filemtime($template);
 
-		if(!file_exists($cacheFile) || filemtime($cacheFile) < $lastEdited || isset($_GET['flush'])) {
+		if(!file_exists($cacheFile) || filemtime($cacheFile) < $lastEdited) {
 			$content = file_get_contents($template);
 			$content = $this->parseTemplateContent($content, $template);