From 21c9b682fcf97e16cec1512cf724757570800e13 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 6 Oct 2022 14:59:48 +0100 Subject: [PATCH 01/32] Added initial support for allowing a theme to extend a parent theme --- modules/cms/ServiceProvider.php | 1 + modules/cms/classes/Theme.php | 66 +++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index 7d8e6fd6eb..b5ceefa83f 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -448,6 +448,7 @@ protected function registerHalcyonModels() { Event::listen('system.console.theme.sync.getAvailableModelClasses', function () { return [ + Classes\Theme::class, Classes\Meta::class, Classes\Page::class, Classes\Layout::class, diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index ab3b3226ad..819df92e37 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -26,7 +26,7 @@ * @package winter\wn-cms-module * @author Alexey Bobkov, Samuel Georges */ -class Theme +class Theme extends CmsObject { /** * @var string Specifies the theme directory name. @@ -48,6 +48,16 @@ class Theme */ protected static $editThemeCache = false; + /** + * @var array Allowable file extensions. + */ + protected $allowedExtensions = ['yaml']; + + /** + * @var string Default file extension. + */ + protected $defaultExtension = 'yaml'; + const ACTIVE_KEY = 'cms::theme.active'; const EDIT_KEY = 'cms::theme.edit'; @@ -55,7 +65,7 @@ class Theme * Loads the theme. * @return self */ - public static function load($dirName) + public static function load($dirName, $file = null) { $theme = new static; $theme->setDirName($dirName); @@ -216,9 +226,9 @@ public static function getActiveTheme() $theme = static::load(static::getActiveThemeCode()); - if (!File::isDirectory($theme->getPath())) { - return self::$activeThemeCache = null; - } +// if (!File::isDirectory($theme->getPath())) { +// return self::$activeThemeCache = null; +// } return self::$activeThemeCache = $theme; } @@ -300,9 +310,9 @@ public static function getEditTheme() $theme = static::load(static::getEditThemeCode()); - if (!File::isDirectory($theme->getPath())) { - return self::$editThemeCache = null; - } +// if (!File::isDirectory($theme->getPath())) { +// return self::$editThemeCache = null; +// } return self::$editThemeCache = $theme; } @@ -340,12 +350,21 @@ public function getConfig() return $this->configCache; } - $path = $this->getPath().'/theme.yaml'; - if (!File::exists($path)) { + $sources = [ + 'filesystem' => new FileDatasource(themes_path(), App::make('files')) + ]; + + if (static::databaseLayerEnabled()) { + $sources['database'] = new DbDatasource($this->getDirName(), 'cms_theme_templates'); + } + + $data = (new AutoDatasource($sources))->selectOne($this->getDirName(), 'theme', 'yaml'); + + if (!$data) { return $this->configCache = []; } - $config = Yaml::parseFile($path); + $config = Yaml::parse($data['content']); /** * @event cms.theme.extendConfig @@ -551,18 +570,27 @@ public function registerHalcyonDatasource() { $resolver = App::make('halcyon'); - if (!$resolver->hasDatasource($this->dirName)) { + if ($resolver->hasDatasource($this->dirName)) { + return; + } + + $sources = []; + if (static::databaseLayerEnabled()) { + $sources['database'] = new DbDatasource($this->dirName, 'cms_theme_templates'); + } + + $sources['filesystem'] = new FileDatasource($this->getPath(), App::make('files')); + + $config = $this->getConfig(); + if (!empty($config['parent'])) { if (static::databaseLayerEnabled()) { - $datasource = new AutoDatasource([ - 'database' => new DbDatasource($this->dirName, 'cms_theme_templates'), - 'filesystem' => new FileDatasource($this->getPath(), App::make('files')), - ]); - } else { - $datasource = new FileDatasource($this->getPath(), App::make('files')); + $sources['parent-database'] = new DbDatasource($config['parent'], 'cms_theme_templates'); } - $resolver->addDatasource($this->dirName, $datasource); + $sources['parent-filesystem'] = new FileDatasource(themes_path($config['parent']), App::make('files')); } + + $resolver->addDatasource($this->dirName, new AutoDatasource($sources)); } /** From 0a4aeea8b8e44910eebfd2ff37e5ce15ba8c8a39 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 6 Oct 2022 15:48:32 +0100 Subject: [PATCH 02/32] Added an assetUrl helper for theme --- modules/cms/classes/Theme.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index 819df92e37..f5d2068c4d 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -421,6 +421,23 @@ public function getFormConfig() return $config; } + public function assetUrl(?string $path): string + { + $config = $this->getConfig(); + + $themeDir = $this->getDirName(); + if (!File::isDirectory(themes_path($this->getDirName())) && !empty($config['parent'])) { + $themeDir = $config['parent']; + } + + $_url = Config::get('cms.themesPath', '/themes') . '/' . $themeDir; + if ($path !== null) { + $_url .= '/' . $path; + } + + return Url::asset($_url); + } + /** * Returns a value from the theme configuration file by its name. * @param string $name Specifies the configuration parameter name. From a95b14cdf754569f6388e5fb21f8ff36ad5f18bc Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 6 Oct 2022 15:49:02 +0100 Subject: [PATCH 03/32] Switched asset url generation to rely on theme assetUrl method --- modules/cms/classes/Controller.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index 903f512433..3b41835bb9 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -1356,11 +1356,7 @@ public function themeUrl($url = null) $_url = Url::to(CombineAssets::combine($url, themes_path().'/'.$themeDir)); } else { - $_url = Config::get('cms.themesPath', '/themes').'/'.$themeDir; - if ($url !== null) { - $_url .= '/'.$url; - } - $_url = Url::asset($_url); + $_url = $this->getTheme()->assetUrl($url); } return $_url; From c49562e7596212c22e319c0597bf5270bea08e81 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 6 Oct 2022 15:49:30 +0100 Subject: [PATCH 04/32] Added early exit if theme directory not exists --- modules/cms/widgets/AssetList.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/modules/cms/widgets/AssetList.php b/modules/cms/widgets/AssetList.php index 5cc92e1314..b385b9cc50 100644 --- a/modules/cms/widgets/AssetList.php +++ b/modules/cms/widgets/AssetList.php @@ -405,6 +405,11 @@ protected function getData() { $assetsPath = $this->getAssetsPath(); + // theme dir does not exist + if (!is_dir(dirname($assetsPath))) { + return []; + } + if (!file_exists($assetsPath) || !is_dir($assetsPath)) { if (!File::makeDirectory($assetsPath)) { throw new ApplicationException(Lang::get( From ef95051b036e941d6acd459898ea4982ffb90add Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 6 Oct 2022 18:02:47 +0100 Subject: [PATCH 05/32] Added fix for loading items at the top of the filesystem --- modules/cms/classes/AutoDatasource.php | 2 +- modules/cms/classes/Theme.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/cms/classes/AutoDatasource.php b/modules/cms/classes/AutoDatasource.php index 3b9aae08e5..1307139aab 100644 --- a/modules/cms/classes/AutoDatasource.php +++ b/modules/cms/classes/AutoDatasource.php @@ -309,7 +309,7 @@ protected function getValidPaths(string $dirName, array $options = []) */ protected function makeFilePath(string $dirName, string $fileName, string $extension) { - return $dirName . '/' . $fileName . '.' . $extension; + return ltrim($dirName . '/' . $fileName . '.' . $extension, '/'); } /** diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index f5d2068c4d..cc426f9f8d 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -351,14 +351,14 @@ public function getConfig() } $sources = [ - 'filesystem' => new FileDatasource(themes_path(), App::make('files')) + 'filesystem' => new FileDatasource(themes_path($this->getDirName()), App::make('files')) ]; if (static::databaseLayerEnabled()) { $sources['database'] = new DbDatasource($this->getDirName(), 'cms_theme_templates'); } - $data = (new AutoDatasource($sources))->selectOne($this->getDirName(), 'theme', 'yaml'); + $data = (new AutoDatasource($sources))->selectOne('', 'theme', 'yaml'); if (!$data) { return $this->configCache = []; From 1bb81ac8895f891c84772cee9fa310884b0975b2 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Fri, 7 Oct 2022 17:17:54 +0100 Subject: [PATCH 06/32] Switched order of execution --- modules/cms/tests/classes/CmsObjectTest.php | 46 ++++++++++----------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/modules/cms/tests/classes/CmsObjectTest.php b/modules/cms/tests/classes/CmsObjectTest.php index 9d3f3b4b99..61bacb711a 100644 --- a/modules/cms/tests/classes/CmsObjectTest.php +++ b/modules/cms/tests/classes/CmsObjectTest.php @@ -265,6 +265,29 @@ public function testRename() $this->assertEquals($testContents, file_get_contents($destFilePath)); } + /** + * @depends testRename + */ + public function testSaveSameName() + { + $theme = Theme::load('apitest'); + + $filePath = $theme->getPath() . '/testobjects/anotherobj.htm'; + $this->assertFileExists($filePath); + + $testContents = 'new content'; + $obj = TestCmsObject::load($theme, 'anotherobj.htm'); + + $obj->fill([ + 'fileName' => 'anotherobj', + 'content' => $testContents + ]); + $obj->save(); + + $this->assertFileExists($filePath); + $this->assertEquals($testContents, file_get_contents($filePath)); + } + /** * @depends testRename */ @@ -289,29 +312,6 @@ public function testRenameToExistingFile() $obj->save(); } - /** - * @depends testRename - */ - public function testSaveSameName() - { - $theme = Theme::load('apitest'); - - $filePath = $theme->getPath() . '/testobjects/anotherobj.htm'; - $this->assertFileExists($filePath); - - $testContents = 'new content'; - $obj = TestCmsObject::load($theme, 'anotherobj.htm'); - - $obj->fill([ - 'fileName' => 'anotherobj', - 'content' => $testContents - ]); - $obj->save(); - - $this->assertFileExists($filePath); - $this->assertEquals($testContents, file_get_contents($filePath)); - } - public function testSaveNewDir() { $theme = Theme::load('apitest'); From 4a71c759a215ba8853b2428d4f9065437ebc1c7b Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 10 Oct 2022 16:12:28 -0600 Subject: [PATCH 07/32] Support child themes referencing their parent theme's localization strings --- modules/cms/ServiceProvider.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index b5ceefa83f..d6e802d743 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -394,6 +394,19 @@ protected function bootBackendLocalization() $langPath = $theme->getPath() . '/lang'; + while ( + !is_null($langPath) + && !File::isDirectory($langPath) + ) { + $config = $theme->getConfig(); + if (empty($config['parent'])) { + $langPath = null; + break; + } + $theme = CmsTheme::load($config['parent']); + $langPath = $theme->getPath() . '/lang'; + } + if (File::isDirectory($langPath)) { Lang::addNamespace('themes.' . $theme->getId(), $langPath); } From 991717a96f5db68b5370466c45916b35bc0df65b Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 18 Oct 2022 21:45:43 -0600 Subject: [PATCH 08/32] Add initial support for child themes referencing combined assets in parent themes --- modules/cms/classes/Controller.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index 3b41835bb9..af3d370136 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -1351,9 +1351,19 @@ public function currentPageUrl($parameters = [], $routePersistence = true) public function themeUrl($url = null) { $themeDir = $this->getTheme()->getDirName(); + $parentTheme = $this->getTheme()->getConfig()['parent'] ?? false; if (is_array($url)) { - $_url = Url::to(CombineAssets::combine($url, themes_path().'/'.$themeDir)); + try { + $combiner = CombineAssets::combine($url, themes_path() . '/' . $themeDir); + } catch (\Exception $ex) { + // @TODO: Find a nicer way to handle this to allow for replacing individual files + if ($parentTheme) { + $combiner = CombineAssets::combine($url, themes_path() . '/' . $parentTheme); + } + } + + $_url = Url::to($combiner); } else { $_url = $this->getTheme()->assetUrl($url); From 527fa69a6ebaa92cddd97619a09d8ad1fcd3a308 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 18 Oct 2022 22:16:40 -0600 Subject: [PATCH 09/32] Avoid cache conflicts when running multiple themes Instead of using 4 characters split across two folders for avoiding cache conflicts just use the relative path to the file directly. Also makes it easier to find files in the cache when debugging --- modules/cms/classes/CodeParser.php | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/modules/cms/classes/CodeParser.php b/modules/cms/classes/CodeParser.php index 66a340b4d0..079f9601e3 100644 --- a/modules/cms/classes/CodeParser.php +++ b/modules/cms/classes/CodeParser.php @@ -5,6 +5,7 @@ use Cache; use Config; use SystemException; +use Winter\Storm\Support\Str; /** * Parses the PHP code section of CMS objects. @@ -233,18 +234,24 @@ protected function storeCachedInfo($result) /** * Returns path to the cached parsed file - * @return string */ - protected function getCacheFilePath() + protected function getCacheFilePath(): string { - $hash = md5($this->filePath); - $result = storage_path().'/cms/cache/'; - $result .= substr($hash, 0, 2).'/'; - $result .= substr($hash, 2, 2).'/'; - $result .= basename($this->filePath); - $result .= '.php'; + $pathSegments = [ + storage_path('cms' . DIRECTORY_SEPARATOR . 'cache'), + trim( + Str::after( + pathinfo($this->filePath, PATHINFO_DIRNAME), + base_path() + ), + DIRECTORY_SEPARATOR + ), + basename($this->filePath) . '.php', + ]; - return $result; + dd(implode(DIRECTORY_SEPARATOR, $pathSegments)); + + return implode(DIRECTORY_SEPARATOR, $pathSegments); } /** From a18e3ed112c3bfb98272d6dfc3c7c3b9de3192b2 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 18 Oct 2022 22:17:11 -0600 Subject: [PATCH 10/32] Remove unintentional debugging statement --- modules/cms/classes/CodeParser.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/cms/classes/CodeParser.php b/modules/cms/classes/CodeParser.php index 079f9601e3..5bb672133a 100644 --- a/modules/cms/classes/CodeParser.php +++ b/modules/cms/classes/CodeParser.php @@ -249,8 +249,6 @@ protected function getCacheFilePath(): string basename($this->filePath) . '.php', ]; - dd(implode(DIRECTORY_SEPARATOR, $pathSegments)); - return implode(DIRECTORY_SEPARATOR, $pathSegments); } From 0a9f21d4bf0d6b4ef844cf8d7d45c70e086e0daf Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 18 Oct 2022 22:20:47 -0600 Subject: [PATCH 11/32] Avoid Twig cache conflicts when running multiple themes Related: https://github.com/wintercms/winter/commit/a18e3ed112c3bfb98272d6dfc3c7c3b9de3192b2 --- modules/cms/ServiceProvider.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index d6e802d743..209a69374f 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -110,7 +110,11 @@ protected function registerTwigParser() if ($useCache) { $options['cache'] = new TwigCacheFilesystem( - storage_path().'/cms/twig', + storage_path(implode(DIRECTORY_SEPARATOR, [ + 'cms', + 'twig', + Config::get('cms.activeTheme') + ])) . DIRECTORY_SEPARATOR, $forceBytecode ? TwigCacheFilesystem::FORCE_BYTECODE_INVALIDATION : 0 ); } From 222b60b29b53d67be9e4f82d4653ce4efc52f5c8 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson <31214002+jaxwilko@users.noreply.github.com> Date: Wed, 19 Oct 2022 09:42:49 +0100 Subject: [PATCH 12/32] Added file detection allowing per file based theme overrides --- modules/cms/classes/Controller.php | 51 ++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index af3d370136..1e7dd4467d 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -4,8 +4,10 @@ use Url; use App; use View; +use File; use Lang; use Flash; +use Cache; use Config; use Session; use Request; @@ -1348,28 +1350,51 @@ public function currentPageUrl($parameters = [], $routePersistence = true) * @param mixed $url Specifies the theme-relative URL. If null, the theme path is returned. * @return string */ - public function themeUrl($url = null) + public function themeUrl($url = null): string + { + return is_array($url) + ? $this->themeUrlArray($url) + : $this->getTheme()->assetUrl($url); + } + + protected function themeUrlArray(array $url): string { $themeDir = $this->getTheme()->getDirName(); $parentTheme = $this->getTheme()->getConfig()['parent'] ?? false; - if (is_array($url)) { - try { - $combiner = CombineAssets::combine($url, themes_path() . '/' . $themeDir); - } catch (\Exception $ex) { - // @TODO: Find a nicer way to handle this to allow for replacing individual files - if ($parentTheme) { - $combiner = CombineAssets::combine($url, themes_path() . '/' . $parentTheme); + $cacheKey = __METHOD__ . '.' . md5(json_encode($url)); + + if (!($assets = Cache::get($cacheKey))) { + $assets = []; + $sources = [ + themes_path($themeDir) + ]; + + if ($parentTheme) { + $sources[] = themes_path($parentTheme); + } + + foreach ($url as $file) { + if (str_starts_with($file, '@')) { + $assets[] = $file; + continue; + } + + foreach ($sources as $source) { + $asset = $source . DIRECTORY_SEPARATOR . $file; + if (File::exists($asset)) { + $assets[] = $asset; + break 2; + } } + + throw new \Exception('Theme URL File not found: ' . $file); } - $_url = Url::to($combiner); - } - else { - $_url = $this->getTheme()->assetUrl($url); + Cache::put($cacheKey, $assets); } - return $_url; + return Url::to(CombineAssets::combine($assets)); } /** From 966edea734a0e330f19ebe2547a1332ed39cb907 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 24 Oct 2022 12:10:13 -0600 Subject: [PATCH 13/32] Set pivot data when initially syncing the relationship Also performs the sync with model instances rather than just raw IDs to support relationships with a custom relatedKey set (the key on the related model's table that is stored on the pivot table to connect the relationship, normally just the primary key but can be anything). --- modules/backend/behaviors/RelationController.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/backend/behaviors/RelationController.php b/modules/backend/behaviors/RelationController.php index 9e0f221ed8..899a36b0fd 100644 --- a/modules/backend/behaviors/RelationController.php +++ b/modules/backend/behaviors/RelationController.php @@ -1392,14 +1392,15 @@ public function onRelationManagePivotCreate() * Add the checked IDs to the pivot table */ $foreignIds = (array) $this->foreignId; - $this->relationObject->sync($foreignIds, false); + $saveData = $this->pivotWidget->getSaveData(); + $foreignModels = $this->relationModel->whereIn($this->relationModel->getKeyName(), $foreignIds)->get(); + $this->relationObject->syncWithPivotValues($foreignModels, $saveData['pivot'] ?? []); /* * Save data to models */ $foreignKeyName = $this->relationModel->getQualifiedKeyName(); $hydratedModels = $this->relationObject->whereIn($foreignKeyName, $foreignIds)->get(); - $saveData = $this->pivotWidget->getSaveData(); foreach ($hydratedModels as $hydratedModel) { $modelsToSave = $this->prepareModelsToSave($hydratedModel, $saveData); From 2b140f449c56dae54571b50cbf7cba20d2bac864 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 26 Oct 2022 02:28:35 -0600 Subject: [PATCH 14/32] Improve databaseLayerEnabled() check --- modules/cms/classes/Theme.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index cc426f9f8d..10c8830776 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -8,6 +8,7 @@ use Cache; use Event; use Config; +use Schema; use Exception; use SystemException; use DirectoryIterator; @@ -576,7 +577,14 @@ public static function databaseLayerEnabled() $enableDbLayer = !Config::get('app.debug', false); } - return $enableDbLayer && App::hasDatabase(); + $key = 'cms.databaseTemplates.hasTables'; + $hasDb = Cache::get($key, null); + if (is_null($hasDb)) { + $hasDb = (bool) App::hasDatabase() && Schema::hasTable('cms_theme_templates'); + Cache::rememberForever($key, function () use ($hasDb) { return $hasDb; }); + } + + return $enableDbLayer && $hasDb; } /** From 3dccee54c78947594e0c1c52ecc67f2a987bcf92 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 26 Oct 2022 02:29:38 -0600 Subject: [PATCH 15/32] Coding style fix --- modules/cms/classes/Theme.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index 10c8830776..aa8c3c52b9 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -581,7 +581,9 @@ public static function databaseLayerEnabled() $hasDb = Cache::get($key, null); if (is_null($hasDb)) { $hasDb = (bool) App::hasDatabase() && Schema::hasTable('cms_theme_templates'); - Cache::rememberForever($key, function () use ($hasDb) { return $hasDb; }); + Cache::rememberForever($key, function () use ($hasDb) { + return $hasDb; + }); } return $enableDbLayer && $hasDb; From 8a64b1ed489eca008d0bff5708506c81797dc734 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 26 Oct 2022 15:40:56 -0600 Subject: [PATCH 16/32] Include parent theme in twig caching path --- modules/cms/ServiceProvider.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index 209a69374f..122a443c52 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -13,6 +13,7 @@ use Cms\Models\ThemeData; use Cms\Classes\CmsObject; use Backend\Models\UserRole; +use Cms\Classes\Theme; use Cms\Classes\Page as CmsPage; use Cms\Classes\ComponentManager; use System\Classes\CombineAssets; @@ -109,11 +110,17 @@ protected function registerTwigParser() ]; if ($useCache) { + $theme = Theme::getActiveTheme(); + $themeDir = $theme->getDirName(); + if ($parent = $theme->getConfig()['parent'] ?? false) { + $themeDir .= '-' . $parent; + } + $options['cache'] = new TwigCacheFilesystem( storage_path(implode(DIRECTORY_SEPARATOR, [ 'cms', 'twig', - Config::get('cms.activeTheme') + $themeDir, ])) . DIRECTORY_SEPARATOR, $forceBytecode ? TwigCacheFilesystem::FORCE_BYTECODE_INVALIDATION : 0 ); From 8ecf401a48fe3f5a1e4a75f5729bac28fdfad814 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 3 Nov 2022 15:51:05 +0000 Subject: [PATCH 17/32] Moved backend localzation into theme activation to allow support for loading on theme activation instead of during service provider registration --- modules/cms/ServiceProvider.php | 35 --------------------------------- modules/cms/classes/Theme.php | 26 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 35 deletions(-) diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index 122a443c52..57eb99b911 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -68,10 +68,6 @@ public function boot() $this->bootMenuItemEvents(); $this->bootRichEditorEvents(); - - if (App::runningInBackend()) { - $this->bootBackendLocalization(); - } } /** @@ -392,37 +388,6 @@ protected function registerBackendSettings() }); } - /** - * Boots localization from an active theme for backend items. - */ - protected function bootBackendLocalization() - { - $theme = CmsTheme::getActiveTheme(); - - if (is_null($theme)) { - return; - } - - $langPath = $theme->getPath() . '/lang'; - - while ( - !is_null($langPath) - && !File::isDirectory($langPath) - ) { - $config = $theme->getConfig(); - if (empty($config['parent'])) { - $langPath = null; - break; - } - $theme = CmsTheme::load($config['parent']); - $langPath = $theme->getPath() . '/lang'; - } - - if (File::isDirectory($langPath)) { - Lang::addNamespace('themes.' . $theme->getId(), $langPath); - } - } - /** * Registers events for menu items. */ diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index aa8c3c52b9..2379e6cc19 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -245,6 +245,11 @@ public static function setActiveTheme($code) Parameter::set(self::ACTIVE_KEY, $code); + if (App::runningInBackend()) { + // Load theme localization + static::registerThemeBackendLocalization(); + } + /** * @event cms.theme.setActiveTheme * Fires when the active theme has been changed. @@ -260,6 +265,27 @@ public static function setActiveTheme($code) Event::fire('cms.theme.setActiveTheme', compact('code')); } + public static function registerThemeBackendLocalization(): void + { + $theme = static::getActiveTheme(); + + $langPath = $theme->getPath() . '/lang'; + + while (!File::isDirectory($langPath)) { + $config = $theme->getConfig(); + if (empty($config['parent'])) { + $langPath = null; + break; + } + $theme = static::load($config['parent']); + $langPath = $theme->getPath() . '/lang'; + } + + if (File::isDirectory($langPath)) { + Lang::addNamespace('themes.' . $theme->getId(), $langPath); + } + } + /** * Returns the edit theme code. * By default the edit theme is loaded from the cms.editTheme parameter, From 8cf656d3bebf72def3b937390735a7de688a9ce0 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 15 Nov 2022 00:36:07 -0600 Subject: [PATCH 18/32] Improve performance of active theme resolution, add type hinting This builds on the work originally done in https://github.com/octobercms/october/pull/3220 to further improve performance, especially in cases where lots of themes are present to choose from or "virtual" themes are being utilized. Previously every single request would iterate over and initialize every single theme which involved booting an autodatasource for each theme and loading the entire contents of the path cache for each datasource of each theme into memory. This commit changes the logic so that the cms.theme.getActiveTheme event will be run first, falling back to asking the cache service, then falling back to the DB if present, and finally defaulting to the value set in the configuration. After it has resolved an active theme it will attempt to cache that resolution permanently which should be fine since changing the active theme in the DB would trigger a cache invalidation when done through the setActiveTheme() method and changing the active theme in the file configuration would trigger a cache invalidation as well. --- modules/cms/classes/CmsObject.php | 2 +- modules/cms/classes/Theme.php | 268 +++++++++++++----------------- 2 files changed, 121 insertions(+), 149 deletions(-) diff --git a/modules/cms/classes/CmsObject.php b/modules/cms/classes/CmsObject.php index 961907f9c0..c2600a505a 100644 --- a/modules/cms/classes/CmsObject.php +++ b/modules/cms/classes/CmsObject.php @@ -124,7 +124,7 @@ public static function loadCached($theme, $fileName) * This method is used internally by the system. * @param \Cms\Classes\Theme $theme Specifies a parent theme. * @param boolean $skipCache Indicates if objects should be reloaded from the disk bypassing the cache. - * @return Collection Returns a collection of CMS objects. + * @return CmsObjectCollection Returns a collection of CMS objects. */ public static function listInTheme($theme, $skipCache = false) { diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index 2379e6cc19..6427752e2d 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -66,21 +66,22 @@ class Theme extends CmsObject * Loads the theme. * @return self */ - public static function load($dirName, $file = null) + public static function load($dirName, $file = null): self { $theme = new static; $theme->setDirName($dirName); $theme->registerHalcyonDatasource(); + if (App::runningInBackend()) { + $theme->registerBackendLocalization(); + } return $theme; } /** * Returns the absolute theme path. - * @param string $dirName Optional theme directory. Defaults to $this->getDirName() - * @return string */ - public function getPath($dirName = null) + public function getPath(?string $dirName = null): string { if (!$dirName) { $dirName = $this->getDirName(); @@ -91,18 +92,16 @@ public function getPath($dirName = null) /** * Sets the theme directory name. - * @return void */ - public function setDirName($dirName) + public function setDirName(string $dirName): void { $this->dirName = $dirName; } /** * Returns the theme directory name. - * @return string */ - public function getDirName() + public function getDirName(): string { return $this->dirName; } @@ -110,19 +109,16 @@ public function getDirName() /** * Helper for {{ theme.id }} twig vars * Returns a unique string for this theme. - * @return string */ - public function getId() + public function getId(): string { return snake_case(str_replace('/', '-', $this->getDirName())); } /** * Determines if a theme with given directory name exists - * @param string $dirName The theme directory - * @return bool */ - public static function exists($dirName) + public static function exists(string $dirName): bool { $theme = static::load($dirName); $path = $theme->getPath(); @@ -133,10 +129,8 @@ public static function exists($dirName) /** * Returns a list of pages in the theme. * This method is used internally in the routing process and in the back-end UI. - * @param boolean $skipCache Indicates if the pages should be reloaded from the disk bypassing the cache. - * @return array Returns an array of \Cms\Classes\Page objects. */ - public function listPages($skipCache = false) + public function listPages(bool $skipCache = false): \Cms\Classes\CmsObjectCollection { return Page::listInTheme($this, $skipCache); } @@ -144,51 +138,21 @@ public function listPages($skipCache = false) /** * Returns true if this theme is the chosen active theme. */ - public function isActiveTheme() + public function isActiveTheme(): bool { $activeTheme = self::getActiveTheme(); - return $activeTheme && $activeTheme->getDirName() == $this->getDirName(); + return $activeTheme && $activeTheme->getDirName() === $this->getDirName(); } /** * Returns the active theme code. * By default the active theme is loaded from the cms.activeTheme parameter, * but this behavior can be overridden by the cms.theme.getActiveTheme event listener. - * @return string * If the theme doesn't exist, returns null. */ - public static function getActiveThemeCode() + public static function getActiveThemeCode(): string { - $activeTheme = Config::get('cms.activeTheme'); - $themes = static::all(); - $havingMoreThemes = count($themes) > 1; - $themeHasChanged = !empty($themes[0]) && $themes[0]->dirName !== $activeTheme; - $checkDatabase = $havingMoreThemes || $themeHasChanged; - - if ($checkDatabase && App::hasDatabase()) { - try { - try { - $expiresAt = now()->addMinutes(1440); - $dbResult = Cache::remember(self::ACTIVE_KEY, $expiresAt, function () { - return Parameter::applyKey(self::ACTIVE_KEY)->value('value'); - }); - } - catch (Exception $ex) { - // Cache failed - $dbResult = Parameter::applyKey(self::ACTIVE_KEY)->value('value'); - } - } - catch (Exception $ex) { - // Database failed - $dbResult = null; - } - - if ($dbResult !== null && static::exists($dbResult)) { - $activeTheme = $dbResult; - } - } - /** * @event cms.theme.getActiveTheme * Overrides the active theme code. @@ -203,23 +167,61 @@ public static function getActiveThemeCode() */ $apiResult = Event::fire('cms.theme.getActiveTheme', [], true); if ($apiResult !== null) { - $activeTheme = $apiResult; + return $apiResult; + } + + // Load the active theme from the configuration + $activeTheme = $configuredTheme = Config::get('cms.activeTheme'); + + // Attempt to load the active theme from the cache before checking the database + try { + $cached = Cache::get(self::ACTIVE_KEY, null); + if ( + is_array($cached) + // Check if the configured theme has changed + && $cached['config'] === $configuredTheme + ) { + return $cached['active']; + } + } catch (Exception $ex) { + // Cache failed + } + + // Check the database + if (App::hasDatabase()) { + try { + $dbResult = Parameter::applyKey(self::ACTIVE_KEY)->value('value'); + } catch (Exception $ex) { + $dbResult = null; + } + + if ($dbResult !== null && static::exists($dbResult)) { + $activeTheme = $dbResult; + } } if (!strlen($activeTheme)) { throw new SystemException(Lang::get('cms::lang.theme.active.not_set')); } + // Cache the results + try { + Cache::forever(self::ACTIVE_KEY, [ + 'config' => $configuredTheme, + 'active' => $activeTheme, + ]); + } catch (Exception $ex) { + // Cache failed + } + return $activeTheme; } - /** * Returns the active theme object. - * @return \Cms\Classes\Theme Returns the loaded theme object. * If the theme doesn't exist, returns null. */ - public static function getActiveTheme() + public static function getActiveTheme(): self { if (self::$activeThemeCache !== false) { return self::$activeThemeCache; @@ -235,21 +237,15 @@ public static function getActiveTheme() } /** - * Sets the active theme. + * Sets the active theme in the database. * The active theme code is stored in the database and overrides the configuration cms.activeTheme parameter. - * @param string $code Specifies the active theme code. */ - public static function setActiveTheme($code) + public static function setActiveTheme(string $code): void { self::resetCache(); Parameter::set(self::ACTIVE_KEY, $code); - if (App::runningInBackend()) { - // Load theme localization - static::registerThemeBackendLocalization(); - } - /** * @event cms.theme.setActiveTheme * Fires when the active theme has been changed. @@ -265,42 +261,17 @@ public static function setActiveTheme($code) Event::fire('cms.theme.setActiveTheme', compact('code')); } - public static function registerThemeBackendLocalization(): void - { - $theme = static::getActiveTheme(); - - $langPath = $theme->getPath() . '/lang'; - - while (!File::isDirectory($langPath)) { - $config = $theme->getConfig(); - if (empty($config['parent'])) { - $langPath = null; - break; - } - $theme = static::load($config['parent']); - $langPath = $theme->getPath() . '/lang'; - } - - if (File::isDirectory($langPath)) { - Lang::addNamespace('themes.' . $theme->getId(), $langPath); - } - } - /** * Returns the edit theme code. * By default the edit theme is loaded from the cms.editTheme parameter, * but this behavior can be overridden by the cms.theme.getEditTheme event listeners. * If the edit theme is not defined in the configuration file, the active theme * is returned. - * @return string + * + * @throws SystemException if the edit theme cannot be determined */ - public static function getEditThemeCode() + public static function getEditThemeCode(): string { - $editTheme = Config::get('cms.editTheme'); - if (!$editTheme) { - $editTheme = static::getActiveThemeCode(); - } - /** * @event cms.theme.getEditTheme * Overrides the edit theme code. @@ -315,7 +286,12 @@ public static function getEditThemeCode() */ $apiResult = Event::fire('cms.theme.getEditTheme', [], true); if ($apiResult !== null) { - $editTheme = $apiResult; + return $apiResult; + } + + $editTheme = Config::get('cms.editTheme'); + if (!$editTheme) { + $editTheme = static::getActiveThemeCode(); } if (!strlen($editTheme)) { @@ -327,9 +303,8 @@ public static function getEditThemeCode() /** * Returns the edit theme. - * @return \Cms\Classes\Theme Returns the loaded theme object. */ - public static function getEditTheme() + public static function getEditTheme(): self { if (self::$editThemeCache !== false) { return self::$editThemeCache; @@ -345,10 +320,9 @@ public static function getEditTheme() } /** - * Returns a list of all themes. - * @return array Returns an array of the Theme objects. + * Returns an array of all themes. */ - public static function all() + public static function all(): array { $it = new DirectoryIterator(themes_path()); $it->rewind(); @@ -369,9 +343,8 @@ public static function all() /** * Reads the theme.yaml file and returns the theme configuration values. - * @return array Returns the parsed configuration file values. */ - public function getConfig() + public function getConfig(): array { if ($this->configCache !== null) { return $this->configCache; @@ -417,9 +390,8 @@ public function getConfig() * Themes have a dedicated `form` option that provide form fields * for customization, this is an immutable accessor for that and * also an solid anchor point for extension. - * @return array */ - public function getFormConfig() + public function getFormConfig(): array { $config = $this->getConfigArray('form'); @@ -448,6 +420,10 @@ public function getFormConfig() return $config; } + /** + * Generates an asset URL for the provided path within the theme, will use the parent theme + * if the current theme does not actually have a directory on the filesystem (i.e. is virtual). + */ public function assetUrl(?string $path): string { $config = $this->getConfig(); @@ -467,12 +443,8 @@ public function assetUrl(?string $path): string /** * Returns a value from the theme configuration file by its name. - * @param string $name Specifies the configuration parameter name. - * @param mixed $default Specifies the default value to return in case if the parameter - * doesn't exist in the configuration file. - * @return mixed Returns the parameter value or a default value */ - public function getConfigValue($name, $default = null) + public function getConfigValue(string $name, mixed $default = null): mixed { return array_get($this->getConfig(), $name, $default); } @@ -480,10 +452,8 @@ public function getConfigValue($name, $default = null) /** * Returns an array value from the theme configuration file by its name. * If the value is a string, it is treated as a YAML file and loaded. - * @param string $name Specifies the configuration parameter name. - * @return array */ - public function getConfigArray($name) + public function getConfigArray(string $name): array { $result = array_get($this->getConfig(), $name, []); @@ -509,11 +479,10 @@ public function getConfigArray($name) /** * Writes to the theme.yaml file with the supplied array values. - * @param array $values Data to write - * @param array $overwrite If true, undefined values are removed. - * @return void + * + * @throws ApplicationException if the theme.yaml file does not exist. */ - public function writeConfig($values = [], $overwrite = false) + public function writeConfig(array $values = [], bool $overwrite = false): void { if (!$overwrite) { $values = $values + (array) $this->getConfig(); @@ -521,7 +490,7 @@ public function writeConfig($values = [], $overwrite = false) $path = $this->getPath().'/theme.yaml'; if (!File::exists($path)) { - throw new ApplicationException('Path does not exist: '.$path); + throw new ApplicationException('Path does not exist: ' . $path); } $contents = Yaml::render($values); @@ -534,14 +503,13 @@ public function writeConfig($values = [], $overwrite = false) /** * Returns the theme preview image URL. * If the image file doesn't exist returns the placeholder image URL. - * @return string Returns the image URL. */ - public function getPreviewImageUrl() + public function getPreviewImageUrl(): string { $previewPath = $this->getConfigValue('previewImage', 'assets/images/theme-preview.png'); - if (File::exists($this->getPath().'/'.$previewPath)) { - return Url::asset('themes/'.$this->getDirName().'/'.$previewPath); + if (File::exists($this->getPath() . '/' . $previewPath)) { + return Url::asset('themes/' . $this->getDirName() . '/' . $previewPath); } return Url::asset('modules/cms/assets/images/default-theme-preview.png'); @@ -549,40 +517,38 @@ public function getPreviewImageUrl() /** * Resets any memory or cache involved with the active or edit theme. - * @return void */ - public static function resetCache() + public static function resetCache(bool $memoryOnly = false): void { self::$activeThemeCache = false; self::$editThemeCache = false; - Cache::forget(self::ACTIVE_KEY); - Cache::forget(self::EDIT_KEY); + if (!$memoryOnly) { + Cache::forget(self::ACTIVE_KEY); + Cache::forget(self::EDIT_KEY); + } } /** * Returns true if this theme has form fields that supply customization data. - * @return bool */ - public function hasCustomData() + public function hasCustomData(): bool { - return $this->getConfigValue('form', false); + return (bool) $this->getConfigValue('form', false); } /** * Returns data specific to this theme - * @return Cms\Models\ThemeData */ - public function getCustomData() + public function getCustomData(): ThemeData { return ThemeData::forTheme($this); } /** * Remove data specific to this theme - * @return bool */ - public function removeCustomData() + public function removeCustomData(): bool { if ($this->hasCustomData()) { return $this->getCustomData()->delete(); @@ -591,38 +557,50 @@ public function removeCustomData() return true; } + /** + * Register the backend localizations provided by this theme and its ancestors. + */ + public function registerBackendLocalization(): void + { + $langPath = $this->getPath() . '/lang'; + + if (File::isDirectory($langPath)) { + Lang::addNamespace($this->getDirName(), $langPath); + } + + // Check the parent theme if present + $config = $this->getConfig(); + if (!empty($config['parent'])) { + $langPath = themes_path($config['parent'] . '/lang'); + if (File::isDirectory($langPath)) { + Lang::addNamespace('themes.' . $config['parent'], $langPath); + } + } + } + /** * Checks to see if the database layer has been enabled - * - * @return boolean */ - public static function databaseLayerEnabled() + public static function databaseLayerEnabled(): bool { $enableDbLayer = Config::get('cms.databaseTemplates', false); if (is_null($enableDbLayer)) { $enableDbLayer = !Config::get('app.debug', false); } - $key = 'cms.databaseTemplates.hasTables'; - $hasDb = Cache::get($key, null); - if (is_null($hasDb)) { - $hasDb = (bool) App::hasDatabase() && Schema::hasTable('cms_theme_templates'); - Cache::rememberForever($key, function () use ($hasDb) { - return $hasDb; - }); - } + $hasDb = Cache::rememberForever('cms.databaseTemplates.hasTables', function () { + return App::hasDatabase() && Schema::hasTable('cms_theme_templates'); + }); return $enableDbLayer && $hasDb; } /** * Ensures this theme is registered as a Halcyon datasource. - * @return void */ - public function registerHalcyonDatasource() + public function registerHalcyonDatasource(): void { $resolver = App::make('halcyon'); - if ($resolver->hasDatasource($this->dirName)) { return; } @@ -648,10 +626,8 @@ public function registerHalcyonDatasource() /** * Get the theme's datasource - * - * @return DatasourceInterface */ - public function getDatasource() + public function getDatasource(): AutoDatasource { $resolver = App::make('halcyon'); return $resolver->datasource($this->getDirName()); @@ -659,8 +635,6 @@ public function getDatasource() /** * Implements the getter functionality. - * @param string $name - * @return void */ public function __get($name) { @@ -673,8 +647,6 @@ public function __get($name) /** * Determine if an attribute exists on the object. - * @param string $key - * @return void */ public function __isset($key) { From 3d77c7a03a51f2157ef0684cd0c10b3591d5b3ac Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 23 Nov 2022 23:12:50 -0600 Subject: [PATCH 19/32] Add ability to specify the cache key of the AutoDatasource --- modules/cms/classes/AutoDatasource.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/modules/cms/classes/AutoDatasource.php b/modules/cms/classes/AutoDatasource.php index 1307139aab..b0fc39fa56 100644 --- a/modules/cms/classes/AutoDatasource.php +++ b/modules/cms/classes/AutoDatasource.php @@ -22,6 +22,11 @@ class AutoDatasource extends Datasource implements DatasourceInterface */ protected $datasources = []; + /** + * @var string The cache key to use for this datasource instance + */ + protected $cacheKey = 'halcyon-datastore-auto'; + /** * @var array Local cache of paths available in the datasources */ @@ -48,10 +53,14 @@ class AutoDatasource extends Datasource implements DatasourceInterface * @param array $datasources Array of datasources to utilize. Lower indexes = higher priority ['datasourceName' => $datasource] * @return void */ - public function __construct(array $datasources) + public function __construct(array $datasources, ?string $cacheKey = null) { $this->datasources = $datasources; + if ($cacheKey) { + $this->cacheKey = $cacheKey; + } + $this->activeDatasourceKey = array_keys($datasources)[0]; $this->populateCache(); @@ -507,7 +516,7 @@ public function makeCacheKey($name = ''): string */ public function getPathsCacheKey(): string { - return 'halcyon-datastore-auto'; + return $this->cacheKey; } /** From 4f0da6cd0c3cbb53bba249ae58c928fdbe4d76df Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 23 Nov 2022 23:13:29 -0600 Subject: [PATCH 20/32] Set the AutoDatasource's cache key based on the current theme --- modules/cms/classes/Theme.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index 6427752e2d..eab2ae8a8e 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -621,7 +621,7 @@ public function registerHalcyonDatasource(): void $sources['parent-filesystem'] = new FileDatasource(themes_path($config['parent']), App::make('files')); } - $resolver->addDatasource($this->dirName, new AutoDatasource($sources)); + $resolver->addDatasource($this->dirName, new AutoDatasource($sources, 'halcyon-datasource-auto-' . $this->dirName)); } /** From 5540dadaafbbd3ec7504a4ad2bcec621976a181e Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Thu, 8 Dec 2022 18:08:59 +0000 Subject: [PATCH 21/32] Added fix to allow nested AutoDatasource instances to manage their own populateCache --- modules/cms/classes/AutoDatasource.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/modules/cms/classes/AutoDatasource.php b/modules/cms/classes/AutoDatasource.php index b0fc39fa56..c554b2f9ee 100644 --- a/modules/cms/classes/AutoDatasource.php +++ b/modules/cms/classes/AutoDatasource.php @@ -68,6 +68,14 @@ public function __construct(array $datasources, ?string $cacheKey = null) $this->postProcessor = new Processor; } + /** + * Returns the in memory path cache map + */ + public function getPathCache(): array + { + return $this->pathCache; + } + /** * Populate the local cache of paths available in each datasource * @@ -78,6 +86,13 @@ public function populateCache($refresh = false) { $pathCache = []; foreach ($this->datasources as $datasource) { + // Allow AutoDatasource instances to handle their own internal caching + if ($datasource instanceof AutoDatasource) { + $datasource->populateCache($refresh); + $pathCache[] = array_merge(...array_reverse($datasource->getPathCache())); + continue; + } + // Remove any existing cache data if ($refresh && $this->allowCacheRefreshes) { Cache::forget($datasource->getPathsCacheKey()); From 5414c6256a76ab600329c37405068d544aae03e3 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Fri, 24 Feb 2023 17:33:47 +0000 Subject: [PATCH 22/32] Added support to the theme filter to validate parent theme for assets --- modules/cms/classes/Theme.php | 43 +++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index eab2ae8a8e..c35762b9e4 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -426,19 +426,42 @@ public function getFormConfig(): array */ public function assetUrl(?string $path): string { - $config = $this->getConfig(); + $cacheTime = Config::get('cms.urlCacheTtl', 10); - $themeDir = $this->getDirName(); - if (!File::isDirectory(themes_path($this->getDirName())) && !empty($config['parent'])) { - $themeDir = $config['parent']; - } + return Cache::remember('winter.cms.assetUrl.' . $path, $cacheTime, function () use ($path) { + $config = $this->getConfig(); + $themeDir = $this->getDirName(); - $_url = Config::get('cms.themesPath', '/themes') . '/' . $themeDir; - if ($path !== null) { - $_url .= '/' . $path; - } + // If the active theme does not have a directory, then just check the parent theme + if (!File::isDirectory(themes_path($this->getDirName())) && !empty($config['parent'])) { + $themeDir = $config['parent']; + } + + // Define a helper for constructing the URL + $urlPath = function ($themeDir, $path) { + $_url = Config::get('cms.themesPath', '/themes') . '/' . $themeDir; + + if ($path !== null) { + $_url .= '/' . $path; + } - return Url::asset($_url); + return $_url; + }; + + $url = $urlPath($themeDir, $path); + + // If the file cannot be found in the theme, generate a url for the parent theme + if (!File::exists(base_path($url)) && !empty($config['parent']) && $themeDir !== $config['parent']) { + $parentUrl = $urlPath($config['parent'], $path); + // If found in the parent, return it + if (File::exists(base_path($parentUrl))) { + return Url::asset($parentUrl); + } + } + + // Default to returning the current theme's url + return Url::asset($url); + }); } /** From 1a8fb449a9f9989f19ce986b6e442c29cac03ece Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Mon, 27 Feb 2023 14:34:43 +0000 Subject: [PATCH 23/32] Added Carbon cache timer --- modules/cms/classes/Theme.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index c35762b9e4..b665ff5196 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -426,7 +426,7 @@ public function getFormConfig(): array */ public function assetUrl(?string $path): string { - $cacheTime = Config::get('cms.urlCacheTtl', 10); + $cacheTime = now()->addMinutes(Config::get('cms.urlCacheTtl', 10)); return Cache::remember('winter.cms.assetUrl.' . $path, $cacheTime, function () use ($path) { $config = $this->getConfig(); From 49d7ea080152d3b3045bca2177aae171cfe373df Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Mon, 27 Feb 2023 17:19:53 +0000 Subject: [PATCH 24/32] Removed whitespace --- modules/cms/classes/Theme.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index b665ff5196..f232225b9b 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -455,7 +455,7 @@ public function assetUrl(?string $path): string $parentUrl = $urlPath($config['parent'], $path); // If found in the parent, return it if (File::exists(base_path($parentUrl))) { - return Url::asset($parentUrl); + return Url::asset($parentUrl); } } From ad3e19eeebaa8b68f75efa413fa335c564078627 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 27 Feb 2023 11:26:40 -0600 Subject: [PATCH 25/32] Update modules/cms/classes/Controller.php --- modules/cms/classes/Controller.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index 0d39c7b8af..065b09955c 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -1358,11 +1358,14 @@ public function currentPageUrl($parameters = [], $routePersistence = true) public function themeUrl($url = null): string { return is_array($url) - ? $this->themeUrlArray($url) + ? $this->themeCombineAssets($url) : $this->getTheme()->assetUrl($url); } - protected function themeUrlArray(array $url): string + /** + * Generates a URL to the AssetCombiner for the provided array of assets + */ + protected function themeCombineAssets(array $url): string { $themeDir = $this->getTheme()->getDirName(); $parentTheme = $this->getTheme()->getConfig()['parent'] ?? false; From 02319de0ed58ef48eebd84469886c547ea00854c Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 27 Feb 2023 11:28:15 -0600 Subject: [PATCH 26/32] Update modules/cms/classes/Controller.php --- modules/cms/classes/Controller.php | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index 065b09955c..3e892c605d 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -1383,6 +1383,7 @@ protected function themeCombineAssets(array $url): string } foreach ($url as $file) { + // Leave Combiner Aliases assets unmodified if (str_starts_with($file, '@')) { $assets[] = $file; continue; From 3c393b74c161f1482ffb9dc59f7028be64c4cad3 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 27 Feb 2023 11:32:50 -0600 Subject: [PATCH 27/32] Update modules/cms/classes/Controller.php --- modules/cms/classes/Controller.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index 3e892c605d..5c18debe20 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -1397,7 +1397,9 @@ protected function themeCombineAssets(array $url): string } } - throw new \Exception('Theme URL File not found: ' . $file); + // Skip combining missing assets and log an error + Log::error("$file could not be found in any of the theme's sources (" . implode(', ', $sources) . ','); + continue; } Cache::put($cacheKey, $assets); From 85fe239da005a9b8024f619c0b4172f371e7ac06 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 27 Feb 2023 11:35:33 -0600 Subject: [PATCH 28/32] Update modules/cms/widgets/AssetList.php --- modules/cms/widgets/AssetList.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cms/widgets/AssetList.php b/modules/cms/widgets/AssetList.php index b385b9cc50..5a1ed709f7 100644 --- a/modules/cms/widgets/AssetList.php +++ b/modules/cms/widgets/AssetList.php @@ -405,7 +405,7 @@ protected function getData() { $assetsPath = $this->getAssetsPath(); - // theme dir does not exist + // theme dir does not exist (i.e. in a child theme without an assets directory if (!is_dir(dirname($assetsPath))) { return []; } From a9e2e01fb6941fcd68132c8e50648fff73e0e9da Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 27 Feb 2023 11:52:48 -0600 Subject: [PATCH 29/32] Update modules/cms/classes/Theme.php --- modules/cms/classes/Theme.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index f232225b9b..6a35d9d221 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -229,9 +229,6 @@ public static function getActiveTheme(): self $theme = static::load(static::getActiveThemeCode()); -// if (!File::isDirectory($theme->getPath())) { -// return self::$activeThemeCache = null; -// } return self::$activeThemeCache = $theme; } From c91fe4ff5cfa6fa829fb252706dd31b1e89b188f Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 27 Feb 2023 12:25:38 -0600 Subject: [PATCH 30/32] Apply suggestions from code review --- modules/cms/classes/Theme.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index 54b4512267..659f59b37f 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -309,9 +309,6 @@ public static function getEditTheme(): self $theme = static::load(static::getEditThemeCode()); -// if (!File::isDirectory($theme->getPath())) { -// return self::$editThemeCache = null; -// } return self::$editThemeCache = $theme; } @@ -347,14 +344,13 @@ public function getConfig(): array return $this->configCache; } + // Attempt to load the theme's config file from whatever datasources are available. $sources = [ 'filesystem' => new FileDatasource(themes_path($this->getDirName()), App::make('files')) ]; - if (static::databaseLayerEnabled()) { $sources['database'] = new DbDatasource($this->getDirName(), 'cms_theme_templates'); } - $data = (new AutoDatasource($sources))->selectOne('', 'theme', 'yaml'); if (!$data) { @@ -423,9 +419,8 @@ public function getFormConfig(): array */ public function assetUrl(?string $path): string { - $cacheTime = now()->addMinutes(Config::get('cms.urlCacheTtl', 10)); - - return Cache::remember('winter.cms.assetUrl.' . $path, $cacheTime, function () use ($path) { + $expiresAt = now()->addMinutes(Config::get('cms.urlCacheTtl', 10)); + return Cache::remember('winter.cms.assetUrl.' . $path, $expiresAt, function () use ($path) { $config = $this->getConfig(); $themeDir = $this->getDirName(); @@ -543,6 +538,7 @@ public static function resetCache(bool $memoryOnly = false): void self::$activeThemeCache = false; self::$editThemeCache = false; + // 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); Cache::forget(self::EDIT_KEY); @@ -641,7 +637,10 @@ public function registerHalcyonDatasource(): void $sources['parent-filesystem'] = new FileDatasource(themes_path($config['parent']), App::make('files')); } - $resolver->addDatasource($this->dirName, new AutoDatasource($sources, 'halcyon-datasource-auto-' . $this->dirName)); + $datasource = count($sources) > 1 + ? new AutoDatasource($sources, 'halcyon-datasource-auto-' . $this->dirName) + : array_shift($sources); + $resolver->addDatasource($this->dirName, $datasource); } /** From 263a1a3c00244c77a6fa2c625e43db95d2d37d22 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Mon, 27 Feb 2023 18:43:02 +0000 Subject: [PATCH 31/32] Removed undefined var and replaced with halycon registered event --- modules/cms/classes/Theme.php | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/modules/cms/classes/Theme.php b/modules/cms/classes/Theme.php index 659f59b37f..0b82fc227f 100644 --- a/modules/cms/classes/Theme.php +++ b/modules/cms/classes/Theme.php @@ -160,12 +160,12 @@ public static function getActiveThemeCode(): string * If a value is returned from this halting event, it will be used as the active * theme code. Example usage: * - * Event::listen('cms.theme.getActiveTheme', function (string $activeTheme) { + * Event::listen('cms.theme.getActiveTheme', function () { * return 'mytheme'; * }); * */ - $apiResult = Event::fire('cms.theme.getActiveTheme', [$activeTheme], true); + $apiResult = Event::fire('cms.theme.getActiveTheme', [], true); if ($apiResult !== null) { return $apiResult; } @@ -640,7 +640,27 @@ public function registerHalcyonDatasource(): void $datasource = count($sources) > 1 ? new AutoDatasource($sources, 'halcyon-datasource-auto-' . $this->dirName) : array_shift($sources); + $resolver->addDatasource($this->dirName, $datasource); + + /** + * @event cms.theme.registerHalcyonDatasource + * Fires immediately after the theme's Datasource has been registered. + * + * Allows for extension of the theme Halcyon Datasource, example usage: + * + * use Cms\Classes\Theme; + * use Winter\Storm\Halcyon\Datasource\Resolver; + * + * Event::listen('cms.theme.registerHalcyonDatasource', function (Theme $theme, Resolver $resolver) { + * $resolver->addDatasource($theme->getDirName(), new AutoDatasource([ + * 'theme' => $theme->getDatasource(), + * 'example' => new ExampleDatasource(), + * ], 'example-autodatasource')); + * }); + * + */ + Event::fire('cms.theme.registerHalcyonDatasource', [$this, $resolver]); } /** From af37464ea3ad7b8761deefb35c6718ec31e42ff7 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 1 Mar 2023 11:53:36 +0000 Subject: [PATCH 32/32] Added child theme tests --- modules/cms/tests/classes/ControllerTest.php | 45 +++++++++++++++++++ modules/cms/tests/classes/ThemeTest.php | 30 +++++++++++++ .../themes/childtest/assets/css/style2.css | 0 .../themes/childtest/content/a/a-content.htm | 1 + .../themes/childtest/partials/a/a-partial.htm | 1 + .../fixtures/themes/childtest/theme.yaml | 2 + 6 files changed, 79 insertions(+) create mode 100644 modules/cms/tests/fixtures/themes/childtest/assets/css/style2.css create mode 100644 modules/cms/tests/fixtures/themes/childtest/content/a/a-content.htm create mode 100644 modules/cms/tests/fixtures/themes/childtest/partials/a/a-partial.htm create mode 100644 modules/cms/tests/fixtures/themes/childtest/theme.yaml diff --git a/modules/cms/tests/classes/ControllerTest.php b/modules/cms/tests/classes/ControllerTest.php index af34603a0a..e888f37462 100644 --- a/modules/cms/tests/classes/ControllerTest.php +++ b/modules/cms/tests/classes/ControllerTest.php @@ -164,6 +164,17 @@ public function testPartials() $this->assertEquals('
LAYOUT PARTIAL

Hey PAGE PARTIAL Homer Simpson A partial

', $response); } + public function testChildThemePartials() + { + /* + * Test partials referred in the layout and page + */ + $theme = Theme::load('childtest'); + $controller = new Controller($theme); + $response = $controller->run('/with-partials')->getContent(); + $this->assertEquals('
LAYOUT PARTIAL

Hey PAGE PARTIAL Homer Simpson A child partial

', $response); + } + public function testContent() { $theme = Theme::load('test'); @@ -172,6 +183,14 @@ public function testContent() $this->assertEquals('
LAYOUT CONTENT

Hey PAGE CONTENT A content

', $response); } + public function testChildThemeContent() + { + $theme = Theme::load('childtest'); + $controller = new Controller($theme); + $response = $controller->run('/with-content')->getContent(); + $this->assertEquals('
LAYOUT CONTENT

Hey PAGE CONTENT A child content

', $response); + } + public function testBlocks() { $theme = Theme::load('test'); @@ -180,6 +199,14 @@ public function testBlocks() $this->assertEquals("
LAYOUT CONTENT BLOCK\n DEFAULT

Hey PAGE CONTENT

SECOND BLOCK", $response); } + public function testChildThemeBlocks() + { + $theme = Theme::load('childtest'); + $controller = new Controller($theme); + $response = $controller->run('/with-placeholder')->getContent(); + $this->assertEquals("
LAYOUT CONTENT BLOCK\n DEFAULT

Hey PAGE CONTENT

SECOND BLOCK", $response); + } + public function testLayoutInSubdirectory() { $theme = Theme::load('test'); @@ -198,6 +225,16 @@ public function testPartialNotFound() $response = $controller->run('/no-partial')->getContent(); } + public function testChildThemePartialNotFound() + { + $this->expectException(\Twig\Error\RuntimeError::class); + $this->expectExceptionMessageMatches('/is\snot\sfound/'); + + $theme = Theme::load('childtest'); + $controller = new Controller($theme); + $response = $controller->run('/no-partial')->getContent(); + } + public function testPageLifeCycle() { $theme = Theme::load('test'); @@ -206,6 +243,14 @@ public function testPageLifeCycle() $this->assertEquals('12345', $response); } + public function testChildThemePageLifeCycle() + { + $theme = Theme::load('childtest'); + $controller = new Controller($theme); + $response = $controller->run('/cycle-test')->getContent(); + $this->assertEquals('12345', $response); + } + protected function configAjaxRequestMock($handler, $partials = false) { $requestMock = $this diff --git a/modules/cms/tests/classes/ThemeTest.php b/modules/cms/tests/classes/ThemeTest.php index e5d5ab06cf..4b680e74e6 100644 --- a/modules/cms/tests/classes/ThemeTest.php +++ b/modules/cms/tests/classes/ThemeTest.php @@ -14,6 +14,8 @@ public function setUp(): void parent::setUp(); Config::set('cms.activeTheme', 'test'); + Config::set('cms.themesPath', '/modules/cms/tests/fixtures/themes'); + Event::flush('cms.theme.getActiveTheme'); Theme::resetCache(); } @@ -92,4 +94,32 @@ public function testApiTheme() $this->assertNotNull($activeTheme); $this->assertEquals('apitest', $activeTheme->getDirName()); } + + public function testChildThemeConfig() + { + Config::set('cms.activeTheme', 'childtest'); + + $theme = Theme::getActiveTheme(); + $config = $theme->getConfig(); + + $this->assertArrayHasKey('parent', $config); + $this->assertEquals('test', $config['parent']); + } + + public function testChildThemeAssetUrl() + { + Config::set('cms.activeTheme', 'childtest'); + + $theme = Theme::getActiveTheme(); + + $this->assertStringContainsString( + 'modules/cms/tests/fixtures/themes/test/assets/css/style1.css', + $theme->assetUrl('assets/css/style1.css') + ); + + $this->assertStringContainsString( + 'modules/cms/tests/fixtures/themes/childtest/assets/css/style2.css', + $theme->assetUrl('assets/css/style2.css') + ); + } } diff --git a/modules/cms/tests/fixtures/themes/childtest/assets/css/style2.css b/modules/cms/tests/fixtures/themes/childtest/assets/css/style2.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/modules/cms/tests/fixtures/themes/childtest/content/a/a-content.htm b/modules/cms/tests/fixtures/themes/childtest/content/a/a-content.htm new file mode 100644 index 0000000000..303d29df6b --- /dev/null +++ b/modules/cms/tests/fixtures/themes/childtest/content/a/a-content.htm @@ -0,0 +1 @@ +A child content diff --git a/modules/cms/tests/fixtures/themes/childtest/partials/a/a-partial.htm b/modules/cms/tests/fixtures/themes/childtest/partials/a/a-partial.htm new file mode 100644 index 0000000000..250ec27d13 --- /dev/null +++ b/modules/cms/tests/fixtures/themes/childtest/partials/a/a-partial.htm @@ -0,0 +1 @@ +A child partial diff --git a/modules/cms/tests/fixtures/themes/childtest/theme.yaml b/modules/cms/tests/fixtures/themes/childtest/theme.yaml new file mode 100644 index 0000000000..eb7364ccfd --- /dev/null +++ b/modules/cms/tests/fixtures/themes/childtest/theme.yaml @@ -0,0 +1,2 @@ +name: ChildTest +parent: test