fix: Shipped apps should include the Nextcloud version in the cache busterfix/cachebuster-stable30

Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>

1 files changed, 66 insertions, 60 deletions

diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php
index d17189a3bd0..3f6521a8e37 100644
--- a/lib/private/TemplateLayout.php
+++ b/lib/private/TemplateLayout.php
@@ -56,11 +56,14 @@ use OCP\IInitialStateService;
 use OCP\INavigationManager;
 use OCP\IURLGenerator;
 use OCP\IUserSession;
+use OCP\L10N\IFactory;
 use OCP\Support\Subscription\IRegistry;
 use OCP\Util;
 
 class TemplateLayout extends \OC_Template {
 	private static $versionHash = '';
+	/** @var string[] */
+	private static $cacheBusterCache = [];
 
 	/** @var CSSResourceLocator|null */
 	public static $cssLocator = null;
@@ -68,38 +71,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');
@@ -120,7 +114,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());
@@ -131,9 +125,9 @@ class TemplateLayout extends \OC_Template {
 			$this->assign('logoUrl', $logoUrl);
 
 			// Set default app name
-			$defaultApp = \OC::$server->getAppManager()->getDefaultAppForUser();
-			$defaultAppInfo = \OC::$server->getAppManager()->getAppInfo($defaultApp);
-			$l10n = \OC::$server->getL10NFactory()->get($defaultApp);
+			$defaultApp = $this->appManager->getDefaultAppForUser();
+			$defaultAppInfo = $this->appManager->getAppInfo($defaultApp);
+			$l10n = \OC::$server->get(IFactory::class)->get($defaultApp);
 			$this->assign('defaultAppName', $l10n->t($defaultAppInfo['name']));
 
 			// Add navigation entry
@@ -213,8 +207,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/');
 			}
@@ -232,7 +225,7 @@ class TemplateLayout extends \OC_Template {
 		$this->assign('language', $lang);
 		$this->assign('locale', $locale);
 
-		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());
@@ -252,7 +245,7 @@ class TemplateLayout extends \OC_Template {
 			$jsConfigHelper = new JSConfigHelper(
 				\OCP\Util::getL10N('lib'),
 				\OCP\Server::get(Defaults::class),
-				\OC::$server->getAppManager(),
+				$this->appManager,
 				\OC::$server->getSession(),
 				\OC::$server->getUserSession()->getUser(),
 				$this->config,
@@ -287,6 +280,7 @@ class TemplateLayout extends \OC_Template {
 		if (\OC::$server->getSystemConfig()->getValue('installed', false)
 			&& !\OCP\Util::needUpgrade()
 			&& $pathInfo !== ''
+			&& $pathInfo !== false
 			&& !preg_match('/^\/login/', $pathInfo)
 			&& $renderAs !== TemplateResponse::RENDER_AS_ERROR
 		) {
@@ -295,7 +289,7 @@ class TemplateLayout extends \OC_Template {
 			// If we ignore the scss compiler,
 			// we need to load the guest css fallback
 			\OC_Util::addStyle('guest');
-			$cssFiles = self::findStylesheetFiles(\OC_Util::$styles, false);
+			$cssFiles = self::findStylesheetFiles(\OC_Util::$styles);
 		}
 
 		$this->assign('cssfiles', []);
@@ -324,51 +318,64 @@ class TemplateLayout extends \OC_Template {
 		$this->assign('id-app-navigation', $renderAs === TemplateResponse::RENDER_AS_USER ? '#app-navigation' : null);
 	}
 
-	/**
-	 * @param string $path
-	 * @param string $file
-	 * @return string
-	 */
-	protected function getVersionHashSuffix($path = false, $file = false) {
+	protected function getVersionHashSuffix(string $path = '', string $file = ''): string {
 		if ($this->config->getSystemValueBool('debug', false)) {
 			// 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 ($path !== '') {
+			$hash = $this->getVersionHashByPath($path);
+		}
+		// If not found try the file
+		if ($hash === false && $file !== '') {
+			$hash = $this->getVersionHashByPath($file);
+		}
+		// As a last resort we use the server version hash
+		if ($hash === false) {
+			$hash = self::$versionHash;
 		}
-		// 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;
+
+		// 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) {
+				// 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];
 	}
 
 	/**
 	 * @param array $styles
 	 * @return array
 	 */
-	public static function findStylesheetFiles($styles, $compileScss = true) {
+	public static function findStylesheetFiles($styles) {
 		if (!self::$cssLocator) {
 			self::$cssLocator = \OCP\Server::get(CSSResourceLocator::class);
 		}
@@ -377,11 +384,10 @@ class TemplateLayout extends \OC_Template {
 	}
 
 	/**
-	 * @param string $path
-	 * @return string|boolean
+	 * @return string|false
 	 */
-	public function getAppNamefromPath($path) {
-		if ($path !== '' && is_string($path)) {
+	public function getAppNamefromPath(string $path) {
+		if ($path !== '') {
 			$pathParts = explode('/', $path);
 			if ($pathParts[0] === 'css') {
 				// This is a scss request