From c50dc06401b7ea08bee4eef81b33586a2e2c1bbd Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 26 Nov 2015 15:22:28 +1300 Subject: [PATCH] API Update ErrorPage to use FilesystemGeneratedAssetHandler --- code/model/ErrorPage.php | 82 ++++++++++++++--------------------- tests/model/ErrorPageTest.php | 11 +++-- 2 files changed, 40 insertions(+), 53 deletions(-) diff --git a/code/model/ErrorPage.php b/code/model/ErrorPage.php index db968edf..9217ed14 100644 --- a/code/model/ErrorPage.php +++ b/code/model/ErrorPage.php @@ -30,18 +30,12 @@ class ErrorPage extends Page { private static $description = 'Custom content for different error cases (e.g. "Page not found")'; /** - * Allows control over writing directly to the static_filepath directory. Only required if relying on - * apache ErrorDocument, and can be turned off otherwise. + * Allows control over writing directly to the configured `GeneratedAssetStore`. * * @config * @var bool */ private static $enable_static_file = true; - /** - * @config - * @var string - */ - private static $static_filepath = ASSETS_PATH; /** * Prefix for storing error files in the {@see GeneratedAssetHandler} store. @@ -119,6 +113,11 @@ class ErrorPage extends Page { $page->publish('Stage', 'Live'); } + // Check if static files are enabled + if(!self::config()->enable_static_file) { + continue; + } + // Ensure this page has cached error content $success = true; if(!$page->hasStaticPage()) { @@ -239,21 +238,15 @@ class ErrorPage extends Page { * @return bool */ protected function hasStaticPage() { + if(!self::config()->enable_static_file) { + return false; + } + // Attempt to retrieve content from generated file handler $filename = $this->getErrorFilename(); $storeFilename = File::join_paths(self::config()->store_filepath, $filename); - $result = self::get_asset_handler()->getGeneratedContent($storeFilename, 0); - if($result) { - return true; - } - - // Fallback to physical store - if(self::config()->enable_static_file) { - $staticPath = self::config()->static_filepath . "/" . $filename; - return file_exists($staticPath); - } - - return false; + $result = self::get_asset_handler()->getContent($storeFilename); + return !empty($result); } /** @@ -262,6 +255,10 @@ class ErrorPage extends Page { * @return true if the page write was successful */ public function writeStaticPage() { + if(!self::config()->enable_static_file) { + return false; + } + // Run the page (reset the theme, it might've been disabled by LeftAndMain::init()) Config::nest(); Config::inst()->update('SSViewer', 'theme_enabled', true); @@ -270,18 +267,11 @@ class ErrorPage extends Page { $errorContent = $response->getBody(); // Store file content in the default store - $filename = $this->getErrorFilename(); - $storeFilename = File::join_paths(self::config()->store_filepath, $filename); - self::get_asset_handler()->updateContent($storeFilename, 0, $errorContent); - - // Write to physical store - if(self::config()->enable_static_file) { - Filesystem::makeFolder(self::config()->static_filepath); - $staticPath = self::config()->static_filepath . "/" . $filename; - if(!file_put_contents($staticPath, $errorContent)) { - return false; - } - } + $storeFilename = File::join_paths( + self::config()->store_filepath, + $this->getErrorFilename() + ); + self::get_asset_handler()->setContent($storeFilename, $errorContent); // Success return true; @@ -300,31 +290,23 @@ class ErrorPage extends Page { } /** - * Returns an absolute filesystem path to a static error file - * which is generated through {@link publish()}. - * - * @param int $statusCode A HTTP Statuscode, mostly 404 or 500 - * @param string $locale A locale, e.g. 'de_DE' (Optional) + * Returns statically cached content for a given error code * * @param int $statusCode A HTTP Statuscode, typically 404 or 500 - * @return string + * @return string|null */ public static function get_content_for_errorcode($statusCode) { - // Attempt to retrieve content from generated file handler - $filename = self::get_error_filename($statusCode); - $storeFilename = File::join_paths(self::config()->store_filepath, $filename); - $result = self::get_asset_handler()->getGeneratedContent($storeFilename, 0); - if($result) { - return $result; + if(!self::config()->enable_static_file) { + return null; } - // Fallback to physical store - if(self::config()->enable_static_file) { - $staticPath = self::config()->static_filepath . "/" . $filename; - if(file_exists($staticPath)) { - return file_get_contents($staticPath); - } - } + // Attempt to retrieve content from generated file handler + $filename = self::get_error_filename($statusCode); + $storeFilename = File::join_paths( + self::config()->store_filepath, + $filename + ); + return self::get_asset_handler()->getContent($storeFilename); } /** diff --git a/tests/model/ErrorPageTest.php b/tests/model/ErrorPageTest.php index 84ea91b5..9606bccf 100644 --- a/tests/model/ErrorPageTest.php +++ b/tests/model/ErrorPageTest.php @@ -18,7 +18,6 @@ class ErrorPageTest extends FunctionalTest { parent::setUp(); // Set temporary asset backend store AssetStoreTest_SpyStore::activate('ErrorPageTest'); - Config::inst()->update('ErrorPage', 'static_filepath', AssetStoreTest_SpyStore::base_path()); Config::inst()->update('ErrorPage', 'enable_static_file', true); Config::inst()->update('Director', 'environment_type', 'live'); $this->logInWithPermission('ADMIN'); @@ -111,8 +110,14 @@ class ErrorPageTest extends FunctionalTest { $page->write(); $page->doPublish(); - // Error content is available, even though the static file does not exist (only in assetstore) - $this->assertNotEmpty(ErrorPage::get_content_for_errorcode('405')); + // Dynamic content is available + $response = ErrorPage::response_for('405'); + $this->assertNotEmpty($response); + $this->assertNotEmpty($response->getBody()); + $this->assertEquals(405, (int)$response->getStatusCode()); + + // Static content is not available + $this->assertEmpty(ErrorPage::get_content_for_errorcode('405')); $expectedErrorPagePath = AssetStoreTest_SpyStore::base_path() . '/error-405.html'; $this->assertFileNotExists($expectedErrorPagePath, 'Error page is not cached in static location'); }