fix(theming): read cachebuster via IAppConfig in ImageManager

miaulalala · miaulalala · commit e99a0177cf70 · 2026-05-07T11:26:49.000+02:00
The cachebuster is written via IAppConfig::setAppValueInt() but was
read back via the deprecated IConfig::getAppValue(). Within the same
request the IConfig layer may return a stale cached value, so the URL
returned by getImageUrl() after an upload could carry the old buster
value - causing the browser to serve the old cached thumbnail on a
second consecutive upload without a page reload.

Fix by injecting IAppConfig (app-scoped) into ImageManager and reading
the cachebuster through the same API that writes it.

Signed-off-by: Anna Larch &lt;anna@nextcloud.com&gt;
AI-Assisted-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php
@@ -7,7 +7,9 @@
 namespace OCA\Theming;
 
 use OCA\Theming\AppInfo\Application;
+use OCA\Theming\ConfigLexicon;
 use OCA\Theming\Service\BackgroundService;
+use OCP\AppFramework\Services\IAppConfig;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
@@ -30,6 +32,7 @@ public function __construct(
 		private LoggerInterface $logger,
 		private ITempManager $tempManager,
 		private BackgroundService $backgroundService,
+		private IAppConfig $appConfig,
 	) {
 	}
 
@@ -40,7 +43,7 @@ public function __construct(
 	 * @return string the image url
 	 */
 	public function getImageUrl(string $key): string {
-		$cacheBusterCounter = $this->config->getAppValue(Application::APP_ID, 'cachebuster', '0');
+		$cacheBusterCounter = (string)$this->appConfig->getAppValueInt(ConfigLexicon::CACHE_BUSTER);
 		if ($this->hasImage($key)) {
 			return $this->urlGenerator->linkToRoute('theming.Theming.getImage', [ 'key' => $key ]) . '?v=' . $cacheBusterCounter;
 		} elseif ($key === 'backgroundDark' && $this->hasImage('background')) {
@@ -139,7 +142,7 @@ public function getCustomImages(): array {
 	 * @throws NotPermittedException
 	 */
 	public function getCacheFolder(): ISimpleFolder {
-		$cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0');
+		$cacheBusterValue = (string)$this->appConfig->getAppValueInt(ConfigLexicon::CACHE_BUSTER);
 		try {
 			$folder = $this->getRootFolder()->getFolder($cacheBusterValue);
 		} catch (NotFoundException $e) {
diff --git a/apps/theming/tests/ImageManagerTest.php b/apps/theming/tests/ImageManagerTest.php
@@ -9,6 +9,7 @@
 
 use OCA\Theming\ImageManager;
 use OCA\Theming\Service\BackgroundService;
+use OCP\AppFramework\Services\IAppConfig;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\SimpleFS\ISimpleFile;
@@ -29,6 +30,7 @@ class ImageManagerTest extends TestCase {
 	private LoggerInterface&MockObject $logger;
 	private ITempManager&MockObject $tempManager;
 	private ISimpleFolder&MockObject $rootFolder;
+	private IAppConfig&MockObject $appConfig;
 	protected ImageManager $imageManager;
 
 	protected function setUp(): void {
@@ -41,6 +43,7 @@ protected function setUp(): void {
 		$this->tempManager = $this->createMock(ITempManager::class);
 		$this->rootFolder = $this->createMock(ISimpleFolder::class);
 		$backgroundService = $this->createMock(BackgroundService::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->imageManager = new ImageManager(
 			$this->config,
 			$this->appData,
@@ -49,6 +52,7 @@ protected function setUp(): void {
 			$this->logger,
 			$this->tempManager,
 			$backgroundService,
+			$this->appConfig,
 		);
 		$this->appData
 			->expects($this->any())
@@ -96,25 +100,29 @@ public function mockGetImage($key, $file) {
 
 	public function testGetImageUrl(): void {
 		$this->checkImagick();
-		$this->config->expects($this->exactly(2))
+		$this->appConfig->expects($this->once())
+			->method('getAppValueInt')
+			->with('cachebuster')
+			->willReturn(0);
+		$this->config->expects($this->once())
 			->method('getAppValue')
-			->willReturnMap([
-				['theming', 'cachebuster', '0', '0'],
-				['theming', 'logoMime', '', '0'],
-			]);
+			->with('theming', 'logoMime', '')
+			->willReturn('image/png');
 		$this->urlGenerator->expects($this->once())
 			->method('linkToRoute')
 			->willReturn('url-to-image');
 		$this->assertEquals('url-to-image?v=0', $this->imageManager->getImageUrl('logo', false));
 	}
 
 	public function testGetImageUrlDefault(): void {
-		$this->config->expects($this->exactly(2))
+		$this->appConfig->expects($this->once())
+			->method('getAppValueInt')
+			->with('cachebuster')
+			->willReturn(0);
+		$this->config->expects($this->once())
 			->method('getAppValue')
-			->willReturnMap([
-				['theming', 'cachebuster', '0', '0'],
-				['theming', 'logoMime', '', ''],
-			]);
+			->with('theming', 'logoMime', '')
+			->willReturn('');
 		$this->urlGenerator->expects($this->once())
 			->method('imagePath')
 			->with('core', 'logo/logo.png')
@@ -124,12 +132,14 @@ public function testGetImageUrlDefault(): void {
 
 	public function testGetImageUrlAbsolute(): void {
 		$this->checkImagick();
-		$this->config->expects($this->exactly(2))
+		$this->appConfig->expects($this->once())
+			->method('getAppValueInt')
+			->with('cachebuster')
+			->willReturn(0);
+		$this->config->expects($this->once())
 			->method('getAppValue')
-			->willReturnMap([
-				['theming', 'cachebuster', '0', '0'],
-				['theming', 'logoMime', '', ''],
-			]);
+			->with('theming', 'logoMime', '')
+			->willReturn('');
 		$this->urlGenerator->expects($this->any())
 			->method('getAbsoluteUrl')
 			->willReturn('url-to-image-absolute?v=0');
@@ -212,10 +222,10 @@ public function testGetImageUnset(): void {
 
 	public function testGetCacheFolder(): void {
 		$folder = $this->createMock(ISimpleFolder::class);
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('theming', 'cachebuster', '0')
-			->willReturn('0');
+		$this->appConfig->expects($this->once())
+			->method('getAppValueInt')
+			->with('cachebuster')
+			->willReturn(0);
 		$this->rootFolder->expects($this->once())
 			->method('getFolder')
 			->with('0')
@@ -224,10 +234,10 @@ public function testGetCacheFolder(): void {
 	}
 	public function testGetCacheFolderCreate(): void {
 		$folder = $this->createMock(ISimpleFolder::class);
-		$this->config->expects($this->exactly(2))
-			->method('getAppValue')
-			->with('theming', 'cachebuster', '0')
-			->willReturn('0');
+		$this->appConfig->expects($this->exactly(2))
+			->method('getAppValueInt')
+			->with('cachebuster')
+			->willReturn(0);
 		$this->rootFolder->expects($this->exactly(2))
 			->method('getFolder')
 			->with('0')
@@ -303,10 +313,10 @@ public function testSetCachedImageCreate(): void {
 
 	private function setupCacheFolder() {
 		$folder = $this->createMock(ISimpleFolder::class);
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('theming', 'cachebuster', '0')
-			->willReturn('0');
+		$this->appConfig->expects($this->once())
+			->method('getAppValueInt')
+			->with('cachebuster')
+			->willReturn(0);
 		$this->rootFolder->expects($this->once())
 			->method('getFolder')
 			->with('0')
@@ -328,10 +338,10 @@ public function testCleanup(): void {
 		$folders[0]->expects($this->once())->method('delete');
 		$folders[1]->expects($this->once())->method('delete');
 		$folders[2]->expects($this->never())->method('delete');
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('theming', 'cachebuster', '0')
-			->willReturn('2');
+		$this->appConfig->expects($this->once())
+			->method('getAppValueInt')
+			->with('cachebuster')
+			->willReturn(2);
 		$this->rootFolder->expects($this->once())
 			->method('getDirectoryListing')
 			->willReturn($folders);
diff --git a/lib/private/Server.php b/lib/private/Server.php
@@ -1078,6 +1078,11 @@ public function __construct(
 					$c->get(LoggerInterface::class),
 					$c->get(ITempManager::class),
 					$backgroundService,
+					new AppConfig(
+						$c->get(IConfig::class),
+						$c->get(IAppConfig::class),
+						'theming',
+					),
 				);
 				return new ThemingDefaults(
 					new AppConfig(
