From b58b3750a7a037cbd9328bf6b14f7865e0a129be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 29 Sep 2025 12:10:56 +0200 Subject: [PATCH] chore: Cleanup typing and deprecated method calls in resource locators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/Template/CSSResourceLocator.php | 60 ++++++++------- lib/private/Template/JSResourceLocator.php | 82 ++++++++++----------- lib/private/Template/ResourceLocator.php | 67 ++++++++--------- lib/private/TemplateLayout.php | 31 ++++---- 4 files changed, 119 insertions(+), 121 deletions(-) diff --git a/lib/private/Template/CSSResourceLocator.php b/lib/private/Template/CSSResourceLocator.php index b501fd69874..59281a36e42 100644 --- a/lib/private/Template/CSSResourceLocator.php +++ b/lib/private/Template/CSSResourceLocator.php @@ -1,37 +1,48 @@ appendIfExist($this->serverroot, $style . '.css') - || $this->appendIfExist($this->serverroot, 'core/' . $style . '.css') + public function doFind(string $resource): void { + $parts = explode('/', $resource, 2); + if (count($parts) < 2) { + return; + } + [$app,$subpath] = $parts; + if ($this->appendIfExist($this->serverroot, $resource . '.css') + || $this->appendIfExist($this->serverroot, 'core/' . $resource . '.css') ) { return; } - $style = substr($style, strpos($style, '/') + 1); - $app_path = \OC_App::getAppPath($app); - $app_url = \OC_App::getAppWebPath($app); - - if ($app_path === false && $app_url === false) { + try { + $app_path = $this->appManager->getAppPath($app); + $app_url = $this->appManager->getAppWebPath($app); + } catch (AppPathNotFoundException $e) { $this->logger->error('Could not find resource {resource} to load', [ - 'resource' => $app . '/' . $style . '.css', + 'resource' => $app . '/' . $subpath . '.css', 'app' => 'cssresourceloader', + 'exception' => $e, ]); return; } @@ -41,24 +52,21 @@ class CSSResourceLocator extends ResourceLocator { // turned into cwd. $app_path = realpath($app_path); - $this->append($app_path, $style . '.css', $app_url); + $this->append($app_path, $subpath . '.css', $app_url); } - /** - * @param string $style - */ - public function doFindTheme($style) { + public function doFindTheme(string $resource): void { $theme_dir = 'themes/' . $this->theme . '/'; - $this->appendIfExist($this->serverroot, $theme_dir . 'apps/' . $style . '.css') - || $this->appendIfExist($this->serverroot, $theme_dir . $style . '.css') - || $this->appendIfExist($this->serverroot, $theme_dir . 'core/' . $style . '.css'); + $this->appendIfExist($this->serverroot, $theme_dir . 'apps/' . $resource . '.css') + || $this->appendIfExist($this->serverroot, $theme_dir . $resource . '.css') + || $this->appendIfExist($this->serverroot, $theme_dir . 'core/' . $resource . '.css'); } - public function append($root, $file, $webRoot = null, $throw = true, $scss = false) { + public function append(string $root, string $file, ?string $webRoot = null, bool $throw = true, bool $scss = false): void { if (!$scss) { parent::append($root, $file, $webRoot, $throw); } else { - if (!$webRoot) { + if ($webRoot === null || $webRoot === '') { $webRoot = $this->findWebRoot($root); if ($webRoot === null) { diff --git a/lib/private/Template/JSResourceLocator.php b/lib/private/Template/JSResourceLocator.php index a6d2d13a2ad..fc35b6c3128 100644 --- a/lib/private/Template/JSResourceLocator.php +++ b/lib/private/Template/JSResourceLocator.php @@ -1,36 +1,36 @@ jsCombiner = $JSCombiner; - $this->appManager = $appManager; + public function __construct( + LoggerInterface $logger, + IConfig $config, + protected JSCombiner $jsCombiner, + protected IAppManager $appManager, + ) { + parent::__construct($logger, $config); } - /** - * @param string $script - */ - public function doFind($script) { + public function doFind(string $resource): void { $theme_dir = 'themes/' . $this->theme . '/'; // Extracting the appId and the script file name - $app = substr($script, 0, strpos($script, '/')); - $scriptName = basename($script); + [$app] = explode('/', $resource, 2); + $scriptName = basename($resource); // Get the app root path $appRoot = $this->serverroot . '/apps/'; $appWebRoot = null; @@ -48,69 +48,65 @@ class JSResourceLocator extends ResourceLocator { // ignore } - if (str_contains($script, '/l10n/')) { + if (str_contains($resource, '/l10n/')) { // For language files we try to load them all, so themes can overwrite // single l10n strings without having to translate all of them. - $found = 0; - $found += $this->appendScriptIfExist($this->serverroot, 'core/' . $script); - $found += $this->appendScriptIfExist($this->serverroot, $theme_dir . 'core/' . $script); - $found += $this->appendScriptIfExist($this->serverroot, $script); - $found += $this->appendScriptIfExist($this->serverroot, $theme_dir . $script); - $found += $this->appendScriptIfExist($appRoot, $script, $appWebRoot); - $found += $this->appendScriptIfExist($this->serverroot, $theme_dir . 'apps/' . $script); + $found = $this->appendScriptIfExist($this->serverroot, 'core/' . $resource) + || $this->appendScriptIfExist($this->serverroot, $theme_dir . 'core/' . $resource) + || $this->appendScriptIfExist($this->serverroot, $resource) + || $this->appendScriptIfExist($this->serverroot, $theme_dir . $resource) + || $this->appendScriptIfExist($appRoot, $resource, $appWebRoot) + || $this->appendScriptIfExist($this->serverroot, $theme_dir . 'apps/' . $resource); if ($found) { return; } - } elseif ($this->appendScriptIfExist($this->serverroot, $theme_dir . 'apps/' . $script) - || $this->appendScriptIfExist($this->serverroot, $theme_dir . $script) - || $this->appendScriptIfExist($this->serverroot, $script) + } elseif ($this->appendScriptIfExist($this->serverroot, $theme_dir . 'apps/' . $resource) + || $this->appendScriptIfExist($this->serverroot, $theme_dir . $resource) + || $this->appendScriptIfExist($this->serverroot, $resource) || $this->appendScriptIfExist($this->serverroot, $theme_dir . "dist/$app-$scriptName") || $this->appendScriptIfExist($this->serverroot, "dist/$app-$scriptName") - || $this->appendScriptIfExist($appRoot, $script, $appWebRoot) - || $this->cacheAndAppendCombineJsonIfExist($this->serverroot, $script . '.json') - || $this->cacheAndAppendCombineJsonIfExist($appRoot, $script . '.json', $app) - || $this->appendScriptIfExist($this->serverroot, $theme_dir . 'core/' . $script) - || $this->appendScriptIfExist($this->serverroot, 'core/' . $script) - || (strpos($scriptName, '/') === -1 && ($this->appendScriptIfExist($this->serverroot, $theme_dir . "dist/core-$scriptName") - || $this->appendScriptIfExist($this->serverroot, "dist/core-$scriptName"))) - || $this->cacheAndAppendCombineJsonIfExist($this->serverroot, 'core/' . $script . '.json') + || $this->appendScriptIfExist($appRoot, $resource, $appWebRoot) + || $this->cacheAndAppendCombineJsonIfExist($this->serverroot, $resource . '.json') + || $this->cacheAndAppendCombineJsonIfExist($appRoot, $resource . '.json', $app) + || $this->appendScriptIfExist($this->serverroot, $theme_dir . 'core/' . $resource) + || $this->appendScriptIfExist($this->serverroot, 'core/' . $resource) + || $this->appendScriptIfExist($this->serverroot, $theme_dir . "dist/core-$scriptName") + || $this->appendScriptIfExist($this->serverroot, "dist/core-$scriptName") + || $this->cacheAndAppendCombineJsonIfExist($this->serverroot, 'core/' . $resource . '.json') ) { return; } // missing translations files will be ignored - if (str_contains($script, '/l10n/')) { + if (str_contains($resource, '/l10n/')) { return; } $this->logger->error('Could not find resource {resource} to load', [ - 'resource' => $script . '.js', + 'resource' => $resource . '.js', 'app' => 'jsresourceloader', ]); } - /** - * @param string $script - */ - public function doFindTheme($script) { + public function doFindTheme(string $resource): void { } /** * Try to find ES6 script file (`.mjs`) with fallback to plain javascript (`.js`) * @see appendIfExist() */ - protected function appendScriptIfExist(string $root, string $file, ?string $webRoot = null) { + protected function appendScriptIfExist(string $root, string $file, ?string $webRoot = null): bool { if (!$this->appendIfExist($root, $file . '.mjs', $webRoot)) { return $this->appendIfExist($root, $file . '.js', $webRoot); } return true; } - protected function cacheAndAppendCombineJsonIfExist($root, $file, $app = 'core') { + protected function cacheAndAppendCombineJsonIfExist(string $root, string $file, string $app = 'core'): bool { if (is_file($root . '/' . $file)) { if ($this->jsCombiner->process($root, $file, $app)) { - $this->append($this->serverroot, $this->jsCombiner->getCachedJS($app, $file), false, false); + $this->append($this->serverroot, $this->jsCombiner->getCachedJS($app, $file), null, false); } else { // Add all the files from the json $files = $this->jsCombiner->getContent($root, $file); diff --git a/lib/private/Template/ResourceLocator.php b/lib/private/Template/ResourceLocator.php index fa52f8e5c0d..59e7b2ccbfb 100644 --- a/lib/private/Template/ResourceLocator.php +++ b/lib/private/Template/ResourceLocator.php @@ -1,51 +1,53 @@ logger = $logger; + public function __construct( + protected LoggerInterface $logger, + \OCP\IConfig $config, + ) { $this->mapping = [ \OC::$SERVERROOT => \OC::$WEBROOT ]; $this->serverroot = \OC::$SERVERROOT; $this->webroot = \OC::$WEBROOT; - $this->theme = \OC_Util::getTheme(); + + $this->theme = $config->getSystemValueString('theme', ''); + + if ($this->theme === '') { + if (is_dir(\OC::$SERVERROOT . '/themes/default')) { + $this->theme = 'default'; + } + } } - /** - * @param string $resource - */ - abstract public function doFind($resource); + abstract public function doFind(string $resource): void; - /** - * @param string $resource - */ - abstract public function doFindTheme($resource); + abstract public function doFindTheme(string $resource): void; /** * Finds the resources and adds them to the list - * - * @param array $resources */ - public function find($resources) { + public function find(array $resources): void { foreach ($resources as $resource) { try { $this->doFind($resource); @@ -74,8 +76,8 @@ abstract class ResourceLocator { * @param string|null $webRoot base for path, default map $root to $webRoot * @return bool True if the resource was found, false otherwise */ - protected function appendIfExist($root, $file, $webRoot = null) { - if ($root !== false && is_file($root . '/' . $file)) { + protected function appendIfExist(string $root, string $file, ?string $webRoot = null): bool { + if (is_file($root . '/' . $file)) { $this->append($root, $file, $webRoot, false); return true; } @@ -95,10 +97,9 @@ abstract class ResourceLocator { * then /srv/www/apps, /srv/www/apps, /srv/www, ... until we find a * valid web root * - * @param string $root * @return string|null The web root or null on failure */ - protected function findWebRoot($root) { + protected function findWebRoot(string $root): ?string { $webRoot = null; $tmpRoot = $root; @@ -135,15 +136,8 @@ abstract class ResourceLocator { * @param bool $throw Throw an exception, when the route does not exist * @throws ResourceNotFoundException Only thrown when $throw is true and the resource is missing */ - protected function append($root, $file, $webRoot = null, $throw = true) { - if (!is_string($root)) { - if ($throw) { - throw new ResourceNotFoundException($file, $webRoot); - } - return; - } - - if (!$webRoot) { + protected function append(string $root, string $file, ?string $webRoot = null, bool $throw = true): void { + if ($webRoot === null || $webRoot === '') { $webRoot = $this->findWebRoot($root); if ($webRoot === null) { @@ -166,9 +160,8 @@ abstract class ResourceLocator { /** * Returns the list of all resources that should be loaded - * @return array */ - public function getResources() { + public function getResources(): array { return $this->resources; } } diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php index b37f919abe7..6c1fe71e4e9 100644 --- a/lib/private/TemplateLayout.php +++ b/lib/private/TemplateLayout.php @@ -7,6 +7,7 @@ declare(strict_types=1); * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OC; use bantu\IniGetWrapper\IniGetWrapper; @@ -42,8 +43,8 @@ class TemplateLayout { /** @var string[] */ private static array $cacheBusterCache = []; - public static ?CSSResourceLocator $cssLocator = null; - public static ?JSResourceLocator $jsLocator = null; + public ?CSSResourceLocator $cssLocator = null; + public ?JSResourceLocator $jsLocator = null; public function __construct( private IConfig $config, @@ -214,7 +215,7 @@ class TemplateLayout { } // Add the js files - $jsFiles = self::findJavascriptFiles(Util::getScripts()); + $jsFiles = $this->findJavascriptFiles(Util::getScripts()); $page->assign('jsfiles', []); if ($this->config->getSystemValueBool('installed', false) && $renderAs != TemplateResponse::RENDER_AS_ERROR) { // this is on purpose outside of the if statement below so that the initial state is prefilled (done in the getConfig() call) @@ -265,12 +266,12 @@ class TemplateLayout { && !preg_match('/^\/login/', $pathInfo) && $renderAs !== TemplateResponse::RENDER_AS_ERROR ) { - $cssFiles = self::findStylesheetFiles(\OC_Util::$styles); + $cssFiles = $this->findStylesheetFiles(\OC_Util::$styles); } else { // If we ignore the scss compiler, // we need to load the guest css fallback Util::addStyle('guest'); - $cssFiles = self::findStylesheetFiles(\OC_Util::$styles); + $cssFiles = $this->findStylesheetFiles(\OC_Util::$styles); } $page->assign('cssfiles', []); @@ -365,12 +366,12 @@ class TemplateLayout { return self::$cacheBusterCache[$path]; } - public static function findStylesheetFiles(array $styles): array { - if (!self::$cssLocator) { - self::$cssLocator = \OCP\Server::get(CSSResourceLocator::class); + private function findStylesheetFiles(array $styles): array { + if ($this->cssLocator === null) { + $this->cssLocator = \OCP\Server::get(CSSResourceLocator::class); } - self::$cssLocator->find($styles); - return self::$cssLocator->getResources(); + $this->cssLocator->find($styles); + return $this->cssLocator->getResources(); } public function getAppNamefromPath(string $path): string|false { @@ -387,12 +388,12 @@ class TemplateLayout { return false; } - public static function findJavascriptFiles(array $scripts): array { - if (!self::$jsLocator) { - self::$jsLocator = \OCP\Server::get(JSResourceLocator::class); + private function findJavascriptFiles(array $scripts): array { + if ($this->jsLocator === null) { + $this->jsLocator = \OCP\Server::get(JSResourceLocator::class); } - self::$jsLocator->find($scripts); - return self::$jsLocator->getResources(); + $this->jsLocator->find($scripts); + return $this->jsLocator->getResources(); } /**