From 531862d47477dfc3868240895a00a39c4dd6ac8e Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 2 Sep 2024 13:24:20 +0200 Subject: fix: Use migration instead of repair step for restoring custom color Signed-off-by: Ferdinand Thiessen --- apps/theming/appinfo/info.xml | 1 - .../composer/composer/autoload_classmap.php | 3 +- apps/theming/composer/composer/autoload_static.php | 3 +- .../lib/Jobs/RestoreBackgroundImageColor.php | 205 +++++++++++++++++++++ .../SeparatePrimaryColorAndBackground.php | 63 ------- .../Migration/Version2006Date20240905111627.php | 88 +++++++++ apps/theming/lib/Service/BackgroundService.php | 26 ++- 7 files changed, 315 insertions(+), 74 deletions(-) create mode 100644 apps/theming/lib/Jobs/RestoreBackgroundImageColor.php delete mode 100644 apps/theming/lib/Migration/SeparatePrimaryColorAndBackground.php create mode 100644 apps/theming/lib/Migration/Version2006Date20240905111627.php (limited to 'apps') diff --git a/apps/theming/appinfo/info.xml b/apps/theming/appinfo/info.xml index bccfb04f70a..2eac0fd5058 100644 --- a/apps/theming/appinfo/info.xml +++ b/apps/theming/appinfo/info.xml @@ -27,7 +27,6 @@ OCA\Theming\Migration\InitBackgroundImagesMigration - OCA\Theming\Migration\SeparatePrimaryColorAndBackground diff --git a/apps/theming/composer/composer/autoload_classmap.php b/apps/theming/composer/composer/autoload_classmap.php index e69caafb007..9b53c0f9fea 100644 --- a/apps/theming/composer/composer/autoload_classmap.php +++ b/apps/theming/composer/composer/autoload_classmap.php @@ -17,10 +17,11 @@ return array( 'OCA\\Theming\\IconBuilder' => $baseDir . '/../lib/IconBuilder.php', 'OCA\\Theming\\ImageManager' => $baseDir . '/../lib/ImageManager.php', 'OCA\\Theming\\Jobs\\MigrateBackgroundImages' => $baseDir . '/../lib/Jobs/MigrateBackgroundImages.php', + 'OCA\\Theming\\Jobs\\RestoreBackgroundImageColor' => $baseDir . '/../lib/Jobs/RestoreBackgroundImageColor.php', 'OCA\\Theming\\Listener\\BeforePreferenceListener' => $baseDir . '/../lib/Listener/BeforePreferenceListener.php', 'OCA\\Theming\\Listener\\BeforeTemplateRenderedListener' => $baseDir . '/../lib/Listener/BeforeTemplateRenderedListener.php', 'OCA\\Theming\\Migration\\InitBackgroundImagesMigration' => $baseDir . '/../lib/Migration/InitBackgroundImagesMigration.php', - 'OCA\\Theming\\Migration\\SeparatePrimaryColorAndBackground' => $baseDir . '/../lib/Migration/SeparatePrimaryColorAndBackground.php', + 'OCA\\Theming\\Migration\\Version2006Date20240905111627' => $baseDir . '/../lib/Migration/Version2006Date20240905111627.php', 'OCA\\Theming\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php', 'OCA\\Theming\\Service\\BackgroundService' => $baseDir . '/../lib/Service/BackgroundService.php', 'OCA\\Theming\\Service\\JSDataService' => $baseDir . '/../lib/Service/JSDataService.php', diff --git a/apps/theming/composer/composer/autoload_static.php b/apps/theming/composer/composer/autoload_static.php index df1c0579b00..184d9ed0761 100644 --- a/apps/theming/composer/composer/autoload_static.php +++ b/apps/theming/composer/composer/autoload_static.php @@ -32,10 +32,11 @@ class ComposerStaticInitTheming 'OCA\\Theming\\IconBuilder' => __DIR__ . '/..' . '/../lib/IconBuilder.php', 'OCA\\Theming\\ImageManager' => __DIR__ . '/..' . '/../lib/ImageManager.php', 'OCA\\Theming\\Jobs\\MigrateBackgroundImages' => __DIR__ . '/..' . '/../lib/Jobs/MigrateBackgroundImages.php', + 'OCA\\Theming\\Jobs\\RestoreBackgroundImageColor' => __DIR__ . '/..' . '/../lib/Jobs/RestoreBackgroundImageColor.php', 'OCA\\Theming\\Listener\\BeforePreferenceListener' => __DIR__ . '/..' . '/../lib/Listener/BeforePreferenceListener.php', 'OCA\\Theming\\Listener\\BeforeTemplateRenderedListener' => __DIR__ . '/..' . '/../lib/Listener/BeforeTemplateRenderedListener.php', 'OCA\\Theming\\Migration\\InitBackgroundImagesMigration' => __DIR__ . '/..' . '/../lib/Migration/InitBackgroundImagesMigration.php', - 'OCA\\Theming\\Migration\\SeparatePrimaryColorAndBackground' => __DIR__ . '/..' . '/../lib/Migration/SeparatePrimaryColorAndBackground.php', + 'OCA\\Theming\\Migration\\Version2006Date20240905111627' => __DIR__ . '/..' . '/../lib/Migration/Version2006Date20240905111627.php', 'OCA\\Theming\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php', 'OCA\\Theming\\Service\\BackgroundService' => __DIR__ . '/..' . '/../lib/Service/BackgroundService.php', 'OCA\\Theming\\Service\\JSDataService' => __DIR__ . '/..' . '/../lib/Service/JSDataService.php', diff --git a/apps/theming/lib/Jobs/RestoreBackgroundImageColor.php b/apps/theming/lib/Jobs/RestoreBackgroundImageColor.php new file mode 100644 index 00000000000..1ec659d0646 --- /dev/null +++ b/apps/theming/lib/Jobs/RestoreBackgroundImageColor.php @@ -0,0 +1,205 @@ +runPreparation(); + break; + case self::STAGE_EXECUTE: + $this->runMigration(); + break; + default: + break; + } + } + + protected function runPreparation(): void { + try { + $qb = $this->dbc->getQueryBuilder(); + $qb2 = $this->dbc->getQueryBuilder(); + + $innerSQL = $qb2->select('userid') + ->from('preferences') + ->where($qb2->expr()->eq('configkey', $qb->createNamedParameter('background_color'))); + + // Get those users, that have a background_image set - not the default, but no background_color. + $result = $qb->selectDistinct('a.userid') + ->from('preferences', 'a') + ->leftJoin('a', $qb->createFunction('('.$innerSQL->getSQL().')'), 'b', 'a.userid = b.userid') + ->where($qb2->expr()->eq('a.configkey', $qb->createNamedParameter('background_image'))) + ->andWhere($qb2->expr()->neq('a.configvalue', $qb->createNamedParameter(BackgroundService::BACKGROUND_DEFAULT))) + ->andWhere($qb2->expr()->isNull('b.userid')) + ->executeQuery(); + + $userIds = $result->fetchAll(\PDO::FETCH_COLUMN); + $this->logger->info('Prepare to restore background information for {users} users', ['users' => count($userIds)]); + $this->storeUserIdsToProcess($userIds); + } catch (\Throwable $t) { + $this->jobList->add(self::class, ['stage' => self::STAGE_PREPARE]); + throw $t; + } + $this->jobList->add(self::class, ['stage' => self::STAGE_EXECUTE]); + } + + /** + * @throws NotPermittedException + * @throws NotFoundException + */ + protected function runMigration(): void { + $allUserIds = $this->readUserIdsToProcess(); + $notSoFastMode = count($allUserIds) > 1000; + + $userIds = array_slice($allUserIds, 0, 1000); + foreach ($userIds as $userId) { + $backgroundColor = $this->config->getUserValue($userId, Application::APP_ID, 'background_color'); + if ($backgroundColor !== '') { + continue; + } + + $background = $this->config->getUserValue($userId, Application::APP_ID, 'background_image'); + switch($background) { + case BackgroundService::BACKGROUND_DEFAULT: + $this->service->setDefaultBackground($userId); + break; + case BackgroundService::BACKGROUND_COLOR: + break; + case BackgroundService::BACKGROUND_CUSTOM: + $this->service->recalculateMeanColor($userId); + break; + default: + // shipped backgrounds + // do not alter primary color + $primary = $this->config->getUserValue($userId, Application::APP_ID, 'primary_color'); + if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$background])) { + $this->service->setShippedBackground($background, $userId); + } else { + $this->service->setDefaultBackground($userId); + } + // Restore primary + if ($primary !== '') { + $this->config->setUserValue($userId, Application::APP_ID, 'primary_color', $primary); + } + } + } + + if ($notSoFastMode) { + $remainingUserIds = array_slice($allUserIds, 1000); + $this->storeUserIdsToProcess($remainingUserIds); + $this->jobList->add(self::class, ['stage' => self::STAGE_EXECUTE]); + } else { + $this->deleteStateFile(); + } + } + + /** + * @throws NotPermittedException + * @throws NotFoundException + */ + protected function readUserIdsToProcess(): array { + $globalFolder = $this->appData->getFolder('global'); + if ($globalFolder->fileExists(self::STATE_FILE_NAME)) { + $file = $globalFolder->getFile(self::STATE_FILE_NAME); + try { + $userIds = \json_decode($file->getContent(), true); + } catch (NotFoundException $e) { + $userIds = []; + } + if ($userIds === null) { + $userIds = []; + } + } else { + $userIds = []; + } + return $userIds; + } + + /** + * @throws NotFoundException + */ + protected function storeUserIdsToProcess(array $userIds): void { + $storableUserIds = \json_encode($userIds); + $globalFolder = $this->appData->getFolder('global'); + try { + if ($globalFolder->fileExists(self::STATE_FILE_NAME)) { + $file = $globalFolder->getFile(self::STATE_FILE_NAME); + } else { + $file = $globalFolder->newFile(self::STATE_FILE_NAME); + } + $file->putContent($storableUserIds); + } catch (NotFoundException $e) { + } catch (NotPermittedException $e) { + $this->logger->warning('Lacking permissions to create {file}', + [ + 'app' => 'theming', + 'file' => self::STATE_FILE_NAME, + 'exception' => $e, + ] + ); + } + } + + /** + * @throws NotFoundException + */ + protected function deleteStateFile(): void { + $globalFolder = $this->appData->getFolder('global'); + if ($globalFolder->fileExists(self::STATE_FILE_NAME)) { + $file = $globalFolder->getFile(self::STATE_FILE_NAME); + try { + $file->delete(); + } catch (NotPermittedException $e) { + $this->logger->info('Could not delete {file} due to permissions. It is safe to delete manually inside data -> appdata -> theming -> global.', + [ + 'app' => 'theming', + 'file' => $file->getName(), + 'exception' => $e, + ] + ); + } + } + } +} diff --git a/apps/theming/lib/Migration/SeparatePrimaryColorAndBackground.php b/apps/theming/lib/Migration/SeparatePrimaryColorAndBackground.php deleted file mode 100644 index 4fe26d0ba89..00000000000 --- a/apps/theming/lib/Migration/SeparatePrimaryColorAndBackground.php +++ /dev/null @@ -1,63 +0,0 @@ -appConfig->getValueString(Application::APP_ID, 'color', ''); - if ($defaultColor !== '') { - // Restore legacy value into new field - $this->appConfig->setValueString(Application::APP_ID, 'background_color', $defaultColor); - $this->appConfig->setValueString(Application::APP_ID, 'primary_color', $defaultColor); - // Delete legacy field - $this->appConfig->deleteKey(Application::APP_ID, 'color'); - // give some feedback - $output->info('Global primary color restored'); - } - - // This can only be executed once because `background_color` is again used with Nextcloud 30, - // so this part only works when updating -> Nextcloud 29 -> 30 - if ($this->appConfig->getValueBool('theming', 'nextcloud_30_migration')) { - return; - } - - $userThemingEnabled = $this->appConfig->getValueBool('theming', 'disable-user-theming'); - if ($userThemingEnabled) { - $output->info('Restoring user primary color'); - // For performance let the DB handle this - $qb = $this->connection->getQueryBuilder(); - // Rename the `background_color` config to `primary_color` as this was the behavior on Nextcloud 29 and older - // with Nextcloud 30 `background_color` is a new option to define the background color independent of the primary color. - $qb->update('preferences') - ->set('configkey', $qb->createNamedParameter('primary_color')) - ->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID))) - ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('background_color'))); - $qb->executeStatement(); - $output->info('Primary color of users restored'); - } - $this->appConfig->setValueBool('theming', 'nextcloud_30_migration', true); - } -} diff --git a/apps/theming/lib/Migration/Version2006Date20240905111627.php b/apps/theming/lib/Migration/Version2006Date20240905111627.php new file mode 100644 index 00000000000..dcec954f585 --- /dev/null +++ b/apps/theming/lib/Migration/Version2006Date20240905111627.php @@ -0,0 +1,88 @@ + Nextcloud 29 -> 30 +class Version2006Date20240905111627 implements \OCP\Migration\IMigrationStep { + + public function __construct( + private IJobList $jobList, + private IAppConfig $appConfig, + private IDBConnection $connection, + ) { + } + + public function name(): string { + return 'Restore custom primary color'; + } + + public function description(): string { + return 'Restore custom primary color after separating primary color from background color'; + } + + public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + // nop + } + + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { + $this->restoreSystemColors($output); + + $userThemingEnabled = $this->appConfig->getValueBool('theming', 'disable-user-theming') === false; + if ($userThemingEnabled) { + $this->restoreUserColors($output); + } + + return null; + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $output->info('Initialize restoring of background colors for custom background images'); + // This is done in a background job as this can take a lot of time for large instances + $this->jobList->add(RestoreBackgroundImageColor::class, ['stage' => RestoreBackgroundImageColor::STAGE_PREPARE]); + } + + private function restoreSystemColors(IOutput $output): void { + $defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'color', ''); + if ($defaultColor === '') { + $output->info('No custom system color configured - skipping'); + } else { + // Restore legacy value into new field + $this->appConfig->setValueString(Application::APP_ID, 'background_color', $defaultColor); + $this->appConfig->setValueString(Application::APP_ID, 'primary_color', $defaultColor); + // Delete legacy field + $this->appConfig->deleteKey(Application::APP_ID, 'color'); + // give some feedback + $output->info('Global primary color restored'); + } + } + + private function restoreUserColors(IOutput $output): void { + $output->info('Restoring user primary color'); + // For performance let the DB handle this + $qb = $this->connection->getQueryBuilder(); + // Rename the `background_color` config to `primary_color` as this was the behavior on Nextcloud 29 and older + // with Nextcloud 30 `background_color` is a new option to define the background color independent of the primary color. + $qb->update('preferences') + ->set('configkey', $qb->createNamedParameter('primary_color')) + ->where($qb->expr()->eq('appid', $qb->createNamedParameter(Application::APP_ID))) + ->andWhere($qb->expr()->eq('configkey', $qb->createNamedParameter('background_color'))); + $qb->executeStatement(); + $output->info('Primary color of users restored'); + } +} diff --git a/apps/theming/lib/Service/BackgroundService.php b/apps/theming/lib/Service/BackgroundService.php index b7b2cf8e221..a0158dcb667 100644 --- a/apps/theming/lib/Service/BackgroundService.php +++ b/apps/theming/lib/Service/BackgroundService.php @@ -207,7 +207,7 @@ class BackgroundService { public function setDefaultBackground(?string $userId = null): void { $userId = $userId ?? $this->userId; - if ($this->userId === null) { + if ($userId === null) { throw new RuntimeException('No currently logged-in user'); } @@ -236,15 +236,18 @@ class BackgroundService { if ($handle === false) { throw new InvalidArgumentException('Invalid image file'); } - $this->getAppDataFolder()->newFile('background.jpg', $file->fopen('r')); + $this->getAppDataFolder()->newFile('background.jpg', $handle); $this->recalculateMeanColor(); } public function recalculateMeanColor(?string $userId = null): void { $userId = $userId ?? $this->userId; - $image = new \OCP\Image(); + if ($userId === null) { + throw new RuntimeException('No currently logged-in user'); + } + $image = new \OCP\Image(); $handle = $this->getAppDataFolder($userId)->getFile('background.jpg')->read(); if ($handle === false || $image->loadFromFileHandle($handle) === false) { throw new InvalidArgumentException('Invalid image file'); @@ -272,23 +275,25 @@ class BackgroundService { if (!array_key_exists($filename, self::SHIPPED_BACKGROUNDS)) { throw new InvalidArgumentException('The given file name is invalid'); } - $this->setColorBackground(self::SHIPPED_BACKGROUNDS[$filename]['background_color']); + $this->setColorBackground(self::SHIPPED_BACKGROUNDS[$filename]['background_color'], $userId); $this->config->setUserValue($userId, Application::APP_ID, 'background_image', $filename); $this->config->setUserValue($userId, Application::APP_ID, 'primary_color', self::SHIPPED_BACKGROUNDS[$filename]['primary_color']); } /** * Set the background to color only + * @param string|null $userId The user to set the color - default to current logged-in user */ - public function setColorBackground(string $color): void { - if ($this->userId === null) { + public function setColorBackground(string $color, ?string $userId = null): void { + $userId = $userId ?? $this->userId; + if ($userId === null) { throw new RuntimeException('No currently logged-in user'); } if (!preg_match('/^#([0-9a-f]{3}|[0-9a-f]{6})$/i', $color)) { throw new InvalidArgumentException('The given color is invalid'); } - $this->config->setUserValue($this->userId, Application::APP_ID, 'background_color', $color); - $this->config->setUserValue($this->userId, Application::APP_ID, 'background_image', self::BACKGROUND_COLOR); + $this->config->setUserValue($userId, Application::APP_ID, 'background_color', $color); + $this->config->setUserValue($userId, Application::APP_ID, 'background_image', self::BACKGROUND_COLOR); } public function deleteBackgroundImage(): void { @@ -393,6 +398,11 @@ class BackgroundService { * @throws NotPermittedException */ private function getAppDataFolder(?string $userId = null): ISimpleFolder { + $userId = $userId ?? $this->userId; + if ($userId === null) { + throw new RuntimeException('No currently logged-in user'); + } + try { $rootFolder = $this->appData->getFolder('users'); } catch (NotFoundException $e) { -- cgit v1.2.3