Merge pull request #47586 from nextcloud/fix/color

fix(theming): Add migration to restore primary color after separating primary and background
This commit is contained in:
Ferdinand Thiessen 2024-09-11 15:10:09 +02:00 committed by GitHub
commit 5fc514877b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 464 additions and 119 deletions

View file

@ -17,9 +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\\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',

View file

@ -32,9 +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\\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',

View file

@ -22,6 +22,7 @@ use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\NotFoundResponse;
use OCP\AppFramework\Services\IAppConfig;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\IConfig;
@ -41,37 +42,20 @@ use ScssPhp\ScssPhp\Compiler;
class ThemingController extends Controller {
public const VALID_UPLOAD_KEYS = ['header', 'logo', 'logoheader', 'background', 'favicon'];
private ThemingDefaults $themingDefaults;
private IL10N $l10n;
private IConfig $config;
private IURLGenerator $urlGenerator;
private IAppManager $appManager;
private ImageManager $imageManager;
private ThemesService $themesService;
private INavigationManager $navigationManager;
public function __construct(
$appName,
IRequest $request,
IConfig $config,
ThemingDefaults $themingDefaults,
IL10N $l,
IURLGenerator $urlGenerator,
IAppManager $appManager,
ImageManager $imageManager,
ThemesService $themesService,
INavigationManager $navigationManager,
private IConfig $config,
private IAppConfig $appConfig,
private ThemingDefaults $themingDefaults,
private IL10N $l10n,
private IURLGenerator $urlGenerator,
private IAppManager $appManager,
private ImageManager $imageManager,
private ThemesService $themesService,
private INavigationManager $navigationManager,
) {
parent::__construct($appName, $request);
$this->themingDefaults = $themingDefaults;
$this->l10n = $l;
$this->config = $config;
$this->urlGenerator = $urlGenerator;
$this->appManager = $appManager;
$this->imageManager = $imageManager;
$this->themesService = $themesService;
$this->navigationManager = $navigationManager;
}
/**
@ -84,6 +68,7 @@ class ThemingController extends Controller {
public function updateStylesheet($setting, $value) {
$value = trim($value);
$error = null;
$saved = false;
switch ($setting) {
case 'name':
if (strlen($value) > 250) {
@ -122,16 +107,25 @@ class ThemingController extends Controller {
case 'primary_color':
if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
$error = $this->l10n->t('The given color is invalid');
} else {
$this->appConfig->setAppValueString('primary_color', $value);
$saved = true;
}
break;
case 'background_color':
if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
$error = $this->l10n->t('The given color is invalid');
} else {
$this->appConfig->setAppValueString('background_color', $value);
$saved = true;
}
break;
case 'disable-user-theming':
if ($value !== 'yes' && $value !== 'no') {
if (!in_array($value, ['yes', 'true', 'no', 'false'])) {
$error = $this->l10n->t('Disable-user-theming should be true or false');
} else {
$this->appConfig->setAppValueBool('disable-user-theming', $value === 'yes' || $value === 'true');
$saved = true;
}
break;
}
@ -144,7 +138,9 @@ class ThemingController extends Controller {
], Http::STATUS_BAD_REQUEST);
}
$this->themingDefaults->set($setting, $value);
if (!$saved) {
$this->themingDefaults->set($setting, $value);
}
return new DataResponse([
'data' => [

View file

@ -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')
->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,
]
);
}
}
}
}

View file

@ -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): 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');
}
}

View file

@ -205,10 +205,15 @@ class BackgroundService {
) {
}
public function setDefaultBackground(): void {
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'background_image');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'background_color');
$this->config->deleteUserValue($this->userId, Application::APP_ID, 'primary_color');
public function setDefaultBackground(?string $userId = null): void {
$userId = $userId ?? $this->userId;
if ($userId === null) {
throw new RuntimeException('No currently logged-in user');
}
$this->config->deleteUserValue($userId, Application::APP_ID, 'background_image');
$this->config->deleteUserValue($userId, Application::APP_ID, 'background_color');
$this->config->deleteUserValue($userId, Application::APP_ID, 'primary_color');
}
/**
@ -227,9 +232,24 @@ class BackgroundService {
/** @var File $file */
$file = $userFolder->get($path);
$image = new \OCP\Image();
$handle = $file->fopen('r');
if ($handle === false) {
throw new InvalidArgumentException('Invalid image file');
}
$this->getAppDataFolder()->newFile('background.jpg', $handle);
if ($image->loadFromFileHandle($file->fopen('r')) === false) {
$this->recalculateMeanColor();
}
public function recalculateMeanColor(?string $userId = null): void {
$userId = $userId ?? $this->userId;
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');
}
@ -237,35 +257,43 @@ class BackgroundService {
if ($meanColor !== false) {
$this->setColorBackground($meanColor);
}
$this->getAppDataFolder()->newFile('background.jpg', $file->fopen('r'));
$this->config->setUserValue($this->userId, Application::APP_ID, 'background_image', self::BACKGROUND_CUSTOM);
$this->config->setUserValue($userId, Application::APP_ID, 'background_image', self::BACKGROUND_CUSTOM);
}
public function setShippedBackground($fileName): void {
if ($this->userId === null) {
/**
* Set background of user to a shipped background identified by the filename
* @param string $filename The shipped background filename
* @param null|string $userId The user to set - defaults to currently logged in user
* @throws RuntimeException If neither $userId is specified nor a user is logged in
* @throws InvalidArgumentException If the specified filename does not match any shipped background
*/
public function setShippedBackground(string $filename, ?string $userId = null): void {
$userId = $userId ?? $this->userId;
if ($userId === null) {
throw new RuntimeException('No currently logged-in user');
}
if (!array_key_exists($fileName, self::SHIPPED_BACKGROUNDS)) {
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->config->setUserValue($this->userId, Application::APP_ID, 'background_image', $fileName);
$this->config->setUserValue($this->userId, Application::APP_ID, 'primary_color', self::SHIPPED_BACKGROUNDS[$fileName]['primary_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 {
@ -366,19 +394,24 @@ class BackgroundService {
/**
* Storing the data in appdata/theming/users/USERID
*
* @return ISimpleFolder
* @param string|null $userId The user to get the folder - default to current user
* @throws NotPermittedException
*/
private function getAppDataFolder(): ISimpleFolder {
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) {
$rootFolder = $this->appData->newFolder('users');
}
try {
return $rootFolder->getFolder($this->userId);
return $rootFolder->getFolder($userId);
} catch (NotFoundException $e) {
return $rootFolder->newFolder($this->userId);
return $rootFolder->newFolder($userId);
}
}
}

View file

@ -11,6 +11,7 @@ use OCP\App\AppPathNotFoundException;
use OCP\App\IAppManager;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
use OCP\IAppConfig;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IL10N;
@ -39,6 +40,7 @@ class ThemingDefaults extends \OC_Defaults {
*/
public function __construct(
private IConfig $config,
private IAppConfig $appConfig,
private IL10N $l,
private IUserSession $userSession,
private IURLGenerator $urlGenerator,
@ -206,9 +208,9 @@ class ThemingDefaults extends \OC_Defaults {
// user-defined background color
if (!empty($user)) {
$userPrimaryColor = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background_color', '');
if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $userPrimaryColor)) {
return $userPrimaryColor;
$userBackgroundColor = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background_color', '');
if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $userBackgroundColor)) {
return $userBackgroundColor;
}
}
@ -221,7 +223,7 @@ class ThemingDefaults extends \OC_Defaults {
*/
public function getDefaultColorPrimary(): string {
// try admin color
$defaultColor = $this->config->getAppValue(Application::APP_ID, 'primary_color', '');
$defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'primary_color', '');
if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $defaultColor)) {
return $defaultColor;
}
@ -234,7 +236,7 @@ class ThemingDefaults extends \OC_Defaults {
* Default background color only taking admin setting into account
*/
public function getDefaultColorBackground(): string {
$defaultColor = $this->config->getAppValue(Application::APP_ID, 'background_color', '');
$defaultColor = $this->appConfig->getValueString(Application::APP_ID, 'background_color');
if (preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $defaultColor)) {
return $defaultColor;
}
@ -344,7 +346,7 @@ class ThemingDefaults extends \OC_Defaults {
$variables['image-login-background'] = "url('".$this->imageManager->getImageUrl('background')."')";
$variables['image-login-plain'] = 'false';
if ($this->config->getAppValue('theming', 'primary_color', '') !== '') {
if ($this->appConfig->getValueString(Application::APP_ID, 'primary_color', '') !== '') {
$variables['color-primary'] = $this->getColorPrimary();
$variables['color-primary-text'] = $this->getTextColorPrimary();
$variables['color-primary-element'] = $this->util->elementColor($this->getColorPrimary());
@ -520,6 +522,6 @@ class ThemingDefaults extends \OC_Defaults {
* Has the admin disabled user customization
*/
public function isUserThemingDisabled(): bool {
return $this->config->getAppValue('theming', 'disable-user-theming', 'no') === 'yes';
return $this->appConfig->getValueBool(Application::APP_ID, 'disable-user-theming');
}
}

View file

@ -13,6 +13,7 @@ use OCA\Theming\ThemingDefaults;
use OCP\App\IAppManager;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Services\IAppConfig;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
@ -25,30 +26,24 @@ use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
class ThemingControllerTest extends TestCase {
/** @var IRequest|MockObject */
private $request;
/** @var IConfig|MockObject */
private $config;
/** @var ThemingDefaults|MockObject */
private $themingDefaults;
/** @var IL10N|MockObject */
private $l10n;
/** @var ThemingController */
private $themingController;
/** @var IAppManager|MockObject */
private $appManager;
/** @var ImageManager|MockObject */
private $imageManager;
/** @var IURLGenerator|MockObject */
private $urlGenerator;
/** @var ThemesService|MockObject */
private $themesService;
/** @var INavigationManager|MockObject */
private $navigationManager;
private IRequest&MockObject $request;
private IConfig&MockObject $config;
private IAppConfig&MockObject $appConfig;
private ThemingDefaults&MockObject $themingDefaults;
private IL10N&MockObject $l10n;
private IAppManager&MockObject $appManager;
private ImageManager&MockObject $imageManager;
private IURLGenerator&MockObject $urlGenerator;
private ThemesService&MockObject $themesService;
private INavigationManager&MockObject $navigationManager;
private ThemingController $themingController;
protected function setUp(): void {
$this->request = $this->createMock(IRequest::class);
$this->config = $this->createMock(IConfig::class);
$this->appConfig = $this->createMock(IAppConfig::class);
$this->themingDefaults = $this->createMock(ThemingDefaults::class);
$this->l10n = $this->createMock(L10N::class);
$this->appManager = $this->createMock(IAppManager::class);
@ -68,6 +63,7 @@ class ThemingControllerTest extends TestCase {
'theming',
$this->request,
$this->config,
$this->appConfig,
$this->themingDefaults,
$this->l10n,
$this->urlGenerator,

View file

@ -10,8 +10,8 @@ use OCA\Theming\Service\BackgroundService;
use OCA\Theming\ThemingDefaults;
use OCA\Theming\Util;
use OCP\App\IAppManager;
use OCP\Files\IAppData;
use OCP\Files\NotFoundException;
use OCP\IAppConfig;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
@ -20,21 +20,20 @@ use OCP\INavigationManager;
use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserSession;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
class ThemingDefaultsTest extends TestCase {
/** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */
private $config;
private IAppConfig&MockObject $appConfig;
private IConfig&MockObject $config;
private \OC_Defaults $defaults;
/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
private $l10n;
/** @var IUserSession|\PHPUnit\Framework\MockObject\MockObject */
private $userSession;
/** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */
private $urlGenerator;
/** @var \OC_Defaults|\PHPUnit\Framework\MockObject\MockObject */
private $defaults;
/** @var IAppData|\PHPUnit\Framework\MockObject\MockObject */
private $appData;
/** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */
private $cacheFactory;
/** @var ThemingDefaults */
@ -54,6 +53,7 @@ class ThemingDefaultsTest extends TestCase {
protected function setUp(): void {
parent::setUp();
$this->appConfig = $this->createMock(IAppConfig::class);
$this->config = $this->createMock(IConfig::class);
$this->l10n = $this->createMock(IL10N::class);
$this->userSession = $this->createMock(IUserSession::class);
@ -72,6 +72,7 @@ class ThemingDefaultsTest extends TestCase {
->willReturn('');
$this->template = new ThemingDefaults(
$this->config,
$this->appConfig,
$this->l10n,
$this->userSession,
$this->urlGenerator,
@ -398,25 +399,31 @@ class ThemingDefaultsTest extends TestCase {
}
public function testGetColorPrimaryWithDefault() {
$this->config
->expects($this->exactly(2))
->method('getAppValue')
->willReturnMap([
['theming', 'disable-user-theming', 'no', 'no'],
['theming', 'primary_color', '', $this->defaults->getColorPrimary()],
]);
$this->appConfig
->expects(self::once())
->method('getValueBool')
->with('theming', 'disable-user-theming')
->willReturn(false);
$this->appConfig
->expects(self::once())
->method('getValueString')
->with('theming', 'primary_color', '')
->willReturn($this->defaults->getColorPrimary());
$this->assertEquals($this->defaults->getColorPrimary(), $this->template->getColorPrimary());
}
public function testGetColorPrimaryWithCustom() {
$this->config
->expects($this->exactly(2))
->method('getAppValue')
->willReturnMap([
['theming', 'disable-user-theming', 'no', 'no'],
['theming', 'primary_color', '', '#fff'],
]);
$this->appConfig
->expects(self::once())
->method('getValueBool')
->with('theming', 'disable-user-theming')
->willReturn(false);
$this->appConfig
->expects(self::once())
->method('getValueString')
->with('theming', 'primary_color', '')
->willReturn('#fff');
$this->assertEquals('#fff', $this->template->getColorPrimary());
}
@ -424,37 +431,37 @@ class ThemingDefaultsTest extends TestCase {
public function dataGetColorPrimary() {
return [
'with fallback default' => [
'disableTheming' => 'no',
'disableTheming' => false,
'primaryColor' => '',
'userPrimaryColor' => '',
'expected' => BackgroundService::DEFAULT_COLOR,
],
'with custom admin primary' => [
'disableTheming' => 'no',
'disableTheming' => false,
'primaryColor' => '#aaa',
'userPrimaryColor' => '',
'expected' => '#aaa',
],
'with custom invalid admin primary' => [
'disableTheming' => 'no',
'disableTheming' => false,
'primaryColor' => 'invalid',
'userPrimaryColor' => '',
'expected' => BackgroundService::DEFAULT_COLOR,
],
'with custom invalid user primary' => [
'disableTheming' => 'no',
'disableTheming' => false,
'primaryColor' => '',
'userPrimaryColor' => 'invalid-name',
'expected' => BackgroundService::DEFAULT_COLOR,
],
'with custom user primary' => [
'disableTheming' => 'no',
'disableTheming' => false,
'primaryColor' => '',
'userPrimaryColor' => '#bbb',
'expected' => '#bbb',
],
'with disabled user theming primary' => [
'disableTheming' => 'yes',
'disableTheming' => true,
'primaryColor' => '#aaa',
'userPrimaryColor' => '#bbb',
'expected' => '#aaa',
@ -465,7 +472,7 @@ class ThemingDefaultsTest extends TestCase {
/**
* @dataProvider dataGetColorPrimary
*/
public function testGetColorPrimary(string $disableTheming, string $primaryColor, string $userPrimaryColor, string $expected) {
public function testGetColorPrimary(bool $disableTheming, string $primaryColor, string $userPrimaryColor, string $expected) {
$user = $this->createMock(IUser::class);
$this->userSession->expects($this->any())
->method('getUser')
@ -473,13 +480,16 @@ class ThemingDefaultsTest extends TestCase {
$user->expects($this->any())
->method('getUID')
->willReturn('user');
$this->config
->expects($this->any())
->method('getAppValue')
->willReturnMap([
['theming', 'disable-user-theming', 'no', $disableTheming],
['theming', 'primary_color', '', $primaryColor],
]);
$this->appConfig
->expects(self::any())
->method('getValueBool')
->with('theming', 'disable-user-theming')
->willReturn($disableTheming);
$this->appConfig
->expects(self::any())
->method('getValueString')
->with('theming', 'primary_color', '')
->willReturn($primaryColor);
$this->config
->expects($this->any())
->method('getUserValue')
@ -699,8 +709,14 @@ class ThemingDefaultsTest extends TestCase {
['theming', 'backgroundMime', '', 'jpeg'],
['theming', 'logoheaderMime', '', 'jpeg'],
['theming', 'faviconMime', '', 'jpeg'],
['theming', 'primary_color', '', $this->defaults->getColorPrimary()],
['theming', 'primary_color', $this->defaults->getColorPrimary(), $this->defaults->getColorPrimary()],
]);
$this->appConfig
->expects(self::atLeastOnce())
->method('getValueString')
->willReturnMap([
['theming', 'primary_color', '', false, $this->defaults->getColorPrimary()],
['theming', 'primary_color', $this->defaults->getColorPrimary(), false, $this->defaults->getColorPrimary()],
]);
$this->util->expects($this->any())->method('invertTextColor')->with($this->defaults->getColorPrimary())->willReturn(false);

View file

@ -185,7 +185,7 @@ class PartitionedQueryBuilder extends ShardedQueryBuilder {
}
public function leftJoin($fromAlias, $join, $alias, $condition = null): self {
return $this->join($fromAlias, $join, $alias, $condition, PartitionQuery::JOIN_MODE_LEFT);
return $this->join($fromAlias, (string)$join, $alias, $condition, PartitionQuery::JOIN_MODE_LEFT);
}
public function join($fromAlias, $join, $alias, $condition = null, $joinMode = PartitionQuery::JOIN_MODE_INNER): self {

View file

@ -764,7 +764,7 @@ class QueryBuilder implements IQueryBuilder {
* </code>
*
* @param string $fromAlias The alias that points to a from clause.
* @param string $join The table name to join.
* @param string|IQueryFunction $join The table name or sub-query to join.
* @param string $alias The alias of the join table.
* @param string|ICompositeExpression|null $condition The condition for the join.
*

View file

@ -1167,6 +1167,7 @@ class Server extends ServerContainer implements IServerContainer {
);
return new ThemingDefaults(
$c->get(\OCP\IConfig::class),
$c->get(\OCP\IAppConfig::class),
$c->getL10N('theming'),
$c->get(IUserSession::class),
$c->get(IURLGenerator::class),

View file

@ -541,12 +541,13 @@ interface IQueryBuilder {
* </code>
*
* @param string $fromAlias The alias that points to a from clause.
* @param string $join The table name to join.
* @param string|IQueryFunction $join The table name to join.
* @param string $alias The alias of the join table.
* @param string|ICompositeExpression|null $condition The condition for the join.
*
* @return $this This QueryBuilder instance.
* @since 8.2.0
* @since 30.0.0 Allow passing IQueryFunction as parameter for `$join` to allow join with a sub-query.
*
* @psalm-taint-sink sql $fromAlias
* @psalm-taint-sink sql $join
@ -1001,11 +1002,14 @@ interface IQueryBuilder {
public function getLastInsertId(): int;
/**
* Returns the table name quoted and with database prefix as needed by the implementation
* Returns the table name quoted and with database prefix as needed by the implementation.
* If a query function is passed the function is casted to string,
* this allows passing functions as sub-queries for join expression.
*
* @param string|IQueryFunction $table
* @return string
* @since 9.0.0
* @since 24.0.0 accepts IQueryFunction as parameter
*/
public function getTableName($table);