address 2nd review

Signed-off-by: Simon L <szaimen@e.mail.de>
nextcloud · Nov 17, 2022 · 756ee39 · 756ee39
1 parent f7c3bc9
commit 756ee39
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 19 deletions.
diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php
@@ -88,15 +88,15 @@ public function __construct(
 	public function getThemedIcon(string $app, string $image): Response {
 		$color = $this->themingDefaults->getColorPrimary();
 		try {
-			$iconFile = $this->imageManager->getCachedImage('icon-' . $app . '-' . str_replace('/', '_',$image) . $color);
+			$iconFileName = $this->imageManager->getCachedImage('icon-' . $app . '-' . $color . '-' . str_replace('/', '_',$image));
 		} catch (NotFoundException $exception) {
 			$icon = $this->iconBuilder->colorSvg($app, $image);
 			if ($icon === false || $icon === '') {
 				return new NotFoundResponse();
 			}
-			$iconFile = $this->imageManager->setCachedImage('icon-' . $app . '-' . str_replace('/', '_',$image) . $color, $icon);
+			$iconFileName = $this->imageManager->setCachedImage('icon-' . $app . '-' . $color . '-' . str_replace('/', '_',$image) . $color);
 		}
-		$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
+		$response = new FileDisplayResponse($iconFileName, Http::STATUS_OK, ['Content-Type' => 'image/svg+xml']);
 		$response->cacheFor(86400, false, true);
 		return $response;
 	}

diff --git a/apps/theming/lib/Service/JSDataService.php b/apps/theming/lib/Service/JSDataService.php
@@ -59,7 +59,7 @@ public function jsonSerialize(): array {
 			'imprintUrl' => $this->themingDefaults->getImprintUrl(),
 			'privacyUrl' => $this->themingDefaults->getPrivacyUrl(),
 			'inverted' => $this->util->invertTextColor($this->themingDefaults->getColorPrimary()),
-			'cacheBuster' => $this->util->getUserCacheBuster() . $this->appConfig->getAppValue(Application::APP_ID, 'cachebuster', '0'),
+			'cacheBuster' => $this->util->getCacheBuster(),
 			'enabledThemes' => $this->themesService->getEnabledThemes(),
 		];
 	}

diff --git a/apps/theming/lib/Service/ThemeInjectionService.php b/apps/theming/lib/Service/ThemeInjectionService.php
@@ -87,18 +87,10 @@ public function injectHeaders() {
 	 * @param string $media media query to use in the <link> element
 	 */
 	private function addThemeHeader(string $themeId, bool $plain = true, string $media = null) {
-		$cacheBuster = $this->config->getAppValue('theming', 'cachebuster', '0');
-		if ($this->userId !== null) {
-			// need to bust the cache for the CSS file when the user background changed as its
-			// URL is served in those files
-			$userCacheBuster = $this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0');
-			$cacheBuster .= $this->userId . '_' . $userCacheBuster;
-		}
-
 		$linkToCSS = $this->urlGenerator->linkToRoute('theming.Theming.getThemeStylesheet', [
 			'themeId' => $themeId,
 			'plain' => $plain,
-			'v' => substr(sha1($cacheBuster), 0, 8),
+			'v' => Util::getCacheBuster(),
 		]);
 		Util::addHeader('link', [
 			'rel' => 'stylesheet',

diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php
@@ -420,7 +420,7 @@ public function replaceImagePath($app, $image) {
 		}
 
 		if ($route) {
-			return $route . '?v=' . $this->util->getUserCacheBuster() . $cacheBusterValue;
+			return $route . '?v=' . $this->util->getCacheBuster();
 		}
 
 		return false;

diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php
@@ -268,13 +268,20 @@ public function isLogoThemed() {
 			|| $this->imageManager->hasImage('logoheader');
 	}
 
-	public function getUserCacheBuster(): string {
-		$userId = \OC::$server->get(IUserSession::class)->getUser() ? \OC::$server->get(IUserSession::class)->getUser()->getUID() : '';
-		$cacheBusterValue = '';
+	public function getCacheBuster(): string {
+		$userSession = \OC::$server->get(IUserSession::class);
+		$userId = '';
+		if (!is_null($userSession)) {
+			$user = $userSession->getUser();
+			if (!is_null($user)) {
+				$userId = $user->getUID();
+			}
+		}
 		if ($userId) {
 			$userCacheBusterValue = (int)$this->config->getUserValue($userId, 'theming', 'userCacheBuster', '0');
-			$cacheBusterValue = substr(sha1($userId . '_' . $userCacheBusterValue), 0, 8);
+			$userCacheBuster = substr(sha1($userId . '_' . $userCacheBusterValue), 0, 8);
 		}
-		return $cacheBusterValue;
+		$systemCacheBuster = $this->config->getAppValue('theming', 'cachebuster', '0');
+		return $userCacheBuster . $systemCacheBuster;
 	}
 }