From 2fae5a30b9327de96a0fc31117aefb4f25fc069c Mon Sep 17 00:00:00 2001 From: DQ Sully Date: Sat, 26 Dec 2015 08:16:58 -0700 Subject: [PATCH] Fix Twig Caching When October would load a file from its changed source, Twig would not see the message until it had gone. See Cms\Classes\Loader->isFresh. This meant a template would not update unless you deleted the Twig cache, or that template's TTL expired. Fix: add another variable (freshness) that would only change after being observed, and accurately reflected if a template's source had been modified --- modules/cms/classes/CmsObject.php | 64 +++++++++++++++++++-- modules/cms/twig/Loader.php | 2 +- tests/unit/cms/classes/CmsObjectTest.php | 73 +++++++++++++++++++++++- 3 files changed, 132 insertions(+), 7 deletions(-) diff --git a/modules/cms/classes/CmsObject.php b/modules/cms/classes/CmsObject.php index 8b6bfccaf..a62690e87 100644 --- a/modules/cms/classes/CmsObject.php +++ b/modules/cms/classes/CmsObject.php @@ -12,6 +12,8 @@ use RecursiveDirectoryIterator; use RecursiveIteratorIterator; use ArrayAccess; use Exception; +use DateTime; +use DateInterval; /** * This is a base class for all CMS objects - content files, pages, partials and layouts. @@ -47,6 +49,14 @@ class CmsObject implements ArrayAccess */ protected $loadedFromCache = false; + /** + * @var int How 'fresh' the file is for Twig caching + * 0: Changed but not noticed/accessed + * 1: Changed and noticed/accessed, will change to 2 on next load + * 2: Unchanged + */ + protected $freshness = 0; + protected static $fillable = [ 'content', 'fileName' @@ -90,6 +100,10 @@ class CmsObject implements ArrayAccess $filePath = static::getFilePath($theme, $fileName); if (array_key_exists($filePath, ObjectMemoryCache::$cache)) { + if(ObjectMemoryCache::$cache[$filePath]->freshness == 1) { + ObjectMemoryCache::$cache[$filePath]->freshness = 2; + } + return ObjectMemoryCache::$cache[$filePath]; } @@ -109,14 +123,20 @@ class CmsObject implements ArrayAccess if ($cached !== false) { /* - * The cached item exists and successfully unserialized. + * The cached item exists and successfully unserialized. * Initialize the object from the cached data. */ + if($cached['freshness'] == 1) { + $cached['freshness'] = 2; + } + Cache::put($key, serialize($cached), $cached['expire']); + $obj = new static($theme); $obj->content = $cached['content']; $obj->fileName = $fileName; $obj->mtime = File::lastModified($filePath); $obj->loadedFromCache = true; + $obj->freshness = $cached['freshness']; $obj->initFromCache($cached); return ObjectMemoryCache::$cache[$filePath] = $obj; @@ -136,12 +156,15 @@ class CmsObject implements ArrayAccess $cached = [ 'mtime' => @File::lastModified($filePath), - 'content' => $obj->content + 'content' => $obj->content, + 'freshness' => 0, + 'expire' => (new DateTime())->add(new DateInterval('PT'.Config::get('cms.parsedPageCacheTTL', 1440).'M')) ]; $obj->loadedFromCache = false; + $obj->freshness = 0; $obj->initCacheItem($cached); - Cache::put($key, serialize($cached), Config::get('cms.parsedPageCacheTTL', 1440)); + Cache::put($key, serialize($cached), $cached['expire']); return ObjectMemoryCache::$cache[$filePath] = $obj; } @@ -183,7 +206,7 @@ class CmsObject implements ArrayAccess } /** - * Returns the maximum allowed path nesting level. + * Returns the maximum allowed path nesting level. * The default value is 2, meaning that files * can only exist in the root directory, or in a subdirectory. * @return mixed Returns the maximum nesting level or null if any level is allowed. @@ -295,6 +318,37 @@ class CmsObject implements ArrayAccess return $this->loadedFromCache; } + /** + * Returns true if the object is fresh for Twig. + * Waits until it is called and the object reloaded to change the object's freshness. + * This method is used by the CMS internally. + * @return boolean + */ + public function isFresh() { + $filePath = $this->getFullPath(); + $key = self::getObjectTypeDirName().crc32($filePath); + $cached = Cache::get($key, false); + + if($cached !== false) { + $cached = @unserialize($cached); + } + + if($this->freshness < 2) { + $this->freshness = 1; + if($cached !== false) { + $cached['freshness'] = 1; + Cache::put($key, serialize($cached), $cached['expire']); + } + return false; + } + + if($cached !== false) { + $cached['freshness'] = 1; + Cache::put($key, serialize($cached), $cached['expire']); + } + return true; + } + /** * Returns the Twig content string. */ @@ -443,7 +497,7 @@ class CmsObject implements ArrayAccess return $result; } - + /** * Returns the absolute file path. * @param $theme Specifies a theme the file belongs to. diff --git a/modules/cms/twig/Loader.php b/modules/cms/twig/Loader.php index 03d6dada5..6a9f7d748 100644 --- a/modules/cms/twig/Loader.php +++ b/modules/cms/twig/Loader.php @@ -57,6 +57,6 @@ class Loader implements Twig_LoaderInterface */ public function isFresh($name, $time) { - return $this->obj->isLoadedFromCache(); + return $this->obj->isFresh(); } } diff --git a/tests/unit/cms/classes/CmsObjectTest.php b/tests/unit/cms/classes/CmsObjectTest.php index 24166c19a..14cc2b330 100644 --- a/tests/unit/cms/classes/CmsObjectTest.php +++ b/tests/unit/cms/classes/CmsObjectTest.php @@ -124,6 +124,77 @@ class CmsObjectTest extends TestCase $this->assertNull($obj); } + public function testFreshness() + { + $theme = Theme::load('test'); + $themePath = $theme->getPath(); + + $filePath1 = $themePath . '/temporary/test.htm'; + if (file_exists($filePath1)) + @unlink($filePath1); + $filePath2 = $themePath . '/temporary/test1.htm'; + if (file_exists($filePath2)) + @unlink($filePath2); + + $this->assertFileNotExists($filePath1); + $this->assertFileNotExists($filePath2); + + file_put_contents($filePath1, '

Test content 1

'); + file_put_contents($filePath2, '

Test content 2

'); + + /* + * First try - both objects should be fresh + */ + $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm'); + $this->assertEquals($obj1->content, '

Test content 1

'); + $this->assertFalse($obj1->isFresh()); + $obj2 = TestTemporaryCmsObject::loadCached($theme, 'test1.htm'); + $this->assertEquals($obj2->content, '

Test content 2

'); + $this->assertFalse($obj1->isFresh()); + $this->assertFalse($obj2->isFresh()); + + /* + * Second try - both objects should not be fresh + */ + CmsObject::clearInternalCache(); + $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm'); + $this->assertTrue($obj1->isFresh()); + $obj2 = TestTemporaryCmsObject::loadCached($theme, 'test1.htm'); + $this->assertTrue($obj1->isFresh()); + $this->assertTrue($obj2->isFresh()); + + /* + * Modify the file. The first object should show as not fresh, until it is observed and reloaded. + */ + sleep(1); // Sleep a second in order to have the update file modification time + file_put_contents($filePath1, '

Updated test content

'); + + CmsObject::clearInternalCache(); + $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm'); + $this->assertEquals($obj1->content, '

Updated test content

'); + $obj2 = TestTemporaryCmsObject::loadCached($theme, 'test1.htm'); + $this->assertEquals($obj2->content, '

Test content 2

'); + $this->assertFalse($obj1->isFresh()); + $this->assertTrue($obj2->isFresh()); + $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm'); + $this->assertTrue($obj1->isFresh()); + + /* + * Modify the file again, but wait to look for change. The object should still show as changed, even after the second load. + */ + sleep(1); // Sleep a second in order to have the update file modification time + file_put_contents($filePath1, '

Updated test content again

'); + + CmsObject::clearInternalCache(); + $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm'); + $this->assertEquals($obj1->content, '

Updated test content again

'); + $obj2 = TestTemporaryCmsObject::loadCached($theme, 'test1.htm'); + $this->assertEquals($obj2->content, '

Test content 2

'); + $this->assertTrue($obj2->isFresh()); + $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm'); + $this->assertFalse($obj1->isFresh()); + } + public function testFillFillable() { $theme = Theme::load('apitest'); @@ -337,4 +408,4 @@ class CmsObjectTest extends TestCase $this->assertFileExists($destFilePath); $this->assertEquals($testContents, file_get_contents($destFilePath)); } -} \ No newline at end of file +}