Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion modules/cms/classes/AutoDatasource.php
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,24 @@ protected function getDatasourceForPath(string $path): DatasourceInterface
return $this->datasources[$datasourceIndex];
}

/**
* Get the path cache entry for the provided path from the first datasource that reports it
*
* @return mixed The datasource's entry for this path, or null if no datasource reports it.
* Database datasources report a last modified timestamp, other datasources
* report `true`, and paths marked as deleted report `false`.
*/
protected function getPathCacheEntry(string $path): mixed
{
foreach ($this->pathCache as $paths) {
if (isset($paths[$path])) {
return $paths[$path];
}
}

return null;
}

/**
* Get all valid paths for the provided directory, removing any paths marked as deleted
*
Expand Down Expand Up @@ -512,7 +530,17 @@ public function delete(string $dirName, string $fileName, string $extension): bo
*/
public function lastModified(string $dirName, string $fileName, string $extension): ?int
{
return $this->getDatasourceForPath($this->makeFilePath($dirName, $fileName, $extension))->lastModified($dirName, $fileName, $extension);
$path = $this->makeFilePath($dirName, $fileName, $extension);

// Database datasources record modification times in the path cache, which lets the
// Halcyon cache validate itself without querying the database on every request.
// Anything else (filesystem sources report `true`, deleted paths report `false`)
// falls through to the datasource so its modification time stays live.
if (!$this->singleDatasourceMode && is_int($mtime = $this->getPathCacheEntry($path))) {
return $mtime;
}

return $this->getDatasourceForPath($path)->lastModified($dirName, $fileName, $extension);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions modules/cms/classes/Theme.php
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,8 @@ public static function resetCache(bool $memoryOnly = false): void
self::$activeThemeCache = false;
self::$editThemeCache = false;

ThemeData::flushCache();

// Sometimes it may be desired to only clear the local cache of the active / edit themes instead of the persistent cache
if (!$memoryOnly) {
Cache::forget(self::ACTIVE_KEY);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

return new class extends \Winter\Storm\Database\Updates\Migration
{
public function up()
{
Schema::table('cms_theme_templates', function ($table) {
// Every datasource query filters on source and path together, so a composite
// index serves them all. This makes the standalone source index redundant.
$table->index(['source', 'path', 'deleted_at']);
$table->dropIndex(['source']);
});
}

public function down()
{
Schema::table('cms_theme_templates', function ($table) {
$table->index(['source']);
$table->dropIndex(['source', 'path', 'deleted_at']);
});
}
};
44 changes: 43 additions & 1 deletion modules/cms/models/ThemeData.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Cms\Classes\Theme as CmsTheme;
use Exception;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Lang;
use System\Classes\CombineAssets;
use System\Models\File;
Expand Down Expand Up @@ -54,6 +55,11 @@ class ThemeData extends Model
*/
protected static $instances = [];

/**
* @var int The number of minutes the theme data is cached for.
*/
protected static $cacheTtl = 1440;

/**
* Before saving the model, strip dynamic attributes applied from config.
* @return void
Expand All @@ -76,13 +82,45 @@ public function beforeSave()
*/
public function afterSave()
{
static::flushCache($this->theme);

try {
CombineAssets::resetCache();
}
catch (Exception $ex) {
}
}

/**
* Clear the cache after deleting so that the record isn't served from the cache.
*/
public function afterDelete()
{
static::flushCache($this->theme);
}

/**
* Returns the cache key used to store the data for the provided theme directory.
*/
public static function getCacheKey(string $dirName): string
{
return 'cms::theme.data.' . $dirName;
}

/**
* Removes both the persistent and in memory cache of the provided theme directory's data.
*/
public static function flushCache(?string $dirName = null): void
{
if (is_null($dirName)) {
self::$instances = [];
return;
}

Cache::forget(static::getCacheKey($dirName));
unset(self::$instances[$dirName]);
}

/**
* Returns a cached version of this model, based on a Theme object.
* @param $theme Cms\Classes\Theme
Expand All @@ -96,7 +134,11 @@ public static function forTheme($theme)
}

try {
$themeData = self::firstOrCreate(['theme' => $dirName]);
// The record is cached rather than queried on every request; it is invalidated
// by afterSave() / afterDelete(), which also covers the initial creation below.
$themeData = self::where('theme', $dirName)
->remember(self::$cacheTtl, self::getCacheKey($dirName))
->first() ?: self::create(['theme' => $dirName]);
}
catch (Exception $ex) {
// Database failed
Expand Down
98 changes: 97 additions & 1 deletion modules/cms/tests/classes/AutoDatasourceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

namespace Cms\Tests\Classes;

use Exception;
use System\Tests\Bootstrap\PluginTestCase;
use Cms\Classes\AutoDatasource;
use Winter\Storm\Database\MemoryCache;
use Winter\Storm\Database\Model;
use Winter\Storm\Halcyon\Datasource\DbDatasource;
use Winter\Storm\Halcyon\Datasource\FileDatasource;
use Winter\Storm\Support\Facades\DB;

class CmsThemeTemplateFixture extends Model
{
Expand Down Expand Up @@ -58,7 +61,8 @@ public function setUp(): void
'source' => 'test',
'path' => 'partials/subdir/test.htm',
'content' => 'AutoDatasource partials/subdir/test.htm',
'file_size' => 39
'file_size' => 39,
'updated_at' => '2019-06-01 12:00:00'
]);

$this->fixtures[] = CmsThemeTemplateFixture::create([
Expand Down Expand Up @@ -118,4 +122,96 @@ public function testSelect()
// One filesystem partial should be marked deleted in database
$this->assertArrayNotHasKey('nesting/level2.htm', $results);
}

public function testPathCacheValueShapes()
{
$pathCache = self::getProtectedProperty($this->datasource, 'pathCache');

// Database records report their last modified time, deleted records report false
$this->assertIsInt($pathCache[0]['partials/subdir/test.htm']);
$this->assertEquals(
strtotime('2019-06-01 12:00:00'),
$pathCache[0]['partials/subdir/test.htm']
);
$this->assertFalse($pathCache[0]['partials/nesting/level2.htm']);

// Filesystem records continue to report true so that their mtime is resolved live
$this->assertTrue($pathCache[1]['partials/layout-partial.htm']);
}

public function testLastModifiedIsServedFromPathCacheWithoutQuerying()
{
// The duplicate query cache would otherwise mask a query issued by this call
MemoryCache::instance()->flush();

DB::connection()->flushQueryLog();
DB::connection()->enableQueryLog();

$mtime = $this->datasource->lastModified('partials', 'subdir/test', 'htm');

$queries = DB::connection()->getQueryLog();
DB::connection()->disableQueryLog();

$this->assertEquals(strtotime('2019-06-01 12:00:00'), $mtime);
$this->assertCount(0, $queries, 'lastModified() should not query the database');
}

public function testLastModifiedFallsBackToFilesystemDatasource()
{
$path = base_path('modules/system/tests/fixtures/themes/test/partials/layout-partial.htm');

$this->assertEquals(
filemtime($path),
$this->datasource->lastModified('partials', 'layout-partial', 'htm')
);
}

public function testLastModifiedIsStableForNullUpdatedAt()
{
$this->fixtures[] = CmsThemeTemplateFixture::create([
'source' => 'test',
'path' => 'partials/no-timestamp.htm',
'content' => 'AutoDatasource partials/no-timestamp.htm',
'file_size' => 40,
'updated_at' => null,
]);

$this->datasource->populateCache(true);

$pathCache = self::getProtectedProperty($this->datasource, 'pathCache');
$cached = $pathCache[0]['partials/no-timestamp.htm'];

// updated_at is nullable, and Carbon::parse(null) resolves to "now". The value is
// resolved once, when the path cache is built, so lastModified() reports it
// consistently rather than returning a different result on every call.
// Note this does not make the Halcyon cache usable for such records: selectOne()
// still resolves their mtime live, so the two disagree and the cache is busted on
// every request. That is pre-existing and not addressed here.
$this->assertIsInt($cached);
$this->assertEquals($cached, $this->datasource->lastModified('partials', 'no-timestamp', 'htm'));
}

public function testLastModifiedThrowsForDeletedPath()
{
$this->expectException(Exception::class);
$this->expectExceptionMessage('partials/nesting/level2.htm is deleted');

$this->datasource->lastModified('partials', 'nesting/level2', 'htm');
}

public function testLastModifiedReflectsUpdatesMadeThroughTheDatasource()
{
$before = $this->datasource->lastModified('partials', 'subdir/test', 'htm');

$this->datasource->update('partials', 'subdir/test', 'htm', 'Updated content');

$after = $this->datasource->lastModified('partials', 'subdir/test', 'htm');

// Editing a template must take effect immediately, without clearing the cache
$this->assertGreaterThan($before, $after);
$this->assertEquals(
'Updated content',
$this->datasource->selectOne('partials', 'subdir/test', 'htm')['content']
);
}
}
11 changes: 11 additions & 0 deletions modules/cms/tests/classes/ThemeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use System\Tests\Bootstrap\TestCase;
use Cms\Classes\Theme;
use Cms\Models\ThemeData;
use Config;
use Event;
use Winter\Storm\Exception\ApplicationException;
Expand Down Expand Up @@ -163,4 +164,14 @@ public function testLoadRejectsPathTraversal()

Theme::load('../../etc');
}

public function testResetCacheClearsThemeData()
{
$themeData = new ThemeData(['theme' => 'test']);
self::setProtectedProperty($themeData, 'instances', ['test' => $themeData]);

Theme::resetCache();

$this->assertEmpty(self::getProtectedProperty($themeData, 'instances'));
}
}
9 changes: 9 additions & 0 deletions modules/cms/tests/fixtures/themes/themedata/theme.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
name: ThemeData Test
description: Theme with customization fields, used to test Cms\Models\ThemeData

form:
fields:
site_name:
label: Site name
type: text
default: Winter
Loading
Loading