From 2b316e79e5f957bfe06385f762c237acaf75b0e7 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Sat, 16 Aug 2014 12:51:17 +1200 Subject: [PATCH] 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 | 1 + api/RestfulService.php | 17 +++++++- control/FlushRequestFilter.php | 24 +++++++++++ control/RequestProcessor.php | 2 +- core/Flushable.php | 20 +++++++++ dev/TestRunner.php | 27 ++++++------ docs/en/howto/phpunit-configuration.md | 20 +++++++-- docs/en/reference/execution-pipeline.md | 9 ++++ docs/en/reference/flushable.md | 56 +++++++++++++++++++++++++ docs/en/topics/caching.md | 9 +++- i18n/i18n.php | 31 ++++++++++---- model/Image.php | 21 ++++++++-- tests/bootstrap.php | 7 ---- view/Requirements.php | 34 +++++++++++++-- view/SSViewer.php | 27 +++++------- 15 files changed, 244 insertions(+), 61 deletions(-) create mode 100644 control/FlushRequestFilter.php create mode 100644 core/Flushable.php create mode 100644 docs/en/reference/flushable.md diff --git a/_config/requestprocessors.yml b/_config/requestprocessors.yml index 1e9310bd3..17ee24025 100644 --- a/_config/requestprocessors.yml +++ b/_config/requestprocessors.yml @@ -5,4 +5,5 @@ Injector: RequestProcessor: properties: filters: + - '%$FlushRequestFilter' - '%$VersionedRequestFilter' diff --git a/api/RestfulService.php b/api/RestfulService.php index 5f8d7ae29..9d1116d42 100644 --- a/api/RestfulService.php +++ b/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()) { diff --git a/control/FlushRequestFilter.php b/control/FlushRequestFilter.php new file mode 100644 index 000000000..ab6be8dd4 --- /dev/null +++ b/control/FlushRequestFilter.php @@ -0,0 +1,24 @@ +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; + } + +} diff --git a/control/RequestProcessor.php b/control/RequestProcessor.php index e391be0a5..566ad8017 100644 --- a/control/RequestProcessor.php +++ b/control/RequestProcessor.php @@ -45,4 +45,4 @@ class RequestProcessor implements RequestFilter { } } } -} \ No newline at end of file +} diff --git a/core/Flushable.php b/core/Flushable.php new file mode 100644 index 000000000..4b1781bb9 --- /dev/null +++ b/core/Flushable.php @@ -0,0 +1,20 @@ +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() { diff --git a/docs/en/howto/phpunit-configuration.md b/docs/en/howto/phpunit-configuration.md index 4ac184feb..92db5bfbe 100644 --- a/docs/en/howto/phpunit-configuration.md +++ b/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 `` flag in your `phpunit.xml` (see [Appenix C: "Setting PHP INI settings"](http://www.phpunit.de/manual/current/en/appendixes.configuration.html)): - + + + + +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: + + + + + diff --git a/docs/en/reference/execution-pipeline.md b/docs/en/reference/execution-pipeline.md index 00184f18d..993250a87 100644 --- a/docs/en/reference/execution-pipeline.md +++ b/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. + + diff --git a/docs/en/reference/flushable.md b/docs/en/reference/flushable.md new file mode 100644 index 000000000..b24634c57 --- /dev/null +++ b/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 + 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 + TEMP_FOLDER . DIRECTORY_SEPARATOR . 'cache' ) )); - SS_Cache::pick_backend('two_level', 'any', 10); \ No newline at end of file + SS_Cache::pick_backend('two_level', 'any', 10); diff --git a/i18n/i18n.php b/i18n/i18n.php index 4b81fe1c9..4acc5eb1a 100644 --- a/i18n/i18n.php +++ b/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 diff --git a/model/Image.php b/model/Image.php index 868f60cab..6cdb17c6a 100644 --- a/model/Image.php +++ b/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); } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index bd02b154e..d95e29bbc 100644 --- a/tests/bootstrap.php +++ b/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 - ); -} diff --git a/view/Requirements.php b/view/Requirements.php index 7db31e0d9..8848c6087 100644 --- a/view/Requirements.php +++ b/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) { diff --git a/view/SSViewer.php b/view/SSViewer.php index d5fc9fba1..f426b3d9c 100644 --- a/view/SSViewer.php +++ b/view/SSViewer.php @@ -555,7 +555,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope { * Caching * * 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);