diff options
author | Vincent Petry <vincent@nextcloud.com> | 2022-10-14 19:01:02 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-14 19:01:02 +0300 |
commit | e487d822ee10eabc64e060718a473bbc7b81cf0d (patch) | |
tree | 190b42ad24fc41802d4fd224e31150f705e01d1e /apps | |
parent | b99e478840214271d5612d0ae152c71cb63f10ea (diff) | |
parent | d094884057d7e499a2b9174c6009ee5968c2dd5d (diff) |
Merge pull request #34612 from nextcloud/stable25-fix-background-appdata-scrope
[stable25] Scope the appdata theming storage for global and users
Diffstat (limited to 'apps')
-rw-r--r-- | apps/theming/lib/Controller/UserThemeController.php | 11 | ||||
-rw-r--r-- | apps/theming/lib/ImageManager.php | 32 | ||||
-rw-r--r-- | apps/theming/lib/Jobs/MigrateBackgroundImages.php | 28 | ||||
-rw-r--r-- | apps/theming/lib/Listener/BeforeTemplateRenderedListener.php | 5 | ||||
-rw-r--r-- | apps/theming/lib/Service/BackgroundService.php | 28 | ||||
-rw-r--r-- | apps/theming/lib/Service/ThemeInjectionService.php | 30 | ||||
-rw-r--r-- | apps/theming/lib/Themes/DefaultTheme.php | 4 | ||||
-rw-r--r-- | apps/theming/src/UserThemes.vue | 4 | ||||
-rw-r--r-- | apps/theming/tests/ImageManagerTest.php | 32 |
9 files changed, 119 insertions, 55 deletions
diff --git a/apps/theming/lib/Controller/UserThemeController.php b/apps/theming/lib/Controller/UserThemeController.php index 091331706a7..635dad34736 100644 --- a/apps/theming/lib/Controller/UserThemeController.php +++ b/apps/theming/lib/Controller/UserThemeController.php @@ -156,7 +156,8 @@ class UserThemeController extends OCSController { * @NoAdminRequired */ public function setBackground(string $type = 'default', string $value = ''): JSONResponse { - $currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'backgroundVersion', '0'); + $currentVersion = (int)$this->config->getUserValue($this->userId, Application::APP_ID, 'userCacheBuster', '0'); + try { switch ($type) { case 'shipped': @@ -179,14 +180,14 @@ class UserThemeController extends OCSController { } catch (\Throwable $e) { return new JSONResponse(['error' => $e->getMessage()], Http::STATUS_INTERNAL_SERVER_ERROR); } + $currentVersion++; - $this->config->setUserValue($this->userId, Application::APP_ID, 'backgroundVersion', (string)$currentVersion); - // FIXME replace with user-specific cachebuster increase https://github.com/nextcloud/server/issues/34472 - $this->themingDefaults->increaseCacheBuster(); + $this->config->setUserValue($this->userId, Application::APP_ID, 'userCacheBuster', (string)$currentVersion); + return new JSONResponse([ 'type' => $type, 'value' => $value, - 'version' => $this->config->getUserValue($this->userId, Application::APP_ID, 'backgroundVersion', $currentVersion) + 'version' => $currentVersion, ]); } } diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index 6f9892af10e..60b695f1c90 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -66,14 +66,13 @@ class ImageManager { IURLGenerator $urlGenerator, ICacheFactory $cacheFactory, ILogger $logger, - ITempManager $tempManager - ) { + ITempManager $tempManager) { $this->config = $config; - $this->appData = $appData; $this->urlGenerator = $urlGenerator; $this->cacheFactory = $cacheFactory; $this->logger = $logger; $this->tempManager = $tempManager; + $this->appData = $appData; } public function getImageUrl(string $key, bool $useSvg = true): string { @@ -106,10 +105,12 @@ class ImageManager { */ public function getImage(string $key, bool $useSvg = true): ISimpleFile { $logo = $this->config->getAppValue('theming', $key . 'Mime', ''); - $folder = $this->appData->getFolder('images'); + $folder = $this->getRootFolder()->getFolder('images'); + if ($logo === '' || !$folder->fileExists($key)) { throw new NotFoundException(); } + if (!$useSvg && $this->shouldReplaceIcons()) { if (!$folder->fileExists($key . '.png')) { try { @@ -127,6 +128,7 @@ class ImageManager { return $folder->getFile($key . '.png'); } } + return $folder->getFile($key); } @@ -158,9 +160,9 @@ class ImageManager { public function getCacheFolder(): ISimpleFolder { $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); try { - $folder = $this->appData->getFolder($cacheBusterValue); + $folder = $this->getRootFolder()->getFolder($cacheBusterValue); } catch (NotFoundException $e) { - $folder = $this->appData->newFolder($cacheBusterValue); + $folder = $this->getRootFolder()->newFolder($cacheBusterValue); $this->cleanup(); } return $folder; @@ -202,13 +204,13 @@ class ImageManager { public function delete(string $key): void { /* ignore exceptions, since we don't want to fail hard if something goes wrong during cleanup */ try { - $file = $this->appData->getFolder('images')->getFile($key); + $file = $this->getRootFolder()->getFolder('images')->getFile($key); $file->delete(); } catch (NotFoundException $e) { } catch (NotPermittedException $e) { } try { - $file = $this->appData->getFolder('images')->getFile($key . '.png'); + $file = $this->getRootFolder()->getFolder('images')->getFile($key . '.png'); $file->delete(); } catch (NotFoundException $e) { } catch (NotPermittedException $e) { @@ -219,9 +221,9 @@ class ImageManager { $this->delete($key); try { - $folder = $this->appData->getFolder('images'); + $folder = $this->getRootFolder()->getFolder('images'); } catch (NotFoundException $e) { - $folder = $this->appData->newFolder('images'); + $folder = $this->getRootFolder()->newFolder('images'); } $target = $folder->newFile($key); @@ -288,7 +290,7 @@ class ImageManager { */ public function cleanup() { $currentFolder = $this->getCacheFolder(); - $folders = $this->appData->getDirectoryListing(); + $folders = $this->getRootFolder()->getDirectoryListing(); foreach ($folders as $folder) { if ($folder->getName() !== 'images' && $folder->getName() !== $currentFolder->getName()) { $folder->delete(); @@ -316,4 +318,12 @@ class ImageManager { $cache->set('shouldReplaceIcons', $value); return $value; } + + private function getRootFolder(): ISimpleFolder { + try { + return $this->appData->getFolder('global'); + } catch (NotFoundException $e) { + return $this->appData->newFolder('global'); + } + } } diff --git a/apps/theming/lib/Jobs/MigrateBackgroundImages.php b/apps/theming/lib/Jobs/MigrateBackgroundImages.php index 97806fa600a..b816a4c8775 100644 --- a/apps/theming/lib/Jobs/MigrateBackgroundImages.php +++ b/apps/theming/lib/Jobs/MigrateBackgroundImages.php @@ -34,6 +34,7 @@ use OCP\BackgroundJob\QueuedJob; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; +use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; class MigrateBackgroundImages extends QueuedJob { @@ -57,7 +58,6 @@ class MigrateBackgroundImages extends QueuedJob { return; } - $themingData = $this->appDataFactory->get(Application::APP_ID); $dashboardData = $this->appDataFactory->get('dashboard'); $userIds = $this->config->getUsersForUserValue('theming', 'background', 'custom'); @@ -79,11 +79,8 @@ class MigrateBackgroundImages extends QueuedJob { // migration $file = $dashboardData->getFolder($userId)->getFile('background.jpg'); - try { - $targetDir = $themingData->getFolder($userId); - } catch (NotFoundException $e) { - $targetDir = $themingData->newFolder($userId); - } + $targetDir = $this->getUserFolder($userId); + if (!$targetDir->fileExists('background.jpg')) { $targetDir->newFile('background.jpg', $file->getContent()); } @@ -104,4 +101,23 @@ class MigrateBackgroundImages extends QueuedJob { $this->jobList->add(self::class); } } + + /** + * Get the root location for users theming data + */ + protected function getUserFolder(string $userId): ISimpleFolder { + $themingData = $this->appDataFactory->get(Application::APP_ID); + + try { + $rootFolder = $themingData->getFolder('users'); + } catch (NotFoundException $e) { + $rootFolder = $themingData->newFolder('users'); + } + + try { + return $rootFolder->getFolder($userId); + } catch (NotFoundException $e) { + return $rootFolder->newFolder($userId); + } + } } diff --git a/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php b/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php index 1d28a1d4399..a6e0923e643 100644 --- a/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php +++ b/apps/theming/lib/Listener/BeforeTemplateRenderedListener.php @@ -90,11 +90,6 @@ class BeforeTemplateRenderedListener implements IEventListener { ); $this->initialState->provideInitialState( - 'backgroundVersion', - $this->config->getUserValue($userId, Application::APP_ID, 'backgroundVersion', 0), - ); - - $this->initialState->provideInitialState( 'themingDefaultBackground', $this->config->getAppValue('theming', 'backgroundMime', ''), ); diff --git a/apps/theming/lib/Service/BackgroundService.php b/apps/theming/lib/Service/BackgroundService.php index 36623735728..3daac7e7215 100644 --- a/apps/theming/lib/Service/BackgroundService.php +++ b/apps/theming/lib/Service/BackgroundService.php @@ -136,19 +136,19 @@ class BackgroundService { private string $userId; private IAppDataFactory $appDataFactory; - public function __construct( - IRootFolder $rootFolder, - IAppDataFactory $appDataFactory, - IConfig $config, - ?string $userId - ) { + public function __construct(IRootFolder $rootFolder, + IAppData $appData, + IConfig $config, + ?string $userId, + IAppDataFactory $appDataFactory) { if ($userId === null) { return; } + $this->rootFolder = $rootFolder; - $this->appData = $appDataFactory->get(Application::APP_ID); $this->config = $config; $this->userId = $userId; + $this->appData = $appData; $this->appDataFactory = $appDataFactory; } @@ -167,12 +167,15 @@ class BackgroundService { public function setFileBackground($path): void { $this->config->setUserValue($this->userId, Application::APP_ID, 'background', 'custom'); $userFolder = $this->rootFolder->getUserFolder($this->userId); + /** @var File $file */ $file = $userFolder->get($path); $image = new \OCP\Image(); + if ($image->loadFromFileHandle($file->fopen('r')) === false) { throw new InvalidArgumentException('Invalid image file'); } + $this->getAppDataFolder()->newFile('background.jpg', $file->fopen('r')); } @@ -207,14 +210,21 @@ class BackgroundService { } /** + * Storing the data in appdata/theming/users/USERID + * * @return ISimpleFolder * @throws NotPermittedException */ private function getAppDataFolder(): ISimpleFolder { try { - return $this->appData->getFolder($this->userId); + $rootFolder = $this->appData->getFolder('users'); + } catch (NotFoundException $e) { + $rootFolder = $this->appData->newFolder('users'); + } + try { + return $rootFolder->getFolder($this->userId); } catch (NotFoundException $e) { - return $this->appData->newFolder($this->userId); + return $rootFolder->newFolder($this->userId); } } } diff --git a/apps/theming/lib/Service/ThemeInjectionService.php b/apps/theming/lib/Service/ThemeInjectionService.php index fec14de96cf..27b65457e7f 100644 --- a/apps/theming/lib/Service/ThemeInjectionService.php +++ b/apps/theming/lib/Service/ThemeInjectionService.php @@ -22,8 +22,11 @@ */ namespace OCA\Theming\Service; +use OCA\Theming\AppInfo\Application; use OCA\Theming\Themes\DefaultTheme; +use OCP\IConfig; use OCP\IURLGenerator; +use OCP\IUserSession; use OCP\Util; class ThemeInjectionService { @@ -31,13 +34,23 @@ class ThemeInjectionService { private IURLGenerator $urlGenerator; private ThemesService $themesService; private DefaultTheme $defaultTheme; + private IConfig $config; + private ?string $userId; public function __construct(IURLGenerator $urlGenerator, ThemesService $themesService, - DefaultTheme $defaultTheme) { + DefaultTheme $defaultTheme, + IConfig $config, + IUserSession $userSession) { $this->urlGenerator = $urlGenerator; $this->themesService = $themesService; $this->defaultTheme = $defaultTheme; + $this->config = $config; + if ($userSession->getUser() !== null) { + $this->userId = $userSession->getUser()->getUID(); + } else { + $this->userId = null; + } } public function injectHeaders() { @@ -50,13 +63,13 @@ class ThemeInjectionService { // Default theme fallback $this->addThemeHeader($defaultTheme->getId()); - + // Themes applied by media queries foreach($mediaThemes as $theme) { $this->addThemeHeader($theme->getId(), true, $theme->getMediaQuery()); } - // Themes + // Themes foreach($this->themesService->getThemes() as $theme) { // Ignore default theme as already processed first if ($theme->getId() === $this->defaultTheme->getId()) { @@ -68,15 +81,24 @@ class ThemeInjectionService { /** * Inject theme header into rendered page - * + * * @param string $themeId the theme ID * @param bool $plain request the :root syntax * @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), ]); Util::addHeader('link', [ 'rel' => 'stylesheet', diff --git a/apps/theming/lib/Themes/DefaultTheme.php b/apps/theming/lib/Themes/DefaultTheme.php index 4dce1dca809..aae4c4eca4c 100644 --- a/apps/theming/lib/Themes/DefaultTheme.php +++ b/apps/theming/lib/Themes/DefaultTheme.php @@ -239,9 +239,11 @@ class DefaultTheme implements ITheme { $user = $this->userSession->getUser(); if ($appManager->isEnabledForUser(Application::APP_ID) && $user !== null) { $themingBackground = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background', 'default'); + $currentVersion = (int)$this->config->getUserValue($user->getUID(), Application::APP_ID, 'userCacheBuster', '0'); if ($themingBackground === 'custom') { - $variables['--image-main-background'] = "url('" . $this->urlGenerator->linkToRouteAbsolute('theming.userTheme.getBackground') . "')"; + $cacheBuster = substr(sha1($user->getUID() . '_' . $currentVersion), 0, 8); + $variables['--image-main-background'] = "url('" . $this->urlGenerator->linkToRouteAbsolute('theming.userTheme.getBackground') . "?v=$cacheBuster')"; } elseif (isset(BackgroundService::SHIPPED_BACKGROUNDS[$themingBackground])) { $variables['--image-main-background'] = "url('" . $this->urlGenerator->linkTo(Application::APP_ID, "/img/background/$themingBackground") . "')"; } elseif (substr($themingBackground, 0, 1) === '#') { diff --git a/apps/theming/src/UserThemes.vue b/apps/theming/src/UserThemes.vue index 1eeeb3c5b21..f9e729afecb 100644 --- a/apps/theming/src/UserThemes.vue +++ b/apps/theming/src/UserThemes.vue @@ -88,7 +88,6 @@ const enforceTheme = loadState('theming', 'enforceTheme', '') const shortcutsDisabled = loadState('theming', 'shortcutsDisabled', false) const background = loadState('theming', 'background') -const backgroundVersion = loadState('theming', 'backgroundVersion') const themingDefaultBackground = loadState('theming', 'themingDefaultBackground') const shippedBackgroundList = loadState('theming', 'shippedBackgrounds') @@ -109,7 +108,6 @@ export default { enforceTheme, shortcutsDisabled, background, - backgroundVersion, themingDefaultBackground, } }, @@ -169,10 +167,10 @@ export default { methods: { updateBackground(data) { this.background = (data.type === 'custom' || data.type === 'default') ? data.type : data.value - this.backgroundVersion = data.version this.updateGlobalStyles() this.$emit('update:background') }, + updateGlobalStyles() { // Override primary-invert-if-bright and color-primary-text if background is set const isBackgroundBright = shippedBackgroundList[this.background]?.theming === 'dark' diff --git a/apps/theming/tests/ImageManagerTest.php b/apps/theming/tests/ImageManagerTest.php index ead9ca113e6..ffb023c970f 100644 --- a/apps/theming/tests/ImageManagerTest.php +++ b/apps/theming/tests/ImageManagerTest.php @@ -56,6 +56,8 @@ class ImageManagerTest extends TestCase { private $logger; /** @var ITempManager|MockObject */ private $tempManager; + /** @var ISimpleFolder|MockObject */ + private $rootFolder; protected function setUp(): void { parent::setUp(); @@ -65,6 +67,7 @@ class ImageManagerTest extends TestCase { $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->logger = $this->createMock(ILogger::class); $this->tempManager = $this->createMock(ITempManager::class); + $this->rootFolder = $this->createMock(ISimpleFolder::class); $this->imageManager = new ImageManager( $this->config, $this->appData, @@ -73,6 +76,11 @@ class ImageManagerTest extends TestCase { $this->logger, $this->tempManager ); + $this->appData + ->expects($this->any()) + ->method('getFolder') + ->with('global') + ->willReturn($this->rootFolder); } private function checkImagick() { @@ -120,7 +128,7 @@ class ImageManagerTest extends TestCase { ->willReturn($newFile); $newFile->expects($this->once()) ->method('putContent'); - $this->appData->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('getFolder') ->with('images') ->willReturn($folder); @@ -200,7 +208,7 @@ class ImageManagerTest extends TestCase { ->method('getAppValue') ->with('theming', 'cachebuster', '0') ->willReturn('0'); - $this->appData->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('getFolder') ->with('0') ->willReturn($folder); @@ -212,18 +220,18 @@ class ImageManagerTest extends TestCase { ->method('getAppValue') ->with('theming', 'cachebuster', '0') ->willReturn('0'); - $this->appData->expects($this->exactly(2)) + $this->rootFolder->expects($this->exactly(2)) ->method('getFolder') ->with('0') ->willReturnOnConsecutiveCalls( $this->throwException(new NotFoundException()), $folder, ); - $this->appData->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('newFolder') ->with('0') ->willReturn($folder); - $this->appData->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('getDirectoryListing') ->willReturn([]); $this->assertEquals($folder, $this->imageManager->getCacheFolder()); @@ -291,7 +299,7 @@ class ImageManagerTest extends TestCase { ->method('getAppValue') ->with('theming', 'cachebuster', '0') ->willReturn('0'); - $this->appData->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('getFolder') ->with('0') ->willReturn($folder); @@ -316,10 +324,10 @@ class ImageManagerTest extends TestCase { ->method('getAppValue') ->with('theming','cachebuster','0') ->willReturn('2'); - $this->appData->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('getDirectoryListing') ->willReturn($folders); - $this->appData->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('getFolder') ->with('2') ->willReturn($folders[2]); @@ -346,24 +354,26 @@ class ImageManagerTest extends TestCase { $folder->expects($this->any()) ->method('getFile') ->willReturn($oldFile); + if ($folderExists) { - $this->appData + $this->rootFolder ->expects($this->any()) ->method('getFolder') ->with('images') ->willReturn($folder); } else { - $this->appData + $this->rootFolder ->expects($this->any()) ->method('getFolder') ->with('images') ->willThrowException(new NotFoundException()); - $this->appData + $this->rootFolder ->expects($this->any()) ->method('newFolder') ->with('images') ->willReturn($folder); } + $folder->expects($this->once()) ->method('newFile') ->with($key) |