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

@@ -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];
         }
 
@@ -112,11 +126,17 @@ class CmsObject implements ArrayAccess
              * 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;
     }
@@ -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.
      */

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();
     }
 }

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, '<p>Test content 1</p>');
+          file_put_contents($filePath2, '<p>Test content 2</p>');
+
+          /*
+           * First try - both objects should be fresh
+           */
+          $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm');
+          $this->assertEquals($obj1->content, '<p>Test content 1</p>');
+          $this->assertFalse($obj1->isFresh());
+          $obj2 = TestTemporaryCmsObject::loadCached($theme, 'test1.htm');
+          $this->assertEquals($obj2->content, '<p>Test content 2</p>');
+          $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, '<p>Updated test content</p>');
+
+          CmsObject::clearInternalCache();
+          $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm');
+          $this->assertEquals($obj1->content, '<p>Updated test content</p>');
+          $obj2 = TestTemporaryCmsObject::loadCached($theme, 'test1.htm');
+          $this->assertEquals($obj2->content, '<p>Test content 2</p>');
+          $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, '<p>Updated test content again</p>');
+
+          CmsObject::clearInternalCache();
+          $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm');
+          $this->assertEquals($obj1->content, '<p>Updated test content again</p>');
+          $obj2 = TestTemporaryCmsObject::loadCached($theme, 'test1.htm');
+          $this->assertEquals($obj2->content, '<p>Test content 2</p>');
+          $this->assertTrue($obj2->isFresh());
+          $obj1 = TestTemporaryCmsObject::loadCached($theme, 'test.htm');
+          $this->assertFalse($obj1->isFresh());
+    }
+
     public function testFillFillable()
     {
         $theme = Theme::load('apitest');