diff --git a/modules/cms/classes/AutoDatasource.php b/modules/cms/classes/AutoDatasource.php index 47d2238046..63852ba8ba 100644 --- a/modules/cms/classes/AutoDatasource.php +++ b/modules/cms/classes/AutoDatasource.php @@ -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 * @@ -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); } /** diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index 98f63d00df..738c6efd37 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -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); diff --git a/modules/cms/database/migrations/2026_08_09_000004_Db_Cms_Theme_Templates_Indexes.php b/modules/cms/database/migrations/2026_08_09_000004_Db_Cms_Theme_Templates_Indexes.php new file mode 100644 index 0000000000..d584768e02 --- /dev/null +++ b/modules/cms/database/migrations/2026_08_09_000004_Db_Cms_Theme_Templates_Indexes.php @@ -0,0 +1,22 @@ +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']); + }); + } +}; diff --git a/modules/cms/models/ThemeData.php b/modules/cms/models/ThemeData.php index 03015851ff..687fb91536 100644 --- a/modules/cms/models/ThemeData.php +++ b/modules/cms/models/ThemeData.php @@ -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; @@ -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 @@ -76,6 +82,8 @@ public function beforeSave() */ public function afterSave() { + static::flushCache($this->theme); + try { CombineAssets::resetCache(); } @@ -83,6 +91,36 @@ public function afterSave() } } + /** + * 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 @@ -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 diff --git a/modules/cms/tests/classes/AutoDatasourceTest.php b/modules/cms/tests/classes/AutoDatasourceTest.php index b481c2afc9..7e970f377c 100644 --- a/modules/cms/tests/classes/AutoDatasourceTest.php +++ b/modules/cms/tests/classes/AutoDatasourceTest.php @@ -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 { @@ -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([ @@ -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'] + ); + } } diff --git a/modules/cms/tests/classes/ThemeTest.php b/modules/cms/tests/classes/ThemeTest.php index e664ba7294..3e904e8811 100644 --- a/modules/cms/tests/classes/ThemeTest.php +++ b/modules/cms/tests/classes/ThemeTest.php @@ -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; @@ -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')); + } } diff --git a/modules/cms/tests/fixtures/themes/themedata/theme.yaml b/modules/cms/tests/fixtures/themes/themedata/theme.yaml new file mode 100644 index 0000000000..90d9ee661d --- /dev/null +++ b/modules/cms/tests/fixtures/themes/themedata/theme.yaml @@ -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 diff --git a/modules/cms/tests/models/ThemeDataTest.php b/modules/cms/tests/models/ThemeDataTest.php new file mode 100644 index 0000000000..ae90acaaaf --- /dev/null +++ b/modules/cms/tests/models/ThemeDataTest.php @@ -0,0 +1,154 @@ +theme = Theme::load('themedata'); + } + + public function tearDown(): void + { + ThemeData::flushCache('themedata'); + + parent::tearDown(); + } + + /** + * Counts the queries made against the theme data table while running the callback. + */ + protected function countQueries(callable $callback): int + { + // Identical queries are deduplicated in memory for the lifetime of a request, which + // would otherwise hide the query this cache is meant to avoid on the next request + MemoryCache::instance()->flush(); + + DB::connection()->flushQueryLog(); + DB::connection()->enableQueryLog(); + + $callback(); + + $queries = DB::connection()->getQueryLog(); + DB::connection()->disableQueryLog(); + + return count(array_filter($queries, function ($query) { + return str_contains($query['query'], 'cms_theme_data'); + })); + } + + /** + * Ensures the record exists and the persistent cache is populated, then drops only the + * in memory cache, simulating the state at the start of a subsequent request. + */ + protected function primeCache(): void + { + ThemeData::forTheme($this->theme); + ThemeData::flushCache(); + + ThemeData::forTheme($this->theme); + ThemeData::flushCache(); + } + + public function testForThemeIsCachedAcrossRequests() + { + $this->primeCache(); + + $queries = $this->countQueries(function () { + ThemeData::forTheme($this->theme); + }); + + $this->assertEquals(0, $queries, 'Theme data should be served from the cache'); + } + + public function testForThemeCreatesRecordOnlyOnce() + { + ThemeData::forTheme($this->theme); + ThemeData::flushCache(); + ThemeData::forTheme($this->theme); + ThemeData::flushCache(); + ThemeData::forTheme($this->theme); + + $this->assertEquals(1, ThemeData::where('theme', 'themedata')->count()); + } + + public function testDefaultValuesAreAppliedToCachedRecords() + { + $this->primeCache(); + + // Defaults are applied by afterFetch(), which must still run for cached rows + $this->assertEquals('Winter', ThemeData::forTheme($this->theme)->site_name); + } + + public function testAfterSaveInvalidatesCache() + { + $this->primeCache(); + + $themeData = ThemeData::forTheme($this->theme); + $themeData->site_name = 'Updated'; + $themeData->save(); + + // Only drop the in memory cache; the persistent cache must have been invalidated + // by afterSave(), otherwise theme customizations would not take effect + ThemeData::flushCache(); + + $this->assertEquals('Updated', ThemeData::forTheme($this->theme)->site_name); + } + + public function testAfterDeleteInvalidatesCache() + { + $this->primeCache(); + + $themeData = ThemeData::forTheme($this->theme); + $themeData->site_name = 'Updated'; + $themeData->save(); + + $this->theme->removeCustomData(); + ThemeData::flushCache(); + + // A new record should be created rather than the deleted one being served + $this->assertNull(ThemeData::forTheme($this->theme)->site_name); + $this->assertEquals(1, ThemeData::where('theme', 'themedata')->count()); + } + + public function testDynamicAttributesSurviveTheCache() + { + $themeData = ThemeData::forTheme($this->theme); + $themeData->site_name = 'From cache'; + $themeData->save(); + + ThemeData::flushCache(); + ThemeData::forTheme($this->theme); + ThemeData::flushCache(); + + $queries = $this->countQueries(function () { + // Dynamic attributes live in the jsonable `data` column and are expanded by + // afterFetch(), which runs during hydration rather than during the query + $this->assertEquals('From cache', ThemeData::forTheme($this->theme)->site_name); + }); + + $this->assertEquals(0, $queries); + } +} diff --git a/modules/system/database/migrations/2026_08_09_000032_Db_System_Request_Logs_Indexes.php b/modules/system/database/migrations/2026_08_09_000032_Db_System_Request_Logs_Indexes.php new file mode 100644 index 0000000000..5b5db085b0 --- /dev/null +++ b/modules/system/database/migrations/2026_08_09_000032_Db_System_Request_Logs_Indexes.php @@ -0,0 +1,18 @@ +index(['url', 'status_code']); + }); + } + + public function down() + { + Schema::table('system_request_logs', function ($table) { + $table->dropIndex(['url', 'status_code']); + }); + } +}; diff --git a/modules/system/tests/models/RequestLogTest.php b/modules/system/tests/models/RequestLogTest.php new file mode 100644 index 0000000000..5d37b5d3b7 --- /dev/null +++ b/modules/system/tests/models/RequestLogTest.php @@ -0,0 +1,91 @@ +getDriverName()) { + case 'sqlite': + $indexes = $connection->select('PRAGMA index_list(' . $connection->getTablePrefix() . $table . ')'); + return array_column(array_map(fn ($row) => (array) $row, $indexes), 'name'); + + case 'mysql': + $indexes = $connection->select('SHOW INDEX FROM ' . $connection->getTablePrefix() . $table); + return array_column(array_map(fn ($row) => (array) $row, $indexes), 'Key_name'); + + case 'pgsql': + $indexes = $connection->select( + 'SELECT indexname AS name FROM pg_indexes WHERE tablename = ?', + [$connection->getTablePrefix() . $table] + ); + return array_column(array_map(fn ($row) => (array) $row, $indexes), 'name'); + } + + $this->markTestSkipped('Unsupported database driver: ' . $connection->getDriverName()); + } + + public function testUrlAndStatusCodeAreIndexed() + { + // Without this index every logged request performs a full table scan of a table + // that only ever grows + $this->assertContains( + 'system_request_logs_url_status_code_index', + $this->getIndexNames('system_request_logs') + ); + } + + public function testAddCreatesRecord() + { + $record = RequestLog::add(404); + + $this->assertNotNull($record); + $this->assertEquals(1, $record->count); + $this->assertEquals(404, $record->status_code); + $this->assertEquals(1, RequestLog::count()); + } + + public function testAddIncrementsExistingRecord() + { + RequestLog::add(404); + RequestLog::add(404); + RequestLog::add(404); + + $this->assertEquals(1, RequestLog::count()); + $this->assertEquals(3, RequestLog::first()->count); + } + + public function testAddSeparatesStatusCodes() + { + RequestLog::add(404); + RequestLog::add(500); + + $this->assertEquals(2, RequestLog::count()); + } + + public function testAddRespectsLogRequestsSetting() + { + LogSetting::set('log_requests', false); + + $this->assertNull(RequestLog::add(404)); + $this->assertEquals(0, RequestLog::count()); + } +}