mirror of
https://github.com/nextcloud/server.git
synced 2026-03-27 04:43:20 -04:00
Merge pull request #48650 from nextcloud/fix/shipped-app-version
fix: Shipped apps should include the Nextcloud version in the cache buster
This commit is contained in:
commit
262ed0ffeb
3 changed files with 143 additions and 53 deletions
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
$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);
|
||||
|
||||
/** @var IInitialStateService */
|
||||
$this->initialState = \OC::$server->get(IInitialStateService::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);
|
||||
}
|
||||
// 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;
|
||||
}
|
||||
// If not found try the file
|
||||
if ($hash === false && is_string($file) && $file !== '') {
|
||||
$hash = $this->getVersionHashByPath($file);
|
||||
}
|
||||
// As a last resort we use the server version hash
|
||||
if ($hash === false) {
|
||||
$hash = self::$versionHash;
|
||||
}
|
||||
|
||||
return '?v=' . self::$versionHash . $themingSuffix;
|
||||
// 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 self::$cacheBusterCache[$path];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
84
tests/lib/TemplateLayoutTest.php
Normal file
84
tests/lib/TemplateLayoutTest.php
Normal file
|
|
@ -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'],
|
||||
];
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
Reference in a new issue