Skip to content

Commit

Permalink
fix: Use migration instead of repair step for restoring custom color
Browse files Browse the repository at this point in the history
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
susnux committed Sep 5, 2024
1 parent c8978f0 commit 2af436f
Show file tree
Hide file tree
Showing 7 changed files with 315 additions and 74 deletions.
1 change: 0 additions & 1 deletion apps/theming/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
<repair-steps>
<post-migration>
<step>OCA\Theming\Migration\InitBackgroundImagesMigration</step>
<step>OCA\Theming\Migration\SeparatePrimaryColorAndBackground</step>
</post-migration>
</repair-steps>

Expand Down
3 changes: 2 additions & 1 deletion apps/theming/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
'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\\Version2006Date20240902111627' => $baseDir . '/../lib/Migration/Version2006Date20240902111627.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',
Expand Down
3 changes: 2 additions & 1 deletion apps/theming/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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\\Version2006Date20240902111627' => __DIR__ . '/..' . '/../lib/Migration/Version2006Date20240902111627.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',
Expand Down
205 changes: 205 additions & 0 deletions apps/theming/lib/Jobs/RestoreBackgroundImageColor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Theming\Jobs;

use OCA\Theming\AppInfo\Application;
use OCA\Theming\Service\BackgroundService;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJobList;
use OCP\BackgroundJob\QueuedJob;
use OCP\Files\IAppData;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\IConfig;
use OCP\IDBConnection;
use Psr\Log\LoggerInterface;

class RestoreBackgroundImageColor extends QueuedJob {

public const STAGE_PREPARE = 'prepare';
public const STAGE_EXECUTE = 'execute';
// will be saved in appdata/theming/global/
protected const STATE_FILE_NAME = '30_background_image_color_restoration.json';

public function __construct(
ITimeFactory $time,
private IConfig $config,
private IAppData $appData,
private IJobList $jobList,
private IDBConnection $dbc,
private LoggerInterface $logger,
private BackgroundService $service,
) {
parent::__construct($time);
}

protected function run(mixed $argument): void {
if (!is_array($argument) || !isset($argument['stage'])) {
throw new \Exception('Job '.self::class.' called with wrong argument');
}

switch ($argument['stage']) {
case self::STAGE_PREPARE:
$this->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')

Check failure

Code scanning / Psalm

ImplicitToStringCast Error

Argument 2 of OCP\DB\QueryBuilder\IQueryBuilder::leftJoin expects string, but OCP\DB\QueryBuilder\IQueryFunction provided with a __toString method

Check failure on line 72 in apps/theming/lib/Jobs/RestoreBackgroundImageColor.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

ImplicitToStringCast

apps/theming/lib/Jobs/RestoreBackgroundImageColor.php:72:21: ImplicitToStringCast: Argument 2 of OCP\DB\QueryBuilder\IQueryBuilder::leftJoin expects string, but OCP\DB\QueryBuilder\IQueryFunction provided with a __toString method (see https://psalm.dev/060)
->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,
]
);
}
}
}
}
63 changes: 0 additions & 63 deletions apps/theming/lib/Migration/SeparatePrimaryColorAndBackground.php

This file was deleted.

88 changes: 88 additions & 0 deletions apps/theming/lib/Migration/Version2006Date20240902111627.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Theming\Migration;

use Closure;
use OCA\Theming\AppInfo\Application;
use OCA\Theming\Jobs\RestoreBackgroundImageColor;
use OCP\BackgroundJob\IJobList;
use OCP\IAppConfig;
use OCP\IDBConnection;
use OCP\Migration\IOutput;

// 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
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) {

Check notice

Code scanning / Psalm

MissingReturnType Note

Method OCA\Theming\Migration\Version2006Date20240905111627::preSchemaChange does not have a return type, expecting 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) {

Check notice

Code scanning / Psalm

MissingReturnType Note

Method OCA\Theming\Migration\Version2006Date20240905111627::postSchemaChange does not have a return type, expecting 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');
}
}
Loading

0 comments on commit 2af436f

Please sign in to comment.