Merge pull request #14559 from jdalsem/viewservicecache

chore(views): inject server cache directly in views service

docs/appendix/upgrade-notes/5.x-to-6.0.rst

@@ -49,3 +49,8 @@ Miscellaneous API changes
 -------------------------
 
 * The interface ``\Elgg\EntityIcon`` has been removed. Implemented functions in ``\ElggEntity`` have been moved to ``\Elgg\Traits\Entity\Icons``
+
+Removed Config values
+------------------------
+
+* ``system_cache_loaded``

engine/classes/Elgg/Application/SystemEventHandlers.php

@@ -117,7 +117,7 @@ public static function initLate() {
 	 * @return void
 	 */
 	public static function ready() {
-		_elgg_services()->systemCache->init();
+		_elgg_services()->views->cacheConfiguration();
 	}
 
 	/**

engine/classes/Elgg/BootService.php

@@ -79,9 +79,7 @@ public function boot(InternalContainer $services) {
 		$debug = $config->getInitialValue('debug') ?? ($config->debug ?: LogLevel::CRITICAL);
 		$services->logger->setLevel($debug);
 
-		if ($config->system_cache_enabled) {
-			$config->system_cache_loaded = $services->views->configureFromCache($services->serverCache);
-		}
+		$services->views->configureFromCache();
 	}
 
 	/**
@@ -92,7 +90,6 @@ public function boot(InternalContainer $services) {
 	public function clearCache() {
 		$this->cache->clear();
 		_elgg_services()->plugins->setBootPlugins(null);
-		_elgg_services()->config->system_cache_loaded = false;
 		_elgg_services()->config->_boot_cache_hit = false;
 	}
 

engine/classes/Elgg/Cache/SystemCache.php

@@ -15,20 +15,14 @@ class SystemCache {
 
 	use Cacheable;
 
-	/**
-	 * @var Config
-	 */
-	protected $config;
-
 	/**
 	 * Constructor
 	 *
 	 * @param BaseCache $cache  Elgg disk cache
 	 * @param Config    $config Elgg config
 	 */
-	public function __construct(BaseCache $cache, Config $config) {
+	public function __construct(BaseCache $cache, protected Config $config) {
 		$this->cache = $cache;
-		$this->config = $config;
 	}
 
 	/**
@@ -119,20 +113,4 @@ public function disable() {
 		$this->config->save('system_cache_enabled', 0);
 		$this->reset();
 	}
-
-	/**
-	 * Initializes the system cache
-	 *
-	 * @return void
-	 */
-	public function init() {
-		if (!$this->isEnabled()) {
-			return;
-		}
-
-		// cache system data if enabled and not loaded
-		if (!$this->config->system_cache_loaded) {
-			_elgg_services()->views->cacheConfiguration(_elgg_services()->serverCache);
-		}
-	}
 }

engine/classes/Elgg/Cache/ViewCacher.php

@@ -11,25 +11,13 @@
  */
 class ViewCacher {
 
-	/**
-	 * @var ViewsService
-	 */
-	private $views;
-
-	/**
-	 * @var Config
-	 */
-	private $config;
-
 	/**
 	 * Constructor
 	 *
 	 * @param ViewsService $views  Views
 	 * @param Config       $config Config
 	 */
-	public function __construct(ViewsService $views, Config $config) {
-		$this->views = $views;
-		$this->config = $config;
+	public function __construct(protected ViewsService $views, protected Config $config) {
 	}
 
 	/**
@@ -38,12 +26,12 @@ public function __construct(ViewsService $views, Config $config) {
 	 * @return void
 	 */
 	public function registerCoreViews() {
-		if ($this->config->system_cache_loaded) {
+		if ($this->views->isViewLocationsLoadedFromCache()) {
 			return;
 		}
 
 		// Core view files in /views
-		$this->views->registerPluginViews(Paths::elgg());
+		$this->views->registerViewsFromPath(Paths::elgg());
 
 		// Core view definitions in /engine/views.php
 		$file = Paths::elgg() . 'engine/views.php';
@@ -54,7 +42,7 @@ public function registerCoreViews() {
 		$spec = Includer::includeFile($file);
 		if (is_array($spec)) {
 			// check for uploaded fontawesome font
-			if (elgg_get_config('font_awesome_zip')) {
+			if ($this->config->font_awesome_zip) {
 				$spec['default']['font-awesome/'] = elgg_get_data_path() . 'fontawesome/webfont/';
 			}
 

engine/classes/Elgg/Config.php

@@ -121,7 +121,6 @@
  * @property string[]      $site_featured_menu_names
  * @property bool          $subresource_integrity_enabled			Should subresources (js/css) get integrity information
  * @property bool          $system_cache_enabled					Is the system cache enabled?
- * @property bool          $system_cache_loaded
  * @property bool          $testing_mode  							Is the current application running (PHPUnit) tests
  * @property string        $time_format  							Preferred PHP time format
  * @property bool          $user_joined_river						Do we need to create a river event when a user joins the site

engine/classes/Elgg/ViewsService.php

@@ -28,70 +28,54 @@ class ViewsService {
 	 * @see ViewsService::fileExists()
 	 * @var array
 	 */
-	protected $file_exists_cache = [];
+	protected array $file_exists_cache = [];
 
 	/**
 	 * @var array
 	 *
 	 * [viewtype][view] => '/path/to/views/style.css'
 	 */
-	private $locations = [];
+	protected array $locations = [];
 
 	/**
 	 * @var array Tracks location changes for views
 	 *
 	 * [viewtype][view][] => '/path/to/views/style.css'
 	 */
-	private $overrides = [];
+	protected array $overrides = [];
 
 	/**
 	 * @var array Simplecache views (view names are keys)
 	 *
 	 * [view] = true
 	 */
-	private $simplecache_views = [];
+	protected array $simplecache_views = [];
 
 	/**
 	 * @var array
 	 *
 	 * [view][priority] = extension_view
 	 */
-	private $extensions = [];
+	protected array $extensions = [];
 
 	/**
 	 * @var string[] A list of fallback viewtypes
 	 */
-	private $fallbacks = [];
+	protected array $fallbacks = [];
 
-	/**
-	 * @var EventsService
-	 */
-	private $events;
-
-	/**
-	 * @var SystemCache|null This is set if the views are configured via cache
-	 */
-	private $cache;
-
-	/**
-	 * @var \Elgg\Http\Request
-	 */
-	private $request;
-
-	/**
-	 * @var string
-	 */
-	private $viewtype;
+	protected ?string $viewtype;
+
+	protected bool $locations_loaded_from_cache = false;
 
 	/**
 	 * Constructor
 	 *
-	 * @param EventsService      $events  Events service
-	 * @param \Elgg\Http\Request $request Http Request
+	 * @param EventsService           $events       Events service
+	 * @param \Elgg\Http\Request      $request      Http Request
+	 * @param \Elgg\Config            $config       Elgg configuration
+	 * @param \Elgg\Cache\SystemCache $server_cache Server cache
 	 */
-	public function __construct(EventsService $events, HttpRequest $request) {
-		$this->events = $events;
-		$this->request = $request;
+	public function __construct(protected EventsService $events, protected HttpRequest $request, protected Config $config, protected SystemCache $server_cache) {
 	}
 
 	/**
@@ -135,15 +119,15 @@ public function getViewtype(): string {
 	 *
 	 * @return string
 	 */
-	private function resolveViewtype(): string {
+	protected function resolveViewtype(): string {
 		if ($this->request) {
 			$view = $this->request->getParam('view', '', false);
 			if ($this->isValidViewtype($view) && !empty($this->locations[$view])) {
 				return $view;
 			}
 		}
 
-		$view = (string) elgg_get_config('view');
+		$view = (string) $this->config->view;
 		if ($this->isValidViewtype($view) && !empty($this->locations[$view])) {
 			return $view;
 		}
@@ -473,7 +457,7 @@ protected function fileExists(string $path): bool {
 	 *
 	 * @return string|false output generated by view file inclusion or false
 	 */
-	private function renderViewFile(string $view, array $vars, string $viewtype, bool $issue_missing_notice): string|false {
+	protected function renderViewFile(string $view, array $vars, string $viewtype, bool $issue_missing_notice): string|false {
 		$file = $this->findViewFile($view, $viewtype);
 		if (!$file) {
 			if ($issue_missing_notice) {
@@ -662,22 +646,22 @@ public function isCacheableView(string $view): bool {
 	}
 
 	/**
-	 * Register a plugin's views
+	 * Register all views in a given path
 	 *
-	 * @param string $path Base path of the plugin
+	 * @param string $path Base path to scan for views
 	 *
 	 * @return bool
 	 */
-	public function registerPluginViews(string $path): bool {
+	public function registerViewsFromPath(string $path): bool {
 		$path = Paths::sanitize($path);
 		$view_dir = "{$path}views/";
 
-		// plugins don't have to have views.
+		// do not fail on non existing views folder
 		if (!is_dir($view_dir)) {
 			return true;
 		}
 
-		// but if they do, they have to be readable
+		// but if folder exists it has to be readable
 		$handle = opendir($view_dir);
 		if ($handle === false) {
 			$this->getLogger()->notice("Unable to register views from the directory: {$view_dir}");
@@ -755,8 +739,8 @@ public function listViews(string $viewtype = 'default'): array {
 	public function getInspectorData(): array {
 		$overrides = $this->overrides;
 
-		if ($this->cache) {
-			$data = $this->cache->load('view_overrides');
+		if ($this->server_cache) {
+			$data = $this->server_cache->load('view_overrides');
 			if (is_array($data)) {
 				$overrides = $data;
 			}
@@ -773,39 +757,56 @@ public function getInspectorData(): array {
 	/**
 	 * Configure locations from the cache
 	 *
-	 * @param SystemCache $cache The system cache
-	 *
-	 * @return bool
+	 * @return void
 	 */
-	public function configureFromCache(SystemCache $cache): bool {
-		$data = $cache->load('view_locations');
+	public function configureFromCache(): void {
+		if (!$this->server_cache->isEnabled()) {
+			return;
+		}
+
+		$data = $this->server_cache->load('view_locations');
 		if (!is_array($data)) {
-			return false;
+			return;
 		}
 
 		$this->locations = $data['locations'];
-		$this->cache = $cache;
-
-		return true;
+		$this->locations_loaded_from_cache = true;
 	}
 
 	/**
 	 * Cache the configuration
 	 *
-	 * @param SystemCache $cache The system cache
-	 *
 	 * @return void
 	 */
-	public function cacheConfiguration(SystemCache $cache): void {
+	public function cacheConfiguration(): void {
+		if (!$this->server_cache->isEnabled()) {
+			return;
+		}
+
+		// only cache if not already loaded
+		if ($this->isViewLocationsLoadedFromCache()) {
+			return;
+		}
+
 		if (empty($this->locations)) {
-			$cache->delete('view_locations');
+			$this->server_cache->delete('view_locations');
 			return;
 		}
 
-		$cache->save('view_locations', ['locations' => $this->locations]);
+		$this->server_cache->save('view_locations', ['locations' => $this->locations]);
 
 		// this is saved just for the inspector and is not loaded in loadAll()
-		$cache->save('view_overrides', $this->overrides);
+		$this->server_cache->save('view_overrides', $this->overrides);
+	}
+
+	/**
+	 * Checks if view_locations have been loaded from cache.
+	 * This can be used to determine if there is a need to (re)load view locations
+	 *
+	 * @return bool
+	 */
+	public function isViewLocationsLoadedFromCache(): bool {
+		return $this->locations_loaded_from_cache;
 	}
 
 	/**
@@ -817,7 +818,7 @@ public function cacheConfiguration(SystemCache $cache): void {
 	 *
 	 * @return void
 	 */
-	private function setViewLocation(string $view, string $viewtype, string $path): void {
+	protected function setViewLocation(string $view, string $viewtype, string $path): void {
 		$view = self::canonicalizeViewName($view);
 		$path = strtr($path, '\\', '/');
 

engine/classes/ElggInstaller.php

@@ -160,7 +160,7 @@ protected function getApp(): Application {
 
 			$app->internal_services->views->setViewtype('installation');
 			$app->internal_services->views->registerViewtypeFallback('installation');
-			$app->internal_services->views->registerPluginViews(Paths::elgg());
+			$app->internal_services->views->registerViewsFromPath(Paths::elgg());
 			$app->internal_services->translator->registerTranslations(Paths::elgg() . 'install/languages/', true);
 
 			return $this->app;

engine/classes/ElggPlugin.php

@@ -887,7 +887,7 @@ protected function registerPublicServices(): void {
 	 * @throws \Elgg\Exceptions\PluginException
 	 */
 	protected function registerViews(): void {
-		if (_elgg_services()->config->system_cache_loaded) {
+		if (_elgg_services()->views->isViewLocationsLoadedFromCache()) {
 			return;
 		}
 
@@ -900,7 +900,7 @@ protected function registerViews(): void {
 		}
 
 		// Allow /views directory files to override
-		if (!$views->registerPluginViews($this->getPath())) {
+		if (!$views->registerViewsFromPath($this->getPath())) {
 			$msg = elgg_echo('ElggPlugin:Exception:CannotRegisterViews', [$this->getID(), $this->guid, $this->getPath()]);
 
 			throw PluginException::factory([