aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--build/psalm-baseline.xml6
-rw-r--r--lib/private/TemplateLayout.php104
-rw-r--r--tests/lib/TemplateLayoutTest.php84
3 files changed, 142 insertions, 52 deletions
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index 16fe161a9ba..1a9b15c00cd 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -2693,12 +2693,8 @@
<code><![CDATA[string]]></code>
</InvalidParamDefault>
<InvalidScalarArgument>
- <code><![CDATA[$appName]]></code>
- <code><![CDATA[$appName]]></code>
+ <code><![CDATA[$appId]]></code>
</InvalidScalarArgument>
- <UndefinedInterfaceMethod>
- <code><![CDATA[getInitialStates]]></code>
- </UndefinedInterfaceMethod>
</file>
<file src="lib/private/URLGenerator.php">
<InvalidReturnStatement>
diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php
index 60c7526435e..2bc9ff5a2af 100644
--- a/lib/private/TemplateLayout.php
+++ b/lib/private/TemplateLayout.php
@@ -31,6 +31,8 @@ use OCP\Util;
class TemplateLayout extends \OC_Template {
private static $versionHash = '';
+ /** @var string[] */
+ private static $cacheBusterCache = [];
/** @var CSSResourceLocator|null */
public static $cssLocator = null;
@@ -38,38 +40,29 @@ class TemplateLayout extends \OC_Template {
/** @var JSResourceLocator|null */
public static $jsLocator = null;
- /** @var IConfig */
- private $config;
-
- /** @var IInitialStateService */
- private $initialState;
-
- /** @var INavigationManager */
- private $navigationManager;
+ private IConfig $config;
+ private IAppManager $appManager;
+ private InitialStateService $initialState;
+ private INavigationManager $navigationManager;
/**
* @param string $renderAs
* @param string $appId application id
*/
public function __construct($renderAs, $appId = '') {
- /** @var IConfig */
- $this->config = \OC::$server->get(IConfig::class);
-
- /** @var IInitialStateService */
- $this->initialState = \OC::$server->get(IInitialStateService::class);
+ $this->config = \OCP\Server::get(IConfig::class);
+ $this->appManager = \OCP\Server::get(IAppManager::class);
+ $this->initialState = \OCP\Server::get(InitialStateService::class);
+ $this->navigationManager = \OCP\Server::get(INavigationManager::class);
- // Add fallback theming variables if theming is disabled
- if ($renderAs !== TemplateResponse::RENDER_AS_USER
- || !\OC::$server->getAppManager()->isEnabledForUser('theming')) {
+ // Add fallback theming variables if not rendered as user
+ if ($renderAs !== TemplateResponse::RENDER_AS_USER) {
// TODO cache generated default theme if enabled for fallback if server is erroring ?
Util::addStyle('theming', 'default');
}
// Decide which page we show
if ($renderAs === TemplateResponse::RENDER_AS_USER) {
- /** @var INavigationManager */
- $this->navigationManager = \OC::$server->get(INavigationManager::class);
-
parent::__construct('core', 'layout.user');
if (in_array(\OC_App::getCurrentApp(), ['settings','admin', 'help']) !== false) {
$this->assign('bodyid', 'body-settings');
@@ -90,7 +83,7 @@ class TemplateLayout extends \OC_Template {
}
// Set body data-theme
$this->assign('enabledThemes', []);
- if (\OC::$server->getAppManager()->isEnabledForUser('theming') && class_exists('\OCA\Theming\Service\ThemesService')) {
+ if ($this->appManager->isEnabledForUser('theming') && class_exists('\OCA\Theming\Service\ThemesService')) {
/** @var \OCA\Theming\Service\ThemesService */
$themesService = \OC::$server->get(\OCA\Theming\Service\ThemesService::class);
$this->assign('enabledThemes', $themesService->getEnabledThemes());
@@ -101,8 +94,8 @@ class TemplateLayout extends \OC_Template {
$this->assign('logoUrl', $logoUrl);
// Set default entry name
- $defaultEntryId = \OCP\Server::get(INavigationManager::class)->getDefaultEntryIdForUser();
- $defaultEntry = \OCP\Server::get(INavigationManager::class)->get($defaultEntryId);
+ $defaultEntryId = $this->navigationManager->getDefaultEntryIdForUser();
+ $defaultEntry = $this->navigationManager->get($defaultEntryId);
$this->assign('defaultAppName', $defaultEntry['name']);
// Add navigation entry
@@ -182,8 +175,7 @@ class TemplateLayout extends \OC_Template {
$showSimpleSignup = true;
}
- $appManager = \OCP\Server::get(IAppManager::class);
- if ($appManager->isEnabledForUser('registration')) {
+ if ($this->appManager->isEnabledForUser('registration')) {
$urlGenerator = \OCP\Server::get(IURLGenerator::class);
$signUpLink = $urlGenerator->getAbsoluteURL('/index.php/apps/registration/');
}
@@ -203,7 +195,7 @@ class TemplateLayout extends \OC_Template {
$this->assign('locale', $locale);
$this->assign('direction', $direction);
- if (\OC::$server->getSystemConfig()->getValue('installed', false)) {
+ if ($this->config->getSystemValueBool('installed', false)) {
if (empty(self::$versionHash)) {
$v = \OC_App::getAppVersions();
$v['core'] = implode('.', \OCP\Util::getVersion());
@@ -224,7 +216,7 @@ class TemplateLayout extends \OC_Template {
\OCP\Server::get(ServerVersion::class),
\OCP\Util::getL10N('lib'),
\OCP\Server::get(Defaults::class),
- \OC::$server->getAppManager(),
+ $this->appManager,
\OC::$server->getSession(),
\OC::$server->getUserSession()->getUser(),
$this->config,
@@ -314,34 +306,52 @@ class TemplateLayout extends \OC_Template {
// allows chrome workspace mapping in debug mode
return '';
}
- $themingSuffix = '';
- $v = [];
- if ($this->config->getSystemValueBool('installed', false)) {
- if (\OC::$server->getAppManager()->isInstalled('theming')) {
- $themingSuffix = '-' . $this->config->getAppValue('theming', 'cachebuster', '0');
- }
- $v = \OC_App::getAppVersions();
+ if ($this->config->getSystemValueBool('installed', false) === false) {
+ // if not installed just return the version hash
+ return '?v=' . self::$versionHash;
}
- // Try the webroot path for a match
- if ($path !== false && $path !== '') {
- $appName = $this->getAppNamefromPath($path);
- if (array_key_exists($appName, $v)) {
- $appVersion = $v[$appName];
- return '?v=' . substr(md5($appVersion), 0, 8) . $themingSuffix;
- }
+ $hash = false;
+ // Try the web-root first
+ if (is_string($path) && $path !== '') {
+ $hash = $this->getVersionHashByPath($path);
+ }
+ // If not found try the file
+ if ($hash === false && is_string($file) && $file !== '') {
+ $hash = $this->getVersionHashByPath($file);
}
- // fallback to the file path instead
- if ($file !== false && $file !== '') {
- $appName = $this->getAppNamefromPath($file);
- if (array_key_exists($appName, $v)) {
- $appVersion = $v[$appName];
- return '?v=' . substr(md5($appVersion), 0, 8) . $themingSuffix;
+ // As a last resort we use the server version hash
+ if ($hash === false) {
+ $hash = self::$versionHash;
+ }
+
+ // The theming app is force-enabled thus the cache buster is always available
+ $themingSuffix = '-' . $this->config->getAppValue('theming', 'cachebuster', '0');
+
+ return '?v=' . $hash . $themingSuffix;
+ }
+
+ private function getVersionHashByPath(string $path): string|false {
+ if (array_key_exists($path, self::$cacheBusterCache) === false) {
+ // Not yet cached, so lets find the cache buster string
+ $appId = $this->getAppNamefromPath($path);
+ if ($appId === false || $appId === null) {
+ // No app Id could be guessed
+ return false;
+ }
+
+ $appVersion = $this->appManager->getAppVersion($appId);
+ // For shipped apps the app version is not a single source of truth, we rather also need to consider the Nextcloud version
+ if ($this->appManager->isShipped($appId)) {
+ $appVersion .= '-' . self::$versionHash;
}
+
+ $hash = substr(md5($appVersion), 0, 8);
+ self::$cacheBusterCache[$path] = $hash;
}
- return '?v=' . self::$versionHash . $themingSuffix;
+ return self::$cacheBusterCache[$path];
}
/**
diff --git a/tests/lib/TemplateLayoutTest.php b/tests/lib/TemplateLayoutTest.php
new file mode 100644
index 00000000000..405f1df7330
--- /dev/null
+++ b/tests/lib/TemplateLayoutTest.php
@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Test;
+
+use OC\InitialStateService;
+use OC\TemplateLayout;
+use OCP\App\IAppManager;
+use OCP\AppFramework\Http\TemplateResponse;
+use OCP\IConfig;
+
+class TemplateLayoutTest extends \Test\TestCase {
+
+
+ /** @dataProvider dataVersionHash */
+ public function testVersionHash($path, $file, $installed, $debug, $expected): void {
+ $appManager = $this->createMock(IAppManager::class);
+ $appManager->expects(self::any())
+ ->method('getAppVersion')
+ ->willReturnCallback(fn ($appId) => match ($appId) {
+ 'shippedApp' => 'shipped_1',
+ 'otherApp' => 'other_2',
+ default => "$appId",
+ });
+ $appManager->expects(self::any())
+ ->method('isShipped')
+ ->willReturnCallback(fn (string $app) => $app === 'shippedApp');
+
+ $config = $this->createMock(IConfig::class);
+ $config->expects(self::atLeastOnce())
+ ->method('getSystemValueBool')
+ ->willReturnMap([
+ ['installed', false, $installed],
+ ['debug', false, $debug],
+ ]);
+ $config->expects(self::any())
+ ->method('getAppValue')
+ ->with('theming', 'cachebuster', '0')
+ ->willReturn('42');
+
+ $initialState = $this->createMock(InitialStateService::class);
+
+ $this->overwriteService(IConfig::class, $config);
+ $this->overwriteService(IAppManager::class, $appManager);
+ $this->overwriteService(InitialStateService::class, $initialState);
+
+ $layout = $this->getMockBuilder(TemplateLayout::class)
+ ->onlyMethods(['getAppNamefromPath'])
+ ->setConstructorArgs([TemplateResponse::RENDER_AS_ERROR])
+ ->getMock();
+
+ self::invokePrivate(TemplateLayout::class, 'versionHash', ['version_hash']);
+
+ $layout->expects(self::any())
+ ->method('getAppNamefromPath')
+ ->willReturnCallback(fn ($appName) => match($appName) {
+ 'apps/shipped' => 'shippedApp',
+ 'other/app.css' => 'otherApp',
+ default => false,
+ });
+
+ $hash = self::invokePrivate($layout, 'getVersionHashSuffix', [$path, $file]);
+ self::assertEquals($expected, $hash);
+ }
+
+ public static function dataVersionHash() {
+ return [
+ 'no hash if in debug mode' => ['apps/shipped', 'style.css', true, true, ''],
+ 'only version hash if not installed' => ['apps/shipped', 'style.css', false, false, '?v=version_hash'],
+ 'version hash with cache buster if app not found' => ['unknown/path', '', true, false, '?v=version_hash-42'],
+ 'version hash with cache buster if neither path nor file provided' => [false, false, true, false, '?v=version_hash-42'],
+ 'app version hash if external app' => ['', 'other/app.css', true, false, '?v=' . substr(md5('other_2'), 0, 8) . '-42'],
+ 'app version and version hash if shipped app' => ['apps/shipped', 'style.css', true, false, '?v=' . substr(md5('shipped_1-version_hash'), 0, 8) . '-42'],
+ 'prefer path over file' => ['apps/shipped', 'other/app.css', true, false, '?v=' . substr(md5('shipped_1-version_hash'), 0, 8) . '-42'],
+ ];
+ }
+
+}